[PATCH] scsi: aic7xxx: fix ahc_delay and ahd_delay

2016-10-14 Thread Denys Vlasenko
They are buggy:

while (usec > 0)
   udelay(usec % 1024);
   usec -= 1024;

For example, for usec = 100*1024 + 1, old code will udelay(1) 101 times,
i.e. it will be approximately equivalent to udelay(101),
not the expected udelay(102400).

This did not bite because all callers use values far from "pathological" ones,
such as 500 and 1000 - these work fine with buggy code.

This was reported in 2006 but was missed.

Signed-off-by: Denys Vlasenko 
CC: James Bottomley 
CC: Hannes Reinicke 
CC: linux-scsi@vger.kernel.org
CC: linux-ker...@vger.kernel.org
---
 drivers/scsi/aic7xxx/aic79xx_osm.c | 7 ---
 drivers/scsi/aic7xxx/aic7xxx_osm.c | 7 ---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/drivers/scsi/aic7xxx/aic79xx_osm.c 
b/drivers/scsi/aic7xxx/aic79xx_osm.c
index 2588b8f..e7a7838 100644
--- a/drivers/scsi/aic7xxx/aic79xx_osm.c
+++ b/drivers/scsi/aic7xxx/aic79xx_osm.c
@@ -380,9 +380,10 @@ ahd_delay(long usec)
 * multi-millisecond waits.  Wait at most
 * 1024us per call.
 */
-   while (usec > 0) {
-   udelay(usec % 1024);
-   usec -= 1024;
+   udelay(usec & 1023);
+   usec >>= 10;
+   while (--usec >= 0) {
+   udelay(1024);
}
 }
 
diff --git a/drivers/scsi/aic7xxx/aic7xxx_osm.c 
b/drivers/scsi/aic7xxx/aic7xxx_osm.c
index fc6a831..c81798e 100644
--- a/drivers/scsi/aic7xxx/aic7xxx_osm.c
+++ b/drivers/scsi/aic7xxx/aic7xxx_osm.c
@@ -388,9 +388,10 @@ ahc_delay(long usec)
 * multi-millisecond waits.  Wait at most
 * 1024us per call.
 */
-   while (usec > 0) {
-   udelay(usec % 1024);
-   usec -= 1024;
+   udelay(usec & 1023);
+   usec >>= 10;
+   while (--usec >= 0) {
+   udelay(1024);
}
 }
 
-- 
2.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] qla2xxx: rewrite code to avoid hitting gcc bug 70646

2016-04-15 Thread Denys Vlasenko
On 04/15/2016 09:05 PM, James Bottomley wrote:
> On Fri, 2016-04-15 at 20:56 +0200, Denys Vlasenko wrote:
>> On 04/15/2016 04:40 PM, James Bottomley wrote:
>>> On Fri, 2016-04-15 at 12:36 +0200, Denys Vlasenko wrote:
>>>> More info here:
>>>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
>>>
>>> This bug is under investigation, so I'd rather not alter code for a
>>> gcc
>>> bug until we know if we can supply options to fix it rather than
>>> changing code.
>>
>>
>> Background. The bug exists in gcc for 2 years, but it is rather
>> hard to trigger, so nobody noticed.
> 
> We know this ... linux-scsi is on the cc for the other thread on this.
> 
>> Unfortunately for kernel, these two commits landed in Linus tree
>> in March 16 and 17:
>>
>>
>> On 04/13/2016 05:36 AM, Josh Poimboeuf wrote:
>>> It occurs with the combination of the following two recent commits:
>>>
>>> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining
>>> of some byteswap operations")
>>> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")
>>
>>
>> and now *many* users of qla2x00 and new-ish gcc are going to
>> very much notice it, as their kernels will start crashing reliably.
>>
>> The commits can be reverted, sure, but they per se do not contain
>> anything unusual. They, together with not very typical construct
>> in qla2x00_get_host_fabric_name, one
>> which boils down to "swab64p(constant_array_of_8_bytes)",
>> just happen to nudge gcc in a right way to finally trigger the bug.
>>
>> So I came with another idea how to forestall the imminent deluge of
>> qla2x00 oops reports - this patch.
> 
> There are actually a raft of checkers that run the upstream code which
> aren't seeing any problem; likely because the code is harder to trigger
> than you think.  So, lets wait until the resolution of the other thread
> before we panic, especially since we're only at -rc3.

I'm not panicking, James.

By sending a workaround, I just want to make sure that *other people*
won't be forced to fix up a problem which surfaced because of *my* patch.


I'm afraid "harder to trigger than you think" is not true.
It is nearly trivial to trigger it now.
I just tried the following on a freshly installed Fedora 21 machine:

$ git clone git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
$ cd linux
$ make defconfig
$ sed '/SCSI_FC_ATTRS/d;/SCSI_LOWLEVEL/d' -i .config
$ make oldconfig# answer "yes" to everything
$ nice make -j22
$ objdump -dr drivers/scsi/qla2xxx/qla_attr.o | grep -A10 
qla2x00_get_host_fabric_name

1540 :
1540:   55  push   %rbp
1541:   48 89 e5mov%rsp,%rbp
1544:   66 66 66 2e 0f 1f 84data32 data32 nopw %cs:0x0(%rax,%rax,1)
154b:   00 00 00 00 00

1550 :
1550:   55  push   %rbp
1551:   48 89 e5mov%rsp,%rbp
1554:   53  push   %rbx
1555:   48 89 d3mov%rdx,%rbx

See?
I'm sure Fedora 22, 23 and 24 will also do that.

--
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] qla2xxx: rewrite code to avoid hitting gcc bug 70646

2016-04-15 Thread Denys Vlasenko
On 04/15/2016 04:40 PM, James Bottomley wrote:
> On Fri, 2016-04-15 at 12:36 +0200, Denys Vlasenko wrote:
>> More info here:
>> https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
> 
> This bug is under investigation, so I'd rather not alter code for a gcc
> bug until we know if we can supply options to fix it rather than
> changing code.


Background. The bug exists in gcc for 2 years, but it is rather
hard to trigger, so nobody noticed.

Unfortunately for kernel, these two commits landed in Linus tree
in March 16 and 17:


On 04/13/2016 05:36 AM, Josh Poimboeuf wrote:
> It occurs with the combination of the following two recent commits:
>
> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force inlining of some 
> byteswap operations")
> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn access")


and now *many* users of qla2x00 and new-ish gcc are going to
very much notice it, as their kernels will start crashing reliably.

The commits can be reverted, sure, but they per se do not contain
anything unusual. They, together with not very typical construct
in qla2x00_get_host_fabric_name, one
which boils down to "swab64p(constant_array_of_8_bytes)",
just happen to nudge gcc in a right way to finally trigger the bug.

So I came with another idea how to forestall the imminent deluge of
qla2x00 oops reports - this patch.
--
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] qla2xxx: rewrite code to avoid hitting gcc bug 70646

2016-04-15 Thread Denys Vlasenko
More info here:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646

Signed-off-by: Denys Vlasenko 
CC: Himanshu Madhani 
CC: James Bottomley 
CC: qla2xxx-upstr...@qlogic.com
CC: Josh Poimboeuf 
CC: linux-scsi@vger.kernel.org
CC: linux-ker...@vger.kernel.org
---
 drivers/scsi/qla2xxx/qla_attr.c | 5 +
 1 file changed, 5 insertions(+)

