On 4/15/21 1:04 AM, Qu Wenruo wrote:
There is a pretty bad abuse of btrfs_writepage_endio_finish_ordered() in
end_compressed_bio_write().

It passes compressed pages to btrfs_writepage_endio_finish_ordered(),
which is only supposed to accept inode pages.

Thankfully the important info here is the inode, so let's pass
btrfs_inode directly into btrfs_writepage_endio_finish_ordered(), and
make @page parameter optional.

By this, end_compressed_bio_write() can happily pass page=NULL while
still get everything done properly.

Also, to cooperate with such modification, replace @page parameter for
trace_btrfs_writepage_end_io_hook() with btrfs_inode.
Although this removes page_index info, the existing start/len should be
enough for most usage.

Signed-off-by: Qu Wenruo <w...@suse.com>
---
  fs/btrfs/compression.c       |  4 +---
  fs/btrfs/ctree.h             |  3 ++-
  fs/btrfs/extent_io.c         | 16 ++++++++++------
  fs/btrfs/inode.c             |  9 +++++----
  include/trace/events/btrfs.h | 19 ++++++++-----------
  5 files changed, 26 insertions(+), 25 deletions(-)

diff --git a/fs/btrfs/compression.c b/fs/btrfs/compression.c
index 2600703fab83..4fbe3e12be71 100644
--- a/fs/btrfs/compression.c
+++ b/fs/btrfs/compression.c
@@ -343,11 +343,9 @@ static void end_compressed_bio_write(struct bio *bio)
         * call back into the FS and do all the end_io operations
         */
        inode = cb->inode;
