Re: linux-next: Tree for Jun 21 [ BROKEN ipc/ipc-msg ]

[Sedat Dilek <sedat.dilek@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2842 bytes --]

On Fri, Jun 21, 2013 at 10:17 AM, Stephen Rothwell <sfr@canb.auug.org.au> wrote:
> Hi all,
>
> Happy solstice!
>
> Changes since 20130620:
>
> Dropped tree: mailbox (really bad merge conflicts with the arm-soc tree)
>
> The net-next tree gained a conflict against the net tree.
>
> The leds tree still had its build failure, so I used the version from
> next-20130607.
>
> The arm-soc tree gained conflicts against the tip, net-next, mfd and
> mailbox trees.
>
> The staging tree still had its build failure for which I disabled some
> code.
>
> The akpm tree lost a few patches that turned up elsewhere and gained
> conflicts against the ftrace and arm-soc trees.
>
> ----------------------------------------------------------------------------
>

[ CC IPC folks ]

Building via 'make deb-pkg' with fakeroot fails here like this:

make: *** [deb-pkg] Terminated
/usr/bin/fakeroot: line 181:  2386 Terminated
FAKEROOTKEY=$FAKEROOTKEY LD_LIBRARY_PATH="$PATHS" LD_PRELOAD="$LIB"
"$@"
semop(1): encountered an error: Identifier removed
semop(2): encountered an error: Invalid argument
semop(1): encountered an error: Identifier removed
semop(1): encountered an error: Identifier removed
semop(1): encountered an error: Invalid argument
semop(1): encountered an error: Invalid argument
semop(1): encountered an error: Invalid argument

The issue is present since next-20130606!

LAST KNOWN GOOD: next-20130605
FIRST KNOWN BAD: next-20130606

KNOWN GOOD: next-20130604
KNOWN BAD:  next-20130607 || next-20130619 || next-20130620 || next-20130621

git-bisect says CULPRIT commit is...

     "ipc,msg: shorten critical region in msgrcv"

NOTE: msg_lock_(check_) routines have to be restored (one more revert needed)!

Reverting both (below) commits makes fakeroot build via 'make dep-pkg" again.

I have tested the revert-patches with next-20130606 and next-20130621
(see file-attachments).

My build-script is attached!

Can someone of the IPC folks look at that?
Thanks!

- Sedat -


P.S.: Commit-IDs listed below.

[ next-20130606 ]

http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/?id=next-20130606

"ipc: remove unused functions"
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=8793fdfb0d0a6ed5916767e29a15d3eb56e04e79

"ipc,msg: shorten critical region in msgrcv"
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=c0ff93322847a54f74a5450032c4df64c17fdaed

[ next-20130621 ]

http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/log/?id=next-20130621

"ipc: remove unused functions"
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=941ce57c81dcceadf55265616ee1e8bef18b0ad3

"ipc,msg: shorten critical region in msgrcv"
http://git.kernel.org/cgit/linux/kernel/git/next/linux-next.git/commit/?id=62190df4081ee8504e3611d45edb40450cb408ac

