Hans Verkuil wrote: > Hi Sakari, Hi Hans, And thanks for the comments! ... >> @@ -103,7 +105,8 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct >> v4l2_event *event) >> ev = list_first_entry(&events->available, struct _v4l2_event, list); >> list_del(&ev->list); >> >> - ev->event.count = !list_empty(&events->available); >> + atomic_dec(&events->navailable); >> + ev->event.count = atomic_read(&events->navailable); > > Combine these two lines to atomic_dec_return(). Will fix this. >> >> spin_unlock_irqrestore(&events->lock, flags); >> >> @@ -159,6 +162,9 @@ void v4l2_event_queue(struct video_device *vdev, >> struct v4l2_event *ev) >> if (!v4l2_event_subscribed(fh, ev->type)) >> continue; >> >> + if (atomic_read(&fh->events.navailable) >= V4L2_MAX_EVENTS) >> + continue; >> + >> _ev = kmem_cache_alloc(event_kmem, GFP_ATOMIC); >> if (!_ev) >> continue; >> @@ -169,6 +175,8 @@ void v4l2_event_queue(struct video_device *vdev, >> struct v4l2_event *ev) >> list_add_tail(&_ev->list, &fh->events.available); >> spin_unlock(&fh->events.lock); >> >> + atomic_inc(&fh->events.navailable); >> + >> wake_up_all(&fh->events.wait); >> } >> >> diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h >> index b11de92..69305c6 100644 >> --- a/include/media/v4l2-event.h >> +++ b/include/media/v4l2-event.h >> @@ -28,6 +28,10 @@ >> #include <linux/types.h> >> #include <linux/videodev2.h> >> >> +#include <asm/atomic.h> >> + >> +#define V4L2_MAX_EVENTS 1024 /* Ought to be enough for >> everyone. */ > > I think this should be programmable by the driver. Most drivers do not use > events at all, so by default it should be 0 or perhaps it can check whether > the ioctl callback structure contains the event ioctls and set it to 0 or > some initial default value. Right. I'll make the event queue size to be defined by the driver. I'm now planning to make a queue for free events common to file handles in video device. A statically allocated queue for each file handle is probably too much overkill. But a device global queue also means that a process that doesn't dequeue its events will starve the others. > And you want this to be controlled on a per-filehandle basis even. If I > look > at ivtv, then most of the device nodes will not have events, only a few > will > support events. And for one device node type I know that there will only be > a single event when stopping the streaming, while another device node type > will get an event each frame. Instead of initialising the events by the V4L2, the driver could do this and specify the queue size at the same time. The overhead for the drivers not using events would be the event information in the video_device structure. That could be made a pointer as well. > So being able to adjust the event queue dynamically will give more control > and prevent unnecessary waste of memory resources. This sounds to me like trying to re-invent the kmem_cache now. :-) If the event queue is allocated in some other means than kmem_cache I think the size should be fixed. The driver probably knows the best what's the reasonable maximum event queue size and that could be allocated statically. If that overflows then so be it. Regards, -- 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
