All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-07-07 23:15 ` Fabio M. De Francesco
  0 siblings, 0 replies; 14+ messages in thread
From: Fabio M. De Francesco @ 2022-07-07 23:15 UTC (permalink / raw)
  To: Eric Biederman, kexec, linux-kernel; +Cc: Fabio M. De Francesco, Ira Weiny

The use of kmap() and kmap_atomic() are being deprecated in favor of
kmap_local_page().

With kmap_local_page(), the mappings are per thread, CPU local and not
globally visible. Furthermore, the mappings can be acquired from any
context (including interrupts).

Therefore, use kmap_local_page() in kexec_core.c because these mappings are
per thread, CPU local, and not globally visible.

Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
enabled.

Suggested-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---

v1->v2: A sentence of the commit message contained an error due to a
mistake in copy-pasting from a previous patch. Replace "aio.c" with
"kexec_core.c".

 kernel/kexec_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 4d34c78334ce..6f98274765d4 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
 		if (result < 0)
 			goto out;
 
-		ptr = kmap(page);
+		ptr = kmap_local_page(page);
 		/* Start with a clear page */
 		clear_page(ptr);
 		ptr += maddr & ~PAGE_MASK;
@@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
 			memcpy(ptr, kbuf, uchunk);
 		else
 			result = copy_from_user(ptr, buf, uchunk);
-		kunmap(page);
+		kunmap_local(ptr);
 		if (result) {
 			result = -EFAULT;
 			goto out;
@@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
 			goto out;
 		}
 		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
-		ptr = kmap(page);
+		ptr = kmap_local_page(page);
 		ptr += maddr & ~PAGE_MASK;
 		mchunk = min_t(size_t, mbytes,
 				PAGE_SIZE - (maddr & ~PAGE_MASK));
@@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
 		else
 			result = copy_from_user(ptr, buf, uchunk);
 		kexec_flush_icache_page(page);
-		kunmap(page);
+		kunmap_local(ptr);
 		arch_kexec_pre_free_pages(page_address(page), 1);
 		if (result) {
 			result = -EFAULT;
-- 
2.36.1


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

* [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-07-07 23:15 ` Fabio M. De Francesco
  0 siblings, 0 replies; 14+ messages in thread
From: Fabio M. De Francesco @ 2022-07-07 23:15 UTC (permalink / raw)
  To: Eric Biederman, kexec, linux-kernel; +Cc: Fabio M. De Francesco, Ira Weiny

The use of kmap() and kmap_atomic() are being deprecated in favor of
kmap_local_page().

With kmap_local_page(), the mappings are per thread, CPU local and not
globally visible. Furthermore, the mappings can be acquired from any
context (including interrupts).

Therefore, use kmap_local_page() in kexec_core.c because these mappings are
per thread, CPU local, and not globally visible.

Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
enabled.

Suggested-by: Ira Weiny <ira.weiny@intel.com>
Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
---

v1->v2: A sentence of the commit message contained an error due to a
mistake in copy-pasting from a previous patch. Replace "aio.c" with
"kexec_core.c".

 kernel/kexec_core.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
index 4d34c78334ce..6f98274765d4 100644
--- a/kernel/kexec_core.c
+++ b/kernel/kexec_core.c
@@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
 		if (result < 0)
 			goto out;
 
-		ptr = kmap(page);
+		ptr = kmap_local_page(page);
 		/* Start with a clear page */
 		clear_page(ptr);
 		ptr += maddr & ~PAGE_MASK;
@@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
 			memcpy(ptr, kbuf, uchunk);
 		else
 			result = copy_from_user(ptr, buf, uchunk);
-		kunmap(page);
+		kunmap_local(ptr);
 		if (result) {
 			result = -EFAULT;
 			goto out;
@@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
 			goto out;
 		}
 		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
-		ptr = kmap(page);
+		ptr = kmap_local_page(page);
 		ptr += maddr & ~PAGE_MASK;
 		mchunk = min_t(size_t, mbytes,
 				PAGE_SIZE - (maddr & ~PAGE_MASK));
@@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
 		else
 			result = copy_from_user(ptr, buf, uchunk);
 		kexec_flush_icache_page(page);
