Re: [RFC][PATCH 1/2] ovl: store path type in dentry

Amir Goldstein
2017-03-28T20:13:13Z

On Tue, Mar 28, 2017 at 12:59 PM, Amir Goldstein <amir73il@gmail.com> wrote:
> On Tue, Mar 28, 2017 at 10:03 AM, Miklos Szeredi <miklos@szeredi.hu> wrote:
>> On Tue, Mar 21, 2017 at 5:16 PM, Amir Goldstein <amir73il@gmail.com> wrote:
>>> ovl_path_type() is called quite often (e.g.: on every file open)
>>> to calculate the path type flags of overlay dentry. Those flags
>>> are rarely changed after dentry instantiation and when changed,
>>> flags can only be added. (e.g.: on copyup, rename and create).
>>>
>>> Store the type value in ovl_entry and update the flags when
>>> needed, so ovl_path_type() just returns the stored value.
>>
>> Since accessing __upperdentry is lockless we need to be careful with
>> memory ordering issues.
>>
>> For example in the new version of ovl_dentry_update() we first store
>> oe->__upperdentry then oe->type. In a concurrent ovl_path_real() we
>> first read oe->type and then based on the OVL_TYPE_UPPER flag read
>> oe->__upperdentry. Without any barriers the above could very easily
>> result in NULL upperdentry being read. The compiler or the CPU could
>> reorder the stores in ovl_dentry_update(), etc...
>>
>
> Right.. thanks for pointing that out. I'll take better care.
>
> FYI, I am just in the midst of adding oe->__tempdentry and
> OVL_TYPE_RO_UPPER for copy up on O_RDONLY.
> I started by trying to "upgrade" oe->__upperdentry from tmpfile
> to linked file, but that was real messy.
>
>> For the gory details see Documentation/memory-barriers.txt
>>
>
> I know about them, but still easy for me to get them wrong...
> Hopefully will have something for you to review by tomorrow.
>

There's a WIP here
https://github.com/amir73il/linux/commits/consistent_fd-wip

It's still buggy, but passes most of the overlay/quick xfstests including
overlay/016 ("Test ro/rw fd data inconsistencies")

It still doesn't have the fixes to safe access oe->type, but if you want to
take a look and stop me if I'm heading in the wrong direction...

Thanks,
Amir.