On Sun, 2 Dec 2018, Hannes Reinecke wrote: > On 12/2/18 10:21 PM, Finn Thain wrote: > > On Sun, 2 Dec 2018, Hannes Reinecke wrote: > > > > > Well, that lone 'kmap' is due to a quirk/errata in the datasheet; > > > essentially > > > we have to PIO a lone byte out of the FIFO to clear it up. > > > And this byte is technically still part of the SCSI data, so we need to > > > stuff it onto the end of the actual data sg list. Which is what the kmap() > > > thingie does. > > > So it really shouldn't be affected by the clustering algorithm. > > > > > > > Sorry, I don't follow. > > > > If it's dead code, can it be removed > > > Oh, it's not dead code. It's required as per datasheet. > > > If it's not, does it require DISABLE_CLUSTERING? > > > No, not really. It just affects the very last byte of the sglist, > so I can't really see how it should be affected by clustering. > AIUI, the scsi_kmap_atomic_sg() call which you added to esp_scsi.c assumes that the sg list elements are page sized and page aligned. DISABLE_CLUSTERING provides that guarantee, but am53c974.c doesn't use it. Is this not a bug? What am I missing? BTW, Documentation/block/biodoc.txt suggests a bounce buffer as an alternative. There are some situations when pages from high memory may need to be kmapped, even if bounce buffers are not necessary. For example a device may need to abort DMA operations and revert to PIO for the transfer, in which case a virtual mapping of the page is required. For SCSI it is also done in some scenarios where the low level driver cannot be trusted to handle a single sg entry correctly. The driver is expected to perform the kmaps as needed on such occasions as appropriate. A driver could also use the blk_queue_bounce() routine on its own to bounce highmem i/o to low memory for specific requests if so desired. -- > Cheers, > > Hannes > >