Okay, I misunderstood the situation. That fix won't work, because mdcache_lru_unref_chunk() takes the qlane lock, and it also takes the content lock, which must be taken before the qlane lock. The problem here is that the parent has a pointer to the chunk and a ref on it, and the chunk has a pointer to the parent but no ref on it. This means that the parent refcount can go to zero while the chunk has a pointer. I think we need to force remove all the chunks when we recycle the parent, and null out the parent pointer in the chunk as well. We'll have to audit all the uses of the parent pointer and make sure the call path has a ref to the parent, or that it can handle a null parent pointer. Thoughts? Ideas? Daniel On 4/10/19 5:40 PM, Pradeep wrote:

Hi Daniel,

I already have that fix (https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/441566) you mention below. I believe this race is different. What I meant is something like this may be needed; otherwise the chunk cleanup will end up using wrong parent pointer. https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/450825

Thanks, Pradeep

On Wed, Apr 10, 2019 at 11:31 AM Daniel Gryniewicz <dang@redhat.com <mailto:dang@redhat.com>> wrote:

This is probably 389f1f09da19c4f42046474af3869815b6bd5959 fixed in 2.7.2.

Daniel

On 4/10/19 1:55 PM, Pradeep wrote: > Hello, > > I'm hitting hang where rmv_detached_dirent() is stuck at the spinlock > for ever. > > #0  0x00007feb01a9a4e5 in pthread_spin_lock () from /lib64/libpthread.so.0 > #1  0x00000000005519e7 in rmv_detached_dirent (parent=0x7feaeb8dd600, > dirent=0x7fea9fbd8700) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_int.h:420 > #2  0x00000000005522e8 in mdcache_avl_remove (parent=0x7feaeb8dd600, > dirent=0x7fea9fbd8700) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_avl.c:256 > #3  0x0000000000547752 in mdcache_clean_dirent_chunk (chunk=0x7fea9f86f970) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_helpers.c:454 > #4  0x00000000005386a2 in lru_clean_chunk (chunk=0x7fea9f86f970) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:2078 > #5  0x000000000053881b in mdcache_lru_unref_chunk (chunk=0x7fea9f86f970) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:2097 > #6  0x000000000053698c in chunk_lru_run_lane (lane=14) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:1509 > #7  0x0000000000536d26 in chunk_lru_run (ctx=0x7feafe00f580) >      at > /usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:1563 > #8  0x0000000000508ad9 in fridgethr_start_routine (arg=0x7feafe00f580) >      at /usr/src/debug/nfs-ganesha-2.7.1/support/fridgethr.c:550 > #9  0x00007feb01a95e25 in start_thread () from /lib64/libpthread.so.0 > #10 0x00007feb0139dbad in clone () from /lib64/libc.so.6 > > 420             pthread_spin_lock(&parent->fsobj.fsdir.spin); > (gdb) print parent->fsobj.fsdir.spin > $1 = -1 > (gdb) print parent->obj_handle.type > $2 = REGULAR_FILE > > Looks like when the thread is in this path, the parent got reused for a > different object in the filesystem. From looking at code, this seems > possible: > > Lets say thread1 goes through the reuse path: > >   * mdcache_lru_get() -> >       o mdcache_lru_clean() -> >           + mdc_clean_entry() -> >               # mdcache_dirent_invalidate_all() >                   * mdcache_lru_unref_chunk() >                   * This will call lru_clean_chunk() only if refcnt is >                     zero. Lets say another thread (thread2 below) >                     incremented it from the background thread. So >                     thread1 will return back and the entry now gets reused. > > > Now the background thread (thread2) comes up to clean chunks and > increments refcnt: > >   * chunk_lru_run() >       o chunk_lru_run_lane() >           + mdcache_lru_unref_chunk() >           + Here we unlock qlane lock and then decrement refcnt. When >             this thread unlocks, thread1 will grab qlane lock and skip >             the chunk because refcnt is > 0. By the time we reach >             mdcache_lru_unref_chunk(), the parent would have been reused >             by thread1 for a different object. Now >             mdcache_lru_unref_chunk() make progress as refcnt became >             zero; but parent is invalid. So it gets stuck in >             rmv_detached_dirent(). > > I think we should hold the qlock in chunk_lru_run_lane() till the refcnt > is decremented to make sure that reaping of parent is not possible. > Since mdcache_lru_unref_chunk() already holds the lock later, we could > probably pass a flag to indicate if the qlane lock is already held or > nor (similar to content_lock). Please let me know if there is a better > approach to refactor this. > > Thanks, > Pradeep > > > > _______________________________________________ > Devel mailing list -- devel@lists.nfs-ganesha.org <mailto:devel@lists.nfs-ganesha.org> > To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org <mailto:devel-leave@lists.nfs-ganesha.org> > _______________________________________________ Devel mailing list -- devel@lists.nfs-ganesha.org <mailto:devel@lists.nfs-ganesha.org> To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org <mailto:devel-leave@lists.nfs-ganesha.org>

