2012-01-06 07:24:31

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCHSET 0/2] pnfs-obj: Important BUG fixing also for @Stable

Hi Trond

I'm sending in two patches that fixes a bad hung, memory leak, a WARN_ON
and plain wrong protocol handling. In regard to objlayout error handling.
These are also aimed at Stable@ for the 3.2 Kernel.
(Actually these bugs are in since 3.0, I might produce another version
for 3.[1,0] after these go in)

I have simulated errors at various layers, reads and writes, in combination
of different tests, and they work as I expect. .i.e IO continues to MDS and
oblayout is returned on error.

Please include these patches for 3.3 and they can go to Stable@ for 3.2
has well.

list of patches:
[PATCH 1/2] pnfs-obj: pNFS errors are communicated on iodata->pnfs_error

This is just a stupidity and neglect. Benny can you understand
what happened here?

[PATCH 2/2] pnfs-obj: Must return layout on IO error

This patch is mainly for pnfs.c but I think it is very simple
and totally not affecting anybody but objlayout. So it should
be safe, and does fixes my main problem.
Fred please review?

Please Review

Thanks
Boaz


2012-01-06 07:38:27

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH 2/2] pnfs-obj: Must return layout on IO error


As mandated by the standard. In case of an IO error, a pNFS
objects layout driver must return it's layout. This is because
all device errors are reported to the server as part of the
layout return buffer.

This is implemented the same way PNFS_LAYOUTRET_ON_SETATTR
is done, through a bit flag on the pnfs_layoutdriver_type->flags
member. The flag is set by the layout driver that wants a
layout_return preformed at pnfs_ld_{write,read}_done in case
of an error.
(Though I have not defined a wrapper like pnfs_ld_layoutret_on_setattr
because this code is never called outside of pnfs.c and pnfs IO
paths)

Without this patch 3.[0-2] Kernels leak memory and have an annoying
WARN_ON after every IO error utilizing the pnfs-obj driver.

[This patch is for 3.2 Kernel. 3.1/0 Kernels need a different patch]
CC: Stable Tree <[email protected]>
Signed-off-by: Boaz Harrosh <[email protected]>
---
fs/nfs/objlayout/objio_osd.c | 3 ++-
fs/nfs/pnfs.c | 12 ++++++++++++
fs/nfs/pnfs.h | 1 +
3 files changed, 15 insertions(+), 1 deletions(-)

diff --git a/fs/nfs/objlayout/objio_osd.c b/fs/nfs/objlayout/objio_osd.c
index c807ab9..55d0128 100644
--- a/fs/nfs/objlayout/objio_osd.c
+++ b/fs/nfs/objlayout/objio_osd.c
@@ -551,7 +551,8 @@ static const struct nfs_pageio_ops objio_pg_write_ops = {
static struct pnfs_layoutdriver_type objlayout_type = {
.id = LAYOUT_OSD2_OBJECTS,
.name = "LAYOUT_OSD2_OBJECTS",
- .flags = PNFS_LAYOUTRET_ON_SETATTR,
+ .flags = PNFS_LAYOUTRET_ON_SETATTR |
+ PNFS_LAYOUTRET_ON_ERROR,

.alloc_layout_hdr = objlayout_alloc_layout_hdr,
.free_layout_hdr = objlayout_free_layout_hdr,
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index 8e672a2..f881a63 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1178,6 +1178,15 @@ void pnfs_ld_write_done(struct nfs_write_data *data)
put_lseg(data->lseg);
data->lseg = NULL;
dprintk("pnfs write error = %d\n", data->pnfs_error);
+ if (NFS_SERVER(data->inode)->pnfs_curr_ld->flags &
+ PNFS_LAYOUTRET_ON_ERROR) {
+ /* Don't lo_commit on error, Server will needs to
+ * preform a file recovery.
+ */
+ clear_bit(NFS_INO_LAYOUTCOMMIT,
+ &NFS_I(data->inode)->flags);
+ pnfs_return_layout(data->inode);
+ }
}
data->mds_ops->rpc_release(data);
}
@@ -1267,6 +1276,9 @@ static void pnfs_ld_handle_read_error(struct nfs_read_data *data)
put_lseg(data->lseg);
data->lseg = NULL;
dprintk("pnfs write error = %d\n", data->pnfs_error);
+ if (NFS_SERVER(data->inode)->pnfs_curr_ld->flags &
+ PNFS_LAYOUTRET_ON_ERROR)
+ pnfs_return_layout(data->inode);

