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. -- To unsubscribe from this list: send the line "unsubscribe linux-nfs" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
