Re: [PATCH v4 2/3] drm/i915: refactor duplicate object vmap functions (reworked)

[Tvrtko Ursulin <tvrtko.ursulin@linux.intel.com>]

On 23/02/16 11:52, Chris Wilson wrote:
> On Tue, Feb 23, 2016 at 11:38:02AM +0000, Chris Wilson wrote:
>> Indeed, we would need a new notifier, pretty much for the sole use of
>> 32b. Grr, that will be a nuisance.
>
> Core changes:
>
> diff --git a/include/linux/vmalloc.h b/include/linux/vmalloc.h
> index d1f1d338af20..542a91c2785f 100644
> --- a/include/linux/vmalloc.h
> +++ b/include/linux/vmalloc.h
> @@ -187,4 +187,8 @@ pcpu_free_vm_areas(struct vm_struct **vms, int nr_vms)
>   #define VMALLOC_TOTAL 0UL
>   #endif
>
> +struct notitifer_block;
> +int register_vmap_notifier(struct notifier_block *nb);
> +int unregister_vmap_notifier(struct notifier_block *nb);
> +
>   #endif /* _LINUX_VMALLOC_H */
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index fb42a5bffe47..0735d82444f7 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -21,6 +21,7 @@
>   #include <linux/debugobjects.h>
>   #include <linux/kallsyms.h>
>   #include <linux/list.h>
> +#include <linux/notifier.h>
>   #include <linux/rbtree.h>
>   #include <linux/radix-tree.h>
>   #include <linux/rcupdate.h>
> @@ -344,6 +345,8 @@ static void __insert_vmap_area(struct vmap_area *va)
>
>   static void purge_vmap_area_lazy(void);
>
> +static BLOCKING_NOTIFIER_HEAD(vmap_notify_list);
> +
>   /*
>    * Allocate a region of KVA of the specified size and alignment, within the
>    * vstart and vend.
> @@ -356,6 +359,7 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
>          struct vmap_area *va;
>          struct rb_node *n;
>          unsigned long addr;
> +       unsigned long freed;
>          int purged = 0;
>          struct vmap_area *first;
>
> @@ -468,6 +472,12 @@ overflow:
>                  purged = 1;
>                  goto retry;
>          }
> +       freed = 0;
> +       blocking_notifier_call_chain(&vmap_notify_list, 0, &freed);
> +       if (freed > 0) {
> +               purged = 0;
> +               goto retry;
> +       }
>          if (printk_ratelimit())
>                  pr_warn("vmap allocation for size %lu failed: "
>                          "use vmalloc=<size> to increase size.\n", size);
> @@ -475,6 +485,18 @@ overflow:
>          return ERR_PTR(-EBUSY);
>   }
>
> +int register_vmap_notifier(struct notifier_block *nb)
> +{
> +       return blocking_notifier_chain_register(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(register_vmap_notifier);
> +
> +int unregister_vmap_notifier(struct notifier_block *nb)
> +{
> +       return blocking_notifier_chain_unregister(&vmap_notify_list, nb);
> +}
> +EXPORT_SYMBOL_GPL(unregister_vmap_notifier);
> +
>   static void __free_vmap_area(struct vmap_area *va)
>   {
>          BUG_ON(RB_EMPTY_NODE(&va->rb_node));
>
>
> That doesn't look too horrendous. Names?

Downside is new deadlock opportunity so GEM callers to vmap would have 
to release the struct mutex.

> register_oovmap_notifier
> register_vmap_nospace_notifier?
> register_vmap_purge_notifier?

register_vmap_shrinker ?

> And the 64k question, how to sell it?

Not sure, maybe with numbers showing that caching the vmapping helps us 
significantly?

Regards,

Tvrtko
_______________________________________________
Intel-gfx mailing list
Intel-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/intel-gfx