diff --git a/drivers/scsi/qla2xxx/qla_attr.c b/drivers/scsi/qla2xxx/qla_attr.c
index 4dc06a13..2dd9c72 100644
--- a/drivers/scsi/qla2xxx/qla_attr.c
+++ b/drivers/scsi/qla2xxx/qla_attr.c
@@ -2003,9 +2003,14 @@ static void
 qla2x00_get_host_fabric_name(struct Scsi_Host *shost)
 {
scsi_qla_host_t *vha = shost_priv(shost);
+   /*
+* This can trigger gcc 4.9/5.3 bug.
+* See https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
uint8_t node_name[WWN_SIZE] = { 0xFF, 0xFF, 0xFF, 0xFF, \
0xFF, 0xFF, 0xFF, 0xFF};
u64 fabric_name = wwn_to_u64(node_name);
+*/
+   u64 fabric_name = (u64)(s64)-1; /* the same as above */
 
if (vha->device_flags & SWITCH_FOUND)
fabric_name = wwn_to_u64(vha->fabric_node_name);
-- 
1.8.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: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)

2016-04-14 Thread Denys Vlasenko
On 04/14/2016 05:57 PM, Josh Poimboeuf wrote:
> On Thu, Apr 14, 2016 at 05:29:06PM +0200, Denys Vlasenko wrote:
>> On 04/13/2016 07:10 PM, Josh Poimboeuf wrote:
>>>>>>>> From the disassembly of drivers/scsi/qla2xxx/qla_attr.o:
>>>>>>>>
>>>>>>>> 2f53 :
>>>>>>>> 2f53:   55  push   %rbp
>>>>>>>> 2f54:   48 89 e5mov%rsp,%rbp
>>>>>>>>
>>>>>>>> 2f57 :
>>>>>>>> 2f57:   55  push   %rbp
>>>>>>>> 2f58:   b9 e8 00 00 00  mov$0xe8,%ecx
>>>>>>>> 2f5d:   48 89 e5mov%rsp,%rbp
>>>>>>>> ...
>>>>>>>>
>>>>>>>> Note that qla2x00_get_host_fabric_name() is inexplicably
>>>>>>>> truncated after
>>>>>>>> setting up the frame pointer.  It falls through to the next
>>>>>>>> function, which is
>>>>>>>> very wrong.
>>>>>>>
>>>>>>> Wow, that's ... interesting.
>>>>>>>
>>>>>>>
>>>>>>>> I can recreate it with either gcc 5.3.1 or gcc 6.0 on
>>>>>>>> linus/master with
>>>>>>>> the .config from the above link.
>>>>>>>>
>>>>>>>> The call chain which appears to trigger the problem is:
>>>>>>>>
>>>>>>>> qla2x00_get_host_fabric_name()
>>>>>>>>   wwn_to_u64()
>>>>>>>> get_unaligned_be64()
>>>>>>>>   be64_to_cpup()
>>>>>>>> __be64_to_cpup() <- changed to __always_inline by this
>>>>>>>> patch
>>>>>>>>
>>>>>>>> It occurs with the combination of the following two recent
>>>>>>>> commits:
>>>>>>>>
>>>>>>>> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force
>>>>>>>> inlining of some byteswap operations")
>>>>>>>> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn
>>>>>>>> access")
>>>>>>>>
>>>>>>>> I can confirm that reverting either patch makes the problem go
>>>>>>>> away.
>>>>>>>> I'm planning on opening a gcc bug tomorrow.
>>>>>>>
>>>>>>>
>>>>>>> Note that if CONFIG_OPTIMIZE_INLINING is not set, _all_ "inline"
>>>>>>> keywords are in fact __always_inline, so the bug must be
>>>>>>> triggering
>>>>>>> even without the patch.
>>>>>>
>>>>>> Makes sense in theory, but the bug doesn't actually trigger when I
>>>>>> revert the patch and set CONFIG_OPTIMIZE_INLINING=n.
>>>>>>
>>>>>> Perhaps even more surprising, it doesn't trigger *with* the patch
>>>>>> and
>>>>>> CONFIG_OPTIMIZE_INLINING=n.
>>>>>
>>>>> [ Adding James to CC since this bug affects scsi. ]
>>>>>
>>>>> Here's the gcc bug:
>>>>>
>>>>>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
>>>>>
>>>>
>>>>
>>>> Actually, adding me doesn't help, I've added linux-scsi.  The summary
>>>> is that there's a but in gcc-5.3.1 which is miscompiling qla_attr.c ...
>>>> this means we're going to have to ask the compiler version of reported
>>>> crashes.
>>>
>>> The bug isn't specific to a compiler version.  I've seen it with gcc
>>> 5.3.1 and gcc 6.0.  I haven't tried any older versions.  And the gcc bug
>>> hasn't been resolved (or even investigated) yet.
>>>
>>> The bug is triggered by a combination of the above two commits from the
>>> 4.6 merge window, so presumably we'd need to revert one of them to avoid
>>> crashes in 4.6.
>>
>> The bug is indeed in the compiler. 4.9 and all later versions are affected.
>> gcc bugzilla now has a reproducer. In abridged form:
>>
>>
>> static inline __attribute__((always_inline)) u64 __swab6

Re: This patch triggers a bad gcc bug (was Re: [PATCH] force inlining of some byteswap operations)

