Re: [ofa-general] Re: [PATCH 1/3] iscsi iser: remove DMA restrictions

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Mike Christie wrote:
Mike Christie wrote:
Erez Zilber wrote:
Erez Zilber wrote:
Pete Wyckoff wrote:
James.Bottomley@xxxxxxxxxxxxxxxxxxxxx wrote on Tue, 12 Feb 2008
15:57 -0600:
On Tue, 2008-02-12 at 16:46 -0500, Pete Wyckoff wrote:
James.Bottomley@xxxxxxxxxxxxxxxxxxxxx wrote on Tue, 12 Feb 2008
15:10 -0600:
On Tue, 2008-02-12 at 15:54 -0500, Pete Wyckoff wrote:
iscsi_iser does not have any hardware DMA restrictions.  Add a
slave_configure function to remove any DMA alignment restriction,
allowing the use of direct IO from arbitrary offsets within a page.
Also disable page bouncing; iser has no restrictions on which
pages it
can address.

Signed-off-by: Pete Wyckoff <pw@xxxxxxx>
---
 drivers/infiniband/ulp/iser/iscsi_iser.c |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index be1b9fb..1b272a6 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -543,6 +543,13 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
  iser_conn_terminate(ib_conn);
 }

+static int iscsi_iser_slave_configure(struct scsi_device *sdev)
+{
+ blk_queue_bounce_limit(sdev->request_queue, BLK_BOUNCE_ANY);
You really don't want to do this.  That signals to the block
layer that
we have an iommu, although it's practically the same thing as a
64 bit
DMA mask ... but I'd just leave it to the DMA mask to set this up
correctly. Anything else is asking for a subtle bug to turn up years
from now when something causes the mask and the limit to be
mismatched.
Oh.  I decided to add that line for symmetry with TCP, and was
convinced by the arguments here:

    commit b6d44fe9582b9d90a0b16f508ac08a90d899bf56
    Author: Mike Christie <michaelc@xxxxxxxxxxx>
    Date:   Thu Jul 26 12:46:47 2007 -0500

    [SCSI] iscsi_tcp: Turn off bounce buffers

    It was found by LSI that on setups with large amounts of memory
we were bouncing buffers when we did not need to. If the iscsi tcp
    code touches the data buffer (or a helper does),
    it will kmap the buffer. iscsi_tcp also does not interact with
hardware,
    so it does not have any hw dma restrictions. This patch sets
the bounce
    buffer settings for our device queue so buffers should not be
bounced
    because of a driver limit.

I don't see a convenient place to callback into particular iscsi
devices to set the DMA mask per-host.  It has to go on the
shost_gendev, right?, but only for TCP and iSER, not qla4xxx, which
handles its DMA mask during device probe.
You should be taking your mask from the underlying infiniband device as
part of the setup, shouldn't you?
I think you're right about this.  All the existing IB HW tries to
set a 64-bit dma mask, but that's no reason to disable the mechanism
entirely in iser.  I'll remove that line that disables bouncing in
my patch.  Perhaps Mike will know if the iscsi_tcp usage is still
appropriate.

Let me make sure that I understand: you say that the IB HW driver (e.g.
ib_mthca) tries to set a 64-bit dma mask:

    err = pci_set_dma_mask(pdev, DMA_64BIT_MASK);
    if (err) {
        dev_warn(&pdev->dev, "Warning: couldn't set 64-bit PCI DMA
mask.\n");
        err = pci_set_dma_mask(pdev, DMA_32BIT_MASK);
        if (err) {
            dev_err(&pdev->dev, "Can't set PCI DMA mask, aborting.\n");
            goto err_free_res;
        }
    }

So, in the example above, the driver will use a 64-bit mask or a 32-bit
mask (or fail). According to that, iSER (and SRP) needs to call
blk_queue_bounce_limit with the appropriate parameter, right?


Roland, James,

I'm trying to fix this potential problem in iSER, and I have some
questions about that. How can I get the DMA mask that the HCA driver is
using (DMA_64BIT_MASK or DMA_32BIT_MASK)? Can I get it somehow from
struct ib_device? Is it in ib_device->device?

I think what Erez is asking, or maybe it is something I was wondering is, that scsi drivers like lpfc or qla2xxx will do something like:

if (dma_set_mask(&scsi_host->pdev->dev, DMA_64BIT_MASK))
    dma_set_mask(&scsi_host->pdev->dev, DMA_32BIT_MASK)

And when __scsi_alloc_queue calls scsi_calculate_bounce_limit it checks the host's parent dma_mask and sets the bounce_limit for the driver.

Does srp/iser need to call the dma_set_mask functions or does the ib_device's device already have the dma info set up?

Nevermind. I misread the mail. We know the ib hw driver sets the mask. I guess what we are debating is if we should set the scsi_host's parent to the ib_device so the dma mask is picked up, or if should just set them in our slave_configure by calling blk_queue_bounce_limit. And if we use the blk_queue_bounce_limit path, what function do we call to get the dma_mask.


Oh man, I should have looked at the code before posting. For this last part, if we do not set a correct host parent I guess we have to just dupicate what scsi_calculate_bounce_limit does. It would be a waste to copy that code for iser. I guess we could modify scsi_calculate_bounce_limit somehow.
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Index of Archives]     [SCSI Target Devel]     [Linux SCSI Target Infrastructure]     [Kernel Newbies]     [IDE]     [Security]     [Git]     [Netfilter]     [Bugtraq]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux ATA RAID]     [Linux IIO]     [Samba]     [Device Mapper]
  Powered by Linux