Re: [f2fs-dev] [RFC PATCH 1/5] mkfs.f2fs: introduce mkfs parameters in f2fs_configuration

2018-02-09 Thread Chao Yu 
Hi Sheng Yong,

On 2018/2/9 11:21, Sheng Yong wrote:
> Hi, Chao
> 
> Add Hyojun.
> 
> On 2018/2/8 21:30, Chao Yu wrote:
>> On 2018/2/6 12:31, Sheng Yong wrote:
>>>   /* only root inode was written before truncating dnodes */
>>>   last_inode_pos = start_inode_pos +
>>> -    c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + quota_inum;
>>> +    c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + c.quota_inum;
>>
>> - f2fs_create_root_dir
>>   - f2fs_write_root_inode
>>    - discard_obsolete_dnode
>>  access c.quota_inum
>>   - f2fs_write_qf_inode
>>     update c.quota_inum
>>
>> Should c.quota_inum be initialized more early?
> 
> I think we should only count quota inodes if thye are already initialized.
> Otherwise, if quota_ino is not enabled, there is only one inode (root) in
> CURSEG_HOT_NODE, and two blocks next to root inode will not be discarded.

IMO, we need to move discard_obsolete_dnode in to f2fs_create_root_dir as below:

- f2fs_create_root_dir
 - f2fs_write_root_inode
 - f2fs_write_qf_inode
 - discard_obsolete_dnode

So the last obsolete position can be calculated correctly.

Thanks,

