linux-kernel - Re: [REGRESSION] ioprio performance hangs, bisected

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] [day] [month] [year] [list]

Message-ID: <CAP-bSRab1C-_aaATfrgWjt9w0fcYUCQCG7u+TCb1FSPSd6CEaA@mail.gmail.com>
Date: Mon, 25 Nov 2024 17:16:39 +0000
From: Chris Bainbridge <chris.bainbridge@...il.com>
To: hch@....de
Cc: LKML <linux-kernel@...r.kernel.org>, axboe@...nel.dk, bvanassche@....org, 
	Linux regressions mailing list <regressions@...ts.linux.dev>, linux-block@...r.kernel.org, 
	semen.protsenko@...aro.org
Subject: Re: [REGRESSION] ioprio performance hangs, bisected

On Mon, 25 Nov 2024 at 15:44, Chris Bainbridge
<chris.bainbridge@...il.com> wrote:
>
> The commit 6975c1a486a4 ("block: remove the ioprio field from struct
> request") appears to have introduced a performance regression. Test
> case is the script /etc/cron.daily/locate from package `locate` (which
> is from findutils locate on Debian). The script runs:

I did a bit of debugging.

The problem is the function req_get_ioprio. The commit changed:

 static inline unsigned short req_get_ioprio(struct request *req)
 {
-       return req->ioprio;
+       if (req->bio)
+               return req->bio->bi_ioprio;
+       return 0;
 }

So when req->bio is NULL this function now returns 0. But previously the values
of req->ioprio were sometimes non-zero. If I revert the commit and then
instrument req_get_ioprio to log where the new return value differs:

  static inline unsigned short req_get_ioprio(struct request *req)
 {
-       return req->ioprio;
+       if (req->bio)
+       {
+              if (req->bio->bi_ioprio != req->ioprio)
+                      printk("req->bio->bi_ioprio != req->ioprio\n");
+               return req->bio->bi_ioprio;
+       }
+       if (req->ioprio != 0)
+               printk("bad ioprio 0 != %u\n", (unsigned int)req->ioprio);
+       return 0;
 }

then log shows:

[   36.922906] bad ioprio 0 != 16387
[   36.923061] bad ioprio 0 != 16387
[   36.930186] bad ioprio 0 != 16387
[   36.930680] bad ioprio 0 != 16387
[   78.875421] bad ioprio 0 != 24583
[   79.228801] bad ioprio 0 != 24583
[   87.411118] bad ioprio 0 != 24583
[   97.419607] bad ioprio 0 != 24583
[   97.421059] bad ioprio 0 != 24583
[  107.210364] bad ioprio 0 != 24583
[  107.210775] bad ioprio 0 != 24583

So, the new function is returning 0 when it would've previously returned these
non-zero values, and returning zero breaks things.