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
