[PATCH 00/07] sysv SHM checkpoint/restart

[PATCH 00/07] sysv SHM checkpoint/restart

[Oren Laadan]

This patchset adds support for IPC shared-memory. It applies on top
of c/r v14-rc2. Tested on x86_32 and verified with the test provided
in the userspace tools. 

Oren Laadan (7):
      ipc: allow allocation of an ipc object with desired identifier
      ipc: helpers to save and restore kern_ipc_perm structures
      sysvipc-shm: checkpoint
      sysvipc-shm: restart
      Infrastructure for work postponed to the end of checkpoint/restart
      sysvipc-shm: export interface from ipc/shm.c to delete ipc shm
      sysvipc-shm: correctly handle deleted (active) ipc shared memory

 checkpoint/Makefile            |    6 +-
 checkpoint/checkpoint.c        |    7 +
 checkpoint/ckpt_mem.c          |    9 +
 checkpoint/restart.c           |    6 +
 checkpoint/rstr_mem.c          |   23 +++
 checkpoint/sys.c               |    7 +
 checkpoint/util_ipc.c          |   81 ++++++++++
 checkpoint/workqueue.c         |   62 ++++++++
 include/linux/checkpoint.h     |   25 +++
 include/linux/checkpoint_hdr.h |   47 ++++++
 include/linux/shm.h            |    4 +
 ipc/Makefile                   |    1 +
 ipc/ckpt_shm.c                 |  339 ++++++++++++++++++++++++++++++++++++++++
 ipc/msg.c                      |   17 ++-
 ipc/sem.c                      |   17 ++-
 ipc/shm.c                      |   34 +++-
 ipc/util.c                     |   42 ++++--
 ipc/util.h                     |   11 +-
 18 files changed, 702 insertions(+), 36 deletions(-)

[RFC v14-rc2][PATCH 1/7] ipc: allow allocation of an ipc object with desired identifier

[Oren Laadan]

During restart, we need to allocate ipc objects that with the same
identifiers as recorded during checkpoint. Modify the allocation
code allow an in-kernel caller to request a specific ipc identifier.
The system call interface remains unchanged.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 ipc/msg.c  |   17 ++++++++++++-----
 ipc/sem.c  |   17 ++++++++++++-----
 ipc/shm.c  |   19 +++++++++++++------
 ipc/util.c |   42 +++++++++++++++++++++++++++++-------------
 ipc/util.h |   11 ++++++++---
 5 files changed, 74 insertions(+), 32 deletions(-)

