| Message ID | 20230906141411.121142-1-poros@redhat.com (mailing list archive) |
|---|---|
| State | Superseded |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,1/2] iavf: add iavf_schedule_aq_request() helper | expand |
On 2023-09-06 08:14, Petr Oros wrote: > Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately > schedule watchdog_task. Helper will be used in cases where it is > necessary to run aq requests asap > > Signed-off-by: Petr Oros <poros@redhat.com> > Co-developed-by: Michal Schmidt <mschmidt@redhat.com> > Signed-off-by: Michal Schmidt <mschmidt@redhat.com> > Co-developed-by: Ivan Vecera <ivecera@redhat.com> > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > --- > drivers/net/ethernet/intel/iavf/iavf.h | 2 +- > drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 2 +- > drivers/net/ethernet/intel/iavf/iavf_main.c | 10 ++++------ > 3 files changed, 6 insertions(+), 8 deletions(-) > > diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h > index 85fba85fbb232b..e110ba3461857b 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf.h > +++ b/drivers/net/ethernet/intel/iavf/iavf.h > @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter); > int iavf_process_config(struct iavf_adapter *adapter); > int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter); > void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags); > -void iavf_schedule_request_stats(struct iavf_adapter *adapter); > +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags); > void iavf_schedule_finish_config(struct iavf_adapter *adapter); > void iavf_reset(struct iavf_adapter *adapter); > void iavf_set_ethtool_ops(struct net_device *netdev); > diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > index a34303ad057d00..90397293525f71 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct net_device *netdev, > unsigned int i; > > /* Explicitly request stats refresh */ > - iavf_schedule_request_stats(adapter); > + iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_REQUEST_STATS); > > iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats); > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c > index 7b300c86ceda73..86d472dfdbc10c 100644 > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags) > } > > /** > - * iavf_schedule_request_stats - Set the flags and schedule statistics request > + * iavf_schedule_aq_request - Set the flags and schedule aq request > * @adapter: board private structure > - * > - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() will explicitly > - * request and refresh ethtool stats > + * @flags: requested aq flags > **/ > -void iavf_schedule_request_stats(struct iavf_adapter *adapter) > +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags) > { > - adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS; > + adapter->aq_required |= flags; > mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0); > } > There are other places where the helper can be used without functional changes, e.g. iavf_add_fdir_ethtool() , iavf_replace_primary_mac() and couple of other places. In all of them, mod_delayed_work() is called after setting the AQ flag. For the sake of consistency, can you use the helper there too? Thanks, Ahmed
On Wed, Sep 06, 2023 at 09:32:59AM -0600, Ahmed Zaki wrote: > > On 2023-09-06 08:14, Petr Oros wrote: > > Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately nit: immediately
Ahmed Zaki píše v St 06. 09. 2023 v 09:32 -0600: > > On 2023-09-06 08:14, Petr Oros wrote: > > Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately > > schedule watchdog_task. Helper will be used in cases where it is > > necessary to run aq requests asap > > > > Signed-off-by: Petr Oros <poros@redhat.com> > > Co-developed-by: Michal Schmidt <mschmidt@redhat.com> > > Signed-off-by: Michal Schmidt <mschmidt@redhat.com> > > Co-developed-by: Ivan Vecera <ivecera@redhat.com> > > Signed-off-by: Ivan Vecera <ivecera@redhat.com> > > --- > > drivers/net/ethernet/intel/iavf/iavf.h | 2 +- > > drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 2 +- > > drivers/net/ethernet/intel/iavf/iavf_main.c | 10 ++++------ > > 3 files changed, 6 insertions(+), 8 deletions(-) > > > > diff --git a/drivers/net/ethernet/intel/iavf/iavf.h > > b/drivers/net/ethernet/intel/iavf/iavf.h > > index 85fba85fbb232b..e110ba3461857b 100644 > > --- a/drivers/net/ethernet/intel/iavf/iavf.h > > +++ b/drivers/net/ethernet/intel/iavf/iavf.h > > @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter); > > int iavf_process_config(struct iavf_adapter *adapter); > > int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter); > > void iavf_schedule_reset(struct iavf_adapter *adapter, u64 > > flags); > > -void iavf_schedule_request_stats(struct iavf_adapter *adapter); > > +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 > > flags); > > void iavf_schedule_finish_config(struct iavf_adapter *adapter); > > void iavf_reset(struct iavf_adapter *adapter); > > void iavf_set_ethtool_ops(struct net_device *netdev); > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > > b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > > index a34303ad057d00..90397293525f71 100644 > > --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > > +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c > > @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct > > net_device *netdev, > > unsigned int i; > > > > /* Explicitly request stats refresh */ > > - iavf_schedule_request_stats(adapter); > > + iavf_schedule_aq_request(adapter, > > IAVF_FLAG_AQ_REQUEST_STATS); > > > > iavf_add_ethtool_stats(&data, adapter, > > iavf_gstrings_stats); > > > > diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c > > b/drivers/net/ethernet/intel/iavf/iavf_main.c > > index 7b300c86ceda73..86d472dfdbc10c 100644 > > --- a/drivers/net/ethernet/intel/iavf/iavf_main.c > > +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c > > @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter > > *adapter, u64 flags) > > } > > > > /** > > - * iavf_schedule_request_stats - Set the flags and schedule > > statistics request > > + * iavf_schedule_aq_request - Set the flags and schedule aq > > request > > * @adapter: board private structure > > - * > > - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() > > will explicitly > > - * request and refresh ethtool stats > > + * @flags: requested aq flags > > **/ > > -void iavf_schedule_request_stats(struct iavf_adapter *adapter) > > +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 > > flags) > > { > > - adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS; > > + adapter->aq_required |= flags; > > mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0); > > } > > > > There are other places where the helper can be used without > functional > changes, e.g. iavf_add_fdir_ethtool() , iavf_replace_primary_mac() > and > couple of other places. In all of them, mod_delayed_work() is called > after setting the AQ flag. For the sake of consistency, can you use > the > helper there too? These two commits is fixes for issue -> net. But on iavf_add_fdir_ethtool and iavf_replace_primary_mac is mod_delayed_work called after spin_unlock_bh -> looks like no functional chages but i would like be sure and better will send this to net-next. Are you ok with this? > > > Thanks, > > Ahmed > >
On 2023-09-07 01:01, Petr Oros wrote: > Ahmed Zaki píše v St 06. 09. 2023 v 09:32 -0600: >> On 2023-09-06 08:14, Petr Oros wrote: >>> Add helper for set iavf aq request AVF_FLAG_AQ_* and imediately >>> schedule watchdog_task. Helper will be used in cases where it is >>> necessary to run aq requests asap >>> >>> Signed-off-by: Petr Oros <poros@redhat.com> >>> Co-developed-by: Michal Schmidt <mschmidt@redhat.com> >>> Signed-off-by: Michal Schmidt <mschmidt@redhat.com> >>> Co-developed-by: Ivan Vecera <ivecera@redhat.com> >>> Signed-off-by: Ivan Vecera <ivecera@redhat.com> >>> --- >>> drivers/net/ethernet/intel/iavf/iavf.h | 2 +- >>> drivers/net/ethernet/intel/iavf/iavf_ethtool.c | 2 +- >>> drivers/net/ethernet/intel/iavf/iavf_main.c | 10 ++++------ >>> 3 files changed, 6 insertions(+), 8 deletions(-) >>> >>> diff --git a/drivers/net/ethernet/intel/iavf/iavf.h >>> b/drivers/net/ethernet/intel/iavf/iavf.h >>> index 85fba85fbb232b..e110ba3461857b 100644 >>> --- a/drivers/net/ethernet/intel/iavf/iavf.h >>> +++ b/drivers/net/ethernet/intel/iavf/iavf.h >>> @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter); >>> int iavf_process_config(struct iavf_adapter *adapter); >>> int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter); >>> void iavf_schedule_reset(struct iavf_adapter *adapter, u64 >>> flags); >>> -void iavf_schedule_request_stats(struct iavf_adapter *adapter); >>> +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 >>> flags); >>> void iavf_schedule_finish_config(struct iavf_adapter *adapter); >>> void iavf_reset(struct iavf_adapter *adapter); >>> void iavf_set_ethtool_ops(struct net_device *netdev); >>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c >>> b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c >>> index a34303ad057d00..90397293525f71 100644 >>> --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c >>> +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c >>> @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct >>> net_device *netdev, >>> unsigned int i; >>> >>> /* Explicitly request stats refresh */ >>> - iavf_schedule_request_stats(adapter); >>> + iavf_schedule_aq_request(adapter, >>> IAVF_FLAG_AQ_REQUEST_STATS); >>> >>> iavf_add_ethtool_stats(&data, adapter, >>> iavf_gstrings_stats); >>> >>> diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c >>> b/drivers/net/ethernet/intel/iavf/iavf_main.c >>> index 7b300c86ceda73..86d472dfdbc10c 100644 >>> --- a/drivers/net/ethernet/intel/iavf/iavf_main.c >>> +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c >>> @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter >>> *adapter, u64 flags) >>> } >>> >>> /** >>> - * iavf_schedule_request_stats - Set the flags and schedule >>> statistics request >>> + * iavf_schedule_aq_request - Set the flags and schedule aq >>> request >>> * @adapter: board private structure >>> - * >>> - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() >>> will explicitly >>> - * request and refresh ethtool stats >>> + * @flags: requested aq flags >>> **/ >>> -void iavf_schedule_request_stats(struct iavf_adapter *adapter) >>> +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 >>> flags) >>> { >>> - adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS; >>> + adapter->aq_required |= flags; >>> mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0); >>> } >>> >> There are other places where the helper can be used without >> functional >> changes, e.g. iavf_add_fdir_ethtool() , iavf_replace_primary_mac() >> and >> couple of other places. In all of them, mod_delayed_work() is called >> after setting the AQ flag. For the sake of consistency, can you use >> the >> helper there too? > These two commits is fixes for issue -> net. But on > iavf_add_fdir_ethtool and iavf_replace_primary_mac is mod_delayed_work > called after spin_unlock_bh -> > looks like no functional chages but i would like be sure and better > will send this to net-next. Are you ok with this? > It is usually better to use the helper in the same commit that introduces it, but no problem. I am OK with sending this later to next. Thanks, Ahmed
diff --git a/drivers/net/ethernet/intel/iavf/iavf.h b/drivers/net/ethernet/intel/iavf/iavf.h index 85fba85fbb232b..e110ba3461857b 100644 --- a/drivers/net/ethernet/intel/iavf/iavf.h +++ b/drivers/net/ethernet/intel/iavf/iavf.h @@ -521,7 +521,7 @@ void iavf_down(struct iavf_adapter *adapter); int iavf_process_config(struct iavf_adapter *adapter); int iavf_parse_vf_resource_msg(struct iavf_adapter *adapter); void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags); -void iavf_schedule_request_stats(struct iavf_adapter *adapter); +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags); void iavf_schedule_finish_config(struct iavf_adapter *adapter); void iavf_reset(struct iavf_adapter *adapter); void iavf_set_ethtool_ops(struct net_device *netdev); diff --git a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c index a34303ad057d00..90397293525f71 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_ethtool.c +++ b/drivers/net/ethernet/intel/iavf/iavf_ethtool.c @@ -362,7 +362,7 @@ static void iavf_get_ethtool_stats(struct net_device *netdev, unsigned int i; /* Explicitly request stats refresh */ - iavf_schedule_request_stats(adapter); + iavf_schedule_aq_request(adapter, IAVF_FLAG_AQ_REQUEST_STATS); iavf_add_ethtool_stats(&data, adapter, iavf_gstrings_stats); diff --git a/drivers/net/ethernet/intel/iavf/iavf_main.c b/drivers/net/ethernet/intel/iavf/iavf_main.c index 7b300c86ceda73..86d472dfdbc10c 100644 --- a/drivers/net/ethernet/intel/iavf/iavf_main.c +++ b/drivers/net/ethernet/intel/iavf/iavf_main.c @@ -314,15 +314,13 @@ void iavf_schedule_reset(struct iavf_adapter *adapter, u64 flags) } /** - * iavf_schedule_request_stats - Set the flags and schedule statistics request + * iavf_schedule_aq_request - Set the flags and schedule aq request * @adapter: board private structure - * - * Sets IAVF_FLAG_AQ_REQUEST_STATS flag so iavf_watchdog_task() will explicitly - * request and refresh ethtool stats + * @flags: requested aq flags **/ -void iavf_schedule_request_stats(struct iavf_adapter *adapter) +void iavf_schedule_aq_request(struct iavf_adapter *adapter, u64 flags) { - adapter->aq_required |= IAVF_FLAG_AQ_REQUEST_STATS; + adapter->aq_required |= flags; mod_delayed_work(adapter->wq, &adapter->watchdog_task, 0); }