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
