On Thu, 18 Jan 2024 at 12:22, Amir Goldstein <amir73il@xxxxxxxxx> wrote:
>
> On Thu, Jan 18, 2024 at 12:41 PM Miklos Szeredi <mszeredi@xxxxxxxxxx> wrote:
> >
> > Add a check on each layer for the xwhiteout feature. This prevents
> > unnecessary checking the overlay.whiteouts xattr when reading a
> > directory if this feature is not enabled, i.e. most of the time.
>
> Does it really have a significant cost or do you just not like the
> unneeded check?
It's probably insignificant. But I don't know and it would be hard to prove.
> IIRC, we anyway check for ORIGIN xattr and IMPURE xattr on
> readdir.
We check those on lookup, not at readdir. Might make sense to check
XWHITEOUTS at lookup regardless of this patch, just for consistency.
> > --- a/fs/overlayfs/overlayfs.h
> > +++ b/fs/overlayfs/overlayfs.h
> > @@ -51,6 +51,7 @@ enum ovl_xattr {
> > OVL_XATTR_PROTATTR,
> > OVL_XATTR_XWHITEOUT,
> > OVL_XATTR_XWHITEOUTS,
> > + OVL_XATTR_FEATURE_XWHITEOUT,
>
> Can we not add a new OVL_XATTR_FEATURE_XWHITEOUT xattr.
>
> Setting OVL_XATTR_XWHITEOUTS on directories with xwhiteouts is
> anyway the responsibility of the layer composer.
>
> Let's just require the layer composer to set OVL_XATTR_XWHITEOUTS
> on the layer root even if it does not have any immediate xwhiteout
> children as "layer may have xwhiteouts" indication. ok?
Okay.
> > @@ -1414,6 +1414,17 @@ int ovl_fill_super(struct super_block *sb, struct fs_context *fc)
> > if (err)
> > goto out_free_oe;
> >
> > + for (i = 0; i < ofs->numlayer; i++) {
> > + struct path path = { .mnt = layers[i].mnt };
> > +
> > + if (path.mnt) {
> > + path.dentry = path.mnt->mnt_root;
> > + err = ovl_path_getxattr(ofs, &path, OVL_XATTR_FEATURE_XWHITEOUT, NULL, 0);
> > + if (err >= 0)
> > + layers[i].feature_xwhiteout = true;
>
>
> Any reason not to do this in ovl_get_layers() when adding the layer?
Well, ovl_get_layers() is called form ovl_get_lowerstack() implying
that it's part of the lower layer setup.
Otherwise I don't see why it could not be in ovl_get_layers(). Maybe
some renaming can help.
Thanks,
Miklos
