diff mbox

patches: add patch for compat/lib-rhashtable.c

Message ID 1495226777-11429-1-git-send-email-arend.vanspriel@broadcom.com (mailing list archive)
State Accepted
Headers show

Commit Message

Arend van Spriel May 19, 2017, 8:46 p.m. UTC
The file compat/lib-rhashtable.c is a copy from the backported kernel
source lib/rhashtable.c. This patch reverts a recent change to that
file, ie. commit 43ca5bc4f72e ("lib/rhashtable.c: simplify a strange
allocation pattern"). It introduced the function gfpflags_allow_blocking()
introduced in 4.4 kernel and kvmalloc() introduced in 4.12-rc1. Looking
at those functions backporting them is complicated so instead add this
patch that reverts the change for kernel prior to 4.12.

Signed-off-by: Arend van Spriel <arend.vanspriel@broadcom.com>
---
 patches/lib-rhashtable.patch | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)
 create mode 100644 patches/lib-rhashtable.patch

Comments

Johannes Berg May 24, 2017, 6:46 a.m. UTC | #1
On Fri, 2017-05-19 at 21:46 +0100, Arend van Spriel wrote:
> The file compat/lib-rhashtable.c is a copy from the backported kernel
> source lib/rhashtable.c. This patch reverts a recent change to that
> file, ie. commit 43ca5bc4f72e ("lib/rhashtable.c: simplify a strange
> allocation pattern"). It introduced the function
> gfpflags_allow_blocking()
> introduced in 4.4 kernel and kvmalloc() introduced in 4.12-rc1.
> Looking
> at those functions backporting them is complicated so instead add
> this
> patch that reverts the change for kernel prior to 4.12.

Thanks Arend!

Why do you think backporting it is complicated though?

kvmalloc() is just kvmalloc_node(), and if we disregard the
__vmalloc_node_flags_caller() but - since kvmalloc() doesn't care about
node anyway - just use __vmalloc() there, it should be easy? The
pgprot_t argument is just PAGE_KERNEL, and the other stuff doesn't
really matter.

gfpflags_allow_blocking() is a pretty simple inline, and even if we'd
implement it to always return false we'd get the old rhashtable
behaviour.

johannes
--
To unsubscribe from this list: send the line "unsubscribe backports" in
Arend van Spriel May 24, 2017, 7:36 p.m. UTC | #2
On 24-05-17 08:46, Johannes Berg wrote:
> On Fri, 2017-05-19 at 21:46 +0100, Arend van Spriel wrote:
>> The file compat/lib-rhashtable.c is a copy from the backported kernel
>> source lib/rhashtable.c. This patch reverts a recent change to that
>> file, ie. commit 43ca5bc4f72e ("lib/rhashtable.c: simplify a strange
>> allocation pattern"). It introduced the function
>> gfpflags_allow_blocking()
>> introduced in 4.4 kernel and kvmalloc() introduced in 4.12-rc1.
>> Looking
>> at those functions backporting them is complicated so instead add
>> this
>> patch that reverts the change for kernel prior to 4.12.
> 
> Thanks Arend!
> 
> Why do you think backporting it is complicated though?
> 
> kvmalloc() is just kvmalloc_node(), and if we disregard the
> __vmalloc_node_flags_caller() but - since kvmalloc() doesn't care about
> node anyway - just use __vmalloc() there, it should be easy? The
> pgprot_t argument is just PAGE_KERNEL, and the other stuff doesn't
> really matter.
> 
> gfpflags_allow_blocking() is a pretty simple inline, and even if we'd
> implement it to always return false we'd get the old rhashtable
> behaviour.

I was reading commit d0164adc89f6 ("mm, page_alloc: distinguish between
being unable to sleep, unwilling to slee...") which introduced
gfpflags_allow_blocking() and did a lot more stuff. So I could not
really determine the implications of backporting
gfpflags_allow_blocking(). We can indeed probably make it work for
rhashtable easily, but will that be appropriate if some other backport
code starts using it.

Regards,
Arend
--
To unsubscribe from this list: send the line "unsubscribe backports" in
Johannes Berg May 30, 2017, 7:19 a.m. UTC | #3
On Wed, 2017-05-24 at 21:36 +0200, Arend van Spriel wrote:

> > kvmalloc() is just kvmalloc_node(), and if we disregard the
> > __vmalloc_node_flags_caller() but - since kvmalloc() doesn't care
> > about
> > node anyway - just use __vmalloc() there, it should be easy? The
> > pgprot_t argument is just PAGE_KERNEL, and the other stuff doesn't
> > really matter.
> > 
> > gfpflags_allow_blocking() is a pretty simple inline, and even if
> > we'd
> > implement it to always return false we'd get the old rhashtable
> > behaviour.
> 
> I was reading commit d0164adc89f6 ("mm, page_alloc: distinguish
> between
> being unable to sleep, unwilling to slee...") which introduced
> gfpflags_allow_blocking() and did a lot more stuff. So I could not
> really determine the implications of backporting
> gfpflags_allow_blocking(). We can indeed probably make it work for
> rhashtable easily, but will that be appropriate if some other
> backport code starts using it.

Huh, ok, that's a good point. I guess I'll take this for now.

johannes
--
To unsubscribe from this list: send the line "unsubscribe backports" in
diff mbox

Patch

diff --git a/patches/lib-rhashtable.patch b/patches/lib-rhashtable.patch
new file mode 100644
index 0000000..9c262b0
--- /dev/null
+++ b/patches/lib-rhashtable.patch
@@ -0,0 +1,32 @@ 
+--- a/compat/lib-rhashtable.c
++++ b/compat/lib-rhashtable.c
+@@ -86,11 +86,26 @@ static int alloc_bucket_locks(struct rhashtable *ht, struct bucket_table *tbl,
+ 		size = min(size, 1U << tbl->nest);
+ 
+ 	if (sizeof(spinlock_t) != 0) {
++#if LINUX_VERSION_IS_LESS(4,12,0)
++		tbl->locks = NULL;
++#ifdef CONFIG_NUMA
++		if (size * sizeof(spinlock_t) > PAGE_SIZE &&
++		    gfp == GFP_KERNEL)
++			tbl->locks = vmalloc(size * sizeof(spinlock_t));
++#endif
++		if (gfp != GFP_KERNEL)
++			gfp |= __GFP_NOWARN | __GFP_NORETRY;
++
++		if (!tbl->locks)
++			tbl->locks = kmalloc_array(size, sizeof(spinlock_t),
++						   gfp);
++#else
+ 		if (gfpflags_allow_blocking(gfp))
+ 			tbl->locks = kvmalloc(size * sizeof(spinlock_t), gfp);
+ 		else
+ 			tbl->locks = kmalloc_array(size, sizeof(spinlock_t),
+ 						   gfp);
++#endif
+ 		if (!tbl->locks)
+ 			return -ENOMEM;
+ 		for (i = 0; i < size; i++)
+-- 
+1.9.1
+