On Apr 22, 2008, at 11:10 AM, Trond Myklebust wrote:
On Tue, 2008-04-22 at 10:19 -0400, Chuck Lever wrote:
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);
^^^^^^^^^^^^^^^^^^^^^^^^^^^^
You need to add this line to your patch.
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.
Then a fix should be applied to auth_gss.c: the EFAULT error message
is
coming from the downcall code. If it is not appropriate to pass that
on
to the RPC caller, then we should substitute an EACCES.
The way I read your patch, PTR_ERR(task) is never returned to
nfs_direct_read_schedule_segment's caller, so it's not possible for
the downcall error code to be propagated upwards.
Instead, if rpc_run_task() fails the _first_ time through the loop
(before any bytes have been started), the loop will exit, but "result"
will already be set to the number of pages grabbed by get_user_pages.
In other words, if no bytes are in flight, "result" will be equal to
or greater than zero, and the logic at the end of the function will
return EFAULT.
Am I wrong about that?
--
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
