linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Gao Xiang <xiang@kernel.org>
To: Joanne Koong <joannelkoong@gmail.com>,
	djwong@kernel.org, hch@infradead.org
Cc: brauner@kernel.org, miklos@szeredi.hu,
	hsiangkao@linux.alibaba.com, linux-block@vger.kernel.org,
	gfs2@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	kernel-team@meta.com, linux-xfs@vger.kernel.org,
	linux-doc@vger.kernel.org
Subject: Re: [PATCH v2 11/16] iomap: add caller-provided callbacks for read and readahead
Date: Tue, 9 Sep 2025 08:14:39 +0800	[thread overview]
Message-ID: <aL9xb5Jw8tvIRMcQ@debian> (raw)
In-Reply-To: <20250908185122.3199171-12-joannelkoong@gmail.com>

Hi Joanne,

On Mon, Sep 08, 2025 at 11:51:17AM -0700, Joanne Koong wrote:
> Add caller-provided callbacks for read and readahead so that it can be
> used generically, especially by filesystems that are not block-based.
> 
> In particular, this:
> * Modifies the read and readahead interface to take in a
>   struct iomap_read_folio_ctx that is publicly defined as:
> 
>   struct iomap_read_folio_ctx {
> 	const struct iomap_read_ops *ops;
> 	struct folio *cur_folio;
> 	struct readahead_control *rac;
> 	void *private;
>   };
> 
>   where struct iomap_read_ops is defined as:
> 
>   struct iomap_read_ops {
>       int (*read_folio_range)(const struct iomap_iter *iter,
>                              struct iomap_read_folio_ctx *ctx,
>                              loff_t pos, size_t len);
>       int (*read_submit)(struct iomap_read_folio_ctx *ctx);
>   };
> 

No, I don't think `struct iomap_read_folio_ctx` has another
`.private` makes any sense, because:

 - `struct iomap_iter *iter` already has `.private` and I think
   it's mainly used for per-request usage; and your new
   `.read_folio_range` already passes
    `const struct iomap_iter *iter` which has `.private`
   I don't think some read-specific `.private` is useful in any
   case, also below.

 - `struct iomap_read_folio_ctx` cannot be accessed by previous
   .iomap_{begin,end} helpers, which means `struct iomap_read_ops`
   is only useful for FUSE read iter/submit logic.

Also after my change, the prototype will be:

int iomap_read_folio(const struct iomap_ops *ops,
		     struct iomap_read_folio_ctx *ctx, void *private2);
void iomap_readahead(const struct iomap_ops *ops,
		     struct iomap_read_folio_ctx *ctx, void *private2);

Is it pretty weird due to `.iomap_{begin,end}` in principle can
only use `struct iomap_iter *` but have no way to access
` struct iomap_read_folio_ctx` to get more enough content for
read requests.

Thanks,
Gao Xiang

  reply	other threads:[~2025-09-09  0:14 UTC|newest]

Thread overview: 60+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-08 18:51 [PATCH v2 00/16] fuse: use iomap for buffered reads + readahead Joanne Koong
2025-09-08 18:51 ` [PATCH v2 01/16] iomap: move async bio read logic into helper function Joanne Koong
2025-09-11 11:09   ` Christoph Hellwig
2025-09-12 16:01     ` Joanne Koong
2025-09-08 18:51 ` [PATCH v2 02/16] iomap: move read/readahead bio submission " Joanne Koong
2025-09-11 11:09   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 03/16] iomap: rename cur_folio_in_bio to folio_owned Joanne Koong
2025-09-11 11:10   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 04/16] iomap: store read/readahead bio generically Joanne Koong
2025-09-11 11:11   ` Christoph Hellwig
2025-09-12 16:10     ` Joanne Koong
2025-09-08 18:51 ` [PATCH v2 05/16] iomap: propagate iomap_read_folio() error to caller Joanne Koong
2025-09-11 11:13   ` Christoph Hellwig
2025-09-12 16:28     ` Joanne Koong
2025-09-15 16:05       ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 06/16] iomap: iterate over entire folio in iomap_readpage_iter() Joanne Koong
2025-09-11 11:15   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 07/16] iomap: rename iomap_readpage_iter() to iomap_read_folio_iter() Joanne Koong
2025-09-11 11:16   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 08/16] iomap: rename iomap_readpage_ctx struct to iomap_read_folio_ctx Joanne Koong
2025-09-11 11:16   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 09/16] iomap: add public start/finish folio read helpers Joanne Koong
2025-09-11 11:16   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 10/16] iomap: make iomap_read_folio_ctx->folio_owned internal Joanne Koong
2025-09-11 11:17   ` Christoph Hellwig
2025-09-08 18:51 ` [PATCH v2 11/16] iomap: add caller-provided callbacks for read and readahead Joanne Koong
2025-09-09  0:14   ` Gao Xiang [this message]
2025-09-09  0:40     ` Gao Xiang
2025-09-09 15:24     ` Joanne Koong
2025-09-09 23:21       ` Gao Xiang
2025-09-10 17:41         ` Joanne Koong
2025-09-11 11:19           ` Christoph Hellwig
2025-09-11 11:26   ` Christoph Hellwig
2025-09-12 17:36     ` Joanne Koong
2025-09-08 18:51 ` [PATCH v2 12/16] iomap: add bias for async read requests Joanne Koong
2025-09-11 11:31   ` Christoph Hellwig
2025-09-12 17:30     ` Joanne Koong
2025-09-15 16:05       ` Christoph Hellwig
2025-09-16 19:14       ` Joanne Koong
2025-09-19 15:04         ` Christoph Hellwig
2025-09-19 17:58           ` Joanne Koong
2025-09-08 18:51 ` [PATCH v2 13/16] iomap: move read/readahead logic out of CONFIG_BLOCK guard Joanne Koong
2025-09-09  2:14   ` Gao Xiang
2025-09-09 15:33     ` Joanne Koong
2025-09-10  4:59       ` Gao Xiang
2025-09-11 11:37         ` Christoph Hellwig
2025-09-11 12:29           ` Gao Xiang
2025-09-11 19:45             ` Joanne Koong
2025-09-12  0:06               ` Gao Xiang
2025-09-12  1:09                 ` Gao Xiang
2025-09-12  1:10                   ` Gao Xiang
2025-09-12 19:56                     ` Joanne Koong
2025-09-12 20:09                       ` Joanne Koong
2025-09-12 23:35                         ` Gao Xiang
2025-09-12 23:20                       ` Gao Xiang
2025-09-11 11:44   ` Christoph Hellwig
2025-09-16 23:23     ` Joanne Koong
2025-09-08 18:51 ` [PATCH v2 14/16] fuse: use iomap for read_folio Joanne Koong
2025-09-08 18:51 ` [PATCH v2 15/16] fuse: use iomap for readahead Joanne Koong
2025-09-08 18:51 ` [PATCH v2 16/16] fuse: remove fc->blkbits workaround for partial writes Joanne Koong

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aL9xb5Jw8tvIRMcQ@debian \
    --to=xiang@kernel.org \
    --cc=brauner@kernel.org \
    --cc=djwong@kernel.org \
    --cc=gfs2@lists.linux.dev \
    --cc=hch@infradead.org \
    --cc=hsiangkao@linux.alibaba.com \
    --cc=joannelkoong@gmail.com \
    --cc=kernel-team@meta.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=miklos@szeredi.hu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).