Re: [PATCH 3/5] nvme: refactor namespace probing

Joel Granados
2022-07-19T13:01:14Z

[-- Attachment #1: Type: text/plain, Size: 2197 bytes --]

On Tue, Jul 19, 2022 at 12:38:45PM +0530, Kanchan Joshi wrote:
> On Mon, Jul 18, 2022 at 07:25:01AM +0200, Christoph Hellwig wrote:
>
> > -static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
> > +static int nvme_update_ns_info_block(struct nvme_ns *ns,
> > + struct nvme_ns_info *info)
> > {
> > - unsigned lbaf = nvme_lbaf_index(id->flbas);
> > + struct nvme_id_ns *id;
> > + unsigned lbaf;
> > int ret;
> >
> > + ret = nvme_identify_ns(ns->ctrl, info->nsid, &id);
> > + if (ret)
> > + return ret;
> > +
> > blk_mq_freeze_queue(ns->disk->queue);
> > + lbaf = nvme_lbaf_index(id->flbas);
> > ns->lba_shift = id->lbaf[lbaf].ds;
> > nvme_set_queue_limits(ns->ctrl, ns->queue);
> >
> > @@ -1967,6 +1995,7 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
> > disk_update_readahead(ns->head->disk);
> > blk_mq_unfreeze_queue(ns->head->disk->queue);
> > }
> > + kfree(id);
> > return 0;
> >
> > out_unfreeze:
> > @@ -1980,9 +2009,30 @@ static int nvme_update_ns_info(struct nvme_ns *ns, struct nvme_id_ns *id)
> > ret = 0;
> > }
> > blk_mq_unfreeze_queue(ns->disk->queue);
> > + kfree(id);
> > return ret;
>
> Seems this new function (nvme_update_ns_info_block) requires kfree at one more place?
> This one:
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index bbdf0f95331f..5a917211e377 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2011,8 +2011,10 @@ static int nvme_update_ns_info_block(struct nvme_ns *ns,
>
> if (blk_queue_is_zoned(ns->queue)) {
> ret = nvme_revalidate_zones(ns);
> - if (ret && !nvme_first_scan(ns->disk))
> + if (ret && !nvme_first_scan(ns->disk)) {
> + kfree(id);
> return ret;
> + }
> }
>

Should we also take care to hide the device?
As I read it the call to nvme_revalidate_zones can return a -ENODEV
(through blk_revalidate_disk_zones) which is the original condition to
hide the block device.

> With that-
> Reviewed-by: Kanchan Joshi <joshi.k@samsung.com>

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 659 bytes --]