From be1f4f48cef17c6ea9350db04c774dc8eb252158 Mon Sep 17 00:00:00 2001 From: David Laight Date: Fri, 4 Jul 2014 14:35:51 +0000 Subject: [PATCH 1/3] net: sctp: Inline the functions from command.c sctp_init_cmd_seq() and sctp_next_cmd() are only called from one place. The call sequence for sctp_add_cmd_sf() is likely to be longer than the inlined code. With sctp_add_cmd_sf() inlined the compiler can optimise repeated calls. Signed-off-by: David Laight Signed-off-by: David S. Miller --- include/net/sctp/command.h | 27 ++++++++++++--- net/sctp/Makefile | 2 +- net/sctp/command.c | 68 -------------------------------------- 3 files changed, 24 insertions(+), 73 deletions(-) delete mode 100644 net/sctp/command.c diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h index 4b7cd695e431..0e91a42065db 100644 --- a/include/net/sctp/command.h +++ b/include/net/sctp/command.h @@ -210,19 +210,38 @@ typedef struct { /* Initialize a block of memory as a command sequence. * Return 0 if the initialization fails. */ -int sctp_init_cmd_seq(sctp_cmd_seq_t *seq); +static inline int sctp_init_cmd_seq(sctp_cmd_seq_t *seq) +{ + memset(seq, 0, sizeof(sctp_cmd_seq_t)); + return 1; /* We always succeed. */ +} + /* Add a command to an sctp_cmd_seq_t. * * Use the SCTP_* constructors defined by SCTP_ARG_CONSTRUCTOR() above * to wrap data which goes in the obj argument. */ -void sctp_add_cmd_sf(sctp_cmd_seq_t *seq, sctp_verb_t verb, sctp_arg_t obj); +static inline void sctp_add_cmd_sf(sctp_cmd_seq_t *seq, sctp_verb_t verb, + sctp_arg_t obj) +{ + BUG_ON(seq->next_free_slot >= SCTP_MAX_NUM_COMMANDS); + + seq->cmds[seq->next_free_slot].verb = verb; + seq->cmds[seq->next_free_slot++].obj = obj; +} /* Return the next command structure in an sctp_cmd_seq. * Return NULL at the end of the sequence. */ -sctp_cmd_t *sctp_next_cmd(sctp_cmd_seq_t *seq); +static inline sctp_cmd_t *sctp_next_cmd(sctp_cmd_seq_t *seq) +{ + sctp_cmd_t *retval = NULL; + + if (seq->next_cmd < seq->next_free_slot) + retval = &seq->cmds[seq->next_cmd++]; + + return retval; +} #endif /* __net_sctp_command_h__ */ - diff --git a/net/sctp/Makefile b/net/sctp/Makefile index 5c30b7a873df..3b4ffb021cf1 100644 --- a/net/sctp/Makefile +++ b/net/sctp/Makefile @@ -8,7 +8,7 @@ obj-$(CONFIG_NET_SCTPPROBE) += sctp_probe.o sctp-y := sm_statetable.o sm_statefuns.o sm_sideeffect.o \ protocol.o endpointola.o associola.o \ transport.o chunk.o sm_make_chunk.o ulpevent.o \ - inqueue.o outqueue.o ulpqueue.o command.o \ + inqueue.o outqueue.o ulpqueue.o \ tsnmap.o bind_addr.o socket.o primitive.o \ output.o input.o debug.o ssnmap.o auth.o diff --git a/net/sctp/command.c b/net/sctp/command.c deleted file mode 100644 index dd7375851618..000000000000 --- a/net/sctp/command.c +++ /dev/null @@ -1,68 +0,0 @@ -/* SCTP kernel implementation Copyright (C) 1999-2001 - * Cisco, Motorola, and IBM - * Copyright 2001 La Monte H.P. Yarroll - * - * This file is part of the SCTP kernel implementation - * - * These functions manipulate sctp command sequences. - * - * This SCTP implementation is free software; - * you can redistribute it and/or modify it under the terms of - * the GNU General Public License as published by - * the Free Software Foundation; either version 2, or (at your option) - * any later version. - * - * This SCTP implementation is distributed in the hope that it - * will be useful, but WITHOUT ANY WARRANTY; without even the implied - * ************************ - * warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. - * See the GNU General Public License for more details. - * - * You should have received a copy of the GNU General Public License - * along with GNU CC; see the file COPYING. If not, see - * . - * - * Please send any bug reports or fixes you make to the - * email address(es): - * lksctp developers - * - * Written or modified by: - * La Monte H.P. Yarroll - * Karl Knutson - */ - -#include -#include -#include - -/* Initialize a block of memory as a command sequence. */ -int sctp_init_cmd_seq(sctp_cmd_seq_t *seq) -{ - memset(seq, 0, sizeof(sctp_cmd_seq_t)); - return 1; /* We always succeed. */ -} - -/* Add a command to a sctp_cmd_seq_t. - * Return 0 if the command sequence is full. - */ -void sctp_add_cmd_sf(sctp_cmd_seq_t *seq, sctp_verb_t verb, sctp_arg_t obj) -{ - BUG_ON(seq->next_free_slot >= SCTP_MAX_NUM_COMMANDS); - - seq->cmds[seq->next_free_slot].verb = verb; - seq->cmds[seq->next_free_slot++].obj = obj; -} - -/* Return the next command structure in a sctp_cmd_seq. - * Returns NULL at the end of the sequence. - */ -sctp_cmd_t *sctp_next_cmd(sctp_cmd_seq_t *seq) -{ - sctp_cmd_t *retval = NULL; - - if (seq->next_cmd < seq->next_free_slot) - retval = &seq->cmds[seq->next_cmd++]; - - return retval; -} - From b9420e1c87838bcb354ae3495852430413dd9e4b Mon Sep 17 00:00:00 2001 From: David Laight Date: Fri, 4 Jul 2014 14:35:51 +0000 Subject: [PATCH 2/3] net: sctp: Optimise the way 'sctp_arg_t' values are initialised. Even if memset() is inlined (as on x86) using it to zero the union generates a memory word write of zero, followed by a write of the smaller field, and then a read of the word. As well as being a lot of instructions the sequence is unlikely to be optimised by the store-load forward hardware so will be slow. Instead allocate a field of the union that is the same size as the entire union and write a zero value to it. The compiler will then generate the required value in a register. Zeroing the union shouldn't be necessary, but this patch series isn't intended to have a behavioural change. Signed-off-by: David Laight Signed-off-by: David S. Miller --- include/net/sctp/command.h | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h index 0e91a42065db..589a1918ad55 100644 --- a/include/net/sctp/command.h +++ b/include/net/sctp/command.h @@ -118,6 +118,7 @@ typedef enum { #define SCTP_MAX_NUM_COMMANDS 14 typedef union { + void *zero_all; /* Set to NULL to clear the entire union */ __s32 i32; __u32 u32; __be32 be32; @@ -154,7 +155,7 @@ typedef union { static inline sctp_arg_t \ SCTP_## name (type arg) \ { sctp_arg_t retval;\ - memset(&retval, 0, sizeof(sctp_arg_t));\ + retval.zero_all = NULL;\ retval.elt = arg;\ return retval;\ } @@ -191,7 +192,7 @@ static inline sctp_arg_t SCTP_NOFORCE(void) static inline sctp_arg_t SCTP_NULL(void) { sctp_arg_t retval; - memset(&retval, 0, sizeof(sctp_arg_t)); + retval.zero_all = NULL; return retval; } @@ -212,7 +213,8 @@ typedef struct { */ static inline int sctp_init_cmd_seq(sctp_cmd_seq_t *seq) { - memset(seq, 0, sizeof(sctp_cmd_seq_t)); + seq->next_free_slot = 0; + seq->next_cmd = 0; return 1; /* We always succeed. */ } From d1a3fe26e97c0f17579c39f8a446c50f7cc3154a Mon Sep 17 00:00:00 2001 From: David Laight Date: Fri, 4 Jul 2014 14:35:57 +0000 Subject: [PATCH 3/3] net: sctp: Use pointers (not array indexes) to access sctp_cmd_seq_t.cmds[]. Using pointers into sctp_cmd_seq_t.cmds[] lets the compiler generate much better code. Use the last entry first to optimise the overflow check. Signed-off-by: David Laight Signed-off-by: David S. Miller --- include/net/sctp/command.h | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/include/net/sctp/command.h b/include/net/sctp/command.h index 589a1918ad55..f22538e68245 100644 --- a/include/net/sctp/command.h +++ b/include/net/sctp/command.h @@ -203,8 +203,8 @@ typedef struct { typedef struct { sctp_cmd_t cmds[SCTP_MAX_NUM_COMMANDS]; - __u8 next_free_slot; - __u8 next_cmd; + sctp_cmd_t *last_used_slot; + sctp_cmd_t *next_cmd; } sctp_cmd_seq_t; @@ -213,8 +213,9 @@ typedef struct { */ static inline int sctp_init_cmd_seq(sctp_cmd_seq_t *seq) { - seq->next_free_slot = 0; - seq->next_cmd = 0; + /* cmds[] is filled backwards to simplify the overflow BUG() check */ + seq->last_used_slot = seq->cmds + SCTP_MAX_NUM_COMMANDS; + seq->next_cmd = seq->last_used_slot; return 1; /* We always succeed. */ } @@ -227,10 +228,13 @@ static inline int sctp_init_cmd_seq(sctp_cmd_seq_t *seq) static inline void sctp_add_cmd_sf(sctp_cmd_seq_t *seq, sctp_verb_t verb, sctp_arg_t obj) { - BUG_ON(seq->next_free_slot >= SCTP_MAX_NUM_COMMANDS); + sctp_cmd_t *cmd = seq->last_used_slot - 1; - seq->cmds[seq->next_free_slot].verb = verb; - seq->cmds[seq->next_free_slot++].obj = obj; + BUG_ON(cmd < seq->cmds); + + cmd->verb = verb; + cmd->obj = obj; + seq->last_used_slot = cmd; } /* Return the next command structure in an sctp_cmd_seq. @@ -238,12 +242,10 @@ static inline void sctp_add_cmd_sf(sctp_cmd_seq_t *seq, sctp_verb_t verb, */ static inline sctp_cmd_t *sctp_next_cmd(sctp_cmd_seq_t *seq) { - sctp_cmd_t *retval = NULL; + if (seq->next_cmd <= seq->last_used_slot) + return NULL; - if (seq->next_cmd < seq->next_free_slot) - retval = &seq->cmds[seq->next_cmd++]; - - return retval; + return --seq->next_cmd; } #endif /* __net_sctp_command_h__ */