2020-03-15 18:19:33

by Uladzislau Rezki

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

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) <[email protected]>
---
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


2020-03-16 15:46:42

by Joel Fernandes

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

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) <[email protected]>
> ---
> 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
>

2020-03-16 18:56:05

by Uladzislau Rezki

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

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) <[email protected]>
> > ---
> > 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

2020-03-16 18:58:07

by Joel Fernandes

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

On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <[email protected]> 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) <[email protected]>
> > > ---
> > > 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

2020-03-16 19:02:11

by Joel Fernandes

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

On Mon, Mar 16, 2020 at 2:57 PM Joel Fernandes <[email protected]> wrote:
>
> On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <[email protected]> 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) <[email protected]>
> > > > ---
> > > > 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.

2020-03-16 19:05:13

by Uladzislau Rezki

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

On Mon, Mar 16, 2020 at 02:57:18PM -0400, Joel Fernandes wrote:
> On Mon, Mar 16, 2020 at 2:55 PM Uladzislau Rezki <[email protected]> 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) <[email protected]>
> > > > ---
> > > > 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

2020-03-16 19:49:37

by Joel Fernandes

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

On Mon, Mar 16, 2020 at 3:03 PM Uladzislau Rezki <[email protected]> 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 <[email protected]> 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) <[email protected]>
> > > > > ---
> > > > > 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