linux-kernel - Re: [PATCH 6/8] mm/swap: get the swap file offset directly

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <87mspkx3cy.fsf@yhuang6-desk2.ccr.corp.intel.com>
Date: Tue, 23 Apr 2024 09:41:17 +0800
From: "Huang, Ying" <ying.huang@...el.com>
To: Kairui Song <ryncsn@...il.com>
Cc: linux-mm@...ck.org,  Kairui Song <kasong@...cent.com>,  Andrew Morton
 <akpm@...ux-foundation.org>,  Matthew Wilcox <willy@...radead.org>,  Chris
 Li <chrisl@...nel.org>,  Barry Song <v-songbaohua@...o.com>,  Ryan Roberts
 <ryan.roberts@....com>,  Neil Brown <neilb@...e.de>,  Minchan Kim
 <minchan@...nel.org>,  Hugh Dickins <hughd@...gle.com>,  David Hildenbrand
 <david@...hat.com>,  Yosry Ahmed <yosryahmed@...gle.com>,
  linux-fsdevel@...r.kernel.org,  linux-kernel@...r.kernel.org
Subject: Re: [PATCH 6/8] mm/swap: get the swap file offset directly

Kairui Song <ryncsn@...il.com> writes:

> From: Kairui Song <kasong@...cent.com>
>
> folio_file_pos and page_file_offset are for mixed usage of swap cache
> and page cache, it can't be page cache here, so introduce a new helper
> to get the swap offset in swap file directly.
>
> Signed-off-by: Kairui Song <kasong@...cent.com>
> ---
>  mm/page_io.c | 6 +++---
>  mm/swap.h    | 5 +++++
>  2 files changed, 8 insertions(+), 3 deletions(-)
>
> diff --git a/mm/page_io.c b/mm/page_io.c
> index ae2b49055e43..93de5aadb438 100644
> --- a/mm/page_io.c
> +++ b/mm/page_io.c
> @@ -279,7 +279,7 @@ static void sio_write_complete(struct kiocb *iocb, long ret)
>  		 * be temporary.
>  		 */
>  		pr_err_ratelimited("Write error %ld on dio swapfile (%llu)\n",
> -				   ret, page_file_offset(page));
> +				   ret, swap_file_pos(page_swap_entry(page)));
>  		for (p = 0; p < sio->pages; p++) {
>  			page = sio->bvec[p].bv_page;
>  			set_page_dirty(page);
> @@ -298,7 +298,7 @@ static void swap_writepage_fs(struct folio *folio, struct writeback_control *wbc
>  	struct swap_iocb *sio = NULL;
>  	struct swap_info_struct *sis = swp_swap_info(folio->swap);
>  	struct file *swap_file = sis->swap_file;
> -	loff_t pos = folio_file_pos(folio);
> +	loff_t pos = swap_file_pos(folio->swap);
>  
>  	count_swpout_vm_event(folio);
>  	folio_start_writeback(folio);
> @@ -429,7 +429,7 @@ static void swap_read_folio_fs(struct folio *folio, struct swap_iocb **plug)
>  {
>  	struct swap_info_struct *sis = swp_swap_info(folio->swap);
>  	struct swap_iocb *sio = NULL;
> -	loff_t pos = folio_file_pos(folio);
> +	loff_t pos = swap_file_pos(folio->swap);
>  
>  	if (plug)
>  		sio = *plug;
> diff --git a/mm/swap.h b/mm/swap.h
> index fc2f6ade7f80..2de83729aaa8 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -7,6 +7,11 @@ struct mempolicy;
>  #ifdef CONFIG_SWAP
>  #include <linux/blk_types.h> /* for bio_end_io_t */
>  
> +static inline loff_t swap_file_pos(swp_entry_t entry)
> +{
> +	return ((loff_t)swp_offset(entry)) << PAGE_SHIFT;
> +}
> +
>  /* linux/mm/page_io.c */
>  int sio_pool_init(void);
>  struct swap_iocb;

I feel that the file concept for swap is kind of confusing.  From the
file cache point of view, one "struct address space" conresponds to one
file.  If so, we have a simple file system on a swap device (block
device backed or file backed), where the size of each file is 64M.  The
swap entry encode the file system (swap_type), the file name
(swap_offset >> SWAP_ADDRESS_SPACE_SHIFT), and the offset in file (lower
bits of swap_offset).

If the above definition is good, it's better to rename swap_file_pos()
to swap_dev_pos(), because it returns the swap device position of the
swap entry.

And, when we reaches consensus on the swap file related concept, we may
document it somewhere and review all naming in swap code to cleanup.

--
Best Regards,
Huang, Ying