Hi,
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
The test case in question is pretty simple. On debian testing, do:
$ sudo apt-get install mariadb-test
$ cd /usr/share/mysql/mysql-test
$ ./mtr --mysqld=--innodb-flush-method=fsync --mysqld=--innodb-use-native-aio=1 --vardir=/dev/shm/mysql --force encryption.innodb_encryption,innodb,undo0 --repeat=200
and if it fails, you'll see something like:
encryption.innodb_encryption 'innodb,undo0' [ 6 pass ] 3120
encryption.innodb_encryption 'innodb,undo0' [ 7 pass ] 3123
encryption.innodb_encryption 'innodb,undo0' [ 8 pass ] 3042
encryption.innodb_encryption 'innodb,undo0' [ 9 fail ]
Test ended at 2023-03-23 16:55:17
CURRENT_TEST: encryption.innodb_encryption
mysqltest: At line 11: query 'SET @start_global_value = @@global.innodb_encryption_threads' failed: ER_UNKNOWN_SYSTEM_VARIABLE (1193): Unknown system variable 'innodb_encryption_threads'
The result from queries just before the failure was:
SET @start_global_value = @@global.innodb_encryption_threads;
- saving '/dev/shm/mysql/log/encryption.innodb_encryption-innodb,undo0/' to '/dev/shm/mysql/log/encryption.innodb_encryption-innodb,undo0/'
***Warnings generated in error logs during shutdown after running tests: encryption.innodb_encryption
2023-03-23 16:55:17 0 [Warning] Plugin 'example_key_management' is of maturity level experimental while the server is stable
2023-03-23 16:55:17 0 [ERROR] InnoDB: Database page corruption on disk or a failed read of file './ibdata1' page [page id: space=0, page number=221]. You may have to recover from a backup.
where data read was not as expected.
Now, there are a number of io_uring changes between .158 and .162, as it
includes the backport that brought 5.10-stable into line with what
5.15-stable includes. I'll spare you all the digging I did to vet those
changes, but the key thing is that it STILL happens on 6.3-git on
powerpc.
After ruling out many things, one key difference between 158 and 162 is
that the former offloaded requests that could not be done nonblocking to
a kthread, and 162 and newer offloads to an IO thread. An IO thread is
just a normal thread created from the application submitting IO, the
only difference is that it never exits to userspace. An IO thread has
the same mm/files/you-name-it from the original task. It really is the
same as a userspace thread created by the application The switch to IO
threads was done exactly because of that, rather than rely on a fragile
scheme of having the kthread worker assume all sorts of identify from
the original task. surprises if things were missed. This is what caused
most of the io_uring security issues in the past.
The IO that mariadb does in this test is pretty simple - a bunch of
largish buffered writes with IORING_OP_WRITEV, and some smallish (16K)
buffered reads with IORING_OP_READV.
Today I finally gave up and ran a basic experiment, which simply
offloads the writes to a kthread. Since powerpc has an interesting
memory coherency model, my suspicion was that the work involved with
switching MMs for the kthread could just be the main difference here.
The patch is really dumb and simple - rather than queue the write to an
IO thread, it just offloads it to a kthread that then does
kthread_use_mm(), perform write with the same write handler,
kthread_unuse_mm(). AND THIS WORKS! Usually the above mtr test would
fail in 2..20 loops, I've now done 200 and 500 loops and it's fine.
Which then leads me to the question, what about the IO thread offload
makes this fail on powerpc (and no other arch I've tested on, including
x86/x86-64/aarch64/hppa64)? The offload should be equivalent to having a
thread in userspace in the application, and having that thread just
perform the writes. Is there some magic involved with the kthread mm
use/unuse that makes this sufficiently consistent on powerpc? I've tried
any mix of isync()/mb and making the flush_dcache_page() unconditionally
done in the filemap read/write helpers, and it still falls flat on its
face with the offload to an IO thread.
I must clearly be missing something here, which is why I'm emailing the
powerpc Gods for help :-)
--
Jens Axboe
Hi,
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
Which variant of powerpc ? 32 or 64 bits ? Book3S or BookE ?
Christophe
The test case in question is pretty simple. On debian testing, do:
$ sudo apt-get install mariadb-test
$ cd /usr/share/mysql/mysql-test
$ ./mtr --mysqld=--innodb-flush-method=fsync --mysqld=--innodb-use-native-aio=1 --vardir=/dev/shm/mysql --force encryption.innodb_encryption,innodb,undo0 --repeat=200
and if it fails, you'll see something like:
encryption.innodb_encryption 'innodb,undo0' [ 6 pass ] 3120
encryption.innodb_encryption 'innodb,undo0' [ 7 pass ] 3123
encryption.innodb_encryption 'innodb,undo0' [ 8 pass ] 3042
encryption.innodb_encryption 'innodb,undo0' [ 9 fail ]
Test ended at 2023-03-23 16:55:17
CURRENT_TEST: encryption.innodb_encryption
mysqltest: At line 11: query 'SET @start_global_value = @@global.innodb_encryption_threads' failed: ER_UNKNOWN_SYSTEM_VARIABLE (1193): Unknown system variable 'innodb_encryption_threads'
The result from queries just before the failure was:
SET @start_global_value = @@global.innodb_encryption_threads;
- saving '/dev/shm/mysql/log/encryption.innodb_encryption-innodb,undo0/' to '/dev/shm/mysql/log/encryption.innodb_encryption-innodb,undo0/'
***Warnings generated in error logs during shutdown after running tests: encryption.innodb_encryption
2023-03-23 16:55:17 0 [Warning] Plugin 'example_key_management' is of maturity level experimental while the server is stable
2023-03-23 16:55:17 0 [ERROR] InnoDB: Database page corruption on disk or a failed read of file './ibdata1' page [page id: space=0, page number=221]. You may have to recover from a backup.
where data read was not as expected.
Now, there are a number of io_uring changes between .158 and .162, as it
includes the backport that brought 5.10-stable into line with what
5.15-stable includes. I'll spare you all the digging I did to vet those
changes, but the key thing is that it STILL happens on 6.3-git on
powerpc.
After ruling out many things, one key difference between 158 and 162 is
that the former offloaded requests that could not be done nonblocking to
a kthread, and 162 and newer offloads to an IO thread. An IO thread is
just a normal thread created from the application submitting IO, the
only difference is that it never exits to userspace. An IO thread has
the same mm/files/you-name-it from the original task. It really is the
same as a userspace thread created by the application The switch to IO
threads was done exactly because of that, rather than rely on a fragile
scheme of having the kthread worker assume all sorts of identify from
the original task. surprises if things were missed. This is what caused
most of the io_uring security issues in the past.
The IO that mariadb does in this test is pretty simple - a bunch of
largish buffered writes with IORING_OP_WRITEV, and some smallish (16K)
buffered reads with IORING_OP_READV.
Today I finally gave up and ran a basic experiment, which simply
offloads the writes to a kthread. Since powerpc has an interesting
memory coherency model, my suspicion was that the work involved with
switching MMs for the kthread could just be the main difference here.
The patch is really dumb and simple - rather than queue the write to an
IO thread, it just offloads it to a kthread that then does
kthread_use_mm(), perform write with the same write handler,
kthread_unuse_mm(). AND THIS WORKS! Usually the above mtr test would
fail in 2..20 loops, I've now done 200 and 500 loops and it's fine.
Which then leads me to the question, what about the IO thread offload
makes this fail on powerpc (and no other arch I've tested on, including
x86/x86-64/aarch64/hppa64)? The offload should be equivalent to having a
thread in userspace in the application, and having that thread just
perform the writes. Is there some magic involved with the kthread mm
use/unuse that makes this sufficiently consistent on powerpc? I've tried
any mix of isync()/mb and making the flush_dcache_page() unconditionally
done in the filemap read/write helpers, and it still falls flat on its
face with the offload to an IO thread.
I must clearly be missing something here, which is why I'm emailing the
powerpc Gods for help :-)
Hi,
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
Which variant of powerpc ? 32 or 64 bits ? Book3S or BookE ?
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2023-03-25 00:16:24
Jens Axboe [off-list ref] writes:
On 3/24/23 1:27?AM, Christophe Leroy wrote:
quoted
Le 23/03/2023 ? 19:54, Jens Axboe a ?crit :
quoted
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
Which variant of powerpc ? 32 or 64 bits ? Book3S or BookE ?
Believe it or not there's still more variables in play, Power9 has two
different MMUs, and Linux can run on two different hypervisors as well
as on bare metal P9 :)
Can you paste the last ~10 lines of /proc/cpuinfo, with the "machine",
"firmware" and "MMU" lines, that should tell us everything we need to
know.
cheers
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
Which variant of powerpc ? 32 or 64 bits ? Book3S or BookE ?
Believe it or not there's still more variables in play, Power9 has two
different MMUs, and Linux can run on two different hypervisors as well
as on bare metal P9 :)
Just my luck :-)
Can you paste the last ~10 lines of /proc/cpuinfo, with the "machine",
"firmware" and "MMU" lines, that should tell us everything we need to
know.
timebase : 512000000
platform : pSeries
model : IBM pSeries (emulated by qemu)
machine : CHRP IBM pSeries (emulated by qemu)
MMU : Radix
I know it reproduces bare metal as well, but no details on what
that box is. I just know I was able to reproduce it in this vm that
I was able to get my hands on.
--
Jens Axboe
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2023-03-25 00:43:44
Jens Axboe [off-list ref] writes:
Hi,
Hi Jens,
Thanks for the report.
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
The test case in question is pretty simple. On debian testing, do:
$ sudo apt-get install mariadb-test
$ cd /usr/share/mysql/mysql-test
$ ./mtr --mysqld=--innodb-flush-method=fsync --mysqld=--innodb-use-native-aio=1 --vardir=/dev/shm/mysql --force encryption.innodb_encryption,innodb,undo0 --repeat=200
I mostly use Fedora, the package name is the same but the mtr binary
ends up in /usr/share/mysql.
and if it fails, you'll see something like:
encryption.innodb_encryption 'innodb,undo0' [ 6 pass ] 3120
encryption.innodb_encryption 'innodb,undo0' [ 7 pass ] 3123
encryption.innodb_encryption 'innodb,undo0' [ 8 pass ] 3042
encryption.innodb_encryption 'innodb,undo0' [ 9 fail ]
Test ended at 2023-03-23 16:55:17
I haven't been able to get this to fail yet. I've done several runs with
--repeat=500 and haven't seen any errors yet.
Are there any CONFIG options I'd need to trip this?
...
Today I finally gave up and ran a basic experiment, which simply
offloads the writes to a kthread. Since powerpc has an interesting
memory coherency model, my suspicion was that the work involved with
switching MMs for the kthread could just be the main difference here.
The patch is really dumb and simple - rather than queue the write to an
IO thread, it just offloads it to a kthread that then does
kthread_use_mm(), perform write with the same write handler,
kthread_unuse_mm(). AND THIS WORKS! Usually the above mtr test would
fail in 2..20 loops, I've now done 200 and 500 loops and it's fine.
Can you share the patch that does that? It would help me track down
where exactly in the io_uring code you're talking about.
Which then leads me to the question, what about the IO thread offload
makes this fail on powerpc (and no other arch I've tested on, including
x86/x86-64/aarch64/hppa64)? The offload should be equivalent to having a
thread in userspace in the application, and having that thread just
perform the writes. Is there some magic involved with the kthread mm
use/unuse that makes this sufficiently consistent on powerpc? I've tried
any mix of isync()/mb and making the flush_dcache_page() unconditionally
done in the filemap read/write helpers, and it still falls flat on its
face with the offload to an IO thread.
My first guess would be that there's some missing barriers between the
thread that queues the IO and the IO worker thread.
I think you're using schedule_work() for that though, which should be a
full barrier. Could it be on the completion side?
I can't think of any magic in kthread_use_mm() other than extra
barriers. In particular kthread_unuse_mm() has an
smp_mb__after_spinlock() which is a full memory barrier on powerpc but
is a nop on some other architectures, x86 at least.
I must clearly be missing something here, which is why I'm emailing the
powerpc Gods for help :-)
Unfortunately the true God of powerpc memory ordering has left us and
ascended into the Metaverse ;)
cheers
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
The test case in question is pretty simple. On debian testing, do:
$ sudo apt-get install mariadb-test
$ cd /usr/share/mysql/mysql-test
$ ./mtr --mysqld=--innodb-flush-method=fsync --mysqld=--innodb-use-native-aio=1 --vardir=/dev/shm/mysql --force encryption.innodb_encryption,innodb,undo0 --repeat=200
I mostly use Fedora, the package name is the same but the mtr binary
ends up in /usr/share/mysql.
quoted
and if it fails, you'll see something like:
encryption.innodb_encryption 'innodb,undo0' [ 6 pass ] 3120
encryption.innodb_encryption 'innodb,undo0' [ 7 pass ] 3123
encryption.innodb_encryption 'innodb,undo0' [ 8 pass ] 3042
encryption.innodb_encryption 'innodb,undo0' [ 9 fail ]
Test ended at 2023-03-23 16:55:17
I haven't been able to get this to fail yet. I've done several runs with
--repeat=500 and haven't seen any errors yet.
Are there any CONFIG options I'd need to trip this?
I don't think you need any special CONFIG options. I'll attach my config
here, and I know the default distro one hits it too. But perhaps the
mariadb version is not new enough? I think you need 10.6 or above, as
will use io_uring by default. What version are you running?
...
quoted
Today I finally gave up and ran a basic experiment, which simply
offloads the writes to a kthread. Since powerpc has an interesting
memory coherency model, my suspicion was that the work involved with
switching MMs for the kthread could just be the main difference here.
The patch is really dumb and simple - rather than queue the write to an
IO thread, it just offloads it to a kthread that then does
kthread_use_mm(), perform write with the same write handler,
kthread_unuse_mm(). AND THIS WORKS! Usually the above mtr test would
fail in 2..20 loops, I've now done 200 and 500 loops and it's fine.
Can you share the patch that does that? It would help me track down
where exactly in the io_uring code you're talking about.
Shoot yes, I actually meant to attach it but then forgot. Below!
quoted
Which then leads me to the question, what about the IO thread offload
makes this fail on powerpc (and no other arch I've tested on, including
x86/x86-64/aarch64/hppa64)? The offload should be equivalent to having a
thread in userspace in the application, and having that thread just
perform the writes. Is there some magic involved with the kthread mm
use/unuse that makes this sufficiently consistent on powerpc? I've tried
any mix of isync()/mb and making the flush_dcache_page() unconditionally
done in the filemap read/write helpers, and it still falls flat on its
face with the offload to an IO thread.
My first guess would be that there's some missing barriers between the
thread that queues the IO and the IO worker thread.
That was my guess too, and I consulted Paul McKenney as well on that.
And he had some ideas of course, in terms of ordering of the CQ ring.
But tried it all out, and it still failed in the same way...
I think you're using schedule_work() for that though, which should be a
full barrier. Could it be on the completion side?
queue_work() for the patch, before that it's io-wq which is an internal
IO thread worker pool. The latter just needs a spin_lock() around
queueing the work, and then a wake of the task. Typing this out, maybe
this is where a barrier is now missing? If the IO thread is already
running rather than sleeping?
I can't think of any magic in kthread_use_mm() other than extra
barriers. In particular kthread_unuse_mm() has an
smp_mb__after_spinlock() which is a full memory barrier on powerpc but
is a nop on some other architectures, x86 at least.
Yeah, I did poke at kthread_use_mm() and the related powerpc bits, but
didn't immediately find anything that seemed promising in this regard.
quoted
I must clearly be missing something here, which is why I'm emailing the
powerpc Gods for help :-)
Unfortunately the true God of powerpc memory ordering has left us and
ascended into the Metaverse ;)
Are there any CONFIG options I'd need to trip this?
I don't think you need any special CONFIG options. I'll attach my config
here, and I know the default distro one hits it too. But perhaps the
mariadb version is not new enough? I think you need 10.6 or above, as
will use io_uring by default. What version are you running?
And here's the .config and the patch for using queue_work().
On Sat Mar 25, 2023 at 11:20 AM AEST, Jens Axboe wrote:
On 3/24/23 7:15?PM, Jens Axboe wrote:
quoted
quoted
Are there any CONFIG options I'd need to trip this?
I don't think you need any special CONFIG options. I'll attach my config
here, and I know the default distro one hits it too. But perhaps the
mariadb version is not new enough? I think you need 10.6 or above, as
will use io_uring by default. What version are you running?
And here's the .config and the patch for using queue_work().
So if you *don't* apply this patch, the work gets queued up with an IO
thread? In io-wq.c? Does that worker end up just doing an io_write()
same as this one?
Can the queueing cause the creation of an IO thread (if one does not
exist, or all blocked?)
I'm wondering what the practical differences are between this patch and
upstream.
kthread_use_mm() should be basically the same as context switching to an
IO thread. There is maybe a difference in that kthread_switch_mm() has
a 'sync' instruction *after* the MMU is switched to the new thread from
the membarrier code, but a regular context switch might not. The MMU
switch does have an isync() after it though, so loads *should* be
prohibited from moving ahead of that.
Something like this adds a sync roughly where kthread_use_mm() has one.
It's a pretty unlikely shot in the dark though. I'm more inclined to
think the work submission to the IO thread might have a problem.
Thanks,
Nick
On Sat Mar 25, 2023 at 11:20 AM AEST, Jens Axboe wrote:
quoted
On 3/24/23 7:15?PM, Jens Axboe wrote:
quoted
quoted
Are there any CONFIG options I'd need to trip this?
I don't think you need any special CONFIG options. I'll attach my config
here, and I know the default distro one hits it too. But perhaps the
mariadb version is not new enough? I think you need 10.6 or above, as
will use io_uring by default. What version are you running?
And here's the .config and the patch for using queue_work().
So if you *don't* apply this patch, the work gets queued up with an IO
thread? In io-wq.c? Does that worker end up just doing an io_write()
same as this one?
Right, without this patch, it gets added to the io-wq work pool. If a
thread is available to run it, it will. If one is not, then one is
created. Eg either event can happen.
That thread does the exact same io_write() again.
Can the queueing cause the creation of an IO thread (if one does not
exist, or all blocked?)
Yep
Since writing this email, I've gone through a lot of different tests.
Here's a rough listing of what I found:
- Like using the hack patch, if I just limit the number of IO thread
workers to 1, it seems to pass. At least longer than before, does 1000
iterations.
- If I pin each IO worker to a single CPU, it also passes.
- If I liberally sprinkle smp_mb() for the io-wq side, test still fails.
I've added one before queueing the work item, and after. One before
the io-wq worker grabs a work item and one after. Eg full hammer
approach. This still fails.
Puzzling... For the "pin each IO worker to a single CPU" I added some
basic code around trying to ensure that a work item queued on CPU X
would be processed by a worker on CPU X, and too a large degree, this
does happen. But since the work list is a normal list, it's quite
possible that some other worker finishes its work on CPU Y just in time
to grab the one from cpu X. I checked and this does happen in the test
case, yet it still passes. This may be because I got a bit lucky, but
seems suspect with thousands of passes of the test case.
Another theory there is that it's perhaps related to an io-wq worker
being rescheduled on a different CPU. Though again puzzled as to why the
smp_mb sprinkling didn't fix that then. I'm going to try and run the
test case with JUST the io-wq worker pinning and not caring about where
the work is processed to see if that does anything.
I'm wondering what the practical differences are between this patch and
upstream.
kthread_use_mm() should be basically the same as context switching to an
IO thread. There is maybe a difference in that kthread_switch_mm() has
a 'sync' instruction *after* the MMU is switched to the new thread from
the membarrier code, but a regular context switch might not. The MMU
switch does have an isync() after it though, so loads *should* be
prohibited from moving ahead of that.
Something like this adds a sync roughly where kthread_use_mm() has one.
It's a pretty unlikely shot in the dark though. I'm more inclined to
think the work submission to the IO thread might have a problem.
Didn't seem to change anything, fails pretty quickly:
[...]
encryption.innodb_encryption 'innodb,undo0' [ 38 pass ] 3083
encryption.innodb_encryption 'innodb,undo0' [ 39 pass ] 3135
encryption.innodb_encryption 'innodb,undo0' [ 40 fail ]
Test ended at 2023-03-27 12:20:46
CURRENT_TEST: encryption.innodb_encryption
mysqltest: At line 11: query 'SET @start_global_value = @@global.innodb_encryption_threads' failed: ER_UNKNOWN_SYSTEM_VARIABLE (1193): Unknown system variable 'innodb_encryption_threads'
The result from queries just before the failure was:
SET @start_global_value = @@global.innodb_encryption_threads;
- saving '/dev/shm/mysql/log/encryption.innodb_encryption-innodb,undo0/' to '/dev/shm/mysql/log/encryption.innodb_encryption-innodb,undo0/'
***Warnings generated in error logs during shutdown after running tests: encryption.innodb_encryption
2023-03-27 12:20:45 0 [Warning] Plugin 'example_key_management' is of maturity level experimental while the server is gamma
2023-03-27 12:20:45 0 [ERROR] InnoDB: Database page corruption on disk or a failed read of file './ibdata1' page [page id: space=0, page number=214]. You may have to recover from a backup.
2023-03-27 12:20:45 0 [ERROR] InnoDB: File './ibdata1' is corrupted
2023-03-27 12:20:45 0 [ERROR] InnoDB: Plugin initialization aborted with error Page read from tablespace is corrupted.
2023-03-27 12:20:45 0 [ERROR] Plugin 'InnoDB' init function returned error.
2023-03-27 12:20:45 0 [ERROR] Plugin 'InnoDB' registration as a STORAGE ENGINE failed.
--
Jens Axboe
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2023-03-27 13:54:04
Jens Axboe [off-list ref] writes:
On 3/24/23 6:42?PM, Michael Ellerman wrote:
quoted
Jens Axboe [off-list ref] writes:
quoted
I got a report sent to me from mariadb, in where 5.10.158 works fine and
5.10.162 is broken. And in fact, current 6.3-rc also fails the test
case. Beware that this email is long, as I'm trying to include
everything that may be relevant...
The test case in question is pretty simple. On debian testing, do:
$ sudo apt-get install mariadb-test
$ cd /usr/share/mysql/mysql-test
$ ./mtr --mysqld=--innodb-flush-method=fsync --mysqld=--innodb-use-native-aio=1 --vardir=/dev/shm/mysql --force encryption.innodb_encryption,innodb,undo0 --repeat=200
I mostly use Fedora, the package name is the same but the mtr binary
ends up in /usr/share/mysql.
quoted
and if it fails, you'll see something like:
encryption.innodb_encryption 'innodb,undo0' [ 6 pass ] 3120
encryption.innodb_encryption 'innodb,undo0' [ 7 pass ] 3123
encryption.innodb_encryption 'innodb,undo0' [ 8 pass ] 3042
encryption.innodb_encryption 'innodb,undo0' [ 9 fail ]
Test ended at 2023-03-23 16:55:17
I haven't been able to get this to fail yet. I've done several runs with
--repeat=500 and haven't seen any errors yet.
Are there any CONFIG options I'd need to trip this?
I don't think you need any special CONFIG options. I'll attach my config
here, and I know the default distro one hits it too. But perhaps the
mariadb version is not new enough? I think you need 10.6 or above, as
will use io_uring by default. What version are you running?
Yeah I had 10.5.
I ended up building mariadb git, and now I have it reproducing on one VM
I have.
For some reason I can't reproduce with the same setup on a bare metal
machine, which is odd.
...
quoted
My first guess would be that there's some missing barriers between the
thread that queues the IO and the IO worker thread.
That was my guess too, and I consulted Paul McKenney as well on that.
And he had some ideas of course, in terms of ordering of the CQ ring.
But tried it all out, and it still failed in the same way...
quoted
I think you're using schedule_work() for that though, which should be a
full barrier. Could it be on the completion side?
queue_work() for the patch, before that it's io-wq which is an internal
IO thread worker pool. The latter just needs a spin_lock() around
queueing the work, and then a wake of the task. Typing this out, maybe
this is where a barrier is now missing? If the IO thread is already
running rather than sleeping?
It sounded promising, but I've tried adding barriers around all the spin
locks and it hasn't made any difference.
Are there barriers in the userspace code also? If so would they be in
liburing or in the actual mariadb code?
Possibly I'm completely wrong about barriers and it's something else,
but I can't think what.
I checked that the liburing tests are passing. Any idea what mariadb is
doing different that's likely to be triggering the bug?
cheers
Can the queueing cause the creation of an IO thread (if one does not
exist, or all blocked?)
Yep
Since writing this email, I've gone through a lot of different tests.
Here's a rough listing of what I found:
- Like using the hack patch, if I just limit the number of IO thread
workers to 1, it seems to pass. At least longer than before, does 1000
iterations.
- If I pin each IO worker to a single CPU, it also passes.
- If I liberally sprinkle smp_mb() for the io-wq side, test still fails.
I've added one before queueing the work item, and after. One before
the io-wq worker grabs a work item and one after. Eg full hammer
approach. This still fails.
Puzzling... For the "pin each IO worker to a single CPU" I added some
basic code around trying to ensure that a work item queued on CPU X
would be processed by a worker on CPU X, and too a large degree, this
does happen. But since the work list is a normal list, it's quite
possible that some other worker finishes its work on CPU Y just in time
to grab the one from cpu X. I checked and this does happen in the test
case, yet it still passes. This may be because I got a bit lucky, but
seems suspect with thousands of passes of the test case.
Another theory there is that it's perhaps related to an io-wq worker
being rescheduled on a different CPU. Though again puzzled as to why the
smp_mb sprinkling didn't fix that then. I'm going to try and run the
test case with JUST the io-wq worker pinning and not caring about where
the work is processed to see if that does anything.
Just pinning each worker to whatever CPU they got created on seemingly
fixes the issue too. This does not mean that each worker will process
work on the CPU on which it was queued, just that each worker will
remain on whatever CPU it originally got created on.
Puzzling...
Note that it is indeed quite possible that this isn't a ppc issue at
all, just shows on ppc. It could be page cache related, or it could even
be a bug in mariadb itself.
--
Jens Axboe
From: Michael Ellerman <mpe@ellerman.id.au> Date: 2023-03-28 12:52:31
Jens Axboe [off-list ref] writes:
quoted
quoted
Can the queueing cause the creation of an IO thread (if one does not
exist, or all blocked?)
Yep
Since writing this email, I've gone through a lot of different tests.
Here's a rough listing of what I found:
- Like using the hack patch, if I just limit the number of IO thread
workers to 1, it seems to pass. At least longer than before, does 1000
iterations.
- If I pin each IO worker to a single CPU, it also passes.
- If I liberally sprinkle smp_mb() for the io-wq side, test still fails.
I've added one before queueing the work item, and after. One before
the io-wq worker grabs a work item and one after. Eg full hammer
approach. This still fails.
Puzzling... For the "pin each IO worker to a single CPU" I added some
basic code around trying to ensure that a work item queued on CPU X
would be processed by a worker on CPU X, and too a large degree, this
does happen. But since the work list is a normal list, it's quite
possible that some other worker finishes its work on CPU Y just in time
to grab the one from cpu X. I checked and this does happen in the test
case, yet it still passes. This may be because I got a bit lucky, but
seems suspect with thousands of passes of the test case.
Another theory there is that it's perhaps related to an io-wq worker
being rescheduled on a different CPU. Though again puzzled as to why the
smp_mb sprinkling didn't fix that then. I'm going to try and run the
test case with JUST the io-wq worker pinning and not caring about where
the work is processed to see if that does anything.
Just pinning each worker to whatever CPU they got created on seemingly
fixes the issue too. This does not mean that each worker will process
work on the CPU on which it was queued, just that each worker will
remain on whatever CPU it originally got created on.
Puzzling...
Note that it is indeed quite possible that this isn't a ppc issue at
all, just shows on ppc. It could be page cache related, or it could even
be a bug in mariadb itself.
I tried binary patching every lwsync to hwsync (read/write to full
barrier) in mariadbd and all the libaries it links. It didn't fix the
problem.
I also tried switching all the kernel barriers/spin locks to using a
hwsync, but that also didn't fix it.
It's still possible there's somewhere that currently has no barrier at
all that needs one, the above would only fix the problem if we have a
read/write barrier that actually needs to be a full barrier.
I also looked at making all TLB invalidates broadcast, regardless of
whether we think the thread has only been on a single CPU. That didn't
help, but I'm not sure I got all places where we do TLB invalidates, so
I'll look at that some more tomorrow.
cheers
Can the queueing cause the creation of an IO thread (if one does not
exist, or all blocked?)
Yep
Since writing this email, I've gone through a lot of different tests.
Here's a rough listing of what I found:
- Like using the hack patch, if I just limit the number of IO thread
workers to 1, it seems to pass. At least longer than before, does 1000
iterations.
- If I pin each IO worker to a single CPU, it also passes.
- If I liberally sprinkle smp_mb() for the io-wq side, test still fails.
I've added one before queueing the work item, and after. One before
the io-wq worker grabs a work item and one after. Eg full hammer
approach. This still fails.
Puzzling... For the "pin each IO worker to a single CPU" I added some
basic code around trying to ensure that a work item queued on CPU X
would be processed by a worker on CPU X, and too a large degree, this
does happen. But since the work list is a normal list, it's quite
possible that some other worker finishes its work on CPU Y just in time
to grab the one from cpu X. I checked and this does happen in the test
case, yet it still passes. This may be because I got a bit lucky, but
seems suspect with thousands of passes of the test case.
Another theory there is that it's perhaps related to an io-wq worker
being rescheduled on a different CPU. Though again puzzled as to why the
smp_mb sprinkling didn't fix that then. I'm going to try and run the
test case with JUST the io-wq worker pinning and not caring about where
the work is processed to see if that does anything.
Just pinning each worker to whatever CPU they got created on seemingly
fixes the issue too. This does not mean that each worker will process
work on the CPU on which it was queued, just that each worker will
remain on whatever CPU it originally got created on.
Puzzling...
Note that it is indeed quite possible that this isn't a ppc issue at
all, just shows on ppc. It could be page cache related, or it could even
be a bug in mariadb itself.
I tried binary patching every lwsync to hwsync (read/write to full
barrier) in mariadbd and all the libaries it links. It didn't fix the
problem.
I also tried switching all the kernel barriers/spin locks to using a
hwsync, but that also didn't fix it.
It's still possible there's somewhere that currently has no barrier at
all that needs one, the above would only fix the problem if we have a
read/write barrier that actually needs to be a full barrier.
I also looked at making all TLB invalidates broadcast, regardless of
whether we think the thread has only been on a single CPU. That didn't
help, but I'm not sure I got all places where we do TLB invalidates, so
I'll look at that some more tomorrow.
Thanks, appreciate your testing! I have no new data points since
yesterday, but the key point from then still seems to be that if an io
worker never reschedules onto a different CPU, then the problem doesn't
occur. This could very well be a page cache issue, if it isn't an issue
on the powerpc side...
--
Jens Axboe