Re: [RFC,PATCH 04/20] svc: xpt_has_wspace

[Tom Tucker <tom@opengridcomputing.com>]

On 8/29/07 12:35 PM, "J. Bruce Fields" <bfields@fieldses.org> wrote:

> On Mon, Aug 20, 2007 at 11:23:30AM -0500, Tom Tucker wrote:
>> @@ -269,21 +253,13 @@ svc_sock_enqueue(struct svc_sock *svsk)
>> BUG_ON(svsk->sk_pool != NULL);
>> svsk->sk_pool = pool;
>>  
>> - set_bit(SOCK_NOSPACE, &svsk->sk_sock->flags);
>> - if (((atomic_read(&svsk->sk_reserved) + serv->sv_max_mesg)*2
>> -      > svc_sock_wspace(svsk))
>> -     && !test_bit(SK_CLOSE, &svsk->sk_flags)
>> -     && !test_bit(SK_CONN, &svsk->sk_flags)) {
>> -  /* Don't enqueue while not enough space for reply */
>> -  dprintk("svc: socket %p  no space, %d*2 > %ld, not enqueued\n",
>> -   svsk->sk_sk, atomic_read(&svsk->sk_reserved)+serv->sv_max_mesg,
>> -   svc_sock_wspace(svsk));
>> + if (!test_bit(SK_CLOSE, &svsk->sk_flags)
>> +     && !test_bit(SK_CONN, &svsk->sk_flags)
>> +     && !svsk->sk_xprt->xpt_has_wspace(svsk)) {
>> svsk->sk_pool = NULL;
>> clear_bit(SK_BUSY, &svsk->sk_flags);
>> goto out_unlock;
>> }
>> - clear_bit(SOCK_NOSPACE, &svsk->sk_sock->flags);
>> -
> 
> The reordering of the set_bit and the checks makes this patch
> non-trivial (unlike the previous ones).
> 
> Where possible it's helpful to split patches into those that are
> entirely "trivial"--e.g. that just move some code around--and those that
> might actually change behavior, and need a little more explanation to
> demonstrate that they're correct.  (And in this case the explanation may
> be very simple--apologies, I don't know the svcsock.c code well.)
> 

This is just bad. I'll create two has_wspace functions, keeping the setting
and clearing of NOSPACE in the same function.

I'll confirm the testing of SK_CLOSE and SK_CONN match the current code as
well.

> --b.



-------------------------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc.
Still grepping through log files to find problems?  Stop.
Now Search log events and configuration files using AJAX and a browser.
Download your FREE copy of Splunk now >>  http://get.splunk.com/
_______________________________________________
NFS maillist  -  NFS@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/nfs