On 4/11/19, Frank Filz <ffilzlnx@mindspring.com> wrote:

...

From: Daniel Gryniewicz [mailto:dang@redhat.com] Sent: Thursday, April 11, 2019 5:28 AM

Okay, I misunderstood the situation. That fix won't work, because mdcache_lru_unref_chunk() takes the qlane lock, and it also takes the content lock, which must be taken before the qlane lock.

The problem here is that the parent has a pointer to the chunk and a ref on it, and the chunk has a pointer to the parent but no ref on it. This means that the parent refcount can go to zero while the chunk has a pointer. I think we need to force remove all the chunks when we recycle the parent, and null out the parent pointer in the chunk as well. We'll have to audit all the uses of the parent pointer and make sure the call path has a ref to the parent, or that it can handle a null parent pointer.

Hi Daniel, By force cleaning, did you have something like this in mind? https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/451311/1/src/FSAL/Stackabl... Thanks, Pradeep

...

Thoughts? Ideas?

This sounds good. It looks like lru_reap_chunk_impl and mdcache_lru_unref_chunk need to be able to handle a NULL parent pointer.

There are other functions that use chunk->parent, but have been passed in the parent. Those functions maybe should assert that parent == chunk->parent (or entry == chunk->parent in the case of avl_dirent_set_deleted. mdcache_clean_dirent_chunk could be passed the parent to facilitate this check (both places it's called already have the parent). Note that function already sets chunk->parent = NULL.

Frank

...

On 4/10/19 5:40 PM, Pradeep wrote:

...

Hi Daniel,

I already have that fix (https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/441566) you mention below. I believe this race is different. What I meant is something like this may be needed; otherwise the chunk cleanup will end up using wrong parent pointer. https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/450825

Thanks, Pradeep

On Wed, Apr 10, 2019 at 11:31 AM Daniel Gryniewicz <dang@redhat.com <mailto:dang@redhat.com>> wrote:

This is probably 389f1f09da19c4f42046474af3869815b6bd5959 fixed in 2.7.2.

Daniel

On 4/10/19 1:55 PM, Pradeep wrote: > Hello, > > I'm hitting hang where rmv_detached_dirent() is stuck at the spinlock > for ever. > > #0 0x00007feb01a9a4e5 in pthread_spin_lock () from /lib64/libpthread.so.0 > #1 0x00000000005519e7 in rmv_detached_dirent (parent=0x7feaeb8dd600, > dirent=0x7fea9fbd8700) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_int.h:420 > #2 0x00000000005522e8 in mdcache_avl_remove (parent=0x7feaeb8dd600, > dirent=0x7fea9fbd8700) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_avl.c:256 > #3 0x0000000000547752 in mdcache_clean_dirent_chunk (chunk=0x7fea9f86f970) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_helpers.c:454 > #4 0x00000000005386a2 in lru_clean_chunk (chunk=0x7fea9f86f970) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:2078 > #5 0x000000000053881b in mdcache_lru_unref_chunk (chunk=0x7fea9f86f970) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:2097 > #6 0x000000000053698c in chunk_lru_run_lane (lane=14) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:1509 > #7 0x0000000000536d26 in chunk_lru_run (ctx=0x7feafe00f580) > at > /usr/src/debug/nfs-ganesha- 2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:1563 > #8 0x0000000000508ad9 in fridgethr_start_routine (arg=0x7feafe00f580) > at /usr/src/debug/nfs-ganesha-2.7.1/support/fridgethr.c:550 > #9 0x00007feb01a95e25 in start_thread () from /lib64/libpthread.so.0 > #10 0x00007feb0139dbad in clone () from /lib64/libc.so.6 > > 420 pthread_spin_lock(&parent->fsobj.fsdir.spin); > (gdb) print parent->fsobj.fsdir.spin > $1 = -1 > (gdb) print parent->obj_handle.type > $2 = REGULAR_FILE > > Looks like when the thread is in this path, the parent got reused for a > different object in the filesystem. From looking at code, this seems > possible: > > Lets say thread1 goes through the reuse path: > > * mdcache_lru_get() -> > o mdcache_lru_clean() -> > + mdc_clean_entry() -> > # mdcache_dirent_invalidate_all() > * mdcache_lru_unref_chunk() > * This will call lru_clean_chunk() only if refcnt is > zero. Lets say another thread (thread2 below) > incremented it from the background thread. So > thread1 will return back and the entry now gets reused. > > > Now the background thread (thread2) comes up to clean chunks and > increments refcnt: > > * chunk_lru_run() > o chunk_lru_run_lane() > + mdcache_lru_unref_chunk() > + Here we unlock qlane lock and then decrement refcnt. When > this thread unlocks, thread1 will grab qlane lock and skip > the chunk because refcnt is > 0. By the time we reach > mdcache_lru_unref_chunk(), the parent would have been reused > by thread1 for a different object. Now > mdcache_lru_unref_chunk() make progress as refcnt became > zero; but parent is invalid. So it gets stuck in > rmv_detached_dirent(). > > I think we should hold the qlock in chunk_lru_run_lane() till the refcnt > is decremented to make sure that reaping of parent is not possible. > Since mdcache_lru_unref_chunk() already holds the lock later, we could > probably pass a flag to indicate if the qlane lock is already held or > nor (similar to content_lock). Please let me know if there is a better > approach to refactor this. > > Thanks, > Pradeep > > > > _______________________________________________ > Devel mailing list -- devel@lists.nfs-ganesha.org <mailto:devel@lists.nfs-ganesha.org> > To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org <mailto:devel-leave@lists.nfs-ganesha.org> > _______________________________________________ Devel mailing list -- devel@lists.nfs-ganesha.org <mailto:devel@lists.nfs-ganesha.org> To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org <mailto: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

On Wed, Apr 17, 2019 at 5:50 AM Daniel Gryniewicz <dang@redhat.com> wrote:

On 4/17/19 12:49 AM, Pradeep wrote:

...

On 4/11/19, Frank Filz <ffilzlnx@mindspring.com> wrote:

...

...

From: Daniel Gryniewicz [mailto:dang@redhat.com] Sent: Thursday, April 11, 2019 5:28 AM

Okay, I misunderstood the situation. That fix won't work, because mdcache_lru_unref_chunk() takes the qlane lock, and it also takes the content lock, which must be taken before the qlane lock.

The problem here is that the parent has a pointer to the chunk and a ref on it, and the chunk has a pointer to the parent but no ref on it. This means that the parent refcount can go to zero while the chunk has a pointer. I think we need to force remove all the chunks when we recycle the parent, and null out the parent pointer in the chunk as well. We'll have to audit all the uses of the parent pointer and make sure the call path has a ref to the parent, or that it can handle a null parent pointer.

Hi Daniel,

By force cleaning, did you have something like this in mind?

https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/451311/1/src/FSAL/Stackabl...

...

I was thinking something more like this:

https://paste.fedoraproject.org/paste/2CJh-0heaj-nQQeb7Ou5KQ

Your solution may work; but it worries me to add an unbounded loop into the mix, especially since this involves multiple locks.

Note, I haven't fully audited all the callpaths to make sure that they can handle parent being NULL. I believe all the ones that can't have a ref on parent, so it should be okay.

Hi Daniel, There may still be one issue with this. The chunk->parent is accessed without any locks. So by the time the local variable ("parent") is used, chunk->parent might have been set to NULL by the reuse path. The danger is this does : (parent && lock(parent->content_lock)). If parent is reused, parent->content_lock is destroyed and reinitialized. Not sure what the behavior will be at this point. I'm wondering if we still need content_lock in mdcache_lru_unref_chunk(). Earlier it was needed to release dirents. Now that dirents are released by mdcache_dirent_invalidate_all() path before calling mdcache_lru_unref_chunk(), the parent is guaranteed to be NULL if refcnt on chunk is zero, right? Another question is do we really need chunk_lru_run_lane() to hold a ref on the chunk? It only moves the chunk between L1 to L2 with qlane lock held. So no other thread could free the chunk during this operation because those threads has to wait for qlane lock. So wondering what scenario would the ref on the chunk is needed. Similar race exists in lru_reap_chunk_impl(). At any point if mdcache_lru_unref_chunk() is called without any locks, it has the risk of parent going away. For example, say you don't get content_lock with pthread_rwlock_trywrlock(), then qlane lock is dropped and mdcache_lru_unref_chunk() is called. During this time, another thread could reuse the parent. Here also you have both qlane lock and content_lock till the chunk is out of LRU queue; so not sure if ref on chunk is needed? Thanks, Pradeep

On 4/21/19 12:34 AM, Ashish Sangwan wrote:

As to what Pradeep said, I have been testing this patch: https://paste.fedoraproject.org/paste/k4ffMzEt0VYDDLjMMNKZWg So far looking good (Not able to hit the race) Let me know if you see any issue with the patch.

This looks like it's probably okay. Daniel

On 4/20/19 11:39 AM, Pradeep wrote:

On Wed, Apr 17, 2019 at 5:50 AM Daniel Gryniewicz <dang@redhat.com <mailto:dang@redhat.com>> wrote:

On 4/17/19 12:49 AM, Pradeep wrote: > On 4/11/19, Frank Filz <ffilzlnx@mindspring.com <mailto:ffilzlnx@mindspring.com>> wrote: >>> From: Daniel Gryniewicz [mailto:dang@redhat.com <mailto:dang@redhat.com>] >>> Sent: Thursday, April 11, 2019 5:28 AM >>> >>> Okay, I misunderstood the situation.  That fix won't work, because >>> mdcache_lru_unref_chunk() takes the qlane lock, and it also takes the >>> content >>> lock, which must be taken before the qlane lock. >>> >>> The problem here is that the parent has a pointer to the chunk and  a ref >>> on it, >>> and the chunk has a pointer to the parent but no ref on it. >>> This means that the parent refcount can go to zero while the chunk has a >>> pointer.  I think we need to force remove all the chunks when we recycle >>> the >>> parent, and null out the parent pointer in the chunk as well. We'll have >>> to audit >>> all the uses of the parent pointer and make sure the call path has a ref >>> to the >>> parent, or that it can handle a null parent pointer. > > Hi Daniel, > > By force cleaning, did you have something like this in mind? > > https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/451311/1/src/FSAL/Stackabl... >

I was thinking something more like this:

https://paste.fedoraproject.org/paste/2CJh-0heaj-nQQeb7Ou5KQ

Your solution may work; but it worries me to add an unbounded loop into the mix, especially since this involves multiple locks.

Note, I haven't fully audited all the callpaths to make sure that they can handle parent being NULL.  I believe all the ones that can't have a ref on parent, so it should be okay.

Hi Daniel,

There may still be one issue with this. The chunk->parent is accessed without any locks. So by the time the local variable ("parent") is used, chunk->parent might have been set to NULL by the reuse path. The danger is this does : (parent && lock(parent->content_lock)). If parent is reused, parent->content_lock is destroyed and reinitialized. Not sure what the behavior will be at this point.

I'm wondering if we still need content_lock in mdcache_lru_unref_chunk(). Earlier it was needed to release dirents. Now that dirents are released by mdcache_dirent_invalidate_all() path before calling mdcache_lru_unref_chunk(), the parent is guaranteed to be NULL if refcnt on chunk is zero, right?

Another question is do we really need chunk_lru_run_lane() to hold a ref on the chunk? It only moves the chunk between L1 to L2 with qlane lock held. So no other thread could free the chunk during this operation because those threads has to wait for qlane lock. So wondering what scenario would the ref on the chunk is needed.

Similar race exists in lru_reap_chunk_impl(). At any point if mdcache_lru_unref_chunk() is called without any locks, it has the risk of parent going away. For example, say you don't get content_lock with pthread_rwlock_trywrlock(), then qlane lock is dropped and mdcache_lru_unref_chunk() is called. During this time, another thread could reuse the parent. Here also you have both qlane lock and content_lock till the chunk is out of LRU queue; so not sure if ref on chunk is needed?

Do Ashish's changes cover this? Or is more necessary? Daniel

On 4/22/19 12:13 PM, Pradeep wrote:

On Mon, Apr 22, 2019 at 7:18 AM Daniel Gryniewicz <dang@redhat.com <mailto:dang@redhat.com>> wrote:

On 4/20/19 11:39 AM, Pradeep wrote: > > > On Wed, Apr 17, 2019 at 5:50 AM Daniel Gryniewicz <dang@redhat.com <mailto:dang@redhat.com> > <mailto:dang@redhat.com <mailto:dang@redhat.com>>> wrote: > >     On 4/17/19 12:49 AM, Pradeep wrote: >      > On 4/11/19, Frank Filz <ffilzlnx@mindspring.com <mailto:ffilzlnx@mindspring.com> >     <mailto:ffilzlnx@mindspring.com <mailto:ffilzlnx@mindspring.com>>> wrote: >      >>> From: Daniel Gryniewicz [mailto:dang@redhat.com <mailto:dang@redhat.com> >     <mailto:dang@redhat.com <mailto:dang@redhat.com>>] >      >>> Sent: Thursday, April 11, 2019 5:28 AM >      >>> >      >>> Okay, I misunderstood the situation.  That fix won't work, because >      >>> mdcache_lru_unref_chunk() takes the qlane lock, and it also >     takes the >      >>> content >      >>> lock, which must be taken before the qlane lock. >      >>> >      >>> The problem here is that the parent has a pointer to the chunk >     and  a ref >      >>> on it, >      >>> and the chunk has a pointer to the parent but no ref on it. >      >>> This means that the parent refcount can go to zero while the >     chunk has a >      >>> pointer.  I think we need to force remove all the chunks when >     we recycle >      >>> the >      >>> parent, and null out the parent pointer in the chunk as well. >     We'll have >      >>> to audit >      >>> all the uses of the parent pointer and make sure the call path >     has a ref >      >>> to the >      >>> parent, or that it can handle a null parent pointer. >      > >      > Hi Daniel, >      > >      > By force cleaning, did you have something like this in mind? >      > >      > > https://review.gerrithub.io/c/ffilz/nfs-ganesha/+/451311/1/src/FSAL/Stackabl... >      > > >     I was thinking something more like this: > > https://paste.fedoraproject.org/paste/2CJh-0heaj-nQQeb7Ou5KQ > >     Your solution may work; but it worries me to add an unbounded loop into >     the mix, especially since this involves multiple locks. > >     Note, I haven't fully audited all the callpaths to make sure that they >     can handle parent being NULL.  I believe all the ones that can't have a >     ref on parent, so it should be okay. > > > Hi Daniel, > > There may still be one issue with this. The chunk->parent is accessed > without any locks. So by the time the local variable ("parent") is used, > chunk->parent might have been set to NULL by the reuse path. The danger > is this does : (parent && lock(parent->content_lock)). If parent is > reused, parent->content_lock is destroyed and reinitialized. Not sure > what the behavior will be at this point. > > I'm wondering if we still need content_lock in > mdcache_lru_unref_chunk(). Earlier it was needed to release dirents. Now > that dirents are released by mdcache_dirent_invalidate_all() path before > calling mdcache_lru_unref_chunk(), the parent is guaranteed to be NULL > if refcnt on chunk is zero, right? > > Another question is do we really need chunk_lru_run_lane() to hold a ref > on the chunk? It only moves the chunk between L1 to L2 with qlane lock > held. So no other thread could free the chunk during this operation > because those threads has to wait for qlane lock. So wondering what > scenario would the ref on the chunk is needed. > > Similar race exists in lru_reap_chunk_impl(). At any point if > mdcache_lru_unref_chunk() is called without any locks, it has the risk > of parent going away. For example, say you don't get content_lock with > pthread_rwlock_trywrlock(), then qlane lock is dropped and > mdcache_lru_unref_chunk() is called. During this time, another thread > could reuse the parent. Here also you have both qlane lock and > content_lock till the chunk is out of LRU queue; so not sure if ref on > chunk is needed? >

Do Ashish's changes cover this?  Or is more necessary?

Thanks Daniel/Ashish. With this patch, do we need the previous patch (now that all calls to mdcache_lru_unref_chunk() already has content_lock)?

https://review.gerrithub.io/#/c/ffilz/nfs-ganesha/+/447484/

No, although it's not causing any problems. Daniel

On Thu, Apr 11, 2019 at 9:36 AM Soumya Koduri <skoduri@redhat.com> wrote:

Hi Pradeep,

Could you please share some details about your workload and in what scenarios you have hit this race? Are there any cache tuning parameters modified?

Hi Soumya,

I don't have a reproducer for this yet. The customer environment where this is hit is using NFS for storing logs from 100s of clients and few clients doing cleanup of old logs (find + rm using mtime) in parallel. This is with default ganesha parameters. From lru_state, I noticed we were over the default limit of 100K entries in MDCACHE (that is why the reap code path is triggered). This is with version 2.7.1 on a posix filesystem. Thanks, Pradeep

Thanks, Soumya

On 4/10/19 11:25 PM, Pradeep wrote:

...

Hello,

I'm hitting hang where rmv_detached_dirent() is stuck at the spinlock for ever.

#0 0x00007feb01a9a4e5 in pthread_spin_lock () from /lib64/libpthread.so.0 #1 0x00000000005519e7 in rmv_detached_dirent (parent=0x7feaeb8dd600, dirent=0x7fea9fbd8700) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_int.h:420

...

#2 0x00000000005522e8 in mdcache_avl_remove (parent=0x7feaeb8dd600, dirent=0x7fea9fbd8700) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_avl.c:256

...

#3 0x0000000000547752 in mdcache_clean_dirent_chunk (chunk=0x7fea9f86f970) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_helpers.c:454

...

#4 0x00000000005386a2 in lru_clean_chunk (chunk=0x7fea9f86f970) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:2078

...

#5 0x000000000053881b in mdcache_lru_unref_chunk (chunk=0x7fea9f86f970) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:2097

...

#6 0x000000000053698c in chunk_lru_run_lane (lane=14) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:1509

...

#7 0x0000000000536d26 in chunk_lru_run (ctx=0x7feafe00f580) at

/usr/src/debug/nfs-ganesha-2.7.1/FSAL/Stackable_FSALs/FSAL_MDCACHE/mdcache_lru.c:1563

...

#8 0x0000000000508ad9 in fridgethr_start_routine (arg=0x7feafe00f580) at /usr/src/debug/nfs-ganesha-2.7.1/support/fridgethr.c:550 #9 0x00007feb01a95e25 in start_thread () from /lib64/libpthread.so.0 #10 0x00007feb0139dbad in clone () from /lib64/libc.so.6

420 pthread_spin_lock(&parent->fsobj.fsdir.spin); (gdb) print parent->fsobj.fsdir.spin $1 = -1 (gdb) print parent->obj_handle.type $2 = REGULAR_FILE

Looks like when the thread is in this path, the parent got reused for a different object in the filesystem. From looking at code, this seems possible:

Lets say thread1 goes through the reuse path:

* mdcache_lru_get() -> o mdcache_lru_clean() -> + mdc_clean_entry() -> # mdcache_dirent_invalidate_all() * mdcache_lru_unref_chunk() * This will call lru_clean_chunk() only if refcnt is zero. Lets say another thread (thread2 below) incremented it from the background thread. So thread1 will return back and the entry now gets reused.

Now the background thread (thread2) comes up to clean chunks and increments refcnt:

* chunk_lru_run() o chunk_lru_run_lane() + mdcache_lru_unref_chunk() + Here we unlock qlane lock and then decrement refcnt. When this thread unlocks, thread1 will grab qlane lock and skip the chunk because refcnt is > 0. By the time we reach mdcache_lru_unref_chunk(), the parent would have been reused by thread1 for a different object. Now mdcache_lru_unref_chunk() make progress as refcnt became zero; but parent is invalid. So it gets stuck in rmv_detached_dirent().

I think we should hold the qlock in chunk_lru_run_lane() till the refcnt is decremented to make sure that reaping of parent is not possible. Since mdcache_lru_unref_chunk() already holds the lock later, we could probably pass a flag to indicate if the qlane lock is already held or nor (similar to content_lock). Please let me know if there is a better approach to refactor this.

Thanks, Pradeep

_______________________________________________ Devel mailing list -- devel@lists.nfs-ganesha.org To unsubscribe send an email to devel-leave@lists.nfs-ganesha.org