* [PATCH 0/3] can: raw: optimize the sizes of struct uniqframe and struct raw_sock
@ 2025-09-15 9:23 Vincent Mailhol
2025-09-15 9:23 ` [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing Vincent Mailhol
` (2 more replies)
0 siblings, 3 replies; 11+ messages in thread
From: Vincent Mailhol @ 2025-09-15 9:23 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde
Cc: linux-can, linux-kernel, Vincent Mailhol
A few bytes can be shaved out of can raw's struct uniqframe and struct
raw_sock.
Patch #1 reorders struct uniqframe fields to save 8 bytes.
Patch #2 and #3 modify struct raw_sock to use bitfields and to reorder
its fields to save 16 bytes in total.
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
Vincent Mailhol (3):
can: raw: reorder struct uniqframe's members to optimise packing
can: raw: use bitfields to store flags in struct raw_sock
can: raw: reorder struct raw_sock's members to optimise packing
net/can/raw.c | 51 ++++++++++++++++++++++++++++++---------------------
1 file changed, 30 insertions(+), 21 deletions(-)
---
base-commit: 5b5ba63a54cc7cb050fa734dbf495ffd63f9cbf7
change-id: 20250915-can-raw-repack-c21aab25cc11
Best regards,
--
Vincent Mailhol <mailhol@kernel.org>
^ permalink raw reply [flat|nested] 11+ messages in thread
* [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing
2025-09-15 9:23 [PATCH 0/3] can: raw: optimize the sizes of struct uniqframe and struct raw_sock Vincent Mailhol
@ 2025-09-15 9:23 ` Vincent Mailhol
2025-09-15 10:18 ` Oliver Hartkopp
2025-09-15 9:23 ` [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock Vincent Mailhol
2025-09-15 9:23 ` [PATCH 3/3] can: raw: reorder struct raw_sock's members to optimise packing Vincent Mailhol
2 siblings, 1 reply; 11+ messages in thread
From: Vincent Mailhol @ 2025-09-15 9:23 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde
Cc: linux-can, linux-kernel, Vincent Mailhol
struct uniqframe has one hole. Reorder the fields to save 8 bytes.
Statistics before:
$ pahole --class_name=uniqframe net/can/raw.o
struct uniqframe {
int skbcnt; /* 0 4 */
/* XXX 4 bytes hole, try to pack */
const struct sk_buff * skb; /* 8 8 */
unsigned int join_rx_count; /* 16 4 */
/* size: 24, cachelines: 1, members: 3 */
/* sum members: 16, holes: 1, sum holes: 4 */
/* padding: 4 */
/* last cacheline: 24 bytes */
};
...and after:
$ pahole --class_name=uniqframe net/can/raw.o
struct uniqframe {
const struct sk_buff * skb; /* 0 8 */
int skbcnt; /* 8 4 */
unsigned int join_rx_count; /* 12 4 */
/* size: 16, cachelines: 1, members: 3 */
/* last cacheline: 16 bytes */
};
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
net/can/raw.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/net/can/raw.c b/net/can/raw.c
index 76b867d21def209f5c6d236604c0e434a1c55a4d..db21d8a8c54d1b6a25a72c7a9d11d5c94f3187b5 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -75,8 +75,8 @@ MODULE_ALIAS("can-proto-1");
*/
struct uniqframe {
- int skbcnt;
const struct sk_buff *skb;
+ int skbcnt;
unsigned int join_rx_count;
};
--
2.49.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock
2025-09-15 9:23 [PATCH 0/3] can: raw: optimize the sizes of struct uniqframe and struct raw_sock Vincent Mailhol
2025-09-15 9:23 ` [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing Vincent Mailhol
@ 2025-09-15 9:23 ` Vincent Mailhol
2025-09-15 10:16 ` Oliver Hartkopp
2025-09-16 18:35 ` Christophe JAILLET
2025-09-15 9:23 ` [PATCH 3/3] can: raw: reorder struct raw_sock's members to optimise packing Vincent Mailhol
2 siblings, 2 replies; 11+ messages in thread
From: Vincent Mailhol @ 2025-09-15 9:23 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde
Cc: linux-can, linux-kernel, Vincent Mailhol
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@kernel.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;
@@ -560,8 +560,8 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
struct can_filter sfilter; /* single filter */
struct net_device *dev = NULL;
can_err_mask_t err_mask = 0;
- int fd_frames;
int count = 0;
+ int flag;
int err = 0;
if (level != SOL_CAN_RAW)
@@ -682,44 +682,48 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
break;
case CAN_RAW_LOOPBACK:
- if (optlen != sizeof(ro->loopback))
+ if (optlen != sizeof(flag))
return -EINVAL;
- if (copy_from_sockptr(&ro->loopback, optval, optlen))
+ if (copy_from_sockptr(&flag, optval, optlen))
return -EFAULT;
+ ro->loopback = !!flag;
break;
case CAN_RAW_RECV_OWN_MSGS:
- if (optlen != sizeof(ro->recv_own_msgs))
+ if (optlen != sizeof(flag))
return -EINVAL;
- if (copy_from_sockptr(&ro->recv_own_msgs, optval, optlen))
+ if (copy_from_sockptr(&flag, optval, optlen))
return -EFAULT;
+ ro->recv_own_msgs = !!flag;
break;
case CAN_RAW_FD_FRAMES:
- if (optlen != sizeof(fd_frames))
+ if (optlen != sizeof(flag))
return -EINVAL;
- if (copy_from_sockptr(&fd_frames, optval, optlen))
+ if (copy_from_sockptr(&flag, optval, optlen))
return -EFAULT;
/* Enabling CAN XL includes CAN FD */
- if (ro->xl_frames && !fd_frames)
+ if (ro->xl_frames && !flag)
return -EINVAL;
- ro->fd_frames = fd_frames;
+ ro->fd_frames = !!flag;
break;
case CAN_RAW_XL_FRAMES:
- if (optlen != sizeof(ro->xl_frames))
+ if (optlen != sizeof(flag))
return -EINVAL;
- if (copy_from_sockptr(&ro->xl_frames, optval, optlen))
+ if (copy_from_sockptr(&flag, optval, optlen))
return -EFAULT;
+ ro->xl_frames = !!flag;
+
/* Enabling CAN XL includes CAN FD */
if (ro->xl_frames)
ro->fd_frames = ro->xl_frames;
@@ -758,6 +762,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
{
struct sock *sk = sock->sk;
struct raw_sock *ro = raw_sk(sk);
+ int flag;
int len;
void *val;
@@ -806,25 +811,29 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
case CAN_RAW_LOOPBACK:
if (len > sizeof(int))
len = sizeof(int);
- val = &ro->loopback;
+ flag = ro->loopback;
+ val = &flag;
break;
case CAN_RAW_RECV_OWN_MSGS:
if (len > sizeof(int))
len = sizeof(int);
- val = &ro->recv_own_msgs;
+ flag = ro->recv_own_msgs;
+ val = &flag;
break;
case CAN_RAW_FD_FRAMES:
if (len > sizeof(int))
len = sizeof(int);
- val = &ro->fd_frames;
+ flag = ro->fd_frames;
+ val = &flag;
break;
case CAN_RAW_XL_FRAMES:
if (len > sizeof(int))
len = sizeof(int);
- val = &ro->xl_frames;
+ flag = ro->xl_frames;
+ val = &flag;
break;
case CAN_RAW_XL_VCID_OPTS: {
--
2.49.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* [PATCH 3/3] can: raw: reorder struct raw_sock's members to optimise packing
2025-09-15 9:23 [PATCH 0/3] can: raw: optimize the sizes of struct uniqframe and struct raw_sock Vincent Mailhol
2025-09-15 9:23 ` [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing Vincent Mailhol
2025-09-15 9:23 ` [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock Vincent Mailhol
@ 2025-09-15 9:23 ` Vincent Mailhol
2025-09-15 17:42 ` Oliver Hartkopp
2 siblings, 1 reply; 11+ messages in thread
From: Vincent Mailhol @ 2025-09-15 9:23 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde
Cc: linux-can, linux-kernel, Vincent Mailhol
struct raw_sock has several holes. Reorder the fields to save 8 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 */
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)));
...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 */
struct can_raw_vcid_options raw_vcid_opts; /* 808 4 */
unsigned int loopback:1; /* 812: 0 4 */
unsigned int recv_own_msgs:1; /* 812: 1 4 */
unsigned int fd_frames:1; /* 812: 2 4 */
unsigned int xl_frames:1; /* 812: 3 4 */
/* XXX 28 bits 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 */
can_err_mask_t err_mask; /* 828 4 */
/* --- cacheline 13 boundary (832 bytes) --- */
int join_filters; /* 832 4 */
int count; /* 836 4 */
struct can_filter dfilter; /* 840 8 */
struct can_filter * filter; /* 848 8 */
struct uniqframe * uniq; /* 856 8 */
/* size: 864, cachelines: 14, members: 20 */
/* sum members: 860 */
/* sum bitfield members: 4 bits, bit holes: 1, sum bit holes: 28 bits */
/* member types with bit holes: 1, total: 1 */
/* forced alignments: 1 */
/* last cacheline: 32 bytes */
} __attribute__((__aligned__(8)));
Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
---
net/can/raw.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/net/can/raw.c b/net/can/raw.c
index cec580ecd58e36931d1be05716e6beb9c93aa271..81f5de63bcfaacf3f51670159fb3d1d7d1fc6020 100644
--- a/net/can/raw.c
+++ b/net/can/raw.c
@@ -87,19 +87,19 @@ struct raw_sock {
struct net_device *dev;
netdevice_tracker dev_tracker;
struct list_head notifier;
+ struct can_raw_vcid_options raw_vcid_opts;
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;
canid_t rx_vcid_mask_shifted;
+ can_err_mask_t err_mask;
int join_filters;
int count; /* number of active filters */
struct can_filter dfilter; /* default/single filter */
struct can_filter *filter; /* pointer to filter(s) */
- can_err_mask_t err_mask;
struct uniqframe __percpu *uniq;
};
--
2.49.1
^ permalink raw reply related [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock
2025-09-15 9:23 ` [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock Vincent Mailhol
@ 2025-09-15 10:16 ` Oliver Hartkopp
2025-09-15 10:47 ` Vincent Mailhol
2025-09-16 18:35 ` Christophe JAILLET
1 sibling, 1 reply; 11+ messages in thread
From: Oliver Hartkopp @ 2025-09-15 10:16 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can, linux-kernel
On 15.09.25 11:23, Vincent Mailhol wrote:
> 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 */
This means that the former data structures (int) are not copied but bits
are set (shifted, ANDed, ORed, etc) right?
So what's the difference in the code the CPU has to process for this
improvement? Is implementing this bitmap more efficient or similar to
copy the (unsigned ints) as-is?
>
> /* 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@kernel.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;
> @@ -560,8 +560,8 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
> struct can_filter sfilter; /* single filter */
> struct net_device *dev = NULL;
> can_err_mask_t err_mask = 0;
> - int fd_frames;
> int count = 0;
> + int flag;
> int err = 0;
>
> if (level != SOL_CAN_RAW)
> @@ -682,44 +682,48 @@ static int raw_setsockopt(struct socket *sock, int level, int optname,
> break;
>
> case CAN_RAW_LOOPBACK:
> - if (optlen != sizeof(ro->loopback))
> + if (optlen != sizeof(flag))
> return -EINVAL;
>
> - if (copy_from_sockptr(&ro->loopback, optval, optlen))
> + if (copy_from_sockptr(&flag, optval, optlen))
> return -EFAULT;
>
> + ro->loopback = !!flag;
This is obviously an additional effort. Instead it a simple copy the
code makes a copy to an extra variable and then an assignment with bit
(shifting) operations.
Best regards,
Oliver
> break;
>
> case CAN_RAW_RECV_OWN_MSGS:
> - if (optlen != sizeof(ro->recv_own_msgs))
> + if (optlen != sizeof(flag))
> return -EINVAL;
>
> - if (copy_from_sockptr(&ro->recv_own_msgs, optval, optlen))
> + if (copy_from_sockptr(&flag, optval, optlen))
> return -EFAULT;
>
> + ro->recv_own_msgs = !!flag;
> break;
>
> case CAN_RAW_FD_FRAMES:
> - if (optlen != sizeof(fd_frames))
> + if (optlen != sizeof(flag))
> return -EINVAL;
>
> - if (copy_from_sockptr(&fd_frames, optval, optlen))
> + if (copy_from_sockptr(&flag, optval, optlen))
> return -EFAULT;
>
> /* Enabling CAN XL includes CAN FD */
> - if (ro->xl_frames && !fd_frames)
> + if (ro->xl_frames && !flag)
> return -EINVAL;
>
> - ro->fd_frames = fd_frames;
> + ro->fd_frames = !!flag;
> break;
>
> case CAN_RAW_XL_FRAMES:
> - if (optlen != sizeof(ro->xl_frames))
> + if (optlen != sizeof(flag))
> return -EINVAL;
>
> - if (copy_from_sockptr(&ro->xl_frames, optval, optlen))
> + if (copy_from_sockptr(&flag, optval, optlen))
> return -EFAULT;
>
> + ro->xl_frames = !!flag;
> +
> /* Enabling CAN XL includes CAN FD */
> if (ro->xl_frames)
> ro->fd_frames = ro->xl_frames;
> @@ -758,6 +762,7 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
> {
> struct sock *sk = sock->sk;
> struct raw_sock *ro = raw_sk(sk);
> + int flag;
> int len;
> void *val;
>
> @@ -806,25 +811,29 @@ static int raw_getsockopt(struct socket *sock, int level, int optname,
> case CAN_RAW_LOOPBACK:
> if (len > sizeof(int))
> len = sizeof(int);
> - val = &ro->loopback;
> + flag = ro->loopback;
> + val = &flag;
> break;
>
> case CAN_RAW_RECV_OWN_MSGS:
> if (len > sizeof(int))
> len = sizeof(int);
> - val = &ro->recv_own_msgs;
> + flag = ro->recv_own_msgs;
> + val = &flag;
> break;
>
> case CAN_RAW_FD_FRAMES:
> if (len > sizeof(int))
> len = sizeof(int);
> - val = &ro->fd_frames;
> + flag = ro->fd_frames;
> + val = &flag;
> break;
>
> case CAN_RAW_XL_FRAMES:
> if (len > sizeof(int))
> len = sizeof(int);
> - val = &ro->xl_frames;
> + flag = ro->xl_frames;
> + val = &flag;
> break;
>
> case CAN_RAW_XL_VCID_OPTS: {
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing
2025-09-15 9:23 ` [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing Vincent Mailhol
@ 2025-09-15 10:18 ` Oliver Hartkopp
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Hartkopp @ 2025-09-15 10:18 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can, linux-kernel
On 15.09.25 11:23, Vincent Mailhol wrote:
> struct uniqframe has one hole. Reorder the fields to save 8 bytes.
>
> Statistics before:
>
> $ pahole --class_name=uniqframe net/can/raw.o
> struct uniqframe {
> int skbcnt; /* 0 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> const struct sk_buff * skb; /* 8 8 */
> unsigned int join_rx_count; /* 16 4 */
>
> /* size: 24, cachelines: 1, members: 3 */
> /* sum members: 16, holes: 1, sum holes: 4 */
> /* padding: 4 */
> /* last cacheline: 24 bytes */
> };
>
> ...and after:
>
> $ pahole --class_name=uniqframe net/can/raw.o
> struct uniqframe {
> const struct sk_buff * skb; /* 0 8 */
> int skbcnt; /* 8 4 */
> unsigned int join_rx_count; /* 12 4 */
>
> /* size: 16, cachelines: 1, members: 3 */
> /* last cacheline: 16 bytes */
> };
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> net/can/raw.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index 76b867d21def209f5c6d236604c0e434a1c55a4d..db21d8a8c54d1b6a25a72c7a9d11d5c94f3187b5 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -75,8 +75,8 @@ MODULE_ALIAS("can-proto-1");
> */
>
> struct uniqframe {
> - int skbcnt;
> const struct sk_buff *skb;
> + int skbcnt;
> unsigned int join_rx_count;
> };
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock
2025-09-15 10:16 ` Oliver Hartkopp
@ 2025-09-15 10:47 ` Vincent Mailhol
2025-09-15 17:41 ` Oliver Hartkopp
0 siblings, 1 reply; 11+ messages in thread
From: Vincent Mailhol @ 2025-09-15 10:47 UTC (permalink / raw)
To: Oliver Hartkopp, Marc Kleine-Budde; +Cc: linux-can, linux-kernel
On 15/09/2025 at 19:16, Oliver Hartkopp wrote:
> On 15.09.25 11:23, Vincent Mailhol wrote:
>> 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 */
>
> This means that the former data structures (int) are not copied but bits are set
> (shifted, ANDed, ORed, etc) right?
>
> So what's the difference in the code the CPU has to process for this
> improvement? Is implementing this bitmap more efficient or similar to copy the
> (unsigned ints) as-is?
It will indeed have to add a couple assembly instructions. But this is peanuts.
In the best case, the out of order execution might very well optimize this so
that not even a CPU tick is wasted. In the worst case, it is a couple CPU ticks.
On the other hands, reducing the size by 16 bytes lowers the risk to have a
cache miss. And removing one cache miss outperforms by an order of magnitude the
penalty of adding a couple assembly instructions.
Well, I did not benchmark it, but this is a commonly accepted trade off.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock
2025-09-15 10:47 ` Vincent Mailhol
@ 2025-09-15 17:41 ` Oliver Hartkopp
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Hartkopp @ 2025-09-15 17:41 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can, linux-kernel
On 15.09.25 12:47, Vincent Mailhol wrote:
> On 15/09/2025 at 19:16, Oliver Hartkopp wrote:
>> On 15.09.25 11:23, Vincent Mailhol wrote:
>>> 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 */
>>
>> This means that the former data structures (int) are not copied but bits are set
>> (shifted, ANDed, ORed, etc) right?
>>
>> So what's the difference in the code the CPU has to process for this
>> improvement? Is implementing this bitmap more efficient or similar to copy the
>> (unsigned ints) as-is?
>
> It will indeed have to add a couple assembly instructions. But this is peanuts.
> In the best case, the out of order execution might very well optimize this so
> that not even a CPU tick is wasted. In the worst case, it is a couple CPU ticks.
>
> On the other hands, reducing the size by 16 bytes lowers the risk to have a
> cache miss. And removing one cache miss outperforms by an order of magnitude the
> penalty of adding a couple assembly instructions.
>
> Well, I did not benchmark it, but this is a commonly accepted trade off.
Ok.
Most accesses of those values like ro->fd_frames are read-only anyway,
which might add an additional AND operation with a constant value.
Therefore your suggested changes are not in the hot path anyway and the
ro->fd_frames = !!flag operation is executed at socket creation time only.
Generally it is interesting the the compiler can handle bits in this way.
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
Thanks!
Oliver
>
> Yours sincerely,
> Vincent Mailhol
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 3/3] can: raw: reorder struct raw_sock's members to optimise packing
2025-09-15 9:23 ` [PATCH 3/3] can: raw: reorder struct raw_sock's members to optimise packing Vincent Mailhol
@ 2025-09-15 17:42 ` Oliver Hartkopp
0 siblings, 0 replies; 11+ messages in thread
From: Oliver Hartkopp @ 2025-09-15 17:42 UTC (permalink / raw)
To: Vincent Mailhol, Marc Kleine-Budde; +Cc: linux-can, linux-kernel
On 15.09.25 11:23, Vincent Mailhol wrote:
> struct raw_sock has several holes. Reorder the fields to save 8 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 */
> 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)));
>
> ...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 */
> struct can_raw_vcid_options raw_vcid_opts; /* 808 4 */
> unsigned int loopback:1; /* 812: 0 4 */
> unsigned int recv_own_msgs:1; /* 812: 1 4 */
> unsigned int fd_frames:1; /* 812: 2 4 */
> unsigned int xl_frames:1; /* 812: 3 4 */
>
> /* XXX 28 bits 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 */
> can_err_mask_t err_mask; /* 828 4 */
> /* --- cacheline 13 boundary (832 bytes) --- */
> int join_filters; /* 832 4 */
> int count; /* 836 4 */
> struct can_filter dfilter; /* 840 8 */
> struct can_filter * filter; /* 848 8 */
> struct uniqframe * uniq; /* 856 8 */
>
> /* size: 864, cachelines: 14, members: 20 */
> /* sum members: 860 */
> /* sum bitfield members: 4 bits, bit holes: 1, sum bit holes: 28 bits */
> /* member types with bit holes: 1, total: 1 */
> /* forced alignments: 1 */
> /* last cacheline: 32 bytes */
> } __attribute__((__aligned__(8)));
>
> Signed-off-by: Vincent Mailhol <mailhol@kernel.org>
Acked-by: Oliver Hartkopp <socketcan@hartkopp.net>
> ---
> net/can/raw.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/net/can/raw.c b/net/can/raw.c
> index cec580ecd58e36931d1be05716e6beb9c93aa271..81f5de63bcfaacf3f51670159fb3d1d7d1fc6020 100644
> --- a/net/can/raw.c
> +++ b/net/can/raw.c
> @@ -87,19 +87,19 @@ struct raw_sock {
> struct net_device *dev;
> netdevice_tracker dev_tracker;
> struct list_head notifier;
> + struct can_raw_vcid_options raw_vcid_opts;
> 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;
> canid_t rx_vcid_mask_shifted;
> + can_err_mask_t err_mask;
> int join_filters;
> int count; /* number of active filters */
> struct can_filter dfilter; /* default/single filter */
> struct can_filter *filter; /* pointer to filter(s) */
> - can_err_mask_t err_mask;
> struct uniqframe __percpu *uniq;
> };
>
>
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock
2025-09-15 9:23 ` [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock Vincent Mailhol
2025-09-15 10:16 ` Oliver Hartkopp
@ 2025-09-16 18:35 ` Christophe JAILLET
2025-09-17 4:43 ` Vincent Mailhol
1 sibling, 1 reply; 11+ messages in thread
From: Christophe JAILLET @ 2025-09-16 18:35 UTC (permalink / raw)
To: Vincent Mailhol, Oliver Hartkopp, Marc Kleine-Budde
Cc: linux-can, linux-kernel
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@kernel.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
^ permalink raw reply [flat|nested] 11+ messages in thread
* Re: [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock
2025-09-16 18:35 ` Christophe JAILLET
@ 2025-09-17 4:43 ` Vincent Mailhol
0 siblings, 0 replies; 11+ messages in thread
From: Vincent Mailhol @ 2025-09-17 4:43 UTC (permalink / raw)
To: Christophe JAILLET, Oliver Hartkopp, Marc Kleine-Budde
Cc: linux-can, linux-kernel
On 17/09/2025 at 03:35, Christophe JAILLET wrote:
> Le 15/09/2025 à 11:23, Vincent Mailhol a écrit :
(...)
>> --- 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.
You are absolutely right. I will add these two to the bitfield in v2.
Yours sincerely,
Vincent Mailhol
^ permalink raw reply [flat|nested] 11+ messages in thread
end of thread, other threads:[~2025-09-17 4:43 UTC | newest]
Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-09-15 9:23 [PATCH 0/3] can: raw: optimize the sizes of struct uniqframe and struct raw_sock Vincent Mailhol
2025-09-15 9:23 ` [PATCH 1/3] can: raw: reorder struct uniqframe's members to optimise packing Vincent Mailhol
2025-09-15 10:18 ` Oliver Hartkopp
2025-09-15 9:23 ` [PATCH 2/3] can: raw: use bitfields to store flags in struct raw_sock Vincent Mailhol
2025-09-15 10:16 ` Oliver Hartkopp
2025-09-15 10:47 ` Vincent Mailhol
2025-09-15 17:41 ` Oliver Hartkopp
2025-09-16 18:35 ` Christophe JAILLET
2025-09-17 4:43 ` Vincent Mailhol
2025-09-15 9:23 ` [PATCH 3/3] can: raw: reorder struct raw_sock's members to optimise packing Vincent Mailhol
2025-09-15 17:42 ` Oliver Hartkopp
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox