On Thu, Oct 19, 2023 at 09:27:46AM -0400, Brian Foster wrote:
> A recent bug report uncovered a scenario where a filesystem never
> runs with freespace_initialized, and therefore the user observes
> significantly degraded write performance by virtue of running the
> early bucket allocator. The associated bug aside, the primary cause
> of the performance drop in this particular instance is that the
> early bucket allocator does not update the allocation cursor. This
> means that every allocation walks the alloc btree from the first
> bucket of the associated device looking for a bucket marked as free
> space.
> 
> Update the early allocator code to set the alloc cursor to the
> prospectively allocated bucket, similar to how the freelist
> allocator behaves. This improves performance of the early bucket
> allocator dramatically (even though it should be bypassed in favor
> of the freelist allocator in most cases).
> 
> Signed-off-by: Brian Foster <[email protected]>
> ---
> 
> cshepherd on #bcache originally reported the early bucket allocator
> problem and helped chase it down to what looks like a members_v2
> regression. I believe he was planning to post a patch for that one.
> 
> Brian
> 
>  fs/bcachefs/alloc_foreground.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/bcachefs/alloc_foreground.c b/fs/bcachefs/alloc_foreground.c
> index 3bc4abd3d7d5..be3fc0f38c79 100644
> --- a/fs/bcachefs/alloc_foreground.c
> +++ b/fs/bcachefs/alloc_foreground.c
> @@ -431,7 +431,7 @@ bch2_bucket_alloc_early(struct btree_trans *trans,
>       }
>       bch2_trans_iter_exit(trans, &iter);
>  
> -     ca->alloc_cursor = alloc_cursor;
> +     ca->alloc_cursor = IS_ERR_OR_NULL(ob) ? alloc_cursor : ob->bucket;

Oh, this code is broken. The local alloc_cursor never gets updated, and
it needs to for where we check if we need to loop around.

The proper fix would be to add

  alloc_cursor = iter.pos.offset;

before the line you changed

Reply via email to