[f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[Chao Yu]

Use sbi->log_sectors_per_block to clean up below calculated one:

unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;

Signed-off-by: Chao Yu <chao@kernel.org>
---
 fs/f2fs/segment.c | 23 +++++++++++------------
 1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 43d537d29b52..9282399cc810 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4768,17 +4768,17 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 {
 	unsigned int wp_segno, wp_blkoff, zone_secno, zone_segno, segno;
 	block_t zone_block, wp_block, last_valid_block;
-	unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
 	int i, s, b, ret;
 	struct seg_entry *se;
 
 	if (zone->type != BLK_ZONE_TYPE_SEQWRITE_REQ)
 		return 0;
 
-	wp_block = fdev->start_blk + (zone->wp >> log_sectors_per_block);
+	wp_block = fdev->start_blk + (zone->wp >> sbi->log_sectors_per_block);
 	wp_segno = GET_SEGNO(sbi, wp_block);
 	wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
-	zone_block = fdev->start_blk + (zone->start >> log_sectors_per_block);
+	zone_block = fdev->start_blk + (zone->start >>
+						sbi->log_sectors_per_block);
 	zone_segno = GET_SEGNO(sbi, zone_block);
 	zone_secno = GET_SEC_FROM_SEG(sbi, zone_segno);
 
@@ -4824,7 +4824,7 @@ static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
 			    "pointer. Reset the write pointer: wp[0x%x,0x%x]",
 			    wp_segno, wp_blkoff);
 		ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
-					zone->len >> log_sectors_per_block);
+				zone->len >> sbi->log_sectors_per_block);
 		if (ret)
 			f2fs_err(sbi, "Discard zone failed: %s (errno=%d)",
 				 fdev->path, ret);
@@ -4885,7 +4885,6 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
 	struct blk_zone zone;
 	unsigned int cs_section, wp_segno, wp_blkoff, wp_sector_off;
 	block_t cs_zone_block, wp_block;
-	unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
 	sector_t zone_sector;
 	int err;
 
@@ -4897,8 +4896,8 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
 		return 0;
 
 	/* report zone for the sector the curseg points to */
