All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Darrick J. Wong" <djwong@kernel.org>
To: Zhang Yi <yi.zhang@huaweicloud.com>
Cc: linux-fsdevel@vger.kernel.org, linux-xfs@vger.kernel.org,
	linux-ext4@vger.kernel.org, brauner@kernel.org,
	hch@infradead.org, yi.zhang@huawei.com, yizhang089@gmail.com,
	yangerkun@huawei.com, yukuai@fnnas.com
Subject: Re: [PATCH 1/4] iomap: correct the range of a partial dirty clear
Date: Thu, 14 May 2026 11:03:20 -0700	[thread overview]
Message-ID: <20260514180320.GA9555@frogsfrogsfrogs> (raw)
In-Reply-To: <20260514062955.1183976-2-yi.zhang@huaweicloud.com>

On Thu, May 14, 2026 at 02:29:52PM +0800, Zhang Yi wrote:
> From: Zhang Yi <yi.zhang@huawei.com>
> 
> The block range calculation in ifs_clear_range_dirty() is incorrect when
> partially clearing a range in a folio. We cannot clear the dirty bit of
> the first block or the last block if the start or end offset is not
> blocksize-aligned. This has not yet caused any issues since we always
> clear a whole folio in iomap_writeback_folio().
> 
> Fix this by rounding up the first block to blocksize alignment, and
> calculate the last block by rounding down (using truncation). Correct
> the nr_blks calculation accordingly.
> 
> Signed-off-by: Zhang Yi <yi.zhang@huawei.com>

Cc: <stable@vger.kernel.org> # v6.6
Fixes: 4ce02c67972211 ("iomap: Add per-block dirty state tracking to improve performance")

> ---
> This is modified from:
>  https://lore.kernel.org/linux-fsdevel/20240812121159.3775074-2-yi.zhang@huaweicloud.com/
> Changes:
>  - Use round_up() instead of DIV_ROUND_UP() to prevent wasted integer
>    division.
> 
>  fs/iomap/buffered-io.c | 12 ++++++++----
>  1 file changed, 8 insertions(+), 4 deletions(-)
> 
> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c
> index d7b648421a70..64351a448a8b 100644
> --- a/fs/iomap/buffered-io.c
> +++ b/fs/iomap/buffered-io.c
> @@ -176,13 +176,17 @@ static void ifs_clear_range_dirty(struct folio *folio,
>  {
>  	struct inode *inode = folio->mapping->host;
>  	unsigned int blks_per_folio = i_blocks_per_folio(inode, folio);
> -	unsigned int first_blk = (off >> inode->i_blkbits);
> -	unsigned int last_blk = (off + len - 1) >> inode->i_blkbits;
> -	unsigned int nr_blks = last_blk - first_blk + 1;
> +	unsigned int first_blk = round_up(off, i_blocksize(inode)) >>
> +				 inode->i_blkbits;

Ok, so now we round off up to the next fsblock to compute first_blk...

> +	unsigned int last_blk = (off + len) >> inode->i_blkbits;

...and last_blk (which is really the next block number after the range
that we're undirtying) is now rounded down.  Presumably off/len have to
be aligned to fsblock granularity so we'll never have to deal with
unaligned situations like (off=324,len=1), right?

>  	unsigned long flags;
>  
> +	if (first_blk >= last_blk)

Do we need this check?  When would the test actually be true?

--D

> +		return;
> +
>  	spin_lock_irqsave(&ifs->state_lock, flags);
> -	bitmap_clear(ifs->state, first_blk + blks_per_folio, nr_blks);
> +	bitmap_clear(ifs->state, first_blk + blks_per_folio,
> +		     last_blk - first_blk);
>  	spin_unlock_irqrestore(&ifs->state_lock, flags);
>  }
>  
> -- 
> 2.52.0
> 
> 

  reply	other threads:[~2026-05-14 18:03 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-14  6:29 [PATCH 0/4] iomap: trivial fixes for ext4 conversion Zhang Yi
2026-05-14  6:29 ` [PATCH 1/4] iomap: correct the range of a partial dirty clear Zhang Yi
2026-05-14 18:03   ` Darrick J. Wong [this message]
2026-05-15  1:20     ` Zhang Yi
2026-05-14  6:29 ` [PATCH 2/4] iomap: support invalidating partial folios Zhang Yi
2026-05-14  6:29 ` [PATCH 3/4] iomap: fix incorrect did_zero setting in iomap_zero_iter() Zhang Yi
2026-05-14  6:29 ` [PATCH 4/4] iomap: fix out-of-bounds bitmap_set() with zero-length range Zhang Yi
2026-05-14 15:08   ` Joanne Koong
2026-05-15  1:57     ` Zhang Yi
2026-05-14 18:10   ` Darrick J. Wong
2026-05-15  1:50     ` Zhang Yi
2026-05-15 17:30       ` Darrick J. Wong
2026-05-16  1:25         ` Zhang Yi

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=20260514180320.GA9555@frogsfrogsfrogs \
    --to=djwong@kernel.org \
    --cc=brauner@kernel.org \
    --cc=hch@infradead.org \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-xfs@vger.kernel.org \
    --cc=yangerkun@huawei.com \
    --cc=yi.zhang@huawei.com \
    --cc=yi.zhang@huaweicloud.com \
    --cc=yizhang089@gmail.com \
    --cc=yukuai@fnnas.com \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.