Re: [PATCH 26/26] netfs, afs: Use writeback retry to deal with alternate keys

Simon Horman
2024-04-01T13:53:58Z

On Thu, Mar 28, 2024 at 04:34:18PM +0000, David Howells wrote:

...

> +void afs_issue_write(struct netfs_io_subrequest *subreq)
> {
> + struct netfs_io_request *wreq = subreq->rreq;
> struct afs_operation *op;
> - struct afs_wb_key *wbk = NULL;
> - loff_t size = iov_iter_count(iter);
> + struct afs_vnode *vnode = AFS_FS_I(wreq->inode);
> + unsigned long long pos = subreq->start + subreq->transferred;
> + size_t len = subreq->len - subreq->transferred;
> int ret = -ENOKEY;
>
> - _enter("%s{%llx:%llu.%u},%llx,%llx",
> + _enter("R=%x[%x],%s{%llx:%llu.%u},%llx,%zx",
> + wreq->debug_id, subreq->debug_index,
> vnode->volume->name,
> vnode->fid.vid,
> vnode->fid.vnode,
> vnode->fid.unique,
> - size, pos);
> + pos, len);
>
> - ret = afs_get_writeback_key(vnode, &wbk);
> - if (ret) {
> - _leave(" = %d [no keys]", ret);
> - return ret;
> - }
> +#if 0 // Error injection
> + if (subreq->debug_index == 3)
> + return netfs_write_subrequest_terminated(subreq, -ENOANO, false);
>
> - op = afs_alloc_operation(wbk->key, vnode->volume);
> - if (IS_ERR(op)) {
> - afs_put_wb_key(wbk);
> - return -ENOMEM;
> + if (!test_bit(NETFS_SREQ_RETRYING, &subreq->flags)) {
> + set_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags);
> + return netfs_write_subrequest_terminated(subreq, -EAGAIN, false);
> }
> +#endif
> +
> + op = afs_alloc_operation(wreq->netfs_priv, vnode->volume);
> + if (IS_ERR(op))
> + return netfs_write_subrequest_terminated(subreq, -EAGAIN, false);
>
> afs_op_set_vnode(op, 0, vnode);
> - op->file[0].dv_delta = 1;
> + op->file[0].dv_delta = 1;
> op->file[0].modification = true;
> - op->store.pos = pos;
> - op->store.size = size;
> - op->flags |= AFS_OPERATION_UNINTR;
> - op->ops = &afs_store_data_operation;
> + op->store.pos = pos;
> + op->store.size = len,

nit: this is probably more intuitively written using len;

> + op->flags |= AFS_OPERATION_UNINTR;
> + op->ops = &afs_store_data_operation;
>
> -try_next_key:
> afs_begin_vnode_operation(op);
>
> - op->store.write_iter = iter;
> - op->store.i_size = max(pos + size, vnode->netfs.remote_i_size);
> - op->mtime = inode_get_mtime(&vnode->netfs.inode);
> + op->store.write_iter = &subreq->io_iter;
> + op->store.i_size = umax(pos + len, vnode->netfs.remote_i_size);
> + op->mtime = inode_get_mtime(&vnode->netfs.inode);
>
> afs_wait_for_operation(op);
> -
> - switch (afs_op_error(op)) {
> + ret = afs_put_operation(op);
> + switch (ret) {
> case -EACCES:
> case -EPERM:
> case -ENOKEY:
> case -EKEYEXPIRED:
> case -EKEYREJECTED:
> case -EKEYREVOKED:
> - _debug("next");
> -
> - ret = afs_get_writeback_key(vnode, &wbk);
> - if (ret == 0) {
> - key_put(op->key);
> - op->key = key_get(wbk->key);
> - goto try_next_key;
> - }
> + /* If there are more keys we can try, use the retry algorithm
> + * to rotate the keys.
> + */
> + if (wreq->netfs_priv2)
> + set_bit(NETFS_SREQ_NEED_RETRY, &subreq->flags);
> break;
> }
>
> - afs_put_wb_key(wbk);
> - _leave(" = %d", afs_op_error(op));
> - return afs_put_operation(op);
> + netfs_write_subrequest_terminated(subreq, ret < 0 ? ret : subreq->len, false);
> }
>
> /*

...