From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-pl1-f178.google.com (mail-pl1-f178.google.com [209.85.214.178]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 4B10F72 for ; Mon, 20 Sep 2021 23:50:15 +0000 (UTC) Received: by mail-pl1-f178.google.com with SMTP id w6so12145964pll.3 for ; Mon, 20 Sep 2021 16:50:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OIoT3CHcF6Ui6SUKTLSeS0m12Ndh8eiq7ywYH+1sD+8=; b=XrS/I4JfqWkCKG8V0E5V7TUsfi4r56XeofNU3jWswDlz+IdF93Kj/PTaFLuvC7QgK9 DUBwWm+2vL7riPfws29ww+AwWsypwgSIm8sz4IX8C4bJVp3lWDJFORZFHFjbwPn5mxYR Br5U/2fRwRcLcjWl3onWZ4KgpuNH0IWRLVD4IL4QdWCAK2Pp73dzL/+NSxCQVOe7wlRQ LLOTdy/m78Ii2ccdfTWWpUr31N0HME/kAKd7VOAXaPY2vB5C7w00QbP8P81wjPnQofGn l/G3WVeRF+GTgDOwEtOjLyErYWnL840cltdtInJuw3AvhiJiMUyx8b2PIdbGrj1DOhQ1 NXbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OIoT3CHcF6Ui6SUKTLSeS0m12Ndh8eiq7ywYH+1sD+8=; b=sicmmyx1eELGJLGresz5TpUZ4QHjskQ+92kUaVIKKtIpeIn0QrlheaPIEg1jQ0+Cea yA0XMHwu6WII8xNmZ3wy7KUel2Bc+VPIOHbHJfQ0WjRPged6V9FpCcgnthHpN+Ad43E3 gDIl8TbSSF43ZBOrSrwLfHMia1FQLQUzMdlwU2d28DmWwmRNqcxWUYE3ipSF+ULrNQBu 9nTvOoKY5aGXrfhKllY8uoKb4OEjITHx/vuXNy2xb9VKxanOFHt0+mW8DvENWU/ta0sm 1+xynsztuqV63sYqepDuugAb2x0ZFAsTfS+xwtdlcytiKzGlMzn/C545yzYNoiQEDJqD euhA== X-Gm-Message-State: AOAM531T9BxLj6Yh16Gpu1mdbNJ03lV1xqrCk72rjhOoe3nel9uzgTlX dPmfZH1fuIUsmebJBI1ihXi3esge1CW2qLNejUS/oQ== X-Google-Smtp-Source: ABdhPJwiPF8vZVbVc35Iub4ypV+u11zyGNfpowHKQKuKj6XlQ+gogtNtDHMHWMfKf9ZJPTPqzwoOIQXgvVfgvCvtytI= X-Received: by 2002:a17:902:cec8:b0:13b:9ce1:b3ef with SMTP id d8-20020a170902cec800b0013b9ce1b3efmr24874704plg.4.1632181814749; Mon, 20 Sep 2021 16:50:14 -0700 (PDT) Precedence: bulk X-Mailing-List: nvdimm@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 References: <20210920072726.1159572-1-hch@lst.de> <20210920072726.1159572-4-hch@lst.de> In-Reply-To: <20210920072726.1159572-4-hch@lst.de> From: Dan Williams Date: Mon, 20 Sep 2021 16:50:03 -0700 Message-ID: Subject: Re: [PATCH 3/3] block: warn if ->groups is set when calling add_disk To: Christoph Hellwig Cc: Jens Axboe , Vishal Verma , Dave Jiang , Ira Weiny , linux-block@vger.kernel.org, Linux NVDIMM Content-Type: text/plain; charset="UTF-8" On Mon, Sep 20, 2021 at 12:30 AM Christoph Hellwig wrote: > > The proper API is to pass the groups to device_add_disk, but the code > used to also allow groups being set before calling *add_disk. Warn > about that but keep the group pointer intact for now so that it can > be removed again after a grace period. > > Signed-off-by: Christoph Hellwig > Fixes: 52b85909f85d ("block: fold register_disk into device_add_disk") > --- > block/genhd.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block/genhd.c b/block/genhd.c > index 7b6e5e1cf9564..409cf608cc5bd 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -439,7 +439,8 @@ int device_add_disk(struct device *parent, struct gendisk *disk, > dev_set_uevent_suppress(ddev, 1); > > ddev->parent = parent; > - ddev->groups = groups; > + if (!WARN_ON_ONCE(ddev->groups)) > + ddev->groups = groups; That feels too compact to me, and dev_WARN_ONCE() might save someone a git blame to look up the reason for the warning: dev_WARN_ONCE(parent, ddev->groups, "unexpected pre-populated attribute group\n"); if (!ddev->groups) ddev->groups = groups; ...but not a deal breaker. Either way you can add: Reviewed-by: Dan Williams Jens, I'm ok for the final spin of this series to go through block.git since the referenced commits in Fixes: went that route, just let me know if you want me to take them. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 18467C28B2E for ; Tue, 21 Sep 2021 02:17:41 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 062AD60F9D for ; Tue, 21 Sep 2021 02:17:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1346108AbhIUCRl (ORCPT ); Mon, 20 Sep 2021 22:17:41 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:34030 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S237840AbhIUB4t (ORCPT ); Mon, 20 Sep 2021 21:56:49 -0400 Received: from mail-pj1-x102b.google.com (mail-pj1-x102b.google.com [IPv6:2607:f8b0:4864:20::102b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 3C84EC0313CE for ; Mon, 20 Sep 2021 16:50:15 -0700 (PDT) Received: by mail-pj1-x102b.google.com with SMTP id v19so13143308pjh.2 for ; Mon, 20 Sep 2021 16:50:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=intel-com.20210112.gappssmtp.com; s=20210112; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=OIoT3CHcF6Ui6SUKTLSeS0m12Ndh8eiq7ywYH+1sD+8=; b=XrS/I4JfqWkCKG8V0E5V7TUsfi4r56XeofNU3jWswDlz+IdF93Kj/PTaFLuvC7QgK9 DUBwWm+2vL7riPfws29ww+AwWsypwgSIm8sz4IX8C4bJVp3lWDJFORZFHFjbwPn5mxYR Br5U/2fRwRcLcjWl3onWZ4KgpuNH0IWRLVD4IL4QdWCAK2Pp73dzL/+NSxCQVOe7wlRQ LLOTdy/m78Ii2ccdfTWWpUr31N0HME/kAKd7VOAXaPY2vB5C7w00QbP8P81wjPnQofGn l/G3WVeRF+GTgDOwEtOjLyErYWnL840cltdtInJuw3AvhiJiMUyx8b2PIdbGrj1DOhQ1 NXbg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=OIoT3CHcF6Ui6SUKTLSeS0m12Ndh8eiq7ywYH+1sD+8=; b=UkMloLxeAtqpfHQrH370S9SyCry3ckcCzXTqq9me3jh+nEKls/nyThKftgR93pgXw1 +0sjEXFsbIxaXIWkqdukgHDB6U62fxaVudEqZQVGc2PULpdIrprtBHU+JspRffEi24gF U5bLsmMWRbSuedTIgVjEalkMLz04sDA3sTaC/avD5ongr2Kc33CIniJKGHhNVf8B13y2 SurCSQBvnyAVMS/j9/l39KJJDUR2r7NGonJhAXNXZGk6k+JYcmVSUHigQ8ZaYUHD9+yY 1HIr+bsfn2frf8dsxDCOwWhU6xctffXzSIiMCB+MwNkP7mrAgGOaOFWARZ6XAKk68fek YKCg== X-Gm-Message-State: AOAM531H1BpmgbegGrxuRmmw6DHCutqm7AJ6tcuc4/Q+6LWEOWpXq1iv z5FX6zdPocSWMyBCkCar7W1koguxG4gh3Msd2fduLvp6dZU= X-Google-Smtp-Source: ABdhPJwiPF8vZVbVc35Iub4ypV+u11zyGNfpowHKQKuKj6XlQ+gogtNtDHMHWMfKf9ZJPTPqzwoOIQXgvVfgvCvtytI= X-Received: by 2002:a17:902:cec8:b0:13b:9ce1:b3ef with SMTP id d8-20020a170902cec800b0013b9ce1b3efmr24874704plg.4.1632181814749; Mon, 20 Sep 2021 16:50:14 -0700 (PDT) MIME-Version: 1.0 References: <20210920072726.1159572-1-hch@lst.de> <20210920072726.1159572-4-hch@lst.de> In-Reply-To: <20210920072726.1159572-4-hch@lst.de> From: Dan Williams Date: Mon, 20 Sep 2021 16:50:03 -0700 Message-ID: Subject: Re: [PATCH 3/3] block: warn if ->groups is set when calling add_disk To: Christoph Hellwig Cc: Jens Axboe , Vishal Verma , Dave Jiang , Ira Weiny , linux-block@vger.kernel.org, Linux NVDIMM Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org On Mon, Sep 20, 2021 at 12:30 AM Christoph Hellwig wrote: > > The proper API is to pass the groups to device_add_disk, but the code > used to also allow groups being set before calling *add_disk. Warn > about that but keep the group pointer intact for now so that it can > be removed again after a grace period. > > Signed-off-by: Christoph Hellwig > Fixes: 52b85909f85d ("block: fold register_disk into device_add_disk") > --- > block/genhd.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) > > diff --git a/block/genhd.c b/block/genhd.c > index 7b6e5e1cf9564..409cf608cc5bd 100644 > --- a/block/genhd.c > +++ b/block/genhd.c > @@ -439,7 +439,8 @@ int device_add_disk(struct device *parent, struct gendisk *disk, > dev_set_uevent_suppress(ddev, 1); > > ddev->parent = parent; > - ddev->groups = groups; > + if (!WARN_ON_ONCE(ddev->groups)) > + ddev->groups = groups; That feels too compact to me, and dev_WARN_ONCE() might save someone a git blame to look up the reason for the warning: dev_WARN_ONCE(parent, ddev->groups, "unexpected pre-populated attribute group\n"); if (!ddev->groups) ddev->groups = groups; ...but not a deal breaker. Either way you can add: Reviewed-by: Dan Williams Jens, I'm ok for the final spin of this series to go through block.git since the referenced commits in Fixes: went that route, just let me know if you want me to take them.