On 8/2/23 04:52, Christoph Hellwig wrote:
+/*
+ * Returns a negative value if @_a has a lower LBA than @_b, zero if
+ * both have the same LBA and a positive value otherwise.
+ */
+static int scsi_cmp_lba(void *priv, const struct list_head *_a,
+ const struct list_head *_b)
+{
+ struct scsi_cmnd *a = list_entry(_a, typeof(*a), eh_entry);
+ struct scsi_cmnd *b = list_entry(_b, typeof(*b), eh_entry);
+ const sector_t pos_a = blk_rq_pos(scsi_cmd_to_rq(a));
+ const sector_t pos_b = blk_rq_pos(scsi_cmd_to_rq(b));
The SCSI core has no concept of LBAs.
Agreed.
Even assuming something like this is a good idea (and I'm very
> doubtful) it would have to live in sd.c.
I can rename scsi_cmp_lba() into scsi_cmp_pos() or scsi_cmp_sector(). As
you know the flow of the sector information is as follows (the below is
incomplete):
* The filesystem allocated a bio, sets bi_sector and calls
bio_add_page().
* The filesystem calls submit_bio() and submit_bio() calls
blk_mq_bio_to_request() indirectly. blk_mq_bio_to_request() copies
bio->bi_iter.bi_sector into rq->__sector.
* scsi_queue_rq() is called and prepares a SCSI CDB by calling
sd_init_command() via a function pointer in sd_template.
* The SCSI CDB is submitted to the LLD by calling
host->hostt->queuecommand().
Since the rq->__sector information is provided before a request is
submitted by the SCSI core and since SCSI ULD drivers are not allowed to
modify that information, I think it is fine to read that information in
the SCSI core by calling blk_rq_pos().
Moving the code for sorting SCSI commands by sector into sd.c would be
cumbersome. It could be done e.g. as follows:
* Add a new function pointer in struct scsi_driver, e.g.
prepare_reinsert.
* In the sd driver, provide an implementation for that callback and make
it iterate over all requests and only sort those requests related to
the sd driver.
* In the scsi_eh_flush_done_q(), add the following logic:
- Iterate a first time over all SCSI commands that will be reinserted
and perform the equivalent of the shell command sort -u on the
scsi_driver.prepare_reinsert function pointers.
- For each distinct scsi_driver.prepare_reinsert function pointer,
call the function it points at and pass the entire list of commands
to that function.
My opinion is that moving the logic of sorting SCSI commands by sector
into the sd driver will result in more complicated code without
providing a real benefit. Did I perhaps overlook something?
Thanks,
Bart.
