Re: [Ugly PATCH] Re: Again: panic kmem_malloc()

2002-10-11 Thread Terry Lambert
Jeff Roberson wrote:
> On Fri, 11 Oct 2002, Terry Lambert wrote:
> > Ben Stuyts wrote:
> > > Is there a way to check the free list of the kernel? Maybe I can find out
> > > what action triggers eating al its memory.
> 
> Maybe you should just increase the size of your kmem_map?  I'll look into
> a better fix but that should do it short term.

I think he's in an overload condition.  Basically, it will always use
all available resources, and then some.  So incresing "all available"
still leaves you with your foot shot off when the system asks for "and
then some".  8-(.

> > 1)page_alloc() in uma is using kmem_malloc() without the
> >   M_NOWAIT flag
> 
> Why is this bogus?

Because it's possible to panic under heavy load.  The only thing
heavy load should cause is slowdown... ideally, proportional to
the load... ;^).


> Yes, I agree, it should wait.  It might be interesting to see what the
> effects of allocating straight out of the kernel_map would be.  Or perhaps
> positioning the kmem_map in such a way that it might be able to expand.  I
> don't like the hard limit here anymore than anyone else does.

I looked at this.  I don't think you can reasonable expand maps
other than the kernel_map.  The allocations are wrong... kernel_map
is, unfortunately, "special".

This is about the 1,000,000th time I've wanted to establish mappings
for all physical RAM, and maybe the entire address space, up front,
instead of leaving it to demand-time.  The problem with this is that
you are talking 4M (or 8M, if you use PSE-36 or PAE).  Then *everything*
becomes allocable at interrupt time, and there's no such thing as tiered
access semantics, only whether or not a mapping is assigned to the pool
you want to allocate out of, or not.


> On reasonable architectures your only worry is wasting pages and not kva.

Heh... "It's not an unreasonable architecture, you're just using an
unreasonable amount of memory on a reasonable architecture".


> > Jeffrey Roberson is going to need to fix UMA allocations, per the
> > comment in this patch, for a more permanent fix.  I've specifically
> > Cc:'ed him on this message.
> 
> Thanks for looking into this terry.  Please, call me Jeff though.

No problem; I copied the name out of the source file; I tend to use
"Terrence" in source files (you can see this in init_main.c and
other places where I did enough work I felt it merited a copyright
to be able to give it away).

-- Terry

To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



Re: [Ugly PATCH] Re: Again: panic kmem_malloc()

2002-10-11 Thread Jeff Roberson

On Fri, 11 Oct 2002, Terry Lambert wrote:

> Ben Stuyts wrote:
> > Is there a way to check the free list of the kernel? Maybe I can find out
> > what action triggers eating al its memory.

Maybe you should just increase the size of your kmem_map?  I'll look into
a better fix but that should do it short term.

>
> ] panic: kmem_malloc(4096): kmem_map too small: 28246016 total allocated.
>
> That's easy: you're calling kmem_malloc() without M_NOWAIT.
>
> That function only operates on the maps kmem_map or mb_map.
>
> It calls vm_map_findspace(), which fails to find space in the
> map.
>
> vm_map_findspace() fails to add space to the map, because it
> only adds space tot he map if the map is kernel_map; all other
> maps fail catastrophically.

Well, for the old allocator this was a panicing situation.  It never
returned memory and kva back to kmem_map.  So you were pretty much done.
>
> The map you are calling it with is kmem_map (if it were mb_map,
> you would get an "Out of mbuf clusters" message on your console,
> and the allocation would fail, regardless of the value of the
> M_NOWAIT flags bit (mbuf allocations do not properly honor the
> lack of an M_NOWAIT flags bit).

It is documented that they do not.  Notice we only have M_TRYWAIT now for
mbufs.
>
> The panic message occurs becuause you asked it to wait for memory
> to be available.
>
> But the code is stupid, and refuses to wait for memory to be
> available, in the case that space can not be found in the map,
> because it does not properly realize that the freeing of memory
> elsewhere can result in freed space in the map.  So it calls
> "panic" instead of waiting.
It will wait for pages to be available.  It just wont wait for KVA to be
available.  This was a somewhat less bogus with the old allocator.
>
> Therefore, it's technically illegal to call kmem_malloc() with
> a third argument that does not include the M_NOWAIT bit, even
> though the function is documented, and obviously intended, to
> permit the use of this flags bit.
No, that's not true.  It will wait if you're low on pages.  It will not
wait if you've run out of kva.  There's a big difference.  Also, WAITOK
more appropriately means "Never return NULL even if you have to panic."
You just assumed it would mean "Never return NULL even if you have to wait".
;-)  I agree that it should mean the latter though.

