* [Intel-wired-lan] [PATCH net-next] ixgbe: Remove unnecessary use of kmap()
@ 2022-09-28 19:27 Anirudh Venkataramanan
2022-09-28 20:40 ` Alexander Duyck
0 siblings, 1 reply; 3+ messages in thread
From: Anirudh Venkataramanan @ 2022-09-28 19:27 UTC (permalink / raw)
To: intel-wired-lan; +Cc: Fabio M . De Francesco, Ira Weiny
Pages for Rx are allocated in ixgbe_alloc_mapped_page() using
dev_alloc_pages(), which does the allocation using flags GFP_ATOMIC |
__GFP_NOWARN. Pages allocated thus can't come from highmem, so remove
calls to kmap() and kunmap().
While here, also remove the local variable "match", and just return
true/false.
I wasn't able to get a hold of a system with an ixgbe NIC to test this
change. This is a pretty trivial change and I don't expect anything to
break, but a "Tested-by" from someone who has the hardware would be nice.
Cc: Alexander Duyck <alexander.duyck@gmail.com>
Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
Cc: Ira Weiny <ira.weiny@intel.com>
Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
Suggested-by: Ira Weiny <ira.weiny@intel.com>
Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
---
drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 9 +++------
1 file changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
index 04f453e..90c0e0b 100644
--- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
+++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
@@ -1960,20 +1960,17 @@ static bool ixgbe_check_lbtest_frame(struct ixgbe_rx_buffer *rx_buffer,
unsigned int frame_size)
{
unsigned char *data;
- bool match = true;
frame_size >>= 1;
- data = kmap(rx_buffer->page) + rx_buffer->page_offset;
+ data = page_address(rx_buffer->page) + rx_buffer->page_offset;
if (data[3] != 0xFF ||
data[frame_size + 10] != 0xBE ||
data[frame_size + 12] != 0xAF)
- match = false;
-
- kunmap(rx_buffer->page);
+ return false;
- return match;
+ return true;
}
static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
--
2.37.2
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove unnecessary use of kmap()
2022-09-28 19:27 [Intel-wired-lan] [PATCH net-next] ixgbe: Remove unnecessary use of kmap() Anirudh Venkataramanan
@ 2022-09-28 20:40 ` Alexander Duyck
2022-09-28 22:22 ` Anirudh Venkataramanan
0 siblings, 1 reply; 3+ messages in thread
From: Alexander Duyck @ 2022-09-28 20:40 UTC (permalink / raw)
To: Anirudh Venkataramanan; +Cc: Ira Weiny, Fabio M . De Francesco, intel-wired-lan
On Wed, Sep 28, 2022 at 12:26 PM Anirudh Venkataramanan
<anirudh.venkataramanan@intel.com> wrote:
>
> Pages for Rx are allocated in ixgbe_alloc_mapped_page() using
> dev_alloc_pages(), which does the allocation using flags GFP_ATOMIC |
> __GFP_NOWARN. Pages allocated thus can't come from highmem, so remove
> calls to kmap() and kunmap().
>
> While here, also remove the local variable "match", and just return
> true/false.
>
> I wasn't able to get a hold of a system with an ixgbe NIC to test this
> change. This is a pretty trivial change and I don't expect anything to
> break, but a "Tested-by" from someone who has the hardware would be nice.
>
> Cc: Alexander Duyck <alexander.duyck@gmail.com>
> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
> Cc: Ira Weiny <ira.weiny@intel.com>
> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
> ---
> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 9 +++------
> 1 file changed, 3 insertions(+), 6 deletions(-)
>
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 04f453e..90c0e0b 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -1960,20 +1960,17 @@ static bool ixgbe_check_lbtest_frame(struct ixgbe_rx_buffer *rx_buffer,
> unsigned int frame_size)
> {
> unsigned char *data;
> - bool match = true;
>
> frame_size >>= 1;
>
> - data = kmap(rx_buffer->page) + rx_buffer->page_offset;
> + data = page_address(rx_buffer->page) + rx_buffer->page_offset;
>
> if (data[3] != 0xFF ||
> data[frame_size + 10] != 0xBE ||
> data[frame_size + 12] != 0xAF)
> - match = false;
> -
> - kunmap(rx_buffer->page);
> + return false;
>
> - return match;
> + return true;
> }
>
> static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
Rather than bothering with the return true/false why not just return
the compare value? So it would be:
return data[3] == 0xFF && data[frame_size + 10] == 0xBE &&
data[frame_size + 12] == 0xAF;
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [Intel-wired-lan] [PATCH net-next] ixgbe: Remove unnecessary use of kmap()
2022-09-28 20:40 ` Alexander Duyck
@ 2022-09-28 22:22 ` Anirudh Venkataramanan
0 siblings, 0 replies; 3+ messages in thread
From: Anirudh Venkataramanan @ 2022-09-28 22:22 UTC (permalink / raw)
To: Alexander Duyck; +Cc: Ira Weiny, Fabio M . De Francesco, intel-wired-lan
On 9/28/2022 1:40 PM, Alexander Duyck wrote:
> On Wed, Sep 28, 2022 at 12:26 PM Anirudh Venkataramanan
> <anirudh.venkataramanan@intel.com> wrote:
>>
>> Pages for Rx are allocated in ixgbe_alloc_mapped_page() using
>> dev_alloc_pages(), which does the allocation using flags GFP_ATOMIC |
>> __GFP_NOWARN. Pages allocated thus can't come from highmem, so remove
>> calls to kmap() and kunmap().
>>
>> While here, also remove the local variable "match", and just return
>> true/false.
>>
>> I wasn't able to get a hold of a system with an ixgbe NIC to test this
>> change. This is a pretty trivial change and I don't expect anything to
>> break, but a "Tested-by" from someone who has the hardware would be nice.
>>
>> Cc: Alexander Duyck <alexander.duyck@gmail.com>
>> Cc: Jesse Brandeburg <jesse.brandeburg@intel.com>
>> Cc: Ira Weiny <ira.weiny@intel.com>
>> Cc: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> Cc: Tony Nguyen <anthony.l.nguyen@intel.com>
>> Suggested-by: Ira Weiny <ira.weiny@intel.com>
>> Suggested-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
>> Signed-off-by: Anirudh Venkataramanan <anirudh.venkataramanan@intel.com>
>> ---
>> drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 9 +++------
>> 1 file changed, 3 insertions(+), 6 deletions(-)
>>
>> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> index 04f453e..90c0e0b 100644
>> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
>> @@ -1960,20 +1960,17 @@ static bool ixgbe_check_lbtest_frame(struct ixgbe_rx_buffer *rx_buffer,
>> unsigned int frame_size)
>> {
>> unsigned char *data;
>> - bool match = true;
>>
>> frame_size >>= 1;
>>
>> - data = kmap(rx_buffer->page) + rx_buffer->page_offset;
>> + data = page_address(rx_buffer->page) + rx_buffer->page_offset;
Okay, Fabio has already made this exact change and it's currently in
Tony's kernel tree (next-queue:dev-queue). I missed it because I was
barking up the wrong (kernel) tree. My apologies for the thrash.
>>
>> if (data[3] != 0xFF ||
>> data[frame_size + 10] != 0xBE ||
>> data[frame_size + 12] != 0xAF)
>> - match = false;
>> -
>> - kunmap(rx_buffer->page);
>> + return false;
>>
>> - return match;
>> + return true;
>> }
>>
>> static u16 ixgbe_clean_test_rings(struct ixgbe_ring *rx_ring,
>
> Rather than bothering with the return true/false why not just return
> the compare value? So it would be:
> return data[3] == 0xFF && data[frame_size + 10] == 0xBE &&
> data[frame_size + 12] == 0xAF;
Sure, I can do a standalone patch for this.
Ani
_______________________________________________
Intel-wired-lan mailing list
Intel-wired-lan@osuosl.org
https://lists.osuosl.org/mailman/listinfo/intel-wired-lan
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-09-28 22:24 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-28 19:27 [Intel-wired-lan] [PATCH net-next] ixgbe: Remove unnecessary use of kmap() Anirudh Venkataramanan
2022-09-28 20:40 ` Alexander Duyck
2022-09-28 22:22 ` Anirudh Venkataramanan
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.