On Wed, May 27, 2020 at 02:16:00PM +0300, Amir Goldstein wrote: [..] > > After check the code, there may some bug need to fix: > > 1. We need to call iput once ovl_check_metacopy_xattr fail. > > 2. We need to call unlock_new_inode or the above iput(also with iput in > > ovl_create_object) will trigger the a WARN_ON since the I_NEW still > > exists. > > 3. We should move the init for upperdentry to the place below > > ovl_check_metacopy_xattr. Or the dentry reference will decrease to > > -1(error path in ovl_create_upper will inc, ovl_destroy_inode too). > > > > OR we don't check metacopy xattr in ovl_get_inode(). > > In ovl_lookup() we already checked metacopy xattr. > No reason to check it again in this subtle context. > > In ovl_lookup() can store value of upper metacopy and after we get > the inode, set the OVL_UPPERDATA inode flag according to > upperdentry && !uppermetacopy. I think reason behind initializing this attr in ovl_get_inode() was that this is OVL_UPPERDATA is an inode property. So conceptually it makes sense to initialize it when inode is being instantiated. And that too under lock so that there are no races. I was trying to think if we can trigger a race if we move OVL_UPPERDATA initialization in ovl_lookup(). But given this is only one way transition, I could not think of any. So for this speicific flag, it probably is ok to initialize outside of ovl_get_inode(). Vivek
