All of lore.kernel.org
 help / color / mirror / Atom feed
From: Fabio M. De Francesco <fmdefrancesco@gmail.com>
To: cluster-devel.redhat.com
Subject: [Cluster-devel] [PATCH v3 5/6] gfs2: Replace kmap() by kmap_local_page() in gfs2_read_super
Date: Sat, 01 Jul 2023 15:45:24 +0200	[thread overview]
Message-ID: <4817807.GXAFRqVoOG@suse> (raw)
In-Reply-To: <7ad72f9d1a97d673483c206d8ac9a88f2e32d3b9.1688073459.git.drv@mailo.com>

On gioved? 29 giugno 2023 23:51:53 CEST Deepak R Varma wrote:
> The use of kmap() is being deprecated in favor of kmap_local_page().
> 
> There are two main problems with kmap(): (1) It comes with an overhead as
> the mapping space is restricted and protected by a global lock for
> synchronization and (2) it also requires global TLB invalidation when the
> kmap?s pool wraps and it might block when the mapping space is fully
> utilized until a slot becomes available.
> 
> With kmap_local_page() the mappings are per thread, CPU local, can take
> page faults, and can be called from any context (including interrupts).
> It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
> the tasks can be preempted and, when they are scheduled to run again, the
> kernel virtual addresses are restored and still valid.
> 
> Therefore, replace kmap() with kmap_local_page() in gfs2_read_super().
> 
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> Changes in v3:
>    - Patch included in patch set
> 
> Changes in v2:
>    - None
> 
>  fs/gfs2/ops_fstype.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 8a27957dbfee..80fe61662412 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -264,9 +264,9 @@ static int gfs2_read_super(struct gfs2_sbd *sdp, 
sector_t
> sector, int silent) __free_page(page);
>  		return -EIO;
>  	}
> -	p = kmap(page);
> +	p = kmap_local_page(page);
>
Deepak,

Could this page ever come from ZONE_HIGHMEM? Can you please check whether we 
really need to kmap*() it or not? 

Fabio
>
>  	gfs2_sb_in(sdp, p);
> -	kunmap(page);
> +	kunmap_local(p);
>  	__free_page(page);
>  	return gfs2_check_sb(sdp, silent);
>  }
> --
> 2.34.1





WARNING: multiple messages have this Message-ID (diff)
From: "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
To: Bob Peterson <rpeterso@redhat.com>,
	Andreas Gruenbacher <agruenba@redhat.com>,
	cluster-devel@redhat.com, linux-kernel@vger.kernel.org,
	Deepak R Varma <drv@mailo.com>
Cc: Ira Weiny <ira.weiny@intel.com>,
	Sumitra Sharma <sumitraartsy@gmail.com>,
	Deepak R Varma <drv@mailo.com>
Subject: Re: [PATCH v3 5/6] gfs2: Replace kmap() by kmap_local_page() in gfs2_read_super
Date: Sat, 01 Jul 2023 15:45:24 +0200	[thread overview]
Message-ID: <4817807.GXAFRqVoOG@suse> (raw)
In-Reply-To: <7ad72f9d1a97d673483c206d8ac9a88f2e32d3b9.1688073459.git.drv@mailo.com>

On giovedì 29 giugno 2023 23:51:53 CEST Deepak R Varma wrote:
> The use of kmap() is being deprecated in favor of kmap_local_page().
> 
> There are two main problems with kmap(): (1) It comes with an overhead as
> the mapping space is restricted and protected by a global lock for
> synchronization and (2) it also requires global TLB invalidation when the
> kmap’s pool wraps and it might block when the mapping space is fully
> utilized until a slot becomes available.
> 
> With kmap_local_page() the mappings are per thread, CPU local, can take
> page faults, and can be called from any context (including interrupts).
> It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore,
> the tasks can be preempted and, when they are scheduled to run again, the
> kernel virtual addresses are restored and still valid.
> 
> Therefore, replace kmap() with kmap_local_page() in gfs2_read_super().
> 
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Deepak R Varma <drv@mailo.com>
> ---
> Changes in v3:
>    - Patch included in patch set
> 
> Changes in v2:
>    - None
> 
>  fs/gfs2/ops_fstype.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/fs/gfs2/ops_fstype.c b/fs/gfs2/ops_fstype.c
> index 8a27957dbfee..80fe61662412 100644
> --- a/fs/gfs2/ops_fstype.c
> +++ b/fs/gfs2/ops_fstype.c
> @@ -264,9 +264,9 @@ static int gfs2_read_super(struct gfs2_sbd *sdp, 
sector_t
> sector, int silent) __free_page(page);
>  		return -EIO;
>  	}
> -	p = kmap(page);
> +	p = kmap_local_page(page);
>
Deepak,

