On 16/04/2021 14:04, Jens Axboe wrote:
> On 4/15/21 6:26 PM, Pavel Begunkov wrote:
>> On 16/04/2021 01:22, Pavel Begunkov wrote:
>>> Late catched 5.12 bug with nasty hangs. Thanks Jens for a reproducer.
>>
>> 1/2 is basically a rip off of one of old Jens' patches, but can't
>> find it anywhere. If you still have it, especially if it was
>> reviewed/etc., may make sense to go with it instead
>
> I wonder if we can do something like the below instead - we don't
> care about a particularly stable count in terms of wakeup
> reliance, and it'd save a nasty sync atomic switch.
But we care about it being monotonous. There are nuances with it.
I think, non sync'ed summing may put it to eternal sleep.
Are you looking to save on switching? It's almost always is already
dying with prior ref_kill
>
> Totally untested...
>
>
> diff --git a/fs/io_uring.c b/fs/io_uring.c
> index 6c182a3a221b..9edbcf01ea49 100644
> --- a/fs/io_uring.c
> +++ b/fs/io_uring.c
> @@ -8928,7 +8928,7 @@ static void io_uring_cancel_sqpoll(struct io_ring_ctx *ctx)
> atomic_inc(&tctx->in_idle);
> do {
> /* read completions before cancelations */
> - inflight = tctx_inflight(tctx, false);
> + inflight = percpu_ref_sum(&ctx->refs);
> if (!inflight)
> break;
> io_uring_try_cancel_requests(ctx, current, NULL);
> @@ -8939,7 +8939,7 @@ static void io_uring_cancel_sqpoll(struct io_ring_ctx *ctx)
> * avoids a race where a completion comes in before we did
> * prepare_to_wait().
> */
> - if (inflight == tctx_inflight(tctx, false))
> + if (inflight == percpu_ref_sum(&ctx->refs))
> schedule();
> finish_wait(&tctx->wait, &wait);
> } while (1);
> diff --git a/include/linux/percpu-refcount.h b/include/linux/percpu-refcount.h
> index 16c35a728b4c..2f29f34bc993 100644
> --- a/include/linux/percpu-refcount.h
> +++ b/include/linux/percpu-refcount.h
> @@ -131,6 +131,7 @@ void percpu_ref_kill_and_confirm(struct percpu_ref *ref,
> void percpu_ref_resurrect(struct percpu_ref *ref);
> void percpu_ref_reinit(struct percpu_ref *ref);
> bool percpu_ref_is_zero(struct percpu_ref *ref);
> +long percpu_ref_sum(struct percpu_ref *ref);
>
> /**
> * percpu_ref_kill - drop the initial ref
> diff --git a/lib/percpu-refcount.c b/lib/percpu-refcount.c
> index a1071cdefb5a..b09ed9fdd32d 100644
> --- a/lib/percpu-refcount.c
> +++ b/lib/percpu-refcount.c
> @@ -475,3 +475,31 @@ void percpu_ref_resurrect(struct percpu_ref *ref)
> spin_unlock_irqrestore(&percpu_ref_switch_lock, flags);
> }
> EXPORT_SYMBOL_GPL(percpu_ref_resurrect);
> +
> +/**
> + * percpu_ref_sum - return approximate ref counts
> + * @ref: perpcu_ref to sum
> + *
> + * Note that this should only really be used to compare refs, as by the
> + * very nature of percpu references, the value may be stale even before it
> + * has been returned.
> + */
> +long percpu_ref_sum(struct percpu_ref *ref)
> +{
> + unsigned long __percpu *percpu_count;
> + long ret;
> +
> + rcu_read_lock();
> + if (__ref_is_percpu(ref, &percpu_count)) {
> + ret = atomic_long_read(&ref->data->count);
> + } else {
> + int cpu;
> +
> + ret = 0;
> + for_each_possible_cpu(cpu)
> + ret += *per_cpu_ptr(percpu_count, cpu);
> + }
> + rcu_read_unlock();
> +
> + return ret;
> +}
>
--
Pavel Begunkov
