2013-01-21 14:52:59

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 0/9] pnfsd fixes

[PATCH 1/9] SQUASHME: pnfsd: fix lrs_present calculation
[PATCH 2/9] SQUASHME: pnfsd: return_on_close all layout segments
[PATCH 3/9] SQUASHME: pnfsd: do not dequeue layout state on
[PATCH 4/9] SQUASHME: pnfsd: remove fs_layout_return sb parameter
[PATCH 5/9] SQUASHME: pnfsd: fix locking around fs_layout_return
[PATCH 6/9] pnfsd: LR_FLAG_CL_EMPTY layout return flag
[PATCH 7/9] pnfsd: LR_FLAG_EMPTY layout return flag
[PATCH 8/9] SQUASHME: pnfsd: Something very wrong with
[PATCH 9/9] SQUASHME: pnfsd: no use for fi_layout_states list


2013-01-21 14:54:20

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 1/9] SQUASHME: pnfsd: fix lrs_present calculation

lrs_present must be set to zero iff the client holds no layout state on
the respective file.

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

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 3931e52..5c50d8a 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -893,7 +893,7 @@ struct super_block *
struct nfs4_layout *lp, *nextlp;

dprintk("%s: clp %p fp %p\n", __func__, clp, fp);
- lrp->lrs_present = 1;
+ lrp->lrs_present = 0;
spin_lock(&layout_lock);
list_for_each_entry_safe (lp, nextlp, &fp->fi_layouts, lo_perfile) {
dprintk("%s: lp %p client %p,%p lo_type %x,%x iomode %d,%d\n",
@@ -901,19 +901,22 @@ struct super_block *
lp->lo_client, clp,
lp->lo_seg.layout_type, lrp->args.lr_seg.layout_type,
lp->lo_seg.iomode, lrp->args.lr_seg.iomode);
- if (lp->lo_client != clp ||
- lp->lo_seg.layout_type != lrp->args.lr_seg.layout_type ||
+ if (lp->lo_client != clp)
+ continue;
+ if (lp->lo_seg.layout_type != lrp->args.lr_seg.layout_type ||
(lp->lo_seg.iomode != lrp->args.lr_seg.iomode &&
lrp->args.lr_seg.iomode != IOMODE_ANY) ||
- !lo_seg_overlapping(&lp->lo_seg, &lrp->args.lr_seg))
+ !lo_seg_overlapping(&lp->lo_seg, &lrp->args.lr_seg)) {
+ lrp->lrs_present = 1;
continue;
+ }
layouts_found++;
trim_layout(&lp->lo_seg, &lrp->args.lr_seg);
if (!lp->lo_seg.length) {
- lrp->lrs_present = 0;
dequeue_layout(lp);
destroy_layout(lp);
- }
+ } else
+ lrp->lrs_present = 1;
}
if (ls && layouts_found && lrp->lrs_present)
update_layout_stateid_locked(ls, &lrp->lr_sid);
--
1.7.11.7


2013-01-21 14:55:02

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 8/9] SQUASHME: pnfsd: Something very wrong with layout_recall(RETURN_FILE)

From: Boaz Harrosh <[email protected]>

In patch:
pnfsd: layout recall layout state

the cl_has_file_layout() is no longer inspecting the layout structures added per file
but is inspecting if file has layout_state.

So it is counting layout_states and not layouts

This is bad because the addition of the layout_states on the file is done before the
call to the filesystem so if the FS does a recall, the nfsd is confused thinking
it already has a layout and issues a recall. Instead of returning -ENOENT, ie list
is empty. The client then truly returns nomaching_layout and when the lo_return(s) are
emulated the system gets stuck is some reference miss-match. (UML so no crash trace)

Now lets say that the state should be set before the call to the FS. Then I don't
see where the state is removed in the case of an ERROR return from FS->layout_get.
Meaning cl_has_file_layout() will always think it has some count.

Also When a layout is returned it is the layout list that is inspected and freed,
so how is the cl_has_file_layout() emptied ?

In any way. I do not agree that it is the state that is needed to be searched
in cl_has_file_layout() but it is layouts that are needed, otherwise the all
layout <---> recall very delicate dance is totally broken.

What was the meaning of the Poet?

I reverted the cl_has_file_layout() to historical processing.

