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 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by smtp.lore.kernel.org (Postfix) with ESMTP id E3590C636CC for ; Tue, 14 Feb 2023 01:26:12 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S229732AbjBNB0L (ORCPT ); Mon, 13 Feb 2023 20:26:11 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:40766 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S229601AbjBNB0I (ORCPT ); Mon, 13 Feb 2023 20:26:08 -0500 Received: from out30-112.freemail.mail.aliyun.com (out30-112.freemail.mail.aliyun.com [115.124.30.112]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id DA4F912061; Mon, 13 Feb 2023 17:26:05 -0800 (PST) X-Alimail-AntiSpam: AC=PASS;BC=-1|-1;BR=01201311R911e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018045168;MF=baolin.wang@linux.alibaba.com;NM=1;PH=DS;RN=5;SR=0;TI=SMTPD_---0VbdH-ho_1676337963; Received: from 30.97.48.79(mailfrom:baolin.wang@linux.alibaba.com fp:SMTPD_---0VbdH-ho_1676337963) by smtp.aliyun-inc.com; Tue, 14 Feb 2023 09:26:03 +0800 Message-ID: Date: Tue, 14 Feb 2023 09:26:03 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:102.0) Gecko/20100101 Thunderbird/102.7.2 Subject: Re: [GIT PULL] hotfixes for 6.2 To: Linus Torvalds , Andrew Morton Cc: linux-mm@kvack.org, mm-commits@vger.kernel.org, linux-kernel@vger.kernel.org References: <20230213140812.db63c7146ebc396691594b73@linux-foundation.org> From: Baolin Wang In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 7bit Precedence: bulk List-ID: X-Mailing-List: linux-kernel@vger.kernel.org On 2/14/2023 6:19 AM, Linus Torvalds wrote: > On Mon, Feb 13, 2023 at 2:08 PM Andrew Morton wrote: >> >> Kuan-Ying Lee (1): >> mm/gup: add folio to list when folio_isolate_lru() succeed > > Ugh. I really hate fixes like this. > > The problem came from mis-understanding the return value of > folio_isolate_lru(), and thinking that it was a boolean > success/failure thing. > > It wasn't, it was an integer "success/errno" thing, and the sense of > the test was wrong. So the patch is > > - if (!folio_isolate_lru(folio)) > + if (folio_isolate_lru(folio)) > continue; > > but at no point was the code *clarified*. > > Wouldn't it have been much better to write the new code to be > > if (folio_isolate_lru(folio) < 0) > continue; > > to actually make it clear that this is a "negative error return check". > > I've pulled this, but I really think that when somebody notices that > we had a silly bug because of a misunderstanding like this, it's not > just that the bug should be fixed, the code should also be *clarified* > at the same time. Yes, agree, I need to check the return value of folio_isolate_lru() every time when looking at the code. I can help to create a patch to make it clear for all users.