Why doesn't the export list have it's own ref? The sentinal ref is for the hash/lookup table, and shouldn't be overloaded, in my opinion.
It can't. The problem is that if it holds a reference, then we never release the penultimate reference until an unexport removes the object from the export list. The export list is a secondary way to get an initial reference and thus cleanup of the export list has to be done in conjunction with removing an object from the hash table to prevent additional initial references. Frank
Daniel
On 4/22/20 5:55 PM, Frank Filz wrote:
Hmm, ok, I thought I considered all the cases where reachability was an issue...
I think actually the correct fix is to re-arrange things so we don't drop the sentinel ref until after cleaning the entry out of the export list... The sentinel ref should remain until the object is truly unreachable.
Frank
-----Original Message----- From: Ashish Sangwan [mailto:ashishsangwan2@gmail.com] Sent: Wednesday, April 22, 2020 11:25 AM To: devel@lists.nfs-ganesha.org; Daniel Gryniewicz <dang@redhat.com>; ffilzlnx@mindspring.com Subject: [NFS-Ganesha-Devel] mdcache unexport race with Rmdir/Unlink
It seems that mdcache unexport can race with remove RPC. Remove operation drops the sentinel ref and hence when the last call to mdcache_put() happen during the execution of remove RPC, the corresponding mdcache_entry will be freed. If an export remove comes in while the remove OP is still executing, in mdcache_unexport() when we call mdcache_lru_ref(entry, LRU_REQ_INITIAL) we can end up taking a ref on the entry whose refcnt has already reached 0 and which is currently going through mdcache_lru_clean() execution in the context of remove. The entry is still present in the mdcache_fsal_export's entry_list because mdcache_lru_clean() has not yet called the mdc_clean_entry(). Now when do mdcache_put() for this entry later in mdcache_unexport(), by this time the entry is already freed and we end up executing mdcache_lru_clean() on a freed entry.
What we need to do inside mdcache_unexport() is to take QLOCK() and check refcount, if it's 0, continue the for loop for next entry, if it's not 0 than increment the ref count, drop the QLOCK and continue with normal current execution path. Does it makes sense? _______________________________________________ Devel mailing list -- devel@lists.nfs-ganesha.org To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org
_______________________________________________ Devel mailing list -- devel@lists.nfs-ganesha.org To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org