From patchwork Fri Feb 24 14:48:03 2023 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: David Howells X-Patchwork-Id: 13151304 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 8B701C61DA4 for ; Fri, 24 Feb 2023 14:48:12 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 119326B0071; Fri, 24 Feb 2023 09:48:12 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 0C98E6B0073; Fri, 24 Feb 2023 09:48:12 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id EAB616B0074; Fri, 24 Feb 2023 09:48:11 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0017.hostedemail.com [216.40.44.17]) by kanga.kvack.org (Postfix) with ESMTP id D7DDC6B0071 for ; Fri, 24 Feb 2023 09:48:11 -0500 (EST) Received: from smtpin05.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id 9C67B404A9 for ; Fri, 24 Feb 2023 14:48:11 +0000 (UTC) X-FDA: 80502465582.05.8317CCE Received: from us-smtp-delivery-124.mimecast.com (us-smtp-delivery-124.mimecast.com [170.10.133.124]) by imf21.hostedemail.com (Postfix) with ESMTP id A71E81C0014 for ; Fri, 24 Feb 2023 14:48:09 +0000 (UTC) Authentication-Results: imf21.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=fIrCMLn3; spf=pass (imf21.hostedemail.com: domain of dhowells@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1677250089; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references:dkim-signature; bh=r3Iwur3DaO9qOJlnUw5ODrEZJsIUBj4dSLzn2wwaUV8=; b=J6aZ6fNoNZgyW9RMAS1SpkqBHAqj+kQotJnkFJ8BZw7bz72Wtb6oBMGzoZAIb9UdXGSKzX QBY4oeHgdCB7HdPxJ+78GSv2xUE1YD2cmux78WzyDtiFT1fBLY7nGrYleDHqN8si2ss1oO tB8WqqsI/qIzH8HhRWIi+M4XoP8iqOQ= ARC-Authentication-Results: i=1; imf21.hostedemail.com; dkim=pass header.d=redhat.com header.s=mimecast20190719 header.b=fIrCMLn3; spf=pass (imf21.hostedemail.com: domain of dhowells@redhat.com designates 170.10.133.124 as permitted sender) smtp.mailfrom=dhowells@redhat.com; dmarc=pass (policy=none) header.from=redhat.com ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1677250089; a=rsa-sha256; cv=none; b=ScjSi0D5NSF3OU4H2cmQK5Ag422yBB5OfELJGOtyg75XYL9HsOzZUnahaLTACBVzeh+0W4 AmMmoU4zT7z0AaJVBv0EhhP1wvBLpWUUV1OfCeVgym5xqFGA2dxktKETa+JNplsFNB7BCf QcN1V8ti5LTbOZCa5WJdkJ74L2Typys= DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1677250089; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: content-transfer-encoding:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=r3Iwur3DaO9qOJlnUw5ODrEZJsIUBj4dSLzn2wwaUV8=; b=fIrCMLn3ol/YTF7g5oQjMempzn3dVGO7feVuHEGVLtlY70a5jnf765W3hxu6DTVxi4QNjK ZcdNt+Qe/RPYBK1D8K+Vmi5hteRxGzcab6COFCpa/PklFqh0NRt/3t376KLSNdvSCfrgNn 6nkz2i6V788aUBKmUsdquuPbo+GV9FY= Received: from mimecast-mx02.redhat.com (mx3-rdu2.redhat.com [66.187.233.73]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.2, cipher=TLS_ECDHE_RSA_WITH_AES_256_GCM_SHA384) id us-mta-346-p9_Us-HwPMethNlGGb1Kzg-1; Fri, 24 Feb 2023 09:48:07 -0500 X-MC-Unique: p9_Us-HwPMethNlGGb1Kzg-1 Received: from smtp.corp.redhat.com (int-mx06.intmail.prod.int.rdu2.redhat.com [10.11.54.6]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx02.redhat.com (Postfix) with ESMTPS id 7677A3C0E213; Fri, 24 Feb 2023 14:48:06 +0000 (UTC) Received: from warthog.procyon.org.uk (unknown [10.33.36.18]) by smtp.corp.redhat.com (Postfix) with ESMTP id 3DF552166B29; Fri, 24 Feb 2023 14:48:04 +0000 (UTC) Organization: Red Hat UK Ltd. Registered Address: Red Hat UK Ltd, Amberley Place, 107-111 Peascod Street, Windsor, Berkshire, SI4 1TE, United Kingdom. Registered in England and Wales under Company Registration No. 3798903 From: David Howells In-Reply-To: <2134430.1677240738@warthog.procyon.org.uk> References: <2134430.1677240738@warthog.procyon.org.uk> <2009825.1677229488@warthog.procyon.org.uk> <20230220135225.91b0f28344c01d5306c31230@linux-foundation.org> To: Linus Torvalds , Steve French Cc: dhowells@redhat.com, Vishal Moola , Andrew Morton , Jan Kara , Paulo Alcantara , Matthew Wilcox , Huang Ying , Baolin Wang , Xin Hao , linux-mm@kvack.org, mm-commits@vger.kernel.org, linux-kernel@vger.kernel.org Subject: [RFC][PATCH] cifs, afs: Revert changes to {cifs,afs}_writepages_region() MIME-Version: 1.0 Content-ID: <2214156.1677250083.1@warthog.procyon.org.uk> Date: Fri, 24 Feb 2023 14:48:03 +0000 Message-ID: <2214157.1677250083@warthog.procyon.org.uk> X-Scanned-By: MIMEDefang 3.1 on 10.11.54.6 X-Rspam-User: X-Rspamd-Server: rspam04 X-Rspamd-Queue-Id: A71E81C0014 X-Stat-Signature: nr7sgiyzdxb3syp9fssb7oi9xbz7nsmh X-HE-Tag: 1677250089-685943 X-HE-Meta: U2FsdGVkX193xIQN/c2q9MU+YFUDEIRL/Irx2u1TeqeNxm00MFJxBr/hhIOZxeoUEL5xAep3SgcM0xHAmulXaVRv0SPZ8SYp40xzDj3yUN67e2wQXSayhtIkqYkKITwYAABD5GtWbboiQc6n/nQKtLJ+cCHOgHu89SRilECDAOlkQWA+VlIuoAnymV4oxyNGCuEEBR9L8EA7HXOO55RZss3xygDV9XvuCqWed8K9wXxtMC94YJQyJAglQZb1K2FgfIz5ugaoLVjls9oNwp2y/Vg0LwUUTz6a2sJPMJihCzRazaXtKHUE3WI66r9fnlYAmn7FBGhA8xH5e49hBxc8r6oIqbvlmfvU3ZIyAij8H9w2hjvsYDOWt1dGvNPFFt6DP9KjU+zxJbM1PjplInk7FlZ6UP5B87/at3j86b26q7gSDxBsIr3w3XV6iChPyKzF+xKbVVK7auowCX/CDgHI48J3TlwbECwZKvvokTEMFrNc1UtobC8o5OO4bxwdLeVv/rEYvN2QUTzHpu6V7PDqycJeCMsuzn5bxOZiwraO+h7Ofwow6DZNlzLpdyLsHtEKce1GtqRMXXkKFowMXKE1op39QBEIU3XVJim23eJ9kEN+OfO3DIFaa+xN6pyq9J3o5n4KZNnwvXu+9fhmUgOpxeCwEtZH0tKikkyPo0VAw4HWYO1JZeyx5bv7ClZtVpHqCQDcFZqvAu5usFrhYSMVQZ69Jnx8yq+al+7apHcPC5r21HJtIo+o0x6Y2X7OtFR6d9t8WYEQ8p616bJB0bfw4jnvN4wMySdLZm3otcj5veijKB4QItxsfR/btIhiibKhD/K8fwS+UHD2yBtRyF5km6J+Dco5CnIuRAOa0ILFURXCZffSxn/H7kJmTasEPt17SJmnd/V/3SvqhI6BnRvGMJE/tTkm9YzH5JTnHrruICFsgZu5zxN4+PiPIhrcpJ9V38cXGWOLUsTAQfWkvlF DJOEFYl/ LXzM3DrreELFBSNNaLiBM+y7OFCAiUdsdweZROCd/ideQof8xC6dioaDVequZT/YV9ucLlVRftfOLiuorCKdGl1nGfCLmJri2nEg0X/dBXdfDfBMeAW+LguwAo6V5ghghQMsCbqqQd+GePSJxVEofQDA2fT1YlDk3dofJfbIDtkmknTrlUgkrgttb7x67yVGhFAhXRBoFnMRcybFeosh5PnPdO5NGdbyXHvQ0UdjZ06yN2uK7wv2LZjy5TyUE8dN6y/mlisyzw1PsYfEEUeEXX5Z5K0ryrQgeFoA+IPSDXwUjqyL06Hg9CvjoOBTWzS5/3h9k6e67cVqmAdL2igJZNCOQiQBGzErFn/xwXfM5zPal8RAwKfNxP9vnoA== X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: Here's a more complex patch that reverts Vishal's patch to afs and your changes to cifs back to the point where find_get_pages_range_tag() was being used to get a single folio and then replace that with a function, filemap_get_folio_tag() that just gets a single folio. An alternative way of doing this would be to make filemap_get_folios_tag() take a limit count. This is likely to be more efficient for the common case as *_extend_writeback() will deal with pages that are contiguous to the starting page before we get on to continuing to process the batch. For filemap_get_folios_tag() to be of use, the batch has to be passed down, and if it contains scattered, non-contiguous pages, these are likely to end up being pinned by the batch for significant periods of time whilst I/O is undertaken on earlier pages. Fix: 3822a7c40997 ("Merge tag 'mm-stable-2023-02-20-13-37' of git://git.kernel.org/pub/scm/linux/kernel/git/akpm/mm") Fix: acc8d8588cb7 ("afs: convert afs_writepages_region() to use filemap_get_folios_tag()") Signed-off-by: David Howells --- fs/afs/write.c | 116 +++++++++++++++++++++++------------------------- fs/cifs/file.c | 114 ++++++++++++++++++++--------------------------- include/linux/pagemap.h | 2 mm/filemap.c | 58 ++++++++++++++++++++++++ 4 files changed, 165 insertions(+), 125 deletions(-) diff --git a/fs/afs/write.c b/fs/afs/write.c index 571f3b9a417e..b04a95262c4f 100644 --- a/fs/afs/write.c +++ b/fs/afs/write.c @@ -704,87 +704,83 @@ static int afs_writepages_region(struct address_space *mapping, bool max_one_loop) { struct folio *folio; - struct folio_batch fbatch; ssize_t ret; - unsigned int i; int n, skips = 0; _enter("%llx,%llx,", start, end); - folio_batch_init(&fbatch); do { pgoff_t index = start / PAGE_SIZE; - n = filemap_get_folios_tag(mapping, &index, end / PAGE_SIZE, - PAGECACHE_TAG_DIRTY, &fbatch); - - if (!n) + folio = filemap_get_folio_tag(mapping, &index, end / PAGE_SIZE, + PAGECACHE_TAG_DIRTY); + if (!folio) break; - for (i = 0; i < n; i++) { - folio = fbatch.folios[i]; - start = folio_pos(folio); /* May regress with THPs */ - _debug("wback %lx", folio_index(folio)); + start = folio_pos(folio); /* May regress with THPs */ - /* At this point we hold neither the i_pages lock nor the - * page lock: the page may be truncated or invalidated - * (changing page->mapping to NULL), or even swizzled - * back from swapper_space to tmpfs file mapping - */ - if (wbc->sync_mode != WB_SYNC_NONE) { - ret = folio_lock_killable(folio); - if (ret < 0) { - folio_batch_release(&fbatch); - return ret; - } - } else { - if (!folio_trylock(folio)) - continue; - } + _debug("wback %lx", folio_index(folio)); - if (folio->mapping != mapping || - !folio_test_dirty(folio)) { - start += folio_size(folio); - folio_unlock(folio); - continue; + /* At this point we hold neither the i_pages lock nor the + * page lock: the page may be truncated or invalidated + * (changing page->mapping to NULL), or even swizzled + * back from swapper_space to tmpfs file mapping + */ + if (wbc->sync_mode != WB_SYNC_NONE) { + ret = folio_lock_killable(folio); + if (ret < 0) { + folio_put(folio); + return ret; + } + } else { + if (!folio_trylock(folio)) { + folio_put(folio); + return 0; } + } - if (folio_test_writeback(folio) || - folio_test_fscache(folio)) { - folio_unlock(folio); - if (wbc->sync_mode != WB_SYNC_NONE) { - folio_wait_writeback(folio); + if (folio_mapping(folio) != mapping || + !folio_test_dirty(folio)) { + start += folio_size(folio); + folio_unlock(folio); + folio_put(folio); + continue; + } + + if (folio_test_writeback(folio) || + folio_test_fscache(folio)) { + folio_unlock(folio); + if (wbc->sync_mode != WB_SYNC_NONE) { + folio_wait_writeback(folio); #ifdef CONFIG_AFS_FSCACHE - folio_wait_fscache(folio); + folio_wait_fscache(folio); #endif - } else { - start += folio_size(folio); - } - if (wbc->sync_mode == WB_SYNC_NONE) { - if (skips >= 5 || need_resched()) { - *_next = start; - _leave(" = 0 [%llx]", *_next); - return 0; - } - skips++; - } - continue; + } else { + start += folio_size(folio); } - - if (!folio_clear_dirty_for_io(folio)) - BUG(); - ret = afs_write_back_from_locked_folio(mapping, wbc, - folio, start, end); - if (ret < 0) { - _leave(" = %zd", ret); - folio_batch_release(&fbatch); - return ret; + folio_put(folio); + if (wbc->sync_mode == WB_SYNC_NONE) { + if (skips >= 5 || need_resched()) + break; + skips++; } + continue; + } - start += ret; + if (!folio_clear_dirty_for_io(folio)) + BUG(); + ret = afs_write_back_from_locked_folio(mapping, wbc, folio, start, end); + folio_put(folio); + if (ret < 0) { + _leave(" = %zd", ret); + return ret; } - folio_batch_release(&fbatch); + start += ret; + + if (max_one_loop) + break; + cond_resched(); } while (wbc->nr_to_write > 0); diff --git a/fs/cifs/file.c b/fs/cifs/file.c index 5365a3299088..121254086e30 100644 --- a/fs/cifs/file.c +++ b/fs/cifs/file.c @@ -2857,92 +2857,76 @@ static int cifs_writepages_region(struct address_space *mapping, struct writeback_control *wbc, loff_t start, loff_t end, loff_t *_next) { - struct folio_batch fbatch; + struct folio *folio; + ssize_t ret; int skips = 0; - folio_batch_init(&fbatch); do { - int nr; pgoff_t index = start / PAGE_SIZE; - nr = filemap_get_folios_tag(mapping, &index, end / PAGE_SIZE, - PAGECACHE_TAG_DIRTY, &fbatch); - if (!nr) + folio = filemap_get_folio_tag(mapping, &index, end / PAGE_SIZE, + PAGECACHE_TAG_DIRTY); + if (!folio) break; - for (int i = 0; i < nr; i++) { - ssize_t ret; - struct folio *folio = fbatch.folios[i]; - -redo_folio: - start = folio_pos(folio); /* May regress with THPs */ + start = folio_pos(folio); /* May regress with THPs */ - /* At this point we hold neither the i_pages lock nor the - * page lock: the page may be truncated or invalidated - * (changing page->mapping to NULL), or even swizzled - * back from swapper_space to tmpfs file mapping - */ - if (wbc->sync_mode != WB_SYNC_NONE) { - ret = folio_lock_killable(folio); - if (ret < 0) - goto write_error; - } else { - if (!folio_trylock(folio)) - goto skip_write; + /* At this point we hold neither the i_pages lock nor the + * page lock: the page may be truncated or invalidated + * (changing page->mapping to NULL), or even swizzled + * back from swapper_space to tmpfs file mapping + */ + if (wbc->sync_mode != WB_SYNC_NONE) { + ret = folio_lock_killable(folio); + if (ret < 0) { + folio_put(folio); + return ret; } - - if (folio_mapping(folio) != mapping || - !folio_test_dirty(folio)) { - folio_unlock(folio); - goto skip_write; + } else { + if (!folio_trylock(folio)) { + folio_put(folio); + return 0; } + } - if (folio_test_writeback(folio) || - folio_test_fscache(folio)) { - folio_unlock(folio); - if (wbc->sync_mode == WB_SYNC_NONE) - goto skip_write; + if (folio_mapping(folio) != mapping || + !folio_test_dirty(folio)) { + start += folio_size(folio); + folio_unlock(folio); + folio_put(folio); + continue; + } + if (folio_test_writeback(folio) || + folio_test_fscache(folio)) { + folio_unlock(folio); + if (wbc->sync_mode != WB_SYNC_NONE) { folio_wait_writeback(folio); #ifdef CONFIG_CIFS_FSCACHE folio_wait_fscache(folio); #endif - goto redo_folio; + } else { + start += folio_size(folio); } - - if (!folio_clear_dirty_for_io(folio)) - /* We hold the page lock - it should've been dirty. */ - WARN_ON(1); - - ret = cifs_write_back_from_locked_folio(mapping, wbc, folio, start, end); - if (ret < 0) - goto write_error; - - start += ret; - continue; - -write_error: - folio_batch_release(&fbatch); - *_next = start; - return ret; - -skip_write: - /* - * Too many skipped writes, or need to reschedule? - * Treat it as a write error without an error code. - */ - if (skips >= 5 || need_resched()) { - ret = 0; - goto write_error; + folio_put(folio); + if (wbc->sync_mode == WB_SYNC_NONE) { + if (skips >= 5 || need_resched()) + break; + skips++; } - - /* Otherwise, just skip that folio and go on to the next */ - skips++; - start += folio_size(folio); continue; } - folio_batch_release(&fbatch); + if (!folio_clear_dirty_for_io(folio)) + /* We hold the page lock - it should've been dirty. */ + WARN_ON(1); + + ret = cifs_write_back_from_locked_folio(mapping, wbc, folio, start, end); + folio_put(folio); + if (ret < 0) + return ret; + + start += ret; cond_resched(); } while (wbc->nr_to_write > 0); diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h index 0acb8e1fb7af..577535633006 100644 --- a/include/linux/pagemap.h +++ b/include/linux/pagemap.h @@ -741,6 +741,8 @@ unsigned filemap_get_folios_contig(struct address_space *mapping, pgoff_t *start, pgoff_t end, struct folio_batch *fbatch); unsigned filemap_get_folios_tag(struct address_space *mapping, pgoff_t *start, pgoff_t end, xa_mark_t tag, struct folio_batch *fbatch); +struct folio *filemap_get_folio_tag(struct address_space *mapping, pgoff_t *start, + pgoff_t end, xa_mark_t tag); struct page *grab_cache_page_write_begin(struct address_space *mapping, pgoff_t index); diff --git a/mm/filemap.c b/mm/filemap.c index 2723104cc06a..1b1e9c661018 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -2339,6 +2339,64 @@ unsigned filemap_get_folios_tag(struct address_space *mapping, pgoff_t *start, } EXPORT_SYMBOL(filemap_get_folios_tag); +/** + * filemap_get_folio_tag - Get the first folio matching @tag + * @mapping: The address_space to search + * @start: The starting page index + * @end: The final page index (inclusive) + * @tag: The tag index + * + * Search for and return the first folios in the mapping starting at index + * @start and up to index @end (inclusive). The folio is returned with an + * elevated reference count. + * + * If a folio is returned, it may start before @start; if it does, it will + * contain @start. The folio may also extend beyond @end; if it does, it will + * contain @end. If folios are added to or removed from the page cache while + * this is running, they may or may not be found by this call. + * + * Return: The folio that was found or NULL. @start is also updated to index + * the next folio for the traversal or will be left pointing after @end. + */ +struct folio *filemap_get_folio_tag(struct address_space *mapping, pgoff_t *start, + pgoff_t end, xa_mark_t tag) +{ + XA_STATE(xas, &mapping->i_pages, *start); + struct folio *folio; + + rcu_read_lock(); + while ((folio = find_get_entry(&xas, end, tag)) != NULL) { + /* + * Shadow entries should never be tagged, but this iteration + * is lockless so there is a window for page reclaim to evict + * a page we saw tagged. Skip over it. + */ + if (xa_is_value(folio)) + continue; + + if (folio_test_hugetlb(folio)) + *start = folio->index + 1; + else + *start = folio_next_index(folio); + goto out; + } + + /* + * We come here when there is no page beyond @end. We take care to not + * overflow the index @start as it confuses some of the callers. This + * breaks the iteration when there is a page at index -1 but that is + * already broke anyway. + */ + if (end == (pgoff_t)-1) + *start = (pgoff_t)-1; + else + *start = end + 1; +out: + rcu_read_unlock(); + return folio; +} +EXPORT_SYMBOL(filemap_get_folio_tag); + /* * CD/DVDs are error prone. When a medium error occurs, the driver may fail * a _large_ part of the i/o request. Imagine the worst scenario: