2013-10-06 07:41:03

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 0/4] pnfsd: refactor nfs4_process_layout_stateid

nfs4_process_layout_stateid is used by layout get, commit, and return.
The layout get case is different in that it allows the client to initiate
the layout stateid using an open/lock/delegation stateid.
Where in the layout commit and return cases an error must be returned
if the client uses a non-layout stateid.

0001-SQUASHME-fix-lo_destroy_list-nested-definition.patch
0002-SQUASHME-pnfsd-nfs4_find_create_layout_stateid.patch
0003-SQUASHME-pnfsd-no-need-for-find_file-in-layout-commi.patch
0004-SQUASHME-pnfsd-fixup-error-code-for-layout_commit.patch

Benny


2013-10-06 07:44:36

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 3/4] SQUASHME: pnfsd: no need for find_file in layout commit/return

the nfs4_file can be found via the layout stateid.

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 78 +++++++++++++++++------------------------------------
1 file changed, 25 insertions(+), 53 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index c00eb03..a117a5d 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -253,18 +253,13 @@ struct sbid_tracker {
}

/*
- * We have looked up the nfs4_file corresponding to the current_fh, and
- * confirmed the clientid. Pull the few tests from nfs4_preprocess_stateid_op()
- * that make sense with a layout stateid.
- *
- * If the layout state was found in cache, grab a reference count on it;
- * otherwise, allocate a new layout state if "do_alloc" is set.
+ * If the layout state was found in cache, grab a reference count on it.
*
* Called with the state_lock held
- * Returns zero and stateid is updated, or error.
+ * Returns zero and the layout state in *lsp, or error.
*/
static __be32
-nfs4_process_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
+nfs4_process_layout_stateid(struct nfs4_client *clp,
stateid_t *stateid, unsigned char typemask,
struct nfs4_layout_state **lsp)
{
@@ -272,8 +267,8 @@ struct sbid_tracker {
__be32 status = 0;
struct nfs4_stid *stid;

- dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
- __func__, clp, fp, STATEID_VAL(stateid));
+ dprintk("--> %s clp %p operation stateid=" STATEID_FMT "\n",
+ __func__, clp, STATEID_VAL(stateid));

nfs4_assert_state_locked();
status = nfsd4_lookup_stateid(stateid, typemask, &stid, true,
@@ -283,17 +278,13 @@ struct sbid_tracker {

/* Is this the first use of this layout ? */
if (stid->sc_type != NFS4_LAYOUT_STID) {
- ls = alloc_init_layout_state(clp, fp, stateid);
- if (!ls) {
- status = nfserr_jukebox;
- goto out;
- }
+ status = nfserr_bad_stateid;
+ goto out;
} else {
ls = container_of(stid, struct nfs4_layout_state, ls_stid);

/* BAD STATEID */
if (stateid->si_generation > ls->ls_stid.sc_stateid.si_generation) {
- dprintk("%s bad stateid 1\n", __func__);
status = nfserr_bad_stateid;
goto out;
}
@@ -304,10 +295,7 @@ struct sbid_tracker {
dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
STATEID_VAL(&ls->ls_stid.sc_stateid), atomic_read(&ls->ls_ref.refcount));

- if (lsp)
- *lsp = ls;
- else
- put_layout_state(ls);
+ *lsp = ls;
out:
dprintk("<-- %s status %d\n", __func__, htonl(status));

@@ -942,30 +930,31 @@ struct super_block *
{
struct inode *ino = current_fh->fh_dentry->d_inode;
struct super_block *sb = ino->i_sb;
- struct nfs4_file *fp = NULL;
struct nfs4_client *clp;
+ struct nfs4_layout_state *ls = NULL;
__be32 nfserr = NFS4_OK;
int status = 0;

lcp->res.lc_size_chg = 0;

nfs4_lock_state();
- fp = find_file(ino);
clp = find_confirmed_client((clientid_t *)&lcp->lc_clientid,
true, net_generic(SVC_NET(rqstp), nfsd_net_id));
- if (!fp || !clp) {
+ if (!clp) {
nfserr = nfserr_inval;
nfs4_unlock_state();
goto out;
}

- nfserr = nfs4_process_layout_stateid(clp, fp, &lcp->lc_sid,
- NFS4_LAYOUT_STID, NULL);
+ nfserr = nfs4_process_layout_stateid(clp, &lcp->lc_sid,
+ NFS4_LAYOUT_STID, &ls);
nfs4_unlock_state();
if (nfserr)
goto out;

if (sb->s_pnfs_op->layout_commit) {
+ struct nfs4_file *fp = ls->ls_file;
+
nfs4_file_lo_lock(fp);
status = sb->s_pnfs_op->layout_commit(ino, &lcp->args, &lcp->res);
nfs4_file_lo_unlock(fp);
@@ -1002,9 +991,6 @@ struct super_block *
}

out:
- if (fp)
- put_nfs4_file(fp);
-
return cpu_to_be32(nfserr);
}

@@ -1261,9 +1247,9 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,
int status = 0;
int layouts_found = 0;
struct inode *ino = current_fh->fh_dentry->d_inode;
- struct nfs4_file *fp = NULL;
struct nfs4_layout_state *ls = NULL;
struct nfs4_client *clp;
+ struct nfs4_file *fp = NULL;
struct nfs4_layoutrecall *clr, *nextclr;
u64 ex_fsid = current_fh->fh_export->ex_fsid;
LIST_HEAD(lo_destroy_list);
@@ -1277,26 +1263,15 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,
goto out_unlock;

if (lrp->args.lr_return_type == RETURN_FILE) {
- fp = find_file(ino);
- if (!fp) {
- dprintk("%s: RETURN_FILE: no nfs4_file for ino %p:%lu\n",
- __func__, ino, ino ? ino->i_ino : 0L);
- /* If we had a layout on the file the nfs4_file would
- * be referenced and we should have found it. Since we
- * don't then it means all layouts were ROC and at this
- * point we returned all of them on file close.
- */
- goto out_unlock;
- }
-
/* Check the stateid */
dprintk("%s PROCESS LO_STATEID inode %p\n", __func__, ino);
- status = nfs4_process_layout_stateid(clp, fp,
+ status = nfs4_process_layout_stateid(clp,
(stateid_t *)&lrp->args.lr_sid,
NFS4_LAYOUT_STID, &ls);
nfs4_unlock_state();
if (status)
goto out;
+ fp = ls->ls_file;
layouts_found = pnfs_return_file_layouts(lrp, ls,
0, &lo_destroy_list);
} else {
@@ -1305,20 +1280,14 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,
0, &lo_destroy_list);
}

- dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
+ dprintk("pNFS %s: clp %p fp %p ino %lu layout_type 0x%x iomode %d "
"return_type %d fsid 0x%llx offset %llu length %llu: "
"layouts_found %d\n",
- __func__, clp, fp, lrp->args.lr_seg.layout_type,
+ __func__, clp, fp, ino->i_ino, lrp->args.lr_seg.layout_type,
lrp->args.lr_seg.iomode, lrp->args.lr_return_type,
ex_fsid,
lrp->args.lr_seg.offset, lrp->args.lr_seg.length, layouts_found);

- nfs4_lock_state();
- if (ls)
- put_layout_state(ls);
- destroy_layout_list(&lo_destroy_list);
- nfs4_unlock_state();
-
/* update layoutrecalls
* note: for RETURN_{FSID,ALL}, fp may be NULL
*/
@@ -1338,10 +1307,13 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,

layoutrecall_list_done_put(&lo_destroy_list);

-out:
- if (fp)
- put_nfs4_file(fp);
+ nfs4_lock_state();
+ if (ls)
+ put_layout_state(ls);
+ destroy_layout_list(&lo_destroy_list);
+ nfs4_unlock_state();

+out:
dprintk("pNFS %s: exit status %d\n", __func__, status);
return status;
out_unlock:
--
1.8.3.1


2013-10-06 07:44:41

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 4/4] SQUASHME: pnfsd: fixup error code for layout_commit

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index a117a5d..e37a169 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -949,8 +949,12 @@ struct super_block *
nfserr = nfs4_process_layout_stateid(clp, &lcp->lc_sid,
NFS4_LAYOUT_STID, &ls);
nfs4_unlock_state();
- if (nfserr)
+ if (nfserr) {
+ /* fixup error code as per RFC5661 */
+ if (nfserr == nfserr_bad_stateid)
+ nfserr = nfserr_badlayout;
goto out;
+ }

if (sb->s_pnfs_op->layout_commit) {
struct nfs4_file *fp = ls->ls_file;
--
1.8.3.1


2013-10-06 15:30:46

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 3/4] SQUASHME: pnfsd: no need for find_file in layout commit/return

On Sun, Oct 06, 2013 at 10:44:32AM +0300, Benny Halevy wrote:
> the nfs4_file can be found via the layout stateid.

Oh, here we go. Really should be folded into the previous one to make
sense.

> /* Is this the first use of this layout ? */
> if (stid->sc_type != NFS4_LAYOUT_STID) {
> + status = nfserr_bad_stateid;
> + goto out;
> } else {

Given that you already have a goto out here there is no need for the
else clause.


2013-10-07 15:03:36

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCH 2/4] SQUASHME: pnfsd: nfs4_find_create_layout_stateid

On 2013-10-06 18:29, Christoph Hellwig wrote:
>> + * If the layout state was found in cache, grab a reference count on it;
>> + * otherwise, allocate a new layout state if "do_alloc" is set.
>
> There is no do_alloc parameter to the function.
>

Yup. Silly copy+paste...

>> + *
>> + * Called with the state_lock held
>> + * Returns zero and the layout state in *lsp, or error.
>> + */
>> +static __be32
>> +nfs4_find_create_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
>> + stateid_t *stateid, unsigned char typemask,
>> + struct nfs4_layout_state **lsp)
>> +{
>> + struct nfs4_layout_state *ls = NULL;
>> + __be32 status = 0;
>> + struct nfs4_stid *stid;
>> +
>> + dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
>> + __func__, clp, fp, STATEID_VAL(stateid));
>> +
>> + nfs4_assert_state_locked();
>> + status = nfsd4_lookup_stateid(stateid, typemask, &stid, true,
>> + net_generic(clp->net, nfsd_net_id));
>> + if (status)
>> + goto out;
>> +
>> + /* Is this the first use of this layout ? */
>> + if (stid->sc_type != NFS4_LAYOUT_STID) {
>> + ls = alloc_init_layout_state(clp, fp, stateid);
>> + if (!ls) {
>> + status = nfserr_jukebox;
>> + goto out;
>> + }
>> + } else {
>> + ls = container_of(stid, struct nfs4_layout_state, ls_stid);
>> +
>> + /* BAD STATEID */
>> + if (stateid->si_generation > ls->ls_stid.sc_stateid.si_generation) {
>> + status = nfserr_bad_stateid;
>> + goto out;
>> + }
>> + get_layout_state(ls);
>> + }
>> + status = 0;
>> +
>> + dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
>> + STATEID_VAL(&ls->ls_stid.sc_stateid), atomic_read(&ls->ls_ref.refcount));
>> +
>> + *lsp = ls;
>> +out:
>> + dprintk("<-- %s status %d\n", __func__, htonl(status));
>> +
>> + return status;
>> +}
>
> Shouldn't there be a corresponding removal of the alloc code from
> nfs4_process_layout_stateid?

Yes, the corresponding code in nfs4_process_layout_stateid
can and should be removed.

I'll resend a new version of this patch.

Thanks!

Benny

>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
> the body of a message to [email protected]
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>

2013-10-07 15:45:27

by Benny Halevy

[permalink] [raw]
Subject: [PATCH v2 2/4] SQUASHME: pnfsd: nfs4_find_create_layout_stateid

Split off nfs4_process_layout_stateid functionality for layout get.
layout commit and return never allocate a new layout stateid.

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 77 ++++++++++++++++++++++++++++++++++++++++++-----------
1 file changed, 61 insertions(+), 16 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 2b421e0..84f0283 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -193,23 +193,20 @@ struct sbid_tracker {
}

/*
- * We have looked up the nfs4_file corresponding to the current_fh, and
- * confirmed the clientid. Pull the few tests from nfs4_preprocess_stateid_op()
- * that make sense with a layout stateid.
- *
* If the layout state was found in cache, grab a reference count on it;
- * otherwise, allocate a new layout state if "do_alloc" is set.
+ * otherwise, allocate a new layout state if the client provided a open/
+ * lock/deleg stateid.
*
* Called with the state_lock held
- * Returns zero and stateid is updated, or error.
+ * Returns zero and the layout state in *lsp, or error.
*/
static __be32
-nfs4_process_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
- stateid_t *stateid, unsigned char typemask,
- struct nfs4_layout_state **lsp)
+nfs4_find_create_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
+ stateid_t *stateid, unsigned char typemask,
+ struct nfs4_layout_state **lsp)
{
struct nfs4_layout_state *ls = NULL;
- __be32 status = 0;
+ __be32 status;
struct nfs4_stid *stid;

dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
@@ -233,13 +230,61 @@ struct sbid_tracker {

/* BAD STATEID */
if (stateid->si_generation > ls->ls_stid.sc_stateid.si_generation) {
- dprintk("%s bad stateid 1\n", __func__);
status = nfserr_bad_stateid;
goto out;
}
get_layout_state(ls);
}
- status = 0;
+
+ dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
+ STATEID_VAL(&ls->ls_stid.sc_stateid), atomic_read(&ls->ls_ref.refcount));
+
+ *lsp = ls;
+out:
+ dprintk("<-- %s status %d\n", __func__, htonl(status));
+
+ return status;
+}
+
+/*
+ * If the layout state was found in cache, grab a reference count on it.
+ *
+ * Called with the state_lock held
+ * Returns zero and the layout state in *lsp, or error.
+ */
+static __be32
+nfs4_process_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
+ stateid_t *stateid, unsigned char typemask,
+ struct nfs4_layout_state **lsp)
+{
+ struct nfs4_layout_state *ls = NULL;
+ __be32 status;
+ struct nfs4_stid *stid;
+
+ dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
+ __func__, clp, fp, STATEID_VAL(stateid));
+
+ nfs4_assert_state_locked();
+ status = nfsd4_lookup_stateid(stateid, typemask, &stid, true,
+ net_generic(clp->net, nfsd_net_id));
+ if (status)
+ goto out;
+
+ /* Is this the first use of this layout ? */
+ if (stid->sc_type != NFS4_LAYOUT_STID) {
+ status = nfserr_bad_stateid;
+ goto out;
+ }
+
+ ls = container_of(stid, struct nfs4_layout_state, ls_stid);
+
+ /* BAD STATEID */
+ if (stateid->si_generation > ls->ls_stid.sc_stateid.si_generation) {
+ dprintk("%s bad stateid 1\n", __func__);
+ status = nfserr_bad_stateid;
+ goto out;
+ }
+ get_layout_state(ls);

