[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <ed2a1b51-782b-4ee1-be75-06a0a742525c@wanadoo.fr>
Date: Tue, 16 Sep 2025 20:35:27 +0200
From: Christophe JAILLET <christophe.jaillet@...adoo.fr>
To: Vincent Mailhol <mailhol@...nel.org>,
Oliver Hartkopp <socketcan@...tkopp.net>,
Marc Kleine-Budde <mkl@...gutronix.de>
Cc: linux-can@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 2/3] can: raw: use bitfields to store flags in struct
raw_sock
Le 15/09/2025 à 11:23, Vincent Mailhol a écrit :
> The loopback, recv_own_msgs, fd_frames and xl_frames fields of struct
> raw_sock just need to store one bit of information.
>
> Declare all those members as a bitfields of type unsigned int and
> width one bit.
>
> Add a temporary variable to raw_setsockopt() and raw_getsockopt() to
> make the conversion between the stored bits and the socket interface.
>
> This reduces struct raw_sock by eight bytes.
>
> Statistics before:
>
> $ pahole --class_name=raw_sock net/can/raw.o
> struct raw_sock {
> struct sock sk __attribute__((__aligned__(8))); /* 0 776 */
>
> /* XXX last struct has 1 bit hole */
>
> /* --- cacheline 12 boundary (768 bytes) was 8 bytes ago --- */
> int bound; /* 776 4 */
> int ifindex; /* 780 4 */
> struct net_device * dev; /* 784 8 */
> netdevice_tracker dev_tracker; /* 792 0 */
> struct list_head notifier; /* 792 16 */
> int loopback; /* 808 4 */
> int recv_own_msgs; /* 812 4 */
> int fd_frames; /* 816 4 */
> int xl_frames; /* 820 4 */
> struct can_raw_vcid_options raw_vcid_opts; /* 824 4 */
> canid_t tx_vcid_shifted; /* 828 4 */
> /* --- cacheline 13 boundary (832 bytes) --- */
> canid_t rx_vcid_shifted; /* 832 4 */
> canid_t rx_vcid_mask_shifted; /* 836 4 */
> int join_filters; /* 840 4 */
> int count; /* 844 4 */
> struct can_filter dfilter; /* 848 8 */
> struct can_filter * filter; /* 856 8 */
> can_err_mask_t err_mask; /* 864 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct uniqframe * uniq; /* 872 8 */
>
> /* size: 880, cachelines: 14, members: 20 */
> /* sum members: 876, holes: 1, sum holes: 4 */
> /* member types with bit holes: 1, total: 1 */
> /* forced alignments: 1 */
> /* last cacheline: 48 bytes */
> } __attribute__((__aligned__(8)));
>
> ...and after:
>
> $ pahole --class_name=raw_sock net/can/raw.o
> struct raw_sock {
> struct sock sk __attribute__((__aligned__(8))); /* 0 776 */
>
> /* XXX last struct has 1 bit hole */
>
> /* --- cacheline 12 boundary (768 bytes) was 8 bytes ago --- */
> int bound; /* 776 4 */
> int ifindex; /* 780 4 */
> struct net_device * dev; /* 784 8 */
> netdevice_tracker dev_tracker; /* 792 0 */
> struct list_head notifier; /* 792 16 */
> unsigned int loopback:1; /* 808: 0 4 */
> unsigned int recv_own_msgs:1; /* 808: 1 4 */
> unsigned int fd_frames:1; /* 808: 2 4 */
> unsigned int xl_frames:1; /* 808: 3 4 */
>
> /* XXX 4 bits hole, try to pack */
> /* Bitfield combined with next fields */
>
> struct can_raw_vcid_options raw_vcid_opts; /* 809 4 */
>
> /* XXX 3 bytes hole, try to pack */
>
> canid_t tx_vcid_shifted; /* 816 4 */
> canid_t rx_vcid_shifted; /* 820 4 */
> canid_t rx_vcid_mask_shifted; /* 824 4 */
> int join_filters; /* 828 4 */
> /* --- cacheline 13 boundary (832 bytes) --- */
> int count; /* 832 4 */
> struct can_filter dfilter; /* 836 8 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct can_filter * filter; /* 848 8 */
> can_err_mask_t err_mask; /* 856 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct uniqframe * uniq; /* 864 8 */
>
> /* size: 872, cachelines: 14, members: 20 */
> /* sum members: 860, holes: 3, sum holes: 11 */
> /* sum bitfield members: 4 bits, bit holes: 1, sum bit holes: 4 bits */
> /* member types with bit holes: 1, total: 1 */
> /* forced alignments: 1 */
> /* last cacheline: 40 bytes */
> } __attribute__((__aligned__(8)));
>
> Signed-off-by: Vincent Mailhol <mailhol@...nel.org>
> ---
> net/can/raw.c | 47 ++++++++++++++++++++++++++++-------------------
> 1 file changed, 28 insertions(+), 19 deletions(-)
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index db21d8a8c54d1b6a25a72c7a9d11d5c94f3187b5..cec580ecd58e36931d1be05716e6beb9c93aa271 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -87,10 +87,10 @@ struct raw_sock {
> struct net_device *dev;
> netdevice_tracker dev_tracker;
> struct list_head notifier;
> - int loopback;
> - int recv_own_msgs;
> - int fd_frames;
> - int xl_frames;
> + unsigned int loopback:1;
> + unsigned int recv_own_msgs:1;
> + unsigned int fd_frames:1;
> + unsigned int xl_frames:1;
> struct can_raw_vcid_options raw_vcid_opts;
> canid_t tx_vcid_shifted;
> canid_t rx_vcid_shifted;
[...]
Hi,
just in case, it looks like bound and join_filters could also be defined
in the bitfield.
just my 2c.
CJ
Powered by blists - more mailing lists