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." https://lore.kernel.org/linux-btrfs/20200117151352.GK3929@xxxxxxxxxxxxx/
