Re: [PATCH 5/9] ovl: Set xattr OVL_XATTR_METACOPY on upper file

[Vivek Goyal <vgoyal@redhat.com>]

On Tue, Oct 10, 2017 at 08:03:05PM +0300, Amir Goldstein wrote:

[..]
> > @@ -511,6 +564,10 @@ static int ovl_copy_up_locked(struct ovl_copy_up_ctx *c)
> >                 goto out_cleanup;
> >
> >         ovl_inode_update(d_inode(c->dentry), newdentry);
> > +       if (c->metadata_only) {
> > +               smp_wmb();
> 
> This wmb does not address the only problem imo.
> The problem is you must not allow inode to appear to have
> An upper dentry before you made sure metacopy flag is visible.
> So first you need to set metacopy flag before updating upper.
> Then you also need to test them is correct order with rmb.

Ok, so conceptually it makse sense to me that OVL_METACOPY flag should
be set before upper dentry is made visible.

So ovl_set_flag(OVL_METACOPY) should move before ovl_inode_update(). 
ovl_inode_update() already has a smp_wmb(). So write side is probably
fine.

But read side, ovl_upperdentry_dereference() only has data dependency
barrier and not smp_rmb(). (lockless_dereference(oi->__upperdentry)). I
am not sure if that's sufficient for ovl_inode->flags read or not. May
be we need to replace data dependency barrier with smp_rmb() in in
ovl_upperdentry_dereference()?

Even if we do this, I think we need barriers around setting/resetting
of OVL_METACOPY for the reset case. I will explain this in other patch.

Vivek