public inbox for linux-nfs@vger.kernel.org
 help / color / mirror / Atom feed
* [PATCH] NFS: Revert pnfs ugliness from the generic NFS read code path
@ 2011-11-10 19:38 Trond Myklebust
  2011-11-14 11:12 ` Benny Halevy
  0 siblings, 1 reply; 4+ messages in thread
From: Trond Myklebust @ 2011-11-10 19:38 UTC (permalink / raw)
  To: linux-nfs

pNFS-specific code belongs in the pnfs layer. It should not be
hijacking generic NFS read or write code paths.

Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
---
 fs/nfs/internal.h |    2 ++
 fs/nfs/pnfs.c     |   26 +++++++++++++++++++++-----
 fs/nfs/read.c     |   14 ++------------
 3 files changed, 25 insertions(+), 17 deletions(-)

diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
index c1a1bd8..3f4d957 100644
--- a/fs/nfs/internal.h
+++ b/fs/nfs/internal.h
@@ -299,6 +299,8 @@ extern void nfs_read_prepare(struct rpc_task *task, void *calldata);
 extern int nfs_generic_pagein(struct nfs_pageio_descriptor *desc,
 		struct list_head *head);
 
+extern void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
+		struct inode *inode);
 extern void nfs_pageio_reset_read_mds(struct nfs_pageio_descriptor *pgio);
 extern void nfs_readdata_release(struct nfs_read_data *rdata);
 
diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
index baf7353..8e672a2 100644
--- a/fs/nfs/pnfs.c
+++ b/fs/nfs/pnfs.c
@@ -1260,6 +1260,25 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
 }
 EXPORT_SYMBOL_GPL(pnfs_generic_pg_writepages);
 
+static void pnfs_ld_handle_read_error(struct nfs_read_data *data)
+{
+	struct nfs_pageio_descriptor pgio;
+
+	put_lseg(data->lseg);
+	data->lseg = NULL;
+	dprintk("pnfs write error = %d\n", data->pnfs_error);
+
+	nfs_pageio_init_read_mds(&pgio, data->inode);
+
+	while (!list_empty(&data->pages)) {
+		struct nfs_page *req = nfs_list_entry(data->pages.next);
+
+		nfs_list_remove_request(req);
+		nfs_pageio_add_request(&pgio, req);
+	}
+	nfs_pageio_complete(&pgio);
+}
+
 /*
  * Called by non rpc-based layout drivers
  */
@@ -1268,11 +1287,8 @@ void pnfs_ld_read_done(struct nfs_read_data *data)
 	if (likely(!data->pnfs_error)) {
 		__nfs4_read_done_cb(data);
 		data->mds_ops->rpc_call_done(&data->task, data);
-	} else {
-		put_lseg(data->lseg);
-		data->lseg = NULL;
-		dprintk("pnfs write error = %d\n", data->pnfs_error);
-	}
+	} else
+		pnfs_ld_handle_read_error(data);
 	data->mds_ops->rpc_release(data);
 }
 EXPORT_SYMBOL_GPL(pnfs_ld_read_done);
diff --git a/fs/nfs/read.c b/fs/nfs/read.c
index 8b48ec6..cfa175c 100644
--- a/fs/nfs/read.c
+++ b/fs/nfs/read.c
@@ -109,7 +109,7 @@ static void nfs_readpage_truncate_uninitialised_page(struct nfs_read_data *data)
 	}
 }
 
-static void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
+void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
 		struct inode *inode)
 {
 	nfs_pageio_init(pgio, inode, &nfs_pageio_read_ops,
@@ -534,23 +534,13 @@ static void nfs_readpage_result_full(struct rpc_task *task, void *calldata)
 static void nfs_readpage_release_full(void *calldata)
 {
 	struct nfs_read_data *data = calldata;
-	struct nfs_pageio_descriptor pgio;
 
-	if (data->pnfs_error) {
-		nfs_pageio_init_read_mds(&pgio, data->inode);
-		pgio.pg_recoalesce = 1;
-	}
 	while (!list_empty(&data->pages)) {
 		struct nfs_page *req = nfs_list_entry(data->pages.next);
 
 		nfs_list_remove_request(req);
-		if (!data->pnfs_error)
-			nfs_readpage_release(req);
-		else
-			nfs_pageio_add_request(&pgio, req);
+		nfs_readpage_release(req);
 	}
-	if (data->pnfs_error)
-		nfs_pageio_complete(&pgio);
 	nfs_readdata_release(calldata);
 }
 
-- 
1.7.7


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

* Re: [PATCH] NFS: Revert pnfs ugliness from the generic NFS read code path
  2011-11-10 19:38 [PATCH] NFS: Revert pnfs ugliness from the generic NFS read code path Trond Myklebust
@ 2011-11-14 11:12 ` Benny Halevy
  2011-11-14 15:49   ` Trond Myklebust
  0 siblings, 1 reply; 4+ messages in thread
From: Benny Halevy @ 2011-11-14 11:12 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs

On 2011-11-10 21:38, Trond Myklebust wrote:
> pNFS-specific code belongs in the pnfs layer. It should not be
> hijacking generic NFS read or write code paths.
> 
> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> ---
>  fs/nfs/internal.h |    2 ++
>  fs/nfs/pnfs.c     |   26 +++++++++++++++++++++-----
>  fs/nfs/read.c     |   14 ++------------
>  3 files changed, 25 insertions(+), 17 deletions(-)
> 
> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> index c1a1bd8..3f4d957 100644
> --- a/fs/nfs/internal.h
> +++ b/fs/nfs/internal.h
> @@ -299,6 +299,8 @@ extern void nfs_read_prepare(struct rpc_task *task, void *calldata);
>  extern int nfs_generic_pagein(struct nfs_pageio_descriptor *desc,
>  		struct list_head *head);
>  
> +extern void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
> +		struct inode *inode);
>  extern void nfs_pageio_reset_read_mds(struct nfs_pageio_descriptor *pgio);
>  extern void nfs_readdata_release(struct nfs_read_data *rdata);
>  
> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> index baf7353..8e672a2 100644
> --- a/fs/nfs/pnfs.c
> +++ b/fs/nfs/pnfs.c
> @@ -1260,6 +1260,25 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
>  }
>  EXPORT_SYMBOL_GPL(pnfs_generic_pg_writepages);
>  
> +static void pnfs_ld_handle_read_error(struct nfs_read_data *data)
> +{
> +	struct nfs_pageio_descriptor pgio;
> +
> +	put_lseg(data->lseg);
> +	data->lseg = NULL;
> +	dprintk("pnfs write error = %d\n", data->pnfs_error);

should be "read error", might as well fix this while you're at it :)

> +
> +	nfs_pageio_init_read_mds(&pgio, data->inode);
> +
> +	while (!list_empty(&data->pages)) {
> +		struct nfs_page *req = nfs_list_entry(data->pages.next);
> +
> +		nfs_list_remove_request(req);
> +		nfs_pageio_add_request(&pgio, req);
> +	}

What about pgio.pg_recoalesce?

Benny

> +	nfs_pageio_complete(&pgio);
> +}
> +
>  /*
>   * Called by non rpc-based layout drivers
>   */
> @@ -1268,11 +1287,8 @@ void pnfs_ld_read_done(struct nfs_read_data *data)
>  	if (likely(!data->pnfs_error)) {
>  		__nfs4_read_done_cb(data);
>  		data->mds_ops->rpc_call_done(&data->task, data);
> -	} else {
> -		put_lseg(data->lseg);
> -		data->lseg = NULL;
> -		dprintk("pnfs write error = %d\n", data->pnfs_error);
> -	}
> +	} else
> +		pnfs_ld_handle_read_error(data);
>  	data->mds_ops->rpc_release(data);
>  }
>  EXPORT_SYMBOL_GPL(pnfs_ld_read_done);
> diff --git a/fs/nfs/read.c b/fs/nfs/read.c
> index 8b48ec6..cfa175c 100644
> --- a/fs/nfs/read.c
> +++ b/fs/nfs/read.c
> @@ -109,7 +109,7 @@ static void nfs_readpage_truncate_uninitialised_page(struct nfs_read_data *data)
>  	}
>  }
>  
> -static void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
> +void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
>  		struct inode *inode)
>  {
>  	nfs_pageio_init(pgio, inode, &nfs_pageio_read_ops,
> @@ -534,23 +534,13 @@ static void nfs_readpage_result_full(struct rpc_task *task, void *calldata)
>  static void nfs_readpage_release_full(void *calldata)
>  {
>  	struct nfs_read_data *data = calldata;
> -	struct nfs_pageio_descriptor pgio;
>  
> -	if (data->pnfs_error) {
> -		nfs_pageio_init_read_mds(&pgio, data->inode);
> -		pgio.pg_recoalesce = 1;
> -	}
>  	while (!list_empty(&data->pages)) {
>  		struct nfs_page *req = nfs_list_entry(data->pages.next);
>  
>  		nfs_list_remove_request(req);
> -		if (!data->pnfs_error)
> -			nfs_readpage_release(req);
> -		else
> -			nfs_pageio_add_request(&pgio, req);
> +		nfs_readpage_release(req);
>  	}
> -	if (data->pnfs_error)
> -		nfs_pageio_complete(&pgio);
>  	nfs_readdata_release(calldata);
>  }
>  

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

* Re: [PATCH] NFS: Revert pnfs ugliness from the generic NFS read code path
  2011-11-14 11:12 ` Benny Halevy
