On 01/09/2014 11:04 AM, Srinivas Eeda wrote: > On 01/09/2014 08:34 AM, Goldwyn Rodrigues wrote: >> On 01/09/2014 10:06 AM, Srinivas Eeda wrote: >>> On 01/09/2014 07:44 AM, Goldwyn Rodrigues wrote: >>>> Hi Srini, >>>> >>>> Thanks for the reply. >>>> >>>> On 01/08/2014 11:30 PM, Srinivas Eeda wrote: >>>>>>>> >From the comments in fs/ocfs2/inode.h:90 it seems, this was >>>>>>>> used in >>>>>>>> legacy ocfs2 systems when a node received unlink votes. Since >>>>>>>> unlink >>>>>>>> votes has been done away with and replaced with open locks, is this >>>>>>>> flag still required? If yes, why? >>>>>>> My understanding is that unlink voting protocol was heavy. So the >>>>>>> following was done to address it. >>>>>>> >>>>>>> To do an unlink, dentry has to be removed. In order to do that the >>>>>>> node >>>>>>> has to get EX lock on the dentry which means all other nodes have to >>>>>>> downconvert. In general EX lock on dentry is acquired only in >>>>>>> unlink and >>>>>>> I assume rename case. So all nodes which down convert the lock mark >>>>>>> their inode OCFS2_INODE_MAYBE_ORPHANED. The only problem with >>>>>>> this is >>>>>>> that dentry on a node can get purged because of memory pressure >>>>>>> which >>>>>>> marks inode as OCFS2_INODE_MAYBE_ORPHANED even when no unlink was >>>>>>> done >>>>>>> on this inode. >>>>>>> >>>>>> >>>>>> I think you are getting confused between dentry_lock (dentry_lockres) >>>>>> and open lock (ip_open_lockres). AFAICS, dentry locks are used to >>>>>> control the remote dentries. >>>>> I was trying to answer why we need OCFS2_INODE_MAYBE_ORPHANED flag, I >>>>> guess I wasn't clear. I'll make an other attempt :). >>>>> >>>>> One way for node A to tell node B that an unlink had happened on >>>>> node A >>>>> is by sending an explicit message(something similar to what we had in >>>>> old release). When node B received such communication it marked inode >>>>> with OCFS2_INODE_MAYBE_ORPHANED flag if it still had the inode in use. >>>>> >>>>> The other way(current implementation) is to indirectly tell it by >>>>> asking >>>>> node B to purge dentry lockres. Once node B has been informed that >>>>> dentry lock has to be released, it assumes inode might have been >>>>> unlinked somewhere and marks the inode with OCFS2_INODE_MAYBE_ORPHANED >>>>> flag. >>>>> >>>>> So, we need OCFS2_INODE_MAYBE_ORPHANED flag to tell node B that it >>>>> should finish the second phase of unlink(remove the inode from file >>>>> system) when it closes the file. >>>> >>>> Okay, but why should node B do the cleanup/wipe when node A initiated >>>> the unlink()? Shouldn't it be done by node A? All node B should do is >>>> to write the inode and clear it from the cache. The sequence is >>>> synchronized by dentry_lock. Right? >>> removing dentry is only the first part. An inode can still be open after >>> that. >> >> Yes, I did not consider that. >> How about using open locks ro_holders count to identify this? That may >> just work. Thanks! > One problem I see in using open lock for this is it could be late. > Consider the scenario where node A removes the dentry and then the node > crashes before trying the try_open_lock. Node B does the file close > later but it doesn't know that the file was unlinked and doesn't do the > clean up. > > To me it appears OCFS2_INODE_MAYBE_ORPHANED is necessary. Any delay it > is causing must be addressed differently.
No, I don't mean to remove the OCFS2_INODE_MAYBE_ORPHANED flag, but set it conditionally in ocfs2_dentry_convert_worker() based on the value of the open locks held. I'll write a patch and test. Thanks! -- Goldwyn _______________________________________________ Ocfs2-devel mailing list Ocfs2-devel@oss.oracle.com https://oss.oracle.com/mailman/listinfo/ocfs2-devel
