Hi Jens,
On Tue, Oct 19, 2021 at 12:40 PM Jens Axboe <axboe@xxxxxxxxx> wrote:
'last' is set if it's the last of a sequence of ->queue_rq() calls. If
you just do sync IO, then last is always set, as there is no sequence.
It's not hard to generate sequences, but on a floppy with basically no
queue depth the most you'd ever get is 2. You could try and set:
/sys/block/<dev>/queue/max_sectors_kb
to 4 for example, and then do something that generates a larger than 4k
write or read. Ideally that should give you more than 1.
Thanks, tried that - that does indeed cause multiple requests queued to
the driver (which rejects them promptly).
Now fails because ataflop_commit_rqs() unconditionally calls
finish_fdc() right after the first request started processing- and
promptly wipes it again.
What is the purpose of .commit_rqs? The PC legacy floppy driver doesn't
use it ...
You only need to care about bd->last if you have something in the driver
that can make it cheaper to commit more than one request. An example is
a driver that fills in requests, and then has an operation to ring the
submission doorbell to flush them out. The latter is what ->commit_rqs
is for.
OK, that's indeed a no-op for our floppy driver, which can queue
exactly one request.
For a floppy driver, just ignore bd->last and don't implement
commit_rqs, I don't think we're squeezing a lot of extra efficiency out
of it through that! Think many hundreds of thousands of IOPS or millions
of IOPS, not a handful of IOPS or less.
I'm not averse to using bd->last to close down only after the last
request in a sequence if it can be done safely (i.e. the requests that
had been rejected are then promptly requeued). But complexity is the
enemy of maintainability, so the nice and easy fix should be enough.
I'll respin and send another version shortly.
Cheers,
Michael
