Re: [PATCH v4] io_uring: limit inflight IO

[Jens Axboe <axboe@kernel.dk>]

On 11/10/19 6:36 AM, Pavel Begunkov wrote:
>> @@ -2997,6 +2997,31 @@ static bool io_get_sqring(struct io_ring_ctx *ctx, struct sqe_submit *s)
>>   	return false;
>>   }
>>   
>> +static bool io_sq_over_limit(struct io_ring_ctx *ctx, unsigned to_submit)
>> +{
>> +	unsigned inflight;
>> +
>> +	if (!list_empty(&ctx->cq_overflow_list)) {
>> +		io_cqring_overflow_flush(ctx, false);
>> +		return true;
>> +	}
>> +
>> +	/*
>> +	 * This doesn't need to be super precise, so only check every once
>> +	 * in a while.
>> +	 */
>> +	if ((ctx->cached_sq_head & ~ctx->sq_mask) !=
>> +	    ((ctx->cached_sq_head + to_submit) & ~ctx->sq_mask))
>> +		return false;
> 
> Still not quite agree, with "!=" it will check almost every time.
> We want to continue with the checks only when we overrun sq_entries
> (i.e. changed higher bits).
> 
> 
> There is a simplified trace: let
> 1) mask == 3, ~mask == 4
> 2) head == 0,
> 3) submitting by one
> 
> 1: (0 & 4) != (1 & 4): false // do checks
> 2: (1 & 4) != (2 & 4): false // do checks
> 3: (2 & 4) != (3 & 4): false // do checks
> 4: (3 & 4) != (4 & 4): true // skip, return true

Huh yes, it's reversed, isn't it. Should be:

	if ((ctx->cached_sq_head & ~ctx->sq_mask) ==
	    ((ctx->cached_sq_head + to_submit) & ~ctx->sq_mask))
		return false;

> The rest looks good,
> Reviewed-by: Pavel Begunkov <asml.silence@gmail.com>

Thanks, I'll make the above change too.

-- 
Jens Axboe