Plus some code cleanup
0001-SQUASHME-pnfs-submit-remove-unused-filelayout_mount_.patch
and some bug fixes
0002-SQUASHME-pnfs-submit-pnfs_try_to_read-write-commit-u.patch
0003-SQUASHME-pnfs-submit-tell-commit-to-use-the-MDS.patch
Remove unused (by file layout) encode_layoutreturn io operation
0004-SQUASHME-pnfs-submit-remove-encode_layoutreturn.patch
0005-SQUASHME-pnfs-submit-add-error-handling-to-layout-re.patch
0006-SQUASHME-pnfs-submit-handle-assassinated-layoutcommi.patch
0007-SQUASHME-pnfs-submit-add-error-handlers-to-layout-ge.patch
Add back encode_layoutreturn io operation
0008-pnfs-post-submit-restore-encode_layoutreturn.patch
Testing:
---------
CONFIG_NFS_V4_1 set: NFSv4.0 NFSv4.1 pNFS
Passes Connectathon tests
Tested layoutget and layoutreturn recovery from NFS4ERR_DEAD_SESSION with the
pyNFS server and the testclient framework.
Still todo:
Recover from NFS4ERR_BAD_STATEID. Currently layoutreturn, layoutget, and
layoutcommit do not pass nfs_stste to the error handlers.
Handle NFS4ERR_BAD_LAYOUT.
CONFIG_NFS_V4_1 not set: NFSv4.o mount passes cthon tests.
-->Andy
From: Andy Adamson <[email protected]>
Redo pnfs4_proc_layoutget() logic to match the other async operations that call
nfs4_wait_for_completion_rpc_task.
pnfs_get_layout_done passes session and delay errors. Handle them with the
generic error handlers.
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 45 +++++++++++++++++++++++++++++----------------
1 files changed, 29 insertions(+), 16 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 35e8a56..6283996 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5446,6 +5446,10 @@ static void nfs4_pnfs_layoutget_done(struct rpc_task *task, void *calldata)
return;
pnfs_get_layout_done(lgp, task->tk_status);
+
+ if (nfs4_async_handle_error(task, server, NULL, NULL) == -EAGAIN)
+ nfs_restart_rpc(task, server->nfs_client);
+
dprintk("<-- %s\n", __func__);
}
@@ -5471,10 +5475,9 @@ static const struct rpc_call_ops nfs4_pnfs_layoutget_call_ops = {
* and deal with retries.
* Currently we can't since we release lgp and its contents.
*/
-int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
+static int _pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
{
- struct inode *ino = lgp->args.inode;
- struct nfs_server *server = NFS_SERVER(ino);
+ struct nfs_server *server = NFS_SERVER(lgp->args.inode);
struct rpc_task *task;
struct rpc_message msg = {
.rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_PNFS_LAYOUTGET],
@@ -5488,35 +5491,45 @@ int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
.callback_data = lgp,
.flags = RPC_TASK_ASYNC,
};
- int status;
+ int status = 0;
dprintk("--> %s\n", __func__);
lgp->res.layout.buf = (void *)__get_free_page(GFP_NOFS);
if (lgp->res.layout.buf == NULL) {
nfs4_pnfs_layoutget_release(lgp);
- status = -ENOMEM;
- goto out;
+ return -ENOMEM;
}
lgp->res.seq_res.sr_slotid = NFS4_MAX_SLOT_TABLE;
task = rpc_run_task(&task_setup_data);
- if (IS_ERR(task)) {
- status = PTR_ERR(task);
- goto out;
- }
+ if (IS_ERR(task))
+ return PTR_ERR(task);
status = nfs4_wait_for_completion_rpc_task(task);
- if (status == 0) {
- status = lgp->status;
- if (status == 0)
- status = pnfs_layout_process(lgp);
- }
- rpc_put_task(task);
+ if (status != 0)
+ goto out;
+ status = lgp->status;
+ if (status != 0)
+ goto out;
+ status = pnfs_layout_process(lgp);
out:
+ rpc_put_task(task);
dprintk("<-- %s status=%d\n", __func__, status);
return status;
}
+int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
+{
+ struct nfs_server *server = NFS_SERVER(lgp->args.inode);
+ struct nfs4_exception exception = { };
+ int err;
+ do {
+ err = nfs4_handle_exception(server, _pnfs4_proc_layoutget(lgp),
+ &exception);
+ } while (exception.retry);
+ return err;
+}
+
static void pnfs_layoutcommit_prepare(struct rpc_task *task, void *data)
{
struct pnfs_layoutcommit_data *ldata =
--
1.6.6
From: Andy Adamson <[email protected]>
Not used by the file layout
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4xdr.c | 15 ++-------------
include/linux/nfs4_pnfs.h | 4 ----
2 files changed, 2 insertions(+), 17 deletions(-)
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index a509bc1..fb50361 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1874,24 +1874,13 @@ encode_layoutreturn(struct xdr_stream *xdr,
*p++ = cpu_to_be32(args->lseg.iomode);
*p = cpu_to_be32(args->return_type);
if (args->return_type == RETURN_FILE) {
- struct layoutdriver_io_operations *ld_io_ops =
- NFS_SERVER(args->inode)->pnfs_curr_ld->ld_io_ops;
-
p = reserve_space(xdr, 16 + NFS4_STATEID_SIZE);
p = xdr_encode_hyper(p, args->lseg.offset);
p = xdr_encode_hyper(p, args->lseg.length);
p = xdr_encode_opaque_fixed(p, &args->stateid.u.data,
NFS4_STATEID_SIZE);
-
- dprintk("%s: call %pF\n", __func__,
- ld_io_ops->encode_layoutreturn);
- if (ld_io_ops->encode_layoutreturn) {
- ld_io_ops->encode_layoutreturn(
- &NFS_I(args->inode)->layout, xdr, args);
- } else {
- p = reserve_space(xdr, 4);
- *p = cpu_to_be32(0);
- }
+ p = reserve_space(xdr, 4);
+ *p = cpu_to_be32(0);
}
hdr->nops++;
hdr->replen += decode_layoutreturn_maxsz;
diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
index d20b5de..782fdd9 100644
--- a/include/linux/nfs4_pnfs.h
+++ b/include/linux/nfs4_pnfs.h
@@ -154,10 +154,6 @@ struct layoutdriver_io_operations {
struct pnfs_layout_segment * (*alloc_lseg) (struct pnfs_layout_type *layoutid, struct nfs4_pnfs_layoutget_res *lgr);
void (*free_lseg) (struct pnfs_layout_segment *lseg);
- void (*encode_layoutreturn) (struct pnfs_layout_type *layoutid,
- struct xdr_stream *xdr,
- const struct nfs4_pnfs_layoutreturn_arg *args);
-
/* Registration information for a new mounted file system
*/
int (*initialize_mountpoint) (struct nfs_client *);
--
1.6.6
From: Andy Adamson <[email protected]>
The nfs_read_data or nfs_write_data is freed during rpc call.
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/pnfs.h | 9 ++++++---
1 files changed, 6 insertions(+), 3 deletions(-)
diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 78776f3..c60eff6 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -105,13 +105,14 @@ static inline enum pnfs_try_status
pnfs_try_to_read_data(struct nfs_read_data *data,
const struct rpc_call_ops *call_ops)
{
+ struct inode *inode = data->inode;
enum pnfs_try_status ret;
if (!data->req->wb_lseg)
return PNFS_NOT_ATTEMPTED;
ret = _pnfs_try_to_read_data(data, call_ops);
if (ret == PNFS_ATTEMPTED)
- nfs_inc_stats(data->inode, NFSIOS_PNFS_READ);
+ nfs_inc_stats(inode, NFSIOS_PNFS_READ);
else
_pnfs_clear_lseg_from_pages(&data->pages);
return ret;
@@ -122,13 +123,14 @@ pnfs_try_to_write_data(struct nfs_write_data *data,
const struct rpc_call_ops *call_ops,
int how)
{
+ struct inode *inode = data->inode;
enum pnfs_try_status ret;
if (!data->req->wb_lseg)
return PNFS_NOT_ATTEMPTED;
ret = _pnfs_try_to_write_data(data, call_ops, how);
if (ret == PNFS_ATTEMPTED)
- nfs_inc_stats(data->inode, NFSIOS_PNFS_WRITE);
+ nfs_inc_stats(inode, NFSIOS_PNFS_WRITE);
else
_pnfs_clear_lseg_from_pages(&data->pages);
return ret;
@@ -139,6 +141,7 @@ pnfs_try_to_commit(struct nfs_write_data *data,
const struct rpc_call_ops *call_ops,
int how)
{
+ struct inode *inode = data->inode;
enum pnfs_try_status ret;
/* Unlike in pnfs_try_to_write_data and pnfs_try_to_read_data,
@@ -148,7 +151,7 @@ pnfs_try_to_commit(struct nfs_write_data *data,
*/
ret = _pnfs_try_to_commit(data, call_ops, how);
if (ret == PNFS_ATTEMPTED)
- nfs_inc_stats(data->inode, NFSIOS_PNFS_COMMIT);
+ nfs_inc_stats(inode, NFSIOS_PNFS_COMMIT);
else
_pnfs_clear_lseg_from_pages(&data->pages);
return ret;
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4xdr.c | 14 ++++++++++++--
include/linux/nfs4_pnfs.h | 4 ++++
2 files changed, 16 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4xdr.c b/fs/nfs/nfs4xdr.c
index fb50361..eeee855 100644
--- a/fs/nfs/nfs4xdr.c
+++ b/fs/nfs/nfs4xdr.c
@@ -1874,13 +1874,23 @@ encode_layoutreturn(struct xdr_stream *xdr,
*p++ = cpu_to_be32(args->lseg.iomode);
*p = cpu_to_be32(args->return_type);
if (args->return_type == RETURN_FILE) {
+ struct layoutdriver_io_operations *ld_io_ops =
+ NFS_SERVER(args->inode)->pnfs_curr_ld->ld_io_ops;
+
p = reserve_space(xdr, 16 + NFS4_STATEID_SIZE);
p = xdr_encode_hyper(p, args->lseg.offset);
p = xdr_encode_hyper(p, args->lseg.length);
p = xdr_encode_opaque_fixed(p, &args->stateid.u.data,
NFS4_STATEID_SIZE);
- p = reserve_space(xdr, 4);
- *p = cpu_to_be32(0);
+ dprintk("%s: call %pF\n", __func__,
+ ld_io_ops->encode_layoutreturn);
+ if (ld_io_ops->encode_layoutreturn) {
+ ld_io_ops->encode_layoutreturn(
+ &NFS_I(args->inode)->layout, xdr, args);
+ } else {
+ p = reserve_space(xdr, 4);
+ *p = cpu_to_be32(0);
+ }
}
hdr->nops++;
hdr->replen += decode_layoutreturn_maxsz;
diff --git a/include/linux/nfs4_pnfs.h b/include/linux/nfs4_pnfs.h
index 782fdd9..d20b5de 100644
--- a/include/linux/nfs4_pnfs.h
+++ b/include/linux/nfs4_pnfs.h
@@ -154,6 +154,10 @@ struct layoutdriver_io_operations {
struct pnfs_layout_segment * (*alloc_lseg) (struct pnfs_layout_type *layoutid, struct nfs4_pnfs_layoutget_res *lgr);
void (*free_lseg) (struct pnfs_layout_segment *lseg);
+ void (*encode_layoutreturn) (struct pnfs_layout_type *layoutid,
+ struct xdr_stream *xdr,
+ const struct nfs4_pnfs_layoutreturn_arg *args);
+
/* Registration information for a new mounted file system
*/
int (*initialize_mountpoint) (struct nfs_client *);
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/pnfs.c | 1 +
1 files changed, 1 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 9a295cf..c1a8066 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1454,6 +1454,7 @@ pnfs_writepages(struct nfs_write_data *wdata, int how)
wdata);
if (trypnfs == PNFS_NOT_ATTEMPTED) {
+ wdata->req->wb_lseg = NULL;
wdata->pdata.lseg = NULL;
put_lseg(lseg);
}
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 2 ++
1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 0ab875f..35e8a56 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5539,6 +5539,8 @@ pnfs_layoutcommit_done(struct rpc_task *task, void *calldata)
data->status = task->tk_status;
nfs4_sequence_done(server, &data->res.seq_res, task->tk_status);
+ if (RPC_ASSASSINATED(task))
+ return;
if (nfs4_async_handle_error(task, server, NULL, NULL) == -EAGAIN)
nfs_restart_rpc(task, server->nfs_client);
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4filelayout.h | 6 ------
1 files changed, 0 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4filelayout.h b/fs/nfs/nfs4filelayout.h
index 29e481d..de8391f 100644
--- a/fs/nfs/nfs4filelayout.h
+++ b/fs/nfs/nfs4filelayout.h
@@ -24,8 +24,6 @@
#define NFS4_PNFS_MAX_MULTI_CNT 64 /* 256 fit into a u8 stripe_index */
#define NFS4_PNFS_MAX_MULTI_DS 2
-#define FILE_MT(inode) ((struct filelayout_mount_type *) \
- (NFS_SERVER(inode)->pnfs_mountid->mountid))
#define FILE_DSADDR(lseg) (container_of(lseg->deviceid, \
struct nfs4_file_layout_dsaddr, \
deviceid))
@@ -76,10 +74,6 @@ struct nfs4_filelayout {
u32 stripe_unit;
};
-struct filelayout_mount_type {
- struct super_block *fl_sb;
-};
-
extern struct nfs_fh *
nfs4_fl_select_ds_fh(struct pnfs_layout_segment *lseg, loff_t offset);
--
1.6.6
From: Andy Adamson <[email protected]>
Signed-off-by: Andy Adamson <[email protected]>
---
fs/nfs/nfs4proc.c | 37 +++++++++++++++++++++++++++----------
1 files changed, 27 insertions(+), 10 deletions(-)
diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
index 5ffdfe1..0ab875f 100644
--- a/fs/nfs/nfs4proc.c
+++ b/fs/nfs/nfs4proc.c
@@ -5641,6 +5641,11 @@ static void nfs4_pnfs_layoutreturn_done(struct rpc_task *task, void *calldata)
dprintk("--> %s\n", __func__);
nfs4_sequence_done(server, &lrp->res.seq_res, task->tk_status);
+ if (RPC_ASSASSINATED(task))
+ return;
+
+ if (nfs4_async_handle_error(task, server, NULL, NULL) == -EAGAIN)
+ nfs_restart_rpc(task, server->nfs_client);
dprintk("<-- %s\n", __func__);
}
@@ -5667,7 +5672,7 @@ static const struct rpc_call_ops nfs4_pnfs_layoutreturn_call_ops = {
.rpc_release = nfs4_pnfs_layoutreturn_release,
};
-int pnfs4_proc_layoutreturn(struct nfs4_pnfs_layoutreturn *lrp, bool wait)
+int _pnfs4_proc_layoutreturn(struct nfs4_pnfs_layoutreturn *lrp, bool issync)
{
struct inode *ino = lrp->args.inode;
struct nfs_server *server = NFS_SERVER(ino);
@@ -5689,21 +5694,33 @@ int pnfs4_proc_layoutreturn(struct nfs4_pnfs_layoutreturn *lrp, bool wait)
dprintk("--> %s\n", __func__);
lrp->res.seq_res.sr_slotid = NFS4_MAX_SLOT_TABLE;
task = rpc_run_task(&task_setup_data);
- if (IS_ERR(task)) {
- status = PTR_ERR(task);
+ if (IS_ERR(task))
+ return PTR_ERR(task);
+ if (!issync)
goto out;
- }
- if (wait) {
- status = nfs4_wait_for_completion_rpc_task(task);
- if (status == 0)
- status = task->tk_status;
- }
- rpc_put_task(task);
+ status = nfs4_wait_for_completion_rpc_task(task);
+ if (status != 0)
+ goto out;
+ status = task->tk_status;
out:
dprintk("<-- %s\n", __func__);
+ rpc_put_task(task);
return status;
}
+int pnfs4_proc_layoutreturn(struct nfs4_pnfs_layoutreturn *lrp, bool issync)
+{
+ struct nfs_server *server = NFS_SERVER(lrp->args.inode);
+ struct nfs4_exception exception = { };
+ int err;
+ do {
+ err = nfs4_handle_exception(server,
+ _pnfs4_proc_layoutreturn(lrp, issync),
+ &exception);
+ } while (exception.retry);
+ return err;
+}
+
int nfs4_pnfs_getdeviceinfo(struct nfs_server *server, struct pnfs_device *pdev)
{
struct nfs4_pnfs_getdeviceinfo_arg args = {
--
1.6.6
On Jun. 23, 2010, 0:56 +0300, [email protected] wrote:
> From: Andy Adamson <[email protected]>
>
> Redo pnfs4_proc_layoutget() logic to match the other async operations that call
> nfs4_wait_for_completion_rpc_task.
>
> pnfs_get_layout_done passes session and delay errors. Handle them with the
> generic error handlers.
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> fs/nfs/nfs4proc.c | 45 +++++++++++++++++++++++++++++----------------
> 1 files changed, 29 insertions(+), 16 deletions(-)
>
> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
> index 35e8a56..6283996 100644
> --- a/fs/nfs/nfs4proc.c
> +++ b/fs/nfs/nfs4proc.c
> @@ -5446,6 +5446,10 @@ static void nfs4_pnfs_layoutget_done(struct rpc_task *task, void *calldata)
> return;
>
> pnfs_get_layout_done(lgp, task->tk_status);
> +
> + if (nfs4_async_handle_error(task, server, NULL, NULL) == -EAGAIN)
> + nfs_restart_rpc(task, server->nfs_client);
> +
> dprintk("<-- %s\n", __func__);
> }
>
> @@ -5471,10 +5475,9 @@ static const struct rpc_call_ops nfs4_pnfs_layoutget_call_ops = {
> * and deal with retries.
> * Currently we can't since we release lgp and its contents.
> */
> -int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
> +static int _pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
> {
> - struct inode *ino = lgp->args.inode;
> - struct nfs_server *server = NFS_SERVER(ino);
> + struct nfs_server *server = NFS_SERVER(lgp->args.inode);
> struct rpc_task *task;
> struct rpc_message msg = {
> .rpc_proc = &nfs4_procedures[NFSPROC4_CLNT_PNFS_LAYOUTGET],
> @@ -5488,35 +5491,45 @@ int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
> .callback_data = lgp,
> .flags = RPC_TASK_ASYNC,
> };
> - int status;
> + int status = 0;
>
> dprintk("--> %s\n", __func__);
>
> lgp->res.layout.buf = (void *)__get_free_page(GFP_NOFS);
> if (lgp->res.layout.buf == NULL) {
> nfs4_pnfs_layoutget_release(lgp);
> - status = -ENOMEM;
> - goto out;
> + return -ENOMEM;
This will bypass the dprintk.
How about adding a out_no_task label after the call to rpc_put_task?
> }
>
> lgp->res.seq_res.sr_slotid = NFS4_MAX_SLOT_TABLE;
> task = rpc_run_task(&task_setup_data);
> - if (IS_ERR(task)) {
> - status = PTR_ERR(task);
> - goto out;
> - }
> + if (IS_ERR(task))
> + return PTR_ERR(task);
ditto.
Benny
> status = nfs4_wait_for_completion_rpc_task(task);
> - if (status == 0) {
> - status = lgp->status;
> - if (status == 0)
> - status = pnfs_layout_process(lgp);
> - }
> - rpc_put_task(task);
> + if (status != 0)
> + goto out;
> + status = lgp->status;
> + if (status != 0)
> + goto out;
> + status = pnfs_layout_process(lgp);
> out:
> + rpc_put_task(task);
> dprintk("<-- %s status=%d\n", __func__, status);
> return status;
> }
>
> +int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
> +{
> + struct nfs_server *server = NFS_SERVER(lgp->args.inode);
> + struct nfs4_exception exception = { };
> + int err;
> + do {
> + err = nfs4_handle_exception(server, _pnfs4_proc_layoutget(lgp),
> + &exception);
> + } while (exception.retry);
> + return err;
> +}
> +
> static void pnfs_layoutcommit_prepare(struct rpc_task *task, void *data)
> {
> struct pnfs_layoutcommit_data *ldata =
On Wed, Jun 23, 2010 at 3:41 AM, Benny Halevy <[email protected]> wro=
te:
> On Jun. 23, 2010, 0:56 +0300, [email protected] wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Redo pnfs4_proc_layoutget() logic to match the other async operation=
s that call
>> nfs4_wait_for_completion_rpc_task.
>>
>> pnfs_get_layout_done passes session and delay errors. Handle them wi=
th the
>> generic error handlers.
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> =A0fs/nfs/nfs4proc.c | =A0 45 +++++++++++++++++++++++++++++---------=
-------
>> =A01 files changed, 29 insertions(+), 16 deletions(-)
>>
>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>> index 35e8a56..6283996 100644
>> --- a/fs/nfs/nfs4proc.c
>> +++ b/fs/nfs/nfs4proc.c
>> @@ -5446,6 +5446,10 @@ static void nfs4_pnfs_layoutget_done(struct r=
pc_task *task, void *calldata)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>>
>> =A0 =A0 =A0 pnfs_get_layout_done(lgp, task->tk_status);
>> +
>> + =A0 =A0 if (nfs4_async_handle_error(task, server, NULL, NULL) =3D=3D=
-EAGAIN)
>> + =A0 =A0 =A0 =A0 =A0 =A0 nfs_restart_rpc(task, server->nfs_client);
>> +
>> =A0 =A0 =A0 dprintk("<-- %s\n", __func__);
>> =A0}
>>
>> @@ -5471,10 +5475,9 @@ static const struct rpc_call_ops nfs4_pnfs_la=
youtget_call_ops =3D {
>> =A0 * and deal with retries.
>> =A0 * Currently we can't since we release lgp and its contents.
>> =A0 */
>> -int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
>> +static int _pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
>> =A0{
>> - =A0 =A0 struct inode *ino =3D lgp->args.inode;
>> - =A0 =A0 struct nfs_server *server =3D NFS_SERVER(ino);
>> + =A0 =A0 struct nfs_server *server =3D NFS_SERVER(lgp->args.inode);
>> =A0 =A0 =A0 struct rpc_task *task;
>> =A0 =A0 =A0 struct rpc_message msg =3D {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .rpc_proc =3D &nfs4_procedures[NFSPROC4_=
CLNT_PNFS_LAYOUTGET],
>> @@ -5488,35 +5491,45 @@ int pnfs4_proc_layoutget(struct nfs4_pnfs_la=
youtget *lgp)
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .callback_data =3D lgp,
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .flags =3D RPC_TASK_ASYNC,
>> =A0 =A0 =A0 };
>> - =A0 =A0 int status;
>> + =A0 =A0 int status =3D 0;
>>
>> =A0 =A0 =A0 dprintk("--> %s\n", __func__);
>>
>> =A0 =A0 =A0 lgp->res.layout.buf =3D (void *)__get_free_page(GFP_NOFS=
);
>> =A0 =A0 =A0 if (lgp->res.layout.buf =3D=3D NULL) {
>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 nfs4_pnfs_layoutget_release(lgp);
>> - =A0 =A0 =A0 =A0 =A0 =A0 status =3D -ENOMEM;
>> - =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> + =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
>
> This will bypass the dprintk.
> How about adding a out_no_task label after the call to rpc_put_task?
Actually, I think we need to lose all of these dprintk's. Look at
_nfs4_proc_delegreturn for example...
-->Andy
>
>> =A0 =A0 =A0 }
>>
>> =A0 =A0 =A0 lgp->res.seq_res.sr_slotid =3D NFS4_MAX_SLOT_TABLE;
>> =A0 =A0 =A0 task =3D rpc_run_task(&task_setup_data);
>> - =A0 =A0 if (IS_ERR(task)) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 status =3D PTR_ERR(task);
>> - =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> - =A0 =A0 }
>> + =A0 =A0 if (IS_ERR(task))
>> + =A0 =A0 =A0 =A0 =A0 =A0 return PTR_ERR(task);
>
> ditto
>
> Benny
>
>> =A0 =A0 =A0 status =3D nfs4_wait_for_completion_rpc_task(task);
>> - =A0 =A0 if (status =3D=3D 0) {
>> - =A0 =A0 =A0 =A0 =A0 =A0 status =3D lgp->status;
>> - =A0 =A0 =A0 =A0 =A0 =A0 if (status =3D=3D 0)
>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 status =3D pnfs_layout_pro=
cess(lgp);
>> - =A0 =A0 }
>> - =A0 =A0 rpc_put_task(task);
>> + =A0 =A0 if (status !=3D 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> + =A0 =A0 status =3D lgp->status;
>> + =A0 =A0 if (status !=3D 0)
>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>> + =A0 =A0 status =3D pnfs_layout_process(lgp);
>> =A0out:
>> + =A0 =A0 rpc_put_task(task);
>> =A0 =A0 =A0 dprintk("<-- %s status=3D%d\n", __func__, status);
>> =A0 =A0 =A0 return status;
>> =A0}
>>
>> +int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
>> +{
>> + =A0 =A0 struct nfs_server *server =3D NFS_SERVER(lgp->args.inode);
>> + =A0 =A0 struct nfs4_exception exception =3D { };
>> + =A0 =A0 int err;
>> + =A0 =A0 do {
>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D nfs4_handle_exception(server, _pnf=
s4_proc_layoutget(lgp),
>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 &exception);
>> + =A0 =A0 } while (exception.retry);
>> + =A0 =A0 return err;
>> +}
>> +
>> =A0static void pnfs_layoutcommit_prepare(struct rpc_task *task, void=
*data)
>> =A0{
>> =A0 =A0 =A0 struct pnfs_layoutcommit_data *ldata =3D
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
On Tue, Jun 22, 2010 at 5:56 PM, <[email protected]> wrote:
> From: Andy Adamson <[email protected]>
>
> Signed-off-by: Andy Adamson <[email protected]>
> ---
> =A0fs/nfs/pnfs.c | =A0 =A01 +
> =A01 files changed, 1 insertions(+), 0 deletions(-)
>
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index 9a295cf..c1a8066 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1454,6 +1454,7 @@ pnfs_writepages(struct nfs_write_data *wdata, i=
nt how)
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wdata);
>
> =A0 =A0 =A0 =A0if (trypnfs =3D=3D PNFS_NOT_ATTEMPTED) {
> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 wdata->req->wb_lseg =3D NULL;
This is wrong. It will be taken care of by
_pnfs_clear_lseg_from_pages called by pnfs_try_to_write_data.
=46red
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0wdata->pdata.lseg =3D NULL;
> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0put_lseg(lseg);
> =A0 =A0 =A0 =A0}
> --
> 1.6.6
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-nfs" =
in
> the body of a message to [email protected]
> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>
On Jun 23, 2010, at 9:34 AM, Fred Isaman wrote:
> On Tue, Jun 22, 2010 at 5:56 PM, <[email protected]> wrote:
>> From: Andy Adamson <[email protected]>
>>
>> Signed-off-by: Andy Adamson <[email protected]>
>> ---
>> fs/nfs/pnfs.c | 1 +
>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>
>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>> index 9a295cf..c1a8066 100644
>> --- a/fs/nfs/pnfs.c
>> +++ b/fs/nfs/pnfs.c
>> @@ -1454,6 +1454,7 @@ pnfs_writepages(struct nfs_write_data *wdata,
>> int how)
>> wdata);
>>
>> if (trypnfs == PNFS_NOT_ATTEMPTED) {
>> + wdata->req->wb_lseg = NULL;
>
> This is wrong. It will be taken care of by
> _pnfs_clear_lseg_from_pages called by pnfs_try_to_write_data.
A partial write, e.g. one that uses nfs_write_partial_ops, does not
set nfs_write_data->pages, so _pnfs_clear_lseg_from_pages does not
take care of clearing the req->wb_lseg.
Perhaps it would be cleaner to add this case to
_pnfs_clear_lseg_from_pages.
-->Andy
>
> Fred
>
>> wdata->pdata.lseg = NULL;
>> put_lseg(lseg);
>> }
>> --
>> 1.6.6
>>
>> --
>> 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
>>
On Wed, Jun 23, 2010 at 9:53 AM, Andy Adamson <[email protected]> wrote=
:
>
> On Jun 23, 2010, at 9:34 AM, Fred Isaman wrote:
>
>> On Tue, Jun 22, 2010 at 5:56 PM, =A0<[email protected]> wrote:
>>>
>>> From: Andy Adamson <[email protected]>
>>>
>>> Signed-off-by: Andy Adamson <[email protected]>
>>> ---
>>> =A0fs/nfs/pnfs.c | =A0 =A01 +
>>> =A01 files changed, 1 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>>> index 9a295cf..c1a8066 100644
>>> --- a/fs/nfs/pnfs.c
>>> +++ b/fs/nfs/pnfs.c
>>> @@ -1454,6 +1454,7 @@ pnfs_writepages(struct nfs_write_data *wdata,=
int
>>> how)
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 wdata);
>>>
>>> =A0 =A0 =A0 if (trypnfs =3D=3D PNFS_NOT_ATTEMPTED) {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 wdata->req->wb_lseg =3D NULL;
>>
>> This is wrong. =A0It will be taken care of by
>> _pnfs_clear_lseg_from_pages called by pnfs_try_to_write_data.
>
> A partial write, e.g. one that uses nfs_write_partial_ops, does not s=
et
> nfs_write_data->pages, so _pnfs_clear_lseg_from_pages does not take c=
are of
> clearing the req->wb_lseg.
>
Hmmm, you are right.
> Perhaps it would be cleaner to add this case to _pnfs_clear_lseg_from=
_pages.
>
I'll look into this, but for now your patch does the correct thing.
=46red
> -->Andy
>
>>
>> Fred
>>
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 wdata->pdata.lseg =3D NULL;
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 put_lseg(lseg);
>>> =A0 =A0 =A0 }
>>> --
>>> 1.6.6
>>>
>>> --
>>> To unsubscribe from this list: send the line "unsubscribe linux-nfs=
" in
>>> the body of a message to [email protected]
>>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.htm=
l
>>>
>
>
On Jun 23, 2010, at 10:11 AM, Fred Isaman wrote:
> On Wed, Jun 23, 2010 at 9:53 AM, Andy Adamson <[email protected]>
> wrote:
>>
>> On Jun 23, 2010, at 9:34 AM, Fred Isaman wrote:
>>
>>> On Tue, Jun 22, 2010 at 5:56 PM, <[email protected]> wrote:
>>>>
>>>> From: Andy Adamson <[email protected]>
>>>>
>>>> Signed-off-by: Andy Adamson <[email protected]>
>>>> ---
>>>> fs/nfs/pnfs.c | 1 +
>>>> 1 files changed, 1 insertions(+), 0 deletions(-)
>>>>
>>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>>>> index 9a295cf..c1a8066 100644
>>>> --- a/fs/nfs/pnfs.c
>>>> +++ b/fs/nfs/pnfs.c
>>>> @@ -1454,6 +1454,7 @@ pnfs_writepages(struct nfs_write_data
>>>> *wdata, int
>>>> how)
>>>> wdata);
>>>>
>>>> if (trypnfs == PNFS_NOT_ATTEMPTED) {
>>>> + wdata->req->wb_lseg = NULL;
>>>
>>> This is wrong. It will be taken care of by
>>> _pnfs_clear_lseg_from_pages called by pnfs_try_to_write_data.
>>
>> A partial write, e.g. one that uses nfs_write_partial_ops, does not
>> set
>> nfs_write_data->pages, so _pnfs_clear_lseg_from_pages does not take
>> care of
>> clearing the req->wb_lseg.
>>
>
> Hmmm, you are right.
>
>> Perhaps it would be cleaner to add this case to
>> _pnfs_clear_lseg_from_pages.
>>
>
> I'll look into this, but for now your patch does the correct thing.
I can resend with it moved to _pnfs_clear_lseg_from_pages.
-->Andy
>
> Fred
>
>> -->Andy
>>
>>>
>>> Fred
>>>
>>>> wdata->pdata.lseg = NULL;
>>>> put_lseg(lseg);
>>>> }
>>>> --
>>>> 1.6.6
>>>>
>>>> --
>>>> 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
>>>>
>>
>>
On Wed, Jun 23, 2010 at 9:04 AM, William A. (Andy) Adamson
<[email protected]> wrote:
> On Wed, Jun 23, 2010 at 3:41 AM, Benny Halevy <[email protected]> w=
rote:
>> On Jun. 23, 2010, 0:56 +0300, [email protected] wrote:
>>> From: Andy Adamson <[email protected]>
>>>
>>> Redo pnfs4_proc_layoutget() logic to match the other async operatio=
ns that call
>>> nfs4_wait_for_completion_rpc_task.
>>>
>>> pnfs_get_layout_done passes session and delay errors. Handle them w=
ith the
>>> generic error handlers.
>>>
>>> Signed-off-by: Andy Adamson <[email protected]>
>>> ---
>>> =A0fs/nfs/nfs4proc.c | =A0 45 +++++++++++++++++++++++++++++--------=
--------
>>> =A01 files changed, 29 insertions(+), 16 deletions(-)
>>>
>>> diff --git a/fs/nfs/nfs4proc.c b/fs/nfs/nfs4proc.c
>>> index 35e8a56..6283996 100644
>>> --- a/fs/nfs/nfs4proc.c
>>> +++ b/fs/nfs/nfs4proc.c
>>> @@ -5446,6 +5446,10 @@ static void nfs4_pnfs_layoutget_done(struct =
rpc_task *task, void *calldata)
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 return;
>>>
>>> =A0 =A0 =A0 pnfs_get_layout_done(lgp, task->tk_status);
>>> +
>>> + =A0 =A0 if (nfs4_async_handle_error(task, server, NULL, NULL) =3D=
=3D -EAGAIN)
>>> + =A0 =A0 =A0 =A0 =A0 =A0 nfs_restart_rpc(task, server->nfs_client)=
;
>>> +
>>> =A0 =A0 =A0 dprintk("<-- %s\n", __func__);
>>> =A0}
>>>
>>> @@ -5471,10 +5475,9 @@ static const struct rpc_call_ops nfs4_pnfs_l=
ayoutget_call_ops =3D {
>>> =A0 * and deal with retries.
>>> =A0 * Currently we can't since we release lgp and its contents.
>>> =A0 */
>>> -int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
>>> +static int _pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
>>> =A0{
>>> - =A0 =A0 struct inode *ino =3D lgp->args.inode;
>>> - =A0 =A0 struct nfs_server *server =3D NFS_SERVER(ino);
>>> + =A0 =A0 struct nfs_server *server =3D NFS_SERVER(lgp->args.inode)=
;
>>> =A0 =A0 =A0 struct rpc_task *task;
>>> =A0 =A0 =A0 struct rpc_message msg =3D {
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .rpc_proc =3D &nfs4_procedures[NFSPROC4=
_CLNT_PNFS_LAYOUTGET],
>>> @@ -5488,35 +5491,45 @@ int pnfs4_proc_layoutget(struct nfs4_pnfs_l=
ayoutget *lgp)
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .callback_data =3D lgp,
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 .flags =3D RPC_TASK_ASYNC,
>>> =A0 =A0 =A0 };
>>> - =A0 =A0 int status;
>>> + =A0 =A0 int status =3D 0;
>>>
>>> =A0 =A0 =A0 dprintk("--> %s\n", __func__);
>>>
>>> =A0 =A0 =A0 lgp->res.layout.buf =3D (void *)__get_free_page(GFP_NOF=
S);
>>> =A0 =A0 =A0 if (lgp->res.layout.buf =3D=3D NULL) {
>>> =A0 =A0 =A0 =A0 =A0 =A0 =A0 nfs4_pnfs_layoutget_release(lgp);
>>> - =A0 =A0 =A0 =A0 =A0 =A0 status =3D -ENOMEM;
>>> - =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>>> + =A0 =A0 =A0 =A0 =A0 =A0 return -ENOMEM;
>>
>> This will bypass the dprintk.
>> How about adding a out_no_task label after the call to rpc_put_task?
>
> Actually, I think we need to lose all of these dprintk's. Look at
> _nfs4_proc_delegreturn for example...
Since pnfs4_proc_layoutget is only called by send_layout which prints
the status, I'll leave this alone for now.
Sometime soon we will need to purge the pnfs-submit branch of dprintks
-->Andy
>
> -->Andy
>
>>
>>> =A0 =A0 =A0 }
>>>
>>> =A0 =A0 =A0 lgp->res.seq_res.sr_slotid =3D NFS4_MAX_SLOT_TABLE;
>>> =A0 =A0 =A0 task =3D rpc_run_task(&task_setup_data);
>>> - =A0 =A0 if (IS_ERR(task)) {
>>> - =A0 =A0 =A0 =A0 =A0 =A0 status =3D PTR_ERR(task);
>>> - =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>>> - =A0 =A0 }
>>> + =A0 =A0 if (IS_ERR(task))
>>> + =A0 =A0 =A0 =A0 =A0 =A0 return PTR_ERR(task);
>>
>> ditto
>
>>
>> Benny
>>
>>> =A0 =A0 =A0 status =3D nfs4_wait_for_completion_rpc_task(task);
>>> - =A0 =A0 if (status =3D=3D 0) {
>>> - =A0 =A0 =A0 =A0 =A0 =A0 status =3D lgp->status;
>>> - =A0 =A0 =A0 =A0 =A0 =A0 if (status =3D=3D 0)
>>> - =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 status =3D pnfs_layout_pr=
ocess(lgp);
>>> - =A0 =A0 }
>>> - =A0 =A0 rpc_put_task(task);
>>> + =A0 =A0 if (status !=3D 0)
>>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>>> + =A0 =A0 status =3D lgp->status;
>>> + =A0 =A0 if (status !=3D 0)
>>> + =A0 =A0 =A0 =A0 =A0 =A0 goto out;
>>> + =A0 =A0 status =3D pnfs_layout_process(lgp);
>>> =A0out:
>>> + =A0 =A0 rpc_put_task(task);
>>> =A0 =A0 =A0 dprintk("<-- %s status=3D%d\n", __func__, status);
>>> =A0 =A0 =A0 return status;
>>> =A0}
>>>
>>> +int pnfs4_proc_layoutget(struct nfs4_pnfs_layoutget *lgp)
>>> +{
>>> + =A0 =A0 struct nfs_server *server =3D NFS_SERVER(lgp->args.inode)=
;
>>> + =A0 =A0 struct nfs4_exception exception =3D { };
>>> + =A0 =A0 int err;
>>> + =A0 =A0 do {
>>> + =A0 =A0 =A0 =A0 =A0 =A0 err =3D nfs4_handle_exception(server, _pn=
fs4_proc_layoutget(lgp),
>>> + =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0 =A0=
=A0 =A0 =A0 &exception);
>>> + =A0 =A0 } while (exception.retry);
>>> + =A0 =A0 return err;
>>> +}
>>> +
>>> =A0static void pnfs_layoutcommit_prepare(struct rpc_task *task, voi=
d *data)
>>> =A0{
>>> =A0 =A0 =A0 struct pnfs_layoutcommit_data *ldata =3D
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-nfs"=
in
>> the body of a message to [email protected]
>> More majordomo info at =A0http://vger.kernel.org/majordomo-info.html
>>
>