| Message ID | 20240117230032.2312067-1-daeho43@gmail.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [f2fs-dev] f2fs-tools: allocate logs after conventional area for HM zoned devices | expand |
On 2024/1/18 7:00, Daeho Jeong wrote: > From: Daeho Jeong <daehojeong@google.com> > > Make to allocate logs after conventional area for HM zoned devices to > spare them for file pinning support. > > Signed-off-by: Daeho Jeong <daehojeong@google.com> Reviewed-by: Chao Yu <chao@kernel.org> Thanks,
+Cc Yongpeng Yang Daeho, Yongpeng reports a potential issue: if c.devices[0].total_segments is larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed end boundary of mainarea. Could you please check that? though it's a corner case. On 2024/1/18 7:00, Daeho Jeong wrote: > From: Daeho Jeong <daehojeong@google.com> > > Make to allocate logs after conventional area for HM zoned devices to > spare them for file pinning support. > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > --- > mkfs/f2fs_format.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c > index f2840c8..91a7f4b 100644 > --- a/mkfs/f2fs_format.c > +++ b/mkfs/f2fs_format.c > @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) > c.cur_seg[CURSEG_COLD_DATA] = 0; > c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); > } else if (c.zoned_mode) { > - c.cur_seg[CURSEG_HOT_NODE] = 0; > + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > + c.devices[0].total_segments : 0; > c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); > c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); > c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: > > +Cc Yongpeng Yang > > Daeho, > > Yongpeng reports a potential issue: if c.devices[0].total_segments is > larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed > end boundary of mainarea. Could you please check that? though it's a corner > case. Can you elaborate more? In the case of F2FS_ZONED_HM, we have the devices[1]. Do you mean the case we format the filesystem intentionally smaller than what devices have? > > On 2024/1/18 7:00, Daeho Jeong wrote: > > From: Daeho Jeong <daehojeong@google.com> > > > > Make to allocate logs after conventional area for HM zoned devices to > > spare them for file pinning support. > > > > Signed-off-by: Daeho Jeong <daehojeong@google.com> > > --- > > mkfs/f2fs_format.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c > > index f2840c8..91a7f4b 100644 > > --- a/mkfs/f2fs_format.c > > +++ b/mkfs/f2fs_format.c > > @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) > > c.cur_seg[CURSEG_COLD_DATA] = 0; > > c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); > > } else if (c.zoned_mode) { > > - c.cur_seg[CURSEG_HOT_NODE] = 0; > > + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > > + c.devices[0].total_segments : 0; > > c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); > > c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); > > c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On 2024/1/26 0:25, Daeho Jeong wrote: > On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: >> >> +Cc Yongpeng Yang >> >> Daeho, >> >> Yongpeng reports a potential issue: if c.devices[0].total_segments is >> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed >> end boundary of mainarea. Could you please check that? though it's a corner >> case. > > Can you elaborate more? Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, and c.devices[0].total_segments is larger than segments of mainare, c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? c.devices[0].total_segments : 0; > In the case of F2FS_ZONED_HM, we have the devices[1]. > Do you mean the case we format the filesystem intentionally smaller > than what devices have? I mean blew case: device[0]: conventional device size = 10240 MB device[1]: zone device size = 2 MB Thanks, > >> >> On 2024/1/18 7:00, Daeho Jeong wrote: >>> From: Daeho Jeong <daehojeong@google.com> >>> >>> Make to allocate logs after conventional area for HM zoned devices to >>> spare them for file pinning support. >>> >>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>> --- >>> mkfs/f2fs_format.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c >>> index f2840c8..91a7f4b 100644 >>> --- a/mkfs/f2fs_format.c >>> +++ b/mkfs/f2fs_format.c >>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) >>> c.cur_seg[CURSEG_COLD_DATA] = 0; >>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); >>> } else if (c.zoned_mode) { >>> - c.cur_seg[CURSEG_HOT_NODE] = 0; >>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>> + c.devices[0].total_segments : 0; >>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); >>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); >>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On Thu, Jan 25, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/1/26 0:25, Daeho Jeong wrote: > > On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: > >> > >> +Cc Yongpeng Yang > >> > >> Daeho, > >> > >> Yongpeng reports a potential issue: if c.devices[0].total_segments is > >> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed > >> end boundary of mainarea. Could you please check that? though it's a corner > >> case. > > > > Can you elaborate more? > > Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. Oh, Got it. Then, how about this? c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? (c.devices[1].start_blkaddr - get_sb(main_blkaddr)) / c.blks_per_seg : 0; > If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, > and c.devices[0].total_segments is larger than segments of mainare, > c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. > > c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > c.devices[0].total_segments : 0; > > > In the case of F2FS_ZONED_HM, we have the devices[1]. > > Do you mean the case we format the filesystem intentionally smaller > > than what devices have? > > I mean blew case: > device[0]: conventional device size = 10240 MB > device[1]: zone device size = 2 MB > > Thanks, > > > > >> > >> On 2024/1/18 7:00, Daeho Jeong wrote: > >>> From: Daeho Jeong <daehojeong@google.com> > >>> > >>> Make to allocate logs after conventional area for HM zoned devices to > >>> spare them for file pinning support. > >>> > >>> Signed-off-by: Daeho Jeong <daehojeong@google.com> > >>> --- > >>> mkfs/f2fs_format.c | 3 ++- > >>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>> > >>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c > >>> index f2840c8..91a7f4b 100644 > >>> --- a/mkfs/f2fs_format.c > >>> +++ b/mkfs/f2fs_format.c > >>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) > >>> c.cur_seg[CURSEG_COLD_DATA] = 0; > >>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); > >>> } else if (c.zoned_mode) { > >>> - c.cur_seg[CURSEG_HOT_NODE] = 0; > >>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > >>> + c.devices[0].total_segments : 0; > >>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); > >>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); > >>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On 2024/1/27 2:17, Daeho Jeong wrote: > On Thu, Jan 25, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >> >> On 2024/1/26 0:25, Daeho Jeong wrote: >>> On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: >>>> >>>> +Cc Yongpeng Yang >>>> >>>> Daeho, >>>> >>>> Yongpeng reports a potential issue: if c.devices[0].total_segments is >>>> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed >>>> end boundary of mainarea. Could you please check that? though it's a corner >>>> case. >>> >>> Can you elaborate more? >> >> Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. > > Oh, Got it. > Then, how about this? > > c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > (c.devices[1].start_blkaddr - > get_sb(main_blkaddr)) / c.blks_per_seg : 0; Better, but log header should align to start blkaddr of zone? Thanks, > >> If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, >> and c.devices[0].total_segments is larger than segments of mainare, >> c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. >> >> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >> c.devices[0].total_segments : 0; >> >>> In the case of F2FS_ZONED_HM, we have the devices[1]. >>> Do you mean the case we format the filesystem intentionally smaller >>> than what devices have? >> >> I mean blew case: >> device[0]: conventional device size = 10240 MB >> device[1]: zone device size = 2 MB >> >> Thanks, >> >>> >>>> >>>> On 2024/1/18 7:00, Daeho Jeong wrote: >>>>> From: Daeho Jeong <daehojeong@google.com> >>>>> >>>>> Make to allocate logs after conventional area for HM zoned devices to >>>>> spare them for file pinning support. >>>>> >>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>>>> --- >>>>> mkfs/f2fs_format.c | 3 ++- >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c >>>>> index f2840c8..91a7f4b 100644 >>>>> --- a/mkfs/f2fs_format.c >>>>> +++ b/mkfs/f2fs_format.c >>>>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) >>>>> c.cur_seg[CURSEG_COLD_DATA] = 0; >>>>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); >>>>> } else if (c.zoned_mode) { >>>>> - c.cur_seg[CURSEG_HOT_NODE] = 0; >>>>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>>> + c.devices[0].total_segments : 0; >>>>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); >>>>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); >>>>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On Sun, Jan 28, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: > > On 2024/1/27 2:17, Daeho Jeong wrote: > > On Thu, Jan 25, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: > >> > >> On 2024/1/26 0:25, Daeho Jeong wrote: > >>> On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: > >>>> > >>>> +Cc Yongpeng Yang > >>>> > >>>> Daeho, > >>>> > >>>> Yongpeng reports a potential issue: if c.devices[0].total_segments is > >>>> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed > >>>> end boundary of mainarea. Could you please check that? though it's a corner > >>>> case. > >>> > >>> Can you elaborate more? > >> > >> Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. > > > > Oh, Got it. > > Then, how about this? > > > > c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > > (c.devices[1].start_blkaddr - > > get_sb(main_blkaddr)) / c.blks_per_seg : 0; > > Better, but log header should align to start blkaddr of zone? It's already aligned here. if (c.zoned_mode && c.ndevs > 1) zone_align_start_offset += (c.devices[0].total_sectors * c.sector_size) % zone_size_bytes; ... for (i = 0; i < c.ndevs; i++) { if (i == 0) { c.devices[i].total_segments = (c.devices[i].total_sectors * c.sector_size - zone_align_start_offset) / segment_size_bytes; c.devices[i].start_blkaddr = 0; c.devices[i].end_blkaddr = c.devices[i].total_segments * c.blks_per_seg - 1 + sb->segment0_blkaddr; } else { c.devices[i].total_segments = c.devices[i].total_sectors / (c.sectors_per_blk * c.blks_per_seg); c.devices[i].start_blkaddr = c.devices[i - 1].end_blkaddr + 1; ... total_meta_zones = ZONE_ALIGN(total_meta_segments * c.blks_per_seg); set_sb(main_blkaddr, get_sb(segment0_blkaddr) + total_meta_zones * c.segs_per_zone * c.blks_per_seg); > > Thanks, > > > > >> If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, > >> and c.devices[0].total_segments is larger than segments of mainare, > >> c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. > >> > >> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > >> c.devices[0].total_segments : 0; > >> > >>> In the case of F2FS_ZONED_HM, we have the devices[1]. > >>> Do you mean the case we format the filesystem intentionally smaller > >>> than what devices have? > >> > >> I mean blew case: > >> device[0]: conventional device size = 10240 MB > >> device[1]: zone device size = 2 MB > >> > >> Thanks, > >> > >>> > >>>> > >>>> On 2024/1/18 7:00, Daeho Jeong wrote: > >>>>> From: Daeho Jeong <daehojeong@google.com> > >>>>> > >>>>> Make to allocate logs after conventional area for HM zoned devices to > >>>>> spare them for file pinning support. > >>>>> > >>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> > >>>>> --- > >>>>> mkfs/f2fs_format.c | 3 ++- > >>>>> 1 file changed, 2 insertions(+), 1 deletion(-) > >>>>> > >>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c > >>>>> index f2840c8..91a7f4b 100644 > >>>>> --- a/mkfs/f2fs_format.c > >>>>> +++ b/mkfs/f2fs_format.c > >>>>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) > >>>>> c.cur_seg[CURSEG_COLD_DATA] = 0; > >>>>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); > >>>>> } else if (c.zoned_mode) { > >>>>> - c.cur_seg[CURSEG_HOT_NODE] = 0; > >>>>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? > >>>>> + c.devices[0].total_segments : 0; > >>>>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); > >>>>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); > >>>>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
cur_seg[CURSEG_COLD_DATA] will exceed end boundary of main area when: device[1]: zone device size = [2 MB ~ 10MB] So, if there are not enough seq zones for six cursegs, we should still assign 0 to c.cur_seg[CURSEG_HOT_NODE] or reserve several conv zones for cursegs. On 1/29/2024 11:47 PM, Daeho Jeong wrote: > On Sun, Jan 28, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >> >> On 2024/1/27 2:17, Daeho Jeong wrote: >>> On Thu, Jan 25, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >>>> >>>> On 2024/1/26 0:25, Daeho Jeong wrote: >>>>> On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: >>>>>> >>>>>> +Cc Yongpeng Yang >>>>>> >>>>>> Daeho, >>>>>> >>>>>> Yongpeng reports a potential issue: if c.devices[0].total_segments is >>>>>> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed >>>>>> end boundary of mainarea. Could you please check that? though it's a corner >>>>>> case. >>>>> >>>>> Can you elaborate more? >>>> >>>> Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. >>> >>> Oh, Got it. >>> Then, how about this? >>> >>> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>> (c.devices[1].start_blkaddr - >>> get_sb(main_blkaddr)) / c.blks_per_seg : 0; >> >> Better, but log header should align to start blkaddr of zone? > > It's already aligned here. > > if (c.zoned_mode && c.ndevs > 1) > zone_align_start_offset += > (c.devices[0].total_sectors * c.sector_size) % > zone_size_bytes; > > ... > > for (i = 0; i < c.ndevs; i++) { > if (i == 0) { > c.devices[i].total_segments = > (c.devices[i].total_sectors * > c.sector_size - zone_align_start_offset) / > segment_size_bytes; > c.devices[i].start_blkaddr = 0; > c.devices[i].end_blkaddr = c.devices[i].total_segments * > c.blks_per_seg - 1 + > sb->segment0_blkaddr; > } else { > c.devices[i].total_segments = > c.devices[i].total_sectors / > (c.sectors_per_blk * c.blks_per_seg); > c.devices[i].start_blkaddr = > c.devices[i - 1].end_blkaddr + 1; > > ... > > total_meta_zones = ZONE_ALIGN(total_meta_segments * > c.blks_per_seg); > > set_sb(main_blkaddr, get_sb(segment0_blkaddr) + total_meta_zones * > c.segs_per_zone * c.blks_per_seg); > >> >> Thanks, >> >>> >>>> If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, >>>> and c.devices[0].total_segments is larger than segments of mainare, >>>> c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. >>>> >>>> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>> c.devices[0].total_segments : 0; >>>> >>>>> In the case of F2FS_ZONED_HM, we have the devices[1]. >>>>> Do you mean the case we format the filesystem intentionally smaller >>>>> than what devices have? >>>> >>>> I mean blew case: >>>> device[0]: conventional device size = 10240 MB >>>> device[1]: zone device size = 2 MB >>>> >>>> Thanks, >>>> >>>>> >>>>>> >>>>>> On 2024/1/18 7:00, Daeho Jeong wrote: >>>>>>> From: Daeho Jeong <daehojeong@google.com> >>>>>>> >>>>>>> Make to allocate logs after conventional area for HM zoned devices to >>>>>>> spare them for file pinning support. >>>>>>> >>>>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>>>>>> --- >>>>>>> mkfs/f2fs_format.c | 3 ++- >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c >>>>>>> index f2840c8..91a7f4b 100644 >>>>>>> --- a/mkfs/f2fs_format.c >>>>>>> +++ b/mkfs/f2fs_format.c >>>>>>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) >>>>>>> c.cur_seg[CURSEG_COLD_DATA] = 0; >>>>>>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); >>>>>>> } else if (c.zoned_mode) { >>>>>>> - c.cur_seg[CURSEG_HOT_NODE] = 0; >>>>>>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>>>>> + c.devices[0].total_segments : 0; >>>>>>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); >>>>>>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); >>>>>>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On 2024/1/29 23:47, Daeho Jeong wrote: > On Sun, Jan 28, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >> >> On 2024/1/27 2:17, Daeho Jeong wrote: >>> On Thu, Jan 25, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >>>> >>>> On 2024/1/26 0:25, Daeho Jeong wrote: >>>>> On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: >>>>>> >>>>>> +Cc Yongpeng Yang >>>>>> >>>>>> Daeho, >>>>>> >>>>>> Yongpeng reports a potential issue: if c.devices[0].total_segments is >>>>>> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed >>>>>> end boundary of mainarea. Could you please check that? though it's a corner >>>>>> case. >>>>> >>>>> Can you elaborate more? >>>> >>>> Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. >>> >>> Oh, Got it. >>> Then, how about this? >>> >>> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>> (c.devices[1].start_blkaddr - >>> get_sb(main_blkaddr)) / c.blks_per_seg : 0; >> >> Better, but log header should align to start blkaddr of zone? > > It's already aligned here. Ah, thanks for the explanation. :) Thanks, > > if (c.zoned_mode && c.ndevs > 1) > zone_align_start_offset += > (c.devices[0].total_sectors * c.sector_size) % > zone_size_bytes; > > ... > > for (i = 0; i < c.ndevs; i++) { > if (i == 0) { > c.devices[i].total_segments = > (c.devices[i].total_sectors * > c.sector_size - zone_align_start_offset) / > segment_size_bytes; > c.devices[i].start_blkaddr = 0; > c.devices[i].end_blkaddr = c.devices[i].total_segments * > c.blks_per_seg - 1 + > sb->segment0_blkaddr; > } else { > c.devices[i].total_segments = > c.devices[i].total_sectors / > (c.sectors_per_blk * c.blks_per_seg); > c.devices[i].start_blkaddr = > c.devices[i - 1].end_blkaddr + 1; > > ... > > total_meta_zones = ZONE_ALIGN(total_meta_segments * > c.blks_per_seg); > > set_sb(main_blkaddr, get_sb(segment0_blkaddr) + total_meta_zones * > c.segs_per_zone * c.blks_per_seg); > >> >> Thanks, >> >>> >>>> If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, >>>> and c.devices[0].total_segments is larger than segments of mainare, >>>> c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. >>>> >>>> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>> c.devices[0].total_segments : 0; >>>> >>>>> In the case of F2FS_ZONED_HM, we have the devices[1]. >>>>> Do you mean the case we format the filesystem intentionally smaller >>>>> than what devices have? >>>> >>>> I mean blew case: >>>> device[0]: conventional device size = 10240 MB >>>> device[1]: zone device size = 2 MB >>>> >>>> Thanks, >>>> >>>>> >>>>>> >>>>>> On 2024/1/18 7:00, Daeho Jeong wrote: >>>>>>> From: Daeho Jeong <daehojeong@google.com> >>>>>>> >>>>>>> Make to allocate logs after conventional area for HM zoned devices to >>>>>>> spare them for file pinning support. >>>>>>> >>>>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>>>>>> --- >>>>>>> mkfs/f2fs_format.c | 3 ++- >>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>> >>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c >>>>>>> index f2840c8..91a7f4b 100644 >>>>>>> --- a/mkfs/f2fs_format.c >>>>>>> +++ b/mkfs/f2fs_format.c >>>>>>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) >>>>>>> c.cur_seg[CURSEG_COLD_DATA] = 0; >>>>>>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); >>>>>>> } else if (c.zoned_mode) { >>>>>>> - c.cur_seg[CURSEG_HOT_NODE] = 0; >>>>>>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>>>>> + c.devices[0].total_segments : 0; >>>>>>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); >>>>>>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); >>>>>>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
On 1/31/2024 2:00 AM, Daeho Jeong wrote: > On Mon, Jan 29, 2024 at 6:17 PM Yongpeng Yang <yangyongpeng1@oppo.com> wrote: >> >> cur_seg[CURSEG_COLD_DATA] will exceed end boundary of main area when: >> device[1]: zone device size = [2 MB ~ 10MB] >> >> So, if there are not enough seq zones for six cursegs, we should still >> assign 0 to c.cur_seg[CURSEG_HOT_NODE] or reserve several conv zones for >> cursegs. > > To tackle that case, how about this? > > } else if (c.zoned_mode) { > c.cur_seg[CURSEG_HOT_NODE] = 0; > if (c.zoned_model == F2FS_ZONED_HM) { > uint32_t conv_zones = > c.devices[0].total_segments / c.segs_per_zone > - total_meta_zones; > > if (total_zones - conv_zones >= avail_zones) > c.cur_seg[CURSEG_HOT_NODE] = > (c.devices[1].start_blkaddr - > get_sb(main_blkaddr)) / c.blks_per_seg; > } > It seems fine, thanks. >> >> On 1/29/2024 11:47 PM, Daeho Jeong wrote: >>> On Sun, Jan 28, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >>>> >>>> On 2024/1/27 2:17, Daeho Jeong wrote: >>>>> On Thu, Jan 25, 2024 at 5:27 PM Chao Yu <chao@kernel.org> wrote: >>>>>> >>>>>> On 2024/1/26 0:25, Daeho Jeong wrote: >>>>>>> On Wed, Jan 24, 2024 at 7:34 PM Chao Yu <chao@kernel.org> wrote: >>>>>>>> >>>>>>>> +Cc Yongpeng Yang >>>>>>>> >>>>>>>> Daeho, >>>>>>>> >>>>>>>> Yongpeng reports a potential issue: if c.devices[0].total_segments is >>>>>>>> larger than segments of mainarea, c.cur_seg[CURSEG_HOT_NODE] will exceed >>>>>>>> end boundary of mainarea. Could you please check that? though it's a corner >>>>>>>> case. >>>>>>> >>>>>>> Can you elaborate more? >>>>>> >>>>>> Since c.cur_seg[CURSEG_HOT_NODE] is an offset started from main_blkaddr. >>>>> >>>>> Oh, Got it. >>>>> Then, how about this? >>>>> >>>>> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>>> (c.devices[1].start_blkaddr - >>>>> get_sb(main_blkaddr)) / c.blks_per_seg : 0; >>>> >>>> Better, but log header should align to start blkaddr of zone? >>> >>> It's already aligned here. >>> >>> if (c.zoned_mode && c.ndevs > 1) >>> zone_align_start_offset += >>> (c.devices[0].total_sectors * c.sector_size) % >>> zone_size_bytes; >>> >>> ... >>> >>> for (i = 0; i < c.ndevs; i++) { >>> if (i == 0) { >>> c.devices[i].total_segments = >>> (c.devices[i].total_sectors * >>> c.sector_size - zone_align_start_offset) / >>> segment_size_bytes; >>> c.devices[i].start_blkaddr = 0; >>> c.devices[i].end_blkaddr = c.devices[i].total_segments * >>> c.blks_per_seg - 1 + >>> sb->segment0_blkaddr; >>> } else { >>> c.devices[i].total_segments = >>> c.devices[i].total_sectors / >>> (c.sectors_per_blk * c.blks_per_seg); >>> c.devices[i].start_blkaddr = >>> c.devices[i - 1].end_blkaddr + 1; >>> >>> ... >>> >>> total_meta_zones = ZONE_ALIGN(total_meta_segments * >>> c.blks_per_seg); >>> >>> set_sb(main_blkaddr, get_sb(segment0_blkaddr) + total_meta_zones * >>> c.segs_per_zone * c.blks_per_seg); >>> >>>> >>>> Thanks, >>>> >>>>> >>>>>> If c.cur_seg[CURSEG_HOT_NODE] was assigned w/ c.devices[0].total_segments, >>>>>> and c.devices[0].total_segments is larger than segments of mainare, >>>>>> c.cur_seg[CURSEG_HOT_NODE] will exceed the end boundary of mainarea. >>>>>> >>>>>> c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>>>> c.devices[0].total_segments : 0; >>>>>> >>>>>>> In the case of F2FS_ZONED_HM, we have the devices[1]. >>>>>>> Do you mean the case we format the filesystem intentionally smaller >>>>>>> than what devices have? >>>>>> >>>>>> I mean blew case: >>>>>> device[0]: conventional device size = 10240 MB >>>>>> device[1]: zone device size = 2 MB >>>>>> >>>>>> Thanks, >>>>>> >>>>>>> >>>>>>>> >>>>>>>> On 2024/1/18 7:00, Daeho Jeong wrote: >>>>>>>>> From: Daeho Jeong <daehojeong@google.com> >>>>>>>>> >>>>>>>>> Make to allocate logs after conventional area for HM zoned devices to >>>>>>>>> spare them for file pinning support. >>>>>>>>> >>>>>>>>> Signed-off-by: Daeho Jeong <daehojeong@google.com> >>>>>>>>> --- >>>>>>>>> mkfs/f2fs_format.c | 3 ++- >>>>>>>>> 1 file changed, 2 insertions(+), 1 deletion(-) >>>>>>>>> >>>>>>>>> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c >>>>>>>>> index f2840c8..91a7f4b 100644 >>>>>>>>> --- a/mkfs/f2fs_format.c >>>>>>>>> +++ b/mkfs/f2fs_format.c >>>>>>>>> @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) >>>>>>>>> c.cur_seg[CURSEG_COLD_DATA] = 0; >>>>>>>>> c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); >>>>>>>>> } else if (c.zoned_mode) { >>>>>>>>> - c.cur_seg[CURSEG_HOT_NODE] = 0; >>>>>>>>> + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? >>>>>>>>> + c.devices[0].total_segments : 0; >>>>>>>>> c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); >>>>>>>>> c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); >>>>>>>>> c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c index f2840c8..91a7f4b 100644 --- a/mkfs/f2fs_format.c +++ b/mkfs/f2fs_format.c @@ -557,7 +557,8 @@ static int f2fs_prepare_super_block(void) c.cur_seg[CURSEG_COLD_DATA] = 0; c.cur_seg[CURSEG_WARM_DATA] = next_zone(CURSEG_COLD_DATA); } else if (c.zoned_mode) { - c.cur_seg[CURSEG_HOT_NODE] = 0; + c.cur_seg[CURSEG_HOT_NODE] = c.zoned_model == F2FS_ZONED_HM ? + c.devices[0].total_segments : 0; c.cur_seg[CURSEG_WARM_NODE] = next_zone(CURSEG_HOT_NODE); c.cur_seg[CURSEG_COLD_NODE] = next_zone(CURSEG_WARM_NODE); c.cur_seg[CURSEG_HOT_DATA] = next_zone(CURSEG_COLD_NODE);