Hans Verkuil wrote: > Hi Sakari, Hi, Hans! > The code looks good, but I'm not so sure about the use of kmem_cache_*. It > seems serious overkill to me. > > Most of the time there will only be a handful of event messages pending. So > setting up a kmem_cache for this seems unnecessary to me. > > A much better way to ensure fast event reporting IMHO would be to keep a > list > of empty event messages rather than destroying an event after it is > dequeued. > > So you have two queues per filehandle: pending and empty; initially both > are > empty. When a new event has to be queued the code checks if there are > events > available for reuse in the empty queue, and if not a new event struct is > allocated and added to the pending queue. I actually had this kind of setup there for a while. Then I thought it'd be too ugly and went for kmem_cache instead. The other reason is that it's convenient to keep the memory allocated even if there are no events subscribed or the device isn't open. For 1000 events that's 96 kiB. I guess an unused kmem_cache object consumes extra memory very little. The cached slabs can be explicitly freed anyway by the driver. The size of the kmem_cache also adjusts based on the number of events in the queue. Allocating kmem_cache objects is fast if they already exist, too. There can be temporary delays from allocation, of course. I can bring it back, sure, if you see a fixed allocation better. -- Sakari Ailus sakari.ailus@xxxxxxxxxxxxxxxxxxxxxxxxxx -- To unsubscribe from this list: send the line "unsubscribe linux-media" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
