Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-03 Thread Yasunori Goto
> On Wed, 3 Oct 2007, Yasunori Goto wrote:
> 
> > > 
> > > That would work. But it would be better to shrink the cache first. The 
> > > first 2 slabs on a node may be empty and the shrinking will remove those. 
> > > If you do not shrink then the code may falsely assume that there are 
> > > objects on the node.
> > 
> > I'm sorry, but I don't think I understand what you mean... :-(
> > Could you explain more? 
> > 
> > Which slabs should be shrinked? kmem_cache_node and kmem_cache_cpu?
> 
> The slab for which you are trying to set the kmem_cache_node pointer to 
> NULL needs to be shrunk.
>  
> > I think kmem_cache_cpu should be disabled by cpu hotplug,
> > not memory/node hotplug. Basically, cpu should be offlined before
> > memory offline on the node.
> 
> Hmmm.. Ok for cpu hotplug you could simply disregard the per cpu 
> structure if the per cpu slab was flushed first.
> 
> However, the per node structure may hold slabs with no objects even after 
> all objects were removed on a node. These need to be flushed by calling
> kmem_cache_shrink() on the slab cache.
> 
> On the other hand: If you can guarantee that they will not be used and 
> that no objects are in them and that you can recover the pages used in 
> different ways then zapping the per node pointer like that is okay.

Thanks for your advise. I'll reconsider and fix my patches.

Bye.

-- 
Yasunori Goto 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-03 Thread Christoph Lameter
On Wed, 3 Oct 2007, Yasunori Goto wrote:

> > 
> > That would work. But it would be better to shrink the cache first. The 
> > first 2 slabs on a node may be empty and the shrinking will remove those. 
> > If you do not shrink then the code may falsely assume that there are 
> > objects on the node.
> 
> I'm sorry, but I don't think I understand what you mean... :-(
> Could you explain more? 
> 
> Which slabs should be shrinked? kmem_cache_node and kmem_cache_cpu?

The slab for which you are trying to set the kmem_cache_node pointer to 
NULL needs to be shrunk.
 
> I think kmem_cache_cpu should be disabled by cpu hotplug,
> not memory/node hotplug. Basically, cpu should be offlined before
> memory offline on the node.

Hmmm.. Ok for cpu hotplug you could simply disregard the per cpu 
structure if the per cpu slab was flushed first.

However, the per node structure may hold slabs with no objects even after 
all objects were removed on a node. These need to be flushed by calling
kmem_cache_shrink() on the slab cache.

On the other hand: If you can guarantee that they will not be used and 
that no objects are in them and that you can recover the pages used in 
different ways then zapping the per node pointer like that is okay.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-03 Thread Yasunori Goto
> On Tue, 2 Oct 2007, Yasunori Goto wrote:
> 
> > Do you mean that just nr_slabs should be checked like followings?
> > I'm not sure this is enough.
> > 
> > :
> > if (s->node[nid]) {
> > n = get_node(s, nid);
> > if (!atomic_read(>nr_slabs)) {
> > s->node[nid] = NULL;
> > kmem_cache_free(kmalloc_caches, n);
> > }
> > }
> > :
> > :
> 
> That would work. But it would be better to shrink the cache first. The 
> first 2 slabs on a node may be empty and the shrinking will remove those. 
> If you do not shrink then the code may falsely assume that there are 
> objects on the node.

I'm sorry, but I don't think I understand what you mean... :-(
Could you explain more? 

Which slabs should be shrinked? kmem_cache_node and kmem_cache_cpu?

I think kmem_cache_cpu should be disabled by cpu hotplug,
not memory/node hotplug. Basically, cpu should be offlined before
memory offline on the node.

Sorry, I'm confusing now...

-- 
Yasunori Goto 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-03 Thread Yasunori Goto
 On Tue, 2 Oct 2007, Yasunori Goto wrote:
 
  Do you mean that just nr_slabs should be checked like followings?
  I'm not sure this is enough.
  
  :
  if (s-node[nid]) {
  n = get_node(s, nid);
  if (!atomic_read(n-nr_slabs)) {
  s-node[nid] = NULL;
  kmem_cache_free(kmalloc_caches, n);
  }
  }
  :
  :
 
 That would work. But it would be better to shrink the cache first. The 
 first 2 slabs on a node may be empty and the shrinking will remove those. 
 If you do not shrink then the code may falsely assume that there are 
 objects on the node.

I'm sorry, but I don't think I understand what you mean... :-(
Could you explain more? 

Which slabs should be shrinked? kmem_cache_node and kmem_cache_cpu?

I think kmem_cache_cpu should be disabled by cpu hotplug,
not memory/node hotplug. Basically, cpu should be offlined before
memory offline on the node.

Sorry, I'm confusing now...

-- 
Yasunori Goto 


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-03 Thread Christoph Lameter
On Wed, 3 Oct 2007, Yasunori Goto wrote:

  
  That would work. But it would be better to shrink the cache first. The 
  first 2 slabs on a node may be empty and the shrinking will remove those. 
  If you do not shrink then the code may falsely assume that there are 
  objects on the node.
 
 I'm sorry, but I don't think I understand what you mean... :-(
 Could you explain more? 
 
 Which slabs should be shrinked? kmem_cache_node and kmem_cache_cpu?

The slab for which you are trying to set the kmem_cache_node pointer to 
NULL needs to be shrunk.
 
 I think kmem_cache_cpu should be disabled by cpu hotplug,
 not memory/node hotplug. Basically, cpu should be offlined before
 memory offline on the node.

Hmmm.. Ok for cpu hotplug you could simply disregard the per cpu 
structure if the per cpu slab was flushed first.

However, the per node structure may hold slabs with no objects even after 
all objects were removed on a node. These need to be flushed by calling
kmem_cache_shrink() on the slab cache.

On the other hand: If you can guarantee that they will not be used and 
that no objects are in them and that you can recover the pages used in 
different ways then zapping the per node pointer like that is okay.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-03 Thread Yasunori Goto
 On Wed, 3 Oct 2007, Yasunori Goto wrote:
 
   
   That would work. But it would be better to shrink the cache first. The 
   first 2 slabs on a node may be empty and the shrinking will remove those. 
   If you do not shrink then the code may falsely assume that there are 
   objects on the node.
  
  I'm sorry, but I don't think I understand what you mean... :-(
  Could you explain more? 
  
  Which slabs should be shrinked? kmem_cache_node and kmem_cache_cpu?
 
 The slab for which you are trying to set the kmem_cache_node pointer to 
 NULL needs to be shrunk.
  
  I think kmem_cache_cpu should be disabled by cpu hotplug,
  not memory/node hotplug. Basically, cpu should be offlined before
  memory offline on the node.
 
 Hmmm.. Ok for cpu hotplug you could simply disregard the per cpu 
 structure if the per cpu slab was flushed first.
 
 However, the per node structure may hold slabs with no objects even after 
 all objects were removed on a node. These need to be flushed by calling
 kmem_cache_shrink() on the slab cache.
 
 On the other hand: If you can guarantee that they will not be used and 
 that no objects are in them and that you can recover the pages used in 
 different ways then zapping the per node pointer like that is okay.

Thanks for your advise. I'll reconsider and fix my patches.

Bye.

-- 
Yasunori Goto 


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-02 Thread Christoph Lameter
On Tue, 2 Oct 2007, Yasunori Goto wrote:

> Do you mean that just nr_slabs should be checked like followings?
> I'm not sure this is enough.
> 
> :
> if (s->node[nid]) {
>   n = get_node(s, nid);
>   if (!atomic_read(>nr_slabs)) {
>   s->node[nid] = NULL;
>   kmem_cache_free(kmalloc_caches, n);
>   }
> }
> :
> :

That would work. But it would be better to shrink the cache first. The 
first 2 slabs on a node may be empty and the shrinking will remove those. 
If you do not shrink then the code may falsely assume that there are 
objects on the node.
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-02 Thread Christoph Lameter
On Tue, 2 Oct 2007, Yasunori Goto wrote:

 Do you mean that just nr_slabs should be checked like followings?
 I'm not sure this is enough.
 
 :
 if (s-node[nid]) {
   n = get_node(s, nid);
   if (!atomic_read(n-nr_slabs)) {
   s-node[nid] = NULL;
   kmem_cache_free(kmalloc_caches, n);
   }
 }
 :
 :

That would work. But it would be better to shrink the cache first. The 
first 2 slabs on a node may be empty and the shrinking will remove those. 
If you do not shrink then the code may falsely assume that there are 
objects on the node.
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-01 Thread Yasunori Goto
> On Mon, 1 Oct 2007, Yasunori Goto wrote:
> 
> > +#ifdef CONFIG_MEMORY_HOTPLUG
> > +static void __slab_callback_offline(int nid)
> > +{
> > +   struct kmem_cache_node *n;
> > +   struct kmem_cache *s;
> > +
> > +   list_for_each_entry(s, _caches, list) {
> > +   if (s->node[nid]) {
> > +   n = get_node(s, nid);
> > +   s->node[nid] = NULL;
> > +   kmem_cache_free(kmalloc_caches, n);
> > +   }
> > +   }
> > +}
> 
> I think we need to bug here if there are still objects on the node that 
> are in use. This will silently discard the objects.
> 
Here is just the rollback code for an allocation failure of
kmem_cache_node in halfway.
So, there is a case some of them are not allocated yet.
Any slabs don't use new kmem_cache_node before the new nodes page is
available --so far--.
But, in the future, here will be useful for node hot-unplug code,
and its check will be necessary.  Ok. I'll add its check.

Do you mean that just nr_slabs should be checked like followings?
I'm not sure this is enough.

:
if (s->node[nid]) {
n = get_node(s, nid);
if (!atomic_read(>nr_slabs)) {
s->node[nid] = NULL;
kmem_cache_free(kmalloc_caches, n);
}
}
:
:

Thanks.

-- 
Yasunori Goto 


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-01 Thread Christoph Lameter
On Mon, 1 Oct 2007, Yasunori Goto wrote:

> +#ifdef CONFIG_MEMORY_HOTPLUG
> +static void __slab_callback_offline(int nid)
> +{
> + struct kmem_cache_node *n;
> + struct kmem_cache *s;
> +
> + list_for_each_entry(s, _caches, list) {
> + if (s->node[nid]) {
> + n = get_node(s, nid);
> + s->node[nid] = NULL;
> + kmem_cache_free(kmalloc_caches, n);
> + }
> + }
> +}

I think we need to bug here if there are still objects on the node that 
are in use. This will silently discard the objects.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-01 Thread Christoph Lameter
On Mon, 1 Oct 2007, Yasunori Goto wrote:

 +#ifdef CONFIG_MEMORY_HOTPLUG
 +static void __slab_callback_offline(int nid)
 +{
 + struct kmem_cache_node *n;
 + struct kmem_cache *s;
 +
 + list_for_each_entry(s, slab_caches, list) {
 + if (s-node[nid]) {
 + n = get_node(s, nid);
 + s-node[nid] = NULL;
 + kmem_cache_free(kmalloc_caches, n);
 + }
 + }
 +}

I think we need to bug here if there are still objects on the node that 
are in use. This will silently discard the objects.

-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/


Re: [Patch / 002](memory hotplug) Callback function to create kmem_cache_node.

2007-10-01 Thread Yasunori Goto
 On Mon, 1 Oct 2007, Yasunori Goto wrote:
 
  +#ifdef CONFIG_MEMORY_HOTPLUG
  +static void __slab_callback_offline(int nid)
  +{
  +   struct kmem_cache_node *n;
  +   struct kmem_cache *s;
  +
  +   list_for_each_entry(s, slab_caches, list) {
  +   if (s-node[nid]) {
  +   n = get_node(s, nid);
  +   s-node[nid] = NULL;
  +   kmem_cache_free(kmalloc_caches, n);
  +   }
  +   }
  +}
 
 I think we need to bug here if there are still objects on the node that 
 are in use. This will silently discard the objects.
 
Here is just the rollback code for an allocation failure of
kmem_cache_node in halfway.
So, there is a case some of them are not allocated yet.
Any slabs don't use new kmem_cache_node before the new nodes page is
available --so far--.
But, in the future, here will be useful for node hot-unplug code,
and its check will be necessary.  Ok. I'll add its check.

Do you mean that just nr_slabs should be checked like followings?
I'm not sure this is enough.

:
if (s-node[nid]) {
n = get_node(s, nid);
if (!atomic_read(n-nr_slabs)) {
s-node[nid] = NULL;
kmem_cache_free(kmalloc_caches, n);
}
}
:
:

Thanks.

-- 
Yasunori Goto 


-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/