Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 23.128.96.32 as permitted sender) client-ip=23.128.96.32;
Message-ID: <dc01e0d2-17db-4531-bc7d-7d637d96c2f1@vivo.com>
Date:   Thu, 30 Nov 2023 09:56:59 +0800
User-Agent: Mozilla Thunderbird
Subject: Re: [PATCH] mm:vmscan: fix shrink sc->nr counter values issue
To:     Matthew Wilcox <willy@infradead.org>
Cc:     Andrew Morton <akpm@linux-foundation.org>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, opensource.kernel@vivo.com
References: <20231129130126.2130-1-justinjiang@vivo.com>
 <ZWdWFx1mXl29dP5U@casper.infradead.org>
From:   zhiguojiang <justinjiang@vivo.com>
In-Reply-To: <ZWdWFx1mXl29dP5U@casper.infradead.org>
Content-Type: text/plain; charset=UTF-8; format=flowed
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?cnF3dDhoWm0xYXN3VEVmTzcwQmJva2NkdHVzUEk5QXdIVUVOaUtnUGkrZEdB?=
 =?utf-8?B?Yk1qQXM5MlBmUktzSE4rVXlXZjgzQW4zbGxPWkVadXZHR1IrQ3kwZ3R2ZW5x?=
 =?utf-8?B?NmNqTnVGNXlWT0pKRzI3U3hNYWhzWXZQL0cyLzZIclZFVDBtLy9xYzNkZDVa?=
 =?utf-8?B?OGpsUUwwazZkblJ2QWlDMTJKWVdIbGZtUnlnOW53cDNwcEZhSGpKUjZsdUNt?=
 =?utf-8?B?d2tyajg4L1YzQmFLU3JPaWlPOC9tWUx0RWFjM0RnVlI4MmpNeFYyWnBBdUhL?=
 =?utf-8?B?eDhJeFFPOGpnaDFYTDlENXhQdUl2VGZaa0lYbmk2MDlJRmd5SklKQ3pKYWhO?=
 =?utf-8?B?c1VadHVFenFneWdGNU9rSU0rcFQ4aVVVaFZUSE8yMThoTHZWWllndGVYbjEx?=
 =?utf-8?B?T1lpWmFVUEx3RHBndlMzcDVVcC9pWFBYb2x2RDIwOWhESkZ4NFdsQ1gvSFgx?=
 =?utf-8?B?cmF3c1V2ZURHcmFhenduemREdm43VDRsWkllN0ZnNG9XcWF5a2tEdzB6bEs1?=
 =?utf-8?B?dlduaDh4OGVVL1p3OVlLekxmZXBxZk0vNUVWSUROSFRocXE4ZStQcnJGa0xC?=
 =?utf-8?B?TFROTkQ3dyszUldJOU9HSEVOeXZEWitvVGkyWG5ya3Vleklsd3FNUXIwOTZH?=
 =?utf-8?B?bGpIZU05QkNzL3BKRXhaY09OSmFHdW9wQ2YyeTdvcUZrbzJwSk9ETTFrY0Vo?=
 =?utf-8?B?RWlTR1djUFY1Nm1CVEVPblQyM0Q2MmlaazRFbGhnSkFpbnFVV2RMYndnK0Nh?=
 =?utf-8?B?bXZheVhwNmlpVzY2bkRXQ1M5enM5dDhweXo4aUFpWU1xVDNOS0RUcWtDMUtJ?=
 =?utf-8?B?SStXY0lFMFJsTitBSllPTHZDY3pESGVEVjdUN2MxUmlFNm9OSjd6RFJoN0pO?=
 =?utf-8?B?NHlzcjlsQjRUY05jbncwYm92ZkdpR2ZRUzFvb05xS2NpZ04zQndPY2xYV0RT?=
 =?utf-8?B?d3FXd2pJNXBiTDVsMWt0SkFaeGF5NDhVUEJKa3ZxZWg2THBkU3MzWkhsUklS?=
 =?utf-8?B?aDF1S0VUa25NVGpKeVNCdXE3RE82c1BpVit6VVpDdDRQMVdKU0ZUWmxNTXUw?=
 =?utf-8?B?MGtHczZ6Q09MOTFzNDFVeXZGV1h1SHdzMDlCMnM5Rm1wL2NIb1d1WEtSVy9U?=
 =?utf-8?B?UHFJT1k0VTJFNDAyOHZnNEZxS3pBdHFvTlF5ekJGRmduWmVZNzFuc2xRajFq?=
 =?utf-8?B?ZFY4UEZJaEpoaHpHWG03LzRNbWM4eGNHMC9RQUtvek5udXlyRFZiNm0xK3hF?=
 =?utf-8?B?QzdIcEZjV1o5QXVvWkxRTXpxa29qTGZIazZWWmpmaFEwZEpHWUFkTDVCWW9V?=
 =?utf-8?B?MS9va1NTVE9waUtpZDBFenBvN0tEaFltSkFwZ3c1NlFTYjZPVDYvUG5TNmt5?=
 =?utf-8?B?c0w2VTRKekZDK0w3ZVdPcU1Nci9rYXpsbjdzUU9Kd2lkU2JLeEJ6bVpQbnB1?=
 =?utf-8?B?REdRS3crbkEzNlp2VnJnclhxNEsvVjFjQnJSQldLOVVwYUJlRnhycndzUG9Q?=
 =?utf-8?B?UkFrbWNkTk5OTW8zaEpRSVEvTGRaellXT2pvNk01TFUrb3VVOEdZd3Z3eXBn?=
 =?utf-8?B?WTMwV1ZFV3ZJNU1JRTJ2bUtrWmxPcWFyVk5NRlVrU1Y2L1lob2F4Wlp4aURa?=
 =?utf-8?B?T1c5T09HeGFYR2xHVWpJbnpMc2E4dXRONjNyenpRN0lMblEzQS8wL2w5MEJB?=
 =?utf-8?B?QU9SN0h3VmZRN20vWC95dURENnA1UkpVNnJncVZ2TG9qcDB1ZGpvbGtMaXV4?=
 =?utf-8?B?UkRKSVA2S0JFdmR2aHorbmlVM1RwUlljZEdkeFlYU3pFTnV0NUdTOXo1d2x5?=
 =?utf-8?B?aVJ0MTZYOE1ndW9RN3lTQzU1aEVENzkwUVM5alZXYmlPU0krMWlIdzBjY0Ra?=
 =?utf-8?B?a1M4Y3RSSndrQldyQXNpSEJUaWQrQlpabWpmM0VwbTdxMWlTN1kzMGFKTzJ5?=
 =?utf-8?B?SnVSTjdRTWJkTzFORmFSNkhFN3ZvaE5EQy94RzVMSyttUGRnMUFPSkZrcVNT?=
 =?utf-8?B?cDVrbDF6MEhDZHlpck9UQnFCRDM2Sm0wTGNJdG94K1E0SVJxc0hmcy93T2hW?=
 =?utf-8?B?VDlOTUV5ZXFBZ0FxNG9qK0laZS94cTE1ZGNiOFhnWjlqd2lOZU9RUlhhSnd0?=
 =?utf-8?Q?W0sLStH1FaV86WUl5Gmn93g5F?=
