On Wed, Apr 06, 2011 at 12:12:35AM +0600, Anton Maksimenkov wrote: > 2011/4/5 Anton Maksimenkov <[email protected]>: > > I have some idea related to allocator for (! __HAVE_PMAP_DIRECT) case. > ... > > My idea is how to resolve that loop by another way. > > We must keep track the number of free kentries. And when we see, in > > uvm_mapent_alloc(), that there is only 1 kentry remains then we must > > allocate more entries immediately. This last kentry may be used by > > uvm_map. So, when it will be done we'll have a page, divide it to new > > fresh entries, and now we safe. > > Then we can proceed further. > > Sorry, I missed the main trick. > There are must be some flag which tells the uvm_mapent_alloc() that it > was called recursively, so in that case it must use and return last > free kentry and not call km_alloc() again. > Furthermore, I think that it must check that if it is called > recursively then splvm() was already done. > > > ?struct vm_map_entry * > > ?uvm_mapent_alloc(struct vm_map *map, int flags) > > ?{ > > - ? ? ? struct vm_map_entry *me, *ne; > > + ? ? ? struct vm_map_entry *me; > ... > if ("recursion flag") { > me = uvm.kentry_free; > if (me == NULL) > panic("uvm_mapent_alloc: uvm.kentry_free is NULL in recursive call"); > uvm.kentry_free = me->next; // NULL > uvm.numof_free_kentries--; // 0 > uvmexp.kmapent++; > me->flags = UVM_MAP_STATIC; > } else if (map->flags & VM_MAP_INTRSAFE || cold) { > > + ? ? ? ? ? ? ? /* > > + ? ? ? ? ? ? ? ?* If there is only one kentry remains we MUST > > + ? ? ? ? ? ? ? ?* allocate more (page of) entries. > > + ? ? ? ? ? ? ? ?* Because uvm_km_kmemalloc (called by uvm_km_alloc) > > + ? ? ? ? ? ? ? ?* may use that last kentry when it tries to uvm_map new > > + ? ? ? ? ? ? ? ?* physical page. > > + ? ? ? ? ? ? ? ?* Then this virtual address of that mapped physpage > > + ? ? ? ? ? ? ? ?* will be returned to us (by uvm_km_alloc) so we can > > + ? ? ? ? ? ? ? ?* allocate more kentries from it and proceed > > + ? ? ? ? ? ? ? ?*/ > > + ? ? ? ? ? ? ? if (uvm.numof_free_kentries == 1 || cold) { > > + ? ? ? ? ? ? ? ? ? ? ? me = km_alloc(PAGE_SIZE, &kv_page, &kp_dirty, > > here setup and pass the "recursion flag" into km_alloc() and it must > pass it into uvm_map() and it must pass it into the > uvm_mapent_alloc(). > > > ? ? ? ? ? ? ? ? ? ? ? ? ? ?&kd_nowait); > > And, as it was pointed by art@, in the km_alloc() the kmem_map must be > used instead of kernel_map. > > Again, excuse me for ugly style of my "diffs"...
Actually, the long-term fix is to make kernel memory interupt safe :D You don't want to recurse in the uvm_km_getpage, it's a hornests nest. It must be interupt safe, spl protected. If you recurse with a flag, you may risk another thread using your "skip my locking etc." flags. While it is not an unsolvable problem, it probably won't improve readability of that code. Also, recurion in the kernel is problematic: we have a very small stack and may run out easily. Could you guarantee that it wouldn't recurse more than once? Ciao, -- Ariane
