[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Thomas Hellstrom
On 11/28/2010 04:13 PM, Daniel Vetter wrote: > Hi Thomas, > > On Sun, Nov 28, 2010 at 03:19:50PM +0100, Thomas Hellstrom wrote: > >> I'm not saying that the current gem code is incorrect. In >> particular, the gem name system allows upping the handle_count even >> if it's zero and the name is j

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Daniel Vetter
Hi Thomas, On Sun, Nov 28, 2010 at 03:19:50PM +0100, Thomas Hellstrom wrote: > I'm not saying that the current gem code is incorrect. In > particular, the gem name system allows upping the handle_count even > if it's zero and the name is just about to be destroyed. I see that > more as a curiosity

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Thomas Hellstrom
On 11/28/2010 02:35 PM, Daniel Vetter wrote: > On Sun, Nov 28, 2010 at 01:32:27PM +0100, Thomas Hellstrom wrote: > >> This is racy, in that the kref_get() can hit a zero refcount. I >> think an ideal thing here would be to add a kref_get_unless_zero() >> for this situation, that returns an erro

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Daniel Vetter
On Sun, Nov 28, 2010 at 01:32:27PM +0100, Thomas Hellstrom wrote: > This is racy, in that the kref_get() can hit a zero refcount. I > think an ideal thing here would be to add a kref_get_unless_zero() > for this situation, that returns an error if the refcount was indeed > zero. I don't think that

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Thomas Hellstrom
On 11/25/2010 11:38 PM, Dave Airlie wrote: > On Thu, 2010-11-25 at 21:40 +, Chris Wilson wrote: > >> For a deferred-free cache of unreferenced bound objects, a simple >> reference count is required without the baggage of kref. >> > eh? > > you've just out of lined kref for no real gai

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Thomas Hellstrom
On 11/28/2010 04:13 PM, Daniel Vetter wrote: Hi Thomas, On Sun, Nov 28, 2010 at 03:19:50PM +0100, Thomas Hellstrom wrote: I'm not saying that the current gem code is incorrect. In particular, the gem name system allows upping the handle_count even if it's zero and the name is just about to

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Daniel Vetter
Hi Thomas, On Sun, Nov 28, 2010 at 03:19:50PM +0100, Thomas Hellstrom wrote: > I'm not saying that the current gem code is incorrect. In > particular, the gem name system allows upping the handle_count even > if it's zero and the name is just about to be destroyed. I see that > more as a curiosity

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Thomas Hellstrom
On 11/28/2010 02:35 PM, Daniel Vetter wrote: On Sun, Nov 28, 2010 at 01:32:27PM +0100, Thomas Hellstrom wrote: This is racy, in that the kref_get() can hit a zero refcount. I think an ideal thing here would be to add a kref_get_unless_zero() for this situation, that returns an error if the r

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Daniel Vetter
On Sun, Nov 28, 2010 at 01:32:27PM +0100, Thomas Hellstrom wrote: > This is racy, in that the kref_get() can hit a zero refcount. I > think an ideal thing here would be to add a kref_get_unless_zero() > for this situation, that returns an error if the refcount was indeed > zero. I don't think that

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-28 Thread Thomas Hellstrom
On 11/25/2010 11:38 PM, Dave Airlie wrote: On Thu, 2010-11-25 at 21:40 +, Chris Wilson wrote: For a deferred-free cache of unreferenced bound objects, a simple reference count is required without the baggage of kref. eh? you've just out of lined kref for no real gain. the whole

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-26 Thread Dave Airlie
On Thu, 2010-11-25 at 21:40 +, Chris Wilson wrote: > For a deferred-free cache of unreferenced bound objects, a simple > reference count is required without the baggage of kref. eh? you've just out of lined kref for no real gain. the whole point of kref is that its standard and doesn't requi

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Alan Cox
> void kref_get(struct kref *kref) > { > WARN_ON(!atomic_read(&kref->refcount)); > atomic_inc(&kref->refcount); > smp_mb__after_atomic_inc(); > } > > which causes havoc when you are trying to keep a list of unreferenced > objects. That's all I'm trying to avoid. You cannot

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Chris Wilson
On Fri, 26 Nov 2010 08:38:29 +1000, Dave Airlie wrote: > On Thu, 2010-11-25 at 21:40 +, Chris Wilson wrote: > > For a deferred-free cache of unreferenced bound objects, a simple > > reference count is required without the baggage of kref. > > eh? The issue with kref is that it does: void kr

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Chris Wilson
For a deferred-free cache of unreferenced bound objects, a simple reference count is required without the baggage of kref. Signed-off-by: Chris Wilson --- drivers/gpu/drm/drm_gem.c | 12 +++- drivers/gpu/drm/drm_info.c |2 +- include/drm/drmP.h | 16 +--- 3 f

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Alan Cox
> void kref_get(struct kref *kref) > { > WARN_ON(!atomic_read(&kref->refcount)); > atomic_inc(&kref->refcount); > smp_mb__after_atomic_inc(); > } > > which causes havoc when you are trying to keep a list of unreferenced > objects. That's all I'm trying to avoid. You cannot

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Chris Wilson
On Fri, 26 Nov 2010 08:38:29 +1000, Dave Airlie wrote: > On Thu, 2010-11-25 at 21:40 +, Chris Wilson wrote: > > For a deferred-free cache of unreferenced bound objects, a simple > > reference count is required without the baggage of kref. > > eh? The issue with kref is that it does: void kr

Re: [PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Dave Airlie
On Thu, 2010-11-25 at 21:40 +, Chris Wilson wrote: > For a deferred-free cache of unreferenced bound objects, a simple > reference count is required without the baggage of kref. eh? you've just out of lined kref for no real gain. the whole point of kref is that its standard and doesn't requi

[PATCH] drm: Replace kref with a simple atomic reference count

2010-11-25 Thread Chris Wilson
For a deferred-free cache of unreferenced bound objects, a simple reference count is required without the baggage of kref. Signed-off-by: Chris Wilson --- drivers/gpu/drm/drm_gem.c | 12 +++- drivers/gpu/drm/drm_info.c |2 +- include/drm/drmP.h | 16 +--- 3 f