Re: [v6 07/15] mm: defining memblock_virt_alloc_try_nid_raw

From: Michal Hocko
Date: Fri Aug 11 2017 - 12:06:57 EST


On Fri 11-08-17 11:58:46, Pasha Tatashin wrote:
> On 08/11/2017 08:39 AM, Michal Hocko wrote:
> >On Mon 07-08-17 16:38:41, Pavel Tatashin wrote:
> >>A new variant of memblock_virt_alloc_* allocations:
> >>memblock_virt_alloc_try_nid_raw()
> >> - Does not zero the allocated memory
> >> - Does not panic if request cannot be satisfied
> >
> >OK, this looks good but I would not introduce memblock_virt_alloc_raw
> >here because we do not have any users. Please move that to "mm: optimize
> >early system hash allocations" which actually uses the API. It would be
> >easier to review it that way.
> >
> >>Signed-off-by: Pavel Tatashin <pasha.tatashin@xxxxxxxxxx>
> >>Reviewed-by: Steven Sistare <steven.sistare@xxxxxxxxxx>
> >>Reviewed-by: Daniel Jordan <daniel.m.jordan@xxxxxxxxxx>
> >>Reviewed-by: Bob Picco <bob.picco@xxxxxxxxxx>
> >
> >other than that
> >Acked-by: Michal Hocko <mhocko@xxxxxxxx>
>
> Sure, I could do this, but as I understood from earlier Dave Miller's
> comments, we should do one logical change at a time. Hence, introduce API in
> one patch use it in another. So, this is how I tried to organize this patch
> set. Is this assumption incorrect?

Well, it really depends. If the patch is really small then adding a new
API along with users is easier to review and backport because you have a
clear view of the usage. I believe this is the case here. But if others
feel otherwise I will not object.

--
Michal Hocko
SUSE Labs