All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 0/5] ipc/msg: Sender/receiver optimizations
@ 2016-07-28 23:33 Davidlohr Bueso
  2016-07-28 23:33 ` [PATCH 1/5] ipc/msg: Implement lockless pipelined wakeups Davidlohr Bueso
                   ` (5 more replies)
  0 siblings, 6 replies; 12+ messages in thread
From: Davidlohr Bueso @ 2016-07-28 23:33 UTC (permalink / raw)
  To: akpm; +Cc: manfred, bigeasy, peterz, tglx, dave, linux-kernel

Hi,

I'm resending Sebastian's sysv msg queue use of wake_qs but updated
to the last observations I need wrt the need of explicit barriers
after removing the whole receiver busy-looping. After some irc exchange
it seems we're both on the same page, and things now look like he had
them earlier, in v2. This is all patch 1.

The rest of the patches are changes I noticed while reviewing patch 1,
which are mainly sender-side rework/optimizations. Details are in each
changelog.

The changes have survived ltp (which has some nasty corner cases for msgsnd
changes), as well as pmsg-shared benchmark.

Applies on Linus's latest - please consider for v4.9.

Thanks!

  ipc/msg: Implement lockless pipelined wakeups
  ipc/msg: Batch queue sender wakeups
  ipc/msg: Make ss_wakeup() kill arg boolean
  ipc/msg: Lockless security checks for msgsnd
  ipc/msg: Avoid waking sender upon full queue

 ipc/msg.c | 210 ++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 101 insertions(+), 109 deletions(-)

-- 
2.6.6

^ permalink raw reply	[flat|nested] 12+ messages in thread
* Re: [PATCH 4/5] ipc/msg: Lockless security checks for msgsnd
@ 2016-09-18  5:10 Manfred Spraul
  2016-09-21 22:21 ` Davidlohr Bueso
  0 siblings, 1 reply; 12+ messages in thread
From: Manfred Spraul @ 2016-09-18  5:10 UTC (permalink / raw)
  To: Davidlohr Bueso; +Cc: Linux Kernel Mailing List, Peter Zijlstra

Hi Davidlohr,

> Just as with msgrcv (along with the rest of sysvipc since a few years
>     ago), perform the security checks without holding the ipc object lock.
Thinking about it: isn't this wrong?

CPU1:
* msgrcv()
* ipcperms()
<sleep>

CPU2:
* msgctl(), change permissions
** msgctl() returns, new permissions should now be in effect
* msgsnd(), send secret message
** msgsnd() returns, new message stored.

CPU1: resumes, receives secret message

Obviously, we could argue that the msgrcv() was already ongoing and 
therefore the old permissions still apply - but then we don't need to 
recheck after sleeping at all.


>     This also reduces the hogging of the lock for the entire duration of a
>     sender, as we drop the lock upon every iteration -- and this is 
> exactly
>     why we also check for racing with RMID in the first place.

Which hogging do you mean? The lock is dopped uppon every iteration, the 
schedule() is in the middle.
Which your patch, the lock are now dropped twice:
> -
>   	for (;;) {
>   		struct msg_sender s;
>   
>   		err = -EACCES;
>   		if (ipcperms(ns, &msq->q_perm, S_IWUGO))
> -			goto out_unlock0;
> +			goto out_unlock1;
> +
> +		ipc_lock_object(&msq->q_perm);
>   
>   		/* raced with RMID? */
>   		if (!ipc_valid_object(&msq->q_perm)) {
> @@ -681,6 +681,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
>   			goto out_unlock0;
>   		}
>   
> +		ipc_unlock_object(&msq->q_perm);
>   	}
>
>
This means the lock is dropped, just for ipcperms().
This doubles the lock acquire/release cycles.

--
     Manfred

^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2016-09-22 19:43 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-07-28 23:33 [PATCH 0/5] ipc/msg: Sender/receiver optimizations Davidlohr Bueso
2016-07-28 23:33 ` [PATCH 1/5] ipc/msg: Implement lockless pipelined wakeups Davidlohr Bueso
2016-07-28 23:33 ` [PATCH 2/5] ipc/msg: Batch queue sender wakeups Davidlohr Bueso
2016-07-28 23:33 ` [PATCH 3/5] ipc/msg: Make ss_wakeup() kill arg boolean Davidlohr Bueso
2016-07-28 23:33 ` [PATCH 4/5] ipc/msg: Lockless security checks for msgsnd Davidlohr Bueso
2016-07-28 23:33 ` [PATCH 5/5] ipc/msg: Avoid waking sender upon full queue Davidlohr Bueso
2016-08-04 16:44 ` [PATCH 0/5] ipc/msg: Sender/receiver optimizations Peter Zijlstra
2016-08-09 12:44   ` Peter Zijlstra
2016-08-09 18:31     ` Andrew Morton
2016-09-18  5:10 [PATCH 4/5] ipc/msg: Lockless security checks for msgsnd Manfred Spraul
2016-09-21 22:21 ` Davidlohr Bueso
2016-09-22 19:42   ` Manfred Spraul

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.