netdev - Re: [PATCH net-next 1/2] igb: Stop PTP related workqueues if aren't necessary

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <f8b9cd9e-651b-8488-5c4f-aacc03a3c9c8@intel.com>
Date: Mon, 14 Aug 2023 15:16:05 -0700
From: Tony Nguyen <anthony.l.nguyen@...el.com>
To: Leon Romanovsky <leon@...nel.org>
CC: <davem@...emloft.net>, <kuba@...nel.org>, <pabeni@...hat.com>,
	<edumazet@...gle.com>, <netdev@...r.kernel.org>, Alessio Igor Bogani
	<alessio.bogani@...ttra.eu>, <richardcochran@...il.com>, "Pucha Himasekhar
 Reddy" <himasekharx.reddy.pucha@...el.com>
Subject: Re: [PATCH net-next 1/2] igb: Stop PTP related workqueues if aren't
 necessary



On 8/13/2023 2:01 AM, Leon Romanovsky wrote:
> On Thu, Aug 10, 2023 at 10:54:09AM -0700, Tony Nguyen wrote:
>> From: Alessio Igor Bogani <alessio.bogani@...ttra.eu>
>>
>> The workqueues ptp_tx_work and ptp_overflow_work are unconditionally allocated
>> by igb_ptp_init(). Stop them if aren't necessary (ptp_clock_register() fails
>> and CONFIG_PTP is disabled).
>>
>> Signed-off-by: Alessio Igor Bogani <alessio.bogani@...ttra.eu>
>> Tested-by: Pucha Himasekhar Reddy <himasekharx.reddy.pucha@...el.com> (A Contingent worker at Intel)
>> Signed-off-by: Tony Nguyen <anthony.l.nguyen@...el.com>
>> ---
>>   drivers/net/ethernet/intel/igb/igb_ptp.c | 6 ++++++
>>   1 file changed, 6 insertions(+)
>>
>> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
>> index 405886ee5261..02276c922ac0 100644
>> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
>> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
>> @@ -1406,7 +1406,13 @@ void igb_ptp_init(struct igb_adapter *adapter)
>>   		dev_info(&adapter->pdev->dev, "added PHC on %s\n",
>>   			 adapter->netdev->name);
>>   		adapter->ptp_flags |= IGB_PTP_ENABLED;
>> +		return;
>>   	}
>> +
>> +	if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
>> +		cancel_delayed_work_sync(&adapter->ptp_overflow_work);
>> +
>> +	cancel_work_sync(&adapter->ptp_tx_work);
> 
> Is it possible to move work initialization to be after call to ptp_clock_register()?

I'm under the impression that everything should be ready to go before 
calling ptp_clock_register() so we shouldn't register until the 
workqueues are set up.

Thanks,
Tony

> diff --git a/drivers/net/ethernet/intel/igb/igb_ptp.c b/drivers/net/ethernet/intel/igb/igb_ptp.c
> index 405886ee5261..56fd2b0fe70c 100644
> --- a/drivers/net/ethernet/intel/igb/igb_ptp.c
> +++ b/drivers/net/ethernet/intel/igb/igb_ptp.c
> @@ -1386,11 +1386,6 @@ void igb_ptp_init(struct igb_adapter *adapter)
>          }
>   
>          spin_lock_init(&adapter->tmreg_lock);
> -       INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
> -
> -       if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
> -               INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
> -                                 igb_ptp_overflow_check);
>   
>          adapter->tstamp_config.rx_filter = HWTSTAMP_FILTER_NONE;
>          adapter->tstamp_config.tx_type = HWTSTAMP_TX_OFF;
> @@ -1407,6 +1402,15 @@ void igb_ptp_init(struct igb_adapter *adapter)
>                           adapter->netdev->name);
>                  adapter->ptp_flags |= IGB_PTP_ENABLED;
>          }
> +
> +       if (!adapter->ptp_clock)
> +               return;
> +
> +       INIT_WORK(&adapter->ptp_tx_work, igb_ptp_tx_work);
> +
> +       if (adapter->ptp_flags & IGB_PTP_OVERFLOW_CHECK)
> +               INIT_DELAYED_WORK(&adapter->ptp_overflow_work,
> +                                 igb_ptp_overflow_check);
>   }
>   
>   /**
> 
> 
> 
> 
>>   }
>>   
>>   /**
>> -- 
>> 2.38.1
>>
>>