Re: Regarding AHCI_MAX_SG and (ATA_HORKAGE_MAX_SEC_1024)

2016-08-10 Thread David Milburn 

On 08/10/2016 12:19 PM, Tom Yan wrote:

On 10 August 2016 at 15:41, David Milburn <dmilb...@redhat.com> wrote:

Hi,

The 168 makes AHCI_CMD_TBL_SZ equal to 2816

AHCI_CMD_TBL_SZ = AHCI_CMD_TBL_HDR_SZ + (AHCI_MAX_SG * 16)
AHCI_CMD_TBL_SZ = 128 + (168 * 16)

I think if you add in AHCI_CMD_SLOT_SZ (1024) and AHCI_RX_FIS_SZ (256)
the DMA is 4K aligned, I think that is where the 168 came from.


Looks like the right guess. Though AHCI_PORT_PRIV_DMA_SZ is not:

AHCI_CMD_SLOT_SZ (1024) + AHCI_CMD_TBL_SZ (2816) + AHCI_RX_FIS_SZ (256) = 4096

but:

AHCI_CMD_SLOT_SZ (1024) + AHCI_CMD_TBL_AR_SZ (2816 * 32 = 90112) +
AHCI_RX_FIS_SZ (256) = 91392

and AHCI_PORT_PRIV_FBS_DMA_SZ is:

AHCI_CMD_SLOT_SZ (1024) + AHCI_CMD_TBL_AR_SZ (2816 * 32 = 90112) +
AHCI_RX_FIS_SZ * 16 (4096) = 95232



Yes, but in both cases mem_dma gets adjusted for AHCI_CMD_SLOT_SZ (1024)
and rx_fis_sz (256 or 4096 in fbs case).

Thanks,
David

Re: Regarding AHCI_MAX_SG and (ATA_HORKAGE_MAX_SEC_1024)

2016-08-10 Thread David Milburn 

On 08/10/2016 12:19 PM, Tom Yan wrote:

On 10 August 2016 at 15:41, David Milburn  wrote:

Hi,

The 168 makes AHCI_CMD_TBL_SZ equal to 2816

AHCI_CMD_TBL_SZ = AHCI_CMD_TBL_HDR_SZ + (AHCI_MAX_SG * 16)
AHCI_CMD_TBL_SZ = 128 + (168 * 16)

I think if you add in AHCI_CMD_SLOT_SZ (1024) and AHCI_RX_FIS_SZ (256)
the DMA is 4K aligned, I think that is where the 168 came from.


Looks like the right guess. Though AHCI_PORT_PRIV_DMA_SZ is not:

AHCI_CMD_SLOT_SZ (1024) + AHCI_CMD_TBL_SZ (2816) + AHCI_RX_FIS_SZ (256) = 4096

but:

AHCI_CMD_SLOT_SZ (1024) + AHCI_CMD_TBL_AR_SZ (2816 * 32 = 90112) +
AHCI_RX_FIS_SZ (256) = 91392

and AHCI_PORT_PRIV_FBS_DMA_SZ is:

AHCI_CMD_SLOT_SZ (1024) + AHCI_CMD_TBL_AR_SZ (2816 * 32 = 90112) +
AHCI_RX_FIS_SZ * 16 (4096) = 95232



Yes, but in both cases mem_dma gets adjusted for AHCI_CMD_SLOT_SZ (1024)
and rx_fis_sz (256 or 4096 in fbs case).

Thanks,
David

Re: Regarding AHCI_MAX_SG and (ATA_HORKAGE_MAX_SEC_1024)

2016-08-10 Thread David Milburn 

Hi,

On 08/10/2016 10:14 AM, Tejun Heo wrote:

Hello, Tom.

On Wed, Aug 10, 2016 at 06:04:10PM +0800, Tom Yan wrote:

On 10 August 2016 at 11:26, Tejun Heo  wrote:

Hmmm.. why not?  The hardware limit is 64k and the driver is using a


Is that referring to the maximum number of entries allowed in the
PRDT, Physical Region Descriptor Table (which is, more precisely,
65535)?


Yeap.


Not necessarily.  A single sg entry can point to an area larger than
PAGE_SIZE.


You mean the 4MB limit of "Data Byte Count" in "DW3: Description
Information" of the PRDT? Is that what max_segment_size (which is set
to a general fallback of 65536:
http://lxr.free-electrons.com/ident?i=dma_get_max_seg_size) is about
in this case?


Ah, ahci isn't setting the hardware limit properly but yeah that's the
maximum segment size.


And my point was, it will be a multiple of 168 anyway, if 1344 is just
an example.


As written above, that probably makes the ahci command table size
nicely aligned.


I think that's what bothers me ultimately, cause I don't see how 168
makes it (more) nicely aligned (or even, aligned to what?).


Hmmm... Looked at the sizes and they don't seem to align to anything
meaningful.  No idea.


The 168 makes AHCI_CMD_TBL_SZ equal to 2816

AHCI_CMD_TBL_SZ = AHCI_CMD_TBL_HDR_SZ + (AHCI_MAX_SG * 16)
AHCI_CMD_TBL_SZ = 128 + (168 * 16)

I think if you add in AHCI_CMD_SLOT_SZ (1024) and AHCI_RX_FIS_SZ (256)
the DMA is 4K aligned, I think that is where the 168 came from.

Thanks,
David

Re: Regarding AHCI_MAX_SG and (ATA_HORKAGE_MAX_SEC_1024)

2016-08-10 Thread David Milburn 

Hi,

On 08/10/2016 10:14 AM, Tejun Heo wrote:

Hello, Tom.

On Wed, Aug 10, 2016 at 06:04:10PM +0800, Tom Yan wrote:

On 10 August 2016 at 11:26, Tejun Heo  wrote:

Hmmm.. why not?  The hardware limit is 64k and the driver is using a


Is that referring to the maximum number of entries allowed in the
PRDT, Physical Region Descriptor Table (which is, more precisely,
65535)?


Yeap.


Not necessarily.  A single sg entry can point to an area larger than
PAGE_SIZE.


You mean the 4MB limit of "Data Byte Count" in "DW3: Description
Information" of the PRDT? Is that what max_segment_size (which is set
to a general fallback of 65536:
http://lxr.free-electrons.com/ident?i=dma_get_max_seg_size) is about
in this case?


Ah, ahci isn't setting the hardware limit properly but yeah that's the
maximum segment size.


And my point was, it will be a multiple of 168 anyway, if 1344 is just
an example.


As written above, that probably makes the ahci command table size
nicely aligned.


I think that's what bothers me ultimately, cause I don't see how 168
makes it (more) nicely aligned (or even, aligned to what?).


Hmmm... Looked at the sizes and they don't seem to align to anything
meaningful.  No idea.


The 168 makes AHCI_CMD_TBL_SZ equal to 2816

AHCI_CMD_TBL_SZ = AHCI_CMD_TBL_HDR_SZ + (AHCI_MAX_SG * 16)
AHCI_CMD_TBL_SZ = 128 + (168 * 16)

I think if you add in AHCI_CMD_SLOT_SZ (1024) and AHCI_RX_FIS_SZ (256)
the DMA is 4K aligned, I think that is where the 168 came from.

Thanks,
David

Re: ata: BUG in ata_sff_hsm_move

2016-01-29 Thread David Milburn 

Hi Tejun,

Can ata_sff_hsm_move grab the lock and save off the task_state,
like this patch?

Thanks,
David

