On Fri, Nov 10, 2017 at 11:43:36AM +0200, Amir Goldstein wrote:
[..]
> > --- a/fs/overlayfs/util.c
> > +++ b/fs/overlayfs/util.c
> > @@ -267,13 +267,26 @@ bool ovl_has_upperdata(struct dentry *dentry) {
> > if (!ovl_should_check_upperdata(dentry))
> > return true;
> >
> > - return ovl_test_flag(OVL_UPPERDATA, d_inode(dentry));
> > + if (!ovl_test_flag(OVL_UPPERDATA, d_inode(dentry)))
> > + return false;
> > + /*
> > + * Pairs with smp_wmb() in ovl_copy_up_meta_inode_data(). Make sure
> > + * if setting of OVL_UPPERDATA is visible, then effects of writes
> > + * before that are visible too.
> > + */
> > + smp_rmb();
> > + return true;
> > }
> >
>
> I think it is not right to have smp_rmb() is the locked path where it
> is not needed
> too costly.
>
> We should probably have a different variant for ovl_already_copied_up_locked()
> and if OVL_UPPERDATA is set for all non-eligible cases, ovl_has_upperdata()
> can have the smp_rmb() and locked variant can just can check the flag directly.
How about adding a parameter "bool locked" to ovl_already_copied_up() and
ovl_has_upperdata(). So if locked is true, smp_rmb() will be skipped by
ovl_has_upperdata().
Vivek
--
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
