* [PATCH] block/floppy: fix contended case in floppy_queue_rq()
@ 2020-05-26 9:49 Jiri Kosina
2020-05-26 18:31 ` Jiri Kosina
2020-05-26 21:25 ` Jens Axboe
0 siblings, 2 replies; 3+ messages in thread
From: Jiri Kosina @ 2020-05-26 9:49 UTC (permalink / raw)
To: Denis Efremov, Jens Axboe; +Cc: linux-block, linux-kernel, Libor Pechacek
From: Jiri Kosina <jkosina@suse.cz>
Since the switch of floppy driver to blk-mq, the contended (fdc_busy) case
in floppy_queue_rq() is not handled correctly.
In case we reach floppy_queue_rq() with fdc_busy set (i.e. with the floppy
locked due to another request still being in-flight), we put the request
on the list of requests and return BLK_STS_OK to the block core, without
actually scheduling delayed work / doing further processing of the
request. This means that processing of this request is postponed until
another request comes and passess uncontended.
Which in some cases might actually never happen and we keep waiting
indefinitely. The simple testcase is
for i in `seq 1 2000`; do echo -en $i '\r'; blkid --info /dev/fd0 2> /dev/null; done
run in quemu. That reliably causes blkid eventually indefinitely hanging
in __floppy_read_block_0() waiting for completion, as the BIO callback
never happens, and no further IO is ever submitted on the (non-existent)
floppy device. This was observed reliably on qemu-emulated device.
Fix that by not queuing the request in the contended case, and return
BLK_STS_RESOURCE instead, so that blk core handles the request
rescheduling and let it pass properly non-contended later.
Fixes: a9f38e1dec107a ("floppy: convert to blk-mq")
Cc: stable@vger.kernel.org
Tested-by: Libor Pechacek <lpechacek@suse.cz>
Signed-off-by: Jiri Kosina <jkosina@suse.cz>
---
drivers/block/floppy.c | 10 +++++-----
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
index c3daa64cb52c..975cd0a6baa1 100644
--- a/drivers/block/floppy.c
+++ b/drivers/block/floppy.c
@@ -2938,17 +2938,17 @@ static blk_status_t floppy_queue_rq(struct blk_mq_hw_ctx *hctx,
(unsigned long long) current_req->cmd_flags))
return BLK_STS_IOERR;
- spin_lock_irq(&floppy_lock);
- list_add_tail(&bd->rq->queuelist, &floppy_reqs);
- spin_unlock_irq(&floppy_lock);
-
if (test_and_set_bit(0, &fdc_busy)) {
/* fdc busy, this new request will be treated when the
current one is done */
is_alive(__func__, "old request running");
- return BLK_STS_OK;
+ return BLK_STS_RESOURCE;
}
+ spin_lock_irq(&floppy_lock);
+ list_add_tail(&bd->rq->queuelist, &floppy_reqs);
+ spin_unlock_irq(&floppy_lock);
+
command_status = FD_COMMAND_NONE;
__reschedule_timeout(MAXTIMEOUT, "fd_request");
set_fdc(0);
--
Jiri Kosina
SUSE Labs
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] block/floppy: fix contended case in floppy_queue_rq()
2020-05-26 9:49 [PATCH] block/floppy: fix contended case in floppy_queue_rq() Jiri Kosina
@ 2020-05-26 18:31 ` Jiri Kosina
2020-05-26 21:25 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Jiri Kosina @ 2020-05-26 18:31 UTC (permalink / raw)
To: Denis Efremov, Jens Axboe, Omar Sandoval
Cc: linux-block, linux-kernel, Libor Pechacek
[ forgot to CC Omar, fixing ]
On Tue, 26 May 2020, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
>
> Since the switch of floppy driver to blk-mq, the contended (fdc_busy) case
> in floppy_queue_rq() is not handled correctly.
>
> In case we reach floppy_queue_rq() with fdc_busy set (i.e. with the floppy
> locked due to another request still being in-flight), we put the request
> on the list of requests and return BLK_STS_OK to the block core, without
> actually scheduling delayed work / doing further processing of the
> request. This means that processing of this request is postponed until
> another request comes and passess uncontended.
>
> Which in some cases might actually never happen and we keep waiting
> indefinitely. The simple testcase is
>
> for i in `seq 1 2000`; do echo -en $i '\r'; blkid --info /dev/fd0 2> /dev/null; done
>
> run in quemu. That reliably causes blkid eventually indefinitely hanging
> in __floppy_read_block_0() waiting for completion, as the BIO callback
> never happens, and no further IO is ever submitted on the (non-existent)
> floppy device. This was observed reliably on qemu-emulated device.
>
> Fix that by not queuing the request in the contended case, and return
> BLK_STS_RESOURCE instead, so that blk core handles the request
> rescheduling and let it pass properly non-contended later.
>
> Fixes: a9f38e1dec107a ("floppy: convert to blk-mq")
> Cc: stable@vger.kernel.org
> Tested-by: Libor Pechacek <lpechacek@suse.cz>
> Signed-off-by: Jiri Kosina <jkosina@suse.cz>
> ---
> drivers/block/floppy.c | 10 +++++-----
> 1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/block/floppy.c b/drivers/block/floppy.c
> index c3daa64cb52c..975cd0a6baa1 100644
> --- a/drivers/block/floppy.c
> +++ b/drivers/block/floppy.c
> @@ -2938,17 +2938,17 @@ static blk_status_t floppy_queue_rq(struct blk_mq_hw_ctx *hctx,
> (unsigned long long) current_req->cmd_flags))
> return BLK_STS_IOERR;
>
> - spin_lock_irq(&floppy_lock);
> - list_add_tail(&bd->rq->queuelist, &floppy_reqs);
> - spin_unlock_irq(&floppy_lock);
> -
> if (test_and_set_bit(0, &fdc_busy)) {
> /* fdc busy, this new request will be treated when the
> current one is done */
> is_alive(__func__, "old request running");
> - return BLK_STS_OK;
> + return BLK_STS_RESOURCE;
> }
>
> + spin_lock_irq(&floppy_lock);
> + list_add_tail(&bd->rq->queuelist, &floppy_reqs);
> + spin_unlock_irq(&floppy_lock);
> +
> command_status = FD_COMMAND_NONE;
> __reschedule_timeout(MAXTIMEOUT, "fd_request");
> set_fdc(0);
>
> --
> Jiri Kosina
> SUSE Labs
>
>
--
Jiri Kosina
SUSE Labs
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] block/floppy: fix contended case in floppy_queue_rq()
2020-05-26 9:49 [PATCH] block/floppy: fix contended case in floppy_queue_rq() Jiri Kosina
2020-05-26 18:31 ` Jiri Kosina
@ 2020-05-26 21:25 ` Jens Axboe
1 sibling, 0 replies; 3+ messages in thread
From: Jens Axboe @ 2020-05-26 21:25 UTC (permalink / raw)
To: Jiri Kosina, Denis Efremov; +Cc: linux-block, linux-kernel, Libor Pechacek
On 5/26/20 3:49 AM, Jiri Kosina wrote:
> From: Jiri Kosina <jkosina@suse.cz>
>
> Since the switch of floppy driver to blk-mq, the contended (fdc_busy) case
> in floppy_queue_rq() is not handled correctly.
>
> In case we reach floppy_queue_rq() with fdc_busy set (i.e. with the floppy
> locked due to another request still being in-flight), we put the request
> on the list of requests and return BLK_STS_OK to the block core, without
> actually scheduling delayed work / doing further processing of the
> request. This means that processing of this request is postponed until
> another request comes and passess uncontended.
>
> Which in some cases might actually never happen and we keep waiting
> indefinitely. The simple testcase is
>
> for i in `seq 1 2000`; do echo -en $i '\r'; blkid --info /dev/fd0 2> /dev/null; done
>
> run in quemu. That reliably causes blkid eventually indefinitely hanging
> in __floppy_read_block_0() waiting for completion, as the BIO callback
> never happens, and no further IO is ever submitted on the (non-existent)
> floppy device. This was observed reliably on qemu-emulated device.
>
> Fix that by not queuing the request in the contended case, and return
> BLK_STS_RESOURCE instead, so that blk core handles the request
> rescheduling and let it pass properly non-contended later.
Applied, thanks.
--
Jens Axboe
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2020-05-26 21:25 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-05-26 9:49 [PATCH] block/floppy: fix contended case in floppy_queue_rq() Jiri Kosina
2020-05-26 18:31 ` Jiri Kosina
2020-05-26 21:25 ` Jens Axboe
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.