Re: [linus:master] [mm] c6307674ed: BUG:sleeping_function_called_from_invalid_context_at_mm/vmalloc.c

[Uladzislau Rezki <urezki@gmail.com>]

On Mon, Apr 27, 2026 at 04:37:11PM +0200, Vlastimil Babka wrote:
> On 4/27/26 16:17, Uladzislau Rezki wrote:
> > On Mon, Apr 27, 2026 at 10:53:52AM +0200, Vlastimil Babka wrote:
> >> On 4/23/26 05:34, Herbert Xu wrote:
> >> > On Wed, Apr 22, 2026 at 10:32:16AM +0200, Uladzislau Rezki wrote:
> >> >>
> >> >> We have added non-sleeping flags for vmalloc() to extend kvmalloc()
> >> >> functionality as folk need those.
> >> >>  
> >> >> Another option, would be: always use vfree_atomic() from the kvfree()
> >> >> path.
> >> >>  
> >> >> Any thoughts?
> >> > 
> >> > Perhaps add a kvfree_atomic that just calls vfree_atomic?
> >> 
> >> kvfree()'s comment says
> >> 
> >> " * Context: Either preemptible task context or not-NMI interrupt."
> >> 
> > I am not sure the description clearly reflects the intended usage.
> > To me it sounds like all contexts but excluding NMI.
> 
> Agree it's not clear, my immediate reaction was the same and took me a bit
> to understand.
> 
> > For example, calling this under spin_lock() will be invalid, as
> > the vfree() path may invoke cond_resched().
> 
> Yeah, it's covered by the "preemptible" word, but not super clear.
> IIUC it all boils down to this in vfree():
> 
> 
>         if (unlikely(in_interrupt())) {
>                 vfree_atomic(addr);
>                 return;
>         }
> 
> Which can do the right thing in an interrupt, but obviously cannot
> automatically recognize the "under spin_lock()" situation.
> 
> So I don't think we can change it now with all the existing callers, but if
> a better wording exists, we could make the comment more clear.
> 
> >> so this is neither. It might be ok then to create kvfree_atomic(). Always
> >> using vfree_atomic() from kvfree() might be wasteful.
> >> 
> >> > For rhashtable it really makes no difference either way.  But it
> >> > would eliminate the unsightly call to is_vmalloc_addr in rhashtable.
> >> > 
> > Seems atomic version makes sense here:
> > 
> > <snip>
> > diff --git a/include/linux/slab.h b/include/linux/slab.h
> > index 15a60b501b95..2b5ab488e96b 100644
> > --- a/include/linux/slab.h
> > +++ b/include/linux/slab.h
> > @@ -1234,6 +1234,9 @@ void *kvrealloc_node_align_noprof(const void *p, size_t size, unsigned long alig
> >  extern void kvfree(const void *addr);
> >  DEFINE_FREE(kvfree, void *, if (!IS_ERR_OR_NULL(_T)) kvfree(_T))
> >  
> > +extern void kvfree_atomic(const void *addr);
> > +DEFINE_FREE(kvfree_atomic, void *, if (!IS_ERR_OR_NULL(_T)) kvfree_atomic(_T))
> > +
> >  extern void kvfree_sensitive(const void *addr, size_t len);
> >  
> >  unsigned int kmem_cache_size(struct kmem_cache *s);
> > diff --git a/mm/slub.c b/mm/slub.c
> > index 2b2d33cc735c..e25a0eab6ff7 100644
> > --- a/mm/slub.c
> > +++ b/mm/slub.c
> > @@ -6802,6 +6802,22 @@ void kvfree(const void *addr)
> >  }
> >  EXPORT_SYMBOL(kvfree);
> >  
> > +/**
> > + * kvfree_atomic() - Free memory.
> > + * @addr: Pointer to allocated memory.
> > + *
> > + * Same as kvfree(), but safe to use in atomic contexts.
> > + * Must not be called from NMI context.
> > + */
> > +void kvfree_atomic(const void *addr)
> > +{
> > +	if (is_vmalloc_addr(addr))
> > +		vfree_atomic(addr);
> > +	else
> > +		kfree(addr);
> > +}
> > +EXPORT_SYMBOL(kvfree_atomic);
> > +
> >  /**
> >   * kvfree_sensitive - Free a data object containing sensitive information.
> >   * @addr: address of the data object to be freed.
> > <snip>
> > 
> > I can post it if no objections.
> 
> LGTM but also should be now used in rhashtable_try_insert() I think?
> 

Like below:

rhashtable_try_insert()
  rhashtable_insert_rehash()
    bucket_table_free_atomic(new_tbl);

the patch for rhashtable.c can look like:

<snip>
diff --git a/lib/rhashtable.c b/lib/rhashtable.c
index 6074ed5f66f3..4111aab8cee4 100644
--- a/lib/rhashtable.c
+++ b/lib/rhashtable.c
@@ -114,6 +114,14 @@ static void bucket_table_free(const struct bucket_table *tbl)
 	kvfree(tbl);
 }
 
+static void bucket_table_free_atomic(const struct bucket_table *tbl)
+{
+	if (tbl->nest)
+		nested_bucket_table_free(tbl);
+
+	kvfree_atomic(tbl);
+}
+
 static void bucket_table_free_rcu(struct rcu_head *head)
 {
 	bucket_table_free(container_of(head, struct bucket_table, rcu));
@@ -473,7 +481,7 @@ static int rhashtable_insert_rehash(struct rhashtable *ht,
 
 	err = rhashtable_rehash_attach(ht, tbl, new_tbl);
 	if (err) {
-		bucket_table_free(new_tbl);
+		bucket_table_free_atomic(new_tbl);
 		if (err == -EEXIST)
 			err = 0;
 	} else
<snip>

--
Uladzislau Rezki