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

On Tue, Aug 04, 2026 at 10:07:39AM +0200, Jan Kara wrote:
> > 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.
> 
> Yes. If we were to write this code from scratch, I'd rather push for
> forming and submitting the bio directly instead of trying to bend the bh
> layer for these temporary bhs. Eventually we might want to do this cleanup
> anyway (since bhs don't really buy us much in this path) but not in this
> patch set.

I think that would be better, but it is out of scope.

> > > >  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;
> 
> For all I know yes, bh->b_folio->mapping should be enough here. But I
> thought folio_mapping() is kind of preferred to open-coding?

It rather depends on the path we're talking about.  Looking at the
implementation of folio_mapping(), it's actually mildly dangerous
to call folio_mapping() in this path.  If we were ever to point a
buffer_head at some memory on a page in swap cache, we'd get a pointer
to a swap_address_space.  I think we'd safely crash in that case because
mapping->host would be NULL, but in other contexts doing things to a
swap mapping would be quite bad.

So I think this should just be folio->mapping; don't risk getting a
pointer to a swap mapping.