sysv sem undo implementation

RFC PATCH New Manfred Spraul 1 participant 1 message Jun 15, 2003

Subsystems (3)

MM core Scheduler exec and binfmt

lore

Share & tools

lore

Manfred Spraul

author

2003-06-15 10:33 UTC #0 lore raw

RFC PATCH 1/1 sysv sem undo implementation

Hi,

I found a few odd lines in the sysv sem undo implementation and wrote a patch to fix them:

- lock_undo() locks the list of undo structures. The lock is held throughout the semop() syscall, but that's unnecessary - we can drop it immediately after the lookup.
- undo structures are only allocated when necessary. The need for undo structures is only noticed in the middle of the semop operation, while holding the semaphore array spinlock. The result is a convoluted unlock&revalidate implementation. I've reordered the code, and now the undo allocation can happen before acquiring the semaphore array spinlock. As a bonus, less code runs under the semaphore array spinlock.
- sem_exit seems to be broken with CLONE_SYSVSEM: the reference count is never decremented if it's > 1.
- sysvsem.sleep_list looks like code to handle oopses: if an oops kills a thread that sleeps in sys_timedsemop(), then sem_exit tries to recover. I've removed that - too fragile.

Attached is my patch, not yet fully tested. What do you think?

--
Manfred

// $Header$
// Kernel Version:

//  VERSION = 2
//  PATCHLEVEL = 5
//  SUBLEVEL = 71
//  EXTRAVERSION =

include/linux/sem.h +2 -2

@@ -128,13 +128,11 @@ struct sem_undo {
 struct sem_undo_list {
 	atomic_t	refcnt;
 	spinlock_t	lock;
-	volatile unsigned long	add_count;
 	struct sem_undo	*proc_list;
 };
 
 struct sysv_sem {
 	struct sem_undo_list *undo_list;
-	struct sem_queue *sleep_list;
 };
 
 asmlinkage long sys_semget (key_t key, int nsems, int semflg);

@@ -143,6 +141,8 @@ asmlinkage long sys_semctl (int semid, i
 asmlinkage long sys_semtimedop(int semid, struct sembuf __user *sops,
 			unsigned nsops, const struct timespec __user *timeout);
 
+void exit_sem(struct task_struct *p);
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SEM_H */

ipc/sem.c +121 ~~-175~~

428 lines not on this page · raw message

ipc/util.c +1 -7

@@ -541,17 +541,11 @@ int copy_semundo(unsigned long clone_fla
 	return 0;
 }
 
-void exit_semundo(struct task_struct *tsk)
+void exit_sem(struct task_struct *tsk)
 {
 	return;
 }
 
-
-void sem_exit (void)
-{
-    return;
-}
-
 asmlinkage long sys_semget (key_t key, int nsems, int semflg)
 {
 	return -ENOSYS;

kernel/fork.c +3 -2

@@ -39,7 +39,7 @@
 #include <asm/tlbflush.h>
 
 extern int copy_semundo(unsigned long clone_flags, struct task_struct *tsk);
-extern void exit_semundo(struct task_struct *tsk);
+extern void exit_sem(struct task_struct *tsk);
 
 /* The idle threads do not count..
  * Protected by write_lock_irq(&tasklist_lock)

@@ -846,6 +846,7 @@ struct task_struct *copy_process(unsigne
 	p->vfork_done = NULL;
 	spin_lock_init(&p->alloc_lock);
 	spin_lock_init(&p->switch_lock);
+	spin_lock_init(&p->proc_lock);
 
 	clear_tsk_thread_flag(p, TIF_SIGPENDING);
 	init_sigpending(&p->pending);

@@ -1033,7 +1034,7 @@ bad_fork_cleanup_fs:
 bad_fork_cleanup_files:
 	exit_files(p); /* blocking */
 bad_fork_cleanup_semundo:
-	exit_semundo(p);
+	exit_sem(p);
 bad_fork_cleanup_security:
 	security_task_free(p);
 bad_fork_cleanup:

kernel/exit.c +1 -1

@@ -698,7 +698,7 @@ NORET_TYPE void do_exit(long code)
 	acct_process(code);
 	__exit_mm(tsk);
 
-	sem_exit();
+	exit_sem(tsk);
 	__exit_files(tsk);
 	__exit_fs(tsk);
 	exit_namespace(tsk);