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

["Jörn Engel" <joern@logfs.org>]

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.

> Did I break your COW link patches? ;)

Nope.  No bovine maladies involved.

Jörn

-- 
The competent programmer is fully aware of the strictly limited size of
his own skull; therefore he approaches the programming task in full
humility, and among other things he avoids clever tricks like the plague.
-- Edsger W. Dijkstra
-
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