public inbox for linux-block@vger.kernel.org
 help / color / mirror / Atom feed
From: Michael Schmitz <schmitzmic@gmail.com>
To: linux-m68k@vger.kernel.org, geert@linux-m68k.org
Cc: schmitzmic@gmail.com, linux-block@vger.kernel.org,
	Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Subject: [PATCH RFC] block - ataflop.c: fix breakage introduced at blk-mq refactoring
Date: Tue, 19 Oct 2021 11:21:57 +1300	[thread overview]
Message-ID: <20211018222157.12238-1-schmitzmic@gmail.com> (raw)

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 drivers' 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.

Testing this change, I've only ever seen single sector requests with the
'last' flag set. If there is a way to send requests to the driver
without that flag set, I'd appreciate a hint. As it now stands,
the driver won't release the ST-DMA lock on requests that don't have
this flag set, but won't accept further requests because the attempt
to acquire the already-held lock once more will fail.

Signed-off-by: Michael Schmitz <schmitzmic@gmail.com>
Fixes: 6ec3938cff95f (ataflop: convert to blk-mq)
CC: linux-block@vger.kernel.org
CC: Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
---
 drivers/block/ataflop.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/drivers/block/ataflop.c b/drivers/block/ataflop.c
index 96e2abe34a72..95469b4a8f78 100644
--- a/drivers/block/ataflop.c
+++ b/drivers/block/ataflop.c
@@ -653,8 +653,10 @@ static inline void copy_buffer(void *from, void *to)
 		*p2++ = *p1++;
 }
 
+/* finish_fdc() handling */
   
-  
+static void (*fdc_finish_action)( void ) = NULL;
+
 
 /* General Interrupt Handling */
 
@@ -1228,6 +1230,12 @@ static void fd_rwsec_done1(int status)
 	}
 	else {
 		/* all sectors finished */
+		void (*handler)( void );
+
+		handler = xchg(&fdc_finish_action, NULL);
+		if (handler)
+			handler();
+
 		fd_end_request_cur(BLK_STS_OK);
 	}
 	return;
@@ -1391,6 +1399,11 @@ static void finish_fdc_done( int dummy )
 	DPRINT(("finish_fdc() finished\n"));
 }
 
+static void queue_finish_fdc( void )
+{
+	fdc_finish_action = finish_fdc;
+}
+
 /* The detection of disk changes is a dark chapter in Atari history :-(
  * Because the "Drive ready" signal isn't present in the Atari
  * hardware, one has to rely on the "Write Protect". This works fine,
@@ -1491,6 +1504,8 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
 	int drive = floppy - unit;
 	int type = floppy->type;
 
+	DPRINT(("Queue request: drive %d type %d\n", drive, type));
+
 	spin_lock_irq(&ataflop_lock);
 	if (fd_request) {
 		spin_unlock_irq(&ataflop_lock);
@@ -1551,7 +1566,7 @@ static blk_status_t ataflop_queue_rq(struct blk_mq_hw_ctx *hctx,
 	do_fd_action( drive );
 
 	if (bd->last)
-		finish_fdc();
+		queue_finish_fdc();
 	atari_enable_irq( IRQ_MFP_FDC );
 
 out:
-- 
2.17.1


             reply	other threads:[~2021-10-18 22:22 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-10-18 22:21 Michael Schmitz [this message]
2021-10-18 22:30 ` [PATCH RFC] block - ataflop.c: fix breakage introduced at blk-mq refactoring Jens Axboe
2021-10-18 22:51   ` Finn Thain
2021-10-18 23:07     ` Jens Axboe
2021-10-18 23:17       ` Finn Thain
2021-10-18 23:28         ` Jens Axboe
2021-10-19  0:14           ` Finn Thain
2021-10-19  0:41             ` Jens Axboe
2021-10-18 23:35   ` Michael Schmitz
2021-10-18 23:40     ` Jens Axboe
2021-10-19  0:42       ` Michael Schmitz
2021-10-19  0:44         ` Jens Axboe
2021-10-18 23:55     ` Omar Sandoval

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20211018222157.12238-1-schmitzmic@gmail.com \
    --to=schmitzmic@gmail.com \
    --cc=geert@linux-m68k.org \
    --cc=linux-block@vger.kernel.org \
    --cc=linux-m68k@vger.kernel.org \
    --cc=penguin-kernel@i-love.sakura.ne.jp \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox