Re: [PATCH] isofs: convert the zisofs read path to use folios

[Tal Zussman]

On 9/7/26 12:34 PM, Jan Kara wrote:
> On Sun 06-09-26 22:14:59, Tal Zussman wrote:
>> Store folios in the array that zisofs_read_folio() passes to
>> zisofs_fill_pages() and zisofs_uncompress_block(), use the folio APIs on
>> them, and grab the folios with __filemap_get_folio() instead of
>> grab_cache_page_nowait(). This removes six compound_head() calls, from
>> page_offset(), the three SetPageUptodate() calls, unlock_page() and
>> put_page(), and the last struct page usage in isofs.
>> 
>> grab_cache_page_nowait() passed FGP_NOFS as well, but FGP_NOWAIT already
>> prevents reclaim entirely, so leave it out, as FGP_NOFS is on its way
>> out [1].
>> 
>> Change poffset to be unsigned int rather than just unsigned while at
>> it.
>> 
>> [1] https://lore.kernel.org/linux-mm/20260830041901.2668-9-willy@xxxxxxxxxxxxx/
>> 
>> Signed-off-by: Tal Zussman <tz2294@xxxxxxxxxxxx>
> 
> Thanks for the conversion! The patch looks mostly good, some smaller
> comments below:
> 

Thanks! Will adjust all of these and send v2.

>> @@ -66,11 +66,12 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
>>  	if (block_size == 0) {
>>  		for ( i = 0 ; i < pcount ; i++ ) {
>>  			unsigned int off = i ? 0 : poffset;
>> +			struct folio *folio = folios[i];
>>  
>> -			if (!pages[i])
>> +			if (!folio)
>>  				continue;
>> -			memzero_page(pages[i], off, PAGE_SIZE - off);
>> -			SetPageUptodate(pages[i]);
>> +			folio_zero_range(folio, off, PAGE_SIZE - off);
> 						     ^^ perhaps
> "folio_size(folio) - off" here?
> 
>> +			folio_mark_uptodate(folio);
>>  		}
>>  		return (((loff_t)pcount) << PAGE_SHIFT) - poffset;
>>  	}
> ...
>> @@ -119,9 +120,11 @@ static loff_t zisofs_uncompress_block(struct inode *inode, loff_t block_start,
>>  
>>  	while (curpage < pcount && curbh < haveblocks &&
>>  	       zerr != Z_STREAM_END) {
>> +		struct folio *folio = folios[curpage];
>> +
>>  		if (!stream.avail_out) {
>> -			if (pages[curpage]) {
>> -				stream.next_out = kmap_local_page(pages[curpage])
>> +			if (folio) {
>> +				stream.next_out = kmap_local_folio(folio, 0)
>>  						+ poffset;
> 						  ^^^ this is just:
> 					kmap_local_folio(folio, poffset);
> 
>>  				stream.avail_out = PAGE_SIZE - poffset;
> 
> And this would be probably more idiomatic as "folio_size(folio) - poffset"
> after the conversion.
> 
> ...
>> @@ -289,9 +292,11 @@ static int zisofs_fill_pages(struct inode *inode, int full_page, int pcount,
>>  		cstart_block++;
>>  	}
>>  
>> -	if (poffset && *pages) {
>> -		memzero_page(*pages, poffset, PAGE_SIZE - poffset);
>> -		SetPageUptodate(*pages);
>> +	if (poffset && *folios) {
>> +		struct folio *folio = *folios;
>> +
>> +		folio_zero_range(folio, poffset, PAGE_SIZE - poffset);
> 						 ^^^ again
> folio_size(folio) here?
> 
>> +		folio_mark_uptodate(folio);
>>  	}
>>  	brelse(bh);
>>  	return 0;
> 
> 								Honza
> -- 
> Jan Kara <jack@xxxxxxxx>
> SUSE Labs, CR
>