From mboxrd@z Thu Jan 1 00:00:00 1970 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 DC1A2EB64D9 for ; Fri, 30 Jun 2023 03:21:10 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 340778D0002; Thu, 29 Jun 2023 23:21:10 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2C9298D0001; Thu, 29 Jun 2023 23:21:10 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1694B8D0002; Thu, 29 Jun 2023 23:21:10 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0014.hostedemail.com [216.40.44.14]) by kanga.kvack.org (Postfix) with ESMTP id 034878D0001 for ; Thu, 29 Jun 2023 23:21:10 -0400 (EDT) Received: from smtpin24.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay05.hostedemail.com (Postfix) with ESMTP id C485A40746 for ; Fri, 30 Jun 2023 03:21:09 +0000 (UTC) X-FDA: 80957963058.24.285AA0F Received: from out30-118.freemail.mail.aliyun.com (out30-118.freemail.mail.aliyun.com [115.124.30.118]) by imf17.hostedemail.com (Postfix) with ESMTP id ED7C94001B for ; Fri, 30 Jun 2023 03:21:06 +0000 (UTC) Authentication-Results: imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of jefflexu@linux.alibaba.com designates 115.124.30.118 as permitted sender) smtp.mailfrom=jefflexu@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1688095268; 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; bh=qSgJwm+xI1OgCAwouxTJv7yjNknP7YEx7HFTlgh10Hs=; b=tYecuJf1O1gjaAVthLbvXCN2Ud39czlbuQRMnf2rS45+aXdqNNAqKUfo4ah2kvy9VNblk1 fp9OkctGqhcUkvH5bZR6G483rsG7ODbDnYBc8uLInWUIfHMk919sskcp3WS+qlNDdShQ4N dVwthVPg70cAikui69HHphM38Q5S4ho= ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1688095268; a=rsa-sha256; cv=none; b=3A2hyQ4hCAbTz6gnP7YRGTu+oEEdrZdTg26CgBlMc3LP/z+WfcFuJ/GpYUIq3WsaupR9Mo 8VGQvoszxHCDlizYBQtkMPvmWlkNXaAOBmcMX/gTy4ziDC6C18nnXy5Gyb/X6tCjxkZhYm bM/p7UD1lk8RDLjc28/9C+TskrjHb/s= ARC-Authentication-Results: i=1; imf17.hostedemail.com; dkim=none; spf=pass (imf17.hostedemail.com: domain of jefflexu@linux.alibaba.com designates 115.124.30.118 as permitted sender) smtp.mailfrom=jefflexu@linux.alibaba.com; dmarc=pass (policy=none) header.from=alibaba.com X-Alimail-AntiSpam:AC=PASS;BC=-1|-1;BR=01201311R111e4;CH=green;DM=||false|;DS=||;FP=0|-1|-1|-1|0|-1|-1|-1;HT=ay29a033018046059;MF=jefflexu@linux.alibaba.com;NM=1;PH=DS;RN=22;SR=0;TI=SMTPD_---0VmGYo1U_1688095261; Received: from 30.13.161.45(mailfrom:jefflexu@linux.alibaba.com fp:SMTPD_---0VmGYo1U_1688095261) by smtp.aliyun-inc.com; Fri, 30 Jun 2023 11:21:02 +0800 Message-ID: Date: Fri, 30 Jun 2023 11:20:59 +0800 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10.15; rv:102.0) Gecko/20100101 Thunderbird/102.12.0 Subject: Re: [Linux-cachefs] [PATCH v7 2/2] mm, netfs, fscache: Stop read optimisation when folio removed from pagecache Content-Language: en-US To: Xiubo Li , David Howells , Andrew Morton Cc: Shyam Prasad N , linux-cifs@vger.kernel.org, linux-nfs@vger.kernel.org, linux-mm@kvack.org, Rohith Surabattula , Linus Torvalds , Dominique Martinet , Jeff Layton , Matthew Wilcox , linux-afs@lists.infradead.org, Christoph Hellwig , Steve French , linux-cachefs@redhat.com, linux-fsdevel@vger.kernel.org, v9fs-developer@lists.sourceforge.net, Ilya Dryomov , linux-ext4@vger.kernel.org, linux-erofs@lists.ozlabs.org, ceph-devel@vger.kernel.org References: <20230628104852.3391651-1-dhowells@redhat.com> <20230628104852.3391651-3-dhowells@redhat.com> <41e1c831-29de-8494-d925-6e2eb379567f@redhat.com> From: Jingbo Xu In-Reply-To: <41e1c831-29de-8494-d925-6e2eb379567f@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Rspamd-Queue-Id: ED7C94001B X-Rspam-User: X-Stat-Signature: g5duaurcit4rm95duu5dj1jh78fhe7kk X-Rspamd-Server: rspam03 X-HE-Tag: 1688095266-969461 X-HE-Meta: U2FsdGVkX1+qY8wwZq1+96hyxwRjOmdGYKuqbs3XQywCnhzxkFMXE4hKDfQD2ddpiDbUO3iHCWdSM/oTmP90neCX2AAQtkkvLIp0rnSQI0HManRuXuGTrJNGRKZl4zMxrT7wnBDGmuSX9vHynf0kKKzmn4yDJGcCz54wJO+VFiQpe+eEWbtX/5xdX7nBg4fFW7WpPyG91O8aUydeDxiW4vgiMpNl5fKekiCX+SmyigfuGLHvBk24BTbK69CwEUvFt5cd1n2vRLntksizcQo1H4SjL4P8MxPuQCGTsMt/SUj1xCyxTyR7wPQOKz3OH/XgNpgf9FNhBUTPDxJUWhTAgo0CZk87SjiBVvbEPQ7rKt+iU4SQz89/xzrZubOf3KQ45CbY0hEe9Ne2DFBvRkbHwWm0Gy9XCUNR2yrikfWHDneMYrGBwLRDZ5D4EVEwqZUQ9Zk7/gGIru1o+rwA2/6SwK0OJSnUg/ujvf8nLe29EwkyNIN/vpziVlacAWunftwW4rAHczMDIkErq4OUZB60aHP4MnpYdlL/wUU2YyASLLY4LHYMWO/Aka6cxSCc0+l87XjWd54WaEb09qZyiS6B3op4x0ihA72GWUQ2QSeJ7qP0rMqB5iU+cr4bw3CGgE8ul7i3oO9wWnDHDBNqCuK4sku1FqMwIzm+haOuTLA2Z2KFwf2S26UL7u+hUG0aO4rc6kW7g+y1lgfnzRfD7Gcmdp5vSzxZ/S6R5VF7baGEOtPqnX6hkNT3yiGH4mSmDyZe1vJ49XduQMZbx02tQEyU7W3l+bZd8IjlecahLAT0D8jIg4LvMSVWyK7Gsm9miULV6QzEVQ1Cn0GzFYgdaTsUMNj+m1EVsRXoScQhPsQ+/4N7mNcU1+fhk/VsPTMFL9XBIFOTbVOtNCKyfhmVUGshPthgqoHkGjtDK8Zfp2JwgZoZx97T/NPbh7EA5XO5u3wOC/oOytqmzqO+vNaw0t3 4WtpX5nd df6zy8XvaFHifqytfXWJclTvku3lTXdQv0LhwJMSvXGYaieQOR/ZOHiOmwyJqkHCLn6cskAof87RToQxQrwlh+PQ8dxVMm546W+0J5i5vEZ2SL+EwAA1Y1PuxblI+T8kbupXY4X6U1zYnuBmByV6eUOZXPlsN++a71RJ3Ht9OKBb+88zp+ug9l5oKIBlfkjU8VLoOUNz/aA0K8GkLUEuHPkwQQ01/XOsNitYfHWCwLN96OvZT2eprgvyEMXFVIvCNfyIsoZBkcrr90vhyIA1VJ1RyxhC/B4WGoXgt/L5WoRENbvXkT9ZRUEcUW0nkv36k5o9GdSGP5lf+7pWhiO6hbooxdLQgnYQNhMM7bhk8YFRivFsWV1StyeCI38VaFd23ZlLWiJ4yNBVItDJzkH0CWAfd42OghsV8b8FZeE/flnTP3IHa2MGT0M/eNKtCoPDNQBnDDOjmt3SX4Nlao0YmFuYGsCT7wRPpFwjIUd0iPOgBAAGl0m1XAqcxKMsAqST8+rNGO1NsTxXiDTUnYOSHSPKJ2OmtnlmcQpkGyiovlXKhSiAq/y8+SPYdaG9MlBjM44cD1rKwF3iBUPc= 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: On 6/29/23 8:39 AM, Xiubo Li wrote: > > On 6/28/23 18:48, David Howells wrote: >> Fscache has an optimisation by which reads from the cache are skipped >> until >> we know that (a) there's data there to be read and (b) that data isn't >> entirely covered by pages resident in the netfs pagecache.  This is done >> with two flags manipulated by fscache_note_page_release(): >> >>     if (... >>         test_bit(FSCACHE_COOKIE_HAVE_DATA, &cookie->flags) && >>         test_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags)) >>         clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags); >> >> where the NO_DATA_TO_READ flag causes cachefiles_prepare_read() to >> indicate >> that netfslib should download from the server or clear the page instead. >> >> The fscache_note_page_release() function is intended to be called from >> ->releasepage() - but that only gets called if PG_private or PG_private_2 >> is set - and currently the former is at the discretion of the network >> filesystem and the latter is only set whilst a page is being written >> to the >> cache, so sometimes we miss clearing the optimisation. >> >> Fix this by following Willy's suggestion[1] and adding an address_space >> flag, AS_RELEASE_ALWAYS, that causes filemap_release_folio() to always >> call >> ->release_folio() if it's set, even if PG_private or PG_private_2 aren't >> set. >> >> Note that this would require folio_test_private() and >> page_has_private() to >> become more complicated.  To avoid that, in the places[*] where these are >> used to conditionalise calls to filemap_release_folio() and >> try_to_release_page(), the tests are removed the those functions just >> jumped to unconditionally and the test is performed there. >> >> [*] There are some exceptions in vmscan.c where the check guards more >> than >> just a call to the releaser.  I've added a function, >> folio_needs_release() >> to wrap all the checks for that. >> >> AS_RELEASE_ALWAYS should be set if a non-NULL cookie is obtained from >> fscache and cleared in ->evict_inode() before >> truncate_inode_pages_final() >> is called. >> >> Additionally, the FSCACHE_COOKIE_NO_DATA_TO_READ flag needs to be cleared >> and the optimisation cancelled if a cachefiles object already contains >> data >> when we open it. >> >> Fixes: 1f67e6d0b188 ("fscache: Provide a function to note the release >> of a page") >> Fixes: 047487c947e8 ("cachefiles: Implement the I/O routines") >> Reported-by: Rohith Surabattula >> Suggested-by: Matthew Wilcox >> Signed-off-by: David Howells >> cc: Matthew Wilcox >> cc: Linus Torvalds >> cc: Steve French >> cc: Shyam Prasad N >> cc: Rohith Surabattula >> cc: Dave Wysochanski >> cc: Dominique Martinet >> cc: Ilya Dryomov >> cc: linux-cachefs@redhat.com >> cc: linux-cifs@vger.kernel.org >> cc: linux-afs@lists.infradead.org >> cc: v9fs-developer@lists.sourceforge.net >> cc: ceph-devel@vger.kernel.org >> cc: linux-nfs@vger.kernel.org >> cc: linux-fsdevel@vger.kernel.org >> cc: linux-mm@kvack.org >> --- >> >> Notes: >>      ver #7) >>       - Make NFS set AS_RELEASE_ALWAYS. >>           ver #4) >>       - Split out merging of >> folio_has_private()/filemap_release_folio() call >>         pairs into a preceding patch. >>       - Don't need to clear AS_RELEASE_ALWAYS in ->evict_inode(). >>           ver #3) >>       - Fixed mapping_clear_release_always() to use clear_bit() not >> set_bit(). >>       - Moved a '&&' to the correct line. >>           ver #2) >>       - Rewrote entirely according to Willy's suggestion[1]. >> >>   fs/9p/cache.c           |  2 ++ >>   fs/afs/internal.h       |  2 ++ >>   fs/cachefiles/namei.c   |  2 ++ >>   fs/ceph/cache.c         |  2 ++ >>   fs/nfs/fscache.c        |  3 +++ >>   fs/smb/client/fscache.c |  2 ++ >>   include/linux/pagemap.h | 16 ++++++++++++++++ >>   mm/internal.h           |  5 ++++- >>   8 files changed, 33 insertions(+), 1 deletion(-) > > Just one question. Shouldn't do this in 'fs/erofs/fscache.c' too ? > Currently the read optimization is not used in fscache ondemand mode (used by erofs), though it may not be intended... cachefiles_ondemand_copen if (size) clear_bit(FSCACHE_COOKIE_NO_DATA_TO_READ, &cookie->flags); The read optimization is disabled as long as the backing file size is not 0 (which is the most case). And thus currently erofs doesn't need to clear FSCACHE_COOKIE_NO_DATA_TO_READ in .release_folio(). -- Thanks, Jingbo