Also cl_has_file_layout() returns true for any layout on a file, but we must
inspect IO_MODE and LSEG for a partial-match, as well.

The below works for me. State also looks good. I can now safely call
cb_recall, from within a layout_get operation.

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

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 0b8c502..3375554 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1192,24 +1192,27 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
}

static bool
-cl_has_file_layout(struct nfs4_client *clp, struct nfs4_file *fp, stateid_t *lsid)
+cl_has_file_layout(struct nfs4_client *clp, struct nfs4_file *fp,
+ stateid_t *lsid, struct nfsd4_pnfs_cb_layout *cbl)
{
- struct nfs4_layout_state *ls;
+ struct nfs4_layout *lo;
+ bool ret = false;

spin_lock(&layout_lock);
- list_for_each_entry (ls, &fp->fi_layout_states, ls_perfile)
- if (same_clid(&ls->ls_stid.sc_stateid.si_opaque.so_clid,
- &clp->cl_clientid)) {
+ list_for_each_entry(lo, &fp->fi_layouts, lo_perfile) {
+ if (same_clid(&lo->lo_client->cl_clientid, &clp->cl_clientid) &&
+ lo_seg_overlapping(&cbl->cbl_seg, &lo->lo_seg) &&
+ (cbl->cbl_seg.iomode & lo->lo_seg.iomode))
goto found;
- }
- spin_unlock(&layout_lock);
- return false;
-
+ }
+ goto unlock;
found:
- update_layout_stateid_locked(ls, lsid);
+ /* Im going to send a recall on this latout update state */
+ update_layout_stateid_locked(lo->lo_state, lsid);
+ ret = true;
+unlock:
spin_unlock(&layout_lock);
-
- return true;
+ return ret;
}

static int
@@ -1241,7 +1244,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
{
switch (cbl->cbl_recall_type) {
case RETURN_FILE:
- return cl_has_file_layout(clp, lrfile, lsid);
+ return cl_has_file_layout(clp, lrfile, lsid, cbl);
case RETURN_FSID:
return cl_has_fsid_layout(clp, &cbl->cbl_fsid);
default:
--
1.7.11.7


2013-01-21 14:55:07

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 9/9] SQUASHME: pnfsd: no use for fi_layout_states list anymore

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 10 ----------
fs/nfsd/nfs4state.c | 1 -
fs/nfsd/pnfsd.h | 1 -
fs/nfsd/state.h | 1 -
4 files changed, 13 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 3375554..4320914 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -157,10 +157,6 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)
return new;
kref_init(&new->ls_ref);
nfsd4_init_stid(&new->ls_stid, clp, NFS4_LAYOUT_STID);
- INIT_LIST_HEAD(&new->ls_perfile);
- spin_lock(&layout_lock);
- list_add(&new->ls_perfile, &fp->fi_layout_states);
- spin_unlock(&layout_lock);
new->ls_roc = false;
return new;
}
@@ -178,11 +174,6 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)
container_of(kref, struct nfs4_layout_state, ls_ref);

nfsd4_unhash_stid(&ls->ls_stid);
- if (!list_empty(&ls->ls_perfile)) {
- spin_lock(&layout_lock);
- list_del(&ls->ls_perfile);
- spin_unlock(&layout_lock);
- }
kfree(ls);
}

@@ -1310,7 +1301,6 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
continue;

