On Wed, Oct 18, 2017 at 08:40:59AM +0300, Amir Goldstein wrote:
> On Wed, Oct 18, 2017 at 12:05 AM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote:
> > OVL_METACOPY in oi->flags can be accessed in lockless manner. So if a file
> > is being copied up metadata only, we need to make sure that upperdentry is
> > visible only after OVL_METACOPY flag has been set. IOW, if oi->__upperdentry
> > is visble to a cpu, then we also need to make sure any updates to OVL_METACOPY
> > flags are visible too.
> >
>
> You know, I have a feeling that this ordering requirement could be simplified or
> completely avoided if you flip the meaning of the flag, i.e.:
>
> bool ovl_dentry_has_upper_data(struct dentry *dentry)
> {
> return ovl_test_flag(OVL_UPPER_DATA, d_inode(dentry));
> }
>
> Then flag visibility requirements are the same as visibility requirements
> for oe->has_upper.
> You probably don't need to add any new barriers for setting setting the flag
> on normal copy up.
> For setting the flag in copy_up_meta_inode_data, and testing the flag
> in ovl_d_real() the requirements stay the same as you implemented in patch 10.
Hi Amir,
I thought about it and IIUC, flipping the bit does not do away with the
ordering requirement w.r.t ovl_inode_update(). For example.
Say on CPU1 a file is being copied up (both data and metadata copy up).
ovl_copy_up_inode()
install inode;
ovl_set_flag(OVL_UPPER_DATA);
ovl_inode_update();
Assume, Say another CPU2 is doing d_real() with flags=0.
ovl_d_real()
real = ovl_dentry_upper(dentry);
if (real) {
if (!ovl_test_flag(OVL_UPPER_DATA, d_inode(dentry)))
goto lower;
}
Now assume that CPU2 has not seen the update of OVL_UPPER_DATA yet. So it
will end up returning a "lower" dentry, while it should have returned
an upper dentry. So ordering requirement is very much still there.
What do you think?
To simplify ordering requirements w.r.t ovl_inode_updat(), can we replace
data dependency barrier in ovl_upperdentry_dereference() with a read
barrier instead (smp_rmb()). That way we will not have to introduce
this additional smp_rmb() everything and code will be simpler.
Thoughts?
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
