linux-kernel - Re: Bug#1093243: Upgrade to 6.1.123 kernel causes mariadb hangs

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: <a2f5ea66-7506-4256-b69c-a2d6c2f72eb4@charbonnet.com>
Date: Sun, 26 Jan 2025 16:48:57 -0600
From: Xan Charbonnet <xan@...rbonnet.com>
To: Jens Axboe <axboe@...nel.dk>, Salvatore Bonaccorso <carnil@...ian.org>,
 Pavel Begunkov <asml.silence@...il.com>
Cc: 1093243@...s.debian.org, Bernhard Schmidt <berni@...ian.org>,
 io-uring@...r.kernel.org, linux-kernel@...r.kernel.org,
 regressions@...ts.linux.dev
Subject: Re: Bug#1093243: Upgrade to 6.1.123 kernel causes mariadb hangs

Since applying the final patch on Friday, I have seen no problems with 
either the backup snapshot or catching up with replication.  It sure 
seems like things are all fixed.  I haven't yet tried it on our 
production Galera cluster, but I expect to on Monday.

Here are Debian packages containing the modified kernel.  Use at your 
own risk of course.  Any feedback about how this works or doesn't work 
would be very helpful.

https://charbonnet.com/linux-image-6.1.0-29-with-proposed-1093243-fix_amd64.deb
https://charbonnet.com/linux-image-6.1.0-30-with-proposed-1093243-fix_amd64.deb




On 1/24/25 14:51, Jens Axboe wrote:
> On 1/24/25 1:33 PM, Salvatore Bonaccorso wrote:
>> Hi Pavel,
>>
>> On Fri, Jan 24, 2025 at 06:40:51PM +0000, Pavel Begunkov wrote:
>>> On 1/24/25 16:30, Xan Charbonnet wrote:
>>>> On 1/24/25 04:33, Pavel Begunkov wrote:
>>>>> Thanks for narrowing it down. Xan, can you try this change please?
>>>>> Waiters can miss wake ups without it, seems to match the description.
>>>>>
>>>>> diff --git a/io_uring/io_uring.c b/io_uring/io_uring.c
>>>>> index 9b58ba4616d40..e5a8ee944ef59 100644
>>>>> --- a/io_uring/io_uring.c
>>>>> +++ b/io_uring/io_uring.c
>>>>> @@ -592,8 +592,10 @@ static inline void __io_cq_unlock_post_flush(struct io_ring_ctx *ctx)
>>>>>         io_commit_cqring(ctx);
>>>>>         spin_unlock(&ctx->completion_lock);
>>>>>         io_commit_cqring_flush(ctx);
>>>>> -    if (!(ctx->flags & IORING_SETUP_DEFER_TASKRUN))
>>>>> +    if (!(ctx->flags & IORING_SETUP_DEFER_TASKRUN)) {
>>>>> +        smp_mb();
>>>>>             __io_cqring_wake(ctx);
>>>>> +    }
>>>>>     }
>>>>>     void io_cq_unlock_post(struct io_ring_ctx *ctx)
>>>>>
>>>>
>>>>
>>>> Thanks Pavel!  Early results look very good for this change.  I'm now running 6.1.120 with your added smp_mb() call.  The backup process which had been quickly triggering the issue has been running longer than it ever did when it would ultimately fail.  So that's great!
>>>>
>>>> One sour note: overnight, replication hung on this machine, which is another failure that started happening with the jump from 6.1.119 to 6.1.123.  The machine was running 6.1.124 with the __io_cq_unlock_post_flush function removed completely.  That's the kernel we had celebrated yesterday for running the backup process successfully.
>>>>
>>>> So, we might have two separate issues to deal with, unfortunately.
>>>
>>> Possible, but it could also be a side effect of reverting the patch.
>>> As usual, in most cases patches are ported either because they're
>>> fixing sth or other fixes depend on it, and it's not yet apparent
>>> to me what happened with this one.
>>
>> I researched bit the lists, and there was the inclusion request on the
>> stable list itself. Looking into the io-uring list I found
>> https://lore.kernel.org/io-uring/CADZouDRFJ9jtXHqkX-PTKeT=GxSwdMC42zEsAKR34psuG9tUMQ@mail.gmail.com/
>> which I think was the trigger to later on include in fact the commit
>> in 6.1.120.
> 
> Yep indeed, was just looking for the backstory and that is why it got
> backported. Just missed the fact that it should've been an
> io_cqring_wake() rather than __io_cqring_wake()...
>