Re: [net-next,06/15] i40e: change flags to use 64 bits

[Shannon Nelson <shannon.nelson@oracle.com>]

On 2/7/2018 7:09 AM, James Hogan wrote:
> On Fri, Jan 26, 2018 at 01:24:50PM -0800, Jeff Kirsher wrote:
>> From: Alice Michael <alice.michael@intel.com>
>>
>> As we have added more flags, we need to now use more
>> bits and have over flooded the 32 bit size.  So
>> make it 64.
>>
>> Also change all the existing bits to unsigned long long
>> bits.
>>
>> Signed-off-by: Alice Michael <alice.michael@intel.com>
>> Tested-by: Andrew Bowers <andrewx.bowers@intel.com>
>> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
>> ---
>>   drivers/net/ethernet/intel/i40e/i40e.h         | 67 +++++++++++++-------------
>>   drivers/net/ethernet/intel/i40e/i40e_ethtool.c |  4 +-
>>   2 files changed, 36 insertions(+), 35 deletions(-)
> 
> ...
> 
>> @@ -4323,7 +4323,7 @@ static int i40e_set_priv_flags(struct net_device *dev, u32 flags)
>>   	 * originally. We'll just punt with an error and log something in the
>>   	 * message buffer.
>>   	 */
>> -	if (cmpxchg(&pf->flags, orig_flags, new_flags) != orig_flags) {
>> +	if (cmpxchg64(&pf->flags, orig_flags, new_flags) != orig_flags) {
> 
> This breaks allyesconfig builds on certain architectures, for example
> MIPS 32-bit with SMP enabled, which doesn't support cmpxchg64:
> 
>    CC      drivers/net/ethernet/intel/i40e/i40e_ethtool.o
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c: In function ‘i40e_set_priv_flags’:
> drivers/net/ethernet/intel/i40e/i40e_ethtool.c:4326:6: error: implicit declaration of function ‘cmpxchg64’; did you mean ‘__cmpxchg’? [-Werror=implicit-function-declaration]
>    if (cmpxchg64(&pf->flags, orig_flags, new_flags) != orig_flags) {
>        ^~~~~~~~~
>        __cmpxchg
> 
> Should the driver now depend on 64BIT or something?

A long time ago this was the original expectation for this driver, but 
it was strongly suggested that in order to play well in the kernel it 
needed to be usable in 32-bit builds as well.  I suspect this sentiment 
remains, and besides we don't want to break existing support, so this 
probably needs to be addressed for 32-bit.

sln

> 
> Cheers
> James
>