IPC: CRIU enhancements fixes and cleanup
cover IPC: CRIU enhancements fixes and cleanup
This patch set simplifies message queue copy feature and clean up it's implementation.
It also adds some debug and fixes an issue, when copy_msg() fails. In this case error have to returned instead of breaking messages loop, because error message pointer is interpreted as -EAGAIN in current implemetation of further message handling.
The following series implements...
---
Stanislav Kinsbursky (4):
ipc: simplify free_copy() call
ipc: convert prepare_copy() from macro to function
ipc: simplify message copying
ipc: add more comments to message copying related code
ipc/msg.c | 55 +++++++++++++++++++++++++++++++++---------------------- ipc/msgutil.c | 5 +++++ 2 files changed, 38 insertions(+), 22 deletions(-)
PATCH 1/4 ipc: simplify free_copy() call
Passing and checking of msgflg to free_copy() is redundant.
This patch sets copy to NULL on declaration instead and checks for non-NULL in free_copy().
Note: in case of copy allocation failure, error is returned immediately. So no need to check for IS_ERR() in free_copy().
--- ipc/msg.c | 14 ++++++++------ 1 files changed, 8 insertions(+), 6 deletions(-)
ipc/msg.c +8 -6
@@ -797,15 +797,17 @@ static inline struct msg_msg *prepare_copy(void __user *buf, size_t bufsz,return copy; } -static inline void free_copy (int msgflg,struct msg_msg *copy) +static inline void free_copy (struct msg_msg *copy) { -if (msgflg &MSG_COPY ) +if (copy)free_msg (copy); }#else -#define free_copy (msgflg, copy) do {} while (0)#define prepare_copy (buf, sz, msgflg, msgtyp, copy_nr) ERR_PTR(-ENOSYS)#define fill_copy (copy_nr, msg_nr, msg, copy) NULL +static inline void free_copy (struct msg_msg *copy) +{ +}#endif long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,
@@ -816,7 +818,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,struct msg_msg *msg;int mode;struct ipc_namespace *ns; -struct msg_msg *copy; +struct msg_msg *copy =NULL ;unsigned long __maybe_unused copy_number;if (msqid <0 || (long ) bufsz <0 )
@@ -831,7 +833,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, msq =msg_lock_check (ns, msqid);if (IS_ERR (msq)) { -free_copy (msgflg, copy); +free_copy (copy);return PTR_ERR (msq); }
@@ -964,7 +966,7 @@ out_unlock: } }if (IS_ERR (msg)) { -free_copy (msgflg, copy); +free_copy (copy);return PTR_ERR (msg); }
PATCH 2/4 ipc: convert prepare_copy() from macro to function
In reply to Stanislav KinsburskyThis code works if CONFIG_CHECKPOINT_RESTORE is disabled.
--- ipc/msg.c | 11 +++++++++-- 1 files changed, 9 insertions(+), 2 deletions(-)
ipc/msg.c +9 -2
@@ -803,8 +803,15 @@ static inline void free_copy(struct msg_msg *copy) free_msg(copy); }#else -#define prepare_copy (buf, sz, msgflg, msgtyp, copy_nr) ERR_PTR(-ENOSYS)#define fill_copy (copy_nr, msg_nr, msg, copy) NULL + +static inline struct msg_msg *prepare_copy (void __user *buf,size_t bufsz, +int msgflg,long *msgtyp, +unsigned long *copy_number) +{ +return ERR_PTR (-ENOSYS ); +} +static inline void free_copy (struct msg_msg *copy) { }
@@ -819,7 +826,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,int mode;struct ipc_namespace *ns;struct msg_msg *copy =NULL ; -unsigned long __maybe_unused copy_number; +unsigned long __maybe_unused copy_number =0 ;if (msqid <0 || (long ) bufsz <0 )return -EINVAL ;
Re: [PATCH 2/4] ipc: convert prepare_copy() from macro to function
Quoted 24 lines · On Wed, 07 Nov 2012 13:05:00 +0300 Stanislav Kinsbursky…
This code works if
CONFIG_CHECKPOINT_RESTOREis disabled....
ipc/msg.c +8
-1@@ -803,8 +803,15 @@ static inline void free_copy(struct msg_msg *copy) free_msg(copy); }#else -#define prepare_copy (buf, sz, msgflg, msgtyp, copy_nr) ERR_PTR(-ENOSYS)#define fill_copy (copy_nr, msg_nr, msg, copy) NULL + +static inline struct msg_msg *prepare_copy (void __user *buf,size_t bufsz, +int msgflg,long *msgtyp, +unsigned long *copy_number) +{ +return ERR_PTR (-ENOSYS ); +} +static inline void free_copy (struct msg_msg *copy) { }
ipc/msg.c long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, · line 829
Quoted 5 lines
ipc/msg.c +1
-1@@ -819,7 +826,7 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp,int mode;struct ipc_namespace *ns;struct msg_msg *copy =NULL ; -unsigned long __maybe_unused copy_number; +unsigned long __maybe_unused copy_number =0 ;
The __maybe_unused here makes no sense. I'll remove it.
Quoted 3 lines
ipc/msg.c +0
-0if (msqid <0 || (long ) bufsz <0 )return -EINVAL ;
PATCH 3/4 ipc: simplify message copying
In reply to Stanislav KinsburskyThis patch removed redundant and confusing fill_copy(). It also adds copy_msg() check for error. In this case exit from the function have to be done instead of break, because further code interprets any error as EAGAIN.
It also defines copy_msg() for the case when CONFIG_CHECKPOINT_RESTORE is disabled.
--- ipc/msg.c | 24 +++++++++--------------- ipc/msgutil.c | 5 +++++ 2 files changed, 14 insertions(+), 15 deletions(-)
ipc/msg.c +9 -15
@@ -770,16 +770,6 @@ static long do_msg_fill(void __user *dest, struct msg_msg *msg, size_t bufsz) }#ifdef CONFIG_CHECKPOINT_RESTORE -static inline struct msg_msg *fill_copy (unsigned long copy_nr, -unsigned long msg_nr, -struct msg_msg *msg, -struct msg_msg *copy) -{ -if (copy_nr == msg_nr) -return copy_msg (msg, copy); -return NULL ; -} -static inline struct msg_msg *prepare_copy (void __user *buf,size_t bufsz,int msgflg,long *msgtyp,unsigned long *copy_number)
@@ -803,8 +793,6 @@ static inline void free_copy(struct msg_msg *copy) free_msg(copy); }#else -#define fill_copy (copy_nr, msg_nr, msg, copy) NULL -static inline struct msg_msg *prepare_copy (void __user *buf,size_t bufsz,int msgflg,long *msgtyp,unsigned long *copy_number)
@@ -868,10 +856,16 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, walk_msg->m_type != 1) { msgtyp = walk_msg->m_type -1 ; } else if (msgflg & MSG_COPY) { - msg =fill_copy (copy_number, msg_counter, - walk_msg, copy); -if (msg) +if (copy_number == msg_counter) { +/* +* Found requested message. +* Copy it. +*/ + msg =copy_msg (msg, copy); +if (IS_ERR (msg)) + goto out_unlock;break ; + } } else break; msg_counter++;
ipc/msgutil.c +5 -0
@@ -138,6 +138,11 @@ struct msg_msg *copy_msg(struct msg_msg *src, struct msg_msg *dst) return dst; } +#else +struct msg_msg *copy_msg (struct msg_msg *src,struct msg_msg *dst) +{ +return ERR_PTR (-ENOSYS ); +}#endif int store_msg (void __user *dest,struct msg_msg *msg,int len) {
PATCH 4/4 ipc: add more comments to message copying related code
In reply to Stanislav Kinsbursky--- ipc/msg.c | 8 ++++++++ 1 files changed, 8 insertions(+), 0 deletions(-)
ipc/msg.c +8 -0
@@ -770,6 +770,10 @@ static long do_msg_fill(void __user *dest, struct msg_msg *msg, size_t bufsz) }#ifdef CONFIG_CHECKPOINT_RESTORE +/* +* This function creates new kernel message structure, large enough to store +* bufsz message bytes. +*/ static inline struct msg_msg *prepare_copy (void __user *buf,size_t bufsz,int msgflg,long *msgtyp,unsigned long *copy_number)
@@ -881,6 +885,10 @@ long do_msgrcv(int msqid, void __user *buf, size_t bufsz, long msgtyp, msg =ERR_PTR (-E2BIG ); goto out_unlock; } +/* +* If we are copying, then do not unlink message and do +* not update queue parameters. +*/ if (msgflg &MSG_COPY ) goto out_unlock;list_del (&msg->m_list);