Hi Stephen,
On 21/08/2023 03:06, Stephen Rothwell wrote:
> Hi all,
>
> Today's linux-next merge of the net-next tree got a conflict in:
>
> include/net/inet_sock.h
>
> between commit:
>
> f866fbc842de ("ipv4: fix data-races around inet->inet_id")
>
> from the net tree and commit:
>
> c274af224269 ("inet: introduce inet->inet_flags")
>
> from the net-next tree.
Thank you for this conflict resolution!
I had the same issue on our side with MPTCP tree when syncing -net and
net-next and I resolved it a bit differently. Here my comment on the
patch you used:
> diff --cc include/net/inet_sock.h
> index 491ceb7ebe5d,acbb93d7607a..000000000000
> --- a/include/net/inet_sock.h
> +++ b/include/net/inet_sock.h
> @@@ -218,12 -218,12 +218,12 @@@ struct inet_sock
> #define inet_dport sk.__sk_common.skc_dport
> #define inet_num sk.__sk_common.skc_num
>
> + unsigned long inet_flags;
> __be32 inet_saddr;
> __s16 uc_ttl;
> - __u16 cmsg_flags;
> - struct ip_options_rcu __rcu *inet_opt;
> - atomic_t inet_id;
> __be16 inet_sport;
> ++ atomic_t inet_id;
> + struct ip_options_rcu __rcu *inet_opt;
I first put "inet_opt", then "inet_id" here to avoid a hole of 4 bytes.
So just switching these two lines.
Here is the output of pahole when using your patch:
> struct inet_sock {
> struct sock sk __attribute__((__aligned__(8))); /* 0 768 */
> /* --- cacheline 12 boundary (768 bytes) --- */
> struct ipv6_pinfo * pinet6; /* 768 8 */
> long unsigned int inet_flags; /* 776 8 */
> __be32 inet_saddr; /* 784 4 */
> __s16 uc_ttl; /* 788 2 */
> __be16 inet_sport; /* 790 2 */
> atomic_t inet_id; /* 792 4 */
>
> /* XXX 4 bytes hole, try to pack */
>
> struct ip_options_rcu * inet_opt; /* 800 8 */
> __u8 tos; /* 808 1 */
> __u8 min_ttl; /* 809 1 */
> __u8 mc_ttl; /* 810 1 */
> __u8 pmtudisc; /* 811 1 */
> __u8 rcv_tos; /* 812 1 */
> __u8 convert_csum; /* 813 1 */
>
> /* XXX 2 bytes hole, try to pack */
>
> int uc_index; /* 816 4 */
> int mc_index; /* 820 4 */
> __be32 mc_addr; /* 824 4 */
And now when "inet_id" is placed after "inet_opt":
> struct inet_sock {
> struct sock sk __attribute__((__aligned__(8))); /* 0 768 */
> /* --- cacheline 12 boundary (768 bytes) --- */
> struct ipv6_pinfo * pinet6; /* 768 8 */
> long unsigned int inet_flags; /* 776 8 */
> __be32 inet_saddr; /* 784 4 */
> __s16 uc_ttl; /* 788 2 */
> __be16 inet_sport; /* 790 2 */
> struct ip_options_rcu * inet_opt; /* 792 8 */
> atomic_t inet_id; /* 800 4 */
> __u8 tos; /* 804 1 */
> __u8 min_ttl; /* 805 1 */
> __u8 mc_ttl; /* 806 1 */
> __u8 pmtudisc; /* 807 1 */
> __u8 rcv_tos; /* 808 1 */
> __u8 convert_csum; /* 809 1 */
>
> /* XXX 2 bytes hole, try to pack */
>
> int uc_index; /* 812 4 */
> int mc_index; /* 816 4 */
> __be32 mc_addr; /* 820 4 */
I noticed that in Eric's patch for the net tree -- f866fbc842de ("ipv4:
fix data-races around inet->inet_id") -- he moved "inet_id" above, I
guess for a similar reason.
Just in case you are interested by taking my version, I attached the
patch I used and the Rerere cache is available there:
https://github.com/multipath-tcp/mptcp-upstream-rr-cache/commit/e63f34f8
Cheers,
Matt
--
Tessares | Belgium | Hybrid Access Solutions
www.tessares.netdiff --cc include/net/inet_sock.h
index acbb93d7607a,491ceb7ebe5d..2de0e4d4a027
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@@ -218,12 -218,12 +218,12 @@@ struct inet_sock
#define inet_dport sk.__sk_common.skc_dport
#define inet_num sk.__sk_common.skc_num
+ unsigned long inet_flags;
__be32 inet_saddr;
__s16 uc_ttl;
- __u16 cmsg_flags;
+ __be16 inet_sport;
struct ip_options_rcu __rcu *inet_opt;
- __u16 inet_id;
+ atomic_t inet_id;
- __be16 inet_sport;
__u8 tos;
__u8 min_ttl;
