From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750941AbdBBHbJ (ORCPT ); Thu, 2 Feb 2017 02:31:09 -0500 Received: from LGEAMRELO13.lge.com ([156.147.23.53]:35494 "EHLO lgeamrelo13.lge.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750808AbdBBHbI (ORCPT ); Thu, 2 Feb 2017 02:31:08 -0500 X-Original-SENDERIP: 156.147.1.126 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 165.244.249.25 X-Original-MAILFROM: minchan@kernel.org X-Original-SENDERIP: 10.177.223.161 X-Original-MAILFROM: minchan@kernel.org Date: Thu, 2 Feb 2017 16:28:26 +0900 From: Minchan Kim To: Michal Hocko CC: , , , , , , , , , , , , , , Subject: Re: [PATCH v5 1/4] mm/migration: make isolate_movable_page() return int type Message-ID: <20170202072826.GC11694@bbox> References: <1485867981-16037-1-git-send-email-ysxie@foxmail.com> <1485867981-16037-2-git-send-email-ysxie@foxmail.com> <20170201064821.GA10342@bbox> <20170201075924.GB5977@dhcp22.suse.cz> <20170201094636.GC10342@bbox> <20170201100022.GI5977@dhcp22.suse.cz> MIME-Version: 1.0 In-Reply-To: <20170201100022.GI5977@dhcp22.suse.cz> User-Agent: Mutt/1.5.24 (2015-08-30) X-MIMETrack: Itemize by SMTP Server on LGEKRMHUB05/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/02/02 16:28:29, Serialize by Router on LGEKRMHUB05/LGE/LG Group(Release 8.5.3FP6|November 21, 2013) at 2017/02/02 16:28:29, Serialize complete at 2017/02/02 16:28:29 Content-Type: text/plain; charset="us-ascii" Content-Disposition: inline Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On Wed, Feb 01, 2017 at 11:00:23AM +0100, Michal Hocko wrote: > On Wed 01-02-17 18:46:36, Minchan Kim wrote: > > On Wed, Feb 01, 2017 at 08:59:24AM +0100, Michal Hocko wrote: > > > On Wed 01-02-17 15:48:21, Minchan Kim wrote: > > > > Hi Yisheng, > > > > > > > > On Tue, Jan 31, 2017 at 09:06:18PM +0800, ysxie@foxmail.com wrote: > > > > > From: Yisheng Xie > > > > > > > > > > This patch changes the return type of isolate_movable_page() > > > > > from bool to int. It will return 0 when isolate movable page > > > > > successfully, return -EINVAL when the page is not a non-lru movable > > > > > page, and for other cases it will return -EBUSY. > > > > > > > > > > There is no functional change within this patch but prepare > > > > > for later patch. > > > > > > > > > > Signed-off-by: Yisheng Xie > > > > > Suggested-by: Michal Hocko > > > > > > > > Sorry for missing this one you guys were discussing. > > > > I don't understand the patch's goal although I read later patches. > > > > > > The point is that the failed isolation has to propagate error up the > > > call chain to the userspace which has initiated the migration. > > > > > > > isolate_movable_pages returns success/fail so that's why I selected > > > > bool rather than int but it seems you guys want to propagate more > > > > detailed error to the user so added -EBUSY and -EINVAL. > > > > > > > > But the question is why isolate_lru_pages doesn't have -EINVAL? > > > > > > It doesn't have to same as isolate_movable_pages. We should just return > > > EBUSY when the page is no longer movable. > > > > Why isolate_lru_page is okay to return -EBUSY in case of race while > > isolate_movable_page should return -EINVAL? > > What's the logic in your mind? I totally cannot understand. > > Let me rephrase. Both should return EBUSY. It means it's binary return value(success: 0 fail : -EBUSY) so IMO, bool is better and caller should return -EBUSY if that functions returns *false*. No need to make deeper propagation level. Anyway, it's trivial so I'm not against it if you want to make isolate_movable_page returns int. Insetad, please remove -EINVAL in this patch and just return -EBUSY for isolate_movable_page to be consistent with isolate_lru_page. Then, we don't need to fix any driver side, either. Even, no need to update any document because you don't add any new error value. That's enough.