Re: [PATCH v7 2/5] block: add task-context bio completion infrastructure

[Barry Song]

On Sat, Aug 29, 2026 at 11:26 PM Tal Zussman <tz2294@xxxxxxxxxxxx> wrote:
>
> On 8/16/26 7:11 AM, Barry Song wrote:
> > Hi Tal,
> >
> > Thanks very much for the patch. I’m just a bit confused that
> > folio_end_dropbehind() only requires in_task(), and nothing more.
> >
> > void folio_end_dropbehind(struct folio *folio)
> > {
> >          if (!folio_test_dropbehind(folio))
> >                  return;
> >
> >          /*
> >           * Hitting !in_task() should not happen off RWF_DONTCACHE writeback,
> >           * but can happen if normal writeback just happens to find dirty folios
> >           * that were created as part of uncached writeback, and that writeback
> >           * would otherwise not need non-IRQ handling. Just skip the
> >           * invalidation in that case.
> >           */
> >          if (in_task() && folio_trylock(folio)) {
> >                  filemap_end_dropbehind(folio);
> >                  folio_unlock(folio);
> >          }
> > }
> >
> > But we’re applying a much stricter check with bio_in_atomic():
> >
> > +static inline bool bio_in_atomic(void)
> > +{
> > +     if (IS_ENABLED(CONFIG_PREEMPTION) && rcu_preempt_depth())
> > +           return true;
> > +     if (!IS_ENABLED(CONFIG_PREEMPT_COUNT))
> > +           return true;
> > +      return !preemptible();
> > +}
> >
> > I feel like something may be missing either in
> > folio_end_dropbehind() itself or somewhere in the
> > filesystem/block-device path.
> >
> > Do you know why they don’t match each other?
> >
> > Best Regards
> > Barry
> >
>
> Hi Barry,
>
> Apologies for the delay.
>
> As far as I can tell, the in_task() in folio_end_dropbehind() wasn't meant
> to be a generic test for if it's safe to sleep, but rather filtered out the
> known unsafe case of buffer_head writeback completing from IRQ context.
> In contrast, the bio_in_atomic() check needs to be stricter since the bi_end_io()
> callbacks could sleep or the callers could be holding locks, RCU, etc.
>
> I see there's some more discussion about this at [1], but I suspect there are
> other instances that are (incorrectly) using in_task() as a proxy for being able
> to sleep (like f2fs_read_end_io(), but I haven't looked too closely at it).
>
> Additionally, it probably only makes sense to use bio_in_atomic() when there's
> some deferral path, since with CONFIG_PREEMPT_COUNT=n, it'll always return true.

Tal, thanks very much for your reply and the kind clarification. It
seems you're right, and I think we should also use `bio_in_atomic()` in
`folio_end_dropbehind()`.

On the other hand, as I mentioned in another thread[1], I really feel that
`bio_in_atomic()` has nothing to do with `bio` at all; it's really a
scheduler thing. We may want to move it to the scheduler layer and
discuss it there.

[1] https://lore.kernel.org/linux-mm/CAGsJ_4xxDpeaPv9Uh_1qsuXsMLkx+J0X=2V6342idLFO5jkTBw@xxxxxxxxxxxxxx/

Best Regards
Barry