Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1362397 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH v2 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k Christoph Hellwig <hch@infradead.org> - 2016-03-22 09:00 +0100
    [PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-03-22 20:40 +0100
      [PATCH v3 1/3] scsi: make some Additional Sense strings more grep'able Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-03-22 20:40 +0100
      [PATCH v3 3/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k Rasmus Villemoes <linux@rasmusvillemoes.dk> - 2016-03-22 20:40 +0100
      Re: [PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k Christoph Hellwig <hch@infradead.org> - 2016-03-23 08:50 +0100
      Re: [PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k "Martin K. Petersen" <martin.petersen@oracle.com> - 2016-03-23 22:00 +0100

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

On Tue, Mar 08, 2016 at 08:52:32PM +0100, Rasmus Villemoes wrote:
> On Tue, Nov 24 2015, Rasmus Villemoes <linux@rasmusvillemoes.dk> wrote:
> 
> > This reduces the impact of choosing CONFIG_SCSI_CONSTANTS by about 8KB.
> >
> 
> ping? does anyone feel like picking these up?

Please resend them against the current tree.  I'm pretty sure I acked
your appraoch, and if I didn't give you an actualy review I'll happily
do that after a repost.

[toc] | [next] | [standalone]

#1362927 — [PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

This is mostly identical to v2 sent in November, rebased (cleanly) to
current mainline. The only change is that I've this time also updated
the Kconfig help text in 3/3, since the 75 KB was a bit
exaggerated. After these patches are applied, the impact of choosing
CONFIG_SCSI_CONSTANTS=y is about 36 KB, while it was about 44 KB
before. I took the liberty of including Hannes' Reviewed-by and
Douglas' Tested-by despite that addition.

Rasmus Villemoes (3):
  scsi: make some Additional Sense strings more grep'able
  scsi: move Additional Sense Codes to separate file
  scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

 drivers/scsi/Kconfig       |   4 +-
 drivers/scsi/constants.c   | 859 ++-------------------------------------------
 drivers/scsi/sense_codes.h | 826 +++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 849 insertions(+), 840 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1362928 — [PATCH v3 1/3] scsi: make some Additional Sense strings more grep'able

There's little point in breaking these strings over multiple lines.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Hannes Reinecke <hare@suse.com>
---
 drivers/scsi/constants.c | 27 +++++++++------------------
 1 file changed, 9 insertions(+), 18 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index fa09d4be2b53..58d94e3c3713 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -346,11 +346,9 @@ static const struct error_info additional[] =
 	{0x0407, "Logical unit not ready, operation in progress"},
 	{0x0408, "Logical unit not ready, long write in progress"},
 	{0x0409, "Logical unit not ready, self-test in progress"},
-	{0x040A, "Logical unit not accessible, asymmetric access state "
-	 "transition"},
+	{0x040A, "Logical unit not accessible, asymmetric access state transition"},
 	{0x040B, "Logical unit not accessible, target port in standby state"},
-	{0x040C, "Logical unit not accessible, target port in unavailable "
-	 "state"},
+	{0x040C, "Logical unit not accessible, target port in unavailable state"},
 	{0x040D, "Logical unit not ready, structure check required"},
 	{0x040E, "Logical unit not ready, security session in progress"},
 	{0x0410, "Logical unit not ready, auxiliary memory not accessible"},
@@ -363,11 +361,9 @@ static const struct error_info additional[] =
 	{0x0417, "Logical unit not ready, calibration required"},
 	{0x0418, "Logical unit not ready, a door is open"},
 	{0x0419, "Logical unit not ready, operating in sequential mode"},
-	{0x041A, "Logical unit not ready, start stop unit command in "
-	 "progress"},
+	{0x041A, "Logical unit not ready, start stop unit command in progress"},
 	{0x041B, "Logical unit not ready, sanitize in progress"},
-	{0x041C, "Logical unit not ready, additional power use not yet "
-	 "granted"},
+	{0x041C, "Logical unit not ready, additional power use not yet granted"},
 	{0x041D, "Logical unit not ready, configuration in progress"},
 	{0x041E, "Logical unit not ready, microcode activation required"},
 	{0x041F, "Logical unit not ready, microcode download required"},
@@ -559,8 +555,7 @@ static const struct error_info additional[] =
 	{0x2300, "Invalid token operation, cause not reportable"},
 	{0x2301, "Invalid token operation, unsupported token type"},
 	{0x2302, "Invalid token operation, remote token usage not supported"},
-	{0x2303, "Invalid token operation, remote rod token creation not "
-	 "supported"},
+	{0x2303, "Invalid token operation, remote rod token creation not supported"},
 	{0x2304, "Invalid token operation, token unknown"},
 	{0x2305, "Invalid token operation, token corrupt"},
 	{0x2306, "Invalid token operation, token revoked"},
@@ -641,8 +636,7 @@ static const struct error_info additional[] =
 	{0x2A0D, "Data encryption capabilities changed"},
 	{0x2A10, "Timestamp changed"},
 	{0x2A11, "Data encryption parameters changed by another i_t nexus"},
-	{0x2A12, "Data encryption parameters changed by vendor specific "
-		 "event"},
+	{0x2A12, "Data encryption parameters changed by vendor specific event"},
 	{0x2A13, "Data encryption key instance counter has changed"},
 	{0x2A14, "SA creation capabilities data has changed"},
 	{0x2A15, "Medium removal prevention preempted"},
@@ -759,8 +753,7 @@ static const struct error_info additional[] =
 	{0x3B19, "Element enabled"},
 	{0x3B1A, "Data transfer device removed"},
 	{0x3B1B, "Data transfer device inserted"},
-	{0x3B1C, "Too many logical objects on partition to support "
-	 "operation"},
+	{0x3B1C, "Too many logical objects on partition to support operation"},
 
 	{0x3D00, "Invalid bits in identify message"},
 
@@ -957,8 +950,7 @@ static const struct error_info additional[] =
 	{0x5D39, "Data channel impending failure throughput performance"},
 	{0x5D3A, "Data channel impending failure seek time performance"},
 	{0x5D3B, "Data channel impending failure spin-up retry count"},
-	{0x5D3C, "Data channel impending failure drive calibration retry "
-	 "count"},
+	{0x5D3C, "Data channel impending failure drive calibration retry count"},
 	{0x5D40, "Servo impending failure general hard drive failure"},
 	{0x5D41, "Servo impending failure drive error rate too high"},
 	{0x5D42, "Servo impending failure data error rate too high"},
@@ -1070,8 +1062,7 @@ static const struct error_info additional[] =
 
 	{0x6E00, "Command to logical unit failed"},
 
-	{0x6F00, "Copy protection key exchange failure - authentication "
-	 "failure"},
+	{0x6F00, "Copy protection key exchange failure - authentication failure"},
 	{0x6F01, "Copy protection key exchange failure - key not present"},
 	{0x6F02, "Copy protection key exchange failure - key not established"},
 	{0x6F03, "Read of scrambled sector without authentication"},
-- 
2.1.4

[toc] | [prev] | [next] | [standalone]

#1362930 — [PATCH v3 3/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

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                                 11312    2824   -8488

The Kconfig help text used to say that CONFIG_SCSI_CONSTANTS=y costs
around 75 KB, but that was a little exaggerated. The actual number was
closer to 44K, and 36K with this patch.

Signed-off-by: Rasmus Villemoes <linux@rasmusvillemoes.dk>
Reviewed-by: Hannes Reinecke <hare@suse.com>
Tested-by: Douglas Gilbert <dgilbert@interlog.com>
---
 drivers/scsi/Kconfig     |  4 ++--
 drivers/scsi/constants.c | 26 +++++++++++++++++++++-----
 2 files changed, 23 insertions(+), 7 deletions(-)

diff --git a/drivers/scsi/Kconfig b/drivers/scsi/Kconfig
index e80768f8e579..47611bda633f 100644
--- a/drivers/scsi/Kconfig
+++ b/drivers/scsi/Kconfig
@@ -202,12 +202,12 @@ config SCSI_ENCLOSURE
 	  certain enclosure conditions to be reported and is not required.
 
 config SCSI_CONSTANTS
-	bool "Verbose SCSI error reporting (kernel size +=75K)"
+	bool "Verbose SCSI error reporting (kernel size += 36K)"
 	depends on SCSI
 	help
 	  The error messages regarding your SCSI hardware will be easier to
 	  understand if you say Y here; it will enlarge your kernel by about
-	  75 KB. If in doubt, say Y.
+	  36 KB. If in doubt, say Y.
 
 config SCSI_LOGGING
 	bool "SCSI logging facility"
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.1.4

[toc] | [prev] | [next] | [standalone]

#1363228 — Re: [PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

Thanks Rasmus,

the whole series looks good to me:

Reviewed-by: Christoph Hellwig <hch@lst.de>

[toc] | [prev] | [next] | [standalone]

#1363687 — Re: [PATCH v3 0/3] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

>>>>> "Rasmus" == Rasmus Villemoes <linux@rasmusvillemoes.dk> writes:

Rasmus> This is mostly identical to v2 sent in November, rebased
Rasmus> (cleanly) to current mainline. The only change is that I've this
Rasmus> time also updated the Kconfig help text in 3/3, since the 75 KB
Rasmus> was a bit exaggerated. After these patches are applied, the
Rasmus> impact of choosing CONFIG_SCSI_CONSTANTS=y is about 36 KB, while
Rasmus> it was about 44 KB before. I took the liberty of including
Rasmus> Hannes' Reviewed-by and Douglas' Tested-by despite that
Rasmus> addition.

Applied to 4.7/scsi-queue.

Thank you!

-- 
Martin K. Petersen	Oracle Linux Engineering

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web