Re: [PATCH v3 1/6] ilog2: create truly constant version for sparse

[Rasmus Villemoes]

On 2018-04-18 23:21, Linus Torvalds wrote:
> On Wed, Apr 18, 2018 at 1:12 AM, Martin Wilck  wrote:
>>
>> No, it doesn't (gcc 7.3.0). -> https://paste.opensuse.org/27471594
>> It doesn't even warn on an expression like this:
>>
>>   #define SIZE (1<<10)
>>   static int foo[ilog2(SIZE)];
> 
> Ok, I think this is the "random gcc versions act differently" issue.
> 
> Sometimes __builtin_constant_p() to a ternary operation acts as a
> constant expression, and sometimes it doesn't.

So __builtin_constant_p on an actual ICE always fold immediately to 1.
Looking at the gcc history, that seems to have been there since at least
2000, but likely forever. And when it's the controlling expression of a
ternary, it's apparently a stronger 1 than a literal 1:

int foo(int);
#define SIZE (1<<10)
#define half(x) (__builtin_constant_p(x) ? (x)>>1 : foo(x))
int bla[half(SIZE)];
int bla2[1 ? 123 : foo(3)+2];

on godbolt.org, gcc 4.1 and gcc4.4 are perfectly happy with this, but
newer gccs complain

  error: variably modified 'bla2' at file scope.

Argh.

> I suspect using the __is_constexpr() trick would make it rather more
> technically correct, but would actually generate much worse code.
> 
> Because it would make us do that dynamic "__ilog2_uXX()" function call
> even when you have a compile-time constant value, if it wasn't
> actually a constant expression (ie a constant argument passed to an
> inline function, for example).

It's a bit ugly, but I suppose one could keep a __builtin_constant_p()
check in the not-ICE-branch, so there's really three cases (ICE,
constant due to various optimizations, really not known at compile
time). But don't we use gcc's intrinsics for fls when available, so that
gcc should be able to know the semantics of __builtin_fls(some-constant)
and hence evaluate that itself at compile time?

Somewhat unrelated, we should probably move the __is_constexpr helper
from kernel.h to some more basic compiler header, to avoid cyclic header
dependencies.

Rasmus

[PATCH] scsi: fnic: use kzalloc in fnic_fcoe_process_vlan_resp

[Rasmus Villemoes]

This saves a little .text and gets rid of the unmotivated line break and
the sizeof(...) style inconsistency.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/fnic/fnic_fcs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
index 999fc7547560..c7bf316d8e83 100644
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -442,15 +442,13 @@ static void fnic_fcoe_process_vlan_resp(struct fnic 
*fnic, struct sk_buff *skb)
vid = ntohs(((struct fip_vlan_desc *)desc)->fd_vlan);
shost_printk(KERN_INFO, fnic->lport->host,
  "process_vlan_resp: FIP VLAN %d\n", vid);
-   vlan = kmalloc(sizeof(*vlan),
-   GFP_ATOMIC);
+   vlan = kzalloc(sizeof(*vlan), GFP_ATOMIC);
if (!vlan) {
/* retry from timer */
spin_unlock_irqrestore(>vlans_lock,
flags);
goto out;
}
-   memset(vlan, 0, sizeof(struct fcoe_vlan));
vlan->vid = vid & 0x0fff;
vlan->state = FIP_VLAN_AVAIL;
list_add_tail(>list, >vlans);
-- 
2.15.1

[PATCH] target-core: don't use "const char*" for a buffer that is written to

[Rasmus Villemoes]

From: Rasmus Villemoes <rasmus.villem...@prevas.dk>

iscsi_parse_pr_out_transport_id launders the const away via a call to
strstr(), and then modifies the buffer (writing a nul byte) through
the return value. It's cleaner to be honest and simply declare the
parameter as "char*", fixing up the call chain, and allowing us to
drop the cast in the return statement.

Amusingly, the two current callers found it necessary to cast a
non-const pointer to a const.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/target/target_core_fabric_lib.c | 6 +++---
 drivers/target/target_core_internal.h   | 2 +-
 drivers/target/target_core_pr.c | 4 ++--
 3 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/drivers/target/target_core_fabric_lib.c 
b/drivers/target/target_core_fabric_lib.c
index 508da345b73f..71a80257a052 100644
--- a/drivers/target/target_core_fabric_lib.c
+++ b/drivers/target/target_core_fabric_lib.c
@@ -273,7 +273,7 @@ static int iscsi_get_pr_transport_id_len(
 
 static char *iscsi_parse_pr_out_transport_id(
struct se_portal_group *se_tpg,
-   const char *buf,
+   char *buf,
u32 *out_tid_len,
char **port_nexus_ptr)
 {
@@ -356,7 +356,7 @@ static char *iscsi_parse_pr_out_transport_id(
}
}
 
-   return (char *)[4];
+   return [4];
 }
 
 int target_get_pr_transport_id_len(struct se_node_acl *nacl,
@@ -405,7 +405,7 @@ int target_get_pr_transport_id(struct se_node_acl *nacl,
 }
 
 const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg,
-   const char *buf, u32 *out_tid_len, char **port_nexus_ptr)
+   char *buf, u32 *out_tid_len, char **port_nexus_ptr)
 {
u32 offset;
 
diff --git a/drivers/target/target_core_internal.h 
b/drivers/target/target_core_internal.h
index 18e3eb16e756..cada158cf1c2 100644
--- a/drivers/target/target_core_internal.h
+++ b/drivers/target/target_core_internal.h
@@ -101,7 +101,7 @@ int target_get_pr_transport_id(struct se_node_acl *nacl,
struct t10_pr_registration *pr_reg, int *format_code,
unsigned char *buf);
 const char *target_parse_pr_out_transport_id(struct se_portal_group *tpg,
-   const char *buf, u32 *out_tid_len, char **port_nexus_ptr);
+   char *buf, u32 *out_tid_len, char **port_nexus_ptr);
 
 /* target_core_hba.c */
 struct se_hba *core_alloc_hba(const char *, u32, u32);
diff --git a/drivers/target/target_core_pr.c b/drivers/target/target_core_pr.c
index dd2cd8048582..09941d1ae6c1 100644
--- a/drivers/target/target_core_pr.c
+++ b/drivers/target/target_core_pr.c
@@ -1597,7 +1597,7 @@ core_scsi3_decode_spec_i_port(
dest_rtpi = tmp_lun->lun_rtpi;
 
i_str = target_parse_pr_out_transport_id(tmp_tpg,
-   (const char *)ptr, _len, 
_ptr);
+   ptr, _len, _ptr);
if (!i_str)
continue;
 
@@ -3285,7 +3285,7 @@ core_scsi3_emulate_pro_register_and_move(struct se_cmd 
*cmd, u64 res_key,
goto out;
}
initiator_str = target_parse_pr_out_transport_id(dest_se_tpg,
-   (const char *)[24], _tid_len, _ptr);
+   [24], _tid_len, _ptr);
if (!initiator_str) {
pr_err("SPC-3 PR REGISTER_AND_MOVE: Unable to locate"
" initiator_str from Transport ID\n");
-- 
2.11.0

[PATCH] scsi: hpsa: use %phN for short hex dumps

[Rasmus Villemoes]

Passing one instead of 8 or 16 arguments reduces the size of the
generated code somewhat:

add/remove: 2/3 grow/shrink: 1/4 up/down: 1772/-2137 (-365)

There's one more candidate, unique_id_show, but that uses %02X, and I'm
not sure it would be ok to start using lowercase there, so I've left it
alone for now.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/hpsa.c | 40 +---
 1 file changed, 13 insertions(+), 27 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index a1d6ab76a514..d74268ffb71e 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -700,9 +700,7 @@ static ssize_t lunid_show(struct device *dev,
}
memcpy(lunid, hdev->scsi3addr, sizeof(lunid));
spin_unlock_irqrestore(>lock, flags);
-   return snprintf(buf, 20, "0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
-   lunid[0], lunid[1], lunid[2], lunid[3],
-   lunid[4], lunid[5], lunid[6], lunid[7]);
+   return snprintf(buf, 20, "0x%8phN\n", lunid);
 }
 
 static ssize_t unique_id_show(struct device *dev,
@@ -2824,14 +2822,8 @@ static void hpsa_print_cmd(struct ctlr_info *h, char 
*txt,
const u8 *cdb = c->Request.CDB;
const u8 *lun = c->Header.LUN.LunAddrBytes;
 
-   dev_warn(>pdev->dev, "%s: LUN:%02x%02x%02x%02x%02x%02x%02x%02x"
-   " 
CDB:%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x%02x\n",
-   txt, lun[0], lun[1], lun[2], lun[3],
-   lun[4], lun[5], lun[6], lun[7],
-   cdb[0], cdb[1], cdb[2], cdb[3],
-   cdb[4], cdb[5], cdb[6], cdb[7],
-   cdb[8], cdb[9], cdb[10], cdb[11],
-   cdb[12], cdb[13], cdb[14], cdb[15]);
+   dev_warn(>pdev->dev, "%s: LUN:%8phN CDB:%16phN\n",
+txt, lun, cdb);
 }
 
 static void hpsa_scsi_interpret_error(struct ctlr_info *h,
@@ -5999,11 +5991,9 @@ static int hpsa_send_reset_as_abort_ioaccel2(struct 
ctlr_info *h,
 
if (h->raid_offload_debug > 0)
dev_info(>pdev->dev,
-   "scsi %d:%d:%d:%d %s scsi3addr 
0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
+   "scsi %d:%d:%d:%d %s scsi3addr 0x%8phN\n",
h->scsi_host->host_no, dev->bus, dev->target, dev->lun,
-   "Reset as abort",
-   scsi3addr[0], scsi3addr[1], scsi3addr[2], scsi3addr[3],
-   scsi3addr[4], scsi3addr[5], scsi3addr[6], scsi3addr[7]);
+   "Reset as abort", scsi3addr);
 
if (!dev->offload_enabled) {
dev_warn(>pdev->dev,
@@ -6020,32 +6010,28 @@ static int hpsa_send_reset_as_abort_ioaccel2(struct 
ctlr_info *h,
/* send the reset */
if (h->raid_offload_debug > 0)
dev_info(>pdev->dev,
-   "Reset as abort: Resetting physical device at scsi3addr 
0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
-   psa[0], psa[1], psa[2], psa[3],
-   psa[4], psa[5], psa[6], psa[7]);
+   "Reset as abort: Resetting physical device at scsi3addr 
0x%8phN\n",
+   psa);
rc = hpsa_do_reset(h, dev, psa, HPSA_PHYS_TARGET_RESET, reply_queue);
if (rc != 0) {
dev_warn(>pdev->dev,
-   "Reset as abort: Failed on physical device at scsi3addr 
0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
-   psa[0], psa[1], psa[2], psa[3],
-   psa[4], psa[5], psa[6], psa[7]);
+   "Reset as abort: Failed on physical device at scsi3addr 
0x%8phN\n",
+   psa);
return rc; /* failed to reset */
}
 
/* wait for device to recover */
if (wait_for_device_to_become_ready(h, psa, reply_queue) != 0) {
dev_warn(>pdev->dev,
-   "Reset as abort: Failed: Device never recovered from 
reset: 0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
-   psa[0], psa[1], psa[2], psa[3],
-   psa[4], psa[5], psa[6], psa[7]);
+   "Reset as abort: Failed: Device never recovered from 
reset: 0x%8phN\n",
+   psa);
return -1;  /* failed to recover */
}
 
/* device recovered */
dev_info(>pdev->dev,
-   "Reset as abort: Device recovered from reset: scsi3addr 
0x%02x%02x%02x%02x%02x%02x%02x%02x\n",
-   psa[0], psa[1], psa[2], psa[3],
-   psa[4], psa[5], psa[6], psa[7]);
+   "Reset as abort: Device recovered from reset: scsi3addr 
0x%8phN\n",
+   psa);
 
return rc; /* success */
 }
-- 
2.1.4

--
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

Re: [PATCH v2 1/7] lib: string: add functions to case-convert strings

[Rasmus Villemoes]

On Tue, Jul 05 2016, Markus Mayer  wrote:

> Add a collection of generic functions to convert strings to lowercase
> or uppercase.
>
> Changing the case of a string (with or without copying it first) seems
> to be a recurring requirement in the kernel that is currently being
> solved by several duplicated implementations doing the same thing. This
> change aims at reducing this code duplication.
>
> +/**
> + * strncpytoupper - Copy a length-limited string and convert to uppercase.
> + * @dst: The buffer to store the result.
> + * @src: The string to convert to uppercase.
> + * @len: Maximum string length. May be 0 to set no limit.
> + *
> + * Returns pointer to terminating '\0' in @dst.
> + */
> +char *strncpytoupper(char *dst, const char *src, size_t len)
> +{
> + size_t i;
> +
> + for (i = 0; src[i] != '\0' && (i < len || !len); i++)
> + dst[i] = toupper(src[i]);
> + if (i < len || !len)
> + dst[i] = '\0';
> +
> + return dst + i;
> +}

Hm, this seems to copy the insane semantics from strncpy of not
guaranteeing '\0'-termination.

Why use 0 as a sentinel, when (size_t)-1 == SIZE_MAX would work just as
well and require a little less code (no || !len)?

I regret suggesting this return semantics and now agree that void would
be better, especially since there doesn't seem to be anyone who can
use this (or any other) return value. How about

if (!len)
   return;

for (i = 0; i < len && src[i]; ++i)
  dst[i] = toupper(src[i]);
dst[i < len ? i : i-1] = '\0';

(I think you must do i < len before testing src[i], since the len
parameter should be an upper bound on the number of bytes to access in
both src and dst).

Rasmus
--
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

Re: [PATCH 1/6] lib: string: add function strtolower()

[Rasmus Villemoes]

On Fri, Jul 01 2016, Markus Mayer  wrote:

> Add a function called strtolower() to convert strings to lower case
> in-place, overwriting the original string.
>
> This seems to be a recurring requirement in the kernel that is
> currently being solved by several duplicated implementations doing the
> same thing.
>
> Signed-off-by: Markus Mayer 
> ---
>  include/linux/string.h |  1 +
>  lib/string.c   | 14 ++
>  2 files changed, 15 insertions(+)
>
> diff --git a/include/linux/string.h b/include/linux/string.h
> index 26b6f6a..aad605e 100644
> --- a/include/linux/string.h
> +++ b/include/linux/string.h
> @@ -116,6 +116,7 @@ extern void * memchr(const void *,int,__kernel_size_t);
>  #endif
>  void *memchr_inv(const void *s, int c, size_t n);
>  char *strreplace(char *s, char old, char new);
> +char *strtolower(char *s);
>  
>  extern void kfree_const(const void *x);
>  
> diff --git a/lib/string.c b/lib/string.c
> index ed83562..6e3b560 100644
> --- a/lib/string.c
> +++ b/lib/string.c
> @@ -952,3 +952,17 @@ char *strreplace(char *s, char old, char new)
>   return s;
>  }
>  EXPORT_SYMBOL(strreplace);
> +
> +char *strtolower(char *s)
> +{
> + char *p;
> +
> +if (unlikely(!s))
> +return NULL;
> +
> + for (p = s; *p; p++)
> + *p = tolower(*p);
> +
> + return s;
> +}
> +EXPORT_SYMBOL(strtolower);

A few suggestions: 

- Make the function take separate src and dst parameters, making it explicitly
  allowed to pass the same value (but not other kinds of overlap, of
  course). That way one can avoid "strcpy(dst, src); strtolower(dst);".

- Drop the NULL check. If someone does "foo->bar = something;
  strtolower(foo->bar); put foo in a global data structure...", the
  dereference of foo->bar may happen much later. Doing the NULL deref
  sooner means it's much easier to find and fix the bug. (Also, other
  str* and mem* functions don't usually check for NULL).

- While it's true that strcpy and memcpy by definition return dst, that's
  mostly useless. If you want it to return anything, please make it
  something that might be used - for example, having stpcpy semantics
  (returning a pointer to dst's terminating \0) means a caller might avoid
  a strlen call.

- Maybe do strtoupper while you're at it. Quick grepping didn't find any
  use for the copy-while-lowercasing, but copy-while-uppercasing can at
  least be used in drivers/acpi/acpica/nsrepair2.c,
  drivers/gpu/drm/nouveau/nvkm/engine/fifo/gk104.c,
  drivers/power/power_supply_sysfs.c along with a bunch of inplace
  uppercasing.


Rasmus
--
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 v3 1/3] scsi: make some Additional Sense strings more grep'able

