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

Groups > linux.kernel > #1452175 > unrolled thread

[PATCH 0/5] ipc/msg: Sender/receiver optimizations

Back to article view | Back to linux.kernel

Contents

  [PATCH 0/5] ipc/msg: Sender/receiver optimizations Davidlohr Bueso <dave@stgolabs.net> - 2016-07-29 01:40 +0200
    [PATCH 3/5] ipc/msg: Make ss_wakeup() kill arg boolean Davidlohr Bueso <dave@stgolabs.net> - 2016-07-29 01:40 +0200
    [PATCH 2/5] ipc/msg: Batch queue sender wakeups Davidlohr Bueso <dave@stgolabs.net> - 2016-07-29 01:40 +0200
    Re: [PATCH 0/5] ipc/msg: Sender/receiver optimizations Peter Zijlstra <peterz@infradead.org> - 2016-08-04 18:50 +0200
      Re: [PATCH 0/5] ipc/msg: Sender/receiver optimizations Peter Zijlstra <peterz@infradead.org> - 2016-08-09 14:50 +0200
        Re: [PATCH 0/5] ipc/msg: Sender/receiver optimizations Andrew Morton <akpm@linux-foundation.org> - 2016-08-09 20:40 +0200

#1452175 — [PATCH 0/5] ipc/msg: Sender/receiver optimizations

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

[toc] | [next] | [standalone]

#1452176 — [PATCH 3/5] ipc/msg: Make ss_wakeup() kill arg boolean

... 'tis annoying.

Signed-off-by: Davidlohr Bueso <dave@stgolabs.net>
---
 ipc/msg.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 395013d58fda..5181259e2ff0 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -167,7 +167,7 @@ static inline void ss_del(struct msg_sender *mss)
 }
 
 static void ss_wakeup(struct list_head *h,
-		      struct wake_q_head *wake_q, int kill)
+		      struct wake_q_head *wake_q, bool kill)
 {
 	struct msg_sender *mss, *t;
 
@@ -204,7 +204,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
 	WAKE_Q(wake_q);
 
 	expunge_all(msq, -EIDRM, &wake_q);
-	ss_wakeup(&msq->q_senders, &wake_q, 1);
+	ss_wakeup(&msq->q_senders, &wake_q, true);
 	msg_rmid(ns, msq);
 	ipc_unlock_object(&msq->q_perm);
 	wake_up_q(&wake_q);
@@ -388,7 +388,7 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 		 * Sleeping senders might be able to send
 		 * due to a larger queue size.
 		 */
-		ss_wakeup(&msq->q_senders, &wake_q, 0);
+		ss_wakeup(&msq->q_senders, &wake_q, false);
 		ipc_unlock_object(&msq->q_perm);
 		wake_up_q(&wake_q);
 
@@ -882,7 +882,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			msq->q_cbytes -= msg->m_ts;
 			atomic_sub(msg->m_ts, &ns->msg_bytes);
 			atomic_dec(&ns->msg_hdrs);
-			ss_wakeup(&msq->q_senders, &wake_q, 0);
+			ss_wakeup(&msq->q_senders, &wake_q, false);
 
 			goto out_unlock0;
 		}
-- 
2.6.6

[toc] | [prev] | [next] | [standalone]

#1452178 — [PATCH 2/5] ipc/msg: Batch queue sender wakeups

Currently the use of wake_qs in sysv msg queues are only
for the receiver tasks that are blocked on the queue. But
blocked sender tasks (due to queue size constraints) still
are awoken with the ipc object lock held, which can be a
problem particularly for small sized queues and far from
gracious for -rt (just like it was for the receiver side).

The paths that actually wakeup a sender are obviously
related to when we are either getting rid of the queue
or after (some) space is freed-up after a receiver takes
the msg (msgrcv). Furthermore, with the exception of msgrcv,
we can always piggy-back on expunge_all that has its own
tasks lined-up for waking. Finally, upon unlinking the
message, it should be no problem delaying the wakeups a
bit until after we've released the lock.

Signed-off-by: Davidlohr Bueso <dbueso@suse.de>
---
 ipc/msg.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 45ce6a3021be..395013d58fda 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -166,14 +166,15 @@ static inline void ss_del(struct msg_sender *mss)
 		list_del(&mss->list);
 }
 
-static void ss_wakeup(struct list_head *h, int kill)
+static void ss_wakeup(struct list_head *h,
+		      struct wake_q_head *wake_q, int kill)
 {
 	struct msg_sender *mss, *t;
 
 	list_for_each_entry_safe(mss, t, h, list) {
 		if (kill)
 			mss->list.next = NULL;
-		wake_up_process(mss->tsk);
+		wake_q_add(wake_q, mss->tsk);
 	}
 }
 
