linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: NeilBrown <neilb-IBi9RG/b67k@public.gmane.org>
To: Kinglong Mee
	<kinglongmee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>,
	Al Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>
Cc: "J. Bruce Fields"
	<bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>,
	"linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org"
	<linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org>,
	linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
	Trond Myklebust
	<trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org>
Subject: [PATCH] fs-pin: allow pin_remove() to be called other than from ->kill()
Date: Wed, 29 Jul 2015 13:59:14 +1000	[thread overview]
Message-ID: <20150729135914.13cb0f86@noble> (raw)
In-Reply-To: <55B5A186.7040004-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>



fs-pin currently assumes when either the vfsmount or the fs_pin wants
to unpin, pin_kill() will be called.
This requires that the ->kill() function can wait for any transient
references to the fs_pin to be released.  If the structure containing
the fs_pin doesn't already have the ability to wait for references,
this can be a burden.

As the fs_pin already has infrastructure for waiting, that can be
leveraged to remove the burden.

In this alternate scenario, only the vfsmount calls pin_kill() when it
wants to unpin.  The owner of the fs_pin() instead calls pin_remove().

The ->kill() function removes any long-term references, and then calls
pin_kill() (recursively).
When the last reference on (the structure containing) the fs_pin is
dropped, pin_remove() will be called and the (recursive) pin_kill()
call will complete.

For this to be safe, the final "put" must *not* free the structure if
pin_kill() has already been called, as that could leave ->kill()
accessing freed data.

So we provide a return value for pin_remove() which reports the old
->done value.

When final put calls pin_remove() it checks that value.
If it was 0, then pin_kill() has not called ->kill and will not,
so final put can free the data structure.
If it was -1, then pin_kill() has called ->kill, and ->kill will
free the data structure - final put must not touch it.

This makes the 'wait' infrastructure of fs_pin available to any
pinning client which wants to use it.

Signed-Off-By: NeilBrown <neilb-IBi9RG/b67k@public.gmane.org>

---
Hi Al,
 do you see this as a workable solution?  I think it will improve the nfsd pining patch
a lot.

Thanks,
NeilBrown


diff --git a/fs/fs_pin.c b/fs/fs_pin.c
index 611b5408f6ec..b7954a9d17da 100644
--- a/fs/fs_pin.c
+++ b/fs/fs_pin.c
@@ -6,16 +6,32 @@
 
 static DEFINE_SPINLOCK(pin_lock);
 
-void pin_remove(struct fs_pin *pin)
+/**
+ * pin_remove - disconnect an fs_pin from the pinned structure.
+ * @pin:	The struct fs_pin which is pinning something.
+ *
+ * Detach a 'pin' which was added by pin_insert().  A return value
+ * of -1 implies that pin_kill() has already been called and that the
+ * ->kill() function now owns the data structure containing @pin.
+ * The function which called pin_remove() must not touch the data structure
+ * again (unless it is the ->kill() function itself).
+ * A return value of 0 implies an uneventful disconnect: pin_kill() has not called,
+ * and will not call, the ->kill() function on this @pin.
+ * Any other return value is a usage error - e.g. repeated call to pin_remove().
+ */
+int pin_remove(struct fs_pin *pin)
 {
+	int ret;
 	spin_lock(&pin_lock);
 	hlist_del_init(&pin->m_list);
 	hlist_del_init(&pin->s_list);
 	spin_unlock(&pin_lock);
 	spin_lock_irq(&pin->wait.lock);
+	ret = pin->done;
 	pin->done = 1;
 	wake_up_locked(&pin->wait);
 	spin_unlock_irq(&pin->wait.lock);
+	return ret;
 }
 
 void pin_insert_group(struct fs_pin *pin, struct vfsmount *m, struct hlist_head *p)
diff --git a/include/linux/fs_pin.h b/include/linux/fs_pin.h
index 3886b3bffd7f..2fe9d3ba09e8 100644
--- a/include/linux/fs_pin.h
+++ b/include/linux/fs_pin.h
@@ -18,7 +18,7 @@ static inline void init_fs_pin(struct fs_pin *p, void (*kill)(struct fs_pin *))
 	p->kill = kill;
 }
 
