lkml.org 
[lkml]   [1999]   [Jan]   [25]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patches in this message
/
Date
From
SubjectRe: 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/

\
 
 \ /
  Last update: 2005-03-22 13:50    [W:0.073 / U:0.604 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site