From: Andreas Gruenbacher <agruenba@redhat.com>
To: Christoph Hellwig <hch@lst.de>
Cc: "Darrick J . Wong" <darrick.wong@oracle.com>,
linux-xfs@vger.kernel.org,
linux-fsdevel <linux-fsdevel@vger.kernel.org>,
cluster-devel <cluster-devel@redhat.com>
Subject: Re: [PATCH 15/15] gfs2: use iomap for buffered I/O in ordered and writeback mode
Date: Mon, 5 Aug 2019 14:27:21 +0200 [thread overview]
Message-ID: <CAHc6FU4wtDwLv_TU6xydtO2h8P8jE1ddPjVqs8=NDFFDNEpiLA@mail.gmail.com> (raw)
In-Reply-To: <20190701215439.19162-16-hch@lst.de>
Christoph,
thanks again for this patch and the rest of the patch queue. There's
one minor bug here (see below). With that and the gfs2_walk_metadata
fix I've just posted to cluster-devel, this is now all working nicely.
On Mon, 1 Jul 2019 at 23:56, Christoph Hellwig <hch@lst.de> wrote:
> Switch to using the iomap readpage and writepage helpers for all I/O in
> the ordered and writeback modes, and thus eliminate using buffer_heads
> for I/O in these cases. The journaled data mode is left untouched.
>
> Signed-off-by: Christoph Hellwig <hch@lst.de>
> ---
> fs/gfs2/aops.c | 59 +++++++++++++++++++++++---------------------------
> fs/gfs2/bmap.c | 47 ++++++++++++++++++++++++++++++----------
> fs/gfs2/bmap.h | 1 +
> 3 files changed, 63 insertions(+), 44 deletions(-)
>
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index 15a234fb8f88..9cdd61a44379 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -91,22 +91,13 @@ static int gfs2_writepage(struct page *page, struct writeback_control *wbc)
> struct inode *inode = page->mapping->host;
> struct gfs2_inode *ip = GFS2_I(inode);
> struct gfs2_sbd *sdp = GFS2_SB(inode);
> - loff_t i_size = i_size_read(inode);
> - pgoff_t end_index = i_size >> PAGE_SHIFT;
> - unsigned offset;
> + struct iomap_writepage_ctx wpc = { };
>
> if (gfs2_assert_withdraw(sdp, gfs2_glock_is_held_excl(ip->i_gl)))
> goto out;
> if (current->journal_info)
> goto redirty;
> - /* Is the page fully outside i_size? (truncate in progress) */
> - offset = i_size & (PAGE_SIZE-1);
> - if (page->index > end_index || (page->index == end_index && !offset)) {
> - page->mapping->a_ops->invalidatepage(page, 0, PAGE_SIZE);
> - goto out;
> - }
> -
> - return nobh_writepage(page, gfs2_get_block_noalloc, wbc);
> + return iomap_writepage(page, wbc, &wpc, &gfs2_writeback_ops);
>
> redirty:
> redirty_page_for_writepage(wbc, page);
> @@ -210,7 +201,8 @@ static int gfs2_writepages(struct address_space *mapping,
> struct writeback_control *wbc)
> {
> struct gfs2_sbd *sdp = gfs2_mapping2sbd(mapping);
> - int ret = mpage_writepages(mapping, wbc, gfs2_get_block_noalloc);
> + struct iomap_writepage_ctx wpc = { };
> + int ret;
>
> /*
> * Even if we didn't write any pages here, we might still be holding
> @@ -218,9 +210,9 @@ static int gfs2_writepages(struct address_space *mapping,
> * want balance_dirty_pages() to loop indefinitely trying to write out
> * pages held in the ail that it can't find.
> */
> + ret = iomap_writepages(mapping, wbc, &wpc, &gfs2_writeback_ops);
> if (ret == 0)
> set_bit(SDF_FORCE_AIL_FLUSH, &sdp->sd_flags);
> -
> return ret;
> }
>
> @@ -469,7 +461,6 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
> return 0;
> }
>
> -
> /**
> * __gfs2_readpage - readpage
> * @file: The file to read a page for
> @@ -479,16 +470,15 @@ static int stuffed_readpage(struct gfs2_inode *ip, struct page *page)
> * reading code as in that case we already hold the glock. Also it's
> * called by gfs2_readpage() once the required lock has been granted.
> */
> -
> static int __gfs2_readpage(void *file, struct page *page)
> {
> - struct gfs2_inode *ip = GFS2_I(page->mapping->host);
> - struct gfs2_sbd *sdp = GFS2_SB(page->mapping->host);
> -
> + struct inode *inode = page->mapping->host;
> + struct gfs2_inode *ip = GFS2_I(inode);
> + struct gfs2_sbd *sdp = GFS2_SB(inode);
> int error;
>
> - if (i_blocksize(page->mapping->host) == PAGE_SIZE &&
> - !page_has_buffers(page)) {
> + if (!gfs2_is_jdata(ip) ||
> + (i_blocksize(inode) == PAGE_SIZE && !page_has_buffers(page))) {
> error = iomap_readpage(page, &gfs2_iomap_ops);
> } else if (gfs2_is_stuffed(ip)) {
> error = stuffed_readpage(ip, page);
> @@ -609,8 +599,12 @@ static int gfs2_readpages(struct file *file, struct address_space *mapping,
> ret = gfs2_glock_nq(&gh);
> if (unlikely(ret))
> goto out_uninit;
> - if (!gfs2_is_stuffed(ip))
> + if (gfs2_is_stuffed(ip))
> + ;
> + else if (gfs2_is_jdata(ip))
> ret = mpage_readpages(mapping, pages, nr_pages, gfs2_block_map);
> + else
> + ret = iomap_readpages(mapping, pages, nr_pages, &gfs2_iomap_ops);
> gfs2_glock_dq(&gh);
> out_uninit:
> gfs2_holder_uninit(&gh);
> @@ -827,17 +821,18 @@ int gfs2_releasepage(struct page *page, gfp_t gfp_mask)
> }
>
> static const struct address_space_operations gfs2_aops = {
> - .writepage = gfs2_writepage,
> - .writepages = gfs2_writepages,
> - .readpage = gfs2_readpage,
> - .readpages = gfs2_readpages,
> - .bmap = gfs2_bmap,
> - .invalidatepage = gfs2_invalidatepage,
> - .releasepage = gfs2_releasepage,
> - .direct_IO = noop_direct_IO,
> - .migratepage = buffer_migrate_page,
> - .is_partially_uptodate = block_is_partially_uptodate,
> - .error_remove_page = generic_error_remove_page,
> + .writepage = gfs2_writepage,
> + .writepages = gfs2_writepages,
> + .readpage = gfs2_readpage,
> + .readpages = gfs2_readpages,
> + .set_page_dirty = iomap_set_page_dirty,
> + .releasepage = iomap_releasepage,
> + .invalidatepage = iomap_invalidatepage,
> + .bmap = gfs2_bmap,
> + .direct_IO = noop_direct_IO,
> + .migratepage = iomap_migrate_page,
> + .is_partially_uptodate = iomap_is_partially_uptodate,
> + .error_remove_page = generic_error_remove_page,
> };
>
> static const struct address_space_operations gfs2_jdata_aops = {
> diff --git a/fs/gfs2/bmap.c b/fs/gfs2/bmap.c
> index b7bd811872cb..b8d795d277c9 100644
> --- a/fs/gfs2/bmap.c
> +++ b/fs/gfs2/bmap.c
> @@ -56,7 +56,6 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh,
> u64 block, struct page *page)
> {
> struct inode *inode = &ip->i_inode;
> - struct buffer_head *bh;
> int release = 0;
>
> if (!page || page->index) {
> @@ -80,20 +79,20 @@ static int gfs2_unstuffer_page(struct gfs2_inode *ip, struct buffer_head *dibh,
> SetPageUptodate(page);
> }
>
> - if (!page_has_buffers(page))
> - create_empty_buffers(page, BIT(inode->i_blkbits),
> - BIT(BH_Uptodate));
> + if (gfs2_is_jdata(ip)) {
> + struct buffer_head *bh;
>
> - bh = page_buffers(page);
> + if (!page_has_buffers(page))
> + create_empty_buffers(page, BIT(inode->i_blkbits),
> + BIT(BH_Uptodate));
>
> - if (!buffer_mapped(bh))
> - map_bh(bh, inode->i_sb, block);
> + bh = page_buffers(page);
> + if (!buffer_mapped(bh))
> + map_bh(bh, inode->i_sb, block);
>
> - set_buffer_uptodate(bh);
> - if (gfs2_is_jdata(ip))
> + set_buffer_uptodate(bh);
> gfs2_trans_add_data(ip->i_gl, bh);
> - else {
> - mark_buffer_dirty(bh);
We need to turn mark_buffer_dirty(bh) into set_page_dirty(page) here
instead of just removing it.
> + } else {
> gfs2_ordered_add_inode(ip);
> }
>
> @@ -1127,7 +1126,8 @@ static int gfs2_iomap_begin(struct inode *inode, loff_t pos, loff_t length,
> struct metapath mp = { .mp_aheight = 1, };
> int ret;
>
> - iomap->flags |= IOMAP_F_BUFFER_HEAD;
> + if (gfs2_is_jdata(ip))
> + iomap->flags |= IOMAP_F_BUFFER_HEAD;
>
> trace_gfs2_iomap_start(ip, pos, length, flags);
> if ((flags & IOMAP_WRITE) && !(flags & IOMAP_DIRECT)) {
> @@ -2431,3 +2431,26 @@ int __gfs2_punch_hole(struct file *file, loff_t offset, loff_t length)
> gfs2_trans_end(sdp);
> return error;
> }
> +
> +static int gfs2_map_blocks(struct iomap_writepage_ctx *wpc, struct inode *inode,
> + loff_t offset)
> +{
> + struct metapath mp = { .mp_aheight = 1, };
> + int ret;
> +
> + if (WARN_ON_ONCE(gfs2_is_stuffed(GFS2_I(inode))))
> + return -EIO;
> +
> + if (offset >= wpc->iomap.offset &&
> + offset < wpc->iomap.offset + wpc->iomap.length)
> + return 0;
> +
> + memset(&wpc->iomap, 0, sizeof(wpc->iomap));
> + ret = gfs2_iomap_get(inode, offset, INT_MAX, 0, &wpc->iomap, &mp);
> + release_metapath(&mp);
> + return ret;
> +}
> +
> +const struct iomap_writeback_ops gfs2_writeback_ops = {
> + .map_blocks = gfs2_map_blocks,
> +};
> diff --git a/fs/gfs2/bmap.h b/fs/gfs2/bmap.h
> index b88fd45ab79f..aed4632d47d3 100644
> --- a/fs/gfs2/bmap.h
> +++ b/fs/gfs2/bmap.h
> @@ -44,6 +44,7 @@ static inline void gfs2_write_calc_reserv(const struct gfs2_inode *ip,
> }
>
> extern const struct iomap_ops gfs2_iomap_ops;
> +extern const struct iomap_writeback_ops gfs2_writeback_ops;
>
> extern int gfs2_unstuff_dinode(struct gfs2_inode *ip, struct page *page);
> extern int gfs2_block_map(struct inode *inode, sector_t lblock,
> --
> 2.20.1
>
Thanks,
Andreas
next prev parent reply other threads:[~2019-08-05 12:27 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2019-07-01 21:54 RFC: use the iomap writepage path in gfs2 Christoph Hellwig
2019-07-01 21:54 ` [PATCH 01/15] FOLD: iomap: make the discard_page method optional Christoph Hellwig
2019-07-01 21:54 ` [PATCH 02/15] FOLD: iomap: make ->submit_ioend optional Christoph Hellwig
2019-07-01 21:54 ` [PATCH 03/15] iomap: zero newly allocated mapped blocks Christoph Hellwig
2019-07-01 21:54 ` [PATCH 04/15] iomap: warn on inline maps iomap_writepage_map Christoph Hellwig
2019-07-01 21:54 ` [PATCH 05/15] iomap: move struct iomap_page to iomap.c Christoph Hellwig
2019-07-01 21:54 ` [PATCH 06/15] HACK: disable lockdep annotation in iomap_dio_rw Christoph Hellwig
2019-07-01 21:54 ` [PATCH 07/15] gfs2: use page_offset in gfs2_page_mkwrite Christoph Hellwig
2019-07-01 21:54 ` [PATCH 08/15] gfs2: remove the unused gfs2_stuffed_write_end function Christoph Hellwig
2019-07-01 21:54 ` [PATCH 09/15] gfs2: merge gfs2_writeback_aops and gfs2_ordered_aops Christoph Hellwig
2019-07-01 21:54 ` [PATCH 10/15] gfs2: merge gfs2_writepage_common into gfs2_writepage Christoph Hellwig
2019-07-01 21:54 ` [PATCH 11/15] gfs2: mark stuffed_readpage static Christoph Hellwig
2019-07-01 21:54 ` [PATCH 12/15] gfs2: use iomap_bmap instead of generic_block_bmap Christoph Hellwig
2019-07-01 21:54 ` [PATCH 13/15] gfs2: implement gfs2_block_zero_range using iomap_zero_range Christoph Hellwig
2019-07-01 21:54 ` [PATCH 14/15] gfs2: don't use buffer_heads in gfs2_allocate_page_backing Christoph Hellwig
2019-07-01 21:54 ` [PATCH 15/15] gfs2: use iomap for buffered I/O in ordered and writeback mode Christoph Hellwig
2019-08-05 12:27 ` Andreas Gruenbacher [this message]
2019-08-06 5:30 ` Christoph Hellwig
2019-09-30 20:49 ` Andreas Gruenbacher
2019-07-03 22:35 ` RFC: use the iomap writepage path in gfs2 Andreas Gruenbacher
2019-07-08 16:03 ` Christoph Hellwig
2019-07-08 17:29 ` Andreas Gruenbacher
2019-07-08 0:01 ` Dave Chinner
2019-07-08 16:19 ` Christoph Hellwig
2019-12-10 10:19 [PATCH 15/15] gfs2: use iomap for buffered I/O in ordered and writeback mode Andreas Gruenbacher
2019-12-12 10:42 ` Christoph Hellwig
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAHc6FU4wtDwLv_TU6xydtO2h8P8jE1ddPjVqs8=NDFFDNEpiLA@mail.gmail.com' \
--to=agruenba@redhat.com \
--cc=cluster-devel@redhat.com \
--cc=darrick.wong@oracle.com \
--cc=hch@lst.de \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-xfs@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).