*lsp = ls;
dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
@@ -576,10 +621,10 @@ struct super_block *
}

/* Check decoded layout stateid */
- nfserr = nfs4_process_layout_stateid(clp, fp, &lgp->lg_sid,
- (NFS4_OPEN_STID | NFS4_LOCK_STID |
- NFS4_DELEG_STID | NFS4_LAYOUT_STID),
- &ls);
+ nfserr = nfs4_find_create_layout_stateid(clp, fp, &lgp->lg_sid,
+ (NFS4_OPEN_STID | NFS4_LOCK_STID |
+ NFS4_DELEG_STID | NFS4_LAYOUT_STID),
+ &ls);
if (nfserr)
goto out_unlock;

--
1.8.3.1



2013-10-06 07:44:28

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 1/4] SQUASHME: fix lo_destroy_list nested definition

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 95a27ec..a9e535f 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1223,8 +1223,6 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,
goto out_unlock;

if (lrp->args.lr_return_type == RETURN_FILE) {
- LIST_HEAD(lo_destroy_list);
-
fp = find_file(ino);
if (!fp) {
dprintk("%s: RETURN_FILE: no nfs4_file for ino %p:%lu\n",
--
1.8.3.1


2013-10-06 07:44:32

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 2/4] SQUASHME: pnfsd: nfs4_find_create_layout_stateid

Split off nfs4_process_layout_stateid functionality for layout get.
layout commit and return never allocate a new layout stateid.

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 62 +++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 58 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index a9e535f..c00eb03 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -199,6 +199,60 @@ struct sbid_tracker {
}

/*
+ * If the layout state was found in cache, grab a reference count on it;
+ * otherwise, allocate a new layout state if "do_alloc" is set.
+ *
+ * Called with the state_lock held
+ * Returns zero and the layout state in *lsp, or error.
+ */
+static __be32
+nfs4_find_create_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
+ stateid_t *stateid, unsigned char typemask,
+ struct nfs4_layout_state **lsp)
+{
+ struct nfs4_layout_state *ls = NULL;
+ __be32 status = 0;
+ struct nfs4_stid *stid;
+
+ dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
+ __func__, clp, fp, STATEID_VAL(stateid));
+
+ nfs4_assert_state_locked();
+ status = nfsd4_lookup_stateid(stateid, typemask, &stid, true,
+ net_generic(clp->net, nfsd_net_id));
+ if (status)
+ goto out;
+
+ /* Is this the first use of this layout ? */
+ if (stid->sc_type != NFS4_LAYOUT_STID) {
+ ls = alloc_init_layout_state(clp, fp, stateid);
+ if (!ls) {
+ status = nfserr_jukebox;
+ goto out;
+ }
+ } else {
+ ls = container_of(stid, struct nfs4_layout_state, ls_stid);
+
+ /* BAD STATEID */
+ if (stateid->si_generation > ls->ls_stid.sc_stateid.si_generation) {
+ status = nfserr_bad_stateid;
+ goto out;
+ }
+ get_layout_state(ls);
+ }
+ status = 0;
+
+ dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
+ STATEID_VAL(&ls->ls_stid.sc_stateid), atomic_read(&ls->ls_ref.refcount));
+
+ *lsp = ls;
+out:
+ dprintk("<-- %s status %d\n", __func__, htonl(status));
+
+ return status;
+}
+
+/*
* We have looked up the nfs4_file corresponding to the current_fh, and
* confirmed the clientid. Pull the few tests from nfs4_preprocess_stateid_op()
* that make sense with a layout stateid.
@@ -776,10 +830,10 @@ struct super_block *
}

/* Check decoded layout stateid */
- nfserr = nfs4_process_layout_stateid(clp, fp, &lgp->lg_sid,
- (NFS4_OPEN_STID | NFS4_LOCK_STID |
- NFS4_DELEG_STID | NFS4_LAYOUT_STID),
- &ls);
+ nfserr = nfs4_find_create_layout_stateid(clp, fp, &lgp->lg_sid,
+ (NFS4_OPEN_STID | NFS4_LOCK_STID |
+ NFS4_DELEG_STID | NFS4_LAYOUT_STID),
+ &ls);
if (nfserr)
goto out_unlock;

