On 26.06.19 г. 6:03 ч., Goldwyn Rodrigues wrote: > On 21:04 25/06, Filipe Manana wrote: >> On Tue, Jun 25, 2019 at 8:58 PM Goldwyn Rodrigues <rgoldwyn@xxxxxxx> wrote: >>> >>> On 9:05 24/06, Christoph Hellwig wrote: >>>> On Fri, Jun 21, 2019 at 02:28:25PM -0500, Goldwyn Rodrigues wrote: >>>>> From: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> >>>>> >>>>> btrfs uses page->private as well to store extent_buffer. Make >>>>> the check stricter to make sure we are using page->private for iop by >>>>> comparing iblocksize < PAGE_SIZE. >>>>> >>>>> Signed-off-by: Goldwyn Rodrigues <rgoldwyn@xxxxxxxx> >>>> >>>> If btrfs uses page->private itself and also uses functions that call >>>> to_iomap_page we have a major problem, as we now have a usage conflict. >>>> >>>> How do you end up here? >>>> >>> >>> Btrfs uses page->private to identify which extent_buffer it belongs to. >>> So, if you read, it fills the page->private. Then you try to write to >>> it, iomap will assume it to be iomap_page pointer. >>> >>> I don't think we can move extent_buffer out of page->private for btrfs. >>> Any other ideas? >> >> The extent buffer is only for pages belonging to the btree inode (i.e. >> pages that correspond to a btree node/lead). >> Haven't looked in detail to this patchset, but you can't do buffered >> writes or direct IO against the btree inode, can you? >> So for file inodes, this problem doesn't exist. > > Why do we call set_page_extent_mapped(page) in lock_and_cleanup_extent_if_needed() or __do_readpage()? > > I must admit, the backtrace crashes that I saw had the page->private set to > EXTENT_PAGE_PRIVATE rather than the extent_buffer pointer. Does that mean > calling this function is not necessary in these codepaths? On a quick look it seems PagePrivate is not used for ordinary data pages. E.g. set_Page_extent_mapped seems to be a leftover from old code. No one is actually checking the EXTENT_PAGE_PRIVATE flag. For data pages what seems to be important is PagePrivate2 for the fixup worker. IMO set_page_extent_mapped should be removed as well as references to pageprivate in the context of ordinary data pages. Chris, any ideas what set_page_extent_mapped was used for? >
