Re: [f2fs-dev] [PATCH] f2fs: make trace enter and end in pairs for unlink

2020-06-19 Thread Chao Yu
On 2020/6/20 10:12, Lihong Kou wrote:
> In the f2fs_unlink we do not add trace end for some
> error paths, just add.
> 
> Signed-off-by: Lihong Kou 

Reviewed-by: Chao Yu 

Thanks,


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] f2fs: make trace enter and end in pairs for unlink

2020-06-19 Thread Lihong Kou
In the f2fs_unlink we do not add trace end for some
error paths, just add.

Signed-off-by: Lihong Kou 
---
 fs/f2fs/namei.c | 10 ++
 1 file changed, 6 insertions(+), 4 deletions(-)

diff --git a/fs/f2fs/namei.c b/fs/f2fs/namei.c
index e94e02c6580a..a15a2831d43b 100644
--- a/fs/f2fs/namei.c
+++ b/fs/f2fs/namei.c
@@ -569,15 +569,17 @@ static int f2fs_unlink(struct inode *dir, struct dentry 
*dentry)
 
trace_f2fs_unlink_enter(dir, dentry);
 
-   if (unlikely(f2fs_cp_error(sbi)))
-   return -EIO;
+   if (unlikely(f2fs_cp_error(sbi))) {
+   err = -EIO;
+   goto fail;
+   }
 
err = dquot_initialize(dir);
if (err)
-   return err;
+   goto fail;
err = dquot_initialize(inode);
if (err)
-   return err;
+   goto fail;
 
de = f2fs_find_entry(dir, >d_name, );
if (!de) {
-- 
2.17.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 1/5] f2fs: fix to wait page writeback before update

2020-06-19 Thread Chao Yu
On 2020/6/20 6:47, Jaegeuk Kim wrote:
> On 06/19, Chao Yu wrote:
>> On 2020/6/19 13:49, Jaegeuk Kim wrote:
>>> On 06/19, Chao Yu wrote:
 Hi Jaegeuk,

 On 2020/6/19 7:59, Jaegeuk Kim wrote:
> Hi Chao,
>
> On 06/18, Chao Yu wrote:
>> to make page content stable for special device like raid.
>
> Could you elaborate the problem a bit?

 Some devices like raid5 wants page content to be stable, because
 it will calculate parity info based page content, if page is not
 stable, parity info could be corrupted, result in data inconsistency
 in stripe.
>>>
>>> I don't get the point, since those pages are brand new pages which were not
>>> modified before. If it's on writeback, we should not modify them regardless
>>> of whatever raid configuration. For example, f2fs_new_node_page() waits for
>>> writeback. Am I missing something?
>>
>> I think we should use f2fs_bug_on(, PageWriteback()) rather than
>> f2fs_wait_on_page_writeback() for brand new page which is allocated just now.
>> For other paths, we can keep rule that waiting for writeback before updating.
>>
>> How do you think?
>>
>> Thanks,
>>
>>>

 Thanks,

>
>>
>> Signed-off-by: Chao Yu 
>> ---
>>  fs/f2fs/dir.c  |  2 ++
>>  fs/f2fs/extent_cache.c | 18 +-
>>  fs/f2fs/f2fs.h |  2 +-
>>  fs/f2fs/file.c |  1 +
>>  fs/f2fs/inline.c   |  2 ++
>>  fs/f2fs/inode.c|  3 +--
>>  6 files changed, 16 insertions(+), 12 deletions(-)
>>
>> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>> index d35976785e8c..91e86747a604 100644
>> --- a/fs/f2fs/dir.c
>> +++ b/fs/f2fs/dir.c
>> @@ -495,6 +495,8 @@ static int make_empty_dir(struct inode *inode,
>>  if (IS_ERR(dentry_page))
>>  return PTR_ERR(dentry_page);
>>  
>> +f2fs_bug_on(F2FS_I_SB(inode), PageWriteback(dentry_page));
>> +
>>  dentry_blk = page_address(dentry_page);
>>  
>>  make_dentry_ptr_block(NULL, , dentry_blk);
>> diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>> index e60078460ad1..686c68b98610 100644
>> --- a/fs/f2fs/extent_cache.c
>> +++ b/fs/f2fs/extent_cache.c
>> @@ -325,9 +325,10 @@ static void __drop_largest_extent(struct 
>> extent_tree *et,
>>  }
>>  
>>  /* return true, if inode page is changed */
>> -static bool __f2fs_init_extent_tree(struct inode *inode, struct 
>> f2fs_extent *i_ext)
>> +static void __f2fs_init_extent_tree(struct inode *inode, struct page 
>> *ipage)
>>  {
>>  struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>> +struct f2fs_extent *i_ext = ipage ? _INODE(ipage)->i_ext : 
>> NULL;
>>  struct extent_tree *et;
>>  struct extent_node *en;
>>  struct extent_info ei;
>> @@ -335,16 +336,18 @@ static bool __f2fs_init_extent_tree(struct inode 
>> *inode, struct f2fs_extent *i_e
>>  if (!f2fs_may_extent_tree(inode)) {
>>  /* drop largest extent */
>>  if (i_ext && i_ext->len) {
>> +f2fs_wait_on_page_writeback(ipage, NODE, true, 
>> true);
>>  i_ext->len = 0;
>> -return true;
>> +set_page_dirty(ipage);
>> +return;
>>  }
>> -return false;
>> +return;
>>  }
>>  
>>  et = __grab_extent_tree(inode);
>>  
>>  if (!i_ext || !i_ext->len)
>> -return false;
>> +return;
>>  
>>  get_extent_info(, i_ext);
>>  
>> @@ -360,17 +363,14 @@ static bool __f2fs_init_extent_tree(struct inode 
>> *inode, struct f2fs_extent *i_e
>>  }
>>  out:
>>  write_unlock(>lock);
>> -return false;
>>  }
>>  
>> -bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent 
>> *i_ext)
>> +void f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
>>  {
>> -bool ret =  __f2fs_init_extent_tree(inode, i_ext);
>> +__f2fs_init_extent_tree(inode, ipage);
>>  
>>  if (!F2FS_I(inode)->extent_tree)
>>  set_inode_flag(inode, FI_NO_EXTENT);
>> -
>> -return ret;
>>  }
>>  
>>  static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
>> diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>> index b35a50f4953c..326c12fa0da5 100644
>> --- a/fs/f2fs/f2fs.h
>> +++ b/fs/f2fs/f2fs.h
>> @@ -3795,7 +3795,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct 
>> rb_root_cached *root,
>>  bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi,
>>   

Re: [f2fs-dev] [PATCH 1/5] f2fs: fix to wait page writeback before update

2020-06-19 Thread Jaegeuk Kim
On 06/19, Chao Yu wrote:
> On 2020/6/19 13:49, Jaegeuk Kim wrote:
> > On 06/19, Chao Yu wrote:
> >> Hi Jaegeuk,
> >>
> >> On 2020/6/19 7:59, Jaegeuk Kim wrote:
> >>> Hi Chao,
> >>>
> >>> On 06/18, Chao Yu wrote:
>  to make page content stable for special device like raid.
> >>>
> >>> Could you elaborate the problem a bit?
> >>
> >> Some devices like raid5 wants page content to be stable, because
> >> it will calculate parity info based page content, if page is not
> >> stable, parity info could be corrupted, result in data inconsistency
> >> in stripe.
> > 
> > I don't get the point, since those pages are brand new pages which were not
> > modified before. If it's on writeback, we should not modify them regardless
> > of whatever raid configuration. For example, f2fs_new_node_page() waits for
> > writeback. Am I missing something?
> 
> I think we should use f2fs_bug_on(, PageWriteback()) rather than
> f2fs_wait_on_page_writeback() for brand new page which is allocated just now.
> For other paths, we can keep rule that waiting for writeback before updating.
> 
> How do you think?
> 
> Thanks,
> 
> > 
> >>
> >> Thanks,
> >>
> >>>
> 
>  Signed-off-by: Chao Yu 
>  ---
>   fs/f2fs/dir.c  |  2 ++
>   fs/f2fs/extent_cache.c | 18 +-
>   fs/f2fs/f2fs.h |  2 +-
>   fs/f2fs/file.c |  1 +
>   fs/f2fs/inline.c   |  2 ++
>   fs/f2fs/inode.c|  3 +--
>   6 files changed, 16 insertions(+), 12 deletions(-)
> 
>  diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
>  index d35976785e8c..91e86747a604 100644
>  --- a/fs/f2fs/dir.c
>  +++ b/fs/f2fs/dir.c
>  @@ -495,6 +495,8 @@ static int make_empty_dir(struct inode *inode,
>   if (IS_ERR(dentry_page))
>   return PTR_ERR(dentry_page);
>   
>  +f2fs_bug_on(F2FS_I_SB(inode), PageWriteback(dentry_page));
>  +
>   dentry_blk = page_address(dentry_page);
>   
>   make_dentry_ptr_block(NULL, , dentry_blk);
>  diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
>  index e60078460ad1..686c68b98610 100644
>  --- a/fs/f2fs/extent_cache.c
>  +++ b/fs/f2fs/extent_cache.c
>  @@ -325,9 +325,10 @@ static void __drop_largest_extent(struct 
>  extent_tree *et,
>   }
>   
>   /* return true, if inode page is changed */
>  -static bool __f2fs_init_extent_tree(struct inode *inode, struct 
>  f2fs_extent *i_ext)
>  +static void __f2fs_init_extent_tree(struct inode *inode, struct page 
>  *ipage)
>   {
>   struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
>  +struct f2fs_extent *i_ext = ipage ? _INODE(ipage)->i_ext : 
>  NULL;
>   struct extent_tree *et;
>   struct extent_node *en;
>   struct extent_info ei;
>  @@ -335,16 +336,18 @@ static bool __f2fs_init_extent_tree(struct inode 
>  *inode, struct f2fs_extent *i_e
>   if (!f2fs_may_extent_tree(inode)) {
>   /* drop largest extent */
>   if (i_ext && i_ext->len) {
>  +f2fs_wait_on_page_writeback(ipage, NODE, true, 
>  true);
>   i_ext->len = 0;
>  -return true;
>  +set_page_dirty(ipage);
>  +return;
>   }
>  -return false;
>  +return;
>   }
>   
>   et = __grab_extent_tree(inode);
>   
>   if (!i_ext || !i_ext->len)
>  -return false;
>  +return;
>   
>   get_extent_info(, i_ext);
>   
>  @@ -360,17 +363,14 @@ static bool __f2fs_init_extent_tree(struct inode 
>  *inode, struct f2fs_extent *i_e
>   }
>   out:
>   write_unlock(>lock);
>  -return false;
>   }
>   
>  -bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent 
>  *i_ext)
>  +void f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
>   {
>  -bool ret =  __f2fs_init_extent_tree(inode, i_ext);
>  +__f2fs_init_extent_tree(inode, ipage);
>   
>   if (!F2FS_I(inode)->extent_tree)
>   set_inode_flag(inode, FI_NO_EXTENT);
>  -
>  -return ret;
>   }
>   
>   static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
>  diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
>  index b35a50f4953c..326c12fa0da5 100644
>  --- a/fs/f2fs/f2fs.h
>  +++ b/fs/f2fs/f2fs.h
>  @@ -3795,7 +3795,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct 
>  rb_root_cached *root,
>   bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi,
>   struct rb_root_cached 
>  *root);
> 

[f2fs-dev] [PATCH] fsck.f2fs: Fix slow fsck in auto-fix mode

2020-06-19 Thread Jaegeuk Kim
From: Robin Hsu 

Split f2fs_init_nid_bitmap() into two disjoint parts:
f2fs_early_init_nid_bitmap(), and
f2fs_late_init_nid_bitmap(),
where f2fs_late_init_nid_bitmap() won't be called in auto-fix mode, when
no errors were found.

f2fs_late_init_nid_bitmap() contains the loop to create NID bitmap from
NAT. which is the main reason for slow fsck.

Signed-off-by: Robin Hsu 
---
 fsck/mount.c | 69 
 1 file changed, 43 insertions(+), 26 deletions(-)

diff --git a/fsck/mount.c b/fsck/mount.c
index fb45941..d0f2eab 100644
--- a/fsck/mount.c
+++ b/fsck/mount.c
@@ -1288,15 +1288,14 @@ pgoff_t current_nat_addr(struct f2fs_sb_info *sbi, 
nid_t start, int *pack)
return block_addr;
 }
 
