diff options
author | David Rientjes <rientjes@google.com> | 2010-10-26 17:21:25 -0400 |
---|---|---|
committer | Linus Torvalds <torvalds@linux-foundation.org> | 2010-10-26 19:52:05 -0400 |
commit | 723548bff1dde9ab6bdb23f4bb92277c4da49473 (patch) | |
tree | 7c4b97cb80003caae19a849e9ffb490be943bbf2 | |
parent | 1e99bad0d9c12a4aaa60cd812c84ef152564bcf5 (diff) |
oom: rewrite error handling for oom_adj and oom_score_adj tunables
It's better to use proper error handling in oom_adjust_write() and
oom_score_adj_write() instead of duplicating the locking order on various
exit paths.
Suggested-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: David Rientjes <rientjes@google.com>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Rik van Riel <riel@redhat.com>
Cc: Ying Han <yinghan@google.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
-rw-r--r-- | fs/proc/base.c | 83 |
1 files changed, 48 insertions, 35 deletions
diff --git a/fs/proc/base.c b/fs/proc/base.c index 6e50c8e65513..34d11ac31f2e 100644 --- a/fs/proc/base.c +++ b/fs/proc/base.c | |||
@@ -1023,36 +1023,39 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf, | |||
1023 | memset(buffer, 0, sizeof(buffer)); | 1023 | memset(buffer, 0, sizeof(buffer)); |
1024 | if (count > sizeof(buffer) - 1) | 1024 | if (count > sizeof(buffer) - 1) |
1025 | count = sizeof(buffer) - 1; | 1025 | count = sizeof(buffer) - 1; |
1026 | if (copy_from_user(buffer, buf, count)) | 1026 | if (copy_from_user(buffer, buf, count)) { |
1027 | return -EFAULT; | 1027 | err = -EFAULT; |
1028 | goto out; | ||
1029 | } | ||
1028 | 1030 | ||
1029 | err = strict_strtol(strstrip(buffer), 0, &oom_adjust); | 1031 | err = strict_strtol(strstrip(buffer), 0, &oom_adjust); |
1030 | if (err) | 1032 | if (err) |
1031 | return -EINVAL; | 1033 | goto out; |
1032 | if ((oom_adjust < OOM_ADJUST_MIN || oom_adjust > OOM_ADJUST_MAX) && | 1034 | if ((oom_adjust < OOM_ADJUST_MIN || oom_adjust > OOM_ADJUST_MAX) && |
1033 | oom_adjust != OOM_DISABLE) | 1035 | oom_adjust != OOM_DISABLE) { |
1034 | return -EINVAL; | 1036 | err = -EINVAL; |
1037 | goto out; | ||
1038 | } | ||
1035 | 1039 | ||
1036 | task = get_proc_task(file->f_path.dentry->d_inode); | 1040 | task = get_proc_task(file->f_path.dentry->d_inode); |
1037 | if (!task) | 1041 | if (!task) { |
1038 | return -ESRCH; | 1042 | err = -ESRCH; |
1043 | goto out; | ||
1044 | } | ||
1039 | if (!lock_task_sighand(task, &flags)) { | 1045 | if (!lock_task_sighand(task, &flags)) { |
1040 | put_task_struct(task); | 1046 | err = -ESRCH; |
1041 | return -ESRCH; | 1047 | goto err_task_struct; |
1042 | } | 1048 | } |
1043 | 1049 | ||
1044 | if (oom_adjust < task->signal->oom_adj && !capable(CAP_SYS_RESOURCE)) { | 1050 | if (oom_adjust < task->signal->oom_adj && !capable(CAP_SYS_RESOURCE)) { |
1045 | unlock_task_sighand(task, &flags); | 1051 | err = -EACCES; |
1046 | put_task_struct(task); | 1052 | goto err_sighand; |
1047 | return -EACCES; | ||
1048 | } | 1053 | } |
1049 | 1054 | ||
1050 | task_lock(task); | 1055 | task_lock(task); |
1051 | if (!task->mm) { | 1056 | if (!task->mm) { |
1052 | task_unlock(task); | 1057 | err = -EINVAL; |
1053 | unlock_task_sighand(task, &flags); | 1058 | goto err_task_lock; |
1054 | put_task_struct(task); | ||
1055 | return -EINVAL; | ||
1056 | } | 1059 | } |
1057 | 1060 | ||
1058 | if (oom_adjust != task->signal->oom_adj) { | 1061 | if (oom_adjust != task->signal->oom_adj) { |
@@ -1080,11 +1083,14 @@ static ssize_t oom_adjust_write(struct file *file, const char __user *buf, | |||
1080 | else | 1083 | else |
1081 | task->signal->oom_score_adj = (oom_adjust * OOM_SCORE_ADJ_MAX) / | 1084 | task->signal->oom_score_adj = (oom_adjust * OOM_SCORE_ADJ_MAX) / |
1082 | -OOM_DISABLE; | 1085 | -OOM_DISABLE; |
1086 | err_task_lock: | ||
1083 | task_unlock(task); | 1087 | task_unlock(task); |
1088 | err_sighand: | ||
1084 | unlock_task_sighand(task, &flags); | 1089 | unlock_task_sighand(task, &flags); |
1090 | err_task_struct: | ||
1085 | put_task_struct(task); | 1091 | put_task_struct(task); |
1086 | 1092 | out: | |
1087 | return count; | 1093 | return err < 0 ? err : count; |
1088 | } | 1094 | } |
1089 | 1095 | ||
1090 | static const struct file_operations proc_oom_adjust_operations = { | 1096 | static const struct file_operations proc_oom_adjust_operations = { |
@@ -1125,36 +1131,39 @@ static ssize_t oom_score_adj_write(struct file *file, const char __user *buf, | |||
1125 | memset(buffer, 0, sizeof(buffer)); | 1131 | memset(buffer, 0, sizeof(buffer)); |
1126 | if (count > sizeof(buffer) - 1) | 1132 | if (count > sizeof(buffer) - 1) |
1127 | count = sizeof(buffer) - 1; | 1133 | count = sizeof(buffer) - 1; |
1128 | if (copy_from_user(buffer, buf, count)) | 1134 | if (copy_from_user(buffer, buf, count)) { |
1129 | return -EFAULT; | 1135 | err = -EFAULT; |
1136 | goto out; | ||
1137 | } | ||
1130 | 1138 | ||
1131 | err = strict_strtol(strstrip(buffer), 0, &oom_score_adj); | 1139 | err = strict_strtol(strstrip(buffer), 0, &oom_score_adj); |
1132 | if (err) | 1140 | if (err) |
1133 | return -EINVAL; | 1141 | goto out; |
1134 | if (oom_score_adj < OOM_SCORE_ADJ_MIN || | 1142 | if (oom_score_adj < OOM_SCORE_ADJ_MIN || |
1135 | oom_score_adj > OOM_SCORE_ADJ_MAX) | 1143 | oom_score_adj > OOM_SCORE_ADJ_MAX) { |
1136 | return -EINVAL; | 1144 | err = -EINVAL; |
1145 | goto out; | ||
1146 | } | ||
1137 | 1147 | ||
1138 | task = get_proc_task(file->f_path.dentry->d_inode); | 1148 | task = get_proc_task(file->f_path.dentry->d_inode); |
1139 | if (!task) | 1149 | if (!task) { |
1140 | return -ESRCH; | 1150 | err = -ESRCH; |
1151 | goto out; | ||
1152 | } | ||
1141 | if (!lock_task_sighand(task, &flags)) { | 1153 | if (!lock_task_sighand(task, &flags)) { |
1142 | put_task_struct(task); | 1154 | err = -ESRCH; |
1143 | return -ESRCH; | 1155 | goto err_task_struct; |
1144 | } | 1156 | } |
1145 | if (oom_score_adj < task->signal->oom_score_adj && | 1157 | if (oom_score_adj < task->signal->oom_score_adj && |
1146 | !capable(CAP_SYS_RESOURCE)) { | 1158 | !capable(CAP_SYS_RESOURCE)) { |
1147 | unlock_task_sighand(task, &flags); | 1159 | err = -EACCES; |
1148 | put_task_struct(task); | 1160 | goto err_sighand; |
1149 | return -EACCES; | ||
1150 | } | 1161 | } |
1151 | 1162 | ||
1152 | task_lock(task); | 1163 | task_lock(task); |
1153 | if (!task->mm) { | 1164 | if (!task->mm) { |
1154 | task_unlock(task); | 1165 | err = -EINVAL; |
1155 | unlock_task_sighand(task, &flags); | 1166 | goto err_task_lock; |
1156 | put_task_struct(task); | ||
1157 | return -EINVAL; | ||
1158 | } | 1167 | } |
1159 | if (oom_score_adj != task->signal->oom_score_adj) { | 1168 | if (oom_score_adj != task->signal->oom_score_adj) { |
1160 | if (oom_score_adj == OOM_SCORE_ADJ_MIN) | 1169 | if (oom_score_adj == OOM_SCORE_ADJ_MIN) |
@@ -1172,10 +1181,14 @@ static ssize_t oom_score_adj_write(struct file *file, const char __user *buf, | |||
1172 | else | 1181 | else |
1173 | task->signal->oom_adj = (oom_score_adj * OOM_ADJUST_MAX) / | 1182 | task->signal->oom_adj = (oom_score_adj * OOM_ADJUST_MAX) / |
1174 | OOM_SCORE_ADJ_MAX; | 1183 | OOM_SCORE_ADJ_MAX; |
1184 | err_task_lock: | ||
1175 | task_unlock(task); | 1185 | task_unlock(task); |
1186 | err_sighand: | ||
1176 | unlock_task_sighand(task, &flags); | 1187 | unlock_task_sighand(task, &flags); |
1188 | err_task_struct: | ||
1177 | put_task_struct(task); | 1189 | put_task_struct(task); |
1178 | return count; | 1190 | out: |
1191 | return err < 0 ? err : count; | ||
1179 | } | 1192 | } |
1180 | 1193 | ||
1181 | static const struct file_operations proc_oom_score_adj_operations = { | 1194 | static const struct file_operations proc_oom_score_adj_operations = { |