Re: [PATCH v2 2/3] arcmsr: adds code for support areca new adapter ARC1203
On Wed, 2015-11-25 at 11:25 +0800, Ching Huang wrote:
> From: Ching Huang
>
> Support areca new PCIe to SATA RAID adapter ARC1203
>
> Signed-of-by: Ching Huang
>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
> --- a/drivers/scsi/arcmsr/arcmsr.h2015-11-25 10:52:16.28647 +0800
> +++ b/drivers/scsi/arcmsr/arcmsr.h2015-11-25 10:52:13.33447 +0800
> @@ -74,6 +74,9 @@ struct device_attribute;
> #ifndef PCI_DEVICE_ID_ARECA_1214
> #define PCI_DEVICE_ID_ARECA_12140x1214
> #endif
> +#ifndef PCI_DEVICE_ID_ARECA_1203
> + #define PCI_DEVICE_ID_ARECA_12030x1203
> +#endif
> /*
>
> **
> **
> @@ -245,6 +248,12 @@ struct FIRMWARE_INFO
> /* window of "instruction flags" from iop to driver */
> #define ARCMSR_IOP2DRV_DOORBELL 0x00020408
> #define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
> +/* window of "instruction flags" from iop to driver */
> +#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
> +#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
> +/* window of "instruction flags" from driver to iop */
> +#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
> +#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
> /* ARECA FLAG LANGUAGE */
> /* ioctl transfer */
> #define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-24 11:35:26.0
> +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-24 18:58:40.640226000
> +0800
> @@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
> static const char *arcmsr_info(struct Scsi_Host *);
> static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
> static void arcmsr_free_irq(struct pci_dev *, struct AdapterControlBlock *);
> +static void arcmsr_wait_firmware_ready(struct AdapterControlBlock *acb);
> static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
> queue_depth)
> {
> if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
> @@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
> .driver_data = ACB_ADAPTER_TYPE_B},
> {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
> .driver_data = ACB_ADAPTER_TYPE_B},
> + {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
> + .driver_data = ACB_ADAPTER_TYPE_B},
> {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
> .driver_data = ACB_ADAPTER_TYPE_A},
> {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
> @@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
> }
> static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
> {
> - struct MessageUnit_B *reg = acb->pmuB;
> + struct MessageUnit_B *reg;
> struct pci_dev *pdev = acb->pdev;
> void *dma_coherent;
> dma_addr_t dma_coherent_handle;
> @@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
> acb->dma_coherent2 = dma_coherent;
> reg = (struct MessageUnit_B *)dma_coherent;
> acb->pmuB = reg;
> - reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned long)acb-
> >mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
> - reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
> - reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned long)acb-
> >mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
> - reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK);
> + if (acb->pdev->device == PCI_DEVICE_ID_ARECA_1203) {
> + reg->drv2iop_doorbell = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_1203);
> + reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK_1203);
> + reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_1203);
> + reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK_1203);
> + } else {
> + reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
> + reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
> + reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
> + reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK);
> + }
This isn't quite readable IMHO. Can't you do:
#define MEM_BASE0(x) (u32 __iomem *)((unsigned lo
Re: [PATCH] Fix a bdi reregistration race, v2
Hi Bart, On Tue, Nov 24, 2015 at 03:23:21PM -0800, Bart Van Assche wrote: > So if a driver stops using a (major, minor) number pair and the same device > number is reused before the bdi device has been released the warning > mentioned in the patch description at the start of this thread is triggered. > This patch fixes that race by removing the bdi device from sysfs during the > __scsi_remove_device() call instead of when the bdi device is released. that's why I suggested only releasing the minor number (or rather dev_t) once we release the BDI, similar to what MD and DM do. But what I really wanted to ask for is what your reproducer looks like. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [Y2038] [PATCH v2] scsi: gdth: replace struct timeval with ktime_get_real_seconds()
On Tuesday 24 November 2015 16:44:07 Alison Schofield wrote: > struct timeval will overflow on 32-bit systems in y2038 and is being > removed from the kernel. Replace the use of struct timeval and > do_gettimeofday() with ktime_get_real_seconds() which provides a 64-bit > seconds value and is y2038 safe. > > gdth driver requires changes in two areas: > > 1) gdth_store_event() loads two u32 timestamp fields for ioctl GDTIOCTL_EVENT > >These timestamp fields are part of struct gdth_evt_str used for passing >event data to userspace. At the first instance of an event we do >(first_stamp=last_stamp="current time"). If that same event repeats, >we do (last_stamp="current time") AND increment same_count to indicate >how many times the event has repeated since first_stamp. > >This patch replaces the use of timeval and do_gettimeofday() with >ktime_get_real_seconds() cast to u32 to extend the timestamp fields >to y2106. > >Beyond y2106, the userspace tools (ie. RAID controller monitors) can >work around the time rollover and this driver would still not need to >change. > >Alternative: The alternative approach is to introduce a new ioctl in gdth >with the u32 time fields defined as u64. This would require userspace >changes now, but not in y2106. > > 2) gdth_show_info() calculates elapsed time using u32 first_stamp > > It is adding events with timestamps to a seq_file. Timestamps are > calculated as the "current time" minus the first_stamp. > > This patch replaces the use of timeval and do_gettimeofday() with > ktime_get_real_seconds() cast to u32 to calculate the timestamp. > > This elapsed time calculation is safe even when the time wraps (beyond > y2106) due to how unsigned subtraction works. A comment has been added > to the code to indicate this safety. > > Alternative: This piece itself doesn't warrant an alternative, but > if we do introduce a new structure & ioctl with u64 timestamps, this > would change accordingly. > > Signed-off-by: Alison Schofield Reviewed-by: Arnd Bergmann -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On 11/20/2015 04:28 PM, Ewan Milne wrote:
> On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
>> Can't we have a joint effort here?
>> I've been spending a _LOT_ of time trying to debug things here, but
>> none of the ideas I've come up with have been able to fix anything.
>
> Yes. I'm not the one primarily looking at it, and we don't have a
> reproducer in-house. We just have the one dump right now.
>
>>
>> I'm almost tempted to increase the count from scsi_alloc_sgtable()
>> by one and be done with ...
>>
>
> That might not fix it if it is a problem with the merge code, though.
>
And indeed, it doesn't.
Seems I finally found the culprit.
What happens is this:
We have two paths, with these seg_boundary_masks:
path-1:seg_boundary_mask = 65535,
path-2:seg_boundary_mask = 4294967295,
consequently the DM request queue has this:
md-1:seg_boundary_mask = 65535,
What happens now is that a request is being formatted, and sent
to path 2. During submission req->nr_phys_segments is formatted
with the limits of path 2, arriving at a count of 3.
Now the request gets retried on path 1, but as the NOMERGE request
flag is set req->nr_phys_segments is never updated.
But blk_rq_map_sg() ignores all counters, and just uses the
bi_vec directly, resulting in a count of 4 -> boom.
So the culprit here is the NOMERGE flag, which is evaluated
via
->dm_dispatch_request()
->blk_insert_cloned_request()
->blk_rq_check_limits()
If the above assessment is correct, the following patch should
fix it:
diff --git a/block/blk-core.c b/block/blk-core.c
index 801ced7..12cccd6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
*/
int blk_rq_check_limits(struct request_queue *q, struct request *rq)
{
- if (!rq_mergeable(rq))
+ if (rq->cmd_type != REQ_TYPE_FS)
return 0;
if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
rq->cmd_flags)) {
Mike? Jens?
Can you comment on it?
Cheers,
Hannes
--
Dr. Hannes ReineckezSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/71] More fixes, cleanup and modernization for NCR5380 drivers
On Wednesday 25 November 2015, Finn Thain wrote: > > On Tue, 24 Nov 2015, Ondrej Zary wrote: > > > On Tuesday 24 November 2015 10:13:17 Finn Thain wrote: > > > > > > On Tue, 24 Nov 2015, Ondrej Zary wrote: > > > > > > > On Tuesday 24 November 2015, Finn Thain wrote: > > > > > > > > > > On Mon, 23 Nov 2015, Ondrej Zary wrote: > > > > > > > > > > > > > > > > > PDMA seems to be broken in multiple ways. NCR5380_pread cannot > > > > > > process less than 128 bytes. In fact, 53C400 datasheet says that > > > > > > it's HW limitation: non-modulo-128-byte transfers should use > > > > > > PIO. > > > > > > > > > > > > Adding > > > > > > transfersize = round_down(transfersize, 128); > > > > > > to generic_NCR5380_dma_xfer_len() improves the situation a bit. > > > > > > > > > > > > After modprobe, some small reads (8, 4, 24 and 64 bytes) are > > > > > > done using PIO, then eight 512-byte reads using PDMA and then it > > > > > > fails on a 254-byte read. First 128 bytes are read using PDMA > > > > > > and the next PDMA operation hangs waiting forever for the host > > > > > > buffer to be ready. > > > > > > > > > > > > > > > > A 128-byte PDMA receive followed by 126-byte PDMA receive? I don't > > > > > see how that is possible given round_down(126, 128) == 0. Was this > > > > > the actual 'len' argument to NCR5380_pread() in g_NCR5380.c? > > > > > > > > No 126-byte PDMA. The 126 bytes were probably lost (or mixed with > > > > the next read?). > > > [...] > > > > The next read was also 254 bytes so another 128-byte PDMA transfer. > > > > > > > > Then modified NCR5380_information_transfer() to transfer the > > > > remaining data (126 bytes in this case) using PIO. It did not help, > > > > the next PDMA transfer failed too. > > > > > > > > > > AFAICT, no change to NCR5380_information_transfer() should be needed. > > > It was always meant to cope with the need to split a transfer between > > > (P)DMA and PIO. > > > > Instead of fixing split transfers, simply forced everything > > non-modulo-128 to PIO: > > The need to split a transfer arises from early chip errata relating to DMA > and the workarounds for them (see the comments in the source). That's why > I believe that the driver was meant to be cope with this. But I don't have > any experimental evidence for it. > > I'm almost certain that these errata aren't applicable to your hardware. > So I don't have any reason to think that your card will allow part of a > transfer to be performed with PDMA and the rest with PIO. So I don't > really object to the patch. > > But I don't understand the need for it either: I have no idea what state > the driver, chip and scsi bus were in when the 126-byte PIO transfer > failed. If the PIO transfer didn't succeed then the entire command should > have failed. The patch was just a quick hack to confirm that PDMA is not completely broken. Now we know that it mostly works so I can investigate the partial PIO problem. > > --- a/drivers/scsi/g_NCR5380.c > > +++ b/drivers/scsi/g_NCR5380.c > > @@ -703,6 +703,10 @@ static int generic_NCR5380_dma_xfer_len(struct > > scsi_cmnd *cmd) > > !(cmd->SCp.this_residual % transfersize)) > > transfersize = 32 * 1024; > > > > + /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */ > > Do you have a download link for this datasheet? http://bitsavers.trailing-edge.com/pdf/ncr/scsi/NCR_53C400.pdf 53C400A datasheet would be great too but haven't found any. I think that PDMA should work with 53C400A too but seems that the driver was never able to do it. Although there is code for port-mapped transfer in NCR5380_pread(), NCR53C400_register_offset is defined to 0 in the port-mapped case. The C400_ register offsets are thus defined with negative offset: #define C400_CONTROL_STATUS_REG NCR53C400_register_offset-8 #define C400_BLOCK_COUNTER_REG NCR53C400_register_offset-7 #define C400_RESUME_TRANSFER_REG NCR53C400_register_offset-6 #define C400_HOST_BUFFER NCR53C400_register_offset-4 This is probably OK for a port-mapped 53C400 (such card must have some glue decoding logic as the 53C400 chip itself can do memory-mapping only) because: /* * On NCR53C400 boards, NCR5380 registers are mapped 8 past * the base address. */ if (overrides[current_override].board == BOARD_NCR53C400) instance->io_port += 8; This means that on a 53C400, first 5380 register will be at base+8 and first C400_ register at base. But on a 53C400A, the 5380 registers are mapped on the base address so the C400_ registers would be below the base, which is obviously wrong. I hope that PDMA will work if I fix the C400_ registers mapping. > > + if (transfersize % 128) > > + transfersize = 0; > > + > > return transfersize; > > } > > > > It seems to work and greatly improves performance: > > # hdparm -t --direct /dev/sdb > >
Re: [PATCH v2 2/3] arcmsr: adds code for support areca new adapter ARC1203
On Wed, 2015-11-25 at 08:18 +0100, Hannes Reinecke wrote:
> On 11/25/2015 04:25 AM, Ching Huang wrote:
> > From: Ching Huang
> >
> > Support areca new PCIe to SATA RAID adapter ARC1203
> >
> > Signed-of-by: Ching Huang
> >
> > ---
> >
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
> > --- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 10:52:16.28647 +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 10:52:13.33447 +0800
> > @@ -74,6 +74,9 @@ struct device_attribute;
> > #ifndef PCI_DEVICE_ID_ARECA_1214
> > #define PCI_DEVICE_ID_ARECA_12140x1214
> > #endif
> > +#ifndef PCI_DEVICE_ID_ARECA_1203
> > + #define PCI_DEVICE_ID_ARECA_12030x1203
> > +#endif
> > /*
> >
> > **
> > **
> > @@ -245,6 +248,12 @@ struct FIRMWARE_INFO
> > /* window of "instruction flags" from iop to driver */
> > #define ARCMSR_IOP2DRV_DOORBELL 0x00020408
> > #define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
> > +/* window of "instruction flags" from iop to driver */
> > +#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
> > +#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
> > +/* window of "instruction flags" from driver to iop */
> > +#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
> > +#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
> > /* ARECA FLAG LANGUAGE */
> > /* ioctl transfer */
> > #define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> > b/drivers/scsi/arcmsr/arcmsr_hba.c
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0
> > +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 18:58:40.640226000
> > +0800
> > @@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
> > static const char *arcmsr_info(struct Scsi_Host *);
> > static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
> > static void arcmsr_free_irq(struct pci_dev *, struct AdapterControlBlock
> > *);
> > +static void arcmsr_wait_firmware_ready(struct AdapterControlBlock *acb);
> > static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
> > queue_depth)
> > {
> > if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
> > @@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
> > .driver_data = ACB_ADAPTER_TYPE_B},
> > {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
> > .driver_data = ACB_ADAPTER_TYPE_B},
> > + {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
> > + .driver_data = ACB_ADAPTER_TYPE_B},
> > {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
> > .driver_data = ACB_ADAPTER_TYPE_A},
> > {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
> > @@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
> > }
> > static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
> > {
> > - struct MessageUnit_B *reg = acb->pmuB;
> > + struct MessageUnit_B *reg;
> > struct pci_dev *pdev = acb->pdev;
> > void *dma_coherent;
> > dma_addr_t dma_coherent_handle;
> > @@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
> > acb->dma_coherent2 = dma_coherent;
> > reg = (struct MessageUnit_B *)dma_coherent;
> > acb->pmuB = reg;
> > - reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
> > - reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
> > - reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
> > - reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK);
> > + if (acb->pdev->device == PCI_DEVICE_ID_ARECA_1203) {
> > + reg->drv2iop_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_1203);
> > + reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK_1203);
> > + reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_1203);
> > + reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK_1203);
> > + } else {
> > + reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
> > + reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
> > + reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
> > + reg->iop2drv_doorbell_mask = (uint32
Re: [PATCH v2 2/3] arcmsr: adds code for support areca new adapter ARC1203
On Wed, 2015-11-25 at 09:43 +0100, Johannes Thumshirn wrote:
> On Wed, 2015-11-25 at 11:25 +0800, Ching Huang wrote:
> > From: Ching Huang
> >
> > Support areca new PCIe to SATA RAID adapter ARC1203
> >
> > Signed-of-by: Ching Huang
> >
> > ---
> >
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
> > --- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 10:52:16.28647 +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 10:52:13.33447 +0800
> > @@ -74,6 +74,9 @@ struct device_attribute;
> > #ifndef PCI_DEVICE_ID_ARECA_1214
> > #define PCI_DEVICE_ID_ARECA_12140x1214
> > #endif
> > +#ifndef PCI_DEVICE_ID_ARECA_1203
> > + #define PCI_DEVICE_ID_ARECA_12030x1203
> > +#endif
> > /*
> >
> >
> > **
> > **
> > @@ -245,6 +248,12 @@ struct FIRMWARE_INFO
> > /* window of "instruction flags" from iop to driver */
> > #define ARCMSR_IOP2DRV_DOORBELL 0x00020408
> > #define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
> > +/* window of "instruction flags" from iop to driver */
> > +#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
> > +#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
> > +/* window of "instruction flags" from driver to iop */
> > +#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
> > +#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
> > /* ARECA FLAG LANGUAGE */
> > /* ioctl transfer */
> > #define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> > b/drivers/scsi/arcmsr/arcmsr_hba.c
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0
> > +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 18:58:40.640226000
> > +0800
> > @@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
> > static const char *arcmsr_info(struct Scsi_Host *);
> > static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
> > static void arcmsr_free_irq(struct pci_dev *, struct AdapterControlBlock
> > *);
> > +static void arcmsr_wait_firmware_ready(struct AdapterControlBlock *acb);
> > static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
> > queue_depth)
> > {
> > if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
> > @@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
> > .driver_data = ACB_ADAPTER_TYPE_B},
> > {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
> > .driver_data = ACB_ADAPTER_TYPE_B},
> > + {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
> > + .driver_data = ACB_ADAPTER_TYPE_B},
> > {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
> > .driver_data = ACB_ADAPTER_TYPE_A},
> > {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
> > @@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
> > }
> > static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
> > {
> > - struct MessageUnit_B *reg = acb->pmuB;
> > + struct MessageUnit_B *reg;
> > struct pci_dev *pdev = acb->pdev;
> > void *dma_coherent;
> > dma_addr_t dma_coherent_handle;
> > @@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
> > acb->dma_coherent2 = dma_coherent;
> > reg = (struct MessageUnit_B *)dma_coherent;
> > acb->pmuB = reg;
> > - reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned long)acb-
> > >mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
> > - reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
> > - reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned long)acb-
> > >mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
> > - reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK);
> > + if (acb->pdev->device == PCI_DEVICE_ID_ARECA_1203) {
> > + reg->drv2iop_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_1203);
> > + reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK_1203);
> > + reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_1203);
> > + reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK_1203);
> > + } else {
> > + reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL);
> > + reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK);
> > + reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned
> > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL);
> > + reg->iop2drv_doorbell_mask = (
iSCSI and scsi-mq
Hi iSCSI experts, I've been starting to research possible implementations of a scsi-mq capable iSCSI. With my research I found the following thread https://groups.google.com/ forum/#!msg/open-iscsi/qLaXJh2Gqt4/A2NIDZ0fT9oJ and wondered what happened to the efforts layed out there (and what have been the results of the discussion at LSF/MM). The one TCP connection per CPU or HW queue approach sounds particularly interesting to me when taking a SW iSCSI initiator. A problem Hannes pointed out here could be network topology changes which result in a different route on a different NIC with a different number of HW queues, so it might not be the ultimate solution. With the offload cards we might need to do something else. Is there anyone else working on a scsi-mq for iSCSI at the moment and is willing to join efforts here? Byte, Johannes -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] scsi: aic7xxx: kfree() NULL pointer cleanups
kfree() handles NULL pointers fine - checking is redundant
Signed-off-by: Anup Limbu
---
drivers/scsi/aic7xxx/aic79xx_core.c | 15 +--
1 file changed, 5 insertions(+), 10 deletions(-)
diff --git a/drivers/scsi/aic7xxx/aic79xx_core.c
b/drivers/scsi/aic7xxx/aic79xx_core.c
index 109e2c9..a710a35 100644
--- a/drivers/scsi/aic7xxx/aic79xx_core.c
+++ b/drivers/scsi/aic7xxx/aic79xx_core.c
@@ -3681,8 +3681,7 @@ ahd_free_tstate(struct ahd_softc *ahd, u_int scsi_id,
char channel, int force)
return;
tstate = ahd->enabled_targets[scsi_id];
- if (tstate != NULL)
- kfree(tstate);
+ kfree(tstate);
ahd->enabled_targets[scsi_id] = NULL;
}
#endif
@@ -6145,8 +6144,7 @@ ahd_set_unit(struct ahd_softc *ahd, int unit)
void
ahd_set_name(struct ahd_softc *ahd, char *name)
{
- if (ahd->name != NULL)
- kfree(ahd->name);
+ kfree(ahd->name);
ahd->name = name;
}
@@ -6213,12 +6211,9 @@ ahd_free(struct ahd_softc *ahd)
kfree(ahd->black_hole);
}
#endif
- if (ahd->name != NULL)
- kfree(ahd->name);
- if (ahd->seep_config != NULL)
- kfree(ahd->seep_config);
- if (ahd->saved_stack != NULL)
- kfree(ahd->saved_stack);
+ kfree(ahd->name);
+ kfree(ahd->seep_config);
+ kfree(ahd->saved_stack);
#ifndef __FreeBSD__
kfree(ahd);
#endif
--
1.9.1
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 0/5] arcmsr: support areca new adapter ARC1203
From: Ching Huang Patch 1 fixes getting wrong configuration data. Patch 2 fixes not release allocated resource if get configuration data failed. Patch 3 modifies codes for more readable. Pacth 4 adds codes to support new adapter ARC1203. Patch 5 changes driver version number. -- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 1/5] arcmsr: fixed getting wrong configuration data
From: Ching Huang
Fixed getting wrong configuration data of adapter type B and type D.
Signed-of-by: Ching Huang
---
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-23 16:25:22.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0 +0800
@@ -2694,15 +2694,15 @@ static bool arcmsr_hbaB_get_config(struc
acb->firm_model,
acb->firm_version);
- acb->signature = readl(®->message_rwbuffer[1]);
+ acb->signature = readl(®->message_rwbuffer[0]);
/*firm_signature,1,00-03*/
- acb->firm_request_len = readl(®->message_rwbuffer[2]);
+ acb->firm_request_len = readl(®->message_rwbuffer[1]);
/*firm_request_len,1,04-07*/
- acb->firm_numbers_queue = readl(®->message_rwbuffer[3]);
+ acb->firm_numbers_queue = readl(®->message_rwbuffer[2]);
/*firm_numbers_queue,2,08-11*/
- acb->firm_sdram_size = readl(®->message_rwbuffer[4]);
+ acb->firm_sdram_size = readl(®->message_rwbuffer[3]);
/*firm_sdram_size,3,12-15*/
- acb->firm_hd_channels = readl(®->message_rwbuffer[5]);
+ acb->firm_hd_channels = readl(®->message_rwbuffer[4]);
/*firm_ide_channels,4,16-19*/
acb->firm_cfg_version = readl(®->message_rwbuffer[25]);
/*firm_cfg_version,25,100-103*/
/*firm_ide_channels,4,16-19*/
@@ -2880,15 +2880,15 @@ static bool arcmsr_hbaD_get_config(struc
iop_device_map++;
count--;
}
- acb->signature = readl(®->msgcode_rwbuffer[1]);
+ acb->signature = readl(®->msgcode_rwbuffer[0]);
/*firm_signature,1,00-03*/
- acb->firm_request_len = readl(®->msgcode_rwbuffer[2]);
+ acb->firm_request_len = readl(®->msgcode_rwbuffer[1]);
/*firm_request_len,1,04-07*/
- acb->firm_numbers_queue = readl(®->msgcode_rwbuffer[3]);
+ acb->firm_numbers_queue = readl(®->msgcode_rwbuffer[2]);
/*firm_numbers_queue,2,08-11*/
- acb->firm_sdram_size = readl(®->msgcode_rwbuffer[4]);
+ acb->firm_sdram_size = readl(®->msgcode_rwbuffer[3]);
/*firm_sdram_size,3,12-15*/
- acb->firm_hd_channels = readl(®->msgcode_rwbuffer[5]);
+ acb->firm_hd_channels = readl(®->msgcode_rwbuffer[4]);
/*firm_hd_channels,4,16-19*/
acb->firm_cfg_version = readl(®->msgcode_rwbuffer[25]);
pr_notice("Areca RAID Controller%d: Model %s, F/W %s\n",
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 2/5] arcmsr: fixes not release allocated resource
From: Ching Huang
Releasing allocated resource if get configuration data failed.
Signed-of-by: Ching Huang
---
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-25 19:04:44.59097 +0800
@@ -2664,7 +2664,7 @@ static bool arcmsr_hbaB_get_config(struc
if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
miscellaneous data' timeout \n", acb->host->host_no);
- return false;
+ goto err_free_dma;
}
count = 8;
while (count){
@@ -2707,6 +2707,10 @@ static bool arcmsr_hbaB_get_config(struc
acb->firm_cfg_version = readl(®->message_rwbuffer[25]);
/*firm_cfg_version,25,100-103*/
/*firm_ide_channels,4,16-19*/
return true;
+err_free_dma:
+ dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
+ acb->dma_coherent2, acb->dma_coherent_handle2);
+ return false;
}
static bool arcmsr_hbaC_get_config(struct AdapterControlBlock *pACB)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 3/5] arcmsr: modifies codes for more readable
From: Ching Huang Modifies codes for more readable Signed-of-by: Ching Huang --- diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h --- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 10:52:16.28647 +0800 +++ b/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 19:10:21.30996 +0800 @@ -288,6 +288,9 @@ struct FIRMWARE_INFO #define ARCMSR_MESSAGE_RBUFFER 0xff00 /* iop message_rwbuffer for message command */ #define ARCMSR_MESSAGE_RWBUFFER 0xfa00 + +#define MEM_BASE0(x) (u32 __iomem *)((unsigned long)acb->mem_base0 + x) +#define MEM_BASE1(x) (u32 __iomem *)((unsigned long)acb->mem_base1 + x) /* **SPEC. for Areca HBC adapter diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c --- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-25 19:04:44.59097 +0800 +++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-25 19:11:27.679958000 +0800 @@ -2649,13 +2649,13 @@ static bool arcmsr_hbaB_get_config(struc acb->dma_coherent2 = dma_coherent; reg = (struct MessageUnit_B *)dma_coherent; acb->pmuB = reg; - reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL); - reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK); - reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL); - reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK); - reg->message_wbuffer = (uint32_t __iomem *)((unsigned long)acb->mem_base1 + ARCMSR_MESSAGE_WBUFFER); - reg->message_rbuffer = (uint32_t __iomem *)((unsigned long)acb->mem_base1 + ARCMSR_MESSAGE_RBUFFER); - reg->message_rwbuffer = (uint32_t __iomem *)((unsigned long)acb->mem_base1 + ARCMSR_MESSAGE_RWBUFFER); + reg->drv2iop_doorbell= MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL); + reg->drv2iop_doorbell_mask = MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK); + reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL); + reg->iop2drv_doorbell_mask = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK); + reg->message_wbuffer = MEM_BASE1(ARCMSR_MESSAGE_WBUFFER); + reg->message_rbuffer = MEM_BASE1(ARCMSR_MESSAGE_RBUFFER); + reg->message_rwbuffer = MEM_BASE1(ARCMSR_MESSAGE_RWBUFFER); iop_firm_model = (char __iomem *)(®->message_rwbuffer[15]); /*firm_model,15,60-67*/ iop_firm_version = (char __iomem *)(®->message_rwbuffer[17]); /*firm_version,17,68-83*/ iop_device_map = (char __iomem *)(®->message_rwbuffer[21]); /*firm_version,21,84-99*/ -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 4/5] arcmsr: adds code for support areca new adapter ARC1203
From: Ching Huang
Support areca new PCIe to SATA RAID adapter ARC1203
Signed-of-by: Ching Huang
---
diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
--- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 19:10:21.30996 +0800
+++ b/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 18:25:42.926038000 +0800
@@ -74,6 +74,9 @@ struct device_attribute;
#ifndef PCI_DEVICE_ID_ARECA_1214
#define PCI_DEVICE_ID_ARECA_12140x1214
#endif
+#ifndef PCI_DEVICE_ID_ARECA_1203
+ #define PCI_DEVICE_ID_ARECA_12030x1203
+#endif
/*
**
**
@@ -245,6 +248,12 @@ struct FIRMWARE_INFO
/* window of "instruction flags" from iop to driver */
#define ARCMSR_IOP2DRV_DOORBELL 0x00020408
#define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
+/* window of "instruction flags" from iop to driver */
+#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
+#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
+/* window of "instruction flags" from driver to iop */
+#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
+#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
/* ARECA FLAG LANGUAGE */
/* ioctl transfer */
#define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c b/drivers/scsi/arcmsr/arcmsr_hba.c
--- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-25 19:11:27.679958000 +0800
+++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-25 18:30:52.239029000 +0800
@@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
static const char *arcmsr_info(struct Scsi_Host *);
static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
static void arcmsr_free_irq(struct pci_dev *, struct AdapterControlBlock *);
+static void arcmsr_wait_firmware_ready(struct AdapterControlBlock *acb);
static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
queue_depth)
{
if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
@@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
.driver_data = ACB_ADAPTER_TYPE_B},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
.driver_data = ACB_ADAPTER_TYPE_B},
+ {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
+ .driver_data = ACB_ADAPTER_TYPE_B},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
.driver_data = ACB_ADAPTER_TYPE_A},
{PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
@@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
}
static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
{
- struct MessageUnit_B *reg = acb->pmuB;
+ struct MessageUnit_B *reg;
struct pci_dev *pdev = acb->pdev;
void *dma_coherent;
dma_addr_t dma_coherent_handle;
@@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
acb->dma_coherent2 = dma_coherent;
reg = (struct MessageUnit_B *)dma_coherent;
acb->pmuB = reg;
- reg->drv2iop_doorbell= MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL);
- reg->drv2iop_doorbell_mask = MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK);
- reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL);
- reg->iop2drv_doorbell_mask = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK);
+ if (acb->pdev->device == PCI_DEVICE_ID_ARECA_1203) {
+ reg->drv2iop_doorbell = MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_1203);
+ reg->drv2iop_doorbell_mask =
MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK_1203);
+ reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_1203);
+ reg->iop2drv_doorbell_mask =
MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK_1203);
+ } else {
+ reg->drv2iop_doorbell= MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL);
+ reg->drv2iop_doorbell_mask =
MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK);
+ reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL);
+ reg->iop2drv_doorbell_mask =
MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK);
+ }
reg->message_wbuffer = MEM_BASE1(ARCMSR_MESSAGE_WBUFFER);
reg->message_rbuffer = MEM_BASE1(ARCMSR_MESSAGE_RBUFFER);
reg->message_rwbuffer = MEM_BASE1(ARCMSR_MESSAGE_RWBUFFER);
@@ -2660,6 +2670,12 @@ static bool arcmsr_hbaB_get_config(struc
iop_firm_version = (char __iomem *)(®->message_rwbuffer[17]);
/*firm_version,17,68-83*/
iop_device_map = (char __iomem *)(®->message_rwbuffer[21]);
/*firm_version,21,84-99*/
+ arcmsr_wait_firmware_ready(acb);
+ writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell);
+ if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
+ printk(KERN_ERR "arcmsr%d: can't set driver mode.\n",
acb->host->host_no);
+ goto err_free_dma;
+ }
writel(ARCMSR_MESSAGE_GET
Re: [PATCH 00/71] More fixes, cleanup and modernization for NCR5380 drivers
On Wed, 25 Nov 2015, Ondrej Zary wrote: > On Wednesday 25 November 2015, Finn Thain wrote: > > > > On Tue, 24 Nov 2015, Ondrej Zary wrote: > > > > > Instead of fixing split transfers, simply forced everything > > > non-modulo-128 to PIO: > > > > [...] > > I don't have any reason to think that your card will allow part of > > a transfer to be performed with PDMA and the rest with PIO. So I don't > > really object to the patch. > > >From looking at the datasheet, I think your patch is correct. Your patch needs to be applied after mine, so if you will sign-off, I'll include it in the series with your Signed-off-by and "From:" header. > > But I don't understand the need for it either: I have no idea what > > state the driver, chip and scsi bus were in when the 126-byte PIO > > transfer failed. If the PIO transfer didn't succeed then the entire > > command should have failed. > > The patch was just a quick hack to confirm that PDMA is not completely > broken. > Now we know that it mostly works so I can investigate the partial PIO > problem. > There may not be any problem to investigate. Because this 53C80 core is embedded in other logic, it's hard to say whether or not the partial PIO algorithm could be expected to work at all. Besides, the DMA errata don't apply to this core. And large transfers will always be divisible by 128 bytes so there's very little to be gained. > [...] > > 53C400A datasheet would be great too but haven't found any. I don't have one either. -- -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v3 5/5] arcmsr: changes driver version number
From: Ching Huang Changes driver version number. Signed-of-by: Ching Huang --- diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h --- a/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 18:25:42.926038000 +0800 +++ b/drivers/scsi/arcmsr/arcmsr.h 2015-11-25 18:04:27.400075000 +0800 @@ -52,7 +52,7 @@ struct device_attribute; #define ARCMSR_MAX_FREECCB_NUM 320 #define ARCMSR_MAX_OUTSTANDING_CMD 255 #endif -#define ARCMSR_DRIVER_VERSION "v1.30.00.04-20140919" +#define ARCMSR_DRIVER_VERSION "v1.30.00.21-20151019" #define ARCMSR_SCSI_INITIATOR_ID 255 #define ARCMSR_MAX_XFER_SECTORS 512 #define ARCMSR_MAX_XFER_SECTORS_B 4096 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] arcmsr: fixed getting wrong configuration data
On Wed, 2015-11-25 at 19:36 +0800, Ching Huang wrote:
> From: Ching Huang
>
> Fixed getting wrong configuration data of adapter type B and type D.
>
> Signed-of-by: Ching Huang
>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-23 16:25:22.0
> +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-24 11:35:26.0
> +0800
> @@ -2694,15 +2694,15 @@ static bool arcmsr_hbaB_get_config(struc
> acb->firm_model,
> acb->firm_version);
>
> - acb->signature = readl(®->message_rwbuffer[1]);
> + acb->signature = readl(®->message_rwbuffer[0]);
> /*firm_signature,1,00-03*/
> - acb->firm_request_len = readl(®->message_rwbuffer[2]);
> + acb->firm_request_len = readl(®->message_rwbuffer[1]);
> /*firm_request_len,1,04-07*/
> - acb->firm_numbers_queue = readl(®->message_rwbuffer[3]);
> + acb->firm_numbers_queue = readl(®->message_rwbuffer[2]);
> /*firm_numbers_queue,2,08-11*/
> - acb->firm_sdram_size = readl(®->message_rwbuffer[4]);
> + acb->firm_sdram_size = readl(®->message_rwbuffer[3]);
> /*firm_sdram_size,3,12-15*/
> - acb->firm_hd_channels = readl(®->message_rwbuffer[5]);
> + acb->firm_hd_channels = readl(®->message_rwbuffer[4]);
> /*firm_ide_channels,4,16-19*/
> acb->firm_cfg_version = readl(®-
> >message_rwbuffer[25]); /*firm_cfg_version,25,100-103*/
> /*firm_ide_channels,4,16-19*/
> @@ -2880,15 +2880,15 @@ static bool arcmsr_hbaD_get_config(struc
> iop_device_map++;
> count--;
> }
> - acb->signature = readl(®->msgcode_rwbuffer[1]);
> + acb->signature = readl(®->msgcode_rwbuffer[0]);
> /*firm_signature,1,00-03*/
> - acb->firm_request_len = readl(®->msgcode_rwbuffer[2]);
> + acb->firm_request_len = readl(®->msgcode_rwbuffer[1]);
> /*firm_request_len,1,04-07*/
> - acb->firm_numbers_queue = readl(®->msgcode_rwbuffer[3]);
> + acb->firm_numbers_queue = readl(®->msgcode_rwbuffer[2]);
> /*firm_numbers_queue,2,08-11*/
> - acb->firm_sdram_size = readl(®->msgcode_rwbuffer[4]);
> + acb->firm_sdram_size = readl(®->msgcode_rwbuffer[3]);
> /*firm_sdram_size,3,12-15*/
> - acb->firm_hd_channels = readl(®->msgcode_rwbuffer[5]);
> + acb->firm_hd_channels = readl(®->msgcode_rwbuffer[4]);
> /*firm_hd_channels,4,16-19*/
> acb->firm_cfg_version = readl(®->msgcode_rwbuffer[25]);
> pr_notice("Areca RAID Controller%d: Model %s, F/W %s\n",
>
>
Reviewed-by: Johannes Thumshirn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] arcmsr: fixes not release allocated resource
On Wed, 2015-11-25 at 19:41 +0800, Ching Huang wrote:
> From: Ching Huang
>
> Releasing allocated resource if get configuration data failed.
>
> Signed-of-by: Ching Huang
>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-24 11:35:26.0
> +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 19:04:44.59097
> +0800
> @@ -2664,7 +2664,7 @@ static bool arcmsr_hbaB_get_config(struc
> if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
> miscellaneous data' timeout \n", acb->host-
> >host_no);
> - return false;
> + goto err_free_dma;
> }
> count = 8;
> while (count){
> @@ -2707,6 +2707,10 @@ static bool arcmsr_hbaB_get_config(struc
> acb->firm_cfg_version = readl(®-
> >message_rwbuffer[25]); /*firm_cfg_version,25,100-103*/
> /*firm_ide_channels,4,16-19*/
> return true;
> +err_free_dma:
> + dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
> + acb->dma_coherent2, acb->dma_coherent_handle2);
> + return false;
> }
>
> static bool arcmsr_hbaC_get_config(struct AdapterControlBlock *pACB)
>
>
Reviewed-by: Johannes Thumshirn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] arcmsr: modifies codes for more readable
On Wed, 2015-11-25 at 19:45 +0800, Ching Huang wrote: > From: Ching Huang > > Modifies codes for more readable > > Signed-of-by: Ching Huang > > --- > > diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h > --- a/drivers/scsi/arcmsr/arcmsr.h2015-11-25 10:52:16.28647 +0800 > +++ b/drivers/scsi/arcmsr/arcmsr.h2015-11-25 19:10:21.30996 +0800 > @@ -288,6 +288,9 @@ struct FIRMWARE_INFO > #define ARCMSR_MESSAGE_RBUFFER 0xff00 > /* iop message_rwbuffer for message command */ > #define ARCMSR_MESSAGE_RWBUFFER 0xfa00 > + > +#define MEM_BASE0(x) (u32 __iomem *)((unsigned long)acb->mem_base0 + > x) > +#define MEM_BASE1(x) (u32 __iomem *)((unsigned long)acb->mem_base1 + > x) > /* > > **SPEC. for Areca HBC adapter > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c > b/drivers/scsi/arcmsr/arcmsr_hba.c > --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 19:04:44.59097 > +0800 > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 19:11:27.679958000 > +0800 > @@ -2649,13 +2649,13 @@ static bool arcmsr_hbaB_get_config(struc > acb->dma_coherent2 = dma_coherent; > reg = (struct MessageUnit_B *)dma_coherent; > acb->pmuB = reg; > - reg->drv2iop_doorbell= (uint32_t __iomem *)((unsigned long)acb- > >mem_base0 + ARCMSR_DRV2IOP_DOORBELL); > - reg->drv2iop_doorbell_mask = (uint32_t __iomem *)((unsigned > long)acb->mem_base0 + ARCMSR_DRV2IOP_DOORBELL_MASK); > - reg->iop2drv_doorbell = (uint32_t __iomem *)((unsigned long)acb- > >mem_base0 + ARCMSR_IOP2DRV_DOORBELL); > - reg->iop2drv_doorbell_mask = (uint32_t __iomem *)((unsigned > long)acb->mem_base0 + ARCMSR_IOP2DRV_DOORBELL_MASK); > - reg->message_wbuffer = (uint32_t __iomem *)((unsigned long)acb- > >mem_base1 + ARCMSR_MESSAGE_WBUFFER); > - reg->message_rbuffer = (uint32_t __iomem *)((unsigned long)acb- > >mem_base1 + ARCMSR_MESSAGE_RBUFFER); > - reg->message_rwbuffer = (uint32_t __iomem *)((unsigned long)acb- > >mem_base1 + ARCMSR_MESSAGE_RWBUFFER); > + reg->drv2iop_doorbell= MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL); > + reg->drv2iop_doorbell_mask = > MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK); > + reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL); > + reg->iop2drv_doorbell_mask = > MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK); > + reg->message_wbuffer = MEM_BASE1(ARCMSR_MESSAGE_WBUFFER); > + reg->message_rbuffer = MEM_BASE1(ARCMSR_MESSAGE_RBUFFER); > + reg->message_rwbuffer = MEM_BASE1(ARCMSR_MESSAGE_RWBUFFER); > iop_firm_model = (char __iomem *)(®->message_rwbuffer[15]); > /*firm_model,15,60-67*/ > iop_firm_version = (char __iomem *)(®->message_rwbuffer[17]); > /*firm_version,17,68-83*/ > iop_device_map = (char __iomem *)(®->message_rwbuffer[21]); > /*firm_version,21,84-99*/ > > Reviewed-by: Johannes Thumshirn -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] arcmsr: adds code for support areca new adapter ARC1203
On Wed, 2015-11-25 at 19:49 +0800, Ching Huang wrote:
> From: Ching Huang
>
> Support areca new PCIe to SATA RAID adapter ARC1203
>
> Signed-of-by: Ching Huang
>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h
> --- a/drivers/scsi/arcmsr/arcmsr.h2015-11-25 19:10:21.30996 +0800
> +++ b/drivers/scsi/arcmsr/arcmsr.h2015-11-25 18:25:42.926038000 +0800
> @@ -74,6 +74,9 @@ struct device_attribute;
> #ifndef PCI_DEVICE_ID_ARECA_1214
> #define PCI_DEVICE_ID_ARECA_12140x1214
> #endif
> +#ifndef PCI_DEVICE_ID_ARECA_1203
> + #define PCI_DEVICE_ID_ARECA_12030x1203
> +#endif
> /*
>
> **
> **
> @@ -245,6 +248,12 @@ struct FIRMWARE_INFO
> /* window of "instruction flags" from iop to driver */
> #define ARCMSR_IOP2DRV_DOORBELL 0x00020408
> #define ARCMSR_IOP2DRV_DOORBELL_MASK 0x0002040C
> +/* window of "instruction flags" from iop to driver */
> +#define ARCMSR_IOP2DRV_DOORBELL_1203 0x00021870
> +#define ARCMSR_IOP2DRV_DOORBELL_MASK_1203 0x00021874
> +/* window of "instruction flags" from driver to iop */
> +#define ARCMSR_DRV2IOP_DOORBELL_1203 0x00021878
> +#define ARCMSR_DRV2IOP_DOORBELL_MASK_1203 0x0002187C
> /* ARECA FLAG LANGUAGE */
> /* ioctl transfer */
> #define ARCMSR_IOP2DRV_DATA_WRITE_OK 0x0001
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 19:11:27.679958000
> +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 18:30:52.239029000
> +0800
> @@ -114,6 +114,7 @@ static void arcmsr_hardware_reset(struct
> static const char *arcmsr_info(struct Scsi_Host *);
> static irqreturn_t arcmsr_interrupt(struct AdapterControlBlock *acb);
> static void arcmsr_free_irq(struct pci_dev *, struct AdapterControlBlock *);
> +static void arcmsr_wait_firmware_ready(struct AdapterControlBlock *acb);
> static int arcmsr_adjust_disk_queue_depth(struct scsi_device *sdev, int
> queue_depth)
> {
> if (queue_depth > ARCMSR_MAX_CMD_PERLUN)
> @@ -157,6 +158,8 @@ static struct pci_device_id arcmsr_devic
> .driver_data = ACB_ADAPTER_TYPE_B},
> {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1202),
> .driver_data = ACB_ADAPTER_TYPE_B},
> + {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1203),
> + .driver_data = ACB_ADAPTER_TYPE_B},
> {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1210),
> .driver_data = ACB_ADAPTER_TYPE_A},
> {PCI_DEVICE(PCI_VENDOR_ID_ARECA, PCI_DEVICE_ID_ARECA_1214),
> @@ -2621,7 +2624,7 @@ static bool arcmsr_hbaA_get_config(struc
> }
> static bool arcmsr_hbaB_get_config(struct AdapterControlBlock *acb)
> {
> - struct MessageUnit_B *reg = acb->pmuB;
> + struct MessageUnit_B *reg;
> struct pci_dev *pdev = acb->pdev;
> void *dma_coherent;
> dma_addr_t dma_coherent_handle;
> @@ -2649,10 +2652,17 @@ static bool arcmsr_hbaB_get_config(struc
> acb->dma_coherent2 = dma_coherent;
> reg = (struct MessageUnit_B *)dma_coherent;
> acb->pmuB = reg;
> - reg->drv2iop_doorbell= MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL);
> - reg->drv2iop_doorbell_mask =
> MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK);
> - reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL);
> - reg->iop2drv_doorbell_mask =
> MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK);
> + if (acb->pdev->device == PCI_DEVICE_ID_ARECA_1203) {
> + reg->drv2iop_doorbell =
> MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_1203);
> + reg->drv2iop_doorbell_mask =
> MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK_1203);
> + reg->iop2drv_doorbell =
> MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_1203);
> + reg->iop2drv_doorbell_mask =
> MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK_1203);
> + } else {
> + reg->drv2iop_doorbell= MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL);
> + reg->drv2iop_doorbell_mask =
> MEM_BASE0(ARCMSR_DRV2IOP_DOORBELL_MASK);
> + reg->iop2drv_doorbell = MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL);
> + reg->iop2drv_doorbell_mask =
> MEM_BASE0(ARCMSR_IOP2DRV_DOORBELL_MASK);
> + }
> reg->message_wbuffer = MEM_BASE1(ARCMSR_MESSAGE_WBUFFER);
> reg->message_rbuffer = MEM_BASE1(ARCMSR_MESSAGE_RBUFFER);
> reg->message_rwbuffer = MEM_BASE1(ARCMSR_MESSAGE_RWBUFFER);
> @@ -2660,6 +2670,12 @@ static bool arcmsr_hbaB_get_config(struc
> iop_firm_version = (char __iomem *)(®->message_rwbuffer[17]);
> /*firm_version,17,68-83*/
> iop_device_map = (char __iomem *)(®->message_rwbuffer[21]);
> /*firm_version,21,84-99*/
>
> + arcmsr_wait_firmware_ready(acb);
> + writel(ARCMSR_MESSAGE_START_DRIVER_MODE, reg->drv2iop_doorbell);
> + if (!arcmsr_hbaB_wait_msgi
Re: [PATCH v3 5/5] arcmsr: changes driver version number
On Wed, 2015-11-25 at 19:52 +0800, Ching Huang wrote: > From: Ching Huang > > Changes driver version number. > > Signed-of-by: Ching Huang > > --- > > diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h > --- a/drivers/scsi/arcmsr/arcmsr.h2015-11-25 18:25:42.926038000 +0800 > +++ b/drivers/scsi/arcmsr/arcmsr.h2015-11-25 18:04:27.400075000 +0800 > @@ -52,7 +52,7 @@ struct device_attribute; > #define ARCMSR_MAX_FREECCB_NUM 320 > #define ARCMSR_MAX_OUTSTANDING_CMD 255 > #endif > -#define ARCMSR_DRIVER_VERSION"v1.30.00.04-20140919" > +#define ARCMSR_DRIVER_VERSION"v1.30.00.21-20151019" > #define ARCMSR_SCSI_INITIATOR_ID > 255 > #define ARCMSR_MAX_XFER_SECTORS > 512 > #define ARCMSR_MAX_XFER_SECTORS_B > 4096 > > Reviewed-by: Johannes Thumshirn -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 1/5] arcmsr: fixed getting wrong configuration data
On 11/25/2015 12:36 PM, Ching Huang wrote: > From: Ching Huang > > Fixed getting wrong configuration data of adapter type B and type D. > > Signed-of-by: Ching Huang > > --- > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] arcmsr: fixes not release allocated resource
On 11/25/2015 12:41 PM, Ching Huang wrote:
> From: Ching Huang
>
> Releasing allocated resource if get configuration data failed.
>
> Signed-of-by: Ching Huang
>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-24 11:35:26.0
> +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 19:04:44.59097
> +0800
> @@ -2664,7 +2664,7 @@ static bool arcmsr_hbaB_get_config(struc
> if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
> miscellaneous data' timeout \n", acb->host->host_no);
> - return false;
> + goto err_free_dma;
> }
> count = 8;
> while (count){
> @@ -2707,6 +2707,10 @@ static bool arcmsr_hbaB_get_config(struc
> acb->firm_cfg_version = readl(®->message_rwbuffer[25]);
> /*firm_cfg_version,25,100-103*/
> /*firm_ide_channels,4,16-19*/
> return true;
> +err_free_dma:
> + dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
> + acb->dma_coherent2, acb->dma_coherent_handle2);
> + return false;
> }
>
> static bool arcmsr_hbaC_get_config(struct AdapterControlBlock *pACB)
>
>
Reviewed-by: Hannes Reinecke
Cheers,
Hannes
--
Dr. Hannes ReineckezSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/5] arcmsr: modifies codes for more readable
On 11/25/2015 12:45 PM, Ching Huang wrote: > From: Ching Huang > > Modifies codes for more readable > > Signed-of-by: Ching Huang > > --- > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 5/5] arcmsr: changes driver version number
On 11/25/2015 12:52 PM, Ching Huang wrote: > From: Ching Huang > > Changes driver version number. > > Signed-of-by: Ching Huang > > --- > > diff -uprN a/drivers/scsi/arcmsr/arcmsr.h b/drivers/scsi/arcmsr/arcmsr.h > --- a/drivers/scsi/arcmsr/arcmsr.h2015-11-25 18:25:42.926038000 +0800 > +++ b/drivers/scsi/arcmsr/arcmsr.h2015-11-25 18:04:27.400075000 +0800 > @@ -52,7 +52,7 @@ struct device_attribute; > #define ARCMSR_MAX_FREECCB_NUM 320 > #define ARCMSR_MAX_OUTSTANDING_CMD 255 > #endif > -#define ARCMSR_DRIVER_VERSION"v1.30.00.04-20140919" > +#define ARCMSR_DRIVER_VERSION"v1.30.00.21-20151019" > #define ARCMSR_SCSI_INITIATOR_ID > 255 > #define ARCMSR_MAX_XFER_SECTORS > 512 > #define ARCMSR_MAX_XFER_SECTORS_B > 4096 > > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 4/5] arcmsr: adds code for support areca new adapter ARC1203
On 11/25/2015 12:49 PM, Ching Huang wrote: > From: Ching Huang > > Support areca new PCIe to SATA RAID adapter ARC1203 > > Signed-of-by: Ching Huang > > --- > Reviewed-by: Hannes Reinecke Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] target: use offset_in_page macro
Use offset_in_page macro instead of (addr & ~PAGE_MASK).
Signed-off-by: Geliang Tang
---
drivers/target/target_core_user.c | 2 +-
drivers/target/tcm_fc/tfc_io.c| 8
2 files changed, 5 insertions(+), 5 deletions(-)
diff --git a/drivers/target/target_core_user.c
b/drivers/target/target_core_user.c
index 937cebf..d5477c0 100644
--- a/drivers/target/target_core_user.c
+++ b/drivers/target/target_core_user.c
@@ -194,7 +194,7 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd
*se_cmd)
static inline void tcmu_flush_dcache_range(void *vaddr, size_t size)
{
- unsigned long offset = (unsigned long) vaddr & ~PAGE_MASK;
+ unsigned long offset = offset_in_page(vaddr);
size = round_up(size+offset, PAGE_SIZE);
vaddr -= offset;
diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
index 847c1aa..6f7c65a 100644
--- a/drivers/target/tcm_fc/tfc_io.c
+++ b/drivers/target/tcm_fc/tfc_io.c
@@ -154,9 +154,9 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
BUG_ON(!page);
from = kmap_atomic(page + (mem_off >> PAGE_SHIFT));
page_addr = from;
- from += mem_off & ~PAGE_MASK;
+ from += offset_in_page(mem_off);
tlen = min(tlen, (size_t)(PAGE_SIZE -
- (mem_off & ~PAGE_MASK)));
+ offset_in_page(mem_off)));
memcpy(to, from, tlen);
kunmap_atomic(page_addr);
to += tlen;
@@ -314,9 +314,9 @@ void ft_recv_write_data(struct ft_cmd *cmd, struct fc_frame
*fp)
to = kmap_atomic(page + (mem_off >> PAGE_SHIFT));
page_addr = to;
- to += mem_off & ~PAGE_MASK;
+ to += offset_in_page(mem_off);
tlen = min(tlen, (size_t)(PAGE_SIZE -
- (mem_off & ~PAGE_MASK)));
+ offset_in_page(mem_off)));
memcpy(to, from, tlen);
kunmap_atomic(page_addr);
--
2.5.0
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] arcmsr: fixes not release allocated resource
On 25.11.2015 12:41, Ching Huang wrote:
> From: Ching Huang
>
> Releasing allocated resource if get configuration data failed.
>
> Signed-of-by: Ching Huang
>
> ---
>
> diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> b/drivers/scsi/arcmsr/arcmsr_hba.c
> --- a/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-24 11:35:26.0
> +0800
> +++ b/drivers/scsi/arcmsr/arcmsr_hba.c2015-11-25 19:04:44.59097
> +0800
> @@ -2664,7 +2664,7 @@ static bool arcmsr_hbaB_get_config(struc
> if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
> miscellaneous data' timeout \n", acb->host->host_no);
> - return false;
> + goto err_free_dma;
> }
> count = 8;
> while (count){
> @@ -2707,6 +2707,10 @@ static bool arcmsr_hbaB_get_config(struc
> acb->firm_cfg_version = readl(®->message_rwbuffer[25]);
> /*firm_cfg_version,25,100-103*/
> /*firm_ide_channels,4,16-19*/
> return true;
> +err_free_dma:
> + dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
> + acb->dma_coherent2, acb->dma_coherent_handle2);
> + return false;
> }
>
The resource should be released here, that is okay.
How works the resource management when the eh_bus_reset_handler
is called ? It looks to me that you allocate a new
acb->dma_coherent2 without releasing it before.
Btw. is it needed in that handler to re-read the firmware-spec?
Tomas
> static bool arcmsr_hbaC_get_config(struct AdapterControlBlock *pACB)
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] target: use offset_in_page macro
Looks fine, Reviewed-by: Sagi Grimberg -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] target: use offset_in_page macro
On Wed, 2015-11-25 at 21:49 +0800, Geliang Tang wrote:
> Use offset_in_page macro instead of (addr & ~PAGE_MASK).
>
> Signed-off-by: Geliang Tang
> ---
> drivers/target/target_core_user.c | 2 +-
> drivers/target/tcm_fc/tfc_io.c| 8
> 2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/target/target_core_user.c
> b/drivers/target/target_core_user.c
> index 937cebf..d5477c0 100644
> --- a/drivers/target/target_core_user.c
> +++ b/drivers/target/target_core_user.c
> @@ -194,7 +194,7 @@ static struct tcmu_cmd *tcmu_alloc_cmd(struct se_cmd
> *se_cmd)
>
> static inline void tcmu_flush_dcache_range(void *vaddr, size_t size)
> {
> - unsigned long offset = (unsigned long) vaddr & ~PAGE_MASK;
> + unsigned long offset = offset_in_page(vaddr);
>
> size = round_up(size+offset, PAGE_SIZE);
> vaddr -= offset;
> diff --git a/drivers/target/tcm_fc/tfc_io.c b/drivers/target/tcm_fc/tfc_io.c
> index 847c1aa..6f7c65a 100644
> --- a/drivers/target/tcm_fc/tfc_io.c
> +++ b/drivers/target/tcm_fc/tfc_io.c
> @@ -154,9 +154,9 @@ int ft_queue_data_in(struct se_cmd *se_cmd)
> BUG_ON(!page);
> from = kmap_atomic(page + (mem_off >> PAGE_SHIFT));
> page_addr = from;
> - from += mem_off & ~PAGE_MASK;
> + from += offset_in_page(mem_off);
> tlen = min(tlen, (size_t)(PAGE_SIZE -
> - (mem_off & ~PAGE_MASK)));
> + offset_in_page(mem_off)));
> memcpy(to, from, tlen);
> kunmap_atomic(page_addr);
> to += tlen;
> @@ -314,9 +314,9 @@ void ft_recv_write_data(struct ft_cmd *cmd, struct
> fc_frame *fp)
>
> to = kmap_atomic(page + (mem_off >> PAGE_SHIFT));
> page_addr = to;
> - to += mem_off & ~PAGE_MASK;
> + to += offset_in_page(mem_off);
> tlen = min(tlen, (size_t)(PAGE_SIZE -
> - (mem_off & ~PAGE_MASK)));
> + offset_in_page(mem_off)));
> memcpy(to, from, tlen);
> kunmap_atomic(page_addr);
>
Reviewed-by: Johannes Thumshirn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] Fix a bdi reregistration race, v2
On 11/25/15 00:47, Christoph Hellwig wrote: But what I really wanted to ask for is what your reproducer looks like. Hello Christoph, This race is hard to trigger. I can trigger it by repeatedly removing and re-adding SRP SCSI devices. Enabling debug options like SLUB debugging and kmemleak helps. I think that is because these debug options slow down the SCSI device removal code and thereby increase the chance that this race is triggered. Bart. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] block: Add badblock management for gendisks
"Verma, Vishal L" writes: > On Tue, 2015-11-24 at 14:14 -0500, Jeff Moyer wrote: >> >> I'm not sure whether it makes sense to continue without badblock >> management for the RAID code. I was hoping Neil would comment on >> that. >> >> -Jeff > > Not sure I follow? I believe I've kept all the badblocks functionality > RAID already had.. What I mean to say is that the RAID code had previously embedded the badblocks structure in one of its other data structures. As a result, you would never get an allocation failure for it. > On a related note, something I observed when testing with md: > > md's badblock list is per-device, and can be seen at: > /sys/block/md0/md/dev-pmem0/bad_blocks > > Now if we have badblocks in the gendisks too, there is also: > /sys/block/pmem0/bad_blocks > > The two are separate 'accounts' maintained by separate drivers (md for > the former, and pmem for the latter). This can potentially be > confusing.. > > Should we consolidate the two, i.e. make md (re)use the gendisk > badblocks for its purposes too? I agree with what Dan said. Cheers, Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] block: Add badblock management for gendisks
On Wed, 2015-11-25 at 10:37 -0500, Jeff Moyer wrote: > "Verma, Vishal L" writes: > > > On Tue, 2015-11-24 at 14:14 -0500, Jeff Moyer wrote: > > > > > > I'm not sure whether it makes sense to continue without badblock > > > management for the RAID code. I was hoping Neil would comment on > > > that. > > > > > > -Jeff > > > > Not sure I follow? I believe I've kept all the badblocks > > functionality > > RAID already had.. > > What I mean to say is that the RAID code had previously embedded the > badblocks structure in one of its other data structures. As a result, > you would never get an allocation failure for it. > Ah I see - I don't think that has effectively changed. 'rdev' still contains a statically allocated badblocks structure (as opposed to gendisk, which just stores a pointer). md used to dynamically allocate the storage space inside badblocks (bb->page), and that is still the case using badblocks_init. -Vishal
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On 11/25/2015 02:04 AM, Hannes Reinecke wrote:
On 11/20/2015 04:28 PM, Ewan Milne wrote:
On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
Can't we have a joint effort here?
I've been spending a _LOT_ of time trying to debug things here, but
none of the ideas I've come up with have been able to fix anything.
Yes. I'm not the one primarily looking at it, and we don't have a
reproducer in-house. We just have the one dump right now.
I'm almost tempted to increase the count from scsi_alloc_sgtable()
by one and be done with ...
That might not fix it if it is a problem with the merge code, though.
And indeed, it doesn't.
Seems I finally found the culprit.
What happens is this:
We have two paths, with these seg_boundary_masks:
path-1:seg_boundary_mask = 65535,
path-2:seg_boundary_mask = 4294967295,
consequently the DM request queue has this:
md-1:seg_boundary_mask = 65535,
What happens now is that a request is being formatted, and sent
to path 2. During submission req->nr_phys_segments is formatted
with the limits of path 2, arriving at a count of 3.
Now the request gets retried on path 1, but as the NOMERGE request
flag is set req->nr_phys_segments is never updated.
But blk_rq_map_sg() ignores all counters, and just uses the
bi_vec directly, resulting in a count of 4 -> boom.
So the culprit here is the NOMERGE flag, which is evaluated
via
->dm_dispatch_request()
->blk_insert_cloned_request()
->blk_rq_check_limits()
If the above assessment is correct, the following patch should
fix it:
diff --git a/block/blk-core.c b/block/blk-core.c
index 801ced7..12cccd6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
*/
int blk_rq_check_limits(struct request_queue *q, struct request *rq)
{
- if (!rq_mergeable(rq))
+ if (rq->cmd_type != REQ_TYPE_FS)
return 0;
if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
rq->cmd_flags)) {
Mike? Jens?
Can you comment on it?
We only support merging on REQ_TYPE_FS already, so how is the above
making it any different? In general, NOMERGE being set or not should not
make a difference. It's only a hint that we need not check further if we
should be merging on this request, since we already tried it once, found
we'd exceed various limits, then set NOMERGE to reflect that.
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On Wed, Nov 25 2015 at 4:04am -0500,
Hannes Reinecke wrote:
> On 11/20/2015 04:28 PM, Ewan Milne wrote:
> > On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
> >> Can't we have a joint effort here?
> >> I've been spending a _LOT_ of time trying to debug things here, but
> >> none of the ideas I've come up with have been able to fix anything.
> >
> > Yes. I'm not the one primarily looking at it, and we don't have a
> > reproducer in-house. We just have the one dump right now.
> >
> >>
> >> I'm almost tempted to increase the count from scsi_alloc_sgtable()
> >> by one and be done with ...
> >>
> >
> > That might not fix it if it is a problem with the merge code, though.
> >
> And indeed, it doesn't.
How did you arrive at that? Do you have a reproducer now?
> Seems I finally found the culprit.
>
> What happens is this:
> We have two paths, with these seg_boundary_masks:
>
> path-1:seg_boundary_mask = 65535,
> path-2:seg_boundary_mask = 4294967295,
>
> consequently the DM request queue has this:
>
> md-1:seg_boundary_mask = 65535,
>
> What happens now is that a request is being formatted, and sent
> to path 2. During submission req->nr_phys_segments is formatted
> with the limits of path 2, arriving at a count of 3.
> Now the request gets retried on path 1, but as the NOMERGE request
> flag is set req->nr_phys_segments is never updated.
> But blk_rq_map_sg() ignores all counters, and just uses the
> bi_vec directly, resulting in a count of 4 -> boom.
>
> So the culprit here is the NOMERGE flag,
NOMERGE is always set in __blk_rq_prep_clone() for cloned requests.
> which is evaluated via
> ->dm_dispatch_request()
> ->blk_insert_cloned_request()
> ->blk_rq_check_limits()
blk_insert_cloned_request() is the only caller of blk_rq_check_limits();
anyway after reading your mail I'm still left wondering if your proposed
patch is correct.
> If the above assessment is correct, the following patch should
> fix it:
>
> diff --git a/block/blk-core.c b/block/blk-core.c
> index 801ced7..12cccd6 100644
> --- a/block/blk-core.c
> +++ b/block/blk-core.c
> @@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
> */
> int blk_rq_check_limits(struct request_queue *q, struct request *rq)
> {
> - if (!rq_mergeable(rq))
> + if (rq->cmd_type != REQ_TYPE_FS)
> return 0;
>
> if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
> rq->cmd_flags)) {
>
>
> Mike? Jens?
> Can you comment on it?
You're not explaining the actual change in the patch very well; I think
you're correct but you're leaving the justification as an exercise to
the reviewer:
blk_rq_check_limits() will call blk_recalc_rq_segments() after the
!rq_mergeable() check but you're saying for this case in question we
never get there -- due to the cloned request having NOMERGE set.
So in blk_rq_check_limits() you've unrolled rq_mergeable() and
open-coded the lone remaining check (rq->cmd_type != REQ_TYPE_FS)
I agree that the (rq->cmd_flags & REQ_NOMERGE_FLAGS) check in
the blk_insert_cloned_request() call-chain (via rq_mergeable()) makes no
sense for cloned requests that always have NOMERGE set.
So you're saying that by having blk_rq_check_limits() go on to call
blk_recalc_rq_segments() this bug will be fixed?
BTW, I think blk_rq_check_limits()'s export should be removed and the
function made static and renamed to blk_clone_rq_check_limits(), again:
blk_insert_cloned_request() is the only caller of blk_rq_check_limits()
Seems prudent to make that change now to be clear that this code is only
used by cloned requests.
Thanks,
Mike
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 2/3] block: Add badblock management for gendisks
"Verma, Vishal L" writes: > On Wed, 2015-11-25 at 10:37 -0500, Jeff Moyer wrote: >> "Verma, Vishal L" writes: >> >> > On Tue, 2015-11-24 at 14:14 -0500, Jeff Moyer wrote: >> > > >> > > I'm not sure whether it makes sense to continue without badblock >> > > management for the RAID code. I was hoping Neil would comment on >> > > that. >> > > >> > > -Jeff >> > >> > Not sure I follow? I believe I've kept all the badblocks >> > functionality >> > RAID already had.. >> >> What I mean to say is that the RAID code had previously embedded the >> badblocks structure in one of its other data structures. As a result, >> you would never get an allocation failure for it. >> > Ah I see - I don't think that has effectively changed. 'rdev' still > contains a statically allocated badblocks structure (as opposed to > gendisk, which just stores a pointer). md used to dynamically allocate > the storage space inside badblocks (bb->page), and that is still the > case using badblocks_init. Ah, ok. Sorry I didn't dig into that further. Thanks, Jeff -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 2/3] block: Add badblock management for gendisks
NVDIMM devices, which can behave more like DRAM rather than block
devices, may develop bad cache lines, or 'poison'. A block device
exposed by the pmem driver can then consume poison via a read (or
write), and cause a machine check. On platforms without machine
check recovery features, this would mean a crash.
The block device maintaining a runtime list of all known sectors that
have poison can directly avoid this, and also provide a path forward
to enable proper handling/recovery for DAX faults on such a device.
Use the new badblock management interfaces to add a badblocks list to
gendisks.
Signed-off-by: Vishal Verma
---
block/genhd.c | 81 +++
include/linux/genhd.h | 6
2 files changed, 87 insertions(+)
diff --git a/block/genhd.c b/block/genhd.c
index 0c706f3..84fd65c 100644
--- a/block/genhd.c
+++ b/block/genhd.c
@@ -20,6 +20,7 @@
#include
#include
#include
+#include
#include "blk.h"
@@ -505,6 +506,20 @@ static int exact_lock(dev_t devt, void *data)
return 0;
}
+static void disk_alloc_badblocks(struct gendisk *disk)
+{
+ disk->bb = kzalloc(sizeof(*(disk->bb)), GFP_KERNEL);
+ if (!disk->bb) {
+ pr_warn("%s: failed to allocate space for badblocks\n",
+ disk->disk_name);
+ return;
+ }
+
+ if (badblocks_init(disk->bb, 1))
+ pr_warn("%s: failed to initialize badblocks\n",
+ disk->disk_name);
+}
+
static void register_disk(struct gendisk *disk)
{
struct device *ddev = disk_to_dev(disk);
@@ -609,6 +624,7 @@ void add_disk(struct gendisk *disk)
disk->first_minor = MINOR(devt);
disk_alloc_events(disk);
+ disk_alloc_badblocks(disk);
/* Register BDI before referencing it from bdev */
bdi = &disk->queue->backing_dev_info;
@@ -657,6 +673,11 @@ void del_gendisk(struct gendisk *disk)
blk_unregister_queue(disk);
blk_unregister_region(disk_devt(disk), disk->minors);
+ if (disk->bb) {
+ badblocks_free(disk->bb);
+ kfree(disk->bb);
+ }
+
part_stat_set_all(&disk->part0, 0);
disk->part0.stamp = 0;
@@ -670,6 +691,63 @@ void del_gendisk(struct gendisk *disk)
}
EXPORT_SYMBOL(del_gendisk);
+/*
+ * The gendisk usage of badblocks does not track acknowledgements for
+ * badblocks. We always assume they are acknowledged.
+ */
+int disk_check_badblocks(struct gendisk *disk, sector_t s, int sectors,
+ sector_t *first_bad, int *bad_sectors)
+{
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_check(disk->bb, s, sectors, first_bad, bad_sectors);
+}
+EXPORT_SYMBOL(disk_check_badblocks);
+
+int disk_set_badblocks(struct gendisk *disk, sector_t s, int sectors)
+{
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_set(disk->bb, s, sectors, 1);
+}
+EXPORT_SYMBOL(disk_set_badblocks);
+
+int disk_clear_badblocks(struct gendisk *disk, sector_t s, int sectors)
+{
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_clear(disk->bb, s, sectors);
+}
+EXPORT_SYMBOL(disk_clear_badblocks);
+
+/* sysfs access to bad-blocks list. */
+static ssize_t disk_badblocks_show(struct device *dev,
+ struct device_attribute *attr,
+ char *page)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_show(disk->bb, page, 0);
+}
+
+static ssize_t disk_badblocks_store(struct device *dev,
+ struct device_attribute *attr,
+ const char *page, size_t len)
+{
+ struct gendisk *disk = dev_to_disk(dev);
+
+ if (!disk->bb)
+ return 0;
+
+ return badblocks_store(disk->bb, page, len, 0);
+}
+
/**
* get_gendisk - get partitioning information for a given device
* @devt: device to get partitioning information for
@@ -988,6 +1066,8 @@ static DEVICE_ATTR(discard_alignment, S_IRUGO,
disk_discard_alignment_show,
static DEVICE_ATTR(capability, S_IRUGO, disk_capability_show, NULL);
static DEVICE_ATTR(stat, S_IRUGO, part_stat_show, NULL);
static DEVICE_ATTR(inflight, S_IRUGO, part_inflight_show, NULL);
+static DEVICE_ATTR(badblocks, S_IRUGO | S_IWUSR, disk_badblocks_show,
+ disk_badblocks_store);
#ifdef CONFIG_FAIL_MAKE_REQUEST
static struct device_attribute dev_attr_fail =
__ATTR(make-it-fail, S_IRUGO|S_IWUSR, part_fail_show, part_fail_store);
@@ -1009,6 +1089,7 @@ static struct attribute *disk_attrs[] = {
&dev_attr_capability.attr,
&dev_attr_stat.attr,
&dev_attr_inflight.attr,
+ &dev_attr_badblocks.attr,
#ifdef CONFIG_FAIL_MAKE_REQUEST
&dev_attr_fail.attr,
#endif
diff --git a/include/linux/genhd.h b/include/linux/genhd.h
index 2adbfa6..5563bde
[PATCH v2 0/3] Badblock tracking for gendisks
v2: - In badblocks_free, make 'page' NULL (patch 1) - Move the core badblocks code to a new .c file (patch 1) (Jens) - Fix a sizeof usage in disk_alloc_badblocks (patch 2) (Dan) - Since disk_alloc_badblocks can fail, check disk->bb for NULL in the genhd wrappers (patch 2) (Jeff) - Update the md conversion to also ise the badblocks init and free functions (patch 3) - Remove the BB_* macros from md.h as they are now in badblocks.h (patch 3) Patch 1 copies badblock management code into a header of its own, making it generally available. It follows common libraries of code such as linked lists, where anyone may embed a core data structure in another place, and use the provided accessor functions to manipulate the data. Patch 2 adds badblock tracking to gendisks (in preparation for use by NVDIMM devices). Right now, it is turned on unconditionally - I'd appreciate comments on if that is the right path. Patch 3 converts md over to use the new badblocks 'library'. I have done some pretty simple testing on this - created a raid 1 device, made sure the sysfs entries show up, and can be used to add and view badblocks. A closer look by the md folks would be nice here. Vishal Verma (3): badblocks: Add core badblock management code block: Add badblock management for gendisks md: convert to use the generic badblocks code block/Makefile| 2 +- block/badblocks.c | 523 ++ block/genhd.c | 81 +++ drivers/md/md.c | 507 ++-- drivers/md/md.h | 40 +--- include/linux/badblocks.h | 53 + include/linux/genhd.h | 6 + 7 files changed, 687 insertions(+), 525 deletions(-) create mode 100644 block/badblocks.c create mode 100644 include/linux/badblocks.h -- 2.5.0 -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 3/3] md: convert to use the generic badblocks code
Retain badblocks as part of rdev, but use the accessor functions from
include/linux/badblocks for all manipulation.
Signed-off-by: Vishal Verma
---
drivers/md/md.c | 507 +++-
drivers/md/md.h | 40 +
2 files changed, 23 insertions(+), 524 deletions(-)
diff --git a/drivers/md/md.c b/drivers/md/md.c
index c702de1..63eab20 100644
--- a/drivers/md/md.c
+++ b/drivers/md/md.c
@@ -34,6 +34,7 @@
#include
#include
+#include
#include
#include
#include
@@ -707,8 +708,7 @@ void md_rdev_clear(struct md_rdev *rdev)
put_page(rdev->bb_page);
rdev->bb_page = NULL;
}
- kfree(rdev->badblocks.page);
- rdev->badblocks.page = NULL;
+ badblocks_free(&rdev->badblocks);
}
EXPORT_SYMBOL_GPL(md_rdev_clear);
@@ -1358,8 +1358,6 @@ static __le32 calc_sb_1_csum(struct mdp_superblock_1 *sb)
return cpu_to_le32(csum);
}
-static int md_set_badblocks(struct badblocks *bb, sector_t s, int sectors,
- int acknowledged);
static int super_1_load(struct md_rdev *rdev, struct md_rdev *refdev, int
minor_version)
{
struct mdp_superblock_1 *sb;
@@ -1484,7 +1482,7 @@ static int super_1_load(struct md_rdev *rdev, struct
md_rdev *refdev, int minor_
count <<= sb->bblog_shift;
if (bb + 1 == 0)
break;
- if (md_set_badblocks(&rdev->badblocks,
+ if (badblocks_set(&rdev->badblocks,
sector, count, 1) == 0)
return -EINVAL;
}
@@ -2226,7 +2224,7 @@ repeat:
rdev_for_each(rdev, mddev) {
if (rdev->badblocks.changed) {
rdev->badblocks.changed = 0;
- md_ack_all_badblocks(&rdev->badblocks);
+ ack_all_badblocks(&rdev->badblocks);
md_error(mddev, rdev);
}
clear_bit(Blocked, &rdev->flags);
@@ -2352,7 +2350,7 @@ repeat:
clear_bit(Blocked, &rdev->flags);
if (any_badblocks_changed)
- md_ack_all_badblocks(&rdev->badblocks);
+ ack_all_badblocks(&rdev->badblocks);
clear_bit(BlockedBadBlocks, &rdev->flags);
wake_up(&rdev->blocked_wait);
}
@@ -2944,11 +2942,17 @@ static ssize_t recovery_start_store(struct md_rdev
*rdev, const char *buf, size_
static struct rdev_sysfs_entry rdev_recovery_start =
__ATTR(recovery_start, S_IRUGO|S_IWUSR, recovery_start_show,
recovery_start_store);
-static ssize_t
-badblocks_show(struct badblocks *bb, char *page, int unack);
-static ssize_t
-badblocks_store(struct badblocks *bb, const char *page, size_t len, int unack);
-
+/* sysfs access to bad-blocks list.
+ * We present two files.
+ * 'bad-blocks' lists sector numbers and lengths of ranges that
+ *are recorded as bad. The list is truncated to fit within
+ *the one-page limit of sysfs.
+ *Writing "sector length" to this file adds an acknowledged
+ *bad block list.
+ * 'unacknowledged-bad-blocks' lists bad blocks that have not yet
+ *been acknowledged. Writing to this file adds bad blocks
+ *without acknowledging them. This is largely for testing.
+ */
static ssize_t bb_show(struct md_rdev *rdev, char *page)
{
return badblocks_show(&rdev->badblocks, page, 0);
@@ -3063,14 +3067,7 @@ int md_rdev_init(struct md_rdev *rdev)
* This reserves the space even on arrays where it cannot
* be used - I wonder if that matters
*/
- rdev->badblocks.count = 0;
- rdev->badblocks.shift = -1; /* disabled until explicitly enabled */
- rdev->badblocks.page = kmalloc(PAGE_SIZE, GFP_KERNEL);
- seqlock_init(&rdev->badblocks.lock);
- if (rdev->badblocks.page == NULL)
- return -ENOMEM;
-
- return 0;
+ return badblocks_init(&rdev->badblocks, 0);
}
EXPORT_SYMBOL_GPL(md_rdev_init);
/*
@@ -8348,253 +8345,7 @@ void md_finish_reshape(struct mddev *mddev)
}
EXPORT_SYMBOL(md_finish_reshape);
-/* Bad block management.
- * We can record which blocks on each device are 'bad' and so just
- * fail those blocks, or that stripe, rather than the whole device.
- * Entries in the bad-block table are 64bits wide. This comprises:
- * Length of bad-range, in sectors: 0-511 for lengths 1-512
- * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
- * A 'shift' can be set so that larger blocks are tracked and
- * consequently larger devices can be covered.
- * 'Acknowledged' flag - 1 bit. - the most significant bit.
- *
- * Locking of the bad-block table uses a seqlock so md_is_badblock
- * might need to retry if it
[PATCH v2 1/3] badblocks: Add core badblock management code
Take the core badblocks implementation from md, and make it generally
available. This follows the same style as kernel implementations of
linked lists, rb-trees etc, where you can have a structure that can be
embedded anywhere, and accessor functions to manipulate the data.
The only changes in this copy of the code are ones to generalize
function/variable names from md-specific ones. Also add init and free
functions.
Signed-off-by: Vishal Verma
---
block/Makefile| 2 +-
block/badblocks.c | 523 ++
include/linux/badblocks.h | 53 +
3 files changed, 577 insertions(+), 1 deletion(-)
create mode 100644 block/badblocks.c
create mode 100644 include/linux/badblocks.h
diff --git a/block/Makefile b/block/Makefile
index 00ecc97..db5f622 100644
--- a/block/Makefile
+++ b/block/Makefile
@@ -8,7 +8,7 @@ obj-$(CONFIG_BLOCK) := bio.o elevator.o blk-core.o blk-tag.o
blk-sysfs.o \
blk-iopoll.o blk-lib.o blk-mq.o blk-mq-tag.o \
blk-mq-sysfs.o blk-mq-cpu.o blk-mq-cpumap.o ioctl.o \
genhd.o scsi_ioctl.o partition-generic.o ioprio.o \
- partitions/
+ badblocks.o partitions/
obj-$(CONFIG_BOUNCE) += bounce.o
obj-$(CONFIG_BLK_DEV_BSG) += bsg.o
diff --git a/block/badblocks.c b/block/badblocks.c
new file mode 100644
index 000..6e07855
--- /dev/null
+++ b/block/badblocks.c
@@ -0,0 +1,523 @@
+/*
+ * Bad block management
+ *
+ * - Heavily based on MD badblocks code from Neil Brown
+ *
+ * Copyright (c) 2015, Intel Corporation.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * This program is distributed in the hope it will be useful, but WITHOUT
+ * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
+ * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
+ * more details.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+/*
+ * We can record which blocks on each device are 'bad' and so just
+ * fail those blocks, or that stripe, rather than the whole device.
+ * Entries in the bad-block table are 64bits wide. This comprises:
+ * Length of bad-range, in sectors: 0-511 for lengths 1-512
+ * Start of bad-range, sector offset, 54 bits (allows 8 exbibytes)
+ * A 'shift' can be set so that larger blocks are tracked and
+ * consequently larger devices can be covered.
+ * 'Acknowledged' flag - 1 bit. - the most significant bit.
+ *
+ * Locking of the bad-block table uses a seqlock so badblocks_check
+ * might need to retry if it is very unlucky.
+ * We will sometimes want to check for bad blocks in a bi_end_io function,
+ * so we use the write_seqlock_irq variant.
+ *
+ * When looking for a bad block we specify a range and want to
+ * know if any block in the range is bad. So we binary-search
+ * to the last range that starts at-or-before the given endpoint,
+ * (or "before the sector after the target range")
+ * then see if it ends after the given start.
+ * We return
+ * 0 if there are no known bad blocks in the range
+ * 1 if there are known bad block which are all acknowledged
+ * -1 if there are bad blocks which have not yet been acknowledged in metadata.
+ * plus the start/length of the first bad section we overlap.
+ */
+int badblocks_check(struct badblocks *bb, sector_t s, int sectors,
+ sector_t *first_bad, int *bad_sectors)
+{
+ int hi;
+ int lo;
+ u64 *p = bb->page;
+ int rv;
+ sector_t target = s + sectors;
+ unsigned seq;
+
+ if (bb->shift > 0) {
+ /* round the start down, and the end up */
+ s >>= bb->shift;
+ target += (1>= bb->shift;
+ sectors = target - s;
+ }
+ /* 'target' is now the first block after the bad range */
+
+retry:
+ seq = read_seqbegin(&bb->lock);
+ lo = 0;
+ rv = 0;
+ hi = bb->count;
+
+ /* Binary search between lo and hi for 'target'
+* i.e. for the last range that starts before 'target'
+*/
+ /* INVARIANT: ranges before 'lo' and at-or-after 'hi'
+* are known not to be the last range before target.
+* VARIANT: hi-lo is the number of possible
+* ranges, and decreases until it reaches 1
+*/
+ while (hi - lo > 1) {
+ int mid = (lo + hi) / 2;
+ sector_t a = BB_OFFSET(p[mid]);
+
+ if (a < target)
+ /* This could still be the one, earlier ranges
+* could not.
+*/
+ lo = mid;
+ else
+ /* This and later ranges are definitely out. */
+
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On 11/25/2015 07:01 PM, Mike Snitzer wrote:
On Wed, Nov 25 2015 at 4:04am -0500,
Hannes Reinecke wrote:
On 11/20/2015 04:28 PM, Ewan Milne wrote:
On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
Can't we have a joint effort here?
I've been spending a _LOT_ of time trying to debug things here, but
none of the ideas I've come up with have been able to fix anything.
Yes. I'm not the one primarily looking at it, and we don't have a
reproducer in-house. We just have the one dump right now.
I'm almost tempted to increase the count from scsi_alloc_sgtable()
by one and be done with ...
That might not fix it if it is a problem with the merge code, though.
And indeed, it doesn't.
How did you arrive at that? Do you have a reproducer now?
Not a reproducer, but several dumps for analysis.
Seems I finally found the culprit.
What happens is this:
We have two paths, with these seg_boundary_masks:
path-1:seg_boundary_mask = 65535,
path-2:seg_boundary_mask = 4294967295,
consequently the DM request queue has this:
md-1:seg_boundary_mask = 65535,
What happens now is that a request is being formatted, and sent
to path 2. During submission req->nr_phys_segments is formatted
with the limits of path 2, arriving at a count of 3.
Now the request gets retried on path 1, but as the NOMERGE request
flag is set req->nr_phys_segments is never updated.
But blk_rq_map_sg() ignores all counters, and just uses the
bi_vec directly, resulting in a count of 4 -> boom.
So the culprit here is the NOMERGE flag,
NOMERGE is always set in __blk_rq_prep_clone() for cloned requests.
Yes.
which is evaluated via
->dm_dispatch_request()
->blk_insert_cloned_request()
->blk_rq_check_limits()
blk_insert_cloned_request() is the only caller of blk_rq_check_limits();
anyway after reading your mail I'm still left wondering if your proposed
patch is correct.
If the above assessment is correct, the following patch should
fix it:
diff --git a/block/blk-core.c b/block/blk-core.c
index 801ced7..12cccd6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
*/
int blk_rq_check_limits(struct request_queue *q, struct request *rq)
{
- if (!rq_mergeable(rq))
+ if (rq->cmd_type != REQ_TYPE_FS)
return 0;
if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
rq->cmd_flags)) {
Mike? Jens?
Can you comment on it?
You're not explaining the actual change in the patch very well; I think
you're correct but you're leaving the justification as an exercise to
the reviewer:
blk_rq_check_limits() will call blk_recalc_rq_segments() after the
!rq_mergeable() check but you're saying for this case in question we
never get there -- due to the cloned request having NOMERGE set.
So in blk_rq_check_limits() you've unrolled rq_mergeable() and
open-coded the lone remaining check (rq->cmd_type != REQ_TYPE_FS)
I agree that the (rq->cmd_flags & REQ_NOMERGE_FLAGS) check in
the blk_insert_cloned_request() call-chain (via rq_mergeable()) makes no
sense for cloned requests that always have NOMERGE set.
So you're saying that by having blk_rq_check_limits() go on to call
blk_recalc_rq_segments() this bug will be fixed?
That is the idea.
I've already established that in all instances I have seen so far
req->nr_phys_segments is _less_ than req->bio->bi_phys_segments.
As it turns out, req->nr_phys_segemnts _would_ have been updated in
blk_rq_check_limits(), but isn't due to the NOMERGE flag being set
for the cloned request.
So each cloned request inherits the values from the original request,
despite the fact that req->nr_phys_segments _has_ to be evaluated in
the final request_queue context, as the queue limits _might_ be
different from the original (merged) queue limits of the multipath
request queue.
BTW, I think blk_rq_check_limits()'s export should be removed and the
function made static and renamed to blk_clone_rq_check_limits(), again:
blk_insert_cloned_request() is the only caller of blk_rq_check_limits()
Actually, seeing Jens' last comment the check for REQ_TYPE_FS is
pointless, too, so we might as well remove the entire if-clause.
Seems prudent to make that change now to be clear that this code is only
used by cloned requests.
Yeah, that would make sense. I'll be preparing a patch.
With a more detailed description :-)
Cheers,
Hannes
--
Dr. Hannes ReineckezSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On 11/25/2015 06:56 PM, Jens Axboe wrote:
On 11/25/2015 02:04 AM, Hannes Reinecke wrote:
On 11/20/2015 04:28 PM, Ewan Milne wrote:
On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
Can't we have a joint effort here?
I've been spending a _LOT_ of time trying to debug things here, but
none of the ideas I've come up with have been able to fix anything.
Yes. I'm not the one primarily looking at it, and we don't have a
reproducer in-house. We just have the one dump right now.
I'm almost tempted to increase the count from scsi_alloc_sgtable()
by one and be done with ...
That might not fix it if it is a problem with the merge code, though.
And indeed, it doesn't.
Seems I finally found the culprit.
What happens is this:
We have two paths, with these seg_boundary_masks:
path-1:seg_boundary_mask = 65535,
path-2:seg_boundary_mask = 4294967295,
consequently the DM request queue has this:
md-1:seg_boundary_mask = 65535,
What happens now is that a request is being formatted, and sent
to path 2. During submission req->nr_phys_segments is formatted
with the limits of path 2, arriving at a count of 3.
Now the request gets retried on path 1, but as the NOMERGE request
flag is set req->nr_phys_segments is never updated.
But blk_rq_map_sg() ignores all counters, and just uses the
bi_vec directly, resulting in a count of 4 -> boom.
So the culprit here is the NOMERGE flag, which is evaluated
via
->dm_dispatch_request()
->blk_insert_cloned_request()
->blk_rq_check_limits()
If the above assessment is correct, the following patch should
fix it:
diff --git a/block/blk-core.c b/block/blk-core.c
index 801ced7..12cccd6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
*/
int blk_rq_check_limits(struct request_queue *q, struct request *rq)
{
- if (!rq_mergeable(rq))
+ if (rq->cmd_type != REQ_TYPE_FS)
return 0;
if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
rq->cmd_flags)) {
Mike? Jens?
Can you comment on it?
We only support merging on REQ_TYPE_FS already, so how is the above
making it any different? In general, NOMERGE being set or not should not
make a difference. It's only a hint that we need not check further if we
should be merging on this request, since we already tried it once, found
we'd exceed various limits, then set NOMERGE to reflect that.
The problem is that NOMERGE does too much, as it inhibits _any_ merging.
Unfortunately, the req->nr_phys_segments value is evaluated in the final
_driver_ context _after_ the merging happend; cf
scsi_lib.c:scsi_init_sgtable().
As nr_phys_segments is inherited from the original request (and never
recalculated with the new request queue limits) the following
blk_rq_map_sg() call might end up at a different calculation, especially
after retrying a request on another path.
Cheers,
Hannes
--
Dr. Hannes ReineckezSeries & Storage
h...@suse.de +49 911 74053 688
SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg
GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton
HRB 21284 (AG Nürnberg)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On 11/25/2015 12:10 PM, Hannes Reinecke wrote:
On 11/25/2015 06:56 PM, Jens Axboe wrote:
On 11/25/2015 02:04 AM, Hannes Reinecke wrote:
On 11/20/2015 04:28 PM, Ewan Milne wrote:
On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
Can't we have a joint effort here?
I've been spending a _LOT_ of time trying to debug things here, but
none of the ideas I've come up with have been able to fix anything.
Yes. I'm not the one primarily looking at it, and we don't have a
reproducer in-house. We just have the one dump right now.
I'm almost tempted to increase the count from scsi_alloc_sgtable()
by one and be done with ...
That might not fix it if it is a problem with the merge code, though.
And indeed, it doesn't.
Seems I finally found the culprit.
What happens is this:
We have two paths, with these seg_boundary_masks:
path-1:seg_boundary_mask = 65535,
path-2:seg_boundary_mask = 4294967295,
consequently the DM request queue has this:
md-1:seg_boundary_mask = 65535,
What happens now is that a request is being formatted, and sent
to path 2. During submission req->nr_phys_segments is formatted
with the limits of path 2, arriving at a count of 3.
Now the request gets retried on path 1, but as the NOMERGE request
flag is set req->nr_phys_segments is never updated.
But blk_rq_map_sg() ignores all counters, and just uses the
bi_vec directly, resulting in a count of 4 -> boom.
So the culprit here is the NOMERGE flag, which is evaluated
via
->dm_dispatch_request()
->blk_insert_cloned_request()
->blk_rq_check_limits()
If the above assessment is correct, the following patch should
fix it:
diff --git a/block/blk-core.c b/block/blk-core.c
index 801ced7..12cccd6 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
*/
int blk_rq_check_limits(struct request_queue *q, struct request *rq)
{
- if (!rq_mergeable(rq))
+ if (rq->cmd_type != REQ_TYPE_FS)
return 0;
if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
rq->cmd_flags)) {
Mike? Jens?
Can you comment on it?
We only support merging on REQ_TYPE_FS already, so how is the above
making it any different? In general, NOMERGE being set or not should not
make a difference. It's only a hint that we need not check further if we
should be merging on this request, since we already tried it once, found
we'd exceed various limits, then set NOMERGE to reflect that.
The problem is that NOMERGE does too much, as it inhibits _any_ merging.
Right, that is the point of the flag from the block layer view, where it
was originally added for the case mentioned.
Unfortunately, the req->nr_phys_segments value is evaluated in the final
_driver_ context _after_ the merging happend; cf
scsi_lib.c:scsi_init_sgtable().
As nr_phys_segments is inherited from the original request (and never
recalculated with the new request queue limits) the following
blk_rq_map_sg() call might end up at a different calculation, especially
after retrying a request on another path.
That all sounds pretty horrible. Why is blk_rq_check_limits() checking
for mergeable at all? If merging is disabled on the request, I'm
assuming that's an attempt at an optimization since we know it won't
change. But that should be tracked separately, like how it's done on the
bio.
--
Jens Axboe
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On Wed, Nov 25 2015 at 2:24pm -0500,
Jens Axboe wrote:
> On 11/25/2015 12:10 PM, Hannes Reinecke wrote:
> >On 11/25/2015 06:56 PM, Jens Axboe wrote:
> >>On 11/25/2015 02:04 AM, Hannes Reinecke wrote:
> >>>On 11/20/2015 04:28 PM, Ewan Milne wrote:
> On Fri, 2015-11-20 at 15:55 +0100, Hannes Reinecke wrote:
> >Can't we have a joint effort here?
> >I've been spending a _LOT_ of time trying to debug things here, but
> >none of the ideas I've come up with have been able to fix anything.
>
> Yes. I'm not the one primarily looking at it, and we don't have a
> reproducer in-house. We just have the one dump right now.
>
> >
> >I'm almost tempted to increase the count from scsi_alloc_sgtable()
> >by one and be done with ...
> >
>
> That might not fix it if it is a problem with the merge code, though.
>
> >>>And indeed, it doesn't.
> >>>Seems I finally found the culprit.
> >>>
> >>>What happens is this:
> >>>We have two paths, with these seg_boundary_masks:
> >>>
> >>>path-1:seg_boundary_mask = 65535,
> >>>path-2:seg_boundary_mask = 4294967295,
> >>>
> >>>consequently the DM request queue has this:
> >>>
> >>>md-1:seg_boundary_mask = 65535,
> >>>
> >>>What happens now is that a request is being formatted, and sent
> >>>to path 2. During submission req->nr_phys_segments is formatted
> >>>with the limits of path 2, arriving at a count of 3.
> >>>Now the request gets retried on path 1, but as the NOMERGE request
> >>>flag is set req->nr_phys_segments is never updated.
> >>>But blk_rq_map_sg() ignores all counters, and just uses the
> >>>bi_vec directly, resulting in a count of 4 -> boom.
> >>>
> >>>So the culprit here is the NOMERGE flag, which is evaluated
> >>>via
> >>>->dm_dispatch_request()
> >>> ->blk_insert_cloned_request()
> >>> ->blk_rq_check_limits()
> >>>
> >>>If the above assessment is correct, the following patch should
> >>>fix it:
> >>>
> >>>diff --git a/block/blk-core.c b/block/blk-core.c
> >>>index 801ced7..12cccd6 100644
> >>>--- a/block/blk-core.c
> >>>+++ b/block/blk-core.c
> >>>@@ -1928,7 +1928,7 @@ EXPORT_SYMBOL(submit_bio);
> >>> */
> >>> int blk_rq_check_limits(struct request_queue *q, struct request *rq)
> >>> {
> >>>- if (!rq_mergeable(rq))
> >>>+ if (rq->cmd_type != REQ_TYPE_FS)
> >>> return 0;
> >>>
> >>> if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q,
> >>>rq->cmd_flags)) {
> >>>
> >>>
> >>>Mike? Jens?
> >>>Can you comment on it?
> >>
> >>We only support merging on REQ_TYPE_FS already, so how is the above
> >>making it any different? In general, NOMERGE being set or not should not
> >>make a difference. It's only a hint that we need not check further if we
> >>should be merging on this request, since we already tried it once, found
> >>we'd exceed various limits, then set NOMERGE to reflect that.
> >>
> >The problem is that NOMERGE does too much, as it inhibits _any_ merging.
>
> Right, that is the point of the flag from the block layer view,
> where it was originally added for the case mentioned.
And we really don't want _any_ merging. The merging, if any, will have
already happened in upper DM-multipath's elevator. So there should be
no need to have the underlying SCSI paths do any merging.
> >Unfortunately, the req->nr_phys_segments value is evaluated in the final
> >_driver_ context _after_ the merging happend; cf
> >scsi_lib.c:scsi_init_sgtable().
> >As nr_phys_segments is inherited from the original request (and never
> >recalculated with the new request queue limits) the following
> >blk_rq_map_sg() call might end up at a different calculation, especially
> >after retrying a request on another path.
>
> That all sounds pretty horrible. Why is blk_rq_check_limits()
> checking for mergeable at all? If merging is disabled on the
> request, I'm assuming that's an attempt at an optimization since we
> know it won't change. But that should be tracked separately, like
> how it's done on the bio.
Not clear to me why it was checking for merging...
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH RESEND] scsi_debug: fix prevent_allow+verify regressions
Martin, sorry for offtopic, but can you pay a little attention to http://www.spinics.net/lists/linux-scsi/msg81778.html ? It seems it wasn't applied. -- With Best Regards, Andy Shevchenko -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: kernel BUG at drivers/scsi/scsi_lib.c:1096!
On Wed, Nov 25 2015 at 3:23pm -0500,
Mike Snitzer wrote:
> On Wed, Nov 25 2015 at 2:24pm -0500,
> Jens Axboe wrote:
>
> > On 11/25/2015 12:10 PM, Hannes Reinecke wrote:
> > >The problem is that NOMERGE does too much, as it inhibits _any_ merging.
> >
> > Right, that is the point of the flag from the block layer view,
> > where it was originally added for the case mentioned.
>
> And we really don't want _any_ merging. The merging, if any, will have
> already happened in upper DM-multipath's elevator. So there should be
> no need to have the underlying SCSI paths do any merging.
>
> > >Unfortunately, the req->nr_phys_segments value is evaluated in the final
> > >_driver_ context _after_ the merging happend; cf
> > >scsi_lib.c:scsi_init_sgtable().
> > >As nr_phys_segments is inherited from the original request (and never
> > >recalculated with the new request queue limits) the following
> > >blk_rq_map_sg() call might end up at a different calculation, especially
> > >after retrying a request on another path.
> >
> > That all sounds pretty horrible. Why is blk_rq_check_limits()
> > checking for mergeable at all? If merging is disabled on the
> > request, I'm assuming that's an attempt at an optimization since we
> > know it won't change. But that should be tracked separately, like
> > how it's done on the bio.
>
> Not clear to me why it was checking for merging...
Ewan pointed out that blk_rq_check_limits()'s call to rq_mergable() was
introduced as part of Martin's DISCARD cleanup that prepared for
WRITE_SAME, see: commit e2a60da74 ("block: Clean up special command handling
logic")
And prior to that, blk_rq_check_limits()'s (rq->cmd_flags & REQ_DISCARD)
check was introduced by some guy crazy doppelganger name "Ike Snitzer",
see commit: 3383977f ("block: update request stacking methods to support
discards")
So basically we probably never needed the extra check in
blk_rq_check_limits() to begin with.. Ike was probably paranoid. ;)
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH 72/71] ncr5380: Fix pseudo-DMA
Pseudo-DMA (PDMA) has been broken for ages, resulting in hangs on 53C400-based cards. According to 53C400 datasheet, PDMA transfer length must be a multiple of 128. Check if that's true and use PIO if it's not. This makes PDMA work on 53C400 (Canon FG2-5202). Signed-off-by: Ondrej Zary --- drivers/scsi/g_NCR5380.c |4 1 file changed, 4 insertions(+) diff --git a/drivers/scsi/g_NCR5380.c b/drivers/scsi/g_NCR5380.c index 0daffe2..a9a237f 100644 --- a/drivers/scsi/g_NCR5380.c +++ b/drivers/scsi/g_NCR5380.c @@ -703,6 +703,10 @@ static int generic_NCR5380_dma_xfer_len(struct scsi_cmnd *cmd) !(cmd->SCp.this_residual % transfersize)) transfersize = 32 * 1024; + /* 53C400 datasheet: non-modulo-128-byte transfers should use PIO */ + if (transfersize % 128) + transfersize = 0; + return transfersize; } -- Ondrej Zary -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 22/71] ncr5380: Eliminate selecting state
On Wednesday 25 November 2015 04:17:09 Finn Thain wrote:
>
> On Tue, 24 Nov 2015, Ondrej Zary wrote:
>
> > On Wednesday 18 November 2015 09:35:17 Finn Thain wrote:
> > > Linux v2.1.105 changed the algorithm for polling for the BSY signal
> > > in NCR5380_select() and NCR5380_main().
> > >
> > > Presently, this code has a bug. Back then, NCR5380_set_timer(hostdata, 1)
> > > meant reschedule main() after sleeping for 10 ms. Repeated 25 times this
> > > provided the recommended 250 ms selection time-out delay. This got broken
> > > when HZ became configurable.
> > >
> > > We could fix this but there's no need to reschedule the main loop. This
> > > BSY polling presently happens when the NCR5380_main() work queue item
> > > calls NCR5380_select(), which in turn schedules NCR5380_main(), which
> > > calls NCR5380_select() again, and so on.
> > >
> > > This algorithm is a deviation from the simpler one in atari_NCR5380.c.
> > > The extra complexity and state is pointless. There's no reason to
> > > stop selection half-way and return to to the main loop when the main
> > > loop can do nothing useful until selection completes.
> > >
> > > So just poll for BSY. We can sleep while polling now that we have a
> > > suitable workqueue.
> >
> > Bisecting slow module initialization pointed to this commit.
>
> That's disappointing. This patch removed some nasty code. Anyway, thanks
> for taking the trouble to bisect.
>
> >
> > Before this commit (2 seconds):
> > [ 60.317374] scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x0,
> > n_io_port 0, base 0xd8000, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize
> > 128, this_id 7, flags { NCR53C400 }, USLEEP_POLL 3, USLEEP_SLEEP 50,
> > options { AUTOPROBE_IRQ PSEUDO_DMA }
> > [ 60.780715] scsi 2:0:1:0: Direct-Access QUANTUM LP240S GM240S01X
> > 4.6 PQ: 0 ANSI: 2 CCS
> > [ 62.606260] sd 2:0:1:0: Attached scsi generic sg1 type 0
> >
> >
> > After this commit (22 seconds):
> > [ 137.511711] scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x0,
> > n_io_port 0, base 0xd8000, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize
> > 128, this_id 7, flags { NCR53C400 }, USLEEP_POLL 3, USLEEP_SLEEP 50,
> > options { AUTOPROBE_IRQ PSEUDO_DMA }
> > [ 145.028532] clocksource: timekeeping watchdog: Marking clocksource 'tsc'
> > as unstable because the skew is too large:
> > [ 145.029767] clocksource: 'acpi_pm' wd_now: a49738
> > wd_last: f4da04 mask: ff
> > [ 145.029828] clocksource: 'tsc' cs_now: 2ea624698e
> > cs_last: 2c710aa17f mask:
> > [ 145.032733] clocksource: Switched to clocksource acpi_pm
>
> I figured that it was okay to sleep from an unbound CPU-intensive
> workqueue but doing so seems to cause problems. (See also patch 66/71
> "Fix soft lockups".)
>
> Perhaps a kthread is needed instead of a workqueue? (This workqueue
> already has it's own kthread, but top shows that it doesn't accrue CPU
> time.)
>
> > [ 145.236951] scsi 2:0:1:0: Direct-Access QUANTUM LP240S GM240S01X
> > 4.6 PQ: 0 ANSI: 2 CCS
> > [ 159.959308] sd 2:0:1:0: Attached scsi generic sg1 type 0
> >
> >
>
> This problem doesn't show up on my hardware, and I'd like to know where
> those 22 seconds are being spent. Would you please apply the entire series
> and add,
> #define NDEBUG (NDEBUG_ARBITRATION | NDEBUG_SELECTION | NDEBUG_MAIN)
> to the top of g_NCR5380.c and send me the messages logged during modprobe?
[ 397.014581] scsi host2: Generic NCR5380/NCR53C400 SCSI, io_port 0x0,
n_io_port 0, base 0xd8000, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize
128, this_id 7, flags { NO_DMA_FIXUP }, options { AUTOPROBE_IRQ PSEUDO_DMA }
[ 412.099695] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 412.103625] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 412.110503] scsi 2:0:1:0: Direct-Access QUANTUM LP240S GM240S01X 4.6
PQ: 0 ANSI: 2 CCS
[ 412.110892] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 412.114154] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 412.119733] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 427.198108] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 442.276586] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 457.354592] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 472.432999] STATUS_REG: 00
BASR: 08
ICR: 00
MODE: 00
[ 487.513027] sd 2:0:1:0: Attached scsi generic sg1 type 0
--
Ondrej Zary
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 00/71] More fixes, cleanup and modernization for NCR5380 drivers
On Wednesday 25 November 2015 10:04:10 Ondrej Zary wrote:
> I think that PDMA should work with 53C400A too but seems that the driver was
> never able to do it.
>
> Although there is code for port-mapped transfer in NCR5380_pread(),
> NCR53C400_register_offset is defined to 0 in the port-mapped case. The C400_
> register offsets are thus defined with negative offset:
>
> #define C400_CONTROL_STATUS_REG NCR53C400_register_offset-8
> #define C400_BLOCK_COUNTER_REG NCR53C400_register_offset-7
> #define C400_RESUME_TRANSFER_REG NCR53C400_register_offset-6
> #define C400_HOST_BUFFER NCR53C400_register_offset-4
>
> This is probably OK for a port-mapped 53C400 (such card must have some glue
> decoding logic as the 53C400 chip itself can do memory-mapping only) because:
>
> /*
> * On NCR53C400 boards, NCR5380 registers are mapped 8 past
> * the base address.
> */
> if (overrides[current_override].board == BOARD_NCR53C400)
> instance->io_port += 8;
>
> This means that on a 53C400, first 5380 register will be at base+8 and first
> C400_ register at base.
>
> But on a 53C400A, the 5380 registers are mapped on the base address so the
> C400_ registers would be below the base, which is obviously wrong. I hope that
> PDMA will work if I fix the C400_ registers mapping.
A quick hack (breaks other chips, needs more work for proper mapping on all
chips):
--- a/drivers/scsi/NCR5380.h
+++ b/drivers/scsi/NCR5380.h
@@ -163,7 +163,7 @@
/* Write any value to this register to start an ini mode DMA receive */
#define START_DMA_INITIATOR_RECEIVE_REG 7 /* wo */
-#define C400_CONTROL_STATUS_REG NCR53C400_register_offset-8/* rw */
+#define C400_CONTROL_STATUS_REG 9 /* rw */
#define CSR_RESET 0x80/* wo Resets 53c400 */
#define CSR_53C80_REG 0x80/* ro 5380 registers busy */
@@ -182,13 +182,13 @@
#endif
/* Number of 128-byte blocks to be transferred */
-#define C400_BLOCK_COUNTER_REG NCR53C400_register_offset-7 /* rw */
+#define C400_BLOCK_COUNTER_REG 10/* rw */
/* Resume transfer after disconnect */
-#define C400_RESUME_TRANSFER_REG NCR53C400_register_offset-6 /* wo */
+#define C400_RESUME_TRANSFER_REG 11/* wo */
/* Access to host buffer stack */
-#define C400_HOST_BUFFER NCR53C400_register_offset-4 /* rw */
+#define C400_HOST_BUFFER 8 /* rw */
/* Note : PHASE_* macros are based on the values of the STATUS register */
--- a/drivers/scsi/g_NCR5380.c
+++ b/drivers/scsi/g_NCR5380.c
@@ -323,7 +323,10 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
#endif
break;
case BOARD_NCR53C400A:
+ flags = FLAG_NO_DMA_FIXUP;
+#ifndef PSEUDO_DMA
flags = FLAG_NO_PSEUDO_DMA;
+#endif
ports = ncr_53c400a_ports;
break;
case BOARD_DTC3181E:
@@ -414,7 +417,8 @@ static int __init generic_NCR5380_detect(struct
scsi_host_template *tpnt)
if (NCR5380_init(instance, flags))
goto out_unregister;
- if (overrides[current_override].board == BOARD_NCR53C400)
+ if (overrides[current_override].board == BOARD_NCR53C400 ||
+ overrides[current_override].board == BOARD_NCR53C400A)
NCR5380_write(C400_CONTROL_STATUS_REG, CSR_BASE);
NCR5380_maybe_reset_bus(instance);
And PDMA works on I/O mapped 53C400A (HP C2502)!
# modprobe g_NCR5380 ncr_irq=7 ncr_addr=0x280 ncr_53c400a=1
[ 1799.939856] scsi host4: Generic NCR5380/NCR53C400 SCSI, io_port 0x280,
n_io_port 16, base 0x0, irq 0, can_queue 16, cmd_per_lun 2, sg_tablesize 128,
this_id 7, flags { NO_DMA_FIXUP }, options { AUTOPROBE_IRQ PSEUDO_DMA }
[ 1816.277018] scsi 4:0:1:0: Direct-Access QUANTUM LP240S GM240S01X 4.6
PQ: 0 ANSI: 2 CCS
[ 1897.899648] sd 4:0:1:0: Attached scsi generic sg1 type 0
[ 1897.917842] sd 4:0:1:0: [sdb] 479350 512-byte logical blocks: (245 MB/234
MiB)
[ 1897.920872] sd 4:0:1:0: [sdb] Write Protect is off
[ 1897.924744] sd 4:0:1:0: [sdb] Write cache: enabled, read cache: enabled,
doesn't support DPO or FUA
[ 1897.967857] sdb: sdb1
[ 1897.993822] sd 4:0:1:0: [sdb] Attached SCSI disk
Nice performance improvement (although it's slower than the memory-mapped
53C400):
# hdparm -t --direct /dev/sdb
/dev/sdb:
Timing O_DIRECT disk reads: 2 MB in 3.99 seconds = 513.57 kB/sec
And it even fixed the IRQ:
# head /proc/interrupts
CPU0
0: 151228XT-PIC timer
1: 9XT-PIC i8042
2: 0XT-PIC cascade
7:115XT-PIC NCR5380
8: 1XT-PIC rtc0
9: 0XT-PIC uhci_hcd:usb1, uhci_hcd:usb2
10: 3256XT-PIC eth0
12:136XT-PIC i8042
14: 3833XT-PIC pata_via
--
Ondrej Z
Re: [PATCH RESEND] scsi_debug: fix prevent_allow+verify regressions
> "Andy" == Andy Shevchenko writes: Andy, Andy> but can you pay a little attention to Andy> http://www.spinics.net/lists/linux-scsi/msg81778.html ? It seems Andy> it wasn't applied. Please resubmit to linux-scsi. We'll take a look. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: use sector_div instead of do_div
> "Arnd" == Arnd Bergmann writes: Arnd> do_div is the wrong way to divide a sector_t, as it is less Arnd> efficient when sector_t is 32-bit wide. With the upcoming do_div Arnd> optimizations, the kernel starts warning about this: Applied. -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: rescan VPD attributes
> "Hannes" == Hannes Reinecke writes: Hannes> The VPD page information might change, so we need to be able to Hannes> update it. This patch implements a VPD page rescan whenever the Hannes> 'rescan' sysfs attribute is triggered. I was looking into merging the ALUA series but this prerequisite patch is lacking reviews... -- Martin K. Petersen Oracle Linux Engineering -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Fix a scsi_host_dev_release() memory leak
> "Bart" == Bart Van Assche writes:
Bart> This patch series consists of two patches: - A first patch that
Bart> converts the code that accesses shost->shost_gendev to manipulate
Bart> the shost reference count into scsi_host_{get,put}() calls. - A
Bart> second patch that fixes the actual memory leak.
I gave a bunch of reviews for v1 but no takers for v2 of the second
patch.
--
Martin K. Petersen Oracle Linux Engineering
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
RE: [PATCH] scsi: rescan VPD attributes
Hi Hannes, I have one probably small nitpick about the patch. I'm not sure how likely what I've put below is likely to happen in real life though. Is there any chance at all that sdev->vpd_pg83_len could change when updated? If there's any chance of that I'd have expected that both the length of and the pointer to the vpd data would need to be protected not just the pointer so someone would have a consistent picture of the vpd and its length. Without that there is a race where someone could be using a new length with the old vpd data. That leaves the potential for a length that exceeds the vpd size if the new data is larger than the old data - I don't know how likely it is but wanted to at least bring it up as something to consider. I'm not so concerned about sdev->vpd_pg80_len changing since it should have 1 of 2 fixed sizes and it seems unlikely that once read the first time a device would increase it in size (but for consistency in the code if you make a change you might want to treat them the same way). Thanks Shane -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: rescan VPD attributes
On 11/26/2015 06:07 AM, Seymour, Shane M wrote: > Hi Hannes, > > I have one probably small nitpick about the patch. I'm not sure how likely > what I've put below is likely > to happen in real life though. > > Is there any chance at all that sdev->vpd_pg83_len could change when updated? > If there's any chance of that I'd have expected that both the length of and the pointer to the vpd data > would need to be protected not just the pointer so someone would have a consistent picture of the vpd > and its length. Without that there is a race where someone could be using a new length with the old vpd > data. That leaves the potential for a length that exceeds the vpd size if the new data is larger than > the old data - I don't know how likely it is but wanted to at least bring it up as something to consider. > Accesses to vpd_pgXX are rcu-protected, so we're ensured that we always see a _valid_ copy. And we know that integer updates are atomic, so we will always see a valid number in vpd_pgXX_len. Both, vpd_pgXX and vpd_pgXX_len, are updated at the same place, and under the same locks/mutexes. And as we're calling 'synchronize_rcu()' after updating vpd_pgXX, which needs to synchronize against all CPUs, I would think that this would take care of updating vpd_pgXX_len, too. But you are right, we can get rid of vpd_pgXX_len and calculate it from the data. However, I'd like to do this with another patch after the ALUA changes are in. Cheers, Hannes -- Dr. Hannes ReineckezSeries & Storage h...@suse.de +49 911 74053 688 SUSE LINUX GmbH, Maxfeldstr. 5, 90409 Nürnberg GF: F. Imendörffer, J. Smithard, J. Guild, D. Upmanyu, G. Norton HRB 21284 (AG Nürnberg) -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 2/5] arcmsr: fixes not release allocated resource
On Wed, 2015-11-25 at 14:55 +0100, Tomas Henzl wrote:
> On 25.11.2015 12:41, Ching Huang wrote:
> > From: Ching Huang
> >
> > Releasing allocated resource if get configuration data failed.
> >
> > Signed-of-by: Ching Huang
> >
> > ---
> >
> > diff -uprN a/drivers/scsi/arcmsr/arcmsr_hba.c
> > b/drivers/scsi/arcmsr/arcmsr_hba.c
> > --- a/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-24 11:35:26.0
> > +0800
> > +++ b/drivers/scsi/arcmsr/arcmsr_hba.c 2015-11-25 19:04:44.59097
> > +0800
> > @@ -2664,7 +2664,7 @@ static bool arcmsr_hbaB_get_config(struc
> > if (!arcmsr_hbaB_wait_msgint_ready(acb)) {
> > printk(KERN_NOTICE "arcmsr%d: wait 'get adapter firmware \
> > miscellaneous data' timeout \n", acb->host->host_no);
> > - return false;
> > + goto err_free_dma;
> > }
> > count = 8;
> > while (count){
> > @@ -2707,6 +2707,10 @@ static bool arcmsr_hbaB_get_config(struc
> > acb->firm_cfg_version = readl(®->message_rwbuffer[25]);
> > /*firm_cfg_version,25,100-103*/
> > /*firm_ide_channels,4,16-19*/
> > return true;
> > +err_free_dma:
> > + dma_free_coherent(&acb->pdev->dev, acb->roundup_ccbsize,
> > + acb->dma_coherent2, acb->dma_coherent_handle2);
> > + return false;
> > }
> >
>
> The resource should be released here, that is okay.
>
> How works the resource management when the eh_bus_reset_handler
> is called ? It looks to me that you allocate a new
> acb->dma_coherent2 without releasing it before.
> Btw. is it needed in that handler to re-read the firmware-spec?
>
> Tomas
>
You are right. In eh_bus_reset_handler, there may re-allocate a new dma
resource if call get_config again.
I will initiate a new patches for this bug after current 5 patches.
Thanks Tomas.
> > static bool arcmsr_hbaC_get_config(struct AdapterControlBlock *pACB)
> >
> >
> > --
> > To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
> > the body of a message to majord...@vger.kernel.org
> > More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 0/2] Fix a scsi_host_dev_release() memory leak
On Wed, Nov 25, 2015 at 10:40:44PM -0500, Martin K. Petersen wrote: > I gave a bunch of reviews for v1 but no takers for v2 of the second > patch. I think v1 is the right thing to do. Sorry for leading Bart in the wrong direction - the two different device structures in Scsi_Host confused me. -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] block: Always check queue limits for cloned requests
When a cloned request is retried on other queues it always needs
to be checked against the queue limits of that queue.
Otherwise the calculations for nr_phys_segments might be wrong,
leading to a crash in scsi_init_sgtable().
To clarify this the patch renames blk_rq_check_limits()
to blk_cloned_rq_check_limits() and removes the symbol
export, as the new function should only be used for
cloned requests and never exported.
Cc: Mike Snitzer
Cc: Ewan Milne
Cc: Jeff Moyer
Signed-off-by: Hannes Reinecke
---
block/blk-core.c | 21 +++--
include/linux/blkdev.h | 1 -
2 files changed, 7 insertions(+), 15 deletions(-)
diff --git a/block/blk-core.c b/block/blk-core.c
index 5131993b..a0af404 100644
--- a/block/blk-core.c
+++ b/block/blk-core.c
@@ -2114,7 +2114,8 @@ blk_qc_t submit_bio(int rw, struct bio *bio)
EXPORT_SYMBOL(submit_bio);
/**
- * blk_rq_check_limits - Helper function to check a request for the queue limit
+ * blk_cloned_rq_check_limits - Helper function to check a cloned request
+ * for new the queue limits
* @q: the queue
* @rq: the request being checked
*
@@ -2125,20 +2126,13 @@ EXPORT_SYMBOL(submit_bio);
*after it is inserted to @q, it should be checked against @q before
*the insertion using this generic function.
*
- *This function should also be useful for request stacking drivers
- *in some cases below, so export this function.
*Request stacking drivers like request-based dm may change the queue
- *limits while requests are in the queue (e.g. dm's table swapping).
- *Such request stacking drivers should check those requests against
- *the new queue limits again when they dispatch those requests,
- *although such checkings are also done against the old queue limits
- *when submitting requests.
+ *limits when retrying requests on other queues. Those requests need
+ *to be checked against the new queue limits again during dispatch.
*/
-int blk_rq_check_limits(struct request_queue *q, struct request *rq)
+static int blk_cloned_rq_check_limits(struct request_queue *q,
+ struct request *rq)
{
- if (!rq_mergeable(rq))
- return 0;
-
if (blk_rq_sectors(rq) > blk_queue_get_max_sectors(q, rq->cmd_flags)) {
printk(KERN_ERR "%s: over max size limit.\n", __func__);
return -EIO;
@@ -2158,7 +2152,6 @@ int blk_rq_check_limits(struct request_queue *q, struct
request *rq)
return 0;
}
-EXPORT_SYMBOL_GPL(blk_rq_check_limits);
/**
* blk_insert_cloned_request - Helper for stacking drivers to submit a request
@@ -2170,7 +2163,7 @@ int blk_insert_cloned_request(struct request_queue *q,
struct request *rq)
unsigned long flags;
int where = ELEVATOR_INSERT_BACK;
- if (blk_rq_check_limits(q, rq))
+ if (blk_cloned_rq_check_limits(q, rq))
return -EIO;
if (rq->rq_disk &&
diff --git a/include/linux/blkdev.h b/include/linux/blkdev.h
index 3fe27f8..d14961b 100644
--- a/include/linux/blkdev.h
+++ b/include/linux/blkdev.h
@@ -773,7 +773,6 @@ extern void blk_rq_set_block_pc(struct request *);
extern void blk_requeue_request(struct request_queue *, struct request *);
extern void blk_add_request_payload(struct request *rq, struct page *page,
unsigned int len);
-extern int blk_rq_check_limits(struct request_queue *q, struct request *rq);
extern int blk_lld_busy(struct request_queue *q);
extern int blk_rq_prep_clone(struct request *rq, struct request *rq_src,
struct bio_set *bs, gfp_t gfp_mask,
--
1.8.5.6
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH] scsi: rescan VPD attributes
On Mon, 2015-11-09 at 13:24 +0100, Hannes Reinecke wrote:
> The VPD page information might change, so we need to be able to
> update it. This patch implements a VPD page rescan whenever
> the 'rescan' sysfs attribute is triggered.
>
> Signed-off-by: Hannes Reinecke
> ---
> drivers/scsi/scsi.c| 20 +---
> drivers/scsi/scsi_scan.c | 4
> drivers/scsi/scsi_sysfs.c | 8 ++--
> drivers/scsi/ses.c | 12 +---
> include/scsi/scsi_device.h | 5 +++--
> 5 files changed, 39 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/scsi/scsi.c b/drivers/scsi/scsi.c
> index 207d6a7..2e4ef56 100644
> --- a/drivers/scsi/scsi.c
> +++ b/drivers/scsi/scsi.c
> @@ -803,7 +803,7 @@ void scsi_attach_vpd(struct scsi_device *sdev)
> int vpd_len = SCSI_VPD_PG_LEN;
> int pg80_supported = 0;
> int pg83_supported = 0;
> - unsigned char *vpd_buf;
> + unsigned char __rcu *vpd_buf, *orig_vpd_buf = NULL;
>
> if (sdev->skip_vpd_pages)
> return;
> @@ -849,8 +849,16 @@ retry_pg80:
> kfree(vpd_buf);
> goto retry_pg80;
> }
> + mutex_lock(&sdev->inquiry_mutex);
> + orig_vpd_buf = sdev->vpd_pg80;
> sdev->vpd_pg80_len = result;
> - sdev->vpd_pg80 = vpd_buf;
> + rcu_assign_pointer(sdev->vpd_pg80, vpd_buf);
> + mutex_unlock(&sdev->inquiry_mutex);
> + synchronize_rcu();
> + if (orig_vpd_buf) {
> + kfree(orig_vpd_buf);
> + orig_vpd_buf = NULL;
> + }
> vpd_len = SCSI_VPD_PG_LEN;
> }
>
> @@ -870,8 +878,14 @@ retry_pg83:
> kfree(vpd_buf);
> goto retry_pg83;
> }
> + mutex_lock(&sdev->inquiry_mutex);
> + orig_vpd_buf = sdev->vpd_pg83;
> sdev->vpd_pg83_len = result;
> - sdev->vpd_pg83 = vpd_buf;
> + rcu_assign_pointer(sdev->vpd_pg83, vpd_buf);
> + mutex_unlock(&sdev->inquiry_mutex);
> + synchronize_rcu();
> + if (orig_vpd_buf)
> + kfree(orig_vpd_buf);
> }
> }
>
> diff --git a/drivers/scsi/scsi_scan.c b/drivers/scsi/scsi_scan.c
> index 3b3dfef..7e0820f 100644
> --- a/drivers/scsi/scsi_scan.c
> +++ b/drivers/scsi/scsi_scan.c
> @@ -236,6 +236,7 @@ static struct scsi_device *scsi_alloc_sdev(struct
> scsi_target *starget,
> INIT_LIST_HEAD(&sdev->starved_entry);
> INIT_LIST_HEAD(&sdev->event_list);
> spin_lock_init(&sdev->list_lock);
> + mutex_init(&sdev->inquiry_mutex);
> INIT_WORK(&sdev->event_work, scsi_evt_thread);
> INIT_WORK(&sdev->requeue_work, scsi_requeue_run_queue);
>
> @@ -1517,6 +1518,9 @@ EXPORT_SYMBOL(scsi_add_device);
> void scsi_rescan_device(struct device *dev)
> {
> device_lock(dev);
> +
> + scsi_attach_vpd(to_scsi_device(dev));
> +
> if (dev->driver && try_module_get(dev->driver->owner)) {
> struct scsi_driver *drv = to_scsi_driver(dev->driver);
>
> diff --git a/drivers/scsi/scsi_sysfs.c b/drivers/scsi/scsi_sysfs.c
> index fdcf0ab..f021423 100644
> --- a/drivers/scsi/scsi_sysfs.c
> +++ b/drivers/scsi/scsi_sysfs.c
> @@ -758,11 +758,15 @@ show_vpd_##_page(struct file *filp, struct kobject
> *kobj,\
> {\
> struct device *dev = container_of(kobj, struct device, kobj);
> \
> struct scsi_device *sdev = to_scsi_device(dev);
> \
> + int ret;\
> if (!sdev->vpd_##_page)
> \
> return -EINVAL;
> \
> - return memory_read_from_buffer(buf, count, &off,\
> - sdev->vpd_##_page, \
> + rcu_read_lock();\
> + ret = memory_read_from_buffer(buf, count, &off,
> \
> + rcu_dereference(sdev->vpd_##_page), \
> sdev->vpd_##_page##_len);\
> + rcu_read_unlock(); \
> + return ret; \
> }\
> static struct bin_attribute dev_attr_vpd_##_page = { \
> .attr = {.name = __stringify(vpd_##_page), .mode = S_IRUGO },
> \
> diff --git a/drivers/scsi/ses.c b/drivers/scsi/ses.c
> index dcb0d76..e234da7 100644
> --- a/drivers/scsi/ses.c
> +++ b/drivers/scsi/ses.c
> @@ -554,17 +554,22 @@ static void ses_match_to_enclosure(struct
> enclosure_device *edev,
> struct scsi_device *sdev)
> {
> unsigned char *des
