linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: "Huang, Ying" <ying.huang@intel.com>
To: Suren Baghdasaryan <surenb@google.com>
Cc: akpm@linux-foundation.org,  willy@infradead.org,
	 hannes@cmpxchg.org, mhocko@suse.com,  josef@toxicpanda.com,
	 jack@suse.cz, ldufour@linux.ibm.com,  laurent.dufour@fr.ibm.com,
	michel@lespinasse.org,  liam.howlett@oracle.com,
	 jglisse@google.com, vbabka@suse.cz,  minchan@google.com,
	 dave@stgolabs.net, punit.agrawal@bytedance.com,
	 lstoakes@gmail.com,  hdanton@sina.com, apopple@nvidia.com,
	 peterx@redhat.com,  david@redhat.com, yuzhao@google.com,
	 dhowells@redhat.com,  hughd@google.com, viro@zeniv.linux.org.uk,
	 brauner@kernel.org,  pasha.tatashin@soleen.com,
	linux-mm@kvack.org,  linux-fsdevel@vger.kernel.org,
	linux-kernel@vger.kernel.org,  kernel-team@android.com,
	Ming Lei <ming.lei@redhat.com>
Subject: Re: [PATCH v2 1/6] swap: remove remnants of polling from read_swap_cache_async
Date: Fri, 09 Jun 2023 09:57:25 +0800	[thread overview]
Message-ID: <877csdpfcq.fsf@yhuang6-desk2.ccr.corp.intel.com> (raw)
In-Reply-To: <20230609005158.2421285-2-surenb@google.com> (Suren Baghdasaryan's message of "Thu, 8 Jun 2023 17:51:53 -0700")

+ Ming Lei for confirmation.

Suren Baghdasaryan <surenb@google.com> writes:

> Commit [1] introduced IO polling support during swapin to reduce
> swap read latency for block devices that can be polled. However later
> commit [2] removed polling support. Therefore it seems safe to remove
> do_poll parameter in read_swap_cache_async and always call swap_readpage
> with synchronous=false waiting for IO completion in folio_lock_or_retry.
>
> [1] commit 23955622ff8d ("swap: add block io poll in swapin path")
> [2] commit 9650b453a3d4 ("block: ignore RWF_HIPRI hint for sync dio")
>
> Suggested-by: Huang Ying <ying.huang@intel.com>
> Signed-off-by: Suren Baghdasaryan <surenb@google.com>

Looks good to me!  Thanks!

Reviewed-by: "Huang, Ying" <ying.huang@intel.com>

> ---
>  mm/madvise.c    |  4 ++--
>  mm/swap.h       |  1 -
>  mm/swap_state.c | 12 +++++-------
>  3 files changed, 7 insertions(+), 10 deletions(-)
>
> diff --git a/mm/madvise.c b/mm/madvise.c
> index b5ffbaf616f5..b1e8adf1234e 100644
> --- a/mm/madvise.c
> +++ b/mm/madvise.c
> @@ -215,7 +215,7 @@ static int swapin_walk_pmd_entry(pmd_t *pmd, unsigned long start,
>  			continue;
>  
>  		page = read_swap_cache_async(entry, GFP_HIGHUSER_MOVABLE,
> -					     vma, index, false, &splug);
> +					     vma, index, &splug);
>  		if (page)
>  			put_page(page);
>  	}
> @@ -252,7 +252,7 @@ static void force_shm_swapin_readahead(struct vm_area_struct *vma,
>  		rcu_read_unlock();
>  
>  		page = read_swap_cache_async(swap, GFP_HIGHUSER_MOVABLE,
> -					     NULL, 0, false, &splug);
> +					     NULL, 0, &splug);
>  		if (page)
>  			put_page(page);
>  
> diff --git a/mm/swap.h b/mm/swap.h
> index 7c033d793f15..8a3c7a0ace4f 100644
> --- a/mm/swap.h
> +++ b/mm/swap.h
> @@ -46,7 +46,6 @@ struct folio *filemap_get_incore_folio(struct address_space *mapping,
>  struct page *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>  				   struct vm_area_struct *vma,
>  				   unsigned long addr,
> -				   bool do_poll,
>  				   struct swap_iocb **plug);
>  struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>  				     struct vm_area_struct *vma,
> diff --git a/mm/swap_state.c b/mm/swap_state.c
> index b76a65ac28b3..a3839de71f3f 100644
> --- a/mm/swap_state.c
> +++ b/mm/swap_state.c
> @@ -517,15 +517,14 @@ struct page *__read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>   */
>  struct page *read_swap_cache_async(swp_entry_t entry, gfp_t gfp_mask,
>  				   struct vm_area_struct *vma,
> -				   unsigned long addr, bool do_poll,
> -				   struct swap_iocb **plug)
> +				   unsigned long addr, struct swap_iocb **plug)
>  {
>  	bool page_was_allocated;
>  	struct page *retpage = __read_swap_cache_async(entry, gfp_mask,
>  			vma, addr, &page_was_allocated);
>  
>  	if (page_was_allocated)
> -		swap_readpage(retpage, do_poll, plug);
> +		swap_readpage(retpage, false, plug);
>  
>  	return retpage;
>  }
> @@ -620,7 +619,7 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>  	struct swap_info_struct *si = swp_swap_info(entry);
>  	struct blk_plug plug;
>  	struct swap_iocb *splug = NULL;
> -	bool do_poll = true, page_allocated;
> +	bool page_allocated;
>  	struct vm_area_struct *vma = vmf->vma;
>  	unsigned long addr = vmf->address;
>  
> @@ -628,7 +627,6 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>  	if (!mask)
>  		goto skip;
>  
> -	do_poll = false;
>  	/* Read a page_cluster sized and aligned cluster around offset. */
>  	start_offset = offset & ~mask;
>  	end_offset = offset | mask;
> @@ -660,7 +658,7 @@ struct page *swap_cluster_readahead(swp_entry_t entry, gfp_t gfp_mask,
>  	lru_add_drain();	/* Push any new pages onto the LRU now */
>  skip:
>  	/* The page was likely read above, so no need for plugging here */
> -	return read_swap_cache_async(entry, gfp_mask, vma, addr, do_poll, NULL);
> +	return read_swap_cache_async(entry, gfp_mask, vma, addr, NULL);
>  }
>  
>  int init_swap_address_space(unsigned int type, unsigned long nr_pages)
> @@ -825,7 +823,7 @@ static struct page *swap_vma_readahead(swp_entry_t fentry, gfp_t gfp_mask,
>  skip:
>  	/* The page was likely read above, so no need for plugging here */
>  	return read_swap_cache_async(fentry, gfp_mask, vma, vmf->address,
> -				     ra_info.win == 1, NULL);
> +				     NULL);
>  }
>  
>  /**


  reply	other threads:[~2023-06-09  1:58 UTC|newest]

Thread overview: 36+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-09  0:51 [PATCH v2 0/6] Per-vma lock support for swap and userfaults Suren Baghdasaryan
2023-06-09  0:51 ` [PATCH v2 1/6] swap: remove remnants of polling from read_swap_cache_async Suren Baghdasaryan
2023-06-09  1:57   ` Huang, Ying [this message]
2023-06-09  3:13     ` Ming Lei
2023-06-09 18:50       ` Suren Baghdasaryan
2023-06-12  4:53   ` Christoph Hellwig
2023-06-09  0:51 ` [PATCH v2 2/6] mm: handle swap page faults under VMA lock if page is uncontended Suren Baghdasaryan
2023-06-09 20:25   ` Peter Xu
2023-06-09 20:35     ` Matthew Wilcox
2023-06-09 20:45       ` Peter Xu
2023-06-09 22:34         ` Suren Baghdasaryan
2023-06-12 13:59           ` Peter Xu
2023-06-12 16:09             ` Suren Baghdasaryan
2023-06-09  0:51 ` [PATCH v2 3/6] mm: add missing VM_FAULT_RESULT_TRACE name for VM_FAULT_COMPLETED Suren Baghdasaryan
2023-06-09 20:34   ` Peter Xu
2023-06-09  0:51 ` [PATCH v2 4/6] mm: drop VMA lock before waiting for migration Suren Baghdasaryan
2023-06-09 20:42   ` Peter Xu
2023-06-09 22:30     ` Suren Baghdasaryan
2023-06-10  1:29       ` Suren Baghdasaryan
2023-06-12 13:36         ` Peter Xu
2023-06-12 16:07           ` Suren Baghdasaryan
2023-06-12 18:34             ` Peter Xu
2023-06-12 18:44               ` Suren Baghdasaryan
2023-06-12 18:57                 ` Peter Xu
2023-06-12 13:28       ` Peter Xu
2023-06-12 15:41         ` Suren Baghdasaryan
2023-06-09  0:51 ` [PATCH v2 5/6] mm: implement folio wait under VMA lock Suren Baghdasaryan
2023-06-09 15:03   ` Matthew Wilcox
2023-06-09 18:49     ` Suren Baghdasaryan
2023-06-09 18:55       ` Suren Baghdasaryan
2023-06-09 19:36         ` Matthew Wilcox
2023-06-09 22:49           ` Suren Baghdasaryan
2023-06-09 20:54   ` Peter Xu
2023-06-09 22:48     ` Suren Baghdasaryan
2023-06-12 13:56       ` Peter Xu
2023-06-09  0:51 ` [PATCH v2 6/6] mm: handle userfaults " Suren Baghdasaryan

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=877csdpfcq.fsf@yhuang6-desk2.ccr.corp.intel.com \
    --to=ying.huang@intel.com \
    --cc=akpm@linux-foundation.org \
    --cc=apopple@nvidia.com \
    --cc=brauner@kernel.org \
    --cc=dave@stgolabs.net \
    --cc=david@redhat.com \
    --cc=dhowells@redhat.com \
    --cc=hannes@cmpxchg.org \
    --cc=hdanton@sina.com \
    --cc=hughd@google.com \
    --cc=jack@suse.cz \
    --cc=jglisse@google.com \
    --cc=josef@toxicpanda.com \
    --cc=kernel-team@android.com \
    --cc=laurent.dufour@fr.ibm.com \
    --cc=ldufour@linux.ibm.com \
    --cc=liam.howlett@oracle.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=lstoakes@gmail.com \
    --cc=mhocko@suse.com \
    --cc=michel@lespinasse.org \
    --cc=minchan@google.com \
    --cc=ming.lei@redhat.com \
    --cc=pasha.tatashin@soleen.com \
    --cc=peterx@redhat.com \
    --cc=punit.agrawal@bytedance.com \
    --cc=surenb@google.com \
    --cc=vbabka@suse.cz \
    --cc=viro@zeniv.linux.org.uk \
    --cc=willy@infradead.org \
    --cc=yuzhao@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 a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox