From: Christoph Hellwig Subject: Re: [PATCH 11/40] ipv6/flowlabel: simplify pid namespace lookup Date: Tue, 15 May 2018 16:56:43 +0200 Message-ID: <20180515145643.GA661@lst.de> References: <20180425154827.32251-1-hch@lst.de> <20180425154827.32251-12-hch@lst.de> <878t8y46sy.fsf@xmission.com> Mime-Version: 1.0 Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Cc: linux-rtc-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alessandro Zummo , Alexandre Belloni , devel-gWbeCf7V1WCQmaza687I9mD2FQJk+8+b@public.gmane.org, linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-scsi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-ide-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Greg Kroah-Hartman , linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, jfs-discussion-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, Christoph Hellwig , linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, netfilter-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexander Viro , Jiri Slaby , Andrew Morton , linux-ext4-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Alexey Dobriyan , megaraidlinux.pdl-dY08KVG/lbpWk0Htik3J/w@public.gmane.org, drbd-dev-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org To: "Eric W. Biederman" Return-path: Content-Disposition: inline In-Reply-To: <878t8y46sy.fsf-aS9lmoZGLiVWk0Htik3J/w@public.gmane.org> List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Sender: drbd-dev-bounces-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org Errors-To: drbd-dev-bounces-cunTk1MwBs8qoQakbn7OcQ@public.gmane.org List-Id: linux-ext4.vger.kernel.org On Sat, May 05, 2018 at 07:37:33AM -0500, Eric W. Biederman wrote: > Christoph Hellwig writes: > > > The shole seq_file sequence already operates under a single RCU lock pair, > > so move the pid namespace lookup into it, and stop grabbing a reference > > and remove all kinds of boilerplate code. > > This is wrong. > > Move task_active_pid_ns(current) from open to seq_start actually means > that the results if you pass this proc file between callers the results > will change. So this breaks file descriptor passing. > > Open is a bad place to access current. In the middle of read/write is > broken. > > > In this particular instance looking up the pid namespace with > task_active_pid_ns was a personal brain fart. What the code should be > doing (with an appropriate helper) is: > > struct pid_namespace *pid_ns = inode->i_sb->s_fs_info; > > Because each mount of proc is bound to a pid namespace. Looking up the > pid namespace from the super_block is a much better way to go. What do you have in mind for the helper? For now I've thrown it in opencoded into my working tree, but I'd be glad to add a helper. struct pid_namespace *proc_pid_namespace(struct inode *inode) { // maybe warn on for s_magic not on procfs?? return inode->i_sb->s_fs_info; } ?