On Mon, Apr 18, 2011 at 10:24:32AM +0600, Anton Maksimenkov wrote:
> 2011/4/18 Ted Unangst <ted.unan...@gmail.com>:
> > + if (uvm.numof_free_kentries < 1) /*check to be safe*/
> > + panic("uvm.numof_free_kentries[%d] < 1\n",
> uvm.numof_free_kentries);
> >>> This diff would take us back to the bad old days when running out of
> entries led
> >>> to a panic.
> >> Totally conversely. As It was showed above, that situation completely
> >> resolved and not fatal.
> >> Other panics I add just for debug and to be safe that algorithm works
> >> correctly. If all work correctly these panics does not appear even in
> >> bad situation (kernel_map exhausted).
> >
> > I believe the above panic will occur whenever there is no free entry,
> > which will happen after the last "reserve" entry is used. If trying
> > to get more fails (most likely because the map is full), then we'll
> > use the reserve and the next allocation will fail.
>
> If you recheck the diff carefully you will see that it is not (at
> least, it must not, if all goes right).
>
> First, if (uvm.numof_free_kentries == 1) then my code tries to
> allocate page, divide it to new kentries and add them to
> uvm.kentry_free.
That's what pool does.
> It will be done only if uvm_km_kmemalloc(kentry_map,...) succeeds. And
> only if this uvm_km_kmemalloc(kentry_map,...) succeeds, it will use
> last reserved kentry
>
> + if (map == kentry_map) {
> ...
> + uvm.kentry_free = me->next;
> + uvm.numof_free_kentries--;
> + uvmexp.kmapent++;
> + me->flags = UVM_MAP_STATIC;
> + } else
>
> But if uvm_km_kmemalloc(kentry_map,...) fails, it will unmap region it
> mapped (check code in uvm_km_kmemalloc,
> http://www.openbsd.org/cgi-bin/cvsweb/src/sys/uvm/uvm_km.c?annotate=1.96,
> line 380: and 424:).
> And during this unmap it RETURN BACK (by uvm_mapent_free()) that last
> reserved kentry it may use.
>
> So if it fails, yes, we can't make new kentries. But reserved kentry
> restored, it still here. And we are safe and ready to try again later.
> Anyhow, even if uvm_km_kmemalloc(kentry_map,...) fails (either if
> kentry_map is full or uvm_pglistalloc failed to get free page), then
> when memory pressure goes down new allocations will succeed. Nothing
> fatal.
> That is the principle - fully predictable states, as it must be.
Your diff is reducable to:
static struct vm_map_entry kentries[REALLY_BIG_NUMBER];
And then managing that memory using vmmap.
Your diff will generate problems with recursive locking:
1) I need an entry for the kernel_map
lock: kernel_map.
2) Therefor I need one from kentry_map, but I only have one left
lock: kentry_map.
3) I re-enter kentry_map to allocate a new entry
lock: kentry_map again!
Currently it works, because you are lucky. Step 3 operates on a map that
didn't get modified by step 2 yet, nor did step 2 depend on any state
potentially changed by step 3. Furthermore, simple_lock is a noop and
queued for destruction (use mutex instead).
> > fail, but it is unlikely. My laptop prints the warning message about
> > running out of static entries quite frequently, but never fails to get
> > a new page.
>
> When my server show some "reasonable amount" ot these "out of static
> entries" then I know it's time to go and reboot it. If not, then,
> sooner or later, I'll see frozen machine (at very wrong time,
> usually).
Show the maps for which allocation failed. It's likely to be kmem_map.
What is the status of kernel malloc? How much memory is used in each of
the malloc groups? Analyze the problem, why does it happen? Can you
narrow the behaviour down to a specific pattern? Once it freezes, break
into ddb and see what's running. What is the machine doing during the
freeze? Submit a bug report. :)
You are the only person I know of that has machines freezing because of
kentry pressure, assuming it is indeed related...
> > An allocation failing when a map is out of space is not
> > just likely, it's a certainty.
>
> Of course. But it is completely predictable, not fatal (if we talk
> about kernel stability).
Predictable != non-fatal.
Running out of map entries is always fatal. We need to have them to be
able to create them. And lots of callers get unhappy somewhere down the
line if they run out of memory.
> By the way, we can setup the kentry_map size based on total memory
> size, calculated when memory subsystem bootstrap. Either we can set it
> rather big and be sure it is enough to allocate all physical memory,
> or we can save more or less VA space with more or less risk that
> kentry_map will be full. That's it, we must choose, since we
> !HAVE_PMAP_DIRECT.
> And we are talking about kernel memory allocations, yes? It's all
> related to maps with VM_MAP_INTRSAFE flag.
For every magic value, users will prove its the wrong value.
> > I don't think the kmthread mechanism is perfect, but it's fairly well
> > understood and does not cause problems that I'm aware of. Are you
> > actually experiencing a problem where uvm_km_getpage is failing?
>
> Maybe. But these "out of static entries" is proved signal, that kernel
> corrupted somehow.
Out of static entries means just that. Corruption is a whole different
beast.
> Furthermore, what state of the kernel will be if all free kmthread's
> pages will be used and all static ketries will be used (and so the
> uvm.kentry_free is NULL)? Or some other "interesting situation"?
> Something will try to allocate memory and (because of unpredictable
> nature of that mechanism) then the kernel may locks itself.
Allocation will fail. Caller will deal with it or panic.
Very predictable and simple. :)
> > a real problem. In general, I don't like any change that introduces a
> > new magic map or fixed limit. We suffered for years from various
> > subsystems reserving KVA that they didn't need (and other systems
> > failing when they ran out).
>
> I strongly tried to avoid this. Yes, "brand new map" seems not so
> pretty. But remember, these kentries is fundamental for all uvm system
> to work as expected. So why the "malloc" is allowed use it's own map,
> while "uvm" itself can't?
Malloc has its own map because uvm is designed to be pageable, while
malloc memory needs to be intr-safe. If we create a special map for
kentries, we could just as well grow kmem_map instead.
> I think that the situation with these
> kentries was not clear when UVM was constructed and that is why the
> ketries mechanism was not well designed.
> And remember, I present this kentry_map when art@ says something like
> "it's bad to use kmem_map", so there were no other way...
Your diff is a different way of achieving what we have. But it's new,
untested and the mechanism is harder to understand. Also, the code path
is longer, which makes the introduction of bugs easier and (thus) makes
for fragile code.
However, you pose that the kentries mechanism was not well designed, but
you don't challenge the design itself, instead opting to follow the same
design with a different implementation. How will that the problem?
The current implementation, like tedu@ says, it well understood, clear
and easy to comprehend. I don't think the current design allows for a
better implementation, given the constraints.
> Anyhow, I understand that it is bad when some static values of KVA
> reserved. But I showed the problem, showed the way to the solution.
> Simple, easy to understand the idea.
Your algorithm is simple and, given biglock protecting us, I believe it
may work. It'll break once biglock gets removed though.
> If you stand against of "new brand kentry_map", but agreed with the
> idea itself, I think there may be other ways to achieve same result
> without new map but with same "finite state machine" approach.
> Of course, it will be (much more?) tricky, but I believe it's possible.
I disagree with the idea itself.
The real problem is an architectural one.
--
Ariane
