linux-kernel - Re: [RFC] Scheduler: DMA Engine regression because of sched/fair changes

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>] [day] [month] [year] [list]
Message-ID: <20220119125513.bpnf563tjc2u6g47@yadro.com>
Date:   Wed, 19 Jan 2022 15:55:13 +0300
From:   Alexander Fomichev <fomichev.ru@...il.com>
To:     Hillf Danton <hdanton@...a.com>
Cc:     Mel Gorman <mgorman@...e.de>, linux-kernel@...r.kernel.org,
        dmaengine@...r.kernel.org, linux@...ro.com,
        Peter Zijlstra <peterz@...radead.org>
Subject: Re: [RFC] Scheduler: DMA Engine regression because of sched/fair
 changes

On Tue, Jan 18, 2022 at 10:04:48AM +0800, Hillf Danton wrote:
> On Mon, 17 Jan 2022 20:44:19 +0300 Alexander Fomichev wrote:
> > On Mon, Jan 17, 2022 at 10:27:01AM +0000, Mel Gorman wrote:
> > > > 1) You're right. When options "noverify=1" and "polling=1" are used.
> > > > then no performance reducing occurs.
> > > 
> > > How about just noverify=1 on its own? It's a stronger indicator that
> > > cache hotness is a factor.
> > > 
> > 
> > With "noverify=1 polled=0" the performance reduction is only 10-20%,
> > but still exists.
> > 
> > -----< v5.15.8-vanilla >-----
> > [17057.866760] dmatest: Added 1 threads using dma0chan0
> > [17060.133880] dmatest: Started 1 threads using dma0chan0
> > [17060.154343] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 49338.85 iops 3157686 KB/s (0)
> > [17063.737887] dmatest: Added 1 threads using dma0chan0
> > [17065.113838] dmatest: Started 1 threads using dma0chan0
> > [17065.137659] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 42183.41 iops 2699738 KB/s (0)
> > [17100.339989] dmatest: Added 1 threads using dma0chan0
> > [17102.190764] dmatest: Started 1 threads using dma0chan0
> > [17102.214285] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 42844.89 iops 2742073 KB/s (0)
> > -----< end >-----
> > 
> > -----< 5.15.8-ioat-ptdma-dirty-fix+ >-----
> > [ 6183.356549] dmatest: Added 1 threads using dma0chan0
> > [ 6187.868237] dmatest: Started 1 threads using dma0chan0
> > [ 6187.887389] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 52753.74 iops 3376239 KB/s (0)
> > [ 6201.913154] dmatest: Added 1 threads using dma0chan0
> > [ 6204.701340] dmatest: Started 1 threads using dma0chan0
> > [ 6204.720490] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 52614.96 iops 3367357 KB/s (0)
> > [ 6285.114603] dmatest: Added 1 threads using dma0chan0
> > [ 6287.031875] dmatest: Started 1 threads using dma0chan0
> > [ 6287.050278] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 54939.01 iops 3516097 KB/s (0)
> > -----< end >-----
> > 
> 
> Check if cold cache provides some room for selecting CPU.
> 
> Only for thoughts now.
> 
> Hillf
> 
> +++ x/kernel/sched/fair.c
> @@ -5889,19 +5889,16 @@ static int
>  wake_affine_idle(int this_cpu, int prev_cpu, int sync)
>  {
>  	/*
> -	 * If this_cpu is idle, it implies the wakeup is from interrupt
> -	 * context. Only allow the move if cache is shared. Otherwise an
> -	 * interrupt intensive workload could force all tasks onto one
> -	 * node depending on the IO topology or IRQ affinity settings.
> -	 *
> -	 * If the prev_cpu is idle and cache affine then avoid a migration.
> -	 * There is no guarantee that the cache hot data from an interrupt
> -	 * is more important than cache hot data on the prev_cpu and from
> -	 * a cpufreq perspective, it's better to have higher utilisation
> -	 * on one CPU.
> +	 * select this cpu if both are idle because of
> +	 * cold shared cache
>  	 */
> -	if (available_idle_cpu(this_cpu) && cpus_share_cache(this_cpu, prev_cpu))
> -		return available_idle_cpu(prev_cpu) ? prev_cpu : this_cpu;
> +	if (cpus_share_cache(this_cpu, prev_cpu)) {
> +		if (available_idle_cpu(this_cpu))
> +			return this_cpu;
> +
> +		if (available_idle_cpu(prev_cpu))
> +			return prev_cpu;
> +	}
>  
>  	if (sync && cpu_rq(this_cpu)->nr_running == 1)
>  		return this_cpu;

Hi Hillf,

The results with your patch are controversial:

-----< v5.15.8-Hillf-Danton-patch+ >-----
[ 1572.178884] dmatest: Added 1 threads using dma0chan0
[ 1577.413535] dmatest: Started 1 threads using dma0chan0
[ 1577.432495] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 53188.66 iops 3404074 KB/s (0)
[ 1592.356173] dmatest: Added 1 threads using dma0chan0
[ 1593.791100] dmatest: Started 1 threads using dma0chan0
[ 1593.815282] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 41668.40 iops 2666777 KB/s (0)
[ 1617.117040] dmatest: Added 1 threads using dma0chan0
[ 1619.545890] dmatest: Started 1 threads using dma0chan0
[ 1619.569639] dmatest: dma0chan0-copy0: summary 1000 tests, 0 failures 42426.81 iops 2715316 KB/s (0)
-----< end >-----

Just to remind, used dmatest parameters:

/sys/module/dmatest/parameters/iterations:1000
/sys/module/dmatest/parameters/alignment:-1
/sys/module/dmatest/parameters/verbose:N
/sys/module/dmatest/parameters/norandom:Y
/sys/module/dmatest/parameters/max_channels:0
/sys/module/dmatest/parameters/dmatest:0
/sys/module/dmatest/parameters/polled:N
/sys/module/dmatest/parameters/threads_per_chan:1
/sys/module/dmatest/parameters/noverify:Y
/sys/module/dmatest/parameters/test_buf_size:1048576
/sys/module/dmatest/parameters/transfer_size:65536
/sys/module/dmatest/parameters/run:N
/sys/module/dmatest/parameters/wait:Y
/sys/module/dmatest/parameters/timeout:2000
/sys/module/dmatest/parameters/xor_sources:3
/sys/module/dmatest/parameters/pq_sources:3


-- 
Regards,
  Alexander