[PATCH 23/31] swim: Don't needlessly re-read sectors

[Finn Thain]

floppy_read_sectors() is confusing because variable names seem to conflate
tracks and cylinders. Rename the track variable, eliminate a division
operation and adopt suitable integer types.

For readahead to work effectively, small sequential reads should not
require waiting for spindle rotation. Unfortunately, the present algorithm
is very inefficient and does a lot of unnecessary waiting.

E.g. if the device is asked to read sectors 1 thru 18, and if sector 9
happens to be under the heads, the driver will proceed to read sectors 9
thru 18, but discard the results, while it waits for sector 1 to arrive.

If sector 1 couldn't be read on the first attempt, and needs a retry, the
driver will proceed to read sectors 2 thru 18, but discard the results,
while it waits for sector 1 to come around again.

In between reading sector 1 and sector 2, the driver needlessly calls
swim_track() and swim_head() again. But what's worse is that, on a 68030
system, re-enabling interrupts after each sector read can result in a full
rotation between sectors (that is a 3 ms wait).

Floppy drivers often implement a track cache, that can be filled in a
single rotation, to solve problems like these. But I think this solution
is much simpler.

For each request, use a sector bitmap to keep a record of sectors read
successfully. Read or retry, as necessary, the requested sectors in
whatever sequence they arrive in. Keep interrupts disabled until the
whole track has passed under the read head.

Fixes: 8852ecd97488 ("m68k: mac - Add SWIM floppy support")
Signed-off-by: Finn Thain <fthain@xxxxxxxxxxxxxx>
---
 drivers/block/swim.c | 91 +++++++++++++++++++++++++-------------------
 1 file changed, 52 insertions(+), 39 deletions(-)

diff --git a/drivers/block/swim.c b/drivers/block/swim.c
index 0bab7ed3daa8..dae401e649d7 100644
--- a/drivers/block/swim.c
+++ b/drivers/block/swim.c
@@ -180,9 +180,9 @@ struct floppy_state {
 	enum media_type	 type;
 	int		 write_protected;
 
-	int		 total_secs;
-	int		 secpercyl;
-	int		 secpertrack;
+	unsigned int	 total_secs;
+	unsigned int	 secpercyl;
+	unsigned int	 secpertrack;
 
 	/* in-use information */
 
@@ -455,68 +455,81 @@ static int floppy_eject(struct floppy_state *fs)
 	return 0;
 }
 
-static inline int swim_read_sector(struct floppy_state *fs,
-				   int side, int track,
-				   int sector, unsigned char *buffer)
+static unsigned int swim_read_sector_range(struct floppy_state *fs,
+					unsigned int side, unsigned int track,
+					unsigned int start, unsigned int count,
+					unsigned char *buffer)
 {
 	struct swim __iomem *base = fs->swd->base;
 	unsigned long flags;
 	struct sector_header header;
-	int ret = -1;
-	short i;
+	int i, bits = 0;
 
-	swim_track(fs, track);
-	swim_head(base, side);
+	for (i = 0; i < 18; i++) { /* A track has at most 18 sectors */
+		if (i >= count)
+			break;
+		bits |= BIT(i);
+	}
 
 	local_irq_save(flags);
 	for (i = 0; i < 36; i++) {
+		if (bits == 0) /* All count sectors were read ok */
+			break;
+
 		if (swim_read_sector_header(base, &header) ||
 		    swim_read(base, error) || header.track != track ||
 		    header.side != side || header.size != 2)
 			continue;
-		if (header.sector == sector) {
-			/* found */
 
-			ret = swim_read_sector_data(base, buffer);
-			if (swim_read(base, error))
-				ret = -EIO;
-			break;
+		if (header.sector >= start && header.sector < start + count) {
+			unsigned int offset = header.sector - start;
+			int ret;
+
+			if ((bits & BIT(offset)) == 0)
+				continue; /* This sector was already read ok */
+
+			ret = swim_read_sector_data(base, buffer + 512 * offset);
+			if (ret == 512 && swim_read(base, error) == 0)
+				bits &= ~BIT(offset);
 		}
 	}
 	local_irq_restore(flags);
 
-	return ret;
+	return bits ? ffs(bits) - 1 : count; /* No. of contiguous ok sectors */
 }
 
 static blk_status_t floppy_read_sectors(struct floppy_state *fs,
-			       int req_sector, int sectors_nb,
-			       unsigned char *buffer)
+					unsigned int req_sector,
+					unsigned int sectors_nb,
+					unsigned char *buffer)
 {
 	struct swim __iomem *base = fs->swd->base;
-	int ret;
-	int side, track, sector;
-	int i, try;
-
+	unsigned int failures = 0;
 
 	swim_drive(base, fs->location);
 	swim_READY_timeout(base);
 
-	for (i = req_sector; i < req_sector + sectors_nb; i++) {
-		int x;
-		track = i / fs->secpercyl;
-		x = i % fs->secpercyl;
-		side = x / fs->secpertrack;
-		sector = x % fs->secpertrack + 1;
-
-		try = 5;
-		do {
-			ret = swim_read_sector(fs, side, track, sector,
-						buffer);
-			if (try-- == 0)
-				return BLK_STS_IOERR;
-		} while (ret != 512);
-
-		buffer += ret;
+	while (sectors_nb) {
+		unsigned int cyl, x, head, sector, n, ret;
+
+		cyl = req_sector / fs->secpercyl;
+		x = req_sector % fs->secpercyl;
+		head = (x >= fs->secpertrack) ? 1 : 0;
+		sector = x % fs->secpertrack;
+		n = min(sectors_nb, fs->secpertrack - sector);
+
+		swim_track(fs, cyl);
+		swim_head(base, head);
+
+		ret = swim_read_sector_range(fs, head, cyl, sector + 1, n, buffer);
+		if (ret != n)
+			++failures;
+		if (failures >= 5)
+			return BLK_STS_IOERR;
+
+		buffer += 512 * ret;
+		sectors_nb -= ret;
+		req_sector += ret;
 	}
 
 	return 0;
-- 
2.52.0