2016-04-14 Thread Denys Vlasenko
On 04/13/2016 07:10 PM, Josh Poimboeuf wrote:
>> From the disassembly of drivers/scsi/qla2xxx/qla_attr.o:
>>
>> 2f53 :
>> 2f53:   55  push   %rbp
>> 2f54:   48 89 e5mov%rsp,%rbp
>>
>> 2f57 :
>> 2f57:   55  push   %rbp
>> 2f58:   b9 e8 00 00 00  mov$0xe8,%ecx
>> 2f5d:   48 89 e5mov%rsp,%rbp
>> ...
>>
>> Note that qla2x00_get_host_fabric_name() is inexplicably
>> truncated after
>> setting up the frame pointer.  It falls through to the next
>> function, which is
>> very wrong.
>
> Wow, that's ... interesting.
>
>
>> I can recreate it with either gcc 5.3.1 or gcc 6.0 on
>> linus/master with
>> the .config from the above link.
>>
>> The call chain which appears to trigger the problem is:
>>
>> qla2x00_get_host_fabric_name()
>>   wwn_to_u64()
>> get_unaligned_be64()
>>   be64_to_cpup()
>> __be64_to_cpup() <- changed to __always_inline by this
>> patch
>>
>> It occurs with the combination of the following two recent
>> commits:
>>
>> - bc27fb68aaad ("include/uapi/linux/byteorder, swab: force
>> inlining of some byteswap operations")
>> - ef3fb2422ffe ("scsi: fc: use get/put_unaligned64 for wwn
>> access")
>>
>> I can confirm that reverting either patch makes the problem go
>> away.
>> I'm planning on opening a gcc bug tomorrow.
>
>
> Note that if CONFIG_OPTIMIZE_INLINING is not set, _all_ "inline"
> keywords are in fact __always_inline, so the bug must be
> triggering
> even without the patch.

 Makes sense in theory, but the bug doesn't actually trigger when I
 revert the patch and set CONFIG_OPTIMIZE_INLINING=n.

 Perhaps even more surprising, it doesn't trigger *with* the patch
 and
 CONFIG_OPTIMIZE_INLINING=n.
>>>
>>> [ Adding James to CC since this bug affects scsi. ]
>>>
>>> Here's the gcc bug:
>>>
>>>   https://gcc.gnu.org/bugzilla/show_bug.cgi?id=70646
>>>
>>
>>
>> Actually, adding me doesn't help, I've added linux-scsi.  The summary
>> is that there's a but in gcc-5.3.1 which is miscompiling qla_attr.c ...
>> this means we're going to have to ask the compiler version of reported
>> crashes.
> 
> The bug isn't specific to a compiler version.  I've seen it with gcc
> 5.3.1 and gcc 6.0.  I haven't tried any older versions.  And the gcc bug
> hasn't been resolved (or even investigated) yet.
> 
> The bug is triggered by a combination of the above two commits from the
> 4.6 merge window, so presumably we'd need to revert one of them to avoid
> crashes in 4.6.

The bug is indeed in the compiler. 4.9 and all later versions are affected.
gcc bugzilla now has a reproducer. In abridged form:


static inline __attribute__((always_inline)) u64 __swab64p(const u64 *p)
{
 return (__builtin_constant_p((u64)(*p)) ? ((u64)( (((u64)(*p) & 
(u64)0x00ffULL) << 56) | (((u64)(*p) & (u64)0xff00ULL) 
<< 40) | (((u64)(*p) & (u64)0x00ffULL) << 24) | (((u64)(*p) & 
(u64)0xff00ULL) << 8) | (((u64)(*p) & (u64)0x00ffULL) 
>> 8) | (((u64)(*p) & (u64)0xff00ULL) >> 24) | (((u64)(*p) & 
(u64)0x00ffULL) >> 40) | (((u64)(*p) & (u64)0xff00ULL) 
>> 56))) : __builtin_bswap64(*p));
}
static inline u64 wwn_to_u64(void *wwn)
{
 return __swab64p(wwn);
}
static void qla2x00_get_host_fabric_name(struct Scsi_Host *shost)
{
 scsi_qla_host_t *vha = shost_priv(shost);
 u8 node_name[8] = { 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF, 0xFF};
 u64 fabric_name = wwn_to_u64(node_name);
 if (vha->device_flags & 0x1)
  fabric_name = wwn_to_u64(vha->fabric_node_name);
 (((struct fc_host_attrs *)(shost)->shost_data)->fabric_name) = fabric_name;
}


Two (or more, there were more before simplification) levels of inlining
are necessary for bug to trigger in this example (folding to one level
makes it go away). "__attribute__((always_inline))" is necessary too.


Since we have lots of __always_inline anyway, this bug has a potential
to miscompile kernels regardless of CONFIG_OPTIMIZE_INLINING setting,
and with or without the patches mentioned above (they just happen
to create a reliable reproducer).

Since it was not detected for two years since gcc 4.9 release,
it must be triggering quite rarely.
--
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/fnic/fnic_scsi.c: Deinline fnic_queue_abort_io_req, save 1792 bytes

2016-04-08 Thread Denys Vlasenko
This function compiles to 511 bytes of machine code.

Abort commands are not time-critical at all.

Signed-off-by: Denys Vlasenko 
CC: James Bottomley 
CC: Hiral Patel 
CC: Suma Ramars 
CC: Brian Uchino 
CC: linux-scsi@vger.kernel.org
CC: linux-ker...@vger.kernel.org
---
 drivers/scsi/fnic/fnic_scsi.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/scsi/fnic/fnic_scsi.c b/drivers/scsi/fnic/fnic_scsi.c
index 266b909..0a3edee 100644
--- a/drivers/scsi/fnic/fnic_scsi.c
+++ b/drivers/scsi/fnic/fnic_scsi.c
@@ -1435,7 +1435,7 @@ wq_copy_cleanup_scsi_cmd:
}
 }
 
-static inline int fnic_queue_abort_io_req(struct fnic *fnic, int tag,
+static int fnic_queue_abort_io_req(struct fnic *fnic, int tag,
  u32 task_req, u8 *fc_lun,
  struct fnic_io_req *io_req)
 {
-- 
2.1.0

--
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 RESEND] bfa: deinline __bfa_trc() and __bfa_trc32()

2016-02-04 Thread Denys Vlasenko
__bfa_trc() compiles to 115 bytes of machine code.
With this .config: http://busybox.net/~vda/kernel_config
there are 1494 calls of __bfa_trc().

__bfa_trc32() is very similar, so it is uninlined too.
However, it appears to be unused, therefore this patch
ifdefs it out.

Change in code size is about 130,000 bytes:

text data  bss   dec hex filename
85975426 22294712 20627456 128897594 7aed23a vmlinux.before
85842882 22294584 20627456 128764922 7accbfa vmlinux

Signed-off-by: Denys Vlasenko 
Acked-by: Anil Gurumurthy 
CC: Fabian Frederick 
CC: Anil Gurumurthy 
CC: Christoph Hellwig 
CC: Guenter Roeck 
CC: Ben Hutchings 
CC: James Bottomley 
CC: linux-ker...@vger.kernel.org
CC: linux-scsi@vger.kernel.org
---
 drivers/scsi/bfa/bfa_core.c | 40 
 drivers/scsi/bfa/bfa_cs.h   | 41 -
 2 files changed, 44 insertions(+), 37 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_core.c b/drivers/scsi/bfa/bfa_core.c
index e3f67b0..3657a00 100644
--- a/drivers/scsi/bfa/bfa_core.c
+++ b/drivers/scsi/bfa/bfa_core.c
@@ -90,6 +90,46 @@ static bfa_ioc_mbox_mcfunc_t  bfa_mbox_isrs[BFI_MC_MAX] = {
 
 
 
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
+{
+   int tail = trcm->tail;
+   struct bfa_trc_s*trc = &trcm->trc[tail];
+
+   if (trcm->stopped)
+   return;
+
+   trc->fileno = (u16) fileno;
+   trc->line = (u16) line;
+   trc->data.u64 = data;
+   trc->timestamp = BFA_TRC_TS(trcm);
+
+   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
+   if (trcm->tail == trcm->head)
+   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
+}
+
+#if 0 /* UNUSED */
+void
+__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data)
+{
+   int tail = trcm->tail;
+   struct bfa_trc_s *trc = &trcm->trc[tail];
+
+   if (trcm->stopped)
+   return;
+
+   trc->fileno = (u16) fileno;
+   trc->line = (u16) line;
+   trc->data.u32.u32 = data;
+   trc->timestamp = BFA_TRC_TS(trcm);
+
+   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
+   if (trcm->tail == trcm->head)
+   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
+}
+#endif
+
 static void
 bfa_com_port_attach(struct bfa_s *bfa)
 {
diff --git a/drivers/scsi/bfa/bfa_cs.h b/drivers/scsi/bfa/bfa_cs.h
index 91a8aa3..dd3154e 100644
--- a/drivers/scsi/bfa/bfa_cs.h
+++ b/drivers/scsi/bfa/bfa_cs.h
@@ -107,44 +107,11 @@ bfa_trc_stop(struct bfa_trc_mod_s *trcm)
trcm->stopped = 1;
 }
 
-static inline void
-__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
-{
-   int tail = trcm->tail;
-   struct bfa_trc_s*trc = &trcm->trc[tail];
-
-   if (trcm->stopped)
-   return;
-
-   trc->fileno = (u16) fileno;
-   trc->line = (u16) line;
-   trc->data.u64 = data;
-   trc->timestamp = BFA_TRC_TS(trcm);
-
-   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
-   if (trcm->tail == trcm->head)
-   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
-
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data);
 
-static inline void
-__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data)
-{
-   int tail = trcm->tail;
-   struct bfa_trc_s *trc = &trcm->trc[tail];
-
-   if (trcm->stopped)
-   return;
-
-   trc->fileno = (u16) fileno;
-   trc->line = (u16) line;
-   trc->data.u32.u32 = data;
-   trc->timestamp = BFA_TRC_TS(trcm);
-
-   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
-   if (trcm->tail == trcm->head)
-   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
+void
+__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data);
 
 #define bfa_sm_fault(__mod, __event)   do {\