/* Return the layout */
- list_del_init(&lo->lo_state->ls_perfile); /* just to be on the safe side */
dequeue_layout(lo);
list_add_tail(&lo->lo_perfile, &lo_destroy_list);
}
diff --git a/fs/nfsd/nfs4state.c b/fs/nfsd/nfs4state.c
index 0553220..1682413 100644
--- a/fs/nfsd/nfs4state.c
+++ b/fs/nfsd/nfs4state.c
@@ -2370,7 +2370,6 @@ static void nfsd4_init_file(struct nfs4_file *fp, struct inode *ino,
memset(fp->fi_access, 0, sizeof(fp->fi_access));
#if defined(CONFIG_PNFSD)
INIT_LIST_HEAD(&fp->fi_layouts);
- INIT_LIST_HEAD(&fp->fi_layout_states);
fp->fi_fsid.major = current_fh->fh_export->ex_fsid;
fp->fi_fsid.minor = 0;
fp->fi_fhlen = current_fh->fh_handle.fh_size;
diff --git a/fs/nfsd/pnfsd.h b/fs/nfsd/pnfsd.h
index 35859ff..fe35466 100644
--- a/fs/nfsd/pnfsd.h
+++ b/fs/nfsd/pnfsd.h
@@ -44,7 +44,6 @@
struct nfs4_layout_state {
struct nfs4_stid ls_stid; /* must be first field */
struct kref ls_ref;
- struct list_head ls_perfile;
bool ls_roc;
};

diff --git a/fs/nfsd/state.h b/fs/nfsd/state.h
index 08a31a7..4ca6f2d 100644
--- a/fs/nfsd/state.h
+++ b/fs/nfsd/state.h
@@ -399,7 +399,6 @@ struct nfs4_file {
bool fi_had_conflict;
#if defined(CONFIG_PNFSD)
struct list_head fi_layouts;
- struct list_head fi_layout_states;
/* used by layoutget / layoutrecall */
struct nfs4_fsid fi_fsid;
u32 fi_fhlen;
--
1.7.11.7


2013-01-21 14:54:37

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 4/9] SQUASHME: pnfsd: remove fs_layout_return sb parameter

From: Boaz Harrosh <[email protected]>

Derive the superblock from the inode argument.
Let the caller provide the right inode, rather than NULL
for RETURN_{FSID,ALL}

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

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index c62af3d..b8d0db8 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -350,11 +350,12 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
put_nfs4_file(fp);
}

-void fs_layout_return(struct super_block *sb, struct inode *ino,
- struct nfsd4_pnfs_layoutreturn *lrp, int flags,
- void *recall_cookie)
+static void fs_layout_return(struct inode *ino,
+ struct nfsd4_pnfs_layoutreturn *lrp,
+ int flags, void *recall_cookie)
{
int ret;
+ struct super_block *sb = ino->i_sb;

if (unlikely(!sb->s_pnfs_op->layout_return))
return;
@@ -362,9 +363,6 @@ void fs_layout_return(struct super_block *sb, struct inode *ino,
lrp->args.lr_flags = flags;
lrp->args.lr_cookie = recall_cookie;

- if (!ino) /* FSID or ALL */
- ino = sb->s_root->d_inode;
-
ret = sb->s_pnfs_op->layout_return(ino, &lrp->args);
dprintk("%s: inode %lu iomode=%d offset=0x%llx length=0x%llx "
"cookie = %p flags 0x%x status=%d\n",
@@ -1081,7 +1079,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
nfs4_unlock_state();

/* call exported filesystem layout_return (ignore return-code) */
- fs_layout_return(sb, ino, lrp, 0, recall_cookie);
+ fs_layout_return(ino, lrp, 0, recall_cookie);

out_no_fs_call:
dprintk("pNFS %s: exit status %d\n", __func__, status);
@@ -1201,9 +1199,8 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
inode = igrab(clr->clr_file->fi_inode);
if (WARN_ON(!inode))
return;
- } else {
- inode = NULL;
- }
+ } else
+ inode = clr->clr_sb->s_root->d_inode;

dprintk("%s: clp %p fp %p: simulating layout_return\n", __func__,
clr->clr_client, clr->clr_file);
@@ -1219,8 +1216,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
recall_cookie = layoutrecall_done(clr);
spin_unlock(&layout_lock);

- fs_layout_return(clr->clr_sb, inode, &lr, LR_FLAG_INTERN,
- recall_cookie);
+ fs_layout_return(inode, &lr, LR_FLAG_INTERN, recall_cookie);
iput(inode);
}

