Skip to content

Commit

Permalink
afs: Fix cleanup of immediately failed async calls
Browse files Browse the repository at this point in the history
If we manage to begin an async call, but fail to transmit any data on it
due to a signal, we then abort it which causes a race between the
notification of call completion from rxrpc and our attempt to cancel the
notification.  The notification will be necessary, however, for async
FetchData to terminate the netfs subrequest.

However, since we get a notification from rxrpc upon completion of a call
(aborted or otherwise), we can just leave it to that.

This leads to calls not getting cleaned up, but appearing in
/proc/net/rxrpc/calls as being aborted with code 6.

Fix this by making the "error_do_abort:" case of afs_make_call() abort the
call and then abandon it to the notification handler.

Fixes: 34fa476 ("afs: Fix race in async call refcounting")
Reported-by: Marc Dionne <marc.dionne@auristor.com>
Signed-off-by: David Howells <dhowells@redhat.com>
Link: https://lore.kernel.org/r/20241216204124.3752367-25-dhowells@redhat.com
cc: linux-afs@lists.infradead.org
Signed-off-by: Christian Brauner <brauner@kernel.org>
  • Loading branch information
David Howells authored and Christian Brauner committed Dec 20, 2024
1 parent f28fc20 commit 9750be9
Show file tree
Hide file tree
Showing 3 changed files with 20 additions and 3 deletions.
9 changes: 9 additions & 0 deletions fs/afs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -1336,6 +1336,15 @@ extern void afs_send_simple_reply(struct afs_call *, const void *, size_t);
extern int afs_extract_data(struct afs_call *, bool);
extern int afs_protocol_error(struct afs_call *, enum afs_eproto_cause);

static inline void afs_see_call(struct afs_call *call, enum afs_call_trace why)
{
int r = refcount_read(&call->ref);

trace_afs_call(call->debug_id, why, r,
atomic_read(&call->net->nr_outstanding_calls),
__builtin_return_address(0));
}

static inline void afs_make_op_call(struct afs_operation *op, struct afs_call *call,
gfp_t gfp)
{
Expand Down
12 changes: 9 additions & 3 deletions fs/afs/rxrpc.c
Original file line number Diff line number Diff line change
Expand Up @@ -430,11 +430,16 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
return;

error_do_abort:
if (ret != -ECONNABORTED) {
if (ret != -ECONNABORTED)
rxrpc_kernel_abort_call(call->net->socket, rxcall,
RX_USER_ABORT, ret,
afs_abort_send_data_error);
} else {
if (call->async) {
afs_see_call(call, afs_call_trace_async_abort);
return;
}

if (ret == -ECONNABORTED) {
len = 0;
iov_iter_kvec(&msg.msg_iter, ITER_DEST, NULL, 0, 0);
rxrpc_kernel_recv_data(call->net->socket, rxcall,
Expand All @@ -445,6 +450,8 @@ void afs_make_call(struct afs_call *call, gfp_t gfp)
call->error = ret;
trace_afs_call_done(call);
error_kill_call:
if (call->async)
afs_see_call(call, afs_call_trace_async_kill);
if (call->type->done)
call->type->done(call);

Expand Down Expand Up @@ -602,7 +609,6 @@ static void afs_deliver_to_call(struct afs_call *call)
abort_code = 0;
call_complete:
afs_set_call_complete(call, ret, remote_abort);
state = AFS_CALL_COMPLETE;
goto done;
}

Expand Down
2 changes: 2 additions & 0 deletions include/trace/events/afs.h
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,8 @@ enum yfs_cm_operation {
*/
#define afs_call_traces \
EM(afs_call_trace_alloc, "ALLOC") \
EM(afs_call_trace_async_abort, "ASYAB") \
EM(afs_call_trace_async_kill, "ASYKL") \
EM(afs_call_trace_free, "FREE ") \
EM(afs_call_trace_get, "GET ") \
EM(afs_call_trace_put, "PUT ") \
Expand Down

0 comments on commit 9750be9

Please sign in to comment.