Currently, at the completion of a storage RPC from writepages, the errors
ENOSPC, EDQUOT, ENOKEY, EACCES, EPERM, EKEYREJECTED and EKEYREVOKED cause
the pages involved to be redirtied and the write to be retried by the VM at
a future time.
However, this is probably not the right thing to do, and, instead, the
writes should be discarded so that the system doesn't get blocked (though
unmounting will discard the uncommitted writes anyway).
Fix this by making afs_write_back_from_locked_page() call afs_kill_pages()
instead of afs_redirty_pages() in those cases.
EKEYEXPIRED is left to redirty the pages on the assumption that the caller
just needs to renew their key. Unknown errors also do that, though it
might be better to squelch those too.
This can be triggered by the generic/285 xfstest. The writes can be
observed in the server logs. If a write fails with ENOSPC (ie. CODE
49733403, UAENOSPC) because a file is made really large, e.g.:
Wed Nov 03 23:21:35.794133 2021 [1589] EVENT YFS_SRX_StData CODE 49733403 NAME --UnAuth-- HOST [192.168.1.2]:7001 ID 32766 FID 1048664:0.172306:30364251 UINT64 17592187027456 UINT64 65536 UINT64 17592187092992 UINT64 0
this should be seen once and not repeated.
Reported-by: Marc Dionne <[email protected]>
Signed-off-by: David Howells <[email protected]>
cc: Jeffrey E Altman <[email protected]>
cc: [email protected]
---
fs/afs/write.c | 14 +++++---------
1 file changed, 5 insertions(+), 9 deletions(-)
diff --git a/fs/afs/write.c b/fs/afs/write.c
index 8b1d9c2f6bec..04f3f87b15cb 100644
--- a/fs/afs/write.c
+++ b/fs/afs/write.c
@@ -620,22 +620,18 @@ static ssize_t afs_write_back_from_locked_page(struct address_space *mapping,
default:
pr_notice("kAFS: Unexpected error from FS.StoreData %d\n", ret);
fallthrough;
- case -EACCES:
- case -EPERM:
- case -ENOKEY:
case -EKEYEXPIRED:
- case -EKEYREJECTED:
- case -EKEYREVOKED:
afs_redirty_pages(wbc, mapping, start, len);
mapping_set_error(mapping, ret);
break;
+ case -EACCES:
+ case -EPERM:
+ case -ENOKEY:
+ case -EKEYREJECTED:
+ case -EKEYREVOKED:
case -EDQUOT:
case -ENOSPC:
- afs_redirty_pages(wbc, mapping, start, len);
- mapping_set_error(mapping, -ENOSPC);
- break;
-
case -EROFS:
case -EIO:
case -EREMOTEIO:
Looks good to me.
Reviewed-by Jeffrey Altman
On 11/3/2021 4:43 PM, David Howells ([email protected]) wrote:
> Currently, at the completion of a storage RPC from writepages, the errors
> ENOSPC, EDQUOT, ENOKEY, EACCES, EPERM, EKEYREJECTED and EKEYREVOKED cause
> the pages involved to be redirtied and the write to be retried by the VM at
> a future time.
>
> However, this is probably not the right thing to do, and, instead, the
> writes should be discarded so that the system doesn't get blocked (though
> unmounting will discard the uncommitted writes anyway).
>
> Fix this by making afs_write_back_from_locked_page() call afs_kill_pages()
> instead of afs_redirty_pages() in those cases.
>
> EKEYEXPIRED is left to redirty the pages on the assumption that the caller
> just needs to renew their key. Unknown errors also do that, though it
> might be better to squelch those too.
>
> This can be triggered by the generic/285 xfstest. The writes can be
> observed in the server logs. If a write fails with ENOSPC (ie. CODE
> 49733403, UAENOSPC) because a file is made really large, e.g.:
>
> Wed Nov 03 23:21:35.794133 2021 [1589] EVENT YFS_SRX_StData CODE 49733403 NAME --UnAuth-- HOST [192.168.1.2]:7001 ID 32766 FID 1048664:0.172306:30364251 UINT64 17592187027456 UINT64 65536 UINT64 17592187092992 UINT64 0
>
> this should be seen once and not repeated.
>
> Reported-by: Marc Dionne <[email protected]>
> Signed-off-by: David Howells <[email protected]>
> cc: Jeffrey E Altman <[email protected]>
> cc: [email protected]
> ---
>
> fs/afs/write.c | 14 +++++---------
> 1 file changed, 5 insertions(+), 9 deletions(-)
>
> diff --git a/fs/afs/write.c b/fs/afs/write.c
> index 8b1d9c2f6bec..04f3f87b15cb 100644
> --- a/fs/afs/write.c
> +++ b/fs/afs/write.c
> @@ -620,22 +620,18 @@ static ssize_t afs_write_back_from_locked_page(struct address_space *mapping,
> default:
> pr_notice("kAFS: Unexpected error from FS.StoreData %d\n", ret);
> fallthrough;
> - case -EACCES:
> - case -EPERM:
> - case -ENOKEY:
> case -EKEYEXPIRED:
> - case -EKEYREJECTED:
> - case -EKEYREVOKED:
> afs_redirty_pages(wbc, mapping, start, len);
> mapping_set_error(mapping, ret);
> break;
>
> + case -EACCES:
> + case -EPERM:
> + case -ENOKEY:
> + case -EKEYREJECTED:
> + case -EKEYREVOKED:
> case -EDQUOT:
> case -ENOSPC:
> - afs_redirty_pages(wbc, mapping, start, len);
> - mapping_set_error(mapping, -ENOSPC);
> - break;
> -
> case -EROFS:
> case -EIO:
> case -EREMOTEIO:
>
>
On Wed, Nov 03, 2021 at 11:43:20PM +0000, David Howells wrote:
> Currently, at the completion of a storage RPC from writepages, the errors
> ENOSPC, EDQUOT, ENOKEY, EACCES, EPERM, EKEYREJECTED and EKEYREVOKED cause
> the pages involved to be redirtied and the write to be retried by the VM at
> a future time.
>
> However, this is probably not the right thing to do, and, instead, the
> writes should be discarded so that the system doesn't get blocked (though
> unmounting will discard the uncommitted writes anyway).
umm. I'm not sure that throwing away the write is the best answer
for some of these errors. Our whole story around error handling in
filesystems, the page cache and the VFS is pretty sad, but I don't think
that this is the right approach.
Ideally, we'd hold onto the writes in the page cache until (eg for ENOSPC
/ EDQUOT), the user has deleted some files, then retry the writes.
We should definitely stop the user dirtying more pages on this mount,
or at least throttle processes which are dirtying new pages (eg in
folio_mark_dirty()), which implies a check of the superblock. Until the
ENOSPC is cleared up, at which time writeback can resume ... of course,
the server won't necessarily notify us when it is cleared up (because
it might be due to a different client filling the storage), so we might
need to peridically re-attempt writeback so that we know whether ENOSPC
has been resolved.
On 11/3/2021 8:22 PM, Matthew Wilcox ([email protected]) wrote:
> On Wed, Nov 03, 2021 at 11:43:20PM +0000, David Howells wrote:
>> Currently, at the completion of a storage RPC from writepages, the errors
>> ENOSPC, EDQUOT, ENOKEY, EACCES, EPERM, EKEYREJECTED and EKEYREVOKED cause
>> the pages involved to be redirtied and the write to be retried by the VM at
>> a future time.
>>
>> However, this is probably not the right thing to do, and, instead, the
>> writes should be discarded so that the system doesn't get blocked (though
>> unmounting will discard the uncommitted writes anyway).
> umm. I'm not sure that throwing away the write is the best answer
> for some of these errors. Our whole story around error handling in
> filesystems, the page cache and the VFS is pretty sad, but I don't think
> that this is the right approach.
>
> Ideally, we'd hold onto the writes in the page cache until (eg for ENOSPC
> / EDQUOT), the user has deleted some files, then retry the writes.
Hi Matthew,
I agree that it would be desirable to avoid discarding user data but in
practice that is hard to do. The proposed behavior change is consistent
with other Unix AFS/AuriStorFS cache manager implementations. There
are many situations which can result in an out of quota or out of space
error where the end user has absolutely no ability to do anything about it.
An EDQUOT error might occur because the AFS volume has reached its
quota. However, the writer only has insert privilege and cannot
delete. The user might not even be able to list the contents of the
volume.
An ENOSPC error might be the result of the backing store for AFS vice
partitions filling due to data being written to other AFS volumes that
the writer has no ability to access or manage.
AFS cache managers frequently implement write-on-close semantics and
will flush dirty content to the fileserver only when the file is closed
or the local cache is out-of-space. Holding onto dirty data that
cannot be flushed to the server on a multi-user timeshare system can
result on unwanted negative impacts on other users of the system.
Another risk is that if dirty data persists locally that the
EDQUOT/ENOSPC errors will be replaced by EACCES or EPERM errors when the
associated authentication credentials expire.
If a back-off strategy is to be implemented in the future, AFS does
provide RPCs that can be used to query the volume's online status, the
maximum quota in one KiB blocks, the blocks in use, the available blocks
in the partition, and the maximum number of blocks in the partition.
Querying RXAFS_GetVolumeStatus or RXYFS_GetVolumeStatus can avoid the
overhead of issuing a StoreData operation that is likely to fail.
Jeffrey Altman