twa generates WARNING upon boot

twa generates WARNING upon boot

["Tóth Attila"]

After an unsuccessful attempt to contact linuxraid@lsi.com, I'm trying to
seek assistance on this list.

I've been seeing WARNINGs upon boot for a while now, without any obvious
symptoms. I got some advice to report it upstream on the hardened-gentoo
mailing list from PaX Team:

"twa_interrupt is from the 3ware 9xxx driver and it seems that it wants to
unmap a page it doesn't own. DEBUG_INFO and addr2line would help to
identify the bad call in twa_interrupt (ffffffffxxxxxxxx in the below
trace) then you can send it upstream ;)."
Where xxxxxxxx was an address taken from a previous trace.

I've recompiled the kernel with DEBUG_INFO and FRAME_POINTERS enabled.

Here is a current trace I see after booting that kernel:
------------[ cut here ]------------
WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:3214
intel_unmap+0x186/0x1f0()
Driver unmaps unmatched page at PFN 0
Modules linked in:
CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.7-hardened-r1 #2
Hardware name: System manufacturer System Product Name/Z8P(N)E-D12(X),
BIOS 1302    06/25/2012
 ffffffffab40bd6b 0000000000000000 0000000000000000 ffffffffab21608f
 ffff880237c03ca8 ffffffffa4ed0fa6 00000000000015d2 ffff880237c03d00
 ffff880237c03ce8 ffffffffa40ad9a0 0000000000000000 ffffffffab21608f
Call Trace:
 <IRQ>  [<ffffffffa4ed0fa6>] dump_stack+0x45/0x5d
 [<ffffffffa40ad9a0>] warn_slowpath_common+0x80/0xc0
 [<ffffffffa40ada44>] warn_slowpath_fmt+0x64/0x90
 [<ffffffffa46de866>] intel_unmap+0x186/0x1f0
 [<ffffffffa46de8ea>] intel_unmap_sg+0x1a/0x30
 [<ffffffffa475ef13>] scsi_dma_unmap+0x73/0x90
 [<ffffffffa47b8683>] twa_interrupt+0x493/0x780
 [<ffffffffa4100f7a>] handle_irq_event_percpu+0x7a/0x130
 [<ffffffffa4101069>] handle_irq_event+0x39/0x60
 [<ffffffffa4104469>] handle_fasteoi_irq+0x89/0x1a0
 [<ffffffffa4005715>] handle_irq+0x85/0x160
 [<ffffffffa4004f6c>] do_IRQ+0x4c/0x100
 [<ffffffffa4edd557>] common_interrupt+0x97/0x97
 <EOI>  [<ffffffffa403d1cc>] ?
