On Wed, Apr 06, 2016 at 03:57:35PM -0400, David Miller wrote:
> From: Joe Perches <joe@xxxxxxxxxxx>
> Date: Wed, 06 Apr 2016 12:53:24 -0700
>
> > On Wed, 2016-04-06 at 14:53 -0300, Marcelo Ricardo Leitner wrote:
> >> It wastes space and gets worse as we add new flags, so convert bit-wide
> >> flags to a bitfield.
> >>
> >> Currently it already saves 4 bytes in sctp_sock, which are left as holes
> >> in it for now. The whole struct needs packing, which should be done in
> >> another patch.
> >>
> >> Note that do_auto_asconf cannot be merged, as explained in the comment
> >> before it.
> >>
> >> Signed-off-by: Marcelo Ricardo Leitner <marcelo.leitner@xxxxxxxxx>
> >> ---
> > []
> >> diff --git a/include/net/sctp/structs.h b/include/net/sctp/structs.h
> > []
> >> @@ -210,14 +210,14 @@ struct sctp_sock {
> >> int user_frag;
> >>
> >> __u32 autoclose;
> >> - __u8 nodelay;
> >> - __u8 disable_fragments;
> >> - __u8 v4mapped;
> >> - __u8 frag_interleave;
> >> __u32 adaptation_ind;
> >> __u32 pd_point;
> >> - __u8 recvrcvinfo;
> >> - __u8 recvnxtinfo;
> >> + __u16 nodelay:1,
> >> + disable_fragments:1,
> >> + v4mapped:1,
> >> + frag_interleave:1,
> >> + recvrcvinfo:1,
> >> + recvnxtinfo:1;
> >
> > Might as well make this __u32 as the next field would be
> > aligned on an atomic_t
On changelog I meant that this (re-)ordering will happen in another
patch. The hole is already there today and there are others to be
considered/fixed too. Please consider this patch as a preparation for
the other one.
After this patch, pahole gives for this struct:
/* size: 1272, cachelines: 20, members: 40 */
/* sum members: 1250, holes: 7, sum holes: 18 */
/* bit holes: 1, sum bit holes: 9 bits */
/* padding: 4 */
/* paddings: 1, sum paddings: 2 */
/* last cacheline: 56 bytes */
Quite some work todo :-)
> > It might be better if these fields didn't use the __ prefix.
>
> Indeed, this isn't in a UAPI file so __ prefixed types really aren't
> appropriate.
The whole file is using __ prefixed types. I can remove them in another
patch instead if that's okay with you.
It's also present in other files not under UAPI:
$ git grep -wl '\(__u32\|__u16\)' include/net/sctp/
include/net/sctp/auth.h
include/net/sctp/checksum.h
include/net/sctp/command.h
include/net/sctp/constants.h
include/net/sctp/sctp.h
include/net/sctp/sm.h
include/net/sctp/structs.h
include/net/sctp/tsnmap.h
include/net/sctp/ulpevent.h
include/net/sctp/ulpqueue.h
We can start changing it progressively but I think it would be better if
we replace them all at once.
Marcelo
--
To unsubscribe from this list: send the line "unsubscribe linux-sctp" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
