lkml.org 
[lkml]   [2021]   [Jul]   [28]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v6 1/6] mm/mempolicy: Add MPOL_PREFERRED_MANY for multiple preferred nodes
    On Wed, Jul 28, 2021 at 02:31:03PM +0200, Michal Hocko wrote:
    > [Sorry for a late review]

    Not at all. Thank you for all your reviews and suggestions from v1
    to v6!

    > On Mon 12-07-21 16:09:29, Feng Tang wrote:
    > [...]
    > > @@ -1887,7 +1909,8 @@ nodemask_t *policy_nodemask(gfp_t gfp, struct mempolicy *policy)
    > > /* Return the node id preferred by the given mempolicy, or the given id */
    > > static int policy_node(gfp_t gfp, struct mempolicy *policy, int nd)
    > > {
    > > - if (policy->mode == MPOL_PREFERRED) {
    > > + if (policy->mode == MPOL_PREFERRED ||
    > > + policy->mode == MPOL_PREFERRED_MANY) {
    > > nd = first_node(policy->nodes);
    > > } else {
    > > /*
    >
    > Do we really want to have the preferred node to be always the first node
    > in the node mask? Shouldn't that strive for a locality as well? Existing
    > callers already prefer numa_node_id() - aka local node - and I belive we
    > shouldn't just throw that away here.

    I think it's about the difference of 'local' and 'prefer/perfer-many'
    policy. There are different kinds of memory HW: HBM(High Bandwidth
    Memory), normal DRAM, PMEM (Persistent Memory), which have different
    price, bandwidth, speed etc. A platform may have two, or all three of
    these types, and there are real use case which want memory comes
    'preferred' node/nodes than the local node.

    And good point for 'local node', if the 'prefer-many' policy's
    nodemask has local node set, we should pick it han this
    'first_node', and the same semantic also applies to the other
    several places you pointed out. Or do I misunderstand you point?

    Thanks,
    Feng

    > > @@ -1931,6 +1954,7 @@ unsigned int mempolicy_slab_node(void)
    > >
    > > switch (policy->mode) {
    > > case MPOL_PREFERRED:
    > > + case MPOL_PREFERRED_MANY:
    > > return first_node(policy->nodes);
    >
    > Similarly here but I am not really familiar with the slab numa code
    > enough to have strong opinions here.
    >
    > > @@ -2173,10 +2198,12 @@ struct page *alloc_pages_vma(gfp_t gfp, int order, struct vm_area_struct *vma,
    > > * node and don't fall back to other nodes, as the cost of
    > > * remote accesses would likely offset THP benefits.
    > > *
    > > - * If the policy is interleave, or does not allow the current
    > > - * node in its nodemask, we allocate the standard way.
    > > + * If the policy is interleave or multiple preferred nodes, or
    > > + * does not allow the current node in its nodemask, we allocate
    > > + * the standard way.
    > > */
    > > - if (pol->mode == MPOL_PREFERRED)
    > > + if ((pol->mode == MPOL_PREFERRED ||
    > > + pol->mode == MPOL_PREFERRED_MANY))
    > > hpage_node = first_node(pol->nodes);
    >
    > Same here.
    >
    > > @@ -2451,6 +2479,9 @@ int mpol_misplaced(struct page *page, struct vm_area_struct *vma, unsigned long
    > > break;
    > >
    > > case MPOL_PREFERRED:
    > > + case MPOL_PREFERRED_MANY:
    > > + if (node_isset(curnid, pol->nodes))
    > > + goto out;
    > > polnid = first_node(pol->nodes);
    > > break;
    >
    > I do not follow what is the point of using first_node here. Either the
    > node is in the mask or it is misplaced. What are you trying to achieve
    > here?
    > --
    > Michal Hocko
    > SUSE Labs

    \
     
     \ /
      Last update: 2021-07-28 16:13    [W:4.254 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site