[PATCH v2 net-next 0/6] Remove uses of kmap_atomic()

[PATCH v2 net-next 0/6] Remove uses of kmap_atomic()

[Anirudh Venkataramanan]

kmap_atomic() is being deprecated. This little series replaces the last
few uses of kmap_atomic() in the networking subsystem.

This series triggered a suggestion [1] that perhaps the Sun Cassini,
LDOM Virtual Switch Driver and the LDOM virtual network drivers should be
removed completely. I plan to do this in a follow up patchset. For
completeness, this series still includes kmap_atomic() conversions that
apply to the above referenced drivers. If for some reason we choose to not
remove these drivers, at least they won't be using kmap_atomic() anymore.

Also, the following maintainer entries for the Chelsio driver seem to be
defunct:

  Vinay Kumar Yadav <vinay.yadav@chelsio.com>
  Rohit Maheshwari <rohitm@chelsio.com>

I can submit a follow up patch to remove these entries, but thought
maybe the folks over at Chelsio would want to look into this first.

Changes v1 -> v2:
  Use memcpy_from_page() in patches 2/6 and 4/6
  Add new patch for the thunderbolt driver
  Update commit messages and cover letter

[1] https://lore.kernel.org/netdev/99629223-ac1b-0f82-50b8-ea307b3b0197@intel.com/T/#m3da3759652a48f958ab852fa5499009b43ff8fdd

Anirudh Venkataramanan (6):
  ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()
  sfc: Use kmap_local_page() instead of kmap_atomic()
  cassini: Use page_address() instead of kmap_atomic()
  cassini: Use memcpy_from_page() instead of k[un]map_atomic()
  sunvnet: Use kmap_local_page() instead of kmap_atomic()
  net: thunderbolt: Use kmap_local_page() instead of kmap_atomic()

 .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 26 +++++-----
 drivers/net/ethernet/sfc/tx.c                 |  4 +-
 drivers/net/ethernet/sun/cassini.c            | 48 ++++++-------------
 drivers/net/ethernet/sun/sunvnet_common.c     |  4 +-
 drivers/net/thunderbolt.c                     |  8 ++--
 5 files changed, 35 insertions(+), 55 deletions(-)


base-commit: e80bd08fd75a644e2337fb535c1afdb6417357ff
-- 
2.37.2

[PATCH v2 net-next 1/6] ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()

[Anirudh Venkataramanan]

kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
the map-memcpy-unmap usage pattern (done using k[un]map_atomic()) with
memcpy_from_page(), which internally uses kmap_local_page() and
kunmap_local(). This renders the variables 'data' and 'vaddr' unnecessary,
and so remove these too.

Note that kmap_atomic() disables preemption and page-fault processing, but
kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
to check if the code being executed between the map/unmap implicitly
depends on page-faults and/or preemption being disabled. If yes, then code
to disable page-faults and/or preemption should also be added for
functional correctness. That however doesn't appear to be the case here,
so just memcpy_from_page() is used.

Also note that the page being mapped is not allocated by the driver, and so
the driver doesn't know if the page is in normal memory. This is the reason
kmap_local_page() is used (via memcpy_from_page()) as opposed to
page_address().

I don't have hardware, so this change has only been compile tested.

Cc: Ayush Sawal <ayush.sawal@chelsio.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
v1 -> v2:
 Use memcpy_from_page() as suggested by Fabio
 Add a "Suggested-by" tag
 Rework commit message
 Some emails cc'd in v1 are defunct. Drop them. The MAINTAINERS file for
 Chelsio drivers likely needs an update
---
 .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 26 +++++++++----------
 1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