diff --git a/ipc/msg.c b/ipc/msg.c
index 2ceab7f..1db7c45 100644
--- a/ipc/msg.c
+++ b/ipc/msg.c
@@ -73,7 +73,7 @@ struct msg_sender {
 #define msg_unlock(msq)		ipc_unlock(&(msq)->q_perm)
 
 static void freeque(struct ipc_namespace *, struct kern_ipc_perm *);
-static int newque(struct ipc_namespace *, struct ipc_params *);
+static int newque(struct ipc_namespace *, struct ipc_params *, int);
 #ifdef CONFIG_PROC_FS
 static int sysvipc_msg_proc_show(struct seq_file *s, void *it);
 #endif
@@ -174,10 +174,12 @@ static inline void msg_rmid(struct ipc_namespace *ns, struct msg_queue *s)
  * newque - Create a new msg queue
  * @ns: namespace
  * @params: ptr to the structure that contains the key and msgflg
+ * @req_id: request desired id if available (-1 if don't care)
  *
  * Called with msg_ids.rw_mutex held (writer)
  */
-static int newque(struct ipc_namespace *ns, struct ipc_params *params)
+static int
+newque(struct ipc_namespace *ns, struct ipc_params *params, int req_id)
 {
 	struct msg_queue *msq;
 	int id, retval;
@@ -201,7 +203,7 @@ static int newque(struct ipc_namespace *ns, struct ipc_params *params)
 	/*
 	 * ipc_addid() locks msq
 	 */
-	id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni);
+	id = ipc_addid(&msg_ids(ns), &msq->q_perm, ns->msg_ctlmni, req_id);
 	if (id < 0) {
 		security_msg_queue_free(msq);
 		ipc_rcu_putref(msq);
@@ -309,7 +311,7 @@ static inline int msg_security(struct kern_ipc_perm *ipcp, int msgflg)
 	return security_msg_queue_associate(msq, msgflg);
 }
 
-SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
+int do_msgget(key_t key, int msgflg, int req_id)
 {
 	struct ipc_namespace *ns;
 	struct ipc_ops msg_ops;
@@ -324,7 +326,12 @@ SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
 	msg_params.key = key;
 	msg_params.flg = msgflg;
 
-	return ipcget(ns, &msg_ids(ns), &msg_ops, &msg_params);
+	return ipcget(ns, &msg_ids(ns), &msg_ops, &msg_params, req_id);
+}
+
+SYSCALL_DEFINE2(msgget, key_t, key, int, msgflg)
+{
+	return do_msgget(key, msgflg, -1);
 }
 
 static inline unsigned long
diff --git a/ipc/sem.c b/ipc/sem.c
index 16a2189..207dbbb 100644
--- a/ipc/sem.c
+++ b/ipc/sem.c
@@ -92,7 +92,7 @@
 #define sem_unlock(sma)		ipc_unlock(&(sma)->sem_perm)
 #define sem_checkid(sma, semid)	ipc_checkid(&sma->sem_perm, semid)
 
-static int newary(struct ipc_namespace *, struct ipc_params *);
+static int newary(struct ipc_namespace *, struct ipc_params *, int);
 static void freeary(struct ipc_namespace *, struct kern_ipc_perm *);
 #ifdef CONFIG_PROC_FS
 static int sysvipc_sem_proc_show(struct seq_file *s, void *it);
@@ -227,11 +227,13 @@ static inline void sem_rmid(struct ipc_namespace *ns, struct sem_array *s)
  * newary - Create a new semaphore set
  * @ns: namespace
  * @params: ptr to the structure that contains key, semflg and nsems
+ * @req_id: request desired id if available (-1 if don't care)
  *
  * Called with sem_ids.rw_mutex held (as a writer)
  */
 
-static int newary(struct ipc_namespace *ns, struct ipc_params *params)
+static int
+newary(struct ipc_namespace *ns, struct ipc_params *params, int req_id)
 {
 	int id;
 	int retval;
@@ -263,7 +265,7 @@ static int newary(struct ipc_namespace *ns, struct ipc_params *params)
 		return retval;
 	}
 
-	id = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni);
+	id = ipc_addid(&sem_ids(ns), &sma->sem_perm, ns->sc_semmni, req_id);
 	if (id < 0) {
 		security_sem_free(sma);
 		ipc_rcu_putref(sma);
@@ -308,7 +310,7 @@ static inline int sem_more_checks(struct kern_ipc_perm *ipcp,
 	return 0;
 }
 
-SYSCALL_DEFINE3(semget, key_t, key, int, nsems, int, semflg)
+int do_semget(key_t key, int nsems, int semflg, int req_id)
 {
 	struct ipc_namespace *ns;
 	struct ipc_ops sem_ops;
@@ -327,7 +329,12 @@ SYSCALL_DEFINE3(semget, key_t, key, int, nsems, int, semflg)
 	sem_params.flg = semflg;
 	sem_params.u.nsems = nsems;
 
-	return ipcget(ns, &sem_ids(ns), &sem_ops, &sem_params);
+	return ipcget(ns, &sem_ids(ns), &sem_ops, &sem_params, req_id);
+}
+
+SYSCALL_DEFINE3(semget, key_t, key, int, nsems, int, semflg)
+{
+	return do_semget(key, nsems, semflg, -1);
 }
 
 /*
diff --git a/ipc/shm.c b/ipc/shm.c
index 05d51d2..4135f28 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -61,7 +61,7 @@ static struct vm_operations_struct shm_vm_ops;
 #define shm_unlock(shp)			\
 	ipc_unlock(&(shp)->shm_perm)
 
-static int newseg(struct ipc_namespace *, struct ipc_params *);
+static int newseg(struct ipc_namespace *, struct ipc_params *, int);
 static void shm_open(struct vm_area_struct *vma);
 static void shm_close(struct vm_area_struct *vma);
 static void shm_destroy (struct ipc_namespace *ns, struct shmid_kernel *shp);
@@ -82,7 +82,7 @@ void shm_init_ns(struct ipc_namespace *ns)
  * Called with shm_ids.rw_mutex (writer) and the shp structure locked.
  * Only shm_ids.rw_mutex remains locked on exit.
  */
-static void do_shm_rmid(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
+void do_shm_rmid(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp)
 {
 	struct shmid_kernel *shp;
 	shp = container_of(ipcp, struct shmid_kernel, shm_perm);
@@ -325,11 +325,13 @@ static struct vm_operations_struct shm_vm_ops = {
  * newseg - Create a new shared memory segment
  * @ns: namespace
  * @params: ptr to the structure that contains key, size and shmflg
+ * @req_id: request desired id if available (-1 if don't care)
  *
  * Called with shm_ids.rw_mutex held as a writer.
  */
 
-static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
+static int
+newseg(struct ipc_namespace *ns, struct ipc_params *params, int req_id)
 {
 	key_t key = params->key;
 	int shmflg = params->flg;
@@ -384,7 +386,7 @@ static int newseg(struct ipc_namespace *ns, struct ipc_params *params)
 	if (IS_ERR(file))
 		goto no_file;
 
-	id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni);
+	id = ipc_addid(&shm_ids(ns), &shp->shm_perm, ns->shm_ctlmni, req_id);
 	if (id < 0) {
 		error = id;
 		goto no_id;
@@ -442,7 +444,7 @@ static inline int shm_more_checks(struct kern_ipc_perm *ipcp,
 	return 0;
 }
 
-SYSCALL_DEFINE3(shmget, key_t, key, size_t, size, int, shmflg)
+int do_shmget(key_t key, size_t size, int shmflg, int req_id)
 {
 	struct ipc_namespace *ns;
 	struct ipc_ops shm_ops;
@@ -458,7 +460,12 @@ SYSCALL_DEFINE3(shmget, key_t, key, size_t, size, int, shmflg)
 	shm_params.flg = shmflg;
 	shm_params.u.size = size;
 
-	return ipcget(ns, &shm_ids(ns), &shm_ops, &shm_params);
+	return ipcget(ns, &shm_ids(ns), &shm_ops, &shm_params, req_id);
+}
+
+SYSCALL_DEFINE3(shmget, key_t, key, size_t, size, int, shmflg)
+{
+	return do_shmget(key, size, shmflg, -1);
 }
 
 static inline unsigned long copy_shmid_to_user(void __user *buf, struct shmid64_ds *in, int version)
diff --git a/ipc/util.c b/ipc/util.c
index 7585a72..58eaa0b 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -256,10 +256,12 @@ int ipc_get_maxid(struct ipc_ids *ids)
  *	Called with ipc_ids.rw_mutex held as a writer.
  */
  
-int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
+int
+ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size, int req_id)
 {
 	uid_t euid;
 	gid_t egid;
+	int lid = 0;
 	int id, err;
 
 	if (size > IPCMNI)
@@ -268,28 +270,41 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
 	if (ids->in_use >= size)
 		return -ENOSPC;
 
+	if (req_id >= 0)
+		lid = ipcid_to_idx(req_id);
+
 	spin_lock_init(&new->lock);
 	new->deleted = 0;
 	rcu_read_lock();
 	spin_lock(&new->lock);
 
-	err = idr_get_new(&ids->ipcs_idr, new, &id);
+	err = idr_get_new_above(&ids->ipcs_idr, new, lid, &id);
 	if (err) {
 		spin_unlock(&new->lock);
 		rcu_read_unlock();
 		return err;
 	}
 
+	if (req_id >= 0) {
+		if (id != lid) {
+			idr_remove(&ids->ipcs_idr, id);
+			spin_unlock(&new->lock);
+			rcu_read_unlock();
+			return -EBUSY;
+		}
+		new->seq = req_id / SEQ_MULTIPLIER;
+	} else {
+		new->seq = ids->seq++;
+		if (ids->seq > ids->seq_max)
+			ids->seq = 0;
+	}
+
 	ids->in_use++;
 
 	current_euid_egid(&euid, &egid);
 	new->cuid = new->uid = euid;
 	new->gid = new->cgid = egid;
 
-	new->seq = ids->seq++;
-	if(ids->seq > ids->seq_max)
-		ids->seq = 0;
-
 	new->id = ipc_buildid(id, new->seq);
 	return id;
 }
@@ -305,7 +320,7 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
  *	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)
+		struct ipc_ops *ops, struct ipc_params *params, int req_id)
 {
 	int err;
 retry:
@@ -315,7 +330,7 @@ retry:
 		return -ENOMEM;
 
 	down_write(&ids->rw_mutex);
-	err = ops->getnew(ns, params);
+	err = ops->getnew(ns, params, req_id);
 	up_write(&ids->rw_mutex);
 
 	if (err == -EAGAIN)
@@ -360,6 +375,7 @@ static int ipc_check_perms(struct kern_ipc_perm *ipcp, struct ipc_ops *ops,
  *	@ids: IPC identifer set
  *	@ops: the actual creation routine to call
  *	@params: its parameters
+ *	@req_id: request desired id if available (-1 if don't care)
  *
  *	This routine is called by sys_msgget, sys_semget() and sys_shmget()
  *	when the key is not IPC_PRIVATE.
@@ -369,7 +385,7 @@ static int ipc_check_perms(struct kern_ipc_perm *ipcp, struct ipc_ops *ops,
  *	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, int req_id)
 {
 	struct kern_ipc_perm *ipcp;
 	int flg = params->flg;
@@ -390,7 +406,7 @@ retry:
 		else if (!err)
 			err = -ENOMEM;
 		else
-			err = ops->getnew(ns, params);
+			err = ops->getnew(ns, params, req_id);
 	} else {
 		/* ipc object has been locked by ipc_findkey() */
 
@@ -751,12 +767,12 @@ struct kern_ipc_perm *ipc_lock_check(struct ipc_ids *ids, int id)
  * Common routine called by sys_msgget(), sys_semget() and sys_shmget().
  */
 int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
-			struct ipc_ops *ops, struct ipc_params *params)
+		struct ipc_ops *ops, struct ipc_params *params, int req_id)
 {
 	if (params->key == IPC_PRIVATE)
-		return ipcget_new(ns, ids, ops, params);
+		return ipcget_new(ns, ids, ops, params, req_id);
 	else
-		return ipcget_public(ns, ids, ops, params);
+		return ipcget_public(ns, ids, ops, params, req_id);
 }
 
 /**
diff --git a/ipc/util.h b/ipc/util.h
index 3646b45..3bef7ce 100644
--- a/ipc/util.h
+++ b/ipc/util.h
@@ -52,7 +52,7 @@ struct ipc_params {
  *      . routine to call for an extra check if needed
  */
 struct ipc_ops {
-	int (*getnew) (struct ipc_namespace *, struct ipc_params *);
+	int (*getnew) (struct ipc_namespace *, struct ipc_params *, int);
 	int (*associate) (struct kern_ipc_perm *, int);
 	int (*more_checks) (struct kern_ipc_perm *, struct ipc_params *);
 };
@@ -75,7 +75,7 @@ void __init ipc_init_proc_interface(const char *path, const char *header,
 #define ipcid_to_idx(id) ((id) % SEQ_MULTIPLIER)
 
 /* must be called with ids->rw_mutex acquired for writing */
-int ipc_addid(struct ipc_ids *, struct kern_ipc_perm *, int);
+int ipc_addid(struct ipc_ids *, struct kern_ipc_perm *, int, int);
 
 /* must be called with ids->rw_mutex acquired for reading */
 int ipc_get_maxid(struct ipc_ids *);
@@ -152,6 +152,11 @@ static inline void ipc_unlock(struct kern_ipc_perm *perm)
 
 struct kern_ipc_perm *ipc_lock_check(struct ipc_ids *ids, int id);
 int ipcget(struct ipc_namespace *ns, struct ipc_ids *ids,
-			struct ipc_ops *ops, struct ipc_params *params);
+		struct ipc_ops *ops, struct ipc_params *params, int req_id);
+
+/* for checkpoint/restart */
+extern int do_shmget(key_t key, size_t size, int shmflg, int req_id);
+extern void do_shm_rmid(struct ipc_namespace *ns, struct kern_ipc_perm *ipcp);
+
 
 #endif
-- 
1.5.4.3

Re: [RFC v14-rc2][PATCH 1/7] ipc: allow allocation of an ipc object with desired identifier

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> -int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
> +int
> +ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size, int req_id)
>  {
>  	uid_t euid;
>  	gid_t egid;
> +	int lid = 0;
>  	int id, err;
> 
>  	if (size > IPCMNI)
> @@ -268,28 +270,41 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
>  	if (ids->in_use >= size)
>  		return -ENOSPC;
> 
> +	if (req_id >= 0)
> +		lid = ipcid_to_idx(req_id);
> +
>  	spin_lock_init(&new->lock);
>  	new->deleted = 0;
>  	rcu_read_lock();
>  	spin_lock(&new->lock);
> 
> -	err = idr_get_new(&ids->ipcs_idr, new, &id);
> +	err = idr_get_new_above(&ids->ipcs_idr, new, lid, &id);
>  	if (err) {
>  		spin_unlock(&new->lock);
>  		rcu_read_unlock();
>  		return err;
>  	}
> 
> +	if (req_id >= 0) {
> +		if (id != lid) {
> +			idr_remove(&ids->ipcs_idr, id);
> +			spin_unlock(&new->lock);
> +			rcu_read_unlock();
> +			return -EBUSY;
> +		}
> +		new->seq = req_id / SEQ_MULTIPLIER;

Should this be
	
	new->seq = req_id % ids->seq_max;

?

> +	} else {
> +		new->seq = ids->seq++;
> +		if (ids->seq > ids->seq_max)
> +			ids->seq = 0;
> +	}
> +
>  	ids->in_use++;
> 
>  	current_euid_egid(&euid, &egid);
>  	new->cuid = new->uid = euid;
>  	new->gid = new->cgid = egid;
> 
> -	new->seq = ids->seq++;
> -	if(ids->seq > ids->seq_max)
> -		ids->seq = 0;
> -

Re: [RFC v14-rc2][PATCH 1/7] ipc: allow allocation of an ipc object with desired identifier

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
>> -int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
>> +int
>> +ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size, int req_id)
>>  {
>>  	uid_t euid;
>>  	gid_t egid;
>> +	int lid = 0;
>>  	int id, err;
>>
>>  	if (size > IPCMNI)
>> @@ -268,28 +270,41 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
>>  	if (ids->in_use >= size)
>>  		return -ENOSPC;
>>
>> +	if (req_id >= 0)
>> +		lid = ipcid_to_idx(req_id);
>> +
>>  	spin_lock_init(&new->lock);
>>  	new->deleted = 0;
>>  	rcu_read_lock();
>>  	spin_lock(&new->lock);
>>
>> -	err = idr_get_new(&ids->ipcs_idr, new, &id);
>> +	err = idr_get_new_above(&ids->ipcs_idr, new, lid, &id);
>>  	if (err) {
>>  		spin_unlock(&new->lock);
>>  		rcu_read_unlock();
>>  		return err;
>>  	}
>>
>> +	if (req_id >= 0) {
>> +		if (id != lid) {
>> +			idr_remove(&ids->ipcs_idr, id);
>> +			spin_unlock(&new->lock);
>> +			rcu_read_unlock();
>> +			return -EBUSY;
>> +		}
>> +		new->seq = req_id / SEQ_MULTIPLIER;
> 
> Should this be
> 	
> 	new->seq = req_id % ids->seq_max;
> 
> ?

This is how the user-visible IPC id is constructed:

	static inline int ipc_buildid(int id, int seq)
	{
	        return SEQ_MULTIPLIER * seq + id;
	}

and I want to get the original 'seq' ....  (the 'id' in this function is
an in-kernel identifier)

> 
>> +	} else {
>> +		new->seq = ids->seq++;
>> +		if (ids->seq > ids->seq_max)
>> +			ids->seq = 0;
>> +	}
>> +
>>  	ids->in_use++;
>>
>>  	current_euid_egid(&euid, &egid);
>>  	new->cuid = new->uid = euid;
>>  	new->gid = new->cgid = egid;
>>
>> -	new->seq = ids->seq++;
>> -	if(ids->seq > ids->seq_max)
>> -		ids->seq = 0;
>> -
> 

Re: [RFC v14-rc2][PATCH 1/7] ipc: allow allocation of an ipc object with desired identifier

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> 
> 
> Serge E. Hallyn wrote:
> > Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> >> -int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
> >> +int
> >> +ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int size, int req_id)
> >>  {
> >>  	uid_t euid;
> >>  	gid_t egid;
> >> +	int lid = 0;
> >>  	int id, err;
> >>
> >>  	if (size > IPCMNI)
> >> @@ -268,28 +270,41 @@ int ipc_addid(struct ipc_ids* ids, struct kern_ipc_perm* new, int size)
> >>  	if (ids->in_use >= size)
> >>  		return -ENOSPC;
> >>
> >> +	if (req_id >= 0)
> >> +		lid = ipcid_to_idx(req_id);
> >> +
> >>  	spin_lock_init(&new->lock);
> >>  	new->deleted = 0;
> >>  	rcu_read_lock();
> >>  	spin_lock(&new->lock);
> >>
> >> -	err = idr_get_new(&ids->ipcs_idr, new, &id);
> >> +	err = idr_get_new_above(&ids->ipcs_idr, new, lid, &id);
> >>  	if (err) {
> >>  		spin_unlock(&new->lock);
> >>  		rcu_read_unlock();
> >>  		return err;
> >>  	}
> >>
> >> +	if (req_id >= 0) {
> >> +		if (id != lid) {
> >> +			idr_remove(&ids->ipcs_idr, id);
> >> +			spin_unlock(&new->lock);
> >> +			rcu_read_unlock();
> >> +			return -EBUSY;
> >> +		}
> >> +		new->seq = req_id / SEQ_MULTIPLIER;
> > 
> > Should this be
> > 	
> > 	new->seq = req_id % ids->seq_max;
> > 
> > ?
> 
> This is how the user-visible IPC id is constructed:
> 
> 	static inline int ipc_buildid(int id, int seq)
> 	{
> 	        return SEQ_MULTIPLIER * seq + id;
> 	}
> 
> and I want to get the original 'seq' ....  (the 'id' in this function is
> an in-kernel identifier)

Hmm, yeah, I guess...  I misunderstood what seq was doing.  In
fact, now I just don't understand what it's doing at all...
Weird.

> >> +	} else {
> >> +		new->seq = ids->seq++;
> >> +		if (ids->seq > ids->seq_max)
> >> +			ids->seq = 0;
> >> +	}
> >> +
> >>  	ids->in_use++;
> >>
> >>  	current_euid_egid(&euid, &egid);
> >>  	new->cuid = new->uid = euid;
> >>  	new->gid = new->cgid = egid;
> >>
> >> -	new->seq = ids->seq++;
> >> -	if(ids->seq > ids->seq_max)
> >> -		ids->seq = 0;
> >> -
> > 

[RFC v14-rc2][PATCH 2/7] ipc: helpers to save and restore kern_ipc_perm structures

[Oren Laadan]

Add the helpers to save and restore the contents of 'struct
kern_ipc_perm'. Add header structures for ipc state. Put
place-holders to save and restore ipc state.

TODO:
This patch does _not_ address the issues of users/groups and the
related security issues. For now, it saves the old user/group of
ipc objects, but does not restore them during restart.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 checkpoint/Makefile            |    3 +-
 checkpoint/checkpoint.c        |    3 +
 checkpoint/restart.c           |    3 +
 checkpoint/util_ipc.c          |   82 ++++++++++++++++++++++++++++++++++++++++
 include/linux/checkpoint.h     |   12 ++++++
 include/linux/checkpoint_hdr.h |   32 +++++++++++++++
 6 files changed, 134 insertions(+), 1 deletions(-)
 create mode 100644 checkpoint/util_ipc.c

diff --git a/checkpoint/Makefile b/checkpoint/Makefile
index 607d864..71061de 100644
--- a/checkpoint/Makefile
+++ b/checkpoint/Makefile
@@ -3,4 +3,5 @@
 #
 
 obj-$(CONFIG_CHECKPOINT) += sys.o checkpoint.o restart.o objhash.o \
-		ckpt_mem.o rstr_mem.o ckpt_file.o rstr_file.o
+	ckpt_mem.o rstr_mem.o ckpt_file.o rstr_file.o \
+	util_ipc.o
diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
index ef35754..2ebaa05 100644
--- a/checkpoint/checkpoint.c
+++ b/checkpoint/checkpoint.c
@@ -572,6 +572,9 @@ int do_checkpoint(struct cr_ctx *ctx, pid_t pid)
 	ret = cr_write_tree(ctx);
 	if (ret < 0)
 		goto out;
+	ret = cr_write_ipc(ctx, ctx->root_nsproxy);
+	if (ret < 0)
+		goto out;
 
 	ret = cr_write_all_tasks(ctx);
 	if (ret < 0)
diff --git a/checkpoint/restart.c b/checkpoint/restart.c
index adebc1c..ed5c725 100644
--- a/checkpoint/restart.c
+++ b/checkpoint/restart.c
@@ -507,6 +507,9 @@ static int do_restart_root(struct cr_ctx *ctx, pid_t pid)
 	ret = cr_read_tree(ctx);
 	if (ret < 0)
 		goto out;
+	ret = cr_read_ipc(ctx);
+	if (ret < 0)
+		goto out;
 
 	ret = cr_ctx_restart(ctx, pid);
 	if (ret < 0)
diff --git a/checkpoint/util_ipc.c b/checkpoint/util_ipc.c
new file mode 100644
index 0000000..70c4b18
--- /dev/null
+++ b/checkpoint/util_ipc.c
@@ -0,0 +1,82 @@
+/*
+ *  Checkpoint logic and helpers
+ *
+ *  Copyright (C) 2009 Oren Laadan
+ *
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License.  See the file COPYING in the main directory of the Linux
+ *  distribution for more details.
+ */
+
+#ifdef CONFIG_SYSVIPC
+
+#include <linux/version.h>
+#include <linux/checkpoint.h>
+#include <linux/checkpoint_hdr.h>
+
+int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy)
+{
+	return 0;
+}
+
+int cr_read_ipc(struct cr_ctx *ctx)
+{
+	return 0;
+}
+
+void cr_fill_ipc_perms(struct cr_hdr_ipc_perms *hh, struct kern_ipc_perm *perm)
+{
+	hh->id = perm->id;
+	hh->key = perm->key;
+	hh->uid = perm->uid;
+	hh->gid = perm->gid;
+	hh->cuid = perm->cuid;
+	hh->cgid = perm->cgid;
+	hh->mode = perm->mode & S_IRWXUGO;
+	hh->seq = perm->seq;
+}
+
+int cr_load_ipc_perms(struct cr_hdr_ipc_perms *hh, struct kern_ipc_perm *perm)
+{
+	if (hh->id < 0)
+		return -EINVAL;
+	if (CR_TST_OVERFLOW_16(hh->uid, perm->uid) ||
+	    CR_TST_OVERFLOW_16(hh->gid, perm->gid) ||
+	    CR_TST_OVERFLOW_16(hh->cuid, perm->cuid) ||
+	    CR_TST_OVERFLOW_16(hh->cgid, perm->cgid) ||
+	    CR_TST_OVERFLOW_16(hh->mode, perm->mode))
+		return -EINVAL;
+	if (hh->seq >= USHORT_MAX)
+		return -EINVAL;
+	if (hh->mode & ~S_IRWXUGO)
+		return -EINVAL;
+
+	/* FIXME: verify the ->mode field makes sense */
+
+	perm->id = hh->id;
+	perm->key = hh->key;
+#if 0 /* FIXME: requires security checks */
+	perm->uid = hh->uid;
+	perm->gid = hh->gid;
+	perm->cuid = hh->cuid;
+	perm->cgid = hh->cgid;
+#endif
+	perm->mode = hh->mode;
+	perm->seq = hh->seq;
+
+	return 0;
+}
+
+#else
+
+int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy)
+{
+	return 0;
+}
+
+int cr_read_ipc(struct cr_ctx *ctx)
+{
+	return 0;
+}
+
+#endif /* CONFIG_SYSVIPC */
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index 59ec563..15a9890 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -14,6 +14,7 @@
 #include <linux/fs.h>
 #include <linux/path.h>
 #include <linux/sched.h>
+#include <linux/nsproxy.h>
 #include <asm/atomic.h>
 
 #define CR_VERSION  3
@@ -111,15 +112,26 @@ extern int cr_write_shmem_contents(struct cr_ctx *ctx, struct inode *inode);
 extern int cr_read_shmem_contents(struct cr_ctx *ctx, struct inode *inode);
 
 extern int do_checkpoint(struct cr_ctx *ctx, pid_t pid);
+extern int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy);
 extern int cr_write_mm(struct cr_ctx *ctx, struct task_struct *t);
 extern int cr_write_fd_table(struct cr_ctx *ctx, struct task_struct *t);
 extern int cr_write_file(struct cr_ctx *ctx, struct file *file);
 
 extern int do_restart(struct cr_ctx *ctx, pid_t pid);
+extern int cr_read_ipc(struct cr_ctx *ctx);
 extern int cr_read_mm(struct cr_ctx *ctx);
 extern int cr_read_fd_table(struct cr_ctx *ctx);
 extern int cr_read_file(struct cr_ctx *ctx, int objref);
 
+#ifdef CONFIG_SYSVIPC
+struct cr_hdr_ipc_perms;
+extern void cr_fill_ipc_perms(struct cr_hdr_ipc_perms *hh,
+			      struct kern_ipc_perm *perm);
+extern int cr_load_ipc_perms(struct cr_hdr_ipc_perms *hh,
+			     struct kern_ipc_perm *perm);
+#endif
+
+
 /* useful macros to copy fields and buffers to/from cr_hdr_xxx structures */
 #define CR_CPT 1
 #define CR_RST 2
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 22b40a2..6efe8dd 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -59,9 +59,23 @@ enum {
 	CR_HDR_FILE,
 	CR_HDR_FD_PIPE,
 
+	CR_HDR_IPC = 401,
+	CR_HDR_IPC_SHM,
+	CR_HDR_IPC_MSG,
+	CR_HDR_IPC_SEM,
+
 	CR_HDR_TAIL = 5001
 };
 
+#define CR_TST_OVERFLOW_16(a, b) \
+	((sizeof(a) > sizeof(b)) && ((a) > SHORT_MAX))
+
+#define CR_TST_OVERFLOW_32(a, b) \
+	((sizeof(a) > sizeof(b)) && ((a) > INT_MAX))
+
+#define CR_TST_OVERFLOW_64(a, b) \
+	((sizeof(a) > sizeof(b)) && ((a) > LONG_MAX))
+
 struct cr_hdr_head {
 	__u64 magic;
 
@@ -184,4 +198,22 @@ struct cr_hdr_fd_pipe {
 	__s32 nr_bufs;
 } __attribute__((aligned(8)));
 
+/* ipc commons */
+struct cr_hdr_ipc {
+	__u32 ipc_type;
+	__u32 ipc_count;
+} __attribute__((aligned(8)));
+
+struct cr_hdr_ipc_perms {
+	__s32 id;
+	__u32 key;
+	__u32 uid;
+	__u32 gid;
+	__u32 cuid;
+	__u32 cgid;
+	__u32 mode;
+	__u32 _padding;
+	__u64 seq;
+} __attribute__((aligned(8)));
+
 #endif /* _CHECKPOINT_CKPT_HDR_H_ */
-- 
1.5.4.3

[RFC v14-rc2][PATCH 3/7] sysvipc-shm: checkpoint

[Oren Laadan]

Checkpoint of sysvipc shared memory is performed in two steps: first,
the entire ipc namespace is dumped as a whole by iterating through all
shm objects and dumping the contents of each one. The shmem inode is
registered in the objhash. Second, for each vma that refers to ipc
shared memory we find the inode in the objhash, and save the objref.

(If we find a new inode, that indicates that the ipc namespace is not
entirely frozen and someone must have manipulated it since step 1).

Handling of shm objects that have been deleted (via IPC_RMID) is left
to a later patch in this series.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 checkpoint/ckpt_mem.c          |    9 +++
 checkpoint/util_ipc.c          |    3 +-
 include/linux/checkpoint.h     |    1 +
 include/linux/checkpoint_hdr.h |   15 ++++
 ipc/Makefile                   |    1 +
 ipc/ckpt_shm.c                 |  138 ++++++++++++++++++++++++++++++++++++++++
 ipc/shm.c                      |   11 +++
 7 files changed, 176 insertions(+), 2 deletions(-)
 create mode 100644 ipc/ckpt_shm.c

diff --git a/checkpoint/ckpt_mem.c b/checkpoint/ckpt_mem.c
index 9315d1b..41c0bdd 100644
--- a/checkpoint/ckpt_mem.c
+++ b/checkpoint/ckpt_mem.c
@@ -558,7 +558,16 @@ static int cr_write_shared_vma_contents(struct cr_ctx *ctx,
 		inode = vma->vm_file->f_dentry->d_inode;
 		ret = cr_write_shmem_contents(ctx, inode);
 		break;
+	case CR_VMA_SHM_IPC:
+		/*
+		 * This doesn't happen, because all IPC regions should have
+		 * been already dumped by now via ipc namespaces; It means
+		 * the ipc_ns has been modified recently during checkpoint.
+		 */
+		ret = -EBUSY;
+		break;
 	case CR_VMA_SHM_ANON_SKIP:
+	case CR_VMA_SHM_IPC_SKIP:
 	case CR_VMA_SHM_FILE_SKIP:
 		/* already saved before .. skip now */
 		break;
diff --git a/checkpoint/util_ipc.c b/checkpoint/util_ipc.c
index 70c4b18..a648579 100644
--- a/checkpoint/util_ipc.c
+++ b/checkpoint/util_ipc.c
@@ -10,13 +10,12 @@
 
 #ifdef CONFIG_SYSVIPC
 
-#include <linux/version.h>
 #include <linux/checkpoint.h>
 #include <linux/checkpoint_hdr.h>
 
 int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy)
 {
-	return 0;
+	return cr_write_ipc_shm(ctx, nsproxy->ipc_ns);
 }
 
 int cr_read_ipc(struct cr_ctx *ctx)
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index 15a9890..8637537 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -129,6 +129,7 @@ extern void cr_fill_ipc_perms(struct cr_hdr_ipc_perms *hh,
 			      struct kern_ipc_perm *perm);
 extern int cr_load_ipc_perms(struct cr_hdr_ipc_perms *hh,
 			     struct kern_ipc_perm *perm);
+extern int cr_write_ipc_shm(struct cr_ctx *ctx, struct ipc_namespace *ipcns);
 #endif
 
 
diff --git a/include/linux/checkpoint_hdr.h b/include/linux/checkpoint_hdr.h
index 6efe8dd..ceb462e 100644
--- a/include/linux/checkpoint_hdr.h
+++ b/include/linux/checkpoint_hdr.h
@@ -138,6 +138,8 @@ enum cr_vma_type {
 	CR_VMA_FILE,		/* private mapped file */
 	CR_VMA_SHM_ANON,	/* shared anonymous */
 	CR_VMA_SHM_ANON_SKIP,	/* shared anonymous, skip contents */
+	CR_VMA_SHM_IPC,		/* shared sysvipc */
+	CR_VMA_SHM_IPC_SKIP,	/* shared sysvipc, skip contents */
 	CR_VMA_SHM_FILE,	/* shared mapped file, only msync */
 	CR_VMA_SHM_FILE_SKIP,	/* shared mapped file, skip msync */
 };
@@ -216,4 +218,17 @@ struct cr_hdr_ipc_perms {
 	__u64 seq;
 } __attribute__((aligned(8)));
 
+struct cr_hdr_ipc_shm {
+	struct cr_hdr_ipc_perms perms;
+	__u64 shm_segsz;
+	__u64 shm_atim;
+	__u64 shm_dtim;
+	__u64 shm_ctim;
+	__s32 shm_cprid;
+	__s32 shm_lprid;
+	__u32 mlock_uid;
+	__u32 flags;
+	__u32 objref;
+} __attribute__((aligned(8)));
+
 #endif /* _CHECKPOINT_CKPT_HDR_H_ */
diff --git a/ipc/Makefile b/ipc/Makefile
index 65c3843..0789ec8 100644
--- a/ipc/Makefile
+++ b/ipc/Makefile
@@ -8,4 +8,5 @@ obj-$(CONFIG_SYSVIPC_SYSCTL) += ipc_sysctl.o
 obj_mq-$(CONFIG_COMPAT) += compat_mq.o
 obj-$(CONFIG_POSIX_MQUEUE) += mqueue.o msgutil.o $(obj_mq-y)
 obj-$(CONFIG_IPC_NS) += namespace.o
+obj-$(CONFIG_CHECKPOINT) += ckpt_shm.o
 
diff --git a/ipc/ckpt_shm.c b/ipc/ckpt_shm.c
new file mode 100644
index 0000000..6a9382a
--- /dev/null
+++ b/ipc/ckpt_shm.c
@@ -0,0 +1,138 @@
+/*
+ *  Checkpoint/restart - dump state of sysvipc shm
+ *
+ *  Copyright (C) 2009 Oren Laadan
+ *
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License.  See the file COPYING in the main directory of the Linux
+ *  distribution for more details.
+ */
+
+#include <linux/mm.h>
+#include <linux/shm.h>
+#include <linux/shmem_fs.h>
+#include <linux/hugetlb.h>
+#include <linux/rwsem.h>
+#include <linux/sched.h>
+#include <linux/syscalls.h>
+#include <linux/nsproxy.h>
+#include <linux/ipc_namespace.h>
+
+#include <linux/msg.h>	/* needed for util.h that uses 'struct msg_msg' */
+#include "util.h"
+
+#include <linux/checkpoint.h>
+#include <linux/checkpoint_hdr.h>
+
+/************************************************************************
+ * ipc checkpoint
+ */
+
+static int cr_fill_ipc_shm_hdr(struct cr_ctx *ctx,
+			       struct cr_hdr_ipc_shm *hh,
+			       struct shmid_kernel *shp)
+{
+	int ret = 0;
+
+	ipc_lock_by_ptr(&shp->shm_perm);
+
+	cr_fill_ipc_perms(&hh->perms, &shp->shm_perm);
+
+	hh->shm_segsz = shp->shm_segsz;
+	hh->shm_atim = shp->shm_atim;
+	hh->shm_dtim = shp->shm_dtim;
+	hh->shm_ctim = shp->shm_ctim;
+	hh->shm_cprid = shp->shm_cprid;
+	hh->shm_lprid = shp->shm_lprid;
+
+	hh->mlock_uid = (shp->mlock_user ? shp->mlock_user->uid : UINT_MAX);
+
+	hh->flags = 0;
+	/* check if shm was setup with SHM_NORESERVE */
+	if (SHMEM_I(shp->shm_file->f_dentry->d_inode)->flags & VM_NORESERVE)
+		hh->flags |= SHM_NORESERVE;
+	/* check if shm was setup with SHM_HUGETLB (unsupported yet) */
+	if (is_file_hugepages(shp->shm_file)) {
+		pr_warning("c/r: unsupported SHM_HUGETLB\n");
+		ret = -ENOSYS;
+	}
+
+	ipc_unlock(&shp->shm_perm);
+	cr_debug("shm: cprid %d lprid %d segsz %lld mlock %d\n",
+		 hh->shm_cprid, hh->shm_lprid, hh->shm_segsz, hh->mlock_uid);
+
+	return ret;
+}
+
+static int cr_do_write_ipc_shm(int id, void *p, void *data)
+{
+	struct cr_hdr h;
+	struct cr_hdr_ipc_shm *hh;
+	struct cr_ctx *ctx = (struct cr_ctx *) data;
+	struct kern_ipc_perm *perm = (struct kern_ipc_perm *) p;
+	struct shmid_kernel *shp;
+	struct inode *inode;
+	int ret;
+
+	shp = container_of(perm, struct shmid_kernel, shm_perm);
+	inode = shp->shm_file->f_dentry->d_inode;
+
+	h.type = CR_HDR_IPC_SHM;
+	h.len = sizeof(*hh);
+
+	hh = cr_hbuf_get(ctx, sizeof(*hh));
+	if (!hh)
+		return -ENOMEM;
+
+	ret = cr_fill_ipc_shm_hdr(ctx, hh, shp);
+	if (ret < 0)
+		goto out;
+
+	ret = cr_obj_add_ptr(ctx, inode, &hh->objref, CR_OBJ_INODE, 0);
+	if (ret < 0)
+		goto out;
+	BUG_ON(ret != 1);	/* must be first time always */
+
+	cr_debug("shm: objref %d\n", hh->objref);
+	ret = cr_write_obj(ctx, &h, hh);
+	if (ret < 0)
+		goto out;
+
+	ret = cr_write_shmem_contents(ctx, inode);
+ out:
+	cr_hbuf_put(ctx, sizeof(*hh));
+	return ret;
+}
+
+int cr_write_ipc_shm(struct cr_ctx *ctx, struct ipc_namespace *ipcns)
+{
+	struct cr_hdr h;
+	struct cr_hdr_ipc *hh;
+	struct ipc_ids *shm_ids = &ipcns->ids[IPC_SHM_IDS];
+	int ret = -ENOMEM;
+
+	down_read(&shm_ids->rw_mutex);
+
+	h.type = CR_HDR_IPC;
+	h.len = sizeof(*hh);
+
+	hh = cr_hbuf_get(ctx, sizeof(*hh));
+	if (!hh)
+		goto out;
+
+	hh->ipc_type = CR_HDR_IPC_SHM;
+	hh->ipc_count = shm_ids->in_use;
+	cr_debug("shm: count %d\n", hh->ipc_count);
+
+	ret = cr_write_obj(ctx, &h, hh);
+	cr_hbuf_put(ctx, sizeof(*hh));
+	if (ret < 0)
+		goto out;
+
+	ret = idr_for_each(&shm_ids->ipcs_idr, cr_do_write_ipc_shm, ctx);
+	cr_debug("shm: ret %d\n", ret);
+
+ out:
+	up_read(&shm_ids->rw_mutex);
+	return ret;
+}
diff --git a/ipc/shm.c b/ipc/shm.c
index 4135f28..5ac6aec 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -39,6 +39,7 @@
 #include <linux/nsproxy.h>
 #include <linux/mount.h>
 #include <linux/ipc_namespace.h>
+#include <linux/checkpoint_hdr.h>
 
 #include <asm/uaccess.h>
 
@@ -244,6 +245,13 @@ static struct mempolicy *shm_get_policy(struct vm_area_struct *vma,
 }
 #endif
 
+#ifdef CONFIG_CHECKPOINT
+static int shm_cr_vma_type(struct vm_area_struct *vma)
+{
+	return CR_VMA_SHM_IPC;
+}
+#endif
+
 static int shm_mmap(struct file * file, struct vm_area_struct * vma)
 {
 	struct shm_file_data *sfd = shm_file_data(file);
@@ -319,6 +327,9 @@ static struct vm_operations_struct shm_vm_ops = {
 	.set_policy = shm_set_policy,
 	.get_policy = shm_get_policy,
 #endif
+#if defined(CONFIG_CHECKPOINT)
+	.cr_vma_type = shm_cr_vma_type,
+#endif
 };
 
 /**
-- 
1.5.4.3

Re: [RFC v14-rc2][PATCH 3/7] sysvipc-shm: checkpoint

[Serge E. Hallyn]

Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
...
>  int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy)
>  {
> -	return 0;
> +	return cr_write_ipc_shm(ctx, nsproxy->ipc_ns);
>  }

What is the plan for implementing support for nested ipc namespaces,
since all ipc data is currently stored as a single global checkpoint
property before the task data?

Are you figuring you'll just do cr_write_ipc_shm() for all nsproxies
which are part of the checkpoint image, and store an ipc_ns id
or nsproxy id alongside it, so that at restart you just recreate all
the ipc namespaces ahead of time, then attach the tasks to their
target namespaces as they are created?

Just wondering...  as it stands the code looks more like it *wants*
to be targeted at only one ipcns per checkpoint image.

-serge

Re: [RFC v14-rc2][PATCH 3/7] sysvipc-shm: checkpoint

[Oren Laadan]

Serge E. Hallyn wrote:
> Quoting Oren Laadan (orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org):
> ...
>>  int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy)
>>  {
>> -	return 0;
>> +	return cr_write_ipc_shm(ctx, nsproxy->ipc_ns);
>>  }
> 
> What is the plan for implementing support for nested ipc namespaces,
> since all ipc data is currently stored as a single global checkpoint
> property before the task data?
> 
> Are you figuring you'll just do cr_write_ipc_shm() for all nsproxies
> which are part of the checkpoint image, and store an ipc_ns id
> or nsproxy id alongside it, so that at restart you just recreate all
> the ipc namespaces ahead of time, then attach the tasks to their
> target namespaces as they are created?
> 
> Just wondering...  as it stands the code looks more like it *wants*
> to be targeted at only one ipcns per checkpoint image.

Good question (i.e. this deserves a commentin the patch description...)

The plan is to call cr_write_ipc() for each ipc namespace as soon as
we discover it, and deposit in the objhash. The second encounter of the
same namespace will simply skip.

This fits well with restart, because we will encounter the new objref
in in teh context of the right task, i.e. one that has that namespace
in its nsproxy, and all operations (e.g. shmget) will take place in
the correct namespace.

So, yes, this will move from the 'global' section into a per-task
'nsproxy-contents' section.

Oren.

[RFC v14-rc2][PATCH 4/7] sysvipc-shm: restart

[Oren Laadan]

Like chekcpoint, restart of sysvipc shared memory is also performed in
two steps: first, the entire ipc namespace is restored as a whole, by
restoring each shm object read from the checkpoint image. The shmem's
file pointer is registered in the objhash. Second, for each vma that
refers to ipc shared memory, we use the objref to find the file in the
objhash, and use that file in calling do_mmap_pgoff().

Handling of shm objects that have been deleted (via IPC_RMID) is left
to a later patch in this series.

Handling of ipc shm mappings that are locked (via SHM_MLOCK) is also
not restored at the moment.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 checkpoint/rstr_mem.c      |   23 ++++++
 checkpoint/util_ipc.c      |    2 +-
 include/linux/checkpoint.h |    3 +
 ipc/ckpt_shm.c             |  167 +++++++++++++++++++++++++++++++++++++++++++-
 4 files changed, 193 insertions(+), 2 deletions(-)

diff --git a/checkpoint/rstr_mem.c b/checkpoint/rstr_mem.c
index 414d6a9..a70f02a 100644
--- a/checkpoint/rstr_mem.c
+++ b/checkpoint/rstr_mem.c
@@ -342,6 +342,24 @@ static struct file *cr_vma_prep_file(struct cr_ctx *ctx, struct cr_hdr_vma *hh)
 		if (!IS_ERR(file))
 			get_file(file);
 		break;
+#ifdef CONFIG_SYSVIPC
+	case CR_VMA_SHM_IPC_SKIP:	/* shared sysvipc mapping skipped */
+		if (!hh->shm_objref || hh->vma_objref)
+			break;
+		file = cr_obj_get_by_ref(ctx, hh->shm_objref, CR_OBJ_FILE);
+		if (!file)
+			file = ERR_PTR(-EINVAL);
+		if (!IS_ERR(file)) {
+			ret = cr_ipc_shm_attach(file,
+						hh->vm_start,
+						hh->vm_flags);
+			if (ret < 0)
+				file = ERR_PTR(ret);
+		}
+		if (!IS_ERR(file))
+			get_file(file);
+		break;
+#endif
 	case CR_VMA_SHM_FILE:		/* shared mapping of a file */
 		if (!hh->shm_objref || !hh->vma_objref)
 			break;
@@ -429,6 +447,10 @@ static int cr_read_vma(struct cr_ctx *ctx, struct mm_struct *mm)
 		goto out;
 	}
 
+	/* yuck: sysvipc shm are already mapped, so skip this */
+	if (vma_type == CR_VMA_SHM_IPC_SKIP)
+		goto contents;
+
 	/* create a new vma */
 	down_write(&mm->mmap_sem);
 	addr = do_mmap_pgoff(file, vm_start, vm_size,
@@ -442,6 +464,7 @@ static int cr_read_vma(struct cr_ctx *ctx, struct mm_struct *mm)
 		goto out;
 	}
 
+ contents:
 	/* read in the contents of this vma */
 	if (shm)
 		ret = cr_read_shared_vma_contents(ctx, file, vma_type);
diff --git a/checkpoint/util_ipc.c b/checkpoint/util_ipc.c
index a648579..adbb639 100644
--- a/checkpoint/util_ipc.c
+++ b/checkpoint/util_ipc.c
@@ -20,7 +20,7 @@ int cr_write_ipc(struct cr_ctx *ctx, struct nsproxy *nsproxy)
 
 int cr_read_ipc(struct cr_ctx *ctx)
 {
-	return 0;
+	return cr_read_ipc_shm(ctx);
 }
 
 void cr_fill_ipc_perms(struct cr_hdr_ipc_perms *hh, struct kern_ipc_perm *perm)
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index 8637537..c556511 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -130,6 +130,9 @@ extern void cr_fill_ipc_perms(struct cr_hdr_ipc_perms *hh,
 extern int cr_load_ipc_perms(struct cr_hdr_ipc_perms *hh,
 			     struct kern_ipc_perm *perm);
 extern int cr_write_ipc_shm(struct cr_ctx *ctx, struct ipc_namespace *ipcns);
+extern int cr_read_ipc_shm(struct cr_ctx *ctx);
+extern int cr_ipc_shm_attach(struct file *file,
+			     unsigned long addr, unsigned long flags);
 #endif
 
 
diff --git a/ipc/ckpt_shm.c b/ipc/ckpt_shm.c
index 6a9382a..ee9b77a 100644
--- a/ipc/ckpt_shm.c
+++ b/ipc/ckpt_shm.c
@@ -14,6 +14,7 @@
 #include <linux/hugetlb.h>
 #include <linux/rwsem.h>
 #include <linux/sched.h>
+#include <linux/file.h>
 #include <linux/syscalls.h>
 #include <linux/nsproxy.h>
 #include <linux/ipc_namespace.h>
@@ -45,7 +46,10 @@ static int cr_fill_ipc_shm_hdr(struct cr_ctx *ctx,
 	hh->shm_cprid = shp->shm_cprid;
 	hh->shm_lprid = shp->shm_lprid;
 
-	hh->mlock_uid = (shp->mlock_user ? shp->mlock_user->uid : UINT_MAX);
+	if (shp->mlock_user)
+		hh->mlock_uid = shp->mlock_user->uid;
+	else
+		hh->mlock_uid = (unsigned int) -1;
 
 	hh->flags = 0;
 	/* check if shm was setup with SHM_NORESERVE */
@@ -136,3 +140,164 @@ int cr_write_ipc_shm(struct cr_ctx *ctx, struct ipc_namespace *ipcns)
 	up_read(&shm_ids->rw_mutex);
 	return ret;
 }
+
+/************************************************************************
+ * ipc restart
+ */
+
+int cr_ipc_shm_attach(struct file *file,
+		      unsigned long vm_addr,
+		      unsigned long vm_flags)
+{
+	mm_segment_t old_fs;
+	unsigned long addr;
+	int shmid, shmflg = 0;
+	int ret;
+
+	shmid = file->f_dentry->d_inode->i_ino;
+
+	if (!(vm_flags & VM_WRITE))
+		shmflg |= SHM_RDONLY;
+
+	old_fs = get_fs();
+	set_fs(get_ds());
+	ret = do_shmat(shmid, (char __user *) vm_addr, shmflg, &addr);
+	set_fs(old_fs);
+
+	BUG_ON(ret >= 0 && addr != vm_addr);
+	return ret;
+}
+
+static int cr_load_ipc_shm_hdr(struct cr_ctx *ctx,
+			       struct cr_hdr_ipc_shm *hh,
+			       struct shmid_kernel *shp)
+{
+	int ret;
+
+	ret = cr_load_ipc_perms(&hh->perms, &shp->shm_perm);
+	if (ret < 0)
+		return ret;
+
+	cr_debug("shm: cprid %d lprid %d segsz %lld mlock %d\n",
+		 hh->shm_cprid, hh->shm_lprid, hh->shm_segsz, hh->mlock_uid);
+
+	if (hh->shm_cprid < 0 || hh->shm_lprid < 0)
+		return -EINVAL;
+
+	shp->shm_segsz = hh->shm_segsz;
+	shp->shm_atim = hh->shm_atim;
+	shp->shm_dtim = hh->shm_dtim;
+	shp->shm_ctim = hh->shm_ctim;
+	shp->shm_cprid = hh->shm_cprid;
+	shp->shm_lprid = hh->shm_lprid;
+
+	return 0;
+}
+
+static int cr_do_read_ipc_shm(struct cr_ctx *ctx)
+{
+	struct cr_hdr_ipc_shm *hh;
+	struct kern_ipc_perm *perms;
+	struct shmid_kernel *shp;
+	struct ipc_ids *shm_ids = &current->nsproxy->ipc_ns->ids[IPC_SHM_IDS];
+	struct file *file;
+	int shmflag;
+	int ret;
+
+	hh = cr_hbuf_get(ctx, sizeof(*hh));
+	if (!hh)
+		return -ENOMEM;
+	ret = cr_read_obj_type(ctx, hh, sizeof(*hh), CR_HDR_IPC_SHM);
+	if (ret < 0)
+		goto out;
+	ret = -EINVAL;
+	if (hh->perms.id < 0)
+		goto out;
+
+#define CR_SHMFL_MASK  (SHM_NORESERVE | SHM_HUGETLB)
+	if (hh->flags & ~CR_SHMFL_MASK)
+		goto out;
+
+	ret = -ENOSYS;
+	if (hh->mlock_uid != (unsigned int) -1)	/* FIXME: support SHM_LOCK */
+		goto out;
+	if (hh->flags & SHM_HUGETLB)	/* FIXME: support SHM_HUGETLB */
+		goto out;
+
+	/* FIXME: this will fail for deleted ipc shm segments */
+
+	shmflag = hh->flags | hh->perms.mode | IPC_CREAT | IPC_EXCL;
+	cr_debug("shm: do_shmget size %lld flag %#x id %d\n",
+		 hh->shm_segsz, shmflag, hh->perms.id);
+	ret = do_shmget(hh->perms.key, hh->shm_segsz, shmflag, hh->perms.id);
+	cr_debug("shm: do_shmget ret %d\n", ret);
+	if (ret < 0)
+		goto out;
+
+	down_write(&shm_ids->rw_mutex);
+
+	ret = -EIDRM;
+	perms = ipc_lock(shm_ids, hh->perms.id);
+	if (IS_ERR(perms)) {	/* this should not happen .. but be safe */
+		up_write(&shm_ids->rw_mutex);
+		ret = PTR_ERR(perms);
+		goto out;
+	}
+
+	shp = container_of(perms, struct shmid_kernel, shm_perm);
+	ret = cr_load_ipc_shm_hdr(ctx, hh, shp);
+	if (ret < 0) {
+		cr_debug("shm: need to remove (%d)\n", ret);
+		do_shm_rmid(current->nsproxy->ipc_ns, perms);
+		up_write(&shm_ids->rw_mutex);
+		goto out;
+	}
+
+	file = shp->shm_file;
+	get_file(file);
+	ipc_unlock(perms);
+	up_write(&shm_ids->rw_mutex);
+
+	/* deposit in objhash and read contents in */
+	ret = cr_obj_add_ref(ctx, file, hh->objref, CR_OBJ_FILE, 0);
+	if (ret < 0)
+		goto file;
+	ret = cr_read_shmem_contents(ctx, file->f_dentry->d_inode);
+ file:
+	fput(file);
+ out:
+	cr_hbuf_put(ctx, sizeof(*hh));
+	return ret;
+}
+
+int cr_read_ipc_shm(struct cr_ctx *ctx)
+{
+	struct cr_hdr_ipc *hh;
+	int n, ret;
+
+	hh = cr_hbuf_get(ctx, sizeof(*hh));
+	if (!hh)
+		return -ENOMEM;
+
+	ret = cr_read_obj_type(ctx, hh, sizeof(*hh), CR_HDR_IPC);
+	if (ret < 0)
+		goto out;
+
+	cr_debug("shm: count %d\n", hh->ipc_count);
+
+	ret = -EINVAL;
+	if (hh->ipc_type != CR_HDR_IPC_SHM)
+		goto out;
+
+	ret = 0;
+	for (n = 0; n < hh->ipc_count; n++) {
+		ret = cr_do_read_ipc_shm(ctx);
+		if (ret < 0)
+			goto out;
+	}
+
+ out:
+	cr_debug("shm: ret %d\n", ret);
+	cr_hbuf_put(ctx, sizeof(*hh));
+	return ret;
+}
-- 
1.5.4.3

Re: [RFC v14-rc2][PATCH 4/7] sysvipc-shm: restart

[Matt Helsley]

On Tue, Mar 31, 2009 at 01:32:29AM -0400, Oren Laadan wrote:
> Like chekcpoint, restart of sysvipc shared memory is also performed in
> two steps: first, the entire ipc namespace is restored as a whole, by
> restoring each shm object read from the checkpoint image. The shmem's
> file pointer is registered in the objhash. Second, for each vma that
> refers to ipc shared memory, we use the objref to find the file in the
> objhash, and use that file in calling do_mmap_pgoff().
> 
> Handling of shm objects that have been deleted (via IPC_RMID) is left
> to a later patch in this series.
> 
> Handling of ipc shm mappings that are locked (via SHM_MLOCK) is also
> not restored at the moment.
> 
> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
> ---
>  checkpoint/rstr_mem.c      |   23 ++++++
>  checkpoint/util_ipc.c      |    2 +-
>  include/linux/checkpoint.h |    3 +
>  ipc/ckpt_shm.c             |  167 +++++++++++++++++++++++++++++++++++++++++++-
>  4 files changed, 193 insertions(+), 2 deletions(-)

<snip>
 
> diff --git a/ipc/ckpt_shm.c b/ipc/ckpt_shm.c
> index 6a9382a..ee9b77a 100644
> --- a/ipc/ckpt_shm.c
> +++ b/ipc/ckpt_shm.c
> @@ -14,6 +14,7 @@
>  #include <linux/hugetlb.h>
>  #include <linux/rwsem.h>
>  #include <linux/sched.h>
> +#include <linux/file.h>
>  #include <linux/syscalls.h>
>  #include <linux/nsproxy.h>
>  #include <linux/ipc_namespace.h>
> @@ -45,7 +46,10 @@ static int cr_fill_ipc_shm_hdr(struct cr_ctx *ctx,
>  	hh->shm_cprid = shp->shm_cprid;
>  	hh->shm_lprid = shp->shm_lprid;
> 
> -	hh->mlock_uid = (shp->mlock_user ? shp->mlock_user->uid : UINT_MAX);
> +	if (shp->mlock_user)
> +		hh->mlock_uid = shp->mlock_user->uid;
> +	else
> +		hh->mlock_uid = (unsigned int) -1;

I think this hunk should be merged with the patch introducing the conditional
expression rather than appear in this patch.

Cheers,
	-Matt Helsley

[RFC v14-rc2][PATCH 5/7] Infrastructure for work postponed to the end of checkpoint/restart

[Oren Laadan]

Add a interface to postpone an action until the end of the entire
checkpoint or restart operation. This is useful when during the
scan of tasks an operation cannot be performed in place, to avoid
the need for a second scan.

One use case is the next patch: when restoring an ipc shared memory
region that has been deleted (but is still attached), during restart
it needs to be create, attached and then deleted. However, creation
and attachment are performed in distinct locations, so deletion can
not be performed on the spot. Instead, this work (delete) is deferred
until later.

The interface is as follows:

cr_workqueue_run(ctx):
  Execute all the pending works in the queue. Returns the number of
  works executed, or an error.

cr_workqueue_add(ctx, function, flags, data, size):
  Enqueue a postponed work. @function is the function to do the work,
  which will be called with @data as an argument. @size tells the
  size of data. @flags is unused at the moment.

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 checkpoint/Makefile        |    3 +-
 checkpoint/checkpoint.c    |    4 +++
 checkpoint/restart.c       |    3 ++
 checkpoint/sys.c           |    7 +++++
 checkpoint/workqueue.c     |   62 ++++++++++++++++++++++++++++++++++++++++++++
 include/linux/checkpoint.h |    9 ++++++
 6 files changed, 87 insertions(+), 1 deletions(-)
 create mode 100644 checkpoint/workqueue.c

diff --git a/checkpoint/Makefile b/checkpoint/Makefile
index 71061de..d14f156 100644
--- a/checkpoint/Makefile
+++ b/checkpoint/Makefile
@@ -2,6 +2,7 @@
 # Makefile for linux checkpoint/restart.
 #
 
-obj-$(CONFIG_CHECKPOINT) += sys.o checkpoint.o restart.o objhash.o \
+obj-$(CONFIG_CHECKPOINT) += sys.o checkpoint.o restart.o \
+	objhash.o workqueue.o \
 	ckpt_mem.o rstr_mem.o ckpt_file.o rstr_file.o \
 	util_ipc.o
diff --git a/checkpoint/checkpoint.c b/checkpoint/checkpoint.c
index 2ebaa05..ffb5d9c 100644
--- a/checkpoint/checkpoint.c
+++ b/checkpoint/checkpoint.c
@@ -584,6 +584,10 @@ int do_checkpoint(struct cr_ctx *ctx, pid_t pid)
 	if (ret < 0)
 		goto out;
 
+	ret = cr_workqueue_run(ctx);
+	if (ret < 0)
+		goto out;
+
 	ctx->crid = atomic_inc_return(&cr_ctx_count);
 
 	/* on success, return (unique) checkpoint identifier */
diff --git a/checkpoint/restart.c b/checkpoint/restart.c
index ed5c725..44f255d 100644
--- a/checkpoint/restart.c
+++ b/checkpoint/restart.c
@@ -530,7 +530,10 @@ static int do_restart_root(struct cr_ctx *ctx, pid_t pid)
 		goto out;
 
 	ret = cr_read_tail(ctx);
+	if (ret < 0)
+		goto out;
 
+	ret = cr_workqueue_run(ctx);
  out:
 	return ret;
 }
diff --git a/checkpoint/sys.c b/checkpoint/sys.c
index 3a925ae..36486cc 100644
--- a/checkpoint/sys.c
+++ b/checkpoint/sys.c
@@ -167,8 +167,14 @@ static void cr_task_arr_free(struct cr_ctx *ctx)
 
 static void cr_ctx_free(struct cr_ctx *ctx)
 {
+	int ret;
+
 	BUG_ON(atomic_read(&ctx->refcount));
 
+	ret = cr_workqueue_run(ctx);
+	if (ret != 0)
+		cr_debug("deferred workqueue had %d entries", ret);
+
 	if (ctx->file)
 		fput(ctx->file);
 
@@ -206,6 +212,7 @@ static struct cr_ctx *cr_ctx_alloc(int fd, unsigned long flags)
 	atomic_set(&ctx->refcount, 0);
 	INIT_LIST_HEAD(&ctx->pgarr_list);
 	INIT_LIST_HEAD(&ctx->pgarr_pool);
+	INIT_LIST_HEAD(&ctx->workqueue);
 	init_waitqueue_head(&ctx->waitq);
 
 	err = -EBADF;
diff --git a/checkpoint/workqueue.c b/checkpoint/workqueue.c
new file mode 100644
index 0000000..e278202
--- /dev/null
+++ b/checkpoint/workqueue.c
@@ -0,0 +1,62 @@
+/*
+ *  Checkpoint-restart - work queue infrastructure to manage deferred work
+ *
+ *  Copyright (C) 2009 Oren Laadan
+ *
+ *  This file is subject to the terms and conditions of the GNU General Public
+ *  License.  See the file COPYING in the main directory of the Linux
+ *  distribution for more details.
+ */
+
+#include <linux/list.h>
+#include <linux/checkpoint.h>
+
+struct cr_workqueue {
+	cr_workqueue_func_t function;
+	unsigned int flags;
+	struct list_head list;
+	char data[0];
+};
+
+int cr_workqueue_add(struct cr_ctx *ctx, cr_workqueue_func_t function,
+		     unsigned int flags, void *data, int size)
+{
+	struct cr_workqueue *wq;
+
+	wq = kmalloc(sizeof(wq) + size, GFP_KERNEL);
+	if (!wq)
+		return -ENOMEM;
+
+	wq->function = function;
+	wq->flags = flags;
+	memcpy(wq->data, data, size);
+
+	cr_debug("adding work %p function %p\n", wq, wq->function);
+	list_add_tail(&ctx->workqueue, &wq->list);
+	return 0;
+}
+
+/*
+ * cr_workqueue_run - perform all work in the work queue
+ * @ctx: checkpoint context
+ *
+ * returns: number of works performed, or < 0 on error
+ */
+int cr_workqueue_run(struct cr_ctx *ctx)
+{
+	struct cr_workqueue *wq, *n;
+	int nr = 0;
+	int ret;
+
+	list_for_each_entry_safe(wq, n, &ctx->workqueue, list) {
+		cr_debug("doing work %p function %p\n", wq, wq->function);
+		ret = wq->function(wq->data);
+		if (ret < 0)
+			cr_debug("wq function failed %d\n", ret);
+		list_del(&wq->list);
+		kfree(wq);
+		nr++;
+	}
+
+	return nr;
+}
diff --git a/include/linux/checkpoint.h b/include/linux/checkpoint.h
index c556511..ba218b6 100644
--- a/include/linux/checkpoint.h
+++ b/include/linux/checkpoint.h
@@ -38,6 +38,7 @@ struct cr_ctx {
 	atomic_t refcount;
 
 	struct cr_objhash *objhash;	/* hash for shared objects */
+	struct list_head workqueue;	/* list of deferred tasks */
 
 	struct list_head pgarr_list;	/* page array to dump VMA contents */
 	struct list_head pgarr_pool;	/* pool of empty page arrays chain */
@@ -71,6 +72,14 @@ extern void cr_hbuf_put(struct cr_ctx *ctx, int n);
 extern void cr_ctx_get(struct cr_ctx *ctx);
 extern void cr_ctx_put(struct cr_ctx *ctx);
 
+/* deferred tasks */
+
+typedef int (*cr_workqueue_func_t)(void *);
+
+extern int cr_workqueue_run(struct cr_ctx *ctx);
+extern int cr_workqueue_add(struct cr_ctx *ctx, cr_workqueue_func_t func,
+			    unsigned int flags, void *data, int size);
+
 /* shared objects handling */
 
 enum {
-- 
1.5.4.3

Re: [RFC v14-rc2][PATCH 5/7] Infrastructure for work postponed to the end of checkpoint/restart

[Dave Hansen]

On Tue, 2009-03-31 at 01:32 -0400, Oren Laadan wrote:
> 
> Add a interface to postpone an action until the end of the entire
> checkpoint or restart operation. This is useful when during the
> scan of tasks an operation cannot be performed in place, to avoid
> the need for a second scan.

Why aren't we using the existing kernel workqueue mechanism?

-- Dave

Re: [RFC v14-rc2][PATCH 5/7] Infrastructure for work postponed to the end of checkpoint/restart

[Oren Laadan]

Dave Hansen wrote:
> On Tue, 2009-03-31 at 01:32 -0400, Oren Laadan wrote:
>> Add a interface to postpone an action until the end of the entire
>> checkpoint or restart operation. This is useful when during the
>> scan of tasks an operation cannot be performed in place, to avoid
>> the need for a second scan.
> 
> Why aren't we using the existing kernel workqueue mechanism?

Because we need to defer to work until the end of the operation:
not earlier, because it we defer it for a reason; not later, because
we will block waiting for it.

The kernel's workqueue schedules the work for 'some time later'.
It may be in particular too early. Although unlikely, it can also
occur arbitrarily later, so finishing and cleaning up a checkpoint
or a restart will have to block on it.

Also, the kernel workqueue cannot make any assumptions about the task
context in which the work is performed. The restart many times builds
on running in the context of some specific restarting task.

Example: this patch assumes a single (common) ipc namespace, but that
is easy to change. To support more than one, we'll need to perform the
deferred ipc action in the context of the process that has that ipc_ns.
(this means that this mechanism will evolve to per-task.)

If we were to use that workqueue, we would probably need to create a
queue per c/r operation to allow efficient flush; recall that each
workqueue comes with its own thread(s). In general, the mechanism
is too heavy.

What we need is a simple way for the c/r operation as a whole, and
later a task in particular, to defer some action until later _in the
restart_ process (not arbitrarily).

I should have named it cr_deferwork, and wrote this ^^^ in the patch.

Oren.

[RFC v14-rc2][PATCH 6/7] sysvipc-shm: export interface from ipc/shm.c to delete ipc shm

[Oren Laadan]

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 include/linux/shm.h |    4 ++++
 ipc/shm.c           |    4 ++--
 2 files changed, 6 insertions(+), 2 deletions(-)

diff --git a/include/linux/shm.h b/include/linux/shm.h
index eca6235..ec36e99 100644
--- a/include/linux/shm.h
+++ b/include/linux/shm.h
@@ -118,6 +118,10 @@ static inline int is_file_shm_hugepages(struct file *file)
 }
 #endif
 
+struct ipc_namespace;
+extern int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
+		       struct shmid_ds __user *buf, int version);
+
 #endif /* __KERNEL__ */
 
 #endif /* _LINUX_SHM_H_ */
diff --git a/ipc/shm.c b/ipc/shm.c
index 5ac6aec..28a8b57 100644
--- a/ipc/shm.c
+++ b/ipc/shm.c
@@ -605,8 +605,8 @@ static void shm_get_stat(struct ipc_namespace *ns, unsigned long *rss,
  * to be held in write mode.
  * NOTE: no locks must be held, the rw_mutex is taken inside this function.
  */
-static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
-		       struct shmid_ds __user *buf, int version)
+int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
+		struct shmid_ds __user *buf, int version)
 {
 	struct kern_ipc_perm *ipcp;
 	struct shmid64_ds shmid64;
-- 
1.5.4.3

Re: [RFC v14-rc2][PATCH 6/7] sysvipc-shm: export interface from ipc/shm.c to delete ipc shm

[Matt Helsley]

On Tue, Mar 31, 2009 at 01:32:31AM -0400, Oren Laadan wrote:
> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>

The description is empty and this patch is rather pointless without 7/7.
Any reason you kept it separate from 7/7? At the very least this patch
needs a brief description.

Cheers,
	-Matt Helsley

> ---
>  include/linux/shm.h |    4 ++++
>  ipc/shm.c           |    4 ++--
>  2 files changed, 6 insertions(+), 2 deletions(-)
> 
> diff --git a/include/linux/shm.h b/include/linux/shm.h
> index eca6235..ec36e99 100644
> --- a/include/linux/shm.h
> +++ b/include/linux/shm.h
> @@ -118,6 +118,10 @@ static inline int is_file_shm_hugepages(struct file *file)
>  }
>  #endif
> 
> +struct ipc_namespace;
> +extern int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
> +		       struct shmid_ds __user *buf, int version);
> +
>  #endif /* __KERNEL__ */
> 
>  #endif /* _LINUX_SHM_H_ */
> diff --git a/ipc/shm.c b/ipc/shm.c
> index 5ac6aec..28a8b57 100644
> --- a/ipc/shm.c
> +++ b/ipc/shm.c
> @@ -605,8 +605,8 @@ static void shm_get_stat(struct ipc_namespace *ns, unsigned long *rss,
>   * to be held in write mode.
>   * NOTE: no locks must be held, the rw_mutex is taken inside this function.
>   */
> -static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
> -		       struct shmid_ds __user *buf, int version)
> +int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
> +		struct shmid_ds __user *buf, int version)
>  {
>  	struct kern_ipc_perm *ipcp;
>  	struct shmid64_ds shmid64;
> -- 
> 1.5.4.3
> 
> _______________________________________________
> Containers mailing list
> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
> https://lists.linux-foundation.org/mailman/listinfo/containers

Re: [RFC v14-rc2][PATCH 6/7] sysvipc-shm: export interface from ipc/shm.c to delete ipc shm

[Oren Laadan]

Matt Helsley wrote:
> On Tue, Mar 31, 2009 at 01:32:31AM -0400, Oren Laadan wrote:
>> Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
> 
> The description is empty and this patch is rather pointless without 7/7.
> Any reason you kept it separate from 7/7? At the very least this patch
> needs a brief description.

Yup, forgot the description. Will add.

I try to isolate changes to existing kernel files into separate patches
than the new c/r code. I think it will be helpful to be able to point out
those changes upon request.

Oren.

> 
> Cheers,
> 	-Matt Helsley
> 
>> ---
>>  include/linux/shm.h |    4 ++++
>>  ipc/shm.c           |    4 ++--
>>  2 files changed, 6 insertions(+), 2 deletions(-)
>>
>> diff --git a/include/linux/shm.h b/include/linux/shm.h
>> index eca6235..ec36e99 100644
>> --- a/include/linux/shm.h
>> +++ b/include/linux/shm.h
>> @@ -118,6 +118,10 @@ static inline int is_file_shm_hugepages(struct file *file)
>>  }
>>  #endif
>>
>> +struct ipc_namespace;
>> +extern int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
>> +		       struct shmid_ds __user *buf, int version);
>> +
>>  #endif /* __KERNEL__ */
>>
>>  #endif /* _LINUX_SHM_H_ */
>> diff --git a/ipc/shm.c b/ipc/shm.c
>> index 5ac6aec..28a8b57 100644
>> --- a/ipc/shm.c
>> +++ b/ipc/shm.c
>> @@ -605,8 +605,8 @@ static void shm_get_stat(struct ipc_namespace *ns, unsigned long *rss,
>>   * to be held in write mode.
>>   * NOTE: no locks must be held, the rw_mutex is taken inside this function.
>>   */
>> -static int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
>> -		       struct shmid_ds __user *buf, int version)
>> +int shmctl_down(struct ipc_namespace *ns, int shmid, int cmd,
>> +		struct shmid_ds __user *buf, int version)
>>  {
>>  	struct kern_ipc_perm *ipcp;
>>  	struct shmid64_ds shmid64;
>> -- 
>> 1.5.4.3
>>
>> _______________________________________________
>> Containers mailing list
>> Containers-cunTk1MwBs9QetFLy7KEm3xJsTq8ys+cHZ5vskTnxNA@public.gmane.org
>> https://lists.linux-foundation.org/mailman/listinfo/containers
> 

[RFC v14-rc2][PATCH 7/7] sysvipc-shm: correctly handle deleted (active) ipc shared memory

[Oren Laadan]

During restart, an ipc shared region may have SHM_DEST, indicating
that it has been originally deleted (while still active). In this
case the task of deleting the region after restoring it is postponed
until the end of the restart; otherwise, it would be quite silly to
delete it at that time, because it will be ... gone :o

Signed-off-by: Oren Laadan <orenl-eQaUEPhvms7ENvBUuze7eA@public.gmane.org>
---
 ipc/ckpt_shm.c |   44 ++++++++++++++++++++++++++++++++++++++++----
 1 files changed, 40 insertions(+), 4 deletions(-)

diff --git a/ipc/ckpt_shm.c b/ipc/ckpt_shm.c
index ee9b77a..aa539cf 100644
--- a/ipc/ckpt_shm.c
+++ b/ipc/ckpt_shm.c
@@ -145,6 +145,25 @@ int cr_write_ipc_shm(struct cr_ctx *ctx, struct ipc_namespace *ipcns)
  * ipc restart
  */
 
+struct cr_wq_ipcshm_del {
+	struct ipc_namespace *ipcns;
+	int id;
+};
+
+static int cr_ipc_shm_delete(void *data)
+{
+	struct cr_wq_ipcshm_del *wqdata = (struct cr_wq_ipcshm_del *) data;
+	mm_segment_t old_fs;
+	int ret;
+
+	old_fs = get_fs();
+	set_fs(get_ds());
+	ret = shmctl_down(wqdata->ipcns, wqdata->id, IPC_RMID, NULL, 0);
+	set_fs(old_fs);
+
+	return ret;
+}
+
 int cr_ipc_shm_attach(struct file *file,
 		      unsigned long vm_addr,
 		      unsigned long vm_flags)
@@ -224,7 +243,25 @@ static int cr_do_read_ipc_shm(struct cr_ctx *ctx)
 	if (hh->flags & SHM_HUGETLB)	/* FIXME: support SHM_HUGETLB */
 		goto out;
 
-	/* FIXME: this will fail for deleted ipc shm segments */
+	/*
+	 * SHM_DEST means that the shm is to be deleted after creation.
+	 * However, deleting before it's actually attached is quite silly.
+	 * Instead, we defer this task to until restart has succeeded.
+	 */
+	if (hh->perms.mode & SHM_DEST) {
+		struct cr_wq_ipcshm_del wqdata;
+
+		/* to not confuse the rest of the code */
+		hh->perms.mode &= ~SHM_DEST;
+
+		wqdata.ipcns = current->nsproxy->ipc_ns;
+		wqdata.id = hh->perms.id;
+
+		ret = cr_workqueue_add(ctx, cr_ipc_shm_delete, 0,
+				       &wqdata, sizeof(wqdata));
+		if (ret < 0)
+			goto out;
+	}
 
 	shmflag = hh->flags | hh->perms.mode | IPC_CREAT | IPC_EXCL;
 	cr_debug("shm: do_shmget size %lld flag %#x id %d\n",
@@ -235,7 +272,6 @@ static int cr_do_read_ipc_shm(struct cr_ctx *ctx)
 		goto out;
 
 	down_write(&shm_ids->rw_mutex);
-
 	ret = -EIDRM;
 	perms = ipc_lock(shm_ids, hh->perms.id);
 	if (IS_ERR(perms)) {	/* this should not happen .. but be safe */
@@ -261,9 +297,9 @@ static int cr_do_read_ipc_shm(struct cr_ctx *ctx)
 	/* deposit in objhash and read contents in */
 	ret = cr_obj_add_ref(ctx, file, hh->objref, CR_OBJ_FILE, 0);
 	if (ret < 0)
-		goto file;
+		goto fput;
 	ret = cr_read_shmem_contents(ctx, file->f_dentry->d_inode);
- file:
+ fput:
 	fput(file);
  out:
 	cr_hbuf_put(ctx, sizeof(*hh));
-- 
1.5.4.3