@@ -1255,8 +1251,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
empty = list_empty(&fp->fi_layouts);
found = true;
dprintk("%s: fp=%p clp=%p: return on close", __func__, fp, clp);
- fs_layout_return(fp->fi_inode->i_sb, fp->fi_inode, &lr,
- LR_FLAG_INTERN,
+ fs_layout_return(fp->fi_inode, &lr, LR_FLAG_INTERN,
empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
}
spin_unlock(&layout_lock);
@@ -1315,7 +1310,7 @@ void pnfs_expire_client(struct nfs4_client *clp)
dprintk("%s: inode %lu lp %p clp %p\n", __func__, inode->i_ino,
lp, clp);

- fs_layout_return(inode->i_sb, inode, &lr, LR_FLAG_EXPIRE,
+ fs_layout_return(inode, &lr, LR_FLAG_EXPIRE,
empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
iput(inode);
}
--
1.7.11.7


2013-01-21 14:54:44

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 5/9] SQUASHME: pnfsd: fix locking around fs_layout_return

From: Boaz Harrosh <[email protected]>

Every code path that eventually called fs_layout_return
had different locking. Some held both the layout_spinlock
has well as nfs-lock. Some one or the other, some none.
Fix all sites to never hold any locks, before calling
The FS.

Also, deprecate PNFS_LAST_LAYOUT_NO_RECALLS.
To be replaced with lr_flags bit

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 176 ++++++++++++++++++++++------------------
include/linux/nfsd/nfsd4_pnfs.h | 2 -
2 files changed, 97 insertions(+), 81 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index b8d0db8..d48abba 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -186,6 +186,9 @@ void pnfs_clear_device_notify(struct nfs4_client *clp)
kfree(ls);
}

+/*
+ * Note: caller must not hold layout_lock
+ */
static void
put_layout_state(struct nfs4_layout_state *ls)
{
@@ -321,6 +324,9 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
dprintk("pNFS %s end\n", __func__);
}

+/*
+ * Note: always called under the layout_lock
+ */
static void
dequeue_layout(struct nfs4_layout *lp)
{
@@ -329,7 +335,7 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
}

/*
- * Note: always called under the layout_lock
+ * Note: caller must not hold layout_lock
*/
static void
destroy_layout(struct nfs4_layout *lp)
@@ -881,10 +887,59 @@ struct super_block *
dprintk("%s:End lo %llu:%lld\n", __func__, lo->offset, lo->length);
}

+static void
+pnfsd_return_lo_list(struct list_head *lo_destroy_list, struct inode *ino_orig,
+ struct nfsd4_pnfs_layoutreturn *lr_orig, int flags,
+ void *cb_cookie)
+{
+ struct nfs4_layout *lo, *nextlp;
+
+ if (list_empty(lo_destroy_list) && cb_cookie) {
+ /* This is a rare race case where at the time of recall there
+ * were some layouts, which got freed before the recall-done.
+ * and the recall was left without any actual layouts to free.
+ * If the FS gave us a cb_cookie it is waiting for it so we
+ * report back about it here.
+ */
+
+ /* Any caller with a cb_cookie must pass a none null
+ * ino_orig and an lr_orig.
+ */
+ struct inode *inode = igrab(ino_orig);
+
+ /* Probably a BUG_ON but just in case. Caller of cb_recall must
+ * take care of this. Please report to ml */
+ if (WARN_ON(!inode))
+ return;
+
+ fs_layout_return(inode, lr_orig, flags, cb_cookie);
+ iput(inode);
+ return;
+ }
+
+ list_for_each_entry_safe(lo, nextlp, lo_destroy_list, lo_perfile) {
+ struct inode *inode = lo->lo_file->fi_inode;
+ struct nfsd4_pnfs_layoutreturn lr;
+ bool empty;
+
+ memset(&lr, 0, sizeof(lr));
+ lr.args.lr_return_type = RETURN_FILE;
+ lr.args.lr_seg = lo->lo_seg;
+
+ list_del(&lo->lo_perfile);
+ empty = list_empty(lo_destroy_list);
+
+ fs_layout_return(inode, &lr, flags, empty ? cb_cookie : NULL);
+
+ destroy_layout(lo); /* this will put the lo_file */
+ }
+}
+
static int
pnfs_return_file_layouts(struct nfs4_client *clp, struct nfs4_file *fp,
struct nfsd4_pnfs_layoutreturn *lrp,
- struct nfs4_layout_state *ls)
+ struct nfs4_layout_state *ls,
+ struct list_head *lo_destroy_list)
{
int layouts_found = 0;
struct nfs4_layout *lp, *nextlp;
@@ -911,7 +966,7 @@ struct super_block *
trim_layout(&lp->lo_seg, &lrp->args.lr_seg);
if (!lp->lo_seg.length) {
dequeue_layout(lp);
- destroy_layout(lp);
+ list_add_tail(&lp->lo_perfile, lo_destroy_list);
} else
lrp->lrs_present = 1;
}
@@ -924,7 +979,8 @@ struct super_block *

