From: ebiederm@xmission.com (Eric W. Biederman)
To: Manfred Spraul <manfred@colorfullife.com>
Cc: Davidlohr Bueso <dave@stgolabs.net>,
Waiman Long <longman@redhat.com>,
Michael Kerrisk <mtk.manpages@gmail.com>,
"Luis R. Rodriguez" <mcgrof@kernel.org>,
Kees Cook <keescook@chromium.org>,
linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
Andrew Morton <akpm@linux-foundation.org>,
Al Viro <viro@zeniv.linux.org.uk>,
Matthew Wilcox <willy@infradead.org>,
Stanislav Kinsbursky <skinsbursky@parallels.com>,
Linux Containers <containers@lists.linux-foundation.org>,
linux-api@vger.kernel.org
Subject: Re: [RFC][PATCH] ipc: Remove IPCMNI
Date: Thu, 29 Mar 2018 15:08:01 -0500 [thread overview]
Message-ID: <87y3ia3ase.fsf@xmission.com> (raw)
In-Reply-To: <3e201de2-bed2-6f7d-0783-700d095142e0@colorfullife.com> (Manfred Spraul's message of "Thu, 29 Mar 2018 10:47:45 +0200")
Manfred Spraul <manfred@colorfullife.com> writes:
>>>>> On 03/14/2018 08:49 PM, Eric W. Biederman wrote:
>>>>>> To make it possible to keep checkpoint/restore working I have renamed
>>>>>> the sysctls from xxx_next_id to xxx_nextid. That is enough change that
>>>>>> a smart CRIU implementation can see that what is exported has changed,
>>>>>> and act accordingly. New kernels will be able to restore the old id's.
>>>>>>
>>>>>> This code still needs some real world testing to verify my assumptions.
>>>>>> And some work with the CRIU implementations to actually add the code
>>>>>> that deals with the new for of id assignment.
>>>>>>
> It means that all existing checkpoint/restore application will not work with a
> new kernel.
> Everyone must first update the checkpoint/restore application, then update the
> kernel.
>
> Is this acceptable?
There is no nead.
I just reread through how next_id is implementated in ipc/util.c and I
had been reading it wrong. There is no need to change the sysctl.
What criu needs is an interface that specifies the next_id to allocate
both the high and the low bits and that is what this the sysctl
provides.
The implemenation could use a little cleanup so it is easier to
understand something like this perhaps:
diff --git a/ipc/util.c b/ipc/util.c
index 3783b7991cc7..2d4ec6e5b70b 100644
--- a/ipc/util.c
+++ b/ipc/util.c
@@ -192,46 +192,32 @@ static struct kern_ipc_perm *ipc_findkey(struct ipc_ids *ids, key_t key)
return NULL;
}
-#ifdef CONFIG_CHECKPOINT_RESTORE
-/*
- * Specify desired id for next allocated IPC object.
- */
-#define ipc_idr_alloc(ids, new) \
- idr_alloc(&(ids)->ipcs_idr, (new), \
- (ids)->next_id < 0 ? 0 : ipcid_to_idx((ids)->next_id),\
- 0, GFP_NOWAIT)
-
-static inline int ipc_buildid(int id, struct ipc_ids *ids,
- struct kern_ipc_perm *new)
-{
- if (ids->next_id < 0) { /* default, behave as !CHECKPOINT_RESTORE */
- new->seq = ids->seq++;
- if (ids->seq > IPCID_SEQ_MAX)
- ids->seq = 0;
- } else {
- new->seq = ipcid_to_seqx(ids->next_id);
- ids->next_id = -1;
- }
- return SEQ_MULTIPLIER * new->seq + id;
-}
-
-#else
-#define ipc_idr_alloc(ids, new) \
- idr_alloc(&(ids)->ipcs_idr, (new), 0, 0, GFP_NOWAIT)
-
-static inline int ipc_buildid(int id, struct ipc_ids *ids,
- struct kern_ipc_perm *new)
+static inline int ipc_idr_alloc(struct ipc_ids *ids, struct kern_ipc_perm *new)
{
+ int id;
+#ifdef CONFIG_CHECKPOINT_RESTORE
+ if (unlikely(new->id >= 0)) {
+ int idx = ipcid_to_idx(new->id);
+ id = idr_alloc(&ids->ipcs_idr, new, idx, 0, GFP_NOWAIT);
+ if (id < 0)
+ return id;
+ if (id != idx) {
+ idr_remove(&ids->ipcs_idr, id);
+ return -EBUSY;
+ }
+ new->seq = ipcid_to_seqx(new->id);
+ return id;
+ }
+#endif
+ id = idr_alloc(&ids->ipcs_idr, new, 0, 0, GFP_NOWAIT);
new->seq = ids->seq++;
if (ids->seq > IPCID_SEQ_MAX)
ids->seq = 0;
-
- return SEQ_MULTIPLIER * new->seq + id;
+ new->id = SEQ_MULTIPLIER * new->seq + id;
+ return id;
}
-#endif /* CONFIG_CHECKPOINT_RESTORE */
-
/**
* ipc_addid - add an ipc identifier
* @ids: ipc identifier set
@@ -269,6 +255,10 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
new->cuid = new->uid = euid;
new->gid = new->cgid = egid;
+ new->id = ids->next_id;
+ if (new->id >= 0)
+ ids->next_id = -1;
+
id = ipc_idr_alloc(ids, new);
idr_preload_end();
@@ -290,8 +280,6 @@ int ipc_addid(struct ipc_ids *ids, struct kern_ipc_perm *new, int limit)
if (id > ids->max_id)
ids->max_id = id;
- new->id = ipc_buildid(id, ids, new);
-
return id;
}
Eric
next prev parent reply other threads:[~2018-03-29 20:09 UTC|newest]
Thread overview: 42+ messages / expand[flat|nested] mbox.gz Atom feed top
2018-03-12 20:15 [PATCH v4 0/6] ipc: Clamp *mni to the real IPCMNI limit Waiman Long
2018-03-12 20:15 ` [PATCH v4 1/6] sysctl: Add flags to support min/max range clamping Waiman Long
2018-03-12 20:44 ` Luis R. Rodriguez
2018-03-12 20:48 ` Waiman Long
2018-03-13 17:46 ` Eric W. Biederman
2018-03-13 18:49 ` Waiman Long
2018-03-12 20:15 ` [PATCH v4 2/6] proc/sysctl: Check for invalid flags bits Waiman Long
2018-03-12 20:46 ` Luis R. Rodriguez
2018-03-12 20:54 ` Waiman Long
2018-03-12 20:59 ` Luis R. Rodriguez
2018-03-12 21:02 ` Waiman Long
2018-03-12 20:52 ` Andrew Morton
2018-03-12 22:12 ` Waiman Long
2018-03-12 22:42 ` Andrew Morton
2018-03-12 20:15 ` [PATCH v4 3/6] sysctl: Warn when a clamped sysctl parameter is set out of range Waiman Long
2018-03-12 20:50 ` Luis R. Rodriguez
2018-03-12 21:07 ` Waiman Long
2018-03-12 21:00 ` Andrew Morton
2018-03-12 21:04 ` Waiman Long
2018-03-12 20:15 ` [PATCH v4 4/6] ipc: Clamp msgmni and shmmni to the real IPCMNI limit Waiman Long
2018-03-13 18:17 ` Eric W. Biederman
2018-03-13 18:39 ` Waiman Long
2018-03-13 20:29 ` Eric W. Biederman
2018-03-13 21:06 ` Waiman Long
2018-03-15 0:49 ` [RFC][PATCH] ipc: Remove IPCMNI Eric W. Biederman
2018-03-15 17:02 ` Waiman Long
2018-03-15 19:00 ` Eric W. Biederman
2018-03-15 21:46 ` Waiman Long
2018-03-29 2:14 ` Davidlohr Bueso
2018-03-29 8:47 ` Manfred Spraul
2018-03-29 10:56 ` Matthew Wilcox
2018-03-29 18:07 ` Manfred Spraul
2018-03-29 18:52 ` Eric W. Biederman
2018-03-29 19:32 ` Matthew Wilcox
2018-03-29 20:08 ` Eric W. Biederman [this message]
2018-03-15 19:45 ` Matthew Wilcox
2018-03-12 20:15 ` [PATCH v4 5/6] ipc: Clamp semmni to the real IPCMNI limit Waiman Long
2018-03-12 20:52 ` Luis R. Rodriguez
2018-03-12 20:59 ` Waiman Long
2018-03-12 20:15 ` [PATCH v4 6/6] test_sysctl: Add range clamping test Waiman Long
2018-03-12 20:53 ` Luis R. Rodriguez
2018-03-12 21:00 ` Waiman Long
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87y3ia3ase.fsf@xmission.com \
--to=ebiederm@xmission.com \
--cc=akpm@linux-foundation.org \
--cc=containers@lists.linux-foundation.org \
--cc=dave@stgolabs.net \
--cc=keescook@chromium.org \
--cc=linux-api@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=longman@redhat.com \
--cc=manfred@colorfullife.com \
--cc=mcgrof@kernel.org \
--cc=mtk.manpages@gmail.com \
--cc=skinsbursky@parallels.com \
--cc=viro@zeniv.linux.org.uk \
--cc=willy@infradead.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).