On Mon, 22 Jan 2024 at 14:18, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > > Something like this looks ok? > > --- a/Documentation/filesystems/overlayfs.rst > +++ b/Documentation/filesystems/overlayfs.rst > @@ -145,7 +145,9 @@ filesystem, an overlay filesystem needs to record > in the upper filesystem > that files have been removed. This is done using whiteouts and opaque > directories (non-directories are always opaque). > > -A whiteout is created as a character device with 0/0 device number. > +A whiteout is created as a character device with 0/0 device number or > +as a regular file with the xattr "trusted.overlay.whiteout". It should also refer to the "whiteouts and opaque directories" section. > + > When a whiteout is found in the upper level of a merged directory, any > matching name in the lower level is ignored, and the whiteout itself > is also hidden. > @@ -154,6 +156,11 @@ A directory is made opaque by setting the xattr > "trusted.overlay.opaque" > to "y". Where the upper filesystem contains an opaque directory, any > directory in the lower filesystem with the same name is ignored. > > +An opaque directory should not conntain any whiteouts, because they do not > +serve any purpose. A merge directory containing regular files with the xattr > +"trusted.overlay.whiteout", should be additionally marked by setting the xattr > +"trusted.overlay.opaque" to "x" on the merge directory itself. I think it's worth noting that this can have a performance impact on readdir, so people don't think xwhiteouts are a drop in replacement. > Alex already did that: > > https://docs.kernel.org/filesystems/overlayfs.html#nesting-overlayfs-mounts Indeed, thanks. > We do not currently have per-directory-per-layer flags in ovl_lowerstack(). > > Your patch was optimizing only per-layer check_xwhiteout. > My patch is optimizing only per-directory check_xwhiteout. > > The important thing is that for the common case (no xwhiteouts) > xwhiteout will never be checked. > > Are you concerned about optimizing check_xwhiteout in a multi lower > overlayfs nested over a composefs overlay mount for the case that > one of the merge dirs in the stack have xwhiteouts and the other do not?? > > I guess we can use a combination of your patch (v2) and my patch (v3) > and do something like this: > > if (last_element && !is_upper && val == 'x') { > d->xwhiteouts = d->layer->xwhiteouts = true; > > ... > > to mark the dentry as OVL_E_XWHITEOUTS > AND mark the layer as having xwhiteouts > and then in readdir we check that > BOTH dentry has xwhiteouts (in some layer) > AND the layer has xwhiteouts (in some directory). > > Is that what you meant? I didn't think it through, but yeah, this sounds good. This way we can also remove the checks against layer not being upper and lowest, right? Thanks, Miklos
