public inbox for linux-fsdevel@vger.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>,
	Christian Brauner <brauner@kernel.org>,
	Carlos Maiolino <cem@kernel.org>,
	"Martin K. Petersen" <martin.petersen@oracle.com>,
	Anuj Gupta <anuj20.g@samsung.com>,
	Kanchan Joshi <joshi.k@samsung.com>,
	Konstantin Komarov <almaz.alexandrovich@paragon-software.com>,
	ntfs3@lists.linux.dev, linux-block@vger.kernel.org,
	nvdimm@lists.linux.dev, linux-fsdevel@vger.kernel.org,
	linux-xfs@vger.kernel.org
Subject: Re: [PATCH 12/16] ntfs3: remove copy and pasted iomap code
Date: Mon, 23 Feb 2026 08:27:39 -0800	[thread overview]
Message-ID: <20260223162739.GC2390353@frogsfrogsfrogs> (raw)
In-Reply-To: <20260223132021.292832-13-hch@lst.de>

On Mon, Feb 23, 2026 at 05:20:12AM -0800, Christoph Hellwig wrote:
> ntfs3 copied the iomap code without attribution or talking to the
> maintainers, to hook into the bio completion for (unexplained) zeroing.

Well they /did/ attribute it...

> Fix this by just overriding the bio completion handler in the submit
> handler.
> 
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
>  fs/ntfs3/inode.c | 51 +++---------------------------------------------
>  1 file changed, 3 insertions(+), 48 deletions(-)
> 
> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c
> index 7ab4e18f8013..60af9f8e0366 100644
> --- a/fs/ntfs3/inode.c
> +++ b/fs/ntfs3/inode.c
> @@ -605,63 +605,18 @@ static void ntfs_iomap_read_end_io(struct bio *bio)
>  	bio_put(bio);
>  }
>  
> -/*
> - * Copied from iomap/bio.c.

...extremely poorly, by not recording their reasons for copy-pasting the
code.

> - */
> -static int ntfs_iomap_bio_read_folio_range(const struct iomap_iter *iter,
> -					   struct iomap_read_folio_ctx *ctx,
> -					   size_t plen)
> -{
> -	struct folio *folio = ctx->cur_folio;
> -	const struct iomap *iomap = &iter->iomap;
> -	loff_t pos = iter->pos;
> -	size_t poff = offset_in_folio(folio, pos);
> -	loff_t length = iomap_length(iter);
> -	sector_t sector;
> -	struct bio *bio = ctx->read_ctx;
> -
> -	sector = iomap_sector(iomap, pos);
> -	if (!bio || bio_end_sector(bio) != sector ||
> -	    !bio_add_folio(bio, folio, plen, poff)) {
> -		gfp_t gfp = mapping_gfp_constraint(folio->mapping, GFP_KERNEL);
> -		gfp_t orig_gfp = gfp;
> -		unsigned int nr_vecs = DIV_ROUND_UP(length, PAGE_SIZE);
> -
> -		if (bio)
> -			submit_bio(bio);
> -
> -		if (ctx->rac) /* same as readahead_gfp_mask */
> -			gfp |= __GFP_NORETRY | __GFP_NOWARN;
> -		bio = bio_alloc(iomap->bdev, bio_max_segs(nr_vecs), REQ_OP_READ,
> -				gfp);
> -		/*
> -		 * If the bio_alloc fails, try it again for a single page to
> -		 * avoid having to deal with partial page reads.  This emulates
> -		 * what do_mpage_read_folio does.
> -		 */
> -		if (!bio)
> -			bio = bio_alloc(iomap->bdev, 1, REQ_OP_READ, orig_gfp);
> -		if (ctx->rac)
> -			bio->bi_opf |= REQ_RAHEAD;
> -		bio->bi_iter.bi_sector = sector;
> -		bio->bi_end_io = ntfs_iomap_read_end_io;
> -		bio_add_folio_nofail(bio, folio, plen, poff);
> -		ctx->read_ctx = bio;
> -	}
> -	return 0;
> -}

Yeah, identical to iomap_bio_read_folio_range except for setting
bi_end_io.

Don't copy-paste stuff.
Reviewed-by: "Darrick J. Wong" <djwong@kernel.org>

