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? -- Goldwyn
