Re: [PATCH 4/8] nfsd4: extend the client_lock to cover cl_lru

[" J. Bruce Fields" <bfields@citi.umich.edu>]

On Wed, May 05, 2010 at 01:44:17AM +0300, Benny Halevy wrote:
> @@ -2557,6 +2562,7 @@ nfs4_laundromat(void)
>  	dprintk("NFSD: laundromat service - starting\n");
>  	if (locks_in_grace())
>  		nfsd4_end_grace();
> +	spin_lock(&client_lock);
>  	list_for_each_safe(pos, next, &client_lru) {
>  		clp = list_entry(pos, struct nfs4_client, cl_lru);
>  		if (time_after((unsigned long)clp->cl_time, (unsigned long)cutoff)) {
> @@ -2565,11 +2571,15 @@ nfs4_laundromat(void)
>  				clientid_val = t;
>  			break;
>  		}
> +		list_del_init(&clp->cl_lru);
> +		spin_unlock(&client_lock);
>  		dprintk("NFSD: purging unused client (clientid %08x)\n",
>  			clp->cl_clientid.cl_id);
>  		nfsd4_remove_clid_dir(clp);
>  		expire_client(clp);
> +		spin_lock(&client_lock);
>  	}
> +	spin_unlock(&client_lock);
>  	INIT_LIST_HEAD(&reaplist);
>  	spin_lock(&recall_lock);
>  	list_for_each_safe(pos, next, &del_recall_lru) {

Careful--the list_for_each_safe() isn't enough to handle the results of
concurrent modifications that might occur while the client_lock is
dropped.

Maybe just use the trick of moving everything to a temporary list, then
doing the real work on that list after dropping the lock?

--b.