default_send_IPI_mask_allbutself_phys+0xbc/0x100
 [<ffffffffa4042e79>] physflat_send_IPI_allbutself+0x19/0x30
 [<ffffffffa40385d8>] native_send_call_func_ipi+0x108/0x140
 [<ffffffffa4125c10>] ? proc_dma_show+0x70/0x70
 [<ffffffffa41264f4>] smp_call_function_many+0x1c4/0x270
 [<ffffffffa4126671>] kick_all_cpus_sync+0x21/0x30
 [<ffffffffa41bb546>] __do_tune_cpucache+0x56/0x4d0
 [<ffffffffa458f4b7>] ? string.isra.3+0x47/0x100
 [<ffffffffa41bb9f7>] do_tune_cpucache+0x37/0xb0
 [<ffffffffa41bbad5>] enable_cpucache+0x65/0x130
 [<ffffffffa4ec5c13>] setup_cpu_cache+0x173/0x270
 [<ffffffffa41bc2c2>] __kmem_cache_create+0x262/0x360
 [<ffffffffa418a5c2>] do_kmem_cache_create+0x92/0x1d0
 [<ffffffffa418a81e>] kmem_cache_create+0x11e/0x1d0
 [<ffffffffafc4c3d0>] ? twa_init+0x36/0x36
 [<ffffffffafc4c4a7>] init_sd+0xd7/0x198
 [<ffffffffa4000384>] do_one_initcall+0x94/0x1a0
 [<ffffffffafc15285>] kernel_init_freeable+0x183/0x22f
 [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
 [<ffffffffa4ec44f9>] kernel_init+0x9/0xf0
 [<ffffffffa4edcdfe>] ret_from_fork+0x3e/0x70
 [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
---[ end trace a39a5826ea41aa47 ]---

The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X motherboard.

I don't know the exact command I have to type to get a meaningful output
for addr2line...

Please let me know what else I should do in order to provide more info.

Thanks:
Dw.
-- 
dr Tóth Attila, Radiológus, 06-20-825-8057
Attila Toth MD, Radiologist, +36-20-825-8057

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

Re: twa generates WARNING upon boot

[adam radford]

On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu> wrote:
> After an unsuccessful attempt to contact linuxraid@lsi.com, I'm trying to
> seek assistance on this list.
>
> I've been seeing WARNINGs upon boot for a while now, without any obvious
> symptoms. I got some advice to report it upstream on the hardened-gentoo
> mailing list from PaX Team:
>
> "twa_interrupt is from the 3ware 9xxx driver and it seems that it wants to
> unmap a page it doesn't own. DEBUG_INFO and addr2line would help to
> identify the bad call in twa_interrupt (ffffffffxxxxxxxx in the below
> trace) then you can send it upstream ;)."
> Where xxxxxxxx was an address taken from a previous trace.
>
> I've recompiled the kernel with DEBUG_INFO and FRAME_POINTERS enabled.
>
> Here is a current trace I see after booting that kernel:
> ------------[ cut here ]------------
> WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:3214
> intel_unmap+0x186/0x1f0()
> Driver unmaps unmatched page at PFN 0
> Modules linked in:
> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.7-hardened-r1 #2
> Hardware name: System manufacturer System Product Name/Z8P(N)E-D12(X),
> BIOS 1302    06/25/2012
>  ffffffffab40bd6b 0000000000000000 0000000000000000 ffffffffab21608f
>  ffff880237c03ca8 ffffffffa4ed0fa6 00000000000015d2 ffff880237c03d00
>  ffff880237c03ce8 ffffffffa40ad9a0 0000000000000000 ffffffffab21608f
> Call Trace:
>  <IRQ>  [<ffffffffa4ed0fa6>] dump_stack+0x45/0x5d
>  [<ffffffffa40ad9a0>] warn_slowpath_common+0x80/0xc0
>  [<ffffffffa40ada44>] warn_slowpath_fmt+0x64/0x90
>  [<ffffffffa46de866>] intel_unmap+0x186/0x1f0
>  [<ffffffffa46de8ea>] intel_unmap_sg+0x1a/0x30
>  [<ffffffffa475ef13>] scsi_dma_unmap+0x73/0x90
>  [<ffffffffa47b8683>] twa_interrupt+0x493/0x780
>  [<ffffffffa4100f7a>] handle_irq_event_percpu+0x7a/0x130
>  [<ffffffffa4101069>] handle_irq_event+0x39/0x60
>  [<ffffffffa4104469>] handle_fasteoi_irq+0x89/0x1a0
>  [<ffffffffa4005715>] handle_irq+0x85/0x160
>  [<ffffffffa4004f6c>] do_IRQ+0x4c/0x100
>  [<ffffffffa4edd557>] common_interrupt+0x97/0x97
>  <EOI>  [<ffffffffa403d1cc>] ?
> default_send_IPI_mask_allbutself_phys+0xbc/0x100
>  [<ffffffffa4042e79>] physflat_send_IPI_allbutself+0x19/0x30
>  [<ffffffffa40385d8>] native_send_call_func_ipi+0x108/0x140
>  [<ffffffffa4125c10>] ? proc_dma_show+0x70/0x70
>  [<ffffffffa41264f4>] smp_call_function_many+0x1c4/0x270
>  [<ffffffffa4126671>] kick_all_cpus_sync+0x21/0x30
>  [<ffffffffa41bb546>] __do_tune_cpucache+0x56/0x4d0
>  [<ffffffffa458f4b7>] ? string.isra.3+0x47/0x100
>  [<ffffffffa41bb9f7>] do_tune_cpucache+0x37/0xb0
>  [<ffffffffa41bbad5>] enable_cpucache+0x65/0x130
>  [<ffffffffa4ec5c13>] setup_cpu_cache+0x173/0x270
>  [<ffffffffa41bc2c2>] __kmem_cache_create+0x262/0x360
>  [<ffffffffa418a5c2>] do_kmem_cache_create+0x92/0x1d0
>  [<ffffffffa418a81e>] kmem_cache_create+0x11e/0x1d0
>  [<ffffffffafc4c3d0>] ? twa_init+0x36/0x36
>  [<ffffffffafc4c4a7>] init_sd+0xd7/0x198
>  [<ffffffffa4000384>] do_one_initcall+0x94/0x1a0
>  [<ffffffffafc15285>] kernel_init_freeable+0x183/0x22f
>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
>  [<ffffffffa4ec44f9>] kernel_init+0x9/0xf0
>  [<ffffffffa4edcdfe>] ret_from_fork+0x3e/0x70
>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
> ---[ end trace a39a5826ea41aa47 ]---
>
> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X motherboard.
>
> I don't know the exact command I have to type to get a meaningful output
> for addr2line...
>
> Please let me know what else I should do in order to provide more info.

Can you re-try with Christoph's patch:

https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de

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

Re: twa generates WARNING upon boot

["Tóth Attila"]

2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
> On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu> wrote:
>> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X motherboard.
>>
>> I don't know the exact command I have to type to get a meaningful
output for addr2line...
>>
>> Please let me know what else I should do in order to provide more info.
>
> Can you re-try with Christoph's patch:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de
>
> -Adam

Dear Adam,

Christoph's patch has been already applied on the kernel in use.
Should I try reverting it?
What else I should try?

Thanks for the help: Dw.
-- 
dr Tóth Attila, Radiológus, 06-20-825-8057
Attila Toth MD, Radiologist, +36-20-825-8057

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

Re: twa generates WARNING upon boot

["Tóth Attila"]

2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
> On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu>
> wrote:
>> Here is a current trace I see after booting that kernel:
>> ------------[ cut here ]------------
>> WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:3214
>> intel_unmap+0x186/0x1f0()
>> Driver unmaps unmatched page at PFN 0
>> Modules linked in:
>> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.7-hardened-r1 #2
>> Hardware name: System manufacturer System Product Name/Z8P(N)E-D12(X),
>> BIOS 1302    06/25/2012
>>  ffffffffab40bd6b 0000000000000000 0000000000000000 ffffffffab21608f
>>  ffff880237c03ca8 ffffffffa4ed0fa6 00000000000015d2 ffff880237c03d00
>>  ffff880237c03ce8 ffffffffa40ad9a0 0000000000000000 ffffffffab21608f
>> Call Trace:
>>  <IRQ>  [<ffffffffa4ed0fa6>] dump_stack+0x45/0x5d
>>  [<ffffffffa40ad9a0>] warn_slowpath_common+0x80/0xc0
>>  [<ffffffffa40ada44>] warn_slowpath_fmt+0x64/0x90
>>  [<ffffffffa46de866>] intel_unmap+0x186/0x1f0
>>  [<ffffffffa46de8ea>] intel_unmap_sg+0x1a/0x30
>>  [<ffffffffa475ef13>] scsi_dma_unmap+0x73/0x90
>>  [<ffffffffa47b8683>] twa_interrupt+0x493/0x780
>>  [<ffffffffa4100f7a>] handle_irq_event_percpu+0x7a/0x130
>>  [<ffffffffa4101069>] handle_irq_event+0x39/0x60
>>  [<ffffffffa4104469>] handle_fasteoi_irq+0x89/0x1a0
>>  [<ffffffffa4005715>] handle_irq+0x85/0x160
>>  [<ffffffffa4004f6c>] do_IRQ+0x4c/0x100
>>  [<ffffffffa4edd557>] common_interrupt+0x97/0x97
>>  <EOI>  [<ffffffffa403d1cc>] ?
>> default_send_IPI_mask_allbutself_phys+0xbc/0x100
>>  [<ffffffffa4042e79>] physflat_send_IPI_allbutself+0x19/0x30
>>  [<ffffffffa40385d8>] native_send_call_func_ipi+0x108/0x140
>>  [<ffffffffa4125c10>] ? proc_dma_show+0x70/0x70
>>  [<ffffffffa41264f4>] smp_call_function_many+0x1c4/0x270
>>  [<ffffffffa4126671>] kick_all_cpus_sync+0x21/0x30
>>  [<ffffffffa41bb546>] __do_tune_cpucache+0x56/0x4d0
>>  [<ffffffffa458f4b7>] ? string.isra.3+0x47/0x100
>>  [<ffffffffa41bb9f7>] do_tune_cpucache+0x37/0xb0
>>  [<ffffffffa41bbad5>] enable_cpucache+0x65/0x130
>>  [<ffffffffa4ec5c13>] setup_cpu_cache+0x173/0x270
>>  [<ffffffffa41bc2c2>] __kmem_cache_create+0x262/0x360
>>  [<ffffffffa418a5c2>] do_kmem_cache_create+0x92/0x1d0
>>  [<ffffffffa418a81e>] kmem_cache_create+0x11e/0x1d0
>>  [<ffffffffafc4c3d0>] ? twa_init+0x36/0x36
>>  [<ffffffffafc4c4a7>] init_sd+0xd7/0x198
>>  [<ffffffffa4000384>] do_one_initcall+0x94/0x1a0
>>  [<ffffffffafc15285>] kernel_init_freeable+0x183/0x22f
>>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
>>  [<ffffffffa4ec44f9>] kernel_init+0x9/0xf0
>>  [<ffffffffa4edcdfe>] ret_from_fork+0x3e/0x70
>>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
>> ---[ end trace a39a5826ea41aa47 ]---
>>
>> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X motherboard.
>>
>
> Can you re-try with Christoph's patch:
>
> https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de

As I've told this patch has been already included in the kernel I'm using
(4.1.7-hardened-r1, which is based on 4.1.7).
Out of curiosity I've reverted the patch and the WARNING no longer appears
during boot...

Thx: Dw.
-- 
dr Tóth Attila, Radiológus, 06-20-825-8057
Attila Toth MD, Radiologist, +36-20-825-8057



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


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

Re: twa generates WARNING upon boot

[James Bottomley]

On Tue, 2015-09-29 at 18:49 +0200, "Tóth Attila" wrote:
> 2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
> > On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu>
> > wrote:
> >> Here is a current trace I see after booting that kernel:
> >> ------------[ cut here ]------------
> >> WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:3214
> >> intel_unmap+0x186/0x1f0()
> >> Driver unmaps unmatched page at PFN 0
> >> Modules linked in:
> >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.7-hardened-r1 #2
> >> Hardware name: System manufacturer System Product Name/Z8P(N)E-D12(X),
> >> BIOS 1302    06/25/2012
> >>  ffffffffab40bd6b 0000000000000000 0000000000000000 ffffffffab21608f
> >>  ffff880237c03ca8 ffffffffa4ed0fa6 00000000000015d2 ffff880237c03d00
> >>  ffff880237c03ce8 ffffffffa40ad9a0 0000000000000000 ffffffffab21608f
> >> Call Trace:
> >>  <IRQ>  [<ffffffffa4ed0fa6>] dump_stack+0x45/0x5d
> >>  [<ffffffffa40ad9a0>] warn_slowpath_common+0x80/0xc0
> >>  [<ffffffffa40ada44>] warn_slowpath_fmt+0x64/0x90
> >>  [<ffffffffa46de866>] intel_unmap+0x186/0x1f0
> >>  [<ffffffffa46de8ea>] intel_unmap_sg+0x1a/0x30
> >>  [<ffffffffa475ef13>] scsi_dma_unmap+0x73/0x90
> >>  [<ffffffffa47b8683>] twa_interrupt+0x493/0x780
> >>  [<ffffffffa4100f7a>] handle_irq_event_percpu+0x7a/0x130
> >>  [<ffffffffa4101069>] handle_irq_event+0x39/0x60
> >>  [<ffffffffa4104469>] handle_fasteoi_irq+0x89/0x1a0
> >>  [<ffffffffa4005715>] handle_irq+0x85/0x160
> >>  [<ffffffffa4004f6c>] do_IRQ+0x4c/0x100
> >>  [<ffffffffa4edd557>] common_interrupt+0x97/0x97
> >>  <EOI>  [<ffffffffa403d1cc>] ?
> >> default_send_IPI_mask_allbutself_phys+0xbc/0x100
> >>  [<ffffffffa4042e79>] physflat_send_IPI_allbutself+0x19/0x30
> >>  [<ffffffffa40385d8>] native_send_call_func_ipi+0x108/0x140
> >>  [<ffffffffa4125c10>] ? proc_dma_show+0x70/0x70
> >>  [<ffffffffa41264f4>] smp_call_function_many+0x1c4/0x270
> >>  [<ffffffffa4126671>] kick_all_cpus_sync+0x21/0x30
> >>  [<ffffffffa41bb546>] __do_tune_cpucache+0x56/0x4d0
> >>  [<ffffffffa458f4b7>] ? string.isra.3+0x47/0x100
> >>  [<ffffffffa41bb9f7>] do_tune_cpucache+0x37/0xb0
> >>  [<ffffffffa41bbad5>] enable_cpucache+0x65/0x130
> >>  [<ffffffffa4ec5c13>] setup_cpu_cache+0x173/0x270
> >>  [<ffffffffa41bc2c2>] __kmem_cache_create+0x262/0x360
> >>  [<ffffffffa418a5c2>] do_kmem_cache_create+0x92/0x1d0
> >>  [<ffffffffa418a81e>] kmem_cache_create+0x11e/0x1d0
> >>  [<ffffffffafc4c3d0>] ? twa_init+0x36/0x36
> >>  [<ffffffffafc4c4a7>] init_sd+0xd7/0x198
> >>  [<ffffffffa4000384>] do_one_initcall+0x94/0x1a0
> >>  [<ffffffffafc15285>] kernel_init_freeable+0x183/0x22f
> >>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
> >>  [<ffffffffa4ec44f9>] kernel_init+0x9/0xf0
> >>  [<ffffffffa4edcdfe>] ret_from_fork+0x3e/0x70
> >>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
> >> ---[ end trace a39a5826ea41aa47 ]---
> >>
> >> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X motherboard.
> >>
> >
> > Can you re-try with Christoph's patch:
> >
> > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de
> 
> As I've told this patch has been already included in the kernel I'm using
> (4.1.7-hardened-r1, which is based on 4.1.7).
> Out of curiosity I've reverted the patch and the WARNING no longer appears
> during boot...

Ah, it looks like there's a bug in the patch, it doesn't take into
account that the driver has a minimum sg map length and it copies for
things under that, so we're likely unmapping something that wasn't
mapped.   It's slightly hard to fix given that the indicator flag we'd
use for this is gone in that patch; does this fix the warning?

James

---

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index add419d..4c2c476 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -151,7 +151,19 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
 static char *twa_string_lookup(twa_message_type *table, unsigned int aen_code);
 
 /* Functions */
+static void twa_scsi_dma_unmap(struct scsi_cmnd *SCpnt, int request_id)
+{
+	TW_Device_Extension *tw_dev = (TW_Device_Extension *)SCpnt->device->host->hostdata;
+	TW_Command_Apache *command_packet = &tw_dev->command_packet_virt[request_id]->command.newcommand;
+
+	if (command_packet->sg_list[0].address == TW_CPU_TO_SGL(tw_dev->generic_buffer_phys[request_id]))
+		/* command is copied not mapped */
+		return;
+
+	twa_scsi_dma_unmap(SCpnt, request_id);
+}
 
+  
 /* Show some statistics about the card */
 static ssize_t twa_show_stats(struct device *dev,
 			      struct device_attribute *attr, char *buf)
@@ -1339,7 +1351,7 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
 				}
 
 				/* Now complete the io */
