Thank you for the quick reply. In dupreq_finish, as part of retiring the drc quite a few locks are acquired and dropped (per entry). I want to fix a bug where drc retire will happen as part of a different function (this will be called from free_expired). The existing logic gets carried over to the new function and I was thinking that we may not have to acquire and release lock so many times.
Thanks, Satya. On Thu, May 4, 2017 at 1:21 AM, Matt Benjamin <mbenja...@redhat.com> wrote: > Hi Satya, > > Sorry, my recommendation would be, we do not change locking to be more coarse > grained, and in general, should update it in response to an indication that > it is incorrect, not to improve readability in the first instance. > > Regards, > > Matt > > ----- Original Message ----- >> From: "Matt Benjamin" <mbenja...@redhat.com> >> To: "Satya Prakash GS" <g.satyaprak...@gmail.com> >> Cc: nfs-ganesha-devel@lists.sourceforge.net, "Malahal Naineni" >> <mala...@gmail.com> >> Sent: Wednesday, May 3, 2017 3:43:06 PM >> Subject: Re: [Nfs-ganesha-devel] reg. drc nested locks >> >> No? >> >> Matt >> >> ----- Original Message ----- >> > From: "Satya Prakash GS" <g.satyaprak...@gmail.com> >> > To: nfs-ganesha-devel@lists.sourceforge.net, "Malahal Naineni" >> > <mala...@gmail.com> >> > Sent: Wednesday, May 3, 2017 3:34:31 PM >> > Subject: [Nfs-ganesha-devel] reg. drc nested locks >> > >> > Hi, >> > >> > In nfs_dupreq_start and nfs_dupreq_finish when allocating/freeing a >> > dupreq_entry we are trying hard to keep both dupreq_q and the rbtree >> > in sync acquiring both the partition lock and the drc (t->mtx, >> > drc->mtx). This requires dropping and reacquiring locks at certain >> > places. Can these nested locks be changed to take locks one after the >> > other. >> > >> > For example at the time of allocation, we could choose to do this - >> > >> > PTHREAD_MUTEX_lock(&t->mtx); /* partition lock */ >> > nv = rbtree_x_cached_lookup(&drc->xt, t, &dk->rbt_k, dk->hk); >> > if (!nv) { >> > dk->refcnt = 2; >> > (void)rbtree_x_cached_insert(&drc->xt, t, >> > &dk->rbt_k, dk->hk); >> > PTHREAD_MUTEX_unlock(&t->mtx); /* partition lock */ >> > >> > PTHREAD_MUTEX_lock(&drc->mtx); >> > TAILQ_INSERT_TAIL(&drc->dupreq_q, dk, fifo_q); >> > ++(drc->size); >> > PTHREAD_MUTEX_unlock(&drc->mtx); >> > } >> > >> > I am assuming this would simplify the lock code a lot. >> > If there is a case where this would introduce a race please let me know. >> > >> > Thanks, >> > Satya. >> > >> > ------------------------------------------------------------------------------ >> > Check out the vibrant tech community on one of the world's most >> > engaging tech sites, Slashdot.org! http://sdm.link/slashdot >> > _______________________________________________ >> > Nfs-ganesha-devel mailing list >> > Nfs-ganesha-devel@lists.sourceforge.net >> > https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel >> > >> >> -- >> Matt Benjamin >> Red Hat, Inc. >> 315 West Huron Street, Suite 140A >> Ann Arbor, Michigan 48103 >> >> http://www.redhat.com/en/technologies/storage >> >> tel. 734-821-5101 >> fax. 734-769-8938 >> cel. 734-216-5309 >> > > -- > Matt Benjamin > Red Hat, Inc. > 315 West Huron Street, Suite 140A > Ann Arbor, Michigan 48103 > > http://www.redhat.com/en/technologies/storage > > tel. 734-821-5101 > fax. 734-769-8938 > cel. 734-216-5309 ------------------------------------------------------------------------------ Check out the vibrant tech community on one of the world's most engaging tech sites, Slashdot.org! http://sdm.link/slashdot _______________________________________________ Nfs-ganesha-devel mailing list Nfs-ganesha-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/nfs-ganesha-devel