Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1422883AbbENXW1 (ORCPT ); Thu, 14 May 2015 19:22:27 -0400 Received: from zeniv.linux.org.uk ([195.92.253.2]:53277 "EHLO ZenIV.linux.org.uk" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S932562AbbENXWV (ORCPT ); Thu, 14 May 2015 19:22:21 -0400 Date: Fri, 15 May 2015 00:21:49 +0100 From: Al Viro To: Linus Torvalds Cc: Neil Brown , Christoph Hellwig , linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org Subject: Re: [PATCH v3 074/110] VFS: replace {, total_}link_count in task_struct with pointer to nameidata Message-ID: <20150514232148.GA24737@ZenIV.linux.org.uk> References: <20150511180650.GA4147@ZenIV.linux.org.uk> <1431367690-5223-74-git-send-email-viro@ZenIV.linux.org.uk> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1431367690-5223-74-git-send-email-viro@ZenIV.linux.org.uk> User-Agent: Mutt/1.5.21 (2010-09-15) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org Content-Length: 2486 Lines: 48 On Mon, May 11, 2015 at 07:07:34PM +0100, Al Viro wrote: > From: NeilBrown > > task_struct currently contains two ad-hoc members for use by the VFS: > link_count and total_link_count. These are only interesting to fs/namei.c, > so exposing them explicitly is poor layering. Incidentally, link_count > isn't used anymore, so it can just die. > > This patches replaces those with a single pointer to 'struct nameidata'. > This structure represents the current filename lookup of which > there can only be one per process, and is a natural place to > store total_link_count. > > This will allow the current "nameidata" argument to all > follow_link operations to be removed as current->nameidata > can be used instead in the _very_ few instances that care about > it at all. > > As there are occasional circumstances where pathname lookup can > recurse, such as through kern_path_locked, we always save and old > current->nameidata (if there is one) when setting a new value, and > make sure any active link_counts are preserved. > > follow_mount and follow_automount now get a 'struct nameidata *' > rather than 'int flags' so that they can directly access > total_link_count, rather than going through 'current'. FWIW, the mainline semantics for total_link_count is bogus. Look: we set it to zero in path_init() (since _way_ back - in fact, 2.4.12 had something like that in path_walk()). Now, consider what happens when something like e.g. NFS referral point crossing does pathname resolution (recursion there is limited to "no more than once" by referral loop prevention logics in NFS code). We get the damn thing quietly reset to zero, no matter how many symlinks had already been crossed. IMO we should drop that assignment in path_init() and let the logics in set_nameidata/restore_nameidata do the right thing. The current semantics is obviously wrong; we should either count the symlink traversals during a referral resolution towards the limit or just limit those to 40 independently from the symlink traversals outside of referral resolution, but resetting the count to zero as soon as we'd hit the referral is clearly bogus. I would prefer the "let's count them towards the limit" variant. Objections? -- 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/