On Tue, Jan 23, 2024 at 11:05 AM Alexander Larsson <alexl@xxxxxxxxxx> wrote:
>
> On Mon, 2024-01-22 at 21:51 +0200, Amir Goldstein wrote:
> > An opaque directory cannot have xwhiteouts, so instead of marking an
> > xwhiteouts directory with a new xattr, overload overlay.opaque xattr
> > for marking both opaque dir ('y') and xwhiteouts dir ('x').
> >
> > This is more efficient as the overlay.opaque xattr is checked during
> > lookup of directory anyway.
> >
> > This also prevents unnecessary checking the xattr when reading a
> > directory without xwhiteouts, i.e. most of the time.
> >
> > Note that the xwhiteouts marker is not checked on the upper layer and
> > on the last layer in lowerstack, where xwhiteouts are not expected.
> >
> > Fixes: bc8df7a3dc03 ("ovl: Add an alternative type of whiteout")
> > Cc: <stable@xxxxxxxxxxxxxxx> # v6.7
> > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx>
> > ---
> >
> > Miklos,
> >
> > This v4 is a combination of your v2 and my v3 patches to optimize
> > xwhiteouts readdir for the intersection of a dentry with xwhiteouts
> > on any layer and a layer with any xwhiteouts on any directory.
> >
> > Alex,
> >
> > Please re-review/test.
>
> Looks good to me. The only thing I worry about is the atomicity of
> ovl_layer_set_xwhiteouts(). Doesn't that need a barrier or something?
>
I think it does not because:
ovl_layer_set_xwhiteouts() is called before adding the overlay dir
dentry to dcache, while readdir of that same directory happens
after the overlay dir dentry is in dcache, so if some cpu observes
ovl_dentry_is_xwhiteouts(), it will also observe layer->has_xwhiteouts
for the layers where xwhiteouts marker was found in that merge dir.
I hope I got this right (Miklos?).
I added this non-trivial comment above ovl_layer_set_xwhiteouts().
> Anyway:
>
> Reviewed-by: Alexander Larsson <alexl@xxxxxxxxxx>
> Tested-by: Alexander Larsson <alexl@xxxxxxxxxx>
>
Pushed to ovl-fixes.
Pending ACK from Miklos.
Thanks,
Amir.