[-- Attachment #2: build_linux-next.sh --]
[-- Type: application/x-sh, Size: 3849 bytes --]

[-- Attachment #3: 3.10.0-rc4-next20130606-3-iniza-small.patch --]
[-- Type: application/octet-stream, Size: 6338 bytes --]

Sedat Dilek (7):
      kbuild: deb-pkg: Try to determine distribution
      kbuild: deb-pkg: Bump year in debian/copyright file
      kbuild: deb-pkg: Update git repository URL in debian/copyright file
      Merge tag 'next-20130606' of git://git.kernel.org/.../next/linux-next into Linux-Next-v20130606
      Merge branch 'deb-pkg-3.10-fixes' into 3.10.0-rc4-next20130606-1-iniza-small
      Revert "ipc,msg: shorten critical region in msgrcv"
      Merge branch 'revert-ipc-msg-next20130606-c0ff93322847' into 3.10.0-rc4-next20130606-3-iniza-small

 ipc/msg.c                | 57 +++++++++++++++++++++---------------------------
 scripts/package/builddeb | 19 +++++++++++++---
 2 files changed, 41 insertions(+), 35 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 3b7b4b5..75ec881 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -862,19 +862,21 @@ static struct msg_msg *find_msg(struct msg_queue *msq, long *msgtyp, int mode)
 	return ERR_PTR(-EAGAIN);
 }
 
-long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgflg,
+
+long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
+	       int msgflg,
 	       long (*msg_handler)(void __user *, struct msg_msg *, size_t))
 {
-	int mode;
 	struct msg_queue *msq;
+	struct msg_msg *msg;
+	int mode;
 	struct ipc_namespace *ns;
-	struct msg_msg *msg, *copy = NULL;
+	struct msg_msg *copy = NULL;
 
 	ns = current->nsproxy->ipc_ns;
 
 	if (msqid < 0 || (long) bufsz < 0)
 		return -EINVAL;
-
 	if (msgflg & MSG_COPY) {
 		copy = prepare_copy(buf, min_t(size_t, bufsz, ns->msg_ctlmax));
 		if (IS_ERR(copy))
@@ -882,10 +884,8 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 	}
 	mode = convert_mode(&msgtyp, msgflg);
 
-	rcu_read_lock();
-	msq = msq_obtain_object_check(ns, msqid);
+	msq = msg_lock_check(ns, msqid);
 	if (IS_ERR(msq)) {
-		rcu_read_unlock();
 		free_copy(copy);
 		return PTR_ERR(msq);
 	}
@@ -895,9 +895,10 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 
 		msg = ERR_PTR(-EACCES);
 		if (ipcperms(ns, &msq->q_perm, S_IRUGO))
-			goto out_unlock1;
+			goto out_unlock;
 
 		msg = find_msg(msq, &msgtyp, mode);
+
 		if (!IS_ERR(msg)) {
 			/*
 			 * Found a suitable message.
@@ -905,7 +906,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			 */
 			if ((bufsz < msg->m_ts) && !(msgflg & MSG_NOERROR)) {
 				msg = ERR_PTR(-E2BIG);
-				goto out_unlock1;
+				goto out_unlock;
 			}
 			/*
 			 * If we are copying, then do not unlink message and do
@@ -913,10 +914,8 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			 */
 			if (msgflg & MSG_COPY) {
 				msg = copy_msg(msg, copy);
-				goto out_unlock1;
+				goto out_unlock;
 			}
-
-			ipc_lock_object(&msq->q_perm);
 			list_del(&msg->m_list);
 			msq->q_qnum--;
 			msq->q_rtime = get_seconds();
@@ -925,17 +924,14 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			atomic_sub(msg->m_ts, &ns->msg_bytes);
 			atomic_dec(&ns->msg_hdrs);
 			ss_wakeup(&msq->q_senders, 0);
-
-			goto out_unlock0;
+			msg_unlock(msq);
+			break;
 		}
-
 		/* No message waiting. Wait for a message */
 		if (msgflg & IPC_NOWAIT) {
 			msg = ERR_PTR(-ENOMSG);
-			goto out_unlock1;
+			goto out_unlock;
 		}
-
-		ipc_lock_object(&msq->q_perm);
 		list_add_tail(&msr_d.r_list, &msq->q_receivers);
 		msr_d.r_tsk = current;
 		msr_d.r_msgtype = msgtyp;
@@ -946,9 +942,8 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			msr_d.r_maxsize = bufsz;
 		msr_d.r_msg = ERR_PTR(-EAGAIN);
 		current->state = TASK_INTERRUPTIBLE;
+		msg_unlock(msq);
 
-		ipc_unlock_object(&msq->q_perm);
-		rcu_read_unlock();
 		schedule();
 
 		/* Lockless receive, part 1:
@@ -978,34 +973,32 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 		 * If there is a message or an error then accept it without
 		 * locking.
 		 */
-		if (msg != ERR_PTR(-EAGAIN))
-			goto out_unlock1;
+		if (msg != ERR_PTR(-EAGAIN)) {
+			rcu_read_unlock();
+			break;
+		}
 
 		/* Lockless receive, part 3:
 		 * Acquire the queue spinlock.
 		 */
-		ipc_lock_object(&msq->q_perm);
+		ipc_lock_by_ptr(&msq->q_perm);
+		rcu_read_unlock();
 
 		/* Lockless receive, part 4:
 		 * Repeat test after acquiring the spinlock.
 		 */
 		msg = (struct msg_msg*)msr_d.r_msg;
 		if (msg != ERR_PTR(-EAGAIN))
-			goto out_unlock0;
+			goto out_unlock;
 
 		list_del(&msr_d.r_list);
 		if (signal_pending(current)) {
 			msg = ERR_PTR(-ERESTARTNOHAND);
-			goto out_unlock0;
+out_unlock:
+			msg_unlock(msq);
+			break;
 		}
-
-		ipc_unlock_object(&msq->q_perm);
 	}
