Skip to content

Commit

Permalink
afs: Simplify error handling
Browse files Browse the repository at this point in the history
Simplify error handling a bit by moving it from the afs_addr_cursor struct
to the afs_operation and afs_vl_cursor structs and using the error
prioritisation function for accumulating errors from multiple sources (AFS
tries to rotate between multiple fileservers, some of which may be
inaccessible or in some state of offlinedness).

Signed-off-by: David Howells <dhowells@redhat.com>
cc: Marc Dionne <marc.dionne@auristor.com>
cc: linux-afs@lists.infradead.org
  • Loading branch information
David Howells committed Dec 24, 2023
1 parent 6f2ff7e commit aa453be
Show file tree
Hide file tree
Showing 15 changed files with 174 additions and 143 deletions.
8 changes: 3 additions & 5 deletions fs/afs/addr_list.c
Original file line number Diff line number Diff line change
Expand Up @@ -386,26 +386,24 @@ bool afs_iterate_addresses(struct afs_addr_cursor *ac)
selected:
ac->index = index;
set_bit(index, &ac->tried);
ac->responded = false;
ac->call_responded = false;
return true;
}

/*
* Release an address list cursor.
*/
int afs_end_cursor(struct afs_addr_cursor *ac)
void afs_end_cursor(struct afs_addr_cursor *ac)
{
struct afs_addr_list *alist;

alist = ac->alist;
if (alist) {
if (ac->responded &&
if (ac->call_responded &&
ac->index != alist->preferred &&
test_bit(ac->alist->preferred, &ac->tried))
WRITE_ONCE(alist->preferred, ac->index);
afs_put_addrlist(alist);
ac->alist = NULL;
}

return ac->error;
}
14 changes: 8 additions & 6 deletions fs/afs/dir.c
Original file line number Diff line number Diff line change
Expand Up @@ -693,8 +693,9 @@ static void afs_do_lookup_success(struct afs_operation *op)
vp = &op->file[0];
abort_code = vp->scb.status.abort_code;
if (abort_code != 0) {
op->ac.abort_code = abort_code;
op->error = afs_abort_to_error(abort_code);
op->call_abort_code = abort_code;
afs_op_set_error(op, afs_abort_to_error(abort_code));
op->cumul_error.abort_code = abort_code;
}
break;

Expand Down Expand Up @@ -846,13 +847,14 @@ static struct inode *afs_do_lookup(struct inode *dir, struct dentry *dentry,
_debug("nr_files %u", op->nr_files);

/* Need space for examining all the selected files */
op->error = -ENOMEM;
if (op->nr_files > 2) {
op->more_files = kvcalloc(op->nr_files - 2,
sizeof(struct afs_vnode_param),
GFP_KERNEL);
if (!op->more_files)
if (!op->more_files) {
afs_op_nomem(op);
goto out_op;
}

for (i = 2; i < op->nr_files; i++) {
vp = &op->more_files[i - 2];
Expand Down Expand Up @@ -1255,7 +1257,7 @@ void afs_check_for_remote_deletion(struct afs_operation *op)
{
struct afs_vnode *vnode = op->file[0].vnode;

switch (op->ac.abort_code) {
switch (afs_op_abort_code(op)) {
case VNOVNODE:
set_bit(AFS_VNODE_DELETED, &vnode->flags);
afs_break_callback(vnode, afs_cb_break_for_deleted);
Expand All @@ -1280,7 +1282,7 @@ static void afs_vnode_new_inode(struct afs_operation *op)
/* ENOMEM or EINTR at a really inconvenient time - just abandon
* the new directory on the server.
*/
op->error = PTR_ERR(inode);
afs_op_accumulate_error(op, PTR_ERR(inode), 0);
return;
}

Expand Down
2 changes: 1 addition & 1 deletion fs/afs/dir_silly.c
Original file line number Diff line number Diff line change
Expand Up @@ -218,7 +218,7 @@ static int afs_do_silly_unlink(struct afs_vnode *dvnode, struct afs_vnode *vnode
/* If there was a conflict with a third party, check the status of the
* unlinked vnode.
*/
if (op->error == 0 && (op->flags & AFS_OPERATION_DIR_CONFLICT)) {
if (op->cumul_error.error == 0 && (op->flags & AFS_OPERATION_DIR_CONFLICT)) {
op->file[1].update_ctime = false;
op->fetch_status.which = 1;
op->ops = &afs_fetch_status_operation;
Expand Down
3 changes: 0 additions & 3 deletions fs/afs/file.c
Original file line number Diff line number Diff line change
Expand Up @@ -245,10 +245,7 @@ static void afs_fetch_data_notify(struct afs_operation *op)
struct netfs_io_subrequest *subreq = req->subreq;
int error = afs_op_error(op);

if (error == -ECONNABORTED)
error = afs_abort_to_error(op->ac.abort_code);
req->error = error;

if (subreq) {
__set_bit(NETFS_SREQ_CLEAR_TAIL, &subreq->flags);
netfs_subreq_terminated(subreq, error ?: req->actual_len, false);
Expand Down
24 changes: 12 additions & 12 deletions fs/afs/fs_operation.c
Original file line number Diff line number Diff line change
Expand Up @@ -169,9 +169,6 @@ static void afs_end_vnode_operation(struct afs_operation *op)
}

afs_drop_io_locks(op);

if (op->error == -ECONNABORTED)
op->error = afs_abort_to_error(op->ac.abort_code);
}

/*
Expand All @@ -182,35 +179,38 @@ void afs_wait_for_operation(struct afs_operation *op)
_enter("");

while (afs_select_fileserver(op)) {
op->call_error = 0;
op->call_abort_code = 0;
op->cb_s_break = op->server->cb_s_break;
if (test_bit(AFS_SERVER_FL_IS_YFS, &op->server->flags) &&
op->ops->issue_yfs_rpc)
op->ops->issue_yfs_rpc(op);
else if (op->ops->issue_afs_rpc)
op->ops->issue_afs_rpc(op);
else
op->ac.error = -ENOTSUPP;
op->call_error = -ENOTSUPP;

if (op->call) {
afs_wait_for_call_to_complete(op->call, &op->ac);
op->error = op->ac.error;
op->call_abort_code = op->call->abort_code;
op->call_error = op->call->error;
op->call_responded = op->call->responded;
op->ac.call_responded = true;
WRITE_ONCE(op->ac.alist->addrs[op->ac.index].last_error,
op->call_error);
afs_put_call(op->call);
}
}

switch (op->error) {
case 0:
if (!afs_op_error(op)) {
_debug("success");
op->ops->success(op);
break;
case -ECONNABORTED:
} else if (op->cumul_error.aborted) {
if (op->ops->aborted)
op->ops->aborted(op);
fallthrough;
default:
} else {
if (op->ops->failed)
op->ops->failed(op);
break;
}

afs_end_vnode_operation(op);
Expand Down
1 change: 1 addition & 0 deletions fs/afs/fsclient.c
Original file line number Diff line number Diff line change
Expand Up @@ -1629,6 +1629,7 @@ int afs_fs_give_up_all_callbacks(struct afs_net *net,
call->server = afs_use_server(server, afs_server_trace_give_up_cb);
afs_make_call(ac, call, GFP_NOFS);
afs_wait_for_call_to_complete(call, ac);
ret = call->error;
afs_put_call(call);
return ret;
}
Expand Down
44 changes: 31 additions & 13 deletions fs/afs/internal.h
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ enum afs_call_state {
struct afs_address {
struct rxrpc_peer *peer;
u16 service_id;
short last_error; /* Last error from this address */
};

/*
Expand Down Expand Up @@ -121,7 +122,6 @@ struct afs_call {
};
void *buffer; /* reply receive buffer */
union {
long ret0; /* Value to reply with instead of 0 */
struct afs_addr_list *ret_alist;
struct afs_vldb_entry *ret_vldb;
char *ret_str;
Expand All @@ -145,6 +145,7 @@ struct afs_call {
bool upgrade; /* T to request service upgrade */
bool intr; /* T if interruptible */
bool unmarshalling_error; /* T if an unmarshalling error occurred */
bool responded; /* Got a response from the call (may be abort) */
u16 service_id; /* Actual service ID (after upgrade) */
unsigned int debug_id; /* Trace ID */
u32 operation_ID; /* operation ID for an incoming call */
Expand Down Expand Up @@ -719,8 +720,10 @@ struct afs_permits {
* Error prioritisation and accumulation.
*/
struct afs_error {
short error; /* Accumulated error */
s32 abort_code; /* Cumulative abort code */
short error; /* Cumulative error */
bool responded; /* T if server responded */
bool aborted; /* T if ->error is from an abort */
};

/*
Expand All @@ -730,10 +733,8 @@ struct afs_addr_cursor {
struct afs_addr_list *alist; /* Current address list (pins ref) */
unsigned long tried; /* Tried addresses */
signed char index; /* Current address */
bool responded; /* T if the current address responded */
unsigned short nr_iterations; /* Number of address iterations */
short error;
u32 abort_code;
bool call_responded;
};

/*
Expand All @@ -746,13 +747,16 @@ struct afs_vl_cursor {
struct afs_vlserver *server; /* Server on which this resides */
struct key *key; /* Key for the server */
unsigned long untried; /* Bitmask of untried servers */
struct afs_error cumul_error; /* Cumulative error */
s32 call_abort_code;
short index; /* Current server */
short error;
short call_error; /* Error from single call */
unsigned short flags;
#define AFS_VL_CURSOR_STOP 0x0001 /* Set to cease iteration */
#define AFS_VL_CURSOR_RETRY 0x0002 /* Set to do a retry */
#define AFS_VL_CURSOR_RETRIED 0x0004 /* Set if started a retry */
unsigned short nr_iterations; /* Number of server iterations */
short nr_iterations; /* Number of server iterations */
bool call_responded; /* T if the current address responded */
};

/*
Expand Down Expand Up @@ -803,8 +807,10 @@ struct afs_operation {
struct dentry *dentry_2; /* Second dentry to be altered */
struct timespec64 mtime; /* Modification time to record */
struct timespec64 ctime; /* Change time to set */
struct afs_error cumul_error; /* Cumulative error */
short nr_files; /* Number of entries in file[], more_files */
short error;
short call_error; /* Error from single call */
s32 call_abort_code; /* Abort code from single call */
unsigned int debug_id;

unsigned int cb_v_break; /* Volume break counter before op */
Expand Down Expand Up @@ -860,6 +866,8 @@ struct afs_operation {
unsigned long untried; /* Bitmask of untried servers */
short index; /* Current server */
short nr_iterations; /* Number of server iterations */
bool call_responded; /* T if the current address responded */


unsigned int flags;
#define AFS_OPERATION_STOP 0x0001 /* Set to cease iteration */
Expand Down Expand Up @@ -976,7 +984,7 @@ bool afs_addr_list_same(const struct afs_addr_list *a,
const struct afs_addr_list *b);
extern struct afs_vlserver_list *afs_dns_query(struct afs_cell *, time64_t *);
extern bool afs_iterate_addresses(struct afs_addr_cursor *);
extern int afs_end_cursor(struct afs_addr_cursor *);
extern void afs_end_cursor(struct afs_addr_cursor *ac);

extern int afs_merge_fs_addr4(struct afs_net *net, struct afs_addr_list *addr,
__be32 xdr, u16 port);
Expand Down Expand Up @@ -1235,17 +1243,27 @@ extern void afs_prioritise_error(struct afs_error *, int, u32);

static inline void afs_op_nomem(struct afs_operation *op)
{
op->error = -ENOMEM;
op->cumul_error.error = -ENOMEM;
}

static inline int afs_op_error(const struct afs_operation *op)
{
return op->error;
return op->cumul_error.error;
}

static inline s32 afs_op_abort_code(const struct afs_operation *op)
{
return op->cumul_error.abort_code;
}

static inline int afs_op_set_error(struct afs_operation *op, int error)
{
return op->error = error;
return op->cumul_error.error = error;
}

static inline void afs_op_accumulate_error(struct afs_operation *op, int error, s32 abort_code)
{
afs_prioritise_error(&op->cumul_error, error, abort_code);
}

/*
Expand Down Expand Up @@ -1619,7 +1637,7 @@ static inline void afs_update_dentry_version(struct afs_operation *op,
struct afs_vnode_param *dir_vp,
struct dentry *dentry)
{
if (!op->error)
if (!op->cumul_error.error)
dentry->d_fsdata =
(void *)(unsigned long)dir_vp->scb.status.data_version;
}
Expand Down
10 changes: 8 additions & 2 deletions fs/afs/misc.c
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,8 @@ void afs_prioritise_error(struct afs_error *e, int error, u32 abort_code)
{
switch (error) {
case 0:
e->aborted = false;
e->error = 0;
return;
default:
if (e->error == -ETIMEDOUT ||
Expand Down Expand Up @@ -161,12 +163,16 @@ void afs_prioritise_error(struct afs_error *e, int error, u32 abort_code)
if (e->responded)
return;
e->error = error;
e->aborted = false;
return;

case -ECONNABORTED:
error = afs_abort_to_error(abort_code);
fallthrough;
e->error = afs_abort_to_error(abort_code);
e->aborted = true;
e->responded = true;
return;
case -ENETRESET: /* Responded, but we seem to have changed address */
e->aborted = false;
e->responded = true;
e->error = error;
return;
Expand Down
Loading

0 comments on commit aa453be

Please sign in to comment.