On Wed, Sep 3, 2025 at 11:07 PM Christoph Hellwig <hch@xxxxxxxxxxxxx> wrote: > > On Wed, Sep 03, 2025 at 01:30:31PM -0700, Darrick J. Wong wrote: > > On Fri, Aug 29, 2025 at 04:56:15PM -0700, Joanne Koong wrote: > > > Use the iomap_iter->private field for stashing any read/readahead bios > > > instead of defining the bio as part of the iomap_readpage_ctx struct. > > > This makes the read/readahead interface more generic. Some filesystems > > > that will be using iomap for read/readahead may not have CONFIG_BLOCK > > > set. > > > > Sorry, but I don't like abusing iomap_iter::private because (a) it's a > > void pointer which means shenanigans; and (b) private exists to store > > some private data for an iomap caller, not iomap itself. Fair enough. For callers that provide custom read handling, their equivalent of "bios" is stored in iter->private, so i was thinking it would be nice symmetry to have the two match, but I get your point. > > I don't think we can do without the void pointer for a generic > lower library, but I fully agree on not using iomap_iter::private. > We'll need that for something caller provided sooner or later. > > The right way ahead is to have a void pointer for the I/O-type specific > context in iomap_readpage_ctx, and then hopefully reasonable type safe > wrappers around it. > Do we need a void pointer for this in iomap_readpage_ctx if the only user of it is bios? For callers who do custom read handling, their pointer is stashed in iter->private where the whole iter then gets passed to ->read_folio_range(iter, ...). It seems like maybe we should just do #ifdef CONFIG_BLOCK struct bio *bio; #endif in that case.