nfs_pageio_init_read_mds(&pgio, data->inode);

diff --git a/fs/nfs/pnfs.h b/fs/nfs/pnfs.h
index 1509530..53d593a 100644
--- a/fs/nfs/pnfs.h
+++ b/fs/nfs/pnfs.h
@@ -68,6 +68,7 @@ enum {
enum layoutdriver_policy_flags {
/* Should the pNFS client commit and return the layout upon a setattr */
PNFS_LAYOUTRET_ON_SETATTR = 1 << 0,
+ PNFS_LAYOUTRET_ON_ERROR = 1 << 1,
};

struct nfs4_deviceid_node;
--
1.7.2.3



2012-01-09 08:09:15

by Benny Halevy

[permalink] [raw]
Subject: Re: [PATCHSET 0/2] pnfs-obj: Important BUG fixing also for @Stable

Fixes included in pnfs-all-3.2-2012-01-08

Thanks,

Benny

On 2012-01-06 09:23, Boaz Harrosh wrote:
> Hi Trond
>
> I'm sending in two patches that fixes a bad hung, memory leak, a WARN_ON
> and plain wrong protocol handling. In regard to objlayout error handling.
> These are also aimed at Stable@ for the 3.2 Kernel.
> (Actually these bugs are in since 3.0, I might produce another version
> for 3.[1,0] after these go in)
>
> I have simulated errors at various layers, reads and writes, in combination
> of different tests, and they work as I expect. .i.e IO continues to MDS and
> oblayout is returned on error.
>
> Please include these patches for 3.3 and they can go to Stable@ for 3.2
> has well.
>
> list of patches:
> [PATCH 1/2] pnfs-obj: pNFS errors are communicated on iodata->pnfs_error
>
> This is just a stupidity and neglect. Benny can you understand
> what happened here?
>
> [PATCH 2/2] pnfs-obj: Must return layout on IO error
>
> This patch is mainly for pnfs.c but I think it is very simple
> and totally not affecting anybody but objlayout. So it should
> be safe, and does fixes my main problem.
> Fred please review?
>
> Please Review
>
> Thanks
> Boaz
> --
> 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

2012-01-06 07:35:35

by Boaz Harrosh

[permalink] [raw]
Subject: [PATCH 1/2] pnfs-obj: pNFS errors are communicated on iodata->pnfs_error


Some time along the way pNFS IO errors were switched to
communicate with a special iodata->pnfs_error member instead
of the regular RPC members. But objlayout was not switched
over.

Fix that!
Without this fix any IO error is hanged, because IO is not
switched to MDS and pages are never cleared or read.

[Applies to 3.2.0. Same bug different patch for 3.1/0 Kernels]
CC: Stable Tree <[email protected]>
Signed-off-by: Boaz Harrosh <[email protected]>
---
fs/nfs/objlayout/objlayout.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/fs/nfs/objlayout/objlayout.c b/fs/nfs/objlayout/objlayout.c
index 72074e3..b3c2903 100644
--- a/fs/nfs/objlayout/objlayout.c
+++ b/fs/nfs/objlayout/objlayout.c
@@ -254,6 +254,8 @@ objlayout_read_done(struct objlayout_io_res *oir, ssize_t status, bool sync)
oir->status = rdata->task.tk_status = status;
if (status >= 0)
rdata->res.count = status;
+ else
+ rdata->pnfs_error = status;
objlayout_iodone(oir);
/* must not use oir after this point */

@@ -334,6 +336,8 @@ objlayout_write_done(struct objlayout_io_res *oir, ssize_t status, bool sync)
if (status >= 0) {
wdata->res.count = status;
wdata->verf.committed = oir->committed;
+ } else {
+ wdata->pnfs_error = status;
}
objlayout_iodone(oir);
/* must not use oir after this point */
--
1.7.2.3