public inbox for linux-xfs@vger.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@kernel.dk>
To: Linus Torvalds <torvalds@linux-foundation.org>
Cc: "Darrick J. Wong" <djwong@kernel.org>,
	Dave Chinner <david@fromorbit.com>, Zorro Lang <zlang@redhat.com>,
	linux-xfs@vger.kernel.org,
	"Eric W. Biederman" <ebiederm@xmission.com>,
	Mike Christie <michael.christie@oracle.com>,
	"Michael S. Tsirkin" <mst@redhat.com>,
	linux-kernel@vger.kernel.org
Subject: Re: [6.5-rc5 regression] core dump hangs (was Re: [Bug report] fstests generic/051 (on xfs) hang on latest linux v6.5-rc5+)
Date: Mon, 12 Jun 2023 11:53:59 -0600	[thread overview]
Message-ID: <8a97ca5d-69ef-d716-9f61-2b9b2a26dd14@kernel.dk> (raw)
In-Reply-To: <CAHk-=whXt9+-YfhgjBYxT9_ATjHbMDZ0yJdK7umrJGU8zBVZ9w@mail.gmail.com>

On 6/12/23 11:51?AM, Linus Torvalds wrote:
> On Mon, Jun 12, 2023 at 10:29?AM Jens Axboe <axboe@kernel.dk> wrote:
>>
>> Looks fine to me to just kill it indeed, whatever we did need this
>> for is definitely no longer the case. I _think_ we used to have
>> something in the worker exit that would potentially sleep which
>> is why we killed it before doing that, now it just looks like dead
>> code.
> 
> Ok, can you (and the fsstress people) confirm that this
> whitespace-damaged patch fixes the coredump issue:
> 
> 
>   --- a/io_uring/io-wq.c
>   +++ b/io_uring/io-wq.c
>   @@ -221,9 +221,6 @@ static void io_worker_exit(..
>         raw_spin_unlock(&wq->lock);
>         io_wq_dec_running(worker);
>         worker->flags = 0;
>   -     preempt_disable();
>   -     current->flags &= ~PF_IO_WORKER;
>   -     preempt_enable();
> 
>         kfree_rcu(worker, rcu);
>         io_worker_ref_put(wq);

Yep, it fixes it on my end and it passes my basic tests as well.

> Jens, I think that the two lines above there, ie the whole
> 
>         io_wq_dec_running(worker);
>         worker->flags = 0;
> 
> thing may actually be the (partial?) reason for those PF_IO_WORKER
> games. It's basically doing "now I'm doing stats by hand", and I
> wonder if now it decrements the running worker one time too much?
> 
> Ie when the finally *dead* worker schedules away, never to return,
> that's when that io_wq_worker_sleeping() case triggers and decrements
> things one more time.
> 
> So there might be some bookkeeping reason for those games, but it
> looks like if that's the case, then that
> 
>         worker->flags = 0;
> 
> will have already taken care of it.
> 
> I wonder if those two lines could just be removed too. But I think
> that's separate from the "let's fix the core dumping" issue.

Something like that was/is my worry. Let me add some tracing to confirm
it's fine, don't fully trust just my inspection of it. I'll send out the
patch separately once done, and then would be great if you can just pick
it up so it won't have to wait until I need to send a pull later in the
week. Particularly as I have nothing planned for 6.4 unless something
else comes up of course.

-- 
Jens Axboe


  reply	other threads:[~2023-06-12 17:54 UTC|newest]

Thread overview: 31+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-11 12:48 [Bug report] fstests generic/051 (on xfs) hang on latest linux v6.5-rc5+ Zorro Lang
2023-06-11 13:40 ` Zorro Lang
2023-06-11 23:01 ` Dave Chinner
2023-06-12  1:51   ` Darrick J. Wong
2023-06-12  2:21     ` [6.5-rc5 regression] core dump hangs (was Re: [Bug report] fstests generic/051 (on xfs) hang on latest linux v6.5-rc5+) Dave Chinner
2023-06-12  3:14       ` Linus Torvalds
2023-06-12  5:16         ` Dave Chinner
2023-06-12  5:34           ` Linus Torvalds
2023-06-12  5:49             ` Dave Chinner
2023-06-12  6:11               ` Linus Torvalds
2023-06-12  8:45                 ` Eric W. Biederman
2023-06-12  9:30                   ` Zorro Lang
2023-06-12 11:27                   ` Dave Chinner
2023-06-12  6:36         ` Zorro Lang
2023-06-12 15:36         ` Darrick J. Wong
2023-06-12 15:52           ` Eric W. Biederman
2023-06-12 15:56           ` Linus Torvalds
2023-06-12 16:27             ` Jens Axboe
2023-06-12 16:38               ` Jens Axboe
2023-06-12 16:42                 ` Linus Torvalds
2023-06-12 16:45                   ` Jens Axboe
2023-06-12 16:57                     ` Linus Torvalds
2023-06-12 17:11                       ` Eric W. Biederman
2023-06-12 17:30                         ` Jens Axboe
2023-06-12 17:29                       ` Jens Axboe
2023-06-12 17:51                         ` Linus Torvalds
2023-06-12 17:53                           ` Jens Axboe [this message]
2023-06-12 17:56                             ` Linus Torvalds
2023-06-12 18:34                           ` Linus Torvalds
2023-06-12 23:33                             ` Dave Chinner
2023-06-12 16:45                   ` Linus Torvalds

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=8a97ca5d-69ef-d716-9f61-2b9b2a26dd14@kernel.dk \
    --to=axboe@kernel.dk \
    --cc=david@fromorbit.com \
    --cc=djwong@kernel.org \
    --cc=ebiederm@xmission.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=michael.christie@oracle.com \
    --cc=mst@redhat.com \
    --cc=torvalds@linux-foundation.org \
    --cc=zlang@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox