[Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Lukas Wunner]

Commit torvalds/linux@88dca4ca5a93 removed the pgprot argument from
__vmalloc().  There's a single invocation in drbd_bitmap.c.  Drop the
argument to stay as close as possible to upstream and apply a cocci
patch for older kernels which reinstates the additional argument.

Reported-by: Michael Hierweck <michael.hierweck@hostsharing.net>
Signed-off-by: Lukas Wunner <lukas@wunner.de>
---
 drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci | 3 +++
 drbd/drbd-kernel-compat/gen_patch_names.c                      | 3 +++
 drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c         | 8 ++++++++
 drbd/drbd_bitmap.c                                             | 3 +--
 4 files changed, 15 insertions(+), 2 deletions(-)
 create mode 100644 drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci
 create mode 100644 drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c

diff --git a/drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci b/drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci
new file mode 100644
index 0000000..335581d
--- /dev/null
+++ b/drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci
@@ -0,0 +1,3 @@
+@@ expression S, G; @@
+- __vmalloc(S, G)
++ __vmalloc(S, G, PAGE_KERNEL)
diff --git a/drbd/drbd-kernel-compat/gen_patch_names.c b/drbd/drbd-kernel-compat/gen_patch_names.c
index 8d5af81..86e9d02 100644
--- a/drbd/drbd-kernel-compat/gen_patch_names.c
+++ b/drbd/drbd-kernel-compat/gen_patch_names.c
@@ -323,6 +323,9 @@ int main(int argc, char **argv)
 	patch(1, "part_stat_h", true, false,
 	      COMPAT_HAVE_PART_STAT_H, "present");
 
+	patch(1, "__vmalloc", true, false,
+	      COMPAT___VMALLOC_HAS_2_PARAMS, "has_2_params");
+
 /* #define BLKDEV_ISSUE_ZEROOUT_EXPORTED */
 /* #define BLKDEV_ZERO_NOUNMAP */
 
diff --git a/drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c b/drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c
new file mode 100644
index 0000000..00e2c28
--- /dev/null
+++ b/drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c
@@ -0,0 +1,8 @@
+/* { "version": "v5.8-rc1", "commit": "88dca4ca5a93d2c09e5bbc6a62fbfc3af83c4fca", "comment": "pgprot argument to __vmalloc was removed", "author": "Christoph Hellwig <hch@lst.de>", "date": "Mon Jun 1 21:51:40 2020 -0700" } */
+
+#include <linux/vmalloc.h>
+
+void foo(void)
+{
+	__vmalloc(0, 0);
+}
diff --git a/drbd/drbd_bitmap.c b/drbd/drbd_bitmap.c
index 4dbed26..e23ed5a 100644
--- a/drbd/drbd_bitmap.c
+++ b/drbd/drbd_bitmap.c
@@ -366,9 +366,7 @@ static struct page **bm_realloc_pages(struct drbd_bitmap *b, unsigned long want)
 	new_pages = kzalloc(bytes, GFP_NOIO | __GFP_NOWARN);
 	if (!new_pages) {
 		new_pages = __vmalloc(bytes,
-				GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO,
-				PAGE_KERNEL);
+				      GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO);
 		if (!new_pages)
 			return NULL;
 	}
-- 
2.27.0

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Elimar Riesebieter]

* Lukas Wunner <lukas@wunner.de> [2020-10-11 18:07 +0200]:

> Commit torvalds/linux@88dca4ca5a93 removed the pgprot argument from
> __vmalloc().  There's a single invocation in drbd_bitmap.c.  Drop the
> argument to stay as close as possible to upstream and apply a cocci
> patch for older kernels which reinstates the additional argument.
> 
> Reported-by: Michael Hierweck <michael.hierweck@hostsharing.net>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>  drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci | 3 +++
>  drbd/drbd-kernel-compat/gen_patch_names.c                      | 3 +++
>  drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c         | 8 ++++++++
>  drbd/drbd_bitmap.c                                             | 3 +--