--D

> -
>  static void ntfs_iomap_bio_submit_read(const struct iomap_iter *iter,
>  		struct iomap_read_folio_ctx *ctx)
>  {
>  	struct bio *bio = ctx->read_ctx;
>  
> +	bio->bi_end_io = ntfs_iomap_read_end_io;
>  	submit_bio(bio);
>  }
>  
>  static const struct iomap_read_ops ntfs_iomap_bio_read_ops = {
> -	.read_folio_range = ntfs_iomap_bio_read_folio_range,
> -	.submit_read = ntfs_iomap_bio_submit_read,
> +	.read_folio_range	= iomap_bio_read_folio_range,
> +	.submit_read		= ntfs_iomap_bio_submit_read,
>  };
>  
>  static int ntfs_read_folio(struct file *file, struct folio *folio)
> -- 
> 2.47.3
> 
> 

  reply	other threads:[~2026-02-23 16:27 UTC|newest]

Thread overview: 26+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-02-23 13:20 support file system generated / verified integrity information v4 Christoph Hellwig
2026-02-23 13:20 ` [PATCH 01/16] block: factor out a bio_integrity_action helper Christoph Hellwig
2026-02-23 13:20 ` [PATCH 02/16] block: factor out a bio_integrity_setup_default helper Christoph Hellwig
2026-02-23 13:20 ` [PATCH 03/16] block: add a bdev_has_integrity_csum helper Christoph Hellwig
2026-02-23 13:20 ` [PATCH 04/16] block: prepare generation / verification helpers for fs usage Christoph Hellwig
2026-02-23 13:20 ` [PATCH 05/16] block: make max_integrity_io_size public Christoph Hellwig
2026-02-23 13:20 ` [PATCH 06/16] block: add fs_bio_integrity helpers Christoph Hellwig
2026-02-23 13:20 ` [PATCH 07/16] block: pass a maxlen argument to bio_iov_iter_bounce Christoph Hellwig
2026-02-23 13:20 ` [PATCH 08/16] iomap: refactor iomap_bio_read_folio_range Christoph Hellwig
2026-02-23 13:20 ` [PATCH 09/16] iomap: pass the iomap_iter to ->submit_read Christoph Hellwig
2026-02-23 13:20 ` [PATCH 10/16] iomap: only call into ->submit_read when there is a read_ctx Christoph Hellwig
2026-02-23 13:20 ` [PATCH 11/16] iomap: allow file systems to hook into buffered read bio submission Christoph Hellwig
2026-02-23 13:20 ` [PATCH 12/16] ntfs3: remove copy and pasted iomap code Christoph Hellwig
2026-02-23 16:27   ` Darrick J. Wong [this message]
2026-02-27 13:46   ` Konstantin Komarov
2026-03-03 15:21     ` Christoph Hellwig
2026-03-04  9:57       ` Konstantin Komarov
2026-02-23 13:20 ` [PATCH 13/16] iomap: add a bioset pointer to iomap_read_folio_ops Christoph Hellwig
2026-02-23 13:20 ` [PATCH 14/16] iomap: support ioends for buffered reads Christoph Hellwig
2026-02-23 13:20 ` [PATCH 15/16] iomap: support T10 protection information Christoph Hellwig
2026-02-23 13:20 ` [PATCH 16/16] xfs: " Christoph Hellwig
2026-03-02 10:11 ` support file system generated / verified integrity information v4 Christian Brauner
2026-03-02 13:48   ` Christoph Hellwig
2026-03-04 12:55     ` Christian Brauner
2026-03-09 14:52 ` (subset) " Jens Axboe
2026-03-10 10:16   ` Christian Brauner

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=20260223162739.GC2390353@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=almaz.alexandrovich@paragon-software.com \
    --cc=anuj20.g@samsung.com \
    --cc=axboe@kernel.dk \
    --cc=brauner@kernel.org \
    --cc=cem@kernel.org \
    --cc=hch@lst.de \
    --cc=joshi.k@samsung.com \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=martin.petersen@oracle.com \
    --cc=ntfs3@lists.linux.dev \
    --cc=nvdimm@lists.linux.dev \
    /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