All of lore.kernel.org
 help / color / mirror / Atom feed
From: Matthew Wilcox <willy@infradead.org>
To: "Huang, Ying" <ying.huang@intel.com>
Cc: Kairui Song <ryncsn@gmail.com>,
	linux-mm@kvack.org, Kairui Song <kasong@tencent.com>,
	Andrew Morton <akpm@linux-foundation.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,
	linux-nfs@vger.kernel.org,
	Trond Myklebust <trond.myklebust@hammerspace.com>,
	Anna Schumaker <anna@kernel.org>,
	linux-afs@lists.infradead.org,
	David Howells <dhowells@redhat.com>,
	Marc Dionne <marc.dionne@auristor.com>
Subject: Re: [PATCH v2 7/8] mm: drop page_index/page_file_offset and convert swap helpers to use folio
Date: Wed, 24 Apr 2024 05:05:49 +0100	[thread overview]
Message-ID: <ZiiFHTwgu8FGio1k@casper.infradead.org> (raw)
In-Reply-To: <87sezbsdwf.fsf@yhuang6-desk2.ccr.corp.intel.com>

On Wed, Apr 24, 2024 at 10:17:04AM +0800, Huang, Ying wrote:
> Kairui Song <ryncsn@gmail.com> writes:
> >  static inline loff_t folio_file_pos(struct folio *folio)
> >  {
> > -	return page_file_offset(&folio->page);
> > +	if (unlikely(folio_test_swapcache(folio)))
> > +		return __folio_swap_dev_pos(folio);
> > +	return ((loff_t)folio->index << PAGE_SHIFT);
> 
> This still looks confusing for me.  The function returns the byte
> position of the folio in its file.  But we returns the swap device
> position of the folio.
> 
> Tried to search folio_file_pos() usage.  The 2 usage in page_io.c is
> swap specific, we can use swap_dev_pos() directly.
> 
> There are also other file system users (NFS and AFS) of
> folio_file_pos(), I don't know why they need to work with swap
> cache. Cced file system maintainers for help.

Time for a history lesson!

In d56b4ddf7781 (2012) we introduced page_file_index() and
page_file_mapping() to support swap-over-NFS.  Writes to the swapfile went
through ->direct_IO but reads went through ->readpage.  So NFS was changed
to remove direct references to page->mapping and page->index because
those aren't right for anon pages (or shmem pages being swapped out).

In e1209d3a7a67 (2022), we stopped using ->readpage in favour of using
->swap_rw.  Now we don't need to use page_file_*(); we get the swap_file
and ki_pos directly in the swap_iocb.  But there are still relics in NFS
that nobody has dared rip out.  And there are all the copy-and-pasted
filesystems that use page_file_* because they don't know any better.

We should delete page_file_*() and folio_file_*().  They shouldn't be
needed any more.

  parent reply	other threads:[~2024-04-24  4:06 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-23 17:03 [PATCH v2 0/8] mm/swap: optimize swap cache search space Kairui Song
2024-04-23 17:03 ` [PATCH v2 1/8] NFS: remove nfs_page_lengthg and usage of page_index Kairui Song
2024-04-23 17:03 ` [PATCH v2 2/8] nilfs2: drop " Kairui Song
2024-04-23 18:48   ` Ryusuke Konishi
2024-04-24  1:57     ` Kairui Song
2024-04-23 17:03 ` [f2fs-dev] [PATCH v2 3/8] f2fs: " Kairui Song
2024-04-23 17:03   ` Kairui Song
2024-04-23 22:58   ` [f2fs-dev] " Matthew Wilcox
2024-04-23 22:58     ` Matthew Wilcox
2024-04-28  3:17     ` [f2fs-dev] " Chao Yu
2024-04-28  3:17       ` Chao Yu
2024-04-23 17:03 ` [PATCH v2 4/8] ceph: " Kairui Song
2024-04-24  0:03   ` Xiubo Li
2024-04-23 17:03 ` [PATCH v2 5/8] cifs: drop usage of page_file_offset Kairui Song
2024-04-23 17:03 ` [PATCH v2 6/8] mm/swap: get the swap file offset directly Kairui Song
2024-04-23 17:03 ` [PATCH v2 7/8] mm: drop page_index/page_file_offset and convert swap helpers to use folio Kairui Song
2024-04-24  2:17   ` Huang, Ying
2024-04-24  2:56     ` Kairui Song
2024-04-24  4:05     ` Matthew Wilcox [this message]
2024-04-24  6:45       ` Huang, Ying
2024-04-24  8:48       ` Kairui Song
2024-04-23 17:03 ` [PATCH v2 8/8] mm/swap: reduce swap cache search space Kairui Song

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=ZiiFHTwgu8FGio1k@casper.infradead.org \
    --to=willy@infradead.org \
    --cc=akpm@linux-foundation.org \
    --cc=anna@kernel.org \
    --cc=chrisl@kernel.org \
    --cc=david@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=hughd@google.com \
    --cc=kasong@tencent.com \
    --cc=linux-afs@lists.infradead.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linux-nfs@vger.kernel.org \
    --cc=marc.dionne@auristor.com \
    --cc=minchan@kernel.org \
    --cc=neilb@suse.de \
    --cc=ryan.roberts@arm.com \
    --cc=ryncsn@gmail.com \
    --cc=trond.myklebust@hammerspace.com \
    --cc=v-songbaohua@oppo.com \
    --cc=ying.huang@intel.com \
    --cc=yosryahmed@google.com \
    /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 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.