-		kunmap(page);
+		kunmap_local(ptr);
 		arch_kexec_pre_free_pages(page_address(page), 1);
 		if (result) {
 			result = -EFAULT;
-- 
2.36.1


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
  2022-07-07 23:15 ` Fabio M. De Francesco
@ 2022-07-21 22:50   ` Ira Weiny
  -1 siblings, 0 replies; 14+ messages in thread
From: Ira Weiny @ 2022-07-21 22:50 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: Eric Biederman, kexec, linux-kernel

On Fri, Jul 08, 2022 at 01:15:50AM +0200, Fabio M. De Francesco wrote:
> The use of kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page().
> 
> With kmap_local_page(), the mappings are per thread, CPU local and not
> globally visible. Furthermore, the mappings can be acquired from any
> context (including interrupts).
> 
> Therefore, use kmap_local_page() in kexec_core.c because these mappings are
> per thread, CPU local, and not globally visible.
> 
> Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> enabled.
> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v1->v2: A sentence of the commit message contained an error due to a
> mistake in copy-pasting from a previous patch. Replace "aio.c" with
> "kexec_core.c".
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4d34c78334ce..6f98274765d4 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  		if (result < 0)
>  			goto out;
>  
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		/* Start with a clear page */
>  		clear_page(ptr);
>  		ptr += maddr & ~PAGE_MASK;
> @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  			memcpy(ptr, kbuf, uchunk);
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			goto out;
>  		}
>  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
>  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
> -- 
> 2.36.1
> 

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-07-21 22:50   ` Ira Weiny
  0 siblings, 0 replies; 14+ messages in thread
From: Ira Weiny @ 2022-07-21 22:50 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: Eric Biederman, kexec, linux-kernel

On Fri, Jul 08, 2022 at 01:15:50AM +0200, Fabio M. De Francesco wrote:
> The use of kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page().
> 
> With kmap_local_page(), the mappings are per thread, CPU local and not
> globally visible. Furthermore, the mappings can be acquired from any
> context (including interrupts).
> 
> Therefore, use kmap_local_page() in kexec_core.c because these mappings are
> per thread, CPU local, and not globally visible.
> 
> Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> enabled.
> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v1->v2: A sentence of the commit message contained an error due to a
> mistake in copy-pasting from a previous patch. Replace "aio.c" with
> "kexec_core.c".
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4d34c78334ce..6f98274765d4 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  		if (result < 0)
>  			goto out;
>  
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		/* Start with a clear page */
>  		clear_page(ptr);
>  		ptr += maddr & ~PAGE_MASK;
> @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  			memcpy(ptr, kbuf, uchunk);
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			goto out;
>  		}
>  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
>  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
> -- 
> 2.36.1
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
  2022-07-07 23:15 ` Fabio M. De Francesco
@ 2022-07-31  1:58   ` Ira Weiny
  -1 siblings, 0 replies; 14+ messages in thread
From: Ira Weiny @ 2022-07-31  1:58 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: Eric Biederman, kexec, linux-kernel

On Fri, Jul 08, 2022 at 01:15:50AM +0200, Fabio M. De Francesco wrote:
> The use of kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page().
> 
> With kmap_local_page(), the mappings are per thread, CPU local and not
> globally visible. Furthermore, the mappings can be acquired from any
> context (including interrupts).
> 
> Therefore, use kmap_local_page() in kexec_core.c because these mappings are
> per thread, CPU local, and not globally visible.
> 
> Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> enabled.
> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
> 
> v1->v2: A sentence of the commit message contained an error due to a
> mistake in copy-pasting from a previous patch. Replace "aio.c" with
> "kexec_core.c".
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4d34c78334ce..6f98274765d4 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  		if (result < 0)
>  			goto out;
>  
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		/* Start with a clear page */
>  		clear_page(ptr);
>  		ptr += maddr & ~PAGE_MASK;
> @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  			memcpy(ptr, kbuf, uchunk);
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			goto out;
>  		}
>  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
>  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
> -- 
> 2.36.1
> 

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-07-31  1:58   ` Ira Weiny
  0 siblings, 0 replies; 14+ messages in thread
From: Ira Weiny @ 2022-07-31  1:58 UTC (permalink / raw)
  To: Fabio M. De Francesco; +Cc: Eric Biederman, kexec, linux-kernel

On Fri, Jul 08, 2022 at 01:15:50AM +0200, Fabio M. De Francesco wrote:
> The use of kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page().
> 
> With kmap_local_page(), the mappings are per thread, CPU local and not
> globally visible. Furthermore, the mappings can be acquired from any
> context (including interrupts).
> 
> Therefore, use kmap_local_page() in kexec_core.c because these mappings are
> per thread, CPU local, and not globally visible.
> 
> Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> enabled.
> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> ---
> 
> v1->v2: A sentence of the commit message contained an error due to a
> mistake in copy-pasting from a previous patch. Replace "aio.c" with
> "kexec_core.c".
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4d34c78334ce..6f98274765d4 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  		if (result < 0)
>  			goto out;
>  
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		/* Start with a clear page */
>  		clear_page(ptr);
>  		ptr += maddr & ~PAGE_MASK;
> @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  			memcpy(ptr, kbuf, uchunk);
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			goto out;
>  		}
>  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
>  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
> -- 
> 2.36.1
> 

