On Sun, 2010-03-07 at 13:59 +0100, Jean Delvare wrote:

Hi Jan,

On Saturday 06 March 2010 08:21:32 pm Jan Engelhardt wrote:

...

Avoid old MUTEX macros and use sema instead, which works for both default and RT kernels.

Do you actually need the power of semaphores? Given that they are all initialized to 0 or 1, I have a doubt. Simple mutexes are more efficient. What about using the DEFINE_MUTEX/mutex_lock()/mutex_unlock() API instead?

Its the "0", that imposes the problem. In RT, you need "ownership" of the mutex. That's why static INIT_MUTEX_LOCKED doesn't work in RT: At static declare time, there is no "owner". Hence the changes to drop DECLARE_MUTEX and change to DEFINE_MUTEX. The proper implementation eliminates INIT_MUTEX_LOCKED and replaces it with DEFINE_MUTEX mutex; .. down(&mutex); Hope that helps clear this part of it up a little. Sven

...

Signed-off-by: Jan Engelhardt

--- fs/novfs/daemon.c | 9 +++++---- fs/novfs/inode.c | 8 +++++--- fs/novfs/profile.c | 5 +++-- fs/novfs/scope.c | 4 ++-- 4 files changed, 15 insertions(+), 11 deletions(-)

Index: linux-2.6.33-rt/fs/novfs/daemon.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/daemon.c +++ linux-2.6.33-rt/fs/novfs/daemon.c @@ -110,16 +110,17 @@ static atomic_t Daemon_Open_Count = ATOM

static unsigned long Daemon_Command_Timeout = TIMEOUT_VALUE;

-static DECLARE_MUTEX(DriveMapLock); +static struct semaphore DriveMapLock; static LIST_HEAD(DriveMapList);

int novfs_max_iosize = PAGE_SIZE;

-void novfs_daemon_queue_init() +void novfs_daemon_queue_init(void) { INIT_LIST_HEAD(&Daemon_Queue.list); spin_lock_init(&Daemon_Queue.lock); - init_MUTEX_LOCKED(&Daemon_Queue.semaphore); + sema_init(&DriveMapLock, 1); + sema_init(&Daemon_Queue.semaphore, 0); }

