Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4filelayout.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/nfs/nfs4filelayout.c b/fs/nfs/nfs4filelayout.c
index b86464b..c456482 100644
--- a/fs/nfs/nfs4filelayout.c
+++ b/fs/nfs/nfs4filelayout.c
@@ -669,8 +669,6 @@ filelayout_check_layout(struct pnfs_layout_hdr *lo,
if (filelayout_test_devid_unavailable(&dsaddr->id_node))
goto out_put;
- fl->dsaddr = dsaddr;
-
if (fl->first_stripe_index >= dsaddr->stripe_count) {
dprintk("%s Bad first_stripe_index %u\n",
__func__, fl->first_stripe_index);
@@ -692,6 +690,8 @@ filelayout_check_layout(struct pnfs_layout_hdr *lo,
nfss->wsize);
}
+ fl->dsaddr = dsaddr;
+
status = 0;
out:
dprintk("--> %s returns %d\n", __func__, status);
--
1.8.3.1
We need to ensure that the initialisation of the data server nfs_client
structure in nfs4_ds_connect is correctly ordered w.r.t. the read of
ds->ds_clp in nfs4_fl_prepare_ds.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4filelayoutdev.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index ea2b2bf..6456fc4 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -185,6 +185,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
if (status)
goto out_put;
+ smp_wmb();
ds->ds_clp = clp;
dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
out:
@@ -812,6 +813,7 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
filelayout_mark_devid_invalid(devid);
goto out;
}
+ smp_rmb();
if (ds->ds_clp)
goto out;
--
1.8.3.1
- Fix an Oops when nfs4_ds_connect() returns an error.
- Always check the device status after waiting for a connect to complete.
Signed-off-by: Trond Myklebust <[email protected]>
---
fs/nfs/nfs4filelayoutdev.c | 14 ++++++++------
1 file changed, 8 insertions(+), 6 deletions(-)
diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
index 95604f6..ea2b2bf 100644
--- a/fs/nfs/nfs4filelayoutdev.c
+++ b/fs/nfs/nfs4filelayoutdev.c
@@ -801,6 +801,7 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
struct nfs4_file_layout_dsaddr *dsaddr = FILELAYOUT_LSEG(lseg)->dsaddr;
struct nfs4_pnfs_ds *ds = dsaddr->ds_list[ds_idx];
struct nfs4_deviceid_node *devid = FILELAYOUT_DEVID_NODE(lseg);
+ struct nfs4_pnfs_ds *ret = ds;
if (filelayout_test_devid_unavailable(devid))
return NULL;
@@ -809,26 +810,27 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
printk(KERN_ERR "NFS: %s: No data server for offset index %d\n",
__func__, ds_idx);
filelayout_mark_devid_invalid(devid);
- return NULL;
+ goto out;
}
if (ds->ds_clp)
- return ds;
+ goto out;
if (test_and_set_bit(NFS4DS_CONNECTING, &ds->ds_state) == 0) {
struct nfs_server *s = NFS_SERVER(lseg->pls_layout->plh_inode);
int err;
err = nfs4_ds_connect(s, ds);
- if (err) {
+ if (err)
nfs4_mark_deviceid_unavailable(devid);
- ds = NULL;
- }
nfs4_clear_ds_conn_bit(ds);
} else {
/* Either ds is connected, or ds is NULL */
nfs4_wait_ds_connect(ds);
}
- return ds;
+ if (filelayout_test_devid_unavailable(devid))
+ ret = NULL;
+out:
+ return ret;
}
module_param(dataserver_retrans, uint, 0644);
--
1.8.3.1
> -----Original Message-----
> From: Jeff Layton [mailto:[email protected]]
> Sent: Monday, September 30, 2013 7:02 AM
> To: Myklebust, Trond
> Cc: [email protected]; Adamson, Andy
> Subject: Re: [PATCH 3/3] NFSv4.1: Ensure memory ordering between
> nfs4_ds_connect and nfs4_fl_prepare_ds
>
> On Thu, 26 Sep 2013 15:39:27 -0400
> Trond Myklebust <[email protected]> wrote:
>
> > We need to ensure that the initialisation of the data server
> > nfs_client structure in nfs4_ds_connect is correctly ordered w.r.t.
> > the read of
> > ds->ds_clp in nfs4_fl_prepare_ds.
> >
> > Signed-off-by: Trond Myklebust <[email protected]>
> > ---
> > fs/nfs/nfs4filelayoutdev.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> > index ea2b2bf..6456fc4 100644
> > --- a/fs/nfs/nfs4filelayoutdev.c
> > +++ b/fs/nfs/nfs4filelayoutdev.c
> > @@ -185,6 +185,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv,
> struct nfs4_pnfs_ds *ds)
> > if (status)
> > goto out_put;
> >
> > + smp_wmb();
> > ds->ds_clp = clp;
> > dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
> > out:
> > @@ -812,6 +813,7 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment
> *lseg, u32 ds_idx)
> > filelayout_mark_devid_invalid(devid);
> > goto out;
> > }
> > + smp_rmb();
> > if (ds->ds_clp)
> > goto out;
> >
>
> I noticed that you had patch #2 in this series marked for stable in your for-
> next branch, but this one wasn't. Should this one also go to stable?
Possibly, but so far it remains at the "theoretical problem" level, and not a "fixes a definite bug". The x86_64 architecture doesn't do out-of-order store, so I have problems testing fixes such as this one...
Cheers
Trond
On Thu, 26 Sep 2013 15:39:27 -0400
Trond Myklebust <[email protected]> wrote:
> We need to ensure that the initialisation of the data server nfs_client
> structure in nfs4_ds_connect is correctly ordered w.r.t. the read of
> ds->ds_clp in nfs4_fl_prepare_ds.
>
> Signed-off-by: Trond Myklebust <[email protected]>
> ---
> fs/nfs/nfs4filelayoutdev.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/fs/nfs/nfs4filelayoutdev.c b/fs/nfs/nfs4filelayoutdev.c
> index ea2b2bf..6456fc4 100644
> --- a/fs/nfs/nfs4filelayoutdev.c
> +++ b/fs/nfs/nfs4filelayoutdev.c
> @@ -185,6 +185,7 @@ nfs4_ds_connect(struct nfs_server *mds_srv, struct nfs4_pnfs_ds *ds)
> if (status)
> goto out_put;
>
> + smp_wmb();
> ds->ds_clp = clp;
> dprintk("%s [new] addr: %s\n", __func__, ds->ds_remotestr);
> out:
> @@ -812,6 +813,7 @@ nfs4_fl_prepare_ds(struct pnfs_layout_segment *lseg, u32 ds_idx)
> filelayout_mark_devid_invalid(devid);
> goto out;
> }
> + smp_rmb();
> if (ds->ds_clp)
> goto out;
>
I noticed that you had patch #2 in this series marked for stable in
your for-next branch, but this one wasn't. Should this one also go to
stable?
--
Jeff Layton <[email protected]>