| Message ID | 20220119143540.601149-1-urezki@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/3] mm/vmalloc: Move draining areas out of caller context | expand |
On Wed, Jan 19, 2022 at 03:35:38PM +0100, Uladzislau Rezki (Sony) wrote: > +static void drain_vmap_area(struct work_struct *work) Nit, but I prefer to have a _work postix for workers just to keep it easy to ready. > /* After this point, we may free va at any time */ > if (unlikely(nr_lazy > lazy_max_pages())) > - try_purge_vmap_area_lazy(); > + if (!atomic_xchg(&drain_vmap_area_work_in_progress, 1)) > + schedule_work(&drain_vmap_area_work); Work items are defined to be single threaded, so I don't think we need the drain_vmap_area_work_in_progress hack. > > /* > -- > 2.30.2 > > ---end quoted text---
> On Wed, Jan 19, 2022 at 03:35:38PM +0100, Uladzislau Rezki (Sony) wrote: > > +static void drain_vmap_area(struct work_struct *work) > > Nit, but I prefer to have a _work postix for workers just to keep > it easy to ready. > Will fix it! > > /* After this point, we may free va at any time */ > > if (unlikely(nr_lazy > lazy_max_pages())) > > - try_purge_vmap_area_lazy(); > > + if (!atomic_xchg(&drain_vmap_area_work_in_progress, 1)) > > + schedule_work(&drain_vmap_area_work); > > Work items are defined to be single threaded, so I don't think we need > the drain_vmap_area_work_in_progress hack. > The motivation with that hack was to prevent the drain work being placed several times at once, i.e. schedule_work() checks only a pending bit. If the work is in run-queue another caller of vfree() will place it one more time, since pending bit is not set because the work is in TASK_RUNNING state. Or am i missing something? -- Vlad Rezki
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index bdc7222f87d4..ed0f9eaa61a9 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -793,6 +793,9 @@ RB_DECLARE_CALLBACKS_MAX(static, free_vmap_area_rb_augment_cb, static void purge_vmap_area_lazy(void); static BLOCKING_NOTIFIER_HEAD(vmap_notify_list); static unsigned long lazy_max_pages(void); +static void drain_vmap_area(struct work_struct *work); +static DECLARE_WORK(drain_vmap_area_work, drain_vmap_area); +static atomic_t drain_vmap_area_work_in_progress; static atomic_long_t nr_vmalloc_pages; @@ -1719,18 +1722,6 @@ static bool __purge_vmap_area_lazy(unsigned long start, unsigned long end) return true; } -/* - * Kick off a purge of the outstanding lazy areas. Don't bother if somebody - * is already purging. - */ -static void try_purge_vmap_area_lazy(void) -{ - if (mutex_trylock(&vmap_purge_lock)) { - __purge_vmap_area_lazy(ULONG_MAX, 0); - mutex_unlock(&vmap_purge_lock); - } -} - /* * Kick off a purge of the outstanding lazy areas. */ @@ -1742,6 +1733,23 @@ static void purge_vmap_area_lazy(void) mutex_unlock(&vmap_purge_lock); } +static void drain_vmap_area(struct work_struct *work) +{ + unsigned long nr_lazy; + + do { + mutex_lock(&vmap_purge_lock); + __purge_vmap_area_lazy(ULONG_MAX, 0); + mutex_unlock(&vmap_purge_lock); + + /* Recheck if further work is required. */ + nr_lazy = atomic_long_read(&vmap_lazy_nr); + } while (nr_lazy > lazy_max_pages()); + + /* We are done at this point. */ + atomic_set(&drain_vmap_area_work_in_progress, 0); +} + /* * Free a vmap area, caller ensuring that the area has been unmapped * and flush_cache_vunmap had been called for the correct range @@ -1768,7 +1776,8 @@ static void free_vmap_area_noflush(struct vmap_area *va) /* After this point, we may free va at any time */ if (unlikely(nr_lazy > lazy_max_pages())) - try_purge_vmap_area_lazy(); + if (!atomic_xchg(&drain_vmap_area_work_in_progress, 1)) + schedule_work(&drain_vmap_area_work); } /*
A caller initiates the drain procces from its context once the drain threshold is reached or passed. There are at least two drawbacks of doing so: a) a caller can be a high-prio or RT task. In that case it can stuck in doing the actual drain of all lazily freed areas. This is not optimal because such tasks usually are latency sensitive where the control should be returned back as soon as possible in order to drive such workloads in time. See 96e2db456135 ("mm/vmalloc: rework the drain logic") b) It is not safe to call vfree() during holding a spinlock due to the vmap_purge_lock mutex. The was a report about this from Zeal Robot <zealci@zte.com.cn> here: https://lore.kernel.org/all/20211222081026.484058-1-chi.minghao@zte.com.cn Moving the drain to the separate work context addresses those issues. Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com> --- mm/vmalloc.c | 35 ++++++++++++++++++++++------------- 1 file changed, 22 insertions(+), 13 deletions(-)