lkml.org 
[lkml]   [2006]   [Aug]   [23]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    From
    SubjectRe: [take12 1/3] kevent: Core files.
    Date
    Hello Evgeniy

    I have one comment/suggestion (minor detail, your work is very good)

    I suggest to add one item in kevent_registered_callbacks[], so that
    kevent_registered_callbacks[KEVENT_MAX] is valid and can act as a fallback.

    In kevent_add_callbacks() you could replace the eventual NULL pointers by
    kevent_break() in
    kevent_registered_callbacks[pos].{callback, enqueue, dequeue}
    like :
    +int kevent_add_callbacks(const struct kevent_callbacks *cb, unsigned int pos)
    +{
    + struct kevent_callbacks *p = &kevent_registered_callbacks[pos];
    + if (pos >= KEVENT_MAX)
    + return -EINVAL;
    + p->enqueue = (cb->enqueue) ? cb->enqueue : kevent_break;
    + p->dequeue = (cb->dequeue) ? cb->dequeue : kevent_break;
    + p->callback = (cb->callback) ? cb->callback : kevent_break;
    + printk(KERN_INFO "KEVENT: Added callbacks for type %u.\n", pos);
    + return 0;
    +}
    (I also added a const qualifier in first function argument, and unsigned int
    pos so that the "if (pos >= KEVENT_MAX)" test catches 'negative' values)
    Then you change kevent_break() to return -EINVAL instead of 0.

    +int kevent_break(struct kevent *k)
    +{
    +       unsigned long flags;
    +
    +       spin_lock_irqsave(&k->ulock, flags);
    +       k->event.ret_flags |= KEVENT_RET_BROKEN;
    +       spin_unlock_irqrestore(&k->ulock, flags);
    +       return -EINVAL;
    +}
    Then avoid the tests in kevent_enqueue()

    +int kevent_enqueue(struct kevent *k)
    +{
    +       return k->callbacks.enqueue(k);
    +}
    And avoid the tests in kevent_dequeue()

    +int kevent_dequeue(struct kevent *k)
    +{
    +       return k->callbacks.dequeue(k);
    +}
    And change kevent_init() to

    +int kevent_init(struct kevent *k)
    +{
    +       spin_lock_init(&k->ulock);
    +       k->flags = 0;
    +
    +       if (unlikely(k->event.type >= KEVENT_MAX))
    + k->event.type = KEVENT_MAX;
    +
    +
    +       k->callbacks = kevent_registered_callbacks[k->event.type];
    +       if (unlikely(k->callbacks.callback == kevent_break))
    +               return kevent_break(k);
    +
    +       return 0;
    +}


    Eric Dumazet
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2006-08-23 10:55    [from the cache]
    ©2003-2011 Jasper Spaans