Re: [syzbot] [ext4?] BUG: sleeping function called from invalid context in ext4_update_super

["Fabio M. De Francesco" <fmdefrancesco@gmail.com>]

On lunedì 12 giugno 2023 02:19:21 CEST Theodore Ts'o wrote:
> On Sun, Jun 11, 2023 at 09:15:56PM +0200, Fabio M. De Francesco wrote:
> > Thanks!
> > 
> > Let me summarize, just to be sure we don't misunderstand each other...
> > 
> > To start off, I'll send out _only_ the patch for the bug reported by 
Syzbot,
> > the one about dropping the call to ext_error() in ext4_get_group_info().
> > 
> > I'll do this by Tuesday. (Sorry, I cannot do it by Monday because I must
> > pass
> > an exam and an interview for a job).

Ted,

Sorry, I sent the patch this morning (local time), that is one day later :-(

It's at https://lore.kernel.org/lkml/20230614100446.14337-1-fmdefrancesco@gmail.com/

> Sure, that'll be fine.
> 
> > However, on the other problems with ext4_grp_locked_error() that you 
noticed
> > in the final part of your first message in this thread I'll need some days
> > more to better understand the context I'm working in.
> 
> Um, I'm not sure what problems you're referring to.  What I said is
> that it works, but you just have to be careful in how you use it (and
> the current callers in mballoc.c are careful).

My poor English made me misunderstanding what you wrote in the final part of 
you first email. My fault, again sorry.

> And similarly, I don't think it's a problem that you need to be
> careful not to call ext4_error() from an atomic context.  You need to
> be careful, and sometimes we screw up.  But in this particular case,
> it's pretty obvious how to fix it, and we don't even need a syzkaller
> reproducer.  :-)

Sure.

> > > I would strongly recommend that you use gce-xfstests or kvm-xfstests
> > > before submitting ext4 patches.

I'll surely do next time, but this was too trivial to necessitate any test. Do 
you agree with me?

> > > In this particular case, it's a
> > > relatively simple patch, but it's a good habit to get into.  See [1]
> > > for more details.
> > > 
> > > [1] https://thunk.org/gce-xfstests
> > 
> > Thanks also for these information.
> > 

Well, I think that this means that you indeed agree for this particular case 
:-)

> > I'm still in search of a reliable way to let atomic context
> > run idle waiting for a status change.

[...]

> So the question is not how to find a "reliable way to let atomic
> context run > idle waiting for a status change".  That's the wrong
> question.  The better question is: "how do you restructure code
> running in an atomic context so it doesn't need to wait for a status
> change"?
> 
> Cheers,
> 
> 					- Ted

Very interesting discussion. 
I skipped the details only for shortening this email.

Again thanks for your precious help,

Fabio