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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <688ead11-c1c0-48b2-b4d1-feeb1278c692@quicinc.com>
Date: Thu, 22 Aug 2024 16:16:20 +0530
From: MANISH PANDEY <quic_mapa@...cinc.com>
To: Bart Van Assche <bvanassche@....org>,
        Sandeep Dhavale
	<dhavale@...gle.com>,
        Dietmar Eggemann <dietmar.eggemann@....com>
CC: Qais Yousef <qyousef@...alina.io>,
        Christian Loehle
	<christian.loehle@....com>, <axboe@...nel.dk>,
        <mingo@...nel.org>, <peterz@...radead.org>,
        <vincent.guittot@...aro.org>, <linux-block@...r.kernel.org>,
        <sudeep.holla@....com>, Jaegeuk Kim
	<jaegeuk@...nel.org>,
        Christoph Hellwig <hch@...radead.org>, <kailash@...gle.com>,
        <tkjos@...gle.com>, <bvanassche@...gle.com>,
        <quic_nitirawa@...cinc.com>, <quic_cang@...cinc.com>,
        <quic_rampraka@...cinc.com>, <quic_narepall@...cinc.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>
Subject: Re: Regarding patch "block/blk-mq: Don't complete locally if
 capacities are different"



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