Re: [PATCH v2] khugepaged: hold invalidate_lock across collapse_file() readahead

[Baolin Wang <baolin.wang@linux.alibaba.com>]

On 9/14/26 6:34 AM, Matthew Wilcox wrote:
> On Sun, Sep 13, 2026 at 11:36:44PM +0700, Nguyen Ngoc Thang wrote:
>> @@ -2271,6 +2272,15 @@ static enum scan_result collapse_file(struct mm_struct *mm, unsigned long addr,
>>   	if (result != SCAN_SUCCEED)
>>   		goto out;
>>   
>> +	/*
>> +	 * Take invalidate_lock before any folio lock: the readahead below
>> +	 * needs it, and truncate holds it while waiting on folio locks.
>> +	 */
>> +	if (!is_shmem) {
>> +		filemap_invalidate_lock_shared(mapping);
>> +		need_unlock = true;
>> +	}
> 
> I'm not a fan of all this surplus commentary.  And what happens if e
> simultaeneously truncate a shmem file and collapse it at the same time?
> I know it doesn't use the invalidate lock, but does it go wrong in some
> other way?

IIUC, shmem uses the folio lock to synchronize truncate and collapse. It 
will check whether truncation has occurred after taking the folio lock 
in shmem_get_folio_gfp():
	
	folio_lock(folio);

	/* Has the folio been truncated or swapped out? */
	if (unlikely(folio->mapping != inode->i_mapping)) {
		folio_unlock(folio);
		folio_put(folio);
		goto repeat;
	}

So shmem looks safe here, unless I'm missing something.