[PATCH -next 0/7] ipc: some misc updates & optimizations

[PATCH -next 0/7] ipc: some misc updates & optimizations

[Davidlohr Bueso]

A few updates to the ipc code, mostly some very needed cleanups (yes, we still
have loads more to do, but it's a start). Nothing particularly interesting,
except perhaps for the last two patches, which include a locking optimization
and barrier documentation for message queues. Absolutely no functional changes.

Applies on top of linux-next +  Manfred's 'whitespace cleanup' patch. Tested
with LTP.

Thanks!

Davidlohr Bueso (7):
  ipc: standardize code comments
  ipc: remove braces for single statements
  ipc: remove useless return statement
  ipc: simplify sysvipc_proc_open return
  ipc: delete seq_max field in struct ipc_ids
  ipc: share ids rwsem when possible in ipcget_public
  ipc,msg: document barriers

 include/linux/ipc_namespace.h |   1 -
 ipc/compat.c                  |  18 +--
 ipc/mqueue.c                  |   6 +-
 ipc/msg.c                     |  19 ++-
 ipc/sem.c                     |  32 ++---
 ipc/shm.c                     |   1 -
 ipc/util.c                    | 328 ++++++++++++++++++++----------------------
 ipc/util.h                    |   1 +
 8 files changed, 198 insertions(+), 208 deletions(-)

-- 
1.8.1.4

[PATCH 1/7] ipc: standardize code comments

[Davidlohr Bueso]

IPC commenting style is all over the place, *specially* in util.c. This patch
orders things a bit.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 ipc/sem.c  |  24 +++----
 ipc/shm.c  |   1 -
 ipc/util.c | 239 +++++++++++++++++++++++++++++--------------------------------
 3 files changed, 125 insertions(+), 139 deletions(-)

diff --git a/ipc/sem.c b/ipc/sem.c
index c40876b..1b1acdb 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -225,7 +225,7 @@ static void unmerge_queues(struct sem_array *sma)
 }
 
 /**
- * merge_queues - Merge single semop queues into global queue
+ * merge_queues - merge single semop queues into global queue
  * @sma: semaphore array
  *
  * This function merges all per-semaphore queues into the global queue.
@@ -474,7 +474,6 @@ static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s)
  *
  * Called with sem_ids.rwsem held (as a writer)
  */
