On Thu, 18 Jan 2024 at 12:22, Amir Goldstein amir73il@gmail.com wrote:
On Thu, Jan 18, 2024 at 12:41 PM Miklos Szeredi mszeredi@redhat.com 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