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

[Vlastimil Babka <vbabka@suse.com>]

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?

> 
> --
> Uladzislau Rezki