Applied that patch to tag 9.0.25-1. Building the module via dkms
gives:

/var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c: In function ‘bm_realloc_pages’:
/var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:368:15: error: too few arguments to function ‘__vmalloc’
   new_pages = __vmalloc(bytes,
               ^~~~~~~~~
In file included from /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:16:
/usr/src/linux-headers-4.19.0-9-common/include/linux/vmalloc.h:80:14: note: declared here
 extern void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot);
              ^~~~~~~~~

Elimar
-- 
  Numeric stability is probably not all that
  important when you're guessing;-)

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Elimar Riesebieter]

* Elimar Riesebieter <riesebie@lxtec.de> [2020-10-11 20:39 +0200]:

> Applied that patch to tag 9.0.25-1. Building the module via dkms
> gives:
> 
> /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c: In function ‘bm_realloc_pages’:
> /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:368:15: error: too few arguments to function ‘__vmalloc’
>    new_pages = __vmalloc(bytes,
>                ^~~~~~~~~
> In file included from /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:16:
> /usr/src/linux-headers-4.19.0-9-common/include/linux/vmalloc.h:80:14: note: declared here
>  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot);
>               ^~~~~~~~~

This was on 4.19. I think the changes must depend on KVERS as the
modules should build from 4.19 upwards. Will try your patch against
5.8 tomorrow.

