Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
Date:   Tue, 5 Jul 2022 11:21:59 -0700
From:   Ira Weiny <ira.weiny@intel.com>
To:     "Fabio M. De Francesco" <fmdefrancesco@gmail.com>
CC:     Jesse Brandeburg <jesse.brandeburg@intel.com>,
        Tony Nguyen <anthony.l.nguyen@intel.com>,
        "David S. Miller" <davem@davemloft.net>,
        "Eric Dumazet" <edumazet@google.com>,
        Jakub Kicinski <kuba@kernel.org>,
        Paolo Abeni <pabeni@redhat.com>,
        Alexei Starovoitov <ast@kernel.org>,
        Daniel Borkmann <daniel@iogearbox.net>,
        Jesper Dangaard Brouer <hawk@kernel.org>,
        "John Fastabend" <john.fastabend@gmail.com>,
        <intel-wired-lan@lists.osuosl.org>, <netdev@vger.kernel.org>,
        <linux-kernel@vger.kernel.org>, <bpf@vger.kernel.org>,
        Alexander Duyck <alexander.duyck@gmail.com>
Subject: Re: [PATCH] ixgbe: Don't call kmap() on page allocated with
 GFP_ATOMIC
Message-ID: <YsSBR5nJovFMHGcB@iweiny-desk3>
References: <20220704140129.6463-1-fmdefrancesco@gmail.com>
Content-Type: text/plain; charset="us-ascii"
Content-Disposition: inline
In-Reply-To: <20220704140129.6463-1-fmdefrancesco@gmail.com>
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?us-ascii?Q?EIYyKp/A0C5lOkLEzgtBLLRZ6f7wlFtk1qUdKIZaLaE/av2ZrAA+LFLeXMoq?=
 =?us-ascii?Q?U34evaYna43DuAWz22tipT+FAUVz2s9bsn+l8w/1bvUHjmJRcj5tfry+rLfy?=
 =?us-ascii?Q?0aBgdC4bo3GpQ17iQU6FHfJDTuAdixotGOGLzPUOaNqDE56lcAnHSM/Am5Rv?=
 =?us-ascii?Q?4lDbcvZO4rMGa0LkfvIAttKfSHEo6U2JYqA2dbmGRCqlKRQtMW52H1gtr8XP?=
 =?us-ascii?Q?83lwNCaL2bWPMSWqXiNqpHwAXlegL1scCnbdOdOh/24S+jd1RilIlOtrHB88?=
 =?us-ascii?Q?DBsJzCHaKTGK47DLfml34P76cnuwZDxy1rtxCuPGNn5H+Q2X9F2Ai3byEiGS?=
 =?us-ascii?Q?vOcmj2beqq3Q4w4Dk1rUXRmG/JwgBYVaD2BrJkpCcBzLPzccnTQ7oYv3mqvl?=
 =?us-ascii?Q?QLKMoUYjkja+hygf54/vQ4c8yE9iR7AvG2Z3RPFdeBz5AthmL1aiJm3pxdi1?=
 =?us-ascii?Q?8GU9c3867zV2JfxKXicZv2racTnwrXNDHV5SI7DO2T5gtfb1eG108yqpH7Zg?=
 =?us-ascii?Q?gIXaiMvPPs6rKtFdBOhqYLWYiWgB1olU8m+7fyVthfS+R/FACH+DfNkIunm0?=
 =?us-ascii?Q?rKj9R0etEVo2XwRCVqLBCvwOoIUzyOLDSvplq8x2LZuTzkK1i/DHXdPljUu/?=
 =?us-ascii?Q?8JtWBiGpldz9sPonCO0qAseiKuTU1rob1RPI4iHkauKx8Ahks/Z+VZng3taY?=
 =?us-ascii?Q?oyjISJ8QBQ+dwZKZdyVwthX5Y0UHr1hiQPma6mdxjF2mW/F223X9MFU4E0mu?=
 =?us-ascii?Q?iZ5SSWVaSc2bDe5rcoBMbXPS33OFd3UW479p93e9Pd802GFWAXXyYRF7vaar?=
 =?us-ascii?Q?LsN2nuc6+GFd65vt/cPbtHAqvIInvNrucLHsca3CygqeqV01RP4J7H6o3Ij7?=
 =?us-ascii?Q?2UvJRATtSgd47SnoxoobfDfTRBw7IpRGB2FH5Fx+HtGSejDApa6UZsAXLqcu?=
 =?us-ascii?Q?oIrlcrb7c/GTWe/dIUK5AiASDa5i1fQfsULUsAPICzlJZ2l3C7NU3OInEGos?=
 =?us-ascii?Q?tSerRKA2QwZ9bW0d873lT6G98kRMCwJ4TcZfTo2jh8UpZX7f8e8yjduBxYTn?=
 =?us-ascii?Q?zjO3799L1huX6Ys7zEUafioYjL4duaRGG9Tjmv/0qRriGJA4OPRoSkqOJmbG?=
 =?us-ascii?Q?TnBZ9NFGSsWUsnt2rCRmM/A2qShDsl8kd9TiuLrnfKdT9GguC/BeTGPKc0Ku?=
 =?us-ascii?Q?XUmoVdAeAA1YyZFaAExeaalCDCOiuIjr7pV3YMLeRYJ8SaNXYUSc0Fe5uCB7?=
 =?us-ascii?Q?cVOp2JHcHoHxHjHXVhmMzS7frrhq26KrrTPYidt7gjtDkzqnSQAI99hfuuS8?=
 =?us-ascii?Q?stE1/0YYl/rFOXNULgN2IMmXOo7t0IQCzdwB9z/y1eMPp0boC1wreBGQNwTN?=
 =?us-ascii?Q?W9QMjrH9uMwkZZiTPBZ+XhW/6E2nIHZ9R5b7ZnHoBwnhGYFHBSgYAcZ1dZBB?=
 =?us-ascii?Q?IYvTGUPGSkf3gAtz8bxroRmS96x/YEizybZTy1ybECzolhHx/IoUrRNL6JdS?=
 =?us-ascii?Q?dAbZgEVOVJZLSBl28oCW2EaZ/TLD7Echnyy2xc2ZAaX8zIVn3rQf8U75qTIg?=
 =?us-ascii?Q?RLE+tKiAWpbrmzLYRUYdr0ZtL61UNPT7QbBnoA6EbevLLMuoQawItb9vjAR5?=
 =?us-ascii?Q?PQHGjk6/Wdyvpg69YK1jI7lq9CTYnRhzamVzilSvJWiV?=
