Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1485822

Re: [PATCH 2/5] ipc/sem: rework task wakeups

Show all headers | View raw

Hi Davidlohr,

On 09/12/2016 01:53 PM, Davidlohr Bueso wrote:
> @@ -1933,22 +1823,32 @@ SYSCALL_DEFINE4(semtimedop, int, semid, struct sembuf __user *, tsops,
>   	queue.alter = alter;
>   
>   	error = perform_atomic_semop(sma, &queue);
> -	if (error == 0) {
> -		/* If the operation was successful, then do
> +	if (error <= 0) { /* non-blocking path */
> +		WAKE_Q(wake_q);
> +
> +		/*
> +		 * 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) {
> +			if (alter)
> +				do_smart_update(sma, sops, nsops, 1, &wake_q);
> +			else
> +				set_semotime(sma, sops);
> + <<<<<<<<<<<<<<<<<<<<<<<<<<<<<<<< Why this empty line?
> +		}
> +
> +		sem_unlock(sma, locknum);
> +		rcu_read_unlock();
> +		wake_up_q(&wake_q);
> +
> +		goto out_free;
>   	}
> -	if (error <= 0)
> -		goto out_unlock_free;
I don't see the strategy:
I've used the approach that cleanup is at the end, to reduce duplicated 
code, even if it means that error codepaths unnecessarily call wakeup 
for an empty list and that the list is always initialized.

With patch 1 of the series, you start to optimize for that.
Now this patch reintroduces some wake_up_q calls for error paths.

So: What is the aim?
I would propose to skip patch 1 and leave the wake_up_q at the end.

Or, if we really want to avoid the wakeup calls, then do it entirely.
Perhaps:
 > if(error == 0) { /* nonblocking codepath 1, with wakeups */
 > [...]
 > }
 > if (error < 0} goto out_unlock_free;
 >
This would have an advantage, because the WAKE_Q would be initialized 
only when needed

--
     Manfred

Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread

Thread

[PATCH -next 0/5] ipc/sem: semop(2) improvements Davidlohr Bueso <dave@stgolabs.net> - 2016-09-12 14:00 +0200
  [PATCH 3/5] ipc/sem: optimize perform_atomic_semop() Davidlohr Bueso <dave@stgolabs.net> - 2016-09-12 14:00 +0200
    Re: [PATCH 3/5] ipc/sem: optimize perform_atomic_semop() Manfred Spraul <manfred@colorfullife.com> - 2016-09-12 20:00 +0200
      Re: [PATCH 3/5] ipc/sem: optimize perform_atomic_semop() Davidlohr Bueso <dave@stgolabs.net> - 2016-09-13 10:40 +0200
        Re: [PATCH 3/5] ipc/sem: optimize perform_atomic_semop() Manfred Spraul <manfred@colorfullife.com> - 2016-09-19 06:50 +0200
  [PATCH 2/5] ipc/sem: rework task wakeups Davidlohr Bueso <dave@stgolabs.net> - 2016-09-12 14:00 +0200
    Re: [PATCH 2/5] ipc/sem: rework task wakeups Manfred Spraul <manfred@colorfullife.com> - 2016-09-13 20:10 +0200
      Re: [PATCH 2/5] ipc/sem: rework task wakeups Davidlohr Bueso <dave@stgolabs.net> - 2016-09-14 17:50 +0200
    Re: [PATCH 2/5] ipc/sem: rework task wakeups Manfred Spraul <manfred@colorfullife.com> - 2016-09-18 16:40 +0200
      Re: [PATCH 2/5] ipc/sem: rework task wakeups Davidlohr Bueso <dave@stgolabs.net> - 2016-09-18 20:30 +0200
  [PATCH 4/5] ipc/sem: explicitly inline check_restart Davidlohr Bueso <dave@stgolabs.net> - 2016-09-12 14:00 +0200
  [PATCH 5/5] ipc/sem: use proper list api for pending_list wakeups Davidlohr Bueso <dave@stgolabs.net> - 2016-09-12 14:00 +0200
    Re: [PATCH 5/5] ipc/sem: use proper list api for pending_list wakeups Manfred Spraul <manfred@colorfullife.com> - 2016-09-18 20:00 +0200

csiph-web