Re: [PATCH v2 3/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Douglas Gilbert]

On 15-11-24 04:42 AM, Rasmus Villemoes wrote:

On 64 bit, struct error_info has 6 bytes of padding, which amounts to
over 4k of wasted space in the additional[] array. We could easily get
rid of that by instead using separate arrays for the codes and the
pointers. However, we can do even better than that and save an
additional 6 bytes per entry: In the table, just store the sizeof()
the corresponding string literal. The cumulative sum of these is then
the appropriate offset into additional_text, which is built from the
concatenation (with '\0's inbetween) of the strings.

$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
function old new   delta
scsi_extd_sense_format   136 160 +24
additional 113122824   -8488

Signed-off-by: Rasmus Villemoes 


Tested-by: Douglas Gilbert 


---
  drivers/scsi/constants.c | 26 +-
  1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 6e813eec4f8d..83458f7a2824 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,30 @@ bool scsi_opcode_sa_name(int opcode, int service_action,

  struct error_info {
unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+   unsigned short size;
  };

+/*
+ * There are 700+ entries in this table. To save space, we don't store
+ * (code, pointer) pairs, which would make sizeof(struct
+ * error_info)==16 on 64 bits. Rather, the second element just stores
+ * the size (including \0) of the corresponding string, and we use the
+ * sum of these to get the appropriate offset into additional_text
+ * defined below. This approach saves 12 bytes per entry.
+ */
  static const struct error_info additional[] =
  {
-#define SENSE_CODE(c, s) {c, s},
+#define SENSE_CODE(c, s) {c, sizeof(s)},
  #include "sense_codes.h"
  #undef SENSE_CODE
-   {0, NULL}
  };

+static const char *additional_text =
+#define SENSE_CODE(c, s) s "\0"
+#include "sense_codes.h"
+#undef SENSE_CODE
+   ;
+
  struct error_info2 {
unsigned char code1, code2_min, code2_max;
const char * str;
@@ -364,11 +377,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
  {
int i;
unsigned short code = ((asc << 8) | ascq);
+   unsigned offset = 0;

*fmt = NULL;
-   for (i = 0; additional[i].text; i++)
+   for (i = 0; i < ARRAY_SIZE(additional); i++) {
if (additional[i].code12 == code)
-   return additional[i].text;
+   return additional_text + offset;
+   offset += additional[i].size;
+   }
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc &&
ascq >= additional2[i].code2_min &&



--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[PATCH v2 3/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Rasmus Villemoes]

On 64 bit, struct error_info has 6 bytes of padding, which amounts to
over 4k of wasted space in the additional[] array. We could easily get
rid of that by instead using separate arrays for the codes and the
pointers. However, we can do even better than that and save an
additional 6 bytes per entry: In the table, just store the sizeof()
the corresponding string literal. The cumulative sum of these is then
the appropriate offset into additional_text, which is built from the
concatenation (with '\0's inbetween) of the strings.

$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 0/0 grow/shrink: 1/1 up/down: 24/-8488 (-8464)
function old new   delta
scsi_extd_sense_format   136 160 +24
additional 113122824   -8488

Signed-off-by: Rasmus Villemoes 
---
 drivers/scsi/constants.c | 26 +-
 1 file changed, 21 insertions(+), 5 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 6e813eec4f8d..83458f7a2824 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,30 @@ bool scsi_opcode_sa_name(int opcode, int service_action,
 
 struct error_info {
unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+   unsigned short size;
 };
 
+/*
+ * There are 700+ entries in this table. To save space, we don't store
+ * (code, pointer) pairs, which would make sizeof(struct
+ * error_info)==16 on 64 bits. Rather, the second element just stores
+ * the size (including \0) of the corresponding string, and we use the
+ * sum of these to get the appropriate offset into additional_text
+ * defined below. This approach saves 12 bytes per entry.
+ */
 static const struct error_info additional[] =
 {
-#define SENSE_CODE(c, s) {c, s},
+#define SENSE_CODE(c, s) {c, sizeof(s)},
 #include "sense_codes.h"
 #undef SENSE_CODE
-   {0, NULL}
 };
 
+static const char *additional_text =
+#define SENSE_CODE(c, s) s "\0"
+#include "sense_codes.h"
+#undef SENSE_CODE
+   ;
+
 struct error_info2 {
unsigned char code1, code2_min, code2_max;
const char * str;
@@ -364,11 +377,14 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
 {
int i;
unsigned short code = ((asc << 8) | ascq);
+   unsigned offset = 0;
 
*fmt = NULL;
-   for (i = 0; additional[i].text; i++)
+   for (i = 0; i < ARRAY_SIZE(additional); i++) {
if (additional[i].code12 == code)
-   return additional[i].text;
+   return additional_text + offset;
+   offset += additional[i].size;
+   }
for (i = 0; additional2[i].fmt; i++) {
if (additional2[i].code1 == asc &&
ascq >= additional2[i].code2_min &&
-- 
2.6.1

--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html