[Rasmus Villemoes]

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

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
Reviewed-by: Hannes Reinecke <h...@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 cali

[PATCH v3 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

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 <li...@rasmusvillemoes.dk>
Reviewed-by: Hannes Reinecke <h...@suse.com>
Tested-by: Douglas Gilbert <dgilb...@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

--
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 v3 2/3] scsi: move Additional Sense Codes to separate file

[Rasmus Villemoes]

This is a purely mechanical move of the list of additional sense codes
to a separate file, in preparation for reducing the impact of choosing
CONFIG_SCSI_CONSTANTS=y by about 8k.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
Reviewed-by: Hannes Reinecke <h...@suse.com>
---
 drivers/scsi/constants.c   | 830 +
 drivers/scsi/sense_codes.h | 826 
 2 files changed, 829 insertions(+), 827 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 58d94e3c3713..6e813eec4f8d 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -295,835 +295,11 @@ struct error_info {
const char * text;
 };
 
-/*
- * The canonical list of T10 Additional Sense Codes is available at:
- * http://www.t10.org/lists/asc-num.txt [most recent: 20141221]
- */
-
 static const struct error_info additional[] =
 {
-   {0x, "No additional sense information"},
-   {0x0001, "Filemark detected"},
-   {0x0002, "End-of-partition/medium detected"},
-   {0x0003, "Setmark detected"},
-   {0x0004, "Beginning-of-partition/medium detected"},
-   {0x0005, "End-of-data detected"},
-   {0x0006, "I/O process terminated"},
-   {0x0007, "Programmable early warning detected"},
-   {0x0011, "Audio play operation in progress"},
-   {0x0012, "Audio play operation paused"},
-   {0x0013, "Audio play operation successfully completed"},
-   {0x0014, "Audio play operation stopped due to error"},
-   {0x0015, "No current audio status to return"},
-   {0x0016, "Operation in progress"},
-   {0x0017, "Cleaning requested"},
-   {0x0018, "Erase operation in progress"},
-   {0x0019, "Locate operation in progress"},
-   {0x001A, "Rewind operation in progress"},
-   {0x001B, "Set capacity operation in progress"},
-   {0x001C, "Verify operation in progress"},
-   {0x001D, "ATA pass through information available"},
-   {0x001E, "Conflicting SA creation request"},
-   {0x001F, "Logical unit transitioning to another power condition"},
-   {0x0020, "Extended copy information available"},
-   {0x0021, "Atomic command aborted due to ACA"},
-
-   {0x0100, "No index/sector signal"},
-
-   {0x0200, "No seek complete"},
-
-   {0x0300, "Peripheral device write fault"},
-   {0x0301, "No write current"},
-   {0x0302, "Excessive write errors"},
-
-   {0x0400, "Logical unit not ready, cause not reportable"},
-   {0x0401, "Logical unit is in process of becoming ready"},
-   {0x0402, "Logical unit not ready, initializing command required"},
-   {0x0403, "Logical unit not ready, manual intervention required"},
-   {0x0404, "Logical unit not ready, format in progress"},
-   {0x0405, "Logical unit not ready, rebuild in progress"},
-   {0x0406, "Logical unit not ready, recalculation in progress"},
-   {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"},
-   {0x040B, "Logical unit not accessible, target port in standby 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"},
-   {0x0411, "Logical unit not ready, notify (enable spinup) required"},
-   {0x0412, "Logical unit not ready, offline"},
-   {0x0413, "Logical unit not ready, SA creation in progress"},
-   {0x0414, "Logical unit not ready, space allocation in progress"},
-   {0x0415, "Logical unit not ready, robotics disabled"},
-   {0x0416, "Logical unit not ready, configuration required"},
-   {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"},
-   {0x041B, "Logical unit not ready, sanitize in pro

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

[Rasmus Villemoes]

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

--
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

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

[Rasmus Villemoes]

On Tue, Nov 24 2015, Rasmus Villemoes <li...@rasmusvillemoes.dk> wrote:

> This reduces the impact of choosing CONFIG_SCSI_CONSTANTS by about 8KB.
>

ping? does anyone feel like picking these up?

Rasmus
--
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] [SCSI] osd: fix signed char versus %02x issue

[Rasmus Villemoes]

If char is signed and one of these bytes happen to have a value
outside the ascii range, the corresponding output will consist of
"ff" followed by the two hex chars that were actually
intended. One way to fix it would be to change the casts to (u8*) aka
(unsigned char*), but it is much simpler (and generates smaller code)
to use the %ph extension which was created for such short hexdumps.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/osd/osd_initiator.c | 5 +
 1 file changed, 1 insertion(+), 4 deletions(-)

diff --git a/drivers/scsi/osd/osd_initiator.c b/drivers/scsi/osd/osd_initiator.c
index 0cccd6033feb..d8a2b5185f56 100644
--- a/drivers/scsi/osd/osd_initiator.c
+++ b/drivers/scsi/osd/osd_initiator.c
@@ -170,10 +170,7 @@ static int _osd_get_print_system_info(struct osd_dev *od,
 
/* FIXME: Where are the time utilities */
pFirst = get_attrs[a++].val_ptr;
-   OSD_INFO("CLOCK  [0x%02x%02x%02x%02x%02x%02x]\n",
-   ((char *)pFirst)[0], ((char *)pFirst)[1],
-   ((char *)pFirst)[2], ((char *)pFirst)[3],
-   ((char *)pFirst)[4], ((char *)pFirst)[5]);
+   OSD_INFO("CLOCK  [0x%6phN]\n", pFirst);
 
if (a < nelem) { /* IBM-OSD-SIM bug, Might not have it */
unsigned len = get_attrs[a].len;
-- 
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

[PATCH v2 2/3] scsi: move Additional Sense Codes to separate file

[Rasmus Villemoes]

This is a purely mechanical move of the list of additional sense codes
to a separate file, in preparation for reducing the impact of choosing
CONFIG_SCSI_CONSTANTS=y by about 8k.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/constants.c   | 830 +
 drivers/scsi/sense_codes.h | 826 
 2 files changed, 829 insertions(+), 827 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 58d94e3c3713..6e813eec4f8d 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -295,835 +295,11 @@ struct error_info {
const char * text;
 };
 
-/*
- * The canonical list of T10 Additional Sense Codes is available at:
- * http://www.t10.org/lists/asc-num.txt [most recent: 20141221]
- */
-
 static const struct error_info additional[] =
 {
-   {0x, "No additional sense information"},
-   {0x0001, "Filemark detected"},
-   {0x0002, "End-of-partition/medium detected"},
-   {0x0003, "Setmark detected"},
-   {0x0004, "Beginning-of-partition/medium detected"},
-   {0x0005, "End-of-data detected"},
-   {0x0006, "I/O process terminated"},
-   {0x0007, "Programmable early warning detected"},
-   {0x0011, "Audio play operation in progress"},
-   {0x0012, "Audio play operation paused"},
-   {0x0013, "Audio play operation successfully completed"},
-   {0x0014, "Audio play operation stopped due to error"},
-   {0x0015, "No current audio status to return"},
-   {0x0016, "Operation in progress"},
-   {0x0017, "Cleaning requested"},
-   {0x0018, "Erase operation in progress"},
-   {0x0019, "Locate operation in progress"},
-   {0x001A, "Rewind operation in progress"},
-   {0x001B, "Set capacity operation in progress"},
-   {0x001C, "Verify operation in progress"},
-   {0x001D, "ATA pass through information available"},
-   {0x001E, "Conflicting SA creation request"},
-   {0x001F, "Logical unit transitioning to another power condition"},
-   {0x0020, "Extended copy information available"},
-   {0x0021, "Atomic command aborted due to ACA"},
-
-   {0x0100, "No index/sector signal"},
-
-   {0x0200, "No seek complete"},
-
-   {0x0300, "Peripheral device write fault"},
-   {0x0301, "No write current"},
-   {0x0302, "Excessive write errors"},
-
-   {0x0400, "Logical unit not ready, cause not reportable"},
-   {0x0401, "Logical unit is in process of becoming ready"},
-   {0x0402, "Logical unit not ready, initializing command required"},
-   {0x0403, "Logical unit not ready, manual intervention required"},
-   {0x0404, "Logical unit not ready, format in progress"},
-   {0x0405, "Logical unit not ready, rebuild in progress"},
-   {0x0406, "Logical unit not ready, recalculation in progress"},
-   {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"},
-   {0x040B, "Logical unit not accessible, target port in standby 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"},
-   {0x0411, "Logical unit not ready, notify (enable spinup) required"},
-   {0x0412, "Logical unit not ready, offline"},
-   {0x0413, "Logical unit not ready, SA creation in progress"},
-   {0x0414, "Logical unit not ready, space allocation in progress"},
-   {0x0415, "Logical unit not ready, robotics disabled"},
-   {0x0416, "Logical unit not ready, configuration required"},
-   {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"},
-   {0x041B, "Logical unit not ready, sanitize in progress"},
-   {0x041C, "Logical unit n

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

[Rasmus Villemoes]

This reduces the impact of choosing CONFIG_SCSI_CONSTANTS by about 8KB.

2dd951ecd511 ("scsi: Conditionally compile in constants.c") updated
the Kconfig help text from 12KB to 75KB. The 12K predated git so was
certainly outdated. But I'm not sure where the 75K comes from; using
size(1) on a defconfig (with/without this config option) vmlinux shows
a difference of about 47K, and 39K after these patches are applied. In
any case, I've left the Kconfig text alone, since I'm not sure I'm
counting the same way the 75K was computed (I'm fairly certain of the
8K delta, however).

Tested with a trivial module calling scsi_extd_sense_format with a few
random known codes and comparing the result to the expected value.

v2: prepend patch to unsplit a few string literals for greppability,
leave the NULL sentinel in the .c file in 2/3 (it's removed in 3/3
either way).

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/constants.c   | 859 ++---
 drivers/scsi/sense_codes.h | 826 +++
 2 files changed, 847 insertions(+), 838 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

-- 
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

[PATCH v2 1/3] scsi: make some Additional Sense strings more grep'able

[Rasmus Villemoes]

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

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 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"},
{0x5

[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 <li...@rasmusvillemoes.dk>
---
 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

Re: [PATCH] scsi: hpsa: fix multiple issues in path_info_show

[Rasmus Villemoes]

On Wed, Oct 28 2015, Don Brace <brace77...@gmail.com> wrote:

> On 10/27/2015 05:16 PM, Rasmus Villemoes wrote:
>> I'm not familiar with this code, but path_info_show() (added in
>> 8270b86243658 "hpsa: add sysfs entry path_info to show box and
>> bay information") seems to be broken in multiple ways.
>>
[snip]
>>
>> We can fix all of that and get rid of the 400 byte stack buffer by
>> simply writing directly to the given output buffer, which the upper
>> layer guarantees is at least PAGE_SIZE. s[c]nprintf doesn't care where
>> it is writing to, so this doesn't make the spin lock hold time any
>> longer. Using scnprintf ensures that output_len always represents the
>> number of bytes actually written to the buffer, so we'll report the
>> proper amount to the upper layer.
>>
>> Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
>>
> Thanks, I added this to my current patch set. This patch will be up
> with you as the author soon.

I see it in mainline now. May I ask why 6 out of 7 scnprintfs were
changed (back) to snprintf? I don't think there's any functional
difference as long as PAGE_SIZE is indeed sufficient, but mixing
snprintf and scnprintf is pretty odd, and there's now a discrepancy
between the commit log and the patch which wasn't in my original - I'd
expect a "[use snprintf because xyz]" note added if the change was
intentional.

Rasmus

--
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

Re: [PATCH v2] string_helpers: fix precision loss for some inputs

[Rasmus Villemoes]

On Wed, Nov 04 2015, James Bottomley <james.bottom...@hansenpartnership.com> 
wrote:

> On Wed, 2015-11-04 at 00:26 +0100, Rasmus Villemoes wrote:
>> On Tue, Nov 03 2015, James Bottomley <james.bottom...@hansenpartnership.com> 
>> wrote:
>> >> Please spell it U32_MAX
>> >
>> > Why?  there's no reason not to use the arithmetic UINT_MAX here.  Either
>> > works, of course but UINT_MAX is standard.
>> 
>> We're dealing with explicitly sized integers
>
> An integer is explicitly sized: it's 32 bits. That's why UINT_MAX is
> a universal constant.

In the Linux universe, yes. It's kind of amusing how you try to argue
based on the UINT_MAX name being (defined in a) standard while at the same
time very much rely on sizeof(int) having a value which is not specified
by the standard. I repeat:

>> U32_MAX is the natural name for the appropriate constant.

(and it's defined right next to UINT_MAX in kernel.h, so it's not like
you'd have to introduce that macro).

>> Also, you could do > U32_MAX instead of >= U32_MAX, but that's unlikely
>> to make any difference (well, except it might generate slightly better
>> code, since it would allow gcc to just test the upper half for being 0,
>> which might be cheaper on some architectures than comparing to a
>> literal).
>
> Heh if we're going to be that concerned about the code generation, then
> we should just tell gcc exactly how to do it instead of hoping it can
> work it out for itself, so
>
> while (blk_size >> 32) {
> ...

Nah, that would still require the compiler to be able to transform that
to the other form, which apparently it isn't. On x86_64, the simplest
is to load U32_MAX once into a register and then do r/r comparisons, but
when it's possible to directly test the upper half (e.g. when the 64 bit
value is represented in a pair of 32 bit registers) that's much
simpler. gcc generates good code for 'blk_size > U32_MAX' on both x86_64
and x86_32, but ends up doing an extra cmp on x86_32 for >=, and ends up
doing mov,shift,test inside the loop on x86_64 for 'blk_size >> 32'.

Rasmus
--
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

Re: [PATCH v2] string_helpers: fix precision loss for some inputs

[Rasmus Villemoes]

On Tue, Nov 03 2015, James Bottomley <james.bottom...@hansenpartnership.com> 
wrote:

> From: James Bottomley <jbottom...@odin.com>
>
> It was noticed that we lose precision in the final calculation for some
> inputs.  The most egregious example is size=3000 blk_size=1900 in units of 10
> should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the
> current algorithm doesn't correctly account for all the remainders in the
> logarithms.  Fix this by doing a correct calculation in the remainders based
> on napier's algorithm.  Additionally, now we have the correct result, we have
> to account for arithmetic rounding because we're printing 3 digits of
> precision.  This means that if the fourth digit is five or greater, we have to
> round up, so add a section to ensure correct rounding.  Finally account for
> all possible inputs correctly, including zero for block size.
>
> Reported-by: Vitaly Kuznetsov <vkuzn...@redhat.com>
> Cc: sta...@vger.kernel.org# delay backport by two months for testing
> Fixes: b9f28d863594c429e1df35a0474d2663ca28b307
> Signed-off-by: James Bottomley <jbottom...@odin.com>
>
> --
>
> v2: updated with a recommendation from Rasmus Villemoes to truncate the
> initial precision at just under 32 bits
>
> diff --git a/lib/string_helpers.c b/lib/string_helpers.c
> index 5939f63..363faca 100644
> --- a/lib/string_helpers.c
> +++ b/lib/string_helpers.c
> @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const enum 
> string_size_units units,
>   [STRING_UNITS_10] = 1000,
>   [STRING_UNITS_2] = 1024,
>   };
> - int i, j;
> - u32 remainder = 0, sf_cap, exp;
> + static const unsigned int rounding[] = { 500, 50, 5, 0};

j necessarily ends up being 0, 1 or 2. Any reason to include the last entry?

> +
> + while (blk_size >= UINT_MAX)
>   i++;
> - }
>  
> - exp = divisor[units] / (u32)blk_size;
> - /*
> -  * size must be strictly greater than exp here to ensure that remainder
> -  * is greater than divisor[units] coming out of the if below.
> -  */
> - if (size > exp) {
> - remainder = do_div(size, divisor[units]);
> - remainder *= blk_size;
> + while (size >= UINT_MAX)
>   i++;

Please spell it U32_MAX. Also, it's not clear why you left out the
do_divs ;-)

Rasmus
--
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

Re: [PATCH v2] string_helpers: fix precision loss for some inputs

[Rasmus Villemoes]

On Tue, Nov 03 2015, James Bottomley <james.bottom...@hansenpartnership.com> 
wrote:

> On Tue, 2015-11-03 at 23:13 +0100, Rasmus Villemoes wrote:
>> On Tue, Nov 03 2015, James Bottomley <james.bottom...@hansenpartnership.com> 
>> wrote:
>> 
>> > From: James Bottomley <jbottom...@odin.com>
>> >
>> > It was noticed that we lose precision in the final calculation for some
>> > inputs.  The most egregious example is size=3000 blk_size=1900 in units of 
>> > 10
>> > should yield 5.70 MB but in fact yields 3.00 MB (oops). This is because the
>> > current algorithm doesn't correctly account for all the remainders in the
>> > logarithms.  Fix this by doing a correct calculation in the remainders 
>> > based
>> > on napier's algorithm.  Additionally, now we have the correct result, we 
>> > have
>> > to account for arithmetic rounding because we're printing 3 digits of
>> > precision.  This means that if the fourth digit is five or greater, we 
>> > have to
>> > round up, so add a section to ensure correct rounding.  Finally account for
>> > all possible inputs correctly, including zero for block size.
>> >
>> > Reported-by: Vitaly Kuznetsov <vkuzn...@redhat.com>
>> > Cc: sta...@vger.kernel.org # delay backport by two months for testing
>> > Fixes: b9f28d863594c429e1df35a0474d2663ca28b307
>> > Signed-off-by: James Bottomley <jbottom...@odin.com>
>> >
>> > --
>> >
>> > v2: updated with a recommendation from Rasmus Villemoes to truncate the
>> > initial precision at just under 32 bits
>> >
>> > diff --git a/lib/string_helpers.c b/lib/string_helpers.c
>> > index 5939f63..363faca 100644
>> > --- a/lib/string_helpers.c
>> > +++ b/lib/string_helpers.c
>> > @@ -43,38 +43,40 @@ void string_get_size(u64 size, u64 blk_size, const 
>> > enum string_size_units units,
>> >[STRING_UNITS_10] = 1000,
>> >[STRING_UNITS_2] = 1024,
>> >};
>> > -  int i, j;
>> > -  u32 remainder = 0, sf_cap, exp;
>> > +  static const unsigned int rounding[] = { 500, 50, 5, 0};
>> 
>> j necessarily ends up being 0, 1 or 2. Any reason to include the last entry?
>
> No reason beyond a vague worry someone might try to increase the printed
> precision by one digit.

But that would seem to require prepending 5000 to that array and
changing various constants below to 1 (aside from checking all
callers to see if they pass a sufficient buffer size) - the 0 doesn't
serve any purpose in that scenario either.

>> > +
>> > +  while (blk_size >= UINT_MAX)
>> >i++;
>> > -  }
>> >  
>> > -  exp = divisor[units] / (u32)blk_size;
>> > -  /*
>> > -   * size must be strictly greater than exp here to ensure that remainder
>> > -   * is greater than divisor[units] coming out of the if below.
>> > -   */
>> > -  if (size > exp) {
>> > -  remainder = do_div(size, divisor[units]);
>> > -  remainder *= blk_size;
>> > +  while (size >= UINT_MAX)
>> >i++;
>> 
>> Please spell it U32_MAX
>
> Why?  there's no reason not to use the arithmetic UINT_MAX here.  Either
> works, of course but UINT_MAX is standard.

We're dealing with explicitly sized integers, and the comment even says
that we're reducing till we fit in 32 bits, so that we can do a
32x32->64 multiplication. U32_MAX is the natural name for the
appropriate constant.

Also, you could do > U32_MAX instead of >= U32_MAX, but that's unlikely
to make any difference (well, except it might generate slightly better
code, since it would allow gcc to just test the upper half for being 0,
which might be cheaper on some architectures than comparing to a
literal).

Rasmus
--
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] scsi: hpsa: fix multiple issues in path_info_show

[Rasmus Villemoes]

I'm not familiar with this code, but path_info_show() (added in
8270b86243658 "hpsa: add sysfs entry path_info to show box and
bay information") seems to be broken in multiple ways.

First, there's

  817 return snprintf(buf, output_len+1, "%s%s%s%s%s%s%s%s",
  818 path[0], path[1], path[2], path[3],
  819 path[4], path[5], path[6], path[7]);

so hopefully output_len contains the combined length of the eight
strings. Otherwise, snprintf will stop copying to the output
buffer, but still end up reporting that combined length - which
in turn would result in user-space getting a bunch of useless nul
bytes (thankfully the upper sysfs layer seems to clear the output
buffer before passing it to the various ->show routines). But we have

  767 output_len = snprintf(path[i],
  768 PATH_STRING_LEN, "[%d:%d:%d:%d] %20.20s ",
  769 h->scsi_host->host_no,
  770 hdev->bus, hdev->target, hdev->lun,
  771 scsi_device_type(hdev->devtype));

so output_len at best contains the length of the last string printed.

Inside the loop, we then otherwise add to output_len. By magic,
we still have PATH_STRING_LEN available every time... This
wouldn't really be a problem if the bean-counting has been done
properly and each line actually does fit in 50 bytes, and maybe
it does, but I don't immediately see why. Suppose we end up
taking this branch:

  802 output_len += snprintf(path[i] + output_len,
  803 PATH_STRING_LEN,
  804 "BOX: %hhu BAY: %hhu %s\n",
  805 box, bay, active);

An optimistic estimate says this uses strlen("BOX: 1 BAY: 2
Active\n") which is 21. Now add the 20 bytes guaranteed by the
%20.20s and then some for the rest of that format string, and
we're easily over 50 bytes. I don't think we can get over 100
bytes even being pessimistic, so this just means we'll scribble
into the next path[i+1] and maybe get that overwritten later,
leading to some garbled output (in fact, since we'd overwrite the
previous string's 0-terminator, we could end up with one very
long string and then print various suffixes of that, leading to
much more than 400 bytes of output). Except of course when we're
filling path[7], where overrunning it means writing random stuff
to the kernel stack, which is usually a lot of fun.

We can fix all of that and get rid of the 400 byte stack buffer by
simply writing directly to the given output buffer, which the upper
layer guarantees is at least PAGE_SIZE. s[c]nprintf doesn't care where
it is writing to, so this doesn't make the spin lock hold time any
longer. Using scnprintf ensures that output_len always represents the
number of bytes actually written to the buffer, so we'll report the
proper amount to the upper layer.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/hpsa.c | 38 +-
 1 file changed, 17 insertions(+), 21 deletions(-)

diff --git a/drivers/scsi/hpsa.c b/drivers/scsi/hpsa.c
index 40669f8dd0df..b892ae90e292 100644
--- a/drivers/scsi/hpsa.c
+++ b/drivers/scsi/hpsa.c
@@ -726,7 +726,6 @@ static ssize_t host_show_hp_ssd_smart_path_enabled(struct 
device *dev,
 }
 
 #define MAX_PATHS 8
-#define PATH_STRING_LEN 50
 
 static ssize_t path_info_show(struct device *dev,
 struct device_attribute *attr, char *buf)
@@ -742,9 +741,7 @@ static ssize_t path_info_show(struct device *dev,
u8 path_map_index = 0;
char *active;
unsigned char phys_connector[2];
-   unsigned char path[MAX_PATHS][PATH_STRING_LEN];
 
-   memset(path, 0, MAX_PATHS * PATH_STRING_LEN);
sdev = to_scsi_device(dev);
h = sdev_to_hba(sdev);
spin_lock_irqsave(>devlock, flags);
@@ -764,8 +761,9 @@ static ssize_t path_info_show(struct device *dev,
else
continue;
 
-   output_len = snprintf(path[i],
-   PATH_STRING_LEN, "[%d:%d:%d:%d] %20.20s ",
+   output_len += scnprintf(buf + output_len,
+   PAGE_SIZE - output_len,
+   "[%d:%d:%d:%d] %20.20s ",
h->scsi_host->host_no,
hdev->bus, hdev->target, hdev->lun,
scsi_device_type(hdev->devtype));
@@ -773,9 +771,9 @@ static ssize_t path_info_show(struct device *dev,
if (is_ext_target(h, hdev) ||
(hdev->devtype == TYPE_RAID) ||
is_logical_dev_addr_mode(hdev->scsi3addr)) {
-   output_len += snprintf(path[i] + output_len,
-   PATH_STRING_LEN, "

Re: [PATCH 1/2] scsi: move Additional Sense Codes to separate file

[Rasmus Villemoes]

On Tue, Oct 06 2015, Rasmus Villemoes <li...@rasmusvillemoes.dk> wrote:

> On Mon, Oct 05 2015, Bart Van Assche <bart.vanass...@sandisk.com> wrote:
>
>> On 10/05/15 02:26, Rasmus Villemoes wrote:
>>> -   {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"},
>>
>> Please convert these multi-line strings into single line string
>> constants such that users can look up these easily with grep.
>
> I can certainly do that, but I'd prefer to do it in a separate patch. I
> really want to keep this as mechanical as possible.
>
>>> +
>>> +SENSE_CODE(0, NULL)
>>
>> The above looks confusing to me. Please leave this out and add { 0,
>> NULL } at the end of the additional[] array instead.
>
> OK, I agree that that would have been cleaner. However, the sentinel
> entry is removed in 2/2 (since we loop using ARRAY_SIZE instead of
> checking for the sentinel). Let me know if you want me to resend the
> 1600 line monster anyway with this addressed.

Should I resend, or can these be picked up as-is? I can easily send a
3/2 making the strings more greppable.

Thanks,
Rasmus
--
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

Re: [PATCH 1/2] scsi: move Additional Sense Codes to separate file

[Rasmus Villemoes]

On Mon, Oct 05 2015, Bart Van Assche <bart.vanass...@sandisk.com> wrote:

> On 10/05/15 02:26, Rasmus Villemoes wrote:
>> -{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"},
>
> Please convert these multi-line strings into single line string
> constants such that users can look up these easily with grep.

I can certainly do that, but I'd prefer to do it in a separate patch. I
really want to keep this as mechanical as possible.

>> +
>> +SENSE_CODE(0, NULL)
>
> The above looks confusing to me. Please leave this out and add { 0,
> NULL } at the end of the additional[] array instead.

OK, I agree that that would have been cleaner. However, the sentinel
entry is removed in 2/2 (since we loop using ARRAY_SIZE instead of
checking for the sentinel). Let me know if you want me to resend the
1600 line monster anyway with this addressed.

Thanks,
Rasmus
--
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

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

[Rasmus Villemoes]

On Mon, Oct 05 2015, Bart Van Assche <bart.vanass...@sandisk.com> wrote:

> On 10/05/15 02:26, Rasmus Villemoes wrote:
>>   struct error_info {
>>  unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
>> -const char * text;
>> +unsigned short size;
>>   };
>
> Had you considered to use the type uint16_t instead of unsigned short ?
>

Yes, but I thought I'd keep it consistent with the other member. AFAIK,
they're one and the same on all relevant arches. I actually think
spelling it u16 for both members would make sense (for the code because
it explicitly is meant to hold two bytes), but again I think that's
better done as a trivial follow-up patch, if we really want to change this.

Rasmus
--
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

Re: RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

[Rasmus Villemoes]

On Tue, Oct 06 2015, Julian Calaby  wrote:

> Hi Rasmus,
>
>>
>> diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
>> index 47aaccd5e68e..ccd34b0481cd 100644
>> --- a/drivers/scsi/constants.c
>> +++ b/drivers/scsi/constants.c
>> @@ -292,17 +292,31 @@ 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
>>  };
>>
>> +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 +378,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;
>
> You don't seem to be accounting for the null bytes here.

Well, no, I account for the nul bytes where I define the table (the
comment actually says as much). sizeof("foo") is 4. Since
additional_text ends up pointing to a string containing

"foo" "\0" "xyzzy" "\0" "..." "\0"

aka

"foo\0xyzzy\0...\0"

this is the right amount to skip. As I said in the cover letter, I did
test this (so that I'd at least catch silly off-by-ones), and I do get
the right strings out.

Thanks,
Rasmus
--
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 0/2] scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

[Rasmus Villemoes]

This reduces the impact of choosing CONFIG_SCSI_CONSTANTS by about 8KB.

2dd951ecd511 ("scsi: Conditionally compile in constants.c") updated
the Kconfig help text from 12KB to 75KB. The 12K predated git so was
certainly outdated. But I'm not sure where the 75K comes from; using
size(1) on a defconfig (with/without this config option) vmlinux shows
a difference of about 47K, and 39K after these patches are applied. In
any case, I've left the Kconfig text alone, since I'm not sure I'm
counting the same way the 75K was computed (I'm fairly certain of the
8K delta, however).

Tested with a trivial module calling scsi_extd_sense_format with a few
random known codes and comparing the result to the expected value.

Rasmus Villemoes (2):
  scsi: move Additional Sense Codes to separate file
  scsi: reduce CONFIG_SCSI_CONSTANTS=y impact by 8k

 drivers/scsi/constants.c   | 860 ++---
 drivers/scsi/sense_codes.h | 835 +++
 2 files changed, 857 insertions(+), 838 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

-- 
2.1.3

--
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 2/2] 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 <li...@rasmusvillemoes.dk>
---
 drivers/scsi/constants.c   | 25 +
 drivers/scsi/sense_codes.h |  2 --
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 47aaccd5e68e..ccd34b0481cd 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,31 @@ 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
 };
 
+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 +378,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 &&
diff --git a/drivers/scsi/sense_codes.h b/drivers/scsi/sense_codes.h
index 54b3939d6309..da84d53b3379 100644
--- a/drivers/scsi/sense_codes.h
+++ b/drivers/scsi/sense_codes.h
@@ -833,5 +833,3 @@ SENSE_CODE(0x746E, "External data encryption control 
timeout")
 SENSE_CODE(0x746F, "External data encryption control error")
 SENSE_CODE(0x7471, "Logical unit access not authorized")
 SENSE_CODE(0x7479, "Security conflict in translated device")
-
-SENSE_CODE(0, NULL)
-- 
2.1.3

--
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 1/2] scsi: move Additional Sense Codes to separate file

[Rasmus Villemoes]

This is a purely mechanical move of the list of additional sense codes
to a separate file, in preparation for reducing the impact of choosing
CONFIG_SCSI_CONSTANTS=y by about 8k..

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/constants.c   | 839 +
 drivers/scsi/sense_codes.h | 837 
 2 files changed, 840 insertions(+), 836 deletions(-)
 create mode 100644 drivers/scsi/sense_codes.h

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index fa09d4be2b53..47aaccd5e68e 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -295,845 +295,12 @@ struct error_info {
const char * text;
 };
 
-/*
- * The canonical list of T10 Additional Sense Codes is available at:
- * http://www.t10.org/lists/asc-num.txt [most recent: 20141221]
- */
 
 static const struct error_info additional[] =
 {
-   {0x, "No additional sense information"},
-   {0x0001, "Filemark detected"},
-   {0x0002, "End-of-partition/medium detected"},
-   {0x0003, "Setmark detected"},
-   {0x0004, "Beginning-of-partition/medium detected"},
-   {0x0005, "End-of-data detected"},
-   {0x0006, "I/O process terminated"},
-   {0x0007, "Programmable early warning detected"},
-   {0x0011, "Audio play operation in progress"},
-   {0x0012, "Audio play operation paused"},
-   {0x0013, "Audio play operation successfully completed"},
-   {0x0014, "Audio play operation stopped due to error"},
-   {0x0015, "No current audio status to return"},
-   {0x0016, "Operation in progress"},
-   {0x0017, "Cleaning requested"},
-   {0x0018, "Erase operation in progress"},
-   {0x0019, "Locate operation in progress"},
-   {0x001A, "Rewind operation in progress"},
-   {0x001B, "Set capacity operation in progress"},
-   {0x001C, "Verify operation in progress"},
-   {0x001D, "ATA pass through information available"},
-   {0x001E, "Conflicting SA creation request"},
-   {0x001F, "Logical unit transitioning to another power condition"},
-   {0x0020, "Extended copy information available"},
-   {0x0021, "Atomic command aborted due to ACA"},
-
-   {0x0100, "No index/sector signal"},
-
-   {0x0200, "No seek complete"},
-
-   {0x0300, "Peripheral device write fault"},
-   {0x0301, "No write current"},
-   {0x0302, "Excessive write errors"},
-
-   {0x0400, "Logical unit not ready, cause not reportable"},
-   {0x0401, "Logical unit is in process of becoming ready"},
-   {0x0402, "Logical unit not ready, initializing command required"},
-   {0x0403, "Logical unit not ready, manual intervention required"},
-   {0x0404, "Logical unit not ready, format in progress"},
-   {0x0405, "Logical unit not ready, rebuild in progress"},
-   {0x0406, "Logical unit not ready, recalculation in progress"},
-   {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"},
-   {0x040B, "Logical unit not accessible, target port in standby 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"},
-   {0x0411, "Logical unit not ready, notify (enable spinup) required"},
-   {0x0412, "Logical unit not ready, offline"},
-   {0x0413, "Logical unit not ready, SA creation in progress"},
-   {0x0414, "Logical unit not ready, space allocation in progress"},
-   {0x0415, "Logical unit not ready, robotics disabled"},
-   {0x0416, "Logical unit not ready, configuration required"},
-   {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"},
-   {0x041B, "Logical unit not ready, sa

Re: RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

[Rasmus Villemoes]

On Sat, Oct 03 2015, Christoph Hellwig <h...@infradead.org> wrote:

> Hi Rasmus,
>
> I like this idea.  But maybe it's also time to just move the constants
> to a plain text file and auto-generate C headers from them?  That way
> the format in which they can be edited is decoupled from the
> representation in the kernel image.

Well, I don't really have an opinion on that part.

In the meantime, I got another idea for doubling the saving to 8k. It
requires a few more code changes and is perhaps also more hacky. 2/2
would be something like below. Please let me know which version you'd
prefer, and I'll send both patches properly.

Thanks,
Rasmus

Subject: [PATCH 2/2] 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 113122824   -8488

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/constants.c   | 25 +
 drivers/scsi/sense_codes.h |  2 --
 2 files changed, 21 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 47aaccd5e68e..ccd34b0481cd 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -292,17 +292,31 @@ 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
 };
 
+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 +378,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 &&
diff --git a/drivers/scsi/sense_codes.h b/drivers/scsi/sense_codes.h
index 54b3939d6309..da84d53b3379 100644
--- a/drivers/scsi/sense_codes.h
+++ b/drivers/scsi/sense_codes.h
@@ -833,5 +833,3 @@ SENSE_CODE(0x746E, "External data encryption control 
timeout")
 SENSE_CODE(0x746F, "External data encryption control error")
 SENSE_CODE(0x7471, "Logical unit access not authorized")
 SENSE_CODE(0x7479, "Security conflict in translated device")
-
-SENSE_CODE(0, NULL)
-- 
--
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] [SCSI] fnic: use kzalloc in fnic_fcoe_process_vlan_resp

[Rasmus Villemoes]

This saves a little .text and avoids the sizeof(...) style
inconsistency.

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/fnic/fnic_fcs.c | 4 +---
 1 file changed, 1 insertion(+), 3 deletions(-)

diff --git a/drivers/scsi/fnic/fnic_fcs.c b/drivers/scsi/fnic/fnic_fcs.c
index bf0bbd42efb5..2d013076bfba 100644
--- a/drivers/scsi/fnic/fnic_fcs.c
+++ b/drivers/scsi/fnic/fnic_fcs.c
@@ -415,15 +415,13 @@ static void fnic_fcoe_process_vlan_resp(struct fnic 
*fnic, struct sk_buff *skb)
vid = ntohs(((struct fip_vlan_desc *)desc)->fd_vlan);
shost_printk(KERN_INFO, fnic->lport->host,
  "process_vlan_resp: FIP VLAN %d\n", vid);
-   vlan = kmalloc(sizeof(*vlan),
-   GFP_ATOMIC);
+   vlan = kzalloc(sizeof(*vlan), GFP_ATOMIC);
if (!vlan) {
/* retry from timer */
spin_unlock_irqrestore(>vlans_lock,
flags);
goto out;
}
-   memset(vlan, 0, sizeof(struct fcoe_vlan));
vlan->vid = vid & 0x0fff;
vlan->state = FIP_VLAN_AVAIL;
list_add_tail(>list, >vlans);
-- 
2.1.3

--
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

RFC: reduce CONFIG_SCSI_CONSTANTS impact by 4k

[Rasmus Villemoes]

struct error_info has 6 bytes of padding on x86_64 (and I assume also
other 64 bit platforms). This currently amounts to about 4k of wasted
space (and presumably a third of that on 32 bit).

We can avoid that by keeping the codes and the strings in separate
arrays. Keeping those in sync should be easy enough if we use the
standard trick of including a table twice.

Patch 2/2 would be something like below; 1/2 is a 1600 line purely
mechanical thing which I won't spam the lists with unless someone else
thinks this is a good idea.

What do you think?

Subject: [PATCH 2/2] scsi: split additional[] array in two

struct error_info has 6 bytes of padding (on 64 bit platforms), which
amounts to over 4K of wasted space in the additional[]
array. Splitting it in two avoids that waste. A BUILD_BUG_ON and the
fact that the two arrays are generated from the same include file
should keep these two arrays in sync.

$ scripts/bloat-o-meter /tmp/vmlinux vmlinux
add/remove: 2/1 grow/shrink: 1/0 up/down: 7073/-11312 (-4239)

Signed-off-by: Rasmus Villemoes <li...@rasmusvillemoes.dk>
---
 drivers/scsi/constants.c | 21 +++--
 1 file changed, 11 insertions(+), 10 deletions(-)

diff --git a/drivers/scsi/constants.c b/drivers/scsi/constants.c
index 14d5069ca3ff..03f28cdcbc31 100644
--- a/drivers/scsi/constants.c
+++ b/drivers/scsi/constants.c
@@ -290,19 +290,19 @@ bool scsi_opcode_sa_name(int opcode, int service_action,
return true;
 }
 
-struct error_info {
-   unsigned short code12;  /* 0x0302 looks better than 0x03,0x02 */
-   const char * text;
+static unsigned short additional_code12[] = {
+#define SENSE_CODE(c, s) c
+#include "sense_codes.h"
+#undef SENSE_CODE
 };
 
-
-static const struct error_info additional[] =
-{
-#define SENSE_CODE(c, s) {c, s}
+static const char *additional_text[] = {
+#define SENSE_CODE(c, s) s
 #include "sense_codes.h"
 #undef SENSE_CODE
 };
 
+
 struct error_info2 {
unsigned char code1, code2_min, code2_max;
const char * str;
@@ -364,11 +364,12 @@ scsi_extd_sense_format(unsigned char asc, unsigned char 
ascq, const char **fmt)
 {
int i;
unsigned short code = ((asc << 8) | ascq);
+   BUILD_BUG_ON(ARRAY_SIZE(additional_code12) != 
ARRAY_SIZE(additional_text));
 
*fmt = NULL;
-   for (i = 0; additional[i].text; i++)
-   if (additional[i].code12 == code)
-   return additional[i].text;
+   for (i = 0; additional_text[i]; i++)
+   if (additional_code12[i] == code)
+   return additional_text[i];
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

Re: [RFC PATCH 0/3] fix *pbl format support

[Rasmus Villemoes]

On Mon, Sep 21 2015, Maurizio Lombardi <mlomb...@redhat.com> wrote:

> Hi Rasmus,
>
> On 09/16/2015 10:35 PM, Rasmus Villemoes wrote:
>> 
>> I just remembered: I noticed a while ago that the qualifier member is
>> only used inside format_decode (in the end, the information is folded
>> into the type member), so one might as well use a local variable for
>> that. This gives option (3): Make field_width a 24 bit bitfield. I think
>> that should be sufficient for any realistic bitmap that one would
>> print. The patch is also rather small. Surprisingly, bloat-o-meter even
>> says that it's also a net win in code size:
>
> I tested your patch with scsi-debug and it works for me:

Thanks for testing it! What do people think of this option?

Rasmus
--
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] scsi: megaraid_sas: Prevent future %p disaster

[Rasmus Villemoes]

There is currently no %po format extension, so currently the letters
on are simply skipped and the pointer is printed as expected (while
missing the word on). However, it is probably only a matter of time
before someone comes up with a %po extension, at which point this is
likely to fail spectacularly.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/megaraid/megaraid_sas_base.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c 
b/drivers/scsi/megaraid/megaraid_sas_base.c
index ff283d23788a..e7c6b9c946d6 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -3105,7 +3105,7 @@ megasas_internal_reset_defer_cmds(struct megasas_instance 
*instance)
for (i = 0; i  max_cmd; i++) {
cmd = instance-cmd_list[i];
if (cmd-sync_cmd == 1 || cmd-scmd) {
-   printk(KERN_NOTICE megasas: moving cmd[%d]:%p:%d:%p
+   printk(KERN_NOTICE megasas: moving cmd[%d]:%p:%d:%p 
on the defer queue as internal\n,
defer_index, cmd, cmd-sync_cmd, cmd-scmd);
 
-- 
2.1.3

--
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] scsi: 3w-xxxx: Drop duplicated function name

[Rasmus Villemoes]

Mentioning the enclosing function twice doesn't add value.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/3w-.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/3w-.c b/drivers/scsi/3w-.c
index c75f2048319f..67539e73fdad 100644
--- a/drivers/scsi/3w-.c
+++ b/drivers/scsi/3w-.c
@@ -860,7 +860,7 @@ static int tw_allocate_memory(TW_Device_Extension *tw_dev, 
int size, int which)
tw_dev-alignment_virtual_address[i] = (unsigned long 
*)((unsigned char *)cpu_addr + (i*size));
break;
default:
-   printk(KERN_WARNING 3w-: tw_allocate_memory(): 
case slip in tw_allocate_memory()\n);
+   printk(KERN_WARNING 3w-: tw_allocate_memory(): 
case slip\n);
return 1;
}
}
-- 
2.1.3

--
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

Re: [PATCH v2 0/6] scsi: Some seq_file cleanups/optimizations

[Rasmus Villemoes]

On Thu, Jan 29 2015, Finn Thain fth...@telegraphics.com.au wrote:

 I have one reservation about this patch series.

 For example, the changes,

 - seq_printf(m, %s, p);
 + seq_puts(m, p);

 These calls are not equivalent because the bounds check is not the same. 
 seq_puts will fail when m-count + strlen(p) == m-size.


So will seq_printf:

int seq_vprintf(struct seq_file *m, const char *f, va_list args)
{
int len;

if (m-count  m-size) {
len = vsnprintf(m-buf + m-count, m-size - m-count, f, args);
if (m-count + len  m-size) {
m-count += len;
return 0;
}
}
seq_set_overflow(m);
return -1;
}

The return value from vsnprintf(%s, p) is by definition the length of
the string p. Yes, vsnprintf may write some of the bytes from the
string to the buffer, but those are effectively discarded if they don't
all fit, since m-count is not updated.

 There's a similar situation with the changes,

 - seq_puts(m, x);
 + seq_putc(m, 'x');

It's true that this may cause 'x' to be printed which it might not have
been before. I think this is a bug in seq_puts - it should use = for
its bounds check. OTOH, seq_printf probably needs to continue using ,
because if the return value is == m-size-m-count, vsnprintf will have
truncated the output, overwriting the last byte with a '\0'.

 Have you considered what the implications might be? Are there any?

I must admit I hadn't thought that deeply about it before, but now it
seems that my patches can only end up utilizing m-buf a bit better
(well, 8 bits, to be precise). If I understand the whole seq_*
interface, overflow will just cause a larger buffer to be allocated and
all the print functions to be called again.

Steven, you've been doing some cleanup in this area, among other things
trying to make all the seq_* functions return void. Could you fill me in
on the status of that?

Rasmus
--
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

Re: [PATCH v2 0/6] scsi: Some seq_file cleanups/optimizations

[Rasmus Villemoes]

On Wed, Dec 03 2014, Rasmus Villemoes li...@rasmusvillemoes.dk wrote:

 These patches mostly replace seq_printf with simpler and faster
 equivalents, e.g. seq_printf(m, something) = seq_puts(m,
 something) and seq_printf(m, \n) = seq_putc(m, '\n). But before
 my Coccinelle scripts could be unleashed I had to clean up an
 unnecessary macro.

 The patches don't change the semantics of the code (well, that's the
 idea anyway), but should make it slightly smaller and faster.

 v2: Redone on top of git://git.infradead.org/users/hch/scsi-queue.git 
 drivers-for-3.19


Ping. Was this picked up, or did it just go to /dev/null?

Rasmus
--
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 6/6] scsi: misc: Print single-character strings with seq_putc

[Rasmus Villemoes]

Using seq_putc to print a single character saves at least a strlen()
call and a memory access, and may also give a small .text reduction.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/NCR5380.c  |  2 +-
 drivers/scsi/advansys.c | 34 +-
 drivers/scsi/aic7xxx/aic79xx_proc.c | 10 +-
 drivers/scsi/aic7xxx/aic7xxx_proc.c | 10 +-
 drivers/scsi/atari_NCR5380.c|  2 +-
 drivers/scsi/dc395x.c   |  4 ++--
 drivers/scsi/esas2r/esas2r_main.c   |  2 +-
 drivers/scsi/in2000.c   |  2 +-
 drivers/scsi/ips.c  |  2 +-
 drivers/scsi/nsp32.c|  2 +-
 drivers/scsi/pcmcia/nsp_cs.c|  4 ++--
 drivers/scsi/qla2xxx/qla_dfs.c  |  2 +-
 drivers/scsi/scsi_proc.c| 10 +-
 drivers/scsi/scsi_trace.c   |  2 +-
 drivers/scsi/wd33c93.c  |  2 +-
 15 files changed, 45 insertions(+), 45 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index a30af00..8981701 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -764,7 +764,7 @@ static void lprint_command(unsigned char *command, struct 
seq_file *m)
lprint_opcode(command[0], m);
for (i = 1, s = COMMAND_SIZE(command[0]); i  s; ++i)
seq_printf(m, %02x , command[i]);
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 }
 
 static void lprint_opcode(int opcode, struct seq_file *m)
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index c4d0910..81ffb0f 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -3088,7 +3088,7 @@ static void asc_prt_asc_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-init_sdtr  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
 #ifdef CONFIG_ISA
if (asc_dvc_varp-bus_type  ASC_IS_ISA) {
@@ -3203,7 +3203,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
seq_puts(m,  Target ID:   );
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %X, i);
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
word = ep_3550-disc_enable;
@@ -3216,7 +3216,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
word = ep_3550-tagqng_able;
@@ -3229,7 +3229,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
word = ep_3550-start_motor;
@@ -3242,7 +3242,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
seq_puts(m,  Synchronous Transfer:);
@@ -3250,7 +3250,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
seq_printf(m,  %c,
   (ep_3550-sdtr_able  ADV_TID_TO_TIDMASK(i)) 
?
   'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
}
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
@@ -3259,7 +3259,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
seq_printf(m,  %c,
   (ep_3550-ultra_able  ADV_TID_TO_TIDMASK(i))
   ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
}
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
@@ -3273,7 +3273,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC38C0800 ||
adv_dvc_varp-chip_type == ADV_CHIP_ASC38C1600) {
@@ -3318,7 +3318,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct

[PATCH v2 4/6] scsi: misc: Replace seq_printf with seq_puts

[Rasmus Villemoes]

Using seq_printf to print a simple string is a lot more expensive than
it needs to be, since seq_puts exists. Replace seq_printf with
seq_puts when possible.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/BusLogic.c | 10 +-
 drivers/scsi/NCR5380.c  |  4 ++--
 drivers/scsi/aic7xxx/aic79xx_proc.c | 38 ++---
 drivers/scsi/aic7xxx/aic7xxx_proc.c | 24 +++
 drivers/scsi/arm/fas216.c   |  6 +++---
 drivers/scsi/atari_NCR5380.c|  4 ++--
 drivers/scsi/atp870u.c  |  6 +++---
 drivers/scsi/dc395x.c   | 17 +
 drivers/scsi/dpt_i2o.c  |  2 +-
 drivers/scsi/eata_pio.c |  2 +-
 drivers/scsi/gdth_proc.c| 24 +++
 drivers/scsi/in2000.c   | 18 +-
 drivers/scsi/ips.c  |  7 +++
 drivers/scsi/megaraid.c |  2 +-
 drivers/scsi/nsp32.c| 14 +++---
 drivers/scsi/pcmcia/nsp_cs.c| 30 ++---
 drivers/scsi/qla2xxx/qla_dfs.c  |  8 
 drivers/scsi/scsi_proc.c| 22 ++---
 drivers/scsi/scsi_trace.c   |  6 +++---
 drivers/scsi/wd33c93.c  | 18 +-
 drivers/scsi/wd7000.c   | 12 ++--
 21 files changed, 136 insertions(+), 138 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index 8d66a64..c7be7bb 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3485,7 +3485,7 @@ static int blogic_show_info(struct seq_file *m, struct 
Scsi_Host *shost)
seq_printf(m, \n\
 Current Driver Queue Depth:%d\n\
 Currently Allocated CCBs:  %d\n, adapter-drvr_qdepth, 
adapter-alloc_ccbs);
-   seq_printf(m, \n\n\
+   seq_puts(m, \n\n\
   DATA TRANSFER STATISTICS\n\
 \n\
 Target Tagged Queuing  Queue Depth  Active  Attempted  Completed\n\
@@ -3500,7 +3500,7 @@ TargetTagged Queuing  Queue Depth  Active  Attempted  
Completed\n\
seq_printf(m,
   %3d   %3u%9u%9u\n, 
adapter-qdepth[tgt], adapter-active_cmds[tgt], tgt_stats[tgt].cmds_tried, 
tgt_stats[tgt].cmds_complete);
}
-   seq_printf(m, \n\
+   seq_puts(m, \n\
 Target  Read Commands  Write Commands   Total Bytes ReadTotal Bytes 
Written\n\
 ==  =  ==  ===  
===\n);
for (tgt = 0; tgt  adapter-maxdev; tgt++) {
@@ -3517,7 +3517,7 @@ Target  Read Commands  Write Commands   Total Bytes Read  
  Total Bytes Written\
else
seq_printf(m,   %9u\n, 
tgt_stats[tgt].byteswritten.units);
}
-   seq_printf(m, \n\
+   seq_puts(m, \n\
 Target  Command0-1KB  1-2KB  2-4KB  4-8KB 8-16KB\n\
 ==  ===  =  =  =  =  =\n);
for (tgt = 0; tgt  adapter-maxdev; tgt++) {
@@ -3533,7 +3533,7 @@ Target  Command0-1KB  1-2KB  2-4KB  4-8KB 
8-16KB\n\
tgt_stats[tgt].write_sz_buckets[0],
tgt_stats[tgt].write_sz_buckets[1], 
tgt_stats[tgt].write_sz_buckets[2], tgt_stats[tgt].write_sz_buckets[3], 
tgt_stats[tgt].write_sz_buckets[4]);
}
-   seq_printf(m, \n\
+   seq_puts(m, \n\
 Target  Command   16-32KB32-64KB   64-128KB   128-256KB   256KB+\n\
 ==  ===  =  =  =  =  =\n);
for (tgt = 0; tgt  adapter-maxdev; tgt++) {
@@ -3549,7 +3549,7 @@ Target  Command   16-32KB32-64KB   64-128KB   
128-256KB   256KB+\n\
tgt_stats[tgt].write_sz_buckets[5],
tgt_stats[tgt].write_sz_buckets[6], 
tgt_stats[tgt].write_sz_buckets[7], tgt_stats[tgt].write_sz_buckets[8], 
tgt_stats[tgt].write_sz_buckets[9]);
}
-   seq_printf(m, \n\n\
+   seq_puts(m, \n\n\
   ERROR RECOVERY STATISTICS\n\
 \n\
  Command Aborts  Bus Device Resets   Host Adapter Resets\n\
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index aca181e..a30af00 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -754,7 +754,7 @@ static int __maybe_unused NCR5380_show_info(struct seq_file 
*m,
 static void lprint_Scsi_Cmnd(struct scsi_cmnd *cmd, struct seq_file *m)
 {
seq_printf(m, scsi%d : destination target %d, lun %llu\n, 
cmd-device-host-host_no, cmd-device-id, cmd-device-lun);
-   seq_printf(m, command = );
+   seq_puts(m, command = );
lprint_command(cmd-cmnd, m);
 }
 
@@ -764,7 +764,7 @@ static void lprint_command(unsigned char *command, struct 
seq_file *m)
lprint_opcode(command[0], m);
for (i = 1, s = COMMAND_SIZE(command[0]); i  s; ++i)
seq_printf(m

[PATCH v2 1/6] scsi: Remove SPRINTF macro

[Rasmus Villemoes]

The macro SPRINTF doesn't save a lot of typing or make the code more
readable, and depending on a specific identifier (m) in the
surrounding scope is generally frowned upon. Nuke it.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/NCR5380.c   |  20 ++-
 drivers/scsi/aha152x.c   | 295 +--
 drivers/scsi/dc395x.c|  78 ++--
 drivers/scsi/nsp32.c |  41 +++---
 drivers/scsi/pcmcia/nsp_cs.c |  50 
 drivers/scsi/wd7000.c|  41 +++---
 6 files changed, 252 insertions(+), 273 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 36244d6..aca181e 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -716,8 +716,6 @@ static int __maybe_unused NCR5380_write_info(struct 
Scsi_Host *instance,
 }
 #endif
 
-#undef SPRINTF
-#define SPRINTF(args...) seq_printf(m, ## args)
 static
 void lprint_Scsi_Cmnd(struct scsi_cmnd *cmd, struct seq_file *m);
 static
@@ -734,19 +732,19 @@ static int __maybe_unused NCR5380_show_info(struct 
seq_file *m,
hostdata = (struct NCR5380_hostdata *) instance-hostdata;
 
 #ifdef PSEUDO_DMA
-   SPRINTF(Highwater I/O busy spin counts: write %d, read %d\n,
+   seq_printf(m, Highwater I/O busy spin counts: write %d, read %d\n,
hostdata-spin_max_w, hostdata-spin_max_r);
 #endif
spin_lock_irq(instance-host_lock);
if (!hostdata-connected)
-   SPRINTF(scsi%d: no currently connected command\n, 
instance-host_no);
+   seq_printf(m, scsi%d: no currently connected command\n, 
instance-host_no);
else
lprint_Scsi_Cmnd((struct scsi_cmnd *) hostdata-connected, m);
-   SPRINTF(scsi%d: issue_queue\n, instance-host_no);
+   seq_printf(m, scsi%d: issue_queue\n, instance-host_no);
for (ptr = (struct scsi_cmnd *) hostdata-issue_queue; ptr; ptr = 
(struct scsi_cmnd *) ptr-host_scribble)
lprint_Scsi_Cmnd(ptr, m);
 
-   SPRINTF(scsi%d: disconnected_queue\n, instance-host_no);
+   seq_printf(m, scsi%d: disconnected_queue\n, instance-host_no);
for (ptr = (struct scsi_cmnd *) hostdata-disconnected_queue; ptr; ptr 
= (struct scsi_cmnd *) ptr-host_scribble)
lprint_Scsi_Cmnd(ptr, m);
spin_unlock_irq(instance-host_lock);
@@ -755,8 +753,8 @@ static int __maybe_unused NCR5380_show_info(struct seq_file 
*m,
 
 static void lprint_Scsi_Cmnd(struct scsi_cmnd *cmd, struct seq_file *m)
 {
-   SPRINTF(scsi%d : destination target %d, lun %llu\n, 
cmd-device-host-host_no, cmd-device-id, cmd-device-lun);
-   SPRINTF(command = );
+   seq_printf(m, scsi%d : destination target %d, lun %llu\n, 
cmd-device-host-host_no, cmd-device-id, cmd-device-lun);
+   seq_printf(m, command = );
lprint_command(cmd-cmnd, m);
 }
 
@@ -765,13 +763,13 @@ static void lprint_command(unsigned char *command, struct 
seq_file *m)
int i, s;
lprint_opcode(command[0], m);
for (i = 1, s = COMMAND_SIZE(command[0]); i  s; ++i)
-   SPRINTF(%02x , command[i]);
-   SPRINTF(\n);
+   seq_printf(m, %02x , command[i]);
+   seq_printf(m, \n);
 }
 
 static void lprint_opcode(int opcode, struct seq_file *m)
 {
-   SPRINTF(%2d (0x%02x), opcode, opcode);
+   seq_printf(m, %2d (0x%02x), opcode, opcode);
 }
 
 
diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 2b960b3..f14ad8a 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -2490,299 +2490,296 @@ static void show_queues(struct Scsi_Host *shpnt)
disp_enintr(shpnt);
 }
 
-#undef SPRINTF
-#define SPRINTF(args...) seq_printf(m, ##args)
-
 static void get_command(struct seq_file *m, Scsi_Cmnd * ptr)
 {
int i;
 
-   SPRINTF(%p: target=%d; lun=%d; cmnd=( ,
+   seq_printf(m, %p: target=%d; lun=%d; cmnd=( ,
ptr, ptr-device-id, (u8)ptr-device-lun);
 
for (i = 0; i  COMMAND_SIZE(ptr-cmnd[0]); i++)
-   SPRINTF(0x%02x , ptr-cmnd[i]);
+   seq_printf(m, 0x%02x , ptr-cmnd[i]);
 
-   SPRINTF(); resid=%d; residual=%d; buffers=%d; phase |,
+   seq_printf(m, ); resid=%d; residual=%d; buffers=%d; phase |,
scsi_get_resid(ptr), ptr-SCp.this_residual,
ptr-SCp.buffers_residual);
 
if (ptr-SCp.phase  not_issued)
-   SPRINTF(not issued|);
+   seq_printf(m, not issued|);
if (ptr-SCp.phase  selecting)
-   SPRINTF(selecting|);
+   seq_printf(m, selecting|);
if (ptr-SCp.phase  disconnected)
-   SPRINTF(disconnected|);
+   seq_printf(m, disconnected|);
if (ptr-SCp.phase  aborted)
-   SPRINTF(aborted|);
+   seq_printf(m, aborted|);
if (ptr-SCp.phase  identified)
-   SPRINTF(identified|);
+   seq_printf(m, identified|);
if (ptr-SCp.phase

[PATCH v2 5/6] scsi: misc: Merge consecutive seq_puts calls

[Rasmus Villemoes]

Consecutive seq_puts calls with literal strings may be replaced by a
single call, saving a little .text.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/advansys.c  | 43 +++
 drivers/scsi/atp870u.c   |  5 ++---
 drivers/scsi/dc395x.c|  4 ++--
 drivers/scsi/pcmcia/nsp_cs.c |  4 ++--
 4 files changed, 21 insertions(+), 35 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index d31fc6d..c4d0910 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2903,11 +2903,9 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
 * the BIOS code segment base address.
 */
if (boardp-bios_signature != 0x55AA) {
-   seq_puts(m, Disabled or Pre-3.1\n);
-   seq_puts(m,
-BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n);
-   seq_puts(m,
-can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m, Disabled or Pre-3.1\n
+   BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n
+   can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
} else {
major = (boardp-bios_version  12)  0xF;
minor = (boardp-bios_version  8)  0xF;
@@ -2923,9 +2921,8 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
 */
if (major  3 || (major = 3  minor  1) ||
(major = 3  minor = 1  letter  ('I' - 'A'))) {
-   seq_puts(m,
-Newer version of ROM BIOS is available at the 
ConnectCom FTP site:\n);
-   seq_puts(m, ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m, Newer version of ROM BIOS is available at 
the ConnectCom FTP site:\n
+   ftp://ftp.connectcom.net/pub\n;);
}
}
 }
@@ -3071,27 +3068,23 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
*m, struct Scsi_Host *shost
seq_puts(m,  Target ID:   );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %d, i);
-   seq_puts(m, \n);
 
-   seq_puts(m,  Disconnects: );
+   seq_puts(m, \n Disconnects: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-disc_enable  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_puts(m, \n);
 
-   seq_puts(m,  Command Queuing: );
+   seq_puts(m, \n Command Queuing: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-use_cmd_qng  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_puts(m, \n);
 
-   seq_puts(m,  Start Motor: );
+   seq_puts(m, \n Start Motor: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-start_motor  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_puts(m, \n);
 
-   seq_puts(m,  Synchronous Transfer:);
+   seq_puts(m, \n Synchronous Transfer:);
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-init_sdtr  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
@@ -3410,10 +3403,9 @@ static void asc_prt_asc_board_info(struct seq_file *m, 
struct Scsi_Host *shost)
   i,
   (v-use_tagged_qng  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
}
-   seq_puts(m, \n);
 
/* Current number of commands waiting for a device. */
-   seq_puts(m,  Command Queue Pending:);
+   seq_puts(m, \n Command Queue Pending:);
for (i = 0; i = ASC_MAX_TID; i++) {
if ((chip_scsi_id == i) ||
((boardp-init_tidmask  ADV_TID_TO_TIDMASK(i)) == 0)) {
@@ -3421,10 +3413,9 @@ static void asc_prt_asc_board_info(struct seq_file *m, 
struct Scsi_Host *shost)
}
seq_printf(m,  %X:%u, i, v-cur_dvc_qng[i]);
}
-   seq_puts(m, \n);
 
/* Current limit on number of commands that can be sent to a device. */
-   seq_puts(m,  Command Queue Limit:);
+   seq_puts(m, \n Command Queue Limit:);
for (i = 0; i = ASC_MAX_TID; i++) {
if ((chip_scsi_id == i) ||
((boardp-init_tidmask  ADV_TID_TO_TIDMASK(i)) == 0)) {
@@ -3432,10 +3423,9 @@ static void asc_prt_asc_board_info(struct seq_file *m, 
struct Scsi_Host *shost)
}
seq_printf(m,  %X:%u, i, v-max_dvc_qng[i]);
}
-   seq_puts(m, \n);
 
/* Indicate whether the device has returned queue full status. */
-   seq_puts(m,  Command Queue Full:);
+   seq_puts(m, \n Command Queue Full:);
for (i = 0; i

[PATCH v2 2/6] scsi/advansys: Replace seq_printf with seq_puts

[Rasmus Villemoes]

Using seq_printf to print a simple string is a lot more expensive than
it needs to be, since seq_puts exists. Replace seq_printf with
seq_puts when possible.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/advansys.c | 155 +++-
 1 file changed, 75 insertions(+), 80 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 6719a33..d31fc6d 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2880,7 +2880,7 @@ static void asc_prt_board_devices(struct seq_file *m, 
struct Scsi_Host *shost)
chip_scsi_id = boardp-dvc_var.adv_dvc_var.chip_scsi_id;
}
 
-   seq_printf(m, Target IDs Detected:);
+   seq_puts(m, Target IDs Detected:);
for (i = 0; i = ADV_MAX_TID; i++) {
if (boardp-init_tidmask  ADV_TID_TO_TIDMASK(i))
seq_printf(m,  %X,, i);
@@ -2896,18 +2896,18 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
struct asc_board *boardp = shost_priv(shost);
ushort major, minor, letter;
 
-   seq_printf(m, \nROM BIOS Version: );
+   seq_puts(m, \nROM BIOS Version: );
 
/*
 * If the BIOS saved a valid signature, then fill in
 * the BIOS code segment base address.
 */
if (boardp-bios_signature != 0x55AA) {
-   seq_printf(m, Disabled or Pre-3.1\n);
-   seq_printf(m,
- BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n);
-   seq_printf(m,
- can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m, Disabled or Pre-3.1\n);
+   seq_puts(m,
+BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n);
+   seq_puts(m,
+can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
} else {
major = (boardp-bios_version  12)  0xF;
minor = (boardp-bios_version  8)  0xF;
@@ -2923,10 +2923,9 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
 */
if (major  3 || (major = 3  minor  1) ||
(major = 3  minor = 1  letter  ('I' - 'A'))) {
-   seq_printf(m,
-  Newer version of ROM BIOS is available at 
the ConnectCom FTP site:\n);
-   seq_printf(m,
-  ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m,
+Newer version of ROM BIOS is available at the 
ConnectCom FTP site:\n);
+   seq_puts(m, ftp://ftp.connectcom.net/pub\n;);
}
}
 }
@@ -3056,11 +3055,10 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
*m, struct Scsi_Host *shost
== ASC_TRUE)
seq_printf(m,  Serial Number: %s\n, serialstr);
else if (ep-adapter_info[5] == 0xBB)
-   seq_printf(m,
-   Default Settings Used for EEPROM-less Adapter.\n);
+   seq_puts(m,
+ Default Settings Used for EEPROM-less Adapter.\n);
else
-   seq_printf(m,
-   Serial Number Signature Not Present.\n);
+   seq_puts(m,  Serial Number Signature Not Present.\n);
 
seq_printf(m,
Host SCSI ID: %u, Host Queue Size: %u, Device Queue Size: 
%u\n,
@@ -3070,34 +3068,34 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
*m, struct Scsi_Host *shost
seq_printf(m,
cntl 0x%x, no_scam 0x%x\n, ep-cntl, ep-no_scam);
 
-   seq_printf(m,  Target ID:   );
+   seq_puts(m,  Target ID:   );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %d, i);
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m,  Disconnects: );
+   seq_puts(m,  Disconnects: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-disc_enable  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m,  Command Queuing: );
+   seq_puts(m,  Command Queuing: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-use_cmd_qng  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m,  Start Motor: );
+   seq_puts(m,  Start Motor: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-start_motor  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m

[PATCH 0/7] scsi: Some seq_file cleanups/optimizations

[Rasmus Villemoes]

These patches mostly replace seq_printf with simpler and faster
equivalents, e.g. seq_printf(m, something) = seq_puts(m,
something) and seq_printf(m, \n) = seq_putc(m, '\n). But before
my Coccinelle scripts could be unleashed I had to clean up some
unnecessary, and in the PRINTP case quite obfuscating, macros.

The patches don't change the semantics of the code (well, that's the
idea anyway), but should make it slightly smaller and faster.

Rasmus Villemoes (7):
  scsi: Remove SPRINTF macro
  scsi/g_NCR5380: Remove obfuscating macros
  scsi/advansys: Replace seq_printf with seq_puts
  scsi/aha152x: Replace seq_printf with seq_puts
  scsi: misc:  Replace seq_printf with seq_puts
  scsi: misc: Merge consecutive seq_puts calls
  scsi: misc: Print single-character strings with seq_putc

 drivers/scsi/BusLogic.c |  10 +-
 drivers/scsi/NCR5380.c  |  42 +++--
 drivers/scsi/advansys.c | 145 -
 drivers/scsi/aha152x.c  | 301 ++--
 drivers/scsi/aic7xxx/aic79xx_proc.c |  38 +++--
 drivers/scsi/aic7xxx/aic7xxx_proc.c |  24 +--
 drivers/scsi/arm/fas216.c   |   6 +-
 drivers/scsi/atari_NCR5380.c|   4 +-
 drivers/scsi/atp870u.c  |   5 +-
 drivers/scsi/dc395x.c   |  78 +-
 drivers/scsi/dpt_i2o.c  |   2 +-
 drivers/scsi/eata_pio.c |   2 +-
 drivers/scsi/esas2r/esas2r_main.c   |   2 +-
 drivers/scsi/g_NCR5380.c|  66 
 drivers/scsi/gdth_proc.c|  24 +--
 drivers/scsi/in2000.c   |  18 +--
 drivers/scsi/ips.c  |   7 +-
 drivers/scsi/megaraid.c |   2 +-
 drivers/scsi/nsp32.c|  41 +++--
 drivers/scsi/pcmcia/nsp_cs.c|  50 +++---
 drivers/scsi/qla2xxx/qla_dfs.c  |   8 +-
 drivers/scsi/scsi_proc.c|  22 +--
 drivers/scsi/scsi_trace.c   |   6 +-
 drivers/scsi/sun3_NCR5380.c |   4 +-
 drivers/scsi/wd33c93.c  |  18 +--
 drivers/scsi/wd7000.c   |  41 +++--
 26 files changed, 460 insertions(+), 506 deletions(-)

-- 
2.0.4

--
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 1/7] scsi: Remove SPRINTF macro

[Rasmus Villemoes]

The macro SPRINTF doesn't save a lot of typing or make the code more
readable, and depending on a specific identifier (m) in the
surrounding scope is generally frowned upon. Nuke it.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/NCR5380.c   |  42 +++---
 drivers/scsi/aha152x.c   | 301 +--
 drivers/scsi/dc395x.c|  78 ++-
 drivers/scsi/nsp32.c |  41 +++---
 drivers/scsi/pcmcia/nsp_cs.c |  50 ---
 drivers/scsi/wd7000.c|  41 +++---
 6 files changed, 266 insertions(+), 287 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 45da3c8..8395930 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -698,8 +698,6 @@ static int __maybe_unused NCR5380_write_info(struct 
Scsi_Host *instance,
return (-ENOSYS);   /* Currently this is a no-op */
 }
 
-#undef SPRINTF
-#define SPRINTF(args...) seq_printf(m, ## args)
 static
 void lprint_Scsi_Cmnd(Scsi_Cmnd * cmd, struct seq_file *m);
 static
@@ -715,45 +713,45 @@ static int __maybe_unused NCR5380_show_info(struct 
seq_file *m,
 
hostdata = (struct NCR5380_hostdata *) instance-hostdata;
 
-   SPRINTF(NCR5380 core release=%d.   , NCR5380_PUBLIC_RELEASE);
+   seq_printf(m, NCR5380 core release=%d.   , NCR5380_PUBLIC_RELEASE);
if (((struct NCR5380_hostdata *) instance-hostdata)-flags  
FLAG_NCR53C400)
-   SPRINTF(ncr53c400 release=%d.  , NCR53C400_PUBLIC_RELEASE);
+   seq_printf(m, ncr53c400 release=%d.  , 
NCR53C400_PUBLIC_RELEASE);
 #ifdef DTC_PUBLIC_RELEASE
-   SPRINTF(DTC 3180/3280 release %d, DTC_PUBLIC_RELEASE);
+   seq_printf(m, DTC 3180/3280 release %d, DTC_PUBLIC_RELEASE);
 #endif
 #ifdef T128_PUBLIC_RELEASE
-   SPRINTF(T128 release %d, T128_PUBLIC_RELEASE);
+   seq_printf(m, T128 release %d, T128_PUBLIC_RELEASE);
 #endif
 #ifdef GENERIC_NCR5380_PUBLIC_RELEASE
-   SPRINTF(Generic5380 release %d, GENERIC_NCR5380_PUBLIC_RELEASE);
+   seq_printf(m, Generic5380 release %d, GENERIC_NCR5380_PUBLIC_RELEASE);
 #endif
 #ifdef PAS16_PUBLIC_RELEASE
-   SPRINTF(PAS16 release=%d, PAS16_PUBLIC_RELEASE);
+   seq_printf(m, PAS16 release=%d, PAS16_PUBLIC_RELEASE);
 #endif
 
-   SPRINTF(\nBase Addr: 0x%05lX, (long) instance-base);
-   SPRINTF(io_port: %04x  , (int) instance-io_port);
+   seq_printf(m, \nBase Addr: 0x%05lX, (long) instance-base);
+   seq_printf(m, io_port: %04x  , (int) instance-io_port);
if (instance-irq == SCSI_IRQ_NONE)
-   SPRINTF(IRQ: None.\n);
+   seq_printf(m, IRQ: None.\n);
else
-   SPRINTF(IRQ: %d.\n, instance-irq);
+   seq_printf(m, IRQ: %d.\n, instance-irq);
 
 #ifdef DTC_PUBLIC_RELEASE
-   SPRINTF(Highwater I/O busy_spin_counts -- write: %d  read: %d\n, 
dtc_wmaxi, dtc_maxi);
+   seq_printf(m, Highwater I/O busy_spin_counts -- write: %d  read: 
%d\n, dtc_wmaxi, dtc_maxi);
 #endif
 #ifdef PAS16_PUBLIC_RELEASE
-   SPRINTF(Highwater I/O busy_spin_counts -- write: %d  read: %d\n, 
pas_wmaxi, pas_maxi);
+   seq_printf(m, Highwater I/O busy_spin_counts -- write: %d  read: 
%d\n, pas_wmaxi, pas_maxi);
 #endif
spin_lock_irq(instance-host_lock);
if (!hostdata-connected)
-   SPRINTF(scsi%d: no currently connected command\n, 
instance-host_no);
+   seq_printf(m, scsi%d: no currently connected command\n, 
instance-host_no);
else
lprint_Scsi_Cmnd((Scsi_Cmnd *) hostdata-connected, m);
-   SPRINTF(scsi%d: issue_queue\n, instance-host_no);
+   seq_printf(m, scsi%d: issue_queue\n, instance-host_no);
for (ptr = (Scsi_Cmnd *) hostdata-issue_queue; ptr; ptr = (Scsi_Cmnd 
*) ptr-host_scribble)
lprint_Scsi_Cmnd(ptr, m);
 
-   SPRINTF(scsi%d: disconnected_queue\n, instance-host_no);
+   seq_printf(m, scsi%d: disconnected_queue\n, instance-host_no);
for (ptr = (Scsi_Cmnd *) hostdata-disconnected_queue; ptr; ptr = 
(Scsi_Cmnd *) ptr-host_scribble)
lprint_Scsi_Cmnd(ptr, m);
spin_unlock_irq(instance-host_lock);
@@ -762,8 +760,8 @@ static int __maybe_unused NCR5380_show_info(struct seq_file 
*m,
 
 static void lprint_Scsi_Cmnd(Scsi_Cmnd * cmd, struct seq_file *m)
 {
-   SPRINTF(scsi%d : destination target %d, lun %llu\n, 
cmd-device-host-host_no, cmd-device-id, cmd-device-lun);
-   SPRINTF(command = );
+   seq_printf(m, scsi%d : destination target %d, lun %llu\n, 
cmd-device-host-host_no, cmd-device-id, cmd-device-lun);
+   seq_printf(m, command = );
lprint_command(cmd-cmnd, m);
 }
 
@@ -772,13 +770,13 @@ static void lprint_command(unsigned char *command, struct 
seq_file *m)
int i, s;
lprint_opcode(command[0], m);
for (i = 1, s = COMMAND_SIZE(command[0]); i  s; ++i)
-   SPRINTF(%02x , command[i]);
-   SPRINTF(\n

[PATCH 7/7] scsi: misc: Print single-character strings with seq_putc

[Rasmus Villemoes]

Using seq_putc to print a single character saves at least a strlen()
call and a memory access, and may also give a small .text reduction.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/NCR5380.c  |  2 +-
 drivers/scsi/advansys.c | 34 +-
 drivers/scsi/aha152x.c  |  2 +-
 drivers/scsi/aic7xxx/aic79xx_proc.c | 10 +-
 drivers/scsi/aic7xxx/aic7xxx_proc.c | 10 +-
 drivers/scsi/atari_NCR5380.c|  2 +-
 drivers/scsi/dc395x.c   |  4 ++--
 drivers/scsi/esas2r/esas2r_main.c   |  2 +-
 drivers/scsi/g_NCR5380.c|  6 +++---
 drivers/scsi/in2000.c   |  2 +-
 drivers/scsi/ips.c  |  2 +-
 drivers/scsi/nsp32.c|  2 +-
 drivers/scsi/pcmcia/nsp_cs.c|  4 ++--
 drivers/scsi/qla2xxx/qla_dfs.c  |  2 +-
 drivers/scsi/scsi_proc.c| 10 +-
 drivers/scsi/scsi_trace.c   |  2 +-
 drivers/scsi/sun3_NCR5380.c |  2 +-
 drivers/scsi/wd33c93.c  |  2 +-
 18 files changed, 50 insertions(+), 50 deletions(-)

diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index a2ed965..50cfa9a 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -771,7 +771,7 @@ static void lprint_command(unsigned char *command, struct 
seq_file *m)
lprint_opcode(command[0], m);
for (i = 1, s = COMMAND_SIZE(command[0]); i  s; ++i)
seq_printf(m, %02x , command[i]);
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 }
 
 static void lprint_opcode(int opcode, struct seq_file *m)
diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 76e7610..5f2e4ce 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -3089,7 +3089,7 @@ static void asc_prt_asc_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-init_sdtr  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
 #ifdef CONFIG_ISA
if (asc_dvc_varp-bus_type  ASC_IS_ISA) {
@@ -3204,7 +3204,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
seq_puts(m,  Target ID:   );
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %X, i);
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
word = ep_3550-disc_enable;
@@ -3217,7 +3217,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
word = ep_3550-tagqng_able;
@@ -3230,7 +3230,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
word = ep_3550-start_motor;
@@ -3243,7 +3243,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
seq_puts(m,  Synchronous Transfer:);
@@ -3251,7 +3251,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
seq_printf(m,  %c,
   (ep_3550-sdtr_able  ADV_TID_TO_TIDMASK(i)) 
?
   'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
}
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
@@ -3260,7 +3260,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
seq_printf(m,  %c,
   (ep_3550-ultra_able  ADV_TID_TO_TIDMASK(i))
   ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
}
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC3550) {
@@ -3274,7 +3274,7 @@ static void asc_prt_adv_board_eeprom(struct seq_file *m, 
struct Scsi_Host *shost
for (i = 0; i = ADV_MAX_TID; i++)
seq_printf(m,  %c,
   (word  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
-   seq_puts(m, \n);
+   seq_putc(m, '\n');
 
if (adv_dvc_varp-chip_type == ADV_CHIP_ASC38C0800

[PATCH 6/7] scsi: misc: Merge consecutive seq_puts calls

[Rasmus Villemoes]

Consecutive seq_puts calls with literal strings may be replaced by a
single call, saving a little .text.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/advansys.c  | 40 ++--
 drivers/scsi/atp870u.c   |  5 ++---
 drivers/scsi/dc395x.c|  4 ++--
 drivers/scsi/pcmcia/nsp_cs.c |  4 ++--
 4 files changed, 20 insertions(+), 33 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 720faea..76e7610 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2903,10 +2903,9 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
 * the BIOS code segment base address.
 */
if (boardp-bios_signature != 0x55AA) {
-   seq_puts(m, Disabled or Pre-3.1\n);
-   seq_puts(m,
-BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n);
seq_puts(m,
+Disabled or Pre-3.1\n
+BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n
 can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
} else {
major = (boardp-bios_version  12)  0xF;
@@ -2923,9 +2922,8 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
 */
if (major  3 || (major = 3  minor  1) ||
(major = 3  minor = 1  letter  ('I' - 'A'))) {
-   seq_puts(m,
-Newer version of ROM BIOS is available at the 
ConnectCom FTP site:\n);
-   seq_puts(m, ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m, Newer version of ROM BIOS is available at 
the ConnectCom FTP site:\n
+   ftp://ftp.connectcom.net/pub\n;);
}
}
 }
@@ -3071,27 +3069,23 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
*m, struct Scsi_Host *shost
seq_puts(m,  Target ID:   );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %d, i);
-   seq_puts(m, \n);
 
-   seq_puts(m,  Disconnects: );
+   seq_puts(m, \n Disconnects: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-disc_enable  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_puts(m, \n);
 
-   seq_puts(m,  Command Queuing: );
+   seq_puts(m, \n Command Queuing: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-use_cmd_qng  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_puts(m, \n);
 
-   seq_puts(m,  Start Motor: );
+   seq_puts(m, \n Start Motor: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-start_motor  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_puts(m, \n);
 
-   seq_puts(m,  Synchronous Transfer:);
+   seq_puts(m, \n Synchronous Transfer:);
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-init_sdtr  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 'N');
@@ -3410,10 +3404,9 @@ static void asc_prt_asc_board_info(struct seq_file *m, 
struct Scsi_Host *shost)
   i,
   (v-use_tagged_qng  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
}
-   seq_puts(m, \n);
 
/* Current number of commands waiting for a device. */
-   seq_puts(m,  Command Queue Pending:);
+   seq_puts(m, \n Command Queue Pending:);
for (i = 0; i = ASC_MAX_TID; i++) {
if ((chip_scsi_id == i) ||
((boardp-init_tidmask  ADV_TID_TO_TIDMASK(i)) == 0)) {
@@ -3421,10 +3414,9 @@ static void asc_prt_asc_board_info(struct seq_file *m, 
struct Scsi_Host *shost)
}
seq_printf(m,  %X:%u, i, v-cur_dvc_qng[i]);
}
-   seq_puts(m, \n);
 
/* Current limit on number of commands that can be sent to a device. */
-   seq_puts(m,  Command Queue Limit:);
+   seq_puts(m, \n Command Queue Limit:);
for (i = 0; i = ASC_MAX_TID; i++) {
if ((chip_scsi_id == i) ||
((boardp-init_tidmask  ADV_TID_TO_TIDMASK(i)) == 0)) {
@@ -3432,10 +3424,9 @@ static void asc_prt_asc_board_info(struct seq_file *m, 
struct Scsi_Host *shost)
}
seq_printf(m,  %X:%u, i, v-max_dvc_qng[i]);
}
-   seq_puts(m, \n);
 
/* Indicate whether the device has returned queue full status. */
-   seq_puts(m,  Command Queue Full:);
+   seq_puts(m, \n Command Queue Full:);
for (i = 0; i = ASC_MAX_TID; i++) {
if ((chip_scsi_id == i) ||
((boardp-init_tidmask  ADV_TID_TO_TIDMASK(i)) == 0)) {
@@ -3447,9 +3438,8

[PATCH 5/7] scsi: misc: Replace seq_printf with seq_puts

[Rasmus Villemoes]

Using seq_printf to print a simple string is a lot more expensive than
it needs to be, since seq_puts exists. Replace seq_printf with
seq_puts when possible.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/BusLogic.c | 10 +-
 drivers/scsi/NCR5380.c  |  6 +++---
 drivers/scsi/aic7xxx/aic79xx_proc.c | 38 ++---
 drivers/scsi/aic7xxx/aic7xxx_proc.c | 24 +++
 drivers/scsi/arm/fas216.c   |  6 +++---
 drivers/scsi/atari_NCR5380.c|  4 ++--
 drivers/scsi/atp870u.c  |  6 +++---
 drivers/scsi/dc395x.c   | 16 
 drivers/scsi/dpt_i2o.c  |  2 +-
 drivers/scsi/eata_pio.c |  2 +-
 drivers/scsi/g_NCR5380.c| 24 +++
 drivers/scsi/gdth_proc.c| 24 +++
 drivers/scsi/in2000.c   | 18 +-
 drivers/scsi/ips.c  |  7 +++
 drivers/scsi/megaraid.c |  2 +-
 drivers/scsi/nsp32.c| 14 +++---
 drivers/scsi/pcmcia/nsp_cs.c| 30 ++---
 drivers/scsi/qla2xxx/qla_dfs.c  |  8 
 drivers/scsi/scsi_proc.c| 22 ++---
 drivers/scsi/scsi_trace.c   |  6 +++---
 drivers/scsi/sun3_NCR5380.c |  4 ++--
 drivers/scsi/wd33c93.c  | 18 +-
 drivers/scsi/wd7000.c   | 12 ++--
 23 files changed, 150 insertions(+), 153 deletions(-)

diff --git a/drivers/scsi/BusLogic.c b/drivers/scsi/BusLogic.c
index 64c7514..7a91d60 100644
--- a/drivers/scsi/BusLogic.c
+++ b/drivers/scsi/BusLogic.c
@@ -3485,7 +3485,7 @@ static int blogic_show_info(struct seq_file *m, struct 
Scsi_Host *shost)
seq_printf(m, \n\
 Current Driver Queue Depth:%d\n\
 Currently Allocated CCBs:  %d\n, adapter-drvr_qdepth, 
adapter-alloc_ccbs);
-   seq_printf(m, \n\n\
+   seq_puts(m, \n\n\
   DATA TRANSFER STATISTICS\n\
 \n\
 Target Tagged Queuing  Queue Depth  Active  Attempted  Completed\n\
@@ -3500,7 +3500,7 @@ TargetTagged Queuing  Queue Depth  Active  Attempted  
Completed\n\
seq_printf(m,
   %3d   %3u%9u%9u\n, 
adapter-qdepth[tgt], adapter-active_cmds[tgt], tgt_stats[tgt].cmds_tried, 
tgt_stats[tgt].cmds_complete);
}
-   seq_printf(m, \n\
+   seq_puts(m, \n\
 Target  Read Commands  Write Commands   Total Bytes ReadTotal Bytes 
Written\n\
 ==  =  ==  ===  
===\n);
for (tgt = 0; tgt  adapter-maxdev; tgt++) {
@@ -3517,7 +3517,7 @@ Target  Read Commands  Write Commands   Total Bytes Read  
  Total Bytes Written\
else
seq_printf(m,   %9u\n, 
tgt_stats[tgt].byteswritten.units);
}
-   seq_printf(m, \n\
+   seq_puts(m, \n\
 Target  Command0-1KB  1-2KB  2-4KB  4-8KB 8-16KB\n\
 ==  ===  =  =  =  =  =\n);
for (tgt = 0; tgt  adapter-maxdev; tgt++) {
@@ -3533,7 +3533,7 @@ Target  Command0-1KB  1-2KB  2-4KB  4-8KB 
8-16KB\n\
tgt_stats[tgt].write_sz_buckets[0],
tgt_stats[tgt].write_sz_buckets[1], 
tgt_stats[tgt].write_sz_buckets[2], tgt_stats[tgt].write_sz_buckets[3], 
tgt_stats[tgt].write_sz_buckets[4]);
}
-   seq_printf(m, \n\
+   seq_puts(m, \n\
 Target  Command   16-32KB32-64KB   64-128KB   128-256KB   256KB+\n\
 ==  ===  =  =  =  =  =\n);
for (tgt = 0; tgt  adapter-maxdev; tgt++) {
@@ -3549,7 +3549,7 @@ Target  Command   16-32KB32-64KB   64-128KB   
128-256KB   256KB+\n\
tgt_stats[tgt].write_sz_buckets[5],
tgt_stats[tgt].write_sz_buckets[6], 
tgt_stats[tgt].write_sz_buckets[7], tgt_stats[tgt].write_sz_buckets[8], 
tgt_stats[tgt].write_sz_buckets[9]);
}
-   seq_printf(m, \n\n\
+   seq_puts(m, \n\n\
   ERROR RECOVERY STATISTICS\n\
 \n\
  Command Aborts  Bus Device Resets   Host Adapter Resets\n\
diff --git a/drivers/scsi/NCR5380.c b/drivers/scsi/NCR5380.c
index 8395930..a2ed965 100644
--- a/drivers/scsi/NCR5380.c
+++ b/drivers/scsi/NCR5380.c
@@ -732,7 +732,7 @@ static int __maybe_unused NCR5380_show_info(struct seq_file 
*m,
seq_printf(m, \nBase Addr: 0x%05lX, (long) instance-base);
seq_printf(m, io_port: %04x  , (int) instance-io_port);
if (instance-irq == SCSI_IRQ_NONE)
-   seq_printf(m, IRQ: None.\n);
+   seq_puts(m, IRQ: None.\n);
else
seq_printf(m, IRQ: %d.\n, instance-irq);
 
@@ -761,7 +761,7 @@ static int __maybe_unused NCR5380_show_info(struct seq_file 
*m

[PATCH 4/7] scsi/aha152x: Replace seq_printf with seq_puts

[Rasmus Villemoes]

Using seq_printf to print a simple string is a lot more expensive than
it needs to be, since seq_puts exists. Replace seq_printf with
seq_puts when possible.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/aha152x.c | 252 -
 1 file changed, 126 insertions(+), 126 deletions(-)

diff --git a/drivers/scsi/aha152x.c b/drivers/scsi/aha152x.c
index 80823ac..05cad58 100644
--- a/drivers/scsi/aha152x.c
+++ b/drivers/scsi/aha152x.c
@@ -2991,21 +2991,21 @@ static void get_command(struct seq_file *m, Scsi_Cmnd * 
ptr)
ptr-SCp.buffers_residual);
 
if (ptr-SCp.phase  not_issued)
-   seq_printf(m, not issued|);
+   seq_puts(m, not issued|);
if (ptr-SCp.phase  selecting)
-   seq_printf(m, selecting|);
+   seq_puts(m, selecting|);
if (ptr-SCp.phase  disconnected)
-   seq_printf(m, disconnected|);
+   seq_puts(m, disconnected|);
if (ptr-SCp.phase  aborted)
-   seq_printf(m, aborted|);
+   seq_puts(m, aborted|);
if (ptr-SCp.phase  identified)
-   seq_printf(m, identified|);
+   seq_puts(m, identified|);
if (ptr-SCp.phase  completed)
-   seq_printf(m, completed|);
+   seq_puts(m, completed|);
if (ptr-SCp.phase  spiordy)
-   seq_printf(m, spiordy|);
+   seq_puts(m, spiordy|);
if (ptr-SCp.phase  syncneg)
-   seq_printf(m, syncneg|);
+   seq_puts(m, syncneg|);
seq_printf(m, ; next=0x%p\n, SCNEXT(ptr));
 }
 
@@ -3016,256 +3016,256 @@ static void get_ports(struct seq_file *m, struct 
Scsi_Host *shpnt)
seq_printf(m, \n%s: %s(%s) , CURRENT_SC ? on bus : waiting, 
states[STATE].name, states[PREVSTATE].name);
 
s = GETPORT(SCSISEQ);
-   seq_printf(m, SCSISEQ( );
+   seq_puts(m, SCSISEQ( );
if (s  TEMODEO)
-   seq_printf(m, TARGET MODE );
+   seq_puts(m, TARGET MODE );
if (s  ENSELO)
-   seq_printf(m, SELO );
+   seq_puts(m, SELO );
if (s  ENSELI)
-   seq_printf(m, SELI );
+   seq_puts(m, SELI );
if (s  ENRESELI)
-   seq_printf(m, RESELI );
+   seq_puts(m, RESELI );
if (s  ENAUTOATNO)
-   seq_printf(m, AUTOATNO );
+   seq_puts(m, AUTOATNO );
if (s  ENAUTOATNI)
-   seq_printf(m, AUTOATNI );
+   seq_puts(m, AUTOATNI );
if (s  ENAUTOATNP)
-   seq_printf(m, AUTOATNP );
+   seq_puts(m, AUTOATNP );
if (s  SCSIRSTO)
-   seq_printf(m, SCSIRSTO );
-   seq_printf(m, ););
+   seq_puts(m, SCSIRSTO );
+   seq_puts(m, ););
 
-   seq_printf(m,  SCSISIG();
+   seq_puts(m,  SCSISIG();
s = GETPORT(SCSISIG);
switch (s  P_MASK) {
case P_DATAO:
-   seq_printf(m, DATA OUT);
+   seq_puts(m, DATA OUT);
break;
case P_DATAI:
-   seq_printf(m, DATA IN);
+   seq_puts(m, DATA IN);
break;
case P_CMD:
-   seq_printf(m, COMMAND);
+   seq_puts(m, COMMAND);
break;
case P_STATUS:
-   seq_printf(m, STATUS);
+   seq_puts(m, STATUS);
break;
case P_MSGO:
-   seq_printf(m, MESSAGE OUT);
+   seq_puts(m, MESSAGE OUT);
break;
case P_MSGI:
-   seq_printf(m, MESSAGE IN);
+   seq_puts(m, MESSAGE IN);
break;
default:
-   seq_printf(m, *invalid*);
+   seq_puts(m, *invalid*);
break;
}
 
-   seq_printf(m, ); );
+   seq_puts(m, ); );
 
seq_printf(m, INTSTAT (%s); , TESTHI(DMASTAT, INTSTAT) ? hi : lo);
 
-   seq_printf(m, SSTAT( );
+   seq_puts(m, SSTAT( );
s = GETPORT(SSTAT0);
if (s  TARGET)
-   seq_printf(m, TARGET );
+   seq_puts(m, TARGET );
if (s  SELDO)
-   seq_printf(m, SELDO );
+   seq_puts(m, SELDO );
if (s  SELDI)
-   seq_printf(m, SELDI );
+   seq_puts(m, SELDI );
if (s  SELINGO)
-   seq_printf(m, SELINGO );
+   seq_puts(m, SELINGO );
if (s  SWRAP)
-   seq_printf(m, SWRAP );
+   seq_puts(m, SWRAP );
if (s  SDONE)
-   seq_printf(m, SDONE );
+   seq_puts(m, SDONE );
if (s  SPIORDY)
-   seq_printf(m, SPIORDY );
+   seq_puts(m, SPIORDY );
if (s  DMADONE)
-   seq_printf(m, DMADONE );
+   seq_puts(m, DMADONE );
 
s = GETPORT(SSTAT1);
if (s  SELTO)
-   seq_printf(m, SELTO

[PATCH 3/7] scsi/advansys: Replace seq_printf with seq_puts

[Rasmus Villemoes]

Using seq_printf to print a simple string is a lot more expensive than
it needs to be, since seq_puts exists. Replace seq_printf with
seq_puts when possible.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/advansys.c | 157 
 1 file changed, 77 insertions(+), 80 deletions(-)

diff --git a/drivers/scsi/advansys.c b/drivers/scsi/advansys.c
index 43761c1..720faea 100644
--- a/drivers/scsi/advansys.c
+++ b/drivers/scsi/advansys.c
@@ -2880,7 +2880,7 @@ static void asc_prt_board_devices(struct seq_file *m, 
struct Scsi_Host *shost)
chip_scsi_id = boardp-dvc_var.adv_dvc_var.chip_scsi_id;
}
 
-   seq_printf(m, Target IDs Detected:);
+   seq_puts(m, Target IDs Detected:);
for (i = 0; i = ADV_MAX_TID; i++) {
if (boardp-init_tidmask  ADV_TID_TO_TIDMASK(i))
seq_printf(m,  %X,, i);
@@ -2896,18 +2896,18 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
struct asc_board *boardp = shost_priv(shost);
ushort major, minor, letter;
 
-   seq_printf(m, \nROM BIOS Version: );
+   seq_puts(m, \nROM BIOS Version: );
 
/*
 * If the BIOS saved a valid signature, then fill in
 * the BIOS code segment base address.
 */
if (boardp-bios_signature != 0x55AA) {
-   seq_printf(m, Disabled or Pre-3.1\n);
-   seq_printf(m,
- BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n);
-   seq_printf(m,
- can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m, Disabled or Pre-3.1\n);
+   seq_puts(m,
+BIOS either disabled or Pre-3.1. If it is pre-3.1, 
then a newer version\n);
+   seq_puts(m,
+can be found at the ConnectCom FTP site: 
ftp://ftp.connectcom.net/pub\n;);
} else {
major = (boardp-bios_version  12)  0xF;
minor = (boardp-bios_version  8)  0xF;
@@ -2923,10 +2923,9 @@ static void asc_prt_adv_bios(struct seq_file *m, struct 
Scsi_Host *shost)
 */
if (major  3 || (major = 3  minor  1) ||
(major = 3  minor = 1  letter  ('I' - 'A'))) {
-   seq_printf(m,
-  Newer version of ROM BIOS is available at 
the ConnectCom FTP site:\n);
-   seq_printf(m,
-  ftp://ftp.connectcom.net/pub\n;);
+   seq_puts(m,
+Newer version of ROM BIOS is available at the 
ConnectCom FTP site:\n);
+   seq_puts(m, ftp://ftp.connectcom.net/pub\n;);
}
}
 }
@@ -3056,11 +3055,10 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
*m, struct Scsi_Host *shost
== ASC_TRUE)
seq_printf(m,  Serial Number: %s\n, serialstr);
else if (ep-adapter_info[5] == 0xBB)
-   seq_printf(m,
-   Default Settings Used for EEPROM-less Adapter.\n);
+   seq_puts(m,
+ Default Settings Used for EEPROM-less Adapter.\n);
else
-   seq_printf(m,
-   Serial Number Signature Not Present.\n);
+   seq_puts(m,  Serial Number Signature Not Present.\n);
 
seq_printf(m,
Host SCSI ID: %u, Host Queue Size: %u, Device Queue Size: 
%u\n,
@@ -3070,34 +3068,34 @@ static void asc_prt_asc_board_eeprom(struct seq_file 
*m, struct Scsi_Host *shost
seq_printf(m,
cntl 0x%x, no_scam 0x%x\n, ep-cntl, ep-no_scam);
 
-   seq_printf(m,  Target ID:   );
+   seq_puts(m,  Target ID:   );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %d, i);
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m,  Disconnects: );
+   seq_puts(m,  Disconnects: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-disc_enable  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m,  Command Queuing: );
+   seq_puts(m,  Command Queuing: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-use_cmd_qng  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m,  Start Motor: );
+   seq_puts(m,  Start Motor: );
for (i = 0; i = ASC_MAX_TID; i++)
seq_printf(m,  %c,
   (ep-start_motor  ADV_TID_TO_TIDMASK(i)) ? 'Y' : 
'N');
-   seq_printf(m, \n);
+   seq_puts(m, \n);
 
-   seq_printf(m

[PATCH 2/7] scsi/g_NCR5380: Remove obfuscating macros

[Rasmus Villemoes]

The macros PRINTP/ANDP make the code harder to read and depend on a
specific identifier name in the surrounding scope. Nuke them.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/g_NCR5380.c | 66 ++--
 1 file changed, 30 insertions(+), 36 deletions(-)

diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c
index b331272..802b64c 100644
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -741,12 +741,9 @@ static inline int NCR5380_pwrite(struct Scsi_Host 
*instance, unsigned char *src,
  
 #include NCR5380.c
 
-#define PRINTP(x) seq_printf(m, x)
-#define ANDP ,
-
 static void sprint_opcode(struct seq_file *m, int opcode)
 {
-   PRINTP(0x%02x  ANDP opcode);
+   seq_printf(m, 0x%02x , opcode);
 }
 
 static void sprint_command(struct seq_file *m, unsigned char *command)
@@ -754,8 +751,8 @@ static void sprint_command(struct seq_file *m, unsigned 
char *command)
int i, s;
sprint_opcode(m, command[0]);
for (i = 1, s = COMMAND_SIZE(command[0]); i  s; ++i)
-   PRINTP(%02x  ANDP command[i]);
-   PRINTP(\n);
+   seq_printf(m, %02x , command[i]);
+   seq_printf(m, \n);
 }
 
 /**
@@ -768,8 +765,8 @@ static void sprint_command(struct seq_file *m, unsigned 
char *command)
 
 static void sprint_Scsi_Cmnd(struct seq_file *m, Scsi_Cmnd * cmd)
 {
-   PRINTP(host number %d destination target %d, lun %llu\n ANDP 
cmd-device-host-host_no ANDP cmd-device-id ANDP cmd-device-lun);
-   PRINTP(command = );
+   seq_printf(m, host number %d destination target %d, lun %llu\n, 
cmd-device-host-host_no, cmd-device-id, cmd-device-lun);
+   seq_printf(m, command = );
sprint_command(m, cmd-cmnd);
 }
 
@@ -806,40 +803,40 @@ static int generic_NCR5380_show_info(struct seq_file *m, 
struct Scsi_Host *scsi_
hostdata = (struct NCR5380_hostdata *) scsi_ptr-hostdata;
 
spin_lock_irqsave(scsi_ptr-host_lock, flags);
-   PRINTP(SCSI host number %d : %s\n ANDP scsi_ptr-host_no ANDP 
scsi_ptr-hostt-name);
-   PRINTP(Generic NCR5380 driver version %d\n ANDP 
GENERIC_NCR5380_PUBLIC_RELEASE);
-   PRINTP(NCR5380 core version %d\n ANDP NCR5380_PUBLIC_RELEASE);
+   seq_printf(m, SCSI host number %d : %s\n, scsi_ptr-host_no, 
scsi_ptr-hostt-name);
+   seq_printf(m, Generic NCR5380 driver version %d\n, 
GENERIC_NCR5380_PUBLIC_RELEASE);
+   seq_printf(m, NCR5380 core version %d\n, NCR5380_PUBLIC_RELEASE);
 #ifdef NCR53C400
-   PRINTP(NCR53C400 extension version %d\n ANDP 
NCR53C400_PUBLIC_RELEASE);
-   PRINTP(NCR53C400 card%s detected\n ANDP(((struct NCR5380_hostdata *) 
scsi_ptr-hostdata)-flags  FLAG_NCR53C400) ?  :  not);
+   seq_printf(m, NCR53C400 extension version %d\n, 
NCR53C400_PUBLIC_RELEASE);
+   seq_printf(m, NCR53C400 card%s detected\n, (((struct NCR5380_hostdata 
*) scsi_ptr-hostdata)-flags  FLAG_NCR53C400) ?  :  not);
 # if NCR53C400_PSEUDO_DMA
-   PRINTP(NCR53C400 pseudo DMA used\n);
+   seq_printf(m, NCR53C400 pseudo DMA used\n);
 # endif
 #else
-   PRINTP(NO NCR53C400 driver extensions\n);
+   seq_printf(m, NO NCR53C400 driver extensions\n);
 #endif
-   PRINTP(Using %s mapping at %s 0x%lx,  ANDP STRVAL(NCR5380_map_config) 
ANDP STRVAL(NCR5380_map_name) ANDP scsi_ptr-NCR5380_instance_name);
+   seq_printf(m, Using %s mapping at %s 0x%lx, , 
STRVAL(NCR5380_map_config), STRVAL(NCR5380_map_name), 
scsi_ptr-NCR5380_instance_name);
if (scsi_ptr-irq == SCSI_IRQ_NONE)
-   PRINTP(no interrupt\n);
+   seq_printf(m, no interrupt\n);
else
-   PRINTP(on interrupt %d\n ANDP scsi_ptr-irq);
+   seq_printf(m, on interrupt %d\n, scsi_ptr-irq);
 
 #ifdef NCR5380_STATS
if (hostdata-connected || hostdata-issue_queue || 
hostdata-disconnected_queue)
-   PRINTP(There are commands pending, transfer rates may be 
crud\n);
+   seq_printf(m, There are commands pending, transfer rates may 
be crud\n);
if (hostdata-pendingr)
-   PRINTP(  %d pending reads ANDP hostdata-pendingr);
+   seq_printf(m,   %d pending reads, hostdata-pendingr);
if (hostdata-pendingw)
-   PRINTP(  %d pending writes ANDP hostdata-pendingw);
+   seq_printf(m,   %d pending writes, hostdata-pendingw);
if (hostdata-pendingr || hostdata-pendingw)
-   PRINTP(\n);
+   seq_printf(m, \n);
shost_for_each_device(dev, scsi_ptr) {
unsigned long br = hostdata-bytes_read[dev-id];
unsigned long bw = hostdata-bytes_write[dev-id];
long tr = hostdata-time_read[dev-id] / HZ;
long tw = hostdata-time_write[dev-id] / HZ;
 
-   PRINTP(  T:%d %s  ANDP dev-id ANDP 
scsi_device_type(dev-type));
+   seq_printf(m,   T:%d %s , dev-id, 
scsi_device_type(dev-type

[PATCH 13/22] scsi: Replace strnicmp with strncasecmp

[Rasmus Villemoes]

The kernel used to contain two functions for length-delimited,
case-insensitive string comparison, strnicmp with correct semantics
and a slightly buggy strncasecmp. The latter is the POSIX name, so
strnicmp was renamed to strncasecmp, and strnicmp made into a wrapper
for the new strncasecmp to avoid breaking existing users.

To allow the compat wrapper strnicmp to be removed at some point in
the future, and to avoid the extra indirection cost, do
s/strnicmp/strncasecmp/g.

Cc: Adaptec OEM Raid Solutions aacr...@adaptec.com
Cc: James E.J. Bottomley jbottom...@parallels.com
Cc: linux-scsi@vger.kernel.org
Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/ips.c| 2 +-
 drivers/scsi/scsi_debug.c | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/scsi/ips.c b/drivers/scsi/ips.c
index 52a216f..e5afc38 100644
--- a/drivers/scsi/ips.c
+++ b/drivers/scsi/ips.c
@@ -528,7 +528,7 @@ ips_setup(char *ips_str)
 * Update the variables
 */
for (i = 0; i  ARRAY_SIZE(options); i++) {
-   if (strnicmp
+   if (strncasecmp
(key, options[i].option_name,
 strlen(options[i].option_name)) == 0) {
if (value)
diff --git a/drivers/scsi/scsi_debug.c b/drivers/scsi/scsi_debug.c
index d19c0e3..c81af36 100644
--- a/drivers/scsi/scsi_debug.c
+++ b/drivers/scsi/scsi_debug.c
@@ -3367,7 +3367,7 @@ static ssize_t opts_store(struct device_driver *ddp, 
const char *buf,
char work[20];
 
 if (1 == sscanf(buf, %10s, work)) {
-   if (0 == strnicmp(work,0x, 2)) {
+   if (0 == strncasecmp(work,0x, 2)) {
if (1 == sscanf(work[2], %x, opts))
goto opts_done;
} else {
-- 
2.0.4

--
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] drivers: scsi: #define missing include guards

[Rasmus Villemoes]

The four files aha1542.h, aha1740.h, gvp11.h and mvme147.h under
drivers/scsi/ contain two-thirds of an include guard, but do not
#define the macro. Add those #defines. git grep says the macro names
are not defined elsewhere.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
For good measure, here's a version with a non-broken commit message.

 drivers/scsi/aha1542.h | 1 +
 drivers/scsi/aha1740.h | 1 +
 drivers/scsi/gvp11.h   | 1 +
 drivers/scsi/mvme147.h | 1 +
 4 files changed, 4 insertions(+)

diff --git a/drivers/scsi/aha1542.h b/drivers/scsi/aha1542.h
index b871d2b..8f4e07a 100644
--- a/drivers/scsi/aha1542.h
+++ b/drivers/scsi/aha1542.h
@@ -1,4 +1,5 @@
 #ifndef _AHA1542_H
+#define _AHA1542_H
 
 /* $Id: aha1542.h,v 1.1 1992/07/24 06:27:38 root Exp root $
  *
diff --git a/drivers/scsi/aha1740.h b/drivers/scsi/aha1740.h
index af23fd6..7ea484f 100644
--- a/drivers/scsi/aha1740.h
+++ b/drivers/scsi/aha1740.h
@@ -1,4 +1,5 @@
 #ifndef _AHA1740_H
+#define _AHA1740_H
 
 /* $Id$
  *
diff --git a/drivers/scsi/gvp11.h b/drivers/scsi/gvp11.h
index 852913c..5aabe90 100644
--- a/drivers/scsi/gvp11.h
+++ b/drivers/scsi/gvp11.h
@@ -1,4 +1,5 @@
 #ifndef GVP11_H
+#define GVP11_H
 
 /* $Id: gvp11.h,v 1.4 1997/01/19 23:07:12 davem Exp $
  *
diff --git a/drivers/scsi/mvme147.h b/drivers/scsi/mvme147.h
index bfd4566..479e9b4 100644
--- a/drivers/scsi/mvme147.h
+++ b/drivers/scsi/mvme147.h
@@ -1,4 +1,5 @@
 #ifndef MVME147_H
+#define MVME147_H
 
 /* $Id: mvme147.h,v 1.4 1997/01/19 23:07:10 davem Exp $
  *
-- 
2.0.4

--
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] drivers: scsi: #define missing include guards

[Rasmus Villemoes]

The four files aha1542.h, aha1740.h, gvp11.h and mvme147.h under
drivers/scsi/ contain two-thirds of an include guard, but do not
elsewhere.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/scsi/aha1542.h | 1 +
 drivers/scsi/aha1740.h | 1 +
 drivers/scsi/gvp11.h   | 1 +
 drivers/scsi/mvme147.h | 1 +
 4 files changed, 4 insertions(+)

diff --git a/drivers/scsi/aha1542.h b/drivers/scsi/aha1542.h
index b871d2b..8f4e07a 100644
--- a/drivers/scsi/aha1542.h
+++ b/drivers/scsi/aha1542.h
@@ -1,4 +1,5 @@
 #ifndef _AHA1542_H
+#define _AHA1542_H
 
 /* $Id: aha1542.h,v 1.1 1992/07/24 06:27:38 root Exp root $
  *
diff --git a/drivers/scsi/aha1740.h b/drivers/scsi/aha1740.h
index af23fd6..7ea484f 100644
--- a/drivers/scsi/aha1740.h
+++ b/drivers/scsi/aha1740.h
@@ -1,4 +1,5 @@
 #ifndef _AHA1740_H
+#define _AHA1740_H
 
 /* $Id$
  *
diff --git a/drivers/scsi/gvp11.h b/drivers/scsi/gvp11.h
index 852913c..5aabe90 100644
--- a/drivers/scsi/gvp11.h
+++ b/drivers/scsi/gvp11.h
@@ -1,4 +1,5 @@
 #ifndef GVP11_H
+#define GVP11_H
 
 /* $Id: gvp11.h,v 1.4 1997/01/19 23:07:12 davem Exp $
  *
diff --git a/drivers/scsi/mvme147.h b/drivers/scsi/mvme147.h
index bfd4566..479e9b4 100644
--- a/drivers/scsi/mvme147.h
+++ b/drivers/scsi/mvme147.h
@@ -1,4 +1,5 @@
 #ifndef MVME147_H
+#define MVME147_H
 
 /* $Id: mvme147.h,v 1.4 1997/01/19 23:07:10 davem Exp $
  *
-- 
1.9.2

--
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] drivers: target: target_core_ua_h: Add #define of include guard

[Rasmus Villemoes]

Clearly the file was meant to contain an include guard, but it was
missing the #define part.

Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
---
 drivers/target/target_core_ua.h | 1 +
 1 file changed, 1 insertion(+)

diff --git a/drivers/target/target_core_ua.h b/drivers/target/target_core_ua.h
index be912b3..a6b56b3 100644
--- a/drivers/target/target_core_ua.h
+++ b/drivers/target/target_core_ua.h
@@ -1,4 +1,5 @@
 #ifndef TARGET_CORE_UA_H
+#define TARGET_CORE_UA_H
 
 /*
  * From spc4r17, Table D.1: ASC and ASCQ Assignement
-- 
1.9.2

--
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

Re: [PATCH] drivers: scsi: #define missing include guards

[Rasmus Villemoes]

Rasmus Villemoes li...@rasmusvillemoes.dk writes:

 The four files aha1542.h, aha1740.h, gvp11.h and mvme147.h under
 drivers/scsi/ contain two-thirds of an include guard, but do not
 elsewhere.


Argh, git commit ate a line because it happened to start with #. This
was supposed to be something like ...but do not #define the macro. git
grep says that the macro names are not defined elsewhere.

Rasmus

--
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

Re: [PATCH] drivers: message: fusion: Simplify rounding

[Rasmus Villemoes]

Joe Lawrence joe.lawre...@stratus.com writes:

 On Tue, 1 Jul 2014, Rasmus Villemoes wrote:

 Rounding up to a multiple of 4 should be done using the ALIGN
 macro. As a bonus, this also makes the generated code smaller.
 
 In GetIocFacts(), sz is assigned to a few lines below without being
 read in the meantime, so it is ok that it doesn't end up with the same
 value as facts-FWImageSize.
 
 Signed-off-by: Rasmus Villemoes li...@rasmusvillemoes.dk
 ---
  drivers/message/fusion/mptbase.c | 7 +--
  drivers/message/fusion/mptctl.c  | 7 +--
  2 files changed, 2 insertions(+), 12 deletions(-)
 
 diff --git a/drivers/message/fusion/mptbase.c 
 b/drivers/message/fusion/mptbase.c
 index ebc0af7..10b16a1 100644
 --- a/drivers/message/fusion/mptbase.c
 +++ b/drivers/message/fusion/mptbase.c
 @@ -3175,12 +3175,7 @@ GetIocFacts(MPT_ADAPTER *ioc, int sleepFlag, int 
 reason)
  facts-FWImageSize = le32_to_cpu(facts-FWImageSize);
  }
  
 -sz = facts-FWImageSize;
 -if ( sz  0x01 )
 -sz += 1;
 -if ( sz  0x02 )
 -sz += 2;
 -facts-FWImageSize = sz;
 +facts-FWImageSize = ALIGN(facts-FWImageSize, 4);
  
  if (!facts-RequestFrameSize) {
  /*  Something is wrong!  */
 diff --git a/drivers/message/fusion/mptctl.c 
 b/drivers/message/fusion/mptctl.c
 index 8a050e8..1004392 100644
 --- a/drivers/message/fusion/mptctl.c
 +++ b/drivers/message/fusion/mptctl.c
 @@ -1749,12 +1749,7 @@ mptctl_replace_fw (unsigned long arg)
  
  /* Allocate memory for the new FW image
   */
 -newFwSize = karg.newImageSize;
 -
 -if (newFwSize  0x01)
 -newFwSize += 1;
 -if (newFwSize  0x02)
 -newFwSize += 2;
 +newFwSize = ALIGN(karg.newImageSize, 4);
  
  mpt_alloc_fw_memory(ioc, newFwSize);
  if (ioc-cached_fw == NULL)

 ALIGN is certainly more readable to me.

 Reviewed-by: Joe Lawrence joe.lawre...@stratus.com

 -- Joe

Did anyone pick this up?

Thanks,
Rasmus
--
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