--
1.8.3.1


2013-10-06 15:29:17

by Christoph Hellwig

[permalink] [raw]
Subject: Re: [PATCH 2/4] SQUASHME: pnfsd: nfs4_find_create_layout_stateid

> + * If the layout state was found in cache, grab a reference count on it;
> + * otherwise, allocate a new layout state if "do_alloc" is set.

There is no do_alloc parameter to the function.

> + *
> + * Called with the state_lock held
> + * Returns zero and the layout state in *lsp, or error.
> + */
> +static __be32
> +nfs4_find_create_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
> + stateid_t *stateid, unsigned char typemask,
> + struct nfs4_layout_state **lsp)
> +{
> + struct nfs4_layout_state *ls = NULL;
> + __be32 status = 0;
> + struct nfs4_stid *stid;
> +
> + dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
> + __func__, clp, fp, STATEID_VAL(stateid));
> +
> + nfs4_assert_state_locked();
> + status = nfsd4_lookup_stateid(stateid, typemask, &stid, true,
> + net_generic(clp->net, nfsd_net_id));
> + if (status)
> + goto out;
> +
> + /* Is this the first use of this layout ? */
> + if (stid->sc_type != NFS4_LAYOUT_STID) {
> + ls = alloc_init_layout_state(clp, fp, stateid);
> + if (!ls) {
> + status = nfserr_jukebox;
> + goto out;
> + }
> + } else {
> + ls = container_of(stid, struct nfs4_layout_state, ls_stid);
> +
> + /* BAD STATEID */
> + if (stateid->si_generation > ls->ls_stid.sc_stateid.si_generation) {
> + status = nfserr_bad_stateid;
> + goto out;
> + }
> + get_layout_state(ls);
> + }
> + status = 0;
> +
> + dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
> + STATEID_VAL(&ls->ls_stid.sc_stateid), atomic_read(&ls->ls_ref.refcount));
> +
> + *lsp = ls;
> +out:
> + dprintk("<-- %s status %d\n", __func__, htonl(status));
> +
> + return status;
> +}

