Re: [PATCH 0/3] [RFC] knfsd: convert to kthread API and remove signaling for shutdown

[Greg Banks <gnb-cP1dWloDopni96+mSzHFpQC/G2K4zDHf@public.gmane.org>]

Neil Brown wrote:
>
> Fair question.
> The new nfsd_mutex primarily protects "nfsd_serv", both the pointer
> itself and various members of the structure that it sometimes points
> to.
> In particular, ->sv_nrthreads but also to some extent sv_temp_socks
> and sv_permsocks.
>   
Those two fields should be guarded by svc_serv->sv_lock only.  In fact
IIRC the only field of svc_serv guarded by the global mutex is
sv_nrthreads in it's role as pseudo-refcount.

> Having said all that, I think I see a race.
> When a new rqst is created at put on sp_all_threads, ->rq_task it not
> set and doesn't get set until the thread runs and gets the mutex.  So
> there is a brief hole when ->rq_task isn't set.  I don't know if that
> can cause a problem, but it feels wrong.
> When you switch to kthreads, you use the fact that kthread_create
> returns a task_struct, and assign that to ->rq_task in
> __svc_create_thread instead of nfsd, which will close that hole.
>   
Yes, that aspect of Jeff's patch is a definite improvement.
> If you look in nfsctl.c, you will probably be able to find plenty of
> places where nfsd_serv is dereferenced without any locking.  These are
> all wrong and need fixing.
>   
Agreed!

-- 
Greg Banks, P.Engineer, SGI Australian Software Group.
The cake is *not* a lie.
I don't speak for SGI.