Message ID | 20230315181902.4177819-13-joel@joelfernandes.org |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a5d:604a:0:0:0:0:0 with SMTP id j10csp51396wrt; Wed, 15 Mar 2023 11:26:05 -0700 (PDT) X-Google-Smtp-Source: AK7set8vhWoWxlWP1y7FMPKaZ48JwZN582gejx3+4mOREiEAlEL5IONo0f5rBGXgKXrvyybPRPaS X-Received: by 2002:a05:6a20:394e:b0:d6:9939:b0a9 with SMTP id r14-20020a056a20394e00b000d69939b0a9mr618261pzg.25.1678904765622; Wed, 15 Mar 2023 11:26:05 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1678904765; cv=none; d=google.com; s=arc-20160816; b=kmDle0LSe5vLE/5JVupMupo5dJRrARNIp7jIDfIj3FYAQCwbF1LXkFCPXcDwo3zoui DLtFasXWLEDWSvddr4Lwb64wnfc01QfRFAlAkjaqyhNqu6Kw1/rF2mJQQfx8ToO1Xh/f 6CiOdT3QYbp7jnlxHK0FrJTOatrOxGgVmBysducZ3uAc5zVUeDmDUPIe1WcXzb65T/C4 Hta6mtGv0HZQ9ayJZUrEK2dC1T9VMI6M+y9Kf3gaTYphmHOzyHAzPjWzd7RZEO7EDJgj 1LRt/ECxsaGqMS6XKoYzeupgWSNXB61ZhueYrzUvAgUOoHPrYsNxWu/6PPmdFj167KHE SO6Q== 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 :references:in-reply-to:message-id:date:subject:cc:to:from :dkim-signature; bh=GhODMKtcWK0LfRf2wdwwZPjRN4HzxNzq88wVUem7HDw=; b=U4NgNfmjsBf2JKqqcIQj51ADF9OCY6VfrJMTbwBx/GLuGoWXfmLW+/4K8b73fUK9XM lkIOYdU4lVZ/NiXtl7zMBRrzeJxB4ipXzzxf4jF5jK6hsuQxTSMrTKtCSf3LCJzHlwP+ FoUCmcJYE6Hn0CYvQQyArkHZqFf926ykiKkE86Bc4V0k0yMsPnSZNjaeREc9+izIXT39 9QIXxfVMdEs5LjqD7+Xq4GzjatcDblQ2tdZPWuqmCRK6K68TviJMkgaXKbyc2nHhihqn stllxdaI3mUFSYEvl3KJsDsqJNnMApDvWMNtA3gKAdM1XtENQiNF94mch9364aIF8M4k DgAw== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@joelfernandes.org header.s=google header.b=C+Y304uK; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id m13-20020a6545cd000000b004fb95253a18si5549944pgr.376.2023.03.15.11.25.53; Wed, 15 Mar 2023 11:26:05 -0700 (PDT) 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; Authentication-Results: mx.google.com; dkim=pass header.i=@joelfernandes.org header.s=google header.b=C+Y304uK; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S231721AbjCOSUn (ORCPT <rfc822;ruipengqi7@gmail.com> + 99 others); Wed, 15 Mar 2023 14:20:43 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54170 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232666AbjCOSUQ (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Wed, 15 Mar 2023 14:20:16 -0400 Received: from mail-qt1-x82d.google.com (mail-qt1-x82d.google.com [IPv6:2607:f8b0:4864:20::82d]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id BE62480E25 for <linux-kernel@vger.kernel.org>; Wed, 15 Mar 2023 11:19:40 -0700 (PDT) Received: by mail-qt1-x82d.google.com with SMTP id c18so17223806qte.5 for <linux-kernel@vger.kernel.org>; Wed, 15 Mar 2023 11:19:40 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=joelfernandes.org; s=google; t=1678904378; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:from:to:cc:subject:date :message-id:reply-to; bh=GhODMKtcWK0LfRf2wdwwZPjRN4HzxNzq88wVUem7HDw=; b=C+Y304uKNIZZWLzEXOsgwjwy501pin0efMM4+4pHeao5pW4Ygdkr844rvREG6JIZNj iHQD4/NBc1VPJlaADkXXuSvNj3OPEv50FU4eSIPGznLD+pOeiJxte4UfTehUiPjQx3Tp Y8vGh4AYquFTIee4Aqt2jKebubt+RqDwdAlr4= X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; t=1678904378; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:cc:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=GhODMKtcWK0LfRf2wdwwZPjRN4HzxNzq88wVUem7HDw=; b=QRcl2/SvSeEOKgcYYb2Kfh0PFo/CayHpJsGKRGQb5M2zM0wf8bFL0y9cC4wNgFxwk+ yGfKngJ1t5q96UqjhGYXHcthXrFBI6aPq1e0qUHa9CsDb7EJH2XkuJm5TyrjwiPV2znq w7cM71ymCSp22XeoKzeMTlNUvUF1ngZFTLXmOvqgIUgN0BKzKBJu4sW/rIw7vuS+ya65 Uelrdybh2OC9Up3owEVfnnvM/nXBrzGSYaklgrz9dvU6FoF3uyYfCUHFfiCKTOAHy2vv sQqd0tPb885XDZwvO7AbOcH3UrexG+3YKK/ke7CtbdlkKEohjKy+hPkp5oL34gJimYeo Usww== X-Gm-Message-State: AO0yUKUnoSoO2eL/9LlRvLiX2xkf87C04OpOeb77nmdGzozoeHDC3p5x HBI6dvXZJXXMEjD89LTcQEKQLg== X-Received: by 2002:ac8:5dcd:0:b0:3d5:f1a2:13f7 with SMTP id e13-20020ac85dcd000000b003d5f1a213f7mr1412046qtx.12.1678904378370; Wed, 15 Mar 2023 11:19:38 -0700 (PDT) Received: from joelboxx.c.googlers.com.com (129.239.188.35.bc.googleusercontent.com. [35.188.239.129]) by smtp.gmail.com with ESMTPSA id v125-20020a379383000000b007458ae32290sm4113974qkd.128.2023.03.15.11.19.37 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Wed, 15 Mar 2023 11:19:38 -0700 (PDT) From: "Joel Fernandes (Google)" <joel@joelfernandes.org> To: Zhu Yanjun <zyjzyj2000@gmail.com>, Jason Gunthorpe <jgg@ziepe.ca>, Leon Romanovsky <leon@kernel.org>, Devesh Sharma <devesh.s.sharma@oracle.com>, Bob Pearson <rpearsonhpe@gmail.com> Cc: "Joel Fernandes (Google)" <joel@joelfernandes.org>, "Paul E . McKenney" <paulmck@kernel.org>, Zhu Yanjun <yanjun.zhu@linux.dev>, linux-rdma@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [PATCH v2 13/14] RDMA/rxe: Rename kfree_rcu() to kvfree_rcu_mightsleep() Date: Wed, 15 Mar 2023 18:19:00 +0000 Message-Id: <20230315181902.4177819-13-joel@joelfernandes.org> X-Mailer: git-send-email 2.40.0.rc1.284.g88254d51c5-goog In-Reply-To: <20230315181902.4177819-1-joel@joelfernandes.org> References: <20230315181902.4177819-1-joel@joelfernandes.org> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-2.1 required=5.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_NONE, SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=unavailable 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-getmail-retrieved-from-mailbox: =?utf-8?q?INBOX?= X-GMAIL-THRID: =?utf-8?q?1760459243333114915?= X-GMAIL-MSGID: =?utf-8?q?1760459243333114915?= |
Series |
[v2,01/14] drbd: Rename kvfree_rcu() to kvfree_rcu_mightsleep()
|
|
Commit Message
Joel Fernandes
March 15, 2023, 6:19 p.m. UTC
The k[v]free_rcu() macro's single-argument form is deprecated. Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal is to avoid accidental use of the single-argument forms, which can introduce functionality bugs in atomic contexts and latency bugs in non-atomic contexts. There is no functionality change with this patch. Link: https://lore.kernel.org/rcu/20230201150815.409582-1-urezki@gmail.com Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()") Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> --- drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
Comments
On 3/15/23 13:19, Joel Fernandes (Google) wrote: > The k[v]free_rcu() macro's single-argument form is deprecated. > Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal > is to avoid accidental use of the single-argument forms, which can > introduce functionality bugs in atomic contexts and latency bugs in > non-atomic contexts. > > There is no functionality change with this patch. > > Link: https://lore.kernel.org/rcu/20230201150815.409582-1-urezki@gmail.com > Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com> > Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com> > Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()") > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > --- > drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c > index b10aa1580a64..ae3a100e18fb 100644 > --- a/drivers/infiniband/sw/rxe/rxe_mr.c > +++ b/drivers/infiniband/sw/rxe/rxe_mr.c > @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata) > return -EINVAL; > > rxe_cleanup(mr); > - kfree_rcu(mr); > + kfree_rcu_mightsleep(mr); > return 0; > } > Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr). Bob
On Wed, Mar 15, 2023 at 2:38 PM Bob Pearson <rpearsonhpe@gmail.com> wrote: > > On 3/15/23 13:19, Joel Fernandes (Google) wrote: > > The k[v]free_rcu() macro's single-argument form is deprecated. > > Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal > > is to avoid accidental use of the single-argument forms, which can > > introduce functionality bugs in atomic contexts and latency bugs in > > non-atomic contexts. > > > > There is no functionality change with this patch. > > > > Link: https://lore.kernel.org/rcu/20230201150815.409582-1-urezki@gmail.com > > Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com> > > Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com> > > Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > > Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()") > > Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > > --- > > drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c > > index b10aa1580a64..ae3a100e18fb 100644 > > --- a/drivers/infiniband/sw/rxe/rxe_mr.c > > +++ b/drivers/infiniband/sw/rxe/rxe_mr.c > > @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata) > > return -EINVAL; > > > > rxe_cleanup(mr); > > - kfree_rcu(mr); > > + kfree_rcu_mightsleep(mr); > > return 0; > > } > > > Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr). I thought you said either was Ok, but yeah that's fine with me. I was trying to play it safe ;-). An extra GP may not hurt, but not having one when it is needed might. I will update it to just be kfree. <quote> > rxe_cleanup(mr); > - kfree_rcu(mr); > + kfree_rcu_mightsleep(mr); > return 0; > } > I would prefer just - kfree_rcu(mr); + kfree(mr); but either one will work. Bob </quote> - Joel
On 3/15/23 13:41, Joel Fernandes wrote: > On Wed, Mar 15, 2023 at 2:38 PM Bob Pearson <rpearsonhpe@gmail.com> wrote: >> >> On 3/15/23 13:19, Joel Fernandes (Google) wrote: >>> The k[v]free_rcu() macro's single-argument form is deprecated. >>> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal >>> is to avoid accidental use of the single-argument forms, which can >>> introduce functionality bugs in atomic contexts and latency bugs in >>> non-atomic contexts. >>> >>> There is no functionality change with this patch. >>> >>> Link: https://lore.kernel.org/rcu/20230201150815.409582-1-urezki@gmail.com >>> Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com> >>> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com> >>> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> >>> Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()") >>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> >>> --- >>> drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c >>> index b10aa1580a64..ae3a100e18fb 100644 >>> --- a/drivers/infiniband/sw/rxe/rxe_mr.c >>> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c >>> @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata) >>> return -EINVAL; >>> >>> rxe_cleanup(mr); >>> - kfree_rcu(mr); >>> + kfree_rcu_mightsleep(mr); >>> return 0; >>> } >>> >> Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr). > > I thought you said either was Ok, but yeah that's fine with me. I was > trying to play it safe ;-). An extra GP may not hurt, but not having > one when it is needed might. I will update it to just be kfree. Thanks. The reason to not add the pause is that it really isn't required and will just make people think that it is. With the current state of the driver the mr cleanup code will disable looking up the mr from its rkey or lkey and then wait until all the references to the mr are dropped before calling kfree. This will always work (unless a new bug is introduced) so no reason to add the rcu delay. Bob > > <quote> >> rxe_cleanup(mr); >> - kfree_rcu(mr); >> + kfree_rcu_mightsleep(mr); >> return 0; >> } >> > > I would prefer just > - kfree_rcu(mr); > + kfree(mr); > > but either one will work. > Bob > </quote> > > - Joel
On Wed, Mar 15, 2023 at 3:05 PM Bob Pearson <rpearsonhpe@gmail.com> wrote: > > On 3/15/23 13:41, Joel Fernandes wrote: > > On Wed, Mar 15, 2023 at 2:38 PM Bob Pearson <rpearsonhpe@gmail.com> wrote: > >> > >> On 3/15/23 13:19, Joel Fernandes (Google) wrote: > >>> The k[v]free_rcu() macro's single-argument form is deprecated. > >>> Therefore switch to the new k[v]free_rcu_mightsleep() variant. The goal > >>> is to avoid accidental use of the single-argument forms, which can > >>> introduce functionality bugs in atomic contexts and latency bugs in > >>> non-atomic contexts. > >>> > >>> There is no functionality change with this patch. > >>> > >>> Link: https://lore.kernel.org/rcu/20230201150815.409582-1-urezki@gmail.com > >>> Acked-by: Zhu Yanjun <zyjzyj2000@gmail.com> > >>> Reviewed-by: Bob Pearson <rpearsonhpe@gmail.com> > >>> Reviewed-by: Paul E. McKenney <paulmck@kernel.org> > >>> Fixes: 72a03627443d ("RDMA/rxe: Remove rxe_alloc()") > >>> Signed-off-by: Joel Fernandes (Google) <joel@joelfernandes.org> > >>> --- > >>> drivers/infiniband/sw/rxe/rxe_mr.c | 2 +- > >>> 1 file changed, 1 insertion(+), 1 deletion(-) > >>> > >>> diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c > >>> index b10aa1580a64..ae3a100e18fb 100644 > >>> --- a/drivers/infiniband/sw/rxe/rxe_mr.c > >>> +++ b/drivers/infiniband/sw/rxe/rxe_mr.c > >>> @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata) > >>> return -EINVAL; > >>> > >>> rxe_cleanup(mr); > >>> - kfree_rcu(mr); > >>> + kfree_rcu_mightsleep(mr); > >>> return 0; > >>> } > >>> > >> Please replace kfree_rcu_mightsleep() by kfree(). There is no need to delay the kfree(mr). > > > > I thought you said either was Ok, but yeah that's fine with me. I was > > trying to play it safe ;-). An extra GP may not hurt, but not having > > one when it is needed might. I will update it to just be kfree. > > Thanks. The reason to not add the pause is that it really isn't required and will just make > people think that it is. With the current state of the driver the mr cleanup code will disable > looking up the mr from its rkey or lkey and then wait until all the references to the mr are dropped > before calling kfree. This will always work (unless a new bug is introduced) so no reason to > add the rcu delay. > Sounds good! I updated it in my queue, and I will repost it in the future pending test results and other comments. thanks, - Joel
diff --git a/drivers/infiniband/sw/rxe/rxe_mr.c b/drivers/infiniband/sw/rxe/rxe_mr.c index b10aa1580a64..ae3a100e18fb 100644 --- a/drivers/infiniband/sw/rxe/rxe_mr.c +++ b/drivers/infiniband/sw/rxe/rxe_mr.c @@ -731,7 +731,7 @@ int rxe_dereg_mr(struct ib_mr *ibmr, struct ib_udata *udata) return -EINVAL; rxe_cleanup(mr); - kfree_rcu(mr); + kfree_rcu_mightsleep(mr); return 0; }