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
