lkml.org 
[lkml]   [2020]   [Jan]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [PATCH 6/8] mm/migrate.c: handle same node and add failure in the same way
    On Sun 19-01-20 11:06:34, Wei Yang wrote:
    > When page is not queued for migration, there are two possible cases:
    >
    > * page already on the target node
    > * failed to add to migration queue
    >
    > Current code handle them differently, this leads to a behavior
    > inconsistency.
    >
    > Usually for each page's status, we just do store for once. While for the
    > page already on the target node, we might store the node information for
    > twice:
    >
    > * once when we found the page is on the target node
    > * second when moving the pages to target node successfully after above
    > action
    >
    > The reason is even we don't add the page to pagelist, but store_status()
    > does store in a range which still contains the page.
    >
    > This patch handles these two cases in the same way to reduce this
    > inconsistency and also make the code a little easier to read.

    Yeah, the improvement is really marginal. I do not feel strongly one way
    or another.

    > Signed-off-by: Wei Yang <richardw.yang@linux.intel.com>

    Acked-by: Michal Hocko <mhocko@suse.com>

    > ---
    > mm/migrate.c | 16 ++++++++--------
    > 1 file changed, 8 insertions(+), 8 deletions(-)
    >
    > diff --git a/mm/migrate.c b/mm/migrate.c
    > index 46a5697b7fc6..aee5aeb082c4 100644
    > --- a/mm/migrate.c
    > +++ b/mm/migrate.c
    > @@ -1657,18 +1657,18 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes,
    > err = add_page_for_migration(mm, addr, current_node,
    > &pagelist, flags & MPOL_MF_MOVE_ALL);
    >
    > - if (!err) {
    > - /* The page is already on the target node */
    > - err = store_status(status, i, current_node, 1);
    > - if (err)
    > - goto out_flush;
    > - continue;
    > - } else if (err > 0) {
    > + if (err > 0) {
    > /* The page is successfully queued for migration */
    > continue;
    > }
    >
    > - err = store_status(status, i, err, 1);
    > + /*
    > + * Two possible cases for err here:
    > + * == 0: page is already on the target node, then store
    > + * current_node to status
    > + * < 0: failed to add page to list, then store err to status
    > + */
    > + err = store_status(status, i, err ? : current_node, 1);
    > if (err)
    > goto out_flush;
    >
    > --
    > 2.17.1
    >

    --
    Michal Hocko
    SUSE Labs

    \
     
     \ /
      Last update: 2020-01-20 11:01    [W:4.649 / U:0.156 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site