@ 2011-11-14 15:49   ` Trond Myklebust
  2011-11-14 16:01     ` Benny Halevy
  0 siblings, 1 reply; 4+ messages in thread
From: Trond Myklebust @ 2011-11-14 15:49 UTC (permalink / raw)
  To: Benny Halevy; +Cc: linux-nfs

On Mon, 2011-11-14 at 13:12 +0200, Benny Halevy wrote: 
> On 2011-11-10 21:38, Trond Myklebust wrote:
> > pNFS-specific code belongs in the pnfs layer. It should not be
> > hijacking generic NFS read or write code paths.
> > 
> > Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
> > ---
> >  fs/nfs/internal.h |    2 ++
> >  fs/nfs/pnfs.c     |   26 +++++++++++++++++++++-----
> >  fs/nfs/read.c     |   14 ++------------
> >  3 files changed, 25 insertions(+), 17 deletions(-)
> > 
> > diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
> > index c1a1bd8..3f4d957 100644
> > --- a/fs/nfs/internal.h
> > +++ b/fs/nfs/internal.h
> > @@ -299,6 +299,8 @@ extern void nfs_read_prepare(struct rpc_task *task, void *calldata);
> >  extern int nfs_generic_pagein(struct nfs_pageio_descriptor *desc,
> >  		struct list_head *head);
> >  
> > +extern void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
> > +		struct inode *inode);
> >  extern void nfs_pageio_reset_read_mds(struct nfs_pageio_descriptor *pgio);
> >  extern void nfs_readdata_release(struct nfs_read_data *rdata);
> >  
> > diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
> > index baf7353..8e672a2 100644
> > --- a/fs/nfs/pnfs.c
> > +++ b/fs/nfs/pnfs.c
> > @@ -1260,6 +1260,25 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
> >  }
> >  EXPORT_SYMBOL_GPL(pnfs_generic_pg_writepages);
> >  
> > +static void pnfs_ld_handle_read_error(struct nfs_read_data *data)
> > +{
> > +	struct nfs_pageio_descriptor pgio;
> > +
> > +	put_lseg(data->lseg);
> > +	data->lseg = NULL;
> > +	dprintk("pnfs write error = %d\n", data->pnfs_error);
> 
> should be "read error", might as well fix this while you're at it :)

Sure...

> > +
> > +	nfs_pageio_init_read_mds(&pgio, data->inode);
> > +
> > +	while (!list_empty(&data->pages)) {
> > +		struct nfs_page *req = nfs_list_entry(data->pages.next);
> > +
> > +		nfs_list_remove_request(req);
> > +		nfs_pageio_add_request(&pgio, req);
> > +	}
> 
> What about pgio.pg_recoalesce?

It was incorrect to set it in the first place. This is an ordinary
coalesce...

-- 
Trond Myklebust
Linux NFS client maintainer

NetApp
Trond.Myklebust@netapp.com
www.netapp.com


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

* Re: [PATCH] NFS: Revert pnfs ugliness from the generic NFS read code path
  2011-11-14 15:49   ` Trond Myklebust
@ 2011-11-14 16:01     ` Benny Halevy
  0 siblings, 0 replies; 4+ messages in thread
From: Benny Halevy @ 2011-11-14 16:01 UTC (permalink / raw)
  To: Trond Myklebust; +Cc: linux-nfs

On 2011-11-14 17:49, Trond Myklebust wrote:
> On Mon, 2011-11-14 at 13:12 +0200, Benny Halevy wrote: 
>> On 2011-11-10 21:38, Trond Myklebust wrote:
>>> pNFS-specific code belongs in the pnfs layer. It should not be
>>> hijacking generic NFS read or write code paths.
>>>
>>> Signed-off-by: Trond Myklebust <Trond.Myklebust@netapp.com>
>>> ---
>>>  fs/nfs/internal.h |    2 ++
>>>  fs/nfs/pnfs.c     |   26 +++++++++++++++++++++-----
>>>  fs/nfs/read.c     |   14 ++------------
>>>  3 files changed, 25 insertions(+), 17 deletions(-)
>>>
>>> diff --git a/fs/nfs/internal.h b/fs/nfs/internal.h
>>> index c1a1bd8..3f4d957 100644
>>> --- a/fs/nfs/internal.h
>>> +++ b/fs/nfs/internal.h
>>> @@ -299,6 +299,8 @@ extern void nfs_read_prepare(struct rpc_task *task, void *calldata);
>>>  extern int nfs_generic_pagein(struct nfs_pageio_descriptor *desc,
>>>  		struct list_head *head);
>>>  
>>> +extern void nfs_pageio_init_read_mds(struct nfs_pageio_descriptor *pgio,
>>> +		struct inode *inode);
>>>  extern void nfs_pageio_reset_read_mds(struct nfs_pageio_descriptor *pgio);
>>>  extern void nfs_readdata_release(struct nfs_read_data *rdata);
>>>  
>>> diff --git a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c
>>> index baf7353..8e672a2 100644
>>> --- a/fs/nfs/pnfs.c
>>> +++ b/fs/nfs/pnfs.c
>>> @@ -1260,6 +1260,25 @@ pnfs_generic_pg_writepages(struct nfs_pageio_descriptor *desc)
>>>  }
>>>  EXPORT_SYMBOL_GPL(pnfs_generic_pg_writepages);
>>>  
>>> +static void pnfs_ld_handle_read_error(struct nfs_read_data *data)
>>> +{
>>> +	struct nfs_pageio_descriptor pgio;
>>> +
>>> +	put_lseg(data->lseg);
>>> +	data->lseg = NULL;
>>> +	dprintk("pnfs write error = %d\n", data->pnfs_error);
>>
>> should be "read error", might as well fix this while you're at it :)
> 
> Sure...
> 
>>> +
>>> +	nfs_pageio_init_read_mds(&pgio, data->inode);
>>> +
>>> +	while (!list_empty(&data->pages)) {
>>> +		struct nfs_page *req = nfs_list_entry(data->pages.next);
>>> +
>>> +		nfs_list_remove_request(req);
>>> +		nfs_pageio_add_request(&pgio, req);
>>> +	}
>>
>> What about pgio.pg_recoalesce?
> 
> It was incorrect to set it in the first place. This is an ordinary
> coalesce...
> 

I see.  Please just document that in the commit message...

Thanks,

Benny

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

end of thread, other threads:[~2011-11-14 16:01 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-10 19:38 [PATCH] NFS: Revert pnfs ugliness from the generic NFS read code path Trond Myklebust
2011-11-14 11:12 ` Benny Halevy
2011-11-14 15:49   ` Trond Myklebust
2011-11-14 16:01     ` Benny Halevy

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox