Messages in this thread Patches in this message | | | Date | Mon, 25 Jan 1999 19:13:50 +0100 (CET) | From | Andrea Arcangeli <> | Subject | Re: down_interruptible and timers |
| |
On Mon, 25 Jan 1999, MOLNAR Ingo wrote:
> #define sema_init(sem, val) \ > (sem)->owner = (sem)->owner_depth = (sem)->waking = 0; \ > (sem)->wait = NULL; \ > (sem)->count = (val); \ > wmb(); > > (wmb() is needed because after sema_init() every driver should be able to > assume that the semaphore is initialized in every CPU's view, and we also
You are writing specific i386 code and wmb() is not an issue there.
> have to be sure that GCC doesnt optimize writes across that point.)
I think gcc can't reorder in a dangeros way. To run code that will start using the seamphore it has to return from the function, to call another function or to insert volatile inline asm.
> runtime initialization of semaphores is perfectly OK, just setting the > counter field makes no sense at all.
Infact. But my point is that the right way to runtime initialize a semaphore is just to use MUTEX_LOCKED and MUTEX. If you look at the code sema_init() was used _only_ as a performance optimization. You had many just zeroed (or just ready) structure and sema_init() was setting to 1 the only thing that had not to be zero. Probably is better to remove sema_init() to avoid people do mistakes? So it will be more clear that setting only .count is a performance hack?
Here a patch that does that:
Index: drivers/char/tty_io.c =================================================================== RCS file: /var/cvs/linux/drivers/char/tty_io.c,v retrieving revision 1.1.2.2 diff -u -r1.1.2.2 tty_io.c --- tty_io.c 1999/01/18 13:56:39 1.1.2.2 +++ linux/drivers/char/tty_io.c 1999/01/25 18:01:22 @@ -1930,10 +1930,10 @@ tty->flip.flag_buf_ptr = tty->flip.flag_buf; tty->flip.tqueue.routine = flush_to_ldisc; tty->flip.tqueue.data = tty; - tty->flip.pty_sem = MUTEX; + atomic_set(&tty->flip.pty_sem.count, 1); tty->tq_hangup.routine = do_tty_hangup; tty->tq_hangup.data = tty; - sema_init(&tty->atomic_read, 1); + atomic_set(&tty->atomic_read.count, 1); } /*
Here I added a wmb() because clear_inode() doesn't run around a spin_lock(), probably it' s not needed though...
Index: fs/inode.c =================================================================== RCS file: /var/cvs/linux/fs/inode.c,v retrieving revision 1.1.2.1 diff -u -r1.1.2.1 inode.c --- inode.c 1999/01/18 01:32:24 1.1.2.1 +++ linux/fs/inode.c 1999/01/25 18:07:25 @@ -130,8 +130,8 @@ init_waitqueue(&inode->i_wait); INIT_LIST_HEAD(&inode->i_hash); INIT_LIST_HEAD(&inode->i_dentry); - sema_init(&inode->i_sem, 1); - sema_init(&inode->i_atomic_write, 1); + atomic_set(&inode->i_sem.count, 1); + atomic_set(&inode->i_atomic_write.count, 1); } static inline void write_inode(struct inode *inode) @@ -500,7 +500,8 @@ inode->i_writecount = 0; inode->i_size = 0; memset(&inode->i_dquot, 0, sizeof(inode->i_dquot)); - sema_init(&inode->i_sem, 1); + atomic_set(&inode->i_sem.count, 1); + wmb(); } /* Index: fs/super.c =================================================================== RCS file: /var/cvs/linux/fs/super.c,v retrieving revision 1.1.2.1 diff -u -r1.1.2.1 super.c --- super.c 1999/01/18 01:32:24 1.1.2.1 +++ linux/fs/super.c 1999/01/25 17:51:17 @@ -104,7 +104,7 @@ lptr->mnt_dev = sb->s_dev; lptr->mnt_flags = sb->s_flags; - sema_init(&lptr->mnt_dquot.semaphore, 1); + atomic_set(&lptr->mnt_dquot.semaphore.count, 1); lptr->mnt_dquot.flags = 0; /* N.B. Is it really OK to have a vfsmount without names? */ Index: include/asm-i386/semaphore.h =================================================================== RCS file: /var/cvs/linux/include/asm-i386/semaphore.h,v retrieving revision 1.1.2.3 diff -u -r1.1.2.3 semaphore.h --- semaphore.h 1999/01/18 22:57:10 1.1.2.3 +++ linux/include/asm-i386/semaphore.h 1999/01/25 17:36:06 @@ -77,8 +77,6 @@ extern spinlock_t semaphore_wake_lock; -#define sema_init(sem, val) atomic_set(&((sem)->count), (val)) - /* * These two _must_ execute atomically wrt each other. * Index: net/ipv4/ip_masq_mfw.c =================================================================== RCS file: /var/cvs/linux/net/ipv4/ip_masq_mfw.c,v retrieving revision 1.1.2.1 diff -u -r1.1.2.1 ip_masq_mfw.c --- ip_masq_mfw.c 1999/01/18 01:34:53 1.1.2.1 +++ linux/net/ipv4/ip_masq_mfw.c 1999/01/25 17:44:52 @@ -79,7 +79,7 @@ }; -static struct semaphore mfw_sema; +static struct semaphore mfw_sema = MUTEX; #ifdef __SMP__ static rwlock_t mfw_lock = RW_LOCK_UNLOCKED; #endif @@ -748,7 +748,6 @@ __initfunc(int ip_mfw_init(void)) { - sema_init(&mfw_sema, 1); return register_ip_masq_mod ((mmod_self=&mfw_mod)); }
The last is a bugfix if the loader hasn't zeroed the .data segment before starting the code.
Andrea Arcangeli
- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.rutgers.edu Please read the FAQ at http://www.tux.org/lkml/
| |