> Make sure we don't have a repeat of this situation by changing the
> API to explicitly return ENOSPC when we fail to allocate. If we fail
> to capture this correctly, it will lead to failures being noticed
> either by ENOSPC escaping to userspace or by causing filesystem
> shutdowns when allocations failure where they really shouldn't.
Yes, the retur 0 on ENOSPC has driven me crazy in the past.
Note that you now also drop the XXX comment on xfs_alloc_vextent_finish
about this.
> diff --git a/fs/xfs/libxfs/xfs_alloc.c b/fs/xfs/libxfs/xfs_alloc.c
> index 27c62f303488..13fda27fabcb 100644
> --- a/fs/xfs/libxfs/xfs_alloc.c
> +++ b/fs/xfs/libxfs/xfs_alloc.c
> @@ -1157,9 +1157,9 @@ xfs_alloc_ag_vextent_small(
> * Can't do the allocation, give up.
> */
> if (flen < args->minlen) {
> - args->agbno = NULLAGBLOCK;
> trace_xfs_alloc_small_notenough(args);
> - flen = 0;
> + error = -ENOSPC;
> + goto error;
I suspect a direct return -ENOSPC here might be better, as we already
have the trace_xfs_alloc_small_notenough tracepoint here, and also
hitting trace_xfs_alloc_small_error wouldn't make much sense (and be
a pointles behavior change).
Looking at the callers of xfs_alloc_ag_vextent_small, both seem to
need an update to check for -ENOSPC explicitly, as they first check
for an error and only after that for i == 0 || len == 0 to detect
the no space case.
> @@ -3375,14 +3370,7 @@ xfs_alloc_vextent_finish(
> args->agno > minimum_agno))
> args->tp->t_highest_agno = args->agno;
>
> - /*
> - * If the allocation failed with an error or we had an ENOSPC result,
> - * preserve the returned error whilst also marking the allocation result
> - * as "no extent allocated". This ensures that callers that fail to
> - * capture the error will still treat it as a failed allocation.
> - */
> - if (alloc_error || args->agbno == NULLAGBLOCK) {
> - args->fsbno = NULLFSBLOCK;
> + if (alloc_error) {
> error = alloc_error;
> goto out_drop_perag;
> }
Maybe throw in a
ASSERT(args->agbno != NULLAGBLOCK);
after this conditional to catch backporting errors and the like?
