Hello Vlad,
On 12/28/21 20:45, Uladzislau Rezki wrote:
[...]
Manfred, could you please have a look and if you have a time test it?
I mean if it solves your issue. You can take over this patch and resend
it, otherwise i can send it myself later if we all agree with it.
I think we mix tasks: We have a bug in ipc/sem.c, thus we need a
solution suitable for stable.
Fixes: fc37a3b8b438 ("[PATCH] ipc sem: use kvmalloc for sem_undo
allocation")
Cc: stable@xxxxxxxxxxxxxxx
I think for stable, there are only two options:
- change ipc/sem.c, call kvfree() after dropping the spinlock
- change kvfree() to use vfree_atomic().
From my point of view, both approaches are fine.
I.e. I'm waiting for feedback from an mm maintainer.
As soon as it is agreed, I will retest the chosen solution.
Now you propose to redesign vfree(), so that vfree() is safe to be
called while holding spinlocks:
<snip>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index d2a00ad4e1dd..b82db44fea60 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1717,17 +1717,10 @@ 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);
- }
-}
+static void purge_vmap_area_lazy(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;
/*
* Kick off a purge of the outstanding lazy areas.
@@ -1740,6 +1733,22 @@ static void purge_vmap_area_lazy(void)
mutex_unlock(&vmap_purge_lock);
}
+static void drain_vmap_area(struct work_struct *work)
+{
+ mutex_lock(&vmap_purge_lock);
+ __purge_vmap_area_lazy(ULONG_MAX, 0);
+ mutex_unlock(&vmap_purge_lock);
+
+ /*
+ * Check if rearming is still required. If not, we are
+ * done and can let a next caller to initiate a new drain.
+ */
+ if (atomic_long_read(&vmap_lazy_nr) > lazy_max_pages())
+ schedule_work(&drain_vmap_area_work);
+ else
+ 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
@@ -1766,7 +1775,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);
}
/*
<snip>
I do now know the mm code well enough to understand the side effects of
the change. And doubt that it is suitable for stable, i.e. we need the
simple patch first.
--
Manfred
