Date:   Tue, 14 Feb 2023 10:21:31 -0800
From:   Mike Kravetz <mike.kravetz@oracle.com>
To:     SeongJae Park <sj@kernel.org>
Cc:     Baolin Wang <baolin.wang@linux.alibaba.com>,
        akpm@linux-foundation.org, torvalds@linux-foundation.org,
        hannes@cmpxchg.org, mhocko@kernel.org, roman.gushchin@linux.dev,
        shakeelb@google.com, muchun.song@linux.dev,
        naoya.horiguchi@nec.com, linmiaohe@huawei.com, david@redhat.com,
        osalvador@suse.de, willy@infradead.org, damon@lists.linux.dev,
        cgroups@vger.kernel.org, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org
Subject: Re: [PATCH v2 3/4] mm: hugetlb: change to return bool for
 isolate_hugetlb()
Message-ID: <Y+vRK7kYP0xVZ9Ya@monkey>
References: <20230214180324.71585-1-sj@kernel.org>
 <20230214180708.71645-1-sj@kernel.org>
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230214180708.71645-1-sj@kernel.org>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?gsPiIEhOP06coPs04NV/rk+TW6v+/oP1W/t6itl3it2H3dBXKRaGBsS1G9oY?=
 =?us-ascii?Q?zht16VMlj9X+9Nx8wBoto/QYh1nGrFwfVXqWLkPeXnFPqSL7NVfXt3X7fsQw?=
 =?us-ascii?Q?WL1SbqzwPgYjzS1s5f17SJEO+Sh2TRlvR9fd30SDlNPT34KdkKRQTAVxYAkY?=
 =?us-ascii?Q?YbWfkpPtrrOjxRJf4G/vA60pJfcWfCbVopHUFIE0ybDEBkPZGSZ0pvG1oMYP?=
 =?us-ascii?Q?6uv87PJgf5N4as6M0HKX4Bt7XItMXqEr0CmEKKUQ4UYois+95vP0lLY5o4HK?=
 =?us-ascii?Q?uCuQyF8xeF8ypp8Wur+fBMmMWBXl1OlJ/KEbddVVo6on0/qiud4tk0Y/zNu8?=
 =?us-ascii?Q?TjTAzonl5dcsUmvKpjCmjfH5D6fGJrLSrT8/MgQZ8rlCU1Fc4a0OC6SIRkb/?=
 =?us-ascii?Q?a6z8arvMj27l5/R3C5PJyo8hYB9ReUVnUrNjiEIx7r2rZtf1j72gxtOakL6W?=
 =?us-ascii?Q?UkIp4NJKOyaSf91GcoX1/NeXI04o7ZZrA+M45RsAcuWRezzCjJyP7l26dLDx?=
 =?us-ascii?Q?BsDO7GckS0r4jmp45vcyeuoIl97NLJNovScfb6DiVMzLKZgn7nORF+bDGCOE?=
 =?us-ascii?Q?wf5rvAFC6l+KGgoosr/mQItp8u6D6SkvMjwMEpzxvPOVn8ieykPd1FBhBoFJ?=
 =?us-ascii?Q?0s/50reKmhnFdP8pUr+KDy/C0BH/QXkQIssYZTdycvUPOV9r1Y5h7o1JFKIE?=
 =?us-ascii?Q?X8/tyFEomeZ6Zn10PeeyGbytc3z63Y9pZF+atBZ6jOH1zJpfLTCiaubP2DH7?=
 =?us-ascii?Q?GoCjbfzj3ErQKXsOPJsxUZwcr6+ewQpRaGOkB9PJM+kzBXRo88EsfBt4+rvg?=
 =?us-ascii?Q?mll8Zg0a1EKBlVCbDgSWGxsp4TRYIMmOpbsUZEPcIKRMvZdNn5rn/BIxQr5O?=
 =?us-ascii?Q?FCA8hqHtkxStsRfrMoOQwb02Xo8bR7kKZAimhWlU06+A/X87zayp9dZwwhMY?=
 =?us-ascii?Q?OkjvGZ6emUIlsFFMVvIj4VsTAJ5ZhpRqMgCSuASH1zWqMIhkhh35kYMc8aOn?=
 =?us-ascii?Q?fIDWxg/vOMjQmShqIoBdbC8lcb4JdX0TUUKOYrF7FdN4rZGDW13YcJlck+Nm?=
 =?us-ascii?Q?qhONnAntplj4QT9Iy16H+TqI8l5+NE3a56cKXe35sRqJlV07wmzPw8GZMgr3?=
 =?us-ascii?Q?j55kEiUQ8r486jrjTZ4PzIpKT/Ku2jgVEu+Zvj/le9+oQ7cHmrLzRg3r3kY4?=
 =?us-ascii?Q?qJLDeFdRXg8Eo6Yoo3GQIHQsYioXqR2PWaLGpYjfRTS82WUGxBFoVGL80x5d?=
 =?us-ascii?Q?N2bUzEuVO04jIH/fbr4gThYBQM/wUBrzhKAa8KsVLxOTrLl8IIvg8lqlNfFA?=
 =?us-ascii?Q?MRVPjFWCR3c01/b1ayc0a1e4SJmtHBATmaZlWJWazj6CCfkO0PjH2G7AD1Bb?=
 =?us-ascii?Q?r6vbAOk5qADbGcb+kyUyTUsntSDCsEuV4tId0lpUJxrHkiAghmFC0gOUGzOw?=
 =?us-ascii?Q?Cii4sQiz5FPMd1A5gUZMmQG9pPWiG6rpPGQ7fSSuOOPfo4qc44nDRK155HT6?=
 =?us-ascii?Q?2p8HoePe8s3EXA2CYIOyC9cXS9XGoA+Bq8SyWdQsSGgqdMR/zRYEpQo1P1fV?=
 =?us-ascii?Q?sSeBbkg3SQ9SmBiMu/7AKNviMpaf/5e7cXTQSqBfw9QKzIj2mlEBXJjrj5sX?=
 =?us-ascii?Q?+Q=3D=3D?=
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-ExternalHop-MessageData-0: =?us-ascii?Q?RFe2z438o4/OAGqonV19CACSi3PwphhHpy2czs+ECkBLBJU7+XPPbtC8Ze2n?=
 =?us-ascii?Q?/Eh26QfukXcbq++ZzlQ5pduvAgvo8VBJBRm0EXPA0y6B1AbyvrIVJkPosui4?=
 =?us-ascii?Q?2+Q3axMXVFqujnGHd8HyiGGaHrWp52Fs2SYP9AxwM9R/SZCasblxq+CfNhFI?=
 =?us-ascii?Q?qIvAPrbqeT2ygDBipYYtVaoiON2gAbHV/VZivaoUn2Am3fUmTGMD5J3Bsbak?=
 =?us-ascii?Q?sFvoAB7ATvDe3e+vF/IPegvi2CKzvkm3mlH/f5mDFi+bsa0cptLldwLArjl5?=
 =?us-ascii?Q?5Kpk8W058R1ucZUh5T0P5/f4EsUcqsyFw3t2gXd9HtW1sONvr8vCN6Ay7ZZr?=
 =?us-ascii?Q?R73/r1N76qLJhFb5K5FZYGED+igTrQplHwZMstPdGDlYfkRVBHBGpwI9Hp4f?=
 =?us-ascii?Q?A6NS+oAnoj42zPUE6FaOsMv59j28Jl4t8iJTcuSAHLGYooib8p5RcUiV5ZWg?=
 =?us-ascii?Q?ARX4DwErwNuWV6wiZCCY8e/d/FLfuyEiE4/jGSG3X2W4aYMXa83lhbRBNvxy?=
 =?us-ascii?Q?cpA/doDc39DF85EOTx1h+/bBGUZW7Yix6sGn+5CCz0X35IdYVWMNC0QwQJfH?=
 =?us-ascii?Q?U1Yngd5pKCZ/dwAEwEj9fj3/ielyTu+xWij2HT/kvsl70JhnriaIvOkicqB+?=
 =?us-ascii?Q?GUErrRJvmjm4X9zrd7ye2d6mmZyegPuTTsXgCfy95JBNUAp7IKrqWA8s9ULI?=
 =?us-ascii?Q?b4RWKTEQ2j5S+fC6cNnFkVfsJs40eMY9fdSSfFVjg9FOHR0CnGeplHPmRLkq?=
 =?us-ascii?Q?ZF9yHyD8z7Lc4lZ4MF+3Ks6JhgA1QQqPCsYYvFSZ9VxRt8T9DCeJFY4Gms0G?=
 =?us-ascii?Q?oEA7B1zCpWQJBEltaRSGTry7/aeNqEk0qaMMZMdzKuaQyqLyHTVVEhyPTQxT?=
 =?us-ascii?Q?oGZv8F9Se4SrPxe+Y2uox4oytYw1Qd4eZ/WOr8WE/8+oLt+/wT/94vbo3TCU?=
 =?us-ascii?Q?7J77BG+6ruoSZlCt4UGG+U10cqWD0jehyburrKpebtIH/xEgWCrgaZNeeMBr?=
 =?us-ascii?Q?kazbWGH/VWtTu5VmgI67K5IkcBsORgO4bMKeTkJgJrmaqY5CgZdhgB5guUZ7?=
 =?us-ascii?Q?PL50EcY59JH08RJRE+Z8TmvxqW+HzpCSlRaganYEkpMWjX2QxEE6Bvzi5N6u?=
 =?us-ascii?Q?jG1K6pTv3M/tHICRIO49Pq2I7od9zR8qMlamsr7ay+oJ1RPJZGF7nqLdTJoh?=
 =?us-ascii?Q?7K6bFxMn/2wp3pQw?=
