> On Tue, 6 Nov 2007 12:30:38 +0100 Jörn Engel <joern@xxxxxxxxx> 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@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
