| Message ID | 20210715202341.2016612-4-mcgrof@kernel.org (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | block: enhance use of GENHD_FL_UP | expand |
On 7/15/21 10:23 PM, Luis Chamberlain 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)) { > if (!test_and_set_bit(MD_BROKEN, &rdev->mddev->flags)) > pr_warn("md: %s: %s array has a missing/failed member\n", > mdname(rdev->mddev), md_type); > Why again did you introduce the wrapper? Shouldn't it be used here? Cheers, Hannes
On Fri, Jul 16, 2021 at 07:51:00AM +0200, Hannes Reinecke wrote: > On 7/15/21 10:23 PM, Luis Chamberlain 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)) { > > if (!test_and_set_bit(MD_BROKEN, &rdev->mddev->flags)) > > pr_warn("md: %s: %s array has a missing/failed member\n", > > mdname(rdev->mddev), md_type); > > > Why again did you introduce the wrapper? > Shouldn't it be used here? Indeed, and that lets us remove the flag copy. Will fix. Thanks for the review. Luis
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
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)) { if (!test_and_set_bit(MD_BROKEN, &rdev->mddev->flags)) pr_warn("md: %s: %s array has a missing/failed member\n", mdname(rdev->mddev), md_type);
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(-)