X-MS-Exchange-CrossTenant-Network-Message-Id: 2d933dc5-8c89-4b51-c902-08da5eb3458b
X-MS-Exchange-CrossTenant-AuthSource: DM4PR11MB6311.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Jul 2022 18:22:08.7635
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: NW/hgAuzwLByyKKMkWAGrHXYO0VAK3/XfIb+2oqcvr7Pic3MkQJG/ktqPYOo11+O1F1SB5Y1ov8wiqmsTq0+aw==
X-MS-Exchange-Transport-CrossTenantHeadersStamped: CH0PR11MB5444
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-5.1 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
        DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED,
        RCVD_IN_MSPIKE_H3,RCVD_IN_MSPIKE_WL,SPF_HELO_NONE,SPF_NONE,
        T_SCC_BODY_TEXT_LINE autolearn=ham autolearn_force=no version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
        lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

On Mon, Jul 04, 2022 at 04:01:29PM +0200, Fabio M. De Francesco wrote:
> Pages allocated with GFP_ATOMIC cannot come from Highmem. This is why
> there is no need to call kmap() on them.

I'm still not 100% sure where this page gets allocated but AFAICT it is
allocated in ixgbe_alloc_mapped_page() which calls dev_alloc_pages() for the
allocation which is where the GFP_ATOMIC is specified.

I think I would add this detail here.

That said, and assuming my analysis is correct, the code looks fine so:

Reviewed-by: Ira Weiny <ira.weiny@intel.com>

> 
> Therefore, don't call kmap() on rx_buffer->page() and instead use a
> plain page_address() to get the kernel address.
> 
> Suggested-by: Ira Weiny <ira.weiny@intel.com>
> Suggested-by: Alexander Duyck <alexander.duyck@gmail.com>
> Signed-off-by: Fabio M. De Francesco <fmdefrancesco@gmail.com>
> ---
>  drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c | 4 +---
>  1 file changed, 1 insertion(+), 3 deletions(-)
> 
> diff --git a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> index 628d0eb0599f..71196fd92f81 100644
> --- a/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> +++ b/drivers/net/ethernet/intel/ixgbe/ixgbe_ethtool.c
> @@ -1966,15 +1966,13 @@ static bool ixgbe_check_lbtest_frame(struct ixgbe_rx_buffer *rx_buffer,
>  
>  	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 match;
>  }
>  
> -- 
> 2.36.1
>