Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1616866 > unrolled thread

Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context Vlastimil Babka <vbabka@suse.cz> - 2017-04-05 13:50 +0200
    Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context Michal Hocko <mhocko@kernel.org> - 2017-04-05 14:20 +0200

#1616866 — Re: [PATCH 1/4] mm/vmalloc: allow to call vfree() in atomic context

On 03/30/2017 04:48 PM, Andrey Ryabinin wrote:
> On 03/30/2017 03:00 PM, Thomas Hellstrom wrote:
> 
>>>  
>>>  	if (unlikely(nr_lazy > lazy_max_pages()))
>>> -		try_purge_vmap_area_lazy();
>>
>> Perhaps a slight optimization would be to schedule work iff
>> !mutex_locked(&vmap_purge_lock) below?
>>
> 
> Makes sense, we don't need to spawn workers if we already purging.
> 
> 
> 
> From: Andrey Ryabinin <aryabinin@virtuozzo.com>
> Subject: mm/vmalloc: allow to call vfree() in atomic context fix
> 
> Don't spawn worker if we already purging.
> 
> Signed-off-by: Andrey Ryabinin <aryabinin@virtuozzo.com>
> ---
>  mm/vmalloc.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/mm/vmalloc.c b/mm/vmalloc.c
> index ea1b4ab..88168b8 100644
> --- a/mm/vmalloc.c
> +++ b/mm/vmalloc.c
> @@ -737,7 +737,8 @@ static void free_vmap_area_noflush(struct vmap_area *va)
>  	/* After this point, we may free va at any time */
>  	llist_add(&va->purge_list, &vmap_purge_list);
>  
> -	if (unlikely(nr_lazy > lazy_max_pages()))
> +	if (unlikely(nr_lazy > lazy_max_pages()) &&
> +	    !mutex_is_locked(&vmap_purge_lock))

So, isn't this racy? (and do we care?)

Vlastimil

>  		schedule_work(&purge_vmap_work);
>  }
>  
> 

[toc] | [next] | [standalone]

#1616879

On Wed 05-04-17 13:42:19, Vlastimil Babka wrote:
> On 03/30/2017 04:48 PM, Andrey Ryabinin wrote:
[...]
> > --- a/mm/vmalloc.c
> > +++ b/mm/vmalloc.c
> > @@ -737,7 +737,8 @@ static void free_vmap_area_noflush(struct vmap_area *va)
> >  	/* After this point, we may free va at any time */
> >  	llist_add(&va->purge_list, &vmap_purge_list);
> >  
> > -	if (unlikely(nr_lazy > lazy_max_pages()))
> > +	if (unlikely(nr_lazy > lazy_max_pages()) &&
> > +	    !mutex_is_locked(&vmap_purge_lock))
> 
> So, isn't this racy? (and do we care?)

yes, it is racy and no we do not care AFAICS. If the lock is held then
somebody is already doing the work on our behalf. If we are unlucky
and that work has been already consumed (read another lazy_max_pages
have been freed) then we would still try to lazy free it during the
allocation. This would be something for the changelog of course.
-- 
Michal Hocko
SUSE Labs

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web