>From 07da483abc88748a666f655f3e1d81a94df88e38 Mon Sep 17 00:00:00 2001 From: Manfred Spraul Date: Tue, 24 Sep 2013 22:53:27 +0200 Subject: [PATCH] ipc/sem.c: Simplify sem_otime handling. The sem_otime handling tries to minimize the number of calls to get_seconds(). This generates some complexity (i.e.: pass around if any operation completed) and introduced one bug (See previous commit to ipc/sem.c). Therefore: Remove the whole logic - this removes 25 lines. Disadvantages: - One line of code duplication in exit_sem(): The function must now update sem_otime, it can't rely on do_smart_update_queue() to perform that task. - Two get_seconds() calls instead of one call for the common case of increasing a semaphore and waking up a sleeping task. TODO: 1) How fast is get_seconds()? Is the optimization worth the effort? 2) Test it. --- ipc/sem.c | 61 ++++++++++++++++++------------------------------------------- 1 file changed, 18 insertions(+), 43 deletions(-) diff --git a/ipc/sem.c b/ipc/sem.c index 8e01e76..5e5d7b1 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -713,15 +713,13 @@ static int check_restart(struct sem_array *sma, struct sem_queue *q) * semaphore. * The tasks that must be woken up are added to @pt. The return code * is stored in q->pid. - * The function returns 1 if at least one operation was completed successfully. */ -static int wake_const_ops(struct sem_array *sma, int semnum, +static void wake_const_ops(struct sem_array *sma, int semnum, struct list_head *pt) { struct sem_queue *q; struct list_head *walk; struct list_head *pending_list; - int semop_completed = 0; if (semnum == -1) pending_list = &sma->pending_const; @@ -742,13 +740,9 @@ static int wake_const_ops(struct sem_array *sma, int semnum, /* operation completed, remove from queue & wakeup */ unlink_queue(sma, q); - wake_up_sem_queue_prepare(pt, q, error); - if (error == 0) - semop_completed = 1; } } - return semop_completed; } /** @@ -761,13 +755,11 @@ static int wake_const_ops(struct sem_array *sma, int semnum, * do_smart_wakeup_zero() checks all required queue for wait-for-zero * operations, based on the actual changes that were performed on the * semaphore array. - * The function returns 1 if at least one operation was completed successfully. */ -static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops, +static void do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops, int nsops, struct list_head *pt) { int i; - int semop_completed = 0; int got_zero = 0; /* first: the per-semaphore queues, if known */ @@ -777,7 +769,7 @@ static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops, if (sma->sem_base[num].semval == 0) { got_zero = 1; - semop_completed |= wake_const_ops(sma, num, pt); + wake_const_ops(sma, num, pt); } } } else { @@ -788,7 +780,7 @@ static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops, for (i = 0; i < sma->sem_nsems; i++) { if (sma->sem_base[i].semval == 0) { got_zero = 1; - semop_completed |= wake_const_ops(sma, i, pt); + wake_const_ops(sma, i, pt); } } } @@ -797,9 +789,7 @@ static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops, * then check the global queue, too. */ if (got_zero) - semop_completed |= wake_const_ops(sma, -1, pt); - - return semop_completed; + wake_const_ops(sma, -1, pt); } @@ -816,15 +806,12 @@ static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops, * The tasks that must be woken up are added to @pt. The return code * is stored in q->pid. * The function internally checks if const operations can now succeed. - * - * The function return 1 if at least one semop was completed successfully. */ -static int update_queue(struct sem_array *sma, int semnum, struct list_head *pt) +static void update_queue(struct sem_array *sma, int semnum, struct list_head *pt) { struct sem_queue *q; struct list_head *walk; struct list_head *pending_list; - int semop_completed = 0; if (semnum == -1) pending_list = &sma->pending_alter; @@ -861,7 +848,6 @@ again: if (error) { restart = 0; } else { - semop_completed = 1; do_smart_wakeup_zero(sma, q->sops, q->nsops, pt); restart = check_restart(sma, q); } @@ -870,15 +856,13 @@ again: if (restart) goto again; } - return semop_completed; } /** - * do_smart_update(sma, sops, nsops, otime, pt) - optimized update_queue + * do_smart_update(sma, sops, nsops, pt) - optimized update_queue * @sma: semaphore array * @sops: operations that were performed * @nsops: number of operations - * @otime: force setting otime * @pt: list head of the tasks that must be woken up. * * do_smart_update() does the required calls to update_queue and wakeup_zero, @@ -888,15 +872,15 @@ again: * It is safe to perform this call after dropping all locks. */ static void do_smart_update(struct sem_array *sma, struct sembuf *sops, int nsops, - int otime, struct list_head *pt) + struct list_head *pt) { int i; - otime |= do_smart_wakeup_zero(sma, sops, nsops, pt); + do_smart_wakeup_zero(sma, sops, nsops, pt); if (!list_empty(&sma->pending_alter)) { /* semaphore array uses the global queue - just process it. */ - otime |= update_queue(sma, -1, pt); + update_queue(sma, -1, pt); } else { if (!sops) { /* @@ -904,7 +888,7 @@ static void do_smart_update(struct sem_array *sma, struct sembuf *sops, int nsop * known. Check all. */ for (i = 0; i < sma->sem_nsems; i++) - otime |= update_queue(sma, i, pt); + update_queue(sma, i, pt); } else { /* * Check the semaphores that were increased: @@ -916,21 +900,11 @@ static void do_smart_update(struct sem_array *sma, struct sembuf *sops, int nsop * value will be too small, too. */ for (i = 0; i < nsops; i++) { - if (sops[i].sem_op > 0) { - otime |= update_queue(sma, - sops[i].sem_num, pt); - } + if (sops[i].sem_op > 0) + update_queue(sma, sops[i].sem_num, pt); } } } - if (otime) { - if (sops == NULL) { - sma->sem_base[0].sem_otime = get_seconds(); - } else { - sma->sem_base[sops[0].sem_num].sem_otime = - get_seconds(); - } - } } @@ -1238,7 +1212,7 @@ static int semctl_setval(struct ipc_namespace *ns, int semid, int semnum, curr->sempid = task_tgid_vnr(current); sma->sem_ctime = get_seconds(); /* maybe some queued-up processes were waiting for this */ - do_smart_update(sma, NULL, 0, 0, &tasks); + do_smart_update(sma, NULL, 0, &tasks); sem_unlock(sma, -1); rcu_read_unlock(); wake_up_sem_queue_do(&tasks); @@ -1366,7 +1340,7 @@ static int semctl_main(struct ipc_namespace *ns, int semid, int semnum, } sma->sem_ctime = get_seconds(); /* maybe some queued-up processes were waiting for this */ - do_smart_update(sma, NULL, 0, 0, &tasks); + do_smart_update(sma, NULL, 0, &tasks); err = 0; goto out_unlock; } @@ -1798,7 +1772,7 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops, task_tgid_vnr(current)); if (error <= 0) { if (alter && error == 0) - do_smart_update(sma, sops, nsops, 1, &tasks); + do_smart_update(sma, sops, nsops, &tasks); goto out_unlock_free; } @@ -2043,7 +2017,8 @@ void exit_sem(struct task_struct *tsk) } /* maybe some queued-up processes were waiting for this */ INIT_LIST_HEAD(&tasks); - do_smart_update(sma, NULL, 0, 1, &tasks); + sma->sem_base[0].sem_otime = get_seconds(); + do_smart_update(sma, NULL, 0, &tasks); sem_unlock(sma, -1); rcu_read_unlock(); wake_up_sem_queue_do(&tasks); -- 1.8.3.1