2022-04-29 15:39:16

by Prakash Sangappa

Subject: [PATCH v4] ipc: Update semtimedop() to use hrtimer

semtimedop() should be converted to use hrtimer like it has been done
for most of the system calls with timeouts. This system call already
takes a struct timespec as an argument and can therefore provide finer
granularity timed wait.

Signed-off-by: Prakash Sangappa <[email protected]>
Reviewed-by: Thomas Gleixner <[email protected]>
Reviewed-by: Davidlohr Bueso <[email protected]>
---
v1->v2:
- Use timespec64_valid() to validate timeout
and other changes as suggested by Thomas Gleixner
v2->v3: Added Reviewed by tag (Thomas)
v3->v4: Added Reviewed by (Davidlohr)

ipc/sem.c | 23 +++++++++++------------
1 file changed, 11 insertions(+), 12 deletions(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index 0dbdb98..44b65b6 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1995,7 +1995,9 @@ long __do_semtimedop(int semid, struct sembuf *sops,
int max, locknum;
bool undos = false, alter = false, dupsop = false;
struct sem_queue queue;
- unsigned long dup = 0, jiffies_left = 0;
+ unsigned long dup = 0;
+ ktime_t expires, *exp = NULL;
+ bool timed_out = false;

if (nsops < 1 || semid < 0)
return -EINVAL;
@@ -2003,12 +2005,11 @@ long __do_semtimedop(int semid, struct sembuf *sops,
return -E2BIG;

if (timeout) {
- if (timeout->tv_sec < 0 || timeout->tv_nsec < 0 ||
- timeout->tv_nsec >= 1000000000L) {
- error = -EINVAL;
- goto out;
- }
- jiffies_left = timespec64_to_jiffies(timeout);
+ if (!timespec64_valid(timeout))
+ return -EINVAL;
+ expires = ktime_add_safe(ktime_get(),
+ timespec64_to_ktime(*timeout));
+ exp = &expires;
}


@@ -2166,10 +2167,8 @@ long __do_semtimedop(int semid, struct sembuf *sops,
sem_unlock(sma, locknum);
rcu_read_unlock();

- if (timeout)
- jiffies_left = schedule_timeout(jiffies_left);
- else
- schedule();
+ timed_out = !schedule_hrtimeout_range(exp,
+ current->timer_slack_ns, HRTIMER_MODE_ABS);

/*
* fastpath: the semop has completed, either successfully or
@@ -2210,7 +2209,7 @@ long __do_semtimedop(int semid, struct sembuf *sops,
/*
* If an interrupt occurred we have to clean up the queue.
*/
- if (timeout && jiffies_left == 0)
+ if (timed_out)
error = -EAGAIN;
} while (error == -EINTR && !signal_pending(current)); /* spurious */

--
2.7.4

2022-05-08 01:02:07

by Manfred Spraul

Subject: Re: [PATCH v4] ipc: Update semtimedop() to use hrtimer

On 4/29/22 01:18, Prakash Sangappa wrote:
> semtimedop() should be converted to use hrtimer like it has been done
> for most of the system calls with timeouts. This system call already
> takes a struct timespec as an argument and can therefore provide finer
> granularity timed wait.
>
> Signed-off-by: Prakash Sangappa <[email protected]>
> Reviewed-by: Thomas Gleixner <[email protected]>
> Reviewed-by: Davidlohr Bueso <[email protected]>
> ---
> v1->v2:
> - Use timespec64_valid() to validate timeout
> and other changes as suggested by Thomas Gleixner
> v2->v3: Added Reviewed by tag (Thomas)
> v3->v4: Added Reviewed by (Davidlohr)
>
> ipc/sem.c | 23 +++++++++++------------
> 1 file changed, 11 insertions(+), 12 deletions(-)
Reviewed-by: Manfred Spraul <[email protected]>
> diff --git a/ipc/sem.c b/ipc/sem.c
> index 0dbdb98..44b65b6 100644
> --- a/ipc/sem.c
> +++ b/ipc/sem.c
> @@ -1995,7 +1995,9 @@ long __do_semtimedop(int semid, struct sembuf *sops,
> int max, locknum;
> bool undos = false, alter = false, dupsop = false;
> struct sem_queue queue;
> - unsigned long dup = 0, jiffies_left = 0;
> + unsigned long dup = 0;
> + ktime_t expires, *exp = NULL;
> + bool timed_out = false;
>
> if (nsops < 1 || semid < 0)
> return -EINVAL;
> @@ -2003,12 +2005,11 @@ long __do_semtimedop(int semid, struct sembuf *sops,
> return -E2BIG;
>
> if (timeout) {
> - if (timeout->tv_sec < 0 || timeout->tv_nsec < 0 ||
> - timeout->tv_nsec >= 1000000000L) {
> - error = -EINVAL;
> - goto out;
> - }
> - jiffies_left = timespec64_to_jiffies(timeout);
> + if (!timespec64_valid(timeout))
> + return -EINVAL;
> + expires = ktime_add_safe(ktime_get(),
> + timespec64_to_ktime(*timeout));
> + exp = &expires;
> }
>
>
> @@ -2166,10 +2167,8 @@ long __do_semtimedop(int semid, struct sembuf *sops,
> sem_unlock(sma, locknum);
> rcu_read_unlock();
>
> - if (timeout)
> - jiffies_left = schedule_timeout(jiffies_left);
> - else
> - schedule();
> + timed_out = !schedule_hrtimeout_range(exp,
> + current->timer_slack_ns, HRTIMER_MODE_ABS);
>
> /*
> * fastpath: the semop has completed, either successfully or
> @@ -2210,7 +2209,7 @@ long __do_semtimedop(int semid, struct sembuf *sops,
> /*
> * If an interrupt occurred we have to clean up the queue.
> */
> - if (timeout && jiffies_left == 0)
> + if (timed_out)
> error = -EAGAIN;
> } while (error == -EINTR && !signal_pending(current)); /* spurious */
>