On Tue, Mar 28, 2017 at 12:59 PM, Amir Goldstein <amir73il@xxxxxxxxx> wrote: > On Tue, Mar 28, 2017 at 10:03 AM, Miklos Szeredi <miklos@xxxxxxxxxx> wrote: >> On Tue, Mar 21, 2017 at 5:16 PM, Amir Goldstein <amir73il@xxxxxxxxx> 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. -- 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
