lkml.org 
[lkml]   [2021]   [May]   [27]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH v1 4/4] mm/mempolicy: kill MPOL_F_LOCAL bit
    On Thu, May 27, 2021 at 10:20:08AM +0200, Michal Hocko wrote:
    > On Wed 26-05-21 13:01:42, Feng Tang wrote:
    > > Now the only remaining case of a real 'local' policy faked by
    > > 'prefer' policy plus MPOL_F_LOCAL bit is:
    > >
    > > A valid 'prefer' policy with a valid 'preferred' node is 'rebind'
    > > to a nodemask which doesn't contains the 'preferred' node, then it
    > > will handle allocation with 'local' policy.
    > >
    > > Add a new 'MPOL_F_LOCAL_TEMP' bit for this case, and kill the
    > > MPOL_F_LOCAL bit, which could simplify the code much.
    >
    > As I've pointed out in the reply to the previous patch. It would have
    > been much better if most of the MPOL_F_LOCAL usage was gone by this
    > patch.
    >
    > I also dislike a new MPOL_F_LOCAL_TEMP. This smells like sneaking the
    > hack back in after you have painstakingly removed it. So this looks like
    > a step backwards to me. I also do not understand why do we need the
    > rebind callback for local policy at all. There is no node mask for local
    > so what is going on here?

    This is the special case 4 for 'perfer' policy with MPOL_F_STATIC_NODES
    flag set, say it prefer node 1, when it is later 'refind' to a new
    nodemask node 2-3, according to current code it will be add the
    MPOL_F_LOCAL bit and performs 'local' policy acctually. And in future
    it is 'rebind' again with a nodemask 1-2, it will be restored back
    to 'prefer' policy with preferred node 1.

    This patch tries to address this special case. I have struggled but
    couldn't think of a good way. Any suggestions? thanks!

    - Feng

    > [...]
    > > +static void mpol_rebind_local(struct mempolicy *pol,
    > > + const nodemask_t *nodes)
    > > +{
    > > + if (unlikely(pol->flags & MPOL_F_STATIC_NODES)) {
    > > + int node = first_node(pol->w.user_nodemask);
    > > +
    > > + BUG_ON(!(pol->flags & MPOL_F_LOCAL_TEMP));
    > > +
    > > + if (node_isset(node, *nodes)) {
    > > + pol->v.preferred_node = node;
    > > + pol->mode = MPOL_PREFERRED;
    > > + pol->flags &= ~MPOL_F_LOCAL_TEMP;
    > > + }
    > > + }
    > > +}
    > > +
    >
    > I have to confess I've got lost here. Could you explain why do you need
    > all this handling for a local policy?
    >
    > > static void mpol_rebind_preferred(struct mempolicy *pol,
    > > const nodemask_t *nodes)
    > > {
    > > @@ -347,13 +363,19 @@ static void mpol_rebind_preferred(struct mempolicy *pol,
    > >
    > > if (node_isset(node, *nodes)) {
    > > pol->v.preferred_node = node;
    > > - pol->flags &= ~MPOL_F_LOCAL;
    > > - } else
    > > - pol->flags |= MPOL_F_LOCAL;
    > > + } else {
    > > + /*
    > > + * If there is no valid node, change the mode to
    > > + * MPOL_LOCAL, which will be restored back when
    > > + * next rebind() sees a valid node.
    > > + */
    > > + pol->mode = MPOL_LOCAL;
    > > + pol->flags |= MPOL_F_LOCAL_TEMP;
    > > + }
    > > } else if (pol->flags & MPOL_F_RELATIVE_NODES) {
    > > mpol_relative_nodemask(&tmp, &pol->w.user_nodemask, nodes);
    > > pol->v.preferred_node = first_node(tmp);
    > > - } else if (!(pol->flags & MPOL_F_LOCAL)) {
    > > + } else {
    > > pol->v.preferred_node = node_remap(pol->v.preferred_node,
    > > pol->w.cpuset_mems_allowed,
    > > *nodes);
    > > @@ -372,7 +394,7 @@ static void mpol_rebind_policy(struct mempolicy *pol, const nodemask_t *newmask)
    > > {
    > > if (!pol)
    > > return;
    > > - if (!mpol_store_user_nodemask(pol) && !(pol->flags & MPOL_F_LOCAL) &&
    > > + if (!mpol_store_user_nodemask(pol) &&
    > > nodes_equal(pol->w.cpuset_mems_allowed, *newmask))
    > > return;
    > >
    > > @@ -425,7 +447,7 @@ static const struct mempolicy_operations mpol_ops[MPOL_MAX] = {
    > > },
    > > [MPOL_LOCAL] = {
    > > .create = mpol_new_local,
    > > - .rebind = mpol_rebind_default,
    > > + .rebind = mpol_rebind_local,
    > > },
    > > };
    > --
    > Michal Hocko
    > SUSE Labs

    \
     
     \ /
      Last update: 2021-05-27 14:11    [W:3.046 / U:0.040 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site