2011-04-15 08:29:39

by M. Mohan Kumar

Subject: [PATCH] net/9p: Handle get_user_pages_fast return properly

Use proper data type to handle get_user_pages_fast error condition. Also
do not treat EFAULT error as fatal.

Signed-off-by: M. Mohan Kumar <[email protected]>
---
net/9p/client.c | 2 +-
net/9p/trans_common.c | 11 +++--------
2 files changed, 4 insertions(+), 9 deletions(-)

diff --git a/net/9p/client.c b/net/9p/client.c
index 691a04a..1612142 100644
--- a/net/9p/client.c
+++ b/net/9p/client.c
@@ -614,7 +614,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)

err = c->trans_mod->request(c, req);
if (err < 0) {
- if (err != -ERESTARTSYS)
+ if (err != -ERESTARTSYS && err != -EFAULT)
c->status = Disconnected;
goto reterr;
}
diff --git a/net/9p/trans_common.c b/net/9p/trans_common.c
index 14265e8..f020da9 100644
--- a/net/9p/trans_common.c
+++ b/net/9p/trans_common.c
@@ -63,7 +63,7 @@ p9_payload_gup(struct p9_req_t *req, size_t *pdata_off, int *pdata_len,
int nr_pages, u8 rw)
{
uint32_t first_page_bytes = 0;
- uint32_t pdata_mapped_pages;
+ int32_t pdata_mapped_pages;
struct trans_rpage_info *rpinfo;

*pdata_off = (__force size_t)req->tc->pubuf & (PAGE_SIZE-1);
@@ -75,14 +75,9 @@ p9_payload_gup(struct p9_req_t *req, size_t *pdata_off, int *pdata_len,
rpinfo = req->tc->private;
pdata_mapped_pages = get_user_pages_fast((unsigned long)req->tc->pubuf,
nr_pages, rw, &rpinfo->rp_data[0]);
+ if (pdata_mapped_pages <= 0)
+ return pdata_mapped_pages;

- if (pdata_mapped_pages < 0) {
- printk(KERN_ERR "get_user_pages_fast failed:%d udata:%p"
- "nr_pages:%d\n", pdata_mapped_pages,
- req->tc->pubuf, nr_pages);
- pdata_mapped_pages = 0;
- return -EIO;
- }
rpinfo->rp_nr_pages = pdata_mapped_pages;
if (*pdata_off) {
*pdata_len = first_page_bytes;
--
1.7.4

2011-04-15 14:53:30

by Venkateswararao Jujjuri

Subject: Re: [V9fs-developer] [PATCH] net/9p: Handle get_user_pages_fast return properly

On 04/15/2011 01:29 AM, M. Mohan Kumar wrote:
> Use proper data type to handle get_user_pages_fast error condition. Also
> do not treat EFAULT error as fatal.
>
> Signed-off-by: M. Mohan Kumar<[email protected]>
> ---
> net/9p/client.c | 2 +-
> net/9p/trans_common.c | 11 +++--------
> 2 files changed, 4 insertions(+), 9 deletions(-)
>
> diff --git a/net/9p/client.c b/net/9p/client.c
> index 691a04a..1612142 100644
> --- a/net/9p/client.c
> +++ b/net/9p/client.c
> @@ -614,7 +614,7 @@ p9_client_rpc(struct p9_client *c, int8_t type, const char *fmt, ...)
>
> err = c->trans_mod->request(c, req);
> if (err< 0) {
> - if (err != -ERESTARTSYS)
> + if (err != -ERESTARTSYS&& err != -EFAULT)
Please analyze other errors that could be returned by
c->trans_mod->request()
if they need any handling here.

> c->status = Disconnected;
> goto reterr;
> }
> diff --git a/net/9p/trans_common.c b/net/9p/trans_common.c
> index 14265e8..f020da9 100644
> --- a/net/9p/trans_common.c
> +++ b/net/9p/trans_common.c
> @@ -63,7 +63,7 @@ p9_payload_gup(struct p9_req_t *req, size_t *pdata_off, int *pdata_len,
> int nr_pages, u8 rw)
> {
> uint32_t first_page_bytes = 0;
> - uint32_t pdata_mapped_pages;
> + int32_t pdata_mapped_pages;
> struct trans_rpage_info *rpinfo;
>
> *pdata_off = (__force size_t)req->tc->pubuf& (PAGE_SIZE-1);
> @@ -75,14 +75,9 @@ p9_payload_gup(struct p9_req_t *req, size_t *pdata_off, int *pdata_len,
> rpinfo = req->tc->private;
> pdata_mapped_pages = get_user_pages_fast((unsigned long)req->tc->pubuf,
> nr_pages, rw,&rpinfo->rp_data[0]);
> + if (pdata_mapped_pages<= 0)
> + return pdata_mapped_pages;
>
> - if (pdata_mapped_pages< 0) {
> - printk(KERN_ERR "get_user_pages_fast failed:%d udata:%p"
> - "nr_pages:%d\n", pdata_mapped_pages,
> - req->tc->pubuf, nr_pages);
> - pdata_mapped_pages = 0;
> - return -EIO;
> - }
I would keep the printk and return the pdata_mapped_pages inside the if
condition.

Thanks,
JV

> rpinfo->rp_nr_pages = pdata_mapped_pages;
> if (*pdata_off) {
> *pdata_len = first_page_bytes;

2011-04-19 15:24:54

by M. Mohan Kumar

Subject: Re: [V9fs-developer] [PATCH] net/9p: Handle get_user_pages_fast return properly

On Friday 15 April 2011 8:23:16 pm Venkateswararao Jujjuri wrote:
> >
> > - if (err != -ERESTARTSYS)
> > + if (err != -ERESTARTSYS&& err != -EFAULT)
>
> Please analyze other errors that could be returned by
> c->trans_mod->request()
> if they need any handling here.

Thanks JV, but we don't need to handle other errors as of now.

I will post a V2 patch with retaining printing error message.
----
M. Mohan Kumar

2011-04-19 23:41:31

by Venkateswararao Jujjuri

Subject: Re: [V9fs-developer] [PATCH] net/9p: Handle get_user_pages_fast return properly

On 04/19/2011 08:24 AM, M. Mohan Kumar wrote:
> On Friday 15 April 2011 8:23:16 pm Venkateswararao Jujjuri wrote:
>>> - if (err != -ERESTARTSYS)
>>> + if (err != -ERESTARTSYS&& err != -EFAULT)
>> Please analyze other errors that could be returned by
>> c->trans_mod->request()
>> if they need any handling here.
> Thanks JV, but we don't need to handle other errors as of now.
*request could also return -EIO and -ENOMEM.
As you said, may be we should leave the default behavior on these
errors(suspend the channel).

Thanks,
JV
> I will post a V2 patch with retaining printing error message.
> ----
> M. Mohan Kumar