bfa_trc(__mod, (((u32)0xDEAD << 16) | __event));\
-- 
1.8.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] bfa: deinline __bfa_trc, wwn2str and fcid2str

2015-09-21 Thread Denys Vlasenko
With this .config: http://busybox.net/~vda/kernel_config_ALLYES_Os,
after deinlining these functions have sizes and callsite counts
as follows:

__bfa_trc: 115 bytes, 1494 calls
wwn2str: 108 bytes, 27 calls
fcid2str: 46 bytes, 3 calls

__bfa_trc32 is similar to __bfa_trc, so it should have been
uninlined too. However, it is also unused.
Anil Gurumurthy suggested I can just delete it, I did so.

Change in code size is about 135,000 bytes:

text data  bss   dec hex filename
91470054 19945080 36421632 147836766 8cfcf5e vmlinux.before
91335078 19944984 36421632 147701694 8cdbfbe vmlinux

Signed-off-by: Denys Vlasenko 
CC: Krishna Gudipati 
CC: Vijaya Mohan Guvva 
CC: Anil Gurumurthy 
CC: James Bottomley 
CC: Fabian Frederick 
CC: Anil Gurumurthy 
CC: Christoph Hellwig 
CC: Guenter Roeck 
CC: Ben Hutchings 
CC: James Bottomley 
CC: linux-ker...@vger.kernel.org
CC: linux-scsi@vger.kernel.org
---
 drivers/scsi/bfa/bfa_core.c | 45 ++
 drivers/scsi/bfa/bfa_cs.h   | 67 +++--
 2 files changed, 49 insertions(+), 63 deletions(-)

diff --git a/drivers/scsi/bfa/bfa_core.c b/drivers/scsi/bfa/bfa_core.c
index e3f67b0..fdf0f4e 100644
--- a/drivers/scsi/bfa/bfa_core.c
+++ b/drivers/scsi/bfa/bfa_core.c
@@ -90,6 +90,51 @@ static bfa_ioc_mbox_mcfunc_t  bfa_mbox_isrs[BFI_MC_MAX] = {
 
 
 
+void
+wwn2str(char *wwn_str, u64 wwn)
+{
+   union {
+   u64 wwn;
+   u8 byte[8];
+   } w;
+
+   w.wwn = wwn;
+   sprintf(wwn_str, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", w.byte[0],
+   w.byte[1], w.byte[2], w.byte[3], w.byte[4], w.byte[5],
+   w.byte[6], w.byte[7]);
+}
+
+void
+fcid2str(char *fcid_str, u32 fcid)
+{
+   union {
+   u32 fcid;
+   u8 byte[4];
+   } f;
+
+   f.fcid = fcid;
+   sprintf(fcid_str, "%02x:%02x:%02x", f.byte[1], f.byte[2], f.byte[3]);
+}
+
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
+{
+   int tail = trcm->tail;
+   struct bfa_trc_s*trc = &trcm->trc[tail];
+
+   if (trcm->stopped)
+   return;
+
+   trc->fileno = (u16) fileno;
+   trc->line = (u16) line;
+   trc->data.u64 = data;
+   trc->timestamp = BFA_TRC_TS(trcm);
+
+   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
+   if (trcm->tail == trcm->head)
+   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
+}
+
 static void
 bfa_com_port_attach(struct bfa_s *bfa)
 {
diff --git a/drivers/scsi/bfa/bfa_cs.h b/drivers/scsi/bfa/bfa_cs.h
index 91a8aa3..3f00eab 100644
--- a/drivers/scsi/bfa/bfa_cs.h
+++ b/drivers/scsi/bfa/bfa_cs.h
@@ -107,44 +107,8 @@ bfa_trc_stop(struct bfa_trc_mod_s *trcm)
trcm->stopped = 1;
 }
 
-static inline void
-__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data)
-{
-   int tail = trcm->tail;
-   struct bfa_trc_s*trc = &trcm->trc[tail];
-
-   if (trcm->stopped)
-   return;
-
-   trc->fileno = (u16) fileno;
-   trc->line = (u16) line;
-   trc->data.u64 = data;
-   trc->timestamp = BFA_TRC_TS(trcm);
-
-   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
-   if (trcm->tail == trcm->head)
-   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
-
-
-static inline void
-__bfa_trc32(struct bfa_trc_mod_s *trcm, int fileno, int line, u32 data)
-{
-   int tail = trcm->tail;
-   struct bfa_trc_s *trc = &trcm->trc[tail];
-
-   if (trcm->stopped)
-   return;
-
-   trc->fileno = (u16) fileno;
-   trc->line = (u16) line;
-   trc->data.u32.u32 = data;
-   trc->timestamp = BFA_TRC_TS(trcm);
-
-   trcm->tail = (trcm->tail + 1) & (BFA_TRC_MAX - 1);
-   if (trcm->tail == trcm->head)
-   trcm->head = (trcm->head + 1) & (BFA_TRC_MAX - 1);
-}
+void
+__bfa_trc(struct bfa_trc_mod_s *trcm, int fileno, int line, u64 data);
 
 #define bfa_sm_fault(__mod, __event)   do {\
bfa_trc(__mod, (((u32)0xDEAD << 16) | __event));\
@@ -324,31 +288,8 @@ bfa_wc_wait(struct bfa_wc_s *wc)
bfa_wc_down(wc);
 }
 
