[lkml]   [2009]   [Nov]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
Messages in this thread
    SubjectRe: [PATCH 10/10] send events using read
    On Tue, 2009-11-03 at 16:59 -0700, Jonathan Corbet wrote:
    > Hi, Eric,
    > This is not a full review, but I did notice a problem as I was trying to
    > figure out the new API...

    This is a rip off of inotify which was introduced in commit 3632dee2f8b
    from Vegard Nossum back in January. I can't seem to find any discussion
    of this before it went into Linus' tree, so if someone knows how this
    patch got to Linus and what was said about it, I'd like to know. Thanks
    for unwittingly finding and inotify bug!

    I looked back over it and it looks to me like it will work although
    there may be a race like situation if there are multiple things trying
    to read events. I can see how with perfect timing and precision Task A
    might try to get the mutex and it is being held by task B. Task B drops
    the mutex and Task A gets it, this causes Task A to be TASK_RUNNABLE.
    Lets assume Task B runs back around the loop and tries to get it while
    Task A still holds it. Task A will drop the mutex and Task B gets it,
    now B is runnable. Repeat until infinity with perfect timing! It's not
    really a live-lock, if either of them ever gets the mutex uncontested or
    if an event ever arrives they are going to sleep and/or break the cycle.

    Certainly I'll take a look at it.

    > > +static ssize_t fanotify_read(struct file *file, char __user *buf,
    > > + size_t count, loff_t *pos)
    > > +{
    > > + struct fsnotify_group *group;
    > > + struct fsnotify_event *kevent;
    > > + char __user *start;
    > > + int ret;
    > > + DEFINE_WAIT(wait);
    > > +
    > > + start = buf;
    > > + group = file->private_data;
    > > +
    > > + pr_debug("%s: group=%p\n", __func__, group);
    > > +
    > > + while (1) {
    > > + prepare_to_wait(&group->notification_waitq, &wait, TASK_INTERRUPTIBLE);
    > > +
    > > + mutex_lock(&group->notification_mutex);
    > > + kevent = get_one_event(group, count);
    > > + mutex_unlock(&group->notification_mutex);
    > prepare_to_wait(), among other things, sets the task state. But then you
    > go into various sleeping calls (mutex_lock(), for starters); that will undo
    > what prepare_to_wait has done. You'll be back in TASK_RUNNING at this
    > point, so you'll never sleep.

     \ /
      Last update: 2009-11-04 01:59    [W:0.022 / U:1.972 seconds]
    ©2003-2017 Jasper Spaans. hosted at Digital OceanAdvertise on this site