[BUG] virtio-pci queue allocation not page-aligned

[BUG] virtio-pci queue allocation not page-aligned

[Hollis Blanchard]

I just spent a number of hours tracking this one down, and I'm not too
thrilled about it. vp_find_vq() does the memory allocation for virtio
PCI rings, and it uses kzalloc() to do it. This is bad because the ring
memory *must* be page-aligned.

According to Anthony, at the time this code was written, various slab
allocators were checked and all happened to return page-aligned buffers.
So how did I hit a problem? I had enabled CONFIG_SLUB_DEBUG_ON while
investigating an unrelated problem, which offset the address by 64
bytes.

One option is to add a BUG_ON(addr & ~PAGE_MASK) to vp_find_vq(). That's
better than nothing, but still stinks.

Another is to use Kconfig to express that slab debugging breaks virtio.
Also pretty lame IMHO, will look pretty funny in the Kconfig file, and
that only solves today's problem. Another slab allocator or a change in
behavior of an existing allocator could mean that "ordinary" allocations
also become non-page-aligned.

Finally, we could use the interface intended for exactly this purpose:
the page allocator. If there's some problem with high memory, don't
allocate it with GFP_HIGHMEM.

-- 
Hollis Blanchard
IBM Linux Technology Center

Re: [BUG] virtio-pci queue allocation not page-aligned

[Anthony Liguori]

Hollis Blanchard wrote:
> Finally, we could use the interface intended for exactly this purpose:
> the page allocator. If there's some problem with high memory, don't
> allocate it with GFP_HIGHMEM.
>   

Can you work up a patch to do this?

Regards,

Anthony Liguori

Re: [BUG] virtio-pci queue allocation not page-aligned

[Rusty Russell]

On Wednesday 03 December 2008 05:38:21 Hollis Blanchard wrote:
> I just spent a number of hours tracking this one down, and I'm not too
> thrilled about it. vp_find_vq() does the memory allocation for virtio
> PCI rings, and it uses kzalloc() to do it. This is bad because the ring
> memory *must* be page-aligned.
>
> According to Anthony, at the time this code was written, various slab
> allocators were checked and all happened to return page-aligned buffers.
> So how did I hit a problem? I had enabled CONFIG_SLUB_DEBUG_ON while
> investigating an unrelated problem, which offset the address by 64
> bytes.
>
> One option is to add a BUG_ON(addr & ~PAGE_MASK) to vp_find_vq(). That's
> better than nothing, but still stinks.

It's a bug, we fix it.  I've complained before, but since there was no 
evidence of it actually breaking, I didn't push.

Prepare a patch, I'll try to get it in this release.

Thanks,
Rusty.

Re: [BUG] virtio-pci queue allocation not page-aligned

[Hollis Blanchard]

On Wed, 2008-12-03 at 08:35 +1030, Rusty Russell wrote:
> On Wednesday 03 December 2008 05:38:21 Hollis Blanchard wrote:
> > I just spent a number of hours tracking this one down, and I'm not too
> > thrilled about it. vp_find_vq() does the memory allocation for virtio
> > PCI rings, and it uses kzalloc() to do it. This is bad because the ring
> > memory *must* be page-aligned.
> >
> > According to Anthony, at the time this code was written, various slab
> > allocators were checked and all happened to return page-aligned buffers.
> > So how did I hit a problem? I had enabled CONFIG_SLUB_DEBUG_ON while
> > investigating an unrelated problem, which offset the address by 64
> > bytes.
> >
> > One option is to add a BUG_ON(addr & ~PAGE_MASK) to vp_find_vq(). That's
> > better than nothing, but still stinks.
> 
> It's a bug, we fix it.  I've complained before, but since there was no 
> evidence of it actually breaking, I didn't push.
> 
> Prepare a patch, I'll try to get it in this release.

virtio: ring queues must be page-aligned

kzalloc() does not guarantee page alignment, and in fact this broke when
I enabled CONFIG_SLUB_DEBUG_ON.

Signed-off-by: Hollis Blanchard <hollisb@us.ibm.com>
---
Tested with virtio-blk root filesystem.

diff --git a/drivers/virtio/virtio_pci.c b/drivers/virtio/virtio_pci.c
--- a/drivers/virtio/virtio_pci.c
+++ b/drivers/virtio/virtio_pci.c
@@ -237,7 +237,8 @@ static struct virtqueue *vp_find_vq(stru
 	info->queue_index = index;
 	info->num = num;
 
-	info->queue = kzalloc(PAGE_ALIGN(vring_size(num)), GFP_KERNEL);
+	info->queue = alloc_pages_exact(PAGE_ALIGN(vring_size(num)),
+	                                GFP_KERNEL|__GFP_ZERO);
 	if (info->queue == NULL) {
 		err = -ENOMEM;
 		goto out_info;


-- 
Hollis Blanchard
IBM Linux Technology Center