linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Al Viro <viro@ZenIV.linux.org.uk>
To: Kinglong Mee <kinglongmee@gmail.com>
Cc: "J. Bruce Fields" <bfields@fieldses.org>,
	"linux-nfs@vger.kernel.org" <linux-nfs@vger.kernel.org>,
	linux-fsdevel@vger.kernel.org, NeilBrown <neilb@suse.de>,
	Trond Myklebust <trond.myklebust@primarydata.com>
Subject: Re: [PATCH 6/6 v5] nfsd: Allows user un-mounting filesystem where nfsd exports base on
Date: Fri, 19 Jun 2015 22:09:56 +0100	[thread overview]
Message-ID: <20150619210956.GN17109@ZenIV.linux.org.uk> (raw)
In-Reply-To: <55812768.1080908@gmail.com>

On Wed, Jun 17, 2015 at 03:53:12PM +0800, Kinglong Mee 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();
> +		complete(&key->ek_done);
> +		pin_kill(&key->ek_pin);
> +	} else
> +		expkey_destroy(key);
>  }

> +static void expkey_pin_kill(struct fs_pin *pin)
> +{
> +	struct svc_expkey *key = container_of(pin, struct svc_expkey, ek_pin);
> +
> +	if (!completion_done(&key->ek_done)) {
> +		schedule_work(&key->ek_work);
> +		wait_for_completion(&key->ek_done);
> +	}
> +	path_put_unpin(&key->ek_path, &key->ek_pin);
> +	expkey_destroy(key);
> +}

So basically you want umount(2) to hang until all references are gone.
How long can they stick around and, more to the point, what happens
if some sucker does path_get(&key->ek_path) while umount(2) had been
waiting?  You'll drop the reference to svc_export you'd copied ->ek_path
from and get through the whole dance, letting umount(2) go.

Now what?  Sure, with your previous patch vfsmount will survive - no oopsen
there.  However, you have had umount run to completion, with filesystem
still not shut down.  This is badly broken.

You can't do "I can grab reference at any time until the call of
expkey_pin_kill()" - it's not going to work.  You _must_ grab it
carefully, being ready to cope with "umount has already decided it's
not busy, so it's a goner and we must fail" kind of situations.

You need to
	grab mount_lock with read_seqlock_excl()
	check that vfsmount isn't marked MNT_SYNC_UMOUNT or MNT_DOOMED
and bump refcount in such case; give up on attempt otherwise.
	drop mount_lock
for those attempts to grab the first reference.  And be ready to cope with
failures.
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in

  reply	other threads:[~2015-06-19 21:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-06-17  7:49 [PATCH 0/6 v5] NFSD: Pin to vfsmount for nfsd exports cache Kinglong Mee
     [not found] ` <5581266C.9080404-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-06-17  7:49   ` [PATCH 1/6 v5] fs_pin: Initialize value for fs_pin explicitly Kinglong Mee
2015-06-17  7:50   ` [PATCH 2/6 v5] fs_pin: Export functions for specific filesystem Kinglong Mee
2015-06-17  7:51   ` [PATCH 3/6 v5] fs_pin: Kill fs_pin under a reference of vfsmnt Kinglong Mee
     [not found]     ` <558126FA.8050608-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-06-19 20:29       ` J. Bruce Fields
2015-06-25 14:14         ` Kinglong Mee
2015-06-19 20:44       ` Al Viro
2015-06-17  7:52   ` [PATCH 4/6 v5] path: New helpers path_get_pin/path_put_unpin for path pin Kinglong Mee
2015-06-17  7:52   ` [PATCH 5/6 v5] sunrpc: New helper cache_force_expire for cache cleanup Kinglong Mee
2015-06-17  7:53   ` [PATCH 6/6 v5] nfsd: Allows user un-mounting filesystem where nfsd exports base on Kinglong Mee
2015-06-19 21:09     ` Al Viro [this message]
2015-06-25 14:46       ` Kinglong Mee

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20150619210956.GN17109@ZenIV.linux.org.uk \
    --to=viro@zeniv.linux.org.uk \
    --cc=bfields@fieldses.org \
    --cc=kinglongmee@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=neilb@suse.de \
    --cc=trond.myklebust@primarydata.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).