Elimar
-- 
  From The Collaborative International Dictionary of English v.0.48 [gcide]:
  .
  arsehole \arse"hole`\ ([aum]rs"h[=o]l`), n.
         1. execretory opening at the end of the alimentary canal.

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Lukas Wunner]

On Sun, Oct 11, 2020 at 08:39:30PM +0200, Elimar Riesebieter wrote:
> Applied that patch to tag 9.0.25-1. Building the module via dkms
> gives:
> 
> /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c: In function ???bm_realloc_pages???:
> /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:368:15: error: too few arguments to function ???__vmalloc???
>    new_pages = __vmalloc(bytes,
>                ^~~~~~~~~
> In file included from /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:16:
> /usr/src/linux-headers-4.19.0-9-common/include/linux/vmalloc.h:80:14: note: declared here
>  extern void *__vmalloc(unsigned long size, gfp_t gfp_mask, pgprot_t prot);
>               ^~~~~~~~~

The patch worked for me with v5.3.  Be aware that with DKMS you have
to patch the source code in /usr/src, then delete the copy in
/var/lib/dkms such that it is recreated based on the patched
source code.

Thanks,

Lukas

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Lukas Wunner]

On Sun, Oct 11, 2020 at 08:47:41PM +0200, Elimar Riesebieter wrote:
> * Elimar Riesebieter <riesebie@lxtec.de> [2020-10-11 20:39 +0200]:
> > /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c: In function "bm_realloc_pages":
> > /var/lib/dkms/drbd/9.0.25-1hsh2/build/src/drbd/drbd_bitmap.c:368:15: error: too few arguments to function "__vmalloc"
> 
> This was on 4.19. I think the changes must depend on KVERS as the
> modules should build from 4.19 upwards. Will try your patch against
> 5.8 tomorrow.

The patch tests whether __vmalloc() expects 2 arguments and applies
the coccinelle patch if not.  The coccinelle patch adds the third
argument to the __vmalloc() invocation.  This works independently
of the specific kernel version.

Thanks,

Lukas

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Elimar Riesebieter]

* Lukas Wunner <lukas@wunner.de> [2020-10-11 20:51 +0200]:

[...]

> Be aware that with DKMS you have
> to patch the source code in /usr/src, then delete the copy in
> /var/lib/dkms such that it is recreated based on the patched
> source code.

This is what my Debian package does automagic ;-)

Elimar
-- 
  Do you smell something burning or is it me?

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Elimar Riesebieter]

* Lukas Wunner <lukas@wunner.de> [2020-10-11 20:51 +0200]:

[...]

> The patch worked for me with v5.3.

Did you build with brdb master? I used tag 9.0.25-1.

Elimar
-- 
  Never make anything simple and efficient when a way
  can be found to make it complex and wonderful ;-)

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Lukas Wunner]

On Sun, Oct 11, 2020 at 09:30:48PM +0200, Elimar Riesebieter wrote:
> * Lukas Wunner <lukas@wunner.de> [2020-10-11 20:51 +0200]:
> > The patch worked for me with v5.3.
> 
> Did you build with brdb master? I used tag 9.0.25-1.

The patch is meant to be applied to the drbd-9.0 branch where all
development seems to happen.  It also applies cleanly to the
drbd-9.0.25-1 tag.

I'm afraid I cannot reproduce the issue you're seeing with your
DKMS package on v4.19.  Using tag drbd-9.0.25-1 and v4.19.95,
the call to __vmalloc() is correctly patched in drbd_bitmap.c
but the build fails for a different reason (conflicting types
for blkdev_issue_zeroout()).  Doesn't look like 4.19 is well
supported.

If you're unsuccessful debugging the build failure on your own,
maybe you can upload your drbd build directory somewhere so I
can take a look.

Thanks,

Lukas

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Elimar Riesebieter]

* Lukas Wunner <lukas@wunner.de> [2020-10-11 18:07 +0200]:
[...]
> --- a/drbd/drbd_bitmap.c
> +++ b/drbd/drbd_bitmap.c
> @@ -366,9 +366,7 @@ static struct page **bm_realloc_pages(struct drbd_bitmap *b, unsigned long want)
          ^ should be 8 otherwise this patch doesn't apply.
>  	new_pages = kzalloc(bytes, GFP_NOIO | __GFP_NOWARN);
>  	if (!new_pages) {
>  		new_pages = __vmalloc(bytes,
> -				GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO,
> -				PAGE_KERNEL);
> +				      GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO);
>  		if (!new_pages)
>  			return NULL;
>  	}
> -- 
> 2.27.0
> 

-- 
  Alles, was viel bedacht wird, wird bedenklich!;-)
         Friedrich Nietzsche

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Elimar Riesebieter]

Hi Lukas,

* Lukas Wunner <lukas@wunner.de> [2020-10-12 02:54 +0200]:

[...] 
> If you're unsuccessful debugging the build failure on your own,
> maybe you can upload your drbd build directory somewhere so I
> can take a look.

https://lxtec.de/drbd/drbd_dkms_buildir_hsh_201012.tar.xz

BTW without your patch the module was build successful with 4.19 and
5.7 though.

Thanks for cooperation
Elimar
-- 
  Never make anything simple and efficient when a way
  can be found to make it complex and wonderful ;-)

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Christoph Böhmwalder]

On 10/11/20 6:07 PM, Lukas Wunner wrote:
> Commit torvalds/linux@88dca4ca5a93 removed the pgprot argument from
> __vmalloc().  There's a single invocation in drbd_bitmap.c.  Drop the
> argument to stay as close as possible to upstream and apply a cocci
> patch for older kernels which reinstates the additional argument.
> 
> Reported-by: Michael Hierweck <michael.hierweck@hostsharing.net>
> Signed-off-by: Lukas Wunner <lukas@wunner.de>
> ---
>   drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci | 3 +++
>   drbd/drbd-kernel-compat/gen_patch_names.c                      | 3 +++
>   drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c         | 8 ++++++++
>   drbd/drbd_bitmap.c                                             | 3 +--
>   4 files changed, 15 insertions(+), 2 deletions(-)
>   create mode 100644 drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci
>   create mode 100644 drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c
> 
> diff --git a/drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci b/drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci
> new file mode 100644
> index 0000000..335581d
> --- /dev/null
> +++ b/drbd/drbd-kernel-compat/cocci/__vmalloc__no_has_2_params.cocci
> @@ -0,0 +1,3 @@
> +@@ expression S, G; @@
> +- __vmalloc(S, G)
> ++ __vmalloc(S, G, PAGE_KERNEL)
> diff --git a/drbd/drbd-kernel-compat/gen_patch_names.c b/drbd/drbd-kernel-compat/gen_patch_names.c
> index 8d5af81..86e9d02 100644
> --- a/drbd/drbd-kernel-compat/gen_patch_names.c
> +++ b/drbd/drbd-kernel-compat/gen_patch_names.c
> @@ -323,6 +323,9 @@ int main(int argc, char **argv)
>   	patch(1, "part_stat_h", true, false,
>   	      COMPAT_HAVE_PART_STAT_H, "present");
>   
> +	patch(1, "__vmalloc", true, false,
> +	      COMPAT___VMALLOC_HAS_2_PARAMS, "has_2_params");
> +
>   /* #define BLKDEV_ISSUE_ZEROOUT_EXPORTED */
>   /* #define BLKDEV_ZERO_NOUNMAP */
>   
> diff --git a/drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c b/drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c
> new file mode 100644
> index 0000000..00e2c28
> --- /dev/null
> +++ b/drbd/drbd-kernel-compat/tests/__vmalloc_has_2_params.c
> @@ -0,0 +1,8 @@
> +/* { "version": "v5.8-rc1", "commit": "88dca4ca5a93d2c09e5bbc6a62fbfc3af83c4fca", "comment": "pgprot argument to __vmalloc was removed", "author": "Christoph Hellwig <hch@lst.de>", "date": "Mon Jun 1 21:51:40 2020 -0700" } */
> +
> +#include <linux/vmalloc.h>
> +
> +void foo(void)
> +{
> +	__vmalloc(0, 0);
> +}
> diff --git a/drbd/drbd_bitmap.c b/drbd/drbd_bitmap.c
> index 4dbed26..e23ed5a 100644
> --- a/drbd/drbd_bitmap.c
> +++ b/drbd/drbd_bitmap.c
> @@ -366,9 +366,7 @@ static struct page **bm_realloc_pages(struct drbd_bitmap *b, unsigned long want)
>   	new_pages = kzalloc(bytes, GFP_NOIO | __GFP_NOWARN);
>   	if (!new_pages) {
>   		new_pages = __vmalloc(bytes,
> -				GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO,
> -				PAGE_KERNEL);
> +				      GFP_NOIO | __GFP_HIGHMEM | __GFP_ZERO);
>   		if (!new_pages)
>   			return NULL;
>   	}
> 

Amazing. Very cool to have someone external figure out and contribute to 
such a complex and niche part of drbd – it's much appreciated!

Applied internally, will be released with the next version. Thanks again!

Regards,
Christoph

-- 
Christoph Böhmwalder
LINBIT | Keeping the Digital World Running
DRBD HA —  Disaster Recovery — Software defined Storage

Re: [Drbd-dev] [PATCH drbd-9.0] compat: __vmalloc() pgprot argument was removed in v5.8

[Lukas Wunner]

Hi Elimar,

my apologies for the delay.

On Mon, Oct 12, 2020 at 01:14:39PM +0200, Elimar Riesebieter wrote:
> * Lukas Wunner <lukas@wunner.de> [2020-10-12 02:54 +0200]:
> > If you're unsuccessful debugging the build failure on your own,
> > maybe you can upload your drbd build directory somewhere so I
> > can take a look.
> 
> https://lxtec.de/drbd/drbd_dkms_buildir_hsh_201012.tar.xz

If coccinelle is not available, drbd-kernel-compat/gen_compat_patch.sh
attempts to fetch the coccinelle-generated patch from the cloud by
sending a POST request to https://drbd.io:2020/api/v1/spatch/9.0.25-1
with the locally-generated compat.h.

Because the cloud service generates the patch for the release version
9.0.25-1 and not for your locally-patched source, you end up with a
patch which doesn't work.

The solution is to disable usage of the cloud service by setting the
environment variable SPAAS=false and adding coccinelle >= 1.0.8 to
the drbd-dkms package's prerequisites.  It's not safe to use the
cloud service if you apply custom patches to the source code.

I was unable to reproduce the issue because I built from the git repo
and gen_compat_patch.sh never uses the cloud service in that case.

HTH,

Lukas