[PATCH v1 0/6] Introduce kvfree_rcu() logic

[PATCH v1 0/6] Introduce kvfree_rcu() logic

[Uladzislau Rezki (Sony)]

This small series introduces kvfree_rcu() API. An interface is the
same as kfree_rcu(), i.e. a structure that is about to be freed,
after GP, has to embed an "rcu_head" inside. Currently we have one
user that is mm/list_lru.c, but there was also request/interest
so there will be new comers.

Also there was a discussion about having kvfree_rcu() but head-less
variant. So this series is also a way forward to it and next step is
to introduce it. For example ext4 needs it.

It is based on dev.2020.03.11b branch.

Uladzislau Rezki (Sony) (6):
  mm/list_lru.c: rename kvfree_rcu() to local variant
  rcu: introduce kvfree_rcu() interface
  rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback
  rcu: rename __is_kfree_rcu_offset() macro
  rcu: rename kfree_call_rcu()/__kfree_rcu()
  mm/list_lru.c: remove kvfree_rcu_local() function

 include/linux/rcupdate.h   | 23 ++++++++++++++++-------
 include/linux/rcutiny.h    |  2 +-
 include/linux/rcutree.h    |  2 +-
 include/trace/events/rcu.h |  8 ++++----
 kernel/rcu/tiny.c          |  7 ++++---
 kernel/rcu/tree.c          | 33 ++++++++++++++++++++-------------
 mm/list_lru.c              | 11 ++---------
 7 files changed, 48 insertions(+), 38 deletions(-)

-- 
2.20.1

[PATCH v1 1/6] mm/list_lru.c: rename kvfree_rcu() to local variant

[Uladzislau Rezki (Sony)]

Rename kvfree_rcu() function to the kvfree_rcu_local()
one. The aim is to introduce the public API that would
conflict with this one.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 mm/list_lru.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/mm/list_lru.c b/mm/list_lru.c
index 0f1f6b06b7f3..386424688f80 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -383,14 +383,14 @@ static void memcg_destroy_list_lru_node(struct list_lru_node *nlru)
 	struct list_lru_memcg *memcg_lrus;
 	/*
 	 * This is called when shrinker has already been unregistered,
-	 * and nobody can use it. So, there is no need to use kvfree_rcu().
+	 * and nobody can use it. So, there is no need to use kvfree_rcu_local().
 	 */
 	memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, true);
 	__memcg_destroy_list_lru_node(memcg_lrus, 0, memcg_nr_cache_ids);
 	kvfree(memcg_lrus);
 }
 
-static void kvfree_rcu(struct rcu_head *head)
+static void kvfree_rcu_local(struct rcu_head *head)
 {
 	struct list_lru_memcg *mlru;
 
@@ -429,7 +429,7 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru,
 	rcu_assign_pointer(nlru->memcg_lrus, new);
 	spin_unlock_irq(&nlru->lock);
 
-	call_rcu(&old->rcu, kvfree_rcu);
+	call_rcu(&old->rcu, kvfree_rcu_local);
 	return 0;
 }
 
-- 
2.20.1

[PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Uladzislau Rezki (Sony)]

kvfree_rcu() can deal with an allocated memory that is obtained
via kvmalloc(). It can return two types of allocated memory or
"pointers", one can belong to regular SLAB allocator and another
one can be vmalloc one. It depends on requested size and memory
pressure.

Based on that, two streams are split, thus if a pointer belongs
to vmalloc allocator it is queued to the list, otherwise SLAB
one is queued into "bulk array" for further processing.

The main reason of such splitting is:
    a) to distinguish kmalloc()/vmalloc() ptrs;
    b) there is no vmalloc_bulk() interface.

As of now we have list_lru.c user that needs such interface,
also there will be new comers. Apart of that it is preparation
to have a head-less variant later.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 include/linux/rcupdate.h |  9 +++++++++
 kernel/rcu/tiny.c        |  3 ++-
 kernel/rcu/tree.c        | 17 ++++++++++++-----
 3 files changed, 23 insertions(+), 6 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index 2be97a83f266..bb270221dbdc 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -845,6 +845,15 @@ do {									\
 		__kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
 } while (0)
 
+/**
+ * kvfree_rcu() - kvfree an object after a grace period.
+ * @ptr:	pointer to kvfree
+ * @rhf:	the name of the struct rcu_head within the type of @ptr.
+ *
+ * Same as kfree_rcu(), just simple alias.
+ */
+#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
+
 /*
  * Place this after a lock-acquisition primitive to guarantee that
  * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index dd572ce7c747..4b99f7b88bee 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -23,6 +23,7 @@
 #include <linux/cpu.h>
 #include <linux/prefetch.h>
 #include <linux/slab.h>
+#include <linux/mm.h>
 
 #include "rcu.h"
 
@@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
 	rcu_lock_acquire(&rcu_callback_map);
 	if (__is_kfree_rcu_offset(offset)) {
 		trace_rcu_invoke_kfree_callback("", head, offset);
-		kfree((void *)head - offset);
+		kvfree((void *)head - offset);
 		rcu_lock_release(&rcu_callback_map);
 		return true;
 	}
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 2f4c91a3713a..1c0a73616872 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
 	}
 
 	/*
-	 * Emergency case only. It can happen under low memory
-	 * condition when an allocation gets failed, so the "bulk"
-	 * path can not be temporary maintained.
+	 * vmalloc() pointers end up here also emergency case. It can
+	 * happen under low memory condition when an allocation gets
+	 * failed, so the "bulk" path can not be temporary maintained.
 	 */
 	for (; head; head = next) {
 		unsigned long offset = (unsigned long)head->func;
@@ -2912,7 +2912,7 @@ static void kfree_rcu_work(struct work_struct *work)
 		trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
 
 		if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
-			kfree((void *)head - offset);
+			kvfree((void *)head - offset);
 
 		rcu_lock_release(&rcu_callback_map);
 		cond_resched_tasks_rcu_qs();
@@ -3084,10 +3084,17 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
 	}
 
 	/*
+	 * We do not queue vmalloc pointers into array,
+	 * instead they are just queued to the list. We
+	 * do it because of:
+	 *    a) to distinguish kmalloc()/vmalloc() ptrs;
+	 *    b) there is no vmalloc_bulk() interface.
+	 *
 	 * Under high memory pressure GFP_NOWAIT can fail,
 	 * in that case the emergency path is maintained.
 	 */
-	if (unlikely(!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func))) {
+	if (is_vmalloc_addr((void *) head - (unsigned long) func) ||
+			!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func)) {
 		head->func = func;
 		head->next = krcp->head;
 		krcp->head = head;
-- 
2.20.1

[PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback

[Uladzislau Rezki (Sony)]

Rename rcu_invoke_kfree_callback to rcu_invoke_kvfree_callback.
Do the same with second trace event, that is rcu_kfree_callback,
it becomes rcu_kvfree_callback. The reason is to be aligned with
kvfree notation.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 include/trace/events/rcu.h | 8 ++++----
 kernel/rcu/tiny.c          | 2 +-
 kernel/rcu/tree.c          | 4 ++--
 3 files changed, 7 insertions(+), 7 deletions(-)

diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
index f9a7811148e2..0ee93d0b1daa 100644
--- a/include/trace/events/rcu.h
+++ b/include/trace/events/rcu.h
@@ -506,13 +506,13 @@ TRACE_EVENT_RCU(rcu_callback,
 
 /*
  * Tracepoint for the registration of a single RCU callback of the special
- * kfree() form.  The first argument is the RCU type, the second argument
+ * kvfree() form.  The first argument is the RCU type, the second argument
  * is a pointer to the RCU callback, the third argument is the offset
  * of the callback within the enclosing RCU-protected data structure,
  * the fourth argument is the number of lazy callbacks queued, and the
  * fifth argument is the total number of callbacks queued.
  */
-TRACE_EVENT_RCU(rcu_kfree_callback,
+TRACE_EVENT_RCU(rcu_kvfree_callback,
 
 	TP_PROTO(const char *rcuname, struct rcu_head *rhp, unsigned long offset,
 		 long qlen),
@@ -596,12 +596,12 @@ TRACE_EVENT_RCU(rcu_invoke_callback,
 
 /*
  * Tracepoint for the invocation of a single RCU callback of the special
- * kfree() form.  The first argument is the RCU flavor, the second
+ * kvfree() form.  The first argument is the RCU flavor, the second
  * argument is a pointer to the RCU callback, and the third argument
  * is the offset of the callback within the enclosing RCU-protected
  * data structure.
  */
-TRACE_EVENT_RCU(rcu_invoke_kfree_callback,
+TRACE_EVENT_RCU(rcu_invoke_kvfree_callback,
 
 	TP_PROTO(const char *rcuname, struct rcu_head *rhp, unsigned long offset),
 
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index 4b99f7b88bee..3dd8e6e207b0 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -86,7 +86,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
 
 	rcu_lock_acquire(&rcu_callback_map);
 	if (__is_kfree_rcu_offset(offset)) {
-		trace_rcu_invoke_kfree_callback("", head, offset);
+		trace_rcu_invoke_kvfree_callback("", head, offset);
 		kvfree((void *)head - offset);
 		rcu_lock_release(&rcu_callback_map);
 		return true;
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 1c0a73616872..eef75cd210fd 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2720,7 +2720,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
 	// If no-CBs CPU gets here, rcu_nocb_try_bypass() acquired ->nocb_lock.
 	rcu_segcblist_enqueue(&rdp->cblist, head);
 	if (__is_kfree_rcu_offset((unsigned long)func))
-		trace_rcu_kfree_callback(rcu_state.name, head,
+		trace_rcu_kvfree_callback(rcu_state.name, head,
 					 (unsigned long)func,
 					 rcu_segcblist_n_cbs(&rdp->cblist));
 	else
@@ -2909,7 +2909,7 @@ static void kfree_rcu_work(struct work_struct *work)
 		next = head->next;
 		debug_rcu_head_unqueue(head);
 		rcu_lock_acquire(&rcu_callback_map);
-		trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
+		trace_rcu_invoke_kvfree_callback(rcu_state.name, head, offset);
 
 		if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
 			kvfree((void *)head - offset);
-- 
2.20.1

[PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro

[Uladzislau Rezki (Sony)]

Rename __is_kfree_rcu_offset to __is_kvfree_rcu_offset.
All RCU paths use kvfree() now instead of kfree(), thus
rename it.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 include/linux/rcupdate.h | 6 +++---
 kernel/rcu/tiny.c        | 2 +-
 kernel/rcu/tree.c        | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index bb270221dbdc..e4961631a44f 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -798,16 +798,16 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 
 /*
  * Does the specified offset indicate that the corresponding rcu_head
- * structure can be handled by kfree_rcu()?
+ * structure can be handled by kvfree_rcu()?
  */
-#define __is_kfree_rcu_offset(offset) ((offset) < 4096)
+#define __is_kvfree_rcu_offset(offset) ((offset) < 4096)
 
 /*
  * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
  */
 #define __kfree_rcu(head, offset) \
 	do { \
-		BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
+		BUILD_BUG_ON(!__is_kvfree_rcu_offset(offset)); \
 		kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
 	} while (0)
 
diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
index 3dd8e6e207b0..aa897c3f2e92 100644
--- a/kernel/rcu/tiny.c
+++ b/kernel/rcu/tiny.c
@@ -85,7 +85,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
 	unsigned long offset = (unsigned long)head->func;
 
 	rcu_lock_acquire(&rcu_callback_map);
-	if (__is_kfree_rcu_offset(offset)) {
+	if (__is_kvfree_rcu_offset(offset)) {
 		trace_rcu_invoke_kvfree_callback("", head, offset);
 		kvfree((void *)head - offset);
 		rcu_lock_release(&rcu_callback_map);
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index eef75cd210fd..bb9544238396 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2719,7 +2719,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
 		return; // Enqueued onto ->nocb_bypass, so just leave.
 	// If no-CBs CPU gets here, rcu_nocb_try_bypass() acquired ->nocb_lock.
 	rcu_segcblist_enqueue(&rdp->cblist, head);
-	if (__is_kfree_rcu_offset((unsigned long)func))
+	if (__is_kvfree_rcu_offset((unsigned long)func))
 		trace_rcu_kvfree_callback(rcu_state.name, head,
 					 (unsigned long)func,
 					 rcu_segcblist_n_cbs(&rdp->cblist));
@@ -2911,7 +2911,7 @@ static void kfree_rcu_work(struct work_struct *work)
 		rcu_lock_acquire(&rcu_callback_map);
 		trace_rcu_invoke_kvfree_callback(rcu_state.name, head, offset);
 
-		if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
+		if (!WARN_ON_ONCE(!__is_kvfree_rcu_offset(offset)))
 			kvfree((void *)head - offset);
 
 		rcu_lock_release(&rcu_callback_map);
-- 
2.20.1

[PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Uladzislau Rezki (Sony)]

Rename kfree_call_rcu() to the kvfree_call_rcu().
The reason is, it is capable of freeing vmalloc()
memory now.

Do the same with __kfree_rcu() macro, it becomes
__kvfree_rcu(), the reason is the same as pointed
above.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 include/linux/rcupdate.h | 8 ++++----
 include/linux/rcutiny.h  | 2 +-
 include/linux/rcutree.h  | 2 +-
 kernel/rcu/tree.c        | 8 ++++----
 4 files changed, 10 insertions(+), 10 deletions(-)

diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
index e4961631a44f..6c660fa1f551 100644
--- a/include/linux/rcupdate.h
+++ b/include/linux/rcupdate.h
@@ -805,10 +805,10 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
 /*
  * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
  */
-#define __kfree_rcu(head, offset) \
+#define __kvfree_rcu(head, offset) \
 	do { \
 		BUILD_BUG_ON(!__is_kvfree_rcu_offset(offset)); \
-		kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
+		kvfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
 	} while (0)
 
 /**
@@ -827,7 +827,7 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
  * Because the functions are not allowed in the low-order 4096 bytes of
  * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
  * If the offset is larger than 4095 bytes, a compile-time error will
- * be generated in __kfree_rcu().  If this error is triggered, you can
+ * be generated in __kvfree_rcu(). If this error is triggered, you can
  * either fall back to use of call_rcu() or rearrange the structure to
  * position the rcu_head structure into the first 4096 bytes.
  *
@@ -842,7 +842,7 @@ do {									\
 	typeof (ptr) ___p = (ptr);					\
 									\
 	if (___p)							\
-		__kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
+		__kvfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
 } while (0)
 
 /**
diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
index 045c28b71f4f..4cae3dd77173 100644
--- a/include/linux/rcutiny.h
+++ b/include/linux/rcutiny.h
@@ -34,7 +34,7 @@ static inline void synchronize_rcu_expedited(void)
 	synchronize_rcu();
 }
 
-static inline void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
+static inline void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
 {
 	call_rcu(head, func);
 }
diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
index 45f3f66bb04d..3a7829d69fef 100644
--- a/include/linux/rcutree.h
+++ b/include/linux/rcutree.h
@@ -33,7 +33,7 @@ static inline void rcu_virt_note_context_switch(int cpu)
 }
 
 void synchronize_rcu_expedited(void);
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
+void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
 
 void rcu_barrier(void);
 bool rcu_eqs_special_set(int cpu);
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index bb9544238396..19e6cb970c38 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -3054,18 +3054,18 @@ kfree_call_rcu_add_ptr_to_bulk(struct kfree_rcu_cpu *krcp,
 }
 
 /*
- * Queue a request for lazy invocation of kfree_bulk()/kfree() after a grace
+ * Queue a request for lazy invocation of kfree_bulk()/kvfree() after a grace
  * period. Please note there are two paths are maintained, one is the main one
  * that uses kfree_bulk() interface and second one is emergency one, that is
  * used only when the main path can not be maintained temporary, due to memory
  * pressure.
  *
- * Each kfree_call_rcu() request is added to a batch. The batch will be drained
+ * Each kvfree_call_rcu() request is added to a batch. The batch will be drained
  * every KFREE_DRAIN_JIFFIES number of jiffies. All the objects in the batch will
  * be free'd in workqueue context. This allows us to: batch requests together to
  * reduce the number of grace periods during heavy kfree_rcu() load.
  */
-void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
+void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
 {
 	unsigned long flags;
 	struct kfree_rcu_cpu *krcp;
@@ -3112,7 +3112,7 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
 		spin_unlock(&krcp->lock);
 	local_irq_restore(flags);
 }
-EXPORT_SYMBOL_GPL(kfree_call_rcu);
+EXPORT_SYMBOL_GPL(kvfree_call_rcu);
 
 void __init kfree_rcu_scheduler_running(void)
 {
-- 
2.20.1

[PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function

[Uladzislau Rezki (Sony)]

Since there is newly introduced kvfree_rcu() API,
there is no need in queuing and using call_rcu()
to kvfree() an object after the GP.

Remove kvfree_rcu_local() function and replace
call_rcu() by new kvfree_rcu() API that does
the same but in more efficient way.

Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
---
 mm/list_lru.c | 13 +++----------
 1 file changed, 3 insertions(+), 10 deletions(-)

diff --git a/mm/list_lru.c b/mm/list_lru.c
index 386424688f80..69becdb22408 100644
--- a/mm/list_lru.c
+++ b/mm/list_lru.c
@@ -12,6 +12,7 @@
 #include <linux/slab.h>
 #include <linux/mutex.h>
 #include <linux/memcontrol.h>
+#include <linux/rcupdate.h>
 #include "slab.h"
 
 #ifdef CONFIG_MEMCG_KMEM
@@ -383,21 +384,13 @@ static void memcg_destroy_list_lru_node(struct list_lru_node *nlru)
 	struct list_lru_memcg *memcg_lrus;
 	/*
 	 * This is called when shrinker has already been unregistered,
-	 * and nobody can use it. So, there is no need to use kvfree_rcu_local().
+	 * and nobody can use it. So, there is no need to use kvfree_rcu().
 	 */
 	memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, true);
 	__memcg_destroy_list_lru_node(memcg_lrus, 0, memcg_nr_cache_ids);
 	kvfree(memcg_lrus);
 }
 
-static void kvfree_rcu_local(struct rcu_head *head)
-{
-	struct list_lru_memcg *mlru;
-
-	mlru = container_of(head, struct list_lru_memcg, rcu);
-	kvfree(mlru);
-}
-
 static int memcg_update_list_lru_node(struct list_lru_node *nlru,
 				      int old_size, int new_size)
 {
@@ -429,7 +422,7 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru,
 	rcu_assign_pointer(nlru->memcg_lrus, new);
 	spin_unlock_irq(&nlru->lock);
 
-	call_rcu(&old->rcu, kvfree_rcu_local);
+	kvfree_rcu(old, rcu);
 	return 0;
 }
 
-- 
2.20.1

Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Joel Fernandes]

On Sun, Mar 15, 2020 at 07:18:39PM +0100, Uladzislau Rezki (Sony) wrote:
> Rename kfree_call_rcu() to the kvfree_call_rcu().
> The reason is, it is capable of freeing vmalloc()
> memory now.
> 
> Do the same with __kfree_rcu() macro, it becomes
> __kvfree_rcu(), the reason is the same as pointed
> above.

Vlad, this patch does not apply to my branch that I shared with you. Sorry if
I was not clear earlier, could we work on the same branch to avoid conflicts?

I based the kfree_rcu shrinker patches on an 'rcu/kfree' branch in my git
tree: https://github.com/joelagnel/linux-kernel/tree/rcu/kfree

For now I manually applied 5/6. All others applied cleanly.

Updated the tree as I continue to review your patches.

thanks,

 - Joel


> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  include/linux/rcupdate.h | 8 ++++----
>  include/linux/rcutiny.h  | 2 +-
>  include/linux/rcutree.h  | 2 +-
>  kernel/rcu/tree.c        | 8 ++++----
>  4 files changed, 10 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index e4961631a44f..6c660fa1f551 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -805,10 +805,10 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>  /*
>   * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
>   */
> -#define __kfree_rcu(head, offset) \
> +#define __kvfree_rcu(head, offset) \
>  	do { \
>  		BUILD_BUG_ON(!__is_kvfree_rcu_offset(offset)); \
> -		kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
> +		kvfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
>  	} while (0)
>  
>  /**
> @@ -827,7 +827,7 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>   * Because the functions are not allowed in the low-order 4096 bytes of
>   * kernel virtual memory, offsets up to 4095 bytes can be accommodated.
>   * If the offset is larger than 4095 bytes, a compile-time error will
> - * be generated in __kfree_rcu().  If this error is triggered, you can
> + * be generated in __kvfree_rcu(). If this error is triggered, you can
>   * either fall back to use of call_rcu() or rearrange the structure to
>   * position the rcu_head structure into the first 4096 bytes.
>   *
> @@ -842,7 +842,7 @@ do {									\
>  	typeof (ptr) ___p = (ptr);					\
>  									\
>  	if (___p)							\
> -		__kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
> +		__kvfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
>  } while (0)
>  
>  /**
> diff --git a/include/linux/rcutiny.h b/include/linux/rcutiny.h
> index 045c28b71f4f..4cae3dd77173 100644
> --- a/include/linux/rcutiny.h
> +++ b/include/linux/rcutiny.h
> @@ -34,7 +34,7 @@ static inline void synchronize_rcu_expedited(void)
>  	synchronize_rcu();
>  }
>  
> -static inline void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> +static inline void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
>  {
>  	call_rcu(head, func);
>  }
> diff --git a/include/linux/rcutree.h b/include/linux/rcutree.h
> index 45f3f66bb04d..3a7829d69fef 100644
> --- a/include/linux/rcutree.h
> +++ b/include/linux/rcutree.h
> @@ -33,7 +33,7 @@ static inline void rcu_virt_note_context_switch(int cpu)
>  }
>  
>  void synchronize_rcu_expedited(void);
> -void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
> +void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func);
>  
>  void rcu_barrier(void);
>  bool rcu_eqs_special_set(int cpu);
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index bb9544238396..19e6cb970c38 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -3054,18 +3054,18 @@ kfree_call_rcu_add_ptr_to_bulk(struct kfree_rcu_cpu *krcp,
>  }
>  
>  /*
> - * Queue a request for lazy invocation of kfree_bulk()/kfree() after a grace
> + * Queue a request for lazy invocation of kfree_bulk()/kvfree() after a grace
>   * period. Please note there are two paths are maintained, one is the main one
>   * that uses kfree_bulk() interface and second one is emergency one, that is
>   * used only when the main path can not be maintained temporary, due to memory
>   * pressure.
>   *
> - * Each kfree_call_rcu() request is added to a batch. The batch will be drained
> + * Each kvfree_call_rcu() request is added to a batch. The batch will be drained
>   * every KFREE_DRAIN_JIFFIES number of jiffies. All the objects in the batch will
>   * be free'd in workqueue context. This allows us to: batch requests together to
>   * reduce the number of grace periods during heavy kfree_rcu() load.
>   */
> -void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
> +void kvfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
>  {
>  	unsigned long flags;
>  	struct kfree_rcu_cpu *krcp;
> @@ -3112,7 +3112,7 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
>  		spin_unlock(&krcp->lock);
>  	local_irq_restore(flags);
>  }
> -EXPORT_SYMBOL_GPL(kfree_call_rcu);
> +EXPORT_SYMBOL_GPL(kvfree_call_rcu);
>  
>  void __init kfree_rcu_scheduler_running(void)
>  {
> -- 
> 2.20.1
> 

Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Joel Fernandes]

On Sun, Mar 15, 2020 at 07:18:36PM +0100, Uladzislau Rezki (Sony) wrote:
> kvfree_rcu() can deal with an allocated memory that is obtained
> via kvmalloc(). It can return two types of allocated memory or
> "pointers", one can belong to regular SLAB allocator and another
> one can be vmalloc one. It depends on requested size and memory
> pressure.
> 
> Based on that, two streams are split, thus if a pointer belongs
> to vmalloc allocator it is queued to the list, otherwise SLAB
> one is queued into "bulk array" for further processing.
> 
> The main reason of such splitting is:
>     a) to distinguish kmalloc()/vmalloc() ptrs;
>     b) there is no vmalloc_bulk() interface.
> 
> As of now we have list_lru.c user that needs such interface,
> also there will be new comers. Apart of that it is preparation
> to have a head-less variant later.
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  include/linux/rcupdate.h |  9 +++++++++
>  kernel/rcu/tiny.c        |  3 ++-
>  kernel/rcu/tree.c        | 17 ++++++++++++-----
>  3 files changed, 23 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index 2be97a83f266..bb270221dbdc 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -845,6 +845,15 @@ do {									\
>  		__kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
>  } while (0)
>  
> +/**
> + * kvfree_rcu() - kvfree an object after a grace period.
> + * @ptr:	pointer to kvfree
> + * @rhf:	the name of the struct rcu_head within the type of @ptr.
> + *
> + * Same as kfree_rcu(), just simple alias.
> + */
> +#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
> +
>  /*
>   * Place this after a lock-acquisition primitive to guarantee that
>   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index dd572ce7c747..4b99f7b88bee 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -23,6 +23,7 @@
>  #include <linux/cpu.h>
>  #include <linux/prefetch.h>
>  #include <linux/slab.h>
> +#include <linux/mm.h>
>  
>  #include "rcu.h"
>  
> @@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
>  	rcu_lock_acquire(&rcu_callback_map);
>  	if (__is_kfree_rcu_offset(offset)) {
>  		trace_rcu_invoke_kfree_callback("", head, offset);
> -		kfree((void *)head - offset);
> +		kvfree((void *)head - offset);
>  		rcu_lock_release(&rcu_callback_map);
>  		return true;
>  	}
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 2f4c91a3713a..1c0a73616872 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
>  	}
>  
>  	/*
> -	 * Emergency case only. It can happen under low memory
> -	 * condition when an allocation gets failed, so the "bulk"
> -	 * path can not be temporary maintained.
> +	 * vmalloc() pointers end up here also emergency case. It can

Suggest rephrase for clarity:

nit: We can end up here either with 1) vmalloc() pointers or 2) low on memory
and could not allocate a bulk array.

thanks,

 - Joel


> +	 * happen under low memory condition when an allocation gets
> +	 * failed, so the "bulk" path can not be temporary maintained.
>  	 */
>  	for (; head; head = next) {
>  		unsigned long offset = (unsigned long)head->func;
> @@ -2912,7 +2912,7 @@ static void kfree_rcu_work(struct work_struct *work)
>  		trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
>  
>  		if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
> -			kfree((void *)head - offset);
> +			kvfree((void *)head - offset);
>  
>  		rcu_lock_release(&rcu_callback_map);
>  		cond_resched_tasks_rcu_qs();
> @@ -3084,10 +3084,17 @@ void kfree_call_rcu(struct rcu_head *head, rcu_callback_t func)
>  	}
>  
>  	/*
> +	 * We do not queue vmalloc pointers into array,
> +	 * instead they are just queued to the list. We
> +	 * do it because of:
> +	 *    a) to distinguish kmalloc()/vmalloc() ptrs;
> +	 *    b) there is no vmalloc_bulk() interface.
> +	 *
>  	 * Under high memory pressure GFP_NOWAIT can fail,
>  	 * in that case the emergency path is maintained.
>  	 */
> -	if (unlikely(!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func))) {
> +	if (is_vmalloc_addr((void *) head - (unsigned long) func) ||
> +			!kfree_call_rcu_add_ptr_to_bulk(krcp, head, func)) {
>  		head->func = func;
>  		head->next = krcp->head;
>  		krcp->head = head;
> -- 
> 2.20.1
> 

Re: [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback

[Joel Fernandes]

On Sun, Mar 15, 2020 at 07:18:37PM +0100, Uladzislau Rezki (Sony) wrote:
> Rename rcu_invoke_kfree_callback to rcu_invoke_kvfree_callback.
> Do the same with second trace event, that is rcu_kfree_callback,
> it becomes rcu_kvfree_callback. The reason is to be aligned with
> kvfree notation.

Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

thanks,

 - Joel
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  include/trace/events/rcu.h | 8 ++++----
>  kernel/rcu/tiny.c          | 2 +-
>  kernel/rcu/tree.c          | 4 ++--
>  3 files changed, 7 insertions(+), 7 deletions(-)
> 
> diff --git a/include/trace/events/rcu.h b/include/trace/events/rcu.h
> index f9a7811148e2..0ee93d0b1daa 100644
> --- a/include/trace/events/rcu.h
> +++ b/include/trace/events/rcu.h
> @@ -506,13 +506,13 @@ TRACE_EVENT_RCU(rcu_callback,
>  
>  /*
>   * Tracepoint for the registration of a single RCU callback of the special
> - * kfree() form.  The first argument is the RCU type, the second argument
> + * kvfree() form.  The first argument is the RCU type, the second argument
>   * is a pointer to the RCU callback, the third argument is the offset
>   * of the callback within the enclosing RCU-protected data structure,
>   * the fourth argument is the number of lazy callbacks queued, and the
>   * fifth argument is the total number of callbacks queued.
>   */
> -TRACE_EVENT_RCU(rcu_kfree_callback,
> +TRACE_EVENT_RCU(rcu_kvfree_callback,
>  
>  	TP_PROTO(const char *rcuname, struct rcu_head *rhp, unsigned long offset,
>  		 long qlen),
> @@ -596,12 +596,12 @@ TRACE_EVENT_RCU(rcu_invoke_callback,
>  
>  /*
>   * Tracepoint for the invocation of a single RCU callback of the special
> - * kfree() form.  The first argument is the RCU flavor, the second
> + * kvfree() form.  The first argument is the RCU flavor, the second
>   * argument is a pointer to the RCU callback, and the third argument
>   * is the offset of the callback within the enclosing RCU-protected
>   * data structure.
>   */
> -TRACE_EVENT_RCU(rcu_invoke_kfree_callback,
> +TRACE_EVENT_RCU(rcu_invoke_kvfree_callback,
>  
>  	TP_PROTO(const char *rcuname, struct rcu_head *rhp, unsigned long offset),
>  
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index 4b99f7b88bee..3dd8e6e207b0 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -86,7 +86,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
>  
>  	rcu_lock_acquire(&rcu_callback_map);
>  	if (__is_kfree_rcu_offset(offset)) {
> -		trace_rcu_invoke_kfree_callback("", head, offset);
> +		trace_rcu_invoke_kvfree_callback("", head, offset);
>  		kvfree((void *)head - offset);
>  		rcu_lock_release(&rcu_callback_map);
>  		return true;
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index 1c0a73616872..eef75cd210fd 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2720,7 +2720,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
>  	// If no-CBs CPU gets here, rcu_nocb_try_bypass() acquired ->nocb_lock.
>  	rcu_segcblist_enqueue(&rdp->cblist, head);
>  	if (__is_kfree_rcu_offset((unsigned long)func))
> -		trace_rcu_kfree_callback(rcu_state.name, head,
> +		trace_rcu_kvfree_callback(rcu_state.name, head,
>  					 (unsigned long)func,
>  					 rcu_segcblist_n_cbs(&rdp->cblist));
>  	else
> @@ -2909,7 +2909,7 @@ static void kfree_rcu_work(struct work_struct *work)
>  		next = head->next;
>  		debug_rcu_head_unqueue(head);
>  		rcu_lock_acquire(&rcu_callback_map);
> -		trace_rcu_invoke_kfree_callback(rcu_state.name, head, offset);
> +		trace_rcu_invoke_kvfree_callback(rcu_state.name, head, offset);
>  
>  		if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
>  			kvfree((void *)head - offset);
> -- 
> 2.20.1
> 

Re: [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro

[Joel Fernandes]

On Sun, Mar 15, 2020 at 07:18:38PM +0100, Uladzislau Rezki (Sony) wrote:
> Rename __is_kfree_rcu_offset to __is_kvfree_rcu_offset.
> All RCU paths use kvfree() now instead of kfree(), thus
> rename it.

This patch LGTM:
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

thanks,

 - Joel
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  include/linux/rcupdate.h | 6 +++---
>  kernel/rcu/tiny.c        | 2 +-
>  kernel/rcu/tree.c        | 4 ++--
>  3 files changed, 6 insertions(+), 6 deletions(-)
> 
> diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> index bb270221dbdc..e4961631a44f 100644
> --- a/include/linux/rcupdate.h
> +++ b/include/linux/rcupdate.h
> @@ -798,16 +798,16 @@ static inline notrace void rcu_read_unlock_sched_notrace(void)
>  
>  /*
>   * Does the specified offset indicate that the corresponding rcu_head
> - * structure can be handled by kfree_rcu()?
> + * structure can be handled by kvfree_rcu()?
>   */
> -#define __is_kfree_rcu_offset(offset) ((offset) < 4096)
> +#define __is_kvfree_rcu_offset(offset) ((offset) < 4096)
>  
>  /*
>   * Helper macro for kfree_rcu() to prevent argument-expansion eyestrain.
>   */
>  #define __kfree_rcu(head, offset) \
>  	do { \
> -		BUILD_BUG_ON(!__is_kfree_rcu_offset(offset)); \
> +		BUILD_BUG_ON(!__is_kvfree_rcu_offset(offset)); \
>  		kfree_call_rcu(head, (rcu_callback_t)(unsigned long)(offset)); \
>  	} while (0)
>  
> diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> index 3dd8e6e207b0..aa897c3f2e92 100644
> --- a/kernel/rcu/tiny.c
> +++ b/kernel/rcu/tiny.c
> @@ -85,7 +85,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
>  	unsigned long offset = (unsigned long)head->func;
>  
>  	rcu_lock_acquire(&rcu_callback_map);
> -	if (__is_kfree_rcu_offset(offset)) {
> +	if (__is_kvfree_rcu_offset(offset)) {
>  		trace_rcu_invoke_kvfree_callback("", head, offset);
>  		kvfree((void *)head - offset);
>  		rcu_lock_release(&rcu_callback_map);
> diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> index eef75cd210fd..bb9544238396 100644
> --- a/kernel/rcu/tree.c
> +++ b/kernel/rcu/tree.c
> @@ -2719,7 +2719,7 @@ __call_rcu(struct rcu_head *head, rcu_callback_t func)
>  		return; // Enqueued onto ->nocb_bypass, so just leave.
>  	// If no-CBs CPU gets here, rcu_nocb_try_bypass() acquired ->nocb_lock.
>  	rcu_segcblist_enqueue(&rdp->cblist, head);
> -	if (__is_kfree_rcu_offset((unsigned long)func))
> +	if (__is_kvfree_rcu_offset((unsigned long)func))
>  		trace_rcu_kvfree_callback(rcu_state.name, head,
>  					 (unsigned long)func,
>  					 rcu_segcblist_n_cbs(&rdp->cblist));
> @@ -2911,7 +2911,7 @@ static void kfree_rcu_work(struct work_struct *work)
>  		rcu_lock_acquire(&rcu_callback_map);
>  		trace_rcu_invoke_kvfree_callback(rcu_state.name, head, offset);
>  
> -		if (!WARN_ON_ONCE(!__is_kfree_rcu_offset(offset)))
> +		if (!WARN_ON_ONCE(!__is_kvfree_rcu_offset(offset)))
>  			kvfree((void *)head - offset);
>  
>  		rcu_lock_release(&rcu_callback_map);
> -- 
> 2.20.1
> 

Re: [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function

[Joel Fernandes]

On Sun, Mar 15, 2020 at 07:18:40PM +0100, Uladzislau Rezki (Sony) wrote:
> Since there is newly introduced kvfree_rcu() API,
> there is no need in queuing and using call_rcu()
> to kvfree() an object after the GP.
> 
> Remove kvfree_rcu_local() function and replace
> call_rcu() by new kvfree_rcu() API that does
> the same but in more efficient way.

This patch LGTM:
Reviewed-by: Joel Fernandes (Google) <joel@joelfernandes.org>

thanks,

 - Joel
> 
> Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> ---
>  mm/list_lru.c | 13 +++----------
>  1 file changed, 3 insertions(+), 10 deletions(-)
> 
> diff --git a/mm/list_lru.c b/mm/list_lru.c
> index 386424688f80..69becdb22408 100644
> --- a/mm/list_lru.c
> +++ b/mm/list_lru.c
> @@ -12,6 +12,7 @@
>  #include <linux/slab.h>
>  #include <linux/mutex.h>
>  #include <linux/memcontrol.h>
> +#include <linux/rcupdate.h>
>  #include "slab.h"
>  
>  #ifdef CONFIG_MEMCG_KMEM
> @@ -383,21 +384,13 @@ static void memcg_destroy_list_lru_node(struct list_lru_node *nlru)
>  	struct list_lru_memcg *memcg_lrus;
>  	/*
>  	 * This is called when shrinker has already been unregistered,
> -	 * and nobody can use it. So, there is no need to use kvfree_rcu_local().
> +	 * and nobody can use it. So, there is no need to use kvfree_rcu().
>  	 */
>  	memcg_lrus = rcu_dereference_protected(nlru->memcg_lrus, true);
>  	__memcg_destroy_list_lru_node(memcg_lrus, 0, memcg_nr_cache_ids);
>  	kvfree(memcg_lrus);
>  }
>  
> -static void kvfree_rcu_local(struct rcu_head *head)
> -{
> -	struct list_lru_memcg *mlru;
> -
> -	mlru = container_of(head, struct list_lru_memcg, rcu);
> -	kvfree(mlru);
> -}
> -
>  static int memcg_update_list_lru_node(struct list_lru_node *nlru,
>  				      int old_size, int new_size)
>  {
> @@ -429,7 +422,7 @@ static int memcg_update_list_lru_node(struct list_lru_node *nlru,
>  	rcu_assign_pointer(nlru->memcg_lrus, new);
>  	spin_unlock_irq(&nlru->lock);
>  
> -	call_rcu(&old->rcu, kvfree_rcu_local);
> +	kvfree_rcu(old, rcu);
>  	return 0;
>  }
>  
> -- 
> 2.20.1
> 

Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Uladzislau Rezki]

On Mon, Mar 16, 2020 at 11:45:39AM -0400, Joel Fernandes wrote:
> On Sun, Mar 15, 2020 at 07:18:36PM +0100, Uladzislau Rezki (Sony) wrote:
> > kvfree_rcu() can deal with an allocated memory that is obtained
> > via kvmalloc(). It can return two types of allocated memory or
> > "pointers", one can belong to regular SLAB allocator and another
> > one can be vmalloc one. It depends on requested size and memory
> > pressure.
> > 
> > Based on that, two streams are split, thus if a pointer belongs
> > to vmalloc allocator it is queued to the list, otherwise SLAB
> > one is queued into "bulk array" for further processing.
> > 
> > The main reason of such splitting is:
> >     a) to distinguish kmalloc()/vmalloc() ptrs;
> >     b) there is no vmalloc_bulk() interface.
> > 
> > As of now we have list_lru.c user that needs such interface,
> > also there will be new comers. Apart of that it is preparation
> > to have a head-less variant later.
> > 
> > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > ---
> >  include/linux/rcupdate.h |  9 +++++++++
> >  kernel/rcu/tiny.c        |  3 ++-
> >  kernel/rcu/tree.c        | 17 ++++++++++++-----
> >  3 files changed, 23 insertions(+), 6 deletions(-)
> > 
> > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > index 2be97a83f266..bb270221dbdc 100644
> > --- a/include/linux/rcupdate.h
> > +++ b/include/linux/rcupdate.h
> > @@ -845,6 +845,15 @@ do {									\
> >  		__kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
> >  } while (0)
> >  
> > +/**
> > + * kvfree_rcu() - kvfree an object after a grace period.
> > + * @ptr:	pointer to kvfree
> > + * @rhf:	the name of the struct rcu_head within the type of @ptr.
> > + *
> > + * Same as kfree_rcu(), just simple alias.
> > + */
> > +#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
> > +
> >  /*
> >   * Place this after a lock-acquisition primitive to guarantee that
> >   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
> > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > index dd572ce7c747..4b99f7b88bee 100644
> > --- a/kernel/rcu/tiny.c
> > +++ b/kernel/rcu/tiny.c
> > @@ -23,6 +23,7 @@
> >  #include <linux/cpu.h>
> >  #include <linux/prefetch.h>
> >  #include <linux/slab.h>
> > +#include <linux/mm.h>
> >  
> >  #include "rcu.h"
> >  
> > @@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
> >  	rcu_lock_acquire(&rcu_callback_map);
> >  	if (__is_kfree_rcu_offset(offset)) {
> >  		trace_rcu_invoke_kfree_callback("", head, offset);
> > -		kfree((void *)head - offset);
> > +		kvfree((void *)head - offset);
> >  		rcu_lock_release(&rcu_callback_map);
> >  		return true;
> >  	}
> > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > index 2f4c91a3713a..1c0a73616872 100644
> > --- a/kernel/rcu/tree.c
> > +++ b/kernel/rcu/tree.c
> > @@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
> >  	}
> >  
> >  	/*
> > -	 * Emergency case only. It can happen under low memory
> > -	 * condition when an allocation gets failed, so the "bulk"
> > -	 * path can not be temporary maintained.
> > +	 * vmalloc() pointers end up here also emergency case. It can
> 
> Suggest rephrase for clarity:
> 
> nit: We can end up here either with 1) vmalloc() pointers or 2) low on memory
> and could not allocate a bulk array.
> 
Let's go with your suggestion. I see that you took patches to your tree.
Could you please update it on your own? Otherwise i can send out V2, so
please let me know.

Thanks for the comments!

--
Vlad Rezki

Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Joel Fernandes]

On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <urezki@gmail.com> wrote:
>
> On Mon, Mar 16, 2020 at 11:45:39AM -0400, Joel Fernandes wrote:
> > On Sun, Mar 15, 2020 at 07:18:36PM +0100, Uladzislau Rezki (Sony) wrote:
> > > kvfree_rcu() can deal with an allocated memory that is obtained
> > > via kvmalloc(). It can return two types of allocated memory or
> > > "pointers", one can belong to regular SLAB allocator and another
> > > one can be vmalloc one. It depends on requested size and memory
> > > pressure.
> > >
> > > Based on that, two streams are split, thus if a pointer belongs
> > > to vmalloc allocator it is queued to the list, otherwise SLAB
> > > one is queued into "bulk array" for further processing.
> > >
> > > The main reason of such splitting is:
> > >     a) to distinguish kmalloc()/vmalloc() ptrs;
> > >     b) there is no vmalloc_bulk() interface.
> > >
> > > As of now we have list_lru.c user that needs such interface,
> > > also there will be new comers. Apart of that it is preparation
> > > to have a head-less variant later.
> > >
> > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > ---
> > >  include/linux/rcupdate.h |  9 +++++++++
> > >  kernel/rcu/tiny.c        |  3 ++-
> > >  kernel/rcu/tree.c        | 17 ++++++++++++-----
> > >  3 files changed, 23 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > index 2be97a83f266..bb270221dbdc 100644
> > > --- a/include/linux/rcupdate.h
> > > +++ b/include/linux/rcupdate.h
> > > @@ -845,6 +845,15 @@ do {                                                                   \
> > >             __kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
> > >  } while (0)
> > >
> > > +/**
> > > + * kvfree_rcu() - kvfree an object after a grace period.
> > > + * @ptr:   pointer to kvfree
> > > + * @rhf:   the name of the struct rcu_head within the type of @ptr.
> > > + *
> > > + * Same as kfree_rcu(), just simple alias.
> > > + */
> > > +#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
> > > +
> > >  /*
> > >   * Place this after a lock-acquisition primitive to guarantee that
> > >   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
> > > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > > index dd572ce7c747..4b99f7b88bee 100644
> > > --- a/kernel/rcu/tiny.c
> > > +++ b/kernel/rcu/tiny.c
> > > @@ -23,6 +23,7 @@
> > >  #include <linux/cpu.h>
> > >  #include <linux/prefetch.h>
> > >  #include <linux/slab.h>
> > > +#include <linux/mm.h>
> > >
> > >  #include "rcu.h"
> > >
> > > @@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
> > >     rcu_lock_acquire(&rcu_callback_map);
> > >     if (__is_kfree_rcu_offset(offset)) {
> > >             trace_rcu_invoke_kfree_callback("", head, offset);
> > > -           kfree((void *)head - offset);
> > > +           kvfree((void *)head - offset);
> > >             rcu_lock_release(&rcu_callback_map);
> > >             return true;
> > >     }
> > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > index 2f4c91a3713a..1c0a73616872 100644
> > > --- a/kernel/rcu/tree.c
> > > +++ b/kernel/rcu/tree.c
> > > @@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
> > >     }
> > >
> > >     /*
> > > -    * Emergency case only. It can happen under low memory
> > > -    * condition when an allocation gets failed, so the "bulk"
> > > -    * path can not be temporary maintained.
> > > +    * vmalloc() pointers end up here also emergency case. It can
> >
> > Suggest rephrase for clarity:
> >
> > nit: We can end up here either with 1) vmalloc() pointers or 2) low on memory
> > and could not allocate a bulk array.
> >
> Let's go with your suggestion. I see that you took patches to your tree.
> Could you please update it on your own? Otherwise i can send out V2, so
> please let me know.

I updated it, "patch -p1" resolved the issue. No need to resend unless
something in my tree looks odd to you :)

> Thanks for the comments!

Thanks!

- Joel

Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Joel Fernandes]

On Mon, Mar 16, 2020 at 2:57 PM Joel Fernandes <joel@joelfernandes.org> wrote:
>
> On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <urezki@gmail.com> wrote:
> >
> > On Mon, Mar 16, 2020 at 11:45:39AM -0400, Joel Fernandes wrote:
> > > On Sun, Mar 15, 2020 at 07:18:36PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > kvfree_rcu() can deal with an allocated memory that is obtained
> > > > via kvmalloc(). It can return two types of allocated memory or
> > > > "pointers", one can belong to regular SLAB allocator and another
> > > > one can be vmalloc one. It depends on requested size and memory
> > > > pressure.
> > > >
> > > > Based on that, two streams are split, thus if a pointer belongs
> > > > to vmalloc allocator it is queued to the list, otherwise SLAB
> > > > one is queued into "bulk array" for further processing.
> > > >
> > > > The main reason of such splitting is:
> > > >     a) to distinguish kmalloc()/vmalloc() ptrs;
> > > >     b) there is no vmalloc_bulk() interface.
> > > >
> > > > As of now we have list_lru.c user that needs such interface,
> > > > also there will be new comers. Apart of that it is preparation
> > > > to have a head-less variant later.
> > > >
> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > > ---
> > > >  include/linux/rcupdate.h |  9 +++++++++
> > > >  kernel/rcu/tiny.c        |  3 ++-
> > > >  kernel/rcu/tree.c        | 17 ++++++++++++-----
> > > >  3 files changed, 23 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > > index 2be97a83f266..bb270221dbdc 100644
> > > > --- a/include/linux/rcupdate.h
> > > > +++ b/include/linux/rcupdate.h
> > > > @@ -845,6 +845,15 @@ do {                                                                   \
> > > >             __kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
> > > >  } while (0)
> > > >
> > > > +/**
> > > > + * kvfree_rcu() - kvfree an object after a grace period.
> > > > + * @ptr:   pointer to kvfree
> > > > + * @rhf:   the name of the struct rcu_head within the type of @ptr.
> > > > + *
> > > > + * Same as kfree_rcu(), just simple alias.
> > > > + */
> > > > +#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
> > > > +
> > > >  /*
> > > >   * Place this after a lock-acquisition primitive to guarantee that
> > > >   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
> > > > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > > > index dd572ce7c747..4b99f7b88bee 100644
> > > > --- a/kernel/rcu/tiny.c
> > > > +++ b/kernel/rcu/tiny.c
> > > > @@ -23,6 +23,7 @@
> > > >  #include <linux/cpu.h>
> > > >  #include <linux/prefetch.h>
> > > >  #include <linux/slab.h>
> > > > +#include <linux/mm.h>
> > > >
> > > >  #include "rcu.h"
> > > >
> > > > @@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
> > > >     rcu_lock_acquire(&rcu_callback_map);
> > > >     if (__is_kfree_rcu_offset(offset)) {
> > > >             trace_rcu_invoke_kfree_callback("", head, offset);
> > > > -           kfree((void *)head - offset);
> > > > +           kvfree((void *)head - offset);
> > > >             rcu_lock_release(&rcu_callback_map);
> > > >             return true;
> > > >     }
> > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > > index 2f4c91a3713a..1c0a73616872 100644
> > > > --- a/kernel/rcu/tree.c
> > > > +++ b/kernel/rcu/tree.c
> > > > @@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
> > > >     }
> > > >
> > > >     /*
> > > > -    * Emergency case only. It can happen under low memory
> > > > -    * condition when an allocation gets failed, so the "bulk"
> > > > -    * path can not be temporary maintained.
> > > > +    * vmalloc() pointers end up here also emergency case. It can
> > >
> > > Suggest rephrase for clarity:
> > >
> > > nit: We can end up here either with 1) vmalloc() pointers or 2) low on memory
> > > and could not allocate a bulk array.
> > >
> > Let's go with your suggestion. I see that you took patches to your tree.
> > Could you please update it on your own? Otherwise i can send out V2, so
> > please let me know.
>
> I updated it, "patch -p1" resolved the issue. No need to resend unless
> something in my tree looks odd to you :)

I added this:

diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index 0e2632622176b..e7963270e7f6a 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -2857,9 +2857,10 @@ static void kfree_rcu_work(struct work_struct *work)
        }

        /*
-        * vmalloc() pointers end up here also emergency case. It can
-        * happen under low memory condition when an allocation gets
-        * failed, so the "bulk" path can not be temporary maintained.
+        * We can end up here either with 1) vmalloc() pointers or 2) were low
+        * on memory and could not allocate a bulk array. It can happen under
+        * low memory condition when an allocation gets failed, so the "bulk"
+        * path can not be temporarly used.
         */
        for (; head; head = next) {
                unsigned long offset = (unsigned long)head->func;

Thanks.

Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Uladzislau Rezki]

On Mon, Mar 16, 2020 at 11:25:41AM -0400, Joel Fernandes wrote:
> On Sun, Mar 15, 2020 at 07:18:39PM +0100, Uladzislau Rezki (Sony) wrote:
> > Rename kfree_call_rcu() to the kvfree_call_rcu().
> > The reason is, it is capable of freeing vmalloc()
> > memory now.
> > 
> > Do the same with __kfree_rcu() macro, it becomes
> > __kvfree_rcu(), the reason is the same as pointed
> > above.
> 
> Vlad, this patch does not apply to my branch that I shared with you. Sorry if
> I was not clear earlier, could we work on the same branch to avoid conflicts?
> 
It was clear to me. Basically i knew that you would be able to apply it
because of slim changes. I based my work on latest Paul's branch simply
because that my current setup was based on that, it would take more time
to switch.

Next changes i will base on your branch.

> I based the kfree_rcu shrinker patches on an 'rcu/kfree' branch in my git
> tree: https://github.com/joelagnel/linux-kernel/tree/rcu/kfree
> 
> For now I manually applied 5/6. All others applied cleanly.
> 
> Updated the tree as I continue to review your patches.
>
Thanks!

--
Vlad Rezki

Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Uladzislau Rezki]

On Mon, Mar 16, 2020 at 02:57:18PM -0400, Joel Fernandes wrote:
> On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <urezki@gmail.com> wrote:
> >
> > On Mon, Mar 16, 2020 at 11:45:39AM -0400, Joel Fernandes wrote:
> > > On Sun, Mar 15, 2020 at 07:18:36PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > kvfree_rcu() can deal with an allocated memory that is obtained
> > > > via kvmalloc(). It can return two types of allocated memory or
> > > > "pointers", one can belong to regular SLAB allocator and another
> > > > one can be vmalloc one. It depends on requested size and memory
> > > > pressure.
> > > >
> > > > Based on that, two streams are split, thus if a pointer belongs
> > > > to vmalloc allocator it is queued to the list, otherwise SLAB
> > > > one is queued into "bulk array" for further processing.
> > > >
> > > > The main reason of such splitting is:
> > > >     a) to distinguish kmalloc()/vmalloc() ptrs;
> > > >     b) there is no vmalloc_bulk() interface.
> > > >
> > > > As of now we have list_lru.c user that needs such interface,
> > > > also there will be new comers. Apart of that it is preparation
> > > > to have a head-less variant later.
> > > >
> > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > > ---
> > > >  include/linux/rcupdate.h |  9 +++++++++
> > > >  kernel/rcu/tiny.c        |  3 ++-
> > > >  kernel/rcu/tree.c        | 17 ++++++++++++-----
> > > >  3 files changed, 23 insertions(+), 6 deletions(-)
> > > >
> > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > > index 2be97a83f266..bb270221dbdc 100644
> > > > --- a/include/linux/rcupdate.h
> > > > +++ b/include/linux/rcupdate.h
> > > > @@ -845,6 +845,15 @@ do {                                                                   \
> > > >             __kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
> > > >  } while (0)
> > > >
> > > > +/**
> > > > + * kvfree_rcu() - kvfree an object after a grace period.
> > > > + * @ptr:   pointer to kvfree
> > > > + * @rhf:   the name of the struct rcu_head within the type of @ptr.
> > > > + *
> > > > + * Same as kfree_rcu(), just simple alias.
> > > > + */
> > > > +#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
> > > > +
> > > >  /*
> > > >   * Place this after a lock-acquisition primitive to guarantee that
> > > >   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
> > > > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > > > index dd572ce7c747..4b99f7b88bee 100644
> > > > --- a/kernel/rcu/tiny.c
> > > > +++ b/kernel/rcu/tiny.c
> > > > @@ -23,6 +23,7 @@
> > > >  #include <linux/cpu.h>
> > > >  #include <linux/prefetch.h>
> > > >  #include <linux/slab.h>
> > > > +#include <linux/mm.h>
> > > >
> > > >  #include "rcu.h"
> > > >
> > > > @@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
> > > >     rcu_lock_acquire(&rcu_callback_map);
> > > >     if (__is_kfree_rcu_offset(offset)) {
> > > >             trace_rcu_invoke_kfree_callback("", head, offset);
> > > > -           kfree((void *)head - offset);
> > > > +           kvfree((void *)head - offset);
> > > >             rcu_lock_release(&rcu_callback_map);
> > > >             return true;
> > > >     }
> > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > > index 2f4c91a3713a..1c0a73616872 100644
> > > > --- a/kernel/rcu/tree.c
> > > > +++ b/kernel/rcu/tree.c
> > > > @@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
> > > >     }
> > > >
> > > >     /*
> > > > -    * Emergency case only. It can happen under low memory
> > > > -    * condition when an allocation gets failed, so the "bulk"
> > > > -    * path can not be temporary maintained.
> > > > +    * vmalloc() pointers end up here also emergency case. It can
> > >
> > > Suggest rephrase for clarity:
> > >
> > > nit: We can end up here either with 1) vmalloc() pointers or 2) low on memory
> > > and could not allocate a bulk array.
> > >
> > Let's go with your suggestion. I see that you took patches to your tree.
> > Could you please update it on your own? Otherwise i can send out V2, so
> > please let me know.
> 
> I updated it, "patch -p1" resolved the issue. No need to resend unless
> something in my tree looks odd to you :)
> 
I knew that! Thanks :)

--
Vlad Rezki

Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface

[Joel Fernandes]

On Mon, Mar 16, 2020 at 3:03 PM Uladzislau Rezki <urezki@gmail.com> wrote:
>
> On Mon, Mar 16, 2020 at 02:57:18PM -0400, Joel Fernandes wrote:
> > On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <urezki@gmail.com> wrote:
> > >
> > > On Mon, Mar 16, 2020 at 11:45:39AM -0400, Joel Fernandes wrote:
> > > > On Sun, Mar 15, 2020 at 07:18:36PM +0100, Uladzislau Rezki (Sony) wrote:
> > > > > kvfree_rcu() can deal with an allocated memory that is obtained
> > > > > via kvmalloc(). It can return two types of allocated memory or
> > > > > "pointers", one can belong to regular SLAB allocator and another
> > > > > one can be vmalloc one. It depends on requested size and memory
> > > > > pressure.
> > > > >
> > > > > Based on that, two streams are split, thus if a pointer belongs
> > > > > to vmalloc allocator it is queued to the list, otherwise SLAB
> > > > > one is queued into "bulk array" for further processing.
> > > > >
> > > > > The main reason of such splitting is:
> > > > >     a) to distinguish kmalloc()/vmalloc() ptrs;
> > > > >     b) there is no vmalloc_bulk() interface.
> > > > >
> > > > > As of now we have list_lru.c user that needs such interface,
> > > > > also there will be new comers. Apart of that it is preparation
> > > > > to have a head-less variant later.
> > > > >
> > > > > Signed-off-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
> > > > > ---
> > > > >  include/linux/rcupdate.h |  9 +++++++++
> > > > >  kernel/rcu/tiny.c        |  3 ++-
> > > > >  kernel/rcu/tree.c        | 17 ++++++++++++-----
> > > > >  3 files changed, 23 insertions(+), 6 deletions(-)
> > > > >
> > > > > diff --git a/include/linux/rcupdate.h b/include/linux/rcupdate.h
> > > > > index 2be97a83f266..bb270221dbdc 100644
> > > > > --- a/include/linux/rcupdate.h
> > > > > +++ b/include/linux/rcupdate.h
> > > > > @@ -845,6 +845,15 @@ do {                                                                   \
> > > > >             __kfree_rcu(&((___p)->rhf), offsetof(typeof(*(ptr)), rhf)); \
> > > > >  } while (0)
> > > > >
> > > > > +/**
> > > > > + * kvfree_rcu() - kvfree an object after a grace period.
> > > > > + * @ptr:   pointer to kvfree
> > > > > + * @rhf:   the name of the struct rcu_head within the type of @ptr.
> > > > > + *
> > > > > + * Same as kfree_rcu(), just simple alias.
> > > > > + */
> > > > > +#define kvfree_rcu(ptr, rhf) kfree_rcu(ptr, rhf)
> > > > > +
> > > > >  /*
> > > > >   * Place this after a lock-acquisition primitive to guarantee that
> > > > >   * an UNLOCK+LOCK pair acts as a full barrier.  This guarantee applies
> > > > > diff --git a/kernel/rcu/tiny.c b/kernel/rcu/tiny.c
> > > > > index dd572ce7c747..4b99f7b88bee 100644
> > > > > --- a/kernel/rcu/tiny.c
> > > > > +++ b/kernel/rcu/tiny.c
> > > > > @@ -23,6 +23,7 @@
> > > > >  #include <linux/cpu.h>
> > > > >  #include <linux/prefetch.h>
> > > > >  #include <linux/slab.h>
> > > > > +#include <linux/mm.h>
> > > > >
> > > > >  #include "rcu.h"
> > > > >
> > > > > @@ -86,7 +87,7 @@ static inline bool rcu_reclaim_tiny(struct rcu_head *head)
> > > > >     rcu_lock_acquire(&rcu_callback_map);
> > > > >     if (__is_kfree_rcu_offset(offset)) {
> > > > >             trace_rcu_invoke_kfree_callback("", head, offset);
> > > > > -           kfree((void *)head - offset);
> > > > > +           kvfree((void *)head - offset);
> > > > >             rcu_lock_release(&rcu_callback_map);
> > > > >             return true;
> > > > >     }
> > > > > diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
> > > > > index 2f4c91a3713a..1c0a73616872 100644
> > > > > --- a/kernel/rcu/tree.c
> > > > > +++ b/kernel/rcu/tree.c
> > > > > @@ -2899,9 +2899,9 @@ static void kfree_rcu_work(struct work_struct *work)
> > > > >     }
> > > > >
> > > > >     /*
> > > > > -    * Emergency case only. It can happen under low memory
> > > > > -    * condition when an allocation gets failed, so the "bulk"
> > > > > -    * path can not be temporary maintained.
> > > > > +    * vmalloc() pointers end up here also emergency case. It can
> > > >
> > > > Suggest rephrase for clarity:
> > > >
> > > > nit: We can end up here either with 1) vmalloc() pointers or 2) low on memory
> > > > and could not allocate a bulk array.
> > > >
> > > Let's go with your suggestion. I see that you took patches to your tree.
> > > Could you please update it on your own? Otherwise i can send out V2, so
> > > please let me know.
> >
> > I updated it, "patch -p1" resolved the issue. No need to resend unless
> > something in my tree looks odd to you :)
> >
> I knew that! Thanks :)

Thank you Vlad :)

 - Joel

Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Andrew Morton]

On Mon, 16 Mar 2020 20:01:44 +0100 Uladzislau Rezki <urezki@gmail.com> wrote:

> On Mon, Mar 16, 2020 at 11:25:41AM -0400, Joel Fernandes wrote:
> > On Sun, Mar 15, 2020 at 07:18:39PM +0100, Uladzislau Rezki (Sony) wrote:
> > > Rename kfree_call_rcu() to the kvfree_call_rcu().
> > > The reason is, it is capable of freeing vmalloc()
> > > memory now.
> > > 
> > > Do the same with __kfree_rcu() macro, it becomes
> > > __kvfree_rcu(), the reason is the same as pointed
> > > above.
> > 
> > Vlad, this patch does not apply to my branch that I shared with you. Sorry if
> > I was not clear earlier, could we work on the same branch to avoid conflicts?
> > 
> It was clear to me. Basically i knew that you would be able to apply it
> because of slim changes. I based my work on latest Paul's branch simply
> because that my current setup was based on that, it would take more time
> to switch.
> 
> Next changes i will base on your branch.
> 
> > I based the kfree_rcu shrinker patches on an 'rcu/kfree' branch in my git
> > tree: https://github.com/joelagnel/linux-kernel/tree/rcu/kfree
> > 
> > For now I manually applied 5/6. All others applied cleanly.
> > 
> > Updated the tree as I continue to review your patches.
> >

I'm not sure what's happening here - these patches aren't yet in
linux-next.

A couple of thoughts:

- Please cc linux-mm@kvack.org on this patchset and anything else
  which impacts MM.

- It's a bit strange to create new infrastructure which has but a
  single call site.  Please tell us much more about "there was also
  request/interest so there will be new comers" to set minds at ease. 

  Who/where are these possible new callsites and when can we expect to
  see that code?

Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Uladzislau Rezki]

Hello, Andrew.

> > > I based the kfree_rcu shrinker patches on an 'rcu/kfree' branch in my git
> > > tree: https://github.com/joelagnel/linux-kernel/tree/rcu/kfree
> > > 
> > > For now I manually applied 5/6. All others applied cleanly.
> > > 
> > > Updated the tree as I continue to review your patches.
> > >
> 
> I'm not sure what's happening here - these patches aren't yet in
> linux-next.
> 
Please have a look here, i explained in detail in my cover latter
what we would like to achieve: https://lkml.org/lkml/2020/3/23/242

> A couple of thoughts:
> 
> - Please cc linux-mm@kvack.org on this patchset and anything else
>   which impacts MM.
> 
OK. I thought that is related to RCU, therefore i did not want to
spam, but i included you :) Next time we will keep linux-mm@kvack.org
in a loop. Sorry for that.

>
> - It's a bit strange to create new infrastructure which has but a
>   single call site.  Please tell us much more about "there was also
>   request/interest so there will be new comers" to set minds at ease. 
> 
>   Who/where are these possible new callsites and when can we expect to
>   see that code?
> 
https://lkml.org/lkml/2020/3/23/242 here i wrote motivation. Please
check it. If you have some questions please ask.

--
Vlad Rezki

Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Joel Fernandes]

On Tue, Apr 21, 2020 at 02:03:58PM +0200, Uladzislau Rezki wrote:
> Hello, Andrew.
> 
> > > > I based the kfree_rcu shrinker patches on an 'rcu/kfree' branch in my git
> > > > tree: https://github.com/joelagnel/linux-kernel/tree/rcu/kfree
> > > > 
> > > > For now I manually applied 5/6. All others applied cleanly.
> > > > 
> > > > Updated the tree as I continue to review your patches.
> > > >
> > 
> > I'm not sure what's happening here - these patches aren't yet in
> > linux-next.
> > 
> Please have a look here, i explained in detail in my cover latter
> what we would like to achieve: https://lkml.org/lkml/2020/3/23/242
> 
> > A couple of thoughts:
> > 
> > - Please cc linux-mm@kvack.org on this patchset and anything else
> >   which impacts MM.
> > 
> OK. I thought that is related to RCU, therefore i did not want to
> spam, but i included you :) Next time we will keep linux-mm@kvack.org
> in a loop. Sorry for that.

Sorry from my side as well. Will keep linux-mm in the loop henceforth.

thanks,

 - Joel


> 
> >
> > - It's a bit strange to create new infrastructure which has but a
> >   single call site.  Please tell us much more about "there was also
> >   request/interest so there will be new comers" to set minds at ease. 
> > 
> >   Who/where are these possible new callsites and when can we expect to
> >   see that code?
> > 
> https://lkml.org/lkml/2020/3/23/242 here i wrote motivation. Please
> check it. If you have some questions please ask.
> 
> --
> Vlad Rezki

Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()

[Paul E. McKenney]

On Wed, Apr 22, 2020 at 07:03:22PM -0400, Joel Fernandes wrote:
> On Tue, Apr 21, 2020 at 02:03:58PM +0200, Uladzislau Rezki wrote:
> > Hello, Andrew.
> > 
> > > > > I based the kfree_rcu shrinker patches on an 'rcu/kfree' branch in my git
> > > > > tree: https://github.com/joelagnel/linux-kernel/tree/rcu/kfree
> > > > > 
> > > > > For now I manually applied 5/6. All others applied cleanly.
> > > > > 
> > > > > Updated the tree as I continue to review your patches.
> > > > >
> > > 
> > > I'm not sure what's happening here - these patches aren't yet in
> > > linux-next.
> > > 
> > Please have a look here, i explained in detail in my cover latter
> > what we would like to achieve: https://lkml.org/lkml/2020/3/23/242
> > 
> > > A couple of thoughts:
> > > 
> > > - Please cc linux-mm@kvack.org on this patchset and anything else
> > >   which impacts MM.
> > > 
> > OK. I thought that is related to RCU, therefore i did not want to
> > spam, but i included you :) Next time we will keep linux-mm@kvack.org
> > in a loop. Sorry for that.
> 
> Sorry from my side as well. Will keep linux-mm in the loop henceforth.

I must confess that it came as a bit of a surprise to me as well.

But what is life without a surprise or two along the way?  ;-)

							Thanx, Paul

> thanks,
> 
>  - Joel
> 
> 
> > 
> > >
> > > - It's a bit strange to create new infrastructure which has but a
> > >   single call site.  Please tell us much more about "there was also
> > >   request/interest so there will be new comers" to set minds at ease. 
> > > 
> > >   Who/where are these possible new callsites and when can we expect to
> > >   see that code?
> > > 
> > https://lkml.org/lkml/2020/3/23/242 here i wrote motivation. Please
> > check it. If you have some questions please ask.
> > 
> > --
> > Vlad Rezki