Re: + embed-a-struct-path-into-struct-nameidata-instead-of-nd-dentrymnt.pa tch added to -mm tree

[Andrew Morton <akpm@linux-foundation.org>]

> On Tue, 6 Nov 2007 12:30:38 +0100 Jörn Engel <joern@logfs.org> wrote:
> On Tue, 6 November 2007 10:11:49 +0100, Jan Blunck wrote:
> > On Mon, Nov 05, Jörn Engel wrote:
> > 
> > > This patch changes some 400 lines, most if not all of which get longer
> > > and more complicated to read.  23 get sufficiently longer to require an
> > > additional linebreak.  I can't remember complexity being invited into
> > > the kernel without good reasoning, yet the patch description is
> > > surprisingly low on reasoning:
> > > > Switch from nd->{dentry,mnt} to nd->path.{dentry,mnt} everywhere.
> > 
> > I don't measure complexity by lines of code or length of lines. Maybe I was
> > not verbose enough in the description, fair.
> 
> If you have a better metric, please share it.  In the paragraph you
> deleted I explicitly asked for _any_ metric that shows favorable
> numbers.  Lacking numbers, we could only argue about our respective
> personal taste.
> 
> > This is a cleanup series. In mostly no case there is a reason why someone
> > would want to use a dentry for itself. This series reflects that fact in
> > nameidata where there is absolutly no reason at all.
> 
> 400+ lines changed in this patch, some 10 in a followup patch that
> combines dentry/vfsmount assignments into a single path assignment.  If
> your argument above was valid, I would expect more simplifications and
> fewer complications.  Call me a sceptic until further patches show up to
> support your point.
> 
> > It enforced the correct
> > order of getting/releasing refcount on <dentry,vfsmount> pairs.
> 
> This argument I buy.
> 
> > It enables us
> > to do some more cleanups wrt lookup (which are coming later).
> 
> Please send those patches.  I invite cleanups that do clean things up
> and won't argue against then. ;)
> 
> > For stacking
> > support in VFS it is essential to have the <dentry,vfsmount> pair in every
> > place where you want to traverse the stack.
> 
> True, but unrelated to this patch.
> 
> > > If churn is the only effect of this, please considere it NAKed again.
> > 
> > I wonder why you didn't speak up when this series was posted to LKML. It was
> > at least posted three times before.
> 
> I did speak up.  Once.  If you missed that thread, please forgive me
> missing those in which the same patch I disapproved of were resent
> without me on Cc.
> 
> I'm not categorically against this struct path business.  It does have
> some advantages at first glance.  But the patch we're arguing about
> clearly makes code more complicated and harder to read.  We should have
> more than superficial benefits if we decide to pay such a cost.

It sounds like we at least need a better overall changlog, please..
-
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html