On Tue, 18 Aug 2015 15:23:59 +0800 Kinglong Mee <kinglongmee@xxxxxxxxx>
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@xxxxxxxx>
Thanks,
NeilBrown
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
