On Thu, Aug 11, 2022 at 10:14 AM Logan Gunthorpe <logang@xxxxxxxxxxxx> wrote:
>
> Refactor raid5_get_active_stripe() without the gotos with an
> explicit infinite loop and some additional nesting.
>
> Suggested-by: Christoph Hellwig <hch@xxxxxxxxxxxxx>
> Signed-off-by: Logan Gunthorpe <logang@xxxxxxxxxxxx>
Applied to md-next.
Thanks!
Song
> ---
> drivers/md/raid5.c | 82 +++++++++++++++++++++++-----------------------
> 1 file changed, 41 insertions(+), 41 deletions(-)
>
> diff --git a/drivers/md/raid5.c b/drivers/md/raid5.c
> index 4456ac51f7c5..1288ef9e1571 100644
> --- a/drivers/md/raid5.c
> +++ b/drivers/md/raid5.c
> @@ -811,54 +811,54 @@ static struct stripe_head *__raid5_get_active_stripe(struct r5conf *conf,
>
> spin_lock_irq(conf->hash_locks + hash);
>
> -retry:
> - if (!noquiesce && conf->quiesce) {
> - /*
> - * Must release the reference to batch_last before waiting,
> - * on quiesce, otherwise the batch_last will hold a reference
> - * to a stripe and raid5_quiesce() will deadlock waiting for
> - * active_stripes to go to zero.
> - */
> - if (ctx && ctx->batch_last) {
> - raid5_release_stripe(ctx->batch_last);
> - ctx->batch_last = NULL;
> - }
> -
> - wait_event_lock_irq(conf->wait_for_quiescent, !conf->quiesce,
> - *(conf->hash_locks + hash));
> - }
> + for (;;) {
> + if (!noquiesce && conf->quiesce) {
> + /*
> + * Must release the reference to batch_last before
> + * waiting, on quiesce, otherwise the batch_last will
> + * hold a reference to a stripe and raid5_quiesce()
> + * will deadlock waiting for active_stripes to go to
> + * zero.
> + */
> + if (ctx && ctx->batch_last) {
> + raid5_release_stripe(ctx->batch_last);
> + ctx->batch_last = NULL;
> + }
>
> - sh = find_get_stripe(conf, sector, conf->generation - previous, hash);
> - if (sh)
> - goto out;
> + wait_event_lock_irq(conf->wait_for_quiescent,
> + !conf->quiesce,
> + *(conf->hash_locks + hash));
> + }
>
> - if (test_bit(R5_INACTIVE_BLOCKED, &conf->cache_state))
> - goto wait_for_stripe;
> + sh = find_get_stripe(conf, sector, conf->generation - previous,
> + hash);
> + if (sh)
> + break;
>
> - sh = get_free_stripe(conf, hash);
> - if (sh) {
> - r5c_check_stripe_cache_usage(conf);
> - init_stripe(sh, sector, previous);
> - atomic_inc(&sh->count);
> - goto out;
> - }
> + if (!test_bit(R5_INACTIVE_BLOCKED, &conf->cache_state)) {
> + sh = get_free_stripe(conf, hash);
> + if (sh) {
> + r5c_check_stripe_cache_usage(conf);
> + init_stripe(sh, sector, previous);
> + atomic_inc(&sh->count);
> + break;
> + }
>
> - if (!test_bit(R5_DID_ALLOC, &conf->cache_state))
> - set_bit(R5_ALLOC_MORE, &conf->cache_state);
> + if (!test_bit(R5_DID_ALLOC, &conf->cache_state))
> + set_bit(R5_ALLOC_MORE, &conf->cache_state);
> + }
>
> -wait_for_stripe:
> - if (noblock)
> - goto out;
> + if (noblock)
> + break;
>
> - set_bit(R5_INACTIVE_BLOCKED, &conf->cache_state);
> - r5l_wake_reclaim(conf->log, 0);
> - wait_event_lock_irq(conf->wait_for_stripe,
> - is_inactive_blocked(conf, hash),
> - *(conf->hash_locks + hash));
> - clear_bit(R5_INACTIVE_BLOCKED, &conf->cache_state);
> - goto retry;
> + set_bit(R5_INACTIVE_BLOCKED, &conf->cache_state);
> + r5l_wake_reclaim(conf->log, 0);
> + wait_event_lock_irq(conf->wait_for_stripe,
> + is_inactive_blocked(conf, hash),
> + *(conf->hash_locks + hash));
> + clear_bit(R5_INACTIVE_BLOCKED, &conf->cache_state);
> + }
>
> -out:
> spin_unlock_irq(conf->hash_locks + hash);
> return sh;
> }
> --
> 2.30.2
>