On Fri, Jan 29, 2016 at 02:40:33PM +0100, Dmitry Vyukov wrote:
> On Fri, Jan 29, 2016 at 2:18 PM, Dmitry Vyukov  wrote:
> > On Fri, Jan 29, 2016 at 1:23 PM, Tejun Heo  wrote:
> >> Hello, Dmitry.
> >>
> >> On Fri, Jan 29, 2016 at 12:59:49PM +0100, Dmitry Vyukov wrote:
> >>> > Hmmm... the port interrupt handler checks for IDLE before calling into
> >>> > hsm_move, so the only explanation would be that something is resetting
> >>> > it to IDLE inbetween.  ce7514526742 ("libata: prevent HSM state change
> >>> > race between ISR and PIO") describes and fixes the same problem.  The
> >>> > fix seems correct and I can't find anywhere else where this can
> >>> > happen.  :(
> >>> >
> >>> > Can you please post the kernel log leading to the BUG?  Also, I don't
> >>> > think that condition needs to be BUG.  I'll change it to WARN.
> >>>
> >>> Here are two logs, in both cases no kernel messages prior to the bug:
> >>> https://gist.githubusercontent.com/dvyukov/5087d633e3620280b6c7/raw/31c9ab1ced92ac5f85cfb15eaf48ec5793c2c3a1/gistfile1.txt
> >>> https://gist.githubusercontent.com/dvyukov/825b2e3d5fb80ae08a9a/raw/03c5a4f4c4bd9d0a304a71cda2da4c92f4b7f1ba/gistfile1.txt
> >>
> >> lol, this is kinda embarrassing.  It looks like the poll path wasn't
> >> doing any locking.  Can you please verify the following patch at least
> >> doesn't crash the machine immediately and if so keep it applied to the
> >> test kernel so that we can verify that the problem actually goes away?
> >
> >
> > Great that you managed to debug it without a repro!
> > I've applied this patch to my tree and will rerun fuzzer. I will
> > notify you if I see this warning again.
> > Thanks
> 
> It now deadlocks at this stack. It is probably not OK to call
> ata_sff_hsm_move holding the spinlock.
> 
> [8.168809] NMI backtrace for cpu 2
> [8.168809] CPU: 2 PID: 17 Comm: kworker/2:0 Not tainted 4.5.0-rc1+ #302
> [8.168809] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS Bochs 01/01/2011
> [8.168809] Workqueue: ata_sff ata_sff_pio_task
> [8.168809] task: 88006d24c740 ti: 88006d09 task.ti:
> 88006d09
> [8.168809] RIP: 0010:[]  []
> delay_tsc+0x18/0x70
> [8.168809] RSP: :88006d097a60  EFLAGS: 0086
> [8.168809] RAX: 8edaeede RBX: 880036e88028 RCX: 
> 001e
> [8.168809] RDX: 0033 RSI: 00338edaee9c RDI: 
> 0001
> [8.168809] RBP: 88006d097a60 R08: 0002 R09: 
> 
> [8.168809] R10: fbfff11baf82 R11: 1134edc5 R12: 
> 880036e88038
> [8.168809] R13: 9a9d2d40 R14: 880036e88030 R15: 
> 99383d98
> [8.168809] FS:  () GS:88006d60()
> knlGS:
> [8.168809] CS:  0010 DS:  ES:  CR0: 8005003b
> [8.168809] CR2:  CR3: 07a4b000 CR4: 
> 06e0
> [8.168809] Stack:
> [8.168809]  88006d097a70 82c0e2da 88006d097aa8
> 814661a9
> [8.168809]  880036e88028 0082 880036e90008
> 880036e90010
> [8.168809]  0001 88006d097ad0 86652c47
> 83a80c4c
> [8.168809] Call Trace:
> [8.168809]  [] __delay+0xa/0x10
> [8.168809]  [] do_raw_spin_lock+0x149/0x2b0
> [8.168809]  [] _raw_spin_lock_irqsave+0xa7/0xd0
> [8.168809]  [] ? ata_hsm_qc_complete+0x12c/0x420
> [8.168809]  [] ata_hsm_qc_complete+0x12c/0x420
> [8.168809]  [] ata_sff_hsm_move+0x1e2/0x1c60
> [8.168809]  [] ? ata_sff_pio_task+0x87/0x530
> [8.168809]  [] ata_sff_pio_task+0x406/0x530
> [8.168809]  [] process_one_work+0x796/0x1440
> [8.168809]  [] ? process_one_work+0x6ca/0x1440
> [8.168809]  [] ? finish_task_switch+0x120/0x5f0
> [8.168809]  [] ? pwq_dec_nr_in_flight+0x2e0/0x2e0
> [8.168809]  [] worker_thread+0xdb/0xfc0
> [8.168809]  [] kthread+0x23f/0x2d0
> [8.168809]  [] ? process_one_work+0x1440/0x1440
> [8.168809]  [] ? kthread_create_on_node+0x3b0/0x3b0
> [8.168809]  [] ? kthread_create_on_node+0x3b0/0x3b0
> [8.168809]  [] ret_from_fork+0x3f/0x70
> [8.168809]  [] ? kthread_create_on_node+0x3b0/0x3b0
> 
> 
> 
> 
> >> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> >> index 608677d..6991efc 100644
> >> --- a/drivers/ata/libata-sff.c
> >> +++ b/drivers/ata/libata-sff.c
> >> @@ -1362,12 +1362,14 @@ static void ata_sff_pio_task(struct work_struct 
> >> *work)
> >> u8 status;
> >> int poll_next;
> >>
> >> +   spin_lock_irq(ap->lock);
> >> +
> >> BUG_ON(ap->sff_pio_task_link == NULL);
> >> /* qc can be NULL if timeout occurred */
> >> qc = ata_qc_from_tag(ap, link->active_tag);
> >> if (!qc) {
> >> ap->sff_pio_task_link = NULL;
> >> -   return;
> >> +

Re: ata: BUG in ata_sff_hsm_move

2016-01-29 Thread David Milburn 

Hi Tejun,

Can ata_sff_hsm_move grab the lock and save off the task_state,
like this patch?

Thanks,
David

On Fri, Jan 29, 2016 at 02:40:33PM +0100, Dmitry Vyukov wrote:
> On Fri, Jan 29, 2016 at 2:18 PM, Dmitry Vyukov  wrote:
> > On Fri, Jan 29, 2016 at 1:23 PM, Tejun Heo  wrote:
> >> Hello, Dmitry.
> >>
> >> On Fri, Jan 29, 2016 at 12:59:49PM +0100, Dmitry Vyukov wrote:
> >>> > Hmmm... the port interrupt handler checks for IDLE before calling into
> >>> > hsm_move, so the only explanation would be that something is resetting
> >>> > it to IDLE inbetween.  ce7514526742 ("libata: prevent HSM state change
> >>> > race between ISR and PIO") describes and fixes the same problem.  The
> >>> > fix seems correct and I can't find anywhere else where this can
> >>> > happen.  :(
> >>> >
> >>> > Can you please post the kernel log leading to the BUG?  Also, I don't
> >>> > think that condition needs to be BUG.  I'll change it to WARN.
> >>>
> >>> Here are two logs, in both cases no kernel messages prior to the bug:
> >>> https://gist.githubusercontent.com/dvyukov/5087d633e3620280b6c7/raw/31c9ab1ced92ac5f85cfb15eaf48ec5793c2c3a1/gistfile1.txt
> >>> https://gist.githubusercontent.com/dvyukov/825b2e3d5fb80ae08a9a/raw/03c5a4f4c4bd9d0a304a71cda2da4c92f4b7f1ba/gistfile1.txt
> >>
> >> lol, this is kinda embarrassing.  It looks like the poll path wasn't
> >> doing any locking.  Can you please verify the following patch at least
> >> doesn't crash the machine immediately and if so keep it applied to the
> >> test kernel so that we can verify that the problem actually goes away?
> >
> >
> > Great that you managed to debug it without a repro!
> > I've applied this patch to my tree and will rerun fuzzer. I will
> > notify you if I see this warning again.
> > Thanks
> 
> It now deadlocks at this stack. It is probably not OK to call
> ata_sff_hsm_move holding the spinlock.
> 
> [8.168809] NMI backtrace for cpu 2
> [8.168809] CPU: 2 PID: 17 Comm: kworker/2:0 Not tainted 4.5.0-rc1+ #302
> [8.168809] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996),
> BIOS Bochs 01/01/2011
> [8.168809] Workqueue: ata_sff ata_sff_pio_task
> [8.168809] task: 88006d24c740 ti: 88006d09 task.ti:
> 88006d09
> [8.168809] RIP: 0010:[]  []
> delay_tsc+0x18/0x70
> [8.168809] RSP: :88006d097a60  EFLAGS: 0086
> [8.168809] RAX: 8edaeede RBX: 880036e88028 RCX: 
> 001e
> [8.168809] RDX: 0033 RSI: 00338edaee9c RDI: 
> 0001
> [8.168809] RBP: 88006d097a60 R08: 0002 R09: 
> 
> [8.168809] R10: fbfff11baf82 R11: 1134edc5 R12: 
> 880036e88038
> [8.168809] R13: 9a9d2d40 R14: 880036e88030 R15: 
> 99383d98
> [8.168809] FS:  () GS:88006d60()
> knlGS:
> [8.168809] CS:  0010 DS:  ES:  CR0: 8005003b
> [8.168809] CR2:  CR3: 07a4b000 CR4: 
> 06e0
> [8.168809] Stack:
> [8.168809]  88006d097a70 82c0e2da 88006d097aa8
> 814661a9
> [8.168809]  880036e88028 0082 880036e90008
> 880036e90010
> [8.168809]  0001 88006d097ad0 86652c47
> 83a80c4c
> [8.168809] Call Trace:
> [8.168809]  [] __delay+0xa/0x10
> [8.168809]  [] do_raw_spin_lock+0x149/0x2b0
> [8.168809]  [] _raw_spin_lock_irqsave+0xa7/0xd0
> [8.168809]  [] ? ata_hsm_qc_complete+0x12c/0x420
> [8.168809]  [] ata_hsm_qc_complete+0x12c/0x420
> [8.168809]  [] ata_sff_hsm_move+0x1e2/0x1c60
> [8.168809]  [] ? ata_sff_pio_task+0x87/0x530
> [8.168809]  [] ata_sff_pio_task+0x406/0x530
> [8.168809]  [] process_one_work+0x796/0x1440
> [8.168809]  [] ? process_one_work+0x6ca/0x1440
> [8.168809]  [] ? finish_task_switch+0x120/0x5f0
> [8.168809]  [] ? pwq_dec_nr_in_flight+0x2e0/0x2e0
> [8.168809]  [] worker_thread+0xdb/0xfc0
> [8.168809]  [] kthread+0x23f/0x2d0
> [8.168809]  [] ? process_one_work+0x1440/0x1440
> [8.168809]  [] ? kthread_create_on_node+0x3b0/0x3b0
> [8.168809]  [] ? kthread_create_on_node+0x3b0/0x3b0
> [8.168809]  [] ret_from_fork+0x3f/0x70
> [8.168809]  [] ? kthread_create_on_node+0x3b0/0x3b0
> 
> 
> 
> 
> >> diff --git a/drivers/ata/libata-sff.c b/drivers/ata/libata-sff.c
> >> index 608677d..6991efc 100644
> >> --- a/drivers/ata/libata-sff.c
> >> +++ b/drivers/ata/libata-sff.c
> >> @@ -1362,12 +1362,14 @@ static void ata_sff_pio_task(struct work_struct 
> >> *work)
> >> u8 status;
> >> int poll_next;
> >>
> >> +   spin_lock_irq(ap->lock);
> >> +
> >> BUG_ON(ap->sff_pio_task_link == NULL);
> >> /* qc can be NULL if timeout occurred */
> >> qc = ata_qc_from_tag(ap, link->active_tag);
> >> if (!qc) {
> >> ap->sff_pio_task_link = NULL;
> >> -

Re: BUG v3.19: reference count warning when removing scsi_debug device

2015-02-18 Thread David Milburn 

On 02/18/2015 07:22 AM, Tomas Winkler wrote:

reference count warning when removing scsi_debug device

ARNING: CPU: 2 PID: 16732 at kernel/module.c:954 module_put+0xc9/0xd0()
[150550.918033] Modules linked in: scsi_debug(-) pci_stub vboxpci(O)
vboxnetadp(O) vboxnetflt(O) vboxdrv(O) nfsv3 rfcomm bnep bluetooth
rpcsec_gss_krb5 nfsv4 hid_generic usbhid pl2303 hid usbserial
snd_hda_codec_hdmi i915 x86_pkg_temp_thermal coretemp kvm_intel
drm_kms_helper kvm drm nfsd snd_hda_codec_realtek
snd_hda_codec_generic ghash_clmulni_intel snd_hda_intel aesni_intel
aes_x86_64 ablk_helper cryptd snd_hda_controller snd_hda_codec
auth_rpcgss snd_hwdep snd_pcm oid_registry nfs_acl nfs snd_seq_midi
snd_seq_midi_event snd_rawmidi snd_seq lrw gf128mul snd_seq_device
snd_timer snd glue_helper microcode mei_me psmouse i2c_algo_bit mei
video tpm_infineon tpm_tis r8169 soundcore serio_raw mii lockd lpc_ich
mac_hid grace sunrpc fscache binfmt_misc nls_iso8859_1 parport_pc
ppdev lp parport [last unloaded: scsi_debug]
[150550.918069] CPU: 2 PID: 16732 Comm: modprobe Tainted: GW
IO   3.19.0-rc5+ #1
[150550.918070] Hardware name: Gigabyte Technology Co., Ltd.
H87M-D3H/H87M-D3H, BIOS F6 08/03/2013
[150550.918071]  81a85373 880229f9bb88 8175a6cb

[150550.918072]   880229f9bbc8 8105382a
81ccf320
[150550.918074]  a070fba0 a070fba0 88022a136010
00800010
[150550.918076] Call Trace:
[150550.918081]  [] dump_stack+0x4c/0x65
[150550.918084]  [] warn_slowpath_common+0x8a/0xc0
[150550.918086]  [] warn_slowpath_null+0x1a/0x20
[150550.918088]  [] module_put+0xc9/0xd0
[150550.918090]  [] scsi_device_put+0x48/0x50
[150550.918092]  [] scsi_disk_put+0x32/0x50
[150550.918093]  [] sd_shutdown+0x8c/0x150
[150550.918095]  [] sd_remove+0x69/0xc0
[150550.918097]  [] __device_release_driver+0x7f/0xf0
[150550.918099]  [] device_release_driver+0x25/0x40
[150550.918101]  [] bus_remove_device+0x124/0x1b0
[150550.918103]  [] device_del+0x13e/0x250
[150550.918105]  [] __scsi_remove_device+0xcd/0xe0
[150550.918107]  [] scsi_forget_host+0x6f/0x80
[150550.918108]  [] scsi_remove_host+0x86/0x140
[150550.918112]  [] sdebug_driver_remove+0x29/0x90
[scsi_debug]
[150550.918113]  [] __device_release_driver+0x7f/0xf0
[150550.918114]  [] device_release_driver+0x25/0x40
[150550.918116]  [] bus_remove_device+0x124/0x1b0
[150550.918117]  [] device_del+0x13e/0x250
[150550.918119]  [] device_unregister+0x22/0x70
[150550.918121]  [] sdebug_remove_adapter+0x50/0x80
[scsi_debug]
[150550.918123]  [] scsi_debug_exit+0x84/0x85f [scsi_debug]
[150550.918125]  [] SyS_delete_module+0x18c/0x210
[150550.918129]  [] ? int_with_check+0x27/0x69
[150550.918131]  [] system_call_fastpath+0x12/0x17
[150550.918132] ---[ end trace 1f300c62b0658728 ]---
[150550.935994] sd 7:0:0:1: [sdc] Synchronizing SCSI cache
[150550.936017] [ cut here ]
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Hi Tomas,

I think that was fixed 3.19-rc7

commit dc4515ea26d6c7fed3d978cd2bd36adc0d057bc5
Author: Rusty Russell 
Date:   Fri Jan 23 13:22:47 2015 +1030

scsi: always increment reference count


Thanks,
David


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

Re: BUG v3.19: reference count warning when removing scsi_debug device

2015-02-18 Thread David Milburn 

On 02/18/2015 07:22 AM, Tomas Winkler wrote:

reference count warning when removing scsi_debug device

ARNING: CPU: 2 PID: 16732 at kernel/module.c:954 module_put+0xc9/0xd0()
[150550.918033] Modules linked in: scsi_debug(-) pci_stub vboxpci(O)
vboxnetadp(O) vboxnetflt(O) vboxdrv(O) nfsv3 rfcomm bnep bluetooth
rpcsec_gss_krb5 nfsv4 hid_generic usbhid pl2303 hid usbserial
snd_hda_codec_hdmi i915 x86_pkg_temp_thermal coretemp kvm_intel
drm_kms_helper kvm drm nfsd snd_hda_codec_realtek
snd_hda_codec_generic ghash_clmulni_intel snd_hda_intel aesni_intel
aes_x86_64 ablk_helper cryptd snd_hda_controller snd_hda_codec
auth_rpcgss snd_hwdep snd_pcm oid_registry nfs_acl nfs snd_seq_midi
snd_seq_midi_event snd_rawmidi snd_seq lrw gf128mul snd_seq_device
snd_timer snd glue_helper microcode mei_me psmouse i2c_algo_bit mei
video tpm_infineon tpm_tis r8169 soundcore serio_raw mii lockd lpc_ich
mac_hid grace sunrpc fscache binfmt_misc nls_iso8859_1 parport_pc
ppdev lp parport [last unloaded: scsi_debug]
[150550.918069] CPU: 2 PID: 16732 Comm: modprobe Tainted: GW
IO   3.19.0-rc5+ #1
[150550.918070] Hardware name: Gigabyte Technology Co., Ltd.
H87M-D3H/H87M-D3H, BIOS F6 08/03/2013
[150550.918071]  81a85373 880229f9bb88 8175a6cb

[150550.918072]   880229f9bbc8 8105382a
81ccf320
[150550.918074]  a070fba0 a070fba0 88022a136010
00800010
[150550.918076] Call Trace:
[150550.918081]  [8175a6cb] dump_stack+0x4c/0x65
[150550.918084]  [8105382a] warn_slowpath_common+0x8a/0xc0
[150550.918086]  [8105391a] warn_slowpath_null+0x1a/0x20
[150550.918088]  [810d6879] module_put+0xc9/0xd0
[150550.918090]  [814f6e18] scsi_device_put+0x48/0x50
[150550.918092]  [8150e5c2] scsi_disk_put+0x32/0x50
[150550.918093]  [8150f8cc] sd_shutdown+0x8c/0x150
[150550.918095]  [8150f9f9] sd_remove+0x69/0xc0
[150550.918097]  [814b83ff] __device_release_driver+0x7f/0xf0
[150550.918099]  [814b8495] device_release_driver+0x25/0x40
[150550.918101]  [814b7d34] bus_remove_device+0x124/0x1b0
[150550.918103]  [814b43de] device_del+0x13e/0x250
[150550.918105]  [8150796d] __scsi_remove_device+0xcd/0xe0
[150550.918107]  [81505f4f] scsi_forget_host+0x6f/0x80
[150550.918108]  [814f85a6] scsi_remove_host+0x86/0x140
[150550.918112]  [a0706a29] sdebug_driver_remove+0x29/0x90
[scsi_debug]
[150550.918113]  [814b83ff] __device_release_driver+0x7f/0xf0
[150550.918114]  [814b8495] device_release_driver+0x25/0x40
[150550.918116]  [814b7d34] bus_remove_device+0x124/0x1b0
[150550.918117]  [814b43de] device_del+0x13e/0x250
[150550.918119]  [814b4512] device_unregister+0x22/0x70
[150550.918121]  [a0706390] sdebug_remove_adapter+0x50/0x80
[scsi_debug]
[150550.918123]  [a070b825] scsi_debug_exit+0x84/0x85f [scsi_debug]
[150550.918125]  [810d8bac] SyS_delete_module+0x18c/0x210
[150550.918129]  [81763fa7] ? int_with_check+0x27/0x69
[150550.918131]  [81763d92] system_call_fastpath+0x12/0x17
[150550.918132] ---[ end trace 1f300c62b0658728 ]---
[150550.935994] sd 7:0:0:1: [sdc] Synchronizing SCSI cache
[150550.936017] [ cut here ]
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Hi Tomas,

I think that was fixed 3.19-rc7

commit dc4515ea26d6c7fed3d978cd2bd36adc0d057bc5
Author: Rusty Russell ru...@rustcorp.com.au
Date:   Fri Jan 23 13:22:47 2015 +1030

scsi: always increment reference count


Thanks,
David


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] ata: Check and set 64-bit DMA mask for platform AHCI driver

2014-06-10 Thread David Milburn 

On 05/23/2014 12:35 PM, suravee.suthikulpa...@amd.com wrote:

From: Suravee Suthikulpanit 

The current platform AHCI drier does not set the dma_mask correctly
for 64-bit DMA capable AHCI controller.  This patch checks the AHCI
capability bit and set the dma_mask and coherent_dma_mask accordingly.

Signed-off-by: Suravee Suthikulpanit 
---
  drivers/ata/libahci_platform.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 7cb3a85..85049ef 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -368,6 +368,15 @@ int ahci_platform_init_host(struct platform_device *pdev,


Hi Suravee,

Would it be better to do the following before ahci_reset_controller()?

/* initialize adapter */
rc = ahci_configure_dma_masks(pdev, hpriv->cap & HOST_CAP_64);
if (rc)
return rc;


Thanks,
David




ahci_init_controller(host);
ahci_print_info(host, "platform");

+   if (hpriv->cap & HOST_CAP_64) {
+   if (!dev->dma_mask)
+   dev->dma_mask = >coherent_dma_mask;
+
+   rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+   if (rc)
+   return rc;
+   }
+
return ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
 _platform_sht);
  }



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

Re: [PATCH 1/1] ata: Check and set 64-bit DMA mask for platform AHCI driver

2014-06-10 Thread David Milburn 

On 05/23/2014 12:35 PM, suravee.suthikulpa...@amd.com wrote:

From: Suravee Suthikulpanit suravee.suthikulpa...@amd.com

The current platform AHCI drier does not set the dma_mask correctly
for 64-bit DMA capable AHCI controller.  This patch checks the AHCI
capability bit and set the dma_mask and coherent_dma_mask accordingly.

Signed-off-by: Suravee Suthikulpanit suravee.suthikulpa...@amd.com
---
  drivers/ata/libahci_platform.c | 9 +
  1 file changed, 9 insertions(+)

diff --git a/drivers/ata/libahci_platform.c b/drivers/ata/libahci_platform.c
index 7cb3a85..85049ef 100644
--- a/drivers/ata/libahci_platform.c
+++ b/drivers/ata/libahci_platform.c
@@ -368,6 +368,15 @@ int ahci_platform_init_host(struct platform_device *pdev,


Hi Suravee,

Would it be better to do the following before ahci_reset_controller()?

/* initialize adapter */
rc = ahci_configure_dma_masks(pdev, hpriv-cap  HOST_CAP_64);
if (rc)
return rc;


Thanks,
David




ahci_init_controller(host);
ahci_print_info(host, platform);

+   if (hpriv-cap  HOST_CAP_64) {
+   if (!dev-dma_mask)
+   dev-dma_mask = dev-coherent_dma_mask;
+
+   rc = dma_set_mask_and_coherent(dev, DMA_BIT_MASK(64));
+   if (rc)
+   return rc;
+   }
+
return ata_host_activate(host, irq, ahci_interrupt, IRQF_SHARED,
 ahci_platform_sht);
  }



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH 1/1] ahci: Do not receive interrupts sent by dummy ports

2014-04-17 Thread David Milburn 

On 04/17/2014 12:06 PM, Tejun Heo wrote:

On Thu, Apr 17, 2014 at 06:06:15PM +0200, Alexander Gordeev wrote:

In multiple MSI mode all AHCI ports (including dummy) get
assigned separate MSI vectors and (as result of execution
pci_enable_msi_exact() function) separate IRQ numbers,
(mapped to the MSI vectors).

Therefore, although interrupts from dummy ports are not
desired they are still enabled. We do not request IRQs
for dummy ports, but that only means we do not assign
AHCI-specific ISRs to corresponding IRQ numbers.

As result, dummy port interrupts still could come and
traverse all the way from the PCI device to the kernel,
causing unnecessary overhead.

This update disables IRQs for dummy ports and prevents
the described issue.

Signed-off-by: Alexander Gordeev 
Cc: Tejun Heo 
Cc: David Milburn 
Cc: linux-...@vger.kernel.org


David, can you please test the patch?




Hi,

I have re-tested successfully, this patch prevents the crash
when using kdump, and I boot tested a system that boots off
ahci and has dummy ports present, no problems seen.

Thanks,
David


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

Re: [PATCH 1/1] ahci: Do not receive interrupts sent by dummy ports

2014-04-17 Thread David Milburn 

On 04/17/2014 12:06 PM, Tejun Heo wrote:

On Thu, Apr 17, 2014 at 06:06:15PM +0200, Alexander Gordeev wrote:

In multiple MSI mode all AHCI ports (including dummy) get
assigned separate MSI vectors and (as result of execution
pci_enable_msi_exact() function) separate IRQ numbers,
(mapped to the MSI vectors).

Therefore, although interrupts from dummy ports are not
desired they are still enabled. We do not request IRQs
for dummy ports, but that only means we do not assign
AHCI-specific ISRs to corresponding IRQ numbers.

As result, dummy port interrupts still could come and
traverse all the way from the PCI device to the kernel,
causing unnecessary overhead.

This update disables IRQs for dummy ports and prevents
the described issue.

Signed-off-by: Alexander Gordeev agord...@redhat.com
Cc: Tejun Heo t...@kernel.org
Cc: David Milburn dmilb...@redhat.com
Cc: linux-...@vger.kernel.org


David, can you please test the patch?




Hi,

I have re-tested successfully, this patch prevents the crash
when using kdump, and I boot tested a system that boots off
ahci and has dummy ports present, no problems seen.

Thanks,
David


--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] bio_integrity_add_page: check for BIO_POOL_NONE before determining nr_vecs on slab

2014-02-05 Thread David Milburn 

On 02/03/2014 09:55 PM, Martin K. Petersen wrote:

"David" == David Milburn  writes:


David> When enabling DIX T10-DIF-TYPE1-IP protection you can hit the
David> bip_vec full condition which fails to attach the integrity
David> metadata and returns 0 back to bio_integrity_prep()

Looks like Kent accidentally broke this when he changed the bvec pool
setup.

David> - if (bip->bip_vcnt >= bvec_nr_vecs(bip->bip_slab)) {
David> + if (bip->bip_slab != BIO_POOL_NONE &&
David> + bip->bip_vcnt >= bvec_nr_vecs(bip->bip_slab)) {
David>   printk(KERN_ERR "%s: bip_vec full\n", __func__);
David>   return 0;
David>   }

We still need to check that the page will actually fit, though:


block: Fix nr_vecs for inline integrity vectors

Commit 9f060e2231ca changed the way we handle allocations for the
integrity vectors. When the vectors are inline there is no associated
slab and consequently bvec_nr_vecs() returns 0. Ensure that we check
against BIP_INLINE_VECS in that case.

Reported-by: David Milburn 
Signed-off-by: Martin K. Petersen 

diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index fc60b31453ee..6dea2b90b4d5 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -114,6 +114,14 @@ void bio_integrity_free(struct bio *bio)
  }
  EXPORT_SYMBOL(bio_integrity_free);

+static inline unsigned int bip_integrity_vecs(struct bio_integrity_payload 
*bip)
+{
+   if (bip->bip_slab == BIO_POOL_NONE)
+   return BIP_INLINE_VECS;
+
+   return bvec_nr_vecs(bip->bip_slab);
+}
+
  /**
   * bio_integrity_add_page - Attach integrity metadata
   * @bio:  bio to update
@@ -129,7 +137,7 @@ int bio_integrity_add_page(struct bio *bio, struct page 
*page,
struct bio_integrity_payload *bip = bio->bi_integrity;
struct bio_vec *iv;

-   if (bip->bip_vcnt >= bvec_nr_vecs(bip->bip_slab)) {
+   if (bip->bip_vcnt >= bip_integrity_vecs(bip)) {
printk(KERN_ERR "%s: bip_vec full\n", __func__);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Hi Martin,

Your patch has been tested successfully.

Thanks for your help,
David

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

Re: [PATCH] bio_integrity_add_page: check for BIO_POOL_NONE before determining nr_vecs on slab

2014-02-05 Thread David Milburn 

On 02/03/2014 09:55 PM, Martin K. Petersen wrote:

David == David Milburn dmilb...@redhat.com writes:


David When enabling DIX T10-DIF-TYPE1-IP protection you can hit the
David bip_vec full condition which fails to attach the integrity
David metadata and returns 0 back to bio_integrity_prep()

Looks like Kent accidentally broke this when he changed the bvec pool
setup.

David - if (bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
David + if (bip-bip_slab != BIO_POOL_NONE 
David + bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
David   printk(KERN_ERR %s: bip_vec full\n, __func__);
David   return 0;
David   }

We still need to check that the page will actually fit, though:


block: Fix nr_vecs for inline integrity vectors

Commit 9f060e2231ca changed the way we handle allocations for the
integrity vectors. When the vectors are inline there is no associated
slab and consequently bvec_nr_vecs() returns 0. Ensure that we check
against BIP_INLINE_VECS in that case.

Reported-by: David Milburn dmilb...@redhat.com
Signed-off-by: Martin K. Petersen martin.peter...@oracle.com

diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index fc60b31453ee..6dea2b90b4d5 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -114,6 +114,14 @@ void bio_integrity_free(struct bio *bio)
  }
  EXPORT_SYMBOL(bio_integrity_free);

+static inline unsigned int bip_integrity_vecs(struct bio_integrity_payload 
*bip)
+{
+   if (bip-bip_slab == BIO_POOL_NONE)
+   return BIP_INLINE_VECS;
+
+   return bvec_nr_vecs(bip-bip_slab);
+}
+
  /**
   * bio_integrity_add_page - Attach integrity metadata
   * @bio:  bio to update
@@ -129,7 +137,7 @@ int bio_integrity_add_page(struct bio *bio, struct page 
*page,
struct bio_integrity_payload *bip = bio-bi_integrity;
struct bio_vec *iv;

-   if (bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
+   if (bip-bip_vcnt = bip_integrity_vecs(bip)) {
printk(KERN_ERR %s: bip_vec full\n, __func__);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/



Hi Martin,

Your patch has been tested successfully.

Thanks for your help,
David

--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] bio_integrity_add_page: check for BIO_POOL_NONE before determining nr_vecs on slab

2014-01-29 Thread David Milburn 
When enabling DIX T10-DIF-TYPE1-IP protection you can hit the
bip_vec full condition which fails to attach the integrity metadata
and returns 0 back to bio_integrity_prep()

[3.837493] sd 0:0:17:1089486880: [sda] Enabling DIX T10-DIF-TYPE1-IP 
protection
[3.839089] sd 0:0:17:1089486880: [sda] Attached SCSI disk
[3.841309] bio_integrity_add_page: bip_vec full


[  439.180928] bio_integrity_prep: sectors 8 len 64
[  439.180929] bio_integrity_prep: nr_pages 1 end 706577 start 706576
[  439.180930] bio_integrity_alloc: bs b2c87880 nr_vecs 1
[  439.180931] bio_integrity_alloc: nr_vecs 1 inline_vecs 4
[  439.180932] bio_integrity_alloc: bip->bip_vec aeaf9158 bip->bip_slab 
15
[  439.180933] bio_integrity_prep: offset 704 nr_pages 1
[  439.180934] bio_integrity_add_page: len 64 offset 704
[  439.180935] bio_integrity_add_page: bip_vec full bip_vcnt 0 bvec_nr_vecs 0 
bip_slab 15


Ultimately you can BUG_ON(!nents) in scsi_alloc_sgtable()

scsi_init_io
 blk_integrity_rq
  scsi_alloc_sgtable

With attached patch device functions normally


[  198.481838] sd 0:0:24:1089486880: [sda] Enabling DIF Type 1 protection
[  198.481845] sd 0:0:24:1089486880: [sda] 209715200 512-byte logical blocks: 
(107 GB/100 GiB)
[  198.482552] sd 0:0:24:1089486880: [sda] Write Protect is off
[  198.482556] sd 0:0:24:1089486880: [sda] Mode Sense: ed 00 00 08
[  198.482889] sd 0:0:24:1089486880: [sda] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  198.485850]  sda: sda1
[  198.485926] sd 0:0:24:1089486880: [sda] Enabling DIX T10-DIF-TYPE1-IP 
protection
[  198.487652] sd 0:0:24:1089486880: [sda] Attached SCSI disk
[  276.566682] XFS (sda1): Mounting Filesystem
[  276.576558] XFS (sda1): Ending clean mount


Signed-off-by: David Milburn 
---
 fs/bio-integrity.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index 45e944f..9d36a09 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -129,7 +129,8 @@ int bio_integrity_add_page(struct bio *bio, struct page 
*page,
struct bio_integrity_payload *bip = bio->bi_integrity;
struct bio_vec *iv;
 
-   if (bip->bip_vcnt >= bvec_nr_vecs(bip->bip_slab)) {
+   if (bip->bip_slab != BIO_POOL_NONE && 
+   bip->bip_vcnt >= bvec_nr_vecs(bip->bip_slab)) {
printk(KERN_ERR "%s: bip_vec full\n", __func__);
return 0;
}
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] bio_integrity_add_page: check for BIO_POOL_NONE before determining nr_vecs on slab

2014-01-29 Thread David Milburn 
When enabling DIX T10-DIF-TYPE1-IP protection you can hit the
bip_vec full condition which fails to attach the integrity metadata
and returns 0 back to bio_integrity_prep()

[3.837493] sd 0:0:17:1089486880: [sda] Enabling DIX T10-DIF-TYPE1-IP 
protection
[3.839089] sd 0:0:17:1089486880: [sda] Attached SCSI disk
[3.841309] bio_integrity_add_page: bip_vec full

debug
[  439.180928] bio_integrity_prep: sectors 8 len 64
[  439.180929] bio_integrity_prep: nr_pages 1 end 706577 start 706576
[  439.180930] bio_integrity_alloc: bs b2c87880 nr_vecs 1
[  439.180931] bio_integrity_alloc: nr_vecs 1 inline_vecs 4
[  439.180932] bio_integrity_alloc: bip-bip_vec aeaf9158 bip-bip_slab 
15
[  439.180933] bio_integrity_prep: offset 704 nr_pages 1
[  439.180934] bio_integrity_add_page: len 64 offset 704
[  439.180935] bio_integrity_add_page: bip_vec full bip_vcnt 0 bvec_nr_vecs 0 
bip_slab 15
debug

Ultimately you can BUG_ON(!nents) in scsi_alloc_sgtable()

scsi_init_io
 blk_integrity_rq
  scsi_alloc_sgtable

With attached patch device functions normally

dmesg
[  198.481838] sd 0:0:24:1089486880: [sda] Enabling DIF Type 1 protection
[  198.481845] sd 0:0:24:1089486880: [sda] 209715200 512-byte logical blocks: 
(107 GB/100 GiB)
[  198.482552] sd 0:0:24:1089486880: [sda] Write Protect is off
[  198.482556] sd 0:0:24:1089486880: [sda] Mode Sense: ed 00 00 08
[  198.482889] sd 0:0:24:1089486880: [sda] Write cache: enabled, read cache: 
enabled, doesn't support DPO or FUA
[  198.485850]  sda: sda1
[  198.485926] sd 0:0:24:1089486880: [sda] Enabling DIX T10-DIF-TYPE1-IP 
protection
[  198.487652] sd 0:0:24:1089486880: [sda] Attached SCSI disk
[  276.566682] XFS (sda1): Mounting Filesystem
[  276.576558] XFS (sda1): Ending clean mount
dmesg

Signed-off-by: David Milburn dmilb...@redhat.com
---
 fs/bio-integrity.c |3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/fs/bio-integrity.c b/fs/bio-integrity.c
index 45e944f..9d36a09 100644
--- a/fs/bio-integrity.c
+++ b/fs/bio-integrity.c
@@ -129,7 +129,8 @@ int bio_integrity_add_page(struct bio *bio, struct page 
*page,
struct bio_integrity_payload *bip = bio-bi_integrity;
struct bio_vec *iv;
 
-   if (bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
+   if (bip-bip_slab != BIO_POOL_NONE  
+   bip-bip_vcnt = bvec_nr_vecs(bip-bip_slab)) {
printk(KERN_ERR %s: bip_vec full\n, __func__);
return 0;
}
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mtip32xx: Add SRSI support

2013-09-03 Thread David Milburn 

Asai Thambi S P wrote:

This patch add support for SRSI(Surprise Removal Surprise Insertion).

Approach:
-
Surprise Removal:
-
On surprise removal of the device, gendisk, request queue, device index, sysfs
entries, etc are retained as long as device is in use - mounted filesystem,
device opened by an application, etc. The service thread breaks out of the main
while loop, waits for pci remove to exit, and then waits for device to become
free. When there no holders of the device, service thread cleans up the block
and device related stuff and returns.

Surprise Insertion:
---
No change, this scenario follows the normal pci probe() function flow.

Signed-off-by: Asai Thambi S P 
---
 drivers/block/mtip32xx/mtip32xx.c |  453 ++---
 drivers/block/mtip32xx/mtip32xx.h |   18 +-
 2 files changed, 289 insertions(+), 182 deletions(-)



Hi Jens,

Will you be considering this patch for 3.12?

Thanks,
David
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH] mtip32xx: Add SRSI support

2013-09-03 Thread David Milburn 

Asai Thambi S P wrote:

This patch add support for SRSI(Surprise Removal Surprise Insertion).

Approach:
-
Surprise Removal:
-
On surprise removal of the device, gendisk, request queue, device index, sysfs
entries, etc are retained as long as device is in use - mounted filesystem,
device opened by an application, etc. The service thread breaks out of the main
while loop, waits for pci remove to exit, and then waits for device to become
free. When there no holders of the device, service thread cleans up the block
and device related stuff and returns.

Surprise Insertion:
---
No change, this scenario follows the normal pci probe() function flow.

Signed-off-by: Asai Thambi S P asamymuth...@micron.com
---
 drivers/block/mtip32xx/mtip32xx.c |  453 ++---
 drivers/block/mtip32xx/mtip32xx.h |   18 +-
 2 files changed, 289 insertions(+), 182 deletions(-)



Hi Jens,

Will you be considering this patch for 3.12?

Thanks,
David
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Re: [PATCH v2] [SCSI] sg: Fix user memory corruption when SG_IO is interrupted by a signal

2013-08-07 Thread David Milburn 

Roland Dreier wrote:

On Wed, Aug 7, 2013 at 7:38 AM, David Milburn  wrote:

I was able to succesfully test this patch overnight, I had been experimenting 
with the
sg driver setting the BIO_NULL_MAPPED flag in sg_rq_end_io_usercontext for a 
orphan process
which prevented the corruption, but your solution seems much better.


Very cool, thanks for the testing.

I actually looked at using BIO_NULL_MAPPED as well, but it seemed a
bit too fragile to me -- it had the right effect of skipping
__bio_copy_iov(), and skipping the __free_pages() stuff in there is OK
because sg owns its pages rather than the bio layer, but all that
seemed vulnerable to being broken by an unrelated change.

Out of curiousity, were you already working on this bug?  Because if
you had fixed it a few weeks earlier we might not have spent so long
wondering WTF was stomping on the memory of one of our processes :)



Hi Roland,

Actually, I was waiting for confirmation from the field which I
recently received, I was getting ready to bring this up on linux-scsi,
sorry I should have brought it up sooner. I wasn't positive that setting
BIO_NULL_MAPPED flag from sg driver was the fix. David Jeffery
came up with a reproducer which I ran overnight on the latest
upstream kernel with your patch.

Thanks,
David



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

Re: [PATCH v2] [SCSI] sg: Fix user memory corruption when SG_IO is interrupted by a signal

2013-08-07 Thread David Milburn 

Roland Dreier wrote:

From: Roland Dreier 

There is a nasty bug in the SCSI SG_IO ioctl that in some circumstances
leads to one process writing data into the address space of some other
random unrelated process if the ioctl is interrupted by a signal.
What happens is the following:

 - A process issues an SG_IO ioctl with direction DXFER_FROM_DEV (ie the
   underlying SCSI command will transfer data from the SCSI device to
   the buffer provided in the ioctl)

 - Before the command finishes, a signal is sent to the process waiting
   in the ioctl.  This will end up waking up the sg_ioctl() code:

result = wait_event_interruptible(sfp->read_wait,
(srp_done(sfp, srp) || sdp->detached));

   but neither srp_done() nor sdp->detached is true, so we end up just
   setting srp->orphan and returning to userspace:

srp->orphan = 1;
write_unlock_irq(>rq_list_lock);
return result;  /* -ERESTARTSYS because signal hit process */

   At this point the original process is done with the ioctl and
   blithely goes ahead handling the signal, reissuing the ioctl, etc.

 - Eventually, the SCSI command issued by the first ioctl finishes and
   ends up in sg_rq_end_io().  At the end of that function, we run through:

write_lock_irqsave(>rq_list_lock, iflags);
if (unlikely(srp->orphan)) {
if (sfp->keep_orphan)
srp->sg_io_owned = 0;
else
done = 0;
}
srp->done = done;
write_unlock_irqrestore(>rq_list_lock, iflags);

if (likely(done)) {
/* Now wake up any sg_read() that is waiting for this
 * packet.
 */
wake_up_interruptible(>read_wait);
kill_fasync(>async_qp, SIGPOLL, POLL_IN);
kref_put(>f_ref, sg_remove_sfp);
} else {
INIT_WORK(>ew.work, sg_rq_end_io_usercontext);
schedule_work(>ew.work);
}

   Since srp->orphan *is* set, we set done to 0 (assuming the
   userspace app has not set keep_orphan via an SG_SET_KEEP_ORPHAN
   ioctl), and therefore we end up scheduling sg_rq_end_io_usercontext()
   to run in a workqueue.

 - In workqueue context we go through sg_rq_end_io_usercontext() ->
   sg_finish_rem_req() -> blk_rq_unmap_user() -> ... ->
   bio_uncopy_user() -> __bio_copy_iov() -> copy_to_user().

   The key point here is that we are doing copy_to_user() on a
   workqueue -- that is, we're on a kernel thread with current->mm
   equal to whatever random previous user process was scheduled before
   this kernel thread.  So we end up copying whatever data the SCSI
   command returned to the virtual address of the buffer passed into
   the original ioctl, but it's quite likely we do this copying into a
   different address space!

As suggested by James Bottomley ,
add a check for current->mm (which is NULL if we're on a kernel thread
without a real userspace address space) in bio_uncopy_user(), and skip
the copy if we're on a kernel thread.

There's no reason that I can think of for any caller of bio_uncopy_user()
to want to do copying on a kernel thread with a random active userspace
address space.

Huge thanks to Costa Sapuntzakis  for the
original pointer to this bug in the sg code.

Signed-off-by: Roland Dreier 
Cc: 
---
 fs/bio.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/bio.c b/fs/bio.c
index 94bbc04..c5eae72 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -1045,12 +1045,22 @@ static int __bio_copy_iov(struct bio *bio, struct 
bio_vec *iovecs,
 int bio_uncopy_user(struct bio *bio)
 {
struct bio_map_data *bmd = bio->bi_private;
-   int ret = 0;
+   struct bio_vec *bvec;
+   int ret = 0, i;
 
-	if (!bio_flagged(bio, BIO_NULL_MAPPED))

-   ret = __bio_copy_iov(bio, bmd->iovecs, bmd->sgvecs,
-bmd->nr_sgvecs, bio_data_dir(bio) == READ,
-0, bmd->is_our_pages);
+   if (!bio_flagged(bio, BIO_NULL_MAPPED)) {
+   /*
+* if we're in a workqueue, the request is orphaned, so
+* don't copy into a random user address space, just free.
+*/
+   if (current->mm)
+   ret = __bio_copy_iov(bio, bmd->iovecs, bmd->sgvecs,
+bmd->nr_sgvecs, bio_data_dir(bio) 
== READ,
+0, bmd->is_our_pages);
+   else if (bmd->is_our_pages)
+   bio_for_each_segment_all(bvec, bio, i)
+   __free_page(bvec->bv_page);
+   }
bio_free_map_data(bmd);
bio_put(bio);
return ret;


Hi Roland,

I was able to succesfully test this patch overnight, I had been 
experimenting with the
sg driver setting the

Re: [PATCH v2] [SCSI] sg: Fix user memory corruption when SG_IO is interrupted by a signal

2013-08-07 Thread David Milburn 

Roland Dreier wrote:

From: Roland Dreier rol...@purestorage.com

There is a nasty bug in the SCSI SG_IO ioctl that in some circumstances
leads to one process writing data into the address space of some other
random unrelated process if the ioctl is interrupted by a signal.
What happens is the following:

 - A process issues an SG_IO ioctl with direction DXFER_FROM_DEV (ie the
   underlying SCSI command will transfer data from the SCSI device to
   the buffer provided in the ioctl)

 - Before the command finishes, a signal is sent to the process waiting
   in the ioctl.  This will end up waking up the sg_ioctl() code:

result = wait_event_interruptible(sfp-read_wait,
(srp_done(sfp, srp) || sdp-detached));

   but neither srp_done() nor sdp-detached is true, so we end up just
   setting srp-orphan and returning to userspace:

srp-orphan = 1;
write_unlock_irq(sfp-rq_list_lock);
return result;  /* -ERESTARTSYS because signal hit process */

   At this point the original process is done with the ioctl and
   blithely goes ahead handling the signal, reissuing the ioctl, etc.

 - Eventually, the SCSI command issued by the first ioctl finishes and
   ends up in sg_rq_end_io().  At the end of that function, we run through:

write_lock_irqsave(sfp-rq_list_lock, iflags);
if (unlikely(srp-orphan)) {
if (sfp-keep_orphan)
srp-sg_io_owned = 0;
else
done = 0;
}
srp-done = done;
write_unlock_irqrestore(sfp-rq_list_lock, iflags);

if (likely(done)) {
/* Now wake up any sg_read() that is waiting for this
 * packet.
 */
wake_up_interruptible(sfp-read_wait);
kill_fasync(sfp-async_qp, SIGPOLL, POLL_IN);
kref_put(sfp-f_ref, sg_remove_sfp);
} else {
INIT_WORK(srp-ew.work, sg_rq_end_io_usercontext);
schedule_work(srp-ew.work);
}

   Since srp-orphan *is* set, we set done to 0 (assuming the
   userspace app has not set keep_orphan via an SG_SET_KEEP_ORPHAN
   ioctl), and therefore we end up scheduling sg_rq_end_io_usercontext()
   to run in a workqueue.

 - In workqueue context we go through sg_rq_end_io_usercontext() -
   sg_finish_rem_req() - blk_rq_unmap_user() - ... -
   bio_uncopy_user() - __bio_copy_iov() - copy_to_user().

   The key point here is that we are doing copy_to_user() on a
   workqueue -- that is, we're on a kernel thread with current-mm
   equal to whatever random previous user process was scheduled before
   this kernel thread.  So we end up copying whatever data the SCSI
   command returned to the virtual address of the buffer passed into
   the original ioctl, but it's quite likely we do this copying into a
   different address space!

As suggested by James Bottomley james.bottom...@hansenpartnership.com,
add a check for current-mm (which is NULL if we're on a kernel thread
without a real userspace address space) in bio_uncopy_user(), and skip
the copy if we're on a kernel thread.

There's no reason that I can think of for any caller of bio_uncopy_user()
to want to do copying on a kernel thread with a random active userspace
address space.

Huge thanks to Costa Sapuntzakis co...@purestorage.com for the
original pointer to this bug in the sg code.

Signed-off-by: Roland Dreier rol...@purestorage.com
Cc: sta...@vger.kernel.org
---
 fs/bio.c | 20 +++-
 1 file changed, 15 insertions(+), 5 deletions(-)

diff --git a/fs/bio.c b/fs/bio.c
index 94bbc04..c5eae72 100644
--- a/fs/bio.c
+++ b/fs/bio.c
@@ -1045,12 +1045,22 @@ static int __bio_copy_iov(struct bio *bio, struct 
bio_vec *iovecs,
 int bio_uncopy_user(struct bio *bio)
 {
struct bio_map_data *bmd = bio-bi_private;
-   int ret = 0;
+   struct bio_vec *bvec;
+   int ret = 0, i;
 
-	if (!bio_flagged(bio, BIO_NULL_MAPPED))

-   ret = __bio_copy_iov(bio, bmd-iovecs, bmd-sgvecs,
-bmd-nr_sgvecs, bio_data_dir(bio) == READ,
-0, bmd-is_our_pages);
+   if (!bio_flagged(bio, BIO_NULL_MAPPED)) {
+   /*
+* if we're in a workqueue, the request is orphaned, so
+* don't copy into a random user address space, just free.
+*/
+   if (current-mm)
+   ret = __bio_copy_iov(bio, bmd-iovecs, bmd-sgvecs,
+bmd-nr_sgvecs, bio_data_dir(bio) 
== READ,
+0, bmd-is_our_pages);
+   else if (bmd-is_our_pages)
+   bio_for_each_segment_all(bvec, bio, i)
+   __free_page(bvec-bv_page);
+   }
bio_free_map_data(bmd);
bio_put(bio);
return ret;


Hi Roland,

I

Re: [PATCH v2] [SCSI] sg: Fix user memory corruption when SG_IO is interrupted by a signal

2013-08-07 Thread David Milburn 

Roland Dreier wrote:

On Wed, Aug 7, 2013 at 7:38 AM, David Milburn dmilb...@redhat.com wrote:

I was able to succesfully test this patch overnight, I had been experimenting 
with the
sg driver setting the BIO_NULL_MAPPED flag in sg_rq_end_io_usercontext for a 
orphan process
which prevented the corruption, but your solution seems much better.


Very cool, thanks for the testing.

I actually looked at using BIO_NULL_MAPPED as well, but it seemed a
bit too fragile to me -- it had the right effect of skipping
__bio_copy_iov(), and skipping the __free_pages() stuff in there is OK
because sg owns its pages rather than the bio layer, but all that
seemed vulnerable to being broken by an unrelated change.

Out of curiousity, were you already working on this bug?  Because if
you had fixed it a few weeks earlier we might not have spent so long
wondering WTF was stomping on the memory of one of our processes :)



Hi Roland,

Actually, I was waiting for confirmation from the field which I
recently received, I was getting ready to bring this up on linux-scsi,
sorry I should have brought it up sooner. I wasn't positive that setting
BIO_NULL_MAPPED flag from sg driver was the fix. David Jeffery
came up with a reproducer which I ran overnight on the latest
upstream kernel with your patch.

Thanks,
David



--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mtip32xx: dynamically allocate buffer in debugfs functions

2013-05-23 Thread David Milburn 
Dynamically allocate buf to prevent warnings:

drivers/block/mtip32xx/mtip32xx.c: In function ‘mtip_hw_read_device_status’:
drivers/block/mtip32xx/mtip32xx.c:2823: warning: the frame size of 1056 bytes 
is larger than 1024 bytes
drivers/block/mtip32xx/mtip32xx.c: In function ‘mtip_hw_read_registers’:
drivers/block/mtip32xx/mtip32xx.c:2894: warning: the frame size of 1056 bytes 
is larger than 1024 bytes
drivers/block/mtip32xx/mtip32xx.c: In function ‘mtip_hw_read_flags’:
drivers/block/mtip32xx/mtip32xx.c:2917: warning: the frame size of 1056 bytes 
is larger than 1024 bytes

Signed-off-by: David Milburn 
---
 drivers/block/mtip32xx/mtip32xx.c |   47 +
 1 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/block/mtip32xx/mtip32xx.c 
b/drivers/block/mtip32xx/mtip32xx.c
index 847107e..baa0996 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -2806,34 +2806,51 @@ static ssize_t show_device_status(struct device_driver 
*drv, char *buf)
 static ssize_t mtip_hw_read_device_status(struct file *f, char __user *ubuf,
size_t len, loff_t *offset)
 {
+   struct driver_data *dd =  (struct driver_data *)f->private_data;
int size = *offset;
-   char buf[MTIP_DFS_MAX_BUF_SIZE];
+   char *buf;
+   int rv = 0;
 
if (!len || *offset)
return 0;
 
+   buf = kzalloc(MTIP_DFS_MAX_BUF_SIZE, GFP_KERNEL);
+   if (!buf) {
+   dev_err(>pdev->dev,
+   "Memory allocation: status buffer\n");
+   return -ENOMEM;
+   }
+
size += show_device_status(NULL, buf);
 
*offset = size <= len ? size : len;
size = copy_to_user(ubuf, buf, *offset);
if (size)
-   return -EFAULT;
+   rv = -EFAULT;
 
-   return *offset;
+   kfree(buf);
+   return rv ? rv : *offset;
 }
 
 static ssize_t mtip_hw_read_registers(struct file *f, char __user *ubuf,
  size_t len, loff_t *offset)
 {
struct driver_data *dd =  (struct driver_data *)f->private_data;
-   char buf[MTIP_DFS_MAX_BUF_SIZE];
+   char *buf;
u32 group_allocated;
int size = *offset;
-   int n;
+   int n, rv = 0;
 
if (!len || size)
return 0;
 
+   buf = kzalloc(MTIP_DFS_MAX_BUF_SIZE, GFP_KERNEL);
+   if (!buf) {
+   dev_err(>pdev->dev,
+   "Memory allocation: register buffer\n");
+   return -ENOMEM;
+   }
+
size += sprintf([size], "H/ S ACTive  : [ 0x");
 
for (n = dd->slot_groups-1; n >= 0; n--)
@@ -2888,21 +2905,30 @@ static ssize_t mtip_hw_read_registers(struct file *f, 
char __user *ubuf,
*offset = size <= len ? size : len;
size = copy_to_user(ubuf, buf, *offset);
if (size)
-   return -EFAULT;
+   rv = -EFAULT;
 
-   return *offset;
+   kfree(buf);
+   return rv ? rv : *offset;
 }
 
 static ssize_t mtip_hw_read_flags(struct file *f, char __user *ubuf,
  size_t len, loff_t *offset)
 {
struct driver_data *dd =  (struct driver_data *)f->private_data;
-   char buf[MTIP_DFS_MAX_BUF_SIZE];
+   char *buf;
int size = *offset;
+   int rv = 0;
 
if (!len || size)
return 0;
 
+   buf = kzalloc(MTIP_DFS_MAX_BUF_SIZE, GFP_KERNEL);
+   if (!buf) {
+   dev_err(>pdev->dev,
+   "Memory allocation: flag buffer\n");
+   return -ENOMEM;
+   }
+
size += sprintf([size], "Flag-port : [ %08lX ]\n",
dd->port->flags);
size += sprintf([size], "Flag-dd   : [ %08lX ]\n",
@@ -2911,9 +2937,10 @@ static ssize_t mtip_hw_read_flags(struct file *f, char 
__user *ubuf,
*offset = size <= len ? size : len;
size = copy_to_user(ubuf, buf, *offset);
if (size)
-   return -EFAULT;
+   rv = -EFAULT;
 
-   return *offset;
+   kfree(buf);
+   return rv ? rv : *offset;
 }
 
 static const struct file_operations mtip_device_status_fops = {
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mtip32xx: dynamically allocate buffer in debugfs functions

2013-05-23 Thread David Milburn 
Dynamically allocate buf to prevent warnings:

drivers/block/mtip32xx/mtip32xx.c: In function ‘mtip_hw_read_device_status’:
drivers/block/mtip32xx/mtip32xx.c:2823: warning: the frame size of 1056 bytes 
is larger than 1024 bytes
drivers/block/mtip32xx/mtip32xx.c: In function ‘mtip_hw_read_registers’:
drivers/block/mtip32xx/mtip32xx.c:2894: warning: the frame size of 1056 bytes 
is larger than 1024 bytes
drivers/block/mtip32xx/mtip32xx.c: In function ‘mtip_hw_read_flags’:
drivers/block/mtip32xx/mtip32xx.c:2917: warning: the frame size of 1056 bytes 
is larger than 1024 bytes

Signed-off-by: David Milburn dmilb...@redhat.com
---
 drivers/block/mtip32xx/mtip32xx.c |   47 +
 1 files changed, 37 insertions(+), 10 deletions(-)

diff --git a/drivers/block/mtip32xx/mtip32xx.c 
b/drivers/block/mtip32xx/mtip32xx.c
index 847107e..baa0996 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -2806,34 +2806,51 @@ static ssize_t show_device_status(struct device_driver 
*drv, char *buf)
 static ssize_t mtip_hw_read_device_status(struct file *f, char __user *ubuf,
size_t len, loff_t *offset)
 {
+   struct driver_data *dd =  (struct driver_data *)f-private_data;
int size = *offset;
-   char buf[MTIP_DFS_MAX_BUF_SIZE];
+   char *buf;
+   int rv = 0;
 
if (!len || *offset)
return 0;
 
+   buf = kzalloc(MTIP_DFS_MAX_BUF_SIZE, GFP_KERNEL);
+   if (!buf) {
+   dev_err(dd-pdev-dev,
+   Memory allocation: status buffer\n);
+   return -ENOMEM;
+   }
+
size += show_device_status(NULL, buf);
 
*offset = size = len ? size : len;
size = copy_to_user(ubuf, buf, *offset);
if (size)
-   return -EFAULT;
+   rv = -EFAULT;
 
-   return *offset;
+   kfree(buf);
+   return rv ? rv : *offset;
 }
 
 static ssize_t mtip_hw_read_registers(struct file *f, char __user *ubuf,
  size_t len, loff_t *offset)
 {
struct driver_data *dd =  (struct driver_data *)f-private_data;
-   char buf[MTIP_DFS_MAX_BUF_SIZE];
+   char *buf;
u32 group_allocated;
int size = *offset;
-   int n;
+   int n, rv = 0;
 
if (!len || size)
return 0;
 
+   buf = kzalloc(MTIP_DFS_MAX_BUF_SIZE, GFP_KERNEL);
+   if (!buf) {
+   dev_err(dd-pdev-dev,
+   Memory allocation: register buffer\n);
+   return -ENOMEM;
+   }
+
size += sprintf(buf[size], H/ S ACTive  : [ 0x);
 
for (n = dd-slot_groups-1; n = 0; n--)
@@ -2888,21 +2905,30 @@ static ssize_t mtip_hw_read_registers(struct file *f, 
char __user *ubuf,
*offset = size = len ? size : len;
size = copy_to_user(ubuf, buf, *offset);
if (size)
-   return -EFAULT;
+   rv = -EFAULT;
 
-   return *offset;
+   kfree(buf);
+   return rv ? rv : *offset;
 }
 
 static ssize_t mtip_hw_read_flags(struct file *f, char __user *ubuf,
  size_t len, loff_t *offset)
 {
struct driver_data *dd =  (struct driver_data *)f-private_data;
-   char buf[MTIP_DFS_MAX_BUF_SIZE];
+   char *buf;
int size = *offset;
+   int rv = 0;
 
if (!len || size)
return 0;
 
+   buf = kzalloc(MTIP_DFS_MAX_BUF_SIZE, GFP_KERNEL);
+   if (!buf) {
+   dev_err(dd-pdev-dev,
+   Memory allocation: flag buffer\n);
+   return -ENOMEM;
+   }
+
size += sprintf(buf[size], Flag-port : [ %08lX ]\n,
dd-port-flags);
size += sprintf(buf[size], Flag-dd   : [ %08lX ]\n,
@@ -2911,9 +2937,10 @@ static ssize_t mtip_hw_read_flags(struct file *f, char 
__user *ubuf,
*offset = size = len ? size : len;
size = copy_to_user(ubuf, buf, *offset);
if (size)
-   return -EFAULT;
+   rv = -EFAULT;
 
-   return *offset;
+   kfree(buf);
+   return rv ? rv : *offset;
 }
 
 static const struct file_operations mtip_device_status_fops = {
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mtip32xx: fix user_buffer check in exec_drive_command

2012-09-12 Thread David Milburn 
Current user_buffer check is incorrect and causes hdparm to fail

# hdparm -I /dev/rssda
 HDIO_DRIVE_CMD(identify) failed: Input/output error

/dev/rssda:

Patching linux-3.6-rc5 hdparm works as expected

# hdparm -I /dev/rssda
/dev/rssda:

ATA device, with non-removable media
Model Number:   DELL_P320h-MTFDGAL350SAH
Serial Number:  121302025F01
Firmware Revision:  B1442808


Reported-by: Tomas Henzl 
Signed-off-by: David Milburn 
---
 drivers/block/mtip32xx/mtip32xx.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/mtip32xx/mtip32xx.c 
b/drivers/block/mtip32xx/mtip32xx.c
index a8fddeb..b24efe3 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -1900,7 +1900,7 @@ static int exec_drive_command(struct mtip_port *port, u8 
*command,
int rv = 0, xfer_sz = command[3];
 
if (xfer_sz) {
-   if (user_buffer)
+   if (!user_buffer)
return -EFAULT;
 
buf = dmam_alloc_coherent(>dd->pdev->dev,
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] mtip32xx: fix user_buffer check in exec_drive_command

2012-09-12 Thread David Milburn 
Current user_buffer check is incorrect and causes hdparm to fail

# hdparm -I /dev/rssda
 HDIO_DRIVE_CMD(identify) failed: Input/output error

/dev/rssda:

Patching linux-3.6-rc5 hdparm works as expected

# hdparm -I /dev/rssda
/dev/rssda:

ATA device, with non-removable media
Model Number:   DELL_P320h-MTFDGAL350SAH
Serial Number:  121302025F01
Firmware Revision:  B1442808
snip

Reported-by: Tomas Henzl the...@redhat.com
Signed-off-by: David Milburn dmilb...@redhat.com
---
 drivers/block/mtip32xx/mtip32xx.c |2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/drivers/block/mtip32xx/mtip32xx.c 
b/drivers/block/mtip32xx/mtip32xx.c
index a8fddeb..b24efe3 100644
--- a/drivers/block/mtip32xx/mtip32xx.c
+++ b/drivers/block/mtip32xx/mtip32xx.c
@@ -1900,7 +1900,7 @@ static int exec_drive_command(struct mtip_port *port, u8 
*command,
int rv = 0, xfer_sz = command[3];
 
if (xfer_sz) {
-   if (user_buffer)
+   if (!user_buffer)
return -EFAULT;
 
buf = dmam_alloc_coherent(port-dd-pdev-dev,
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] libata-scsi: ata_task_ioctl should return ATA registers from sense data

2006-12-18 Thread David Milburn 

User applications using the HDIO_DRIVE_TASK ioctl through libata
expect specific ATA registers to be returned to userspace. Verified
that ata_task_ioctl correctly returns register values to the
smartctl application.

Signed-off-by: David Milburn <[EMAIL PROTECTED]>
---
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a4790be..1966294 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -273,8 +273,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user 
*arg)
 {
int rc = 0;
u8 scsi_cmd[MAX_COMMAND_SIZE];
-   u8 args[7];
-   struct scsi_sense_hdr sshdr;
+   u8 args[7], *sensebuf = NULL;
+   int cmd_result;

if (arg == NULL)
return -EINVAL;
@@ -282,10 +282,14 @@ int ata_task_ioctl(struct scsi_device *scsidev, void 
__user *arg)
if (copy_from_user(args, arg, sizeof(args)))
return -EFAULT;

+   sensebuf = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_NOIO);
+   if (!sensebuf)
+   return -ENOMEM;
+
memset(scsi_cmd, 0, sizeof(scsi_cmd));
scsi_cmd[0]  = ATA_16;
scsi_cmd[1]  = (3 << 1); /* Non-data */
-   /* scsi_cmd[2] is already 0 -- no off.line, cc, or data xfer */
+   scsi_cmd[2]  = 0x20; /* cc but no off.line or data xfer */
scsi_cmd[4]  = args[1];
scsi_cmd[6]  = args[2];
scsi_cmd[8]  = args[3];
@@ -295,11 +299,46 @@ int ata_task_ioctl(struct scsi_device *scsidev, void 
__user *arg)

/* Good values for timeout and retries?  Values below
   from scsi_ioctl_send_command() for default case... */
-   if (scsi_execute_req(scsidev, scsi_cmd, DMA_NONE, NULL, 0, ,
-(10*HZ), 5))
+   cmd_result = scsi_execute(scsidev, scsi_cmd, DMA_NONE, NULL, 0,
+ sensebuf, (10*HZ), 5, 0);
+
+   if (driver_byte(cmd_result) == DRIVER_SENSE) {/* sense data available */
+   u8 *desc = sensebuf + 8;
+   cmd_result &= ~(0xFF<<24); /* DRIVER_SENSE is not an error */
+
+   /* If we set cc then ATA pass-through will cause a
+* check condition even if no error. Filter that. */
+   if (cmd_result & SAM_STAT_CHECK_CONDITION) {
+   struct scsi_sense_hdr sshdr;
+   scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
+);
+   if (sshdr.sense_key==0 &&
+   sshdr.asc==0 && sshdr.ascq==0)
+   cmd_result &= ~SAM_STAT_CHECK_CONDITION;
+   }
+
+   /* Send userspace ATA registers */
+   if (sensebuf[0] == 0x72 && /* format is "descriptor" */
+   desc[0] == 0x09) { /* code is "ATA Descriptor" */
+   args[0] = desc[13];/* status */
+   args[1] = desc[3]; /* error */
+   args[2] = desc[5]; /* sector count (0:7) */
+   args[3] = desc[7]; /* lbal */
+   args[4] = desc[9]; /* lbam */
+   args[5] = desc[11];/* lbah */
+   args[6] = desc[12];/* select */
+   if (copy_to_user(arg, args, sizeof(args)))
+   rc = -EFAULT;
+   }
+   }
+
+   if (cmd_result) {
rc = -EIO;
+   goto error;
+   }

-   /* Need code to retrieve data from check condition? */
+ error:
+   kfree(sensebuf);
return rc;
 }
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[PATCH] libata-scsi: ata_task_ioctl should return ATA registers from sense data

2006-12-18 Thread David Milburn 

User applications using the HDIO_DRIVE_TASK ioctl through libata
expect specific ATA registers to be returned to userspace. Verified
that ata_task_ioctl correctly returns register values to the
smartctl application.

Signed-off-by: David Milburn [EMAIL PROTECTED]
---
diff --git a/drivers/ata/libata-scsi.c b/drivers/ata/libata-scsi.c
index a4790be..1966294 100644
--- a/drivers/ata/libata-scsi.c
+++ b/drivers/ata/libata-scsi.c
@@ -273,8 +273,8 @@ int ata_task_ioctl(struct scsi_device *scsidev, void __user 
*arg)
 {
int rc = 0;
u8 scsi_cmd[MAX_COMMAND_SIZE];
-   u8 args[7];
-   struct scsi_sense_hdr sshdr;
+   u8 args[7], *sensebuf = NULL;
+   int cmd_result;

if (arg == NULL)
return -EINVAL;
@@ -282,10 +282,14 @@ int ata_task_ioctl(struct scsi_device *scsidev, void 
__user *arg)
if (copy_from_user(args, arg, sizeof(args)))
return -EFAULT;

+   sensebuf = kzalloc(SCSI_SENSE_BUFFERSIZE, GFP_NOIO);
+   if (!sensebuf)
+   return -ENOMEM;
+
memset(scsi_cmd, 0, sizeof(scsi_cmd));
scsi_cmd[0]  = ATA_16;
scsi_cmd[1]  = (3  1); /* Non-data */
-   /* scsi_cmd[2] is already 0 -- no off.line, cc, or data xfer */
+   scsi_cmd[2]  = 0x20; /* cc but no off.line or data xfer */
scsi_cmd[4]  = args[1];
scsi_cmd[6]  = args[2];
scsi_cmd[8]  = args[3];
@@ -295,11 +299,46 @@ int ata_task_ioctl(struct scsi_device *scsidev, void 
__user *arg)

/* Good values for timeout and retries?  Values below
   from scsi_ioctl_send_command() for default case... */
-   if (scsi_execute_req(scsidev, scsi_cmd, DMA_NONE, NULL, 0, sshdr,
-(10*HZ), 5))
+   cmd_result = scsi_execute(scsidev, scsi_cmd, DMA_NONE, NULL, 0,
+ sensebuf, (10*HZ), 5, 0);
+
+   if (driver_byte(cmd_result) == DRIVER_SENSE) {/* sense data available */
+   u8 *desc = sensebuf + 8;
+   cmd_result = ~(0xFF24); /* DRIVER_SENSE is not an error */
+
+   /* If we set cc then ATA pass-through will cause a
+* check condition even if no error. Filter that. */
+   if (cmd_result  SAM_STAT_CHECK_CONDITION) {
+   struct scsi_sense_hdr sshdr;
+   scsi_normalize_sense(sensebuf, SCSI_SENSE_BUFFERSIZE,
+sshdr);
+   if (sshdr.sense_key==0 
+   sshdr.asc==0  sshdr.ascq==0)
+   cmd_result = ~SAM_STAT_CHECK_CONDITION;
+   }
+
+   /* Send userspace ATA registers */
+   if (sensebuf[0] == 0x72  /* format is descriptor */
+   desc[0] == 0x09) { /* code is ATA Descriptor */
+   args[0] = desc[13];/* status */
+   args[1] = desc[3]; /* error */
+   args[2] = desc[5]; /* sector count (0:7) */
+   args[3] = desc[7]; /* lbal */
+   args[4] = desc[9]; /* lbam */
+   args[5] = desc[11];/* lbah */
+   args[6] = desc[12];/* select */
+   if (copy_to_user(arg, args, sizeof(args)))
+   rc = -EFAULT;
+   }
+   }
+
+   if (cmd_result) {
rc = -EIO;
+   goto error;
+   }

-   /* Need code to retrieve data from check condition? */
+ error:
+   kfree(sensebuf);
return rc;
 }
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/