-				scsi_dma_unmap(cmd);
+				twa_scsi_dma_unmap(cmd, request_id);
 				cmd->scsi_done(cmd);
 				tw_dev->state[request_id] = TW_S_COMPLETED;
 				twa_free_request_id(tw_dev, request_id);
@@ -1582,7 +1594,7 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
 				struct scsi_cmnd *cmd = tw_dev->srb[i];
 
 				cmd->result = (DID_RESET << 16);
-				scsi_dma_unmap(cmd);
+				twa_scsi_dma_unmap(cmd, i);
 				cmd->scsi_done(cmd);
 			}
 		}
@@ -1765,12 +1777,12 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
 	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
 	switch (retval) {
 	case SCSI_MLQUEUE_HOST_BUSY:
-		scsi_dma_unmap(SCpnt);
+		twa_scsi_dma_unmap(SCpnt, request_id);
 		twa_free_request_id(tw_dev, request_id);
 		break;
 	case 1:
 		SCpnt->result = (DID_ERROR << 16);
-		scsi_dma_unmap(SCpnt);
+		twa_scsi_dma_unmap(SCpnt, request_id);
 		done(SCpnt);
 		tw_dev->state[request_id] = TW_S_COMPLETED;
 		twa_free_request_id(tw_dev, request_id);


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

Re: twa generates WARNING upon boot

[James Bottomley]

On Tue, 2015-09-29 at 10:37 -0700, James Bottomley wrote:
> On Tue, 2015-09-29 at 18:49 +0200, "Tóth Attila" wrote:
> > 2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
> > > On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu>
> > > wrote:
> > >> Here is a current trace I see after booting that kernel:
> > >> ------------[ cut here ]------------
> > >> WARNING: CPU: 0 PID: 1 at drivers/iommu/intel-iommu.c:3214
> > >> intel_unmap+0x186/0x1f0()
> > >> Driver unmaps unmatched page at PFN 0
> > >> Modules linked in:
> > >> CPU: 0 PID: 1 Comm: swapper/0 Not tainted 4.1.7-hardened-r1 #2
> > >> Hardware name: System manufacturer System Product Name/Z8P(N)E-D12(X),
> > >> BIOS 1302    06/25/2012
> > >>  ffffffffab40bd6b 0000000000000000 0000000000000000 ffffffffab21608f
> > >>  ffff880237c03ca8 ffffffffa4ed0fa6 00000000000015d2 ffff880237c03d00
> > >>  ffff880237c03ce8 ffffffffa40ad9a0 0000000000000000 ffffffffab21608f
> > >> Call Trace:
> > >>  <IRQ>  [<ffffffffa4ed0fa6>] dump_stack+0x45/0x5d
> > >>  [<ffffffffa40ad9a0>] warn_slowpath_common+0x80/0xc0
> > >>  [<ffffffffa40ada44>] warn_slowpath_fmt+0x64/0x90
> > >>  [<ffffffffa46de866>] intel_unmap+0x186/0x1f0
> > >>  [<ffffffffa46de8ea>] intel_unmap_sg+0x1a/0x30
> > >>  [<ffffffffa475ef13>] scsi_dma_unmap+0x73/0x90
> > >>  [<ffffffffa47b8683>] twa_interrupt+0x493/0x780
> > >>  [<ffffffffa4100f7a>] handle_irq_event_percpu+0x7a/0x130
> > >>  [<ffffffffa4101069>] handle_irq_event+0x39/0x60
> > >>  [<ffffffffa4104469>] handle_fasteoi_irq+0x89/0x1a0
> > >>  [<ffffffffa4005715>] handle_irq+0x85/0x160
> > >>  [<ffffffffa4004f6c>] do_IRQ+0x4c/0x100
> > >>  [<ffffffffa4edd557>] common_interrupt+0x97/0x97
> > >>  <EOI>  [<ffffffffa403d1cc>] ?
> > >> default_send_IPI_mask_allbutself_phys+0xbc/0x100
> > >>  [<ffffffffa4042e79>] physflat_send_IPI_allbutself+0x19/0x30
> > >>  [<ffffffffa40385d8>] native_send_call_func_ipi+0x108/0x140
> > >>  [<ffffffffa4125c10>] ? proc_dma_show+0x70/0x70
> > >>  [<ffffffffa41264f4>] smp_call_function_many+0x1c4/0x270
> > >>  [<ffffffffa4126671>] kick_all_cpus_sync+0x21/0x30
> > >>  [<ffffffffa41bb546>] __do_tune_cpucache+0x56/0x4d0
> > >>  [<ffffffffa458f4b7>] ? string.isra.3+0x47/0x100
> > >>  [<ffffffffa41bb9f7>] do_tune_cpucache+0x37/0xb0
> > >>  [<ffffffffa41bbad5>] enable_cpucache+0x65/0x130
> > >>  [<ffffffffa4ec5c13>] setup_cpu_cache+0x173/0x270
> > >>  [<ffffffffa41bc2c2>] __kmem_cache_create+0x262/0x360
> > >>  [<ffffffffa418a5c2>] do_kmem_cache_create+0x92/0x1d0
> > >>  [<ffffffffa418a81e>] kmem_cache_create+0x11e/0x1d0
> > >>  [<ffffffffafc4c3d0>] ? twa_init+0x36/0x36
> > >>  [<ffffffffafc4c4a7>] init_sd+0xd7/0x198
> > >>  [<ffffffffa4000384>] do_one_initcall+0x94/0x1a0
> > >>  [<ffffffffafc15285>] kernel_init_freeable+0x183/0x22f
> > >>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
> > >>  [<ffffffffa4ec44f9>] kernel_init+0x9/0xf0
> > >>  [<ffffffffa4edcdfe>] ret_from_fork+0x3e/0x70
> > >>  [<ffffffffa4ec44f0>] ? rest_init+0x80/0x80
> > >> ---[ end trace a39a5826ea41aa47 ]---
> > >>
> > >> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X motherboard.
> > >>
> > >
> > > Can you re-try with Christoph's patch:
> > >
> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de
> > 
> > As I've told this patch has been already included in the kernel I'm using
> > (4.1.7-hardened-r1, which is based on 4.1.7).
> > Out of curiosity I've reverted the patch and the WARNING no longer appears
> > during boot...
> 
> Ah, it looks like there's a bug in the patch, it doesn't take into
> account that the driver has a minimum sg map length and it copies for
> things under that, so we're likely unmapping something that wasn't
> mapped.   It's slightly hard to fix given that the indicator flag we'd
> use for this is gone in that patch; does this fix the warning?

Actually, strike that, it would still barf on internally generated
commands (and it's recursively calling twa_scsi_dma_unmap).  This does a
partial revert of the state check which should pick up all cases the
input didn't go through the mapping.

James

---

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index add419d..751ed66 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -151,7 +151,13 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
 static char *twa_string_lookup(twa_message_type *table, unsigned int aen_code);
 
 /* Functions */
+static void twa_scsi_dma_unmap(struct scsi_cmnd *SCpnt)
+{
+	if (SCpnt->SCp.phase == TW_PHASE_SGLIST)
+		scsi_dma_unmap(SCpnt);
+}
 
+  
 /* Show some statistics about the card */
 static ssize_t twa_show_stats(struct device *dev,
 			      struct device_attribute *attr, char *buf)
@@ -1339,7 +1345,7 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
 				}
 
 				/* Now complete the io */
-				scsi_dma_unmap(cmd);
+				twa_scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 				tw_dev->state[request_id] = TW_S_COMPLETED;
 				twa_free_request_id(tw_dev, request_id);
@@ -1582,7 +1588,7 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
 				struct scsi_cmnd *cmd = tw_dev->srb[i];
 
 				cmd->result = (DID_RESET << 16);
-				scsi_dma_unmap(cmd);
+				twa_scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 			}
 		}
@@ -1762,15 +1768,18 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
 	/* Save the scsi command for use by the ISR */
 	tw_dev->srb[request_id] = SCpnt;
 
