Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Mon, 19 Sep 2022 17:50:52 -0700
From:   Jaegeuk Kim <jaegeuk@kernel.org>
To:     Wu Bo <bo.wu@vivo.com>
Cc:     chao@kernel.org, linux-f2fs-devel@lists.sourceforge.net,
        linux-kernel@vger.kernel.org, Philippe De Muyter <phdm@macq.eu>
Subject: Re: [PATCH 1/1] f2fs: fix to check space of current segment journal
Message-ID: <YykObKyz+XT1QCyh@google.com>
References: <20220914040423.539-1-bo.wu@vivo.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220914040423.539-1-bo.wu@vivo.com>
Precedence: bulk

On 09/14, 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@macq.eu>
> Signed-off-by: Wu Bo <bo.wu@vivo.com>
> ---
>  fs/f2fs/node.c    |  6 +++---
>  fs/f2fs/segment.c | 10 +++++-----
>  2 files changed, 8 insertions(+), 8 deletions(-)
> 
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index e06a0c478b39..971d8b9ccdf1 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -2995,13 +2995,13 @@ static int __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.
>  	 */
> +	down_write(&curseg->journal_rwsem);
>  	if ((cpc->reason & CP_UMOUNT) ||
>  		!__has_cursum_space(journal, set->entry_cnt, NAT_JOURNAL))

I think this is for NAT which was covered by nat_tree_lock. So, we don't need
this under journal_rwsem.

>  		to_journal = false;
>  
> -	if (to_journal) {
> -		down_write(&curseg->journal_rwsem);
> -	} else {
> +	if (!to_journal) {
> +		up_write(&curseg->journal_rwsem);
>  		page = get_next_nat_page(sbi, start_nid);
>  		if (IS_ERR(page))
>  			return PTR_ERR(page);
> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
> index 0de21f82d7bc..d545032d2f6f 100644
> --- a/fs/f2fs/segment.c
> +++ b/fs/f2fs/segment.c
> @@ -3914,13 +3914,13 @@ int f2fs_lookup_journal_in_cursum(struct f2fs_journal *journal, int type,
>  			if (le32_to_cpu(nid_in_journal(journal, i)) == val)
>  				return i;
>  		}
> -		if (alloc && __has_cursum_space(journal, 1, NAT_JOURNAL))
> +		if (alloc)
>  			return update_nats_in_cursum(journal, 1);
>  	} else if (type == SIT_JOURNAL) {
>  		for (i = 0; i < sits_in_cursum(journal); i++)
>  			if (le32_to_cpu(segno_in_journal(journal, i)) == val)
>  				return i;
> -		if (alloc && __has_cursum_space(journal, 1, SIT_JOURNAL))
> +		if (alloc)
>  			return update_sits_in_cursum(journal, 1);
>  	}
>  	return -1;
> @@ -4085,13 +4085,13 @@ void f2fs_flush_sit_entries(struct f2fs_sb_info *sbi, struct cp_control *cpc)
>  						(unsigned long)MAIN_SEGS(sbi));
>  		unsigned int segno = start_segno;
>  
> +		down_write(&curseg->journal_rwsem);
>  		if (to_journal &&
>  			!__has_cursum_space(journal, ses->entry_cnt, SIT_JOURNAL))
>  			to_journal = false;
>  
> -		if (to_journal) {
> -			down_write(&curseg->journal_rwsem);
> -		} else {
> +		if (!to_journal) {
> +			up_write(&curseg->journal_rwsem);
>  			page = get_next_sit_page(sbi, start_segno);
>  			raw_sit = page_address(page);
>  		}
> -- 
> 2.36.1