Re: [PATCH] hv: Fix unnecessary sleeping in hv_synic_alloc

[Jia-Ju Bai]

On 2017/12/18 16:54, Vitaly Kuznetsov wrote:
> Jia-Ju Bai <baijiaju1990@xxxxxxxxx> writes:
>
> > The kzalloc function is called with GFP_ATOMIC.
> > But according to driver call graph, it is not in atomic context,
> > namely no spinlock is held nor in an interrupt handler.
> >
> > This GFP_ATOMIC is unnecessary, and replace with GFP_KERNEL.
> >
> > Signed-off-by: Jia-Ju Bai <baijiaju1990@xxxxxxxxx>
> > ---
> >   drivers/hv/hv.c |    2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/hv/hv.c b/drivers/hv/hv.c
> > index 8267439..b0d025a 100644
> > --- a/drivers/hv/hv.c
> > +++ b/drivers/hv/hv.c
> > @@ -150,7 +150,7 @@ int hv_synic_alloc(void)
> >   	int cpu;
> >
> >   	hv_context.hv_numa_map = kzalloc(sizeof(struct cpumask) * nr_node_ids,
> > -					 GFP_ATOMIC);
> > +					 GFP_KERNEL);
> >   	if (hv_context.hv_numa_map == NULL) {
> >   		pr_err("Unable to allocate NUMA map\n");
> >   		goto err;
> Reviewed-by: Vitaly Kuznetsov <vkuznets@xxxxxxxxxx>
>
> The subject line is not very accurate: GFP_KERNEL you switch too is the
> one supporting 'sleep' when there's not enough memory, not
> GFP_ATOMIC so you don't actually "Fix unnecessary sleeping". I'd suggest
> you use something like "hyper-v: use GFP_KERNEL for hv_context.hv_numa_map"

Thanks for you suggestion :)
Okay, I found my description is not correct, too, sorry.
I will revise it and resubmit the patch.


Thanks,
Jia-Ju Bai