On Sat, Oct 28, 2006 at 03:34:52PM +0200, Eric Dumazet ([EMAIL PROTECTED]) 
wrote:
> >>>+  list_del(&k->ready_entry);
> >>Arg... no
> >>
> >>You cannot call list_del() , then check overflow_kevent.
> >>
> >>I you call list_del on what happens to be the kevent pointed by 
> >>overflow_kevent, you loose...
> >
> >This function is always called from appropriate context, where it is
> >guaranteed that it is safe to call list_del:
> >1. when kevent is removed. It is called after check, that given kevent 
> >is in the ready queue.
> >2. when dequeued from ready queue, which means that it can be removed
> >from that queue.
> >
> 
> Could you please check the list_del() function ?
> 
> file include/linux/list.h
> 
> static inline void list_del(struct list_head *entry)
> {
>   __list_del(entry->prev, entry->next);
>   entry->next = LIST_POISON1;
>   entry->prev = LIST_POISON2;
> }
> 
> So, after calling list_del(&k->read_entry);
> next and prev are basically destroyed.
> 
> So when you write later :
> 
> +        if (!err || u->overflow_kevent == k) {
> +            if (u->overflow_kevent->ready_entry.next == &u->ready_list)
> +                u->overflow_kevent = NULL;
> +            else
> +                u->overflow_kevent = + 
> list_entry(u->overflow_kevent->ready_entry.next, + 
> struct kevent, ready_entry);
> +        }
> 
> 
> then you have a problem, since
> 
> list_entry(k->ready_entry.next, struct kevent, ready_entry);
> 
> will give you garbage.

Ok, I understand you now.
To remove this issue we can delete entry from the list after all checks
with overflow_kevent pointer are completed, i.e. have something like
this:

diff --git a/kernel/kevent/kevent_user.c b/kernel/kevent/kevent_user.c
index 711a8a8..f3fec9b 100644
--- a/kernel/kevent/kevent_user.c
+++ b/kernel/kevent/kevent_user.c
@@ -235,6 +235,36 @@ static void kevent_free_rcu(struct rcu_h
 }
 
 /*
+ * Must be called under u->ready_lock.
+ * This function removes kevent from ready queue and 
+ * tries to add new kevent into ring buffer.
+ */
+static void kevent_remove_ready(struct kevent *k)
+{
+       struct kevent_user *u = k->user;
+
+       if (++u->pring[0]->uidx == KEVENT_MAX_EVENTS)
+               u->pring[0]->uidx = 0;
+
+       if (u->overflow_kevent) {
+               int err;
+
+               err = kevent_user_ring_add_event(u->overflow_kevent);
+               if (!err || u->overflow_kevent == k) {
+                       if (u->overflow_kevent->ready_entry.next == 
&u->ready_list)
+                               u->overflow_kevent = NULL;
+                       else
+                               u->overflow_kevent = 
+                                       
list_entry(u->overflow_kevent->ready_entry.next, 
+                                                       struct kevent, 
ready_entry);
+               }
+       }
+       list_del(&k->ready_entry);
+       k->flags &= ~KEVENT_READY;
+       u->ready_num--;
+}
+
+/*
  * Complete kevent removing - it dequeues kevent from storage list
  * if it is requested, removes kevent from ready list, drops userspace
  * control block reference counter and schedules kevent freeing through RCU.
@@ -248,11 +278,8 @@ static void kevent_finish_user_complete(
                kevent_dequeue(k);
 
        spin_lock_irqsave(&u->ready_lock, flags);
-       if (k->flags & KEVENT_READY) {
-               list_del(&k->ready_entry);
-               k->flags &= ~KEVENT_READY;
-               u->ready_num--;
-       }
+       if (k->flags & KEVENT_READY)
+               kevent_remove_ready(k);
        spin_unlock_irqrestore(&u->ready_lock, flags);
 
        kevent_user_put(u);
@@ -303,25 +330,7 @@ static struct kevent *kqueue_dequeue_rea
        spin_lock_irqsave(&u->ready_lock, flags);
        if (u->ready_num && !list_empty(&u->ready_list)) {
                k = list_entry(u->ready_list.next, struct kevent, ready_entry);
-               list_del(&k->ready_entry);
-               k->flags &= ~KEVENT_READY;
-               u->ready_num--;
-               if (++u->pring[0]->uidx == KEVENT_MAX_EVENTS)
-                       u->pring[0]->uidx = 0;
-               
-               if (u->overflow_kevent) {
-                       int err;
-
-                       err = kevent_user_ring_add_event(u->overflow_kevent);
-                       if (!err) {
-                               if (u->overflow_kevent->ready_entry.next == 
&u->ready_list)
-                                       u->overflow_kevent = NULL;
-                               else
-                                       u->overflow_kevent = 
-                                               
list_entry(u->overflow_kevent->ready_entry.next, 
-                                                               struct kevent, 
ready_entry);
-                       }
-               }
+               kevent_remove_ready(k);
        }
        spin_unlock_irqrestore(&u->ready_lock, flags);
 

Thanks.

> Eric

-- 
        Evgeniy Polyakov
-
To unsubscribe from this list: send the line "unsubscribe netdev" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to