Re: [PATCH v3 17/19] cifs: discard functions should not return failure

[Enzo Matsumiya <ematsumiya@suse.de>]

On 04/28, nspmangalore@gmail.com wrote:
>From: Shyam Prasad N <sprasad@microsoft.com>
>
>If discard functions for readv and query_dir return error, the
>callback functions can be skipped. This can end up with hung
>syscalls due to the completion functions not getting called.
>
>This change ensures that both these discard functions return
>success even when discard from socket returned error. This
>ensures that at least after the mid for the response is found,
>the callback doesn't get skipped, and we do not leave syscalls
>waiting.

Then we'll also be returning wrong errors to callers?

Either way, if this is a special case for query dir, just
call __cifs_readv_discard() from the new
cifs_query_dir_receive() and ignore the error there
(but you really shouldn't).

__cifs_readv_discard() is used in other places (mostly indirectly)
and returns important errors from deep down cifs_readv_from_socket()
(e.g. -ECONNABORTED, -ESHUTDOWN, -EINTR) so modifying it for all
callers doesn't sound good.

>Signed-off-by: Shyam Prasad N <sprasad@microsoft.com>
>---
> fs/smb/client/transport.c | 16 ++++++++--------
> 1 file changed, 8 insertions(+), 8 deletions(-)
>
>diff --git a/fs/smb/client/transport.c b/fs/smb/client/transport.c
>index 1e9e6f3f9a06f..85fa5751781a8 100644
>--- a/fs/smb/client/transport.c
>+++ b/fs/smb/client/transport.c
>@@ -1137,13 +1137,13 @@ static int
> __cifs_readv_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid,
> 		     bool malformed)
> {
>-	int length;
>-
>-	length = cifs_discard_remaining_data(server);
>+	cifs_discard_remaining_data(server);
> 	dequeue_mid(server, mid, malformed);
> 	mid->resp_buf = server->smallbuf;
> 	server->smallbuf = NULL;
>-	return length;
>+
>+	/* Once the mid is dequeued, the callback must run to terminate the subreq. */
>+	return 0;
> }
>
> static int
>@@ -1158,13 +1158,13 @@ static int
> cifs_query_dir_discard(struct TCP_Server_Info *server, struct mid_q_entry *mid,
> 		      bool malformed)
> {
>-	int length;
>-
>-	length = cifs_discard_remaining_data(server);
>+	cifs_discard_remaining_data(server);
> 	dequeue_mid(server, mid, malformed);
> 	mid->resp_buf = server->smallbuf;
> 	server->smallbuf = NULL;
>-	return length;
>+
>+	/* Once the mid is dequeued, the callback must run to complete qd_io->done. */
>+	return 0;
> }
>
> /*
>-- 
>2.43.0
>
>