-
-out_unlock0:
-	ipc_unlock_object(&msq->q_perm);
-out_unlock1:
-	rcu_read_unlock();
 	if (IS_ERR(msg)) {
 		free_copy(copy);
 		return PTR_ERR(msg);
diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index acb8650..7d7c9d8 100644
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -172,9 +172,22 @@ else
 fi
 maintainer="$name <$email>"
 
+# Try to determine distribution
+if [ -e $(which lsb_release) ]; then
+       codename=$(lsb_release --codename --short)
+       if [ "$codename" != "" ]; then
+		distribution=$codename
+       else
+		distribution="UNRELEASED"
+		echo "WARNING: The distribution could NOT be determined!"
+       fi
+else
+       echo "HINT: Install lsb_release binary, this helps to identify your distribution!"
+fi
+
 # Generate a simple changelog template
 cat <<EOF > debian/changelog
-linux-upstream ($packageversion) unstable; urgency=low
+linux-upstream ($packageversion) $distribution; urgency=low
 
   * Custom built Linux kernel.
 
@@ -188,10 +201,10 @@ This is a packacked upstream version of the Linux kernel.
 The sources may be found at most Linux ftp sites, including:
 ftp://ftp.kernel.org/pub/linux/kernel
 
-Copyright: 1991 - 2009 Linus Torvalds and others.
+Copyright: 1991 - 2013 Linus Torvalds and others.
 
 The git repository for mainline kernel development is at:
-git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
+git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by

[-- Attachment #4: 3.10.0-rc6-next20130621-2-iniza-small.patch --]
[-- Type: application/octet-stream, Size: 7638 bytes --]

Sedat Dilek (8):
      kbuild: deb-pkg: Try to determine distribution
      kbuild: deb-pkg: Bump year in debian/copyright file
      kbuild: deb-pkg: Update git repository URL in debian/copyright file
      Merge tag 'next-20130621' of git://git.kernel.org/.../next/linux-next into Linux-Next-v20130621
      Merge branch 'deb-pkg-3.10-fixes' into 3.10.0-rc6-next20130621-1-iniza-small
      Revert "ipc: remove unused functions"
      Revert "ipc,msg: shorten critical region in msgrcv"
      Merge branch 'revert-ipc-next20130621-941ce57c81dc-62190df4081e' into 3.10.0-rc6-next20130621-2-iniza-small

 ipc/msg.c                | 82 +++++++++++++++++++++++++++++-------------------
 ipc/sem.c                |  1 +
 scripts/package/builddeb | 19 +++++++++--
 3 files changed, 67 insertions(+), 35 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index a1cf70e..f2a1a8f 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -141,6 +141,31 @@ void __init msg_init(void)
 				IPC_MSG_IDS, sysvipc_msg_proc_show);
 }
 