-static inline void
-wwn2str(char *wwn_str, u64 wwn)
-{
-   union {
-   u64 wwn;
-   u8 byte[8];
-   } w;
-
-   w.wwn = wwn;
-   sprintf(wwn_str, "%02x:%02x:%02x:%02x:%02x:%02x:%02x:%02x", w.byte[0],
-   w.byte[1], w.byte[2], w.byte[3], w.byte[4], w.byte[5],
-   w.byte[6], w.byte[7]);
-}
-
-static inline void
-fcid2str(char *fcid_str, u32 fcid)
-{
-   union {
-   u32 fcid;
-   u8 byte[4];
-   } f;
-
-   f.fcid = f

Re: [PATCH] drivers/scsi/lpfc/lpfc_hw.h: Some minor cleanup.

2007-10-30 Thread Denys Vlasenko
On Tuesday 30 October 2007 10:54, Richard Knutsson wrote:
> Signed-off-by: Richard Knutsson <[EMAIL PROTECTED]>
> ---
> Diffed against linus-git
> Checked with script/checkpatch.pl
> 
> 
> diff --git a/drivers/scsi/lpfc/lpfc_hw.h b/drivers/scsi/lpfc/lpfc_hw.h
> index 451accd..6f56528 100644
> --- a/drivers/scsi/lpfc/lpfc_hw.h
> +++ b/drivers/scsi/lpfc/lpfc_hw.h
> @@ -3158,31 +3158,30 @@ struct lpfc_sli2_slim {
>   *
>   * Parameters:
>   * device : struct pci_dev 's device field
> - *
> - * return 1 => TRUE
> - *0 => FALSE
>   */
> -static inline int
> +static inline bool
>  lpfc_is_LC_HBA(unsigned short device)
>  {
> - if ((device == PCI_DEVICE_ID_TFLY) ||
> - (device == PCI_DEVICE_ID_PFLY) ||
> - (device == PCI_DEVICE_ID_LP101) ||
> - (device == PCI_DEVICE_ID_BMID) ||
> - (device == PCI_DEVICE_ID_BSMB) ||
> - (device == PCI_DEVICE_ID_ZMID) ||
> - (device == PCI_DEVICE_ID_ZSMB) ||
> - (device == PCI_DEVICE_ID_RFLY))
> - return 1;
> - else
> - return 0;
> + switch (device) {
> + case PCI_DEVICE_ID_TFLY:
> + case PCI_DEVICE_ID_PFLY:
> + case PCI_DEVICE_ID_LP101:
> + case PCI_DEVICE_ID_BMID:
> + case PCI_DEVICE_ID_BSMB:
> + case PCI_DEVICE_ID_ZMID:
> + case PCI_DEVICE_ID_ZSMB:
> + case PCI_DEVICE_ID_RFLY:
> + return true;
> + }
> +
> + return false;
>  }

Why is this patch useful? I'd rather do this instead:

-static inline int
+static int

(this function has three callsites, thus de-inlining will
make code smaller)
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [GIT PATCH] final SCSI pieces for the merge window

2007-10-30 Thread Denys Vlasenko
On Wednesday 24 October 2007 15:27, Matthew Wilcox wrote:
> On Wed, Oct 24, 2007 at 09:28:10AM -0400, James Bottomley wrote:
> > OK, so it's no secret that I'm the last of the subsystem maintainers
> > whose day job isn't working on the linux kernel.  If you want a full
> > time person, who did you have in mind?
> 
> I'm willing to take on the role of scsi git-monkey.  Alternatively, we
> could split the scsi maintainer role the same way that Dave and Jeff
> do for net where Dave handles the core and Jeff handles the drivers.
> Or we can negotiate some other arrangement.

That would be great. Maybe my aic7xxx debloating patches
which were submitted four times already (IIRC)
will be looked at at last.
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] debloat aic7xxx and aic79xx drivers

2007-10-18 Thread Denys Vlasenko
Hi Andrew, James,

On Monday 15 October 2007 14:53, Gabriel C wrote:
> >> Compile tested and applies cleanly to 2.6.23.
> >> I don't have this hardware anymore and cannot run test these patches.
> > 
> > I can test these patches on an aic7892 controller later on today if you 
> > want.
> 
> Works fine for me tested on :
> 
> 03:0e.0 SCSI storage controller [0100]: Adaptec AIC-7892P U160/m [9005:008f] 
> (rev 02)

I hope this is enough for these patches to be accepted.

If it is not, please let me know what do I need to do more.
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] debloat aic7xxx and aic79xx drivers

2007-10-15 Thread Denys Vlasenko
On Sunday 14 October 2007 18:47, Gabriel C wrote:
> > Compile tested and applies cleanly to 2.6.23.
> > I don't have this hardware anymore and cannot run test these patches.
> 
> I can test these patches on an aic7892 controller later on today if you want.

I'd appreciate that.

Do you, by any chance, use aic94xx driver too (drivers/scsi/aic94xx/*)?
After i'm done with aic7xxx, I may attack this one.

> BTW while you seems to care about this driver could you have a look at :
> 
> http://bugzilla.kernel.org/show_bug.cgi?id=3062 ?!?

I am a desktop Linux user, so far I don't use suspend at all. Sorry.
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] debloat aic7xxx and aic79xx drivers

2007-10-14 Thread Denys Vlasenko
Adds more consts

Signed-off-by: Denys Vlasenko <[EMAIL PROTECTED]>
--
vda

diff -urpN linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx.h linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx.h
--- linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx.h	2007-10-14 15:05:07.0 +0100
+++ linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx.h	2007-10-14 15:21:55.0 +0100
@@ -813,9 +813,9 @@ struct ahd_tmode_tstate {
  * to parity errors in each phase table. 
  */
 struct ahd_phase_table_entry {
-uint8_t phase;
-uint8_t mesg_out; /* Message response to parity errors */
-	char *phasemsg;
+	uint8_t phase;
+	uint8_t mesg_out; /* Message response to parity errors */
+	const char *phasemsg;
 };
 
 /** Serial EEPROM Format **/
@@ -1328,9 +1328,9 @@ extern const int ahd_num_aic7770_devs;
 /**/
 
 /* PCI Front End */
-struct	ahd_pci_identity *ahd_find_pci_device(ahd_dev_softc_t);
+const struct	ahd_pci_identity *ahd_find_pci_device(ahd_dev_softc_t);
 int			  ahd_pci_config(struct ahd_softc *,
-	 struct ahd_pci_identity *);
+	 const struct ahd_pci_identity *);
 int	ahd_pci_test_register_access(struct ahd_softc *);
 
 /** SCB and SCB queue management **/
diff -urpN linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx_core.c linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx_core.c
--- linux-2.6.23-aic-2-addstatic/drivers/scsi/aic7xxx/aic79xx_core.c	2007-10-14 15:05:07.0 +0100
+++ linux-2.6.23-aic-3-addconst/drivers/scsi/aic7xxx/aic79xx_core.c	2007-10-14 15:21:38.0 +0100
@@ -52,7 +52,7 @@
 
 
 /* Lookup Tables **/
