virtio-gpu vq code isn't thread safe

virtio-gpu vq code isn't thread safe

[Dave Airlie]

Hey Gerd,

This probably doesn't matter yet, but they virtgpu_vq buffer
allocation code you added isn't threadsafe.

I'm not sure what was wrong with the original code I wrote, you never
contacted me with reasons for rewriting it, but the Linux kmalloc
isn't that slow, but if we want some form of cached allocator you
probably should just use kmem_cache_create instead of what you have
there.

The second any code hits that from two threads its going to explode as
the free buffers list isn't protected with a lock of any kind, I know
userspace might not be able to hit this at present, but this is a big
problem when we add the 3D code.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: virtio-gpu vq code isn't thread safe

[Gerd Hoffmann]

On Di, 2015-06-16 at 14:55 +1000, Dave Airlie wrote:
> Hey Gerd,
> 
> This probably doesn't matter yet, but they virtgpu_vq buffer
> allocation code you added isn't threadsafe.

Right, I'll add a lock (or do you have a patch already?)

> I'm not sure what was wrong with the original code I wrote, you never
> contacted me with reasons for rewriting it, but the Linux kmalloc
> isn't that slow, but if we want some form of cached allocator you
> probably should just use kmem_cache_create instead of what you have
> there.

That came up during patch review.  Problem isn't kmalloc performance,
kmalloc (ENOMEM) but error handling.  When we allocate stuff in advance
a bunch of nasty error paths simply can't happen.

cheers,
  Gerd


_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel

Re: virtio-gpu vq code isn't thread safe

[Dave Airlie]

On 16 June 2015 at 17:12, Gerd Hoffmann <kraxel@redhat.com> wrote:
> On Di, 2015-06-16 at 14:55 +1000, Dave Airlie wrote:
>> Hey Gerd,
>>
>> This probably doesn't matter yet, but they virtgpu_vq buffer
>> allocation code you added isn't threadsafe.
>
> Right, I'll add a lock (or do you have a patch already?)

I've got a patch in my tree to add a spinlock around it,

http://cgit.freedesktop.org/~airlied/linux/commit/?h=virtio-gpu&id=e5adb6e41394ac16dbbaffc961e196103f7b07fa

but I'd need to rebase it out and send it, feel free to steal and add
my S-o-b to it.

Dave.
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/dri-devel