Messages in this thread | | | Subject | Re: [PATCH] lib/idr.c: Zero memory properly in idr_remove_all | From | Kristian Høgsberg <> | Date | Mon, 12 Jan 2009 15:38:32 -0500 |
| |
On Mon, 2009-01-12 at 20:53 +0100, Manfred Spraul wrote: > Kristian Høgsberg wrote: > > The problem > > isn't about returning un-zeroed-out objects to the kmem cache, the > > problem is returning them to the idr free list. > > > I think this is wrong: > The slab allocator assumes that the objects that are given to > kmem_cache_free() are properly constructed. > I.e.: No additional constructor is called prior to returning the object > from the next kmem_cache_alloc() call.
That's fine, the ctor associated with the kmem cache is called, and in the case of idr, it does a memset().
> > Every idr use I've seen could just do the whole thing > > under a mutex and not worry about the awkward retry idea. > Unfortunately there are some users that do idr_get_new() within a spinlock. > e.g. from drivers/gpu/drm/drm_gem.c: > > if (idr_pre_get(&file_priv->object_idr, GFP_KERNEL) == 0) > > return -ENOMEM; > > > > /* do the allocation under our spinlock */ > > spin_lock(&file_priv->table_lock); > > ret = idr_get_new_above(&file_priv->object_idr, obj, 1, handlep); > > spin_unlock(&file_priv->table_lock); > :-(
That's valid use of the idr interface, idr_get_new_above() won't block or allocate, just return -EAGAIN if the idr_layer struct allocated by idr_pre_get() got snatched up in between the two calls. My complaint was that in many cases you don't need to access the idr from interrupt context and you can then just put the idr_pre_get() and idr_get_new_above() under one big mutex. Even so, many people just copy-n-paste the boiler plate code that does the idr_pre_get()/idr_get_new_above()/if(EAGAIN) goto retry sequence.
cheers, Kristian
-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |