Re: [RFC PATCH] SUNRPC: protect transport processing with rw sem

[Stanislav Kinsbursky <skinsbursky@parallels.com>]

30.01.2013 02:57, J. Bruce Fields пишет:
> On Tue, Jan 29, 2013 at 02:03:30PM +0300, Stanislav Kinsbursky wrote:
>> There could be a service transport, which is processed by service thread and
>> racing in the same time with per-net service shutdown like listed below:
>>
>> CPU#0:                            CPU#1:
>>
>> svc_recv                        svc_close_net
>> svc_get_next_xprt (list_del_init(xpt_ready))
>>                              svc_close_list (set XPT_BUSY and XPT_CLOSE)
>>                              svc_clear_pools(xprt was gained on CPU#0 already)
>>                              svc_delete_xprt (set XPT_DEAD)
>> svc_handle_xprt (is XPT_CLOSE => svc_delete_xprt()
>> BUG()
>>
>> There could be different solutions of the problem.
>> Probably, the patch doesn't implement the best one, but I hope the simple one.
>> IOW, it protects critical section (dequeuing of pending transport and
>> enqueuing  it back to the pool) by per-service rw semaphore,
>
> It's actually per-thread (per-struct svc_rqst) here.
>

Yes, sure.

>> taken for read.
>> On per-net transports shutdown, this semaphore have to be taken for write.
>
> There's no down_write in this patch.  Did you forget this part?
>

See "fs/nfs/callback.c" part

> The server rpc code goes to some care not to write to any global
> structure, to prevent server threads running on multiple cores from
> bouncing cache lines between them.
>

This is just an idea. I.e. I wasn't trying to polish the patch - just to share the vision.

> But my understanding is that even down_read() does modify the semaphore.
> So we might want something like the percpu semaphore describe in
> Documentation/percpu-rw-semaphore.txt.
>

Sure, I'll have a look.


-- 
Best regards,
Stanislav Kinsbursky