void novfs_daemon_queue_exit(void) @@ -163,7 +164,7 @@ int Queue_Daemon_Command(void *request, que->status = QUEUE_SENDING; que->flags = 0;

- init_MUTEX_LOCKED(&que->semaphore); + sema_init(&que->semaphore, 0);

que->sequence = atomic_inc_return(&Sequence);

Index: linux-2.6.33-rt/fs/novfs/inode.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/inode.c +++ linux-2.6.33-rt/fs/novfs/inode.c @@ -298,11 +298,11 @@ static atomic_t novfs_Inode_Number = ATO struct dentry *novfs_root = NULL; char *novfs_current_mnt = NULL;

-DECLARE_MUTEX(InodeList_lock); +struct semaphore InodeList_lock;

LIST_HEAD(InodeList);

-DECLARE_MUTEX(TimeDir_Lock); +struct semaphore TimeDir_Lock; uint64_t lastTime; char lastDir[PATH_MAX];

@@ -3766,7 +3766,7 @@ struct inode *novfs_get_inode(struct sup id->cntDC = 1;

INIT_LIST_HEAD(&id->DirCache); - init_MUTEX(&id->DirCacheLock); + sema_init(&id->DirCacheLock, 1);

id->FileHandle = 0; id->CacheFlag = 0; @@ -3971,6 +3971,8 @@ int __init init_novfs(void) { int retCode;

+ sema_init(&InodeList_lock, 1); + sema_init(&TimeDir_Lock, 1); lastDir[0] = 0; lastTime = get_nanosecond_time();

Index: linux-2.6.33-rt/fs/novfs/profile.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/profile.c +++ linux-2.6.33-rt/fs/novfs/profile.c @@ -60,7 +60,7 @@ static struct proc_dir_entry *dbg_file = static struct proc_dir_entry *dentry_file = NULL; static struct proc_dir_entry *inode_file = NULL;

-static DECLARE_MUTEX(LocalPrint_lock); +static struct semaphore LocalPrint_lock;

static ssize_t User_proc_write_DbgBuffer(struct file *file, const char __user *buf, size_t nbytes, loff_t *ppos) { @@ -622,8 +622,9 @@ uint64_t get_nanosecond_time() return (retVal); }

-void novfs_profile_init() +void novfs_profile_init(void) { + sema_init(&LocalPrint_lock, 1); if (novfs_procfs_dir) dbg_dir = novfs_procfs_dir; else Index: linux-2.6.33-rt/fs/novfs/scope.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/scope.c +++ linux-2.6.33-rt/fs/novfs/scope.c @@ -633,8 +633,8 @@ char *novfs_scope_dget_path(struct dentr void novfs_scope_init(void) { INIT_LIST_HEAD(&Scope_List); - init_MUTEX(&Scope_Lock); - init_MUTEX_LOCKED(&Scope_Thread_Delay); + sema_init(&Scope_Lock, 1); + sema_init(&Scope_Thread_Delay, 0); kthread_run(Scope_Cleanup_Thread, NULL, "novfs_ST"); }

-- Jean Delvare Suse L3

-- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-kernel+help@opensuse.org

On Sunday 2010-03-07 16:35, Jeff Mahoney wrote:

On 03/06/2010 02:21 PM, Jan Engelhardt wrote:

...

Avoid old MUTEX macros and use sema instead, which works for both default and RT kernels.

No.

They aren't the "old MUTEX macros", they're used *everywhere* in the regular kernel. If the RT kernel doesn't implement them, that's a bug in the RT kernel, not novfs.

RT has them... but with different names.

There was quite a bit of effort put into replacing semaphore sites with mutexes a while ago. Where a while ago is defined as about 4 years ago.

So, why do people continue adding bad code with init_MUTEX then?

Replacing a mutex with a semaphore when the required behavior is mutual exclusion is, quite simply, a regression.

- -Jeff

...

Signed-off-by: Jan Engelhardt

--- fs/novfs/daemon.c | 9 +++++---- fs/novfs/inode.c | 8 +++++--- fs/novfs/profile.c | 5 +++-- fs/novfs/scope.c | 4 ++-- 4 files changed, 15 insertions(+), 11 deletions(-)

Index: linux-2.6.33-rt/fs/novfs/daemon.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/daemon.c +++ linux-2.6.33-rt/fs/novfs/daemon.c @@ -110,16 +110,17 @@ static atomic_t Daemon_Open_Count = ATOM

static unsigned long Daemon_Command_Timeout = TIMEOUT_VALUE;

-static DECLARE_MUTEX(DriveMapLock); +static struct semaphore DriveMapLock; static LIST_HEAD(DriveMapList);

int novfs_max_iosize = PAGE_SIZE;

-void novfs_daemon_queue_init() +void novfs_daemon_queue_init(void) { INIT_LIST_HEAD(&Daemon_Queue.list); spin_lock_init(&Daemon_Queue.lock); - init_MUTEX_LOCKED(&Daemon_Queue.semaphore); + sema_init(&DriveMapLock, 1); + sema_init(&Daemon_Queue.semaphore, 0); }

void novfs_daemon_queue_exit(void) @@ -163,7 +164,7 @@ int Queue_Daemon_Command(void *request, que->status = QUEUE_SENDING; que->flags = 0;

- init_MUTEX_LOCKED(&que->semaphore); + sema_init(&que->semaphore, 0);

que->sequence = atomic_inc_return(&Sequence);

Index: linux-2.6.33-rt/fs/novfs/inode.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/inode.c +++ linux-2.6.33-rt/fs/novfs/inode.c @@ -298,11 +298,11 @@ static atomic_t novfs_Inode_Number = ATO struct dentry *novfs_root = NULL; char *novfs_current_mnt = NULL;

-DECLARE_MUTEX(InodeList_lock); +struct semaphore InodeList_lock;

LIST_HEAD(InodeList);

-DECLARE_MUTEX(TimeDir_Lock); +struct semaphore TimeDir_Lock; uint64_t lastTime; char lastDir[PATH_MAX];

@@ -3766,7 +3766,7 @@ struct inode *novfs_get_inode(struct sup id->cntDC = 1;

INIT_LIST_HEAD(&id->DirCache); - init_MUTEX(&id->DirCacheLock); + sema_init(&id->DirCacheLock, 1);

id->FileHandle = 0; id->CacheFlag = 0; @@ -3971,6 +3971,8 @@ int __init init_novfs(void) { int retCode;

+ sema_init(&InodeList_lock, 1); + sema_init(&TimeDir_Lock, 1); lastDir[0] = 0; lastTime = get_nanosecond_time();

Index: linux-2.6.33-rt/fs/novfs/profile.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/profile.c +++ linux-2.6.33-rt/fs/novfs/profile.c @@ -60,7 +60,7 @@ static struct proc_dir_entry *dbg_file = static struct proc_dir_entry *dentry_file = NULL; static struct proc_dir_entry *inode_file = NULL;

-static DECLARE_MUTEX(LocalPrint_lock); +static struct semaphore LocalPrint_lock;

static ssize_t User_proc_write_DbgBuffer(struct file *file, const char __user *buf, size_t nbytes, loff_t *ppos) { @@ -622,8 +622,9 @@ uint64_t get_nanosecond_time() return (retVal); }

-void novfs_profile_init() +void novfs_profile_init(void) { + sema_init(&LocalPrint_lock, 1); if (novfs_procfs_dir) dbg_dir = novfs_procfs_dir; else Index: linux-2.6.33-rt/fs/novfs/scope.c =================================================================== --- linux-2.6.33-rt.orig/fs/novfs/scope.c +++ linux-2.6.33-rt/fs/novfs/scope.c @@ -633,8 +633,8 @@ char *novfs_scope_dget_path(struct dentr void novfs_scope_init(void) { INIT_LIST_HEAD(&Scope_List); - init_MUTEX(&Scope_Lock); - init_MUTEX_LOCKED(&Scope_Thread_Delay); + sema_init(&Scope_Lock, 1); + sema_init(&Scope_Thread_Delay, 0); kthread_run(Scope_Cleanup_Thread, NULL, "novfs_ST"); }

- -- Jeff Mahoney SUSE Labs -----BEGIN PGP SIGNATURE----- Version: GnuPG v2.0.13 (GNU/Linux) Comment: Using GnuPG with SUSE - http://enigmail.mozdev.org/

iEYEARECAAYFAkuTx7AACgkQLPWxlyuTD7IZCQCfc6ohZKCKlRvy3nLc4KP318sA /jwAoJ4aJY8Bx4GeiwjXtS8Mw4UmbDQz =7t5A -----END PGP SIGNATURE----- -- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-kernel+help@opensuse.org

-- To unsubscribe, e-mail: opensuse-kernel+unsubscribe@opensuse.org For additional commands, e-mail: opensuse-kernel+help@opensuse.org