All of lore.kernel.org
 help / color / mirror / Atom feed
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>, Coly Li <colyli@suse.de>,
	Song Liu <song@kernel.org>, Luis Chamberlain <mcgrof@kernel.org>,
	linux-block <linux-block@vger.kernel.org>,
	linux-bcache@vger.kernel.org, linux-raid@vger.kernel.org,
	linux-mmc <linux-mmc@vger.kernel.org>,
	linux-nvme@lists.infradead.org
Subject: Re: [PATCH 2/8] mmc: block: cleanup gendisk creation
Date: Mon, 9 Aug 2021 13:30:24 +0200	[thread overview]
Message-ID: <CAPDyKFoSKwamqRdQNkgwKTixSwXPEf9dB4jSiOh73DqXOZ1yGg@mail.gmail.com> (raw)
In-Reply-To: <20210809064028.1198327-3-hch@lst.de>

On Mon, 9 Aug 2021 at 08:44, Christoph Hellwig <hch@lst.de> wrote:
>
> Restructure mmc_blk_probe to avoid a failure path with a half created
> disk.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Let's try to funnel this via Jens' tree. As long as his tree is based
upon v5.14-rc3 or later I don't think we should have any problem with
conflicts.

Kind regards
Uffe


> ---
>  drivers/mmc/core/block.c | 49 ++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 4ac3e1b93e7e..4c11f171e56d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2328,7 +2328,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                                               sector_t size,
>                                               bool default_ro,
>                                               const char *subname,
> -                                             int area_type)
> +                                             int area_type,
> +                                             unsigned int part_type)
>  {
>         struct mmc_blk_data *md;
>         int devidx, ret;
> @@ -2375,6 +2376,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>         kref_init(&md->kref);
>
>         md->queue.blkdata = md;
> +       md->part_type = part_type;
>
>         md->disk->major = MMC_BLOCK_MAJOR;
>         md->disk->minors = perdev_minors;
> @@ -2427,6 +2429,10 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                 md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
>                 cap_str, md->read_only ? "(ro)" : "");
>
> +       /* used in ->open, must be set before add_disk: */
> +       if (area_type == MMC_BLK_DATA_AREA_MAIN)
> +               dev_set_drvdata(&card->dev, md);
> +       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
>         return md;
>
>   err_kfree:
> @@ -2456,7 +2462,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
>         }
>
>         return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
> -                                       MMC_BLK_DATA_AREA_MAIN);
> +                                       MMC_BLK_DATA_AREA_MAIN, 0);
>  }
>
>  static int mmc_blk_alloc_part(struct mmc_card *card,
> @@ -2470,10 +2476,9 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
>         struct mmc_blk_data *part_md;
>
>         part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
> -                                   subname, area_type);
> +                                   subname, area_type, part_type);
>         if (IS_ERR(part_md))
>                 return PTR_ERR(part_md);
> -       part_md->part_type = part_type;
>         list_add(&part_md->part, &md->part);
>
>         return 0;
> @@ -2674,20 +2679,13 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
>
>  static void mmc_blk_remove_req(struct mmc_blk_data *md)
>  {
> -       struct mmc_card *card;
> -
> -       if (md) {
> -               /*
> -                * Flush remaining requests and free queues. It
> -                * is freeing the queue that stops new requests
> -                * from being accepted.
> -                */
> -               card = md->queue.card;
> -               if (md->disk->flags & GENHD_FL_UP)
> -                       del_gendisk(md->disk);
> -               mmc_cleanup_queue(&md->queue);
> -               mmc_blk_put(md);
> -       }
> +       /*
> +        * Flush remaining requests and free queues. It is freeing the queue
> +        * that stops new requests from being accepted.
> +        */
> +       del_gendisk(md->disk);
> +       mmc_cleanup_queue(&md->queue);
> +       mmc_blk_put(md);
>  }
>
>  static void mmc_blk_remove_parts(struct mmc_card *card,
> @@ -2876,7 +2874,7 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
>
>  static int mmc_blk_probe(struct mmc_card *card)
>  {
> -       struct mmc_blk_data *md, *part_md;
> +       struct mmc_blk_data *md;
>         int ret = 0;
>
>         /*
> @@ -2904,19 +2902,6 @@ static int mmc_blk_probe(struct mmc_card *card)
>         if (ret)
>                 goto out;
>
> -       dev_set_drvdata(&card->dev, md);
> -
> -       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
> -       if (ret)
> -               goto out;
> -
> -       list_for_each_entry(part_md, &md->part, part) {
> -               device_add_disk(part_md->parent, part_md->disk,
> -                               mmc_disk_attr_groups);
> -               if (ret)
> -                       goto out;
> -       }
> -
>         /* Add two debugfs entries */
>         mmc_blk_add_debugfs(card, md);
>
> --
> 2.30.2
>

WARNING: multiple messages have this Message-ID (diff)
From: Ulf Hansson <ulf.hansson@linaro.org>
To: Christoph Hellwig <hch@lst.de>
Cc: Jens Axboe <axboe@kernel.dk>, Coly Li <colyli@suse.de>,
	Song Liu <song@kernel.org>,  Luis Chamberlain <mcgrof@kernel.org>,
	linux-block <linux-block@vger.kernel.org>,
	linux-bcache@vger.kernel.org, linux-raid@vger.kernel.org,
	 linux-mmc <linux-mmc@vger.kernel.org>,
	linux-nvme@lists.infradead.org
Subject: Re: [PATCH 2/8] mmc: block: cleanup gendisk creation
Date: Mon, 9 Aug 2021 13:30:24 +0200	[thread overview]
Message-ID: <CAPDyKFoSKwamqRdQNkgwKTixSwXPEf9dB4jSiOh73DqXOZ1yGg@mail.gmail.com> (raw)
In-Reply-To: <20210809064028.1198327-3-hch@lst.de>

On Mon, 9 Aug 2021 at 08:44, Christoph Hellwig <hch@lst.de> wrote:
>
> Restructure mmc_blk_probe to avoid a failure path with a half created
> disk.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>

Acked-by: Ulf Hansson <ulf.hansson@linaro.org>

Let's try to funnel this via Jens' tree. As long as his tree is based
upon v5.14-rc3 or later I don't think we should have any problem with
conflicts.

Kind regards
Uffe


> ---
>  drivers/mmc/core/block.c | 49 ++++++++++++++--------------------------
>  1 file changed, 17 insertions(+), 32 deletions(-)
>
> diff --git a/drivers/mmc/core/block.c b/drivers/mmc/core/block.c
> index 4ac3e1b93e7e..4c11f171e56d 100644
> --- a/drivers/mmc/core/block.c
> +++ b/drivers/mmc/core/block.c
> @@ -2328,7 +2328,8 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                                               sector_t size,
>                                               bool default_ro,
>                                               const char *subname,
> -                                             int area_type)
> +                                             int area_type,
> +                                             unsigned int part_type)
>  {
>         struct mmc_blk_data *md;
>         int devidx, ret;
> @@ -2375,6 +2376,7 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>         kref_init(&md->kref);
>
>         md->queue.blkdata = md;
> +       md->part_type = part_type;
>
>         md->disk->major = MMC_BLOCK_MAJOR;
>         md->disk->minors = perdev_minors;
> @@ -2427,6 +2429,10 @@ static struct mmc_blk_data *mmc_blk_alloc_req(struct mmc_card *card,
>                 md->disk->disk_name, mmc_card_id(card), mmc_card_name(card),
>                 cap_str, md->read_only ? "(ro)" : "");
>
> +       /* used in ->open, must be set before add_disk: */
> +       if (area_type == MMC_BLK_DATA_AREA_MAIN)
> +               dev_set_drvdata(&card->dev, md);
> +       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
>         return md;
>
>   err_kfree:
> @@ -2456,7 +2462,7 @@ static struct mmc_blk_data *mmc_blk_alloc(struct mmc_card *card)
>         }
>
>         return mmc_blk_alloc_req(card, &card->dev, size, false, NULL,
> -                                       MMC_BLK_DATA_AREA_MAIN);
> +                                       MMC_BLK_DATA_AREA_MAIN, 0);
>  }
>
>  static int mmc_blk_alloc_part(struct mmc_card *card,
> @@ -2470,10 +2476,9 @@ static int mmc_blk_alloc_part(struct mmc_card *card,
>         struct mmc_blk_data *part_md;
>
>         part_md = mmc_blk_alloc_req(card, disk_to_dev(md->disk), size, default_ro,
> -                                   subname, area_type);
> +                                   subname, area_type, part_type);
>         if (IS_ERR(part_md))
>                 return PTR_ERR(part_md);
> -       part_md->part_type = part_type;
>         list_add(&part_md->part, &md->part);
>
>         return 0;
> @@ -2674,20 +2679,13 @@ static int mmc_blk_alloc_parts(struct mmc_card *card, struct mmc_blk_data *md)
>
>  static void mmc_blk_remove_req(struct mmc_blk_data *md)
>  {
> -       struct mmc_card *card;
> -
> -       if (md) {
> -               /*
> -                * Flush remaining requests and free queues. It
> -                * is freeing the queue that stops new requests
> -                * from being accepted.
> -                */
> -               card = md->queue.card;
> -               if (md->disk->flags & GENHD_FL_UP)
> -                       del_gendisk(md->disk);
> -               mmc_cleanup_queue(&md->queue);
> -               mmc_blk_put(md);
> -       }
> +       /*
> +        * Flush remaining requests and free queues. It is freeing the queue
> +        * that stops new requests from being accepted.
> +        */
> +       del_gendisk(md->disk);
> +       mmc_cleanup_queue(&md->queue);
> +       mmc_blk_put(md);
>  }
>
>  static void mmc_blk_remove_parts(struct mmc_card *card,
> @@ -2876,7 +2874,7 @@ static void mmc_blk_remove_debugfs(struct mmc_card *card,
>
>  static int mmc_blk_probe(struct mmc_card *card)
>  {
> -       struct mmc_blk_data *md, *part_md;
> +       struct mmc_blk_data *md;
>         int ret = 0;
>
>         /*
> @@ -2904,19 +2902,6 @@ static int mmc_blk_probe(struct mmc_card *card)
>         if (ret)
>                 goto out;
>
> -       dev_set_drvdata(&card->dev, md);
> -
> -       device_add_disk(md->parent, md->disk, mmc_disk_attr_groups);
> -       if (ret)
> -               goto out;
> -
> -       list_for_each_entry(part_md, &md->part, part) {
> -               device_add_disk(part_md->parent, part_md->disk,
> -                               mmc_disk_attr_groups);
> -               if (ret)
> -                       goto out;
> -       }
> -
>         /* Add two debugfs entries */
>         mmc_blk_add_debugfs(card, md);
>
> --
> 2.30.2
>

_______________________________________________
Linux-nvme mailing list
Linux-nvme@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-nvme

  reply	other threads:[~2021-08-09 11:31 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-08-09  6:40 remove GENHD_FL_UP Christoph Hellwig
2021-08-09  6:40 ` Christoph Hellwig
2021-08-09  6:40 ` [PATCH 1/8] mmc: block: let device_add_disk create disk attributes Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-09 11:30   ` Ulf Hansson
2021-08-09 11:30     ` Ulf Hansson
2021-08-09  6:40 ` [PATCH 2/8] mmc: block: cleanup gendisk creation Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-09 11:30   ` Ulf Hansson [this message]
2021-08-09 11:30     ` Ulf Hansson
2021-08-09  6:40 ` [PATCH 3/8] nvme: remove the GENHD_FL_UP check in nvme_ns_remove Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-09  6:40 ` [PATCH 4/8] nvme: replace the GENHD_FL_UP check in nvme_mpath_shutdown_disk Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-09  6:40 ` [PATCH 5/8] sx8: use the internal state machine to check if del_gendisk needs to be called Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-09  6:40 ` [PATCH 6/8] bcache: add proper error unwinding in bcache_device_init Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-12 15:48   ` Coly Li
2021-08-12 15:48     ` Coly Li
2021-08-09  6:40 ` [PATCH 7/8] bcache: move the del_gendisk call out of bcache_device_free Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-12  8:38   ` Coly Li
2021-08-12  8:38     ` Coly Li
2021-08-09  6:40 ` [PATCH 8/8] block: remove GENHD_FL_UP Christoph Hellwig
2021-08-09  6:40   ` Christoph Hellwig
2021-08-12 16:30 ` Jens Axboe
2021-08-12 16:30   ` Jens Axboe

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=CAPDyKFoSKwamqRdQNkgwKTixSwXPEf9dB4jSiOh73DqXOZ1yGg@mail.gmail.com \
    --to=ulf.hansson@linaro.org \
    --cc=axboe@kernel.dk \
    --cc=colyli@suse.de \
    --cc=hch@lst.de \
    --cc=linux-bcache@vger.kernel.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-mmc@vger.kernel.org \
    --cc=linux-nvme@lists.infradead.org \
    --cc=linux-raid@vger.kernel.org \
    --cc=mcgrof@kernel.org \
    --cc=song@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.