Re: [PATCH v3 0/5] btrfs: remove buffer heads form superblock handling

Johannes Thumshirn
2020-01-30T16:09:53Z

On 30/01/2020 16:56, Christoph Hellwig wrote:
> On Thu, Jan 30, 2020 at 03:53:37PM +0000, Johannes Thumshirn wrote:
>> On 30/01/2020 14:39, Christoph Hellwig wrote:
>>> On Thu, Jan 30, 2020 at 01:15:30PM +0100, David Sterba wrote:
>>>>> Sure but with hch's proposed change to using read_cache_page_gfp() this
>>>>> doesn't make too much sense anymore at least for the read path.
>>>>>
>>>>> Maybe "use page cache for superblock reading"?
>>>>
>>>> That works too. We might need a new iteration that summarizes up all the
>>>> feedback so far, so we have same code to refer to.
>>>
>>> Per my question on the second patch: why even use the page cache at
>>> all. btrfs already caches the value outside the pagecache, so why
>>> even bother with the page cache overhead?
>>>
>> This is what my first version did, alloc_page() and submit_bio()
>> directly [1]. But reviewers told me to go the route via page cache.
>
> I only see your patch at the url, not any reply. What is the issue
> of not using the page cache? Also you really shoudn't need a separate
> alloc_page - you should be able to use the already cached superblock
> as the destination and source of I/O, assuming they are properly aligned
> (and if not that could be fixed easily).
>

Care to elaborate? Who would have cached the superblock, when we haven't
mounted the FS yet.

So here's the answer from that thread:

"IIRC we had some funny bugs when mount and device scan (udev) raced
just after mkfs, the page cache must be used so there's no way to read
stale data."