On Mon, Nov 6, 2017 at 11:17 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: > On Thu, Nov 2, 2017 at 9:38 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: >> ovl_rename() updates dir cache version for impure old parent if an entry >> with copy up origin is moved into old parent, but it did not update >> cache version if the entry moved out of old parent has a copy up origin. >> >> Signed-off-by: Amir Goldstein <amir73il@xxxxxxxxx> >> --- >> fs/overlayfs/dir.c | 5 +++-- >> 1 file changed, 3 insertions(+), 2 deletions(-) >> >> diff --git a/fs/overlayfs/dir.c b/fs/overlayfs/dir.c >> index ef533198be45..26aef3b5f007 100644 >> --- a/fs/overlayfs/dir.c >> +++ b/fs/overlayfs/dir.c >> @@ -1075,9 +1075,10 @@ static int ovl_rename(struct inode *olddir, struct dentry *old, >> drop_nlink(d_inode(new)); >> } >> >> - ovl_dentry_version_inc(old->d_parent, >> - !overwrite && ovl_type_origin(new)); >> + ovl_dentry_version_inc(old->d_parent, ovl_type_origin(old)); >> ovl_dentry_version_inc(new->d_parent, ovl_type_origin(old)); >> + if (!overwrite) >> + ovl_dentry_version_inc(old->d_parent, ovl_type_origin(new)); > > How about the opposite (i.e. newdir losing impurity)? That can happen two ways: > > - overwriting copied up file (new) with pure one (old) > - exchange copied up file (new) with pure one (old) > > Also I'd merge the two version increments into one, although that > doesn't really matter: > > - ovl_dentry_version_inc(old->d_parent, > - !overwrite && ovl_type_origin(new)); > - ovl_dentry_version_inc(new->d_parent, ovl_type_origin(old)); > + ovl_dentry_version_inc(old->d_parent, ovl_type_origin(old) || > + (!overwrite && ovl_type_origin(new))); > + ovl_dentry_version_inc(new->d_parent, > + ovl_type_origin(old) || ovl_type_origin(new)); > > Fixed up the patch, but tell me if I'm missing something here. > I think you are. Both my version and your version are wrong because the argument 'impurity' should really be 'impurity_change' so if we have: oldimpurity = ovl_type_origin(old); newimpurity = d_inode(new) ? ovl_type_origin(new) : false; version increment should really be something like this: ovl_dentry_version_inc(old->d_parent, oldimpurity ^ (overwrite ? newimpurity : false)); ovl_dentry_version_inc(new->d_parent, oldimpurity ^ newimpurity); I think... Amir. -- 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
