Tested-by: Jia He<[email protected]> # cat /proc/sysvipc/sem key semid perms nsems uid gid cuid cgid otime ctime -1 32768 666 1 0 0 0 0 1380185570 1380185570
On Thu, 26 Sep 2013 07:08:55 +0200 from [email protected] wrote: > Hi Jia, > > Could you check if the patch below resolves the bug you have reported? > Just this patch, i.e. without your proposal. > > I want to leave the optimization for the get_seconds() call: > We must update sem_otime in two places anyway > (either perform_atomic_semop() and exit_sem() or > do_smart_update() and semtimedop()) > > -- > Manfred > > In commit 0a2b9d4c,the update of semaphore's sem_otime(last semop time) > was moved to one central position (do_smart_update). > > But: Since do_smart_update() is only called for operations that modify > the array, this means that wait-for-zero semops do not update sem_otime > anymore. > > The fix is simple: > Non-alter operations must update sem_otime. > > Signed-off-by: Manfred Spraul <[email protected]> > Reported-by: Jia He <[email protected]> > --- > ipc/sem.c | 41 +++++++++++++++++++++++++++++------------ > 1 file changed, 29 insertions(+), 12 deletions(-) > > diff --git a/ipc/sem.c b/ipc/sem.c > index e5d9bb8..ec83f79 100644 > --- a/ipc/sem.c > +++ b/ipc/sem.c > @@ -910,6 +910,24 @@ again: > } > > /** > + * set_semotime(sma, sops) - set sem_otime > + * @sma: semaphore array > + * @sops: operations that modified the array, may be NULL > + * > + * sem_otime is replicated to avoid cache line trashing. > + * This function sets one instance to the current time. > + */ > +static void set_semotime(struct sem_array *sma, struct sembuf *sops) > +{ > + if (sops == NULL) { > + sma->sem_base[0].sem_otime = get_seconds(); > + } else { > + sma->sem_base[sops[0].sem_num].sem_otime = > + get_seconds(); > + } > +} > + > +/** > * do_smart_update(sma, sops, nsops, otime, pt) - optimized update_queue > * @sma: semaphore array > * @sops: operations that were performed > @@ -959,17 +977,10 @@ static void do_smart_update(struct sem_array *sma, > struct sembuf *sops, int nsop > } > } > } > - 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(); > - } > - } > + if (otime) > + set_semotime(sma, sops); > } > > - > /* The following counts are associated to each semaphore: > * semncnt number of tasks waiting on semval being nonzero > * semzcnt number of tasks waiting on semval being zero > @@ -1831,10 +1842,16 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf > __user *, tsops, > > error = perform_atomic_semop(sma, sops, nsops, un, > task_tgid_vnr(current)); > - if (error <= 0) { > - if (alter && error == 0) > + if (error == 0) { > + /* If the operation was successful, then do > + * the required updates. > + */ > + if (alter) > do_smart_update(sma, sops, nsops, 1, &tasks); > - > + else > + set_semotime(sma, sops); > + } > + if (error <= 0) { > goto out_unlock_free; > } > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