static int
pnfs_return_client_layouts(struct nfs4_client *clp,
- struct nfsd4_pnfs_layoutreturn *lrp, u64 ex_fsid)
+ struct nfsd4_pnfs_layoutreturn *lrp, u64 ex_fsid,
+ struct list_head *lo_destroy_list)
{
int layouts_found = 0;
struct nfs4_layout *lp, *nextlp;
@@ -942,7 +998,7 @@ struct super_block *

layouts_found++;
dequeue_layout(lp);
- destroy_layout(lp);
+ list_add_tail(&lp->lo_perfile, lo_destroy_list);
}
spin_unlock(&layout_lock);

@@ -1004,10 +1060,10 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
struct inode *ino = current_fh->fh_dentry->d_inode;
struct nfs4_file *fp = NULL;
struct nfs4_client *clp;
- struct nfs4_layout_state *ls = NULL;
struct nfs4_layoutrecall *clr, *nextclr;
u64 ex_fsid = current_fh->fh_export->ex_fsid;
void *recall_cookie = NULL;
+ LIST_HEAD(lo_destroy_list);

dprintk("NFSD: %s\n", __func__);

@@ -1017,6 +1073,8 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
goto out;

if (lrp->args.lr_return_type == RETURN_FILE) {
+ struct nfs4_layout_state *ls = NULL;
+
fp = find_file(ino);
if (!fp) {
nfs4_unlock_state();
@@ -1037,12 +1095,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
goto out_put_file;

/* update layouts */
- layouts_found = pnfs_return_file_layouts(clp, fp, lrp, ls);
- /* optimize for the all-empty case */
- if (list_empty(&fp->fi_layouts))
- recall_cookie = PNFS_LAST_LAYOUT_NO_RECALLS;
+ layouts_found = pnfs_return_file_layouts(clp, fp, lrp, ls,
+ &lo_destroy_list);
+ put_layout_state(ls);
} else {
- layouts_found = pnfs_return_client_layouts(clp, lrp, ex_fsid);
+ layouts_found = pnfs_return_client_layouts(clp, lrp, ex_fsid,
+ &lo_destroy_list);
}

dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
@@ -1073,13 +1131,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
out_put_file:
if (fp)
put_nfs4_file(fp);
- if (ls)
- put_layout_state(ls);
+
out:
nfs4_unlock_state();

- /* call exported filesystem layout_return (ignore return-code) */
- fs_layout_return(ino, lrp, 0, recall_cookie);
+ pnfsd_return_lo_list(&lo_destroy_list, ino ? ino : sb->s_root->d_inode,
+ lrp, 0, recall_cookie);

out_no_fs_call:
dprintk("pNFS %s: exit status %d\n", __func__, status);
@@ -1194,30 +1251,26 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
};
struct inode *inode;
void *recall_cookie;
-
- if (clr->clr_file) {
- inode = igrab(clr->clr_file->fi_inode);
- if (WARN_ON(!inode))
- return;
- } else
- inode = clr->clr_sb->s_root->d_inode;
+ LIST_HEAD(lo_destroy_list);

dprintk("%s: clp %p fp %p: simulating layout_return\n", __func__,
clr->clr_client, clr->clr_file);

if (clr->cb.cbl_recall_type == RETURN_FILE)
pnfs_return_file_layouts(clr->clr_client, clr->clr_file, &lr,
- NULL);
+ NULL, &lo_destroy_list);
else
pnfs_return_client_layouts(clr->clr_client, &lr,
- clr->cb.cbl_fsid.major);
+ clr->cb.cbl_fsid.major,
+ &lo_destroy_list);

spin_lock(&layout_lock);
recall_cookie = layoutrecall_done(clr);
spin_unlock(&layout_lock);

- fs_layout_return(inode, &lr, LR_FLAG_INTERN, recall_cookie);
- iput(inode);
+ inode = clr->clr_file->fi_inode ?: clr->clr_sb->s_root->d_inode;
+ pnfsd_return_lo_list(&lo_destroy_list, inode, &lr, LR_FLAG_INTERN,
+ recall_cookie);
}

