Re: [PATCH v2] mm: cma: NUMA node interface

[Aslan Bakirov <aslanbekirov@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2839 bytes --]

On Thu, Apr 2, 2020 at 11:07 PM Matthew Wilcox <willy@infradead.org> wrote:

> On Thu, Apr 02, 2020 at 02:58:10PM -0700, Aslan Bakirov wrote:
> > I've noticed that there is no interfaces exposed by CMA which would let
> me
> > to declare contigous memory on particular NUMA node.
>
> Do you have a user for this functionality?
>
Yes, we are using this functionality in second part of this patchset, We
use this interface
in hugetlb_cma_reserve() to reserve CMA on specific node.


>
> > +++ b/include/linux/cma.h
> > @@ -24,10 +24,20 @@ extern phys_addr_t cma_get_base(const struct cma
> *cma);
> >  extern unsigned long cma_get_size(const struct cma *cma);
> >  extern const char *cma_get_name(const struct cma *cma);
> >
> > -extern int __init cma_declare_contiguous(phys_addr_t base,
> > +extern int __init cma_declare_contiguous_nid(phys_addr_t base,
> >                       phys_addr_t size, phys_addr_t limit,
> >                       phys_addr_t alignment, unsigned int order_per_bit,
> > -                     bool fixed, const char *name, struct cma
> **res_cma);
> > +                     bool fixed, const char *name, struct cma **res_cma,
> > +                     int nid);
> > +static inline int __init cma_declare_contiguous(phys_addr_t base,
> > +                     phys_addr_t size, phys_addr_t limit,
> > +                     phys_addr_t alignment, unsigned int order_per_bit,
> > +                     bool fixed, const char *name, struct cma **res_cma)
> > +                     {
> > +                             return cma_declare_contiguous_nid(base,
> size,
> > +                                             limit, alignment,
> order_per_bit,
> > +                                             fixed, name, res_cma,
> NUMA_NO_NODE);
> > +                     }
>
> ... what even is this indentation?
>
> Sorry, about indentation problems, fix is coming in the next version.

> +phys_addr_t memblock_alloc_range_nid(phys_addr_t size,
> > +                                       phys_addr_t align, phys_addr_t
> start,
> > +                                       phys_addr_t end, int nid, bool
> exact_nid);
>
> >80 columns.  checkpatch should warn you of nits like this.
>
> >               if (base < highmem_start && limit > highmem_start) {
> > -                     addr = memblock_phys_alloc_range(size, alignment,
> > -                                                      highmem_start,
> limit);
> > +                     addr = memblock_alloc_range_nid(size, alignment,
> > +                                                      highmem_start,
> limit, nid, false);
>
> The deep indentation makes it hard to add new parameters.  I'd do it as:
>                         addr = memblock_alloc_range_nid(size, alignment,
>                                         highmem_start, limit, nid, false);
>
>
>

[-- Attachment #2: Type: text/html, Size: 4042 bytes --]