Re: [PATCH 0/5] block: introduce helpers for allocating io buffer from slab

[Bart Van Assche <bvanassche@acm.org>]

On Thu, 2018-10-18 at 07:03 -0700, Matthew Wilcox wrote:
> On Thu, Oct 18, 2018 at 09:18:12PM +0800, Ming Lei wrote:
> > Filesystems may allocate io buffer from slab, and use this buffer to
> > submit bio. This way may break storage drivers if they have special
> > requirement on DMA alignment.
> 
> Before we go down this road, could we have a discussion about what
> hardware actually requires this?  Storage has this weird assumption that
> I/Os must be at least 512 byte aligned in memory, and I don't know where
> this idea comes from.  Network devices can do arbitrary byte alignment.
> Even USB controllers can do arbitrary byte alignment.  Sure, performance
> is going to suck and there are definite risks on some architectures
> with doing IOs that are sub-cacheline aligned, but why is storage such a
> special snowflake that we assume that host controllers are only capable
> of doing 512-byte aligned DMAs?
> 
> I just dragged out the NCR53c810 data sheet from 1993, and it's capable of
> doing arbitrary alignment of DMA.  NVMe is capable of 4-byte aligned DMA.
> What hardware needs this 512 byte alignment?

How about starting with modifying the queue_dma_alignment() function? The
current implementation of that function is as follows:

static inline int queue_dma_alignment(struct request_queue *q)
{
	return q ? q->dma_alignment : 511;
}

In other words, for block drivers that do not set the DMA alignment
explicitly it is assumed that these drivers need 512 byte alignment. I think
the "512 byte alignment as default" was introduced in 2002. From Thomas
Gleixner's history tree, commit ad519c6902fb:

+static inline int queue_dma_alignment(request_queue_t *q)
+{
+       int retval = 511;
+
+       if (q && q->dma_alignment)
+               retval = q->dma_alignment;
+
+       return retval;
+}
+
+static inline int bdev_dma_aligment(struct block_device *bdev)
+{
+       return queue_dma_alignment(bdev_get_queue(bdev));
+}
+

Bart.