On Apr 21, 2008, at 8:17 PM, Trond Myklebust wrote:
On Mon, 2008-04-21 at 15:55 -0400, Chuck Lever wrote:
Hi Trond-
On Apr 19, 2008, at 4:40 PM, Trond Myklebust wrote:
Signed-off-by: Trond Myklebust <Trond.Myklebust@xxxxxxxxxx>
---
fs/nfs/direct.c | 10 ++++++----
fs/nfs/read.c | 23 +++++++++++++++--------
fs/nfs/write.c | 33 +++++++++++++++++++--------------
3 files changed, 40 insertions(+), 26 deletions(-)
diff --git a/fs/nfs/direct.c b/fs/nfs/direct.c
index abf8e02..4757a2b 100644
--- a/fs/nfs/direct.c
+++ b/fs/nfs/direct.c
@@ -347,8 +347,9 @@ static ssize_t
nfs_direct_read_schedule_segment(struct nfs_direct_req *dreq,
NFS_PROTO(inode)->read_setup(data, &msg);
task = rpc_run_task(&task_setup_data);
- if (!IS_ERR(task))
- rpc_put_task(task);
+ if (IS_ERR(task))
+ break;
+ rpc_put_task(task);
dprintk("NFS: %5u initiated direct read call "
"(req %s/%Ld, %zu bytes @ offset %Lu)\n",
My reading of this logic suggests that if the very first
rpc_run_task() call in the loop fails, we'll return -EFAULT instead
of
something sensible, like -ENOMEM.
I'm confused. How could ENOMEM be a sensible substitute for EFAULT?
My point is that EFAULT isn't an appropriate return code if
rpc_run_task() fails. EFAULT is appropriate only when the I/O engine
can't access the user's memory.
If no bytes were read, an error code that reflects the problem should
be returned.
task = rpc_run_task(&task_setup_data);
if (IS_ERR(task) {
result = PTR_ERR(task);
break;
}
rpc_put_task(task);
The logic at the end of nfs_direct_read_schedule_segment() will take
care to return the number of bytes read, or an error code if no bytes
were read.
--
Chuck Lever
chuck[dot]lever[at]oracle[dot]com
--
To unsubscribe from this list: send the line "unsubscribe linux-nfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
