2018-11-12 13:59:36

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: [PATCH] flexfiles: use per-mirror specified stateid for IO

rfc8435 says:

For tight coupling, ffds_stateid provides the stateid to be used by
the client to access the file.

However current implementation replaces per-mirror provided stateid with
by open or lock stateid.

Ensure that per-mirror stateid is used by ff_layout_write_prepare_v4 and
nfs4_ff_layout_prepare_ds.

Signed-off-by: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Rick Macklem <[email protected]>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 21 +++++++++++++--------
fs/nfs/flexfilelayout/flexfilelayout.h | 2 ++
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 17 +++++++++++++++++
3 files changed, 32 insertions(+), 8 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 86bcba40ca61..4c7f042db9c4 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1363,10 +1363,6 @@ static void ff_layout_read_prepare_v4(struct rpc_task *task, void *data)

if (ff_layout_read_prepare_common(task, hdr))
return;
-
- if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
- hdr->args.lock_context, FMODE_READ) == -EIO)
- rpc_exit(task, -EIO); /* lost lock, terminate I/O */
}

static void ff_layout_read_call_done(struct rpc_task *task, void *data)
@@ -1544,10 +1540,6 @@ static void ff_layout_write_prepare_v4(struct rpc_task *task, void *data)

if (ff_layout_write_prepare_common(task, hdr))
return;
-
- if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
- hdr->args.lock_context, FMODE_WRITE) == -EIO)
- rpc_exit(task, -EIO); /* lost lock, terminate I/O */
}

static void ff_layout_write_call_done(struct rpc_task *task, void *data)
@@ -1713,6 +1705,7 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
u32 idx = hdr->pgio_mirror_idx;
int vers;
struct nfs_fh *fh;
+ nfs4_stateid *stateid;

