[PATCH 04/04]
This patch introduces the registration of a callback routine that recomputes
msg_ctlmni upon memory add / remove.
A notifier block structure has been added to the ipc namespace structure.
Each time a new ipc namespace is allocated, this notifier block callback
routine is registered into the memory hotplug notifier chain (it is
unregistered when the ipc namespace is freed). That routine is then activated
when memory is added or removed, in order to recompute msgmni for that
namespace.
Signed-off-by: Nadia Derbey <[email protected]>
---
include/linux/ipc.h | 25 +++++++++++++++++++++
include/linux/memory.h | 1
ipc/msg.c | 2 -
ipc/util.c | 57 +++++++++++++++++++++++++++++++++++++++++++++++++
ipc/util.h | 2 +
5 files changed, 86 insertions(+), 1 deletion(-)
Index: linux-2.6.24-rc7/include/linux/ipc.h
===================================================================
--- linux-2.6.24-rc7.orig/include/linux/ipc.h 2008-01-11 16:00:50.000000000 +0100
+++ linux-2.6.24-rc7/include/linux/ipc.h 2008-01-11 16:52:04.000000000 +0100
@@ -81,6 +81,10 @@ struct ipc_kludge {
#include <linux/kref.h>
#include <linux/spinlock.h>
+#ifdef CONFIG_MEMORY_HOTPLUG
+#include <linux/notifier.h>
+#include <linux/memory.h>
+#endif /* CONFIG_MEMORY_HOTPLUG */
#define IPCMNI 32768 /* <= MAX_INT limit for ipc arrays (including sysctl changes) */
@@ -118,6 +122,10 @@ struct ipc_namespace {
size_t shm_ctlall;
int shm_ctlmni;
int shm_tot;
+
+#ifdef CONFIG_MEMORY_HOTPLUG
+ struct notifier_block ipc_memory_hotplug;
+#endif
};
extern struct ipc_namespace init_ipc_ns;
@@ -153,6 +161,23 @@ static inline void put_ipc_ns(struct ipc
#endif
}
+#ifdef CONFIG_MEMORY_HOTPLUG
+
+extern void register_ipc_hotplug_notifier(struct ipc_namespace *);
+
+static inline void unregister_ipc_hotplug_notifier(struct ipc_namespace *ns)
+{
+ unregister_memory_notifier(&ns->ipc_memory_hotplug);
+}
+
+#else /* CONFIG_MEMORY_HOTPLUG */
+
+#define register_ipc_hotplug_notifier(ns) do { } while (0)
+#define unregister_ipc_hotplug_notifier(ns) do { } while (0)
+
+#endif /* CONFIG_MEMORY_HOTPLUG */
+
+
#endif /* __KERNEL__ */
#endif /* _LINUX_IPC_H */
Index: linux-2.6.24-rc7/include/linux/memory.h
===================================================================
--- linux-2.6.24-rc7.orig/include/linux/memory.h 2008-01-11 16:26:03.000000000 +0100
+++ linux-2.6.24-rc7/include/linux/memory.h 2008-01-11 16:53:10.000000000 +0100
@@ -59,6 +59,7 @@ struct mem_section;
* order in the callback chain)
*/
#define SLAB_CALLBACK_PRI 1
+#define IPC_CALLBACK_PRI 10
#ifndef CONFIG_MEMORY_HOTPLUG_SPARSE
static inline int memory_dev_init(void)
Index: linux-2.6.24-rc7/ipc/util.h
===================================================================
--- linux-2.6.24-rc7.orig/ipc/util.h 2008-01-11 14:08:48.000000000 +0100
+++ linux-2.6.24-rc7/ipc/util.h 2008-01-11 16:56:23.000000000 +0100
@@ -134,6 +134,8 @@ extern int ipcget_new(struct ipc_namespa
extern int ipcget_public(struct ipc_namespace *, struct ipc_ids *,
struct ipc_ops *, struct ipc_params *);
+extern void recompute_msgmni(struct ipc_namespace *);
+
static inline int ipc_buildid(int id, int seq)
{
return SEQ_MULTIPLIER * seq + id;
Index: linux-2.6.24-rc7/ipc/util.c
===================================================================
--- linux-2.6.24-rc7.orig/ipc/util.c 2008-01-11 16:12:38.000000000 +0100
+++ linux-2.6.24-rc7/ipc/util.c 2008-01-11 16:58:29.000000000 +0100
@@ -33,6 +33,7 @@
#include <linux/audit.h>
#include <linux/nsproxy.h>
#include <linux/rwsem.h>
+#include <linux/memory.h>
#include <asm/unistd.h>
@@ -54,6 +55,50 @@ struct ipc_namespace init_ipc_ns = {
atomic_t nr_ipc_ns = ATOMIC_INIT(1);
+#ifdef CONFIG_MEMORY_HOTPLUG
+
+static int ipc_memory_callback(struct notifier_block *self,
+ unsigned long action, void *arg)
+{
+ struct ipc_namespace *ns;
+
+ switch (action) {
+ case MEM_ONLINE: /* memory successfully brought online */
+ case MEM_OFFLINE: /* or offline: it's time to recompute xxxmni */
+ ns = container_of(self, struct ipc_namespace,
+ ipc_memory_hotplug);
+ /*
+ * No need to get a reference on the ns: the 1st job of
+ * free_ipc_ns() is to unregister the callback routine.
+ * blocking_notifier_chain_unregister takes the wr lock to do
+ * it.
+ * When this callback routine is called the rd lock is held by
+ * blocking_notifier_call_chain.
+ * So the ipc ns cannot be freed while we are here.
+ */
+ recompute_msgmni(ns);
+ break;
+ case MEM_GOING_ONLINE:
+ case MEM_GOING_OFFLINE:
+ case MEM_CANCEL_ONLINE:
+ case MEM_CANCEL_OFFLINE:
+ default:
+ break;
+ }
+
+ return NOTIFY_OK;
+}
+
+void register_ipc_hotplug_notifier(struct ipc_namespace *ns)
+{
+ memset(&ns->ipc_memory_hotplug, 0, sizeof(ns->ipc_memory_hotplug));
+ ns->ipc_memory_hotplug.notifier_call = ipc_memory_callback;
+ ns->ipc_memory_hotplug.priority = IPC_CALLBACK_PRI;
+ register_memory_notifier(&ns->ipc_memory_hotplug);
+}
+
+#endif /* CONFIG_MEMORY_HOTPLUG */
+
static struct ipc_namespace *clone_ipc_ns(struct ipc_namespace *old_ns)
{
int err;
@@ -76,6 +121,8 @@ static struct ipc_namespace *clone_ipc_n
if (err)
goto err_shm;
+ register_ipc_hotplug_notifier(ns);
+
kref_init(&ns->kref);
return ns;
@@ -111,6 +158,15 @@ void free_ipc_ns(struct kref *kref)
struct ipc_namespace *ns;
ns = container_of(kref, struct ipc_namespace, kref);
+ /*
+ * Unregistering the hotplug notifier at the beginning guarantees
+ * that the ipc namespace won't be freed while we are inside the
+ * the callback routine. Since the blocking_notifier_chain_XXX
+ * routines hold a rw lock on the notifier list,
+ * unregister_ipc_hotplug_notifier() won't take the rw lock before
+ * blocking_notifier_call_chain() has released the rd lock.
+ */
+ unregister_ipc_hotplug_notifier(ns);
sem_exit_ns(ns);
msg_exit_ns(ns);
shm_exit_ns(ns);
@@ -130,6 +186,7 @@ static int __init ipc_init(void)
sem_init();
msg_init();
shm_init();
+ register_ipc_hotplug_notifier(&init_ipc_ns);
return 0;
}
__initcall(ipc_init);
Index: linux-2.6.24-rc7/ipc/msg.c
===================================================================
--- linux-2.6.24-rc7.orig/ipc/msg.c 2008-01-11 16:08:32.000000000 +0100
+++ linux-2.6.24-rc7/ipc/msg.c 2008-01-11 17:03:31.000000000 +0100
@@ -85,7 +85,7 @@ static int sysvipc_msg_proc_show(struct
* queues should occupy at most 1/MSG_MEM_SCALE of lowmem.
* Also take into account the number of nsproxies created so far.
*/
-static void recompute_msgmni(struct ipc_namespace *ns)
+void recompute_msgmni(struct ipc_namespace *ns)
{
struct sysinfo i;
unsigned long allowed;
--
Hello Nadia-san.
> @@ -118,6 +122,10 @@ struct ipc_namespace {
> size_t shm_ctlall;
> int shm_ctlmni;
> int shm_tot;
> +
> +#ifdef CONFIG_MEMORY_HOTPLUG
> + struct notifier_block ipc_memory_hotplug;
> +#endif
> };
I'm sorry, but I don't see why each ipc namespace must have each callbacks
of memory hotplug.
I prefer only one callback for each subsystem, not for each namespace.
In addition, the recompute_msgmni() calculation looks very similar for
all ipc namespace.
Or do you wish each ipc namespace have different callback for the future?
BTW, have you ever tested this patch? If you don't have any test environment
for memory hotplug code, then I'll check it. :-)
Bye.
--
Yasunori Goto
Yasunori Goto wrote:
> Hello Nadia-san.
>
>
>>@@ -118,6 +122,10 @@ struct ipc_namespace {
>> size_t shm_ctlall;
>> int shm_ctlmni;
>> int shm_tot;
>>+
>>+#ifdef CONFIG_MEMORY_HOTPLUG
>>+ struct notifier_block ipc_memory_hotplug;
>>+#endif
>> };
>
>
> I'm sorry, but I don't see why each ipc namespace must have each callbacks
> of memory hotplug.
> I prefer only one callback for each subsystem, not for each namespace.
> In addition, the recompute_msgmni() calculation looks very similar for
> all ipc namespace.
> Or do you wish each ipc namespace have different callback for the future?
>
Actually, this is what I wanted to do at the very beginning: have a
single callback that would recompute the msgmni for each ipc namespace.
But the issue here is that the namespaces are not linked to each other,
so I had no simple way to go through all the namespaces.
I solved the issue by having a callback for any single ipc namespace and
make it recompute the msgmni value for itslef.
>
>
> BTW, have you ever tested this patch? If you don't have any test environment
> for memory hotplug code, then I'll check it. :-)
Well, I tested it but not in "real configuration": what I did is that I
changed the status by hand under sysfs to offline. I also changed
remove_memory() in mm/memory_hotplug.c in the following way (instead of
returninf EINVAL):
1) decrease the total_ram pages
2) call memory_notify(MEM_OFFLINE, NULL)
and checked that the msgmni was recomputed.
But sure, if you are candidate to test it, that would be great!
Regards,
Nadia
> Yasunori Goto wrote:
> > Hello Nadia-san.
> >
> >
> >>@@ -118,6 +122,10 @@ struct ipc_namespace {
> >> size_t shm_ctlall;
> >> int shm_ctlmni;
> >> int shm_tot;
> >>+
> >>+#ifdef CONFIG_MEMORY_HOTPLUG
> >>+ struct notifier_block ipc_memory_hotplug;
> >>+#endif
> >> };
> >
> >
> > I'm sorry, but I don't see why each ipc namespace must have each callbacks
> > of memory hotplug.
> > I prefer only one callback for each subsystem, not for each namespace.
> > In addition, the recompute_msgmni() calculation looks very similar for
> > all ipc namespace.
> > Or do you wish each ipc namespace have different callback for the future?
> >
>
> Actually, this is what I wanted to do at the very beginning: have a
> single callback that would recompute the msgmni for each ipc namespace.
> But the issue here is that the namespaces are not linked to each other,
> so I had no simple way to go through all the namespaces.
> I solved the issue by having a callback for any single ipc namespace and
> make it recompute the msgmni value for itslef.
The recompute_msg() must be called when new ipc_namespace is created/removed
as you mentioned. I think namespaces should be linked each other for it
in the end....
> >
> > BTW, have you ever tested this patch? If you don't have any test environment
> > for memory hotplug code, then I'll check it. :-)
>
> Well, I tested it but not in "real configuration": what I did is that I
> changed the status by hand under sysfs to offline. I also changed
> remove_memory() in mm/memory_hotplug.c in the following way (instead of
> returninf EINVAL):
> 1) decrease the total_ram pages
> 2) call memory_notify(MEM_OFFLINE, NULL)
>
> and checked that the msgmni was recomputed.
You can also online again after offline by writing sysfs.
> But sure, if you are candidate to test it, that would be great!
Ok. I'll check it too.
Bye.
--
Yasunori Goto
Yasunori Goto wrote:
>>Yasunori Goto wrote:
>>
>>>Hello Nadia-san.
>>>
>>>
>>>
>>>>@@ -118,6 +122,10 @@ struct ipc_namespace {
>>>> size_t shm_ctlall;
>>>> int shm_ctlmni;
>>>> int shm_tot;
>>>>+
>>>>+#ifdef CONFIG_MEMORY_HOTPLUG
>>>>+ struct notifier_block ipc_memory_hotplug;
>>>>+#endif
>>>>};
>>>
>>>
>>>I'm sorry, but I don't see why each ipc namespace must have each callbacks
>>>of memory hotplug.
>>>I prefer only one callback for each subsystem, not for each namespace.
>>>In addition, the recompute_msgmni() calculation looks very similar for
>>>all ipc namespace.
>>>Or do you wish each ipc namespace have different callback for the future?
>>>
>>
>>Actually, this is what I wanted to do at the very beginning: have a
>>single callback that would recompute the msgmni for each ipc namespace.
>>But the issue here is that the namespaces are not linked to each other,
>>so I had no simple way to go through all the namespaces.
>>I solved the issue by having a callback for any single ipc namespace and
>>make it recompute the msgmni value for itslef.
>
>
> The recompute_msg() must be called when new ipc_namespace is created/removed
> as you mentioned. I think namespaces should be linked each other for it
> in the end....
Not if I do it the same way as for memory hotplug:
1) definee a "namespace event notifier"
2) insert another notifier block into the ipc namespace.
3) The callback routines in the notifier chain would be activated upon
namespace creation / removal.
But I'm still thinking about it: Matt Helsley suggested that we might
just copy the old namespace's value when creating a new namespace.
There's also the issue of an msgmni that has been changed via procfs:
may be we should not unconditionally recompute it upon namespace
creation / removal, so unregister the callback for the owning namespace
as soon as msgmni has been changed from userspace.
Regards,
Nadia