On Sat, Mar 30, 2024 at 09:21:25PM +0800, Baoquan He wrote:
> On 03/30/24 at 01:55pm, Uladzislau Rezki wrote:
> > On Fri, Mar 29, 2024 at 03:44:40PM +0800, Baoquan He wrote:
> > > On 03/28/24 at 03:03pm, Uladzislau Rezki (Sony) wrote:
> ......snip
> > > How about below change about va_start_lowest? Personal preference, not
> > > strong opinion.
> > >
> > > diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> > > index 9b1a41e12d70..bd6a66c54ad2 100644
> > > --- a/mm/vmalloc.c
> > > +++ b/mm/vmalloc.c
> > > @@ -1046,19 +1046,19 @@ __find_vmap_area_exceed_addr(unsigned long addr, struct rb_root *root)
> > > static struct vmap_node *
> > > find_vmap_area_exceed_addr_lock(unsigned long addr, struct vmap_area **va)
> > > {
> > > - unsigned long va_start_lowest;
> > > + unsigned long va_start_lowest = ULONG_MAX;
> > > struct vmap_node *vn;
> > > int i;
> > >
> > > repeat:
> > > - for (i = 0, va_start_lowest = 0; i < nr_vmap_nodes; i++) {
> > > + for (i = 0; i < nr_vmap_nodes; i++) {
> > > vn = &vmap_nodes[i];
> > >
> > > spin_lock(&vn->busy.lock);
> > > *va = __find_vmap_area_exceed_addr(addr, &vn->busy.root);
> > >
> > > if (*va)
> > > - if (!va_start_lowest || (*va)->va_start < va_start_lowest)
> > > + if ((*va)->va_start < va_start_lowest)
> > > va_start_lowest = (*va)->va_start;
> > > spin_unlock(&vn->busy.lock);
> > > }
> > > @@ -1069,7 +1069,7 @@ find_vmap_area_exceed_addr_lock(unsigned long addr, struct vmap_area **va)
> > > * been removed concurrently thus we need to proceed
> > > * with next one what is a rare case.
> > > */
> > > - if (va_start_lowest) {
> > > + if (va_start_lowest != ULONG_MAX) {
> > > vn = addr_to_node(va_start_lowest);
> > >
> > > spin_lock(&vn->busy.lock);
> > >
> > >
> > To me it looks as incomplete. The "va_start_lowest" should be initialized
> > when repeat. Otherwise we can end up with an infinite repeating because
> > va_start_lowest != ULONG_MAX.
>
> You are right. Anyway, it's just a suggestion from a different code
> style, please feel free to adjust it in or leave the patch as is.
> >
>
OK!
Thank you.
--
Uladzislau Rezki
