[PATCH v2] f2fs: continuous counting for 'issued' in __issue_discard_cmd_orderly()

[PATCH v2] f2fs: continuous counting for 'issued' in __issue_discard_cmd_orderly()

[Yuwei Guan]

As the 'dcc->discard_granularity' and 'dcc->max_ordered_discard' can be set
at the user space, and if the 'dcc->max_ordered_discard' is set larger than
'dcc->discard_granularity' in DPOLICY_BG mode, or it's a volume device,
discard_granularity can be tuned to 1 in f2fs_tuning_parameters(),
it will may send more requests than the number of 'dpolicy->max_requests'
in issue_discard_thread().

This patch will fix the issue.

Signed-off-by: Yuwei Guan <Yuwei.Guan@zeekrlife.com>
---
 fs/f2fs/segment.c | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c
index a9099a754dd2..908b9a9bd82d 100644
--- a/fs/f2fs/segment.c
+++ b/fs/f2fs/segment.c
@@ -1379,8 +1379,8 @@ static void __queue_discard_cmd(struct f2fs_sb_info *sbi,
 	mutex_unlock(&SM_I(sbi)->dcc_info->cmd_lock);
 }
 
-static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
-					struct discard_policy *dpolicy)
+static void __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
+					struct discard_policy *dpolicy, int *issued)
 {
 	struct discard_cmd_control *dcc = SM_I(sbi)->dcc_info;
 	struct discard_cmd *prev_dc = NULL, *next_dc = NULL;
@@ -1388,7 +1388,6 @@ static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
 	struct discard_cmd *dc;
 	struct blk_plug plug;
 	unsigned int pos = dcc->next_pos;
-	unsigned int issued = 0;
 	bool io_interrupted = false;
 
 	mutex_lock(&dcc->cmd_lock);
@@ -1415,9 +1414,9 @@ static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
 		}
 
 		dcc->next_pos = dc->lstart + dc->len;
-		err = __submit_discard_cmd(sbi, dpolicy, dc, &issued);
+		err = __submit_discard_cmd(sbi, dpolicy, dc, issued);
 
-		if (issued >= dpolicy->max_requests)
+		if (*issued >= dpolicy->max_requests)
 			break;
 next:
 		node = rb_next(&dc->rb_node);
@@ -1433,10 +1432,8 @@ static unsigned int __issue_discard_cmd_orderly(struct f2fs_sb_info *sbi,
 
 	mutex_unlock(&dcc->cmd_lock);
 
-	if (!issued && io_interrupted)
-		issued = -1;
-
-	return issued;
+	if (!(*issued) && io_interrupted)
+		*issued = -1;
 }
 static unsigned int __wait_all_discard_cmd(struct f2fs_sb_info *sbi,
 					struct discard_policy *dpolicy);
@@ -1464,8 +1461,10 @@ static int __issue_discard_cmd(struct f2fs_sb_info *sbi,
 		if (i + 1 < dpolicy->granularity)
 			break;
 
-		if (i + 1 < dcc->max_ordered_discard && dpolicy->ordered)
-			return __issue_discard_cmd_orderly(sbi, dpolicy);
+		if (i + 1 < dcc->max_ordered_discard && dpolicy->ordered) {
+			__issue_discard_cmd_orderly(sbi, dpolicy, &issued);
+			return issued;
+		}
 
 		pend_list = &dcc->pend_list[i];
 
-- 
2.34.1

Re: [PATCH v2] f2fs: continuous counting for 'issued' in __issue_discard_cmd_orderly()

[Bagas Sanjaya]

[-- Attachment #1: Type: text/plain, Size: 1294 bytes --]

On Sun, Dec 11, 2022 at 08:18:52PM +0800, Yuwei Guan wrote:
> As the 'dcc->discard_granularity' and 'dcc->max_ordered_discard' can be set
> at the user space, and if the 'dcc->max_ordered_discard' is set larger than
> 'dcc->discard_granularity' in DPOLICY_BG mode, or it's a volume device,
> discard_granularity can be tuned to 1 in f2fs_tuning_parameters(),
> it will may send more requests than the number of 'dpolicy->max_requests'
> in issue_discard_thread().
> 

You don't know how to stop sentences (aka "abusing" comma), so I read
above as uber-long sentence. Care to reword? There are many cases when I
have to reply with such rewording, but this time I choose not to do
because I'm lazy at the time I write this reply.

> This patch will fix the issue.

Fix by what? I don't understand the code.

> 
> Signed-off-by: Yuwei Guan <Yuwei.Guan@zeekrlife.com>

You send from your Gmail address but have SoB from either personal email
domain or is this a random company? If it is the latter, please talk
with your company to fix the mail system so that you can send from its
domain (and receive traffic from mailing lists). In any case, your email
address in From header and SoB must match.

Thanks. 

-- 
An old man doll... just what I always wanted! - Clara

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 228 bytes --]

Re: [PATCH v2] f2fs: continuous counting for 'issued' in __issue_discard_cmd_orderly()

[Yuwei Guan]

Bagas Sanjaya <bagasdotme@gmail.com> 于2022年12月11日周日 21:20写道：
>
> On Sun, Dec 11, 2022 at 08:18:52PM +0800, Yuwei Guan wrote:
> > As the 'dcc->discard_granularity' and 'dcc->max_ordered_discard' can be set
> > at the user space, and if the 'dcc->max_ordered_discard' is set larger than
> > 'dcc->discard_granularity' in DPOLICY_BG mode, or it's a volume device,
> > discard_granularity can be tuned to 1 in f2fs_tuning_parameters(),
> > it will may send more requests than the number of 'dpolicy->max_requests'
> > in issue_discard_thread().
> >
>
> You don't know how to stop sentences (aka "abusing" comma), so I read
> above as uber-long sentence. Care to reword? There are many cases when I
> have to reply with such rewording, but this time I choose not to do
> because I'm lazy at the time I write this reply.
>
Hi Bagas,
Thanks for your review.
Sorry for submitting a poorly described patch,
I will update a v3 patch to rewrite the issue description and solution.
> > This patch will fix the issue.
>
> Fix by what? I don't understand the code.
>
> >
> > Signed-off-by: Yuwei Guan <Yuwei.Guan@zeekrlife.com>
>
> You send from your Gmail address but have SoB from either personal email
> domain or is this a random company? If it is the latter, please talk
> with your company to fix the mail system so that you can send from its
> domain (and receive traffic from mailing lists). In any case, your email
> address in From header and SoB must match.
>
The email system has been fixed,
and the email addresses will match in the future.
> Thanks.
>
> --
> An old man doll... just what I always wanted! - Clara