@@ -203,7 +204,7 @@ static void freeque(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
 	WAKE_Q(wake_q);
 
 	expunge_all(msq, -EIDRM, &wake_q);
-	ss_wakeup(&msq->q_senders, 1);
+	ss_wakeup(&msq->q_senders, &wake_q, 1);
 	msg_rmid(ns, msq);
 	ipc_unlock_object(&msq->q_perm);
 	wake_up_q(&wake_q);
@@ -331,7 +332,6 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 	struct kern_ipc_perm *ipcp;
 	struct msqid64_ds uninitialized_var(msqid64);
 	struct msg_queue *msq;
-	WAKE_Q(wake_q);
 	int err;
 
 	if (cmd == IPC_SET) {
@@ -362,6 +362,9 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 		freeque(ns, ipcp);
 		goto out_up;
 	case IPC_SET:
+	{
+		WAKE_Q(wake_q);
+
 		if (msqid64.msg_qbytes > ns->msg_ctlmnb &&
 		    !capable(CAP_SYS_RESOURCE)) {
 			err = -EPERM;
@@ -376,15 +379,21 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 		msq->q_qbytes = msqid64.msg_qbytes;
 
 		msq->q_ctime = get_seconds();
-		/* sleeping receivers might be excluded by
+		/*
+		 * Sleeping receivers might be excluded by
 		 * stricter permissions.
 		 */
 		expunge_all(msq, -EAGAIN, &wake_q);
-		/* sleeping senders might be able to send
+		/*
+		 * Sleeping senders might be able to send
 		 * due to a larger queue size.
 		 */
-		ss_wakeup(&msq->q_senders, 0);
-		break;
+		ss_wakeup(&msq->q_senders, &wake_q, 0);
+		ipc_unlock_object(&msq->q_perm);
+		wake_up_q(&wake_q);
+
+		goto out_unlock1;
+	}
 	default:
 		err = -EINVAL;
 		goto out_unlock1;
@@ -392,7 +401,6 @@ static int msgctl_down(struct ipc_namespace *ns, int msqid, int cmd,
 
 out_unlock0:
 	ipc_unlock_object(&msq->q_perm);
-	wake_up_q(&wake_q);
 out_unlock1:
 	rcu_read_unlock();
 out_up:
@@ -809,6 +817,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 	struct msg_queue *msq;
 	struct ipc_namespace *ns;
 	struct msg_msg *msg, *copy = NULL;
+	WAKE_Q(wake_q);
 
 	ns = current->nsproxy->ipc_ns;
 
@@ -873,7 +882,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			msq->q_cbytes -= msg->m_ts;
 			atomic_sub(msg->m_ts, &ns->msg_bytes);
 			atomic_dec(&ns->msg_hdrs);
-			ss_wakeup(&msq->q_senders, 0);
+			ss_wakeup(&msq->q_senders, &wake_q, 0);
 
 			goto out_unlock0;
 		}
@@ -945,6 +954,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 
 out_unlock0:
 	ipc_unlock_object(&msq->q_perm);
+	wake_up_q(&wake_q);
 out_unlock1:
 	rcu_read_unlock();
 	if (IS_ERR(msg)) {
-- 
2.6.6

[toc] | [prev] | [next] | [standalone]

#1456535

On Thu, Jul 28, 2016 at 04:33:34PM -0700, Davidlohr Bueso wrote:
> 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.

Not really my area, but over all the patches look good.

Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

[toc] | [prev] | [next] | [standalone]

#1458703

On Thu, Aug 04, 2016 at 06:44:09PM +0200, Peter Zijlstra wrote:
> On Thu, Jul 28, 2016 at 04:33:34PM -0700, Davidlohr Bueso wrote:
> > 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.
> 
> Not really my area, but over all the patches look good.
> 
> Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Andrew, will you pick these up, or should I route then through
tip/locking or something?

[toc] | [prev] | [next] | [standalone]

#1459054

On Tue, 9 Aug 2016 14:44:26 +0200 Peter Zijlstra <peterz@infradead.org> wrote:

> On Thu, Aug 04, 2016 at 06:44:09PM +0200, Peter Zijlstra wrote:
> > On Thu, Jul 28, 2016 at 04:33:34PM -0700, Davidlohr Bueso wrote:
> > > 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.
> > 
> > Not really my area, but over all the patches look good.
> > 
> > Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>

Thanks.

> Andrew, will you pick these up, or should I route then through
> tip/locking or something?

I'll be processing these later today/tomorrow. 

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web