diff mbox series

[1/4] mm: Implement kmem objects freeing queue

Message ID 20190321214512.11524-2-longman@redhat.com (mailing list archive)
State Not Applicable
Headers show
Series Signal: Fix hard lockup problem in flush_sigqueue() | expand

Commit Message

Waiman Long March 21, 2019, 9:45 p.m. UTC
When releasing kernel data structures, freeing up the memory
occupied by those objects is usually the last step. To avoid races,
the release operation is commonly done with a lock held. However, the
freeing operations do not need to be under lock, but are in many cases.

In some complex cases where the locks protect many different memory
objects, that can be a problem especially if some memory debugging
features like KASAN are enabled. In those cases, freeing memory objects
under lock can greatly lengthen the lock hold time. This can even lead
to soft/hard lockups in some extreme cases.

To make it easer to defer freeing memory objects until after unlock,
a kernel memory freeing queue mechanism is now added. It is modelled
after the wake_q mechanism for waking up tasks without holding a lock.

Now kmem_free_q_add() can be called to add memory objects into a freeing
queue. Later on, kmem_free_up_q() can be called to free all the memory
objects in the freeing queue after releasing the lock.

Signed-off-by: Waiman Long <longman@redhat.com>
---
 include/linux/slab.h | 28 ++++++++++++++++++++++++++++
 mm/slab_common.c     | 41 +++++++++++++++++++++++++++++++++++++++++
 2 files changed, 69 insertions(+)

Comments

Christoph Lameter (Ampere) March 22, 2019, 5:47 p.m. UTC | #1
On Thu, 21 Mar 2019, Waiman Long wrote:

> When releasing kernel data structures, freeing up the memory
> occupied by those objects is usually the last step. To avoid races,
> the release operation is commonly done with a lock held. However, the
> freeing operations do not need to be under lock, but are in many cases.
>
> In some complex cases where the locks protect many different memory
> objects, that can be a problem especially if some memory debugging
> features like KASAN are enabled. In those cases, freeing memory objects
> under lock can greatly lengthen the lock hold time. This can even lead
> to soft/hard lockups in some extreme cases.
>
> To make it easer to defer freeing memory objects until after unlock,
> a kernel memory freeing queue mechanism is now added. It is modelled
> after the wake_q mechanism for waking up tasks without holding a lock.

It is already pretty easy. You just store the pointer to the slab object
in a local variable, finish all the unlocks and then free the objects.
This is done in numerous places of the kernel.

I fear that the automated mechanism will make the code more difficult to
read and result in a loss of clarity of the sequencing of events in
releasing locks and objects.

Also there is already kfree_rcu which does a similar thing to what you are
proposing here and is used in numerous places.
diff mbox series

Patch

diff --git a/include/linux/slab.h b/include/linux/slab.h
index 11b45f7ae405..6116fcecbd8f 100644
--- a/include/linux/slab.h
+++ b/include/linux/slab.h
@@ -762,4 +762,32 @@  int slab_dead_cpu(unsigned int cpu);
 #define slab_dead_cpu		NULL
 #endif
 
+/*
+ * Freeing queue node for freeing kmem_cache slab objects later.
+ * The node is put at the beginning of the memory object and so the object
+ * size cannot be smaller than sizeof(kmem_free_q_node).
+ */
+struct kmem_free_q_node {
+	struct kmem_free_q_node *next;
+	struct kmem_cache *cachep;	/* NULL if alloc'ed by kmalloc */
+};
+
+struct kmem_free_q_head {
+	struct kmem_free_q_node *first;
+	struct kmem_free_q_node **lastp;
+};
+
+#define DEFINE_KMEM_FREE_Q(name)	\
+	struct kmem_free_q_head name = { NULL, &name.first }
+
+static inline void kmem_free_q_init(struct kmem_free_q_head *head)
+{
+	head->first = NULL;
+	head->lastp = &head->first;
+}
+
+extern void kmem_free_q_add(struct kmem_free_q_head *head,
+			    struct kmem_cache *cachep, void *object);
+extern void kmem_free_up_q(struct kmem_free_q_head *head);
+
 #endif	/* _LINUX_SLAB_H */
diff --git a/mm/slab_common.c b/mm/slab_common.c
index 03eeb8b7b4b1..dba20b4208f1 100644
--- a/mm/slab_common.c
+++ b/mm/slab_common.c
@@ -1597,6 +1597,47 @@  void kzfree(const void *p)
 }
 EXPORT_SYMBOL(kzfree);
 
+/**
+ * kmem_free_q_add - add a kmem object to a freeing queue
+ * @head: freeing queue head
+ * @cachep: kmem_cache pointer (NULL for kmalloc'ed objects)
+ * @object: kmem object to be freed put into the queue
+ *
+ * Put a kmem object into the freeing queue to be freed later.
+ */
+void kmem_free_q_add(struct kmem_free_q_head *head, struct kmem_cache *cachep,
+		     void *object)
+{
+	struct kmem_free_q_node *node = object;
+
+	WARN_ON_ONCE(cachep && cachep->object_size < sizeof(*node));
+	node->next = NULL;
+	node->cachep = cachep;
+	*(head->lastp) = node;
+	head->lastp = &node->next;
+}
+EXPORT_SYMBOL_GPL(kmem_free_q_add);
+
+/**
+ * kmem_free_up_q - free all the objects in the freeing queue
+ * @head: freeing queue head
+ *
+ * Free all the objects in the freeing queue.
+ */
+void kmem_free_up_q(struct kmem_free_q_head *head)
+{
+	struct kmem_free_q_node *node, *next;
+
+	for (node = head->first; node; node = next) {
+		next = node->next;
+		if (node->cachep)
+			kmem_cache_free(node->cachep, node);
+		else
+			kfree(node);
+	}
+}
+EXPORT_SYMBOL_GPL(kmem_free_up_q);
+
 /* Tracepoints definitions. */
 EXPORT_TRACEPOINT_SYMBOL(kmalloc);
 EXPORT_TRACEPOINT_SYMBOL(kmem_cache_alloc);