Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1755344Ab2HNM4m (ORCPT ); Tue, 14 Aug 2012 08:56:42 -0400 Received: from charlotte.tuxdriver.com ([70.61.120.58]:53251 "EHLO smtp.tuxdriver.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1754601Ab2HNM4l (ORCPT ); Tue, 14 Aug 2012 08:56:41 -0400 Date: Tue, 14 Aug 2012 08:56:24 -0400 From: Neil Horman To: John Fastabend Cc: viro@ZenIV.linux.org.uk, netdev@vger.kernel.org, davem@davemloft.net, linux-kernel@vger.kernel.org, joe@perches.com Subject: Re: [net PATCH v2 1/2] net: netprio: fix files lock and remove useless d_path bits Message-ID: <20120814125623.GB18731@hmsreliant.think-freely.org> References: <20120814024320.6983.92222.stgit@jf-dev1-dcblab> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20120814024320.6983.92222.stgit@jf-dev1-dcblab> User-Agent: Mutt/1.5.21 (2010-09-15) X-Spam-Score: -2.9 (--) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2605 Lines: 84 On Mon, Aug 13, 2012 at 07:43:21PM -0700, John Fastabend wrote: > Add lock to prevent a race with a file closing and also remove > useless and ugly sscanf code. The extra code was never needed > and the case it supposedly protected against is in fact handled > correctly by sock_from_file as pointed out by Al Viro. > > CC: Neil Horman > Reported-by: Al Viro > Signed-off-by: John Fastabend > --- > > net/core/netprio_cgroup.c | 22 ++++------------------ > 1 files changed, 4 insertions(+), 18 deletions(-) > > diff --git a/net/core/netprio_cgroup.c b/net/core/netprio_cgroup.c > index ed0c043..f65dba3 100644 > --- a/net/core/netprio_cgroup.c > +++ b/net/core/netprio_cgroup.c > @@ -277,12 +277,6 @@ out_free_devname: > void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset) > { > struct task_struct *p; > - char *tmp = kzalloc(sizeof(char) * PATH_MAX, GFP_KERNEL); > - > - if (!tmp) { > - pr_warn("Unable to attach cgrp due to alloc failure!\n"); > - return; > - } > > cgroup_taskset_for_each(p, cgrp, tset) { > unsigned int fd; > @@ -296,32 +290,24 @@ void net_prio_attach(struct cgroup *cgrp, struct cgroup_taskset *tset) > continue; > } > > - rcu_read_lock(); > + spin_lock(&files->file_lock); > fdt = files_fdtable(files); > for (fd = 0; fd < fdt->max_fds; fd++) { > - char *path; > struct file *file; > struct socket *sock; > - unsigned long s; > - int rv, err = 0; > + int err; > > file = fcheck_files(files, fd); > if (!file) > continue; > > - path = d_path(&file->f_path, tmp, PAGE_SIZE); > - rv = sscanf(path, "socket:[%lu]", &s); > - if (rv <= 0) > - continue; > - > sock = sock_from_file(file, &err); > - if (!err) > + if (sock) > sock_update_netprioidx(sock->sk, p); > } > - rcu_read_unlock(); > + spin_unlock(&files->file_lock); > task_unlock(p); > } > - kfree(tmp); > } > > static struct cftype ss_files[] = { > This looks ok to me, but I've already shown my inability to review code that interfaces with VFS. Al, what do you think? Neil > -- > To unsubscribe from this list: send the line "unsubscribe netdev" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/