-static int f2fs_init_nid_bitmap(struct f2fs_sb_info *sbi)
+/* will not init nid_bitmap from nat */
+static int f2fs_early_init_nid_bitmap(struct f2fs_sb_info *sbi)
 {
struct f2fs_nm_info *nm_i = NM_I(sbi);
int nid_bitmap_size = (nm_i->max_nid + BITS_PER_BYTE - 1) / 
BITS_PER_BYTE;
struct curseg_info *curseg = CURSEG_I(sbi, CURSEG_HOT_DATA);
struct f2fs_summary_block *sum = curseg->sum_blk;
struct f2fs_journal *journal = >journal;
-   struct f2fs_nat_block *nat_block;
-   block_t start_blk;
nid_t nid;
int i;
 
@@ -1310,28 +1309,6 @@ static int f2fs_init_nid_bitmap(struct f2fs_sb_info *sbi)
/* arbitrarily set 0 bit */
f2fs_set_bit(0, nm_i->nid_bitmap);
 
-   nat_block = malloc(F2FS_BLKSIZE);
-   if (!nat_block) {
-   free(nm_i->nid_bitmap);
-   return -ENOMEM;
-   }
-
-   f2fs_ra_meta_pages(sbi, 0, NAT_BLOCK_OFFSET(nm_i->max_nid),
-   META_NAT);
-
-   for (nid = 0; nid < nm_i->max_nid; nid++) {
-   if (!(nid % NAT_ENTRY_PER_BLOCK)) {
-   int ret;
-
-   start_blk = current_nat_addr(sbi, nid, NULL);
-   ret = dev_read_block(nat_block, start_blk);
-   ASSERT(ret >= 0);
-   }
-
-   if (nat_block->entries[nid % NAT_ENTRY_PER_BLOCK].block_addr)
-   f2fs_set_bit(nid, nm_i->nid_bitmap);
-   }
-
if (nats_in_cursum(journal) > NAT_JOURNAL_ENTRIES) {
MSG(0, "\tError: f2fs_init_nid_bitmap truncate n_nats(%u) to "
"NAT_JOURNAL_ENTRIES(%lu)\n",
@@ -1361,6 +1338,41 @@ static int f2fs_init_nid_bitmap(struct f2fs_sb_info *sbi)
if (addr != NULL_ADDR)
f2fs_set_bit(nid, nm_i->nid_bitmap);
}
+   return 0;
+}
+
+/* will init nid_bitmap from nat */
+static int f2fs_late_init_nid_bitmap(struct f2fs_sb_info *sbi)
+{
+   struct f2fs_nm_info *nm_i = NM_I(sbi);
+   struct f2fs_nat_block *nat_block;
+   block_t start_blk;
+   nid_t nid;
+
+   if (!(c.func == SLOAD || c.func == FSCK))
+   return 0;
+
+   nat_block = malloc(F2FS_BLKSIZE);
+   if (!nat_block) {
+   free(nm_i->nid_bitmap);
+   return -ENOMEM;
+   }
+
+   f2fs_ra_meta_pages(sbi, 0, NAT_BLOCK_OFFSET(nm_i->max_nid),
+   META_NAT);
+   for (nid = 0; nid < nm_i->max_nid; nid++) {
+   if (!(nid % NAT_ENTRY_PER_BLOCK)) {
+   int ret;
+
+   start_blk = current_nat_addr(sbi, nid, NULL);
+   ret = dev_read_block(nat_block, start_blk);
+   ASSERT(ret >= 0);
+   }
+
+   if (nat_block->entries[nid % NAT_ENTRY_PER_BLOCK].block_addr)
+   f2fs_set_bit(nid, nm_i->nid_bitmap);
+   }
+
free(nat_block);
return 0;
 }
@@ -1565,7 +1577,7 @@ int init_node_manager(struct f2fs_sb_info *sbi)
 
/* copy version bitmap */
memcpy(nm_i->nat_bitmap, version_bitmap, nm_i->bitmap_size);
-   return f2fs_init_nid_bitmap(sbi);
+   return f2fs_early_init_nid_bitmap(sbi);
 }
 
 int build_node_manager(struct f2fs_sb_info *sbi)
@@ -3463,6 +3475,11 @@ int f2fs_do_mount(struct f2fs_sb_info *sbi)
if (!f2fs_should_proceed(sb, get_cp(ckpt_flags)))
return 1;
 
+   if (f2fs_late_init_nid_bitmap(sbi)) {
+   ERR_MSG("f2fs_late_init_nid_bitmap failed\n");
+   return -1;
+   }
+
/* Check nat_bits */
if (c.func == FSCK && is_set_ckpt_flags(cp, CP_NAT_BITS_FLAG)) {
if (check_nat_bits(sbi, sb, cp) && c.fix_on)
-- 
2.27.0.111.gc72c7da667-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 2/2] gfs2: Rework read and page fault locking

2020-06-19 Thread Matthew Wilcox
On Fri, Jun 19, 2020 at 11:39:16AM +0200, Andreas Gruenbacher wrote:
>  static int gfs2_readpage(struct file *file, struct page *page)
>  {
> - struct address_space *mapping = page->mapping;
> - struct gfs2_inode *ip = GFS2_I(mapping->host);
> - struct gfs2_holder gh;
>   int error;
>  
> - unlock_page(page);
> - gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, );
> - error = gfs2_glock_nq();
> - if (unlikely(error))
> - goto out;
> - error = AOP_TRUNCATED_PAGE;
> - lock_page(page);
> - if (page->mapping == mapping && !PageUptodate(page))
> - error = __gfs2_readpage(file, page);
> - else
> - unlock_page(page);
> - gfs2_glock_dq();
> -out:
> - gfs2_holder_uninit();
> - if (error && error != AOP_TRUNCATED_PAGE)
> + error = __gfs2_readpage(file, page);
> + if (error)
>   lock_page(page);
>   return error;

