Re: [f2fs-dev] [PATCH] f2fs: fix to use correct segment type in f2fs_allocate_data_block()
On 2024/2/24 4:52, Jaegeuk Kim wrote:
On 02/23, Chao Yu wrote:
@type in f2fs_allocate_data_block() indicates log header's type, it
can be CURSEG_COLD_DATA_PINNED or CURSEG_ALL_DATA_ATGC, rather than
type of data/node, however IS_DATASEG()/IS_NODESEG() only accept later
one, fix it.
Fixes: 093749e296e2 ("f2fs: support age threshold based garbage collection")
Signed-off-by: Chao Yu
---
fs/f2fs/segment.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d0209ea77dd2..76422f50e6cc 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -3505,12 +3505,12 @@ void f2fs_allocate_data_block(struct f2fs_sb_info *sbi,
struct page *page,
locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr));
- if (IS_DATASEG(type))
+ if (IS_DATASEG(se->type))
We have se only when type is CURSEG_ALL_DATA_ATGC. We may need to change
Oops, correct.
IS_DATASEG()?
Sure, I guess one other way is to use curseg->seg_type, let me know if
you prefer to change IS_DATASEG().
Thanks,
atomic64_inc(&sbi->allocated_data_blocks);
up_write(&sit_i->sentry_lock);
- if (page && IS_NODESEG(type)) {
+ if (page && IS_NODESEG(se->type)) {
fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
f2fs_inode_chksum_set(sbi, page);
--
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 v2] mkfs.f2fs: should give section-aligned reserved segments
The reserved segments should be aligned to the section boundary.
Signed-off-by: Jaegeuk Kim
---
v2:
- fix bug
include/f2fs_fs.h | 12 +++-
1 file changed, 7 insertions(+), 5 deletions(-)
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 9056e02acd29..fc56396fa358 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -1760,25 +1760,27 @@ extern uint32_t f2fs_get_usable_segments(struct
f2fs_super_block *sb);
#define ZONE_ALIGN(blks) SIZE_ALIGN(blks, c.blks_per_seg * \
c.segs_per_zone)
-static inline double get_reserved(struct f2fs_super_block *sb, double ovp)
+static inline uint32_t get_reserved(struct f2fs_super_block *sb, double ovp)
{
- double reserved;
uint32_t usable_main_segs = f2fs_get_usable_segments(sb);
uint32_t segs_per_sec = round_up(usable_main_segs,
get_sb(section_count));
+ uint32_t reserved;
if (c.conf_reserved_sections)
reserved = c.conf_reserved_sections * segs_per_sec;
else
reserved = (100 / ovp + 1 + NR_CURSEG_TYPE) * segs_per_sec;
- return reserved;
+ /* Let's keep the section alignment */
+ return round_up(reserved, segs_per_sec) * segs_per_sec;
}
static inline double get_best_overprovision(struct f2fs_super_block *sb)
{
- double reserved, ovp, candidate, end, diff, space;
+ double ovp, candidate, end, diff, space;
double max_ovp = 0, max_space = 0;
uint32_t usable_main_segs = f2fs_get_usable_segments(sb);
+ uint32_t reserved;
if (get_sb(segment_count_main) < 256) {
candidate = 10;
@@ -1795,7 +1797,7 @@ static inline double get_best_overprovision(struct
f2fs_super_block *sb)
ovp = (usable_main_segs - reserved) * candidate / 100;
if (ovp < 0)
continue;
- space = usable_main_segs - max(reserved, ovp) -
+ space = usable_main_segs - max((double)reserved, ovp) -
2 * get_sb(segs_per_sec);
if (max_space < space) {
max_space = space;
--
2.44.0.rc0.258.g7320e95886-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH] mkfs.f2fs: should give section-aligned reserved segments
The reserved segments should be aligned to the section boundary. Signed-off-by: Jaegeuk Kim --- include/f2fs_fs.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h index 9056e02acd29..2e93503cada9 100644 --- a/include/f2fs_fs.h +++ b/include/f2fs_fs.h @@ -1771,7 +1771,8 @@ static inline double get_reserved(struct f2fs_super_block *sb, double ovp) else reserved = (100 / ovp + 1 + NR_CURSEG_TYPE) * segs_per_sec; - return reserved; + /* Let's keep the section alignment */ + return round_up(reserved, segs_per_sec); } static inline double get_best_overprovision(struct f2fs_super_block *sb) -- 2.44.0.rc0.258.g7320e95886-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 5/5] f2fs: allow to mount if cap is 100
Don't block mounting the partition, if cap is 100%.
Signed-off-by: Jaegeuk Kim
---
fs/f2fs/segment.c | 3 +++
1 file changed, 3 insertions(+)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index 6d586ae8b55f..f11361152d2a 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -904,6 +904,9 @@ int f2fs_disable_cp_again(struct f2fs_sb_info *sbi, block_t
unusable)
{
int ovp_hole_segs =
(overprovision_segments(sbi) - reserved_segments(sbi));
+
+ if (F2FS_OPTION(sbi).unusable_cap_perc == 100)
+ return 0;
if (unusable > F2FS_OPTION(sbi).unusable_cap)
return -EAGAIN;
if (is_sbi_flag_set(sbi, SBI_CP_DISABLED_QUICK) &&
--
2.44.0.rc0.258.g7320e95886-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 4/5] f2fs: prevent an f2fs_gc loop during disable_checkpoint
Don't get stuck in the f2fs_gc loop while disabling checkpoint. Instead, we have a time-based management. Signed-off-by: Jaegeuk Kim --- fs/f2fs/super.c | 1 + 1 file changed, 1 insertion(+) diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c index fc7f1a9fbbda..7d9b92978709 100644 --- a/fs/f2fs/super.c +++ b/fs/f2fs/super.c @@ -2191,6 +2191,7 @@ static int f2fs_disable_checkpoint(struct f2fs_sb_info *sbi) .init_gc_type = FG_GC, .should_migrate_blocks = false, .err_gc_skipped = true, + .no_bg_gc = true, .nr_free_secs = 1 }; f2fs_down_write(&sbi->gc_lock); -- 2.44.0.rc0.258.g7320e95886-goog ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 2/5] f2fs: fix write pointers all the time
Even if the roll forward recovery stopped due to any error, we have to fix
the write pointers in order to mount the disk from the previous checkpoint.
Signed-off-by: Jaegeuk Kim
---
fs/f2fs/recovery.c | 2 +-
fs/f2fs/super.c| 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/f2fs/recovery.c b/fs/f2fs/recovery.c
index b3baec666afe..8bbecb5f9323 100644
--- a/fs/f2fs/recovery.c
+++ b/fs/f2fs/recovery.c
@@ -913,7 +913,7 @@ int f2fs_recover_fsync_data(struct f2fs_sb_info *sbi, bool
check_only)
* and the f2fs is not read only, check and fix zoned block devices'
* write pointer consistency.
*/
- if (!err && fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
+ if (fix_curseg_write_pointer && !f2fs_readonly(sbi->sb) &&
f2fs_sb_has_blkzoned(sbi)) {
err = f2fs_fix_curseg_write_pointer(sbi);
if (!err)
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 2e41142d07c0..4d03ce1109ad 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4673,7 +4673,7 @@ static int f2fs_fill_super(struct super_block *sb, void
*data, int silent)
* If the f2fs is not readonly and fsync data recovery succeeds,
* check zoned block devices' write pointer consistency.
*/
- if (!err && !f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
+ if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
err = f2fs_check_write_pointer(sbi);
if (err)
goto free_meta;
--
2.44.0.rc0.258.g7320e95886-goog
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
[f2fs-dev] [PATCH 3/5] f2fs: print zone status in string and some log
No functional change, but add some more logs.
Signed-off-by: Jaegeuk Kim
---
fs/f2fs/segment.c | 34 --
fs/f2fs/super.c | 1 +
2 files changed, 25 insertions(+), 10 deletions(-)
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index d4f228e6f771..6d586ae8b55f 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -4912,6 +4912,16 @@ static int sanity_check_curseg(struct f2fs_sb_info *sbi)
}
#ifdef CONFIG_BLK_DEV_ZONED
+const char *f2fs_zone_status[BLK_ZONE_COND_OFFLINE + 1] = {
+ [BLK_ZONE_COND_NOT_WP] = "NOT_WP",
+ [BLK_ZONE_COND_EMPTY] = "EMPTY",
+ [BLK_ZONE_COND_IMP_OPEN]= "IMPLICITE_OPEN",
+ [BLK_ZONE_COND_EXP_OPEN]= "EXPLICITE_OPEN",
+ [BLK_ZONE_COND_CLOSED] = "CLOSED",
+ [BLK_ZONE_COND_READONLY]= "READONLY",
+ [BLK_ZONE_COND_FULL]= "FULL",
+ [BLK_ZONE_COND_OFFLINE] = "OFFLINE",
+};
static int check_zone_write_pointer(struct f2fs_sb_info *sbi,
struct f2fs_dev_info *fdev,
@@ -4928,18 +4938,22 @@ static int check_zone_write_pointer(struct f2fs_sb_info
*sbi,
zone_block = fdev->start_blk + (zone->start >> log_sectors_per_block);
zone_segno = GET_SEGNO(sbi, zone_block);
+ /*
+* Get # of valid block of the zone.
+*/
+ valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
+
/*
* Skip check of zones cursegs point to, since
* fix_curseg_write_pointer() checks them.
*/
if (zone_segno >= MAIN_SEGS(sbi) ||
- IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno)))
+ IS_CURSEC(sbi, GET_SEC_FROM_SEG(sbi, zone_segno))) {
+ f2fs_notice(sbi, "Open zones: valid block[0x%x,0x%x] cond[%s]",
+ zone_segno, valid_block_cnt,
+ f2fs_zone_status[zone->cond]);
return 0;
-
- /*
-* Get # of valid block of the zone.
-*/
- valid_block_cnt = get_valid_blocks(sbi, zone_segno, true);
+ }
if ((!valid_block_cnt && zone->cond == BLK_ZONE_COND_EMPTY) ||
(valid_block_cnt && zone->cond == BLK_ZONE_COND_FULL))
@@ -4947,8 +4961,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info
*sbi,
if (!valid_block_cnt) {
f2fs_notice(sbi, "Zone without valid block has non-zero write "
- "pointer. Reset the write pointer: cond[0x%x]",
- zone->cond);
+ "pointer. Reset the write pointer: cond[%s]",
+ f2fs_zone_status[zone->cond]);
ret = __f2fs_issue_discard_zone(sbi, fdev->bdev, zone_block,
zone->len >> log_sectors_per_block);
if (ret)
@@ -4965,8 +4979,8 @@ static int check_zone_write_pointer(struct f2fs_sb_info
*sbi,
* selected for write operation until it get discarded.
*/
f2fs_notice(sbi, "Valid blocks are not aligned with write "
- "pointer: valid block[0x%x,0x%x] cond[0x%x]",
- zone_segno, valid_block_cnt, zone->cond);
+ "pointer: valid block[0x%x,0x%x] cond[%s]",
+ zone_segno, valid_block_cnt, f2fs_zone_status[zone->cond]);
ret = blkdev_zone_mgmt(fdev->bdev, REQ_OP_ZONE_FINISH,
zone->start, zone->len, GFP_NOFS);
diff --git a/fs/f2fs/super.c b/fs/f2fs/super.c
index 4d03ce1109ad..fc7f1a9fbbda 100644
--- a/fs/f2fs/super.c
+++ b/fs/f2fs/super.c
@@ -4674,6 +4674,7 @@ static int f2fs_fill_super(struct super_block *sb, void
*data, int silent)
* check zoned block devices' write pointer consistency.
*/
if (!f2fs_readonly(sb) && f2fs_sb_has_blkzoned(sbi)) {
+ f2fs_notice(sbi, "Checking entire write pointers");
err = f2fs_check_write_pointer(sbi);
if (err)
goto free_meta;
--
2.44.0.rc0.258.g7320e95886-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/5] f2fs: check number of blocks in a current section
In cfd66bb715fd ("f2fs: fix deadloop in foreground GC"), we needed to check
the number of blocks in a section instead of the segment.
In addtion, let's check the entire node sections when checking the avaiable
node block space. It does not match one to one per temperature, but currently
we don't have exact dirty page count per temperature. Hence, use a rough
estimation.
Fixes: cfd66bb715fd ("f2fs: fix deadloop in foreground GC")
Signed-off-by: Jaegeuk Kim
---
fs/f2fs/segment.h | 22 +++---
1 file changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/f2fs/segment.h b/fs/f2fs/segment.h
index 3edd3809b6b5..15bf5edd9b3c 100644
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -561,23 +561,23 @@ static inline bool has_curseg_enough_space(struct
f2fs_sb_info *sbi,
unsigned int node_blocks, unsigned int dent_blocks)
{
- unsigned int segno, left_blocks;
+ unsigned segno, left_blocks;
int i;
- /* check current node segment */
+ /* check current node sections, which counts very roughly */
+ left_blocks = 0;
for (i = CURSEG_HOT_NODE; i <= CURSEG_COLD_NODE; i++) {
segno = CURSEG_I(sbi, i)->segno;
- left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
- get_seg_entry(sbi, segno)->ckpt_valid_blocks;
-
- if (node_blocks > left_blocks)
- return false;
+ left_blocks += CAP_BLKS_PER_SEC(sbi) -
+ get_ckpt_valid_blocks(sbi, segno, true);
}
+ if (node_blocks > left_blocks)
+ return false;
- /* check current data segment */
+ /* check current data section for dentry blocks. */
segno = CURSEG_I(sbi, CURSEG_HOT_DATA)->segno;
- left_blocks = f2fs_usable_blks_in_seg(sbi, segno) -
- get_seg_entry(sbi, segno)->ckpt_valid_blocks;
+ left_blocks = CAP_BLKS_PER_SEC(sbi) -
+ get_ckpt_valid_blocks(sbi, segno, true);
if (dent_blocks > left_blocks)
return false;
return true;
@@ -626,7 +626,7 @@ static inline bool has_not_enough_free_secs(struct
f2fs_sb_info *sbi,
if (free_secs > upper_secs)
return false;
- else if (free_secs <= lower_secs)
+ if (free_secs <= lower_secs)
return true;
return !curseg_space;
}
--
2.44.0.rc0.258.g7320e95886-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] f2fs: fix to use correct segment type in f2fs_allocate_data_block()
On 02/23, Chao Yu wrote:
> @type in f2fs_allocate_data_block() indicates log header's type, it
> can be CURSEG_COLD_DATA_PINNED or CURSEG_ALL_DATA_ATGC, rather than
> type of data/node, however IS_DATASEG()/IS_NODESEG() only accept later
> one, fix it.
>
> Fixes: 093749e296e2 ("f2fs: support age threshold based garbage collection")
> Signed-off-by: Chao Yu
> ---
> fs/f2fs/segment.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index d0209ea77dd2..76422f50e6cc 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3505,12 +3505,12 @@ void f2fs_allocate_data_block(struct f2fs_sb_info
> *sbi, struct page *page,
> locate_dirty_segment(sbi, GET_SEGNO(sbi, old_blkaddr));
> locate_dirty_segment(sbi, GET_SEGNO(sbi, *new_blkaddr));
>
> - if (IS_DATASEG(type))
> + if (IS_DATASEG(se->type))
We have se only when type is CURSEG_ALL_DATA_ATGC. We may need to change
IS_DATASEG()?
> atomic64_inc(&sbi->allocated_data_blocks);
>
> up_write(&sit_i->sentry_lock);
>
> - if (page && IS_NODESEG(type)) {
> + if (page && IS_NODESEG(se->type)) {
> fill_node_footer_blkaddr(page, NEXT_FREE_BLKADDR(sbi, curseg));
>
> f2fs_inode_chksum_set(sbi, page);
> --
> 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] [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 13:46:53 -0500
Steven Rostedt wrote:
> Now one thing I could do is to not remove the parameter, but just add:
>
> WARN_ON_ONCE((src) != __data_offsets->item##_ptr_);
>
> in the __assign_str() macro to make sure that it's still the same that is
> assigned. But I'm not sure how useful that is, and still causes burden to
> have it. I never really liked the passing of the string in two places to
> begin with.
Hmm, maybe I'll just add this patch for 6.9 and then in 6.10 do the
parameter removal.
-- Steve
diff --git a/include/trace/stages/stage6_event_callback.h
b/include/trace/stages/stage6_event_callback.h
index 0c0f50bcdc56..7372e2c2a0c4 100644
--- a/include/trace/stages/stage6_event_callback.h
+++ b/include/trace/stages/stage6_event_callback.h
@@ -35,6 +35,7 @@ #define __assign_str(dst, src)
do {\
char *__str__ = __get_str(dst); \
int __len__ = __get_dynamic_array_len(dst) - 1; \
+ WARN_ON_ONCE((src) != __data_offsets.dst##_ptr_); \
memcpy(__str__, __data_offsets.dst##_ptr_ ? : \
EVENT_NULL_STR, __len__);\
__str__[__len__] = '\0';\
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, Feb 23, 2024 at 01:46:53PM -0500, Steven Rostedt wrote: > On Fri, 23 Feb 2024 10:30:45 -0800 > Jeff Johnson wrote: > > > On 2/23/2024 9:56 AM, Steven Rostedt wrote: > > > From: "Steven Rostedt (Google)" > > > > > > [ > > >This is a treewide change. I will likely re-create this patch again in > > >the second week of the merge window of v6.9 and submit it then. Hoping > > >to keep the conflicts that it will cause to a minimum. > > > ] > > > > > > With the rework of how the __string() handles dynamic strings where it > > > saves off the source string in field in the helper structure[1], the > > > assignment of that value to the trace event field is stored in the helper > > > value and does not need to be passed in again. > > > > Just curious if this could be done piecemeal by first changing the > > macros to be variadic macros which allows you to ignore the extra > > argument. The callers could then be modified in their separate trees. > > And then once all the callers have be merged, the macros could be > > changed to no longer be variadic. > > I weighed doing that, but I think ripping off the band-aid is a better > approach. One thing I found is that leaving unused parameters in the macros > can cause bugs itself. I found one case doing my clean up, where an unused > parameter in one of the macros was bogus, and when I made it a used > parameter, it broke the build. > > I think for tree-wide changes, the preferred approach is to do one big > patch at once. And since this only affects TRACE_EVENT() macros, it > hopefully would not be too much of a burden (although out of tree users may > suffer from this, but do we care?) Agreed on doing it all at once, it'll be way less spam for people to deal with. Tangentially related though, what would make me really happy is if we could create the string with in the TP__fast_assign() section. I have to have a bunch of annoying wrappers right now because the string length has to be known when we invoke the tracepoint. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 14:50:49 -0500
Kent Overstreet wrote:
> Tangentially related though, what would make me really happy is if we
> could create the string with in the TP__fast_assign() section. I have to
> have a bunch of annoying wrappers right now because the string length
> has to be known when we invoke the tracepoint.
You can use __string_len() to determine the string length in the tracepoint
(which is executed in the TP_fast_assign() section).
My clean up patches will make __assign_str_len() obsolete too (I'm working
on them now), and you can just use __assign_str().
I noticed that I don't have a string_len example in the sample code and I'm
actually writing it now.
// cutting out everything else:
TRACE_EVENT(foo_bar,
TP_PROTO(const char *foo, int bar),
TP_ARGS(foo, bar),
TP_STRUCT__entry(
__string_len( lstr, foo,bar < strlen(foo) ? bar :
strlen(foo) )
),
TP_fast_assign(
__assign_str(lstr, foo);
// Note, the above is with my updates, without them, you need to duplicate the
logic
// __assign_str_len(lstr, foo, bar < strlen(foo) ? bar :
strlen(foo));
),
TP_printk("%s", __get_str(lstr))
);
The above will allocate "bar < strlen(foo) ? bar : strlen(foo)" size on the
ring buffer. As the size is already stored, my clean up code uses that
instead of requiring duplicating the logic again.
-- Steve
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On 2/23/2024 9:56 AM, Steven Rostedt wrote: > From: "Steven Rostedt (Google)" > > [ >This is a treewide change. I will likely re-create this patch again in >the second week of the merge window of v6.9 and submit it then. Hoping >to keep the conflicts that it will cause to a minimum. > ] > > With the rework of how the __string() handles dynamic strings where it > saves off the source string in field in the helper structure[1], the > assignment of that value to the trace event field is stored in the helper > value and does not need to be passed in again. Just curious if this could be done piecemeal by first changing the macros to be variadic macros which allows you to ignore the extra argument. The callers could then be modified in their separate trees. And then once all the callers have be merged, the macros could be changed to no longer be variadic. ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 10:30:45 -0800 Jeff Johnson wrote: > On 2/23/2024 9:56 AM, Steven Rostedt wrote: > > From: "Steven Rostedt (Google)" > > > > [ > >This is a treewide change. I will likely re-create this patch again in > >the second week of the merge window of v6.9 and submit it then. Hoping > >to keep the conflicts that it will cause to a minimum. > > ] > > > > With the rework of how the __string() handles dynamic strings where it > > saves off the source string in field in the helper structure[1], the > > assignment of that value to the trace event field is stored in the helper > > value and does not need to be passed in again. > > Just curious if this could be done piecemeal by first changing the > macros to be variadic macros which allows you to ignore the extra > argument. The callers could then be modified in their separate trees. > And then once all the callers have be merged, the macros could be > changed to no longer be variadic. I weighed doing that, but I think ripping off the band-aid is a better approach. One thing I found is that leaving unused parameters in the macros can cause bugs itself. I found one case doing my clean up, where an unused parameter in one of the macros was bogus, and when I made it a used parameter, it broke the build. I think for tree-wide changes, the preferred approach is to do one big patch at once. And since this only affects TRACE_EVENT() macros, it hopefully would not be too much of a burden (although out of tree users may suffer from this, but do we care?) Now one thing I could do is to not remove the parameter, but just add: WARN_ON_ONCE((src) != __data_offsets->item##_ptr_); in the __assign_str() macro to make sure that it's still the same that is assigned. But I'm not sure how useful that is, and still causes burden to have it. I never really liked the passing of the string in two places to begin with. -- Steve ___ 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: introduce SEGS_TO_BLKS/BLKS_TO_SEGS for cleanup
Chao,
I applied the below as well in order to keep zone capacity back.
--- a/fs/f2fs/segment.h
+++ b/fs/f2fs/segment.h
@@ -101,11 +101,10 @@ static inline void sanity_check_seg_type(struct
f2fs_sb_info *sbi,
NULL_SEGNO : GET_L2R_SEGNO(FREE_I(sbi), \
GET_SEGNO_FROM_SEG0(sbi, blk_addr)))
#define CAP_BLKS_PER_SEC(sbi) \
- (SEGS_PER_SEC(sbi) * BLKS_PER_SEG(sbi) -\
-(sbi)->unusable_blocks_per_sec)
+ (BLKS_PER_SEC(sbi) - (sbi)->unusable_blocks_per_sec)
#define CAP_SEGS_PER_SEC(sbi) \
- (SEGS_PER_SEC(sbi) - ((sbi)->unusable_blocks_per_sec >> \
- (sbi)->log_blocks_per_seg))
+ (SEGS_PER_SEC(sbi) -\
+ BLKS_TO_SEGS(sbi, (sbi)->unusable_blocks_per_sec)
#define GET_SEC_FROM_SEG(sbi, segno) \
(((segno) == -1) ? -1 : (segno) / SEGS_PER_SEC(sbi))
#define GET_SEG_FROM_SEC(sbi, secno) \
On 02/21, Chao Yu wrote:
> Just cleanup, no functional change.
>
> Signed-off-by: Chao Yu
> ---
> fs/f2fs/debug.c | 7 +++
> fs/f2fs/f2fs.h| 14 --
> fs/f2fs/gc.c | 10 +-
> fs/f2fs/gc.h | 4 ++--
> fs/f2fs/segment.c | 12 ++--
> fs/f2fs/segment.h | 8
> fs/f2fs/super.c | 16
> fs/f2fs/sysfs.c | 4 ++--
> 8 files changed, 38 insertions(+), 37 deletions(-)
>
> diff --git a/fs/f2fs/debug.c b/fs/f2fs/debug.c
> index 6617195bd27e..12893477f2e4 100644
> --- a/fs/f2fs/debug.c
> +++ b/fs/f2fs/debug.c
> @@ -134,7 +134,7 @@ static void update_general_status(struct f2fs_sb_info
> *sbi)
> si->cur_ckpt_time = sbi->cprc_info.cur_time;
> si->peak_ckpt_time = sbi->cprc_info.peak_time;
> spin_unlock(&sbi->cprc_info.stat_lock);
> - si->total_count = (int)sbi->user_block_count / BLKS_PER_SEG(sbi);
> + si->total_count = BLKS_TO_SEGS(sbi, (int)sbi->user_block_count);
> si->rsvd_segs = reserved_segments(sbi);
> si->overp_segs = overprovision_segments(sbi);
> si->valid_count = valid_user_blocks(sbi);
> @@ -175,11 +175,10 @@ static void update_general_status(struct f2fs_sb_info
> *sbi)
> si->alloc_nids = NM_I(sbi)->nid_cnt[PREALLOC_NID];
> si->io_skip_bggc = sbi->io_skip_bggc;
> si->other_skip_bggc = sbi->other_skip_bggc;
> - si->util_free = (int)(free_user_blocks(sbi) >> sbi->log_blocks_per_seg)
> + si->util_free = (int)(BLKS_TO_SEGS(sbi, free_user_blocks(sbi)))
> * 100 / (int)(sbi->user_block_count >> sbi->log_blocks_per_seg)
> / 2;
> - si->util_valid = (int)(written_block_count(sbi) >>
> - sbi->log_blocks_per_seg)
> + si->util_valid = (int)(BLKS_TO_SEGS(sbi, written_block_count(sbi)))
> * 100 / (int)(sbi->user_block_count >> sbi->log_blocks_per_seg)
> / 2;
> si->util_invalid = 50 - si->util_free - si->util_valid;
> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
> index dad2774ca72f..8a6fd4352a0e 100644
> --- a/fs/f2fs/f2fs.h
> +++ b/fs/f2fs/f2fs.h
> @@ -1813,12 +1813,14 @@ struct f2fs_sb_info {
> };
>
> /* Definitions to access f2fs_sb_info */
> -#define BLKS_PER_SEG(sbi)\
> - ((sbi)->blocks_per_seg)
> -#define BLKS_PER_SEC(sbi)\
> - ((sbi)->segs_per_sec << (sbi)->log_blocks_per_seg)
> -#define SEGS_PER_SEC(sbi)\
> - ((sbi)->segs_per_sec)
> +#define SEGS_TO_BLKS(sbi, segs) \
> + ((segs) << (sbi)->log_blocks_per_seg)
> +#define BLKS_TO_SEGS(sbi, blks) \
> + ((blks) >> (sbi)->log_blocks_per_seg)
> +
> +#define BLKS_PER_SEG(sbi)((sbi)->blocks_per_seg)
> +#define BLKS_PER_SEC(sbi)(SEGS_TO_BLKS(sbi, (sbi)->segs_per_sec))
> +#define SEGS_PER_SEC(sbi)((sbi)->segs_per_sec)
>
> __printf(3, 4)
> void f2fs_printk(struct f2fs_sb_info *sbi, bool limit_rate, const char *fmt,
> ...);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 3ff126316d42..6d160d50e14e 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -301,7 +301,7 @@ static unsigned int get_max_cost(struct f2fs_sb_info *sbi,
>
> /* LFS */
> if (p->gc_mode == GC_GREEDY)
> - return 2 * BLKS_PER_SEG(sbi) * p->ofs_unit;
> + return SEGS_TO_BLKS(sbi, 2 * p->ofs_unit);
> else if (p->gc_mode == GC_CB)
> return UINT_MAX;
> else if (p->gc_mode == GC_AT)
> @@ -347,7 +347,7 @@ static unsigned int get_cb_cost(struct f2fs_sb_info *sbi,
> unsigned int segno)
> mtime = div_u64(mtime, SEGS_PER_SEC(sbi));
> vblocks = div_u64(vblocks, SEGS_PER_SEC(sbi));
>
> - u = (vblocks * 100) >> sbi->log_blocks_per_seg;
> + u = BLKS_TO_SEGS(sbi, vblocks * 100);
>
> /* Handle if the syst
Re: [f2fs-dev] [FYI][PATCH] tracing/treewide: Remove second parameter of __assign_str()
On Fri, 23 Feb 2024 12:56:34 -0500 Steven Rostedt wrote: > Note, the same updates will need to be done for: > > __assign_str_len() > __assign_rel_str() > __assign_rel_str_len() Correction: The below macros do not pass in their source to the entry macros, so they will not need to be updated. -- Steve > __assign_bitmask() > __assign_rel_bitmask() > __assign_cpumask() > __assign_rel_cpumask() ___ 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 v2] f2fs: kill zone-capacity support
On 02/22, Matias Bjørling wrote: > On 21-02-2024 18:27, Jaegeuk Kim wrote: > > > > Doesn't this break practically all ZNS NVMe devices? > > > > Yes, so here I'm in questioning who is really using w/ zone capacity. If > > there's > > no user complaining, I'd like to deprecate this, since this adds code > > complexity > > and unnecessary checks. > > > > Hi Jaegeuk, > > I like to make a couple of points to hopefully keep the support in a little > while longer. > > - NVMe-based zone devices continue to be developed with the pow2 zone size > and zone size != zone cap features. There was some divergence in the > first-gen drives. However, all the second-gen drives I know of are > implemented with those features in mind. > > - A very active community is doing work using f2fs, and many of those > members are working with the ZN540s device (which exposes a pow2 zone size). > > - For drives with a capacity of less than 16TiB, f2fs is an excellent file > system to use and is really useful for various use cases. We're using the > f2fs daily for a couple of our workloads. > > Work is ongoing on btrfs and XFS to support zoned storage devices, but they > have yet to be through the trenches as much as f2fs has been with its zone > support. So it would be great to have f2fs continue to support the pow2 zone > sizes, as it is a valuable feature for the currently used and second-gen > drives that have been released or are soon becoming available. > > If there is a performance concern with the feature re: ZUFS, maybe the pow2 > implementation could be slightly more computationally expensive, as the > feature, anyway, typically is used on more beefy systems. Thanks, Matias for the background. It seems to be fair for keeping this for a while tho, IMO, non-pow2 could address both parties. > > Regards, > Matias ___ Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [f2fs-dev] [PATCH v3 2/2] f2fs: support file pinning for zoned devices
Hi Chao,
I've tested the patch and queued in -dev, so can you take a look at it and
propose any change on top of it? Then, we can discuss further on it.
On 02/23, Chao Yu wrote:
> On 2024/2/14 1:38, Daeho Jeong wrote:
> > From: Daeho Jeong
> >
> > Support file pinning with conventional storage area for zoned devices
> >
> > Signed-off-by: Daeho Jeong
> > Signed-off-by: Jaegeuk Kim
> > ---
> > v3: check the hole when migrating blocks for swap.
> > do not use the remainder of cold pin section.
> > v2: flush previous dirty pages before swapon.
> > do not re-check for the last extent of swap area.
> > merge this patch with swap file pinning support patch.
> > ---
> > fs/f2fs/data.c| 58 ++-
> > fs/f2fs/f2fs.h| 17 +++-
> > fs/f2fs/file.c| 24 -
> > fs/f2fs/gc.c | 14 +++---
> > fs/f2fs/segment.c | 69 +--
> > fs/f2fs/segment.h | 10 +++
> > 6 files changed, 154 insertions(+), 38 deletions(-)
> >
> > diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> > index 828c797cd47c..0c9aa3082fcf 100644
> > --- a/fs/f2fs/data.c
> > +++ b/fs/f2fs/data.c
> > @@ -3839,25 +3839,34 @@ static int f2fs_migrate_blocks(struct inode *inode,
> > block_t start_blk,
> > unsigned int blkofs;
> > unsigned int blk_per_sec = BLKS_PER_SEC(sbi);
> > unsigned int secidx = start_blk / blk_per_sec;
> > - unsigned int end_sec = secidx + blkcnt / blk_per_sec;
> > + unsigned int end_sec;
> > int ret = 0;
> > + if (!blkcnt)
> > + return 0;
> > + end_sec = secidx + (blkcnt - 1) / blk_per_sec;
> > +
> > f2fs_down_write(&F2FS_I(inode)->i_gc_rwsem[WRITE]);
> > filemap_invalidate_lock(inode->i_mapping);
> > set_inode_flag(inode, FI_ALIGNED_WRITE);
> > set_inode_flag(inode, FI_OPU_WRITE);
> > - for (; secidx < end_sec; secidx++) {
> > + for (; secidx <= end_sec; secidx++) {
> > + unsigned int blkofs_end = secidx == end_sec ?
> > + (blkcnt - 1) % blk_per_sec : blk_per_sec - 1;
> > +
> > f2fs_down_write(&sbi->pin_sem);
> > - f2fs_lock_op(sbi);
> > - f2fs_allocate_new_section(sbi, CURSEG_COLD_DATA_PINNED, false);
> > - f2fs_unlock_op(sbi);
> > + ret = f2fs_allocate_pinning_section(sbi);
> > + if (ret) {
> > + f2fs_up_write(&sbi->pin_sem);
> > + break;
> > + }
> > set_inode_flag(inode, FI_SKIP_WRITES);
> > - for (blkofs = 0; blkofs < blk_per_sec; blkofs++) {
> > + for (blkofs = 0; blkofs <= blkofs_end; blkofs++) {
> > struct page *page;
> > unsigned int blkidx = secidx * blk_per_sec + blkofs;
> > @@ -3946,27 +3955,34 @@ static int check_swap_activate(struct
> > swap_info_struct *sis,
> > nr_pblocks = map.m_len;
> > if ((pblock - SM_I(sbi)->main_blkaddr) & sec_blks_mask ||
> > - nr_pblocks & sec_blks_mask) {
> > + nr_pblocks & sec_blks_mask ||
> > + !f2fs_valid_pinned_area(sbi, pblock)) {
> > + bool last_extent = false;
> > +
> > not_aligned++;
> > nr_pblocks = roundup(nr_pblocks, blks_per_sec);
> > if (cur_lblock + nr_pblocks > sis->max)
> > nr_pblocks -= blks_per_sec;
> > + /* this extent is last one */
> > if (!nr_pblocks) {
> > - /* this extent is last one */
> > - nr_pblocks = map.m_len;
> > - f2fs_warn(sbi, "Swapfile: last extent is not
> > aligned to section");
> > - goto next;
> > + nr_pblocks = last_lblock - cur_lblock;
> > + last_extent = true;
> > }
> > ret = f2fs_migrate_blocks(inode, cur_lblock,
> > nr_pblocks);
> > - if (ret)
> > + if (ret) {
> > + if (ret == -ENOENT)
> > + ret = -EINVAL;
> > goto out;
> > - goto retry;
> > + }
> > +
> > + if (!last_extent)
> > + goto retry;
> > }
> > -next:
> > +
> > if (cur_lblock + nr_pblocks >= sis->max)
> > nr_pblocks = sis->max - cur_lblock;
> > @@ -4004,17 +4020,17 @@ static int f2fs_swap_activate(struct
> > swap_info_struct *sis, struct file *file,
> > sector_t *span)
> > {
> > struct inode *inode = file_inode(file);
> > + struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
> > int ret;
> > if (!S_ISREG(inode->i_mode))
> >
