NVMe: Re-attach waitqueue following device reset
Subsystems (2)
PATCH 1/1 NVMe: Re-attach waitqueue following device reset
Bios queued to sq_cong prior to and during a device reset may not get re-issued or may get issued very slowly following the completion of the reset sequence. The waitqueue was detached as part of the sequence and never re-attached, forcing all new make_request() bios to be queued and the queue to be flushed out without the help of the completion path waking the kthread.
This patch re-attaches the waitqueue as part of the reset process.
---
drivers/block/nvme-core.c +17 -0
@@ -2329,6 +2329,23 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)nvme_free_queues (dev, nr_io_queues +1 );nvme_assign_io_queues (dev); +/* Re-attach the waitqueue */ +for (i = dev->queue_count -1 ; i >=1 ; i--) { +struct nvme_queue *nvmeq =raw_nvmeq (dev, i); + +if (nvmeq) { +spin_lock_irq (&nvmeq->q_lock); +init_waitqueue_head (&nvmeq->sq_full); +init_waitqueue_entry (&nvmeq->sq_cong_wait, + nvme_thread); +if (bio_list_peek (&nvmeq->sq_cong) || + !list_empty (&nvmeq->iod_bio)) +add_wait_queue (&nvmeq->sq_full, + &nvmeq->sq_cong_wait); +spin_unlock_irq (&nvmeq->q_lock); + } + } +return 0 ; free_queues:
PATCH 1/1 NVMe: Re-attach waitqueue following device reset
Quoted 6 lines · Sam Bradshaw
Bios queued to
sq_congprior to and during a device reset may not get re-issued or may get issued very slowly following the completion of the reset sequence. The waitqueue was detached as part of the sequence and never re-attached, forcing all newmake_request()bios to be queued and the queue to be flushed out without the help of the completion path waking the kthread.
Hi Sam,
Are you using the latest from linux-nvme tree? I could have sworn this bug was fixed in commit:
Please let me know if there's still a gap.
Quoted 32 lines
This patch re-attaches the waitqueue as part of the reset process.
Signed-off-by Sam Bradshaw <sbradshaw at micron.com>---
drivers/block/nvme-core.c +17
-0@@ -2329,6 +2329,23 @@ static int nvme_setup_io_queues(struct nvme_dev *dev)nvme_free_queues (dev, nr_io_queues +1 );nvme_assign_io_queues (dev); +/* Re-attach the waitqueue */ +for (i = dev->queue_count -1 ; i >=1 ; i--) { +struct nvme_queue *nvmeq =raw_nvmeq (dev, i); + +if (nvmeq) { +spin_lock_irq (&nvmeq->q_lock); +init_waitqueue_head (&nvmeq->sq_full); +init_waitqueue_entry (&nvmeq->sq_cong_wait, + nvme_thread); +if (bio_list_peek (&nvmeq->sq_cong) || + !list_empty (&nvmeq->iod_bio)) +add_wait_queue (&nvmeq->sq_full, + &nvmeq->sq_cong_wait); +spin_unlock_irq (&nvmeq->q_lock); + } + } +return 0 ; free_queues:
PATCH 1/1 NVMe: Re-attach waitqueue following device reset
Quoted 20 lines
-----Original Message-----
From: Keith Busch [mailto:keith.busch at intel.com]
Sent: Tuesday, November 04, 2014 6:55 AM
To: Sam Bradshaw (sbradshaw)
Cc: linux-nvme at lists.infradead.org
Subject: Re: [PATCH] NVMe: Re-attach waitqueue following device resetQuoted 4 lines · Sam Bradshaw
Bios queued to
sq_congprior to and during a device reset may not get re-issued or may get issued very slowly following the completion of the reset sequence. The waitqueue was detached as part of the sequence and never re-attached, forcing all newmake_request()biosto
Quoted 2 lines
be queued and the queue to be flushed out without the help of the completion path waking the kthread.
Hi Sam,
Quoted 7 lines
Are you using the latest from linux-nvme tree? I could have sworn this bug was fixed in commit:
http://git.infradead.org/users/willy/linux-
nvme.git/commitdiff/3559b4d2159aaae42f0fc0fa22e40312c45d4606Please let me know if there's still a gap.
Yes, I was apparently testing against an old tree. The commit above fixes the problem I was observing.
I still see queued bio's getting lost if a successful device reset is followed by a failed resume. I'm working on a patch for that case.
-Sam
PATCH 1/1 NVMe: Re-attach waitqueue following device reset
Quoted 2 lines
I still see queued bio's getting lost if a successful device reset is followed by a failed resume. I'm working on a patch for that case.
Oh, that doesn't sound good. If the device fails to resume, the driver requests the pci layer remove it. That then calls the driver's .remove which clears the bio lists after the queue is freed.
This sequence may deadlock when bios are queued if you don't have this commit:
But if you do have that commit, then something else is amiss.