I don't think this is right.  If you return an error from ->readpage, I'm
pretty sure you're supposed to unlock that page.  Looking at
generic_file_buffered_read():

error = mapping->a_ops->readpage(filp, page);
if (unlikely(error)) {
if (error == AOP_TRUNCATED_PAGE) {
put_page(page);
error = 0;
goto find_page;
}
goto readpage_error;
}
...
readpage_error:
put_page(page);
goto out;
...
out:
ra->prev_pos = prev_index;
ra->prev_pos <<= PAGE_SHIFT;
ra->prev_pos |= prev_offset;

*ppos = ((loff_t)index << PAGE_SHIFT) + offset;
file_accessed(filp);
return written ? written : error;

so we don't call unlock_page() in generic code, which means the next time
we try to get this page, we'll do ...

page = find_get_page(mapping, index);
...
if (!PageUptodate(page)) {
error = wait_on_page_locked_killable(page);
and presumably we'll wait forever because nobody is going to unlock this
page?

> @@ -598,16 +582,9 @@ static void gfs2_readahead(struct readahead_control *rac)
>  {
>   struct inode *inode = rac->mapping->host;
>   struct gfs2_inode *ip = GFS2_I(inode);
> - struct gfs2_holder gh;
>  
> - gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, );
> - if (gfs2_glock_nq())
> - goto out_uninit;
>   if (!gfs2_is_stuffed(ip))
>   mpage_readahead(rac, gfs2_block_map);
> - gfs2_glock_dq();
> -out_uninit:
> - gfs2_holder_uninit();
>  }

Not for this patch, obviously, but why do you go to the effort of using
iomap_readpage() to implement gfs2_readpage(), but don't use iomap for
gfs2_readahead()?  Far more pages are brought in through ->readahead
than are brought in through ->readpage.

>  static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
>  {
> + struct gfs2_inode *ip;
> + struct gfs2_holder gh;
> + size_t written = 0;
>   ssize_t ret;
>  
> + gfs2_holder_mark_uninitialized();
>   if (iocb->ki_flags & IOCB_DIRECT) {
>   ret = gfs2_file_direct_read(iocb, to);

Again, future work, but you probably want to pass in  here so you
don't have to eat up another 32 bytes or so of stack space on an unused
gfs2_holder.

>   if (likely(ret != -ENOTBLK))
>   return ret;
>   iocb->ki_flags &= ~IOCB_DIRECT;
>   }
> - return generic_file_read_iter(iocb, to);
> + iocb->ki_flags |= IOCB_CACHED;
> + ret = generic_file_read_iter(iocb, to);
> + iocb->ki_flags &= ~IOCB_CACHED;
> + if (ret >= 0) {
> + if (!iov_iter_count(to))
> + return ret;
> + written = ret;
> + } else {
> + switch(ret) {
> + case -EAGAIN:
> + if (iocb->ki_flags & IOCB_NOWAIT)
> + return ret;
> + break;
> + case -ECANCELED:
> + break;
> + default:
> + return ret;
> + }
> + }

I'm wondering if we want to do this in common code rather than making it
something special only a few filesystems do (either because they care
about workloads with many threads accessing the same file, or because
their per-file locks are very heavy-weight).



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH 2/2] gfs2: Rework read and page fault locking

2020-06-19 Thread Andreas Gruenbacher
The cache consistency model of filesystems like gfs2 is such that if
data is found in the page cache, the data is up to date and can be used
without taking any filesystem locks.  If a page is not cached,
filesystem locks must be taken before populating the page cache.

Thus far,  gfs2 has taken the filesystem locks inside the ->readpage and
->readpages address space operations.  This was already causing lock
ordering problems, but commit d4388340ae0b ("fs: convert mpage_readpages
to mpage_readahead") made things worse: the ->readahead operation is
called with the pages to readahead locked, so grabbing the inode's glock
can now deadlock with processes which are holding the inode glock while
trying to lock the same pages.

Fix this by taking the inode glock in the ->read_iter file and ->fault
vm operations.  To avoid taking the inode glock when the data is already
cached, the ->read_iter file operation first tries to read the data with
the IOCB_CACHED flag set.  If that fails, the inode glock is locked and
the operation is repeated without the IOCB_CACHED flag.

Signed-off-by: Andreas Gruenbacher 
---
 fs/gfs2/aops.c | 27 ++
 fs/gfs2/file.c | 61 --
 2 files changed, 61 insertions(+), 27 deletions(-)

diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
index 72c9560f4467..73c2fe768a3f 100644
--- a/fs/gfs2/aops.c
+++ b/fs/gfs2/aops.c
@@ -513,26 +513,10 @@ static int __gfs2_readpage(void *file, struct page *page)
 
 static int gfs2_readpage(struct file *file, struct page *page)
 {
-   struct address_space *mapping = page->mapping;
-   struct gfs2_inode *ip = GFS2_I(mapping->host);
-   struct gfs2_holder gh;
int error;
 
-   unlock_page(page);
-   gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, );
-   error = gfs2_glock_nq();
-   if (unlikely(error))
-   goto out;
-   error = AOP_TRUNCATED_PAGE;
-   lock_page(page);
-   if (page->mapping == mapping && !PageUptodate(page))
-   error = __gfs2_readpage(file, page);
-   else
-   unlock_page(page);
-   gfs2_glock_dq();
-out:
-   gfs2_holder_uninit();
-   if (error && error != AOP_TRUNCATED_PAGE)
+   error = __gfs2_readpage(file, page);
+   if (error)
lock_page(page);
return error;
 }
