On Tue, Apr 19, 2011 at 11:26:16AM +0600, Anton Maksimenkov wrote:
> 2011/4/19 Ariane van der Steldt <[email protected]>:
> > Your diff is reducable to:
> > ?static struct vm_map_entry kentries[REALLY_BIG_NUMBER];
> > And then managing that memory using vmmap.
> 
> Yes, more or less... But your example eats VA memory and physical
> pages immediately and forever, while my diff tried to eat it on demand
> (it even may be constructed in a way to free physical pages when there
> will be many free kentries, in some cases of course)...

Allocating a map eats VA. A submap is simply a local instance of the
vmmap allocator, complete with address space that is managed and taken
away from the global kernel. And what's worse, it's a contig range, so
it will worsen the fragmentation burden (the same amount of
fragmentation will live in a smaller area now).

pmap_kenter_pa will actually do the physical page management, in a
better way (since you know what the memory is used for, you can make
more intelligent decisions). Kinda what uvm_km_getpage_pla() does.

> > 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.
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> why? where you get it?
> 
> > 3) I re-enter kentry_map to allocate a new entry
> > ? lock: kentry_map again!
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
> that's must be first and right lock. it occures in
> uvm_map(kentry_map,...) when it called from
> uvm_km_kmemalloc(kentry_map,...).

You're right. You avoid the recursion. I'm sorry.

> > potentially changed by step 3. Furthermore, simple_lock is a noop and
> > queued for destruction (use mutex instead).
> 
> Yes. That's why I introduced mutex, as an example (but, my mistake, I
> missed it in uvm_mapent_free).
> 
> > 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. :)
> 
> Yes, yes... But usually it freezes to death, console not responding,
> so no ddb, no reports...

"My box freezes. This is the dmesg. This is what I do with it.
 I can't enter ddb because my machine doesn't respond to my method of
 entering ddb.
 I tried this and this and this other thing with these results.
 Please help."
Something along those lines?
We want complete bug reports. That also means that, if we ask for
something you can't provide, you tell us that (and why, because it's
another symptom).

Please consider that you are submitting a diff, solving a problem,
that we currently don't see as a problem. The whole kernel is filled
with panics that never happen. And this isn't a panic, it's a warning.
Look at the code. The warning message is printed, but a new entry was
allocated succesfully. If it wasn't, you'd have paniced instead.
You're not panicing, these are not the droids you're looking for.

> > You are the only person I know of that has machines freezing because of
> > kentry pressure, assuming it is indeed related...
> 
> Yes, again. I'm not 100% sure that it is because of kentries pressure,
> but it was a "signal".

So put that in the bug report.

> >> Of course. But it is completely predictable, not fatal (if we talk
> >> about kernel stability).
> > Predictable != non-fatal.
> So what? Predictable states are much easier to repeat, trace and debug
> than random states.
> 
> > Running out of map entries is always fatal. We need to have them to be
> > able to create them.
> Oh... This is exactly what I try to avoid. If 1 kentry remain -
> allocate more. If can't - return it back, may be next try will
> success.
> 
> >> 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.
> 
> Are you about sysctl or VM_MAP_INTRSAFE flag?

Size of the map. Whichever size you choose, it'll be wrong.

> BTW, there are many
> sysctl values, in misc@ we can see that users play with them in a very
> strange ways. Making their system unusable. So what? Remove that?

Proving that knobification is generally a bad thing.

> > I disagree with the idea itself.

Where I mean the kentry map. I disagree with the kentry map.
As far as I can tell, that is the main idea behind your diff.

> > The real problem is an architectural one.
> 
> Ok, I got it. I hear same again and again, but did not hear that
> someone discussed the design new architecture, so the ideas gets

The architecture in this case being kernel memory defaulting to
pageable, non-intr-safe memory, that we can't allocate memory in
the allocator. :)

> Looks like you mean to say to leave it, and try to be happy with amd64. Sad...

You can always switch to sparc64. ;)

You're not fixing the issue, you're moving it somewhere else.
When kentry map runs out of space, things die (whereas currently, they
only complain loudly).

Your diff limits the memory for the kentry map. For many users you will
create the map too large, starving other kernel tasks from virtual
memory. And for many users you will create it too small, causing their
system to be unable to run programs after a while (if not ceasing to
respond completely). Likely, users will have both behaviours at the
same time.

The big idea behind the current situation is that the kernel_map gets
used for entries that control the intrsafe maps. This is important: the
kernel map is big compared to any intr-safe map. We want to keep that
behaviour. The kernel_map also gets shared with many other subsystems.
KVA not used by vmmap may be put to good use. Once you put it in a map,
no-one can touch it.

Lose the map. Make sure the entries are allocated from kernel_map.
It'd be nice to have a pool.

> accumulated in /dev/null.

Your diff did not accumulate in /dev/null. If it did, you wouldn't
get any response to it. I like that you take an interest in uvm. That's
also why I point out why your diff won't work. You seem enthusiastic,
so I don't think criticism on your diff is wasted on you.
You now have feedback. People don't like your diff, told you why. You
now have options:
1  you can defend the good parts of your diff and poll or work on
   something better for the parts we didn't like
2  you can decide that the idea behind the diff was wrong after all and
   work on something else

You didn't get negative feedback on the reserved entry. It works for
your map, maybe it'll also work for the kernel_map? Most comments here
are against the kentry map. We don't like how it isn't dynamic.
Look at the rejection arguments and distill where you need to improve
your idea.
-- 
Ariane

Reply via email to