-       cb->compressed_pages[0]->mapping = cb->inode->i_mapping;
-       btrfs_writepage_endio_finish_ordered(cb->compressed_pages[0],
+       btrfs_writepage_endio_finish_ordered(BTRFS_I(inode), NULL,
                        cb->start, cb->start + cb->len - 1,
                        bio->bi_status == BLK_STS_OK);
-       cb->compressed_pages[0]->mapping = NULL;
end_compressed_writeback(inode, cb);
        /* note, our inode could be gone now */
diff --git a/fs/btrfs/ctree.h b/fs/btrfs/ctree.h
index 2c858d5349c8..505bc6674bcc 100644
--- a/fs/btrfs/ctree.h
+++ b/fs/btrfs/ctree.h
@@ -3175,7 +3175,8 @@ int btrfs_run_delalloc_range(struct btrfs_inode *inode, 
struct page *locked_page
                u64 start, u64 end, int *page_started, unsigned long 
*nr_written,
                struct writeback_control *wbc);
  int btrfs_writepage_cow_fixup(struct page *page, u64 start, u64 end);
-void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
+void btrfs_writepage_endio_finish_ordered(struct btrfs_inode *inode,
+                                         struct page *page, u64 start,
                                          u64 end, int uptodate);
  extern const struct dentry_operations btrfs_dentry_operations;
  extern const struct iomap_ops btrfs_dio_iomap_ops;
diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
index 7d1fca9b87f0..6d712418b67b 100644
--- a/fs/btrfs/extent_io.c
+++ b/fs/btrfs/extent_io.c
@@ -2711,10 +2711,13 @@ blk_status_t btrfs_submit_read_repair(struct inode 
*inode,
void end_extent_writepage(struct page *page, int err, u64 start, u64 end)
  {
+       struct btrfs_inode *inode;
        int uptodate = (err == 0);
        int ret = 0;
- btrfs_writepage_endio_finish_ordered(page, start, end, uptodate);
+       ASSERT(page && page->mapping);
+       inode = BTRFS_I(page->mapping->host);
+       btrfs_writepage_endio_finish_ordered(inode, page, start, end, uptodate);
if (!uptodate) {
                ClearPageUptodate(page);
@@ -3739,7 +3742,8 @@ static noinline_for_stack int 
__extent_writepage_io(struct btrfs_inode *inode,
                u32 iosize;
if (cur >= i_size) {
-                       btrfs_writepage_endio_finish_ordered(page, cur, end, 1);
+                       btrfs_writepage_endio_finish_ordered(inode, page, cur,
+                                                            end, 1);
                        break;
                }
                em = btrfs_get_extent(inode, NULL, 0, cur, end - cur + 1);
@@ -3777,8 +3781,8 @@ static noinline_for_stack int 
__extent_writepage_io(struct btrfs_inode *inode,
                        if (compressed)
                                nr++;
                        else
-                               btrfs_writepage_endio_finish_ordered(page, cur,
-                                                       cur + iosize - 1, 1);
+                               btrfs_writepage_endio_finish_ordered(inode,
+                                               page, cur, cur + iosize - 1, 1);
                        cur += iosize;
                        continue;
                }
@@ -4842,8 +4846,8 @@ int extent_write_locked_range(struct inode *inode, u64 
start, u64 end,
                if (clear_page_dirty_for_io(page))
                        ret = __extent_writepage(page, &wbc_writepages, &epd);
                else {
-                       btrfs_writepage_endio_finish_ordered(page, start,
-                                                   start + PAGE_SIZE - 1, 1);
+                       btrfs_writepage_endio_finish_ordered(BTRFS_I(inode),
+                                       page, start, start + PAGE_SIZE - 1, 1);
                        unlock_page(page);
                }
                put_page(page);
diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
index 554effbf307e..752f0c78e1df 100644
--- a/fs/btrfs/inode.c
+++ b/fs/btrfs/inode.c
@@ -951,7 +951,8 @@ static noinline void submit_compressed_extents(struct 
async_chunk *async_chunk)
                        const u64 end = start + async_extent->ram_size - 1;
p->mapping = inode->vfs_inode.i_mapping;
-                       btrfs_writepage_endio_finish_ordered(p, start, end, 0);
+                       btrfs_writepage_endio_finish_ordered(inode, p, start,
+                                                            end, 0);
p->mapping = NULL;
                        extent_clear_unlock_delalloc(inode, start, end, NULL, 0,
@@ -3058,16 +3059,16 @@ static void finish_ordered_fn(struct btrfs_work *work)
        btrfs_finish_ordered_io(ordered_extent);
  }
-void btrfs_writepage_endio_finish_ordered(struct page *page, u64 start,
+void btrfs_writepage_endio_finish_ordered(struct btrfs_inode *inode,
+                                         struct page *page, u64 start,
                                          u64 end, int uptodate)
  {
-       struct btrfs_inode *inode = BTRFS_I(page->mapping->host);
        struct btrfs_fs_info *fs_info = inode->root->fs_info;
        struct btrfs_ordered_extent *ordered_extent = NULL;
        struct btrfs_workqueue *wq;
ASSERT(end + 1 - start < U32_MAX);
-       trace_btrfs_writepage_end_io_hook(page, start, end, uptodate);
+       trace_btrfs_writepage_end_io_hook(inode, start, end, uptodate);
ClearPagePrivate2(page);
        if (!btrfs_dec_test_ordered_pending(inode, &ordered_extent, start,
diff --git a/include/trace/events/btrfs.h b/include/trace/events/btrfs.h
index 0551ea65374f..556967cb9688 100644
--- a/include/trace/events/btrfs.h
+++ b/include/trace/events/btrfs.h
@@ -654,34 +654,31 @@ DEFINE_EVENT(btrfs__writepage, __extent_writepage,
TRACE_EVENT(btrfs_writepage_end_io_hook, - TP_PROTO(const struct page *page, u64 start, u64 end, int uptodate),
+       TP_PROTO(const struct btrfs_inode *inode, u64 start, u64 end,
+                int uptodate),
- TP_ARGS(page, start, end, uptodate),
+       TP_ARGS(inode, start, end, uptodate),
TP_STRUCT__entry_btrfs(
                __field(        u64,     ino            )
-               __field(        unsigned long, index    )

You don't need to remove this, you could just do something like

(start & PAGE_MASK) >> PAGE_SHIFT

Check my math there, I'm a little foggy this morning. I'd rather err on the side of not removing stuff from tracepoints that we can still get. Especially once we start dealing with bugs from subpage support, it may be useful to track per-page operations via the tracepoints. Otherwise this is a solid change. Thanks,

Josef

Reply via email to