[f2fs-dev] [PATCH 0/3] F2FS Tools 16K Bug Fixes

[f2fs-dev] [PATCH 0/3] F2FS Tools 16K Bug Fixes

[Daniel Rosenberg via Linux-f2fs-devel]

I missed a couple things in my previous patch set. This fixes Quotas,
the -c cache option, and a debug print. The issues can be seen by running:

truncate test.dat --size 256M
make_f2fs -g android -O project_quota,extra_attr -w 16384 -b 16384 test.dat 16384
fsck.f2fs -f -c 10000 --debug-cache test.dat

Daniel Rosenberg (3):
  f2fs-tools: Fix debug size print
  f2fs-tools: Wait for Block Size to initialize Cache
  f2fs-tools: Fix dqb_curspace to reflect blocksize

 fsck/fsck.c        | 3 ++-
 fsck/mount.c       | 5 +++++
 mkfs/f2fs_format.c | 4 ++--
 3 files changed, 9 insertions(+), 3 deletions(-)


base-commit: f71fbf8f3dee4eefdddac1abaaf4ae76bb9a48b3
-- 
2.43.0.rc0.421.g78406f8d94-goog



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

[f2fs-dev] [PATCH 1/3] f2fs-tools: Fix debug size print

[Daniel Rosenberg via Linux-f2fs-devel]

The conversion from block size to MB in this debug statement assumes a
block size of 4K. This switches it to properly use the filesystem's
block size.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fsck/fsck.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/fsck/fsck.c b/fsck/fsck.c
index 55eddca..509b485 100644
--- a/fsck/fsck.c
+++ b/fsck/fsck.c
@@ -3494,7 +3494,8 @@ int fsck_verify(struct f2fs_sb_info *sbi)
 	max_blks = SM_I(sbi)->main_blkaddr + (data_secs + node_secs) *
 							BLKS_PER_SEC(sbi);
 	printf("[FSCK] Max image size: %"PRIu64" MB, Free space: %"PRIu64" MB\n",
-						max_blks >> 8, free_blks >> 8);
+						max_blks >> (20 - F2FS_BLKSIZE_BITS),
+						free_blks >> (20 - F2FS_BLKSIZE_BITS));
 	printf("[FSCK] Unreachable nat entries                       ");
 	if (nr_unref_nid == 0x0) {
 		printf(" [Ok..] [0x%x]\n", nr_unref_nid);
-- 
2.43.0.rc0.421.g78406f8d94-goog



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

Re: [f2fs-dev] [PATCH 1/3] f2fs-tools: Fix debug size print

[Chao Yu]

On 2023/11/18 10:03, Daniel Rosenberg wrote:
> The conversion from block size to MB in this debug statement assumes a
> block size of 4K. This switches it to properly use the filesystem's
> block size.
> 
> Signed-off-by: Daniel Rosenberg <drosen@google.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


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

[f2fs-dev] [PATCH 2/3] f2fs-tools: Wait for Block Size to initialize Cache

[Daniel Rosenberg via Linux-f2fs-devel]

The cache is initialized during the first read, however, it requires the
block size to establish its buffer. This disables the cache until the
block size is known.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 fsck/mount.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/fsck/mount.c b/fsck/mount.c
index 72516f4..658e601 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -3860,8 +3860,12 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
 {
 	struct f2fs_checkpoint *cp = NULL;
 	struct f2fs_super_block *sb = NULL;
+	int num_cache_entry = c.cache_config.num_cache_entry;
 	int ret;
 
+	/* Must not initiate cache until block size is known */
+	c.cache_config.num_cache_entry = 0;
+
 	sbi->active_logs = NR_CURSEG_TYPE;
 	ret = validate_super_block(sbi, SB0_ADDR);
 	if (ret) {
@@ -3881,6 +3885,7 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
 		}
 	}
 	sb = F2FS_RAW_SUPER(sbi);
+	c.cache_config.num_cache_entry = num_cache_entry;
 
 	ret = check_sector_size(sb);
 	if (ret)
-- 
2.43.0.rc0.421.g78406f8d94-goog



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

Re: [f2fs-dev] [PATCH 2/3] f2fs-tools: Wait for Block Size to initialize Cache

[Chao Yu]

On 2023/11/18 10:03, Daniel Rosenberg wrote:
> The cache is initialized during the first read, however, it requires the
> block size to establish its buffer. This disables the cache until the
> block size is known.

Hi Daniel,

How about this? It be more explicit to indicate the logic?

---
  fsck/mount.c      | 2 ++
  include/f2fs_fs.h | 3 +++
  lib/libf2fs_io.c  | 4 ++++
  3 files changed, 9 insertions(+)

diff --git a/fsck/mount.c b/fsck/mount.c
index 72516f4..4dfb996 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1238,6 +1238,8 @@ int init_sb_info(struct f2fs_sb_info *sbi)
  	MSG(0, "Info: total FS sectors = %"PRIu64" (%"PRIu64" MB)\n",
  				total_sectors, total_sectors >>
  						(20 - get_sb(log_sectorsize)));
+
+	c.cache_config.blksize_initialized = true;
  	return 0;
  }

diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 6df2e73..e377d48 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -460,6 +460,9 @@ typedef struct {
  	unsigned max_hash_collision;

  	bool dbg_en;
+
+	/* whether blksize has been initialized */
+	bool blksize_initialized;
  } dev_cache_config_t;

  /* f2fs_configration for compression used for sload.f2fs */
diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
index 39d3777..bb77418 100644
--- a/lib/libf2fs_io.c
+++ b/lib/libf2fs_io.c
@@ -199,6 +199,10 @@ void dcache_init(void)
  {
  	long n;

+	/* Must not initiate cache until block size is known */
+	if (!c.cache_config.blksize_initialized)
+		return;
+
  	if (c.cache_config.num_cache_entry <= 0)
  		return;

-- 
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 2/3] f2fs-tools: Wait for Block Size to initialize Cache

[Daniel Rosenberg via Linux-f2fs-devel]

On Sun, Nov 26, 2023 at 5:42 PM Chao Yu <chao@kernel.org> wrote:
>
> Hi Daniel,
>
> How about this? It be more explicit to indicate the logic?
>
> ---
>   fsck/mount.c      | 2 ++
>   include/f2fs_fs.h | 3 +++
>   lib/libf2fs_io.c  | 4 ++++
>   3 files changed, 9 insertions(+)
>
> diff --git a/fsck/mount.c b/fsck/mount.c
> index 72516f4..4dfb996 100644
> --- a/fsck/mount.c
> +++ b/fsck/mount.c
> @@ -1238,6 +1238,8 @@ int init_sb_info(struct f2fs_sb_info *sbi)
>         MSG(0, "Info: total FS sectors = %"PRIu64" (%"PRIu64" MB)\n",
>                                 total_sectors, total_sectors >>
>                                                 (20 - get_sb(log_sectorsize)));
> +
> +       c.cache_config.blksize_initialized = true;
>         return 0;
>   }
>
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 6df2e73..e377d48 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -460,6 +460,9 @@ typedef struct {
>         unsigned max_hash_collision;
>
>         bool dbg_en;
> +
> +       /* whether blksize has been initialized */
> +       bool blksize_initialized;
>   } dev_cache_config_t;
>
>   /* f2fs_configration for compression used for sload.f2fs */
> diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
> index 39d3777..bb77418 100644
> --- a/lib/libf2fs_io.c
> +++ b/lib/libf2fs_io.c
> @@ -199,6 +199,10 @@ void dcache_init(void)
>   {
>         long n;
>
> +       /* Must not initiate cache until block size is known */
> +       if (!c.cache_config.blksize_initialized)
> +               return;
> +
>         if (c.cache_config.num_cache_entry <= 0)
>                 return;
>
> --
> 2.40.1
>
>
I think that works too. I was initially going to go with code like
that, but I was unsure if read/writes that aren't in the fsck path
used caching as well. There's a few uses of those in the mkfs/ paths,
but I don't think there's anything that sets
cache_config.num_cache_entry there. I was worried about missing code
paths like that, so I had restricted the changes to where I knew we
were reading the superblock off disk. If there is a problem, it could
be covered by setting c.cache_config.blksize_initialized = true within
the mkfs code after the blocksize is set in f2fs_parse_options. But
that isn't needed if there's no caching in mkfs.


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

Re: [f2fs-dev] [PATCH 2/3] f2fs-tools: Wait for Block Size to initialize Cache

[Chao Yu]

On 2023/11/28 8:52, Daniel Rosenberg wrote:
> On Sun, Nov 26, 2023 at 5:42 PM Chao Yu <chao@kernel.org> wrote:
>>
>> Hi Daniel,
>>
>> How about this? It be more explicit to indicate the logic?
>>
>> ---
>>    fsck/mount.c      | 2 ++
>>    include/f2fs_fs.h | 3 +++
>>    lib/libf2fs_io.c  | 4 ++++
>>    3 files changed, 9 insertions(+)
>>
>> diff --git a/fsck/mount.c b/fsck/mount.c
>> index 72516f4..4dfb996 100644
>> --- a/fsck/mount.c
>> +++ b/fsck/mount.c
>> @@ -1238,6 +1238,8 @@ int init_sb_info(struct f2fs_sb_info *sbi)
>>          MSG(0, "Info: total FS sectors = %"PRIu64" (%"PRIu64" MB)\n",
>>                                  total_sectors, total_sectors >>
>>                                                  (20 - get_sb(log_sectorsize)));
>> +
>> +       c.cache_config.blksize_initialized = true;
>>          return 0;
>>    }
>>
>> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
>> index 6df2e73..e377d48 100644
>> --- a/include/f2fs_fs.h
>> +++ b/include/f2fs_fs.h
>> @@ -460,6 +460,9 @@ typedef struct {
>>          unsigned max_hash_collision;
>>
>>          bool dbg_en;
>> +
>> +       /* whether blksize has been initialized */
>> +       bool blksize_initialized;
>>    } dev_cache_config_t;
>>
>>    /* f2fs_configration for compression used for sload.f2fs */
>> diff --git a/lib/libf2fs_io.c b/lib/libf2fs_io.c
>> index 39d3777..bb77418 100644
>> --- a/lib/libf2fs_io.c
>> +++ b/lib/libf2fs_io.c
>> @@ -199,6 +199,10 @@ void dcache_init(void)
>>    {
>>          long n;
>>
>> +       /* Must not initiate cache until block size is known */
>> +       if (!c.cache_config.blksize_initialized)
>> +               return;
>> +
>>          if (c.cache_config.num_cache_entry <= 0)
>>                  return;
>>
>> --
>> 2.40.1
>>
>>
> I think that works too. I was initially going to go with code like
> that, but I was unsure if read/writes that aren't in the fsck path
> used caching as well. There's a few uses of those in the mkfs/ paths,
> but I don't think there's anything that sets
> cache_config.num_cache_entry there. I was worried about missing code
> paths like that, so I had restricted the changes to where I knew we
> were reading the superblock off disk. If there is a problem, it could
> be covered by setting c.cache_config.blksize_initialized = true within
> the mkfs code after the blocksize is set in f2fs_parse_options. But
> that isn't needed if there's no caching in mkfs.

Oh, I think the most useful case of dcache is fsck, not sure about resize,
but I don't think it will be benefit to mkfs/sload, based on IO pattern of
these cases.

IMO, we can get rid of dcache for mkfs/sload cases in order to avoid
maintaining complicated design/logic.

Thanks,


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

[f2fs-dev] [PATCH 3/3] f2fs-tools: Fix dqb_curspace to reflect blocksize

[Daniel Rosenberg via Linux-f2fs-devel]

The initial sizes for dqblk.dqb_curspace should reflect the block size,
as that's the minimal filesize.

Signed-off-by: Daniel Rosenberg <drosen@google.com>
---
 mkfs/f2fs_format.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index f2840c8..b46bc15 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -1362,9 +1362,9 @@ static int f2fs_write_default_quota(int qtype, __le32 raw_id)
 	dqblk.dqb_bhardlimit = cpu_to_le64(0);
 	dqblk.dqb_bsoftlimit = cpu_to_le64(0);
 	if (c.lpf_ino)
-		dqblk.dqb_curspace = cpu_to_le64(8192);
+		dqblk.dqb_curspace = cpu_to_le64(F2FS_BLKSIZE * 2);
 	else
-		dqblk.dqb_curspace = cpu_to_le64(4096);
+		dqblk.dqb_curspace = cpu_to_le64(F2FS_BLKSIZE);
 	dqblk.dqb_btime = cpu_to_le64(0);
 	dqblk.dqb_itime = cpu_to_le64(0);
 
-- 
2.43.0.rc0.421.g78406f8d94-goog



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

Re: [f2fs-dev] [PATCH 3/3] f2fs-tools: Fix dqb_curspace to reflect blocksize

[Chao Yu]

On 2023/11/18 10:03, Daniel Rosenberg wrote:
> The initial sizes for dqblk.dqb_curspace should reflect the block size,
> as that's the minimal filesize.
> 
> Signed-off-by: Daniel Rosenberg <drosen@google.com>

Reviewed-by: Chao Yu <chao@kernel.org>

Thanks,


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