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

J. Bruce Fields
2010-05-07T22:29:27Z

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.