-
 static int newary(struct ipc_namespace *ns, struct ipc_params *params)
 {
 	int id;
@@ -682,7 +681,7 @@ static void wake_up_sem_queue_prepare(struct list_head *pt,
 }
 
 /**
- * wake_up_sem_queue_do(pt) - do the actual wake-up
+ * wake_up_sem_queue_do - do the actual wake-up
  * @pt: list of tasks to be woken up
  *
  * Do the actual wake-up.
@@ -748,7 +747,7 @@ static int check_restart(struct sem_array *sma, struct sem_queue *q)
 }
 
 /**
- * wake_const_ops(sma, semnum, pt) - Wake up non-alter tasks
+ * wake_const_ops - wake up non-alter tasks
  * @sma: semaphore array.
  * @semnum: semaphore that was modified.
  * @pt: list head for the tasks that must be woken up.
@@ -798,15 +797,14 @@ static int wake_const_ops(struct sem_array *sma, int semnum,
 }
 
 /**
- * do_smart_wakeup_zero(sma, sops, nsops, pt) - wakeup all wait for zero tasks
+ * do_smart_wakeup_zero - wakeup all wait for zero tasks
  * @sma: semaphore array
  * @sops: operations that were performed
  * @nsops: number of operations
  * @pt: list head of the tasks that must be woken up.
  *
- * do_smart_wakeup_zero() checks all required queue for wait-for-zero
- * operations, based on the actual changes that were performed on the
- * semaphore array.
+ * Checks all required queue for wait-for-zero operations, based
+ * on the actual changes that were performed on the semaphore array.
  * The function returns 1 if at least one operation was completed successfully.
  */
 static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops,
@@ -850,7 +848,7 @@ static int do_smart_wakeup_zero(struct sem_array *sma, struct sembuf *sops,
 
 
 /**
- * update_queue(sma, semnum): Look for tasks that can be completed.
+ * update_queue - look for tasks that can be completed.
  * @sma: semaphore array.
  * @semnum: semaphore that was modified.
  * @pt: list head for the tasks that must be woken up.
@@ -920,7 +918,7 @@ again:
 }
 
 /**
- * set_semotime(sma, sops) - set sem_otime
+ * set_semotime - set sem_otime
  * @sma: semaphore array
  * @sops: operations that modified the array, may be NULL
  *
@@ -938,7 +936,7 @@ static void set_semotime(struct sem_array *sma, struct sembuf *sops)
 }
 
 /**
- * do_smart_update(sma, sops, nsops, otime, pt) - optimized update_queue
+ * do_smart_update - optimized update_queue
  * @sma: semaphore array
  * @sops: operations that were performed
  * @nsops: number of operations
@@ -1647,7 +1645,7 @@ static struct sem_undo *lookup_undo(struct sem_undo_list *ulp, int semid)
 }
 
 /**
- * find_alloc_undo - Lookup (and if not present create) undo array
+ * find_alloc_undo - lookup (and if not present create) undo array
  * @ns: namespace
  * @semid: semaphore array id
  *
@@ -1737,7 +1735,7 @@ out:
 
 
 /**
- * get_queue_result - Retrieve the result code from sem_queue
+ * get_queue_result - retrieve the result code from sem_queue
  * @q: Pointer to queue structure
  *
  * Retrieve the return code from the pending queue. If IN_WAKEUP is found in
diff --git a/ipc/shm.c b/ipc/shm.c
index 88c59c1e..7645961 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -477,7 +477,6 @@ static const struct vm_operations_struct shm_vm_ops = {
  *
  * Called with shm_ids.rwsem held as a writer.
  */
-
 static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 {
 	key_t key = params->key;
diff --git a/ipc/util.c b/ipc/util.c
index fdf03fa..af976fc6 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -110,15 +110,15 @@ static struct notifier_block ipc_memory_nb = {
 };
 
 /**
- *	ipc_init	-	initialise IPC subsystem
+ * ipc_init - initialise ipc subsystem
  *
- *	The various system5 IPC resources (semaphores, messages and shared
- *	memory) are initialised
- *	A callback routine is registered into the memory hotplug notifier
- *	chain: since msgmni scales to lowmem this callback routine will be
- *	called upon successful memory add / remove to recompute msmgni.
+ * The various sysv ipc resources (semaphores, messages and shared
+ * memory) are initialised.
+ *
+ * A callback routine is registered into the memory hotplug notifier
+ * chain: since msgmni scales to lowmem this callback routine will be
+ * called upon successful memory add / remove to recompute msmgni.
  */
- 
 static int __init ipc_init(void)
 {
 	sem_init();
@@ -131,13 +131,12 @@ static int __init ipc_init(void)
 __initcall(ipc_init);
 
 /**
- *	ipc_init_ids		-	initialise IPC identifiers
- *	@ids: Identifier set
+ * ipc_init_ids	- initialise ipc identifiers
+ * @ids: ipc identifier set
  *
- *	Set up the sequence range to use for the ipc identifier range (limited
- *	below IPCMNI) then initialise the ids idr.
+ * Set up the sequence range to use for the ipc identifier range (limited
+ * below IPCMNI) then initialise the ids idr.
  */
- 
 void ipc_init_ids(struct ipc_ids *ids)
 {
 	init_rwsem(&ids->rwsem);
@@ -159,11 +158,11 @@ void ipc_init_ids(struct ipc_ids *ids)
 #ifdef CONFIG_PROC_FS
 static const struct file_operations sysvipc_proc_fops;
 /**
- *	ipc_init_proc_interface	-  Create a proc interface for sysipc types using a seq_file interface.
- *	@path: Path in procfs
- *	@header: Banner to be printed at the beginning of the file.
- *	@ids: ipc id table to iterate.
- *	@show: show routine.
+ * ipc_init_proc_interface -  create a proc interface for sysipc types using a seq_file interface.
+ * @path: Path in procfs
+ * @header: Banner to be printed at the beginning of the file.
+ * @ids: ipc id table to iterate.
+ * @show: show routine.
  */
 void __init ipc_init_proc_interface(const char *path, const char *header,
 		int ids, int (*show)(struct seq_file *, void *))
@@ -191,16 +190,15 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
 #endif
 
 /**
- *	ipc_findkey	-	find a key in an ipc identifier set	
- *	@ids: Identifier set
- *	@key: The key to find
+ * ipc_findkey	- find a key in an ipc identifier set
+ * @ids: ipc identifier set
+ * @key: key to find
  *	
- *	Requires ipc_ids.rwsem locked.
- *	Returns the LOCKED pointer to the ipc structure if found or NULL
- *	if not.
- *	If key is found ipc points to the owning ipc structure
+ * Returns the locked pointer to the ipc structure if found or NULL
+ * otherwise. If key is found ipc points to the owning ipc structure
+ *
+ * Called with ipc_ids.rwsem held.
  */
- 
 static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
 {
 	struct kern_ipc_perm *ipc;
@@ -227,12 +225,11 @@ static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
 }
 
 /**
- *	ipc_get_maxid	-	get the last assigned id
- *	@ids: IPC identifier set
+ * ipc_get_maxid - get the last assigned id
+ * @ids: ipc identifier set
  *
- *	Called with ipc_ids.rwsem held.
+ * Called with ipc_ids.rwsem held.
  */
-
 int ipc_get_maxid(struct ipc_ids *ids)
 {
 	struct kern_ipc_perm *ipc;
@@ -258,17 +255,17 @@ int ipc_get_maxid(struct ipc_ids *ids)
 }
 
 /**
- *	ipc_addid	-	add an IPC identifier
- *	@ids: IPC identifier set
- *	@new: new IPC permission set
- *	@size: limit for the number of used ids
+ * ipc_addid - add an ipc identifier
+ * @ids: ipc identifier set
+ * @new: new ipc permission set
+ * @size: limit for the number of used ids
  *
- *	Add an entry 'new' to the IPC ids idr. The permissions object is
- *	initialised and the first free entry is set up and the id assigned
- *	is returned. The 'new' entry is returned in a locked state on success.
- *	On failure the entry is not locked and a negative err-code is returned.
+ * Add an entry 'new' to the ipc ids idr. The permissions object is
+ * initialised and the first free entry is set up and the id assigned
+ * is returned. The 'new' entry is returned in a locked state on success.
+ * On failure the entry is not locked and a negative err-code is returned.
  *
- *	Called with writer ipc_ids.rwsem held.
+ * Called with writer ipc_ids.rwsem held.
  */
 int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size)
 {
@@ -320,14 +317,14 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size)
 }
 
 /**
- *	ipcget_new	-	create a new ipc object
- *	@ns: namespace
- *	@ids: IPC identifer set
- *	@ops: the actual creation routine to call
- *	@params: its parameters
- *
- *	This routine is called by sys_msgget, sys_semget() and sys_shmget()
- *	when the key is IPC_PRIVATE.
+ * ipcget_new -	create a new ipc object
+ * @ns: ipc namespace
+ * @ids: ipc identifer set
+ * @ops: the actual creation routine to call
+ * @params: its parameters
+ *
+ * This routine is called by sys_msgget, sys_semget() and sys_shmget()
+ * when the key is IPC_PRIVATE.
  */
 static int ipcget_new(struct ipc_namespace *ns, struct ipc_ids *ids,
 		struct ipc_ops *ops, struct ipc_params *params)
@@ -341,19 +338,19 @@ static int ipcget_new(struct ipc_namespace *ns, struct ipc_ids *ids,
 }
 
 /**
- *	ipc_check_perms	-	check security and permissions for an IPC
- *	@ns: IPC namespace
- *	@ipcp: ipc permission set
- *	@ops: the actual security routine to call
- *	@params: its parameters
+ * ipc_check_perms - check security and permissions for an ipc object
+ * @ns: ipc namespace
+ * @ipcp: ipc permission set
+ * @ops: the actual security routine to call
+ * @params: its parameters
  *
- *	This routine is called by sys_msgget(), sys_semget() and sys_shmget()
- *      when the key is not IPC_PRIVATE and that key already exists in the
- *      ids IDR.
+ * This routine is called by sys_msgget(), sys_semget() and sys_shmget()
+ * when the key is not IPC_PRIVATE and that key already exists in the
+ * ds IDR.
  *
- *	On success, the IPC id is returned.
+ * On success, the ipc id is returned.
  *
- *	It is called with ipc_ids.rwsem and ipcp->lock held.
+ * It is called with ipc_ids.rwsem and ipcp->lock held.
  */
 static int ipc_check_perms(struct ipc_namespace *ns,
 			   struct kern_ipc_perm *ipcp,
@@ -374,18 +371,18 @@ static int ipc_check_perms(struct ipc_namespace *ns,
 }
 
 /**
- *	ipcget_public	-	get an ipc object or create a new one
- *	@ns: namespace
- *	@ids: IPC identifer set
- *	@ops: the actual creation routine to call
- *	@params: its parameters
- *
- *	This routine is called by sys_msgget, sys_semget() and sys_shmget()
- *	when the key is not IPC_PRIVATE.
- *	It adds a new entry if the key is not found and does some permission
- *      / security checkings if the key is found.
- *
- *	On success, the ipc id is returned.
+ * ipcget_public - get an ipc object or create a new one
+ * @ns: ipc namespace
+ * @ids: ipc identifer set
+ * @ops: the actual creation routine to call
+ * @params: its parameters
+ *
+ * This routine is called by sys_msgget, sys_semget() and sys_shmget()
+ * when the key is not IPC_PRIVATE.
+ * It adds a new entry if the key is not found and does some permission
+ * / security checkings if the key is found.
+ *
+ * On success, the ipc id is returned.
  */
 static int ipcget_public(struct ipc_namespace *ns, struct ipc_ids *ids,
 		struct ipc_ops *ops, struct ipc_params *params)
@@ -431,14 +428,13 @@ static int ipcget_public(struct ipc_namespace *ns, struct ipc_ids *ids,
 
 
 /**
- *	ipc_rmid	-	remove an IPC identifier
- *	@ids: IPC identifier set
- *	@ipcp: ipc perm structure containing the identifier to remove
+ * ipc_rmid - remove an ipc identifier
+ * @ids: ipc identifier set
+ * @ipcp: ipc perm structure containing the identifier to remove
  *
- *	ipc_ids.rwsem (as a writer) and the spinlock for this ID are held
- *	before this function is called, and remain locked on the exit.
+ * ipc_ids.rwsem (as a writer) and the spinlock for this ID are held
+ * before this function is called, and remain locked on the exit.
  */
- 
 void ipc_rmid(struct ipc_ids *ids, struct kern_ipc_perm *ipcp)
 {
 	int lid = ipcid_to_idx(ipcp->id);
@@ -453,13 +449,12 @@ void ipc_rmid(struct ipc_ids *ids, struct kern_ipc_perm *ipcp)
 }
 
 /**
- *	ipc_alloc	-	allocate ipc space
- *	@size: size desired
+ * ipc_alloc -	allocate ipc space
+ * @size: size desired
  *
- *	Allocate memory from the appropriate pools and return a pointer to it.
- *	NULL is returned if the allocation fails
+ * Allocate memory from the appropriate pools and return a pointer to it.
+ * NULL is returned if the allocation fails
  */
- 
 void *ipc_alloc(int size)
 {
 	void *out;
@@ -471,14 +466,13 @@ void *ipc_alloc(int size)
 }
 
 /**
- *	ipc_free        -       free ipc space
- *	@ptr: pointer returned by ipc_alloc
- *	@size: size of block
+ * ipc_free - free ipc space
+ * @ptr: pointer returned by ipc_alloc
+ * @size: size of block
  *
- *	Free a block created with ipc_alloc(). The caller must know the size
- *	used in the allocation call.
+ * Free a block created with ipc_alloc(). The caller must know the size
+ * used in the allocation call.
  */
-
 void ipc_free(void *ptr, int size)
 {
 	if (size > PAGE_SIZE)
@@ -488,11 +482,11 @@ void ipc_free(void *ptr, int size)
 }
 
 /**
- *	ipc_rcu_alloc	-	allocate ipc and rcu space 
- *	@size: size desired
+ * ipc_rcu_alloc - allocate ipc and rcu space
+ * @size: size desired
  *
- *	Allocate memory for the rcu header structure +  the object.
- *	Returns the pointer to the object or NULL upon failure.
+ * Allocate memory for the rcu header structure +  the object.
+ * Returns the pointer to the object or NULL upon failure.
  */
 void *ipc_rcu_alloc(int size)
 {
@@ -534,17 +528,16 @@ void ipc_rcu_free(struct rcu_head *head)
 }
 
 /**
- *	ipcperms	-	check IPC permissions
- *	@ns: IPC namespace
- *	@ipcp: IPC permission set
- *	@flag: desired permission set.
+ * ipcperms - check ipc permissions
+ * @ns: ipc namespace
+ * @ipcp: ipc permission set
+ * @flag: desired permission set
  *
- *	Check user, group, other permissions for access
- *	to ipc resources. return 0 if allowed
+ * Check user, group, other permissions for access
+ * to ipc resources. return 0 if allowed
  *
- *	@flag will most probably be 0 or S_...UGO from <linux/stat.h>
+ * @flag will most probably be 0 or S_...UGO from <linux/stat.h>
  */
- 
 int ipcperms(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp, short flag)
 {
 	kuid_t euid = current_euid();
@@ -572,15 +565,13 @@ int ipcperms(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp, short flag)
  */
 
 /**
- *	kernel_to_ipc64_perm	-	convert kernel ipc permissions to user
- *	@in: kernel permissions
- *	@out: new style IPC permissions
+ * kernel_to_ipc64_perm	- convert kernel ipc permissions to user
+ * @in: kernel permissions
+ * @out: new style ipc permissions
  *
- *	Turn the kernel object @in into a set of permissions descriptions
- *	for returning to userspace (@out).
+ * Turn the kernel object @in into a set of permissions descriptions
+ * for returning to userspace (@out).
  */
- 
-
 void kernel_to_ipc64_perm(struct kern_ipc_perm *in, struct ipc64_perm *out)
 {
 	out->key	= in->key;
@@ -593,14 +584,13 @@ void kernel_to_ipc64_perm(struct kern_ipc_perm *in, struct ipc64_perm *out)
 }
 
 /**
- *	ipc64_perm_to_ipc_perm	-	convert new ipc permissions to old
- *	@in: new style IPC permissions
- *	@out: old style IPC permissions
+ * ipc64_perm_to_ipc_perm - convert new ipc permissions to old
+ * @in: new style ipc permissions
+ * @out: old style ipc permissions
  *
- *	Turn the new style permissions object @in into a compatibility
- *	object and store it into the @out pointer.
+ * Turn the new style permissions object @in into a compatibility
+ * object and store it into the @out pointer.
  */
- 
 void ipc64_perm_to_ipc_perm(struct ipc64_perm *in, struct ipc_perm *out)
 {
 	out->key	= in->key;
@@ -635,8 +625,8 @@ struct kern_ipc_perm *ipc_obtain_object(struct ipc_ids *ids, int id)
 }
 
 /**
- * ipc_lock - Lock an ipc structure without rwsem held
- * @ids: IPC identifier set
+ * ipc_lock - lock an ipc structure without rwsem held
+ * @ids: ipc identifier set
  * @id: ipc id to look for
  *
  * Look for an id in the ipc ids idr and lock the associated ipc object.
@@ -693,11 +683,11 @@ out:
 
 /**
  * ipcget - Common sys_*get() code
- * @ns : namsepace
- * @ids : IPC identifier set
- * @ops : operations to be called on ipc object creation, permission checks
- *        and further checks
- * @params : the parameters needed by the previous operations.
+ * @ns: namsepace
+ * @ids: ipc identifier set
+ * @ops: operations to be called on ipc object creation, permission checks
+ *       and further checks
+ * @params: the parameters needed by the previous operations.
  *
  * Common routine called by sys_msgget(), sys_semget() and sys_shmget().
  */
@@ -711,7 +701,7 @@ int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
 }
 
 /**
- * ipc_update_perm - update the permissions of an IPC.
+ * ipc_update_perm - update the permissions of an ipc object
  * @in:  the permission given as input.
  * @out: the permission of the ipc to set.
  */
@@ -732,7 +722,7 @@ int ipc_update_perm(struct ipc64_perm *in, struct kern_ipc_perm *out)
 
 /**
  * ipcctl_pre_down_nolock - retrieve an ipc and check permissions for some IPC_XXX cmd
- * @ns:  the ipc namespace
+ * @ns:  ipc namespace
  * @ids:  the table of ids where to look for the ipc
  * @id:   the id of the ipc to retrieve
  * @cmd:  the cmd to check
@@ -779,14 +769,13 @@ err:
 
 
 /**
- *	ipc_parse_version	-	IPC call version
- *	@cmd: pointer to command
+ * ipc_parse_version - ipc call version
+ * @cmd: pointer to command
  *
- *	Return IPC_64 for new style IPC and IPC_OLD for old style IPC. 
- *	The @cmd value is turned from an encoding command and version into
- *	just the command code.
+ * Return IPC_64 for new style IPC and IPC_OLD for old style IPC.
+ * The @cmd value is turned from an encoding command and version into
+ * just the command code.
  */
- 
 int ipc_parse_version(int *cmd)
 {
 	if (*cmd & IPC_64) {
-- 
1.8.1.4

[PATCH 2/7] ipc: remove braces for single statements

[Davidlohr Bueso]

Deal with checkpatch messages:
     WARNING: braces {} are not necessary for single statement blocks

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 ipc/compat.c | 18 +++++++++---------
 ipc/mqueue.c |  6 +++---
 ipc/sem.c    |  8 +++-----
 ipc/util.c   |  3 +--
 4 files changed, 16 insertions(+), 19 deletions(-)

diff --git a/ipc/compat.c b/ipc/compat.c
index ed0530b..f71e962 100644
--- a/ipc/compat.c
+++ b/ipc/compat.c
@@ -288,11 +288,11 @@ static long do_compat_semctl(int first, int second, int third, u32 pad)
 		break;
 
 	case IPC_SET:
-		if (version == IPC_64) {
+		if (version == IPC_64)
 			err = get_compat_semid64_ds(&s64, compat_ptr(pad));
-		} else {
+		else
 			err = get_compat_semid_ds(&s64, compat_ptr(pad));
-		}
+
 		up64 = compat_alloc_user_space(sizeof(s64));
 		if (copy_to_user(up64, &s64, sizeof(s64)))
 			err = -EFAULT;
@@ -515,11 +515,11 @@ long compat_sys_msgctl(int first, int second, void __user *uptr)
 		break;
 
 	case IPC_SET:
-		if (version == IPC_64) {
+		if (version == IPC_64)
 			err = get_compat_msqid64(&m64, uptr);
-		} else {
+		else
 			err = get_compat_msqid(&m64, uptr);
-		}
+
 		if (err)
 			break;
 		p = compat_alloc_user_space(sizeof(m64));
@@ -702,11 +702,11 @@ long compat_sys_shmctl(int first, int second, void __user *uptr)
 
 
 	case IPC_SET:
-		if (version == IPC_64) {
+		if (version == IPC_64)
 			err = get_compat_shmid64_ds(&s64, uptr);
-		} else {
+		else
 			err = get_compat_shmid_ds(&s64, uptr);
-		}
+
 		if (err)
 			break;
 		p = compat_alloc_user_space(sizeof(s64));
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index bb36aae..ccf1f9f 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -1303,11 +1303,11 @@ retry:
 out_fput:
 	fdput(f);
 out:
-	if (sock) {
+	if (sock)
 		netlink_detachskb(sock, nc);
-	} else if (nc) {
+	else if (nc)
 		dev_kfree_skb(nc);
-	}
+
 	return ret;
 }
 
diff --git a/ipc/sem.c b/ipc/sem.c
index 1b1acdb..bee5554 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -492,9 +492,9 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
 
 	size = sizeof(*sma) + nsems * sizeof(struct sem);
 	sma = ipc_rcu_alloc(size);
-	if (!sma) {
+	if (!sma)
 		return -ENOMEM;
-	}
+
 	memset(sma, 0, size);
 
 	sma->sem_perm.mode = (semflg & S_IRWXUGO);
@@ -1967,10 +1967,8 @@ sleep_again:
 	 * If queue.status != -EINTR we are woken up by another process.
 	 * Leave without unlink_queue(), but with sem_unlock().
 	 */
-
-	if (error != -EINTR) {
+	if (error != -EINTR)
 		goto out_unlock_free;
-	}
 
 	/*
 	 * If an interrupt occurred we have to clean up the queue
diff --git a/ipc/util.c b/ipc/util.c
index af976fc6..cfbd8fa 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -183,9 +183,8 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
 			       NULL,           /* parent dir */
 			       &sysvipc_proc_fops,
 			       iface);
-	if (!pde) {
+	if (!pde)
 		kfree(iface);
-	}
 }
 #endif
 
-- 
1.8.1.4

[PATCH 3/7] ipc: remove useless return statement

[Davidlohr Bueso]

Only found in ipc_rmid().

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 ipc/util.c | 4 ----
 1 file changed, 4 deletions(-)

diff --git a/ipc/util.c b/ipc/util.c
index cfbd8fa..7afe7de 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -439,12 +439,8 @@ void ipc_rmid(struct ipc_ids *ids, struct kern_ipc_perm *ipcp)
 	int lid = ipcid_to_idx(ipcp->id);
 
 	idr_remove(&ids->ipcs_idr, lid);
-
 	ids->in_use--;
-
 	ipcp->deleted = true;
-
-	return;
 }
 
 /**
-- 
1.8.1.4

[PATCH 4/7] ipc: simplify sysvipc_proc_open return

[Davidlohr Bueso]

Get rid of silly/useless label jumping.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 ipc/util.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/ipc/util.c b/ipc/util.c
index 7afe7de..cecb46e 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -911,8 +911,10 @@ static int sysvipc_proc_open(struct inode *inode, struct file *file)
 		goto out;
 
 	ret = seq_open(file, &sysvipc_proc_seqops);
-	if (ret)
-		goto out_kfree;
+	if (ret) {
+		kfree(iter);
+		goto out;
+	}
 
 	seq = file->private_data;
 	seq->private = iter;
@@ -921,9 +923,6 @@ static int sysvipc_proc_open(struct inode *inode, struct file *file)
 	iter->ns    = get_ipc_ns(current->nsproxy->ipc_ns);
 out:
 	return ret;
-out_kfree:
-	kfree(iter);
-	goto out;
 }
 
 static int sysvipc_proc_release(struct inode *inode, struct file *file)
-- 
1.8.1.4

[PATCH 5/7] ipc: delete seq_max field in struct ipc_ids

[Davidlohr Bueso]

This field is only used to reset the ids seq number if it exceeds the smaller of
INT_MAX/SEQ_MULTIPLIER and USHRT_MAX, and can therefore be moved out of the
structure and into its own macro. Since each ipc_namespace contains a table of
3 pointers to struct ipc_ids we can save space in instruction text:

   text    data     bss     dec     hex filename
  56232    2348      24   58604    e4ec ipc/built-in.o
  56216    2348      24   58588    e4dc ipc/built-in.o-after

Reviewed-by: Jonathan Gonzalez <jgonzalez@linets.cl>
Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 include/linux/ipc_namespace.h |  1 -
 ipc/util.c                    | 13 ++-----------
 ipc/util.h                    |  1 +
 3 files changed, 3 insertions(+), 12 deletions(-)

diff --git a/include/linux/ipc_namespace.h b/include/linux/ipc_namespace.h
index f6c82de..e7831d2 100644
--- a/include/linux/ipc_namespace.h
+++ b/include/linux/ipc_namespace.h
@@ -21,7 +21,6 @@ struct user_namespace;
 struct ipc_ids {
 	int in_use;
 	unsigned short seq;
-	unsigned short seq_max;
 	struct rw_semaphore rwsem;
 	struct idr ipcs_idr;
 	int next_id;
diff --git a/ipc/util.c b/ipc/util.c
index cecb46e..e1b4c6d 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -139,19 +139,10 @@ __initcall(ipc_init);
  */
 void ipc_init_ids(struct ipc_ids *ids)
 {
-	init_rwsem(&ids->rwsem);
-
 	ids->in_use = 0;
 	ids->seq = 0;
 	ids->next_id = -1;
-	{
-		int seq_limit = INT_MAX/SEQ_MULTIPLIER;
-		if (seq_limit > USHRT_MAX)
-			ids->seq_max = USHRT_MAX;
-		 else
-			ids->seq_max = seq_limit;
-	}
-
+	init_rwsem(&ids->rwsem);
 	idr_init(&ids->ipcs_idr);
 }
 
@@ -304,7 +295,7 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size)
 
 	if (next_id < 0) {
 		new->seq = ids->seq++;
-		if (ids->seq > ids->seq_max)
+		if (ids->seq > IPCID_SEQ_MAX)
 			ids->seq = 0;
 	} else {
 		new->seq = ipcid_to_seqx(next_id);
diff --git a/ipc/util.h b/ipc/util.h
index d64db3e..9c47d6f 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -100,6 +100,7 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
 
 #define ipcid_to_idx(id) ((id) % SEQ_MULTIPLIER)
 #define ipcid_to_seqx(id) ((id) / SEQ_MULTIPLIER)
+#define IPCID_SEQ_MAX min_t(int, INT_MAX/SEQ_MULTIPLIER, USHRT_MAX)
 
 /* must be called with ids->rwsem acquired for writing */
 int ipc_addid(struct ipc_ids *, struct kern_ipc_perm *, int);
-- 
1.8.1.4

[PATCH 6/7] ipc: share ids rwsem when possible in ipcget_public

[Davidlohr Bueso]

... and rewrite the function. For scenarios where the key is found and we end
up just doing different routinary checks, we can downgrade the ids->rwsem and
share it among concurrent readers. These checks include the following, which
are all safe to share the lock:

ops->more_checks() >>  sem_more_checks(), shm_more_checks()
ipc_check_perms()  >>  ipcperms(),ops->associate() >> [lsm]_[ipctype]_associate()

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 ipc/util.c | 60 +++++++++++++++++++++++++++++++++---------------------------
 1 file changed, 33 insertions(+), 27 deletions(-)

diff --git a/ipc/util.c b/ipc/util.c
index e1b4c6d..fc5c655 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -183,7 +183,7 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
  * ipc_findkey	- find a key in an ipc identifier set
  * @ids: ipc identifier set
  * @key: key to find
- *	
+ *
  * Returns the locked pointer to the ipc structure if found or NULL
  * otherwise. If key is found ipc points to the owning ipc structure
  *
@@ -375,48 +375,54 @@ static int ipc_check_perms(struct ipc_namespace *ns,
  * On success, the ipc id is returned.
  */
 static int ipcget_public(struct ipc_namespace *ns, struct ipc_ids *ids,
-		struct ipc_ops *ops, struct ipc_params *params)
+			 struct ipc_ops *ops, struct ipc_params *params)
 {
 	struct kern_ipc_perm *ipcp;
 	int flg = params->flg;
-	int err;
+	int err = 0;
 
-	/*
-	 * Take the lock as a writer since we are potentially going to add
-	 * a new entry + read locks are not "upgradable"
-	 */
 	down_write(&ids->rwsem);
 	ipcp = ipc_findkey(ids, params->key);
-	if (ipcp == NULL) {
+
+	if (!ipcp) {
 		/* key not used */
 		if (!(flg & IPC_CREAT))
 			err = -ENOENT;
-		else
+		else /* create new ipc object */
 			err = ops->getnew(ns, params);
-	} else {
-		/* ipc object has been locked by ipc_findkey() */
-
-		if (flg & IPC_CREAT && flg & IPC_EXCL)
-			err = -EEXIST;
-		else {
-			err = 0;
-			if (ops->more_checks)
-				err = ops->more_checks(ipcp, params);
-			if (!err)
-				/*
-				 * ipc_check_perms returns the IPC id on
-				 * success
-				 */
-				err = ipc_check_perms(ns, ipcp, ops, params);
-		}
+
+		goto done_write;
+	}
+
+	if ((flg & IPC_CREAT) && (flg & IPC_EXCL)) {
+		/* ipc object was locked by successful ipc_findkey() lookup */
 		ipc_unlock(ipcp);
+		err = -ENOENT;
+
+		goto done_write;
 	}
-	up_write(&ids->rwsem);
 
+	/*
+	 * The key was found, so we will just perform routinary checks on
+	 * ipc the object. Share the lock among other readers.
+	 */
+	downgrade_write(&ids->rwsem);
+
+	if (ops->more_checks)
+		err = ops->more_checks(ipcp, params);
+	if (!err)
+		/* returns the IPC id on success */
+		err = ipc_check_perms(ns, ipcp, ops, params);
+
+	ipc_unlock(ipcp);
+
+	up_read(&ids->rwsem);
+	return err;
+done_write:
+	up_write(&ids->rwsem);
 	return err;
 }
 
-
 /**
  * ipc_rmid - remove an ipc identifier
  * @ids: ipc identifier set
-- 
1.8.1.4

[PATCH 7/7] ipc,msg: document barriers

[Davidlohr Bueso]

Both expunge_all() and pipeline_send() rely on both a nil msg value and a
full barrier to guarantee the correct ordering when waking up a task. While
its counter part at the receiving end is well documented for the lockless
recv algorithm, we still need to document these specific smp_mb() calls.

Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
---
 ipc/msg.c | 19 +++++++++++++++++--
 1 file changed, 17 insertions(+), 2 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 4377f4a..8c1cc76 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -253,8 +253,14 @@ static void expunge_all(struct msg_queue *msq, int res)
 	struct msg_receiver *msr, *t;
 
 	list_for_each_entry_safe(msr, t, &msq->q_receivers, r_list) {
-		msr->r_msg = NULL;
+		msr->r_msg = NULL; /* initialize expunge ordering */
 		wake_up_process(msr->r_tsk);
+		/*
+		 * Ensure that the wakeup is visible before setting r_msg as
+		 * the receiving end depends on it: either spinning on a nil,
+		 * or dealing with -EAGAIN cases. See lockless reveice part 1
+		 * and 2 in do_msgrcv().
+		 */
 		smp_mb();
 		msr->r_msg = ERR_PTR(res);
 	}
@@ -638,15 +644,22 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg)
 
 			list_del(&msr->r_list);
 			if (msr->r_maxsize < msg->m_ts) {
+				/* initialize pipelined send ordering */
 				msr->r_msg = NULL;
 				wake_up_process(msr->r_tsk);
-				smp_mb();
+				smp_mb(); /* see barrier comment below */
 				msr->r_msg = ERR_PTR(-E2BIG);
 			} else {
 				msr->r_msg = NULL;
 				msq->q_lrpid = task_pid_vnr(msr->r_tsk);
 				msq->q_rtime = get_seconds();
 				wake_up_process(msr->r_tsk);
+				/*
+				 * Ensure that the wakeup is visible before
+				 * setting r_msg, as the receiving end depends
+				 * on it. See lockless reveice part 1 and 2 in
+				 * do_msgrcv().
+				 */
 				smp_mb();
 				msr->r_msg = msg;
 
@@ -654,6 +667,7 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg)
 			}
 		}
 	}
