Re: [PATCH 0/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
Well??? I responded yesterday but I still don't see it on the list so
at the expense of some getting a double response I'll try again ...
On Mon, Jul 22, 2013 at 6:27 PM, adam radford aradf...@gmail.com wrote:
On Tue, Jul 9, 2013 at 11:10 PM, James Bottomley
james.bottom...@hansenpartnership.com wrote:
On Tue, 2013-07-09 at 15:12 -0700, adam radford wrote:
On Tue, Jul 9, 2013 at 2:18 PM, James Bottomley
Adam, you do drive by coding on this for LSI ... ack or reject, please.
I have just now located my box of MegaRAID Parallel SCSI controllers.
I will review and test the patch series from Myron and respond by next
Monday.
Thanks,
James
Unfortunately my box of MegaRAID Parallel SCSI controllers only
contains only cards intended for megaraid_mbox.c (I tested all 20 of
them),
and does not contain any of the following really old Symbios based
megaraid cards:
static struct pci_device_id megaraid_pci_tbl[] = {
{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID2,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_AMI_MEGARAID3,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{0,}
};
which I had located previously before the company headquarters moved.
I cannot currently locate any of the above 3 controllers anywhere at
LSI headquarters after an exhaustive search, so I cannot test the
patches to megaraid.c from Myron @ RedHat.
Adam:
Thanks for looking/trying.
Myron, do you actually have the hardware and have you tested the
patches yourself ?
No, the closest to the above that I have access to is a NEC platform
with a PCI_VENDOR_ID_NCR vendor's PCI_DEVICE_ID_AMI_MEGARAID3 device.
I stumbled onto the issue because I want to get rid of the driver's use
of 'struct pci_dev'. It seems, from my looking, to be the only driver
that keeps local copies of such, and upon further investigation I
noticed that the intended use of the local copies is flawed to the point
that keeping them makes no sense (which further supported my desire to
get rid of them).
Myron
-Adam
--
To unsubscribe from this list: send the line unsubscribe linux-pci 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/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
On Tue, Jul 9, 2013 at 11:10 PM, James Bottomley
james.bottom...@hansenpartnership.com wrote:
On Tue, 2013-07-09 at 15:12 -0700, adam radford wrote:
On Tue, Jul 9, 2013 at 2:18 PM, James Bottomley
Adam, you do drive by coding on this for LSI ... ack or reject, please.
I have just now located my box of MegaRAID Parallel SCSI controllers.
I will review and test the patch series from Myron and respond by next
Monday.
Thanks,
James
Unfortunately my box of MegaRAID Parallel SCSI controllers only
contains only cards intended for megaraid_mbox.c (I tested all 20 of
them),
and does not contain any of the following really old Symbios based
megaraid cards:
static struct pci_device_id megaraid_pci_tbl[] = {
{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{PCI_VENDOR_ID_AMI, PCI_DEVICE_ID_AMI_MEGARAID2,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{PCI_VENDOR_ID_INTEL, PCI_DEVICE_ID_AMI_MEGARAID3,
PCI_ANY_ID, PCI_ANY_ID, 0, 0, 0},
{0,}
};
which I had located previously before the company headquarters moved.
I cannot currently locate any of the above 3 controllers anywhere at
LSI headquarters after an exhaustive search, so I cannot test the
patches to megaraid.c from Myron @ RedHat.
Myron, do you actually have the hardware and have you tested the
patches yourself ?
-Adam
--
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/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
On Tue, 2013-07-09 at 15:12 -0700, adam radford wrote: On Tue, Jul 9, 2013 at 2:18 PM, James Bottomley Adam, you do drive by coding on this for LSI ... ack or reject, please. I have just now located my box of MegaRAID Parallel SCSI controllers. I will review and test the patch series from Myron and respond by next Monday. Thanks, James -- 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 0/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
Is the megaraid driver still actively used and maintained? I originally posted this series on 06.07.2013 and after receiving no comments, pinged the list again on 06.17.2013 and still received no comments/feedback. Trying again as I believe there is a real issue here, which I'd like confirmation on, and we really should remove the local copy/usage of 'struct pci_dev' that this driver currently maintains. While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability drivers/scsi/megaraid.c | 152 --- drivers/scsi/megaraid.h |1 2 files changed, 27 insertions(+), 126 deletions(-) -- -- 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/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
On Tue, 2013-07-09 at 14:39 -0600, Myron Stowe wrote: Is the megaraid driver still actively used and maintained? I originally posted this series on 06.07.2013 and after receiving no comments, pinged the list again on 06.17.2013 and still received no comments/feedback. Trying again as I believe there is a real issue here, which I'd like confirmation on, and we really should remove the local copy/usage of 'struct pci_dev' that this driver currently maintains. While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability Adam, you do drive by coding on this for LSI ... ack or reject, please. James -- 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/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
On Tue, Jul 9, 2013 at 2:18 PM, James Bottomley james.bottom...@hansenpartnership.com wrote: On Tue, 2013-07-09 at 14:39 -0600, Myron Stowe wrote: Is the megaraid driver still actively used and maintained? I originally posted this series on 06.07.2013 and after receiving no comments, pinged the list again on 06.17.2013 and still received no comments/feedback. Trying again as I believe there is a real issue here, which I'd like confirmation on, and we really should remove the local copy/usage of 'struct pci_dev' that this driver currently maintains. While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability Adam, you do drive by coding on this for LSI ... ack or reject, please. James James, I have just now located my box of MegaRAID Parallel SCSI controllers. I will review and test the patch series from Myron and respond by next Monday. -Adam -- 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/3] [SCSI] megaraid: Remove local (struct pci_dev) pdev's
Bump. Any comments? I think there is a real issue here so I'd like some confirmation on that at least. On Fri, Jun 7, 2013 at 10:23 AM, Myron Stowe myron.st...@redhat.com wrote: While the megaraid device itself may be 64-bit DMA capable, 32-bit address restricted DMA buffers are apparently required for internal commands as is denoted by a couple of comments - For all internal commands, the buffer must be allocated in 4GB address range - within the driver. If the device is 64-bit DMA capable then, once it is setup, any subsequent DMA allocations for internal commands would not be properly restricted due to megaraid_probe_one() having called pci_set_dma_mask() on pdev with DMA_BIT_MASK(64). The driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures which are then set and used for allocating 32-bit address space restricted DMA buffers[1] but I don't believe that the implementation works as intended. Assume that the megaraid device is 64-bit DMA capable. While probing the device and attaching the megaraid driver, pci_set_dma_mask() is called with the originating pdev and a DMA_BIT_MASK of 64. As a result, any subsequent dynamic DMA related allocations associated with the originating pdev will acquire 64-bit based buffers, which do not meet the addressing restrictions for internal commands. megaraid_probe_one(struct pci_dev *pdev, ...) ... pci_set_dma_mask(pdev, DMA_BIT_MASK(64)); As mentioned, the driver attempts to solve this by using make_local_pdev() to dynamically create local pci_dev structures - local pdev's - which are set with a DMA_BIT_MASK of 32. make_local_pdev alloc_pci_dev memcpy pci_set_dma_mask dma_set_mask *dev-dma_mask = mask; The local pdev is then used in allocating a DMA buffer in an attempt to meet the 4 GB restriction. For a 64-bit DMA capable device, the originating pdev will have its 'dma_mask' set to 0x after the driver attaches. Subsequently, when an internal command is initiated, make_local_pdev() is called. make_local_pdev() uses the PCI's core to allocate a local pdev and then copies the originating pdev content into the newly allocated local pdev. As a result of copying the originating pdev content into the local pdev, pdev-dev.dma_mask will be pointing back to the originating pdev's 'dma_mask' member, not the local pdev's as intended. Thus, when make_local_pdev() calls pci_set_dma_mask() in an attempt to set the local pdev's DMA mask to 32 it will instead overwrite the originating pdev's DMA mask. Thus, after any user initiated commands are issued, all subsequent DMA allocations will be 32-bit restricted from that point onward regardless of whether they are internal commands or otherwise. This patch fixes the issue by removing the setup of DMA_BIT_MASK to 64 in megaraid_probe_one(), leaving the driver with default 32-bit DMA capabilities, as it currently ends up in such a state anyway after any internal commands are initiated. [1] It seems strange that both mega_buffer/buf_dma_handle and make_local_pdev() both exist for internal commands but this has been the case for a long time - at least since 2.6.12-rc2. Perhaps there is some coalescing that could be done. --- Myron Stowe (3): [SCSI] megaraid: Remove 64-bit DMA related dead code [SCSI] megaraid: Remove local (struct pci_dev) pdev's [SCSI] megaraid: Remove 64-bit DMA_BIT_MASK capability drivers/scsi/megaraid.c | 152 --- drivers/scsi/megaraid.h |1 2 files changed, 27 insertions(+), 126 deletions(-) -- -- To unsubscribe from this list: send the line unsubscribe linux-pci 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
