Re: [PATCH 02/19] buffer: allow a buffer_head to point at memory outside the page cache

On Mon, Aug 03, 2026 at 06:13:19PM +0200, Jan Kara wrote:
> On Sat 01-08-26 18:00:46, Chao Shi wrote:
> > jbd2 builds a temporary buffer_head to write out the frozen copy of a
> > metadata block, and that copy lives in slab memory.  Today jbd2 points the
> > temporary buffer at the slab folio backing it.  A slab folio's ->mapping is
> > not an address_space, so anything that follows bh->b_folio->mapping there
> > gets garbage rather than NULL; mark_buffer_write_io_error() does exactly
> > that, and we are about to start calling it on this buffer.
> > 
> > Rather than teach every such helper about slab folios, allow bh->b_folio to
> > be NULL and let b_data point straight at the memory.  Code that needs the
> > folio has to check.  There are two places in this file:
> > 
> >  - __bh_submit() adds the data by virtual address using
> >    bio_add_virt_nofail(), and skips the cgroup accounting: a buffer that is
> >    not in the page cache has no owning folio to attribute writeback to.
> > 
> >  - buffer_set_crypto_ctx() returns early.  fscrypt has no interest in a
> >    buffer that is not part of a file mapping, which is why it already
> >    returns when folio_mapping() comes back NULL.
> > 
> > Nothing sets b_folio to NULL yet, so this patch is a no-op on its own.
> > 
> > This is deliberately not a general capability.  Buffers over highmem have
> > no permanent kernel virtual address, which is why folio_set_bh() records a
> > folio and an offset instead of an address.  A folio-less buffer_head is
> > only valid over memory that is always mapped, and must not be passed to
> > bh_offset().
> 
> There are much more things you cannot do with a bh that doesn't have valid
> b_folio - touch_buffer(), end_buffer_async_read(), ... and many more. But
> that's a bussiness of the code that sets up such temporary bhs. I agree
> that setting b_folio to NULL will if nothing else lead to much more obvious
> failures than when we accidentally get slab folio. So I'd prefer we update
> the description a bit in this direction but otherwise feel free to add:

Yes, agreed.  I think it's fine to add in some 'if (!bh->b_folio)'
tests, but only where they're needed for the occasional consumer.  This
really is a rare (but legitimate) case.

> >  static void buffer_set_crypto_ctx(struct bio *bio, const struct buffer_head *bh,
> >  				  gfp_t gfp_mask)
> >  {
> > -	const struct address_space *mapping = folio_mapping(bh->b_folio);
> > +	const struct address_space *mapping;
> >  
> >  	/*
> >  	 * The ext4 journal (jbd2) can submit a buffer_head it directly created
> > -	 * for a non-pagecache page.  fscrypt doesn't care about these.
> > +	 * for memory that is not in the page cache at all.  fscrypt doesn't
> > +	 * care about these.
> >  	 */
> > +	if (!bh->b_folio)
> > +		return;
> > +	mapping = folio_mapping(bh->b_folio);
> >  	if (!mapping)
> >  		return;

Do we want to call folio_mapping() here?  The only case where I can see
this being useful is if we attach a buffer_head to an anonymous folio,
and I don't see a good reason to do that.  I think this can just be

	mapping = bh->b_folio->mapping;