linux-nfs.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] NFSv4.1: filelayout_check_layout: don't set the layout devinfo data on failure
@ 2013-09-26 19:39 Trond Myklebust
  2013-09-26 19:39 ` [PATCH 2/3] NFSv4.1: nfs4_fl_prepare_ds - fix bugs when the connect attempt fails Trond Myklebust
  0 siblings, 1 reply; 5+ messages in thread
From: Trond Myklebust @ 2013-09-26 19:39 UTC (permalink / raw)
  To: linux-nfs; +Cc: andros, jlayton

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 2/3] NFSv4.1: nfs4_fl_prepare_ds - fix bugs when the connect attempt fails
  2013-09-26 19:39 [PATCH 1/3] NFSv4.1: filelayout_check_layout: don't set the layout devinfo data on failure Trond Myklebust
@ 2013-09-26 19:39 ` Trond Myklebust
  2013-09-26 19:39   ` [PATCH 3/3] NFSv4.1: Ensure memory ordering between nfs4_ds_connect and nfs4_fl_prepare_ds Trond Myklebust
  0 siblings, 1 reply; 5+ messages in thread
From: Trond Myklebust @ 2013-09-26 19:39 UTC (permalink / raw)
  To: linux-nfs; +Cc: andros, jlayton

- 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 <Trond.Myklebust@netapp.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* [PATCH 3/3] NFSv4.1: Ensure memory ordering between nfs4_ds_connect and nfs4_fl_prepare_ds
  2013-09-26 19:39 ` [PATCH 2/3] NFSv4.1: nfs4_fl_prepare_ds - fix bugs when the connect attempt fails Trond Myklebust
@ 2013-09-26 19:39   ` Trond Myklebust
  2013-09-30 11:01     ` Jeff Layton
  0 siblings, 1 reply; 5+ messages in thread
From: Trond Myklebust @ 2013-09-26 19:39 UTC (permalink / raw)
  To: linux-nfs; +Cc: andros, jlayton

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 <Trond.Myklebust@netapp.com>
---
 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


^ permalink raw reply related	[flat|nested] 5+ messages in thread

* Re: [PATCH 3/3] NFSv4.1: Ensure memory ordering between nfs4_ds_connect and nfs4_fl_prepare_ds
  2013-09-26 19:39   ` [PATCH 3/3] NFSv4.1: Ensure memory ordering between nfs4_ds_connect and nfs4_fl_prepare_ds Trond Myklebust
@ 2013-09-30 11:01     ` Jeff Layton
  2013-09-30 14:27       ` Myklebust, Trond
  0 siblings, 1 reply; 5+ messages in thread
From: Jeff Layton @ 2013-09-30 11:01 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs, andros

On Thu, 26 Sep 2013 15:39:27 -0400
Trond Myklebust <Trond.Myklebust@netapp.com> 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 <Trond.Myklebust@netapp.com>
> ---
>  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 <jlayton@redhat.com>

^ permalink raw reply	[flat|nested] 5+ messages in thread

* RE: [PATCH 3/3] NFSv4.1: Ensure memory ordering between nfs4_ds_connect and nfs4_fl_prepare_ds
  2013-09-30 11:01     ` Jeff Layton
@ 2013-09-30 14:27       ` Myklebust, Trond
  0 siblings, 0 replies; 5+ messages in thread
From: Myklebust, Trond @ 2013-09-30 14:27 UTC (permalink / raw)
  To: Jeff Layton; +Cc: linux-nfs@vger.kernel.org, Adamson, Andy

> -----Original Message-----
> From: Jeff Layton [mailto:jlayton@redhat.com]
> Sent: Monday, September 30, 2013 7:02 AM
> To: Myklebust, Trond
> Cc: linux-nfs@vger.kernel.org; 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 <Trond.Myklebust@netapp.com> 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 <Trond.Myklebust@netapp.com>
> > ---
> >  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


^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, other threads:[~2013-09-30 14:27 UTC | newest]

Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-09-26 19:39 [PATCH 1/3] NFSv4.1: filelayout_check_layout: don't set the layout devinfo data on failure Trond Myklebust
2013-09-26 19:39 ` [PATCH 2/3] NFSv4.1: nfs4_fl_prepare_ds - fix bugs when the connect attempt fails Trond Myklebust
2013-09-26 19:39   ` [PATCH 3/3] NFSv4.1: Ensure memory ordering between nfs4_ds_connect and nfs4_fl_prepare_ds Trond Myklebust
2013-09-30 11:01     ` Jeff Layton
2013-09-30 14:27       ` Myklebust, Trond

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).