[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Message-ID: <20100517195145.GB18568@shell>
Date: Mon, 17 May 2010 15:51:45 -0400
From: Valerie Aurora <vaurora@...hat.com>
To: Neil Brown <neilb@...e.de>
Cc: Alexander Viro <viro@...iv.linux.org.uk>,
linux-fsdevel@...r.kernel.org, linux-kernel@...r.kernel.org,
Christoph Hellwig <hch@...radead.org>,
Jan Blunck <jblunck@...e.de>,
David Woodhouse <dwmw2@...radead.org>,
linux-nfs@...r.kernel.org, "J. Bruce Fields" <bfields@...ldses.org>
Subject: Re: [PATCH 05/39] whiteout/NFSD: Don't return information about whiteouts to userspace
On Fri, May 07, 2010 at 07:18:08AM +1000, Neil Brown wrote:
> On Thu, 6 May 2010 14:01:51 -0400
> Valerie Aurora <vaurora@...hat.com> wrote:
>
> > On Tue, May 04, 2010 at 09:37:31AM +1000, Neil Brown wrote:
> > > On Mon, 3 May 2010 16:12:04 -0700
> > > Valerie Aurora <vaurora@...hat.com> wrote:
> > >
> > > > From: Jan Blunck <jblunck@...e.de>
> > > >
> > > > Userspace isn't ready for handling another file type, so silently drop
> > > > whiteout directory entries before they leave the kernel.
> > >
> > > Feels very intrusive doesn't it....
> > >
> > > Have you considered something like the following?
> >
> > Hrm, I see how that could be more elegant, but I'd rather avoid yet
> > another layer of function pointer passing around. This code is
> > already hard enough to review...
>
> Yes, the extra indirection is a bit of a negative, but I don't think this
> patch is harder to review than the alternate.
> From a numerical perspective, with this patch you only need to look at the
> various places that ->readdir is called to be sure it is always correct.
> There are about 3. With the original you need to look at ever filldir
> function. Jan has found 9.
>
> And from a maintainability perspective, I think my approach is safer. Given
> that there are 9 filldir functions already, the chance that a need will be
> found for another seems good, and the chance that the coder will know to
> check for DT_WHT is a best even. Conversely if another call to ->readdir
> were added it is likely that nothing would need to be done.
>
> Of course just counting things doesn't give a completely picture but I think
> it can be indicative.
Okay, good points. Let me try it out after getting this next rewrite done.
Thanks,
-VAL
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists