Re: [PATCH net-next] net: core: change bool members of struct net_device to bitfield members

[Heiner Kallweit <hkallweit1@gmail.com>]

On 08.10.2018 22:07, Randy Dunlap wrote:
> On 10/8/18 1:00 PM, Heiner Kallweit wrote:
>> bool is good as parameter type or function return type, but if used
>> for struct members it consumes more memory than needed.
>> Changing the bool members of struct net_device to bitfield members
>> allows to decrease the memory footprint of this struct.
>>
>> Signed-off-by: Heiner Kallweit <hkallweit1@gmail.com>
>> ---
>>  include/linux/netdevice.h | 24 +++++++++++++-----------
>>  1 file changed, 13 insertions(+), 11 deletions(-)
>>
>> diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
>> index 76603ee13..3d7b8df2e 100644
>> --- a/include/linux/netdevice.h
>> +++ b/include/linux/netdevice.h
>> @@ -1651,10 +1651,6 @@ enum netdev_priv_flags {
>>   * 	@dev_port:		Used to differentiate devices that share
>>   * 				the same function
>>   *	@addr_list_lock:	XXX: need comments on this one
>> - *	@uc_promisc:		Counter that indicates promiscuous mode
>> - *				has been enabled due to the need to listen to
>> - *				additional unicast addresses in a device that
>> - *				does not implement ndo_set_rx_mode()
>>   *	@uc:			unicast mac addresses
>>   *	@mc:			multicast mac addresses
>>   *	@dev_addrs:		list of device hw addresses
>> @@ -1714,11 +1710,9 @@ enum netdev_priv_flags {
>>   *	@link_watch_list:	XXX: need comments on this one
>>   *
>>   *	@reg_state:		Register/unregister state machine
>> - *	@dismantle:		Device is going to be freed
>>   *	@rtnl_link_state:	This enum represents the phases of creating
>>   *				a new link
>>   *
>> - *	@needs_free_netdev:	Should unregister perform free_netdev?
>>   *	@priv_destructor:	Called from unregister
>>   *	@npinfo:		XXX: need comments on this one
>>   * 	@nd_net:		Network namespace this network device is inside
>> @@ -1758,6 +1752,15 @@ enum netdev_priv_flags {
>>   *	@qdisc_tx_busylock: lockdep class annotating Qdisc->busylock spinlock
>>   *	@qdisc_running_key: lockdep class annotating Qdisc->running seqcount
>>   *
>> + *	@uc_promisc:	Counter that indicates promiscuous mode
>> + *			has been enabled due to the need to listen to
>> + *			additional unicast addresses in a device that
>> + *			does not implement ndo_set_rx_mode()
> 
> Hi,
> 
> I see that all you did is copy/paste that text (above), but I wouldn't call
> a single bit a [1-bit] Counter.
> 
I stumbled across this comment too. Neither a bool member nor a 1-bit
bitfield member should be called a counter. I kept the original comment, 
but I'm totally fine with changing Counter -> Flag and will provide a v2.

> thanks,
>