Re: [PATCH 1/2] nvme: fix racy access to FDP placement ID array

[Hari Mishal]

Hi Kanchan,

I think your patch does solve all the problems. Should I resubmit the
patch as a V2 or a new one?

Cheers,
Hari

On Mon, Jul 27, 2026 at 4:22 PM Kanchan Joshi <joshi.k@xxxxxxxxxxx> wrote:
>
> Hari,
>
> Can you please try this patch and see if this solves what you saw?
>
>   drivers/nvme/host/core.c | 38 +++++++++++++++++---------------------
>   drivers/nvme/host/nvme.h |  1 +
>   2 files changed, 18 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c
> index 453c1f0b2dd0..09cb22f6509c 100644
> --- a/drivers/nvme/host/core.c
> +++ b/drivers/nvme/host/core.c
> @@ -2320,14 +2320,6 @@ static int nvme_query_fdp_info(struct nvme_ns
> *ns, struct nvme_ns_info *info)
>          size_t size;
>          int i, ret;
>
> -       /*
> -        * The FDP configuration is static for the lifetime of the
> namespace,
> -        * so return immediately if we've already registered this
> namespace's
> -        * streams.
> -        */
> -       if (head->nr_plids)
> -               return 0;
> -
>          ret = nvme_get_features(ctrl, NVME_FEAT_FDP, info->endgid, NULL, 0,
>                                  &fdp);
>          if (ret) {
> @@ -2374,6 +2366,7 @@ static int nvme_query_fdp_info(struct nvme_ns *ns,
> struct nvme_ns_info *info)
>
>          for (i = 0; i < head->nr_plids; i++)
>                  head->plids[i] = le16_to_cpu(ruhs->ruhsd[i].pid);
> +       head->write_stream_granularity = min(info->runs, U32_MAX);
>   free:
>          kfree(ruhs);
>          return ret;
> @@ -2421,12 +2414,6 @@ static int nvme_update_ns_info_block(struct
> nvme_ns *ns,
>                          goto out;
>          }
>
> -       if (ns->ctrl->ctratt & NVME_CTRL_ATTR_FDPS) {
> -               ret = nvme_query_fdp_info(ns, info);
> -               if (ret < 0)
> -                       goto out;
> -       }
> -
>          if (nvme_invalid_lba_sz(le64_to_cpu(id->nsze),
>                          id->lbaf[lbaf].ds - SECTOR_SHIFT, &capacity)) {
>                  dev_warn_once(ns->ctrl->device,
> @@ -2469,10 +2456,7 @@ static int nvme_update_ns_info_block(struct
> nvme_ns *ns,
>                  capacity = 0;
>
>          lim.max_write_streams = ns->head->nr_plids;
> -       if (lim.max_write_streams)
> -               lim.write_stream_granularity = min(info->runs, U32_MAX);
> -       else
> -               lim.write_stream_granularity = 0;
> +       lim.write_stream_granularity = ns->head->write_stream_granularity;
>
>          /*
>           * Only set the DEAC bit if the device guarantees that reads from
> @@ -3964,9 +3948,10 @@ static void nvme_add_ns_cdev(struct nvme_ns *ns)
>          set_bit(NVME_NS_CDEV_LIVE, &ns->flags);
>   }
>
> -static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
> +static struct nvme_ns_head *nvme_alloc_ns_head(struct nvme_ns *ns,
>                  struct nvme_ns_info *info)
>   {
> +       struct nvme_ctrl *ctrl = ns->ctrl;
>          struct nvme_ns_head *head;
>          size_t size = sizeof(*head);
>          int ret = -ENOMEM;
> @@ -4002,15 +3987,26 @@ static struct nvme_ns_head
> *nvme_alloc_ns_head(struct nvme_ctrl *ctrl,
>          } else
>                  head->effects = ctrl->effects;
>
> +       if (ctrl->ctratt & NVME_CTRL_ATTR_FDPS) {
> +               ns->head = head;
> +               ret = nvme_query_fdp_info(ns, info);
> +               if (ret < 0)
> +                       goto out_clear_ns_head;
> +       }
> +
>          ret = nvme_mpath_alloc_disk(ctrl, head);
>          if (ret)
> -               goto out_cleanup_srcu;
> +               goto out_cleanup_fdp;
>
>          list_add_tail(&head->entry, &ctrl->subsys->nsheads);
>
>          kref_get(&ctrl->subsys->ref);
>
>          return head;
> +out_cleanup_fdp:
> +       kfree(head->plids);
> +out_clear_ns_head:
> +       ns->head = NULL;
>   out_cleanup_srcu:
>          cleanup_srcu_struct(&head->srcu);
>   out_ida_remove:
> @@ -4103,7 +4099,7 @@ static int nvme_init_ns_head(struct nvme_ns *ns,
> struct nvme_ns_info *info)
>                                  info->nsid);
>                          goto out_unlock;
>                  }
> -               head = nvme_alloc_ns_head(ctrl, info);
> +               head = nvme_alloc_ns_head(ns, info);
>                  if (IS_ERR(head)) {
>                          ret = PTR_ERR(head);
>                          goto out_unlock;
> diff --git a/drivers/nvme/host/nvme.h b/drivers/nvme/host/nvme.h
> index 824651cc898d..3ae73b36866d 100644
> --- a/drivers/nvme/host/nvme.h
> +++ b/drivers/nvme/host/nvme.h
> @@ -560,6 +560,7 @@ struct nvme_ns_head {
>
>          u16                     nr_plids;
>          u16                     *plids;
> +       u32                     write_stream_granularity;
>   #ifdef CONFIG_NVME_MULTIPATH
>          struct bio_list         requeue_list;
>          spinlock_t              requeue_lock;
> --
> 2.25.1
>