Shouldn't there be a corresponding removal of the alloc code from
nfs4_process_layout_stateid?


2013-10-07 15:50:28

by Benny Halevy

[permalink] [raw]
Subject: [PATCH v2 3/4] SQUASHME: pnfsd: no need for find_file in layout commit/return

the nfs4_file can be found via the layout stateid.

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 64 ++++++++++++++++++++++-------------------------------
1 file changed, 26 insertions(+), 38 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 52fd2d1..474c183 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -259,7 +259,7 @@ struct sbid_tracker {
* Returns zero and the layout state in *lsp, or error.
*/
static __be32
-nfs4_process_layout_stateid(struct nfs4_client *clp, struct nfs4_file *fp,
+nfs4_process_layout_stateid(struct nfs4_client *clp,
stateid_t *stateid, unsigned char typemask,
struct nfs4_layout_state **lsp)
{
@@ -267,8 +267,8 @@ struct sbid_tracker {
__be32 status;
struct nfs4_stid *stid;

- dprintk("--> %s clp %p fp %p operation stateid=" STATEID_FMT "\n",
- __func__, clp, fp, STATEID_VAL(stateid));
+ dprintk("--> %s clp %p operation stateid=" STATEID_FMT "\n",
+ __func__, clp, STATEID_VAL(stateid));

nfs4_assert_state_locked();
status = nfsd4_lookup_stateid(stateid, typemask, &stid, true,
@@ -295,10 +295,7 @@ struct sbid_tracker {
dprintk("%s: layout stateid=" STATEID_FMT " ref=%d\n", __func__,
STATEID_VAL(&ls->ls_stid.sc_stateid), atomic_read(&ls->ls_ref.refcount));

- if (lsp)
- *lsp = ls;
- else
- put_layout_state(ls);
+ *lsp = ls;
out:
dprintk("<-- %s status %d\n", __func__, htonl(status));

@@ -920,30 +917,31 @@ struct super_block *
{
struct inode *ino = current_fh->fh_dentry->d_inode;
struct super_block *sb = ino->i_sb;
- struct nfs4_file *fp = NULL;
struct nfs4_client *clp;
+ struct nfs4_layout_state *ls = NULL;
__be32 nfserr = NFS4_OK;
int status = 0;

lcp->res.lc_size_chg = 0;

nfs4_lock_state();
- fp = find_file(ino);
clp = find_confirmed_client((clientid_t *)&lcp->lc_clientid,
true, net_generic(SVC_NET(rqstp), nfsd_net_id));
- if (!fp || !clp) {
+ if (!clp) {
nfserr = nfserr_inval;
nfs4_unlock_state();
goto out;
}

- nfserr = nfs4_process_layout_stateid(clp, fp, &lcp->lc_sid,
- NFS4_LAYOUT_STID, NULL);
+ nfserr = nfs4_process_layout_stateid(clp, &lcp->lc_sid,
+ NFS4_LAYOUT_STID, &ls);
nfs4_unlock_state();
if (nfserr)
goto out;

if (sb->s_pnfs_op->layout_commit) {
+ struct nfs4_file *fp = ls->ls_file;
+
nfs4_file_lo_lock(fp);
status = sb->s_pnfs_op->layout_commit(ino, &lcp->args, &lcp->res);
nfs4_file_lo_unlock(fp);
@@ -980,8 +978,11 @@ struct super_block *
}

out:
- if (fp)
- put_nfs4_file(fp);
+ if (ls) {
+ nfs4_lock_state();
+ put_layout_state(ls);
+ nfs4_unlock_state();
+ }

return cpu_to_be32(nfserr);
}
@@ -1255,26 +1256,15 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,
goto out_unlock;

if (lrp->args.lr_return_type == RETURN_FILE) {
- fp = find_file(ino);
- if (!fp) {
- dprintk("%s: RETURN_FILE: no nfs4_file for ino %p:%lu\n",
- __func__, ino, ino ? ino->i_ino : 0L);
- /* If we had a layout on the file the nfs4_file would
- * be referenced and we should have found it. Since we
- * don't then it means all layouts were ROC and at this
- * point we returned all of them on file close.
- */
- goto out_unlock;
- }
-
/* Check the stateid */
dprintk("%s PROCESS LO_STATEID inode %p\n", __func__, ino);
- status = nfs4_process_layout_stateid(clp, fp,
+ status = nfs4_process_layout_stateid(clp,
(stateid_t *)&lrp->args.lr_sid,
NFS4_LAYOUT_STID, &ls);
nfs4_unlock_state();
if (status)
goto out;
+ fp = ls->ls_file;
layouts_found = pnfs_return_file_layouts(lrp, ls,
0, &lo_destroy_list);
} else {
@@ -1283,20 +1273,14 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,
0, &lo_destroy_list);
}

- dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
+ dprintk("pNFS %s: clp %p fp %p ino %lu layout_type 0x%x iomode %d "
"return_type %d fsid 0x%llx offset %llu length %llu: "
"layouts_found %d\n",
- __func__, clp, fp, lrp->args.lr_seg.layout_type,
+ __func__, clp, fp, ino->i_ino, lrp->args.lr_seg.layout_type,
lrp->args.lr_seg.iomode, lrp->args.lr_return_type,
ex_fsid,
lrp->args.lr_seg.offset, lrp->args.lr_seg.length, layouts_found);

- nfs4_lock_state();
- if (ls)
- put_layout_state(ls);
- destroy_layout_list(&lo_destroy_list);
- nfs4_unlock_state();
-
/* update layoutrecalls
* note: for RETURN_{FSID,ALL}, fp may be NULL
*/
@@ -1316,10 +1300,14 @@ int nfs4_pnfs_return_layout(struct svc_rqst *rqstp,

layoutrecall_list_done_put(&lo_destroy_list);

-out:
- if (fp)
- put_nfs4_file(fp);
+ /* put_layout_state here as we need to keep a reference on ls_file */
+ nfs4_lock_state();
+ if (ls)
+ put_layout_state(ls);
+ destroy_layout_list(&lo_destroy_list);
+ nfs4_unlock_state();

+out:
dprintk("pNFS %s: exit status %d\n", __func__, status);
return status;
out_unlock:
--
1.8.3.1