Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20210715202341.2016612-1-mcgrof@kernel.org> <20210715202341.2016612-4-mcgrof@kernel.org>
In-Reply-To: <20210715202341.2016612-4-mcgrof@kernel.org>
From:   "Guilherme G. Piccoli" <kernel@gpiccoli.net>
Date:   Wed, 28 Jul 2021 16:20:01 -0300
Message-ID: <CALJn8nOWtFBnuzqPpuOH6nqikUzZGUYWEnvwCFvmrCrC5MWSmw@mail.gmail.com>
Subject: Re: [RFC 3/6] md: replace GENHD_FL_UP with GENHD_FL_DISK_ADDED on is_mddev_broken()
To:     Luis Chamberlain <mcgrof@kernel.org>, hch@infradead.org
Cc:     axboe@kernel.dk, hare@suse.de,
        Bart Van Assche <bvanassche@acm.org>,
        Ming Lei <ming.lei@redhat.com>, jack@suse.cz, osandov@fb.com,
        linux-block@vger.kernel.org, linux-kernel@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Thu, Jul 15, 2021 at 5:24 PM Luis Chamberlain <mcgrof@kernel.org> wrote:
>
> The GENHD_FL_DISK_ADDED flag is what we really want, as the
> flag GENHD_FL_UP could be set on a semi-initialized device.
>
> Signed-off-by: Luis Chamberlain <mcgrof@kernel.org>
> ---
>  drivers/md/md.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/md/md.h b/drivers/md/md.h
> index 832547cf038f..80561bca1f51 100644
> --- a/drivers/md/md.h
> +++ b/drivers/md/md.h
> @@ -766,7 +766,7 @@ static inline bool is_mddev_broken(struct md_rdev *rdev, const char *md_type)
>  {
>         int flags = rdev->bdev->bd_disk->flags;
>
> -       if (!(flags & GENHD_FL_UP)) {
> +       if (!(flags & GENHD_FL_DISK_ADDED)) {

Thanks for the patch Luis! And thanks Christoph for looping me in on
the last iteration.
I think specifically for md, both flags are interchangeable - if
add_disk() is not completed, I'm pretty sure we cannot have the array
properly working hence we shouldn't ever reach this check for such
device. Nevertheless, technically speaking Christoph seems correct and
we are checking here in fact if the disk was del_gendisk'ed().
My opinion is that we don't need to change this usage, if possible,
but I'm not strongly against the change if you feel it fits better.

Just double-checking - after del_gendisk(), this flag is removed anyways right?
Oh, and if possible, loop me in CC for next revisions, using this email.
Cheers,


Guilherme