diff mbox series

f2fs-tools: allocate logs after conventional area for HM zoned devices

Message ID 20240117230032.2312067-1-daeho43@gmail.com
State New
Headers
Series f2fs-tools: allocate logs after conventional area for HM zoned devices |

Commit Message

Daeho Jeong Jan. 17, 2024, 11 p.m. UTC 
  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(-)

Comments

Chao Yu Jan. 22, 2024, 6:40 a.m. UTC | #1 
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,
Chao Yu Jan. 25, 2024, 3:34 a.m. UTC | #2 
+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);
Daeho Jeong Jan. 25, 2024, 4:25 p.m. UTC | #3 
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);
Chao Yu Jan. 26, 2024, 1:27 a.m. UTC | #4 
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);
Daeho Jeong Jan. 26, 2024, 6:17 p.m. UTC | #5 
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);
Chao Yu Jan. 29, 2024, 1:27 a.m. UTC | #6 
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);
Daeho Jeong Jan. 29, 2024, 3:47 p.m. UTC | #7 
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);
Yongpeng Yang Jan. 30, 2024, 2:17 a.m. UTC | #8 
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);
Chao Yu Jan. 30, 2024, 2:29 p.m. UTC | #9 
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);
Yongpeng Yang Jan. 31, 2024, 2:14 a.m. UTC | #10 
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 mbox series

Patch

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);