From: David Howells <dhowells@redhat.com> To: Matthew Wilcox <willy@infradead.org> Cc: dhowells@redhat.com, torvalds@linux-foundation.org, Alexander Viro <viro@zeniv.linux.org.uk>, Christoph Hellwig <hch@lst.de>, Jeff Layton <jlayton@redhat.com>, Dave Wysochanski <dwysocha@redhat.com>, Trond Myklebust <trondmy@hammerspace.com>, Anna Schumaker <anna.schumaker@netapp.com>, Steve French <sfrench@samba.org>, Eric Van Hensbergen <ericvh@gmail.com>, linux-cachefs@redhat.com, linux-afs@lists.infradead.org, linux-nfs@vger.kernel.org, linux-cifs@vger.kernel.org, ceph-devel@vger.kernel.org, v9fs-developer@lists.sourceforge.net, linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org Subject: Re: Upcoming: fscache rewrite Date: Thu, 30 Jul 2020 13:36:14 +0100 [thread overview] Message-ID: <488587.1596112574@warthog.procyon.org.uk> (raw) In-Reply-To: <20200730121622.GB23808@casper.infradead.org> Matthew Wilcox <willy@infradead.org> wrote: > I suspect you don't need to call find_get_pages_contig(). If you look > at __readahead_batch() in pagemap.h, it does basically what you want > (other than being wrapped up inside the readahead iterator). You require > the pages already be pinned in the xarray, so there's no need for the > page_cache_get_speculative() dance that find_get_pages_contig) does, > nor the check for xa_is_value(). I'll have a look at that. > My main concern with your patchset is that it introduces a new page flag Technically, the flag already exists - I'm just using it for something different than the old fscache code used it for. > to sleep on which basically means "I am writing this page to the fscache". > I don't understand why you need it; you've elevated the refcount on > the pages so they're not going to get reused for another purpose. > All it does (as far as I can tell) is make a task calling truncate() > wait for the page to finish being written to cache, which isn't actually > necessary. It's also used to prevent starting overlapping async DIO writes to the cache. See fscache_read_done(), where it's set to cover writing what we've just read from the server to the cache, and afs_write_back_from_locked_page(), where it's set to cover writing the data to be written back to the cache. David
WARNING: multiple messages have this Message-ID (diff)
From: David Howells <dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> To: Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> Cc: dhowells-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, torvalds-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b@public.gmane.org, Alexander Viro <viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn@public.gmane.org>, Christoph Hellwig <hch-jcswGhMUV9g@public.gmane.org>, Jeff Layton <jlayton-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Dave Wysochanski <dwysocha-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>, Trond Myklebust <trondmy-F/q8l9xzQnoyLce1RVWEUA@public.gmane.org>, Anna Schumaker <anna.schumaker-HgOvQuBEEgTQT0dZR+AlfA@public.gmane.org>, Steve French <sfrench-eUNUBHrolfbYtjvyW6yDsg@public.gmane.org>, Eric Van Hensbergen <ericvh-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>, linux-cachefs-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org, linux-afs-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org, linux-nfs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-cifs-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, ceph-devel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, v9fs-developer-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f@public.gmane.org, linux-fsdevel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org Subject: Re: Upcoming: fscache rewrite Date: Thu, 30 Jul 2020 13:36:14 +0100 [thread overview] Message-ID: <488587.1596112574@warthog.procyon.org.uk> (raw) In-Reply-To: <20200730121622.GB23808-FZi0V3Vbi30CUdFEqe4BF2D2FQJk+8+b@public.gmane.org> Matthew Wilcox <willy-wEGCiKHe2LqWVfeAwA7xHQ@public.gmane.org> wrote: > I suspect you don't need to call find_get_pages_contig(). If you look > at __readahead_batch() in pagemap.h, it does basically what you want > (other than being wrapped up inside the readahead iterator). You require > the pages already be pinned in the xarray, so there's no need for the > page_cache_get_speculative() dance that find_get_pages_contig) does, > nor the check for xa_is_value(). I'll have a look at that. > My main concern with your patchset is that it introduces a new page flag Technically, the flag already exists - I'm just using it for something different than the old fscache code used it for. > to sleep on which basically means "I am writing this page to the fscache". > I don't understand why you need it; you've elevated the refcount on > the pages so they're not going to get reused for another purpose. > All it does (as far as I can tell) is make a task calling truncate() > wait for the page to finish being written to cache, which isn't actually > necessary. It's also used to prevent starting overlapping async DIO writes to the cache. See fscache_read_done(), where it's set to cover writing what we've just read from the server to the cache, and afs_write_back_from_locked_page(), where it's set to cover writing the data to be written back to the cache. David
next prev parent reply other threads:[~2020-07-30 12:36 UTC|newest] Thread overview: 21+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-07-30 11:51 Upcoming: fscache rewrite David Howells 2020-07-30 11:51 ` David Howells 2020-07-30 12:16 ` Matthew Wilcox 2020-07-30 12:16 ` Matthew Wilcox 2020-07-30 12:36 ` David Howells [this message] 2020-07-30 12:36 ` David Howells 2020-07-30 13:08 ` Jeff Layton 2020-07-30 13:08 ` Jeff Layton 2020-08-03 16:30 ` [GIT PULL] " David Howells 2020-08-03 16:30 ` David Howells 2020-08-10 15:16 ` [GIT PULL] fscache rewrite -- please drop for now David Howells 2020-08-10 15:34 ` Steve French 2020-08-10 15:48 ` David Howells 2020-08-10 16:09 ` Steve French 2020-08-10 16:35 ` David Wysochanski 2020-08-10 17:06 ` Jeff Layton 2020-08-17 19:07 ` Steven French 2020-08-10 16:40 ` Christoph Hellwig 2020-08-27 15:28 ` David Howells 2020-08-27 16:18 ` Dominique Martinet 2020-08-27 17:14 ` David Howells
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=488587.1596112574@warthog.procyon.org.uk \ --to=dhowells@redhat.com \ --cc=anna.schumaker@netapp.com \ --cc=ceph-devel@vger.kernel.org \ --cc=dwysocha@redhat.com \ --cc=ericvh@gmail.com \ --cc=hch@lst.de \ --cc=jlayton@redhat.com \ --cc=linux-afs@lists.infradead.org \ --cc=linux-cachefs@redhat.com \ --cc=linux-cifs@vger.kernel.org \ --cc=linux-fsdevel@vger.kernel.org \ --cc=linux-kernel@vger.kernel.org \ --cc=linux-nfs@vger.kernel.org \ --cc=sfrench@samba.org \ --cc=torvalds@linux-foundation.org \ --cc=trondmy@hammerspace.com \ --cc=v9fs-developer@lists.sourceforge.net \ --cc=viro@zeniv.linux.org.uk \ --cc=willy@infradead.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: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.