> 
> I'm a bit confused about the `offset' scale in discard_obsolete_dnode.
> It seems `if (offset >= start_inode_pos || offset <= last_inode_pos)'
> will always be true?
> 
> Hi, Hyojun, could you please also have a look at this, thanks :)
> 
> diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
> index 0fc8b30..6c9ae22 100644
> --- a/mkfs/f2fs_format.c
> +++ b/mkfs/f2fs_format.c
> @@ -963,7 +963,7 @@ static int discard_obsolete_dnode(struct f2fs_node 
> *raw_node, u_int64_t offset)
>     }
>     offset = next_blkaddr;
>     /* should avoid recursive chain due to stale data */
> -   if (offset >= start_inode_pos || offset <= last_inode_pos)
> +   if (offset <= last_inode_pos)
>     break;
>     } while (1);
> 
> thanks,
> Sheng
>>
>> Thanks,
>>
>> .
>>
> 

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [RFC PATCH 1/5] mkfs.f2fs: introduce mkfs parameters in f2fs_configuration

2018-02-08 Thread Sheng Yong 

Hi, Chao

Add Hyojun.

On 2018/2/8 21:30, Chao Yu wrote:

On 2018/2/6 12:31, Sheng Yong wrote:

/* only root inode was written before truncating dnodes */
last_inode_pos = start_inode_pos +
-   c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + quota_inum;
+   c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + c.quota_inum;


- f2fs_create_root_dir
  - f2fs_write_root_inode
   - discard_obsolete_dnode
 access c.quota_inum
  - f2fs_write_qf_inode
update c.quota_inum

Should c.quota_inum be initialized more early?


I think we should only count quota inodes if thye are already initialized.
Otherwise, if quota_ino is not enabled, there is only one inode (root) in
CURSEG_HOT_NODE, and two blocks next to root inode will not be discarded.

I'm a bit confused about the `offset' scale in discard_obsolete_dnode.
It seems `if (offset >= start_inode_pos || offset <= last_inode_pos)'
will always be true?

Hi, Hyojun, could you please also have a look at this, thanks :)

diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 0fc8b30..6c9ae22 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -963,7 +963,7 @@ static int discard_obsolete_dnode(struct f2fs_node 
*raw_node, u_int64_t offset)
}
offset = next_blkaddr;
/* should avoid recursive chain due to stale data */
-   if (offset >= start_inode_pos || offset <= last_inode_pos)
+   if (offset <= last_inode_pos)
break;
} while (1);

thanks,
Sheng


Thanks,

.




--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

Re: [f2fs-dev] [RFC PATCH 1/5] mkfs.f2fs: introduce mkfs parameters in f2fs_configuration

2018-02-08 Thread Chao Yu 
On 2018/2/6 12:31, Sheng Yong wrote:
>   /* only root inode was written before truncating dnodes */
>   last_inode_pos = start_inode_pos +
> - c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + quota_inum;
> + c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + c.quota_inum;

- f2fs_create_root_dir
 - f2fs_write_root_inode
  - discard_obsolete_dnode
access c.quota_inum
 - f2fs_write_qf_inode
   update c.quota_inum

Should c.quota_inum be initialized more early?

Thanks,

--
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel

[f2fs-dev] [RFC PATCH 1/5] mkfs.f2fs: introduce mkfs parameters in f2fs_configuration

2018-02-05 Thread Sheng Yong 
Introduce new parameters in f2fs_configuration for mkfs:
  * next_free_nid: save the next free nid
  * quota_inum: save how many blocks are used for quota inodes
  * quota_dnum: save how many blocks are used for quota data

Use these parameters to avoid duplicated calculation of these values.

Signed-off-by: Sheng Yong 
---
 include/f2fs_fs.h  |  5 +
 mkfs/f2fs_format.c | 46 +-
 2 files changed, 22 insertions(+), 29 deletions(-)

diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 548a3e8..ca4522d 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -365,6 +365,11 @@ struct f2fs_configuration {
int large_nat_bitmap;
__le32 feature; /* defined features */
 
+   /* mkfs parameters */
+   u_int32_t next_free_nid;
+   u_int32_t quota_inum;
+   u_int32_t quota_dnum;
+
/* defragmentation parameters */
int defrag_shrink;
u_int64_t defrag_start;
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 4fb429e..bda3c9d 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -159,7 +159,6 @@ static int f2fs_prepare_super_block(void)
u_int32_t sit_bitmap_size, max_sit_bitmap_size;
u_int32_t max_nat_bitmap_size, max_nat_segments;
u_int32_t total_zones;
-   u_int32_t next_ino;
enum quota_type qtype;
int i;
 
@@ -411,7 +410,7 @@ static int f2fs_prepare_super_block(void)
set_sb(node_ino, 1);
set_sb(meta_ino, 2);
set_sb(root_ino, 3);
-   next_ino = 4;
+   c.next_free_nid = 4;
 
if (c.feature & cpu_to_le32(F2FS_FEATURE_QUOTA_INO)) {
quotatype_bits = QUOTA_USR_BIT | QUOTA_GRP_BIT;
@@ -422,9 +421,9 @@ static int f2fs_prepare_super_block(void)
for (qtype = 0; qtype < F2FS_MAX_QUOTAS; qtype++) {
if (!((1 << qtype) & quotatype_bits))
continue;
-   sb->qf_ino[qtype] = cpu_to_le32(next_ino++);
+   sb->qf_ino[qtype] = cpu_to_le32(c.next_free_nid++);
MSG(0, "Info: add quota type = %u => %u\n",
-   qtype, next_ino - 1);
+   qtype, c.next_free_nid - 1);
}
 
if (total_zones <= 6) {
@@ -558,7 +557,6 @@ static int f2fs_write_check_point_pack(void)
char *sum_compact, *sum_compact_p;
struct f2fs_summary *sum_entry;
enum quota_type qtype;
-   u_int32_t quota_inum, quota_dnum;
int off;
int ret = -1;
 
@@ -610,16 +608,9 @@ static int f2fs_write_check_point_pack(void)
set_cp(cur_data_segno[i], 0x);
}
 
-   quota_inum = quota_dnum = 0;
-   for (qtype = 0; qtype < F2FS_MAX_QUOTAS; qtype++)
-   if (sb->qf_ino[qtype]) {
-   quota_inum++;
-   quota_dnum += QUOTA_DATA(qtype);
-   }
-
-   set_cp(cur_node_blkoff[0], 1 + quota_inum);
-   set_cp(cur_data_blkoff[0], 1 + quota_dnum);
-   set_cp(valid_block_count, 2 + quota_inum + quota_dnum);
+   set_cp(cur_node_blkoff[0], 1 + c.quota_inum);
+   set_cp(cur_data_blkoff[0], 1 + c.quota_dnum);
+   set_cp(valid_block_count, 2 + c.quota_inum + c.quota_dnum);
set_cp(rsvd_segment_count, c.reserved_segments);
set_cp(overprov_segment_count, (get_sb(segment_count_main) -
get_cp(rsvd_segment_count)) *
@@ -651,9 +642,9 @@ static int f2fs_write_check_point_pack(void)
 
set_cp(ckpt_flags, flags);
set_cp(cp_pack_start_sum, 1 + get_sb(cp_payload));
-   set_cp(valid_node_count, 1 + quota_inum);
-   set_cp(valid_inode_count, 1 + quota_inum);
-   set_cp(next_free_nid, get_sb(root_ino) + 1 + quota_inum);
+   set_cp(valid_node_count, 1 + c.quota_inum);
+   set_cp(valid_inode_count, 1 + c.quota_inum);
+   set_cp(next_free_nid, c.next_free_nid);
set_cp(sit_ver_bitmap_bytesize, ((get_sb(segment_count_sit) / 2) <<
get_sb(log_blocks_per_seg)) / 8);
 
@@ -711,7 +702,7 @@ static int f2fs_write_check_point_pack(void)
SET_SUM_TYPE((>footer), SUM_TYPE_DATA);
 
journal = >journal;
-   journal->n_nats = cpu_to_le16(1 + quota_inum);
+   journal->n_nats = cpu_to_le16(1 + c.quota_inum);
journal->nat_j.entries[0].nid = sb->root_ino;
journal->nat_j.entries[0].ne.version = 0;
journal->nat_j.entries[0].ne.ino = sb->root_ino;
@@ -741,9 +732,9 @@ static int f2fs_write_check_point_pack(void)
journal->sit_j.entries[0].segno = cp->cur_node_segno[0];
journal->sit_j.entries[0].se.vblocks =
cpu_to_le16((CURSEG_HOT_NODE << 10) |
-   (1 + quota_inum));
+   (1 + c.quota_inum));
f2fs_set_bit(0, (char *)journal->sit_j.entries[0].se.valid_map);
-   for (i =