>
>
> In -current, there is exactly one place where kmem_malloc() is
> called with the kmem_map as its first argument: in the function
> page_alloc() in vm/uma_core.c.
>
>
> So, you have two bogus things happening:
>
> 1)page_alloc() in uma is using kmem_malloc() without the
>   M_NOWAIT flag
Why is this bogus?
>
> 2)kmem_malloc() without the M_NOWAIT flag panics, INSTEAD
>   OF FRICKING WAITING, LIKE YOU ARE TELLING IT TO DO.
>   :-0_ <- Dr. Evil
Yes, I agree, it should wait.  It might be interesting to see what the
effects of allocating straight out of the kernel_map would be.  Or perhaps
positioning the kmem_map in such a way that it might be able to expand.  I
don't like the hard limit here anymore than anyone else does.  On
reasonable architectures your only worry is wasting pages and not kva.
Now with UMA the VM can tell it that it's using too many pages and so the
system is self tuning.  We'll have to do something to help kva crippled
architectures (x86) in the near term.
>
> Probably, page_alloc should be rewritten to not use kmem_malloc(),
> and to use the kmem_alloc_wait() instead.
>
> Please find a (relatively bogus) patch attached, which could cause
> things to block for a long time, but will avoid the panic.
>
> Jeffrey Roberson is going to need to fix UMA allocations, per the
> comment in this patch, for a more permanent fix.  I've specifically
> Cc:'ed him on this message.

Thanks for looking into this terry.  Please, call me Jeff though.


To Unsubscribe: send mail to [EMAIL PROTECTED]
with "unsubscribe freebsd-current" in the body of the message



[Ugly PATCH] Re: Again: panic kmem_malloc()

2002-10-11 Thread Terry Lambert

Ben Stuyts wrote:
> Is there a way to check the free list of the kernel? Maybe I can find out
> what action triggers eating al its memory.

] panic: kmem_malloc(4096): kmem_map too small: 28246016 total allocated.

That's easy: you're calling kmem_malloc() without M_NOWAIT.

That function only operates on the maps kmem_map or mb_map.

It calls vm_map_findspace(), which fails to find space in the
map.

vm_map_findspace() fails to add space to the map, because it
only adds space tot he map if the map is kernel_map; all other
maps fail catastrophically.

The map you are calling it with is kmem_map (if it were mb_map,
you would get an "Out of mbuf clusters" message on your console,
and the allocation would fail, regardless of the value of the
M_NOWAIT flags bit (mbuf allocations do not properly honor the
lack of an M_NOWAIT flags bit).

The panic message occurs becuause you asked it to wait for memory
to be available.

But the code is stupid, and refuses to wait for memory to be
available, in the case that space can not be found in the map,
because it does not properly realize that the freeing of memory
elsewhere can result in freed space in the map.  So it calls
"panic" instead of waiting.

Therefore, it's technically illegal to call kmem_malloc() with
a third argument that does not include the M_NOWAIT bit, even
though the function is documented, and obviously intended, to
permit the use of this flags bit.


In -current, there is exactly one place where kmem_malloc() is
called with the kmem_map as its first argument: in the function
page_alloc() in vm/uma_core.c.


So, you have two bogus things happening:

1)  page_alloc() in uma is using kmem_malloc() without the
M_NOWAIT flag

2)  kmem_malloc() without the M_NOWAIT flag panics, INSTEAD
OF FRICKING WAITING, LIKE YOU ARE TELLING IT TO DO.
:-0_ <- Dr. Evil

Probably, page_alloc should be rewritten to not use kmem_malloc(),
and to use the kmem_alloc_wait() instead.

Please find a (relatively bogus) patch attached, which could cause
things to block for a long time, but will avoid the panic.

Jeffrey Roberson is going to need to fix UMA allocations, per the
comment in this patch, for a more permanent fix.  I've specifically
Cc:'ed him on this message.

-- Terry

Index: uma_core.c
===
RCS file: /cvs/src/sys/vm/uma_core.c,v
retrieving revision 1.38
diff -c -r1.38 uma_core.c
*** uma_core.c  28 Sep 2002 17:15:33 -  1.38
--- uma_core.c  11 Oct 2002 15:19:15 -
***
*** 781,787 
void *p;/* Returned page */
  
*pflag = UMA_SLAB_KMEM;
!   p = (void *) kmem_malloc(kmem_map, bytes, wait);

return (p);
  }
--- 781,798 
void *p;/* Returned page */
  
*pflag = UMA_SLAB_KMEM;
!   /*
!* XXX Bogus
!* kmem_malloc() can panic if called without M_NOWAIT on kmem_map;
!* work around this by calling kmem_alloc_wait() instead.  This is
!* really bogus, because it can hang indefinitely.  Jeffrey Roberson
!* needs to fix this to do all UMA allocations out of kernel_map,
!* instead, so pmap_growkernel() can be used, instead of hanging.
!*/
!   if (wait & M_NOWAIT)
!   p = (void *) kmem_malloc(kmem_map, bytes, wait);
!   else
!   p = (void *) kmem_alloc_wait(kmem_map, bytes);

return (p);
  }