Re: [PATCH v2 1/2] arm64/mm: Allow __create_pgd_mapping() to propagate pgtable_alloc() errors

[Linu Cherian <linu.cherian@arm.com>]

Hi Kevin,

On Tue, Oct 14, 2025 at 11:29:05AM +0200, Kevin Brodsky wrote:
> On 13/10/2025 10:02, Linu Cherian wrote:
> > [...]
> >
> > @@ -1877,23 +1926,30 @@ int arch_add_memory(int nid, u64 start, u64 size,
> >  	if (force_pte_mapping())
> >  		flags |= NO_BLOCK_MAPPINGS | NO_CONT_MAPPINGS;
> >  
> > -	__create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
> > -			     size, params->pgprot, pgd_pgtable_alloc_init_mm,
> > -			     flags);
> > +	ret = __create_pgd_mapping(swapper_pg_dir, start, __phys_to_virt(start),
> > +				   size, params->pgprot, pgd_pgtable_alloc_init_mm,
> > +				   flags);
> > +
> 
> Nit: usually no empty line between a call and a trivial if (ret).

Ack.

> 
> > +	if (ret)
> > +		goto out;
> >  
> >  	memblock_clear_nomap(start, size);
> >  
> >  	ret = __add_pages(nid, start >> PAGE_SHIFT, size >> PAGE_SHIFT,
> >  			   params);
> >  	if (ret)
> > -		__remove_pgd_mapping(swapper_pg_dir,
> > -				     __phys_to_virt(start), size);
> > +		goto out;
> >  	else {
> >  		/* Address of hotplugged memory can be smaller */
> >  		max_pfn = max(max_pfn, PFN_UP(start + size));
> >  		max_low_pfn = max_pfn;
> >  	}
> >  
> > +	return 0;
> > +
> > +out:
> 
> Nit: since this is an error path, it'd be clearer to call the label
> "err" rather than "out".

Ack.

Thanks,
Linu Cherian.