_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
  2022-07-07 23:15 ` Fabio M. De Francesco
@ 2022-08-02  3:06   ` Baoquan He
  -1 siblings, 0 replies; 14+ messages in thread
From: Baoquan He @ 2022-08-02  3:06 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> The use of kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page().
> 
> With kmap_local_page(), the mappings are per thread, CPU local and not
> globally visible. Furthermore, the mappings can be acquired from any
> context (including interrupts).
> 
> Therefore, use kmap_local_page() in kexec_core.c because these mappings are
> per thread, CPU local, and not globally visible.
> 
> Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> enabled.

Wondering what arch you tested with. 

This looks good, but may not benefit much. Say so because I doubt
how many 32bit systems are using kexec/kdump mechanism.

Anyway, 

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v1->v2: A sentence of the commit message contained an error due to a
> mistake in copy-pasting from a previous patch. Replace "aio.c" with
> "kexec_core.c".
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4d34c78334ce..6f98274765d4 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  		if (result < 0)
>  			goto out;
>  
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		/* Start with a clear page */
>  		clear_page(ptr);
>  		ptr += maddr & ~PAGE_MASK;
> @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  			memcpy(ptr, kbuf, uchunk);
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			goto out;
>  		}
>  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
>  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
> -- 
> 2.36.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-08-02  3:06   ` Baoquan He
  0 siblings, 0 replies; 14+ messages in thread
From: Baoquan He @ 2022-08-02  3:06 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> The use of kmap() and kmap_atomic() are being deprecated in favor of
> kmap_local_page().
> 
> With kmap_local_page(), the mappings are per thread, CPU local and not
> globally visible. Furthermore, the mappings can be acquired from any
> context (including interrupts).
> 
> Therefore, use kmap_local_page() in kexec_core.c because these mappings are
> per thread, CPU local, and not globally visible.
> 
> Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> enabled.

Wondering what arch you tested with. 

This looks good, but may not benefit much. Say so because I doubt
how many 32bit systems are using kexec/kdump mechanism.

Anyway, 

Acked-by: Baoquan He <bhe@redhat.com>

> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
> 
> v1->v2: A sentence of the commit message contained an error due to a
> mistake in copy-pasting from a previous patch. Replace "aio.c" with
> "kexec_core.c".
> 
>  kernel/kexec_core.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> index 4d34c78334ce..6f98274765d4 100644
> --- a/kernel/kexec_core.c
> +++ b/kernel/kexec_core.c
> @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  		if (result < 0)
>  			goto out;
>  
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		/* Start with a clear page */
>  		clear_page(ptr);
>  		ptr += maddr & ~PAGE_MASK;
> @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage *image,
>  			memcpy(ptr, kbuf, uchunk);
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		if (result) {
>  			result = -EFAULT;
>  			goto out;
> @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  			goto out;
>  		}
>  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> -		ptr = kmap(page);
> +		ptr = kmap_local_page(page);
>  		ptr += maddr & ~PAGE_MASK;
>  		mchunk = min_t(size_t, mbytes,
>  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage *image,
>  		else
>  			result = copy_from_user(ptr, buf, uchunk);
>  		kexec_flush_icache_page(page);
> -		kunmap(page);
> +		kunmap_local(ptr);
>  		arch_kexec_pre_free_pages(page_address(page), 1);
>  		if (result) {
>  			result = -EFAULT;
> -- 
> 2.36.1
> 
> 
> _______________________________________________
> kexec mailing list
> kexec@lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/kexec
> 


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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
  2022-08-02  3:06   ` Baoquan He