/* Return On Close:
@@ -1228,39 +1281,31 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
{
struct nfs4_layout *lo, *nextlp;
- bool found = false;
+ LIST_HEAD(lo_destroy_list);

+ /* TODO: We need to also free layout recalls like pnfs_expire_client */
dprintk("%s: fp=%p clp=%p", __func__, fp, clp);
spin_lock(&layout_lock);
list_for_each_entry_safe (lo, nextlp, &fp->fi_layouts, lo_perfile) {
- struct nfsd4_pnfs_layoutreturn lr;
- bool empty;
-
/* Check for a match */
if (lo->lo_client != clp)
continue;

/* Return the layout */
- memset(&lr, 0, sizeof(lr));
- lr.args.lr_return_type = RETURN_FILE;
- lr.args.lr_seg = lo->lo_seg;
list_del_init(&lo->lo_state->ls_perfile); /* just to be on the safe side */
dequeue_layout(lo);
- destroy_layout(lo); /* do not access lp after this */
-
- empty = list_empty(&fp->fi_layouts);
- found = true;
- dprintk("%s: fp=%p clp=%p: return on close", __func__, fp, clp);
- fs_layout_return(fp->fi_inode, &lr, LR_FLAG_INTERN,
- empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
+ list_add_tail(&lo->lo_perfile, &lo_destroy_list);
}
spin_unlock(&layout_lock);
- if (!found)
- dprintk("%s: no layout found", __func__);
+
+ pnfsd_return_lo_list(&lo_destroy_list, NULL, NULL, LR_FLAG_INTERN, NULL);
}

