| Message ID | 20240429190500.30979-6-ryncsn@gmail.com (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | mm/swap: clean up and optimize swap cache index | expand |
Wouldn't this make it harder to fix the regression when swap file support was temporarily removed from cifs.ko (due to the folio migration)? I was hoping to come back to fixing swapfile support for cifs.ko in 6.10-rc (which used to pass the various xfstests for this but code got removed with folios/netfs changes). -----Original Message----- From: Kairui Song <ryncsn@gmail.com> Sent: Monday, April 29, 2024 2:05 PM To: linux-mm@kvack.org Cc: Andrew Morton <akpm@linux-foundation.org>; Huang, Ying <ying.huang@intel.com>; Matthew Wilcox <willy@infradead.org>; Chris Li <chrisl@kernel.org>; Barry Song <v-songbaohua@oppo.com>; Ryan Roberts <ryan.roberts@arm.com>; Neil Brown <neilb@suse.de>; Minchan Kim <minchan@kernel.org>; Hugh Dickins <hughd@google.com>; David Hildenbrand <david@redhat.com>; Yosry Ahmed <yosryahmed@google.com>; linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; Kairui Song <kasong@tencent.com>; Steven French <Steven.French@microsoft.com>; Namjae Jeon <linkinjeon@kernel.org>; Paulo Alcantara (SUSE) <pc@manguebit.com>; Shyam Prasad <Shyam.Prasad@microsoft.com>; Bharath S M <bharathsm@microsoft.com> Subject: [EXTERNAL] [PATCH v3 05/12] cifs: drop usage of page_file_offset [Some people who received this message don't often get email from ryncsn@gmail.com. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ] From: Kairui Song <kasong@tencent.com> page_file_offset is only needed for mixed usage of page cache and swap cache, for pure page cache usage, the caller can just use page_offset instead. It can't be a swap cache page here, so just drop it and convert it to use folio. Signed-off-by: Kairui Song <kasong@tencent.com> Cc: Steve French <stfrench@microsoft.com> Cc: Namjae Jeon <linkinjeon@kernel.org> Cc: Paulo Alcantara <pc@manguebit.com> Cc: Shyam Prasad N <sprasad@microsoft.com> Cc: Bharath SM <bharathsm@microsoft.com> --- fs/smb/client/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 9be37d0fe724..388343b0fceb 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -4828,7 +4828,7 @@ static int cifs_readpage_worker(struct file *file, struct page *page, static int cifs_read_folio(struct file *file, struct folio *folio) { struct page *page = &folio->page; - loff_t offset = page_file_offset(page); + loff_t offset = folio_pos(folio); int rc = -EACCES; unsigned int xid; -- 2.44.0
On Mon, Apr 29, 2024 at 08:19:31PM +0000, Steven French wrote:
> Wouldn't this make it harder to fix the regression when swap file support was temporarily removed from cifs.ko (due to the folio migration)? I was hoping to come back to fixing swapfile support for cifs.ko in 6.10-rc (which used to pass the various xfstests for this but code got removed with folios/netfs changes).
It was neither the folio conversion nor the netfs conversion which
removed the claim of swap support from cifs, but NeilBrown's
introduction of ->swap_rw. In commit e1209d3a7a67 he claims that
Only two filesystems set SWP_FS_OPS:
- cifs sets the flag, but ->direct_IO always fails so swap cannot work.
- nfs sets the flag, but ->direct_IO calls generic_write_checks()
which has failed on swap files for several releases.
As I recall the xfstests only checked that swapon/swapoff works; they
don't actually test that writing to swap and reading back from it work.
Makes sense - I will try to look at this fixing the swapon and reading from swap over smb3.1.1 mounts in the next few weeks, but if you have a good example of sample code (from one of the other FS that does this well) that would help. -----Original Message----- From: Matthew Wilcox <willy@infradead.org> Sent: Monday, April 29, 2024 3:26 PM To: Steven French <Steven.French@microsoft.com> Cc: Kairui Song <kasong@tencent.com>; linux-mm@kvack.org; Andrew Morton <akpm@linux-foundation.org>; Huang, Ying <ying.huang@intel.com>; Chris Li <chrisl@kernel.org>; Barry Song <v-songbaohua@oppo.com>; Ryan Roberts <ryan.roberts@arm.com>; Neil Brown <neilb@suse.de>; Minchan Kim <minchan@kernel.org>; Hugh Dickins <hughd@google.com>; David Hildenbrand <david@redhat.com>; Yosry Ahmed <yosryahmed@google.com>; linux-fsdevel@vger.kernel.org; linux-kernel@vger.kernel.org; Namjae Jeon <linkinjeon@kernel.org>; Paulo Alcantara (SUSE) <pc@manguebit.com>; Shyam Prasad <Shyam.Prasad@microsoft.com>; Bharath S M <bharathsm@microsoft.com> Subject: Re: [EXTERNAL] [PATCH v3 05/12] cifs: drop usage of page_file_offset On Mon, Apr 29, 2024 at 08:19:31PM +0000, Steven French wrote: > Wouldn't this make it harder to fix the regression when swap file support was temporarily removed from cifs.ko (due to the folio migration)? I was hoping to come back to fixing swapfile support for cifs.ko in 6.10-rc (which used to pass the various xfstests for this but code got removed with folios/netfs changes). It was neither the folio conversion nor the netfs conversion which removed the claim of swap support from cifs, but NeilBrown's introduction of ->swap_rw. In commit e1209d3a7a67 he claims that Only two filesystems set SWP_FS_OPS: - cifs sets the flag, but ->direct_IO always fails so swap cannot work. - nfs sets the flag, but ->direct_IO calls generic_write_checks() which has failed on swap files for several releases. As I recall the xfstests only checked that swapon/swapoff works; they don't actually test that writing to swap and reading back from it work.
On Tue, Apr 30, 2024 at 4:19 AM Steven French <Steven.French@microsoft.com> wrote: > > Wouldn't this make it harder to fix the regression when swap file support was temporarily removed from cifs.ko (due to the folio migration)? I was hoping to come back to fixing swapfile support for cifs.ko in 6.10-rc (which used to pass the various xfstests for this but code got removed with folios/netfs changes). > Hi Steven, I think this won't cause any trouble for that, the new swap_rw interface should have splitted the swap cache from fs side, which made the code struct cleaner, and I think that's the right path. NFS is using that already. With this interface, .read_folio (cifs_read_folio) should never be called for swap cache folios.
diff --git a/fs/smb/client/file.c b/fs/smb/client/file.c index 9be37d0fe724..388343b0fceb 100644 --- a/fs/smb/client/file.c +++ b/fs/smb/client/file.c @@ -4828,7 +4828,7 @@ static int cifs_readpage_worker(struct file *file, struct page *page, static int cifs_read_folio(struct file *file, struct folio *folio) { struct page *page = &folio->page; - loff_t offset = page_file_offset(page); + loff_t offset = folio_pos(folio); int rc = -EACCES; unsigned int xid;