* [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement
@ 2023-02-01 11:51 Hyunmin Lee
2023-02-01 11:59 ` Christophe Leroy
` (3 more replies)
0 siblings, 4 replies; 5+ messages in thread
From: Hyunmin Lee @ 2023-02-01 11:51 UTC (permalink / raw)
To: Andrew Morton, Uladzislau Rezki, Christoph Hellwig
Cc: linux-mm, linux-kernel, Gwan-gyeong Mun, Jeungwoo Yoo,
Sangyun Kim, Mike Rapoport, Christophe Leroy
As per the coding standards, in the event of an abnormal condition that
should not occur under normal circumstances, the kernel should attempt
recovery and proceed with execution, rather than halting the machine.
Specifically, in the alloc_vmap_area() function, use a simple if()
instead of using BUG_ON() halting the machine.
Co-Developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Co-Developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
Co-Developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
Signed-off-by: Hyunmin Lee <hn.min.lee@gmail.com>
Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
---
v1->v2 : Add commit description
v2->v3 : Change WARN_ON() to if()
v3->v4 : Use only one if() for three conditions
---
mm/vmalloc.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 74afa2208558..3b7e8856be35 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1587,9 +1587,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
int purged = 0;
int ret;
- BUG_ON(!size);
- BUG_ON(offset_in_page(size));
- BUG_ON(!is_power_of_2(align));
+ if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align)))
+ return ERR_PTR(-EINVAL);
if (unlikely(!vmap_initialized))
return ERR_PTR(-EBUSY);
--
2.25.1
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement
2023-02-01 11:51 [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement Hyunmin Lee
@ 2023-02-01 11:59 ` Christophe Leroy
2023-02-01 12:13 ` Hyeonggon Yoo
` (2 subsequent siblings)
3 siblings, 0 replies; 5+ messages in thread
From: Christophe Leroy @ 2023-02-01 11:59 UTC (permalink / raw)
To: Hyunmin Lee, Andrew Morton, Uladzislau Rezki, Christoph Hellwig
Cc: linux-mm, linux-kernel, Gwan-gyeong Mun, Jeungwoo Yoo,
Sangyun Kim, Mike Rapoport
Le 01/02/2023 à 12:51, Hyunmin Lee a écrit :
> As per the coding standards, in the event of an abnormal condition that
> should not occur under normal circumstances, the kernel should attempt
> recovery and proceed with execution, rather than halting the machine.
>
> Specifically, in the alloc_vmap_area() function, use a simple if()
> instead of using BUG_ON() halting the machine.
>
> Co-Developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Co-Developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Co-Developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Signed-off-by: Hyunmin Lee <hn.min.lee@gmail.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Reviewed-by: Christophe Leroy <christophe.leroy@csgroup.eu>
> ---
> v1->v2 : Add commit description
> v2->v3 : Change WARN_ON() to if()
> v3->v4 : Use only one if() for three conditions
> ---
> mm/vmalloc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 74afa2208558..3b7e8856be35 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1587,9 +1587,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> int purged = 0;
> int ret;
>
> - BUG_ON(!size);
> - BUG_ON(offset_in_page(size));
> - BUG_ON(!is_power_of_2(align));
> + if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align)))
> + return ERR_PTR(-EINVAL);
>
> if (unlikely(!vmap_initialized))
> return ERR_PTR(-EBUSY);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement
2023-02-01 11:51 [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement Hyunmin Lee
2023-02-01 11:59 ` Christophe Leroy
@ 2023-02-01 12:13 ` Hyeonggon Yoo
2023-02-01 12:19 ` Mike Rapoport
2023-02-01 14:22 ` Uladzislau Rezki
3 siblings, 0 replies; 5+ messages in thread
From: Hyeonggon Yoo @ 2023-02-01 12:13 UTC (permalink / raw)
To: Hyunmin Lee
Cc: Andrew Morton, Uladzislau Rezki, Christoph Hellwig, linux-mm,
linux-kernel, Gwan-gyeong Mun, Jeungwoo Yoo, Sangyun Kim,
Mike Rapoport, Christophe Leroy
On Wed, Feb 01, 2023 at 08:51:42PM +0900, Hyunmin Lee wrote:
> As per the coding standards, in the event of an abnormal condition that
> should not occur under normal circumstances, the kernel should attempt
> recovery and proceed with execution, rather than halting the machine.
>
> Specifically, in the alloc_vmap_area() function, use a simple if()
> instead of using BUG_ON() halting the machine.
>
> Co-Developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Co-Developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Co-Developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Signed-off-by: Hyunmin Lee <hn.min.lee@gmail.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> v1->v2 : Add commit description
> v2->v3 : Change WARN_ON() to if()
> v3->v4 : Use only one if() for three conditions
> ---
> mm/vmalloc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 74afa2208558..3b7e8856be35 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1587,9 +1587,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> int purged = 0;
> int ret;
>
> - BUG_ON(!size);
> - BUG_ON(offset_in_page(size));
> - BUG_ON(!is_power_of_2(align));
> + if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align)))
> + return ERR_PTR(-EINVAL);
>
> if (unlikely(!vmap_initialized))
> return ERR_PTR(-EBUSY);
Reviewed-by: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Thanks!
> --
> 2.25.1
Regards,
Hyeonggon
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement
2023-02-01 11:51 [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement Hyunmin Lee
2023-02-01 11:59 ` Christophe Leroy
2023-02-01 12:13 ` Hyeonggon Yoo
@ 2023-02-01 12:19 ` Mike Rapoport
2023-02-01 14:22 ` Uladzislau Rezki
3 siblings, 0 replies; 5+ messages in thread
From: Mike Rapoport @ 2023-02-01 12:19 UTC (permalink / raw)
To: Hyunmin Lee
Cc: Andrew Morton, Uladzislau Rezki, Christoph Hellwig, linux-mm,
linux-kernel, Gwan-gyeong Mun, Jeungwoo Yoo, Sangyun Kim,
Christophe Leroy
On Wed, Feb 01, 2023 at 08:51:42PM +0900, Hyunmin Lee wrote:
> As per the coding standards, in the event of an abnormal condition that
> should not occur under normal circumstances, the kernel should attempt
> recovery and proceed with execution, rather than halting the machine.
>
> Specifically, in the alloc_vmap_area() function, use a simple if()
> instead of using BUG_ON() halting the machine.
>
> Co-Developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Co-Developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Co-Developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Signed-off-by: Hyunmin Lee <hn.min.lee@gmail.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
Reviewed-by: Mike Rapoport (IBM) <rppt@kernel.org>
> ---
> v1->v2 : Add commit description
> v2->v3 : Change WARN_ON() to if()
> v3->v4 : Use only one if() for three conditions
> ---
> mm/vmalloc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 74afa2208558..3b7e8856be35 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1587,9 +1587,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> int purged = 0;
> int ret;
>
> - BUG_ON(!size);
> - BUG_ON(offset_in_page(size));
> - BUG_ON(!is_power_of_2(align));
> + if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align)))
> + return ERR_PTR(-EINVAL);
>
> if (unlikely(!vmap_initialized))
> return ERR_PTR(-EBUSY);
> --
> 2.25.1
>
--
Sincerely yours,
Mike.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement
2023-02-01 11:51 [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement Hyunmin Lee
` (2 preceding siblings ...)
2023-02-01 12:19 ` Mike Rapoport
@ 2023-02-01 14:22 ` Uladzislau Rezki
3 siblings, 0 replies; 5+ messages in thread
From: Uladzislau Rezki @ 2023-02-01 14:22 UTC (permalink / raw)
To: Hyunmin Lee
Cc: Andrew Morton, Uladzislau Rezki, Christoph Hellwig, linux-mm,
linux-kernel, Gwan-gyeong Mun, Jeungwoo Yoo, Sangyun Kim,
Mike Rapoport, Christophe Leroy
On Wed, Feb 01, 2023 at 08:51:42PM +0900, Hyunmin Lee wrote:
> As per the coding standards, in the event of an abnormal condition that
> should not occur under normal circumstances, the kernel should attempt
> recovery and proceed with execution, rather than halting the machine.
>
> Specifically, in the alloc_vmap_area() function, use a simple if()
> instead of using BUG_ON() halting the machine.
>
> Co-Developed-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Co-Developed-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Co-Developed-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Signed-off-by: Hyunmin Lee <hn.min.lee@gmail.com>
> Signed-off-by: Gwan-gyeong Mun <gwan-gyeong.mun@intel.com>
> Signed-off-by: Jeungwoo Yoo <casionwoo@gmail.com>
> Signed-off-by: Sangyun Kim <sangyun.kim@snu.ac.kr>
> Cc: Hyeonggon Yoo <42.hyeyoo@gmail.com>
> ---
> v1->v2 : Add commit description
> v2->v3 : Change WARN_ON() to if()
> v3->v4 : Use only one if() for three conditions
> ---
> mm/vmalloc.c | 5 ++---
> 1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 74afa2208558..3b7e8856be35 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1587,9 +1587,8 @@ static struct vmap_area *alloc_vmap_area(unsigned long size,
> int purged = 0;
> int ret;
>
> - BUG_ON(!size);
> - BUG_ON(offset_in_page(size));
> - BUG_ON(!is_power_of_2(align));
> + if (unlikely(!size || offset_in_page(size) || !is_power_of_2(align)))
> + return ERR_PTR(-EINVAL);
>
> if (unlikely(!vmap_initialized))
> return ERR_PTR(-EBUSY);
> --
> 2.25.1
>
Makes sense to me.
Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
--
Uladzislau Rezki
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-02-01 14:22 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-02-01 11:51 [PATCH v4] mm/vmalloc: replace BUG_ON to a simple if statement Hyunmin Lee
2023-02-01 11:59 ` Christophe Leroy
2023-02-01 12:13 ` Hyeonggon Yoo
2023-02-01 12:19 ` Mike Rapoport
2023-02-01 14:22 ` Uladzislau Rezki
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox