Re: [PATCH 0/7 v3] overlay filesystem prototype

[Miklos Szeredi]

On Tue, 21 Sep 2010, Neil Brown wrote:
> On Mon, 20 Sep 2010 20:04:04 +0200
> Miklos Szeredi <miklos@xxxxxxxxxx> wrote:
> 
> > Here's an updated patch series.
> > 
> > For now I reverted Neil's revalidation patch.  Not requiring strict
> > read-only would make sense for just trying it out and experimenting.
> > But for real uses, I'm not sure...
> 
> :-)
> 
> I think you significantly reduce the value by insisting on read-only but as
> this is purely a theoretical perspective at the moment (I have no concrete
> use-case) I won't push it.

I'm not insisting on read-only.  I think there's value to enabling
external modification in special circumstances, but not by default.

I also think it might not be a good idea to waste CPU cycles and brain
cycles on implementing (an always imperfect) revalidation.  A similar
mechanism is "mount -oremount mountpoint", which simply throws out all
unused dentries, effectively forcing a revalidation of the whole
filesystem.  Less selective than ->d_revalidate(), but it shouldn't be
a big issue since the underlying filesystem dentries remain cached.

> I had another patch I was working on which caused overlayfs to keep negative
> dentries in upperdentry or lowerdentry rather than just setting them to
> NULL.  This would allow revalidation to notice objects appearing in the
> underlying filesystem.  I guess you won't want that now .... I think it made
> some of the code a bit neater, but I never finished it so I cannot be sure of
> the overall effect.
> 
> I'm curious as to why upperdentry is now called __upperdentry - it isn't
> clear from a quick reading..

(I need to add more comments to the code...)

The underscores are meant to imply that unlinke ->lowerdentry it's not
qute safe to access directly.  See discussion about memory barriers:

  http://lkml.org/lkml/2010/9/19/142

It's not as clear as one might like.  To sum up:

  - an smp_read_barrier_depends() is needed on SMP Alpha, but not on
    other archs

  - an ACCESS_ONCE() is needed on some theoretical compiler that does
    weird optimizations, but apparently not on current versions of gcc

The VFS lives happily without either, locklessly accessing
dentry->d_inode.

Thanks,
Miklos
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/