[PATCH 1/3] nvme: multipath: Implemented new iopolicy "queue-depth"

[Ewan Milne]

The patch uses the existing flag NVME_MPATH_IO_STATS in the request
for simplicity to see if an atomic_inc() was performed before doing the _dec().
If we really need to keep the count for passthrough requests (maybe we do)
then we would need another flag.

The reason to use the atomic_dec_if_positive() in the 2nd patch and set the
nr_active to zero when changing the iopolicy is so that if e.g. someone changes
the iopolicy repeatedly while requests are in-flight the counter will
work properly.

(I have a test case and another path to make the nr_active counter
visible in sysfs,
btw, if that would be useful.)

-Ewan

On Tue, Nov 7, 2023 at 4:49 PM Keith Busch <kbusch at kernel.org> wrote:
>
> On Tue, Nov 07, 2023 at 04:23:29PM -0500, Ewan D. Milne wrote:
> >  void nvme_mpath_default_iopolicy(struct nvme_subsystem *subsys)
> >  {
> > @@ -130,6 +133,7 @@ void nvme_mpath_start_request(struct request *rq)
> >       if (!blk_queue_io_stat(disk->queue) || blk_rq_is_passthrough(rq))
> >               return;
> >
> > +     atomic_inc(&ns->ctrl->nr_active);
>
> Did you intend to skip counting nr_active if disk stats are not enabled,
> or if it's a passthough command?
>
> Also, we should restrict the atomics for other new IO Depth  policy.
>
> >       nvme_req(rq)->flags |= NVME_MPATH_IO_STATS;
> >       nvme_req(rq)->start_time = bdev_start_io_acct(disk->part0, req_op(rq),
> >                                                     jiffies);
> > @@ -142,6 +146,8 @@ void nvme_mpath_end_request(struct request *rq)
> >
> >       if (!(nvme_req(rq)->flags & NVME_MPATH_IO_STATS))
> >               return;
> > +
> > +     atomic_dec(&ns->ctrl->nr_active);
>
> And if disk stats were disabled, this dec isn't paired with an inc.
>