-static char *ahd_chip_names[] =
+static const char *const ahd_chip_names[] =
 {
 	"NONE",
 	"aic7901",
@@ -65,11 +65,11 @@ static const u_int num_chip_names = ARRA
  * Hardware error codes.
  */
 struct ahd_hard_error_entry {
-uint8_t errno;
-	char *errmesg;
+	uint8_t errno;
+	const char *errmesg;
 };
 
-static struct ahd_hard_error_entry ahd_hard_errors[] = {
+static const struct ahd_hard_error_entry ahd_hard_errors[] = {
 	{ DSCTMOUT,	"Discard Timer has timed out" },
 	{ ILLOPCODE,	"Illegal Opcode in sequencer program" },
 	{ SQPARERR,	"Sequencer Parity Error" },
@@ -79,7 +79,7 @@ static struct ahd_hard_error_entry ahd_h
 };
 static const u_int num_errors = ARRAY_SIZE(ahd_hard_errors);
 
-static struct ahd_phase_table_entry ahd_phase_table[] =
+static const struct ahd_phase_table_entry ahd_phase_table[] =
 {
 	{ P_DATAOUT,	MSG_NOOP,		"in Data-out phase"	},
 	{ P_DATAIN,	MSG_INITIATOR_DET_ERR,	"in Data-in phase"	},
@@ -213,7 +213,7 @@ static void		ahd_dumpseq(struct ahd_soft
 #endif
 static void		ahd_loadseq(struct ahd_softc *ahd);
 static int		ahd_check_patch(struct ahd_softc *ahd,
-	struct patch **start_patch,
+	const struct patch **start_patch,
 	u_int start_instr, u_int *skip_addr);
 static u_int		ahd_resolve_seqaddr(struct ahd_softc *ahd,
 	u_int address);
@@ -254,7 +254,7 @@ static void		ahd_freeze_devq(struct ahd_
 	struct scb *scb);
 static void		ahd_handle_scb_status(struct ahd_softc *ahd,
 	  struct scb *scb);
-static struct ahd_phase_table_entry* ahd_lookup_phase_entry(int phase);
+static const struct ahd_phase_table_entry* ahd_lookup_phase_entry(int phase);
 static void		ahd_shutdown(void *arg);
 static void		ahd_update_coalescing_values(struct ahd_softc *ahd,
 		 u_int timer,
@@ -4337,11 +4337,11 @@ ahd_print_devinfo(struct ahd_softc *ahd,
 	   devinfo->target, devinfo->lun);
 }
 
-static struct ahd_phase_table_entry*
+static const struct ahd_phase_table_entry*
 ahd_lookup_phase_entry(int phase)
 {
-	struct ahd_phase_table_entry *entry;
-	struct ahd_phase_table_entry *last_entry;
+	const struct ahd_phase_table_entry *entry;
+	const struct ahd_phase_table_entry *last_entry;
 
 	/*
 	 * num_phases doesn't include the default entry which
@@ -9358,7 +9358,7 @@ ahd_loadseq(struct ahd_softc *ahd)
 	struct	cs cs_table[num_critical_sections];
 	u_int	begin_set[num_critical_sections];
 	u_int	end_set[num_critical_sections];
-	struct	patch *cur_patch;
+	const struct patch *cur_patch;
 	u_int	cs_count;
 	u_int	cur_cs;
 	u_int	i;
@@ -9513,11 +9513,11 @@ ahd_loadseq(struct ahd_softc *ahd)
 }
 
 static int
-ahd_check_patch(struct ahd_softc *ahd, struct patch **start_patch,
+ahd_check_patch(struct ahd_softc *ahd, const struct patch **start_patch,
 		u_int start_instr, u_int *skip_addr)
 {
-	struct	patch *cur_patch;
-	struct	patch *last_patch;
+	const struct patch *cur_patch;
+	const struct patch *last_patch;
 	u_int	num_patches;
 
 	num_patches = ARRAY_SIZE(patches);
@@ -9551,7 +9551,7 @@ ahd_check_

[PATCH 0/3] debloat aic7xxx and aic79xx drivers

2007-10-14 Thread Denys Vlasenko
Hi,

Following patches debloat drivers/scsi/aic7xxx/*.
I also had to add prototypes for ahc_lookup_scb
and ahd_lookup_scb to .h files.

1-debloat.patch
Deinlines and moves big functions from .h to .c files.
Adds prototypes for ahc_lookup_scb and ahd_lookup_scb to .h files.

2-addstatic.patch
Adds statics, #ifdefs out huge amount of unused code, adds consts

3-addconst.patch
Adds more consts

Driver code/data size reductions:

Build with debugging on (CONFIG_AIC7XXX_DEBUG_ENABLE=y):
   textdata bss dec hex filename
 310865   499221204  361991   58607 
linux-2.6.23.t/drivers/scsi/aic7xxx/built-in.o
 22198727541204  225945   37299 
linux-2.6.23-aic-3-addconst.t/drivers/scsi/aic7xxx/built-in.o

With debugging off:
   textdata bss dec hex filename
 298896   427541172  342822   53b26 
linux-2.6.23.tt/drivers/scsi/aic7xxx/built-in.o
 21606827541172  219994   35b5a 
linux-2.6.23-aic-3-addconst.tt/drivers/scsi/aic7xxx/built-in.o

make namespacecheck goes from 400+ functions to:
  drivers/scsi/aic7xxx/aic79xx_core.o
ahd_inq
ahd_inw
ahd_outq
ahd_outw
  drivers/scsi/aic7xxx/aic79xx_osm.o
ahd_insb
  drivers/scsi/aic7xxx/aic7xxx_core.o
ahc_inq
ahc_outq
  drivers/scsi/aic7xxx/aic7xxx_osm.o
ahc_insb

None of these patches touch any logic, code changes are pretty minimal.

Compile tested and applies cleanly to 2.6.23.
I don't have this hardware anymore and cannot run test these patches.

Please apply.

Signed-off-by: Denys Vlasenko <[EMAIL PROTECTED]>
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/3] debloat aic7xxx and aic79xx drivers

2007-08-31 Thread Denys Vlasenko
On Friday 31 August 2007 17:27, [EMAIL PROTECTED] wrote:
> On Fri, 31 Aug 2007 16:13:59 BST, Denys Vlasenko said:
> >
> >textdata bss dec hex filename
> >  261433   500181172  312623   4c52f 
> > linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o
> >  199654   500181172  250844   3d3dc 
> > linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o
> >  184014   213141172  206500   326a4 
> > linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o
> >  20237828501172  206400   32640 
> > linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o
> > 
> > 1-debloat.patchdeinlines a lot of functions
> > 2-addstatic.patch  adds statics, #ifdefs out huge amount of unused code, 
> > adds consts
> > 3-addconst.patch   adds more consts
> 
> Yowza.  Looking at aic1->aic2, it looks like 20K became 'const', and only
> 3K *wasn't* 'const'?  

Exactly. There are firmware images/patches and a lot of structures
with char* members pointing to text/messages.

Btw, aic94xx driver needs the same treatment.
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH 3/3] debloat aic7xxx and aic79xx drivers

2007-08-31 Thread Denys Vlasenko
On Friday 31 August 2007 16:16, Denys Vlasenko wrote:
> On Friday 31 August 2007 16:15, Denys Vlasenko wrote:
> > On Friday 31 August 2007 16:13, Denys Vlasenko wrote:
> > > Attached are three patches which fix that:
> > > 
> > >textdata bss dec hex filename
> > >  261433   500181172  312623   4c52f 
> > > linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o
> > >  199654   500181172  250844   3d3dc 
> > > linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o
> > >  184014   213141172  206500   326a4 
> > > linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o
> > >  20237828501172  206400   32640 
> > > linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o
> > > 
> > > 1-debloat.patchdeinlines a lot of functions
> > > 2-addstatic.patch  adds statics, #ifdefs out huge amount of unused code, 
> > > adds consts
> > > 3-addconst.patch   adds more consts
--
vda


linux-2.6.23-rc1-aic7xxx-3-addconst.patch.bz2
Description: BZip2 compressed data


[PATCH 2/3] debloat aic7xxx and aic79xx drivers

2007-08-31 Thread Denys Vlasenko
On Friday 31 August 2007 16:15, Denys Vlasenko wrote:
> On Friday 31 August 2007 16:13, Denys Vlasenko wrote:
> > Attached are three patches which fix that:
> > 
> >textdata bss dec hex filename
> >  261433   500181172  312623   4c52f 
> > linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o
> >  199654   500181172  250844   3d3dc 
> > linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o
> >  184014   213141172  206500   326a4 
> > linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o
> >  20237828501172  206400   32640 
> > linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o
> > 
> > 1-debloat.patchdeinlines a lot of functions
> > 2-addstatic.patch  adds statics, #ifdefs out huge amount of unused code, 
> > adds consts
> > 3-addconst.patch   adds more consts
--
vda



linux-2.6.23-rc1-aic7xxx-2-addstatic.patch.bz2
Description: BZip2 compressed data


[PATCH 1/3] debloat aic7xxx and aic79xx drivers

2007-08-31 Thread Denys Vlasenko
On Friday 31 August 2007 16:13, Denys Vlasenko wrote:
> Attached are three patches which fix that:
> 
>textdata bss dec hex filename
>  261433   500181172  312623   4c52f 
> linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o
>  199654   500181172  250844   3d3dc 
> linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o
>  184014   213141172  206500   326a4 
> linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o
>  20237828501172  206400   32640 
> linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o
> 
> 1-debloat.patchdeinlines a lot of functions
> 2-addstatic.patch  adds statics, #ifdefs out huge amount of unused code, adds 
> consts
> 3-addconst.patch   adds more consts
--
vda




linux-2.6.23-rc1-aic7xxx-1-debloat.patch.bz2
Description: BZip2 compressed data


[PATCH 0/3] debloat aic7xxx and aic79xx drivers

2007-08-31 Thread Denys Vlasenko
On Friday 31 August 2007 14:42, Hannes Reinecke wrote:
> Jan Engelhardt wrote:
> > On Aug 30 2007 13:02, Matthew Wilcox wrote:
> >>> Well, you can send it to Linus/Andrew, that will usually upset people
> >>> and they start commenting on it. Or they don't, and everything is fine.
> >>> (The "default y" approach so to speak ;-)
> >>
> >> The problem is that we don't really have a maintainer for the aic7xyz
> >> drivers any more.  Volunteers welcome.  NOT IT!
> >
> > Take it before someone else does!
>
> Well, the semi-official maintainers are James B. and me.
>
> So I might as well do it officially.

Cool.

Thanks to Arjan's insistence, I also took a look at adding statics
and unexpectedly discovered yet another 50 kbytes of dead code
(I'm not kidding).

Attached are three patches which fix that:

   textdata bss dec hex filename
 261433   500181172  312623   4c52f 
linux-2.6.23-rc1.org.t/drivers/scsi/aic7xxx/built-in.o
 199654   500181172  250844   3d3dc 
linux-2.6.23-rc1.aic.t/drivers/scsi/aic7xxx/built-in.o
 184014   213141172  206500   326a4 
linux-2.6.23-rc1.aic1.t/drivers/scsi/aic7xxx/built-in.o
 20237828501172  206400   32640 
linux-2.6.23-rc1.aic2.t/drivers/scsi/aic7xxx/built-in.o

1-debloat.patchdeinlines a lot of functions
2-addstatic.patch  adds statics, #ifdefs out huge amount of unused code, adds 
consts
3-addconst.patch   adds more consts

make namespacecheck goes from 400+ functions to:
  drivers/scsi/aic7xxx/aic79xx_core.o
ahd_inq
ahd_inw
ahd_outq
ahd_outw
  drivers/scsi/aic7xxx/aic79xx_osm.o
ahd_insb
  drivers/scsi/aic7xxx/aic7xxx_core.o
ahc_inq
ahc_outq
  drivers/scsi/aic7xxx/aic7xxx_osm.o
ahc_insb

None of these patches actually touch any logic, code changes
are pretty minimal.

Compile tested and applies cleanly to 2.6.23-rc1, applies with some fuzz to 
2.6.23-rc3.

Please propagate to mainline.
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] debloat aic7xxx and aic79xx drivers by deinlining

2007-08-28 Thread Denys Vlasenko
On Tuesday 28 August 2007 16:17, Arjan van de Ven wrote:
> On Tue, 28 Aug 2007 12:56:34 +010
>
> Denys Vlasenko <[EMAIL PROTECTED]> wrote:
> > >   make namespacecheck
> >
> > Thanks, nice tool.
> >
> > aic7xxx is kind of not very nice in this regard.
> >
> > See below what I get even on non-patched driver.
> >
> > I am willing to clean it up, but I still would like
> > "debloating" patch to be accepted.
>
> Fwiw I do like your debloat patch a lot; it's just only half the
> equation ... if you also do the namespace fixes, I bet the driver
> debloats even more...

Yes, I know, and I am happy to do that too. I just don't know
whether patches will be accepted.

Why this patch is not commented on by scsi people?
Am I sending patches to wrong people/lists?
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH] debloat aic7xxx and aic79xx drivers by deinlining

2007-08-28 Thread Denys Vlasenko
On Monday 27 August 2007 03:03, Adrian Bunk wrote:
> On Sun, Aug 26, 2007 at 04:21:30PM +0100, Denys Vlasenko wrote:
> > On Saturday 25 August 2007 22:57, Arjan van de Ven wrote:
> >...
> >
> > > Did you run the find static
> > > script or are you waiting for Adrian to do that ;-)
> >
> > $ find -name '*find*static*'
> > $
>
>   make namespacecheck

Thanks, nice tool.

aic7xxx is kind of not very nice in this regard.

See below what I get even on non-patched driver.

I am willing to clean it up, but I still would like
"debloating" patch to be accepted.

Otherwise I'm left in the dark whether _any_ patches
touching aic7xxx are ever looked at, or not.

Okay, the list:

Externally defined symbols with no external references

  drivers/scsi/aic7xxx/aic79xx_reg_print.o
ahd_abrtbitptr_print
ahd_abrtbyteptr_print
ahd_accum_print
ahd_accum_save_print
ahd_ahd_pci_config_base_print
ahd_allocfifo_scbptr_print
ahd_allones_print
ahd_allzeros_print
ahd_annexcol_print
ahd_annexdat_print
ahd_arbctl_print
ahd_arg_1_print
ahd_arg_2_print
ahd_attrptr_print
ahd_brdctl_print
ahd_brddat_print
ahd_brkaddr0_print
ahd_brkaddr1_print
ahd_businitid_print
ahd_bustargid_print
ahd_ccscbacnt_print
ahd_ccscbaddr_print
ahd_ccscbadr_bk_print
ahd_ccscbram_print
ahd_ccsgaddr_print
ahd_ccsgram_print
ahd_cdblimit_print
ahd_clrerr_print
ahd_clrint_print
ahd_clrlqiint0_print
ahd_clrlqiint1_print
ahd_clrlqoint0_print
ahd_clrlqoint1_print
ahd_clrseqintsrc_print
ahd_clrseqintstat_print
ahd_clrsint0_print
ahd_clrsint1_print
ahd_clrsint2_print
ahd_clrsint3_print
ahd_cmc_rambist_print
ahd_cmcpcistat_print
ahd_cmcrxmsg0_print
ahd_cmcrxmsg1_print
ahd_cmcrxmsg2_print
ahd_cmcrxmsg3_print
ahd_cmcseqbcnt_print
ahd_cmcspltstat0_print
ahd_cmcspltstat1_print
ahd_cmdlenptr_print
ahd_cmdptr_print
ahd_cmdrsvd0_print
ahd_cmds_pending_print
ahd_cmdsize_table_print
ahd_complete_dma_scb_head_print
ahd_complete_dma_scb_tail_print
ahd_complete_on_qfreeze_head_print
ahd_complete_scb_dmainprog_head_print
ahd_complete_scb_head_print
ahd_crccontrol_print
ahd_curaddr_print
ahd_currscb_print
ahd_data_count_odd_print
ahd_datalenptr_print
ahd_dchrxmsg0_print
ahd_dchrxmsg1_print
ahd_dchrxmsg2_print
ahd_dchrxmsg3_print
ahd_dchseqbcnt_print
ahd_dchspltstat0_print
ahd_dchspltstat1_print
ahd_df0pcistat_print
ahd_df1pcistat_print
ahd_dfbcnt_print
ahd_dfbkptr_print
ahd_dfdat_print
ahd_dfdbctl_print
ahd_dff_thrsh_print
ahd_dfftag_print
ahd_dfptrs_print
ahd_dfraddr_print
ahd_dfscnt_print
ahd_dfwaddr_print
ahd_dgrpcrci_print
ahd_dindex_print
ahd_dindir_print
ahd_dlcount_print
ahd_dmaparams_print
ahd_dscommand0_print
ahd_dspackctl_print
ahd_dspdatactl_print
ahd_dspfltrctl_print
ahd_dspreqctl_print
ahd_dspselect_print
ahd_error_print
ahd_fairness_print
ahd_flagptr_print
ahd_flags_print
ahd_flexadr_print
ahd_flexcnt_print
ahd_flexdata_print
ahd_flexdmastat_print
ahd_function1_print
ahd_gsfifo_print
ahd_haddr_print
ahd_hcnt_print
ahd_hcntrl_print
ahd_hescb_qoff_print
ahd_hnscb_qoff_print
ahd_hodmaadr_print
ahd_hodmacnt_print
ahd_hodmaen_print
ahd_idptr_print
ahd_initiator_tag_print
ahd_int_coalescing_cmdcount_print
ahd_int_coalescing_maxcmds_print
ahd_int_coalescing_mincmds_print
ahd_int_coalescing_timer_print
ahd_intvec1_addr_print
ahd_intvec2_addr_print
ahd_iopdnctl_print
ahd_iownid_print
ahd_kernel_tqinpos_print
ahd_last_msg_print
ahd_lastaddr_print
ahd_lastscb_print
ahd_local_hs_mailbox_print
ahd_longjmp_addr_print
ahd_lqctl0_print
ahd_lqctl1_print
ahd_lqctl2_print
ahd_lqimode0_print
ahd_lqimode1_print
ahd_lqin_print
ahd_lqistate_print
ahd_lqomode0_print
ahd_lqomode1_print
ahd_lqoscsctl_print
ahd_lqostate_print
ahd_lqrsvd01_print
ahd_lqrsvd16_print
ahd_lqrsvd17_print
ahd_lunlen_print
ahd_lunptr_print
ahd_maxcmd2rcv_print
ahd_maxcmd_print
ahd_maxcmdbytes_print
ahd_maxcmdcnt_print
ahd_mode_ptr_print
ahd_msg_out_print
ahd_msipcistat_print
ahd_multargid_print
ahd_negconopts_print
ahd_negoaddr_print
ahd_negoffset_print
ahd_negperiod_print
ahd_negppropts_print
ahd_next_queued_scb_addr_print
ahd_nextscb_print
ahd_none_print
ahd_nsenable_print
ahd_optionmode_print
ahd_os_space_cnt_print
ahd_ost_print
ahd_ovlyaddr_print
ahd_ovlypcistat_print
ahd_ovlyrxmsg0_print
ahd_ovlyrxmsg1_print
ahd_ovlyrxmsg2_print
ahd_ovlyrxmsg3_print
ahd_ovlyseqb

Re: [PATCH] debloat aic7xxx and aic79xx drivers by deinlining

2007-08-26 Thread Denys Vlasenko
On Saturday 25 August 2007 22:57, Arjan van de Ven wrote:
> On Sat, 25 Aug 2007 22:57:07 +0100
>
> Denys Vlasenko <[EMAIL PROTECTED]> wrote:
> > Hi,
> >
> > Attached patch deinlines and moves big functions from .h to .c files
> > in drivers/scsi/aic7xxx/*. I also had to add prototypes for
> > ahc_lookup_scb and ahd_lookup_scb to .h files.
>
> one question... how many of these can actually be static (or would be
> if they were in their only-caller-c-file) ?

Seeing this patch ignored two or three times already,
I tried to make as non-intrusive change as possible,
thus maximizing the chances that it will be applied.

Come on, this patch saves 60k-90k of text, or 30-40% of driver size.
This is ridiculous to have such enormous bloat.

In the functions I converted there are no non-static functions
which are not declared in a .h file.

There are indeed functions which are used only in one .c file:

ahc_check_cmdcmpltqueues: aic7xxx_core.c
ahc_get_sense_bufaddr: aic7xxx_core.c
ahc_hscb_busaddr: aic7xxx_core.c
ahc_inq: aic7xxx_core.c
ahc_outq: aic7xxx_core.c
ahc_pause_bug_fix: aic7xxx_core.c
ahc_sg_bus_to_virt: aic7xxx_core.c
ahc_sg_virt_to_bus: aic7xxx_core.c
ahc_swap_with_next_hscb: aic7xxx_core.c
ahc_sync_qoutfifo: aic7xxx_core.c
ahc_sync_scb: aic7xxx_core.c
ahc_sync_tqinfifo: aic7xxx_core.c
ahc_targetcmd_offset: aic7xxx_core.c
ahc_update_residual: aic7xxx_core.c
ahd_assert_modes: aic79xx_core.c
ahd_check_cmdcmpltqueues: aic79xx_core.c
ahd_get_hescb_qoff: aic79xx_core.c
ahd_get_hnscb_qoff: aic79xx_core.c
ahd_get_sdscb_qoff: aic79xx_core.c
ahd_get_sescb_qoff: aic79xx_core.c
ahd_get_snscb_qoff: aic79xx_core.c
ahd_inl_scbram: aic79xx_core.c
ahd_inq: aic79xx_core.c
ahd_inq_scbram: aic79xx_core.c
ahd_outq: aic79xx_core.c
ahd_set_hescb_qoff: aic79xx_core.c
ahd_set_hnscb_qoff: aic79xx_core.c
ahd_set_sdscb_qoff: aic79xx_core.c
ahd_set_sescb_qoff: aic79xx_core.c
ahd_set_snscb_qoff: aic79xx_core.c
ahd_setup_data_scb: aic79xx_core.c
ahd_setup_noxfer_scb: aic79xx_core.c
ahd_setup_scb_common: aic79xx_core.c
ahd_sg_bus_to_virt: aic79xx_core.c
ahd_sg_virt_to_bus: aic79xx_core.c
ahd_swap_with_next_hscb: aic79xx_core.c
ahd_sync_qoutfifo: aic79xx_core.c
ahd_sync_scb: aic79xx_core.c
ahd_sync_sense: aic79xx_core.c
ahd_sync_tqinfifo: aic79xx_core.c
ahd_targetcmd_offset: aic79xx_core.c
ahd_update_modes: aic79xx_core.c

Converting them to static may save additional 3 or 4k,
if gcc will be smart enough...

I, too, have many questions regarding this driver:
why __inline? why u_int?... and who's maintainer, btw?

> Did you run the find static
> script or are you waiting for Adrian to do that ;-)

$ find -name '*find*static*'
$
--
vda
-
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html