On Thu, 2017-04-27 at 07:46 +0200, Hannes Reinecke wrote:
> On 04/26/2017 08:37 PM, Bart Van Assche wrote:
> > +   clone = blk_get_request(q, rq->cmd_flags | REQ_NOMERGE, GFP_ATOMIC);
> >     if (IS_ERR(clone)) {
> >             /* EBUSY, ENODEV or EWOULDBLOCK: requeue */
> > -           return r;
> > +           pr_debug("blk_get_request() returned %ld%s - requeuing\n",
> > +                    PTR_ERR(clone), blk_queue_dying(q) ?
> > +                    " (path offline)" : "");
> > +           if (blk_queue_dying(q)) {
> > +                   atomic_inc(&m->pg_init_in_progress);
> > +                   activate_path(pgpath);
> > +                   return DM_MAPIO_REQUEUE;
> > +           }
> > +           return DM_MAPIO_DELAY_REQUEUE;
> >     }
> >     clone->bio = clone->biotail = NULL;
> >     clone->rq_disk = bdev->bd_disk;
> 
> At the very least this does warrant some inline comments.
> Why do we call activate_path() here, seeing that the queue is dying?

Hello Hannes,

activate_path() is not only able to activate a path but can also change
the state of a path to offline. The body of the activate_path() function
makes that clear and that is why I had not added a comment above the
activate_path() call:

static void activate_path(struct pgpath *pgpath)
{
        struct request_queue *q = bdev_get_queue(pgpath->path.dev->bdev);

        if (pgpath->is_active && !blk_queue_dying(q))
                scsi_dh_activate(q, pg_init_done, pgpath);
        else
                pg_init_done(pgpath, SCSI_DH_DEV_OFFLINED);
}

Bart.

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to