Re: kmem-pool-uvm

[Mindaugas Rasiukevicius <rmind%netbsd.org@localhost>]

Hello Lars,

Sorry for late reply.

Lars Heidieker <lars%heidieker.de@localhost> wrote:
> >> i uploaded a new version of the kmem-pool-vmem-uvm patch: 
> >> ftp://ftp.netbsd.org/pub/NetBSD/misc/para/kmem-pool-vmem-uvm.patch
> >> 
> <...>
> 
> I've uploaded a new Version of the patch and a small patch that
> replaces malloc with kmem in the ufs/ext2 (mainly) readdir code as
> those allocation were (due to blocksize) 4k which made them just too
> large with the malloc-header for the max 4k caches currently
> implemented in the patch.

Thanks, I think the changes in essence are great and we should have
them for netbsd-6.  Few comments:

- Having IPL_VM lock for kmem(9) can increase masking of interrupts and
  given that we want to reduce allocations from the interrupt context,
  it is not really desirable.  However, there is a long way to go and at
  this point I think it is not a problem, since kmem cache layer will
  minimise the use of the lock.

- As already said, in the long term we want to minimise memory allocations
  from the interrupt context.  With kmem(9) becoming interrupt-safe, there
  is a potential for abuse.  I think that the general purpose kmem_alloc(9)
  and kmem_free(9) routines should explicitly prohibit calls from interrupt
  context by keeping the asserts on !cpu_intr_p() and !cpu_softintr_p().
  For this purpose, there can be e.g. kmem_intr_alloc()/kmem_intr_free(),
  so we would have strict interface level separation preventing for abuse.
  It would also be easier to find such allocations from interrupt-context.

- I would leave uvm_km_alloc/free() interface naming as is and instead of
  heap_va_arena would have something similar to old, e.g. kmem_va_arena.
  Also, perhaps s/heap_meta_arena/kmem_va_meta_arena/.

- From bt_refillglobal:

+       if (vmem_alloc(heap_meta_arena, PAGE_SIZE,
+           (flags & ~VM_FITMASK) | VM_INSTANTFIT | VM_POPULATING, &va) != 0) { 
+               return ENOMEM;
+       }    

Error path leaks vmem_btag_lock.

-- 
Mindaugas