The patch titled taskstats: cleanup reply assembling has been added to the -mm tree. Its filename is taskstats-cleanup-reply-assembling.patch See http://www.zip.com.au/~akpm/linux/patches/stuff/added-to-mm.txt to find out what to do about this ------------------------------------------------------ Subject: taskstats: cleanup reply assembling From: Oleg Nesterov <oleg@xxxxxxxxxx> Thomas Graf wrote: > > nla_nest_start() may return NULL, either rely on prepare_reply() to be > correct and BUG() on failure or do proper error handling for all > functions. nla_put() in taskstat.c can fail only if the 'size' argument of alloc_skb() was not right. This is a kernel bug, we should not hide it. So add 'BUG()' on error path and check for 'na == NULL'. > genlmsg_cancel() is only required in error paths for dumping > procedures. So we can remove 'genlmsg_cancel()' calls and 'void *reply' (saves 227 bytes). Signed-off-by: Oleg Nesterov <oleg@xxxxxxxxxx> Cc: Thomas Graf <tgraf@xxxxxxx> Cc: Shailabh Nagar <nagar@xxxxxxxxxxxxxx> Cc: Balbir Singh <balbir@xxxxxxxxxx> Cc: Jay Lan <jlan@xxxxxxx> Signed-off-by: Andrew Morton <akpm@xxxxxxxx> --- kernel/taskstats.c | 38 ++++++++++++++++---------------------- 1 files changed, 16 insertions(+), 22 deletions(-) diff -puN kernel/taskstats.c~taskstats-cleanup-reply-assembling kernel/taskstats.c --- a/kernel/taskstats.c~taskstats-cleanup-reply-assembling +++ a/kernel/taskstats.c @@ -69,7 +69,7 @@ enum actions { }; static int prepare_reply(struct genl_info *info, u8 cmd, struct sk_buff **skbp, - void **replyp, size_t size) + size_t size) { struct sk_buff *skb; void *reply; @@ -99,7 +99,6 @@ static int prepare_reply(struct genl_inf } *skbp = skb; - *replyp = reply; return 0; } @@ -356,11 +355,13 @@ static struct taskstats *mk_reply(struct struct nlattr *na, *ret; int aggr; - aggr = TASKSTATS_TYPE_AGGR_TGID; - if (type == TASKSTATS_TYPE_PID) - aggr = TASKSTATS_TYPE_AGGR_PID; + aggr = (type == TASKSTATS_TYPE_PID) + ? TASKSTATS_TYPE_AGGR_PID + : TASKSTATS_TYPE_AGGR_TGID; na = nla_nest_start(skb, aggr); + if (!na) + goto err; if (nla_put(skb, type, sizeof(pid), &pid) < 0) goto err; ret = nla_reserve(skb, TASKSTATS_TYPE_STATS, sizeof(struct taskstats)); @@ -378,7 +379,6 @@ static int taskstats_user_cmd(struct sk_ int rc = 0; struct sk_buff *rep_skb; struct taskstats *stats; - void *reply; size_t size; cpumask_t mask; @@ -400,7 +400,7 @@ static int taskstats_user_cmd(struct sk_ size = nla_total_size(sizeof(u32)) + nla_total_size(sizeof(struct taskstats)) + nla_total_size(0); - rc = prepare_reply(info, TASKSTATS_CMD_NEW, &rep_skb, &reply, size); + rc = prepare_reply(info, TASKSTATS_CMD_NEW, &rep_skb, size); if (rc < 0) return rc; @@ -409,27 +409,24 @@ static int taskstats_user_cmd(struct sk_ u32 pid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_PID]); stats = mk_reply(rep_skb, TASKSTATS_TYPE_PID, pid); if (!stats) - goto nla_err; + goto err; rc = fill_pid(pid, NULL, stats); if (rc < 0) - goto nla_err; + goto err; } else if (info->attrs[TASKSTATS_CMD_ATTR_TGID]) { u32 tgid = nla_get_u32(info->attrs[TASKSTATS_CMD_ATTR_TGID]); stats = mk_reply(rep_skb, TASKSTATS_TYPE_TGID, tgid); if (!stats) - goto nla_err; + goto err; rc = fill_tgid(tgid, NULL, stats); if (rc < 0) - goto nla_err; + goto err; } else goto err; return send_reply(rep_skb, info->snd_pid); - -nla_err: - genlmsg_cancel(rep_skb, reply); err: nlmsg_free(rep_skb); return rc; @@ -466,7 +463,6 @@ void taskstats_exit(struct task_struct * struct listener_list *listeners; struct taskstats *stats; struct sk_buff *rep_skb; - void *reply; size_t size; int is_thread_group; @@ -491,17 +487,17 @@ void taskstats_exit(struct task_struct * if (list_empty(&listeners->list)) return; - rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, &reply, size); + rc = prepare_reply(NULL, TASKSTATS_CMD_NEW, &rep_skb, size); if (rc < 0) return; stats = mk_reply(rep_skb, TASKSTATS_TYPE_PID, tsk->pid); if (!stats) - goto nla_err; + goto err; rc = fill_pid(tsk->pid, tsk, stats); if (rc < 0) - goto nla_err; + goto err; /* * Doesn't matter if tsk is the leader or the last group member leaving @@ -511,16 +507,14 @@ void taskstats_exit(struct task_struct * stats = mk_reply(rep_skb, TASKSTATS_TYPE_TGID, tsk->tgid); if (!stats) - goto nla_err; + goto err; memcpy(stats, tsk->signal->stats, sizeof(*stats)); send: send_cpu_listeners(rep_skb, listeners); return; - -nla_err: - genlmsg_cancel(rep_skb, reply); +err: nlmsg_free(rep_skb); } _ Patches currently in -mm which might be from oleg@xxxxxxxxxx are origin.patch taskstats_exit_alloc-optimize-simplify.patch taskstats-cleanup-do_exit-path.patch taskstats-cleanup-signal-stats-allocation.patch taskstats-factor-out-reply-assembling.patch taskstats-use-nla_reserve-for-reply-assembling.patch taskstats-cleanup-reply-assembling.patch tty-signal-tty-locking.patch do_task_stat-dont-take-tty_mutex.patch do_acct_process-dont-take-tty_mutex.patch trivial-make-set_special_pids-static.patch sys_unshare-remove-a-broken-clone_sighand-code.patch sys_setpgid-eliminate-unnecessary-do_each_task_pidpidtype_pgid.patch session_of_pgrp-kill-unnecessary-do_each_task_pidpidtype_pgid.patch pidhash-temporary-debug-checks.patch - To unsubscribe from this list: send the line "unsubscribe mm-commits" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html