Re: [PATCH v4 5/7] V4L: Events: Count event queue length

2010-02-13 Thread Hans Verkuil 
On Wednesday 10 February 2010 15:58:07 Sakari Ailus wrote:
 Update the count field properly by setting it to exactly to number of
 further available events.

After working with this for a bit I realized that the max_alloc thing is not
going to work. The idea behind it was that you can increase the event queue
up to max_alloc if there is no more free room left in the event queue for a
particular file handle. Nice idea, but the only place you can do that is in
v4l2_event_queue and doing it there will produce a locking nightmare.

The right place is when you subscribe to an event. Basically you just want
two functions: v4l2_event_alloc and v4l2_event_free. The latter replaces
v4l2_event_exit, the first replaces v4l2_event_init and the current
v4l2_event_alloc.

The new v4l2_event_alloc just specifies the size of the event queue that you
want. If it is the first time this function is called, then the fh-events
struct is allocated first. Next it just checks if the total number of allocated
events is less than the specified amount and if so it allocates events until
the required number is reached.

So typically you would only call v4l2_event_alloc when the user subscribes
to a particular event. And based on the event type you can call this function
with the desired queue size. For example in the case of ivtv the EOS event
would need just a single event while the VSYNC event might need 60.

So as long as the user does not subscribe to an event there is no memory
wasted. Once he does you will only allocate as much memory as is needed
for that particular event.

 
 Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
 ---
  drivers/media/video/v4l2-event.c |   29 +
  include/media/v4l2-event.h   |6 +-
  2 files changed, 22 insertions(+), 13 deletions(-)
 

cut

 diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
 index 580c9d4..671c8f7 100644
 --- a/include/media/v4l2-event.h
 +++ b/include/media/v4l2-event.h
 @@ -28,6 +28,8 @@
  #include linux/types.h
  #include linux/videodev2.h
  
 +#include asm/atomic.h

This header is not needed.

Regards,

Hans

 +
  struct v4l2_fh;
  struct video_device;
  
 @@ -45,11 +47,13 @@ struct v4l2_events {
   wait_queue_head_t   wait;
   struct list_headsubscribed; /* Subscribed events */
   struct list_headavailable; /* Dequeueable event */
 + unsigned intnavailable;
 + unsigned intmax_alloc; /* Never allocate more. */
   struct list_headfree; /* Events ready for use */
  };
  
  int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);
 -int v4l2_event_init(struct v4l2_fh *fh, unsigned int n);
 +int v4l2_event_init(struct v4l2_fh *fh, unsigned int n, unsigned int 
 max_alloc);
  void v4l2_event_exit(struct v4l2_fh *fh);
  int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event);
  struct v4l2_subscribed_event *v4l2_event_subscribed(
 

-- 
Hans Verkuil - video4linux developer - sponsored by TANDBERG
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v4 5/7] V4L: Events: Count event queue length

2010-02-10 Thread Sakari Ailus 
Update the count field properly by setting it to exactly to number of
further available events.

Signed-off-by: Sakari Ailus sakari.ai...@maxwell.research.nokia.com
---
 drivers/media/video/v4l2-event.c |   29 +
 include/media/v4l2-event.h   |6 +-
 2 files changed, 22 insertions(+), 13 deletions(-)

diff --git a/drivers/media/video/v4l2-event.c b/drivers/media/video/v4l2-event.c
index d13c1e9..bbdc149 100644
--- a/drivers/media/video/v4l2-event.c
+++ b/drivers/media/video/v4l2-event.c
@@ -41,7 +41,13 @@ int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n)
return -ENOMEM;
 
spin_lock_irqsave(fh-vdev-fh_lock, flags);
+   if (events-max_alloc == 0) {
+   spin_unlock_irqrestore(fh-vdev-fh_lock, flags);
+   kfree(kev);
+   return -ENOMEM;
+   }
list_add_tail(kev-list, events-free);
+   events-max_alloc--;
spin_unlock_irqrestore(fh-vdev-fh_lock, flags);
}
 
@@ -73,7 +79,7 @@ void v4l2_event_exit(struct v4l2_fh *fh)
 }
 EXPORT_SYMBOL_GPL(v4l2_event_exit);
 
-int v4l2_event_init(struct v4l2_fh *fh, unsigned int n)
+int v4l2_event_init(struct v4l2_fh *fh, unsigned int n, unsigned int max_alloc)
 {
int ret;
 
@@ -87,6 +93,9 @@ int v4l2_event_init(struct v4l2_fh *fh, unsigned int n)
INIT_LIST_HEAD(fh-events-available);
INIT_LIST_HEAD(fh-events-subscribed);
 
+   fh-events-navailable = 0;
+   fh-events-max_alloc = max_alloc;
+
ret = v4l2_event_alloc(fh, n);
if (ret  0)
v4l2_event_exit(fh);
@@ -108,11 +117,13 @@ int v4l2_event_dequeue(struct v4l2_fh *fh, struct 
v4l2_event *event)
return -ENOENT;
}
 
+   WARN_ON(events-navailable == 0);
+
kev = list_first_entry(events-available, struct v4l2_kevent, list);
list_move(kev-list, events-free);
+   events-navailable--;
 
-   kev-event.count = !list_empty(events-available);
-
+   kev-event.count = events-navailable;
*event = kev-event;
 
spin_unlock_irqrestore(fh-vdev-fh_lock, flags);
@@ -175,6 +186,8 @@ void v4l2_event_queue(struct video_device *vdev, struct 
v4l2_event *ev)
kev-event = *ev;
list_move_tail(kev-list, events-available);
 
+   events-navailable++;
+
wake_up_all(events-wait);
}
 
@@ -184,15 +197,7 @@ EXPORT_SYMBOL_GPL(v4l2_event_queue);
 
 int v4l2_event_pending(struct v4l2_fh *fh)
 {
-   struct v4l2_events *events = fh-events;
-   unsigned long flags;
-   int ret;
-
-   spin_lock_irqsave(fh-vdev-fh_lock, flags);
-   ret = !list_empty(events-available);
-   spin_unlock_irqrestore(fh-vdev-fh_lock, flags);
-
-   return ret;
+   return fh-events-navailable;
 }
 EXPORT_SYMBOL_GPL(v4l2_event_pending);
 
diff --git a/include/media/v4l2-event.h b/include/media/v4l2-event.h
index 580c9d4..671c8f7 100644
--- a/include/media/v4l2-event.h
+++ b/include/media/v4l2-event.h
@@ -28,6 +28,8 @@
 #include linux/types.h
 #include linux/videodev2.h
 
+#include asm/atomic.h
+
 struct v4l2_fh;
 struct video_device;
 
@@ -45,11 +47,13 @@ struct v4l2_events {
wait_queue_head_t   wait;
struct list_headsubscribed; /* Subscribed events */
struct list_headavailable; /* Dequeueable event */
+   unsigned intnavailable;
+   unsigned intmax_alloc; /* Never allocate more. */
struct list_headfree; /* Events ready for use */
 };
 
 int v4l2_event_alloc(struct v4l2_fh *fh, unsigned int n);
-int v4l2_event_init(struct v4l2_fh *fh, unsigned int n);
+int v4l2_event_init(struct v4l2_fh *fh, unsigned int n, unsigned int 
max_alloc);
 void v4l2_event_exit(struct v4l2_fh *fh);
 int v4l2_event_dequeue(struct v4l2_fh *fh, struct v4l2_event *event);
 struct v4l2_subscribed_event *v4l2_event_subscribed(
-- 
1.5.6.5

--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html