2022-06-06 09:10:14

by Baoquan He

Subject: [PATCH 1/5] mm/vmalloc: remove the unnecessary type check

In function adjust_va_to_fit_type(), it checks all values of passed
in fit type, including NOTHING_FIT in the else branch. In fact, it's
unnecessary to check NOTHING_FIT since the check has been done before
adjust_va_to_fit_type() is called in all call sites. So clean it up.

Signed-off-by: Baoquan He <[email protected]>
---
mm/vmalloc.c | 2 --
1 file changed, 2 deletions(-)

diff --git a/mm/vmalloc.c b/mm/vmalloc.c
index 07db42455dd4..b7a138ab7b79 100644
--- a/mm/vmalloc.c
+++ b/mm/vmalloc.c
@@ -1418,8 +1418,6 @@ adjust_va_to_fit_type(struct vmap_area *va,
* Shrink this VA to remaining size.
*/
va->va_start = nva_start_addr + size;
- } else {
- return -1;
}

if (type != FL_FIT_TYPE) {
--
2.34.1

2022-06-06 21:45:57

by Uladzislau Rezki

Subject: Re: [PATCH 1/5] mm/vmalloc: remove the unnecessary type check

> In function adjust_va_to_fit_type(), it checks all values of passed
> in fit type, including NOTHING_FIT in the else branch. In fact, it's
> unnecessary to check NOTHING_FIT since the check has been done before
> adjust_va_to_fit_type() is called in all call sites. So clean it up.
>
> Signed-off-by: Baoquan He <[email protected]>
> ---
> mm/vmalloc.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index 07db42455dd4..b7a138ab7b79 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -1418,8 +1418,6 @@ adjust_va_to_fit_type(struct vmap_area *va,
> * Shrink this VA to remaining size.
> */
> va->va_start = nva_start_addr + size;
> - } else {
> - return -1;
> }
>
> if (type != FL_FIT_TYPE) {
> --
> 2.34.1
>
Why not just invoke the classify_va_fit_type() inside the adjust_va_to_fit_type()?
In this case we do not need to rely on upper-stack checks and the split logic
becomes fully implemented in one solid function.

--
Uladzislau Rezki

2022-06-07 04:05:56

by Baoquan He

Subject: Re: [PATCH 1/5] mm/vmalloc: remove the unnecessary type check

On 06/06/22 at 10:36pm, Uladzislau Rezki wrote:
> > In function adjust_va_to_fit_type(), it checks all values of passed
> > in fit type, including NOTHING_FIT in the else branch. In fact, it's
> > unnecessary to check NOTHING_FIT since the check has been done before
> > adjust_va_to_fit_type() is called in all call sites. So clean it up.
> >
> > Signed-off-by: Baoquan He <[email protected]>
> > ---
> > mm/vmalloc.c | 2 --
> > 1 file changed, 2 deletions(-)
> >
> > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > index 07db42455dd4..b7a138ab7b79 100644
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -1418,8 +1418,6 @@ adjust_va_to_fit_type(struct vmap_area *va,
> > * Shrink this VA to remaining size.
> > */
> > va->va_start = nva_start_addr + size;
> > - } else {
> > - return -1;
> > }
> >
> > if (type != FL_FIT_TYPE) {
> > --
> > 2.34.1
> >
> Why not just invoke the classify_va_fit_type() inside the adjust_va_to_fit_type()?
> In this case we do not need to rely on upper-stack checks and the split logic
> becomes fully implemented in one solid function.

Ah, that's much better, let me post a new one. Thanks a lot.