On Mon, Jan 31, 2022 at 5:12 PM David Howells <dhowells@xxxxxxxxxx> wrote: > > > Hi Amir, > > How about this as a set of patches to do what you suggest[1] and hoist the > handler functions for I_OVL_INUSE into common code and rename the flag to > I_EXCL_INUSE. This can then be shared with cachefiles - allowing me to get > rid of S_KERNEL_FILE. > They look like what I had in mind. Unfortunately, I had forgotten about another use that ovl makes of the flag (see comment on patch 1/5). I'd made a suggestion on how to get rid of that use case, but I hope this won't complicate things too much for you. > I did split out the functionality for preventing file/dir removal to a > separate flag, I_NO_REMOVE, so that it's not tied to I_EXCL_INUSE in case > overlayfs doesn't want to use it. The downside to that, though is that it > requires a separate locking of i_lock to set/clear it. > > I also added four general tracepoints to log successful lock/unlock, > failure to lock and a bad unlock. The lock tracepoints log which driver > asked for the lock and all tracepoints allow the driver to log an arbitrary > reference number (in cachefiles's case this is the object debug ID). > > Questions: > > (1) Should it be using a flag in i_state or a flag in i_flags? I'm not > sure what the difference is really. Me neither. > > (2) Do we really need to take i_lock when testing I_EXCL_INUSE? Would > READ_ONCE() suffice? > For ovl_is_inuse() I think READ_ONCE() should suffice. Thanks, Amir.