dprintk("--> %s ino %lu pgbase %u req %zu@%llu\n",
__func__, hdr->inode->i_ino,
@@ -1742,6 +1735,12 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
fh = nfs4_ff_layout_select_ds_fh(lseg, idx);
if (fh)
hdr->args.fh = fh;
+
+ stateid = nfs4_ff_layout_select_ds_stateid(lseg, idx);
+ if (!stateid)
+ goto out_failed;
+ nfs4_stateid_copy(&hdr->args.stateid, stateid);
+
/*
* Note that if we ever decide to split across DSes,
* then we may need to handle dense-like offsets.
@@ -1774,6 +1773,7 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
loff_t offset = hdr->args.offset;
int vers;
struct nfs_fh *fh;
+ nfs4_stateid *stateid;
int idx = hdr->pgio_mirror_idx;

ds = nfs4_ff_layout_prepare_ds(lseg, idx, true);
@@ -1804,6 +1804,11 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
if (fh)
hdr->args.fh = fh;

+ stateid = nfs4_ff_layout_select_ds_stateid(lseg, idx);
+ if (!stateid)
+ goto out_failed;
+ nfs4_stateid_copy(&hdr->args.stateid, stateid);
+
/*
* Note that if we ever decide to split across DSes,
* then we may need to handle dense-like offsets.
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 411798346e48..fdfbcb471999 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -215,6 +215,8 @@ unsigned int ff_layout_fetch_ds_ioerr(struct pnfs_layout_hdr *lo,
unsigned int maxnum);
struct nfs_fh *
nfs4_ff_layout_select_ds_fh(struct pnfs_layout_segment *lseg, u32 mirror_idx);
+nfs4_stateid *
+nfs4_ff_layout_select_ds_stateid(struct pnfs_layout_segment *lseg, u32 mirror_idx);

struct nfs4_pnfs_ds *
nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx,
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 74d8d5352438..91787cf68057 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -370,6 +370,23 @@ nfs4_ff_layout_select_ds_fh(struct pnfs_layout_segment *lseg, u32 mirror_idx)
return fh;
}

+nfs4_stateid *
+nfs4_ff_layout_select_ds_stateid(struct pnfs_layout_segment *lseg, u32 mirror_idx)
+{
+ struct nfs4_ff_layout_mirror *mirror = FF_LAYOUT_COMP(lseg, mirror_idx);
+ nfs4_stateid *stateid = NULL;
+
+ if (!ff_layout_mirror_valid(lseg, mirror, false)) {
+ pr_err_ratelimited("NFS: %s: No data server for mirror offset index %d\n",
+ __func__, mirror_idx);
+ goto out;
+ }
+
+ stateid = &mirror->stateid;
+out:
+ return stateid;
+}
+
/**
* nfs4_ff_layout_prepare_ds - prepare a DS connection for an RPC call
* @lseg: the layout segment we're operating on
--
2.17.2



2018-11-20 21:19:00

by Trond Myklebust

[permalink] [raw]
Subject: Re: [PATCH] flexfiles: use per-mirror specified stateid for IO

Hi Tigran,

On Mon, 2018-11-12 at 14:59 +0100, Tigran Mkrtchyan wrote:
> rfc8435 says:
>
> For tight coupling, ffds_stateid provides the stateid to be used by
> the client to access the file.
>
> However current implementation replaces per-mirror provided stateid
> with
> by open or lock stateid.
>
> Ensure that per-mirror stateid is used by ff_layout_write_prepare_v4
> and
> nfs4_ff_layout_prepare_ds.
>
> Signed-off-by: Tigran Mkrtchyan <[email protected]>
> Signed-off-by: Rick Macklem <[email protected]>
> ---
> fs/nfs/flexfilelayout/flexfilelayout.c | 21 +++++++++++++--------
> fs/nfs/flexfilelayout/flexfilelayout.h | 2 ++
> fs/nfs/flexfilelayout/flexfilelayoutdev.c | 17 +++++++++++++++++
> 3 files changed, 32 insertions(+), 8 deletions(-)
>
> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c
> b/fs/nfs/flexfilelayout/flexfilelayout.c
> index 86bcba40ca61..4c7f042db9c4 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayout.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayout.c
> @@ -1363,10 +1363,6 @@ static void ff_layout_read_prepare_v4(struct
> rpc_task *task, void *data)
>
> if (ff_layout_read_prepare_common(task, hdr))
> return;
> -
> - if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
> - hdr->args.lock_context, FMODE_READ) == -EIO)
> - rpc_exit(task, -EIO); /* lost lock, terminate I/O */
> }
>
> static void ff_layout_read_call_done(struct rpc_task *task, void
> *data)
> @@ -1544,10 +1540,6 @@ static void ff_layout_write_prepare_v4(struct
> rpc_task *task, void *data)
>
> if (ff_layout_write_prepare_common(task, hdr))
> return;
> -
> - if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
> - hdr->args.lock_context, FMODE_WRITE) == -EIO)
> - rpc_exit(task, -EIO); /* lost lock, terminate I/O */
> }
>
> static void ff_layout_write_call_done(struct rpc_task *task, void
> *data)
> @@ -1713,6 +1705,7 @@ ff_layout_read_pagelist(struct nfs_pgio_header
> *hdr)
> u32 idx = hdr->pgio_mirror_idx;
> int vers;
> struct nfs_fh *fh;
> + nfs4_stateid *stateid;
>
> dprintk("--> %s ino %lu pgbase %u req %zu@%llu\n",
> __func__, hdr->inode->i_ino,
> @@ -1742,6 +1735,12 @@ ff_layout_read_pagelist(struct nfs_pgio_header
> *hdr)
> fh = nfs4_ff_layout_select_ds_fh(lseg, idx);
> if (fh)
> hdr->args.fh = fh;
> +
> + stateid = nfs4_ff_layout_select_ds_stateid(lseg, idx);
> + if (!stateid)
> + goto out_failed;
> + nfs4_stateid_copy(&hdr->args.stateid, stateid);
> +
> /*
> * Note that if we ever decide to split across DSes,
> * then we may need to handle dense-like offsets.
> @@ -1774,6 +1773,7 @@ ff_layout_write_pagelist(struct nfs_pgio_header
> *hdr, int sync)
> loff_t offset = hdr->args.offset;
> int vers;
> struct nfs_fh *fh;
> + nfs4_stateid *stateid;
> int idx = hdr->pgio_mirror_idx;
>
> ds = nfs4_ff_layout_prepare_ds(lseg, idx, true);
> @@ -1804,6 +1804,11 @@ ff_layout_write_pagelist(struct
> nfs_pgio_header *hdr, int sync)
> if (fh)
> hdr->args.fh = fh;
>
> + stateid = nfs4_ff_layout_select_ds_stateid(lseg, idx);
> + if (!stateid)
> + goto out_failed;
> + nfs4_stateid_copy(&hdr->args.stateid, stateid);

Since the above 3 lines are repeated for both functions, and also force
the stack allocation of a temporary pointer, why not just have the
nfs4_ff_layout_select_ds_stateid() copy the stateid?

> +
> /*
> * Note that if we ever decide to split across DSes,
> * then we may need to handle dense-like offsets.
> diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h
> b/fs/nfs/flexfilelayout/flexfilelayout.h
> index 411798346e48..fdfbcb471999 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayout.h
> +++ b/fs/nfs/flexfilelayout/flexfilelayout.h
> @@ -215,6 +215,8 @@ unsigned int ff_layout_fetch_ds_ioerr(struct
> pnfs_layout_hdr *lo,
> unsigned int maxnum);
> struct nfs_fh *
> nfs4_ff_layout_select_ds_fh(struct pnfs_layout_segment *lseg, u32
> mirror_idx);
> +nfs4_stateid *
> +nfs4_ff_layout_select_ds_stateid(struct pnfs_layout_segment *lseg,
> u32 mirror_idx);
>
> struct nfs4_pnfs_ds *
> nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32
> ds_idx,
> diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> index 74d8d5352438..91787cf68057 100644
> --- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> +++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
> @@ -370,6 +370,23 @@ nfs4_ff_layout_select_ds_fh(struct
> pnfs_layout_segment *lseg, u32 mirror_idx)
> return fh;
> }
>
> +nfs4_stateid *
> +nfs4_ff_layout_select_ds_stateid(struct pnfs_layout_segment *lseg,
> u32 mirror_idx)
> +{
> + struct nfs4_ff_layout_mirror *mirror = FF_LAYOUT_COMP(lseg,
> mirror_idx);
> + nfs4_stateid *stateid = NULL;
> +
> + if (!ff_layout_mirror_valid(lseg, mirror, false)) {
> + pr_err_ratelimited("NFS: %s: No data server for mirror
> offset index %d\n",
> + __func__, mirror_idx);
> + goto out;
> + }
> +
> + stateid = &mirror->stateid;
> +out:
> + return stateid;
> +}
> +
> /**
> * nfs4_ff_layout_prepare_ds - prepare a DS connection for an RPC
> call
> * @lseg: the layout segment we're operating on
--
Trond Myklebust
Linux NFS client maintainer, Hammerspace
[email protected]


2018-11-21 11:26:10

by Mkrtchyan, Tigran

[permalink] [raw]
Subject: [PATCH v2] flexfiles: use per-mirror specified stateid for IO

rfc8435 says:

For tight coupling, ffds_stateid provides the stateid to be used by
the client to access the file.

However current implementation replaces per-mirror provided stateid with
by open or lock stateid.

Ensure that per-mirror stateid is used by ff_layout_write_prepare_v4 and
nfs4_ff_layout_prepare_ds.

Signed-off-by: Tigran Mkrtchyan <[email protected]>
Signed-off-by: Rick Macklem <[email protected]>
---
fs/nfs/flexfilelayout/flexfilelayout.c | 21 +++++++++------------
fs/nfs/flexfilelayout/flexfilelayout.h | 4 ++++
fs/nfs/flexfilelayout/flexfilelayoutdev.c | 19 +++++++++++++++++++
3 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/fs/nfs/flexfilelayout/flexfilelayout.c b/fs/nfs/flexfilelayout/flexfilelayout.c
index 86bcba40ca61..74b36ed883ca 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.c
+++ b/fs/nfs/flexfilelayout/flexfilelayout.c
@@ -1361,12 +1361,7 @@ static void ff_layout_read_prepare_v4(struct rpc_task *task, void *data)
task))
return;

- if (ff_layout_read_prepare_common(task, hdr))
- return;
-
- if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
- hdr->args.lock_context, FMODE_READ) == -EIO)
- rpc_exit(task, -EIO); /* lost lock, terminate I/O */
+ ff_layout_read_prepare_common(task, hdr);
}