-void pin_remove(struct fs_pin *);
+int pin_remove(struct fs_pin *);
 void pin_insert_group(struct fs_pin *, struct vfsmount *, struct hlist_head *);
 void pin_insert(struct fs_pin *, struct vfsmount *);
 void pin_kill(struct fs_pin *);
--
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

  parent reply	other threads:[~2015-07-29  3:59 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-07-27  3:05 [PATCH 0/9 v8] NFSD: Pin to vfsmount for nfsd exports cache Kinglong Mee
     [not found] ` <55B5A012.1030006-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-07-27  3:06   ` [PATCH 1/9 v8] fs_pin: Initialize value for fs_pin explicitly Kinglong Mee
2015-07-29  0:25     ` NeilBrown
2015-07-29 19:41       ` J. Bruce Fields
     [not found]         ` <20150729194155.GC21949-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2015-07-29 21:48           ` NeilBrown
2015-07-30  0:36             ` J. Bruce Fields
2015-07-30 12:28               ` Kinglong Mee
2015-07-27  3:07   ` [PATCH 2/9 v8] fs_pin: Export functions for specific filesystem Kinglong Mee
2015-07-29  0:30     ` NeilBrown
2015-07-30 12:31       ` Kinglong Mee
2015-07-27  3:07   ` [PATCH 3/9 v8] path: New helpers path_get_pin/path_put_unpin for path pin Kinglong Mee
2015-07-27  3:09   ` [PATCH 6/9 v8] sunrpc/nfsd: Remove redundant code by exports seq_operations functions Kinglong Mee
2015-07-29  2:15     ` NeilBrown
2015-07-27  3:12   ` [PATCH 9/9 v8] nfsd: Allows user un-mounting filesystem where nfsd exports base on Kinglong Mee
     [not found]     ` <55B5A186.7040004-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-07-29  3:56       ` NeilBrown
2015-07-30 13:30         ` Kinglong Mee
2015-07-29  3:59       ` NeilBrown [this message]
2015-08-10 11:37         ` [PATCH] fs-pin: allow pin_remove() to be called other than from ->kill() Kinglong Mee
2015-08-18  6:07         ` Kinglong Mee
2015-08-18  6:21           ` NeilBrown
2015-08-18  6:37             ` Kinglong Mee
2015-07-27  3:08 ` [PATCH 4/9 v8] fs: New helper legitimize_mntget() for getting a legitimize mnt Kinglong Mee
     [not found]   ` <55B5A0B0.7060604-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-07-29  2:06     ` NeilBrown
2015-07-30 13:17       ` Kinglong Mee
2015-07-27  3:09 ` [PATCH 5/9 v8] sunrpc: Store cache_detail in seq_file's private directly Kinglong Mee
2015-07-29  2:11   ` NeilBrown
2015-07-27  3:10 ` [PATCH 7/9 v8] sunrpc: Switch to using hash list instead single list Kinglong Mee
2015-07-29  2:19   ` NeilBrown
2015-07-29 19:51     ` J. Bruce Fields
     [not found]       ` <20150729195151.GD21949-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org>
2015-07-30 13:01         ` Kinglong Mee
2015-07-27  3:10 ` [PATCH 8/9 v8] sunrpc: New helper cache_delete_entry for deleting cache_head directly Kinglong Mee
     [not found]   ` <55B5A135.9050800-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
2015-07-29  2:29     ` NeilBrown
2015-07-30 13:14       ` 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=20150729135914.13cb0f86@noble \
    --to=neilb-ibi9rg/b67k@public.gmane.org \
    --cc=bfields-uC3wQj2KruNg9hUCZPvPmw@public.gmane.org \
    --cc=kinglongmee-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org \
    --cc=linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org \
    --cc=trond.myklebust-7I+n7zu2hftEKMMhf/gKZA@public.gmane.org \
    --cc=viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org \
    /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).