On 04/27/2017 05:11 PM, Bart Van Assche wrote:
> 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);
> }
>
So why not call 'pg_init_done()' directly and avoid the confusion?
Cheers,
Hannes
--
Dr. Hannes Reinecke Teamlead Storage & Networking
hare@xxxxxxx +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)