Re: [PATCH 6/8] btrfs: store a kernel virtual address in struct sector_ptr

[Qu Wenruo <wqu@suse.com>]

在 2025/4/14 12:34, Qu Wenruo 写道:
> 
> 
> 在 2025/4/10 15:04, Christoph Hellwig 写道:
>> On Thu, Apr 10, 2025 at 08:04:25AM +0930, Qu Wenruo wrote:
>>> 在 2025/4/9 20:40, Christoph Hellwig 写道:
>>>> All data pointed to by struct sector_ptr is non-highmem kernel memory.
>>>> Simplify the code by using a void pointer instead of a page + offset
>>>> pair and dropping all the kmap calls.
>>>
>>> But the higher level bio from btrfs filemaps can have highmem pages.
>>>
>>> That's why we keep the kmap/kunmap.
>>
>> Where do filemap pages come into the raid code?   As far as I can see
>> they are always copied, and the memory is only allocated in the raid
>> code.  As seen in this code we have two direct allocations that I
>> convered to __get_free_page, and one case that uses the bulk page
>> allocator where I use page_address.  Or did I miss something?
> 
> The function sector_in_rbio() can force to use the sector_ptr in 
> bio_sectors, and by default we use bio_sectors first.
> 
> Thus all the kmap/kunmap pairs are needed for call sites that is 
> grabbing the sector through sector_in_rbio().
> 
> This includes:
> 
> - generate_pq_vertical()
> - rmw_assemble_write_bios()
> - verify_one_sector() for READ_REBUILD
> - recovery_vertical() for READ_REBUILD
> 
> So I'm afraid we still need the kmap/kunmap for now.

Most of the series still looks very good to me, for this problem I'll 
changed it to use physical addresses, and keep the kmap/kunmap instead.

When the modified series is properly tested on PAE systems I'll send out 
the refreshed version for review.

Thanks,
Qu

> 
>>
>>> Or is there a way to set the filemap to no use any highmem pages?
>>
>> You can restrict the allocation of a mapping to avoid highmem using
>> mapping_set_gfp_mask().  But that would not help with direct I/O IFF
>> user pages came into this code.
> 
> Fine, my dream of getting rid of highmem inside btrfs is still really a 
> dream...
> 
> Thanks,
> Qu
>