Could this page ever come from ZONE_HIGHMEM? Can you please check whether we 
really need to kmap*() it or not? 

Fabio
>
>  	gfs2_sb_in(sdp, p);
> -	kunmap(page);
> +	kunmap_local(p);
>  	__free_page(page);
>  	return gfs2_check_sb(sdp, silent);
>  }
> --
> 2.34.1





  reply	other threads:[~2023-07-01 13:45 UTC|newest]

Thread overview: 30+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2023-06-29 21:48 [Cluster-devel] [PATCH v3 0/6] gfs2: kmap{_atomic} conversion to kmap_local_{page/folio} Deepak R Varma
2023-06-29 21:48 ` Deepak R Varma
2023-06-29 21:49 ` [Cluster-devel] [PATCH v3 1/6] gfs2: Replace kmap_atomic() by kmap_local_page() in stuffed_readpage Deepak R Varma
2023-06-29 21:49   ` Deepak R Varma
2023-07-01 11:02   ` [Cluster-devel] " Fabio M. De Francesco
2023-07-01 11:02     ` Fabio M. De Francesco
2023-06-29 21:50 ` [Cluster-devel] [PATCH v3 2/6] gfs2: Replace kmap_atomic()+memcpy by memcpy_from_page() Deepak R Varma
2023-06-29 21:50   ` Deepak R Varma
2023-07-01 11:17   ` [Cluster-devel] " Fabio M. De Francesco
2023-07-01 11:17     ` Fabio M. De Francesco
2023-06-29 21:50 ` [Cluster-devel] [PATCH v3 3/6] gfs2: Replace kmap() by kmap_local_page() in gfs2_unstuffer_page Deepak R Varma
2023-06-29 21:50   ` Deepak R Varma
2023-07-01 13:01   ` [Cluster-devel] " Fabio M. De Francesco
2023-07-01 13:01     ` Fabio M. De Francesco
2023-06-29 21:51 ` [Cluster-devel] [PATCH v3 4/6] gfs2: Replace kmap_atomic() by kmap_local_page() in lops.c Deepak R Varma
2023-06-29 21:51   ` Deepak R Varma
2023-07-01 13:16   ` [Cluster-devel] " Fabio M. De Francesco
2023-07-01 13:16     ` Fabio M. De Francesco
2023-06-29 21:51 ` [Cluster-devel] [PATCH v3 5/6] gfs2: Replace kmap() by kmap_local_page() in gfs2_read_super Deepak R Varma
2023-06-29 21:51   ` Deepak R Varma
2023-07-01 13:45   ` Fabio M. De Francesco [this message]
2023-07-01 13:45     ` Fabio M. De Francesco
2023-06-29 21:52 ` [Cluster-devel] [PATCH v3 6/6] gfs2: Replace kmap_atomic() by kmap_local_page() in gfs2_write_buf_to_page Deepak R Varma
2023-06-29 21:52   ` Deepak R Varma
2023-07-01 13:54   ` [Cluster-devel] " Fabio M. De Francesco
2023-07-01 13:54     ` Fabio M. De Francesco
2023-08-10 15:28     ` [Cluster-devel] " Deepak R Varma
2023-08-10 15:28       ` Deepak R Varma
2023-07-03  9:17 ` [Cluster-devel] [PATCH v3 0/6] gfs2: kmap{_atomic} conversion to kmap_local_{page/folio} Andreas Gruenbacher
2023-07-03  9:17   ` Andreas Gruenbacher

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=4817807.GXAFRqVoOG@suse \
    --to=fmdefrancesco@gmail.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.