| Message ID | 20220607105958.382076-2-bhe@redhat.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | Cleanup patches of vmalloc | expand |
> > In function adjust_va_to_fit_type(), it checks all values of passed > in fit type, including NOTHING_FIT in the else branch. However, the > check of NOTHING_FIT has been done inside adjust_va_to_fit_type() and > before it's called in all call sites. > > In fact, both of these two functions are coupled tightly, since > classify_va_fit_type() is doing the preparation work for > adjust_va_to_fit_type(). So putting invocation of classify_va_fit_type() > inside adjust_va_to_fit_type() can simplify code logic and the redundant > check of NOTHING_FIT issue will go away. > > Suggested-by: Uladzislau Rezki (Sony) <urezki@gmail.com> > Signed-off-by: Baoquan He <bhe@redhat.com> > --- > mm/vmalloc.c | 23 ++++++----------------- > 1 file changed, 6 insertions(+), 17 deletions(-) > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c > index 07db42455dd4..f9d45aa90b7c 100644 > --- a/mm/vmalloc.c > +++ b/mm/vmalloc.c > @@ -1335,10 +1335,10 @@ classify_va_fit_type(struct vmap_area *va, > > static __always_inline int > adjust_va_to_fit_type(struct vmap_area *va, > - unsigned long nva_start_addr, unsigned long size, > - enum fit_type type) > + unsigned long nva_start_addr, unsigned long size) > { > struct vmap_area *lva = NULL; > + enum fit_type type = classify_va_fit_type(va, nva_start_addr, size); > > if (type == FL_FIT_TYPE) { > /* > @@ -1444,7 +1444,6 @@ __alloc_vmap_area(unsigned long size, unsigned long align, > bool adjust_search_size = true; > unsigned long nva_start_addr; > struct vmap_area *va; > - enum fit_type type; > int ret; > > /* > @@ -1472,14 +1471,9 @@ __alloc_vmap_area(unsigned long size, unsigned long align, > if (nva_start_addr + size > vend) > return vend; > > - /* Classify what we have found. */ > - type = classify_va_fit_type(va, nva_start_addr, size); > - if (WARN_ON_ONCE(type == NOTHING_FIT)) > - return vend; > - > /* Update the free vmap_area. */ > - ret = adjust_va_to_fit_type(va, nva_start_addr, size, type); > - if (ret) > + ret = adjust_va_to_fit_type(va, nva_start_addr, size); > + if (WARN_ON_ONCE(ret)) > return vend; > > #if DEBUG_AUGMENT_LOWEST_MATCH_CHECK > @@ -3735,7 +3729,6 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned long *offsets, > int area, area2, last_area, term_area; > unsigned long base, start, size, end, last_end, orig_start, orig_end; > bool purged = false; > - enum fit_type type; > > /* verify parameters and allocate data structures */ > BUG_ON(offset_in_page(align) || !is_power_of_2(align)); > @@ -3846,15 +3839,11 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned long *offsets, > /* It is a BUG(), but trigger recovery instead. */ > goto recovery; > > - type = classify_va_fit_type(va, start, size); > - if (WARN_ON_ONCE(type == NOTHING_FIT)) > + ret = adjust_va_to_fit_type(va, start, size); > + if (WARN_ON_ONCE(unlikely(ret))) > /* It is a BUG(), but trigger recovery instead. */ > goto recovery; > > - ret = adjust_va_to_fit_type(va, start, size, type); > - if (unlikely(ret)) > - goto recovery; > - > /* Allocated area. */ > va = vas[area]; > va->va_start = start; > -- > 2.34.1 > Reviewed-by: Uladzislau Rezki (Sony) <urezki@gmail.com>
diff --git a/mm/vmalloc.c b/mm/vmalloc.c index 07db42455dd4..f9d45aa90b7c 100644 --- a/mm/vmalloc.c +++ b/mm/vmalloc.c @@ -1335,10 +1335,10 @@ classify_va_fit_type(struct vmap_area *va, static __always_inline int adjust_va_to_fit_type(struct vmap_area *va, - unsigned long nva_start_addr, unsigned long size, - enum fit_type type) + unsigned long nva_start_addr, unsigned long size) { struct vmap_area *lva = NULL; + enum fit_type type = classify_va_fit_type(va, nva_start_addr, size); if (type == FL_FIT_TYPE) { /* @@ -1444,7 +1444,6 @@ __alloc_vmap_area(unsigned long size, unsigned long align, bool adjust_search_size = true; unsigned long nva_start_addr; struct vmap_area *va; - enum fit_type type; int ret; /* @@ -1472,14 +1471,9 @@ __alloc_vmap_area(unsigned long size, unsigned long align, if (nva_start_addr + size > vend) return vend; - /* Classify what we have found. */ - type = classify_va_fit_type(va, nva_start_addr, size); - if (WARN_ON_ONCE(type == NOTHING_FIT)) - return vend; - /* Update the free vmap_area. */ - ret = adjust_va_to_fit_type(va, nva_start_addr, size, type); - if (ret) + ret = adjust_va_to_fit_type(va, nva_start_addr, size); + if (WARN_ON_ONCE(ret)) return vend; #if DEBUG_AUGMENT_LOWEST_MATCH_CHECK @@ -3735,7 +3729,6 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned long *offsets, int area, area2, last_area, term_area; unsigned long base, start, size, end, last_end, orig_start, orig_end; bool purged = false; - enum fit_type type; /* verify parameters and allocate data structures */ BUG_ON(offset_in_page(align) || !is_power_of_2(align)); @@ -3846,15 +3839,11 @@ struct vm_struct **pcpu_get_vm_areas(const unsigned long *offsets, /* It is a BUG(), but trigger recovery instead. */ goto recovery; - type = classify_va_fit_type(va, start, size); - if (WARN_ON_ONCE(type == NOTHING_FIT)) + ret = adjust_va_to_fit_type(va, start, size); + if (WARN_ON_ONCE(unlikely(ret))) /* It is a BUG(), but trigger recovery instead. */ goto recovery; - ret = adjust_va_to_fit_type(va, start, size, type); - if (unlikely(ret)) - goto recovery; - /* Allocated area. */ va = vas[area]; va->va_start = start;
In function adjust_va_to_fit_type(), it checks all values of passed in fit type, including NOTHING_FIT in the else branch. However, the check of NOTHING_FIT has been done inside adjust_va_to_fit_type() and before it's called in all call sites. In fact, both of these two functions are coupled tightly, since classify_va_fit_type() is doing the preparation work for adjust_va_to_fit_type(). So putting invocation of classify_va_fit_type() inside adjust_va_to_fit_type() can simplify code logic and the redundant check of NOTHING_FIT issue will go away. Suggested-by: Uladzislau Rezki (Sony) <urezki@gmail.com> Signed-off-by: Baoquan He <bhe@redhat.com> --- mm/vmalloc.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-)