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.
