Re: [f2fs-dev] [PATCH 1/1] f2fs: fix to check space of current segment journal

[Wu Bo]

On 2022/9/14 16:08, Philippe De Muyter wrote:
> Hello Wu Bo,
>
> On Wed, Sep 14, 2022 at 12:04:23PM +0800, Wu Bo wrote:
>> As Philippe De Muyter reported:
>> https://lore.kernel.org/linux-f2fs-devel/20220913224908.GA25100@172.21.0.10/T/#u
>>
>> The warning log showed that when finding a new space for nat the journal
>> space turned out to be full. This because the journal_rwsem is not
>> locked before the journal space checking. The journal space may become
>> full just after we check it.
>>
>> Reported-by: Philippe De Muyter <phdm@xxxxxxx>
>> Signed-off-by: Wu Bo <bo.wu@xxxxxxxx>
>> ---
>>  fs/f2fs/node.c    |  6 +++---
>>  fs/f2fs/segment.c | 10 +++++-----
>>  2 files changed, 8 insertions(+), 8 deletions(-)
>>
>
> Thank you for your patch.
>
> Unfortunately it does not apply to my 4.1.15 or newer 4.1.y sources,
> and I do not have the knowledge of f2fs internals to modify your
> patch myself.  E.g. 4.1.y lacks the '.journal' field in the
> 'struct curseg_info'.
>
> Could you make a version suitable for 4.1.y ?

My patch is just try to fix the 'offset < 0' warning you have meet. The
probability of this is very low.

To the fsck fixed report you found when doing fsck.f2fs, 'reset
i_gc_failures' log seems normal. And 'Unreachable nat entries' maybe
caused by the 'offset < 0' exception.

If your filesystem doesn't report fsck failures after these 2 cases, I
think you don't need to worry about it too much.

Here is the patch for v4.1.y:

diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
index 8ab0cf1930bd..fc4d87a1ddf0 100644
--- a/fs/f2fs/node.c
+++ b/fs/f2fs/node.c
@@ -1837,12 +1837,12 @@ static void __flush_nat_entry_set(struct f2fs_sb_info *sbi,
 	 * #1, flush nat entries to journal in current hot data summary block.
 	 * #2, flush nat entries to nat page.
 	 */
+	mutex_lock(&curseg->curseg_mutex);
 	if (!__has_cursum_space(sum, set->entry_cnt, NAT_JOURNAL))
 		to_journal = false;
 
-	if (to_journal) {
-		mutex_lock(&curseg->curseg_mutex);
-	} else {
+	if (!to_journal) {
+		mutex_unlock(&curseg->curseg_mutex);
 		page = get_next_nat_page(sbi, start_nid);
 		nat_blk = page_address(page);
 		f2fs_bug_on(sbi, !nat_blk);
>
>
> Best regards
>
> Philippe
>
>