On Wed, May 24, 2017 at 3:29 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > An upper dir is marked "impure" to let ovl_iterate() know that this > directory may contain non pure upper entries whose d_ino may need to be > read from the origin inode. > > We already mark a non-merge dir "impure" when moving a non-pure child > entry inside it, to let ovl_iterate() know not to iterate the non-merge > dir directly. > > Mark also a merge dir "impure" when moving a non-pure child entry inside > it and when copying up a child entry inside it. > > This can be used to optimize ovl_iterate() to perform a "pure merge" of > upper and lower directories, merging the content of the directories, > without having to read d_ino from origin inodes. > > Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> > --- > fs/overlayfs/copy_up.c | 7 +++++++ > fs/overlayfs/dir.c | 31 ++++++------------------------- > fs/overlayfs/namei.c | 20 -------------------- > fs/overlayfs/overlayfs.h | 14 +++++++++++--- > fs/overlayfs/super.c | 5 ++++- > fs/overlayfs/util.c | 31 +++++++++++++++++++++++++++++++ > 6 files changed, 59 insertions(+), 49 deletions(-) > > > Miklos, > > Please consider this patch for v4.12-rc3 on top of the one already in > overlayfs-next. > > Not sure how common the use case of "pure merge" is, but the optimization > of not doing the origin lookup in ovl_interate() is trivial [1], so better > have the merge dir marked "impure" now when introducing overlay.origin. Miklos, Ping for rc4. FYI, I have been testing this patch along with ovl-constino and the rest of my dev branch over the past week and everything looks sane. Please see one clarification/disclaimer below, which I don't think warrants a problem w.r.t merging this patch to rc4. Thanks, Amir. > > v3: > - Fix bug that slipped into ovl_check_dir_xattr() in v1 > > v2: > - Check on mount if upper dir is impure > - Tested the optimization [1] with consistent st_ino/d_ino patch > > [1] https://github.com/amir73il/linux/blob/ovl-constino/fs/overlayfs/readdir.c#L413 > > diff --git a/fs/overlayfs/copy_up.c b/fs/overlayfs/copy_up.c > index 843ed2a..1ae1790 100644 > --- a/fs/overlayfs/copy_up.c > +++ b/fs/overlayfs/copy_up.c > @@ -459,6 +459,13 @@ static int ovl_copy_up_one(struct dentry *parent, struct dentry *dentry, > ovl_path_upper(parent, &parentpath); > upperdir = parentpath.dentry; > > + /* Mark parent "impure" because it may now contain non-pure upper */ > + if (!ovl_dentry_is_impure(parent)) { > + err = ovl_set_impure(parent, upperdir); > + if (err) > + return err; > + } > + Disclaimer: --------------- ovl_dentry_is_impure() here is not tested under any lock and with no explicit memory barriers nor does ovl_dentry_set_impure() inside ovl_set_impure(). The worst outcome for this code seems to be that overlay.impure xattr can be set twice, which shouldn't be a problem (?). For v4.13, when ovl_iterate() will test ovl_dentry_is_impure(), it will be more important to make sure that ovl_iterate() does not read an old false value of oe->impure and cause some inconsistency. I think that the (real) parent i_mutex lock taken further down copy up and the (overlay) parent i_mutex lock taken in iterate_dir() guaranty the required memory barriers (?). Perhaps it would be safer to test ovl_is_impuredir(upperdentry) in ovl_iterate() and not rely on the memory cached value of oe->impure. -- To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
