Message ID | 20231114212414.3498074-1-jaegeuk@kernel.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a05:6358:a59:b0:164:83eb:24d7 with SMTP id 25csp2213011rwb; Tue, 14 Nov 2023 13:24:23 -0800 (PST) X-Google-Smtp-Source: AGHT+IF8FOahjwS9gswWtxCnUug7Whoq5HKQIOMuvWg+bZRBaepuFzSXGlUsnWWxD6zq2FC9fiIN X-Received: by 2002:a05:6a20:42a4:b0:186:9a3f:f2fb with SMTP id o36-20020a056a2042a400b001869a3ff2fbmr7052716pzj.29.1699997063658; Tue, 14 Nov 2023 13:24:23 -0800 (PST) ARC-Seal: i=1; a=rsa-sha256; t=1699997063; cv=none; d=google.com; s=arc-20160816; b=ePaWPuh03geLLZIwYHYSovfb30HMWZnXSI69bZ5zISbI8Dd7i5n501t5JU572ccoq+ RP+enE15Xz05aZ2/hFczKdNk/y2PGPbTtuyI9de88E87VyczPiMoU3om6fLxNPOk2K2r qPZHqNDhAgWUA3OtH8j3PMprqMTsxK0r3nciCgvjM0wRzu2hnZDeG0ccucfvUfYmnb1/ Y/Qc2N4crjb0mayJTUJAnen61U7WUXokVKGBGbLXM4/P1cOZ1B2yEHvf/SDgLevCUnaz A6CapVf3R8T9v2si5yuENJNIbJD12IWWtrFAX5jsOluaUone149iI+Lqjxjv++uQXHx9 XzLA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :message-id:date:subject:cc:to:from:dkim-signature; bh=LuIPnnYr5TSO+onvsDHgfP1mjgy8YnI6AehTZ/YphVU=; fh=rqHW7NazYv3wAmgqDKAbZUXWTmf0VaM5epB9R4zfTUE=; b=JDDRZ8XezBfN62PyXh/NLX+pgCADM4+bVI6Ts4TSgSBHzC1FBAD95lGxP5BJ/ndlyV anTYzzzXCwfQj8K4mvPu9jK3FoLUjWTrKi4NitpqX1YbHCEpNaawGRGED+b1onBzl/Y8 SQT6+brYhPg4K83cmJTTs5Z4OZ2x60RWrGV3A6jYA7HwAtBNwTfOLwbNaI9Ihm/tbvDt 4c0qRYT+Vfw37+cGxGQcTI+/vLWtqqnGidoNhy6FlXrCwoJUenUrQh9+KEwZVHXVPe96 zKMVg4vo926ZWqKLm3EP0blJoiL4sUlzlMxCEFBipS/QqiBOpD6iM3F5qcDkck+UxSCq W75A== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=KICe4scF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from snail.vger.email (snail.vger.email. [2620:137:e000::3:7]) by mx.google.com with ESMTPS id d4-20020a056a0010c400b006bf349fd067si8694892pfu.156.2023.11.14.13.24.23 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 14 Nov 2023 13:24:23 -0800 (PST) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) client-ip=2620:137:e000::3:7; Authentication-Results: mx.google.com; dkim=pass header.i=@kernel.org header.s=k20201202 header.b=KICe4scF; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::3:7 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=kernel.org Received: from out1.vger.email (depot.vger.email [IPv6:2620:137:e000::3:0]) by snail.vger.email (Postfix) with ESMTP id E0442801CBC3; Tue, 14 Nov 2023 13:24:22 -0800 (PST) X-Virus-Status: Clean X-Virus-Scanned: clamav-milter 0.103.11 at snail.vger.email Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234080AbjKNVYU (ORCPT <rfc822;lhua1029@gmail.com> + 29 others); Tue, 14 Nov 2023 16:24:20 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:47026 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229569AbjKNVYT (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Tue, 14 Nov 2023 16:24:19 -0500 Received: from smtp.kernel.org (relay.kernel.org [52.25.139.140]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id AA26E9D for <linux-kernel@vger.kernel.org>; Tue, 14 Nov 2023 13:24:16 -0800 (PST) Received: by smtp.kernel.org (Postfix) with ESMTPSA id 28BCCC433C8; Tue, 14 Nov 2023 21:24:16 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1699997056; bh=Xg3Aq8eLknKwR53mKiNNcoHu3xpZPIqvXq+7lnhfHuE=; h=From:To:Cc:Subject:Date:From; b=KICe4scFNWNDvIMI8aCanOcqY/ivc4C1xjtVGG06F94sRN0kpvqyA2cZtgUiREMYM ExBjEfJLAJb/zJ/Aftrsy+DexxEps+vAiLUcHPqQUIHOjOyiOcQ/H7a6ZXqkORUiB8 lkZqsQeiqKgV9WbEmQzZBYhZ+6p8d5vuo5mP8r7R+Q186yeOtRjxg7buYw4T+8uvEO WscsZfHi44cMJogwDcC77Eo74UXohsiQ6ECTVj7QBnWeaMPQCipBHnqJNrKfLrGpGg Vm0rpnsMaPL7q/O2UB2BmGYGouokvO7USre1sLHr0XIg4lhdTxjuFeoaw7E5LiJa2O QzKkJUDWBKE7g== From: Jaegeuk Kim <jaegeuk@kernel.org> To: linux-kernel@vger.kernel.org, linux-f2fs-devel@lists.sourceforge.net Cc: Jaegeuk Kim <jaegeuk@kernel.org> Subject: [PATCH] f2fs: skip adding a discard command if exists Date: Tue, 14 Nov 2023 13:24:14 -0800 Message-ID: <20231114212414.3498074-1-jaegeuk@kernel.org> X-Mailer: git-send-email 2.43.0.rc0.421.g78406f8d94-goog MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (snail.vger.email [0.0.0.0]); Tue, 14 Nov 2023 13:24:22 -0800 (PST) X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1782576120660805026 X-GMAIL-MSGID: 1782576120660805026 |
Series |
f2fs: skip adding a discard command if exists
|
|
Commit Message
Jaegeuk Kim
Nov. 14, 2023, 9:24 p.m. UTC
When recovering zoned UFS, sometimes we add the same zone to discard multiple
times. Simple workaround is to bypass adding it.
Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org>
---
fs/f2fs/segment.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
Comments
On 2023/11/15 5:24, Jaegeuk Kim wrote: > When recovering zoned UFS, sometimes we add the same zone to discard multiple > times. Simple workaround is to bypass adding it. What about skipping f2fs_bug_on() just for zoned UFS case? so that the check condition can still be used for non-zoned UFS case. Thanks, > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > fs/f2fs/segment.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > index 727d016318f9..f4ffd64b44b2 100644 > --- a/fs/f2fs/segment.c > +++ b/fs/f2fs/segment.c > @@ -1380,7 +1380,8 @@ static void __insert_discard_cmd(struct f2fs_sb_info *sbi, > p = &(*p)->rb_right; > leftmost = false; > } else { > - f2fs_bug_on(sbi, 1); > + /* Let's skip to add, if exists */ > + return; > } > } >
On 11/15, Chao Yu wrote: > On 2023/11/15 5:24, Jaegeuk Kim wrote: > > When recovering zoned UFS, sometimes we add the same zone to discard multiple > > times. Simple workaround is to bypass adding it. > > What about skipping f2fs_bug_on() just for zoned UFS case? so that the check > condition can still be used for non-zoned UFS case. Hmm, I've never seen this bug_on before, but even this really happens, it does not make sense to move forward to create duplicate commands resulting in a loop. So, the question is, do we really need to check this? Have we hit this before? > > Thanks, > > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > --- > > fs/f2fs/segment.c | 3 ++- > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > index 727d016318f9..f4ffd64b44b2 100644 > > --- a/fs/f2fs/segment.c > > +++ b/fs/f2fs/segment.c > > @@ -1380,7 +1380,8 @@ static void __insert_discard_cmd(struct f2fs_sb_info *sbi, > > p = &(*p)->rb_right; > > leftmost = false; > > } else { > > - f2fs_bug_on(sbi, 1); > > + /* Let's skip to add, if exists */ > > + return; > > } > > }
On 2023/11/15 10:45, Jaegeuk Kim wrote: > On 11/15, Chao Yu wrote: >> On 2023/11/15 5:24, Jaegeuk Kim wrote: >>> When recovering zoned UFS, sometimes we add the same zone to discard multiple >>> times. Simple workaround is to bypass adding it. >> >> What about skipping f2fs_bug_on() just for zoned UFS case? so that the check >> condition can still be used for non-zoned UFS case. > > Hmm, I've never seen this bug_on before, but even this really happens, it does I've never seen it was been triggered as well. > not make sense to move forward to create duplicate commands resulting in a loop. Agreed. It looks those codes were copied from extent_cache code base, do we need to fix all cases to avoid loop? > > So, the question is, do we really need to check this? Have we hit this before? Not sure, just be worry about that flaw of newly developed feature can make code run into that branch. Thanks, > >> >> Thanks, >> >>> >>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> >>> --- >>> fs/f2fs/segment.c | 3 ++- >>> 1 file changed, 2 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c >>> index 727d016318f9..f4ffd64b44b2 100644 >>> --- a/fs/f2fs/segment.c >>> +++ b/fs/f2fs/segment.c >>> @@ -1380,7 +1380,8 @@ static void __insert_discard_cmd(struct f2fs_sb_info *sbi, >>> p = &(*p)->rb_right; >>> leftmost = false; >>> } else { >>> - f2fs_bug_on(sbi, 1); >>> + /* Let's skip to add, if exists */ >>> + return; >>> } >>> }
On 11/15, Chao Yu wrote: > On 2023/11/15 10:45, Jaegeuk Kim wrote: > > On 11/15, Chao Yu wrote: > > > On 2023/11/15 5:24, Jaegeuk Kim wrote: > > > > When recovering zoned UFS, sometimes we add the same zone to discard multiple > > > > times. Simple workaround is to bypass adding it. > > > > > > What about skipping f2fs_bug_on() just for zoned UFS case? so that the check > > > condition can still be used for non-zoned UFS case. > > > > Hmm, I've never seen this bug_on before, but even this really happens, it does > > I've never seen it was been triggered as well. > > > not make sense to move forward to create duplicate commands resulting in a loop. > > Agreed. > > It looks those codes were copied from extent_cache code base, do we need to fix > all cases to avoid loop? Not sure other cases yet.. let's do one by one, since I hit this in real. > > > > > So, the question is, do we really need to check this? Have we hit this before? > Not sure, just be worry about that flaw of newly developed feature can make > code run into that branch. > > Thanks, > > > > > > > > > Thanks, > > > > > > > > > > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > > > > --- > > > > fs/f2fs/segment.c | 3 ++- > > > > 1 file changed, 2 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c > > > > index 727d016318f9..f4ffd64b44b2 100644 > > > > --- a/fs/f2fs/segment.c > > > > +++ b/fs/f2fs/segment.c > > > > @@ -1380,7 +1380,8 @@ static void __insert_discard_cmd(struct f2fs_sb_info *sbi, > > > > p = &(*p)->rb_right; > > > > leftmost = false; > > > > } else { > > > > - f2fs_bug_on(sbi, 1); > > > > + /* Let's skip to add, if exists */ > > > > + return; > > > > } > > > > }
On 2023/11/18 1:41, Jaegeuk Kim wrote: > Not sure other cases yet.. let's do one by one, since I hit this in real. Sure. >>>>> Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> Reviewed-by: Chao Yu <chao@kernel.org> Thansk,
Hello: This patch was applied to jaegeuk/f2fs.git (dev) by Jaegeuk Kim <jaegeuk@kernel.org>: On Tue, 14 Nov 2023 13:24:14 -0800 you wrote: > When recovering zoned UFS, sometimes we add the same zone to discard multiple > times. Simple workaround is to bypass adding it. > > Signed-off-by: Jaegeuk Kim <jaegeuk@kernel.org> > --- > fs/f2fs/segment.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) Here is the summary with links: - [f2fs-dev] f2fs: skip adding a discard command if exists https://git.kernel.org/jaegeuk/f2fs/c/bbd3efed3383 You are awesome, thank you!
diff --git a/fs/f2fs/segment.c b/fs/f2fs/segment.c index 727d016318f9..f4ffd64b44b2 100644 --- a/fs/f2fs/segment.c +++ b/fs/f2fs/segment.c @@ -1380,7 +1380,8 @@ static void __insert_discard_cmd(struct f2fs_sb_info *sbi, p = &(*p)->rb_right; leftmost = false; } else { - f2fs_bug_on(sbi, 1); + /* Let's skip to add, if exists */ + return; } }