Message ID | 20200728190842.1284145-3-anthony.l.nguyen@intel.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | 40GbE Intel Wired LAN Driver Updates 2020-07-28 | expand |
On Tue, 28 Jul 2020 12:08:38 -0700 Tony Nguyen wrote: > From: Li RongQing <lirongqing@baidu.com> > > page_address() accesses struct page only when WANT_PAGE_VIRTUAL > or HASHED_PAGE_VIRTUAL is defined, otherwise it returns address > based on offset, so we prefetch it conditionally > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> > --- > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > index 3e5c566ceb01..5d408fe26063 100644 > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > @@ -1953,7 +1953,9 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring, > struct i40e_rx_buffer *rx_buffer; > > rx_buffer = i40e_rx_bi(rx_ring, rx_ring->next_to_clean); > +#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL) > prefetchw(rx_buffer->page); > +#endif Looks like something that belongs in a common header not (potentially multiple) C sources.
> -----邮件原件----- > 发件人: Jakub Kicinski [mailto:kuba@kernel.org] > 发送时间: 2020年7月29日 4:14 > 收件人: Tony Nguyen <anthony.l.nguyen@intel.com> > 抄送: davem@davemloft.net; Li,Rongqing <lirongqing@baidu.com>; > netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; > jeffrey.t.kirsher@intel.com; Andrew Bowers <andrewx.bowers@intel.com> > 主题: Re: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally > > On Tue, 28 Jul 2020 12:08:38 -0700 Tony Nguyen wrote: > > From: Li RongQing <lirongqing@baidu.com> > > > > page_address() accesses struct page only when WANT_PAGE_VIRTUAL or > > HASHED_PAGE_VIRTUAL is defined, otherwise it returns address based on > > offset, so we prefetch it conditionally > > > > Signed-off-by: Li RongQing <lirongqing@baidu.com> > > Tested-by: Andrew Bowers <andrewx.bowers@intel.com> > > Signed-off-by: Tony Nguyen <anthony.l.nguyen@intel.com> > > --- > > drivers/net/ethernet/intel/i40e/i40e_txrx.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c > > b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > > index 3e5c566ceb01..5d408fe26063 100644 > > --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c > > +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c > > @@ -1953,7 +1953,9 @@ static struct i40e_rx_buffer > *i40e_get_rx_buffer(struct i40e_ring *rx_ring, > > struct i40e_rx_buffer *rx_buffer; > > > > rx_buffer = i40e_rx_bi(rx_ring, rx_ring->next_to_clean); > > +#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL) > > prefetchw(rx_buffer->page); Maybe I change prefetchw to prefetch. refcount of rx_buffer page will be added here originally, so prefetchw is needed, but after commit 1793668c3b8c ("i40e/i40evf: Update code to better handle incrementing page count"), and refcount is not added everytime, so change prefetchw as prefetch, > > +#endif > > Looks like something that belongs in a common header not (potentially > multiple) C sources. Not clear, how should I change? -Li
On Wed, 29 Jul 2020 06:20:47 +0000 Li,Rongqing wrote: > > Looks like something that belongs in a common header not (potentially > > multiple) C sources. > > Not clear, how should I change? Can you add something like: static inline void prefetch_page_address(struct page *page) { #if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL) prefetch(page); #endif } to mm.h or prefetch.h or i40e.h or some other header? That's preferred over adding #ifs directly in the source code.
> -----邮件原件----- > 发件人: Jakub Kicinski [mailto:kuba@kernel.org] > 发送时间: 2020年7月30日 5:20 > 收件人: Li,Rongqing <lirongqing@baidu.com> > 抄送: Tony Nguyen <anthony.l.nguyen@intel.com>; davem@davemloft.net; > netdev@vger.kernel.org; nhorman@redhat.com; sassmann@redhat.com; > jeffrey.t.kirsher@intel.com; Andrew Bowers <andrewx.bowers@intel.com> > 主题: Re: [net-next 2/6] i40e: prefetch struct page of Rx buffer conditionally > > On Wed, 29 Jul 2020 06:20:47 +0000 Li,Rongqing wrote: > > > Looks like something that belongs in a common header not > > > (potentially > > > multiple) C sources. > > > > Not clear, how should I change? > > Can you add something like: > > static inline void prefetch_page_address(struct page *page) { #if > defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL) > prefetch(page); > #endif > } > > to mm.h or prefetch.h or i40e.h or some other header? That's preferred over > adding #ifs directly in the source code. Thanks, I will send a new version -Li
diff --git a/drivers/net/ethernet/intel/i40e/i40e_txrx.c b/drivers/net/ethernet/intel/i40e/i40e_txrx.c index 3e5c566ceb01..5d408fe26063 100644 --- a/drivers/net/ethernet/intel/i40e/i40e_txrx.c +++ b/drivers/net/ethernet/intel/i40e/i40e_txrx.c @@ -1953,7 +1953,9 @@ static struct i40e_rx_buffer *i40e_get_rx_buffer(struct i40e_ring *rx_ring, struct i40e_rx_buffer *rx_buffer; rx_buffer = i40e_rx_bi(rx_ring, rx_ring->next_to_clean); +#if defined(WANT_PAGE_VIRTUAL) || defined(HASHED_PAGE_VIRTUAL) prefetchw(rx_buffer->page); +#endif /* we are reusing so sync this buffer for CPU use */ dma_sync_single_range_for_cpu(rx_ring->dev,