void pnfs_expire_client(struct nfs4_client *clp)
{
+ struct nfs4_layout *lo, *nextlo;
+ LIST_HEAD(lo_destroy_list);
+
for (;;) {
struct nfs4_layoutrecall *lrp = NULL;

@@ -1280,40 +1325,17 @@ void pnfs_expire_client(struct nfs4_client *clp)
put_layoutrecall(lrp);
}

- for (;;) {
- struct nfs4_layout *lp = NULL;
- struct inode *inode = NULL;
- struct nfsd4_pnfs_layoutreturn lr;
- bool empty = false;
-
- spin_lock(&layout_lock);
- if (!list_empty(&clp->cl_layouts)) {
- lp = list_entry(clp->cl_layouts.next,
- struct nfs4_layout, lo_perclnt);
- inode = igrab(lp->lo_file->fi_inode);
- memset(&lr, 0, sizeof(lr));
- lr.args.lr_return_type = RETURN_FILE;
- lr.args.lr_seg = lp->lo_seg;
- empty = list_empty(&lp->lo_file->fi_layouts);
- BUG_ON(lp->lo_client != clp);
- list_del_init(&lp->lo_state->ls_perfile); /* just to be on the safe side */
- dequeue_layout(lp);
- destroy_layout(lp); /* do not access lp after this */
- }
- spin_unlock(&layout_lock);
- if (!lp)
- break;
-
- if (WARN_ON(!inode))
- break;
-
- dprintk("%s: inode %lu lp %p clp %p\n", __func__, inode->i_ino,
- lp, clp);
-
- fs_layout_return(inode, &lr, LR_FLAG_EXPIRE,
- empty ? PNFS_LAST_LAYOUT_NO_RECALLS : NULL);
- iput(inode);
+ spin_lock(&layout_lock);
+ list_for_each_entry_safe (lo, nextlo, &clp->cl_layouts, lo_perclnt) {
+ BUG_ON(lo->lo_client != clp);
+ dequeue_layout(lo);
+ list_add_tail(&lo->lo_perfile, &lo_destroy_list);
+ dprintk("%s: inode %lu lp %p clp %p\n", __func__,
+ lo->lo_file->fi_inode->i_ino, lo, clp);
}
+ spin_unlock(&layout_lock);
+
+ pnfsd_return_lo_list(&lo_destroy_list, NULL, NULL, LR_FLAG_EXPIRE, NULL);
}

struct create_recall_list_arg {
@@ -1516,10 +1538,6 @@ struct create_recall_list_arg {

INIT_LIST_HEAD(&todolist);

- /* If no cookie provided by FS, return a default one */
- if (!cbl->cbl_cookie)
- cbl->cbl_cookie = PNFS_LAST_LAYOUT_NO_RECALLS;
-
status = create_layout_recall_list(&todolist, &todo_len, cbl, lrfile);
if (list_empty(&todolist)) {
status = -ENOENT;
diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index 76427d0..ec77175 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -110,8 +110,6 @@ struct nfsd4_pnfs_layoutcommit_res {
u64 lc_newsize; /* response */
};

-#define PNFS_LAST_LAYOUT_NO_RECALLS ((void *)-1) /* used with lr_cookie below */
-
enum layoutreturn_flags {
LR_FLAG_INTERN = 1 << 0, /* internal return */
LR_FLAG_EXPIRE = 1 << 1, /* return on client expiration */
--
1.7.11.7


2013-01-21 14:54:26

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 2/9] SQUASHME: pnfsd: return_on_close all layout segments

As per errata #3226
http://www.rfc-editor.org/errata_search.php?rfc=5661
return_on_close refers to all segments acquired by the client
not only those marked with logr_return_on_close

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

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 5c50d8a..fe3f693 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1242,7 +1242,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
bool empty;

/* Check for a match */
- if (!lo->lo_state->ls_roc || lo->lo_client != clp)
+ if (lo->lo_client != clp)
continue;

/* Return the layout */
--
1.7.11.7


2013-01-21 14:54:55

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 7/9] pnfsd: LR_FLAG_EMPTY layout return flag

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 8 +++++++-
include/linux/nfsd/nfsd4_pnfs.h | 1 +
2 files changed, 8 insertions(+), 1 deletion(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index 2d4f23d..0b8c502 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -921,15 +921,21 @@ struct super_block *
struct inode *inode = lo->lo_file->fi_inode;
struct nfsd4_pnfs_layoutreturn lr;
bool empty;
+ int lr_flags = flags;

memset(&lr, 0, sizeof(lr));
lr.args.lr_return_type = RETURN_FILE;
lr.args.lr_seg = lo->lo_seg;

+ spin_lock(&layout_lock);
+ if (list_empty(&lo->lo_file->fi_layouts))
+ lr_flags |= LR_FLAG_EMPTY;
+ spin_unlock(&layout_lock);
+
list_del(&lo->lo_perfile);
empty = list_empty(lo_destroy_list);

- fs_layout_return(inode, &lr, flags, empty ? cb_cookie : NULL);
+ fs_layout_return(inode, &lr, lr_flags, empty ? cb_cookie : NULL);

destroy_layout(lo); /* this will put the lo_file */
}
diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index bfea9a6..ecb412e 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -114,6 +114,7 @@ enum layoutreturn_flags {
LR_FLAG_INTERN = 1 << 0, /* internal return */
LR_FLAG_EXPIRE = 1 << 1, /* return on client expiration */
LR_FLAG_CL_EMPTY = 1 << 2, /* No more layout for returning client */
+ LR_FLAG_EMPTY = 1 << 3, /* No more layout for file */
};

struct nfsd4_pnfs_layoutreturn_arg {
--
1.7.11.7


2013-01-21 14:54:32

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 3/9] SQUASHME: pnfsd: do not dequeue layout state on destroy_layout

the layout state may be shared by a number of layout segments,
each held by struct nfs4_layout and we may be destroying just one of them
e.g. on layoutreturn.

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

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index fe3f693..c62af3d 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -345,7 +345,6 @@ static void update_layout_roc(struct nfs4_layout_state *ls, bool roc)
__func__, lp, clp, fp, fp->fi_inode);

kmem_cache_free(pnfs_layout_slab, lp);
- list_del_init(&ls->ls_perfile);
/* release references taken by init_layout */
put_layout_state(ls);
put_nfs4_file(fp);
@@ -1249,6 +1248,7 @@ void pnfsd_roc(struct nfs4_client *clp, struct nfs4_file *fp)
memset(&lr, 0, sizeof(lr));
lr.args.lr_return_type = RETURN_FILE;
lr.args.lr_seg = lo->lo_seg;
+ list_del_init(&lo->lo_state->ls_perfile); /* just to be on the safe side */
dequeue_layout(lo);
destroy_layout(lo); /* do not access lp after this */

@@ -1301,6 +1301,7 @@ void pnfs_expire_client(struct nfs4_client *clp)
lr.args.lr_seg = lp->lo_seg;
empty = list_empty(&lp->lo_file->fi_layouts);
BUG_ON(lp->lo_client != clp);
+ list_del_init(&lp->lo_state->ls_perfile); /* just to be on the safe side */
dequeue_layout(lp);
destroy_layout(lp); /* do not access lp after this */
}
--
1.7.11.7


2013-01-21 14:54:49

by Benny Halevy

[permalink] [raw]
Subject: [PATCH 6/9] pnfsd: LR_FLAG_CL_EMPTY layout return flag

Signed-off-by: Benny Halevy <[email protected]>
---
fs/nfsd/nfs4pnfsd.c | 17 +++++++++++++----
include/linux/nfsd/nfsd4_pnfs.h | 1 +
2 files changed, 14 insertions(+), 4 deletions(-)

diff --git a/fs/nfsd/nfs4pnfsd.c b/fs/nfsd/nfs4pnfsd.c
index d48abba..2d4f23d 100644
--- a/fs/nfsd/nfs4pnfsd.c
+++ b/fs/nfsd/nfs4pnfsd.c
@@ -1064,6 +1064,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
u64 ex_fsid = current_fh->fh_export->ex_fsid;
void *recall_cookie = NULL;
LIST_HEAD(lo_destroy_list);
+ int lr_flags = 0;

dprintk("NFSD: %s\n", __func__);

@@ -1098,9 +1099,12 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
layouts_found = pnfs_return_file_layouts(clp, fp, lrp, ls,
&lo_destroy_list);
put_layout_state(ls);
+ if (!lrp->lrs_present)
+ lr_flags |= LR_FLAG_CL_EMPTY;
} else {
layouts_found = pnfs_return_client_layouts(clp, lrp, ex_fsid,
&lo_destroy_list);
+ lr_flags |= LR_FLAG_CL_EMPTY;
}

