From mboxrd@z Thu Jan  1 00:00:00 1970
From: Nguyen, Anthony L <anthony.l.nguyen@intel.com>
Date: Tue, 25 Jan 2022 00:31:28 +0000
Subject: [Intel-wired-lan] [PATCH net-next v2] iavf: Add reset to
 watchdog task
In-Reply-To: <CO1PR11MB48811E81C70B27754A77D92F995B9@CO1PR11MB4881.namprd11.prod.outlook.com>
References: <CO1PR11MB48811E81C70B27754A77D92F995B9@CO1PR11MB4881.namprd11.prod.outlook.com>
Message-ID: <fcb47365347a6eea578315cc9c8e6fa218be66b4.camel@intel.com>
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-Transfer-Encoding: 7bit
To: intel-wired-lan@osuosl.org
List-ID: <intel-wired-lan.osuosl.org>

On Fri, 2022-01-21 at 14:58 +0000, Sokolowski, Jan wrote:
> Remove the reset as separate task and implement
> the reset procedure inside the watchdog state machine.
> This allows to avoid additional overhead for processes
> synchronization. The reset flags are still used to mark
> reset progress for asynchonous invoked callback functions.
> 
> Signed-off-by: Jakub Pawlak <jakub.pawlak@intel.com>
> Signed-off-by: Jan Sokolowski <jan.sokolowski@intel.com>
> Signed-off-by: Arkadiusz Kubalewski <arkadiusz.kubalewski@intel.com>
> Signed-off-by: Aleksandr Loktionov <aleksandr.loktionov@intel.com>
> ---

A changelog would be nice.

Also:
WARNING: From:/Signed-off-by: email name mismatch: 'From: "Sokolowski,
Jan" <jan.sokolowski@intel.com>' != 'Signed-off-by: Jan Sokolowski
<jan.sokolowski@intel.com>'


> ?drivers/net/ethernet/intel/iavf/iavf.h??????? |? 1 -
> ?.../net/ethernet/intel/iavf/iavf_ethtool.c??? | 12 ++--
> ?drivers/net/ethernet/intel/iavf/iavf_main.c?? | 67 +++++++----------
> --
> ?.../net/ethernet/intel/iavf/iavf_virtchnl.c?? |? 4 +-
> ?4 files changed, 30 insertions(+), 54 deletions(-)

<snip>


> -static void iavf_reset_task(struct work_struct *work)
> +static void iavf_handle_hw_reset(struct iavf_adapter *adapter)
> ?{
> -???????struct iavf_adapter *adapter = container_of(work,
> -???????????????????????????????????????????????????? struct
> iavf_adapter,
> -???????????????????????????????????????????????????? reset_task);
> ????????struct virtchnl_vf_resource *vfres = adapter->vf_res;
> ????????struct net_device *netdev = adapter->netdev;
> ????????struct iavf_hw *hw = &adapter->hw;
> @@ -2651,12 +2647,9 @@ static void iavf_reset_task(struct work_struct
> *work)
> ????????if (mutex_is_locked(&adapter->remove_lock))
> ????????????????return;
> ?
> -???????if (iavf_lock_timeout(&adapter->crit_lock, 200)) {
> -???????????????schedule_work(&adapter->reset_task);
> -???????????????return;
> -???????}
> -???????while (!mutex_trylock(&adapter->client_lock))
> -???????????????usleep_range(500, 1000);

I missed this on v1, but doesn't this unbalance the client_lock?

> +???????adapter->flags |= IAVF_FLAG_RESET_PENDING;
> +???????adapter->flags &= ~IAVF_FLAG_RESET_NEEDED;
> +
> ????????if (CLIENT_ENABLED(adapter)) {
> ????????????????adapter->flags &= ~(IAVF_FLAG_CLIENT_NEEDS_OPEN |
> ??????????????????????????????????? IAVF_FLAG_CLIENT_NEEDS_CLOSE |