X-OriginatorOrg: oracle.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 063c2e74-6880-420e-0248-08db0eb84dd0
X-MS-Exchange-CrossTenant-AuthSource: BY5PR10MB4196.namprd10.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 14 Feb 2023 18:21:34.7337
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 4e2c6054-71cb-48f1-bd6c-3a9705aca71b
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: WFpu2gQjXuJuuhLRTThzaEpCNPVVsXAkt6+A+Pteuxy1VqGtz5nSCBe2RLh58ZpN4epYh/rqT2vV2F6RPWC65Q==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: IA0PR10MB7275
X-Proofpoint-Virus-Version: vendor=baseguard
 engine=ICAP:2.0.219,Aquarius:18.0.930,Hydra:6.0.562,FMLib:17.11.170.22
 definitions=2023-02-14_13,2023-02-14_01,2023-02-09_01
X-Proofpoint-Spam-Details: rule=notspam policy=default score=0 phishscore=0 spamscore=0 malwarescore=0
 mlxlogscore=999 bulkscore=0 mlxscore=0 adultscore=0 suspectscore=0
 classifier=spam adjust=0 reason=mlx scancount=1 engine=8.12.0-2212070000
 definitions=main-2302140158
X-Proofpoint-GUID: YpJPd9PD1Vu55G4RzSrP83UKJkVI4Fmh
X-Proofpoint-ORIG-GUID: YpJPd9PD1Vu55G4RzSrP83UKJkVI4Fmh
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On 02/14/23 18:07, SeongJae Park wrote:
> On Tue, 14 Feb 2023 18:03:24 +0000 SeongJae Park <sj@kernel.org> wrote:
> 
> > On Tue, 14 Feb 2023 21:59:31 +0800 Baolin Wang <baolin.wang@linux.alibaba.com> wrote:
> > 
> > > Now the isolate_hugetlb() only returns 0 or -EBUSY, and most users did not
> > > care about the negative value, thus we can convert the isolate_hugetlb()
> > > to return a boolean value to make code more clear when checking the
> > > hugetlb isolation state. Moreover converts 2 users which will consider
> > > the negative value returned by isolate_hugetlb().
> > > 
> > > No functional changes intended.
> > > 
> > > Signed-off-by: Baolin Wang <baolin.wang@linux.alibaba.com>
> > > ---
> > >  include/linux/hugetlb.h |  6 +++---
> > >  mm/hugetlb.c            | 12 ++++++++----
> > >  mm/memory-failure.c     |  2 +-
> > >  mm/mempolicy.c          |  2 +-
> > >  mm/migrate.c            |  2 +-
> > >  5 files changed, 14 insertions(+), 10 deletions(-)
> > > 
> > > diff --git a/include/linux/hugetlb.h b/include/linux/hugetlb.h
> > > index df6dd624ccfe..5f5e4177b2e0 100644
> [...]
> > > diff --git a/mm/migrate.c b/mm/migrate.c
> > > index 53010a142e7f..c5136fa48638 100644
> > > --- a/mm/migrate.c
> > > +++ b/mm/migrate.c
> > > @@ -2128,7 +2128,7 @@ static int add_page_for_migration(struct mm_struct *mm, unsigned long addr,
> > >  		if (PageHead(page)) {
> > >  			err = isolate_hugetlb(page_folio(page), pagelist);
> > >  			if (!err)
> > > -				err = 1;
> > > +				err = -EBUSY;
> > 
> > Again, I think this is confusing.  'err' is 'bool', not 'int'.
> 
> I mean, 'err' is not 'bool' but 'int', sorry.  See? This confuses me ;)
> 

Yes,
in the case here (and elsewhere) I like David's suggestion of using a separate
bool such as 'isolated' to capture the return value of the isolate function.
Then, the statement:

	err = isolated ? 0 : -EBUSY;

would be pretty clear.
-- 
Mike Kravetz