@@ -598,16 +582,9 @@ static void gfs2_readahead(struct readahead_control *rac)
 {
struct inode *inode = rac->mapping->host;
struct gfs2_inode *ip = GFS2_I(inode);
-   struct gfs2_holder gh;
 
-   gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, );
-   if (gfs2_glock_nq())
-   goto out_uninit;
if (!gfs2_is_stuffed(ip))
mpage_readahead(rac, gfs2_block_map);
-   gfs2_glock_dq();
-out_uninit:
-   gfs2_holder_uninit();
 }
 
 /**
diff --git a/fs/gfs2/file.c b/fs/gfs2/file.c
index fe305e4bfd37..f729b0ff2a3c 100644
--- a/fs/gfs2/file.c
+++ b/fs/gfs2/file.c
@@ -558,8 +558,29 @@ static vm_fault_t gfs2_page_mkwrite(struct vm_fault *vmf)
return block_page_mkwrite_return(ret);
 }
 
+static vm_fault_t gfs2_fault(struct vm_fault *vmf)
+{
+   struct inode *inode = file_inode(vmf->vma->vm_file);
+   struct gfs2_inode *ip = GFS2_I(inode);
+   struct gfs2_holder gh;
+   vm_fault_t ret;
+   int err;
+
+   gfs2_holder_init(ip->i_gl, LM_ST_SHARED, 0, );
+   err = gfs2_glock_nq();
+   if (err) {
+   ret = block_page_mkwrite_return(err);
+   goto out_uninit;
+   }
+   ret = filemap_fault(vmf);
+   gfs2_glock_dq();
+out_uninit:
+   gfs2_holder_uninit();
+   return ret;
+}
+
 static const struct vm_operations_struct gfs2_vm_ops = {
-   .fault = filemap_fault,
+   .fault = gfs2_fault,
.map_pages = filemap_map_pages,
.page_mkwrite = gfs2_page_mkwrite,
 };
@@ -824,15 +845,51 @@ static ssize_t gfs2_file_direct_write(struct kiocb *iocb, 
struct iov_iter *from)
 
 static ssize_t gfs2_file_read_iter(struct kiocb *iocb, struct iov_iter *to)
 {
+   struct gfs2_inode *ip;
+   struct gfs2_holder gh;
+   size_t written = 0;
ssize_t ret;
 
+   gfs2_holder_mark_uninitialized();
if (iocb->ki_flags & IOCB_DIRECT) {
ret = gfs2_file_direct_read(iocb, to);
if (likely(ret != -ENOTBLK))
return ret;
iocb->ki_flags &= ~IOCB_DIRECT;
}
-   return generic_file_read_iter(iocb, to);
+   iocb->ki_flags |= IOCB_CACHED;
+   ret = generic_file_read_iter(iocb, to);
+   iocb->ki_flags &= ~IOCB_CACHED;
+   if (ret >= 0) {
+   if (!iov_iter_count(to))
+   return ret;
+   written = ret;
+   } else {
+   switch(ret) {
+   case -EAGAIN:
+   if (iocb->ki_flags & IOCB_NOWAIT)
+   return ret;
+   break;
+   case 

[f2fs-dev] [PATCH 1/2] fs: Add IOCB_CACHED flag for generic_file_read_iter

2020-06-19 Thread Andreas Gruenbacher
Add an IOCB_CACHED flag which indicates to generic_file_read_iter that
it should only regard the page cache, without triggering any filesystem
I/O for the actual request or for readahead.  With this flag, -EAGAIN is
returned when regular I/O would be triggered similar to the IOCB_NOWAIT
flag, and -ECANCELED is returned when readahead would be triggered.

This allows the caller to perform a tentative read out of the page
cache, and to retry the read if the requested pages are not cached.

Please see the next commit for what this is used for.

Signed-off-by: Andreas Gruenbacher 
---
 include/linux/fs.h |  1 +
 mm/filemap.c   | 16 ++--
 2 files changed, 15 insertions(+), 2 deletions(-)

diff --git a/include/linux/fs.h b/include/linux/fs.h
index 6c4ab4dc1cd7..74eade571b1c 100644
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -315,6 +315,7 @@ enum rw_hint {
 #define IOCB_SYNC  (1 << 5)
 #define IOCB_WRITE (1 << 6)
 #define IOCB_NOWAIT(1 << 7)
+#define IOCB_CACHED(1 << 8)
 
 struct kiocb {
struct file *ki_filp;
diff --git a/mm/filemap.c b/mm/filemap.c
index f0ae9a6308cb..bd11f27bf6ae 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -2028,7 +2028,7 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
 
page = find_get_page(mapping, index);
if (!page) {
-   if (iocb->ki_flags & IOCB_NOWAIT)
+   if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_CACHED))
goto would_block;
page_cache_sync_readahead(mapping,
ra, filp,
@@ -2038,12 +2038,17 @@ ssize_t generic_file_buffered_read(struct kiocb *iocb,
goto no_cached_page;
}
if (PageReadahead(page)) {
+   if (iocb->ki_flags & IOCB_CACHED) {
+   put_page(page);
+   error = -ECANCELED;
+   goto out;
+   }
page_cache_async_readahead(mapping,
ra, filp, page,
index, last_index - index);
}
if (!PageUptodate(page)) {
-   if (iocb->ki_flags & IOCB_NOWAIT) {
+   if (iocb->ki_flags & (IOCB_NOWAIT | IOCB_CACHED)) {
put_page(page);
goto would_block;
}
@@ -2249,6 +2254,13 @@ EXPORT_SYMBOL_GPL(generic_file_buffered_read);
  *
  * This is the "read_iter()" routine for all filesystems
  * that can use the page cache directly.
+ *
+ * In the IOCB_NOWAIT flag in iocb->ki_flags indicates that -EAGAIN should be
+ * returned if completing the request would require I/O; this does not prevent
+ * readahead.  The IOCB_CACHED flag indicates that -EAGAIN should be returned
+ * as under the IOCB_NOWAIT flag, and that -ECANCELED should be returned when
+ * readhead would be triggered.
+ *
  * Return:
  * * number of bytes copied, even for partial reads
  * * negative error code if nothing was read
-- 
2.26.2



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [RFC PATCH 0/2] gfs2 readahead regression in v5.8-rc1

2020-06-19 Thread Andreas Gruenbacher
Hello,

can the two patches in this set still be considered for v5.8?

Commit d4388340ae0b ("fs: convert mpage_readpages to mpage_readahead")
which converts gfs2 and other filesystems to use the new ->readahead
address space operation is leading to deadlocks between the inode glocks
and page locks: ->readahead is called with the pages to readahead
already locked.  When gfs2_readahead then tries to lock the associated
inode glock, another process already holding the inode glock may be
trying to lock the same pages.

We could work around this in gfs by using a LM_FLAG_TRY lock in
->readahead for now.  The real reason for this deadlock is that gfs2
shouldn't be taking the inode glock in ->readahead in the first place
though, so I'd prefer to fix this "properly" instead.  Unfortunately,
this depends on a new IOCB_CACHED flag for generic_file_read_iter.

A previous version was posted in November:

https://lore.kernel.org/linux-fsdevel/20191122235324.17245-1-agrue...@redhat.com/

Thanks,
Andreas

Andreas Gruenbacher (2):
  fs: Add IOCB_CACHED flag for generic_file_read_iter
  gfs2: Rework read and page fault locking

 fs/gfs2/aops.c | 27 ++--
 fs/gfs2/file.c | 61 --
 include/linux/fs.h |  1 +
 mm/filemap.c   | 16 ++--
 4 files changed, 76 insertions(+), 29 deletions(-)


base-commit: af42d3466bdc8f39806b26f593604fdc54140bcb
-- 
2.26.2



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH v4] f2fs-tools: introduce set_node_footer to initialize node footer

2020-06-19 Thread zhaowuyun--- via Linux-f2fs-devel
From: Wuyun Zhao 

the filesystem will use the cold flag, so deal with it to avoid
potential issue of inconsistency

Signed-off-by: Wuyun Zhao 
Reviewed-by: Chao Yu 
---
 fsck/dir.c |  7 ++-
 fsck/node.c| 12 +---
 include/f2fs_fs.h  | 23 +++
 mkfs/f2fs_format.c | 34 +-
 4 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/fsck/dir.c b/fsck/dir.c
index 5f4f75e..b067aec 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -517,11 +517,8 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
}
 
set_file_temperature(sbi, node_blk, de->name);
-
-   node_blk->footer.ino = cpu_to_le32(de->ino);
-   node_blk->footer.nid = cpu_to_le32(de->ino);
-   node_blk->footer.flag = 0;
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, de->ino, de->ino, 0,
+   le64_to_cpu(ckpt->checkpoint_ver), 0, S_ISDIR(mode));
 
if (S_ISDIR(mode)) {
make_empty_dir(sbi, node_blk);
diff --git a/fsck/node.c b/fsck/node.c
index 229a99c..ef7ed0b 100644
--- a/fsck/node.c
+++ b/fsck/node.c
@@ -61,7 +61,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
 block_t new_node_block(struct f2fs_sb_info *sbi,
struct dnode_of_data *dn, unsigned int ofs)
 {
-   struct f2fs_node *f2fs_inode;
+   struct f2fs_node *f2fs_inode = dn->inode_blk;
struct f2fs_node *node_blk;
struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
struct f2fs_summary sum;
@@ -69,16 +69,14 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
block_t blkaddr = NULL_ADDR;
int type;
int ret;
-
-   f2fs_inode = dn->inode_blk;
+   nid_t ino = le32_to_cpu(f2fs_inode->footer.ino);
+   u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
 
node_blk = calloc(BLOCK_SZ, 1);
ASSERT(node_blk);
 
-   node_blk->footer.nid = cpu_to_le32(dn->nid);
-   node_blk->footer.ino = f2fs_inode->footer.ino;
-   node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver, 0,
+   S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
 
type = CURSEG_COLD_NODE;
if (IS_DNODE(node_blk)) {
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 709bfd8..ab19eb7 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -923,6 +923,29 @@ struct f2fs_node {
struct node_footer footer;
 } __attribute__((packed));
 
+static inline void set_cold_node(struct f2fs_node *rn, bool is_dir)
+{
+   unsigned int flag = le32_to_cpu(rn->footer.flag);
+
+   if (is_dir)
+   flag &= ~(0x1 << COLD_BIT_SHIFT);
+   else
+   flag |= (0x1 << COLD_BIT_SHIFT);
+   rn->footer.flag = cpu_to_le32(flag);
+}
+
+static inline void set_node_footer(struct f2fs_node *rn, nid_t nid, nid_t ino,
+   u32 ofs, u64 ver, block_t blkaddr,
+   bool is_dir)
+{
+   rn->footer.nid = cpu_to_le32(nid);
+   rn->footer.ino = cpu_to_le32(ino);
+   rn->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
+   rn->footer.cp_ver = cpu_to_le64(ver);
+   rn->footer.next_blkaddr = cpu_to_le32(blkaddr);
+   set_cold_node(rn, is_dir);
+}
+
 /*
  * For NAT entries
  */
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 44575e0..656023a 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -1094,6 +1094,9 @@ static int f2fs_write_root_inode(void)
struct f2fs_node *raw_node = NULL;
u_int64_t blk_size_bytes, data_blk_nor;
u_int64_t main_area_node_seg_blk_offset = 0;
+   nid_t nid = le32_to_cpu(sb->root_ino);
+   block_t blkaddr = get_sb(main_blkaddr) +
+   c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + 1;
 
raw_node = calloc(F2FS_BLKSIZE, 1);
if (raw_node == NULL) {
@@ -1101,13 +1104,7 @@ static int f2fs_write_root_inode(void)
return -1;
}
 
-   raw_node->footer.nid = sb->root_ino;
-   raw_node->footer.ino = sb->root_ino;
-   raw_node->footer.cp_ver = cpu_to_le64(1);
-   raw_node->footer.next_blkaddr = cpu_to_le32(
-   get_sb(main_blkaddr) +
-   c.cur_seg[CURSEG_HOT_NODE] *
-   c.blks_per_seg + 1);
+   set_node_footer(raw_node, nid, nid, 0, 1, blkaddr, 1);
 
