From: Benny Halevy <bhalevy@panasas.com>
To: Alexandros Batsakis <batsakis@netapp.com>
Cc: linux-nfs@vger.kernel.org
Subject: Re: [PATCH 2/8] pnfs-submit: clean locking infrastructure
Date: Wed, 26 May 2010 11:28:35 +0300 [thread overview]
Message-ID: <4BFCDBB3.7010406@panasas.com> (raw)
In-Reply-To: <1274119004-30213-3-git-send-email-batsakis@netapp.com>
On May. 17, 2010, 20:56 +0300, Alexandros Batsakis <batsakis@netapp.com> wrote:
> (also minor cleanup of pnfs_free_layout())
>
> Signed-off-by: Alexandros Batsakis <batsakis@netapp.com>
>
> Conflicts:
>
> fs/nfs/pnfs.c
> ---
> fs/nfs/pnfs.c | 80 +++++++++++++++++++++++++++++++++++++-------------------
> 1 files changed, 53 insertions(+), 27 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index b72c013..74cb998 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1,4 +1,4 @@
> -/*
> + /*
just picking nit...
Benny
> * linux/fs/nfs/pnfs.c
> *
> * pNFS functions to call and manage layout drivers.
> @@ -60,6 +60,8 @@ static int pnfs_initialized;
> static void pnfs_free_layout(struct pnfs_layout_type *lo,
> struct nfs4_pnfs_layout_segment *range);
> static enum pnfs_try_status pnfs_commit(struct nfs_write_data *data, int sync);
> +static inline void lock_current_layout(struct nfs_inode *nfsi);
> +static inline void unlock_current_layout(struct nfs_inode *nfsi);
>
> /* Locking:
> *
> @@ -153,16 +155,17 @@ pnfs_need_layoutcommit(struct nfs_inode *nfsi, struct nfs_open_context *ctx)
> {
> dprintk("%s: has_layout=%d layoutcommit_ctx=%p ctx=%p\n", __func__,
> has_layout(nfsi), nfsi->layout.layoutcommit_ctx, ctx);
> - spin_lock(&nfsi->lo_lock);
> +
> + lock_current_layout(nfsi);
> if (has_layout(nfsi) && !nfsi->layout.layoutcommit_ctx) {
> nfsi->layout.layoutcommit_ctx = get_nfs_open_context(ctx);
> nfsi->change_attr++;
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> dprintk("%s: Set layoutcommit_ctx=%p\n", __func__,
> nfsi->layout.layoutcommit_ctx);
> return;
> }
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> }
>
> /* Update last_write_offset for layoutcommit.
> @@ -175,7 +178,7 @@ pnfs_update_last_write(struct nfs_inode *nfsi, loff_t offset, size_t extent)
> {
> loff_t end_pos;
>
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> if (offset < nfsi->layout.pnfs_write_begin_pos)
> nfsi->layout.pnfs_write_begin_pos = offset;
> end_pos = offset + extent - 1; /* I'm being inclusive */
> @@ -187,7 +190,7 @@ pnfs_update_last_write(struct nfs_inode *nfsi, loff_t offset, size_t extent)
> (unsigned long) offset ,
> (unsigned long) nfsi->layout.pnfs_write_begin_pos,
> (unsigned long) nfsi->layout.pnfs_write_end_pos);
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> }
>
> /* Unitialize a mountpoint in a layout driver */
> @@ -296,12 +299,27 @@ pnfs_unregister_layoutdriver(struct pnfs_layoutdriver_type *ld_type)
> * pNFS client layout cache
> */
> #if defined(CONFIG_SMP)
> +#define BUG_ON_LOCKED_LO(lo) \
> + BUG_ON(spin_is_locked(&PNFS_NFS_INODE(lo)->lo_lock))
> #define BUG_ON_UNLOCKED_LO(lo) \
> BUG_ON(!spin_is_locked(&PNFS_NFS_INODE(lo)->lo_lock))
> #else /* CONFIG_SMP */
> +#define BUG_ON_LOCKED_LO(lo) do {} while (0)
> #define BUG_ON_UNLOCKED_LO(lo) do {} while (0)
> #endif /* CONFIG_SMP */
>
> +static inline void lock_current_layout(struct nfs_inode *nfsi)
> +{
> + BUG_ON_LOCKED_LO((&nfsi->layout));
> + spin_lock(&nfsi->lo_lock);
> +}
> +
> +static inline void unlock_current_layout(struct nfs_inode *nfsi)
> +{
> + BUG_ON_UNLOCKED_LO((&nfsi->layout));
> + spin_unlock(&nfsi->lo_lock);
> +}
> +
> /*
> * get and lock nfsi->layout
> */
> @@ -310,10 +328,10 @@ get_lock_current_layout(struct nfs_inode *nfsi)
> {
> struct pnfs_layout_type *lo;
>
> + lock_current_layout(nfsi);
> lo = &nfsi->layout;
> - spin_lock(&nfsi->lo_lock);
> if (!lo->ld_data) {
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> return NULL;
> }
>
> @@ -333,7 +351,12 @@ put_unlock_current_layout(struct pnfs_layout_type *lo)
> BUG_ON_UNLOCKED_LO(lo);
> BUG_ON(lo->refcount <= 0);
>
> - if (--lo->refcount == 0 && list_empty(&lo->segs)) {
> + lo->refcount--;
> +
> + if (lo->refcount > 0)
> + goto out;
> +
> + if (list_empty(&lo->segs)) {
> struct layoutdriver_io_operations *io_ops =
> PNFS_LD_IO_OPS(lo);
>
> @@ -347,7 +370,8 @@ put_unlock_current_layout(struct pnfs_layout_type *lo)
> list_del_init(&nfsi->lo_inodes);
> spin_unlock(&clp->cl_lock);
> }
> - spin_unlock(&nfsi->lo_lock);
> +out:
> + unlock_current_layout(nfsi);
> }
>
> void
> @@ -356,7 +380,7 @@ pnfs_layout_release(struct pnfs_layout_type *lo, atomic_t *count,
> {
> struct nfs_inode *nfsi = PNFS_NFS_INODE(lo);
>
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> if (range)
> pnfs_free_layout(lo, range);
> atomic_dec(count);
> @@ -375,6 +399,8 @@ pnfs_destroy_layout(struct nfs_inode *nfsi)
> };
>
> lo = get_lock_current_layout(nfsi);
> + if (!lo)
> + return;
> pnfs_free_layout(lo, &range);
> put_unlock_current_layout(lo);
> }
> @@ -652,7 +678,7 @@ pnfs_return_layout_barrier(struct nfs_inode *nfsi,
> struct pnfs_layout_segment *lseg;
> bool ret = false;
>
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> list_for_each_entry (lseg, &nfsi->layout.segs, fi_list) {
> if (!should_free_lseg(lseg, range))
> continue;
> @@ -666,7 +692,7 @@ pnfs_return_layout_barrier(struct nfs_inode *nfsi,
> }
> if (atomic_read(&nfsi->layout.lgetcount))
> ret = true;
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
>
> dprintk("%s:Return %d\n", __func__, ret);
> return ret;
> @@ -756,7 +782,7 @@ _pnfs_return_layout(struct inode *ino, struct nfs4_pnfs_layout_segment *range,
> /* unlock w/o put rebalanced by eventual call to
> * pnfs_layout_release
> */
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
>
> if (pnfs_return_layout_barrier(nfsi, &arg)) {
> dprintk("%s: waiting\n", __func__);
> @@ -887,7 +913,7 @@ static int pnfs_wait_schedule(void *word)
> *
> * Note: If successful, nfsi->lo_lock is taken and the caller
> * must put and unlock current_layout by using put_unlock_current_layout()
> - * when the returned layout is released.
> + * directly or pnfs_layout_release() when the returned layout is released.
> */
> static struct pnfs_layout_type *
> get_lock_alloc_layout(struct inode *ino)
> @@ -922,7 +948,7 @@ get_lock_alloc_layout(struct inode *ino)
> struct nfs_client *clp = NFS_SERVER(ino)->nfs_client;
>
> /* must grab the layout lock before the client lock */
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
>
> spin_lock(&clp->cl_lock);
> if (list_empty(&nfsi->lo_inodes))
> @@ -1038,10 +1064,10 @@ void drain_layoutreturns(struct pnfs_layout_type *lo)
> while (atomic_read(&lo->lretcount)) {
> struct nfs_inode *nfsi = PNFS_NFS_INODE(lo);
>
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> dprintk("%s: waiting\n", __func__);
> wait_event(nfsi->lo_waitq, (atomic_read(&lo->lretcount) == 0));
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> }
> }
>
> @@ -1080,13 +1106,13 @@ pnfs_update_layout(struct inode *ino,
> /* Check to see if the layout for the given range already exists */
> lseg = pnfs_has_layout(lo, &arg, take_ref, !take_ref);
> if (lseg && !lseg->valid) {
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> if (take_ref)
> put_lseg(lseg);
> for (;;) {
> prepare_to_wait(&nfsi->lo_waitq, &__wait,
> TASK_KILLABLE);
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> lseg = pnfs_has_layout(lo, &arg, take_ref, !take_ref);
> if (!lseg || lseg->valid)
> break;
> @@ -1099,7 +1125,7 @@ pnfs_update_layout(struct inode *ino,
> result = -ERESTARTSYS;
> break;
> }
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> schedule();
> }
> finish_wait(&nfsi->lo_waitq, &__wait);
> @@ -1136,7 +1162,7 @@ pnfs_update_layout(struct inode *ino,
> /* Matching dec is done in .rpc_release (on non-error paths) */
> atomic_inc(&lo->lgetcount);
> /* Lose lock, but not reference, match this with pnfs_layout_release */
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
>
> result = get_layout(ino, ctx, &arg, lsegpp, lo);
> out:
> @@ -1286,7 +1312,7 @@ pnfs_layout_process(struct nfs4_pnfs_layoutget *lgp)
> *lgp->lsegpp = lseg;
> }
>
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> pnfs_insert_layout(lo, lseg);
>
> if (res->return_on_close) {
> @@ -1297,7 +1323,7 @@ pnfs_layout_process(struct nfs4_pnfs_layoutget *lgp)
>
> /* Done processing layoutget. Set the layout stateid */
> pnfs_set_layout_stateid(lo, &res->stateid);
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> out:
> return status;
> }
> @@ -2212,7 +2238,7 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync)
> if (!data)
> return -ENOMEM;
>
> - spin_lock(&nfsi->lo_lock);
> + lock_current_layout(nfsi);
> if (!nfsi->layout.layoutcommit_ctx)
> goto out_unlock;
>
> @@ -2233,7 +2259,7 @@ pnfs_layoutcommit_inode(struct inode *inode, int sync)
> nfsi->layout.layoutcommit_ctx = NULL;
>
> /* release lock on pnfs layoutcommit attrs */
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
>
> data->is_sync = sync;
> status = pnfs4_proc_layoutcommit(data);
> @@ -2242,7 +2268,7 @@ out:
> return status;
> out_unlock:
> pnfs_layoutcommit_free(data);
> - spin_unlock(&nfsi->lo_lock);
> + unlock_current_layout(nfsi);
> goto out;
> }
>
next prev parent reply other threads:[~2010-05-26 8:28 UTC|newest]
Thread overview: 28+ messages / expand[flat|nested] mbox.gz Atom feed top
2010-05-17 17:56 [PATCH 0/8] pnfs-submit: Forgetful cleint and some layout cleanups Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 1/8] pnfs-submit: clean struct nfs_inode Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 2/8] pnfs-submit: clean locking infrastructure Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 3/8] pnfs-submit: remove lgetcount, lretcount (outstanding LAYOUTGETs/LAYOUTRETUNs) Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 4/8] pnfs-submit: change stateid to be a union Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 5/8] pnfs-submit: request whole file layouts only Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 6/8] pnfs-submit: change layouts list to be similar to the other state list management Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 7/8] pnfs-submit: forgetful client model Alexandros Batsakis
2010-05-17 17:56 ` [PATCH 8/8] pnfs-submit: support for cb_recall_any (layouts) Alexandros Batsakis
2010-05-26 10:48 ` Benny Halevy
2010-05-17 20:38 ` [PATCH 7/8] pnfs-submit: forgetful client model J. Bruce Fields
2010-05-18 0:06 ` Alexandros Batsakis
2010-05-18 14:16 ` J. Bruce Fields
2010-05-18 17:33 ` Alexandros Batsakis
2010-05-18 18:22 ` J. Bruce Fields
2010-05-26 9:20 ` Benny Halevy
2010-05-27 18:38 ` Batsakis, Alexandros
2010-05-26 8:49 ` [PATCH 6/8] pnfs-submit: change layouts list to be similar to the other state list management Benny Halevy
2010-05-26 8:42 ` [PATCH 5/8] pnfs-submit: request whole file layouts only Benny Halevy
2010-05-26 8:26 ` [PATCH 4/8] pnfs-submit: change stateid to be a union Benny Halevy
2010-05-26 8:28 ` Benny Halevy [this message]
2010-05-28 17:27 ` [PATCH 2/8] pnfs-submit: clean locking infrastructure Fred Isaman
[not found] ` <AANLkTinsHI0fHYdpUlq-MsMX0BmsLGvdAbrKx7M5ydjw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
2010-05-28 18:27 ` Alexandros Batsakis
-- strict thread matches above, loose matches on Subject: below --
2010-06-07 21:11 [PATCH 0/8] forgetful client v2 Alexandros Batsakis
2010-06-07 21:11 ` [PATCH 1/8] pnfs-submit: clean struct nfs_inode Alexandros Batsakis
2010-06-07 21:11 ` [PATCH 2/8] pnfs-submit: clean locking infrastructure Alexandros Batsakis
2010-06-08 7:30 ` Christoph Hellwig
2010-06-08 7:34 ` Benny Halevy
2010-05-05 17:00 [PATCH 0/8] pnfs-submit: forgetful client v2 Alexandros Batsakis
2010-05-05 17:00 ` [PATCH 1/8] pnfs-submit: clean struct nfs_inode Alexandros Batsakis
2010-05-05 17:00 ` [PATCH 2/8] pnfs-submit: clean locking infrastructure Alexandros Batsakis
2010-06-07 14:34 ` Fred Isaman
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=4BFCDBB3.7010406@panasas.com \
--to=bhalevy@panasas.com \
--cc=batsakis@netapp.com \
--cc=linux-nfs@vger.kernel.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).