X-OriginatorOrg: vivo.com
X-MS-Exchange-CrossTenant-Network-Message-Id: 3b3ba365-0b2a-42d5-2446-08dbf147a655
X-MS-Exchange-CrossTenant-AuthSource: JH0PR06MB6849.apcprd06.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 30 Nov 2023 01:57:04.7719
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 923e42dc-48d5-4cbe-b582-1a797a6412ed
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 31CcVvKVpRzPzMh0bMjWkgYvD5OQovY9elBINg5TxO2NwpFadNi9NAqDbSXldCP/L3xaDqZcxo/YGzF0gcSDcA==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: KL1PR06MB5994
X-Spam-Status: No, score=-0.9 required=5.0 tests=DKIM_SIGNED,DKIM_VALID,
	DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,
	SPF_HELO_NONE,SPF_PASS,T_SCC_BODY_TEXT_LINE autolearn=unavailable
	autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on agentk.vger.email
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org
X-Greylist: Sender passed SPF test, not delayed by milter-greylist-4.6.4 (agentk.vger.email [0.0.0.0]); Wed, 29 Nov 2023 17:57:26 -0800 (PST)


在 2023/11/29 23:17, Matthew Wilcox 写道:
> On Wed, Nov 29, 2023 at 09:01:26PM +0800, Zhiguo Jiang wrote:
>> It is needed to ensure sc->nr.unqueued_dirty > 0, which can avoid to
>> set PGDAT_DIRTY flag when sc->nr.unqueued_dirty and sc->nr.file_taken
>> are both zero at the same time.
> Have you observed this happening, or is this from code review?

Found in code review. The other sc->nr parameters are also judged whether they themselves are zero first in shrink_node.

>
>> It can't be guaranteed for the PGDAT_WRITEBACK flag that only pages
>> marked for immediate reclaim are on evictable LRUs in other following
>> shrink processes of the same kswapd shrink recycling. So when both a
>> small amount of pages marked for immediate reclaim and a large amount
>> of pages marked for non-immediate reclaim are on evictable LRUs at the
>> same time, if it's only determined that there is at least a page marked
>> for immediate reclaim on evictable LRUs, kswapd shrink is throttled to
>> sleep, which will increase kswapd process consumption.
>>
>> It can be fixed to throttle kswapd shrink when sc->nr.immediate is equal
>> to sc->nr.file_taken.
> So you're fixing two distinct things in the same patch?
It can be understood as two issues, and I will submit them separately.
>
>> +++ b/mm/vmscan.c
>> @@ -5915,17 +5915,17 @@ static void shrink_node(pg_data_t *pgdat, struct scan_control *sc)
>>   			set_bit(PGDAT_WRITEBACK, &pgdat->flags);
>>   
>>   		/* Allow kswapd to start writing pages during reclaim.*/
>> -		if (sc->nr.unqueued_dirty == sc->nr.file_taken)
>> +		if (sc->nr.unqueued_dirty && sc->nr.unqueued_dirty == sc->nr.file_taken)
>>   			set_bit(PGDAT_DIRTY, &pgdat->flags);
>>   
>>   		/*
>> -		 * If kswapd scans pages marked for immediate
>> +		 * If kswapd scans massive pages marked for immediate
> I don't understand why you've added the word "massive".  Do you mean
> that the pages are large, or that kswapd has scanned a lot of pages?
The added "massive" means that there are a large number of pages marked 
for immediate reclaim on evictable LRUs.

The added "massive" is relative to the situation that there is only a 
small amount of pages marked for immediate reclaim or even only one page 
marked for immediate reclaim on the evictable LRUs for throttle kswapd, 
and I think this situation don't need to throttle, because there may be 
other types of pages on evictable LRUs.
>
>>   		 * reclaim and under writeback (nr_immediate), it
>>   		 * implies that pages are cycling through the LRU
>>   		 * faster than they are written so forcibly stall
>>   		 * until some pages complete writeback.
>>   		 */
>> -		if (sc->nr.immediate)
>> +		if (sc->nr.immediate && sc->nr.immediate == sc->nr.file_taken)
>>   			reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK);
>>   	}