Re: [PATCH 1/5] vfs, overlayfs, cachefiles: Turn I_OVL_INUSE into something generic

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Jan 31, 2022 at 5:32 PM David Howells <dhowells@xxxxxxxxxx> wrote:
>
> Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> > Please leave ovl_* as wrappers instead of changing super.c.
>
> Do you want them turning into inline functions?
>

inline would be fine.

But I just noticed something that may wreck this party.

The assumption, when I proposed this merger, was that an inode for
upper/work and is exclusively owned by ovl driver, so there should be no
conflicts with other drivers setting inuse flag.

However, in ovl_check_layer(), we walk back to root to verify that an ovl
layer of a new instance is not a descendant of another ovl upper/work inuse.
So the meaning of I_OVL_INUSE is somewhat stronger than an exclusive inode lock.
It implies ownership on the entire subtree.

I guess there is no conflict with cachefiles since ovl upper/work should not be
intersecting with any cachefiles storage, but that complicates the
semantics when
it comes to a generic flag.

OTOH, I am not sure if this check on ovl mount is so smart to begin with.
This check was added (after the exclusive ownership meaning) to silence syzbot
that kept mutating to weird overlapping ovl setups.
I think that a better approach would be to fail a lookup in the upper layer
that results with a d_mountpoint() - those are not expected inside the overlay
upper layer AFAICT.

Anyway, I can make those changes if Miklos agrees with them, but I don't
want to complicate your work on this, so maybe for now, create the I_EXCL_INUSE
generic flag without changing overlayfs and I can make the cleanup to get rid of
I_OVL_INUSE later.

Thanks,
Amir.

--
Linux-cachefs mailing list
Linux-cachefs@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/linux-cachefs




[Index of Archives]     [LARTC]     [Bugtraq]     [Yosemite Forum]
  Powered by Linux