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

[Chao Yu <chao@kernel.org>]

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