dprintk("pNFS %s: clp %p fp %p layout_type 0x%x iomode %d "
@@ -1136,7 +1140,7 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
nfs4_unlock_state();

pnfsd_return_lo_list(&lo_destroy_list, ino ? ino : sb->s_root->d_inode,
- lrp, 0, recall_cookie);
+ lrp, lr_flags, recall_cookie);

out_no_fs_call:
dprintk("pNFS %s: exit status %d\n", __func__, status);
@@ -1252,24 +1256,29 @@ int nfs4_pnfs_return_layout(struct super_block *sb, struct svc_fh *current_fh,
struct inode *inode;
void *recall_cookie;
LIST_HEAD(lo_destroy_list);
+ int lr_flags = LR_FLAG_INTERN;

dprintk("%s: clp %p fp %p: simulating layout_return\n", __func__,
clr->clr_client, clr->clr_file);

- if (clr->cb.cbl_recall_type == RETURN_FILE)
+ if (clr->cb.cbl_recall_type == RETURN_FILE) {
pnfs_return_file_layouts(clr->clr_client, clr->clr_file, &lr,
NULL, &lo_destroy_list);
- else
+ if (!lr.lrs_present)
+ lr_flags |= LR_FLAG_CL_EMPTY;
+ } else {
pnfs_return_client_layouts(clr->clr_client, &lr,
clr->cb.cbl_fsid.major,
&lo_destroy_list);
+ lr_flags |= LR_FLAG_CL_EMPTY;
+ }

spin_lock(&layout_lock);
recall_cookie = layoutrecall_done(clr);
spin_unlock(&layout_lock);

inode = clr->clr_file->fi_inode ?: clr->clr_sb->s_root->d_inode;
- pnfsd_return_lo_list(&lo_destroy_list, inode, &lr, LR_FLAG_INTERN,
+ pnfsd_return_lo_list(&lo_destroy_list, inode, &lr, lr_flags,
recall_cookie);
}

diff --git a/include/linux/nfsd/nfsd4_pnfs.h b/include/linux/nfsd/nfsd4_pnfs.h
index ec77175..bfea9a6 100644
--- a/include/linux/nfsd/nfsd4_pnfs.h
+++ b/include/linux/nfsd/nfsd4_pnfs.h
@@ -113,6 +113,7 @@ struct nfsd4_pnfs_layoutcommit_res {
enum layoutreturn_flags {
LR_FLAG_INTERN = 1 << 0, /* internal return */
LR_FLAG_EXPIRE = 1 << 1, /* return on client expiration */
+ LR_FLAG_CL_EMPTY = 1 << 2, /* No more layout for returning client */
};

struct nfsd4_pnfs_layoutreturn_arg {
--
1.7.11.7