On Fri, May 4, 2018 at 6:54 PM, Vivek Goyal <vgoyal@xxxxxxxxxx> wrote: > On Fri, May 04, 2018 at 10:04:22AM +0300, Amir Goldstein wrote: [...] >> > >> > Just maybe missing the case of nlink=1 and ORIGIN cannot be >> > followed because lower fs doesn't support file handles. >> > I just wanted to make sure this case is handled correctly. >> > >> >> OK. re-read the code and ready to explain the situation. >> >> The meaning of new OVL_CONST_INO flag is: >> "inode preserves st_ino across copy up". >> If lower fs support file handles it also means: >> "...and across eviction from inode cache". >> but if lower fs does not support file handles it means: >> "...while non-dir inode remains in cache" >> >> How does METACOPY change the situation? >> very slightly - with METACOPY, instead of st_ino change >> after copy_up+cache_evict, st_ino will not change after >> meta_copy_up+cache_evict, but will change after >> data_copy_up+cache_evict, which is not a problem as >> far as I can tell. >> >> Specifically, removing METACOPY xattr will not change >> st_ino nor hash_by_lower until after cache eviction and as >> long as we have one-to-one mapping of upper inode to >> lower inode that's good enough. > > Hi Amir, > > Thanks for detailed explanation. I now understand your concern. > > How about I pass in the origin information in ovl_get_inode() and > to ovl_hash_bylower(). And we set bylower=1 for a metacopy dentry > only if an upper origin could be found and verified? > > That way behavior will be similar to a file with nlink=1 and ORIGIN > not there. And we will not have to worry about thinking another > special corner case and writing test cases for it. > > Too many corner cases are bad for future code maintenance and easy > to break. > > WDYT? > I think you should leave it for now and maybe we will address it later. The reason I am not very concerned anymore is because I realize we are already exposed to situation of lower without ORIGIN - this is how a just copied up inode looks like when lower fs doesn't support file handles. So METACOPY is not adding a corner case, it just increases the window of time where the corner case exists beyond inode eviction from cache. 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
