Re: [RFC PATCH v4 05/12] iomap: Add initial support for buffered RWF_WRITETHROUGH

[Pankaj Raghav (Samsung)]

One general comment, all your commit headers are starting with
uppercase.

iomap: Add initial support for buffered RWF_WRITETHROUGH

s/Add/add

Overall this looks good to me. I will run some tests via fio. Do you
have a branch with WRITETHROUGH by any chance?

On Mon, Sep 28, 2026 at 05:33:06PM +0530, Ojaswin Mujoo wrote:
> +static int iomap_writethrough_iter(struct iomap_writethrough_ctx *wt_ctx,
> +				   struct iomap_iter *iter, struct iov_iter *i,
> +				   const struct iomap_writethrough_ops *wt_ops)
> +
> +{
> +	ssize_t total_written = 0, pending = 0;
> +	loff_t submit_start_pos;
> +	int status = 0;
> +	struct address_space *mapping = iter->inode->i_mapping;
> +	size_t chunk = mapping_max_folio_size(mapping);
> +	unsigned int bdp_flags = (iter->flags & IOMAP_NOWAIT) ? BDP_ASYNC : 0;
> +	unsigned int bs = i_blocksize(iter->inode);
> +
> +	/* copied over based on how DIO handles these flags */
> +	if (iter->iomap.type == IOMAP_UNWRITTEN)
> +		wt_ctx->flags |= IOMAP_DIO_UNWRITTEN;
> +	if (iter->iomap.flags & IOMAP_F_SHARED)
> +		wt_ctx->flags |= IOMAP_DIO_COW;
> +
> +	if (!(iter->flags & IOMAP_WRITETHROUGH))
> +		return -EINVAL;
> +
> +	/*
> +	 * IOMAP_INLINE mappings have NULL bdev and would cause
> +	 * iomap_sector() to dereference invalid memory. Reject them.
> +	 */

Nit:
I noticed that gfs2 sets the bdev even for IOMAP_INLINE. A better
filesystem agnostic comment might be something like this?

/*
 * Inline data lives in the inode's metadata buffer, so it cannot be
 * written via a bio built from the folio.
 */

> +	if (iter->iomap.type == IOMAP_INLINE)
> +		return -EINVAL;
> +
> +	do {
> +		 * blocks in the bvec again.
> +		 */
> +		if (wt_ctx->nr_bvecs && prev_pos + prev_len > pos_aligned) {
> +			size_t delta = prev_pos + prev_len - pos_aligned;
> +
> +			/* Everything already added to bvec, nothing to do */
> +			if (delta >= len_aligned)
> +				goto put_folio;
> +
> +			pos_aligned += delta;
> +			off_aligned += delta;
> +			len_aligned -= delta;
> +		}
> +
> +		prev_pos = off_aligned;

I think this is a mistake?
prev_pos = pos_aligned; ?

> +		prev_len = len_aligned;
> +
> +		iomap_folio_prepare_writethrough(folio, off_aligned,
> +						 len_aligned);
> +
> +		if (!wt_ctx->nr_bvecs) {
> +			wt_ctx->bio_pos = round_down(pos, bs);

We could reuse pos_aligned variable here instead of recalculating?

> +			submit_start_pos = pos;
> +ssize_t iomap_file_writethrough_write(struct kiocb *iocb, struct iov_iter *i,
> +				      const struct iomap_writethrough_ops *wt_ops,
> +				      void *private)
> +{
> +	struct inode *inode = iocb->ki_filp->f_mapping->host;
> +	struct iomap_iter iter = {
> +		.inode		= inode,
> +		.pos		= iocb->ki_pos,
> +		.len		= iov_iter_count(i),
> +		.flags		= IOMAP_WRITE | IOMAP_WRITETHROUGH,
> +		.private	= private,
> +	};
> +	struct iomap_writethrough_ctx *wt_ctx;
> +	unsigned int max_bvecs;
> +	ssize_t ret;
> +	struct blk_plug plug;
> +	size_t min_folio_bytes = PAGE_SIZE
> +				 << mapping_min_folio_order(inode->i_mapping);

min_folio_nr_bytes could be used here.
> +
> +	/*
> +	 * For now we don't support any other flag with WRITETHROUGH
> +	 */
> +	if (!(iocb->ki_flags & IOCB_WRITETHROUGH))
> +		return -EINVAL;
> +	if (iocb->ki_flags & (IOCB_DONTCACHE))
> +		return -EINVAL;
> +	if (iocb_is_dsync(iocb))
> +		/* D_SYNC support not implemented yet */
> +		return -EOPNOTSUPP;
> +	if (!is_sync_kiocb(iocb))
> +		/* aio support not implemented yet */
> +		return -EOPNOTSUPP;
> +

-- 
Pankaj