On 10/18/21 6:42 PM, Michael Schmitz wrote: > 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. Right, and the only reason the depth is set to 2 is to allow one for merging purposes. >> 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. With just 2 requests, any sequence is going to be pretty limited :-). My recommendation would be to just ignore bd->last and treat any request as a standalone unit. Should make for easier code too, and you won't have two different cases to handle. > I'll respin and send another version shortly. Great, thanks. -- Jens Axboe
