| Message ID | 20250414112554.3025113-2-ming.lei@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | ublk: simplify & improve IO canceling | expand |
On Mon, Apr 14, 2025 at 07:25:42PM +0800, Ming Lei wrote: > In ublk_stop_dev(), if ublk device state becomes UBLK_S_DEV_DEAD, we > will return immediately. This way is correct, but not enough, because > ublk device may transition to other state, such UBLK_S_DEV_QUIECED, > when it may have been stopped already. Then kernel panic is triggered. How can this happen? If a device is stopped, it is in the UBLK_S_DEV_DEAD state. Won't that make us fall out of this check in ublk_nosrv_work, so we wont transition to UBLK_S_DEV_QUIESCED or other nosrv states? mutex_lock(&ub->mutex); if (ub->dev_info.state != UBLK_S_DEV_LIVE) goto unlock;
On Mon, Apr 14, 2025 at 01:44:47PM -0600, Uday Shankar wrote: > On Mon, Apr 14, 2025 at 07:25:42PM +0800, Ming Lei wrote: > > In ublk_stop_dev(), if ublk device state becomes UBLK_S_DEV_DEAD, we > > will return immediately. This way is correct, but not enough, because > > ublk device may transition to other state, such UBLK_S_DEV_QUIECED, > > when it may have been stopped already. Then kernel panic is triggered. > > How can this happen? If a device is stopped, it is in the > UBLK_S_DEV_DEAD state. Won't that make us fall out of this check in > ublk_nosrv_work, so we wont transition to UBLK_S_DEV_QUIESCED or other > nosrv states? > > mutex_lock(&ub->mutex); > if (ub->dev_info.state != UBLK_S_DEV_LIVE) > goto unlock; You are right. I just verified that all tests can pass after reverting this patch. Thanks, Ming
diff --git a/drivers/block/ublk_drv.c b/drivers/block/ublk_drv.c index cdb1543fa4a9..15de4881f25b 100644 --- a/drivers/block/ublk_drv.c +++ b/drivers/block/ublk_drv.c @@ -1784,7 +1784,7 @@ static void ublk_stop_dev(struct ublk_device *ub) struct gendisk *disk; mutex_lock(&ub->mutex); - if (ub->dev_info.state == UBLK_S_DEV_DEAD) + if (!ub->ub_disk) goto unlock; if (ublk_nosrv_dev_should_queue_io(ub)) { if (ub->dev_info.state == UBLK_S_DEV_LIVE)
In ublk_stop_dev(), if ublk device state becomes UBLK_S_DEV_DEAD, we will return immediately. This way is correct, but not enough, because ublk device may transition to other state, such UBLK_S_DEV_QUIECED, when it may have been stopped already. Then kernel panic is triggered. Fix it by checking ->ub_disk directly, this way is simpler and effective since ub->mutex covers ->ub_disk change. Fixes: bbae8d1f526b ("ublk_drv: consider recovery feature in aborting mechanism") Signed-off-by: Ming Lei <ming.lei@redhat.com> --- drivers/block/ublk_drv.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)