raw_node->i.i_mode = cpu_to_le16(0x41ed);
if (c.lpf_ino)
@@ -1256,6 +1253,10 @@ static int f2fs_write_qf_inode(int qtype)
u_int64_t main_area_node_seg_blk_offset = 0;
__le32 raw_id;
int i;
+   nid_t qf_ino = le32_to_cpu(sb->qf_ino[qtype]);
+   block_t blkaddr = get_sb(main_blkaddr) +
+   c.cur_seg[CURSEG_HOT_NODE] *
+   

Re: [f2fs-dev] [PATCH v3] f2fs-tools: introduce set_node_footer to initialize node footer

2020-06-19 Thread Chao Yu
On 2020/6/19 16:45, zhaowu...@wingtech.com wrote:
> From: Wuyun Zhao 
> 
> the filesystem will use the cold flag, so deal with it to avoid
> potential issue of inconsistency
> 
> Signed-off-by: Wuyun Zhao 

Reviewed-by: Chao Yu 

Thanks,


___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] [PATCH] f2fs: fix to check page dirty status before writeback

2020-06-19 Thread Chao Yu
In f2fs_write_raw_pages(), we need to check page dirty status before
writeback, because there could be a racer (e.g. reclaimer) helps
writebacking the dirty page.

Signed-off-by: Chao Yu 
---
 fs/f2fs/compress.c | 6 ++
 1 file changed, 6 insertions(+)

diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
index 5643aa2b8377..ce5645b5c833 100644
--- a/fs/f2fs/compress.c
+++ b/fs/f2fs/compress.c
@@ -1322,6 +1322,12 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
congestion_wait(BLK_RW_ASYNC,
DEFAULT_IO_TIMEOUT);
lock_page(cc->rpages[i]);
+
+   if (!PageDirty(cc->rpages[i])) {
+   unlock_page(cc->rpages[i]);
+   continue;
+   }
+
clear_page_dirty_for_io(cc->rpages[i]);
goto retry_write;
}
-- 
2.26.2



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel


[f2fs-dev] 回复: [PATCH v2] f2fs-tools: introduce set_node_footer to initialize node footer

2020-06-19 Thread Zac via Linux-f2fs-devel
> On 2020/6/19 14:37, zhaowu...@wingtech.com wrote:
> > From: Wuyun Zhao 
> >
> > the filesystem will use the cold flag, so deal with it to avoid
> > potential issue of inconsistency
> >
> > Signed-off-by: Wuyun Zhao 
> > ---
> >  fsck/dir.c |  6 +-
> >  fsck/node.c|  9 +
> >  include/f2fs_fs.h  | 20 
> >  mkfs/f2fs_format.c | 14 +-
> >  4 files changed, 31 insertions(+), 18 deletions(-)
> >
> > diff --git a/fsck/dir.c b/fsck/dir.c
> > index 5f4f75e..64aa539 100644
> > --- a/fsck/dir.c
> > +++ b/fsck/dir.c
> > @@ -517,11 +517,7 @@ static void init_inode_block(struct f2fs_sb_info
> *sbi,
> > }
> >
> > set_file_temperature(sbi, node_blk, de->name);
> > -
> > -   node_blk->footer.ino = cpu_to_le32(de->ino);
> > -   node_blk->footer.nid = cpu_to_le32(de->ino);
> > -   node_blk->footer.flag = 0;
> > -   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> > +   set_node_footer(node_blk, de->ino, de->ino, 0,
> le64_to_cpu(ckpt->checkpoint_ver), S_ISDIR(mode));
> >
> > if (S_ISDIR(mode)) {
> > make_empty_dir(sbi, node_blk);
> > diff --git a/fsck/node.c b/fsck/node.c
> > index 229a99c..66e8a81 100644
> > --- a/fsck/node.c
> > +++ b/fsck/node.c
> > @@ -69,16 +69,17 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
> > block_t blkaddr = NULL_ADDR;
> > int type;
> > int ret;
> > +   u32 ino;
> 
> nid_t ino;
> 
> > +   u64 cp_ver;
> >
> > f2fs_inode = dn->inode_blk;
> >
> > node_blk = calloc(BLOCK_SZ, 1);
> > ASSERT(node_blk);
> >
> > -   node_blk->footer.nid = cpu_to_le32(dn->nid);
> > -   node_blk->footer.ino = f2fs_inode->footer.ino;
> > -   node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
> > -   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> > +   ino = le32_to_cpu(f2fs_inode->footer.ino);
> > +   cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
> > +   set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver,
> S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
> >
> > type = CURSEG_COLD_NODE;
> > if (IS_DNODE(node_blk)) {
> > diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> > index 709bfd8..3583df4 100644
> > --- a/include/f2fs_fs.h
> > +++ b/include/f2fs_fs.h
> > @@ -923,6 +923,26 @@ struct f2fs_node {
> > struct node_footer footer;
> >  } __attribute__((packed));
> >
> > +static inline void set_cold_node(struct f2fs_node *rn, bool is_dir)
> > +{
> > +   unsigned int flag = le32_to_cpu(rn->footer.flag);
> > +
> > +   if (is_dir)
> > +   flag &= ~(0x1 << COLD_BIT_SHIFT);
> > +   else
> > +   flag |= (0x1 << COLD_BIT_SHIFT);
> > +   rn->footer.flag = cpu_to_le32(flag);
> > +}
> > +
> > +static inline void set_node_footer(struct f2fs_node *rn, u32 nid, u32
ino,
> u32 ofs, u64 ver, bool is_dir)
> 
> Sorry, I forgot to add parameter for next_blkaddr...
> 
> Could you check this?

Sent v3 patch, please help to review, thanks.

> From: Wuyun Zhao 
> Subject: [PATCH] f2fs-tools: introduce set_node_footer to initialize node
>  footer
> 
> the filesystem will use the cold flag, so deal with it to avoid
> potential issue of inconsistency
> 
> Signed-off-by: Wuyun Zhao 
> ---
>  fsck/dir.c |  7 ++-
>  fsck/node.c|  8 
>  include/f2fs_fs.h  | 23 +++
>  mkfs/f2fs_format.c | 34 +-
>  4 files changed, 42 insertions(+), 30 deletions(-)
> 
> diff --git a/fsck/dir.c b/fsck/dir.c
> index 5f4f75ebed77..b067aec9cffd 100644
> --- a/fsck/dir.c
> +++ b/fsck/dir.c
> @@ -517,11 +517,8 @@ static void init_inode_block(struct f2fs_sb_info
> *sbi,
>   }
> 
>   set_file_temperature(sbi, node_blk, de->name);
> -
> - node_blk->footer.ino = cpu_to_le32(de->ino);
> - node_blk->footer.nid = cpu_to_le32(de->ino);
> - node_blk->footer.flag = 0;
> - node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> + set_node_footer(node_blk, de->ino, de->ino, 0,
> + le64_to_cpu(ckpt->checkpoint_ver), 0,
S_ISDIR(mode));
> 
>   if (S_ISDIR(mode)) {
>   make_empty_dir(sbi, node_blk);
> diff --git a/fsck/node.c b/fsck/node.c
> index 229a99cab481..da010ff669cc 100644
> --- a/fsck/node.c
> +++ b/fsck/node.c
> @@ -69,16 +69,16 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
>   block_t blkaddr = NULL_ADDR;
>   int type;
>   int ret;
> + nid_t ino = le32_to_cpu(f2fs_inode->footer.ino);
> + u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
> 
>   f2fs_inode = dn->inode_blk;
> 
>   node_blk = calloc(BLOCK_SZ, 1);
>   ASSERT(node_blk);
> 
> - node_blk->footer.nid = cpu_to_le32(dn->nid);
> - node_blk->footer.ino = f2fs_inode->footer.ino;
> - node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
> - node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> + set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver, 0,
> + S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
> 
>   type = CURSEG_COLD_NODE;
>   if 

[f2fs-dev] [PATCH v3] f2fs-tools: introduce set_node_footer to initialize node footer

2020-06-19 Thread zhaowuyun--- via Linux-f2fs-devel
From: Wuyun Zhao 

the filesystem will use the cold flag, so deal with it to avoid
potential issue of inconsistency

Signed-off-by: Wuyun Zhao 
---
 fsck/dir.c |  7 ++-
 fsck/node.c| 12 +---
 include/f2fs_fs.h  | 23 +++
 mkfs/f2fs_format.c | 34 +-
 4 files changed, 43 insertions(+), 33 deletions(-)

diff --git a/fsck/dir.c b/fsck/dir.c
index 5f4f75e..b067aec 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -517,11 +517,8 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
}
 
set_file_temperature(sbi, node_blk, de->name);
-
-   node_blk->footer.ino = cpu_to_le32(de->ino);
-   node_blk->footer.nid = cpu_to_le32(de->ino);
-   node_blk->footer.flag = 0;
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, de->ino, de->ino, 0,
+   le64_to_cpu(ckpt->checkpoint_ver), 0, S_ISDIR(mode));
 
if (S_ISDIR(mode)) {
make_empty_dir(sbi, node_blk);
diff --git a/fsck/node.c b/fsck/node.c
index 229a99c..ef7ed0b 100644
--- a/fsck/node.c
+++ b/fsck/node.c
@@ -61,7 +61,7 @@ void set_data_blkaddr(struct dnode_of_data *dn)
 block_t new_node_block(struct f2fs_sb_info *sbi,
struct dnode_of_data *dn, unsigned int ofs)
 {
-   struct f2fs_node *f2fs_inode;
+   struct f2fs_node *f2fs_inode = dn->inode_blk;
struct f2fs_node *node_blk;
struct f2fs_checkpoint *ckpt = F2FS_CKPT(sbi);
struct f2fs_summary sum;
@@ -69,16 +69,14 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
block_t blkaddr = NULL_ADDR;
int type;
int ret;
-
-   f2fs_inode = dn->inode_blk;
+   nid_t ino = le32_to_cpu(f2fs_inode->footer.ino);
+   u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
 
node_blk = calloc(BLOCK_SZ, 1);
ASSERT(node_blk);
 
-   node_blk->footer.nid = cpu_to_le32(dn->nid);
-   node_blk->footer.ino = f2fs_inode->footer.ino;
-   node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver, 0,
+   S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
 
type = CURSEG_COLD_NODE;
if (IS_DNODE(node_blk)) {
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 709bfd8..ab19eb7 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -923,6 +923,29 @@ struct f2fs_node {
struct node_footer footer;
 } __attribute__((packed));
 
+static inline void set_cold_node(struct f2fs_node *rn, bool is_dir)
+{
+   unsigned int flag = le32_to_cpu(rn->footer.flag);
+
+   if (is_dir)
+   flag &= ~(0x1 << COLD_BIT_SHIFT);
+   else
+   flag |= (0x1 << COLD_BIT_SHIFT);
+   rn->footer.flag = cpu_to_le32(flag);
+}
+
+static inline void set_node_footer(struct f2fs_node *rn, nid_t nid, nid_t ino,
+   u32 ofs, u64 ver, block_t blkaddr,
+   bool is_dir)
+{
+   rn->footer.nid = cpu_to_le32(nid);
+   rn->footer.ino = cpu_to_le32(ino);
+   rn->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
+   rn->footer.cp_ver = cpu_to_le64(ver);
+   rn->footer.next_blkaddr = cpu_to_le32(blkaddr);
+   set_cold_node(rn, is_dir);
+}
+
 /*
  * For NAT entries
  */
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 44575e0..656023a 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -1094,6 +1094,9 @@ static int f2fs_write_root_inode(void)
struct f2fs_node *raw_node = NULL;
u_int64_t blk_size_bytes, data_blk_nor;
u_int64_t main_area_node_seg_blk_offset = 0;
+   nid_t nid = le32_to_cpu(sb->root_ino);
+   block_t blkaddr = get_sb(main_blkaddr) +
+   c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg + 1;
 
raw_node = calloc(F2FS_BLKSIZE, 1);
if (raw_node == NULL) {
@@ -1101,13 +1104,7 @@ static int f2fs_write_root_inode(void)
return -1;
}
 
-   raw_node->footer.nid = sb->root_ino;
-   raw_node->footer.ino = sb->root_ino;
-   raw_node->footer.cp_ver = cpu_to_le64(1);
-   raw_node->footer.next_blkaddr = cpu_to_le32(
-   get_sb(main_blkaddr) +
-   c.cur_seg[CURSEG_HOT_NODE] *
-   c.blks_per_seg + 1);
+   set_node_footer(raw_node, nid, nid, 0, 1, blkaddr, 1);
 
raw_node->i.i_mode = cpu_to_le16(0x41ed);
if (c.lpf_ino)
@@ -1256,6 +1253,10 @@ static int f2fs_write_qf_inode(int qtype)
u_int64_t main_area_node_seg_blk_offset = 0;
__le32 raw_id;
int i;
+   nid_t qf_ino = le32_to_cpu(sb->qf_ino[qtype]);
+   block_t blkaddr = get_sb(main_blkaddr) +
+   c.cur_seg[CURSEG_HOT_NODE] *
+   c.blks_per_seg + 1 + qtype + 

Re: [f2fs-dev] [PATCH v2] f2fs-tools: introduce set_node_footer to initialize node footer

2020-06-19 Thread Chao Yu
On 2020/6/19 14:37, zhaowu...@wingtech.com wrote:
> From: Wuyun Zhao 
> 
> the filesystem will use the cold flag, so deal with it to avoid
> potential issue of inconsistency
> 
> Signed-off-by: Wuyun Zhao 
> ---
>  fsck/dir.c |  6 +-
>  fsck/node.c|  9 +
>  include/f2fs_fs.h  | 20 
>  mkfs/f2fs_format.c | 14 +-
>  4 files changed, 31 insertions(+), 18 deletions(-)
> 
> diff --git a/fsck/dir.c b/fsck/dir.c
> index 5f4f75e..64aa539 100644
> --- a/fsck/dir.c
> +++ b/fsck/dir.c
> @@ -517,11 +517,7 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
>   }
>  
>   set_file_temperature(sbi, node_blk, de->name);
> -
> - node_blk->footer.ino = cpu_to_le32(de->ino);
> - node_blk->footer.nid = cpu_to_le32(de->ino);
> - node_blk->footer.flag = 0;
> - node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> + set_node_footer(node_blk, de->ino, de->ino, 0, 
> le64_to_cpu(ckpt->checkpoint_ver), S_ISDIR(mode));
>  
>   if (S_ISDIR(mode)) {
>   make_empty_dir(sbi, node_blk);
> diff --git a/fsck/node.c b/fsck/node.c
> index 229a99c..66e8a81 100644
> --- a/fsck/node.c
> +++ b/fsck/node.c
> @@ -69,16 +69,17 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
>   block_t blkaddr = NULL_ADDR;
>   int type;
>   int ret;
> + u32 ino;

nid_t ino;

> + u64 cp_ver;
>  
>   f2fs_inode = dn->inode_blk;
>  
>   node_blk = calloc(BLOCK_SZ, 1);
>   ASSERT(node_blk);
>  
> - node_blk->footer.nid = cpu_to_le32(dn->nid);
> - node_blk->footer.ino = f2fs_inode->footer.ino;
> - node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
> - node_blk->footer.cp_ver = ckpt->checkpoint_ver;
> + ino = le32_to_cpu(f2fs_inode->footer.ino);
> + cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
> + set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver, 
> S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
>  
>   type = CURSEG_COLD_NODE;
>   if (IS_DNODE(node_blk)) {
> diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
> index 709bfd8..3583df4 100644
> --- a/include/f2fs_fs.h
> +++ b/include/f2fs_fs.h
> @@ -923,6 +923,26 @@ struct f2fs_node {
>   struct node_footer footer;
>  } __attribute__((packed));
>  
> +static inline void set_cold_node(struct f2fs_node *rn, bool is_dir)
> +{
> + unsigned int flag = le32_to_cpu(rn->footer.flag);
> +
> + if (is_dir)
> + flag &= ~(0x1 << COLD_BIT_SHIFT);
> + else
> + flag |= (0x1 << COLD_BIT_SHIFT);
> + rn->footer.flag = cpu_to_le32(flag);
> +}
> +
> +static inline void set_node_footer(struct f2fs_node *rn, u32 nid, u32 ino, 
> u32 ofs, u64 ver, bool is_dir)

Sorry, I forgot to add parameter for next_blkaddr...

Could you check this?

From: Wuyun Zhao 
Subject: [PATCH] f2fs-tools: introduce set_node_footer to initialize node
 footer

the filesystem will use the cold flag, so deal with it to avoid
potential issue of inconsistency

Signed-off-by: Wuyun Zhao 
---
 fsck/dir.c |  7 ++-
 fsck/node.c|  8 
 include/f2fs_fs.h  | 23 +++
 mkfs/f2fs_format.c | 34 +-
 4 files changed, 42 insertions(+), 30 deletions(-)

diff --git a/fsck/dir.c b/fsck/dir.c
index 5f4f75ebed77..b067aec9cffd 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -517,11 +517,8 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
}

set_file_temperature(sbi, node_blk, de->name);
-
-   node_blk->footer.ino = cpu_to_le32(de->ino);
-   node_blk->footer.nid = cpu_to_le32(de->ino);
-   node_blk->footer.flag = 0;
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, de->ino, de->ino, 0,
+   le64_to_cpu(ckpt->checkpoint_ver), 0, S_ISDIR(mode));

if (S_ISDIR(mode)) {
make_empty_dir(sbi, node_blk);
diff --git a/fsck/node.c b/fsck/node.c
index 229a99cab481..da010ff669cc 100644
--- a/fsck/node.c
+++ b/fsck/node.c
@@ -69,16 +69,16 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
block_t blkaddr = NULL_ADDR;
int type;
int ret;
+   nid_t ino = le32_to_cpu(f2fs_inode->footer.ino);
+   u64 cp_ver = le64_to_cpu(ckpt->checkpoint_ver);

f2fs_inode = dn->inode_blk;

node_blk = calloc(BLOCK_SZ, 1);
ASSERT(node_blk);

-   node_blk->footer.nid = cpu_to_le32(dn->nid);
-   node_blk->footer.ino = f2fs_inode->footer.ino;
-   node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver, 0,
+   S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));

type = CURSEG_COLD_NODE;
if (IS_DNODE(node_blk)) {
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 265f50c69fd1..0fe478ed820c 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -923,6 +923,29 @@ struct f2fs_node {
struct 

Re: [f2fs-dev] [PATCH 1/5] f2fs: fix to wait page writeback before update

2020-06-19 Thread Chao Yu
On 2020/6/19 13:49, Jaegeuk Kim wrote:
> On 06/19, Chao Yu wrote:
>> Hi Jaegeuk,
>>
>> On 2020/6/19 7:59, Jaegeuk Kim wrote:
>>> Hi Chao,
>>>
>>> On 06/18, Chao Yu wrote:
 to make page content stable for special device like raid.
>>>
>>> Could you elaborate the problem a bit?
>>
>> Some devices like raid5 wants page content to be stable, because
>> it will calculate parity info based page content, if page is not
>> stable, parity info could be corrupted, result in data inconsistency
>> in stripe.
> 
> I don't get the point, since those pages are brand new pages which were not
> modified before. If it's on writeback, we should not modify them regardless
> of whatever raid configuration. For example, f2fs_new_node_page() waits for
> writeback. Am I missing something?

I think we should use f2fs_bug_on(, PageWriteback()) rather than
f2fs_wait_on_page_writeback() for brand new page which is allocated just now.
For other paths, we can keep rule that waiting for writeback before updating.

How do you think?

Thanks,

> 
>>
>> Thanks,
>>
>>>

 Signed-off-by: Chao Yu 
 ---
  fs/f2fs/dir.c  |  2 ++
  fs/f2fs/extent_cache.c | 18 +-
  fs/f2fs/f2fs.h |  2 +-
  fs/f2fs/file.c |  1 +
  fs/f2fs/inline.c   |  2 ++
  fs/f2fs/inode.c|  3 +--
  6 files changed, 16 insertions(+), 12 deletions(-)

 diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
 index d35976785e8c..91e86747a604 100644
 --- a/fs/f2fs/dir.c
 +++ b/fs/f2fs/dir.c
 @@ -495,6 +495,8 @@ static int make_empty_dir(struct inode *inode,
if (IS_ERR(dentry_page))
return PTR_ERR(dentry_page);
  
 +  f2fs_bug_on(F2FS_I_SB(inode), PageWriteback(dentry_page));
 +
dentry_blk = page_address(dentry_page);
  
make_dentry_ptr_block(NULL, , dentry_blk);
 diff --git a/fs/f2fs/extent_cache.c b/fs/f2fs/extent_cache.c
 index e60078460ad1..686c68b98610 100644
 --- a/fs/f2fs/extent_cache.c
 +++ b/fs/f2fs/extent_cache.c
 @@ -325,9 +325,10 @@ static void __drop_largest_extent(struct extent_tree 
 *et,
  }
  
  /* return true, if inode page is changed */
 -static bool __f2fs_init_extent_tree(struct inode *inode, struct 
 f2fs_extent *i_ext)
 +static void __f2fs_init_extent_tree(struct inode *inode, struct page 
 *ipage)
  {
struct f2fs_sb_info *sbi = F2FS_I_SB(inode);
 +  struct f2fs_extent *i_ext = ipage ? _INODE(ipage)->i_ext : NULL;
struct extent_tree *et;
struct extent_node *en;
struct extent_info ei;
 @@ -335,16 +336,18 @@ static bool __f2fs_init_extent_tree(struct inode 
 *inode, struct f2fs_extent *i_e
if (!f2fs_may_extent_tree(inode)) {
/* drop largest extent */
if (i_ext && i_ext->len) {
 +  f2fs_wait_on_page_writeback(ipage, NODE, true, true);
i_ext->len = 0;
 -  return true;
 +  set_page_dirty(ipage);
 +  return;
}
 -  return false;
 +  return;
}
  
et = __grab_extent_tree(inode);
  
if (!i_ext || !i_ext->len)
 -  return false;
 +  return;
  
get_extent_info(, i_ext);
  
 @@ -360,17 +363,14 @@ static bool __f2fs_init_extent_tree(struct inode 
 *inode, struct f2fs_extent *i_e
}
  out:
write_unlock(>lock);
 -  return false;
  }
  
 -bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent *i_ext)
 +void f2fs_init_extent_tree(struct inode *inode, struct page *ipage)
  {
 -  bool ret =  __f2fs_init_extent_tree(inode, i_ext);
 +  __f2fs_init_extent_tree(inode, ipage);
  
if (!F2FS_I(inode)->extent_tree)
set_inode_flag(inode, FI_NO_EXTENT);
 -
 -  return ret;
  }
  
  static bool f2fs_lookup_extent_tree(struct inode *inode, pgoff_t pgofs,
 diff --git a/fs/f2fs/f2fs.h b/fs/f2fs/f2fs.h
 index b35a50f4953c..326c12fa0da5 100644
 --- a/fs/f2fs/f2fs.h
 +++ b/fs/f2fs/f2fs.h
 @@ -3795,7 +3795,7 @@ struct rb_entry *f2fs_lookup_rb_tree_ret(struct 
 rb_root_cached *root,
  bool f2fs_check_rb_tree_consistence(struct f2fs_sb_info *sbi,
struct rb_root_cached *root);
  unsigned int f2fs_shrink_extent_tree(struct f2fs_sb_info *sbi, int 
 nr_shrink);
 -bool f2fs_init_extent_tree(struct inode *inode, struct f2fs_extent 
 *i_ext);
 +void f2fs_init_extent_tree(struct inode *inode, struct page *ipage);
  void f2fs_drop_extent_tree(struct inode *inode);
  unsigned int f2fs_destroy_extent_node(struct inode *inode);
  void f2fs_destroy_extent_tree(struct inode *inode);
 diff --git a/fs/f2fs/file.c b/fs/f2fs/file.c
 index 3268f8dd59bb..1862073b96d2 100644
 

Re: [f2fs-dev] [PATCH 3/4] f2fs: add inline encryption support

2020-06-19 Thread Chao Yu
On 2020/6/19 12:20, Eric Biggers wrote:
> On Fri, Jun 19, 2020 at 10:39:34AM +0800, Chao Yu wrote:
>> Hi Eric,
>>
>> On 2020/6/19 2:13, Eric Biggers wrote:
>>> Hi Chao,
>>>
>>> On Thu, Jun 18, 2020 at 06:06:02PM +0800, Chao Yu wrote:
> @@ -936,8 +972,11 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>  
>   inc_page_count(sbi, WB_DATA_TYPE(bio_page));
>  
> - if (io->bio && !io_is_mergeable(sbi, io->bio, io, fio,
> - io->last_block_in_bio, fio->new_blkaddr))
> + if (io->bio &&
> + (!io_is_mergeable(sbi, io->bio, io, fio, io->last_block_in_bio,
> +   fio->new_blkaddr) ||
> +  !f2fs_crypt_mergeable_bio(io->bio, fio->page->mapping->host,
> +fio->page->index, fio)))

 bio_page->index, fio)))

>   __submit_merged_bio(io);
>  alloc_new:
>   if (io->bio == NULL) {
> @@ -949,6 +988,8 @@ void f2fs_submit_page_write(struct f2fs_io_info *fio)
>   goto skip;
>   }
>   io->bio = __bio_alloc(fio, BIO_MAX_PAGES);
> + f2fs_set_bio_crypt_ctx(io->bio, fio->page->mapping->host,
> +fio->page->index, fio, GFP_NOIO);

 bio_page->index, fio, GFP_NOIO);

>>>
>>> We're using ->mapping->host and ->index.  Ordinarily that would mean the 
>>> page
>>> needs to be a pagecache page.  But bio_page can also be a compressed page 
>>> or a
>>> bounce page containing fs-layer encrypted contents.
>>
>> I'm concerning about compression + inlinecrypt case.
>>
>>>
>>> Is your suggestion to keep using fio->page->mapping->host (since encrypted 
>>> pages
>>
>> Yup,
>>
>>> don't have a mapping), but start using bio_page->index (since f2fs 
>>> apparently
>>
>> I meant that we need to use bio_page->index as tweak value in write path to
>> keep consistent as we did in read path, otherwise we may read the wrong
>> decrypted data later to incorrect tweak value.
>>
>> - f2fs_read_multi_pages (only comes from compression inode)
>>  - f2fs_alloc_dic
>>   - f2fs_set_compressed_page(page, cc->inode,
>>  start_idx + i + 1, dic);
>> ^
>>   - dic->cpages[i] = page;
>>  - for ()
>>  struct page *page = dic->cpages[i];
>>  if (!bio)
>>- f2fs_grab_read_bio(..., page->index,..)
>> - f2fs_set_bio_crypt_ctx(..., first_idx, ..)   /* first_idx == 
>> cpage->index */
>>
>> You can see that cpage->index was set to page->index + 1, that's why we need
>> to use one of cpage->index/page->index as tweak value all the time rather 
>> than
>> using both index mixed in read/write path.
>>
>> But note that for fs-layer encryption, we have used cpage->index as tweak 
>> value,
>> so here I suggest we can keep consistent to use cpage->index in inlinecrypt 
>> case.
> 
> Yes, inlinecrypt mustn't change the ciphertext that gets written to disk.
> 
>>
>>> *does* set ->index for compressed pages, and if the file uses fs-layer
>>> encryption then f2fs_set_bio_crypt_ctx() won't use the index anyway)?
>>>
>>> Does this mean the code is currently broken for compression + inline 
>>> encryption
>>> because it's using the wrong ->index?  I think the answer is no, since
>>
>> I guess it's broken now for compression + inlinecrypt case.
>>
>>> f2fs_write_compressed_pages() will still pass the first 'nr_cpages' 
>>> pagecache
>>> pages along with the compressed pages.  In that case, your suggestion would 
>>> be a
>>> cleanup rather than a fix?
>>
>> That's a fix.
>>
> 
> FWIW, I tested this, and it actually works both before and after your 
> suggested
> change.  The reason is that f2fs_write_compressed_pages() actually passes the
> pagecache pages sequentially starting at 'start_idx_of_cluster(cc) + 1' for 
> the
> length of the compressed cluster.  That matches the '+ 1' adjustment 
> elsewhere,
> so we have fio->page->index == bio_page->index.

I've checked the code, yes, that's correct.

> 
> I personally think the way the f2fs compression code works is really 
> confusing.
> Compressed pages don't have a 1:1 correspondence to pagecache pages, so there
> should *not* be a pagecache page passed around when writing a compressed page.

The only place we always use fio->page is:
- f2fs_submit_page_write
 - trace_f2fs_submit_page_write(fio->page,..)
  - f2fs__submit_page_bio
   __entry->dev = page_file_mapping(page)->host->i_sb->s_dev;
   __entry->ino = page_file_mapping(page)->host->i_ino;

For compression case, we can get rid of using this parameter because bio_page
(fio->compressed_page) has correct mapping info, however for fs-layer encryption
case, bio_page (fio->encrypted_page, allocated by fscrypt_alloc_bounce_page())
has not correct mapping info.

> 
> Anyway, here's the test script I used in case anyone else wants to use it.  
> But
> we really need to write a proper f2fs compression + 

[f2fs-dev] [PATCH v2] f2fs-tools: introduce set_node_footer to initialize node footer

2020-06-19 Thread zhaowuyun--- via Linux-f2fs-devel
From: Wuyun Zhao 

the filesystem will use the cold flag, so deal with it to avoid
potential issue of inconsistency

Signed-off-by: Wuyun Zhao 
---
 fsck/dir.c |  6 +-
 fsck/node.c|  9 +
 include/f2fs_fs.h  | 20 
 mkfs/f2fs_format.c | 14 +-
 4 files changed, 31 insertions(+), 18 deletions(-)

diff --git a/fsck/dir.c b/fsck/dir.c
index 5f4f75e..64aa539 100644
--- a/fsck/dir.c
+++ b/fsck/dir.c
@@ -517,11 +517,7 @@ static void init_inode_block(struct f2fs_sb_info *sbi,
}
 
set_file_temperature(sbi, node_blk, de->name);
-
-   node_blk->footer.ino = cpu_to_le32(de->ino);
-   node_blk->footer.nid = cpu_to_le32(de->ino);
-   node_blk->footer.flag = 0;
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   set_node_footer(node_blk, de->ino, de->ino, 0, 
le64_to_cpu(ckpt->checkpoint_ver), S_ISDIR(mode));
 
if (S_ISDIR(mode)) {
make_empty_dir(sbi, node_blk);
diff --git a/fsck/node.c b/fsck/node.c
index 229a99c..66e8a81 100644
--- a/fsck/node.c
+++ b/fsck/node.c
@@ -69,16 +69,17 @@ block_t new_node_block(struct f2fs_sb_info *sbi,
block_t blkaddr = NULL_ADDR;
int type;
int ret;
+   u32 ino;
+   u64 cp_ver;
 
f2fs_inode = dn->inode_blk;
 
node_blk = calloc(BLOCK_SZ, 1);
ASSERT(node_blk);
 
-   node_blk->footer.nid = cpu_to_le32(dn->nid);
-   node_blk->footer.ino = f2fs_inode->footer.ino;
-   node_blk->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
-   node_blk->footer.cp_ver = ckpt->checkpoint_ver;
+   ino = le32_to_cpu(f2fs_inode->footer.ino);
+   cp_ver = le64_to_cpu(ckpt->checkpoint_ver);
+   set_node_footer(node_blk, dn->nid, ino, ofs, cp_ver, 
S_ISDIR(le16_to_cpu(f2fs_inode->i.i_mode)));
 
type = CURSEG_COLD_NODE;
if (IS_DNODE(node_blk)) {
diff --git a/include/f2fs_fs.h b/include/f2fs_fs.h
index 709bfd8..3583df4 100644
--- a/include/f2fs_fs.h
+++ b/include/f2fs_fs.h
@@ -923,6 +923,26 @@ struct f2fs_node {
struct node_footer footer;
 } __attribute__((packed));
 
+static inline void set_cold_node(struct f2fs_node *rn, bool is_dir)
+{
+   unsigned int flag = le32_to_cpu(rn->footer.flag);
+
+   if (is_dir)
+   flag &= ~(0x1 << COLD_BIT_SHIFT);
+   else
+   flag |= (0x1 << COLD_BIT_SHIFT);
+   rn->footer.flag = cpu_to_le32(flag);
+}
+
+static inline void set_node_footer(struct f2fs_node *rn, u32 nid, u32 ino, u32 
ofs, u64 ver, bool is_dir)
+{
+   rn->footer.nid = cpu_to_le32(nid);
+   rn->footer.ino = cpu_to_le32(ino);
+   rn->footer.flag = cpu_to_le32(ofs << OFFSET_BIT_SHIFT);
+   rn->footer.cp_ver = cpu_to_le64(ver);
+   set_cold_node(rn, is_dir);
+}
+
 /*
  * For NAT entries
  */
diff --git a/mkfs/f2fs_format.c b/mkfs/f2fs_format.c
index 44575e0..b371adf 100644
--- a/mkfs/f2fs_format.c
+++ b/mkfs/f2fs_format.c
@@ -1101,9 +1101,7 @@ static int f2fs_write_root_inode(void)
return -1;
}
 
-   raw_node->footer.nid = sb->root_ino;
-   raw_node->footer.ino = sb->root_ino;
-   raw_node->footer.cp_ver = cpu_to_le64(1);
+   set_node_footer(raw_node, le32_to_cpu(sb->root_ino), 
le32_to_cpu(sb->root_ino), 0, 1, 1);
raw_node->footer.next_blkaddr = cpu_to_le32(
get_sb(main_blkaddr) +
c.cur_seg[CURSEG_HOT_NODE] *
@@ -1256,6 +1254,7 @@ static int f2fs_write_qf_inode(int qtype)
u_int64_t main_area_node_seg_blk_offset = 0;
__le32 raw_id;
int i;
+   u32 qf_ino;
 
raw_node = calloc(F2FS_BLKSIZE, 1);
if (raw_node == NULL) {
@@ -1263,9 +1262,8 @@ static int f2fs_write_qf_inode(int qtype)
return -1;
}
 
-   raw_node->footer.nid = sb->qf_ino[qtype];
-   raw_node->footer.ino = sb->qf_ino[qtype];
-   raw_node->footer.cp_ver = cpu_to_le64(1);
+   qf_ino = le32_to_cpu(sb->qf_ino[qtype]);
+   set_node_footer(raw_node, qf_ino, qf_ino, 0, 1, 0);
raw_node->footer.next_blkaddr = cpu_to_le32(
get_sb(main_blkaddr) +
c.cur_seg[CURSEG_HOT_NODE] *
@@ -1457,9 +1455,7 @@ static int f2fs_write_lpf_inode(void)
return -1;
}
 
-   raw_node->footer.nid = cpu_to_le32(c.lpf_ino);
-   raw_node->footer.ino = raw_node->footer.nid;
-   raw_node->footer.cp_ver = cpu_to_le64(1);
+   set_node_footer(raw_node, c.lpf_ino, c.lpf_ino, 0, 1, 1);
raw_node->footer.next_blkaddr = cpu_to_le32(
get_sb(main_blkaddr) +
c.cur_seg[CURSEG_HOT_NODE] * c.blks_per_seg +
-- 
2.7.4



___
Linux-f2fs-devel mailing list
Linux-f2fs-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel