RCU Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH v1 0/6] Introduce kvfree_rcu() logic
@ 2020-03-15 18:18 Uladzislau Rezki (Sony)
  2020-03-15 18:18 ` [PATCH v1 1/6] mm/list_lru.c: rename kvfree_rcu() to local variant Uladzislau Rezki (Sony)
                   ` (5 more replies)
  0 siblings, 6 replies; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 1/6] mm/list_lru.c: rename kvfree_rcu() to local variant
  2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
@ 2020-03-15 18:18 ` Uladzislau Rezki (Sony)
  2020-03-15 18:18 ` [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface Uladzislau Rezki (Sony)
                   ` (4 subsequent siblings)
  5 siblings, 0 replies; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
  2020-03-15 18:18 ` [PATCH v1 1/6] mm/list_lru.c: rename kvfree_rcu() to local variant Uladzislau Rezki (Sony)
@ 2020-03-15 18:18 ` Uladzislau Rezki (Sony)
  2020-03-16 15:45   ` Joel Fernandes
  2020-03-15 18:18 ` [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback Uladzislau Rezki (Sony)
                   ` (3 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback
  2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
  2020-03-15 18:18 ` [PATCH v1 1/6] mm/list_lru.c: rename kvfree_rcu() to local variant Uladzislau Rezki (Sony)
  2020-03-15 18:18 ` [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface Uladzislau Rezki (Sony)
@ 2020-03-15 18:18 ` Uladzislau Rezki (Sony)
  2020-03-16 15:47   ` Joel Fernandes
  2020-03-15 18:18 ` [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro Uladzislau Rezki (Sony)
                   ` (2 subsequent siblings)
  5 siblings, 1 reply; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro
  2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
                   ` (2 preceding siblings ...)
  2020-03-15 18:18 ` [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback Uladzislau Rezki (Sony)
@ 2020-03-15 18:18 ` Uladzislau Rezki (Sony)
  2020-03-16 15:48   ` Joel Fernandes
  2020-03-15 18:18 ` [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu() Uladzislau Rezki (Sony)
  2020-03-15 18:18 ` [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function Uladzislau Rezki (Sony)
  5 siblings, 1 reply; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()
  2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
                   ` (3 preceding siblings ...)
  2020-03-15 18:18 ` [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro Uladzislau Rezki (Sony)
@ 2020-03-15 18:18 ` Uladzislau Rezki (Sony)
  2020-03-16 15:25   ` Joel Fernandes
  2020-03-15 18:18 ` [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function Uladzislau Rezki (Sony)
  5 siblings, 1 reply; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function
  2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
                   ` (4 preceding siblings ...)
  2020-03-15 18:18 ` [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu() Uladzislau Rezki (Sony)
@ 2020-03-15 18:18 ` Uladzislau Rezki (Sony)
  2020-03-16 15:49   ` Joel Fernandes
  5 siblings, 1 reply; 18+ messages in thread
From: Uladzislau Rezki (Sony) @ 2020-03-15 18:18 UTC (permalink / raw)
  To: LKML, Paul E . McKenney, Joel Fernandes
  Cc: RCU, Andrew Morton, Uladzislau Rezki, Steven Rostedt,
	Oleksiy Avramchenko

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


^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()
  2020-03-15 18:18 ` [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu() Uladzislau Rezki (Sony)
@ 2020-03-16 15:25   ` Joel Fernandes
  2020-03-16 19:01     ` Uladzislau Rezki
  0 siblings, 1 reply; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 15:25 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-15 18:18 ` [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface Uladzislau Rezki (Sony)
@ 2020-03-16 15:45   ` Joel Fernandes
  2020-03-16 18:55     ` Uladzislau Rezki
  0 siblings, 1 reply; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 15:45 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback
  2020-03-15 18:18 ` [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback Uladzislau Rezki (Sony)
@ 2020-03-16 15:47   ` Joel Fernandes
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 15:47 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro
  2020-03-15 18:18 ` [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro Uladzislau Rezki (Sony)
@ 2020-03-16 15:48   ` Joel Fernandes
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 15:48 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function
  2020-03-15 18:18 ` [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function Uladzislau Rezki (Sony)
@ 2020-03-16 15:49   ` Joel Fernandes
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 15:49 UTC (permalink / raw)
  To: Uladzislau Rezki (Sony)
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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
> 

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-16 15:45   ` Joel Fernandes
@ 2020-03-16 18:55     ` Uladzislau Rezki
  2020-03-16 18:57       ` Joel Fernandes
  0 siblings, 1 reply; 18+ messages in thread
From: Uladzislau Rezki @ 2020-03-16 18:55 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Uladzislau Rezki (Sony),
	LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-16 18:55     ` Uladzislau Rezki
@ 2020-03-16 18:57       ` Joel Fernandes
  2020-03-16 19:01         ` Joel Fernandes
  2020-03-16 19:03         ` Uladzislau Rezki
  0 siblings, 2 replies; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 18:57 UTC (permalink / raw)
  To: Uladzislau Rezki
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-16 18:57       ` Joel Fernandes
@ 2020-03-16 19:01         ` Joel Fernandes
  2020-03-16 19:03         ` Uladzislau Rezki
  1 sibling, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 19:01 UTC (permalink / raw)
  To: Uladzislau Rezki
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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.

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu()
  2020-03-16 15:25   ` Joel Fernandes
@ 2020-03-16 19:01     ` Uladzislau Rezki
  0 siblings, 0 replies; 18+ messages in thread
From: Uladzislau Rezki @ 2020-03-16 19:01 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Uladzislau Rezki (Sony),
	LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-16 18:57       ` Joel Fernandes
  2020-03-16 19:01         ` Joel Fernandes
@ 2020-03-16 19:03         ` Uladzislau Rezki
  2020-03-16 19:48           ` Joel Fernandes
  1 sibling, 1 reply; 18+ messages in thread
From: Uladzislau Rezki @ 2020-03-16 19:03 UTC (permalink / raw)
  To: Joel Fernandes
  Cc: Uladzislau Rezki, LKML, Paul E . McKenney, RCU, Andrew Morton,
	Steven Rostedt, Oleksiy Avramchenko

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

* Re: [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface
  2020-03-16 19:03         ` Uladzislau Rezki
@ 2020-03-16 19:48           ` Joel Fernandes
  0 siblings, 0 replies; 18+ messages in thread
From: Joel Fernandes @ 2020-03-16 19:48 UTC (permalink / raw)
  To: Uladzislau Rezki
  Cc: LKML, Paul E . McKenney, RCU, Andrew Morton, Steven Rostedt,
	Oleksiy Avramchenko

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

^ permalink raw reply	[flat|nested] 18+ messages in thread

end of thread, back to index

Thread overview: 18+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-15 18:18 [PATCH v1 0/6] Introduce kvfree_rcu() logic Uladzislau Rezki (Sony)
2020-03-15 18:18 ` [PATCH v1 1/6] mm/list_lru.c: rename kvfree_rcu() to local variant Uladzislau Rezki (Sony)
2020-03-15 18:18 ` [PATCH v1 2/6] rcu: introduce kvfree_rcu() interface Uladzislau Rezki (Sony)
2020-03-16 15:45   ` Joel Fernandes
2020-03-16 18:55     ` Uladzislau Rezki
2020-03-16 18:57       ` Joel Fernandes
2020-03-16 19:01         ` Joel Fernandes
2020-03-16 19:03         ` Uladzislau Rezki
2020-03-16 19:48           ` Joel Fernandes
2020-03-15 18:18 ` [PATCH v1 3/6] rcu: rename rcu_invoke_kfree_callback/rcu_kfree_callback Uladzislau Rezki (Sony)
2020-03-16 15:47   ` Joel Fernandes
2020-03-15 18:18 ` [PATCH v1 4/6] rcu: rename __is_kfree_rcu_offset() macro Uladzislau Rezki (Sony)
2020-03-16 15:48   ` Joel Fernandes
2020-03-15 18:18 ` [PATCH v1 5/6] rcu: rename kfree_call_rcu()/__kfree_rcu() Uladzislau Rezki (Sony)
2020-03-16 15:25   ` Joel Fernandes
2020-03-16 19:01     ` Uladzislau Rezki
2020-03-15 18:18 ` [PATCH v1 6/6] mm/list_lru.c: remove kvfree_rcu_local() function Uladzislau Rezki (Sony)
2020-03-16 15:49   ` Joel Fernandes

RCU Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/rcu/0 rcu/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 rcu rcu/ https://lore.kernel.org/rcu \
		rcu@vger.kernel.org
	public-inbox-index rcu

Example config snippet for mirrors

Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.rcu


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git