On Thu, Oct 18, 2018 at 07:33:50PM -0600, Jens Axboe wrote: > On 10/18/18 7:28 PM, Ming Lei wrote: > > On Thu, Oct 18, 2018 at 08:27:28AM -0600, Jens Axboe wrote: > >> On 10/18/18 7:18 AM, Ming Lei wrote: > >>> Now we only check if DMA IO buffer is aligned to queue_dma_alignment() > >>> for pass-through request, and it isn't done for normal IO request. > >>> > >>> Given the check has to be done on each bvec, it isn't efficient to add the > >>> check in generic_make_request_checks(). > >>> > >>> This patch addes one WARN in blk_queue_split() for capturing this issue. > >> > >> I don't want to do this, because then we are forever doomed to > >> have something that fully loops a bio at submission time. I > >> absolutely hate the splitting we have and the need for it, > >> hopefully it can go away for a subset of IOs at some point. > >> > >> In many ways, this seems to be somewhat of a made-up problem, I don't > >> recall a single bug report for something like this over decades of > >> working with the IO stack. 512b alignment restrictions for DMA seems > >> absolutely insane. I know people claim they exist, but clearly that > >> isn't a hard requirement or we would have been boned years ago. > > > > There are still some drivers with this requirement: > > > > drivers/ata/libata-scsi.c:1308: blk_queue_update_dma_alignment(q, > > sdev->sector_size - 1); > > drivers/ata/pata_macio.c:812: > > blk_queue_update_dma_alignment(sdev->request_queue, 31); > > drivers/ata/pata_macio.c:827: > > blk_queue_update_dma_alignment(sdev->request_queue, 15); > > drivers/block/ps3disk.c:470: blk_queue_dma_alignment(queue, > > dev->blk_size-1); > > drivers/block/rsxx/dev.c:282: > > blk_queue_dma_alignment(card->queue, blk_size - 1); > > drivers/block/xen-blkfront.c:957: blk_queue_dma_alignment(rq, 511); > > drivers/ide/ide-cd.c:1512: blk_queue_dma_alignment(q, 31); > > drivers/message/fusion/mptscsih.c:2388: blk_queue_dma_alignment > > (sdev->request_queue, 512 - 1); > > drivers/staging/rts5208/rtsx.c:94: > > blk_queue_dma_alignment(sdev->request_queue, (512 - 1)); > > drivers/usb/image/microtek.c:329: > > blk_queue_dma_alignment(s->request_queue, (512 - 1)); > > drivers/usb/storage/scsiglue.c:92: > > blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1)); > > drivers/usb/storage/uas.c:818: > > blk_queue_update_dma_alignment(sdev->request_queue, (512 - 1)); > > Of course, I too can grep :-) > > My point is that these settings might not match reality. And the > WARN_ON(), as implemented, is going to trigger on any device that > DOESN'T set the alignment, as Bart pointed out.
It is just a WARN_ON_ONCE() which exactly shows something which need further attention, then related people may take a look and we can move on. So I think it is correct thing to do. Thanks, Ming
