From: John Hubbard <jhubbard@nvidia.com>
To: Jan Kara <jack@suse.cz>, <linux-fsdevel@vger.kernel.org>
Cc: <linux-block@vger.kernel.org>, <linux-mm@kvack.org>,
David Howells <dhowells@redhat.com>,
David Hildenbrand <david@redhat.com>
Subject: Re: [PATCH 3/5] mm: Do not try to write pinned folio during memory cleaning writeback
Date: Thu, 9 Feb 2023 17:54:14 -0800 [thread overview]
Message-ID: <4961eb2d-c36b-d6a5-6a43-0c35d24606c0@nvidia.com> (raw)
In-Reply-To: <20230209123206.3548-3-jack@suse.cz>
On 2/9/23 04:31, Jan Kara wrote:
> When a folio is pinned, there is no point in trying to write it during
> memory cleaning writeback. We cannot reclaim the folio until it is
> unpinned anyway and we cannot even be sure the folio is really clean.
> On top of that writeback of such folio may be problematic as the data
> can change while the writeback is running thus causing checksum or
> DIF/DIX failures. So just don't bother doing memory cleaning writeback
> for pinned folios.
>
> Signed-off-by: Jan Kara <jack@suse.cz>
> ---
> fs/9p/vfs_addr.c | 2 +-
> fs/afs/file.c | 2 +-
> fs/afs/write.c | 6 +++---
> fs/btrfs/extent_io.c | 14 +++++++-------
> fs/btrfs/free-space-cache.c | 2 +-
> fs/btrfs/inode.c | 2 +-
> fs/btrfs/subpage.c | 2 +-
Hi Jan!
Just a quick note that this breaks the btrfs build in subpage.c.
Because, unfortunately, btrfs creates 6 sets of functions via calls to a
macro: IMPLEMENT_BTRFS_PAGE_OPS(). And that expects only one argument to
the clear_page_func, and thus to clear_page_dirty_for_io().
It seems infeasible (right?) to add another argument to the other
clear_page_func functions, which include:
ClearPageUptodate
ClearPageError
end_page_writeback
ClearPageOrdered
ClearPageChecked
, so I expect IMPLEMENT_BTRFS_PAGE_OPS() may need to be partially
unrolled, in order to pass in the new writeback control arg to
clear_page_dirty_for_io().
thanks,
--
John Hubbard
NVIDIA
> fs/ceph/addr.c | 6 +++---
> fs/cifs/file.c | 6 +++---
> fs/ext4/inode.c | 4 ++--
> fs/f2fs/checkpoint.c | 4 ++--
> fs/f2fs/compress.c | 2 +-
> fs/f2fs/data.c | 2 +-
> fs/f2fs/dir.c | 2 +-
> fs/f2fs/gc.c | 4 ++--
> fs/f2fs/inline.c | 2 +-
> fs/f2fs/node.c | 10 +++++-----
> fs/fuse/file.c | 2 +-
> fs/gfs2/aops.c | 2 +-
> fs/nfs/write.c | 2 +-
> fs/nilfs2/page.c | 2 +-
> fs/nilfs2/segment.c | 8 ++++----
> fs/orangefs/inode.c | 2 +-
> fs/ubifs/file.c | 2 +-
> include/linux/pagemap.h | 5 +++--
> mm/folio-compat.c | 4 ++--
> mm/migrate.c | 2 +-
> mm/page-writeback.c | 24 ++++++++++++++++++++----
> mm/vmscan.c | 2 +-
> 29 files changed, 73 insertions(+), 56 deletions(-)
>
> diff --git a/fs/9p/vfs_addr.c b/fs/9p/vfs_addr.c
> index 97599edbc300..a14ff3c02eb1 100644
> --- a/fs/9p/vfs_addr.c
> +++ b/fs/9p/vfs_addr.c
> @@ -221,7 +221,7 @@ static int v9fs_launder_folio(struct folio *folio)
> {
> int retval;
>
> - if (folio_clear_dirty_for_io(folio)) {
> + if (folio_clear_dirty_for_io(NULL, folio)) {
> retval = v9fs_vfs_write_folio_locked(folio);
> if (retval)
> return retval;
> diff --git a/fs/afs/file.c b/fs/afs/file.c
> index 68d6d5dc608d..8a81ac9c12fa 100644
> --- a/fs/afs/file.c
> +++ b/fs/afs/file.c
> @@ -453,7 +453,7 @@ static void afs_invalidate_dirty(struct folio *folio, size_t offset,
>
> undirty:
> trace_afs_folio_dirty(vnode, tracepoint_string("undirty"), folio);
> - folio_clear_dirty_for_io(folio);
> + folio_clear_dirty_for_io(NULL, folio);
> full_invalidate:
> trace_afs_folio_dirty(vnode, tracepoint_string("inval"), folio);
> folio_detach_private(folio);
> diff --git a/fs/afs/write.c b/fs/afs/write.c
> index 19df10d63323..9a5e6d59040c 100644
> --- a/fs/afs/write.c
> +++ b/fs/afs/write.c
> @@ -555,7 +555,7 @@ static void afs_extend_writeback(struct address_space *mapping,
> folio = page_folio(pvec.pages[i]);
> trace_afs_folio_dirty(vnode, tracepoint_string("store+"), folio);
>
> - if (!folio_clear_dirty_for_io(folio))
> + if (!folio_clear_dirty_for_io(NULL, folio))
> BUG();
> if (folio_start_writeback(folio))
> BUG();
> @@ -769,7 +769,7 @@ static int afs_writepages_region(struct address_space *mapping,
> continue;
> }
>
> - if (!folio_clear_dirty_for_io(folio))
> + if (!folio_clear_dirty_for_io(NULL, folio))
> BUG();
> ret = afs_write_back_from_locked_folio(mapping, wbc, folio, start, end);
> folio_put(folio);
> @@ -1000,7 +1000,7 @@ int afs_launder_folio(struct folio *folio)
> _enter("{%lx}", folio->index);
>
> priv = (unsigned long)folio_get_private(folio);
> - if (folio_clear_dirty_for_io(folio)) {
> + if (folio_clear_dirty_for_io(NULL, folio)) {
> f = 0;
> t = folio_size(folio);
> if (folio_test_private(folio)) {
> diff --git a/fs/btrfs/extent_io.c b/fs/btrfs/extent_io.c
> index 9bd32daa9b9a..2026f567cbdd 100644
> --- a/fs/btrfs/extent_io.c
> +++ b/fs/btrfs/extent_io.c
> @@ -215,7 +215,7 @@ void extent_range_clear_dirty_for_io(struct inode *inode, u64 start, u64 end)
> while (index <= end_index) {
> page = find_get_page(inode->i_mapping, index);
> BUG_ON(!page); /* Pages should be in the extent_io_tree */
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> put_page(page);
> index++;
> }
> @@ -2590,7 +2590,7 @@ static int write_one_subpage_eb(struct extent_buffer *eb,
> no_dirty_ebs = btrfs_subpage_clear_and_test_dirty(fs_info, page,
> eb->start, eb->len);
> if (no_dirty_ebs)
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
>
> bio_ctrl->end_io_func = end_bio_subpage_eb_writepage;
>
> @@ -2633,7 +2633,7 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
> for (i = 0; i < num_pages; i++) {
> struct page *p = eb->pages[i];
>
> - clear_page_dirty_for_io(p);
> + clear_page_dirty_for_io(NULL, p);
> set_page_writeback(p);
> ret = submit_extent_page(REQ_OP_WRITE | write_flags, wbc,
> bio_ctrl, disk_bytenr, p,
> @@ -2655,7 +2655,7 @@ static noinline_for_stack int write_one_eb(struct extent_buffer *eb,
> if (unlikely(ret)) {
> for (; i < num_pages; i++) {
> struct page *p = eb->pages[i];
> - clear_page_dirty_for_io(p);
> + clear_page_dirty_for_io(NULL, p);
> unlock_page(p);
> }
> }
> @@ -3083,7 +3083,7 @@ static int extent_write_cache_pages(struct address_space *mapping,
> }
>
> if (PageWriteback(page) ||
> - !clear_page_dirty_for_io(page)) {
> + !clear_page_dirty_for_io(wbc, page)) {
> unlock_page(page);
> continue;
> }
> @@ -3174,7 +3174,7 @@ int extent_write_locked_range(struct inode *inode, u64 start, u64 end)
> */
> ASSERT(PageLocked(page));
> ASSERT(PageDirty(page));
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> ret = __extent_writepage(page, &wbc_writepages, &bio_ctrl);
> ASSERT(ret <= 0);
> if (ret < 0) {
> @@ -4698,7 +4698,7 @@ static void btree_clear_page_dirty(struct page *page)
> {
> ASSERT(PageDirty(page));
> ASSERT(PageLocked(page));
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> xa_lock_irq(&page->mapping->i_pages);
> if (!PageDirty(page))
> __xa_clear_mark(&page->mapping->i_pages,
> diff --git a/fs/btrfs/free-space-cache.c b/fs/btrfs/free-space-cache.c
> index 0d250d052487..02ec9987fc17 100644
> --- a/fs/btrfs/free-space-cache.c
> +++ b/fs/btrfs/free-space-cache.c
> @@ -507,7 +507,7 @@ static int io_ctl_prepare_pages(struct btrfs_io_ctl *io_ctl, bool uptodate)
> }
>
> for (i = 0; i < io_ctl->num_pages; i++)
> - clear_page_dirty_for_io(io_ctl->pages[i]);
> + clear_page_dirty_for_io(NULL, io_ctl->pages[i]);
>
> return 0;
> }
> diff --git a/fs/btrfs/inode.c b/fs/btrfs/inode.c
> index 98a800b8bd43..26820c697c5b 100644
> --- a/fs/btrfs/inode.c
> +++ b/fs/btrfs/inode.c
> @@ -3002,7 +3002,7 @@ static void btrfs_writepage_fixup_worker(struct btrfs_work *work)
> */
> mapping_set_error(page->mapping, ret);
> end_extent_writepage(page, ret, page_start, page_end);
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> SetPageError(page);
> }
> btrfs_page_clear_checked(inode->root->fs_info, page, page_start, PAGE_SIZE);
> diff --git a/fs/btrfs/subpage.c b/fs/btrfs/subpage.c
> index dd46b978ac2c..f85b5a2ccdab 100644
> --- a/fs/btrfs/subpage.c
> +++ b/fs/btrfs/subpage.c
> @@ -515,7 +515,7 @@ void btrfs_subpage_clear_dirty(const struct btrfs_fs_info *fs_info,
>
> last = btrfs_subpage_clear_and_test_dirty(fs_info, page, start, len);
> if (last)
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> }
>
> void btrfs_subpage_set_writeback(const struct btrfs_fs_info *fs_info,
> diff --git a/fs/ceph/addr.c b/fs/ceph/addr.c
> index cac4083e387a..ff940c9cd1cf 100644
> --- a/fs/ceph/addr.c
> +++ b/fs/ceph/addr.c
> @@ -926,7 +926,7 @@ static int ceph_writepages_start(struct address_space *mapping,
> folio->index, ceph_wbc.i_size);
> if ((ceph_wbc.size_stable ||
> folio_pos(folio) >= i_size_read(inode)) &&
> - folio_clear_dirty_for_io(folio))
> + folio_clear_dirty_for_io(wbc, folio))
> folio_invalidate(folio, 0,
> folio_size(folio));
> folio_unlock(folio);
> @@ -948,7 +948,7 @@ static int ceph_writepages_start(struct address_space *mapping,
> wait_on_page_fscache(page);
> }
>
> - if (!clear_page_dirty_for_io(page)) {
> + if (!clear_page_dirty_for_io(wbc, page)) {
> dout("%p !clear_page_dirty_for_io\n", page);
> unlock_page(page);
> continue;
> @@ -1282,7 +1282,7 @@ ceph_find_incompatible(struct page *page)
>
> /* yay, writeable, do it now (without dropping page lock) */
> dout(" page %p snapc %p not current, but oldest\n", page, snapc);
> - if (clear_page_dirty_for_io(page)) {
> + if (clear_page_dirty_for_io(NULL, page)) {
> int r = writepage_nounlock(page, NULL);
> if (r < 0)
> return ERR_PTR(r);
> diff --git a/fs/cifs/file.c b/fs/cifs/file.c
> index 22dfc1f8b4f1..93e36829896e 100644
> --- a/fs/cifs/file.c
> +++ b/fs/cifs/file.c
> @@ -2342,7 +2342,7 @@ cifs_writev_requeue(struct cifs_writedata *wdata)
> for (j = 0; j < nr_pages; j++) {
> wdata2->pages[j] = wdata->pages[i + j];
> lock_page(wdata2->pages[j]);
> - clear_page_dirty_for_io(wdata2->pages[j]);
> + clear_page_dirty_for_io(NULL, wdata2->pages[j]);
> }
>
> wdata2->sync_mode = wdata->sync_mode;
> @@ -2582,7 +2582,7 @@ wdata_prepare_pages(struct cifs_writedata *wdata, unsigned int found_pages,
> wait_on_page_writeback(page);
>
> if (PageWriteback(page) ||
> - !clear_page_dirty_for_io(page)) {
> + !clear_page_dirty_for_io(wbc, page)) {
> unlock_page(page);
> break;
> }
> @@ -5076,7 +5076,7 @@ static int cifs_launder_folio(struct folio *folio)
>
> cifs_dbg(FYI, "Launder page: %lu\n", folio->index);
>
> - if (folio_clear_dirty_for_io(folio))
> + if (folio_clear_dirty_for_io(&wbc, folio))
> rc = cifs_writepage_locked(&folio->page, &wbc);
>
> folio_wait_fscache(folio);
> diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
> index 46078651ce32..7082b6ba8e12 100644
> --- a/fs/ext4/inode.c
> +++ b/fs/ext4/inode.c
> @@ -1616,7 +1616,7 @@ static void mpage_release_unused_pages(struct mpage_da_data *mpd,
> BUG_ON(folio_test_writeback(folio));
> if (invalidate) {
> if (folio_mapped(folio))
> - folio_clear_dirty_for_io(folio);
> + folio_clear_dirty_for_io(NULL, folio);
> block_invalidate_folio(folio, 0,
> folio_size(folio));
> folio_clear_uptodate(folio);
> @@ -2106,7 +2106,7 @@ static int mpage_submit_page(struct mpage_da_data *mpd, struct page *page)
> int err;
>
> BUG_ON(page->index != mpd->first_page);
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> /*
> * We have to be very careful here! Nothing protects writeback path
> * against i_size changes and the page can be writeably mapped into
> diff --git a/fs/f2fs/checkpoint.c b/fs/f2fs/checkpoint.c
> index 56f7d0d6a8b2..37f951b11153 100644
> --- a/fs/f2fs/checkpoint.c
> +++ b/fs/f2fs/checkpoint.c
> @@ -435,7 +435,7 @@ long f2fs_sync_meta_pages(struct f2fs_sb_info *sbi, enum page_type type,
>
> f2fs_wait_on_page_writeback(page, META, true, true);
>
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(NULL, page))
> goto continue_unlock;
>
> if (__f2fs_write_meta_page(page, &wbc, io_type)) {
> @@ -1415,7 +1415,7 @@ static void commit_checkpoint(struct f2fs_sb_info *sbi,
> memcpy(page_address(page), src, PAGE_SIZE);
>
> set_page_dirty(page);
> - if (unlikely(!clear_page_dirty_for_io(page)))
> + if (unlikely(!clear_page_dirty_for_io(NULL, page)))
> f2fs_bug_on(sbi, 1);
>
> /* writeout cp pack 2 page */
> diff --git a/fs/f2fs/compress.c b/fs/f2fs/compress.c
> index 2532f369cb10..efd09e280b2c 100644
> --- a/fs/f2fs/compress.c
> +++ b/fs/f2fs/compress.c
> @@ -1459,7 +1459,7 @@ static int f2fs_write_raw_pages(struct compress_ctx *cc,
> if (!PageDirty(cc->rpages[i]))
> goto continue_unlock;
>
> - if (!clear_page_dirty_for_io(cc->rpages[i]))
> + if (!clear_page_dirty_for_io(NULL, cc->rpages[i]))
> goto continue_unlock;
>
> ret = f2fs_write_single_data_page(cc->rpages[i], &_submitted,
> diff --git a/fs/f2fs/data.c b/fs/f2fs/data.c
> index 97e816590cd9..f1d622b64690 100644
> --- a/fs/f2fs/data.c
> +++ b/fs/f2fs/data.c
> @@ -3102,7 +3102,7 @@ static int f2fs_write_cache_pages(struct address_space *mapping,
> goto continue_unlock;
> }
>
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(NULL, page))
> goto continue_unlock;
>
> #ifdef CONFIG_F2FS_FS_COMPRESSION
> diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c
> index 8e025157f35c..73005e711b83 100644
> --- a/fs/f2fs/dir.c
> +++ b/fs/f2fs/dir.c
> @@ -938,7 +938,7 @@ void f2fs_delete_entry(struct f2fs_dir_entry *dentry, struct page *page,
> if (bit_pos == NR_DENTRY_IN_BLOCK &&
> !f2fs_truncate_hole(dir, page->index, page->index + 1)) {
> f2fs_clear_page_cache_dirty_tag(page);
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> ClearPageUptodate(page);
>
> clear_page_private_gcing(page);
> diff --git a/fs/f2fs/gc.c b/fs/f2fs/gc.c
> index 6e2cae3d2e71..1f647287e3eb 100644
> --- a/fs/f2fs/gc.c
> +++ b/fs/f2fs/gc.c
> @@ -1361,7 +1361,7 @@ static int move_data_block(struct inode *inode, block_t bidx,
> f2fs_invalidate_compress_page(fio.sbi, fio.old_blkaddr);
>
> set_page_dirty(fio.encrypted_page);
> - if (clear_page_dirty_for_io(fio.encrypted_page))
> + if (clear_page_dirty_for_io(NULL, fio.encrypted_page))
> dec_page_count(fio.sbi, F2FS_DIRTY_META);
>
> set_page_writeback(fio.encrypted_page);
> @@ -1446,7 +1446,7 @@ static int move_data_page(struct inode *inode, block_t bidx, int gc_type,
> f2fs_wait_on_page_writeback(page, DATA, true, true);
>
> set_page_dirty(page);
> - if (clear_page_dirty_for_io(page)) {
> + if (clear_page_dirty_for_io(NULL, page)) {
> inode_dec_dirty_pages(inode);
> f2fs_remove_dirty_inode(inode);
> }
> diff --git a/fs/f2fs/inline.c b/fs/f2fs/inline.c
> index 21a495234ffd..2bfade0ead67 100644
> --- a/fs/f2fs/inline.c
> +++ b/fs/f2fs/inline.c
> @@ -170,7 +170,7 @@ int f2fs_convert_inline_page(struct dnode_of_data *dn, struct page *page)
> set_page_dirty(page);
>
> /* clear dirty state */
> - dirty = clear_page_dirty_for_io(page);
> + dirty = clear_page_dirty_for_io(NULL, page);
>
> /* write data page to try to make data consistent */
> set_page_writeback(page);
> diff --git a/fs/f2fs/node.c b/fs/f2fs/node.c
> index dde4c0458704..6f5571cac2b3 100644
> --- a/fs/f2fs/node.c
> +++ b/fs/f2fs/node.c
> @@ -124,7 +124,7 @@ static void clear_node_page_dirty(struct page *page)
> {
> if (PageDirty(page)) {
> f2fs_clear_page_cache_dirty_tag(page);
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> dec_page_count(F2FS_P_SB(page), F2FS_DIRTY_NODES);
> }
> ClearPageUptodate(page);
> @@ -1501,7 +1501,7 @@ static void flush_inline_data(struct f2fs_sb_info *sbi, nid_t ino)
> if (!PageDirty(page))
> goto page_out;
>
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(NULL, page))
> goto page_out;
>
> ret = f2fs_write_inline_data(inode, page);
> @@ -1696,7 +1696,7 @@ int f2fs_move_node_page(struct page *node_page, int gc_type)
>
> set_page_dirty(node_page);
>
> - if (!clear_page_dirty_for_io(node_page)) {
> + if (!clear_page_dirty_for_io(NULL, node_page)) {
> err = -EAGAIN;
> goto out_page;
> }
> @@ -1803,7 +1803,7 @@ int f2fs_fsync_node_pages(struct f2fs_sb_info *sbi, struct inode *inode,
> set_page_dirty(page);
> }
>
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(NULL, page))
> goto continue_unlock;
>
> ret = __write_node_page(page, atomic &&
> @@ -2011,7 +2011,7 @@ int f2fs_sync_node_pages(struct f2fs_sb_info *sbi,
> write_node:
> f2fs_wait_on_page_writeback(page, NODE, true, true);
>
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(NULL, page))
> goto continue_unlock;
>
> set_fsync_mark(page, 0);
> diff --git a/fs/fuse/file.c b/fs/fuse/file.c
> index 875314ee6f59..cb9561128b4b 100644
> --- a/fs/fuse/file.c
> +++ b/fs/fuse/file.c
> @@ -2399,7 +2399,7 @@ static int fuse_write_end(struct file *file, struct address_space *mapping,
> static int fuse_launder_folio(struct folio *folio)
> {
> int err = 0;
> - if (folio_clear_dirty_for_io(folio)) {
> + if (folio_clear_dirty_for_io(NULL, folio)) {
> struct inode *inode = folio->mapping->host;
>
> /* Serialize with pending writeback for the same page */
> diff --git a/fs/gfs2/aops.c b/fs/gfs2/aops.c
> index e782b4f1d104..cf784dd5fd3b 100644
> --- a/fs/gfs2/aops.c
> +++ b/fs/gfs2/aops.c
> @@ -247,7 +247,7 @@ static int gfs2_write_jdata_pagevec(struct address_space *mapping,
> }
>
> BUG_ON(PageWriteback(page));
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(wbc, page))
> goto continue_unlock;
>
> trace_wbc_writepage(wbc, inode_to_bdi(inode));
> diff --git a/fs/nfs/write.c b/fs/nfs/write.c
> index 80c240e50952..c07b686c84ce 100644
> --- a/fs/nfs/write.c
> +++ b/fs/nfs/write.c
> @@ -2089,7 +2089,7 @@ int nfs_wb_page(struct inode *inode, struct page *page)
>
> for (;;) {
> wait_on_page_writeback(page);
> - if (clear_page_dirty_for_io(page)) {
> + if (clear_page_dirty_for_io(&wbc, page)) {
> ret = nfs_writepage_locked(page, &wbc);
> if (ret < 0)
> goto out_error;
> diff --git a/fs/nilfs2/page.c b/fs/nilfs2/page.c
> index 39b7eea2642a..9c3cc20b446e 100644
> --- a/fs/nilfs2/page.c
> +++ b/fs/nilfs2/page.c
> @@ -456,7 +456,7 @@ int __nilfs_clear_page_dirty(struct page *page)
> __xa_clear_mark(&mapping->i_pages, page_index(page),
> PAGECACHE_TAG_DIRTY);
> xa_unlock_irq(&mapping->i_pages);
> - return clear_page_dirty_for_io(page);
> + return clear_page_dirty_for_io(NULL, page);
> }
> xa_unlock_irq(&mapping->i_pages);
> return 0;
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index 76c3bd88b858..123494739030 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -1644,7 +1644,7 @@ static void nilfs_begin_page_io(struct page *page)
> return;
>
> lock_page(page);
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> set_page_writeback(page);
> unlock_page(page);
> }
> @@ -1662,7 +1662,7 @@ static void nilfs_segctor_prepare_write(struct nilfs_sc_info *sci)
> if (bh->b_page != bd_page) {
> if (bd_page) {
> lock_page(bd_page);
> - clear_page_dirty_for_io(bd_page);
> + clear_page_dirty_for_io(NULL, bd_page);
> set_page_writeback(bd_page);
> unlock_page(bd_page);
> }
> @@ -1676,7 +1676,7 @@ static void nilfs_segctor_prepare_write(struct nilfs_sc_info *sci)
> if (bh == segbuf->sb_super_root) {
> if (bh->b_page != bd_page) {
> lock_page(bd_page);
> - clear_page_dirty_for_io(bd_page);
> + clear_page_dirty_for_io(NULL, bd_page);
> set_page_writeback(bd_page);
> unlock_page(bd_page);
> bd_page = bh->b_page;
> @@ -1691,7 +1691,7 @@ static void nilfs_segctor_prepare_write(struct nilfs_sc_info *sci)
> }
> if (bd_page) {
> lock_page(bd_page);
> - clear_page_dirty_for_io(bd_page);
> + clear_page_dirty_for_io(NULL, bd_page);
> set_page_writeback(bd_page);
> unlock_page(bd_page);
> }
> diff --git a/fs/orangefs/inode.c b/fs/orangefs/inode.c
> index 4df560894386..829de5553a77 100644
> --- a/fs/orangefs/inode.c
> +++ b/fs/orangefs/inode.c
> @@ -501,7 +501,7 @@ static int orangefs_launder_folio(struct folio *folio)
> .nr_to_write = 0,
> };
> folio_wait_writeback(folio);
> - if (folio_clear_dirty_for_io(folio)) {
> + if (folio_clear_dirty_for_io(&wbc, folio)) {
> r = orangefs_writepage_locked(&folio->page, &wbc);
> folio_end_writeback(folio);
> }
> diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
> index f2353dd676ef..41d764fd5511 100644
> --- a/fs/ubifs/file.c
> +++ b/fs/ubifs/file.c
> @@ -1159,7 +1159,7 @@ static int do_truncation(struct ubifs_info *c, struct inode *inode,
> */
> ubifs_assert(c, PagePrivate(page));
>
> - clear_page_dirty_for_io(page);
> + clear_page_dirty_for_io(NULL, page);
> if (UBIFS_BLOCKS_PER_PAGE_SHIFT)
> offset = new_size &
> (PAGE_SIZE - 1);
> diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
> index 29e1f9e76eb6..81c42a95cf8d 100644
> --- a/include/linux/pagemap.h
> +++ b/include/linux/pagemap.h
> @@ -1059,8 +1059,9 @@ static inline void folio_cancel_dirty(struct folio *folio)
> if (folio_test_dirty(folio))
> __folio_cancel_dirty(folio);
> }
> -bool folio_clear_dirty_for_io(struct folio *folio);
> -bool clear_page_dirty_for_io(struct page *page);
> +bool folio_clear_dirty_for_io(struct writeback_control *wbc,
> + struct folio *folio);
> +bool clear_page_dirty_for_io(struct writeback_control *wbc, struct page *page);
> void folio_invalidate(struct folio *folio, size_t offset, size_t length);
> int __must_check folio_write_one(struct folio *folio);
> static inline int __must_check write_one_page(struct page *page)
> diff --git a/mm/folio-compat.c b/mm/folio-compat.c
> index 69ed25790c68..748f82def674 100644
> --- a/mm/folio-compat.c
> +++ b/mm/folio-compat.c
> @@ -63,9 +63,9 @@ int __set_page_dirty_nobuffers(struct page *page)
> }
> EXPORT_SYMBOL(__set_page_dirty_nobuffers);
>
> -bool clear_page_dirty_for_io(struct page *page)
> +bool clear_page_dirty_for_io(struct writeback_control *wbc, struct page *page)
> {
> - return folio_clear_dirty_for_io(page_folio(page));
> + return folio_clear_dirty_for_io(wbc, page_folio(page));
> }
> EXPORT_SYMBOL(clear_page_dirty_for_io);
>
> diff --git a/mm/migrate.c b/mm/migrate.c
> index a4d3fc65085f..0bda652153b9 100644
> --- a/mm/migrate.c
> +++ b/mm/migrate.c
> @@ -870,7 +870,7 @@ static int writeout(struct address_space *mapping, struct folio *folio)
> /* No write method for the address space */
> return -EINVAL;
>
> - if (!folio_clear_dirty_for_io(folio))
> + if (!folio_clear_dirty_for_io(&wbc, folio))
> /* Someone else already triggered a write */
> return -EAGAIN;
>
> diff --git a/mm/page-writeback.c b/mm/page-writeback.c
> index ad608ef2a243..2d70070e533c 100644
> --- a/mm/page-writeback.c
> +++ b/mm/page-writeback.c
> @@ -2465,7 +2465,7 @@ int write_cache_pages(struct address_space *mapping,
> }
>
> BUG_ON(PageWriteback(page));
> - if (!clear_page_dirty_for_io(page))
> + if (!clear_page_dirty_for_io(wbc, page))
> goto continue_unlock;
>
> trace_wbc_writepage(wbc, inode_to_bdi(mapping->host));
> @@ -2628,7 +2628,7 @@ int folio_write_one(struct folio *folio)
>
> folio_wait_writeback(folio);
>
> - if (folio_clear_dirty_for_io(folio)) {
> + if (folio_clear_dirty_for_io(&wbc, folio)) {
> folio_get(folio);
> ret = mapping->a_ops->writepage(&folio->page, &wbc);
> if (ret == 0)
> @@ -2924,7 +2924,7 @@ EXPORT_SYMBOL(__folio_cancel_dirty);
>
> /*
> * Clear a folio's dirty flag, while caring for dirty memory accounting.
> - * Returns true if the folio was previously dirty.
> + * Returns true if the folio was previously dirty and should be written back.
> *
> * This is for preparing to put the folio under writeout. We leave
> * the folio tagged as dirty in the xarray so that a concurrent
> @@ -2935,8 +2935,14 @@ EXPORT_SYMBOL(__folio_cancel_dirty);
> *
> * This incoherency between the folio's dirty flag and xarray tag is
> * unfortunate, but it only exists while the folio is locked.
> + *
> + * If the folio is pinned, its writeback is problematic so we just don't bother
> + * for memory cleaning writeback - this is why writeback control is passed in.
> + * If it is NULL, we assume pinned pages are not expected (e.g. this can be
> + * a metadata page) and warn if the page is actually pinned.
> */
> -bool folio_clear_dirty_for_io(struct folio *folio)
> +bool folio_clear_dirty_for_io(struct writeback_control *wbc,
> + struct folio *folio)
> {
> struct address_space *mapping = folio_mapping(folio);
> bool ret = false;
> @@ -2975,6 +2981,16 @@ bool folio_clear_dirty_for_io(struct folio *folio)
> */
> if (folio_mkclean(folio))
> folio_mark_dirty(folio);
> + /*
> + * For pinned folios we have no chance to reclaim them anyway
> + * and we cannot be sure folio is ever clean. So memory
> + * cleaning writeback is pointless. Just skip it.
> + */
> + if (wbc && wbc->sync_mode == WB_SYNC_NONE &&
> + folio_maybe_dma_pinned(folio))
> + return false;
> + /* Catch callers not expecting pinned pages */
> + WARN_ON_ONCE(!wbc && folio_maybe_dma_pinned(folio));
> /*
> * We carefully synchronise fault handlers against
> * installing a dirty pte and marking the folio dirty
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index ab3911a8b116..71a226b47ac6 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -1283,7 +1283,7 @@ static pageout_t pageout(struct folio *folio, struct address_space *mapping,
> if (mapping->a_ops->writepage == NULL)
> return PAGE_ACTIVATE;
>
> - if (folio_clear_dirty_for_io(folio)) {
> + if (folio_clear_dirty_for_io(NULL, folio)) {
> int res;
> struct writeback_control wbc = {
> .sync_mode = WB_SYNC_NONE,
next prev parent reply other threads:[~2023-02-10 1:54 UTC|newest]
Thread overview: 26+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-02-09 12:31 [PATCH RFC 0/5] Writeback handling of pinned pages Jan Kara
2023-02-09 12:31 ` [PATCH 1/5] mm: Do not reclaim private data from pinned page Jan Kara
2023-02-09 16:17 ` Matthew Wilcox
2023-02-10 11:29 ` Jan Kara
2023-02-13 9:55 ` Christoph Hellwig
2023-02-14 13:06 ` Jan Kara
2023-02-14 21:40 ` John Hubbard
2023-02-16 11:56 ` Jan Kara
2023-02-13 9:01 ` David Hildenbrand
2023-02-14 13:00 ` Jan Kara
2023-02-09 12:31 ` [PATCH 2/5] ext4: Drop workaround for mm reclaiming fs private page data Jan Kara
2023-02-09 12:31 ` [PATCH 3/5] mm: Do not try to write pinned folio during memory cleaning writeback Jan Kara
2023-02-10 1:54 ` John Hubbard [this message]
2023-02-10 2:10 ` John Hubbard
2023-02-10 10:42 ` Jan Kara
2023-02-10 10:54 ` Jan Kara
2023-02-09 12:31 ` [PATCH 4/5] block: Add support for bouncing pinned pages Jan Kara
2023-02-13 9:59 ` Christoph Hellwig
2023-02-14 13:56 ` Jan Kara
2023-02-15 4:59 ` Dave Chinner
2023-02-15 6:24 ` Christoph Hellwig
2023-02-16 12:33 ` Jan Kara
2023-02-20 6:22 ` Christoph Hellwig
2023-02-27 11:39 ` Jan Kara
2023-02-27 13:36 ` Christoph Hellwig
2023-02-09 12:31 ` [PATCH 5/5] iomap: Bounce pinned pages during writeback Jan Kara
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=4961eb2d-c36b-d6a5-6a43-0c35d24606c0@nvidia.com \
--to=jhubbard@nvidia.com \
--cc=david@redhat.com \
--cc=dhowells@redhat.com \
--cc=jack@suse.cz \
--cc=linux-block@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-mm@kvack.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