index da9973b..1a5fdd7 100644
--- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
+++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
@@ -1839,9 +1839,7 @@ static int chcr_short_record_handler(struct chcr_ktls_info *tx_info,
 		 */
 		if (prior_data_len) {
 			int i = 0;
-			u8 *data = NULL;
 			skb_frag_t *f;
-			u8 *vaddr;
 			int frag_size = 0, frag_delta = 0;
 
 			while (remaining > 0) {
@@ -1853,24 +1851,24 @@ static int chcr_short_record_handler(struct chcr_ktls_info *tx_info,
 				i++;
 			}
 			f = &record->frags[i];
-			vaddr = kmap_atomic(skb_frag_page(f));
-
-			data = vaddr + skb_frag_off(f)  + remaining;
 			frag_delta = skb_frag_size(f) - remaining;
 
 			if (frag_delta >= prior_data_len) {
-				memcpy(prior_data, data, prior_data_len);
-				kunmap_atomic(vaddr);
+				memcpy_from_page(prior_data, skb_frag_page(f),
+						 skb_frag_off(f) + remaining,
+						 prior_data_len);
 			} else {
-				memcpy(prior_data, data, frag_delta);
-				kunmap_atomic(vaddr);
+				memcpy_from_page(prior_data, skb_frag_page(f),
+						 skb_frag_off(f) + remaining,
+						 frag_delta);
+
 				/* get the next page */
 				f = &record->frags[i + 1];
-				vaddr = kmap_atomic(skb_frag_page(f));
-				data = vaddr + skb_frag_off(f);
-				memcpy(prior_data + frag_delta,
-				       data, (prior_data_len - frag_delta));
-				kunmap_atomic(vaddr);
+
+				memcpy_from_page(prior_data + frag_delta,
+						 skb_frag_page(f),
+						 skb_frag_off(f),
+						 prior_data_len - frag_delta);
 			}
 			/* reset tcp_seq as per the prior_data_required len */
 			tcp_seq -= prior_data_len;
-- 
2.37.2

Re: [PATCH v2 net-next 1/6] ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()

[Ayush Sawal]

On 11/24/2022 2:22 AM, Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> the map-memcpy-unmap usage pattern (done using k[un]map_atomic()) with
> memcpy_from_page(), which internally uses kmap_local_page() and
> kunmap_local(). This renders the variables 'data' and 'vaddr' unnecessary,
> and so remove these too.
>
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just memcpy_from_page() is used.
>
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used (via memcpy_from_page()) as opposed to
> page_address().
>
> I don't have hardware, so this change has only been compile tested.
>
> Cc: Ayush Sawal <ayush.sawal@chelsio.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> v1 -> v2:
>   Use memcpy_from_page() as suggested by Fabio
>   Add a "Suggested-by" tag
>   Rework commit message
>   Some emails cc'd in v1 are defunct. Drop them. The MAINTAINERS file for
>   Chelsio drivers likely needs an update
> ---


Thanks for the patch.

Acked-by: Ayush Sawal <ayush.sawal@chelsio.com>

>   .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 26 +++++++++----------
>   1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
> index da9973b..1a5fdd7 100644
> --- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
> +++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
> @@ -1839,9 +1839,7 @@ static int chcr_short_record_handler(struct chcr_ktls_info *tx_info,
>   		 */
>   		if (prior_data_len) {
>   			int i = 0;
> -			u8 *data = NULL;
>   			skb_frag_t *f;
> -			u8 *vaddr;
>   			int frag_size = 0, frag_delta = 0;
>   
>   			while (remaining > 0) {
> @@ -1853,24 +1851,24 @@ static int chcr_short_record_handler(struct chcr_ktls_info *tx_info,
>   				i++;
>   			}
>   			f = &record->frags[i];
> -			vaddr = kmap_atomic(skb_frag_page(f));
> -
> -			data = vaddr + skb_frag_off(f)  + remaining;
>   			frag_delta = skb_frag_size(f) - remaining;
>   
>   			if (frag_delta >= prior_data_len) {
> -				memcpy(prior_data, data, prior_data_len);
> -				kunmap_atomic(vaddr);
> +				memcpy_from_page(prior_data, skb_frag_page(f),
> +						 skb_frag_off(f) + remaining,
> +						 prior_data_len);
>   			} else {
> -				memcpy(prior_data, data, frag_delta);
> -				kunmap_atomic(vaddr);
> +				memcpy_from_page(prior_data, skb_frag_page(f),
> +						 skb_frag_off(f) + remaining,
> +						 frag_delta);
> +
>   				/* get the next page */
>   				f = &record->frags[i + 1];
> -				vaddr = kmap_atomic(skb_frag_page(f));
> -				data = vaddr + skb_frag_off(f);
> -				memcpy(prior_data + frag_delta,
> -				       data, (prior_data_len - frag_delta));
> -				kunmap_atomic(vaddr);
> +
> +				memcpy_from_page(prior_data + frag_delta,
> +						 skb_frag_page(f),
> +						 skb_frag_off(f),
> +						 prior_data_len - frag_delta);
>   			}
>   			/* reset tcp_seq as per the prior_data_required len */
>   			tcp_seq -= prior_data_len;

-- 
This email has been checked for viruses by Avast antivirus software.
www.avast.com

Re: [PATCH v2 net-next 1/6] ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()

[Anirudh Venkataramanan]

On 11/24/2022 2:56 AM, Ayush Sawal wrote:
> 
> On 11/24/2022 2:22 AM, Anirudh Venkataramanan wrote:
>> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
>> the map-memcpy-unmap usage pattern (done using k[un]map_atomic()) with
>> memcpy_from_page(), which internally uses kmap_local_page() and
>> kunmap_local(). This renders the variables 'data' and 'vaddr' 
>> unnecessary,
>> and so remove these too.
>>
>> Note that kmap_atomic() disables preemption and page-fault processing, 
>> but
>> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
>> to check if the code being executed between the map/unmap implicitly
>> depends on page-faults and/or preemption being disabled. If yes, then 
>> code
>> to disable page-faults and/or preemption should also be added for
>> functional correctness. That however doesn't appear to be the case here,
>> so just memcpy_from_page() is used.
>>
>> Also note that the page being mapped is not allocated by the driver, 
>> and so
>> the driver doesn't know if the page is in normal memory. This is the 
>> reason
>> kmap_local_page() is used (via memcpy_from_page()) as opposed to
>> page_address().
>>
>> I don't have hardware, so this change has only been compile tested.
>>
>> Cc: Ayush Sawal <ayush.sawal@chelsio.com>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
>> ---
>> v1 -> v2:
>>   Use memcpy_from_page() as suggested by Fabio
>>   Add a "Suggested-by" tag
>>   Rework commit message
>>   Some emails cc'd in v1 are defunct. Drop them. The MAINTAINERS file for
>>   Chelsio drivers likely needs an update
>> ---
> 
> 
> Thanks for the patch.
> 
> Acked-by: Ayush Sawal <ayush.sawal@chelsio.com>

Thanks Ayush.

Please update the maintainers file for Chelsio drivers?

Ani

Re: [PATCH v2 net-next 1/6] ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:14 CET Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> the map-memcpy-unmap usage pattern (done using k[un]map_atomic()) with
> memcpy_from_page(), which internally uses kmap_local_page() and
> kunmap_local(). This renders the variables 'data' and 'vaddr' unnecessary,
> and so remove these too.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just memcpy_from_page() is used.

Just two marginal notes:

It looks like you are explaining your mental process and teaching how  
developers should approach these kinds of conversions. Although I'm OK with 
this exposition, a plain assurance that the code being executed between the 
map / unmap did not depend on page-faults and/or preemption being disabled 
would have sufficed. :-)

Ira were suggesting to not use "it appears" and replace with stronger and 
clearer assertions like "it is" or "I can confirm that" (or whatever else like  
these).

As said, no problems at all with regard to the overall goodness of this and 
the other patches.

> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used (via memcpy_from_page()) as opposed to
> page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Ayush Sawal <ayush.sawal@chelsio.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> v1 -> v2:
>  Use memcpy_from_page() as suggested by Fabio
>  Add a "Suggested-by" tag
>  Rework commit message
>  Some emails cc'd in v1 are defunct. Drop them. The MAINTAINERS file for
>  Chelsio drivers likely needs an update
> ---
>  .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 26 +++++++++----------
>  1 file changed, 12 insertions(+), 14 deletions(-)

It looks good to me.

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Thanks,

Fabio

> diff --git a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
> b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c index
> da9973b..1a5fdd7 100644
> --- a/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
> +++ b/drivers/net/ethernet/chelsio/inline_crypto/ch_ktls/chcr_ktls.c
> @@ -1839,9 +1839,7 @@ static int chcr_short_record_handler(struct
> chcr_ktls_info *tx_info, */
>  		if (prior_data_len) {
>  			int i = 0;
> -			u8 *data = NULL;
>  			skb_frag_t *f;
> -			u8 *vaddr;
>  			int frag_size = 0, frag_delta = 0;
> 
>  			while (remaining > 0) {
> @@ -1853,24 +1851,24 @@ static int chcr_short_record_handler(struct
> chcr_ktls_info *tx_info, i++;
>  			}
>  			f = &record->frags[i];
> -			vaddr = kmap_atomic(skb_frag_page(f));
> -
> -			data = vaddr + skb_frag_off(f)  + remaining;
>  			frag_delta = skb_frag_size(f) - remaining;
> 
>  			if (frag_delta >= prior_data_len) {
> -				memcpy(prior_data, data, 
prior_data_len);
> -				kunmap_atomic(vaddr);
> +				memcpy_from_page(prior_data, 
skb_frag_page(f),
> +						 skb_frag_off(f) + 
remaining,
> +						 prior_data_len);
>  			} else {
> -				memcpy(prior_data, data, frag_delta);
> -				kunmap_atomic(vaddr);
> +				memcpy_from_page(prior_data, 
skb_frag_page(f),
> +						 skb_frag_off(f) + 
remaining,
> +						 frag_delta);
> +
>  				/* get the next page */
>  				f = &record->frags[i + 1];
> -				vaddr = kmap_atomic(skb_frag_page(f));
> -				data = vaddr + skb_frag_off(f);
> -				memcpy(prior_data + frag_delta,
> -				       data, (prior_data_len - 
frag_delta));
> -				kunmap_atomic(vaddr);
> +
> +				memcpy_from_page(prior_data + 
frag_delta,
> +						 skb_frag_page(f),
> +						 skb_frag_off(f),
> +						 prior_data_len - 
frag_delta);
>  			}
>  			/* reset tcp_seq as per the prior_data_required 
len */
>  			tcp_seq -= prior_data_len;
> --
> 2.37.2

[PATCH v2 net-next 2/6] sfc: Use kmap_local_page() instead of kmap_atomic()

[Anirudh Venkataramanan]

kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
respectively.

Note that kmap_atomic() disables preemption and page-fault processing, but
kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
to check if the code being executed between the map/unmap implicitly
depends on page-faults and/or preemption being disabled. If yes, then code
to disable page-faults and/or preemption should also be added for
functional correctness. That however doesn't appear to be the case here,
so just kmap_local_page() is used.

Also note that the page being mapped is not allocated by the driver, and so
the driver doesn't know if the page is in normal memory. This is the reason
kmap_local_page() is used as opposed to page_address().

I don't have hardware, so this change has only been compile tested.

Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Cc: Edward Cree <ecree.xilinx@gmail.com>
Cc: Martin Habets <habetsm.xilinx@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
v1 -> v2: Update commit message
---
 drivers/net/ethernet/sfc/tx.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
index c5f88f7..4ed4082 100644
--- a/drivers/net/ethernet/sfc/tx.c
+++ b/drivers/net/ethernet/sfc/tx.c
@@ -207,11 +207,11 @@ static void efx_skb_copy_bits_to_pio(struct efx_nic *efx, struct sk_buff *skb,
 		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
 		u8 *vaddr;
 
-		vaddr = kmap_atomic(skb_frag_page(f));
+		vaddr = kmap_local_page(skb_frag_page(f));
 
 		efx_memcpy_toio_aligned_cb(efx, piobuf, vaddr + skb_frag_off(f),
 					   skb_frag_size(f), copy_buf);
-		kunmap_atomic(vaddr);
+		kunmap_local(vaddr);
 	}
 
 	EFX_WARN_ON_ONCE_PARANOID(skb_shinfo(skb)->frag_list);
-- 
2.37.2

Re: [PATCH v2 net-next 2/6] sfc: Use kmap_local_page() instead of kmap_atomic()

[Martin Habets]

On Wed, Nov 23, 2022 at 12:52:15PM -0800, Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
> respectively.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just kmap_local_page() is used.
> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used as opposed to page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Cc: Edward Cree <ecree.xilinx@gmail.com>
> Cc: Martin Habets <habetsm.xilinx@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>

Acked-by: Martin Habets <habetsm.xilinx@gmail.com>

> ---
> v1 -> v2: Update commit message
> ---
>  drivers/net/ethernet/sfc/tx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
> index c5f88f7..4ed4082 100644
> --- a/drivers/net/ethernet/sfc/tx.c
> +++ b/drivers/net/ethernet/sfc/tx.c
> @@ -207,11 +207,11 @@ static void efx_skb_copy_bits_to_pio(struct efx_nic *efx, struct sk_buff *skb,
>  		skb_frag_t *f = &skb_shinfo(skb)->frags[i];
>  		u8 *vaddr;
>  
> -		vaddr = kmap_atomic(skb_frag_page(f));
> +		vaddr = kmap_local_page(skb_frag_page(f));
>  
>  		efx_memcpy_toio_aligned_cb(efx, piobuf, vaddr + skb_frag_off(f),
>  					   skb_frag_size(f), copy_buf);
> -		kunmap_atomic(vaddr);
> +		kunmap_local(vaddr);
>  	}
>  
>  	EFX_WARN_ON_ONCE_PARANOID(skb_shinfo(skb)->frag_list);
> -- 
> 2.37.2

Re: [PATCH v2 net-next 2/6] sfc: Use kmap_local_page() instead of kmap_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:15 CET Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
> respectively.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just kmap_local_page() is used.
> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used as opposed to page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Cc: Edward Cree <ecree.xilinx@gmail.com>
> Cc: Martin Habets <habetsm.xilinx@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> v1 -> v2: Update commit message
> ---
>  drivers/net/ethernet/sfc/tx.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Thanks,

Fabio

> diff --git a/drivers/net/ethernet/sfc/tx.c b/drivers/net/ethernet/sfc/tx.c
> index c5f88f7..4ed4082 100644
> --- a/drivers/net/ethernet/sfc/tx.c
> +++ b/drivers/net/ethernet/sfc/tx.c
> @@ -207,11 +207,11 @@ static void efx_skb_copy_bits_to_pio(struct efx_nic
> *efx, struct sk_buff *skb, skb_frag_t *f = &skb_shinfo(skb)->frags[i];
>  		u8 *vaddr;
> 
> -		vaddr = kmap_atomic(skb_frag_page(f));
> +		vaddr = kmap_local_page(skb_frag_page(f));
> 
>  		efx_memcpy_toio_aligned_cb(efx, piobuf, vaddr + 
skb_frag_off(f),
>  					   skb_frag_size(f), 
copy_buf);
> -		kunmap_atomic(vaddr);
> +		kunmap_local(vaddr);
>  	}
> 
>  	EFX_WARN_ON_ONCE_PARANOID(skb_shinfo(skb)->frag_list);
> --
> 2.37.2

[PATCH v2 net-next 3/6] cassini: Use page_address() instead of kmap_atomic()

[Anirudh Venkataramanan]

Pages for Rx buffers are allocated in cas_page_alloc() using either
GFP_ATOMIC or GFP_KERNEL. Memory allocated with GFP_KERNEL/GFP_ATOMIC can't
come from highmem and so there's no need to kmap() them. Just use
page_address() instead. This makes the variable 'addr' unnecessary, so
remove it too.

Note that kmap_atomic() disables preemption and page-fault processing,
but page_address() doesn't. When removing uses of kmap_atomic(), one has to
check if the code being executed between the map/unmap implicitly depends
on page-faults and/or preemption being disabled. If yes, then code to
disable page-faults and/or preemption should also be added for functional
correctness. That however doesn't appear to be the case here, so just
page_address() is used.

I don't have hardware, so this change has only been compile tested.

Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
v1 -> v2: Update commit message
---
 drivers/net/ethernet/sun/cassini.c | 34 ++++++++++--------------------
 1 file changed, 11 insertions(+), 23 deletions(-)

diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 0aca193..2f66cfc 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -1915,7 +1915,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 	int off, swivel = RX_SWIVEL_OFF_VAL;
 	struct cas_page *page;
 	struct sk_buff *skb;
-	void *addr, *crcaddr;
+	void *crcaddr;
 	__sum16 csum;
 	char *p;
 
@@ -1936,7 +1936,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 	skb_reserve(skb, swivel);
 
 	p = skb->data;
-	addr = crcaddr = NULL;
+	crcaddr = NULL;
 	if (hlen) { /* always copy header pages */
 		i = CAS_VAL(RX_COMP2_HDR_INDEX, words[1]);
 		page = cp->rx_pages[CAS_VAL(RX_INDEX_RING, i)][CAS_VAL(RX_INDEX_NUM, i)];
@@ -1948,12 +1948,10 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 			i += cp->crc_size;
 		dma_sync_single_for_cpu(&cp->pdev->dev, page->dma_addr + off,
 					i, DMA_FROM_DEVICE);
-		addr = cas_page_map(page->buffer);
-		memcpy(p, addr + off, i);
+		memcpy(p, page_address(page->buffer) + off, i);
 		dma_sync_single_for_device(&cp->pdev->dev,
 					   page->dma_addr + off, i,
 					   DMA_FROM_DEVICE);
-		cas_page_unmap(addr);
 		RX_USED_ADD(page, 0x100);
 		p += hlen;
 		swivel = 0;
@@ -1984,12 +1982,11 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 		/* make sure we always copy a header */
 		swivel = 0;
 		if (p == (char *) skb->data) { /* not split */
-			addr = cas_page_map(page->buffer);
-			memcpy(p, addr + off, RX_COPY_MIN);
+			memcpy(p, page_address(page->buffer) + off,
+			       RX_COPY_MIN);
 			dma_sync_single_for_device(&cp->pdev->dev,
 						   page->dma_addr + off, i,
 						   DMA_FROM_DEVICE);
-			cas_page_unmap(addr);
 			off += RX_COPY_MIN;
 			swivel = RX_COPY_MIN;
 			RX_USED_ADD(page, cp->mtu_stride);
@@ -2036,10 +2033,8 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 			RX_USED_ADD(page, hlen + cp->crc_size);
 		}
 
-		if (cp->crc_size) {
-			addr = cas_page_map(page->buffer);
-			crcaddr  = addr + off + hlen;
-		}
+		if (cp->crc_size)
+			crcaddr = page_address(page->buffer) + off + hlen;
 
 	} else {
 		/* copying packet */
@@ -2061,12 +2056,10 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 			i += cp->crc_size;
 		dma_sync_single_for_cpu(&cp->pdev->dev, page->dma_addr + off,
 					i, DMA_FROM_DEVICE);
-		addr = cas_page_map(page->buffer);
-		memcpy(p, addr + off, i);
+		memcpy(p, page_address(page->buffer) + off, i);
 		dma_sync_single_for_device(&cp->pdev->dev,
 					   page->dma_addr + off, i,
 					   DMA_FROM_DEVICE);
-		cas_page_unmap(addr);
 		if (p == (char *) skb->data) /* not split */
 			RX_USED_ADD(page, cp->mtu_stride);
 		else
@@ -2081,20 +2074,17 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 						page->dma_addr,
 						dlen + cp->crc_size,
 						DMA_FROM_DEVICE);
-			addr = cas_page_map(page->buffer);
-			memcpy(p, addr, dlen + cp->crc_size);
+			memcpy(p, page_address(page->buffer), dlen + cp->crc_size);
 			dma_sync_single_for_device(&cp->pdev->dev,
 						   page->dma_addr,
 						   dlen + cp->crc_size,
 						   DMA_FROM_DEVICE);
-			cas_page_unmap(addr);
 			RX_USED_ADD(page, dlen + cp->crc_size);
 		}
 end_copy_pkt:
-		if (cp->crc_size) {
-			addr    = NULL;
+		if (cp->crc_size)
 			crcaddr = skb->data + alloclen;
-		}
+
 		skb_put(skb, alloclen);
 	}
 
@@ -2103,8 +2093,6 @@ static int cas_rx_process_pkt(struct cas *cp, struct cas_rx_comp *rxc,
 		/* checksum includes FCS. strip it out. */
 		csum = csum_fold(csum_partial(crcaddr, cp->crc_size,
 					      csum_unfold(csum)));
-		if (addr)
-			cas_page_unmap(addr);
 	}
 	skb->protocol = eth_type_trans(skb, cp->dev);
 	if (skb->protocol == htons(ETH_P_IP)) {
-- 
2.37.2

Re: [PATCH v2 net-next 3/6] cassini: Use page_address() instead of kmap_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:16 CET Anirudh Venkataramanan wrote:
> Pages for Rx buffers are allocated in cas_page_alloc() using either
> GFP_ATOMIC or GFP_KERNEL. Memory allocated with GFP_KERNEL/GFP_ATOMIC can't
> come from highmem and so there's no need to kmap() them. Just use
> page_address() instead. This makes the variable 'addr' unnecessary, so
> remove it too.
> 
> Note that kmap_atomic() disables preemption and page-fault processing,
> but page_address() doesn't. When removing uses of kmap_atomic(), one has to
> check if the code being executed between the map/unmap implicitly depends
> on page-faults and/or preemption being disabled. If yes, then code to
> disable page-faults and/or preemption should also be added for functional
> correctness. That however doesn't appear to be the case here, so just
> page_address() is used.
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> v1 -> v2: Update commit message
> ---
>  drivers/net/ethernet/sun/cassini.c | 34 ++++++++++--------------------
>  1 file changed, 11 insertions(+), 23 deletions(-)

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Thanks,

Fabio

> diff --git a/drivers/net/ethernet/sun/cassini.c
> b/drivers/net/ethernet/sun/cassini.c index 0aca193..2f66cfc 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -1915,7 +1915,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, int off, swivel = RX_SWIVEL_OFF_VAL;
>  	struct cas_page *page;
>  	struct sk_buff *skb;
> -	void *addr, *crcaddr;
> +	void *crcaddr;
>  	__sum16 csum;
>  	char *p;
> 
> @@ -1936,7 +1936,7 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, skb_reserve(skb, swivel);
> 
>  	p = skb->data;
> -	addr = crcaddr = NULL;
> +	crcaddr = NULL;
>  	if (hlen) { /* always copy header pages */
>  		i = CAS_VAL(RX_COMP2_HDR_INDEX, words[1]);
>  		page = cp->rx_pages[CAS_VAL(RX_INDEX_RING, i)]
[CAS_VAL(RX_INDEX_NUM, i)];
> @@ -1948,12 +1948,10 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, i += cp->crc_size;
>  		dma_sync_single_for_cpu(&cp->pdev->dev, page->dma_addr + 
off,
>  					i, DMA_FROM_DEVICE);
> -		addr = cas_page_map(page->buffer);
> -		memcpy(p, addr + off, i);
> +		memcpy(p, page_address(page->buffer) + off, i);
>  		dma_sync_single_for_device(&cp->pdev->dev,
>  					   page->dma_addr + off, i,
>  					   DMA_FROM_DEVICE);
> -		cas_page_unmap(addr);
>  		RX_USED_ADD(page, 0x100);
>  		p += hlen;
>  		swivel = 0;
> @@ -1984,12 +1982,11 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, /* make sure we always copy a header */
>  		swivel = 0;
>  		if (p == (char *) skb->data) { /* not split */
> -			addr = cas_page_map(page->buffer);
> -			memcpy(p, addr + off, RX_COPY_MIN);
> +			memcpy(p, page_address(page->buffer) + off,
> +			       RX_COPY_MIN);
>  			dma_sync_single_for_device(&cp->pdev->dev,
>  						   page->dma_addr 
+ off, i,
>  						   
DMA_FROM_DEVICE);
> -			cas_page_unmap(addr);
>  			off += RX_COPY_MIN;
>  			swivel = RX_COPY_MIN;
>  			RX_USED_ADD(page, cp->mtu_stride);
> @@ -2036,10 +2033,8 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, RX_USED_ADD(page, hlen + cp->crc_size);
>  		}
> 
> -		if (cp->crc_size) {
> -			addr = cas_page_map(page->buffer);
> -			crcaddr  = addr + off + hlen;
> -		}
> +		if (cp->crc_size)
> +			crcaddr = page_address(page->buffer) + off + 
hlen;
> 
>  	} else {
>  		/* copying packet */
> @@ -2061,12 +2056,10 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, i += cp->crc_size;
>  		dma_sync_single_for_cpu(&cp->pdev->dev, page->dma_addr + 
off,
>  					i, DMA_FROM_DEVICE);
> -		addr = cas_page_map(page->buffer);
> -		memcpy(p, addr + off, i);
> +		memcpy(p, page_address(page->buffer) + off, i);
>  		dma_sync_single_for_device(&cp->pdev->dev,
>  					   page->dma_addr + off, i,
>  					   DMA_FROM_DEVICE);
> -		cas_page_unmap(addr);
>  		if (p == (char *) skb->data) /* not split */
>  			RX_USED_ADD(page, cp->mtu_stride);
>  		else
> @@ -2081,20 +2074,17 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, page->dma_addr,
>  						dlen + cp-
>crc_size,
>  						DMA_FROM_DEVICE);
> -			addr = cas_page_map(page->buffer);
> -			memcpy(p, addr, dlen + cp->crc_size);
> +			memcpy(p, page_address(page->buffer), dlen + cp-
>crc_size);
>  			dma_sync_single_for_device(&cp->pdev->dev,
>  						   page->dma_addr,
>  						   dlen + cp-
>crc_size,
>  						   
DMA_FROM_DEVICE);
> -			cas_page_unmap(addr);
>  			RX_USED_ADD(page, dlen + cp->crc_size);
>  		}
>  end_copy_pkt:
> -		if (cp->crc_size) {
> -			addr    = NULL;
> +		if (cp->crc_size)
>  			crcaddr = skb->data + alloclen;
> -		}
> +
>  		skb_put(skb, alloclen);
>  	}
> 
> @@ -2103,8 +2093,6 @@ static int cas_rx_process_pkt(struct cas *cp, struct
> cas_rx_comp *rxc, /* checksum includes FCS. strip it out. */
>  		csum = csum_fold(csum_partial(crcaddr, cp->crc_size,
>  					      csum_unfold(csum)));
> -		if (addr)
> -			cas_page_unmap(addr);
>  	}
>  	skb->protocol = eth_type_trans(skb, cp->dev);
>  	if (skb->protocol == htons(ETH_P_IP)) {
> --
> 2.37.2

[PATCH v2 net-next 4/6] cassini: Use memcpy_from_page() instead of k[un]map_atomic()

[Anirudh Venkataramanan]

kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
the map-memcpy-unmap usage pattern (done using k[un]map_atomic()) with
memcpy_from_page(), which internally uses kmap_local_page() and
kunmap_local(). This renders the variable 'vaddr' unnecessary, and so
remove this too.

Note that kmap_atomic() disables preemption and page-fault processing, but
kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
to check if the code being executed between the map/unmap implicitly
depends on page-faults and/or preemption being disabled. If yes, then code
to disable page-faults and/or preemption should also be added for
functional correctness. That however doesn't appear to be the case here,
so just memcpy_from_page() is used.

Also note that the page being mapped is not allocated by the driver, and so
the driver doesn't know if the page is in normal memory. This is the reason
kmap_local_page() is used (via memcpy_from_page()) as opposed to
page_address().

I don't have hardware, so this change has only been compile tested.

Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
v1 -> v2:
 Use memcpy_from_page() as suggested by Fabio
 Add a "Suggested-by" tag
 Rework commit message
---
 drivers/net/ethernet/sun/cassini.c | 14 ++++----------
 1 file changed, 4 insertions(+), 10 deletions(-)

diff --git a/drivers/net/ethernet/sun/cassini.c b/drivers/net/ethernet/sun/cassini.c
index 2f66cfc..4ef05ba 100644
--- a/drivers/net/ethernet/sun/cassini.c
+++ b/drivers/net/ethernet/sun/cassini.c
@@ -90,8 +90,6 @@
 #include <linux/uaccess.h>
 #include <linux/jiffies.h>
 
-#define cas_page_map(x)      kmap_atomic((x))
-#define cas_page_unmap(x)    kunmap_atomic((x))
 #define CAS_NCPUS            num_online_cpus()
 
 #define cas_skb_release(x)  netif_rx(x)
@@ -2781,18 +2779,14 @@ static inline int cas_xmit_tx_ringN(struct cas *cp, int ring,
 
 		tabort = cas_calc_tabort(cp, skb_frag_off(fragp), len);
 		if (unlikely(tabort)) {
-			void *addr;
-
 			/* NOTE: len is always > tabort */
 			cas_write_txd(cp, ring, entry, mapping, len - tabort,
 				      ctrl, 0);
 			entry = TX_DESC_NEXT(ring, entry);
-
-			addr = cas_page_map(skb_frag_page(fragp));
-			memcpy(tx_tiny_buf(cp, ring, entry),
-			       addr + skb_frag_off(fragp) + len - tabort,
-			       tabort);
-			cas_page_unmap(addr);
+			memcpy_from_page(tx_tiny_buf(cp, ring, entry),
+					 skb_frag_page(fragp),
+					 skb_frag_off(fragp) + len - tabort,
+					 tabort);
 			mapping = tx_tiny_map(cp, ring, entry, tentry);
 			len     = tabort;
 		}
-- 
2.37.2

Re: [PATCH v2 net-next 4/6] cassini: Use memcpy_from_page() instead of k[un]map_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:17 CET Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> the map-memcpy-unmap usage pattern (done using k[un]map_atomic()) with
> memcpy_from_page(), which internally uses kmap_local_page() and
> kunmap_local(). This renders the variable 'vaddr' unnecessary, and so
> remove this too.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just memcpy_from_page() is used.
> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used (via memcpy_from_page()) as opposed to
> page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> v1 -> v2:
>  Use memcpy_from_page() as suggested by Fabio
>  Add a "Suggested-by" tag
>  Rework commit message
> ---
>  drivers/net/ethernet/sun/cassini.c | 14 ++++----------
>  1 file changed, 4 insertions(+), 10 deletions(-)

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Thanks,

Fabio

> diff --git a/drivers/net/ethernet/sun/cassini.c
> b/drivers/net/ethernet/sun/cassini.c index 2f66cfc..4ef05ba 100644
> --- a/drivers/net/ethernet/sun/cassini.c
> +++ b/drivers/net/ethernet/sun/cassini.c
> @@ -90,8 +90,6 @@
>  #include <linux/uaccess.h>
>  #include <linux/jiffies.h>
> 
> -#define cas_page_map(x)      kmap_atomic((x))
> -#define cas_page_unmap(x)    kunmap_atomic((x))
>  #define CAS_NCPUS            num_online_cpus()
> 
>  #define cas_skb_release(x)  netif_rx(x)
> @@ -2781,18 +2779,14 @@ static inline int cas_xmit_tx_ringN(struct cas *cp,
> int ring,
> 
>  		tabort = cas_calc_tabort(cp, skb_frag_off(fragp), len);
>  		if (unlikely(tabort)) {
> -			void *addr;
> -
>  			/* NOTE: len is always > tabort */
>  			cas_write_txd(cp, ring, entry, mapping, len - 
tabort,
>  				      ctrl, 0);
>  			entry = TX_DESC_NEXT(ring, entry);
> -
> -			addr = cas_page_map(skb_frag_page(fragp));
> -			memcpy(tx_tiny_buf(cp, ring, entry),
> -			       addr + skb_frag_off(fragp) + len - 
tabort,
> -			       tabort);
> -			cas_page_unmap(addr);
> +			memcpy_from_page(tx_tiny_buf(cp, ring, entry),
> +					 skb_frag_page(fragp),
> +					 skb_frag_off(fragp) + len - 
tabort,
> +					 tabort);
>  			mapping = tx_tiny_map(cp, ring, entry, tentry);
>  			len     = tabort;
>  		}
> --
> 2.37.2

[PATCH v2 net-next 5/6] sunvnet: Use kmap_local_page() instead of kmap_atomic()

[Anirudh Venkataramanan]

kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
respectively.

Note that kmap_atomic() disables preemption and page-fault processing, but
kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
to check if the code being executed between the map/unmap implicitly
depends on page-faults and/or preemption being disabled. If yes, then code
to disable page-faults and/or preemption should also be added for
functional correctness. That however doesn't appear to be the case here,
so just kmap_local_page() is used.

Also note that the page being mapped is not allocated by the driver, and so
the driver doesn't know if the page is in normal memory. This is the reason
kmap_local_page() is used as opposed to page_address().

I don't have hardware, so this change has only been compile tested.

Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
v1 -> v2: Update commit message
---
 drivers/net/ethernet/sun/sunvnet_common.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/net/ethernet/sun/sunvnet_common.c b/drivers/net/ethernet/sun/sunvnet_common.c
index 80fde5f..a6211b9 100644
--- a/drivers/net/ethernet/sun/sunvnet_common.c
+++ b/drivers/net/ethernet/sun/sunvnet_common.c
@@ -1085,13 +1085,13 @@ static inline int vnet_skb_map(struct ldc_channel *lp, struct sk_buff *skb,
 		u8 *vaddr;
 
 		if (nc < ncookies) {
-			vaddr = kmap_atomic(skb_frag_page(f));
+			vaddr = kmap_local_page(skb_frag_page(f));
 			blen = skb_frag_size(f);
 			blen += 8 - (blen & 7);
 			err = ldc_map_single(lp, vaddr + skb_frag_off(f),
 					     blen, cookies + nc, ncookies - nc,
 					     map_perm);
-			kunmap_atomic(vaddr);
+			kunmap_local(vaddr);
 		} else {
 			err = -EMSGSIZE;
 		}
-- 
2.37.2

Re: [PATCH v2 net-next 5/6] sunvnet: Use kmap_local_page() instead of kmap_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:18 CET Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
> respectively.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just kmap_local_page() is used.
> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used as opposed to page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> v1 -> v2: Update commit message
> ---
>  drivers/net/ethernet/sun/sunvnet_common.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Thanks,

Fabio

> diff --git a/drivers/net/ethernet/sun/sunvnet_common.c
> b/drivers/net/ethernet/sun/sunvnet_common.c index 80fde5f..a6211b9 100644
> --- a/drivers/net/ethernet/sun/sunvnet_common.c
> +++ b/drivers/net/ethernet/sun/sunvnet_common.c
> @@ -1085,13 +1085,13 @@ static inline int vnet_skb_map(struct ldc_channel 
*lp,
> struct sk_buff *skb, u8 *vaddr;
> 
>  		if (nc < ncookies) {
> -			vaddr = kmap_atomic(skb_frag_page(f));
> +			vaddr = kmap_local_page(skb_frag_page(f));
>  			blen = skb_frag_size(f);
>  			blen += 8 - (blen & 7);
>  			err = ldc_map_single(lp, vaddr + 
skb_frag_off(f),
>  					     blen, cookies + nc, 
ncookies - nc,
>  					     map_perm);
> -			kunmap_atomic(vaddr);
> +			kunmap_local(vaddr);
>  		} else {
>  			err = -EMSGSIZE;
>  		}
> --
> 2.37.2

[PATCH v2 net-next 6/6] net: thunderbolt: Use kmap_local_page() instead of kmap_atomic()

[Anirudh Venkataramanan]

kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
respectively.

Note that kmap_atomic() disables preemption and page-fault processing, but
kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
to check if the code being executed between the map/unmap implicitly
depends on page-faults and/or preemption being disabled. If yes, then code
to disable page-faults and/or preemption should also be added for
functional correctness. That however doesn't appear to be the case here,
so just kmap_local_page() is used.

Also note that the page being mapped is not allocated by the driver, and so
the driver doesn't know if the page is in normal memory. This is the reason
kmap_local_page() is used as opposed to page_address().

I don't have hardware, so this change has only been compile tested.

Cc: Michael Jamet <michael.jamet@intel.com>
Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
Cc: Yehezkel Bernat <YehezkelShB@gmail.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
 drivers/net/thunderbolt.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
index a52ee2b..b20cd37 100644
--- a/drivers/net/thunderbolt.c
+++ b/drivers/net/thunderbolt.c
@@ -1051,7 +1051,7 @@ static void *tbnet_kmap_frag(struct sk_buff *skb, unsigned int frag_num,
 	const skb_frag_t *frag = &skb_shinfo(skb)->frags[frag_num];
 
 	*len = skb_frag_size(frag);
-	return kmap_atomic(skb_frag_page(frag)) + skb_frag_off(frag);
+	return kmap_local_page(skb_frag_page(frag)) + skb_frag_off(frag);
 }
 
 static netdev_tx_t tbnet_start_xmit(struct sk_buff *skb,
@@ -1109,7 +1109,7 @@ static netdev_tx_t tbnet_start_xmit(struct sk_buff *skb,
 			dest += len;
 
 			if (unmap) {
-				kunmap_atomic(src);
+				kunmap_local(src);
 				unmap = false;
 			}
 
@@ -1147,7 +1147,7 @@ static netdev_tx_t tbnet_start_xmit(struct sk_buff *skb,
 		dest += len;
 
 		if (unmap) {
-			kunmap_atomic(src);
+			kunmap_local(src);
 			unmap = false;
 		}
 
@@ -1162,7 +1162,7 @@ static netdev_tx_t tbnet_start_xmit(struct sk_buff *skb,
 	memcpy(dest, src, data_len);
 
 	if (unmap)
-		kunmap_atomic(src);
+		kunmap_local(src);
 
 	if (!tbnet_xmit_csum_and_map(net, skb, frames, frame_index + 1))
 		goto err_drop;
-- 
2.37.2

Re: [PATCH v2 net-next 6/6] net: thunderbolt: Use kmap_local_page() instead of kmap_atomic()

[Mika Westerberg]

On Wed, Nov 23, 2022 at 12:52:19PM -0800, Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
> respectively.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just kmap_local_page() is used.
> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used as opposed to page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Michael Jamet <michael.jamet@intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>

Acked-by: Mika Westerberg <mika.westerberg@linux.intel.com>

Re: [PATCH v2 net-next 6/6] net: thunderbolt: Use kmap_local_page() instead of kmap_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:19 CET Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated in favor of kmap_local_page(). Replace
> kmap_atomic() and kunmap_atomic() with kmap_local_page() and kunmap_local()
> respectively.
> 
> Note that kmap_atomic() disables preemption and page-fault processing, but
> kmap_local_page() doesn't. When converting uses of kmap_atomic(), one has
> to check if the code being executed between the map/unmap implicitly
> depends on page-faults and/or preemption being disabled. If yes, then code
> to disable page-faults and/or preemption should also be added for
> functional correctness. That however doesn't appear to be the case here,
> so just kmap_local_page() is used.
> 
> Also note that the page being mapped is not allocated by the driver, and so
> the driver doesn't know if the page is in normal memory. This is the reason
> kmap_local_page() is used as opposed to page_address().
> 
> I don't have hardware, so this change has only been compile tested.
> 
> Cc: Michael Jamet <michael.jamet@intel.com>
> Cc: Mika Westerberg <mika.westerberg@linux.intel.com>
> Cc: Yehezkel Bernat <YehezkelShB@gmail.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
>  drivers/net/thunderbolt.c | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)

Reviewed-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>

Thanks,

Fabio

> diff --git a/drivers/net/thunderbolt.c b/drivers/net/thunderbolt.c
> index a52ee2b..b20cd37 100644
> --- a/drivers/net/thunderbolt.c
> +++ b/drivers/net/thunderbolt.c
> @@ -1051,7 +1051,7 @@ static void *tbnet_kmap_frag(struct sk_buff *skb,
> unsigned int frag_num, const skb_frag_t *frag =
> &skb_shinfo(skb)->frags[frag_num];
> 
>  	*len = skb_frag_size(frag);
> -	return kmap_atomic(skb_frag_page(frag)) + skb_frag_off(frag);
> +	return kmap_local_page(skb_frag_page(frag)) + skb_frag_off(frag);
>  }
> 
>  static netdev_tx_t tbnet_start_xmit(struct sk_buff *skb,
> @@ -1109,7 +1109,7 @@ static netdev_tx_t tbnet_start_xmit(struct sk_buff 
*skb,
> dest += len;
> 
>  			if (unmap) {
> -				kunmap_atomic(src);
> +				kunmap_local(src);
>  				unmap = false;
>  			}
> 
> @@ -1147,7 +1147,7 @@ static netdev_tx_t tbnet_start_xmit(struct sk_buff 
*skb,
> dest += len;
> 
>  		if (unmap) {
> -			kunmap_atomic(src);
> +			kunmap_local(src);
>  			unmap = false;
>  		}
> 
> @@ -1162,7 +1162,7 @@ static netdev_tx_t tbnet_start_xmit(struct sk_buff 
*skb,
> memcpy(dest, src, data_len);
> 
>  	if (unmap)
> -		kunmap_atomic(src);
> +		kunmap_local(src);
> 
>  	if (!tbnet_xmit_csum_and_map(net, skb, frames, frame_index + 1))
>  		goto err_drop;
> --
> 2.37.2

Re: [PATCH v2 net-next 0/6] Remove uses of kmap_atomic()

[patchwork-bot+netdevbpf]

Hello:

This series was applied to netdev/net-next.git (master)
by David S. Miller <davem@davemloft.net>:

On Wed, 23 Nov 2022 12:52:13 -0800 you wrote:
> kmap_atomic() is being deprecated. This little series replaces the last
> few uses of kmap_atomic() in the networking subsystem.
> 
> This series triggered a suggestion [1] that perhaps the Sun Cassini,
> LDOM Virtual Switch Driver and the LDOM virtual network drivers should be
> removed completely. I plan to do this in a follow up patchset. For
> completeness, this series still includes kmap_atomic() conversions that
> apply to the above referenced drivers. If for some reason we choose to not
> remove these drivers, at least they won't be using kmap_atomic() anymore.
> 
> [...]

Here is the summary with links:
  - [v2,net-next,1/6] ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()
    https://git.kernel.org/netdev/net-next/c/51337ef07a40
  - [v2,net-next,2/6] sfc: Use kmap_local_page() instead of kmap_atomic()
    https://git.kernel.org/netdev/net-next/c/f61e6d3ca4da
  - [v2,net-next,3/6] cassini: Use page_address() instead of kmap_atomic()
    https://git.kernel.org/netdev/net-next/c/c191445874bb
  - [v2,net-next,4/6] cassini: Use memcpy_from_page() instead of k[un]map_atomic()
    https://git.kernel.org/netdev/net-next/c/e3128591b55a
  - [v2,net-next,5/6] sunvnet: Use kmap_local_page() instead of kmap_atomic()
    https://git.kernel.org/netdev/net-next/c/350d351389e9
  - [v2,net-next,6/6] net: thunderbolt: Use kmap_local_page() instead of kmap_atomic()
    https://git.kernel.org/netdev/net-next/c/c3a8d375f3b9

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html

Re: [PATCH v2 net-next 0/6] Remove uses of kmap_atomic()

[Fabio M. De Francesco]

On mercoledì 23 novembre 2022 21:52:13 CET Anirudh Venkataramanan wrote:
> kmap_atomic() is being deprecated. This little series replaces the last
> few uses of kmap_atomic() in the networking subsystem.
> 
> This series triggered a suggestion [1] that perhaps the Sun Cassini,
> LDOM Virtual Switch Driver and the LDOM virtual network drivers should be
> removed completely. I plan to do this in a follow up patchset. For
> completeness, this series still includes kmap_atomic() conversions that
> apply to the above referenced drivers. If for some reason we choose to not
> remove these drivers, at least they won't be using kmap_atomic() anymore.
> 
> Also, the following maintainer entries for the Chelsio driver seem to be
> defunct:
> 
>   Vinay Kumar Yadav <vinay.yadav@chelsio.com>
>   Rohit Maheshwari <rohitm@chelsio.com>
> 
> I can submit a follow up patch to remove these entries, but thought
> maybe the folks over at Chelsio would want to look into this first.
> 
> Changes v1 -> v2:
>   Use memcpy_from_page() in patches 2/6 and 4/6
>   Add new patch for the thunderbolt driver
>   Update commit messages and cover letter
> 
> [1]
> https://lore.kernel.org/netdev/99629223-ac1b-0f82-50b8-ea307b3b0197@intel.com
> /T/#m3da3759652a48f958ab852fa5499009b43ff8fdd
> 
> Anirudh Venkataramanan (6):
>   ch_ktls: Use memcpy_from_page() instead of k[un]map_atomic()
>   sfc: Use kmap_local_page() instead of kmap_atomic()
>   cassini: Use page_address() instead of kmap_atomic()
>   cassini: Use memcpy_from_page() instead of k[un]map_atomic()
>   sunvnet: Use kmap_local_page() instead of kmap_atomic()
>   net: thunderbolt: Use kmap_local_page() instead of kmap_atomic()
> 
>  .../chelsio/inline_crypto/ch_ktls/chcr_ktls.c | 26 +++++-----
>  drivers/net/ethernet/sfc/tx.c                 |  4 +-
>  drivers/net/ethernet/sun/cassini.c            | 48 ++++++-------------
>  drivers/net/ethernet/sun/sunvnet_common.c     |  4 +-
>  drivers/net/thunderbolt.c                     |  8 ++--
>  5 files changed, 35 insertions(+), 55 deletions(-)
> 
> 
> base-commit: e80bd08fd75a644e2337fb535c1afdb6417357ff
> --
> 2.37.2

I noticed too late that your patches were already applied. The message from 
the patchwork bot was the latest. I'm sorry for the noise: my tags were 
unnecessary but I didn't yet know :-(

However, again thanks for helping with these conversions,

Fabio