@ 2022-08-02  6:20     ` Fabio M. De Francesco
  -1 siblings, 0 replies; 14+ messages in thread
From: Fabio M. De Francesco @ 2022-08-02  6:20 UTC (permalink / raw)
  To: Baoquan He; +Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > The use of kmap() and kmap_atomic() are being deprecated in favor of
> > kmap_local_page().
> > 
> > With kmap_local_page(), the mappings are per thread, CPU local and not
> > globally visible. Furthermore, the mappings can be acquired from any
> > context (including interrupts).
> > 
> > Therefore, use kmap_local_page() in kexec_core.c because these mappings 
are
> > per thread, CPU local, and not globally visible.
> > 
> > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > enabled.
> 
> Wondering what arch you tested with.

I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
This is usually an information that I add in the commit messages of all the 
recent conversions I'm working on across the entire kernel.

Another important information (overlooked again this time) is that (1) 
kmap() comes with an overhead as mapping space is restricted and protected 
by a global lock for synchronization and (2) it also requires global TLB 
invalidation when the kmap’s pool wraps and it might block when the mapping 
space is fully utilized until a slot becomes available.

More information about why these kmap() to kmap_local_page() conversions 
are needed / preferred can be found in the recent changes I made to 
highmem.rst. They are already in mainline since about two months.

A second round of additional changes has been taken by Andrew M. just few 
days ago.

My goal is to convert the most of the kmap() call sites that are still left 
across the whole kernel. I'm not yet sure that these kinds of conversions 
can be done everywhere, especially if the kernel virtual address of the 
mapping is handed to other contexts, because this would invalidate the 
pointer returned by kmap_local_page().  

> This looks good, but may not benefit much. Say so because I doubt
> how many 32bit systems are using kexec/kdump mechanism.

I really cannot say nothing about how many 32 bits systems are using kexec/
kdump mechanism, however I still think that the conversions are worth 
everywhere. 

> Anyway, 
> 
> Acked-by: Baoquan He <bhe@redhat.com>
> 

Thank you so much!

Fabio

> > 
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > 
> > v1->v2: A sentence of the commit message contained an error due to a
> > mistake in copy-pasting from a previous patch. Replace "aio.c" with
> > "kexec_core.c".
> > 
> >  kernel/kexec_core.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > index 4d34c78334ce..6f98274765d4 100644
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage 
*image,
> >  		if (result < 0)
> >  			goto out;
> >  
> > -		ptr = kmap(page);
> > +		ptr = kmap_local_page(page);
> >  		/* Start with a clear page */
> >  		clear_page(ptr);
> >  		ptr += maddr & ~PAGE_MASK;
> > @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage 
*image,
> >  			memcpy(ptr, kbuf, uchunk);
> >  		else
> >  			result = copy_from_user(ptr, buf, uchunk);
> > -		kunmap(page);
> > +		kunmap_local(ptr);
> >  		if (result) {
> >  			result = -EFAULT;
> >  			goto out;
> > @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage 
*image,
> >  			goto out;
> >  		}
> >  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> > -		ptr = kmap(page);
> > +		ptr = kmap_local_page(page);
> >  		ptr += maddr & ~PAGE_MASK;
> >  		mchunk = min_t(size_t, mbytes,
> >  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> > @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage 
*image,
> >  		else
> >  			result = copy_from_user(ptr, buf, uchunk);
> >  		kexec_flush_icache_page(page);
> > -		kunmap(page);
> > +		kunmap_local(ptr);
> >  		arch_kexec_pre_free_pages(page_address(page), 1);
> >  		if (result) {
> >  			result = -EFAULT;
> > -- 
> > 2.36.1
> > 
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> > 
> 
> 





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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-08-02  6:20     ` Fabio M. De Francesco
  0 siblings, 0 replies; 14+ messages in thread
From: Fabio M. De Francesco @ 2022-08-02  6:20 UTC (permalink / raw)
  To: Baoquan He; +Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > The use of kmap() and kmap_atomic() are being deprecated in favor of
> > kmap_local_page().
> > 
> > With kmap_local_page(), the mappings are per thread, CPU local and not
> > globally visible. Furthermore, the mappings can be acquired from any
> > context (including interrupts).
> > 
> > Therefore, use kmap_local_page() in kexec_core.c because these mappings 
are
> > per thread, CPU local, and not globally visible.
> > 
> > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > enabled.
> 
> Wondering what arch you tested with.

I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
This is usually an information that I add in the commit messages of all the 
recent conversions I'm working on across the entire kernel.

Another important information (overlooked again this time) is that (1) 
kmap() comes with an overhead as mapping space is restricted and protected 
by a global lock for synchronization and (2) it also requires global TLB 
invalidation when the kmap’s pool wraps and it might block when the mapping 
space is fully utilized until a slot becomes available.

More information about why these kmap() to kmap_local_page() conversions 
are needed / preferred can be found in the recent changes I made to 
highmem.rst. They are already in mainline since about two months.

A second round of additional changes has been taken by Andrew M. just few 
days ago.

My goal is to convert the most of the kmap() call sites that are still left 
across the whole kernel. I'm not yet sure that these kinds of conversions 
can be done everywhere, especially if the kernel virtual address of the 
mapping is handed to other contexts, because this would invalidate the 
pointer returned by kmap_local_page().  

> This looks good, but may not benefit much. Say so because I doubt
> how many 32bit systems are using kexec/kdump mechanism.

I really cannot say nothing about how many 32 bits systems are using kexec/
kdump mechanism, however I still think that the conversions are worth 
everywhere. 

> Anyway, 
> 
> Acked-by: Baoquan He <bhe@redhat.com>
> 

Thank you so much!

Fabio

> > 
> > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > ---
> > 
> > v1->v2: A sentence of the commit message contained an error due to a
> > mistake in copy-pasting from a previous patch. Replace "aio.c" with
> > "kexec_core.c".
> > 
> >  kernel/kexec_core.c | 8 ++++----
> >  1 file changed, 4 insertions(+), 4 deletions(-)
> > 
> > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > index 4d34c78334ce..6f98274765d4 100644
> > --- a/kernel/kexec_core.c
> > +++ b/kernel/kexec_core.c
> > @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage 
*image,
> >  		if (result < 0)
> >  			goto out;
> >  
> > -		ptr = kmap(page);
> > +		ptr = kmap_local_page(page);
> >  		/* Start with a clear page */
> >  		clear_page(ptr);
> >  		ptr += maddr & ~PAGE_MASK;
> > @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage 
*image,
> >  			memcpy(ptr, kbuf, uchunk);
> >  		else
> >  			result = copy_from_user(ptr, buf, uchunk);
> > -		kunmap(page);
> > +		kunmap_local(ptr);
> >  		if (result) {
> >  			result = -EFAULT;
> >  			goto out;
> > @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage 
*image,
> >  			goto out;
> >  		}
> >  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> > -		ptr = kmap(page);
> > +		ptr = kmap_local_page(page);
> >  		ptr += maddr & ~PAGE_MASK;
> >  		mchunk = min_t(size_t, mbytes,
> >  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> > @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage 
*image,
> >  		else
> >  			result = copy_from_user(ptr, buf, uchunk);
> >  		kexec_flush_icache_page(page);
> > -		kunmap(page);
> > +		kunmap_local(ptr);
> >  		arch_kexec_pre_free_pages(page_address(page), 1);
> >  		if (result) {
> >  			result = -EFAULT;
> > -- 
> > 2.36.1
> > 
> > 
> > _______________________________________________
> > kexec mailing list
> > kexec@lists.infradead.org
> > http://lists.infradead.org/mailman/listinfo/kexec
> > 
> 
> 





_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
  2022-08-02  6:20     ` Fabio M. De Francesco
@ 2022-08-02  7:32       ` Baoquan He
  -1 siblings, 0 replies; 14+ messages in thread
From: Baoquan He @ 2022-08-02  7:32 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On 08/02/22 at 08:20am, Fabio M. De Francesco wrote:
> On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> > On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > > The use of kmap() and kmap_atomic() are being deprecated in favor of
> > > kmap_local_page().
> > > 
> > > With kmap_local_page(), the mappings are per thread, CPU local and not
> > > globally visible. Furthermore, the mappings can be acquired from any
> > > context (including interrupts).
> > > 
> > > Therefore, use kmap_local_page() in kexec_core.c because these mappings 
> are
> > > per thread, CPU local, and not globally visible.
> > > 
> > > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > > enabled.
> > 
> > Wondering what arch you tested with.
> 
> I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
> This is usually an information that I add in the commit messages of all the 
> recent conversions I'm working on across the entire kernel.
> 
> Another important information (overlooked again this time) is that (1) 
> kmap() comes with an overhead as mapping space is restricted and protected 
> by a global lock for synchronization and (2) it also requires global TLB 
> invalidation when the kmap’s pool wraps and it might block when the mapping 
> space is fully utilized until a slot becomes available.

Thanks a lot for update with more details, Fabio. Maybe these can be
added into log with v3 if you think they are worth adding, and with my
Ack since no code change related. You decide.

Thanks again for the effort.

> 
> More information about why these kmap() to kmap_local_page() conversions 
> are needed / preferred can be found in the recent changes I made to 
> highmem.rst. They are already in mainline since about two months.
> 
> A second round of additional changes has been taken by Andrew M. just few 
> days ago.
> 
> My goal is to convert the most of the kmap() call sites that are still left 
> across the whole kernel. I'm not yet sure that these kinds of conversions 
> can be done everywhere, especially if the kernel virtual address of the 
> mapping is handed to other contexts, because this would invalidate the 
> pointer returned by kmap_local_page().  
> 
> > This looks good, but may not benefit much. Say so because I doubt
> > how many 32bit systems are using kexec/kdump mechanism.
> 
> I really cannot say nothing about how many 32 bits systems are using kexec/
> kdump mechanism, however I still think that the conversions are worth 
> everywhere. 

Yes, agree.

> 
> > Anyway, 
> > 
> > Acked-by: Baoquan He <bhe@redhat.com>
> > 
> 
> Thank you so much!
> 
> Fabio
> 
> > > 
> > > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > ---
> > > 
> > > v1->v2: A sentence of the commit message contained an error due to a
> > > mistake in copy-pasting from a previous patch. Replace "aio.c" with
> > > "kexec_core.c".
> > > 
> > >  kernel/kexec_core.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > index 4d34c78334ce..6f98274765d4 100644
> > > --- a/kernel/kexec_core.c
> > > +++ b/kernel/kexec_core.c
> > > @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
> > >  		if (result < 0)
> > >  			goto out;
> > >  
> > > -		ptr = kmap(page);
> > > +		ptr = kmap_local_page(page);
> > >  		/* Start with a clear page */
> > >  		clear_page(ptr);
> > >  		ptr += maddr & ~PAGE_MASK;
> > > @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
> > >  			memcpy(ptr, kbuf, uchunk);
> > >  		else
> > >  			result = copy_from_user(ptr, buf, uchunk);
> > > -		kunmap(page);
> > > +		kunmap_local(ptr);
> > >  		if (result) {
> > >  			result = -EFAULT;
> > >  			goto out;
> > > @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage 
> *image,
> > >  			goto out;
> > >  		}
> > >  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> > > -		ptr = kmap(page);
> > > +		ptr = kmap_local_page(page);
> > >  		ptr += maddr & ~PAGE_MASK;
> > >  		mchunk = min_t(size_t, mbytes,
> > >  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> > > @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage 
> *image,
> > >  		else
> > >  			result = copy_from_user(ptr, buf, uchunk);
> > >  		kexec_flush_icache_page(page);
> > > -		kunmap(page);
> > > +		kunmap_local(ptr);
> > >  		arch_kexec_pre_free_pages(page_address(page), 1);
> > >  		if (result) {
> > >  			result = -EFAULT;
> > > -- 
> > > 2.36.1
> > > 
> > > 
> > > _______________________________________________
> > > kexec mailing list
> > > kexec@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/kexec
> > > 
> > 
> > 
> 
> 
> 
> 


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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-08-02  7:32       ` Baoquan He
  0 siblings, 0 replies; 14+ messages in thread
From: Baoquan He @ 2022-08-02  7:32 UTC (permalink / raw)
  To: Fabio M. De Francesco
  Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On 08/02/22 at 08:20am, Fabio M. De Francesco wrote:
> On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> > On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > > The use of kmap() and kmap_atomic() are being deprecated in favor of
> > > kmap_local_page().
> > > 
> > > With kmap_local_page(), the mappings are per thread, CPU local and not
> > > globally visible. Furthermore, the mappings can be acquired from any
> > > context (including interrupts).
> > > 
> > > Therefore, use kmap_local_page() in kexec_core.c because these mappings 
> are
> > > per thread, CPU local, and not globally visible.
> > > 
> > > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > > enabled.
> > 
> > Wondering what arch you tested with.
> 
> I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
> This is usually an information that I add in the commit messages of all the 
> recent conversions I'm working on across the entire kernel.
> 
> Another important information (overlooked again this time) is that (1) 
> kmap() comes with an overhead as mapping space is restricted and protected 
> by a global lock for synchronization and (2) it also requires global TLB 
> invalidation when the kmap’s pool wraps and it might block when the mapping 
> space is fully utilized until a slot becomes available.

Thanks a lot for update with more details, Fabio. Maybe these can be
added into log with v3 if you think they are worth adding, and with my
Ack since no code change related. You decide.

Thanks again for the effort.

> 
> More information about why these kmap() to kmap_local_page() conversions 
> are needed / preferred can be found in the recent changes I made to 
> highmem.rst. They are already in mainline since about two months.
> 
> A second round of additional changes has been taken by Andrew M. just few 
> days ago.
> 
> My goal is to convert the most of the kmap() call sites that are still left 
> across the whole kernel. I'm not yet sure that these kinds of conversions 
> can be done everywhere, especially if the kernel virtual address of the 
> mapping is handed to other contexts, because this would invalidate the 
> pointer returned by kmap_local_page().  
> 
> > This looks good, but may not benefit much. Say so because I doubt
> > how many 32bit systems are using kexec/kdump mechanism.
> 
> I really cannot say nothing about how many 32 bits systems are using kexec/
> kdump mechanism, however I still think that the conversions are worth 
> everywhere. 

Yes, agree.

> 
> > Anyway, 
> > 
> > Acked-by: Baoquan He <bhe@redhat.com>
> > 
> 
> Thank you so much!
> 
> Fabio
> 
> > > 
> > > Suggested-by: Ira Weiny <ira.weiny@intel.com>
> > > Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> > > ---
> > > 
> > > v1->v2: A sentence of the commit message contained an error due to a
> > > mistake in copy-pasting from a previous patch. Replace "aio.c" with
> > > "kexec_core.c".
> > > 
> > >  kernel/kexec_core.c | 8 ++++----
> > >  1 file changed, 4 insertions(+), 4 deletions(-)
> > > 
> > > diff --git a/kernel/kexec_core.c b/kernel/kexec_core.c
> > > index 4d34c78334ce..6f98274765d4 100644
> > > --- a/kernel/kexec_core.c
> > > +++ b/kernel/kexec_core.c
> > > @@ -814,7 +814,7 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
> > >  		if (result < 0)
> > >  			goto out;
> > >  
> > > -		ptr = kmap(page);
> > > +		ptr = kmap_local_page(page);
> > >  		/* Start with a clear page */
> > >  		clear_page(ptr);
> > >  		ptr += maddr & ~PAGE_MASK;
> > > @@ -827,7 +827,7 @@ static int kimage_load_normal_segment(struct kimage 
> *image,
> > >  			memcpy(ptr, kbuf, uchunk);
> > >  		else
> > >  			result = copy_from_user(ptr, buf, uchunk);
> > > -		kunmap(page);
> > > +		kunmap_local(ptr);
> > >  		if (result) {
> > >  			result = -EFAULT;
> > >  			goto out;
> > > @@ -878,7 +878,7 @@ static int kimage_load_crash_segment(struct kimage 
> *image,
> > >  			goto out;
> > >  		}
> > >  		arch_kexec_post_alloc_pages(page_address(page), 1, 0);
> > > -		ptr = kmap(page);
> > > +		ptr = kmap_local_page(page);
> > >  		ptr += maddr & ~PAGE_MASK;
> > >  		mchunk = min_t(size_t, mbytes,
> > >  				PAGE_SIZE - (maddr & ~PAGE_MASK));
> > > @@ -894,7 +894,7 @@ static int kimage_load_crash_segment(struct kimage 
> *image,
> > >  		else
> > >  			result = copy_from_user(ptr, buf, uchunk);
> > >  		kexec_flush_icache_page(page);
> > > -		kunmap(page);
> > > +		kunmap_local(ptr);
> > >  		arch_kexec_pre_free_pages(page_address(page), 1);
> > >  		if (result) {
> > >  			result = -EFAULT;
> > > -- 
> > > 2.36.1
> > > 
> > > 
> > > _______________________________________________
> > > kexec mailing list
> > > kexec@lists.infradead.org
> > > http://lists.infradead.org/mailman/listinfo/kexec
> > > 
> > 
> > 
> 
> 
> 
> 


_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
  2022-08-02  7:32       ` Baoquan He
@ 2022-08-02 11:02         ` Fabio M. De Francesco
  -1 siblings, 0 replies; 14+ messages in thread
From: Fabio M. De Francesco @ 2022-08-02 11:02 UTC (permalink / raw)
  To: Baoquan He; +Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On martedì 2 agosto 2022 09:32:46 CEST Baoquan He wrote:
> On 08/02/22 at 08:20am, Fabio M. De Francesco wrote:
> > On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> > > On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > > > The use of kmap() and kmap_atomic() are being deprecated in favor 
of
> > > > kmap_local_page().
> > > > 
> > > > With kmap_local_page(), the mappings are per thread, CPU local and 
not
> > > > globally visible. Furthermore, the mappings can be acquired from 
any
> > > > context (including interrupts).
> > > > 
> > > > Therefore, use kmap_local_page() in kexec_core.c because these 
mappings 
> > are
> > > > per thread, CPU local, and not globally visible.
> > > > 
> > > > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > > > enabled.
> > > 
> > > Wondering what arch you tested with.
> > 
> > I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
> > This is usually an information that I add in the commit messages of all 
the 
> > recent conversions I'm working on across the entire kernel.
> > 
> > Another important information (overlooked again this time) is that (1) 
> > kmap() comes with an overhead as mapping space is restricted and 
protected 
> > by a global lock for synchronization and (2) it also requires global 
TLB 
> > invalidation when the kmap’s pool wraps and it might block when the 
mapping 
> > space is fully utilized until a slot becomes available.
> 
> Thanks a lot for update with more details, Fabio.

You're welcome.

> Maybe these can be
> added into log with v3 if you think they are worth adding, and with my
> Ack since no code change related. You decide.

For weeks I've been thinking that these details were not necessary in the 
commit message of these conversions. Later I realized that a fair number of 
maintainers weren't aware of why we should change something that had been 
working "properly" for years. Perhaps they thought that we would face too 
many risks of breaking things versus little rewards.

After discussing with Greg K-H (who was initially not fond of these changes 
to core parts of firmware loading mechanism), he suggested that a proper 
commit log with the necessary information could have helped himself and the 
other maintainers who may not yet had time to deepen this topics.

This patch to "kexec" slipped in with no such additions.

Therefore, I agree with you in full: I'll add these information and send a 
new version ASAP.

> Thanks again for the effort.

Thanks so much for the "Acked-by:" tag from you.

Fabio  

[snip]



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

* Re: [PATCH v2] kexec: Replace kmap() with kmap_local_page()
@ 2022-08-02 11:02         ` Fabio M. De Francesco
  0 siblings, 0 replies; 14+ messages in thread
From: Fabio M. De Francesco @ 2022-08-02 11:02 UTC (permalink / raw)
  To: Baoquan He; +Cc: Eric Biederman, kexec, akpm, linux-kernel, Ira Weiny

On martedì 2 agosto 2022 09:32:46 CEST Baoquan He wrote:
> On 08/02/22 at 08:20am, Fabio M. De Francesco wrote:
> > On martedì 2 agosto 2022 05:06:54 CEST Baoquan He wrote:
> > > On 07/08/22 at 01:15am, Fabio M. De Francesco wrote:
> > > > The use of kmap() and kmap_atomic() are being deprecated in favor 
of
> > > > kmap_local_page().
> > > > 
> > > > With kmap_local_page(), the mappings are per thread, CPU local and 
not
> > > > globally visible. Furthermore, the mappings can be acquired from 
any
> > > > context (including interrupts).
> > > > 
> > > > Therefore, use kmap_local_page() in kexec_core.c because these 
mappings 
> > are
> > > > per thread, CPU local, and not globally visible.
> > > > 
> > > > Tested on a QEMU + KVM 32-bits VM booting a kernel with HIGHMEM64GB
> > > > enabled.
> > > 
> > > Wondering what arch you tested with.
> > 
> > I'm sorry, I forgot to say that I use x86_32 with 4GB to 6GB RAM.
> > This is usually an information that I add in the commit messages of all 
the 
> > recent conversions I'm working on across the entire kernel.
> > 
> > Another important information (overlooked again this time) is that (1) 
> > kmap() comes with an overhead as mapping space is restricted and 
protected 
> > by a global lock for synchronization and (2) it also requires global 
TLB 
> > invalidation when the kmap’s pool wraps and it might block when the 
mapping 
> > space is fully utilized until a slot becomes available.
> 
> Thanks a lot for update with more details, Fabio.

You're welcome.

> Maybe these can be
> added into log with v3 if you think they are worth adding, and with my
> Ack since no code change related. You decide.

For weeks I've been thinking that these details were not necessary in the 
commit message of these conversions. Later I realized that a fair number of 
maintainers weren't aware of why we should change something that had been 
working "properly" for years. Perhaps they thought that we would face too 
many risks of breaking things versus little rewards.

After discussing with Greg K-H (who was initially not fond of these changes 
to core parts of firmware loading mechanism), he suggested that a proper 
commit log with the necessary information could have helped himself and the 
other maintainers who may not yet had time to deepen this topics.

This patch to "kexec" slipped in with no such additions.

Therefore, I agree with you in full: I'll add these information and send a 
new version ASAP.

> Thanks again for the effort.

Thanks so much for the "Acked-by:" tag from you.

Fabio  

[snip]



_______________________________________________
kexec mailing list
kexec@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/kexec

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

end of thread, other threads:[~2022-08-02 11:02 UTC | newest]

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-07-07 23:15 [PATCH v2] kexec: Replace kmap() with kmap_local_page() Fabio M. De Francesco
2022-07-07 23:15 ` Fabio M. De Francesco
2022-07-21 22:50 ` Ira Weiny
2022-07-21 22:50   ` Ira Weiny
2022-07-31  1:58 ` Ira Weiny
2022-07-31  1:58   ` Ira Weiny
2022-08-02  3:06 ` Baoquan He
2022-08-02  3:06   ` Baoquan He
2022-08-02  6:20   ` Fabio M. De Francesco
2022-08-02  6:20     ` Fabio M. De Francesco
2022-08-02  7:32     ` Baoquan He
2022-08-02  7:32       ` Baoquan He
2022-08-02 11:02       ` Fabio M. De Francesco
2022-08-02 11:02         ` Fabio M. De Francesco

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.