-	zone_sector = (sector_t)(cs_zone_block - zbd->start_blk)
-		<< log_sectors_per_block;
+	zone_sector = (sector_t)(cs_zone_block - zbd->start_blk) <<
+						sbi->log_sectors_per_block;
 	err = blkdev_report_zones(zbd->bdev, zone_sector, 1,
 				  report_one_zone_cb, &zone);
 	if (err != 1) {
@@ -4910,10 +4909,10 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
 	if (zone.type != BLK_ZONE_TYPE_SEQWRITE_REQ)
 		return 0;
 
-	wp_block = zbd->start_blk + (zone.wp >> log_sectors_per_block);
+	wp_block = zbd->start_blk + (zone.wp >> sbi->log_sectors_per_block);
 	wp_segno = GET_SEGNO(sbi, wp_block);
 	wp_blkoff = wp_block - START_BLOCK(sbi, wp_segno);
-	wp_sector_off = zone.wp & GENMASK(log_sectors_per_block - 1, 0);
+	wp_sector_off = zone.wp & GENMASK(sbi->log_sectors_per_block - 1, 0);
 
 	if (cs->segno == wp_segno && cs->next_blkoff == wp_blkoff &&
 		wp_sector_off == 0)
@@ -4940,8 +4939,8 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
 	if (!zbd)
 		return 0;
 
-	zone_sector = (sector_t)(cs_zone_block - zbd->start_blk)
-		<< log_sectors_per_block;
+	zone_sector = (sector_t)(cs_zone_block - zbd->start_blk) <<
+						sbi->log_sectors_per_block;
 	err = blkdev_report_zones(zbd->bdev, zone_sector, 1,
 				  report_one_zone_cb, &zone);
 	if (err != 1) {
@@ -4959,7 +4958,7 @@ static int fix_curseg_write_pointer(struct f2fs_sb_info *sbi, int type)
 			    "Reset the zone: curseg[0x%x,0x%x]",
 			    type, cs->segno, cs->next_blkoff);
 		err = __f2fs_issue_discard_zone(sbi, zbd->bdev,	cs_zone_block,
-					zone.len >> log_sectors_per_block);
+					zone.len >> sbi->log_sectors_per_block);
 		if (err) {
 			f2fs_err(sbi, "Discard zone failed: %s (errno=%d)",
 				 zbd->path, err);
-- 
2.40.1



_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[patchwork-bot+f2fs]

Hello:

This patch was applied to jaegeuk/f2fs.git (dev)
by Jaegeuk Kim <jaegeuk@kernel.org>:

On Tue, 23 May 2023 20:35:21 +0800 you wrote:
> Use sbi->log_sectors_per_block to clean up below calculated one:
> 
> unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
> 
> Signed-off-by: Chao Yu <chao@kernel.org>
> ---
>  fs/f2fs/segment.c | 23 +++++++++++------------
>  1 file changed, 11 insertions(+), 12 deletions(-)

Here is the summary with links:
  - [f2fs-dev] f2fs: clean up w/ sbi->log_sectors_per_block
    https://git.kernel.org/jaegeuk/f2fs/c/584212446615

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html




_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[Shinichiro Kawasaki via Linux-f2fs-devel]

On May 23, 2023 / 20:35, Chao Yu wrote:
> Use sbi->log_sectors_per_block to clean up below calculated one:
> 
> unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;

Hello Chao,

When I ran workloads on f2fs using v6.5-rcX with fixes [1][2] and a zoned block
devices with 4kb logical block size, I observe mount failure as follows. When
I revert this commit, the failure goes away.

[  167.781975][ T1555] F2FS-fs (dm-0): IO Block Size:        4 KB
[  167.890728][ T1555] F2FS-fs (dm-0): Found nat_bits in checkpoint
[  171.482588][ T1555] F2FS-fs (dm-0): Zone without valid block has non-zero write pointer. Reset the write pointer: wp[0x1300,0x8]
[  171.496000][ T1555] F2FS-fs (dm-0): (0) : Unaligned zone reset attempted (block 280000 + 80000)
[  171.505037][ T1555] F2FS-fs (dm-0): Discard zone failed:  (errno=-5)

The patch replaced "sbi->log_blocksize - SECTOR_SHIFT" with
"sbi->log_sectors_per_block". However, I think these two are not equal when the
device has 4k logical block size. The former uses Linux kernel sector size 512
byte. The latter use 512b sector size or 4kb sector size depending on the
device. mkfs.f2fs obtains logical block size via BLKSSZGET ioctl from the device
and reflects it to the value sbi->log_sector_size_per_block. This causes
unexpected write pointer calculations in check_zone_write_pointer(). This
resulted in unexpected zone reset and the mount failure.

I think this patch needs revert. What do you think?

[1] https://lkml.kernel.org/linux-f2fs-devel/20230711050101.GA19128@lst.de/
[2] https://lore.kernel.org/linux-f2fs-devel/20230804091556.2372567-1-shinichiro.kawasaki@wdc.com/

_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[Jaegeuk Kim]

On 08/04, Shinichiro Kawasaki wrote:
> On May 23, 2023 / 20:35, Chao Yu wrote:
> > Use sbi->log_sectors_per_block to clean up below calculated one:
> > 
> > unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
> 
> Hello Chao,
> 
> When I ran workloads on f2fs using v6.5-rcX with fixes [1][2] and a zoned block
> devices with 4kb logical block size, I observe mount failure as follows. When
> I revert this commit, the failure goes away.
> 
> [  167.781975][ T1555] F2FS-fs (dm-0): IO Block Size:        4 KB
> [  167.890728][ T1555] F2FS-fs (dm-0): Found nat_bits in checkpoint
> [  171.482588][ T1555] F2FS-fs (dm-0): Zone without valid block has non-zero write pointer. Reset the write pointer: wp[0x1300,0x8]
> [  171.496000][ T1555] F2FS-fs (dm-0): (0) : Unaligned zone reset attempted (block 280000 + 80000)
> [  171.505037][ T1555] F2FS-fs (dm-0): Discard zone failed:  (errno=-5)
> 
> The patch replaced "sbi->log_blocksize - SECTOR_SHIFT" with
> "sbi->log_sectors_per_block". However, I think these two are not equal when the
> device has 4k logical block size. The former uses Linux kernel sector size 512
> byte. The latter use 512b sector size or 4kb sector size depending on the
> device. mkfs.f2fs obtains logical block size via BLKSSZGET ioctl from the device
> and reflects it to the value sbi->log_sector_size_per_block. This causes
> unexpected write pointer calculations in check_zone_write_pointer(). This
> resulted in unexpected zone reset and the mount failure.
> 
> I think this patch needs revert. What do you think?

Yeah, applied the revert.

> 
> [1] https://lkml.kernel.org/linux-f2fs-devel/20230711050101.GA19128@lst.de/
> [2] https://lore.kernel.org/linux-f2fs-devel/20230804091556.2372567-1-shinichiro.kawasaki@wdc.com/


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[Chao Yu]

On 2023/8/5 3:21, Jaegeuk Kim wrote:
> On 08/04, Shinichiro Kawasaki wrote:
>> On May 23, 2023 / 20:35, Chao Yu wrote:
>>> Use sbi->log_sectors_per_block to clean up below calculated one:
>>>
>>> unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
>>
>> Hello Chao,
>>
>> When I ran workloads on f2fs using v6.5-rcX with fixes [1][2] and a zoned block
>> devices with 4kb logical block size, I observe mount failure as follows. When
>> I revert this commit, the failure goes away.
>>
>> [  167.781975][ T1555] F2FS-fs (dm-0): IO Block Size:        4 KB
>> [  167.890728][ T1555] F2FS-fs (dm-0): Found nat_bits in checkpoint
>> [  171.482588][ T1555] F2FS-fs (dm-0): Zone without valid block has non-zero write pointer. Reset the write pointer: wp[0x1300,0x8]
>> [  171.496000][ T1555] F2FS-fs (dm-0): (0) : Unaligned zone reset attempted (block 280000 + 80000)
>> [  171.505037][ T1555] F2FS-fs (dm-0): Discard zone failed:  (errno=-5)
>>
>> The patch replaced "sbi->log_blocksize - SECTOR_SHIFT" with
>> "sbi->log_sectors_per_block". However, I think these two are not equal when the
>> device has 4k logical block size. The former uses Linux kernel sector size 512
>> byte. The latter use 512b sector size or 4kb sector size depending on the
>> device. mkfs.f2fs obtains logical block size via BLKSSZGET ioctl from the device
>> and reflects it to the value sbi->log_sector_size_per_block. This causes
>> unexpected write pointer calculations in check_zone_write_pointer(). This
>> resulted in unexpected zone reset and the mount failure.
>>
>> I think this patch needs revert. What do you think?
> 
> Yeah, applied the revert.

Jaegeuk, could you please send the patch to mailing list?

Thanks,

> 
>>
>> [1] https://lkml.kernel.org/linux-f2fs-devel/20230711050101.GA19128@lst.de/
>> [2] https://lore.kernel.org/linux-f2fs-devel/20230804091556.2372567-1-shinichiro.kawasaki@wdc.com/


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[Jaegeuk Kim]

On 08/07, Chao Yu wrote:
> On 2023/8/5 3:21, Jaegeuk Kim wrote:
> > On 08/04, Shinichiro Kawasaki wrote:
> > > On May 23, 2023 / 20:35, Chao Yu wrote:
> > > > Use sbi->log_sectors_per_block to clean up below calculated one:
> > > > 
> > > > unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
> > > 
> > > Hello Chao,
> > > 
> > > When I ran workloads on f2fs using v6.5-rcX with fixes [1][2] and a zoned block
> > > devices with 4kb logical block size, I observe mount failure as follows. When
> > > I revert this commit, the failure goes away.
> > > 
> > > [  167.781975][ T1555] F2FS-fs (dm-0): IO Block Size:        4 KB
> > > [  167.890728][ T1555] F2FS-fs (dm-0): Found nat_bits in checkpoint
> > > [  171.482588][ T1555] F2FS-fs (dm-0): Zone without valid block has non-zero write pointer. Reset the write pointer: wp[0x1300,0x8]
> > > [  171.496000][ T1555] F2FS-fs (dm-0): (0) : Unaligned zone reset attempted (block 280000 + 80000)
> > > [  171.505037][ T1555] F2FS-fs (dm-0): Discard zone failed:  (errno=-5)
> > > 
> > > The patch replaced "sbi->log_blocksize - SECTOR_SHIFT" with
> > > "sbi->log_sectors_per_block". However, I think these two are not equal when the
> > > device has 4k logical block size. The former uses Linux kernel sector size 512
> > > byte. The latter use 512b sector size or 4kb sector size depending on the
> > > device. mkfs.f2fs obtains logical block size via BLKSSZGET ioctl from the device
> > > and reflects it to the value sbi->log_sector_size_per_block. This causes
> > > unexpected write pointer calculations in check_zone_write_pointer(). This
> > > resulted in unexpected zone reset and the mount failure.
> > > 
> > > I think this patch needs revert. What do you think?
> > 
> > Yeah, applied the revert.
> 
> Jaegeuk, could you please send the patch to mailing list?

I sent. Thanks.

> 
> Thanks,
> 
> > 
> > > 
> > > [1] https://lkml.kernel.org/linux-f2fs-devel/20230711050101.GA19128@lst.de/
> > > [2] https://lore.kernel.org/linux-f2fs-devel/20230804091556.2372567-1-shinichiro.kawasaki@wdc.com/


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [PATCH] f2fs: clean up w/ sbi->log_sectors_per_block

[Chao Yu]

On 2023/8/4 17:43, Shinichiro Kawasaki wrote:
> On May 23, 2023 / 20:35, Chao Yu wrote:
>> Use sbi->log_sectors_per_block to clean up below calculated one:
>>
>> unsigned int log_sectors_per_block = sbi->log_blocksize - SECTOR_SHIFT;
> 
> Hello Chao,
> 
> When I ran workloads on f2fs using v6.5-rcX with fixes [1][2] and a zoned block
> devices with 4kb logical block size, I observe mount failure as follows. When
> I revert this commit, the failure goes away.
> 
> [  167.781975][ T1555] F2FS-fs (dm-0): IO Block Size:        4 KB
> [  167.890728][ T1555] F2FS-fs (dm-0): Found nat_bits in checkpoint
> [  171.482588][ T1555] F2FS-fs (dm-0): Zone without valid block has non-zero write pointer. Reset the write pointer: wp[0x1300,0x8]
> [  171.496000][ T1555] F2FS-fs (dm-0): (0) : Unaligned zone reset attempted (block 280000 + 80000)
> [  171.505037][ T1555] F2FS-fs (dm-0): Discard zone failed:  (errno=-5)
> 
> The patch replaced "sbi->log_blocksize - SECTOR_SHIFT" with
> "sbi->log_sectors_per_block". However, I think these two are not equal when the
> device has 4k logical block size. The former uses Linux kernel sector size 512
> byte. The latter use 512b sector size or 4kb sector size depending on the
> device. mkfs.f2fs obtains logical block size via BLKSSZGET ioctl from the device
> and reflects it to the value sbi->log_sector_size_per_block. This causes
> unexpected write pointer calculations in check_zone_write_pointer(). This
> resulted in unexpected zone reset and the mount failure.
> 
> I think this patch needs revert. What do you think?

Hi Shinichiro,

Oh, my bad, I think we need to revert that patch.

Thanks for the report! :)

Thanks,

> 
> [1] https://lkml.kernel.org/linux-f2fs-devel/20230711050101.GA19128@lst.de/
> [2] https://lore.kernel.org/linux-f2fs-devel/20230804091556.2372567-1-shinichiro.kawasaki@wdc.com/


_______________________________________________
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel