Re: [PATCH 6/6 v9] nfsd: Allows user un-mounting filesystem where nfsd exports base on

[NeilBrown <neilb-IBi9RG/b67k@public.gmane.org>]

On Tue, 18 Aug 2015 15:23:59 +0800 Kinglong Mee <kinglongmee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
wrote:

>  static void expkey_put(struct kref *ref)
>  {
>  	struct svc_expkey *key = container_of(ref, struct svc_expkey, h.ref);
>  
>  	if (test_bit(CACHE_VALID, &key->h.flags) &&
> -	    !test_bit(CACHE_NEGATIVE, &key->h.flags))
> -		path_put(&key->ek_path);
> -	auth_domain_put(key->ek_client);
> -	kfree(key);
> +	    !test_bit(CACHE_NEGATIVE, &key->h.flags)) {
> +		rcu_read_lock();
> +		if (path_put_unpin(&key->ek_path, &key->ek_pin))
> +			return ;
> +	}

That rcu_read_lock() is unbalanced and not needed.

Comment applies below in svc_export code too.


> +static void expkey_pin_kill(struct fs_pin *pin)
> +{
> +	struct svc_expkey *key = container_of(pin, struct svc_expkey, ek_pin);
> +	cache_delete_entry(key->cd, &key->h);
> +	/* Must call pin_kill to wait the last reference be put */
> +	pin_kill(&key->ek_pin);
> +	expkey_destroy(key);
>  }

This, on the other hand, needs rcu_read_lock().  pin_kill() expected
rcu to be readlocked, and will drop the lock.

Same comment for svc_export code.


Did you test with lockdep enabled?  That should have caught these
issues.

Otherwise it looks good.

Reviewed-by: NeilBrown <neilb-IBi9RG/b67k@public.gmane.org>

Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html