+/*
+ * msg_lock_(check_) routines are called in the paths where the rw_mutex
+ * is not held.
+ */
+static inline struct msg_queue *msg_lock(struct ipc_namespace *ns, int id)
+{
+	struct kern_ipc_perm *ipcp = ipc_lock(&msg_ids(ns), id);
+
+	if (IS_ERR(ipcp))
+		return (struct msg_queue *)ipcp;
+
+	return container_of(ipcp, struct msg_queue, q_perm);
+}
+
+static inline struct msg_queue *msg_lock_check(struct ipc_namespace *ns,
+						int id)
+{
+	struct kern_ipc_perm *ipcp = ipc_lock_check(&msg_ids(ns), id);
+
+	if (IS_ERR(ipcp))
+		return (struct msg_queue *)ipcp;
+
+	return container_of(ipcp, struct msg_queue, q_perm);
+}
+
 static inline struct msg_queue *msq_obtain_object(struct ipc_namespace *ns, int id)
 {
 	struct kern_ipc_perm *ipcp = ipc_obtain_object(&msg_ids(ns), id);
@@ -860,19 +885,21 @@ static struct msg_msg *find_msg(struct msg_queue *msq, long *msgtyp, int mode)
 	return ERR_PTR(-EAGAIN);
 }
 
-long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgflg,
+
+long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
+	       int msgflg,
 	       long (*msg_handler)(void __user *, struct msg_msg *, size_t))
 {
-	int mode;
 	struct msg_queue *msq;
+	struct msg_msg *msg;
+	int mode;
 	struct ipc_namespace *ns;
-	struct msg_msg *msg, *copy = NULL;
+	struct msg_msg *copy = NULL;
 
 	ns = current->nsproxy->ipc_ns;
 
 	if (msqid < 0 || (long) bufsz < 0)
 		return -EINVAL;
-
 	if (msgflg & MSG_COPY) {
 		copy = prepare_copy(buf, min_t(size_t, bufsz, ns->msg_ctlmax));
 		if (IS_ERR(copy))
@@ -880,10 +907,8 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 	}
 	mode = convert_mode(&msgtyp, msgflg);
 
-	rcu_read_lock();
-	msq = msq_obtain_object_check(ns, msqid);
+	msq = msg_lock_check(ns, msqid);
 	if (IS_ERR(msq)) {
-		rcu_read_unlock();
 		free_copy(copy);
 		return PTR_ERR(msq);
 	}
@@ -893,9 +918,10 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 
 		msg = ERR_PTR(-EACCES);
 		if (ipcperms(ns, &msq->q_perm, S_IRUGO))
-			goto out_unlock1;
+			goto out_unlock;
 
 		msg = find_msg(msq, &msgtyp, mode);
+
 		if (!IS_ERR(msg)) {
 			/*
 			 * Found a suitable message.
@@ -903,7 +929,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			 */
 			if ((bufsz < msg->m_ts) && !(msgflg & MSG_NOERROR)) {
 				msg = ERR_PTR(-E2BIG);
-				goto out_unlock1;
+				goto out_unlock;
 			}
 			/*
 			 * If we are copying, then do not unlink message and do
@@ -911,10 +937,8 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			 */
 			if (msgflg & MSG_COPY) {
 				msg = copy_msg(msg, copy);
-				goto out_unlock1;
+				goto out_unlock;
 			}
-
-			ipc_lock_object(&msq->q_perm);
 			list_del(&msg->m_list);
 			msq->q_qnum--;
 			msq->q_rtime = get_seconds();
@@ -923,17 +947,14 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			atomic_sub(msg->m_ts, &ns->msg_bytes);
 			atomic_dec(&ns->msg_hdrs);
 			ss_wakeup(&msq->q_senders, 0);
-
-			goto out_unlock0;
+			msg_unlock(msq);
+			break;
 		}
-
 		/* No message waiting. Wait for a message */
 		if (msgflg & IPC_NOWAIT) {
 			msg = ERR_PTR(-ENOMSG);
-			goto out_unlock1;
+			goto out_unlock;
 		}
-
-		ipc_lock_object(&msq->q_perm);
 		list_add_tail(&msr_d.r_list, &msq->q_receivers);
 		msr_d.r_tsk = current;
 		msr_d.r_msgtype = msgtyp;
@@ -944,9 +965,8 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 			msr_d.r_maxsize = bufsz;
 		msr_d.r_msg = ERR_PTR(-EAGAIN);
 		current->state = TASK_INTERRUPTIBLE;
+		msg_unlock(msq);
 
-		ipc_unlock_object(&msq->q_perm);
-		rcu_read_unlock();
 		schedule();
 
 		/* Lockless receive, part 1:
@@ -976,34 +996,32 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, int msgfl
 		 * If there is a message or an error then accept it without
 		 * locking.
 		 */
-		if (msg != ERR_PTR(-EAGAIN))
-			goto out_unlock1;
+		if (msg != ERR_PTR(-EAGAIN)) {
+			rcu_read_unlock();
+			break;
+		}
 
 		/* Lockless receive, part 3:
 		 * Acquire the queue spinlock.
 		 */
-		ipc_lock_object(&msq->q_perm);
+		ipc_lock_by_ptr(&msq->q_perm);
+		rcu_read_unlock();
 
 		/* Lockless receive, part 4:
 		 * Repeat test after acquiring the spinlock.
 		 */
 		msg = (struct msg_msg*)msr_d.r_msg;
 		if (msg != ERR_PTR(-EAGAIN))
-			goto out_unlock0;
+			goto out_unlock;
 
 		list_del(&msr_d.r_list);
 		if (signal_pending(current)) {
 			msg = ERR_PTR(-ERESTARTNOHAND);
-			goto out_unlock0;
+out_unlock:
+			msg_unlock(msq);
+			break;
 		}
-
-		ipc_unlock_object(&msq->q_perm);
 	}
