On Tue, Dec 11, 2007 at 05:32:17PM -0600, Tom Tucker wrote:
> @@ -1053,11 +1060,10 @@ svc_tcp_accept(struct svc_sock *svsk)
> else if (err != -EAGAIN && net_ratelimit())
> printk(KERN_WARNING "%s: accept failed (err %d)!\n",
> serv->sv_name, -err);
> - return;
> + return NULL;
> }
>
> set_bit(SK_CONN, &svsk->sk_flags);
> - svc_sock_enqueue(svsk);
>
> err = kernel_getpeername(newsock, sin, &slen);
> if (err < 0) {
Why did we need that svc_sock_enqueue here, and why don't we any more?
(And if we don't, but we still need the set_bit, then we need to fix the
comment at the top of the file claiming the svc_sock_enqueue() is always
required after setting SK_CONN.)
--b.
On 12/14/07 1:55 PM, "J. Bruce Fields" <[email protected]> wrote:
> On Tue, Dec 11, 2007 at 05:32:17PM -0600, Tom Tucker wrote:
>> @@ -1053,11 +1060,10 @@ svc_tcp_accept(struct svc_sock *svsk)
>> else if (err != -EAGAIN && net_ratelimit())
>> printk(KERN_WARNING "%s: accept failed (err %d)!\n",
>> serv->sv_name, -err);
>> - return;
>> + return NULL;
>> }
>>
>> set_bit(SK_CONN, &svsk->sk_flags);
>> - svc_sock_enqueue(svsk);
>>
>> err = kernel_getpeername(newsock, sin, &slen);
>> if (err < 0) {
>
> Why did we need that svc_sock_enqueue here, and why don't we any more?
Calling svc_sock_enqueue while holding the BUSY bit is a no-op. We can
certainly break this out and update the comment.
> (And if we don't, but we still need the set_bit, then we need to fix the
> comment at the top of the file claiming the svc_sock_enqueue() is always
> required after setting SK_CONN.)
We need to call svc_sock_enqueue, but it is done indirectly through
svc_sock_received -- that calls svc_sock_enqueue after clearing the BUSY
bit.
>
> --b.
On 12/14/07 1:55 PM, "J. Bruce Fields" <[email protected]> wrote:
> On Tue, Dec 11, 2007 at 05:32:17PM -0600, Tom Tucker wrote:
>> @@ -1053,11 +1060,10 @@ svc_tcp_accept(struct svc_sock *svsk)
>> else if (err != -EAGAIN && net_ratelimit())
>> printk(KERN_WARNING "%s: accept failed (err %d)!\n",
>> serv->sv_name, -err);
>> - return;
>> + return NULL;
>> }
>>
>> set_bit(SK_CONN, &svsk->sk_flags);
>> - svc_sock_enqueue(svsk);
>>
>> err = kernel_getpeername(newsock, sin, &slen);
>> if (err < 0) {
>
> Why did we need that svc_sock_enqueue here, and why don't we any more?
> (And if we don't, but we still need the set_bit, then we need to fix the
> comment at the top of the file claiming the svc_sock_enqueue() is always
> required after setting SK_CONN.)
I moved this to it's own patch with a description of why it was removed.
>
> --b.
> -
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html