block: ataflop: fix breakage introduced at blk-mq refactoring

Refactoring of the Atari floppy driver when converting to blk-mq
has broken the state machine in not-so-subtle ways:

finish_fdc() must be called when operations on the floppy device
have completed. This is crucial in order to relase the ST-DMA
lock, which protects against concurrent access to the ST-DMA
controller by other drivers (some DMA related, most just related
to device register access - broken beyond compare, I know).

When rewriting the driver's old do_request() function, the fact
that finish_fdc() was called only when all queued requests had
completed appears to have been overlooked. Instead, the new
request function calls finish_fdc() immediately after the last
request has been queued. finish_fdc() executes a dummy seek after
most requests, and this overwrites the state machine's interrupt
hander that was set up to wait for completion of the read/write
request just prior. To make matters worse, finish_fdc() is called
before device interrupts are re-enabled, making certain that the
read/write interupt is missed.

Shifting the finish_fdc() call into the read/write request
completion handler ensures the driver waits for the request to
actually complete. With a queue depth of 2, we won't see long
request sequences, so calling finish_fdc() unconditionally just
adds a little overhead for the dummy seeks, and keeps the code
simple.

While we're at it, kill ataflop_commit_rqs() which does nothing
but run finish_fdc() unconditionally, again likely wiping out an
in-flight request.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
Fixes: 6ec3938cff ("ataflop: convert to blk-mq")
CC: linux-block@vger.kernel.org
CC: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Link: https://lore.kernel.org/r/20211019061321.26425-1-schmitzmic@gmail.com
Signed-off-by: Jens Axboe <axboe@kernel.dk>
This commit is contained in:
Michael Schmitz 2021-10-19 19:13:21 +13:00 committed by Jens Axboe
parent 8663b210f8
commit 86d46fdaa1

View File

@ -655,9 +655,6 @@ static inline void copy_buffer(void *from, void *to)
*p2++ = *p1++; *p2++ = *p1++;
} }
/* General Interrupt Handling */ /* General Interrupt Handling */
static void (*FloppyIRQHandler)( int status ) = NULL; static void (*FloppyIRQHandler)( int status ) = NULL;
@ -1230,6 +1227,7 @@ static void fd_rwsec_done1(int status)
} }
else { else {
/* all sectors finished */ /* all sectors finished */
finish_fdc();
fd_end_request_cur(BLK_STS_OK); fd_end_request_cur(BLK_STS_OK);
} }
return; return;
@ -1477,15 +1475,6 @@ static void setup_req_params( int drive )
ReqTrack, ReqSector, (unsigned long)ReqData )); ReqTrack, ReqSector, (unsigned long)ReqData ));
} }
static void ataflop_commit_rqs(struct blk_mq_hw_ctx *hctx)
{
spin_lock_irq(&ataflop_lock);
atari_disable_irq(IRQ_MFP_FDC);
finish_fdc();
atari_enable_irq(IRQ_MFP_FDC);
spin_unlock_irq(&ataflop_lock);
}
static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx, static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
const struct blk_mq_queue_data *bd) const struct blk_mq_queue_data *bd)
{ {
@ -1493,6 +1482,8 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
int drive = floppy - unit; int drive = floppy - unit;
int type = floppy->type; int type = floppy->type;
DPRINT(("Queue request: drive %d type %d last %d\n", drive, type, bd->last));
spin_lock_irq(&ataflop_lock); spin_lock_irq(&ataflop_lock);
if (fd_request) { if (fd_request) {
spin_unlock_irq(&ataflop_lock); spin_unlock_irq(&ataflop_lock);
@ -1552,8 +1543,6 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
setup_req_params( drive ); setup_req_params( drive );
do_fd_action( drive ); do_fd_action( drive );
if (bd->last)
finish_fdc();
atari_enable_irq( IRQ_MFP_FDC ); atari_enable_irq( IRQ_MFP_FDC );
out: out:
@ -1964,7 +1953,6 @@ static const struct block_device_operations floppy_fops = {
static const struct blk_mq_ops ataflop_mq_ops = { static const struct blk_mq_ops ataflop_mq_ops = {
.queue_rq = ataflop_queue_rq, .queue_rq = ataflop_queue_rq,
.commit_rqs = ataflop_commit_rqs,
}; };
static int ataflop_alloc_disk(unsigned int drive, unsigned int type) static int ataflop_alloc_disk(unsigned int drive, unsigned int type)