Messages in this thread |  | | Date | Thu, 12 Oct 2000 11:50:13 -0400 | From | Jakub Jelinek <> | Subject | Re: TODO: drivers/pcmcia/ds.c: ds_read & ds_write. SMP locks are missing fix |
| |
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 majordomo@vger.kernel.org Please read the FAQ at http://www.tux.org/lkml/
|  |