-
-out_unlock0:
-	ipc_unlock_object(&msq->q_perm);
-out_unlock1:
-	rcu_read_unlock();
 	if (IS_ERR(msg)) {
 		free_copy(copy);
 		return PTR_ERR(msg);
diff --git a/ipc/sem.c b/ipc/sem.c
index 4108889..b93f9c9 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -1455,6 +1455,7 @@ static int semctl_down(struct ipc_namespace *ns, int semid,
 				      &semid64.sem_perm, 0);
 	if (IS_ERR(ipcp)) {
 		err = PTR_ERR(ipcp);
+		/* the ipc lock is not held upon failure */
 		goto out_unlock1;
 	}
 
diff --git a/scripts/package/builddeb b/scripts/package/builddeb
index acb8650..7d7c9d8 100644
--- a/scripts/package/builddeb
+++ b/scripts/package/builddeb
@@ -172,9 +172,22 @@ else
 fi
 maintainer="$name <$email>"
 
+# Try to determine distribution
+if [ -e $(which lsb_release) ]; then
+       codename=$(lsb_release --codename --short)
+       if [ "$codename" != "" ]; then
+		distribution=$codename
+       else
+		distribution="UNRELEASED"
+		echo "WARNING: The distribution could NOT be determined!"
+       fi
+else
+       echo "HINT: Install lsb_release binary, this helps to identify your distribution!"
+fi
+
 # Generate a simple changelog template
 cat <<EOF > debian/changelog
-linux-upstream ($packageversion) unstable; urgency=low
+linux-upstream ($packageversion) $distribution; urgency=low
 
   * Custom built Linux kernel.
 
@@ -188,10 +201,10 @@ This is a packacked upstream version of the Linux kernel.
 The sources may be found at most Linux ftp sites, including:
 ftp://ftp.kernel.org/pub/linux/kernel
 
-Copyright: 1991 - 2009 Linus Torvalds and others.
+Copyright: 1991 - 2013 Linus Torvalds and others.
 
 The git repository for mainline kernel development is at:
-git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git
+git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
 
     This program is free software; you can redistribute it and/or modify
     it under the terms of the GNU General Public License as published by