From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-7.0 required=3.0 tests=INCLUDES_PATCH, MAILING_LIST_MULTI,SIGNED_OFF_BY,SPF_HELO_NONE,SPF_PASS autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B1119C2D0CE for ; Fri, 24 Jan 2020 07:21:32 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 7B5CB2071E for ; Fri, 24 Jan 2020 07:21:32 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 7B5CB2071E Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=kernel.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 180996B0006; Fri, 24 Jan 2020 02:21:32 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 132076B000E; Fri, 24 Jan 2020 02:21:32 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 046DC6B0266; Fri, 24 Jan 2020 02:21:32 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0254.hostedemail.com [216.40.44.254]) by kanga.kvack.org (Postfix) with ESMTP id E27246B0006 for ; Fri, 24 Jan 2020 02:21:31 -0500 (EST) Received: from smtpin18.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with SMTP id 8D257181AEF07 for ; Fri, 24 Jan 2020 07:21:31 +0000 (UTC) X-FDA: 76411682382.18.hose03_477acc4d03c16 X-HE-Tag: hose03_477acc4d03c16 X-Filterd-Recvd-Size: 4682 Received: from mail-wm1-f68.google.com (mail-wm1-f68.google.com [209.85.128.68]) by imf07.hostedemail.com (Postfix) with ESMTP for ; Fri, 24 Jan 2020 07:21:31 +0000 (UTC) Received: by mail-wm1-f68.google.com with SMTP id p17so675953wma.1 for ; Thu, 23 Jan 2020 23:21:30 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=p8GGZGknnfG7tWMIgP6ZXfED0+y7Mb/Mp5pjgGPqFBI=; b=pEZzLYVkHjX77+irIJHtsiYWHuHD5yunJS91RZZEmJqvxC1KJx9NsBDg3Ns9KucI1b lZLwDXuVMxJCdJRrJ7C+fHI4/MzqRmwe5OOKOMHQF9bDUWpr/krHToqLBjo9YIcvLScW j0cg6mo9SZDn7lubN59NDwk8X9B2KTsQoefURu97v+uxLOUP88v74HKp4xOWV9IyOYMI NNnTyQ1BRoMSsn1UCuPZ6TznnihWH5OjwXC1skd8+Rit5OUDyB0HHPA01Zsh6WHMQ4+t /EHxZvmbguMgXj1DsxByrM2Sz2J0XfzJWtROCTHrzdpQsh1/soYcLu9VVIESUTLT7npo Qq/Q== X-Gm-Message-State: APjAAAXDsIWMLfdi7QGA8RScccsw/Por3OHzh+1p/nnnkV2hd5w8cYNt WiZIEzXA9YiPoNSNewq6BtM= X-Google-Smtp-Source: APXvYqxZA/sxsm8auIKrVRbVBr6COTkmGT+oQd+RxOnmNrKTN486dIZDxPoq6au0E/2YrqPhKbLTOg== X-Received: by 2002:a05:600c:2187:: with SMTP id e7mr737753wme.11.1579850489739; Thu, 23 Jan 2020 23:21:29 -0800 (PST) Received: from localhost (ip-37-188-162-110.eurotel.cz. [37.188.162.110]) by smtp.gmail.com with ESMTPSA id v14sm6322363wrm.28.2020.01.23.23.21.28 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 23 Jan 2020 23:21:29 -0800 (PST) Date: Fri, 24 Jan 2020 08:21:27 +0100 From: Michal Hocko To: Wei Yang Cc: akpm@linux-foundation.org, yang.shi@linux.alibaba.com, jhubbard@nvidia.com, vbabka@suse.cz, cl@linux.com, linux-mm@kvack.org, linux-kernel@vger.kernel.org Subject: Re: [Patch v2] mm/migrate.c: also overwrite error when it is bigger than zero Message-ID: <20200124072127.GO29276@dhcp22.suse.cz> References: <20200119065753.21694-1-richardw.yang@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200119065753.21694-1-richardw.yang@linux.intel.com> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: [Sorry I have missed this patch previously] On Sun 19-01-20 14:57:53, Wei Yang wrote: > If we get here after successfully adding page to list, err would be > 1 to indicate the page is queued in the list. > > Current code has two problems: > > * on success, 0 is not returned > * on error, if add_page_for_migratioin() return 1, and the following err1 > from do_move_pages_to_node() is set, the err1 is not returned since err > is 1 This made my really scratch my head to grasp. So essentially err > 0 will happen when we reach the end of the loop and rely on the out_flush flushing to migrate the batch. Then err contains the add_page_for_migratioin return value. And that would leak to the userspace. What would you say about the following wording instead? " out_flush part of do_pages_move is responsible for migrating the last batch that accumulated while processing the input in the loop. do_move_pages_to_node return value is supposed to override any preexisting error (e.g. when the user input is garbage) but the current logic is wrong because add_page_for_migration returns 1 when successfully adding a page into the batch and therefore this will be the last err value after the loop is processed without any actual error. We want to override that value of course because do_pages_move would return 1 to the userspace even without any errors. " > And these behaviors break the user interface. > > Fixes: e0153fc2c760 ("mm: move_pages: return valid node id in status if the > page is already on the target node"). > Signed-off-by: Wei Yang Acked-by: Michal Hocko > > --- > v2: > * put more words to explain the error case > --- > mm/migrate.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/mm/migrate.c b/mm/migrate.c > index 86873b6f38a7..430fdccc733e 100644 > --- a/mm/migrate.c > +++ b/mm/migrate.c > @@ -1676,7 +1676,7 @@ static int do_pages_move(struct mm_struct *mm, nodemask_t task_nodes, > err1 = do_move_pages_to_node(mm, &pagelist, current_node); > if (!err1) > err1 = store_status(status, start, current_node, i - start); > - if (!err) > + if (err >= 0) > err = err1; > out: > return err; > -- > 2.17.1 > -- Michal Hocko SUSE Labs