Re: Remove uvm hint address selector

2017-01-15 Thread Mark Kettenis
> Date: Sun, 15 Jan 2017 15:06:50 +0100
> From: Stefan Kempf 
> 
> When uvm pivots are enabled, allocations
> with an address hint provided by the user program are
> handled by the uaddr_hint selector.
> 
> I'd like to remove the uaddr_hint selector. The uaddr_rnd selector
> already deals with hinted allocations correctly, so why not use
> it for hinted allocations in the pivot selector?
> 
> This diff is a part of making pivots work.
> Getting rid of the hint selectors makes the code simpler.
> I'd to commit this part first, to make the actual pivot
> diffs smaller.
> 
> ok?

No objection from me.

> Some more details:
> 
> If you have multiple selectors per address space that manage
> overlapping address ranges, some selectors have priority
> over others. E.g. only the brk selector is allowed to make
> allocations within the brk area. The rnd selector
> gets all these checks right, but the hint selector does
> not, which causes panics when you use pivots and try to
> do a hinted allocation within the brk area.
> 
> So better use the code which is known to work for hinted
> allocations. Once pivots are enabled, maybe the parts of the
> rnd allocator that deal with hinted allocations can be factored
> out into a common function.
> 
> Index: uvm/uvm_addr.c
> ===
> RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
> retrieving revision 1.22
> diff -u -p -r1.22 uvm_addr.c
> --- uvm/uvm_addr.c16 Sep 2016 02:50:54 -  1.22
> +++ uvm/uvm_addr.c14 Jan 2017 05:53:17 -
> @@ -46,17 +46,10 @@
>  
>  /* Pool with uvm_addr_state structures. */
>  struct pool uaddr_pool;
> -struct pool uaddr_hint_pool;
>  struct pool uaddr_bestfit_pool;
>  struct pool uaddr_pivot_pool;
>  struct pool uaddr_rnd_pool;
>  
> -/* uvm_addr state for hint based selector. */
> -struct uaddr_hint_state {
> - struct uvm_addr_stateuaddr;
> - vsize_t  max_dist;
> -};
> -
>  /* uvm_addr state for bestfit selector. */
>  struct uaddr_bestfit_state {
>   struct uvm_addr_stateubf_uaddr;
> @@ -118,7 +111,6 @@ void   uaddr_kremove(struct vm_map *,
>  void  uaddr_kbootstrapdestroy(struct uvm_addr_state *);
>  
>  void  uaddr_destroy(struct uvm_addr_state *);
> -void  uaddr_hint_destroy(struct uvm_addr_state *);
>  void  uaddr_kbootstrap_destroy(struct uvm_addr_state *);
>  void  uaddr_rnd_destroy(struct uvm_addr_state *);
>  void  uaddr_bestfit_destroy(struct uvm_addr_state *);
> @@ -138,10 +130,6 @@ int   uaddr_rnd_select(struct vm_map 
> *,
>   struct uvm_addr_state *, struct vm_map_entry **,
>   vaddr_t *, vsize_t, vaddr_t, vaddr_t, vm_prot_t,
>   vaddr_t);
> -int   uaddr_hint_select(struct vm_map *,
> - struct uvm_addr_state*, struct vm_map_entry **,
> - vaddr_t *, vsize_t, vaddr_t, vaddr_t, vm_prot_t,
> - vaddr_t);
>  int   uaddr_bestfit_select(struct vm_map *,
>   struct uvm_addr_state*, struct vm_map_entry **,
>   vaddr_t *, vsize_t, vaddr_t, vaddr_t, vm_prot_t,
> @@ -290,8 +278,6 @@ uvm_addr_init(void)
>  {
>   pool_init(&uaddr_pool, sizeof(struct uvm_addr_state), 0,
>   IPL_VM, PR_WAITOK, "uaddr", NULL);
> - pool_init(&uaddr_hint_pool, sizeof(struct uaddr_hint_state), 0,
> - IPL_VM, PR_WAITOK, "uaddrhint", NULL);
>   pool_init(&uaddr_bestfit_pool, sizeof(struct uaddr_bestfit_state), 0,
>   IPL_VM, PR_WAITOK, "uaddrbest", NULL);
>   pool_init(&uaddr_pivot_pool, sizeof(struct uaddr_pivot_state), 0,
> @@ -740,116 +726,6 @@ uaddr_rnd_print(struct uvm_addr_state *u
>  #endif
>  
>  /*
> - * An allocator that selects an address within distance of the hint.
> - *
> - * If no hint is given, the allocator refuses to allocate.
> - */
> -const struct uvm_addr_functions uaddr_hint_functions = {
> - .uaddr_select = &uaddr_hint_select,
> - .uaddr_destroy = &uaddr_hint_destroy,
> - .uaddr_name = "uaddr_hint"
> -};
> -
> -/*
> - * Create uaddr_hint state.
> - */
> -struct uvm_addr_state *
> -uaddr_hint_create(vaddr_t minaddr, vaddr_t maxaddr, vsize_t max_dist)
> -{
> - struct uaddr_hint_state *ua_hint;
> -
> - KASSERT(uaddr_hint_pool.pr_size == sizeof(*ua_hint));
> -
> - ua_hint = pool_get(&uaddr_hint_pool, PR_WAITOK);
> - ua_hint->uaddr.uaddr_minaddr = minaddr;
> - ua_hint->uaddr.uaddr_maxaddr = maxaddr;
> - ua_hint->uaddr.uaddr_functions = &uaddr_hint_functions;
> - ua_hint->max_dist = max_dist;
> - return &ua_hint->uaddr;
> -}
> -
> -/*
> - * Destroy uaddr_hint state.
> - */
> -void
> -uaddr_hint_destroy(struct uvm_addr_state *uaddr)
> -{
> - pool_put(&uaddr_hint_pool, uaddr);
> -}
> -
> 

Remove uvm hint address selector

2017-01-15 Thread Stefan Kempf
When uvm pivots are enabled, allocations
with an address hint provided by the user program are
handled by the uaddr_hint selector.

I'd like to remove the uaddr_hint selector. The uaddr_rnd selector
already deals with hinted allocations correctly, so why not use
it for hinted allocations in the pivot selector?

This diff is a part of making pivots work.
Getting rid of the hint selectors makes the code simpler.
I'd to commit this part first, to make the actual pivot
diffs smaller.

ok?

Some more details:

If you have multiple selectors per address space that manage
overlapping address ranges, some selectors have priority
over others. E.g. only the brk selector is allowed to make
allocations within the brk area. The rnd selector
gets all these checks right, but the hint selector does
not, which causes panics when you use pivots and try to
do a hinted allocation within the brk area.

So better use the code which is known to work for hinted
allocations. Once pivots are enabled, maybe the parts of the
rnd allocator that deal with hinted allocations can be factored
out into a common function.

Index: uvm/uvm_addr.c
===
RCS file: /cvs/src/sys/uvm/uvm_addr.c,v
retrieving revision 1.22
diff -u -p -r1.22 uvm_addr.c
--- uvm/uvm_addr.c  16 Sep 2016 02:50:54 -  1.22
+++ uvm/uvm_addr.c  14 Jan 2017 05:53:17 -
@@ -46,17 +46,10 @@
 
 /* Pool with uvm_addr_state structures. */
 struct pool uaddr_pool;
-struct pool uaddr_hint_pool;
 struct pool uaddr_bestfit_pool;
 struct pool uaddr_pivot_pool;
 struct pool uaddr_rnd_pool;
 
-/* uvm_addr state for hint based selector. */
-struct uaddr_hint_state {
-   struct uvm_addr_stateuaddr;
-   vsize_t  max_dist;
-};
-
 /* uvm_addr state for bestfit selector. */
 struct uaddr_bestfit_state {
struct uvm_addr_stateubf_uaddr;
@@ -118,7 +111,6 @@ void uaddr_kremove(struct vm_map *,
 voiduaddr_kbootstrapdestroy(struct uvm_addr_state *);
 
 voiduaddr_destroy(struct uvm_addr_state *);
-voiduaddr_hint_destroy(struct uvm_addr_state *);
 voiduaddr_kbootstrap_destroy(struct uvm_addr_state *);
 voiduaddr_rnd_destroy(struct uvm_addr_state *);
 voiduaddr_bestfit_destroy(struct uvm_addr_state *);
@@ -138,10 +130,6 @@ int uaddr_rnd_select(struct vm_map 
*,
struct uvm_addr_state *, struct vm_map_entry **,
vaddr_t *, vsize_t, vaddr_t, vaddr_t, vm_prot_t,
vaddr_t);
-int uaddr_hint_select(struct vm_map *,
-   struct uvm_addr_state*, struct vm_map_entry **,
-   vaddr_t *, vsize_t, vaddr_t, vaddr_t, vm_prot_t,
-   vaddr_t);
 int uaddr_bestfit_select(struct vm_map *,
struct uvm_addr_state*, struct vm_map_entry **,
vaddr_t *, vsize_t, vaddr_t, vaddr_t, vm_prot_t,
@@ -290,8 +278,6 @@ uvm_addr_init(void)
 {
pool_init(&uaddr_pool, sizeof(struct uvm_addr_state), 0,
IPL_VM, PR_WAITOK, "uaddr", NULL);
-   pool_init(&uaddr_hint_pool, sizeof(struct uaddr_hint_state), 0,
-   IPL_VM, PR_WAITOK, "uaddrhint", NULL);
pool_init(&uaddr_bestfit_pool, sizeof(struct uaddr_bestfit_state), 0,
IPL_VM, PR_WAITOK, "uaddrbest", NULL);
pool_init(&uaddr_pivot_pool, sizeof(struct uaddr_pivot_state), 0,
@@ -740,116 +726,6 @@ uaddr_rnd_print(struct uvm_addr_state *u
 #endif
 
 /*
- * An allocator that selects an address within distance of the hint.
- *
- * If no hint is given, the allocator refuses to allocate.
- */
-const struct uvm_addr_functions uaddr_hint_functions = {
-   .uaddr_select = &uaddr_hint_select,
-   .uaddr_destroy = &uaddr_hint_destroy,
-   .uaddr_name = "uaddr_hint"
-};
-
-/*
- * Create uaddr_hint state.
- */
-struct uvm_addr_state *
-uaddr_hint_create(vaddr_t minaddr, vaddr_t maxaddr, vsize_t max_dist)
-{
-   struct uaddr_hint_state *ua_hint;
-
-   KASSERT(uaddr_hint_pool.pr_size == sizeof(*ua_hint));
-
-   ua_hint = pool_get(&uaddr_hint_pool, PR_WAITOK);
-   ua_hint->uaddr.uaddr_minaddr = minaddr;
-   ua_hint->uaddr.uaddr_maxaddr = maxaddr;
-   ua_hint->uaddr.uaddr_functions = &uaddr_hint_functions;
-   ua_hint->max_dist = max_dist;
-   return &ua_hint->uaddr;
-}
-
-/*
- * Destroy uaddr_hint state.
- */
-void
-uaddr_hint_destroy(struct uvm_addr_state *uaddr)
-{
-   pool_put(&uaddr_hint_pool, uaddr);
-}
-
-/*
- * Hint selector.
- *
- * Attempts to find an address that is within max_dist of the hint.
- */
-int
-uaddr_hint_select(struct vm_map *map, struct uvm_addr_state *uaddr_param,
-struct vm_map_entry **entry_out, vaddr_t *addr_out,
-vsize_t sz,