Re: Regarding patch "block/blk-mq: Don't complete locally if capacities are different"

MANISH PANDEY
2024-08-22T10:46:47Z

On 8/21/2024 10:52 PM, Bart Van Assche wrote:
> On 8/21/24 5:29 AM, MANISH PANDEY wrote:
>> How about introducing a new rq_affinity ( may be rq_affinity = 3) for
>> using cpus_equal_capacity() using new flag QUEUE_FLAG_SAME_CAPACITY.
>>
>> if (cpu == rq->mq_ctx->cpu ||
>>      (!test_bit(QUEUE_FLAG_SAME_FORCE, &rq->q->queue_flags) &&
>>        cpus_share_cache(cpu, rq->mq_ctx->cpu) &&
>> +      (test_bit(QUEUE_FLAG_CPU_CAPACITY, &rq->q->queue_flags))
>>         && cpus_equal_capacity(cpu, rq->mq_ctx->cpu)))
>>          return false;
>>
>> Could you please consider raising similar change, if this seems fine
>> for all.
>
> I'm not sure that a change like the above would be acceptable.
>
> What is the performance impact of the above change? Redirecting
> completion interrupts from a slow core to a fast core causes additional
> cache misses if the I/O was submitted from a slow core. Are there
> perhaps use cases for which the above change slows down I/O?
>
> Thanks,
>
> Bart.

Hi Bart,

> What is the performance impact of the above change?
No impact at all, as we are not changing the logic, we are just
proposing an on/off switch and give flexibility to users. Let the user
choose what's the best for their system.

Intention behind proposing a new flag is like we shouldn't break the
backward compatibility, as the change is also included in stable release
branches.

/* same CPU or cache domain and capacity? Complete locally */
if (cpu == rq->mq_ctx->cpu ||
(!test_bit(QUEUE_FLAG_SAME_FORCE, &rq->q->queue_flags) &&
cpus_share_cache(cpu, rq->mq_ctx->cpu) &&
+ (test_bit(QUEUE_FLAG_CPU_CAPACITY, &rq->q->queue_flags) ||
cpus_equal_capacity(cpu, rq->mq_ctx->cpu))))
return false;

So basically below would act as on/ off switch
QUEUE_FLAG_CPU_CAPACITY - with rq_affinity=1 , it will be clear
- with rq_affinity=3 , it will be set.

Regards
Mansih