+	/* Initialize phase to zero */
+	SCpnt->SCp.phase = TW_PHASE_INITIAL;
+
 	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
 	switch (retval) {
 	case SCSI_MLQUEUE_HOST_BUSY:
-		scsi_dma_unmap(SCpnt);
+		twa_scsi_dma_unmap(SCpnt);
 		twa_free_request_id(tw_dev, request_id);
 		break;
 	case 1:
 		SCpnt->result = (DID_ERROR << 16);
-		scsi_dma_unmap(SCpnt);
+		twa_scsi_dma_unmap(SCpnt);
 		done(SCpnt);
 		tw_dev->state[request_id] = TW_S_COMPLETED;
 		twa_free_request_id(tw_dev, request_id);
@@ -1845,6 +1854,8 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
 				if (sg_count < 0)
 					goto out;
 
+				srb->SCp.phase = TW_PHASE_SGLIST;
+
 				scsi_for_each_sg(srb, sg, sg_count, i) {
 					command_packet->sg_list[i].address = TW_CPU_TO_SGL(sg_dma_address(sg));
 					command_packet->sg_list[i].length = cpu_to_le32(sg_dma_len(sg));


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

Re: twa generates WARNING upon boot

["Tóth Attila"]

2015.Szeptember 29.(K) 20:02 időpontban James Bottomley ezt írta:
> On Tue, 2015-09-29 at 10:37 -0700, James Bottomley wrote:
>> On Tue, 2015-09-29 at 18:49 +0200, "Tóth Attila" wrote:
>> > 2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
>> > > On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu>
>> > > wrote:
>> > >> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X
>> > >>
>> > >
>> > > Can you re-try with Christoph's patch:
>> > >
>> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de
>> >
>> > As I've told this patch has been already included in the kernel I'm
>> using
>> > (4.1.7-hardened-r1, which is based on 4.1.7).
>> > Out of curiosity I've reverted the patch and the WARNING no longer
>> appears
>> > during boot...
>>
>> Ah, it looks like there's a bug in the patch, it doesn't take into
>> account that the driver has a minimum sg map length and it copies for
>> things under that, so we're likely unmapping something that wasn't
>> mapped.   It's slightly hard to fix given that the indicator flag we'd
>> use for this is gone in that patch; does this fix the warning?
>
> Actually, strike that, it would still barf on internally generated
> commands (and it's recursively calling twa_scsi_dma_unmap).  This does a
> partial revert of the state check which should pick up all cases the
> input didn't go through the mapping.

Just to be clear before I try something out.
In this latter patch you are resurrecting TW_PHASE_SGLIST, which has been
ment to be removed in Christoph's patch from the header.
I guess I have to reintroduce those phase defines on top of the changes
below in your latest email.
Or please let me know what should I exactly try out.

Thanks: Dw.
-- 
dr Tóth Attila, Radiológus, 06-20-825-8057
Attila Toth MD, Radiologist, +36-20-825-8057

>
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index add419d..751ed66 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -151,7 +151,13 @@ static void
> twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
>  static char *twa_string_lookup(twa_message_type *table, unsigned int
> aen_code);
>
>  /* Functions */
> +static void twa_scsi_dma_unmap(struct scsi_cmnd *SCpnt)
> +{
> +	if (SCpnt->SCp.phase == TW_PHASE_SGLIST)
> +		scsi_dma_unmap(SCpnt);
> +}
>
> +
>  /* Show some statistics about the card */
>  static ssize_t twa_show_stats(struct device *dev,
>  			      struct device_attribute *attr, char *buf)
> @@ -1339,7 +1345,7 @@ static irqreturn_t twa_interrupt(int irq, void
> *dev_instance)
>  				}
>
>  				/* Now complete the io */
> -				scsi_dma_unmap(cmd);
> +				twa_scsi_dma_unmap(cmd);
>  				cmd->scsi_done(cmd);
>  				tw_dev->state[request_id] = TW_S_COMPLETED;
>  				twa_free_request_id(tw_dev, request_id);
> @@ -1582,7 +1588,7 @@ static int
> twa_reset_device_extension(TW_Device_Extension *tw_dev)
>  				struct scsi_cmnd *cmd = tw_dev->srb[i];
>
>  				cmd->result = (DID_RESET << 16);
> -				scsi_dma_unmap(cmd);
> +				twa_scsi_dma_unmap(cmd);
>  				cmd->scsi_done(cmd);
>  			}
>  		}
> @@ -1762,15 +1768,18 @@ static int twa_scsi_queue_lck(struct scsi_cmnd
> *SCpnt, void (*done)(struct scsi_
>  	/* Save the scsi command for use by the ISR */
>  	tw_dev->srb[request_id] = SCpnt;
>
> +	/* Initialize phase to zero */
> +	SCpnt->SCp.phase = TW_PHASE_INITIAL;
> +
>  	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
>  	switch (retval) {
>  	case SCSI_MLQUEUE_HOST_BUSY:
> -		scsi_dma_unmap(SCpnt);
> +		twa_scsi_dma_unmap(SCpnt);
>  		twa_free_request_id(tw_dev, request_id);
>  		break;
>  	case 1:
>  		SCpnt->result = (DID_ERROR << 16);
> -		scsi_dma_unmap(SCpnt);
> +		twa_scsi_dma_unmap(SCpnt);
>  		done(SCpnt);
>  		tw_dev->state[request_id] = TW_S_COMPLETED;
>  		twa_free_request_id(tw_dev, request_id);
> @@ -1845,6 +1854,8 @@ static int
> twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
>  				if (sg_count < 0)
>  					goto out;
>
> +				srb->SCp.phase = TW_PHASE_SGLIST;
> +
>  				scsi_for_each_sg(srb, sg, sg_count, i) {
>  					command_packet->sg_list[i].address =
> TW_CPU_TO_SGL(sg_dma_address(sg));
>  					command_packet->sg_list[i].length = cpu_to_le32(sg_dma_len(sg));
>
>


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

Re: twa generates WARNING upon boot

[James Bottomley]

On Tue, 2015-09-29 at 20:25 +0200, "Tóth Attila" wrote:
> 2015.Szeptember 29.(K) 20:02 időpontban James Bottomley ezt írta:
> > On Tue, 2015-09-29 at 10:37 -0700, James Bottomley wrote:
> >> On Tue, 2015-09-29 at 18:49 +0200, "Tóth Attila" wrote:
> >> > 2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
> >> > > On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila" <atoth@atoth.sote.hu>
> >> > > wrote:
> >> > >> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X
> >> > >>
> >> > >
> >> > > Can you re-try with Christoph's patch:
> >> > >
> >> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de
> >> >
> >> > As I've told this patch has been already included in the kernel I'm
> >> using
> >> > (4.1.7-hardened-r1, which is based on 4.1.7).
> >> > Out of curiosity I've reverted the patch and the WARNING no longer
> >> appears
> >> > during boot...
> >>
> >> Ah, it looks like there's a bug in the patch, it doesn't take into
> >> account that the driver has a minimum sg map length and it copies for
> >> things under that, so we're likely unmapping something that wasn't
> >> mapped.   It's slightly hard to fix given that the indicator flag we'd
> >> use for this is gone in that patch; does this fix the warning?
> >
> > Actually, strike that, it would still barf on internally generated
> > commands (and it's recursively calling twa_scsi_dma_unmap).  This does a
> > partial revert of the state check which should pick up all cases the
> > input didn't go through the mapping.
> 
> Just to be clear before I try something out.
> In this latter patch you are resurrecting TW_PHASE_SGLIST, which has been
> ment to be removed in Christoph's patch from the header.
> I guess I have to reintroduce those phase defines on top of the changes
> below in your latest email.
> Or please let me know what should I exactly try out.

Heh, sorry forgot the header change.  This should be the complete patch
attached.

James

---

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index add419d..751ed66 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -151,7 +151,13 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
 static char *twa_string_lookup(twa_message_type *table, unsigned int aen_code);
 
 /* Functions */
+static void twa_scsi_dma_unmap(struct scsi_cmnd *SCpnt)
+{
+	if (SCpnt->SCp.phase == TW_PHASE_SGLIST)
+		scsi_dma_unmap(SCpnt);
+}
 
+  
 /* Show some statistics about the card */
 static ssize_t twa_show_stats(struct device *dev,
 			      struct device_attribute *attr, char *buf)
@@ -1339,7 +1345,7 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
 				}
 
 				/* Now complete the io */
-				scsi_dma_unmap(cmd);
+				twa_scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 				tw_dev->state[request_id] = TW_S_COMPLETED;
 				twa_free_request_id(tw_dev, request_id);
@@ -1582,7 +1588,7 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
 				struct scsi_cmnd *cmd = tw_dev->srb[i];
 
 				cmd->result = (DID_RESET << 16);
-				scsi_dma_unmap(cmd);
+				twa_scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 			}
 		}
@@ -1762,15 +1768,18 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
 	/* Save the scsi command for use by the ISR */
 	tw_dev->srb[request_id] = SCpnt;
 
+	/* Initialize phase to zero */
+	SCpnt->SCp.phase = TW_PHASE_INITIAL;
+
 	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
 	switch (retval) {
 	case SCSI_MLQUEUE_HOST_BUSY:
-		scsi_dma_unmap(SCpnt);
+		twa_scsi_dma_unmap(SCpnt);
 		twa_free_request_id(tw_dev, request_id);
 		break;
 	case 1:
 		SCpnt->result = (DID_ERROR << 16);
-		scsi_dma_unmap(SCpnt);
+		twa_scsi_dma_unmap(SCpnt);
 		done(SCpnt);
 		tw_dev->state[request_id] = TW_S_COMPLETED;
 		twa_free_request_id(tw_dev, request_id);
@@ -1845,6 +1854,8 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
 				if (sg_count < 0)
 					goto out;
 
+				srb->SCp.phase = TW_PHASE_SGLIST;
+
 				scsi_for_each_sg(srb, sg, sg_count, i) {
 					command_packet->sg_list[i].address = TW_CPU_TO_SGL(sg_dma_address(sg));
 					command_packet->sg_list[i].length = cpu_to_le32(sg_dma_len(sg));
diff --git a/drivers/scsi/3w-9xxx.h b/drivers/scsi/3w-9xxx.h
index 0fdc83c..c88d5bd 100644
--- a/drivers/scsi/3w-9xxx.h
+++ b/drivers/scsi/3w-9xxx.h
@@ -324,6 +324,10 @@ static twa_message_type twa_error_table[] = {
 #define TW_CURRENT_DRIVER_BUILD 0
 #define TW_CURRENT_DRIVER_BRANCH 0
 
+/* Phase defines */
+#define TW_PHASE_INITIAL 0
+#define TW_PHASE_SGLIST  1
+
 /* Misc defines */
 #define TW_9550SX_DRAIN_COMPLETED	      0xFFFF
 #define TW_SECTOR_SIZE                        512


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

Re: twa generates WARNING upon boot

["Tóth Attila"]

2015.Szeptember 29.(K) 20:33 időpontban James Bottomley ezt írta:
> On Tue, 2015-09-29 at 20:25 +0200, "Tóth Attila" wrote:
>> 2015.Szeptember 29.(K) 20:02 időpontban James Bottomley ezt írta:
>> > On Tue, 2015-09-29 at 10:37 -0700, James Bottomley wrote:
>> >> On Tue, 2015-09-29 at 18:49 +0200, "Tóth Attila" wrote:
>> >> > 2015.Szeptember 27.(V) 23:19 időpontban adam radford ezt írta:
>> >> > > On Sun, Sep 27, 2015 at 4:56 AM, "Tóth Attila"
>> <atoth@atoth.sote.hu>
>> >> > > wrote:
>> >> > >> The 3ware card is a 9650SE-12ML running in a Asus Z8PE-D12X
>> >> > >>
>> >> > >
>> >> > > Can you re-try with Christoph's patch:
>> >> > >
>> >> > > https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de
>> >> >
>> >> > As I've told this patch has been already included in the kernel I'm
>> >> using
>> >> > (4.1.7-hardened-r1, which is based on 4.1.7).
>> >> > Out of curiosity I've reverted the patch and the WARNING no longer
>> >> appears
>> >> > during boot...
>> >>
>> >> Ah, it looks like there's a bug in the patch, it doesn't take into
>> >> account that the driver has a minimum sg map length and it copies for
>> >> things under that, so we're likely unmapping something that wasn't
>> >> mapped.   It's slightly hard to fix given that the indicator flag
>> we'd
>> >> use for this is gone in that patch; does this fix the warning?
>> >
>> > Actually, strike that, it would still barf on internally generated
>> > commands (and it's recursively calling twa_scsi_dma_unmap).  This does
>> a
>> > partial revert of the state check which should pick up all cases the
>> > input didn't go through the mapping.
>>
>> Just to be clear before I try something out.
>> In this latter patch you are resurrecting TW_PHASE_SGLIST, which has
>> been
>> ment to be removed in Christoph's patch from the header.
>> I guess I have to reintroduce those phase defines on top of the changes
>> below in your latest email.
>> Or please let me know what should I exactly try out.
>
> Heh, sorry forgot the header change.  This should be the complete patch
> attached.

After applying your latest patch I can see no more WARNINGs upon reboot!

Please note, that apart from the culprit 3w-9xxx commit
(https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=118c855b5623f3e2e6204f02623d88c09e0c34de),
two other parent commits were introducing similar changes to 3w-xxxx and
3w-sas drivers
(https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=9cd9554615cba14f0877cc9972a6537ad2bdde61,
https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/?id=579d69bc1fd56d5af5761969aa529d1d1c188300).
Those two may also need similar treatment.

Many thanks:
Dw.
-- 
dr Tóth Attila, Radiológus, 06-20-825-8057
Attila Toth MD, Radiologist, +36-20-825-8057

> ---
>
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index add419d..751ed66 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -151,7 +151,13 @@ static void
> twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
>  static char *twa_string_lookup(twa_message_type *table, unsigned int
> aen_code);
>
>  /* Functions */
> +static void twa_scsi_dma_unmap(struct scsi_cmnd *SCpnt)
> +{
> +	if (SCpnt->SCp.phase == TW_PHASE_SGLIST)
> +		scsi_dma_unmap(SCpnt);
> +}
>
> +
>  /* Show some statistics about the card */
>  static ssize_t twa_show_stats(struct device *dev,
>  			      struct device_attribute *attr, char *buf)
> @@ -1339,7 +1345,7 @@ static irqreturn_t twa_interrupt(int irq, void
> *dev_instance)
>  				}
>
>  				/* Now complete the io */
> -				scsi_dma_unmap(cmd);
> +				twa_scsi_dma_unmap(cmd);
>  				cmd->scsi_done(cmd);
>  				tw_dev->state[request_id] = TW_S_COMPLETED;
>  				twa_free_request_id(tw_dev, request_id);
> @@ -1582,7 +1588,7 @@ static int
> twa_reset_device_extension(TW_Device_Extension *tw_dev)
>  				struct scsi_cmnd *cmd = tw_dev->srb[i];
>
>  				cmd->result = (DID_RESET << 16);
> -				scsi_dma_unmap(cmd);
> +				twa_scsi_dma_unmap(cmd);
>  				cmd->scsi_done(cmd);
>  			}
>  		}
> @@ -1762,15 +1768,18 @@ static int twa_scsi_queue_lck(struct scsi_cmnd
> *SCpnt, void (*done)(struct scsi_
>  	/* Save the scsi command for use by the ISR */
>  	tw_dev->srb[request_id] = SCpnt;
>
> +	/* Initialize phase to zero */
> +	SCpnt->SCp.phase = TW_PHASE_INITIAL;
> +
>  	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
>  	switch (retval) {
>  	case SCSI_MLQUEUE_HOST_BUSY:
> -		scsi_dma_unmap(SCpnt);
> +		twa_scsi_dma_unmap(SCpnt);
>  		twa_free_request_id(tw_dev, request_id);
>  		break;
>  	case 1:
>  		SCpnt->result = (DID_ERROR << 16);
> -		scsi_dma_unmap(SCpnt);
> +		twa_scsi_dma_unmap(SCpnt);
>  		done(SCpnt);
>  		tw_dev->state[request_id] = TW_S_COMPLETED;
>  		twa_free_request_id(tw_dev, request_id);
> @@ -1845,6 +1854,8 @@ static int
> twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
>  				if (sg_count < 0)
>  					goto out;
>
> +				srb->SCp.phase = TW_PHASE_SGLIST;
> +
>  				scsi_for_each_sg(srb, sg, sg_count, i) {
>  					command_packet->sg_list[i].address =
> TW_CPU_TO_SGL(sg_dma_address(sg));
>  					command_packet->sg_list[i].length = cpu_to_le32(sg_dma_len(sg));
> diff --git a/drivers/scsi/3w-9xxx.h b/drivers/scsi/3w-9xxx.h
> index 0fdc83c..c88d5bd 100644
> --- a/drivers/scsi/3w-9xxx.h
> +++ b/drivers/scsi/3w-9xxx.h
> @@ -324,6 +324,10 @@ static twa_message_type twa_error_table[] = {
>  #define TW_CURRENT_DRIVER_BUILD 0
>  #define TW_CURRENT_DRIVER_BRANCH 0
>
> +/* Phase defines */
> +#define TW_PHASE_INITIAL 0
> +#define TW_PHASE_SGLIST  1
> +
>  /* Misc defines */
>  #define TW_9550SX_DRAIN_COMPLETED	      0xFFFF
>  #define TW_SECTOR_SIZE                        512
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


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

Re: twa generates WARNING upon boot

[Christoph Hellwig]

Hi all,

I'd like to propose the following patch intead.  It uses a helper
to check the conditions for the copied commands, and also fixes another
place to use it which uses a different and I think buggy check:

This avoids the usage of scsi_cmnd.SCp which I'd like to get rid of
mid-term.

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index add419d..74eac90 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -212,6 +212,17 @@ static const struct file_operations twa_fops = {
 	.llseek		= noop_llseek,
 };
 
+/*
+ * The controllers use an inline buffer instead of a mapped SGL for small,
+ * single entry buffers.  Note that we treat a zero-length transfer like
+ * a mapped SGL.
+ */
+static bool twa_command_mapped(struct scsi_cmnd *cmd)
+{
+	return scsi_sg_count(cmd) != 1 ||
+		scsi_bufflen(cmd) >= TW_MIN_SGL_LENGTH;
+}
+
 /* This function will complete an aen request from the isr */
 static int twa_aen_complete(TW_Device_Extension *tw_dev, int request_id)
 {
@@ -1339,7 +1350,8 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
 				}
 
 				/* Now complete the io */
-				scsi_dma_unmap(cmd);
+				if (twa_command_mapped(cmd))
+					scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 				tw_dev->state[request_id] = TW_S_COMPLETED;
 				twa_free_request_id(tw_dev, request_id);
@@ -1582,7 +1594,8 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
 				struct scsi_cmnd *cmd = tw_dev->srb[i];
 
 				cmd->result = (DID_RESET << 16);
-				scsi_dma_unmap(cmd);
+				if (twa_command_mapped(cmd))
+					scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 			}
 		}
@@ -1765,12 +1778,14 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
 	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
 	switch (retval) {
 	case SCSI_MLQUEUE_HOST_BUSY:
-		scsi_dma_unmap(SCpnt);
+		if (twa_command_mapped(cmd))
+			scsi_dma_unmap(SCpnt);
 		twa_free_request_id(tw_dev, request_id);
 		break;
 	case 1:
 		SCpnt->result = (DID_ERROR << 16);
-		scsi_dma_unmap(SCpnt);
+		if (twa_command_mapped(cmd))
+			scsi_dma_unmap(SCpnt);
 		done(SCpnt);
 		tw_dev->state[request_id] = TW_S_COMPLETED;
 		twa_free_request_id(tw_dev, request_id);
@@ -1831,8 +1846,7 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
 		/* Map sglist from scsi layer to cmd packet */
 
 		if (scsi_sg_count(srb)) {
-			if ((scsi_sg_count(srb) == 1) &&
-			    (scsi_bufflen(srb) < TW_MIN_SGL_LENGTH)) {
+			if (!twa_command_mapped(srb)) {
 				if (srb->sc_data_direction == DMA_TO_DEVICE ||
 				    srb->sc_data_direction == DMA_BIDIRECTIONAL)
 					scsi_sg_copy_to_buffer(srb,
@@ -1905,7 +1919,7 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
 {
 	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
 
-	if (scsi_bufflen(cmd) < TW_MIN_SGL_LENGTH &&
+	if (!twa_command_mapped(srb) &&
 	    (cmd->sc_data_direction == DMA_FROM_DEVICE ||
 	     cmd->sc_data_direction == DMA_BIDIRECTIONAL)) {
 		if (scsi_sg_count(cmd) == 1) {

Re: twa generates WARNING upon boot

[James Bottomley]

On Wed, 2015-09-30 at 09:08 -0700, Christoph Hellwig wrote:
> Hi all,
> 
> I'd like to propose the following patch intead.  It uses a helper
> to check the conditions for the copied commands, and also fixes another
> place to use it which uses a different and I think buggy check:
> 
> This avoids the usage of scsi_cmnd.SCp which I'd like to get rid of
> mid-term.
> 
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index add419d..74eac90 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -212,6 +212,17 @@ static const struct file_operations twa_fops = {
>  	.llseek		= noop_llseek,
>  };
>  
> +/*
> + * The controllers use an inline buffer instead of a mapped SGL for small,
> + * single entry buffers.  Note that we treat a zero-length transfer like
> + * a mapped SGL.
> + */
> +static bool twa_command_mapped(struct scsi_cmnd *cmd)
> +{
> +	return scsi_sg_count(cmd) != 1 ||
> +		scsi_bufflen(cmd) >= TW_MIN_SGL_LENGTH;
> +}

I already thought of this.  Unfortunately, it fails if the internally
posted command is a single sector (the size of TW_MIN_SGL_LENGTH), which
is true for most of them.

James

Re: Re: twa generates WARNING upon boot

[kbuild test robot]

[-- Attachment #1: Type: text/plain, Size: 15337 bytes --]

Hi Christoph,

[auto build test results on v4.3-rc3 -- if it's inappropriate base, please ignore]

config: x86_64-allmodconfig (attached as .config)
reproduce:
  git checkout 6e392493504e88ec3b44596c22f08acf4eed11ee
  # save the attached .config to linux build tree
  make ARCH=x86_64 

All error/warnings (new ones prefixed by >>):

   drivers/scsi/3w-9xxx.c:394:15: sparse: cast to restricted __le16
   drivers/scsi/3w-9xxx.c:491:61: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:491:61: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:492:58: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:492:58:    expected unsigned int [unsigned] [usertype] length
   drivers/scsi/3w-9xxx.c:492:58:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:494:54: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:494:54:    expected unsigned short [unsigned] [usertype] parameter_count
   drivers/scsi/3w-9xxx.c:494:54:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:499:25: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:499:25:    expected unsigned short [unsigned] [usertype] table_id
   drivers/scsi/3w-9xxx.c:499:25:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:500:29: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:500:29:    expected unsigned short [unsigned] [usertype] parameter_id
   drivers/scsi/3w-9xxx.c:500:29:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:501:37: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:501:37:    expected unsigned short [unsigned] [usertype] parameter_size_bytes
   drivers/scsi/3w-9xxx.c:501:37:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:508:23: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:508:23:    expected unsigned int [unsigned] [assigned] [usertype] schedulertime
   drivers/scsi/3w-9xxx.c:508:23:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:996:17: sparse: cast to restricted __le16
   drivers/scsi/3w-9xxx.c:1129:41: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1129:41:    expected unsigned short [unsigned] [usertype] message_credits
   drivers/scsi/3w-9xxx.c:1129:41:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1135:34: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1135:34:    expected unsigned int [unsigned] [usertype] features
   drivers/scsi/3w-9xxx.c:1135:34:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1139:40: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1139:40:    expected unsigned short [unsigned] [usertype] fw_srl
   drivers/scsi/3w-9xxx.c:1139:40:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1140:44: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1140:44:    expected unsigned short [unsigned] [usertype] fw_arch_id
   drivers/scsi/3w-9xxx.c:1140:44:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1141:43: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1141:43:    expected unsigned short [unsigned] [usertype] fw_branch
   drivers/scsi/3w-9xxx.c:1141:43:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1142:42: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1142:42:    expected unsigned short [unsigned] [usertype] fw_build
   drivers/scsi/3w-9xxx.c:1142:42:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1154:43: sparse: cast to restricted __le16
   drivers/scsi/3w-9xxx.c:1155:47: sparse: cast to restricted __le16
   drivers/scsi/3w-9xxx.c:1156:46: sparse: cast to restricted __le16
   drivers/scsi/3w-9xxx.c:1157:45: sparse: cast to restricted __le16
   drivers/scsi/3w-9xxx.c:1158:48: sparse: cast to restricted __le32
   drivers/scsi/3w-9xxx.c:1390:46: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1390:46:    expected unsigned short [unsigned] [usertype] request_id__lunl
   drivers/scsi/3w-9xxx.c:1390:46:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1393:58: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1393:58: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1394:55: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1394:55:    expected unsigned int [unsigned] [usertype] length
   drivers/scsi/3w-9xxx.c:1394:55:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1396:47: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1396:47:    expected unsigned short [unsigned] [usertype] sgl_entries__lunh
   drivers/scsi/3w-9xxx.c:1396:47:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1408:40: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1408:40: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1409:37: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1409:37:    expected unsigned int [unsigned] [usertype] length
   drivers/scsi/3w-9xxx.c:1409:37:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1835:50: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1835:50:    expected unsigned short [unsigned] [usertype] request_id__lunl
   drivers/scsi/3w-9xxx.c:1835:50:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1838:50: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1838:50:    expected unsigned short [unsigned] [usertype] request_id__lunl
   drivers/scsi/3w-9xxx.c:1838:50:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1855:70: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1855:70: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1856:67: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1856:67:    expected unsigned int [unsigned] [usertype] length
   drivers/scsi/3w-9xxx.c:1856:67:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1863:78: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1863:78: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1864:75: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1864:75:    expected unsigned int [unsigned] [usertype] length
   drivers/scsi/3w-9xxx.c:1864:75:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1865:82: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1865:82: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1871:59: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1871:59:    expected unsigned short [unsigned] [usertype] sgl_entries__lunh
   drivers/scsi/3w-9xxx.c:1871:59:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1876:62: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1876:62: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1877:59: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1877:59:    expected unsigned int [unsigned] [usertype] length
   drivers/scsi/3w-9xxx.c:1877:59:    got restricted __le32 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1878:66: sparse: restricted __le64 degrades to integer
   drivers/scsi/3w-9xxx.c:1878:66: sparse: restricted __le32 degrades to integer
   drivers/scsi/3w-9xxx.c:1883:51: sparse: incorrect type in assignment (different base types)
   drivers/scsi/3w-9xxx.c:1883:51:    expected unsigned short [unsigned] [usertype] sgl_entries__lunh
   drivers/scsi/3w-9xxx.c:1883:51:    got restricted __le16 [usertype] <noident>
   drivers/scsi/3w-9xxx.c:1922:33: sparse: undefined identifier 'srb'
   drivers/scsi/3w-9xxx.c:1781:40: sparse: undefined identifier 'cmd'
   drivers/scsi/3w-9xxx.c:1787:40: sparse: undefined identifier 'cmd'
   drivers/scsi/3w-9xxx.c: In function 'twa_scsi_queue_lck':
>> drivers/scsi/3w-9xxx.c:1781:26: error: 'cmd' undeclared (first use in this function)
      if (twa_command_mapped(cmd))
                             ^
   drivers/scsi/3w-9xxx.c:1781:26: note: each undeclared identifier is reported only once for each function it appears in
   drivers/scsi/3w-9xxx.c: In function 'twa_scsiop_execute_scsi_complete':
>> drivers/scsi/3w-9xxx.c:1922:26: error: 'srb' undeclared (first use in this function)
     if (!twa_command_mapped(srb) &&
                             ^

vim +/cmd +1781 drivers/scsi/3w-9xxx.c

  1775		/* Save the scsi command for use by the ISR */
  1776		tw_dev->srb[request_id] = SCpnt;
  1777	
  1778		retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
  1779		switch (retval) {
  1780		case SCSI_MLQUEUE_HOST_BUSY:
> 1781			if (twa_command_mapped(cmd))
  1782				scsi_dma_unmap(SCpnt);
  1783			twa_free_request_id(tw_dev, request_id);
  1784			break;
  1785		case 1:
  1786			SCpnt->result = (DID_ERROR << 16);
  1787			if (twa_command_mapped(cmd))
  1788				scsi_dma_unmap(SCpnt);
  1789			done(SCpnt);
  1790			tw_dev->state[request_id] = TW_S_COMPLETED;
  1791			twa_free_request_id(tw_dev, request_id);
  1792			retval = 0;
  1793		}
  1794	out:
  1795		return retval;
  1796	} /* End twa_scsi_queue() */
  1797	
  1798	static DEF_SCSI_QCMD(twa_scsi_queue)
  1799	
  1800	/* This function hands scsi cdb's to the firmware */
  1801	static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id, char *cdb, int use_sg, TW_SG_Entry *sglistarg)
  1802	{
  1803		TW_Command_Full *full_command_packet;
  1804		TW_Command_Apache *command_packet;
  1805		u32 num_sectors = 0x0;
  1806		int i, sg_count;
  1807		struct scsi_cmnd *srb = NULL;
  1808		struct scatterlist *sglist = NULL, *sg;
  1809		int retval = 1;
  1810	
  1811		if (tw_dev->srb[request_id]) {
  1812			srb = tw_dev->srb[request_id];
  1813			if (scsi_sglist(srb))
  1814				sglist = scsi_sglist(srb);
  1815		}
  1816	
  1817		/* Initialize command packet */
  1818		full_command_packet = tw_dev->command_packet_virt[request_id];
  1819		full_command_packet->header.header_desc.size_header = 128;
  1820		full_command_packet->header.status_block.error = 0;
  1821		full_command_packet->header.status_block.severity__reserved = 0;
  1822	
  1823		command_packet = &full_command_packet->command.newcommand;
  1824		command_packet->status = 0;
  1825		command_packet->opcode__reserved = TW_OPRES_IN(0, TW_OP_EXECUTE_SCSI);
  1826	
  1827		/* We forced 16 byte cdb use earlier */
  1828		if (!cdb)
  1829			memcpy(command_packet->cdb, srb->cmnd, TW_MAX_CDB_LEN);
  1830		else
  1831			memcpy(command_packet->cdb, cdb, TW_MAX_CDB_LEN);
  1832	
  1833		if (srb) {
  1834			command_packet->unit = srb->device->id;
  1835			command_packet->request_id__lunl =
  1836				cpu_to_le16(TW_REQ_LUN_IN(srb->device->lun, request_id));
  1837		} else {
  1838			command_packet->request_id__lunl =
  1839				cpu_to_le16(TW_REQ_LUN_IN(0, request_id));
  1840			command_packet->unit = 0;
  1841		}
  1842	
  1843		command_packet->sgl_offset = 16;
  1844	
  1845		if (!sglistarg) {
  1846			/* Map sglist from scsi layer to cmd packet */
  1847	
  1848			if (scsi_sg_count(srb)) {
  1849				if (!twa_command_mapped(srb)) {
  1850					if (srb->sc_data_direction == DMA_TO_DEVICE ||
  1851					    srb->sc_data_direction == DMA_BIDIRECTIONAL)
  1852						scsi_sg_copy_to_buffer(srb,
  1853								       tw_dev->generic_buffer_virt[request_id],
  1854								       TW_SECTOR_SIZE);
  1855					command_packet->sg_list[0].address = TW_CPU_TO_SGL(tw_dev->generic_buffer_phys[request_id]);
  1856					command_packet->sg_list[0].length = cpu_to_le32(TW_MIN_SGL_LENGTH);
  1857				} else {
  1858					sg_count = scsi_dma_map(srb);
  1859					if (sg_count < 0)
  1860						goto out;
  1861	
  1862					scsi_for_each_sg(srb, sg, sg_count, i) {
  1863						command_packet->sg_list[i].address = TW_CPU_TO_SGL(sg_dma_address(sg));
  1864						command_packet->sg_list[i].length = cpu_to_le32(sg_dma_len(sg));
  1865						if (command_packet->sg_list[i].address & TW_CPU_TO_SGL(TW_ALIGNMENT_9000_SGL)) {
  1866							TW_PRINTK(tw_dev->host, TW_DRIVER, 0x2e, "Found unaligned sgl address during execute scsi");
  1867							goto out;
  1868						}
  1869					}
  1870				}
  1871				command_packet->sgl_entries__lunh = cpu_to_le16(TW_REQ_LUN_IN((srb->device->lun >> 4), scsi_sg_count(tw_dev->srb[request_id])));
  1872			}
  1873		} else {
  1874			/* Internal cdb post */
  1875			for (i = 0; i < use_sg; i++) {
  1876				command_packet->sg_list[i].address = TW_CPU_TO_SGL(sglistarg[i].address);
  1877				command_packet->sg_list[i].length = cpu_to_le32(sglistarg[i].length);
> 1878				if (command_packet->sg_list[i].address & TW_CPU_TO_SGL(TW_ALIGNMENT_9000_SGL)) {
  1879					TW_PRINTK(tw_dev->host, TW_DRIVER, 0x2f, "Found unaligned sgl address during internal post");
  1880					goto out;
  1881				}
  1882			}
  1883			command_packet->sgl_entries__lunh = cpu_to_le16(TW_REQ_LUN_IN(0, use_sg));
  1884		}
  1885	
  1886		if (srb) {
  1887			if (srb->cmnd[0] == READ_6 || srb->cmnd[0] == WRITE_6)
  1888				num_sectors = (u32)srb->cmnd[4];
  1889	
  1890			if (srb->cmnd[0] == READ_10 || srb->cmnd[0] == WRITE_10)
  1891				num_sectors = (u32)srb->cmnd[8] | ((u32)srb->cmnd[7] << 8);
  1892		}
  1893	
  1894		/* Update sector statistic */
  1895		tw_dev->sector_count = num_sectors;
  1896		if (tw_dev->sector_count > tw_dev->max_sector_count)
  1897			tw_dev->max_sector_count = tw_dev->sector_count;
  1898	
  1899		/* Update SG statistics */
  1900		if (srb) {
  1901			tw_dev->sgl_entries = scsi_sg_count(tw_dev->srb[request_id]);
  1902			if (tw_dev->sgl_entries > tw_dev->max_sgl_entries)
  1903				tw_dev->max_sgl_entries = tw_dev->sgl_entries;
  1904		}
  1905	
  1906		/* Now post the command to the board */
  1907		if (srb) {
  1908			retval = twa_post_command_packet(tw_dev, request_id, 0);
  1909		} else {
  1910			twa_post_command_packet(tw_dev, request_id, 1);
  1911			retval = 0;
  1912		}
  1913	out:
  1914		return retval;
  1915	} /* End twa_scsiop_execute_scsi() */
  1916	
  1917	/* This function completes an execute scsi operation */
  1918	static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int request_id)
  1919	{
  1920		struct scsi_cmnd *cmd = tw_dev->srb[request_id];
  1921	
> 1922		if (!twa_command_mapped(srb) &&
  1923		    (cmd->sc_data_direction == DMA_FROM_DEVICE ||
  1924		     cmd->sc_data_direction == DMA_BIDIRECTIONAL)) {
  1925			if (scsi_sg_count(cmd) == 1) {

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation

[-- Attachment #2: .config.gz --]
[-- Type: application/octet-stream, Size: 50052 bytes --]

Re: twa generates WARNING upon boot

[Christoph Hellwig]

On Wed, Sep 30, 2015 at 09:15:30AM -0700, James Bottomley wrote:
> I already thought of this.  Unfortunately, it fails if the internally
> posted command is a single sector (the size of TW_MIN_SGL_LENGTH), which
> is true for most of them.

Which internally posted command?  All the usual suspects set
tw_dev->srb[request_id] to NULL and thus neither have a scsi_cmnd
associated with them, nor will they ever reach a code path where we call
twa_command_mapped().

Re: twa generates WARNING upon boot

[James Bottomley]

On Wed, 2015-09-30 at 09:31 -0700, Christoph Hellwig wrote:
> On Wed, Sep 30, 2015 at 09:15:30AM -0700, James Bottomley wrote:
> > I already thought of this.  Unfortunately, it fails if the internally
> > posted command is a single sector (the size of TW_MIN_SGL_LENGTH), which
> > is true for most of them.
> 
> Which internally posted command?  All the usual suspects set
> tw_dev->srb[request_id] to NULL and thus neither have a scsi_cmnd
> associated with them, nor will they ever reach a code path where we call
> twa_command_mapped().

Are you sure?  The init trace that kicked all this off still has
twa_init in it.  I was figuring the most likely suspect is
twa_get_param().

James

Re: twa generates WARNING upon boot

[Christoph Hellwig]

On Wed, Sep 30, 2015 at 09:36:24AM -0700, James Bottomley wrote:
> Are you sure?  The init trace that kicked all this off still has
> twa_init in it.  I was figuring the most likely suspect is
> twa_get_param().

That one doesnt set ->srb at all, but given that it's called before
any SCSI commands can sent ->srb will be all NULL.  It certainly
doesn't use a scsi_cmnd anywhere and does not expect to hit any
code path calling scsi_dma_unmap.  If it ends up there it's a bad bug
and would blow up a lot more spectacularly.

Re: twa generates WARNING upon boot

[James Bottomley]

On Wed, 2015-09-30 at 09:41 -0700, Christoph Hellwig wrote:
> On Wed, Sep 30, 2015 at 09:36:24AM -0700, James Bottomley wrote:
> > Are you sure?  The init trace that kicked all this off still has
> > twa_init in it.  I was figuring the most likely suspect is
> > twa_get_param().
> 
> That one doesnt set ->srb at all, but given that it's called before
> any SCSI commands can sent ->srb will be all NULL.  It certainly
> doesn't use a scsi_cmnd anywhere and does not expect to hit any
> code path calling scsi_dma_unmap.  If it ends up there it's a bad bug
> and would blow up a lot more spectacularly.

OK, post a compilable version of the patch and lets get the reporter to
try it out.  Not resurrecting esoteric flags suits me.

James

Re: twa generates WARNING upon boot

[Christoph Hellwig]

On Wed, Sep 30, 2015 at 09:43:28AM -0700, James Bottomley wrote:
> OK, post a compilable version of the patch and lets get the reporter to
> try it out.  Not resurrecting esoteric flags suits me.

Right here:

diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
index add419d..a56a7b2 100644
--- a/drivers/scsi/3w-9xxx.c
+++ b/drivers/scsi/3w-9xxx.c
@@ -212,6 +212,17 @@ static const struct file_operations twa_fops = {
 	.llseek		= noop_llseek,
 };
 
+/*
+ * The controllers use an inline buffer instead of a mapped SGL for small,
+ * single entry buffers.  Note that we treat a zero-length transfer like
+ * a mapped SGL.
+ */
+static bool twa_command_mapped(struct scsi_cmnd *cmd)
+{
+	return scsi_sg_count(cmd) != 1 ||
+		scsi_bufflen(cmd) >= TW_MIN_SGL_LENGTH;
+}
+
 /* This function will complete an aen request from the isr */
 static int twa_aen_complete(TW_Device_Extension *tw_dev, int request_id)
 {
@@ -1339,7 +1350,8 @@ static irqreturn_t twa_interrupt(int irq, void *dev_instance)
 				}
 
 				/* Now complete the io */
-				scsi_dma_unmap(cmd);
+				if (twa_command_mapped(cmd))
+					scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 				tw_dev->state[request_id] = TW_S_COMPLETED;
 				twa_free_request_id(tw_dev, request_id);
@@ -1582,7 +1594,8 @@ static int twa_reset_device_extension(TW_Device_Extension *tw_dev)
 				struct scsi_cmnd *cmd = tw_dev->srb[i];
 
 				cmd->result = (DID_RESET << 16);
-				scsi_dma_unmap(cmd);
+				if (twa_command_mapped(cmd))
+					scsi_dma_unmap(cmd);
 				cmd->scsi_done(cmd);
 			}
 		}
@@ -1765,12 +1778,14 @@ static int twa_scsi_queue_lck(struct scsi_cmnd *SCpnt, void (*done)(struct scsi_
 	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
 	switch (retval) {
 	case SCSI_MLQUEUE_HOST_BUSY:
-		scsi_dma_unmap(SCpnt);
+		if (twa_command_mapped(SCpnt))
+			scsi_dma_unmap(SCpnt);
 		twa_free_request_id(tw_dev, request_id);
 		break;
 	case 1:
 		SCpnt->result = (DID_ERROR << 16);
-		scsi_dma_unmap(SCpnt);
+		if (twa_command_mapped(SCpnt))
+			scsi_dma_unmap(SCpnt);
 		done(SCpnt);
 		tw_dev->state[request_id] = TW_S_COMPLETED;
 		twa_free_request_id(tw_dev, request_id);
@@ -1831,8 +1846,7 @@ static int twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
 		/* Map sglist from scsi layer to cmd packet */
 
 		if (scsi_sg_count(srb)) {
-			if ((scsi_sg_count(srb) == 1) &&
-			    (scsi_bufflen(srb) < TW_MIN_SGL_LENGTH)) {
+			if (!twa_command_mapped(srb)) {
 				if (srb->sc_data_direction == DMA_TO_DEVICE ||
 				    srb->sc_data_direction == DMA_BIDIRECTIONAL)
 					scsi_sg_copy_to_buffer(srb,
@@ -1905,7 +1919,7 @@ static void twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
 {
 	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
 
-	if (scsi_bufflen(cmd) < TW_MIN_SGL_LENGTH &&
+	if (!twa_command_mapped(cmd) &&
 	    (cmd->sc_data_direction == DMA_FROM_DEVICE ||
 	     cmd->sc_data_direction == DMA_BIDIRECTIONAL)) {
 		if (scsi_sg_count(cmd) == 1) {

Re: twa generates WARNING upon boot

["Tóth Attila"]

2015.Szeptember 30.(Sze) 18:44 időpontban Christoph Hellwig ezt írta:
> On Wed, Sep 30, 2015 at 09:43:28AM -0700, James Bottomley wrote:
>> OK, post a compilable version of the patch and lets get the reporter to
>> try it out.  Not resurrecting esoteric flags suits me.

No WARNINGs upon reboot using this patch, either. The controller seems to
work properly so far.

Thanks:
Dw.
-- 
dr Tóth Attila, Radiológus, 06-20-825-8057
Attila Toth MD, Radiologist, +36-20-825-8057

> Right here:
>
> diff --git a/drivers/scsi/3w-9xxx.c b/drivers/scsi/3w-9xxx.c
> index add419d..a56a7b2 100644
> --- a/drivers/scsi/3w-9xxx.c
> +++ b/drivers/scsi/3w-9xxx.c
> @@ -212,6 +212,17 @@ static const struct file_operations twa_fops = {
>  	.llseek		= noop_llseek,
>  };
>
> +/*
> + * The controllers use an inline buffer instead of a mapped SGL for
> small,
> + * single entry buffers.  Note that we treat a zero-length transfer like
> + * a mapped SGL.
> + */
> +static bool twa_command_mapped(struct scsi_cmnd *cmd)
> +{
> +	return scsi_sg_count(cmd) != 1 ||
> +		scsi_bufflen(cmd) >= TW_MIN_SGL_LENGTH;
> +}
> +
>  /* This function will complete an aen request from the isr */
>  static int twa_aen_complete(TW_Device_Extension *tw_dev, int request_id)
>  {
> @@ -1339,7 +1350,8 @@ static irqreturn_t twa_interrupt(int irq, void
> *dev_instance)
>  				}
>
>  				/* Now complete the io */
> -				scsi_dma_unmap(cmd);
> +				if (twa_command_mapped(cmd))
> +					scsi_dma_unmap(cmd);
>  				cmd->scsi_done(cmd);
>  				tw_dev->state[request_id] = TW_S_COMPLETED;
>  				twa_free_request_id(tw_dev, request_id);
> @@ -1582,7 +1594,8 @@ static int
> twa_reset_device_extension(TW_Device_Extension *tw_dev)
>  				struct scsi_cmnd *cmd = tw_dev->srb[i];
>
>  				cmd->result = (DID_RESET << 16);
> -				scsi_dma_unmap(cmd);
> +				if (twa_command_mapped(cmd))
> +					scsi_dma_unmap(cmd);
>  				cmd->scsi_done(cmd);
>  			}
>  		}
> @@ -1765,12 +1778,14 @@ static int twa_scsi_queue_lck(struct scsi_cmnd
> *SCpnt, void (*done)(struct scsi_
>  	retval = twa_scsiop_execute_scsi(tw_dev, request_id, NULL, 0, NULL);
>  	switch (retval) {
>  	case SCSI_MLQUEUE_HOST_BUSY:
> -		scsi_dma_unmap(SCpnt);
> +		if (twa_command_mapped(SCpnt))
> +			scsi_dma_unmap(SCpnt);
>  		twa_free_request_id(tw_dev, request_id);
>  		break;
>  	case 1:
>  		SCpnt->result = (DID_ERROR << 16);
> -		scsi_dma_unmap(SCpnt);
> +		if (twa_command_mapped(SCpnt))
> +			scsi_dma_unmap(SCpnt);
>  		done(SCpnt);
>  		tw_dev->state[request_id] = TW_S_COMPLETED;
>  		twa_free_request_id(tw_dev, request_id);
> @@ -1831,8 +1846,7 @@ static int
> twa_scsiop_execute_scsi(TW_Device_Extension *tw_dev, int request_id,
>  		/* Map sglist from scsi layer to cmd packet */
>
>  		if (scsi_sg_count(srb)) {
> -			if ((scsi_sg_count(srb) == 1) &&
> -			    (scsi_bufflen(srb) < TW_MIN_SGL_LENGTH)) {
> +			if (!twa_command_mapped(srb)) {
>  				if (srb->sc_data_direction == DMA_TO_DEVICE ||
>  				    srb->sc_data_direction == DMA_BIDIRECTIONAL)
>  					scsi_sg_copy_to_buffer(srb,
> @@ -1905,7 +1919,7 @@ static void
> twa_scsiop_execute_scsi_complete(TW_Device_Extension *tw_dev, int re
>  {
>  	struct scsi_cmnd *cmd = tw_dev->srb[request_id];
>
> -	if (scsi_bufflen(cmd) < TW_MIN_SGL_LENGTH &&
> +	if (!twa_command_mapped(cmd) &&
>  	    (cmd->sc_data_direction == DMA_FROM_DEVICE ||
>  	     cmd->sc_data_direction == DMA_BIDIRECTIONAL)) {
>  		if (scsi_sg_count(cmd) == 1) {
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


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