| Message ID | 20240812121159.3775074-2-yi.zhang@huaweicloud.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | iomap: some minor non-critical fixes and improvements when block size < folio size | expand |
On Mon, Aug 12, 2024 at 08:11:54PM +0800, Zhang Yi wrote: > From: Zhang Yi <yi.zhang@huawei.com> > > The block range calculation in ifs_clear_range_dirty() is incorrect when > partial clear a range in a folio. We can't clear the dirty bit of the > first block or the last block if the start or end offset is blocksize > unaligned, this has not yet caused any issue since we always clear a > whole folio in iomap_writepage_map()->iomap_clear_range_dirty(). Fix > this by round up the first block and round down the last block and > correct the calculation of nr_blks. > > Signed-off-by: Zhang Yi <yi.zhang@huawei.com> > --- > fs/iomap/buffered-io.c | 9 ++++++--- > 1 file changed, 6 insertions(+), 3 deletions(-) > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > index f420c53d86ac..4da453394aaf 100644 > --- a/fs/iomap/buffered-io.c > +++ b/fs/iomap/buffered-io.c > @@ -138,11 +138,14 @@ 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 = DIV_ROUND_UP(off, i_blocksize(inode)); Is there a round up macro that doesn't involve integer division? --D > + unsigned int last_blk = (off + len) >> inode->i_blkbits; > + unsigned int nr_blks = last_blk - first_blk; > unsigned long flags; > > + if (!nr_blks) > + return; > + > spin_lock_irqsave(&ifs->state_lock, flags); > bitmap_clear(ifs->state, first_blk + blks_per_folio, nr_blks); > spin_unlock_irqrestore(&ifs->state_lock, flags); > -- > 2.39.2 > >
On 2024/8/13 0:33, Darrick J. Wong wrote: > On Mon, Aug 12, 2024 at 08:11:54PM +0800, Zhang Yi wrote: >> From: Zhang Yi <yi.zhang@huawei.com> >> >> The block range calculation in ifs_clear_range_dirty() is incorrect when >> partial clear a range in a folio. We can't clear the dirty bit of the >> first block or the last block if the start or end offset is blocksize >> unaligned, this has not yet caused any issue since we always clear a >> whole folio in iomap_writepage_map()->iomap_clear_range_dirty(). Fix >> this by round up the first block and round down the last block and >> correct the calculation of nr_blks. >> >> Signed-off-by: Zhang Yi <yi.zhang@huawei.com> >> --- >> fs/iomap/buffered-io.c | 9 ++++++--- >> 1 file changed, 6 insertions(+), 3 deletions(-) >> >> diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c >> index f420c53d86ac..4da453394aaf 100644 >> --- a/fs/iomap/buffered-io.c >> +++ b/fs/iomap/buffered-io.c >> @@ -138,11 +138,14 @@ 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 = DIV_ROUND_UP(off, i_blocksize(inode)); > > Is there a round up macro that doesn't involve integer division? > Sorry, I don't find a common macro now, if we want to avoid integer division, how about open code here? first_blk = round_up(off, i_blocksize(inode)) >> inode->i_blkbits; Thanks, Yi. > >> + unsigned int last_blk = (off + len) >> inode->i_blkbits; >> + unsigned int nr_blks = last_blk - first_blk; >> unsigned long flags; >> >> + if (!nr_blks) >> + return; >> + >> spin_lock_irqsave(&ifs->state_lock, flags); >> bitmap_clear(ifs->state, first_blk + blks_per_folio, nr_blks); >> spin_unlock_irqrestore(&ifs->state_lock, flags); >> -- >> 2.39.2 >> >>
On Mon, Aug 12, 2024 at 09:33:39AM -0700, Darrick J. Wong wrote: > On Mon, Aug 12, 2024 at 08:11:54PM +0800, Zhang Yi wrote: > > From: Zhang Yi <yi.zhang@huawei.com> > > > > The block range calculation in ifs_clear_range_dirty() is incorrect when > > partial clear a range in a folio. We can't clear the dirty bit of the > > first block or the last block if the start or end offset is blocksize > > unaligned, this has not yet caused any issue since we always clear a > > whole folio in iomap_writepage_map()->iomap_clear_range_dirty(). Fix > > this by round up the first block and round down the last block and > > correct the calculation of nr_blks. > > > > Signed-off-by: Zhang Yi <yi.zhang@huawei.com> > > --- > > fs/iomap/buffered-io.c | 9 ++++++--- > > 1 file changed, 6 insertions(+), 3 deletions(-) > > > > diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c > > index f420c53d86ac..4da453394aaf 100644 > > --- a/fs/iomap/buffered-io.c > > +++ b/fs/iomap/buffered-io.c > > @@ -138,11 +138,14 @@ 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 = DIV_ROUND_UP(off, i_blocksize(inode)); > > Is there a round up macro that doesn't involve integer division? i_blocksize() is supposed to be a power of 2, so yes: /** * round_up - round up to next specified power of 2 * @x: the value to round * @y: multiple to round up to (must be a power of 2) * * Rounds @x up to next multiple of @y (which must be a power of 2). * To perform arbitrary rounding up, use roundup() below. */ #define round_up(x, y) ((((x)-1) | __round_mask(x, y))+1) -Dave.
diff --git a/fs/iomap/buffered-io.c b/fs/iomap/buffered-io.c index f420c53d86ac..4da453394aaf 100644 --- a/fs/iomap/buffered-io.c +++ b/fs/iomap/buffered-io.c @@ -138,11 +138,14 @@ 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 = DIV_ROUND_UP(off, i_blocksize(inode)); + unsigned int last_blk = (off + len) >> inode->i_blkbits; + unsigned int nr_blks = last_blk - first_blk; unsigned long flags; + if (!nr_blks) + return; + spin_lock_irqsave(&ifs->state_lock, flags); bitmap_clear(ifs->state, first_blk + blks_per_folio, nr_blks); spin_unlock_irqrestore(&ifs->state_lock, flags);