Re: [PATCH] uas: Add missing le16_to_cpu calls to asm1051 / asm1053 usb-id check
Hans de Goede writes:
> --- a/drivers/usb/storage/uas-detect.h
> +++ b/drivers/usb/storage/uas-detect.h
> @@ -73,8 +73,8 @@ static int uas_use_uas_driver(struct usb_interface *intf,
>* broken on the ASM1051, use the number of streams to differentiate.
>* New ASM1053-s also support 32 streams, but have a different prod-id.
>*/
> - if (udev->descriptor.idVendor == 0x174c &&
> - udev->descriptor.idProduct == 0x55aa) {
> + if (le16_to_cpu(udev->descriptor.idVendor) == 0x174c &&
> + le16_to_cpu(udev->descriptor.idProduct) == 0x55aa) {
> if (udev->speed < USB_SPEED_SUPER) {
> /* No streams info, assume ASM1051 */
> flags |= US_FL_IGNORE_UAS;
Not that it matters much in this case, but I believe converting the
constants is preferred so that this can be resolved at build time
instead of runtime.
I.e.
if (udev->descriptor.idVendor == cpu_to_le16(0x174c) &&
etc
Bjørn
--
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] megaraid_sas: fix memory leak if SGL has zero length entries
Bjørn Mork writes:
> commit 98cb7e44 ([SCSI] megaraid_sas: Sanity check user
> supplied length before passing it to dma_alloc_coherent())
> introduced a memory leak. Memory allocated for entries
> following zero length SGL entries will not be freed.
>
> Reference: http://bugs.debian.org/688198
> Cc:
> Signed-off-by: Bjørn Mork
> ---
> drivers/scsi/megaraid/megaraid_sas_base.c | 10 ++
> 1 file changed, 6 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
> b/drivers/scsi/megaraid/megaraid_sas_base.c
> index d2c5366..12b6be4 100644
> --- a/drivers/scsi/megaraid/megaraid_sas_base.c
> +++ b/drivers/scsi/megaraid/megaraid_sas_base.c
> @@ -4854,10 +4854,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance
> *instance,
> sense, sense_handle);
> }
>
> - for (i = 0; i < ioc->sge_count && kbuff_arr[i]; i++) {
> - dma_free_coherent(&instance->pdev->dev,
> - kern_sge32[i].length,
> - kbuff_arr[i], kern_sge32[i].phys_addr);
> + for (i = 0; i < ioc->sge_count; i++) {
> + if (kbuff_arr[i])
> + dma_free_coherent(&instance->pdev->dev,
> + kern_sge32[i].length,
> + kbuff_arr[i],
> + kern_sge32[i].phys_addr);
> }
>
> megasas_return_cmd(instance, cmd);
This patch was acked by Adam Radford 4 Dec 2012:
http://permalink.gmane.org/gmane.linux.kernel.stable/36537
but it looks like it got lost somewhere after that.
Please let me know asap if it should be resent. I'm otherwise going to
clean it out of my todo queue.
Bjørn
--
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 4/7][SCSI] mpt3sas: Infinite loops can occur if MPI2_IOCSTATUS_CONFIG_INVALID_PAGE is not returned
Sreekanth Reddy writes:
> Infinite loop can occur if IOCStatus is not equal to
> MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value in the while loops in functions
> _scsih_search_responding_sas_devices,
> _scsih_search_responding_raid_devices and
> _scsih_search_responding_expanders
>
> So, Instead of checking for MPI2_IOCSTATUS_CONFIG_INVALID_PAGE value,
> in this patch code is modified to check for IOCStatus not equals to
> MPI2_IOCSTATUS_SUCCESS to break the while loop.
>
> Signed-off-by: Sreekanth Reddy
> Cc: sta...@vger.kernel.org
> ---
> drivers/scsi/mpt3sas/mpt3sas_scsih.c | 16
> 1 files changed, 4 insertions(+), 12 deletions(-)
>
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index 9ebc9dc..632eba7 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -6406,7 +6406,7 @@ _scsih_search_responding_sas_devices(struct
> MPT3SAS_ADAPTER *ioc)
> handle))) {
> ioc_status = le16_to_cpu(mpi_reply.IOCStatus) &
> MPI2_IOCSTATUS_MASK;
> - if (ioc_status == MPI2_IOCSTATUS_CONFIG_INVALID_PAGE)
> + if (ioc_status != MPI2_IOCSTATUS_SUCCESS)
> break;
> handle = le16_to_cpu(sas_device_pg0.DevHandle);
> device_info = le32_to_cpu(sas_device_pg0.DeviceInfo);
Doesn't the same apply to the code mpt2sas driver you copied this code
from?
Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 1/1] Create megaraid ioctl device node
"Patrick Monnerat" writes: > From: Patrick Monnerat > > Create ioctl device node for megaraid_sas driver. Let this node be > managed by udev. Fix a typo. Or maybe just simplify it all and use a misc device instead? See how this is done in e.g. drivers/scsi/mpt3sas/mpt3sas_ctl.c or drivers/message/fusion/mptctl.c Bjørn -- 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] mpt2sas: prevent double free on error path
Jörn Engel writes:
> diff --git a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> index c6bdc92..43b3a98 100644
> --- a/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> +++ b/drivers/scsi/mpt2sas/mpt2sas_scsih.c
> @@ -570,6 +570,18 @@ _scsih_sas_device_find_by_handle(struct MPT2SAS_ADAPTER
> *ioc, u16 handle)
> return NULL;
> }
>
> +static void free_sas_device(struct kref *kref)
> +{
> + struct _sas_device *sas_device = container_of(kref, struct _sas_device,
> + kref);
> + kfree(sas_device);
> +}
> +
> +static void put_sas_device(struct _sas_device *sas_device)
> +{
> + kref_put(&sas_device->kref, free_sas_device);
> +}
> +
> /**
> * _scsih_sas_device_remove - remove sas_device from list.
> * @ioc: per adapter object
> @@ -583,14 +595,19 @@ _scsih_sas_device_remove(struct MPT2SAS_ADAPTER *ioc,
> struct _sas_device *sas_device)
> {
> unsigned long flags;
> + int was_on_list = 0;
>
> if (!sas_device)
> return;
>
> spin_lock_irqsave(&ioc->sas_device_lock, flags);
> - list_del(&sas_device->list);
> - kfree(sas_device);
> + if (!list_empty(&sas_device->list)) {
> + list_del_init(&sas_device->list);
> + was_on_list = 1;
> + }
> spin_unlock_irqrestore(&ioc->sas_device_lock, flags);
> + if (was_on_list)
> + put_sas_device(sas_device);
> }
>
How about the copy of this code in drivers/scsi/mpt3sas/mpt3sas_scsih.c?
Is that safe, or does it need fixing as well?
Bjørn
--
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 -next] [SCSI] mpt3sas: remove unused variables
writes:
> diff --git a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> index ac9dbc2..61a1a45 100644
> --- a/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> +++ b/drivers/scsi/mpt3sas/mpt3sas_scsih.c
> @@ -2754,13 +2754,11 @@ _scsih_block_io_to_children_attached_directly(struct
> MPT3SAS_ADAPTER *ioc,
> int i;
> u16 handle;
> u16 reason_code;
> - u8 phy_number;
>
> for (i = 0; i < event_data->NumEntries; i++) {
> handle = le16_to_cpu(event_data->PHY[i].AttachedDevHandle);
> if (!handle)
> continue;
> - phy_number = event_data->StartPhyNum + i;
> reason_code = event_data->PHY[i].PhyStatus &
> MPI2_EVENT_SAS_TOPO_RC_MASK;
> if (reason_code == MPI2_EVENT_SAS_TOPO_RC_DELAY_NOT_RESPONDING)
There is an identical copy of this code, including that bug, in
drivers/scsi/mpt2sas/mpt2sas_scsih.c.
If you insist on the stupid code duplication, then please try to keep
your bugs syncronized! If you are going to fix this bug for the mpt3sas
driver, then you *have* to fix it for mpt2sas as well.
Yes, I believe this is a mess and you have already demonstrated that you
are incapable of managing it.
Bjørn
--
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] megaraid_sas: fix memory leak if SGL has zero length entries
commit 98cb7e44 ([SCSI] megaraid_sas: Sanity check user
supplied length before passing it to dma_alloc_coherent())
introduced a memory leak. Memory allocated for entries
following zero length SGL entries will not be freed.
Reference: http://bugs.debian.org/688198
Cc:
Signed-off-by: Bjørn Mork
---
drivers/scsi/megaraid/megaraid_sas_base.c | 10 ++
1 file changed, 6 insertions(+), 4 deletions(-)
diff --git a/drivers/scsi/megaraid/megaraid_sas_base.c
b/drivers/scsi/megaraid/megaraid_sas_base.c
index d2c5366..12b6be4 100644
--- a/drivers/scsi/megaraid/megaraid_sas_base.c
+++ b/drivers/scsi/megaraid/megaraid_sas_base.c
@@ -4854,10 +4854,12 @@ megasas_mgmt_fw_ioctl(struct megasas_instance *instance,
sense, sense_handle);
}
- for (i = 0; i < ioc->sge_count && kbuff_arr[i]; i++) {
- dma_free_coherent(&instance->pdev->dev,
- kern_sge32[i].length,
- kbuff_arr[i], kern_sge32[i].phys_addr);
+ for (i = 0; i < ioc->sge_count; i++) {
+ if (kbuff_arr[i])
+ dma_free_coherent(&instance->pdev->dev,
+ kern_sge32[i].length,
+ kbuff_arr[i],
+ kern_sge32[i].phys_addr);
}
megasas_return_cmd(instance, cmd);
--
1.7.10.4
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH][SCSI] mpt3sas: Paer 1 of MPI API headers
writes: > This patch contains MPI API headers > This patch is part 1 of MPI API headers. > Signed-off-by: Sreekanth Reddy > Reviewed-by: Nagalakshmi Nandigama > --- > > > diff --git a/drivers/scsi/mpt3sas/mpi/mpi2.h b/drivers/scsi/mpt3sas/mpi/mpi2.h > new file mode 100644 > index 000..03317ff > --- /dev/null > +++ b/drivers/scsi/mpt3sas/mpi/mpi2.h > @@ -0,0 +1,1164 @@ Why can't this and the other headers be shared between the mpt2sas and mpt3sas drivers? Looks like you are duplicating a lot of code already present in drivers/scsi/mpt2sas. Why? That's a recipe for maintenance hell... And it makes this submission impossible to review. Bjørn -- 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
