On Thu, Oct 12, 2000 at 11:38:11AM -0400, Yong Chi wrote:
> Hopefully this will do for SMP locks.  =)

Holding a spinlock for this long (especially when you might sleep there in two
places (interruptible_sleep_on, put_user)) is basically a bad idea.
spinlocks are designed to be holded only for short time.
Either protect just a small critical section with a spinlock, or use
semaphores.

> --- ds.c.bak  Wed Oct 11 13:05:16 2000
> +++ ds.c      Thu Oct 12 11:25:20 2000
> @@ -95,6 +95,7 @@
>      u_int            user_magic;
>      int                      event_head, event_tail;
>      event_t          event[MAX_EVENTS];
> +    spinlock_t          lock;
>      struct user_info_t       *next;
>  } user_info_t;
>  
> @@ -567,6 +568,7 @@
>      user->event_tail = user->event_head = 0;
>      user->next = s->user;
>      user->user_magic = USER_MAGIC;
> +    spin_lock_init(&user->lock);
>      s->user = user;
>      file->private_data = user;
>      
> @@ -616,6 +618,7 @@
>      socket_t i = MINOR(file->f_dentry->d_inode->i_rdev);
>      socket_info_t *s;
>      user_info_t *user;
> +    ssize_t retval=4;
>  
>      DEBUG(2, "ds_read(socket %d)\n", i);
>      
> @@ -625,16 +628,23 @@
>       return -EINVAL;
>      s = &socket_table[i];
>      user = file->private_data;
> -    if (CHECK_USER(user))
> -     return -EIO;
> -    
> +    spin_lock(&user->lock);
> +    if (CHECK_USER(user)) {
> +     retval= -EIO;
> +        goto read_out;
> +    }
> +
>      if (queue_empty(user)) {
>       interruptible_sleep_on(&s->queue);
>       if (signal_pending(current))
> -         return -EINTR;
> +         retval= -EINTR;
> +            goto read_out;
>      }
>      put_user(get_queued_event(user), (int *)buf);
> -    return 4;
> +
> +read_out:
> +    spin_unlock(&user->lock);
> +    return retval;
>  } /* ds_read */
>  
>  /*====================================================================*/
> @@ -645,6 +655,7 @@
>      socket_t i = MINOR(file->f_dentry->d_inode->i_rdev);
>      socket_info_t *s;
>      user_info_t *user;
> +    ssize_t retval=4;
>  
>      DEBUG(2, "ds_write(socket %d)\n", i);
>      
> @@ -656,18 +667,25 @@
>       return -EBADF;
>      s = &socket_table[i];
>      user = file->private_data;
> -    if (CHECK_USER(user))
> -     return -EIO;
> +    spin_lock(&user->lock);
> +    if (CHECK_USER(user)) {
> +     retval= -EIO;
> +     goto write_out;
> +    }
>  
>      if (s->req_pending) {
>       s->req_pending--;
>       get_user(s->req_result, (int *)buf);
>       if ((s->req_result != 0) || (s->req_pending == 0))
>           wake_up_interruptible(&s->request);
> -    } else
> -     return -EIO;
> +    } else {
> +     retval= -EIO;
> +     goto write_out;
> +    }
>  
> -    return 4;
> +write_out:
> +    spin_unlock(&user->lock);
> +    return retval;
>  } /* ds_write */
>  
>  /*====================================================================*/


        Jakub
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
Please read the FAQ at http://www.tux.org/lkml/

Reply via email to