+
 	return 0;
 }
 
@@ -716,6 +730,7 @@ long do_msgsnd(int msqid, long mtype, void __user *mtext,
 			goto out_unlock0;
 		}
 
+		/* enqueue the sender and prepare to block */
 		ss_add(msq, &s);
 
 		if (!ipc_rcu_getref(msq)) {
-- 
1.8.1.4

Re: [PATCH 7/7] ipc,msg: document barriers

[Mike Galbraith]

On Fri, 2014-01-10 at 10:48 -0800, Davidlohr Bueso wrote: 
> Both expunge_all() and pipeline_send() rely on both a nil msg value and a
> full barrier to guarantee the correct ordering when waking up a task. While
> its counter part at the receiving end is well documented for the lockless
> recv algorithm, we still need to document these specific smp_mb() calls.
> 
> Signed-off-by: Davidlohr Bueso <davidlohr@hp.com>
> ---
>  ipc/msg.c | 19 +++++++++++++++++--
>  1 file changed, 17 insertions(+), 2 deletions(-)
> 
> diff --git a/ipc/msg.c b/ipc/msg.c
> index 4377f4a..8c1cc76 100644
> --- a/ipc/msg.c
> +++ b/ipc/msg.c
> @@ -253,8 +253,14 @@ static void expunge_all(struct msg_queue *msq, int res)
>  	struct msg_receiver *msr, *t;
>  
>  	list_for_each_entry_safe(msr, t, &msq->q_receivers, r_list) {
> -		msr->r_msg = NULL;
> +		msr->r_msg = NULL; /* initialize expunge ordering */
>  		wake_up_process(msr->r_tsk);
> +		/*
> +		 * Ensure that the wakeup is visible before setting r_msg as
> +		 * the receiving end depends on it: either spinning on a nil,
> +		 * or dealing with -EAGAIN cases. See lockless reveice part 1
                                                                 ^^^^^^^ lysdexic fingers

> +		 * and 2 in do_msgrcv().
> +		 */
>  		smp_mb();
>  		msr->r_msg = ERR_PTR(res);
>  	}
> @@ -638,15 +644,22 @@ static inline int pipelined_send(struct msg_queue *msq, struct msg_msg *msg)
>  
>  			list_del(&msr->r_list);
>  			if (msr->r_maxsize < msg->m_ts) {
> +				/* initialize pipelined send ordering */
>  				msr->r_msg = NULL;
>  				wake_up_process(msr->r_tsk);
> -				smp_mb();
> +				smp_mb(); /* see barrier comment below */
>  				msr->r_msg = ERR_PTR(-E2BIG);
>  			} else {
>  				msr->r_msg = NULL;
>  				msq->q_lrpid = task_pid_vnr(msr->r_tsk);
>  				msq->q_rtime = get_seconds();
>  				wake_up_process(msr->r_tsk);
> +				/*
> +				 * Ensure that the wakeup is visible before
> +				 * setting r_msg, as the receiving end depends
> +				 * on it. See lockless reveice part 1 and 2 in
                                                         ^^^^^^^ ditto

Re: [PATCH -next 0/7] ipc: some misc updates & optimizations

[Manfred Spraul]

Hi David,

On 01/10/2014 07:48 PM, Davidlohr Bueso wrote:
> A few updates to the ipc code, mostly some very needed cleanups (yes, we still
> have loads more to do, but it's a start). Nothing particularly interesting,
> except perhaps for the last two patches, which include a locking optimization
> and barrier documentation for message queues. Absolutely no functional changes.
>
> Applies on top of linux-next +  Manfred's 'whitespace cleanup' patch. Tested
> with LTP.
Now you confuse me.
In your other mail, you write that my patch doesn't apply to linux-next:

On 01/07/2014 06:12 PM, Davidlohr Bueso wrote:
> This patch doesn't apply on top of linux-next (which now includes 
> Rafael's changes), could you please resend? I'm planning some more 
> cleanups on top of this so, if Andrew agrees, I can include these 
> changes to my patchset. Thanks, Davidlohr 

Perhaps: Could you include (and update, if required) my patch into your 
patchset?
It's probably simpler if we have only one set of ipc cleanups around.

> Thanks!
>
> Davidlohr Bueso (7):
>    ipc: standardize code comments
>    ipc: remove braces for single statements
>    ipc: remove useless return statement
>    ipc: simplify sysvipc_proc_open return
>    ipc: delete seq_max field in struct ipc_ids
Acked-by: Manfred Spraul <manfred@colorfullife.com>
>    ipc: share ids rwsem when possible in ipcget_public
This patch is a real change. Could you at least make it the last one in 
the series?
I will try to review it, but I don't know yet when I will find the time.

>    ipc,msg: document barriers
Acked-by: Manfred Spraul <manfred@colorfullife.com>

--
     Manfred

Re: [PATCH -next 0/7] ipc: some misc updates & optimizations

[Davidlohr Bueso]

On Sat, 2014-01-11 at 10:02 +0100, Manfred Spraul wrote:
> Hi David,
> 
> On 01/10/2014 07:48 PM, Davidlohr Bueso wrote:
> > A few updates to the ipc code, mostly some very needed cleanups (yes, we still
> > have loads more to do, but it's a start). Nothing particularly interesting,
> > except perhaps for the last two patches, which include a locking optimization
> > and barrier documentation for message queues. Absolutely no functional changes.
> >
> > Applies on top of linux-next +  Manfred's 'whitespace cleanup' patch. Tested
> > with LTP.
> Now you confuse me.
> In your other mail, you write that my patch doesn't apply to linux-next:

Just ignore that email, Andrew then applied (and fixed the merge
conflict) your whitespace cleanup patch and was already in linux-next by
the time I sent this patchset, so no need to worry/coordinate.

> 
> On 01/07/2014 06:12 PM, Davidlohr Bueso wrote:
> > This patch doesn't apply on top of linux-next (which now includes 
> > Rafael's changes), could you please resend? I'm planning some more 
> > cleanups on top of this so, if Andrew agrees, I can include these 
> > changes to my patchset. Thanks, Davidlohr 
> 
> Perhaps: Could you include (and update, if required) my patch into your 
> patchset?
> It's probably simpler if we have only one set of ipc cleanups around.
> 
> > Thanks!
> >
> > Davidlohr Bueso (7):
> >    ipc: standardize code comments
> >    ipc: remove braces for single statements
> >    ipc: remove useless return statement
> >    ipc: simplify sysvipc_proc_open return
> >    ipc: delete seq_max field in struct ipc_ids
> Acked-by: Manfred Spraul <manfred@colorfullife.com>
> >    ipc: share ids rwsem when possible in ipcget_public
> This patch is a real change. Could you at least make it the last one in 
> the series?
> I will try to review it, but I don't know yet when I will find the time.

Thanks, I definitely want your ack/review for this one.

> 
> >    ipc,msg: document barriers
> Acked-by: Manfred Spraul <manfred@colorfullife.com>
> 
> --
>      Manfred