static void ff_layout_read_call_done(struct rpc_task *task, void *data)
@@ -1542,12 +1537,7 @@ static void ff_layout_write_prepare_v4(struct rpc_task *task, void *data)
task))
return;

- if (ff_layout_write_prepare_common(task, hdr))
- return;
-
- if (nfs4_set_rw_stateid(&hdr->args.stateid, hdr->args.context,
- hdr->args.lock_context, FMODE_WRITE) == -EIO)
- rpc_exit(task, -EIO); /* lost lock, terminate I/O */
+ ff_layout_write_prepare_common(task, hdr);
}

static void ff_layout_write_call_done(struct rpc_task *task, void *data)
@@ -1742,6 +1732,10 @@ ff_layout_read_pagelist(struct nfs_pgio_header *hdr)
fh = nfs4_ff_layout_select_ds_fh(lseg, idx);
if (fh)
hdr->args.fh = fh;
+
+ if (!nfs4_ff_layout_select_ds_stateid(lseg, idx, &hdr->args.stateid))
+ goto out_failed;
+
/*
* Note that if we ever decide to split across DSes,
* then we may need to handle dense-like offsets.
@@ -1804,6 +1798,9 @@ ff_layout_write_pagelist(struct nfs_pgio_header *hdr, int sync)
if (fh)
hdr->args.fh = fh;

+ if (!nfs4_ff_layout_select_ds_stateid(lseg, idx, &hdr->args.stateid))
+ goto out_failed;
+
/*
* Note that if we ever decide to split across DSes,
* then we may need to handle dense-like offsets.
diff --git a/fs/nfs/flexfilelayout/flexfilelayout.h b/fs/nfs/flexfilelayout/flexfilelayout.h
index 411798346e48..de50a342d5a5 100644
--- a/fs/nfs/flexfilelayout/flexfilelayout.h
+++ b/fs/nfs/flexfilelayout/flexfilelayout.h
@@ -215,6 +215,10 @@ unsigned int ff_layout_fetch_ds_ioerr(struct pnfs_layout_hdr *lo,
unsigned int maxnum);
struct nfs_fh *
nfs4_ff_layout_select_ds_fh(struct pnfs_layout_segment *lseg, u32 mirror_idx);
+int
+nfs4_ff_layout_select_ds_stateid(struct pnfs_layout_segment *lseg,
+ u32 mirror_idx,
+ nfs4_stateid *stateid);

struct nfs4_pnfs_ds *
nfs4_ff_layout_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx,
diff --git a/fs/nfs/flexfilelayout/flexfilelayoutdev.c b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
index 74d8d5352438..d23347389626 100644
--- a/fs/nfs/flexfilelayout/flexfilelayoutdev.c
+++ b/fs/nfs/flexfilelayout/flexfilelayoutdev.c
@@ -370,6 +370,25 @@ nfs4_ff_layout_select_ds_fh(struct pnfs_layout_segment *lseg, u32 mirror_idx)
return fh;
}

+int
+nfs4_ff_layout_select_ds_stateid(struct pnfs_layout_segment *lseg,
+ u32 mirror_idx,
+ nfs4_stateid *stateid)
+{
+ struct nfs4_ff_layout_mirror *mirror = FF_LAYOUT_COMP(lseg, mirror_idx);
+
+ if (!ff_layout_mirror_valid(lseg, mirror, false)) {
+ pr_err_ratelimited("NFS: %s: No data server for mirror offset index %d\n",
+ __func__, mirror_idx);
+ goto out;
+ }
+
+ nfs4_stateid_copy(stateid, &mirror->stateid);
+ return 1;
+out:
+ return 0;
+}
+
/**
* nfs4_ff_layout_prepare_ds - prepare a DS connection for an RPC call
* @lseg: the layout segment we're operating on
--
2.17.2