Re: Publishing iSER limitations

2008-05-05 Thread Mike Christie

Pete Wyckoff wrote:
> [EMAIL PROTECTED] wrote on Sun, 04 May 2008 11:21 +0300:
>> Pete Wyckoff wrote:
>>> [EMAIL PROTECTED] wrote on Mon, 28 Apr 2008 11:20 -0500:
 Erez Zilber wrote:
> I understand that IB's requirements are too difficult to force. I guess
> that we will do something like:
>
> * If the scatterlist is aligned, just use it as is.
> * If some elements at the beginning/end of the scatterlist are
>   unaligned, we can copy only them (and use the rest of the
>   scatterlist as is). We saw that sometimes (e.g. GFS), only the 1st
>   element is unaligned, so copying the whole data is unnecessary.
> * Else, copy the whole scatterlist to buffers that iSER allocates.
>   This is what we do today with unaligned buffers.
>
>   
 Pete, we are discissing the dma_alignment patch you sent. Above are the 
 dma 
 restrictions the iser driver has right now. For the 3rd requirement does 
 that mean we want to set the alignment to be page aligned right?
 
>>> Erez's alignment rules leave out one case:  when the sg list
>>> has only one element, there are no restrictions on start/end
>>> alignment.
>>>   
>> Yeah, this is the trivial case (which is already covered in the iSER code).
>>
>>> The iser alignment requirements are too complex to express with the
>>> q->dma_alignment approach.  In iser, as Erez suggests, bouncing can
>>> be done.  His second (*) would be a nice addition that would
>>> definitely help some of our apps.
>>>   
>> We will try to send a patch for that soon.
>>
>>> My patch "iscsi iser: remove DMA alignment restriction" changes
>>> iser to make the alignment 0, rather than the default 512, to avoid
>>> the bounce buffering in __blk_rq_map_user().
>> Just to make sure that I understand - "alignment 0 " means that the
>> block layer doesn't care at all about alignment?
> 
> Yep.  The lines to think about are in __blk_rq_map_user:
> 
> alignment = queue_dma_alignment(q) | q->dma_pad_mask;
> if (!(uaddr & alignment) && !(len & alignment))
> bio = bio_map_user(q, NULL, uaddr, len, reading);
> else
> bio = bio_copy_user(q, uaddr, len, reading);
> 
> As it stands, queue_dma_alignment(q) returns 511.  That forces the
> use of the bio_copy_user path for anything that isn't aligned (start
> and end) on a 512 byte boundary.  Changing it to page alignment
> isn't what we want either.  Zero disables the copy path, and lets
> iser do everything.  You can add printks and test my patch using sg
> or bsg to submit from userspace, to see.
> 

Oh yeah, Pete is right. Since iser has that code already to handle its 
issues we can avoid all the mess by just setting it to zero.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-05-05 Thread Mike Christie

Pete Wyckoff wrote:
> [EMAIL PROTECTED] wrote on Mon, 28 Apr 2008 11:20 -0500:
>> Erez Zilber wrote:
>>> Mike Christie wrote:
 Erez Zilber wrote:
   
> This new thread summarizes and continues a discussion that we (Mike, Or
> and myself) had outside the list. This is what we have so far:
>
> * Having a parent device: commit
>   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
>   ok, and will solve the DMA mask problem. I think that it's cleaner
>   than calling slave_alloc etc. However, this code cannot be used
>   outside the bnx2i branch. I think that we need to create another
>   patch (based on this one) to submit upstream. Mike - what do you
>   think?
> 
 I am going to push that code soon. Since it is not critical for 2.6.26 I 
 am waiting to push it for .27.
   
>>> I agree - the DMA mask is not so urgent. However, as you said, Pete is
>>> waiting with his patch. Do you want to post something in linux-scsi?
>> Yeah, will do. I ccd him here too, so we could talk about the dma alignment 
>> patch.
> 
> Sorry for the delay.  This commit id has changed, and I'm not
> finding what you mean.  Perhaps this is the endpoint stuff,
> which is good.
> 

Sorry about that. I had to redo some patches. It is the endpoint stuff.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-05-04 Thread Pete Wyckoff

[EMAIL PROTECTED] wrote on Sun, 04 May 2008 11:21 +0300:
> Pete Wyckoff wrote:
> > [EMAIL PROTECTED] wrote on Mon, 28 Apr 2008 11:20 -0500:
> >> Erez Zilber wrote:
> >>> I understand that IB's requirements are too difficult to force. I guess
> >>> that we will do something like:
> >>>
> >>> * If the scatterlist is aligned, just use it as is.
> >>> * If some elements at the beginning/end of the scatterlist are
> >>>   unaligned, we can copy only them (and use the rest of the
> >>>   scatterlist as is). We saw that sometimes (e.g. GFS), only the 1st
> >>>   element is unaligned, so copying the whole data is unnecessary.
> >>> * Else, copy the whole scatterlist to buffers that iSER allocates.
> >>>   This is what we do today with unaligned buffers.
> >>>
> >>>   
> >> Pete, we are discissing the dma_alignment patch you sent. Above are the 
> >> dma 
> >> restrictions the iser driver has right now. For the 3rd requirement does 
> >> that mean we want to set the alignment to be page aligned right?
> >> 
> >
> > Erez's alignment rules leave out one case:  when the sg list
> > has only one element, there are no restrictions on start/end
> > alignment.
> >   
> 
> Yeah, this is the trivial case (which is already covered in the iSER code).
> 
> > The iser alignment requirements are too complex to express with the
> > q->dma_alignment approach.  In iser, as Erez suggests, bouncing can
> > be done.  His second (*) would be a nice addition that would
> > definitely help some of our apps.
> >   
> 
> We will try to send a patch for that soon.
> 
> > My patch "iscsi iser: remove DMA alignment restriction" changes
> > iser to make the alignment 0, rather than the default 512, to avoid
> > the bounce buffering in __blk_rq_map_user().
> 
> Just to make sure that I understand - "alignment 0 " means that the
> block layer doesn't care at all about alignment?

Yep.  The lines to think about are in __blk_rq_map_user:

alignment = queue_dma_alignment(q) | q->dma_pad_mask;
if (!(uaddr & alignment) && !(len & alignment))
bio = bio_map_user(q, NULL, uaddr, len, reading);
else
bio = bio_copy_user(q, uaddr, len, reading);

As it stands, queue_dma_alignment(q) returns 511.  That forces the
use of the bio_copy_user path for anything that isn't aligned (start
and end) on a 512 byte boundary.  Changing it to page alignment
isn't what we want either.  Zero disables the copy path, and lets
iser do everything.  You can add printks and test my patch using sg
or bsg to submit from userspace, to see.

-- Pete

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-05-04 Thread Erez Zilber

Pete Wyckoff wrote:

> [EMAIL PROTECTED] wrote on Mon, 28 Apr 2008 11:20 -0500:
>   
>> Erez Zilber wrote:
>> 
>>>
>>> I understand that IB's requirements are too difficult to force. I guess
>>> that we will do something like:
>>>
>>> * If the scatterlist is aligned, just use it as is.
>>> * If some elements at the beginning/end of the scatterlist are
>>>   unaligned, we can copy only them (and use the rest of the
>>>   scatterlist as is). We saw that sometimes (e.g. GFS), only the 1st
>>>   element is unaligned, so copying the whole data is unnecessary.
>>> * Else, copy the whole scatterlist to buffers that iSER allocates.
>>>   This is what we do today with unaligned buffers.
>>>
>>>   
>> Pete, we are discissing the dma_alignment patch you sent. Above are the dma 
>> restrictions the iser driver has right now. For the 3rd requirement does 
>> that mean we want to set the alignment to be page aligned right?
>> 
>
> Erez's alignment rules leave out one case:  when the sg list
> has only one element, there are no restrictions on start/end
> alignment.
>   

Yeah, this is the trivial case (which is already covered in the iSER code).

> The iser alignment requirements are too complex to express with the
> q->dma_alignment approach.  In iser, as Erez suggests, bouncing can
> be done.  His second (*) would be a nice addition that would
> definitely help some of our apps.
>   

We will try to send a patch for that soon.

> My patch "iscsi iser: remove DMA alignment restriction" changes
> iser to make the alignment 0, rather than the default 512, to avoid
> the bounce buffering in __blk_rq_map_user().

Just to make sure that I understand - "alignment 0 " means that the
block layer doesn't care at all about alignment?

>   Else confirming
> single-element or multi-element sg lists will be bounced
> unnecessarily.  As bouncing already exists in iser, anything that
> the block layer might do is purely duplicative, so it is best to
> turn it off.  This can be done now.
>
> Here it is for reference.  I updated the commentary; hopefully
> it makes sense.  Expect some fuzz as this is against an older
> 2.6.25-rc4.
>
>   -- Pete
>
> commit 47eaf146cde1ddd0a80aa45a1f50514f6a05b928
> Author: Pete Wyckoff <[EMAIL PROTECTED]>
> Date:   Fri May 2 10:31:58 2008 -0400
>
> iscsi iser: remove block-layer DMA alignment restriction
> 
> iser has rather complex rules for data alignment that come from
> its particular use of memory mapping hardware on IB.  iser uses
> its own bouncing layer to satisfy these requirements.
> 
> This patch removes the 512-byte DMA alignment requirement that
> is imposed by default at the block layer.  Without this change,
> IO from userspace (through sg or bsg, for example) would be
> bounced at the block layer even if that is not required by iser.
> 
> Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>
>
> diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
> b/drivers/infiniband/ulp/iser/iscsi_iser.c
> index be1b9fb..313f102 100644
> --- a/drivers/infiniband/ulp/iser/iscsi_iser.c
> +++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
> @@ -543,6 +543,12 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
>   iser_conn_terminate(ib_conn);
>  }
>  
> +static int iscsi_iser_slave_configure(struct scsi_device *sdev)
> +{
> + blk_queue_dma_alignment(sdev->request_queue, 0);
> + return 0;
> +}
> +
>  static struct scsi_host_template iscsi_iser_sht = {
>   .module = THIS_MODULE,
>   .name   = "iSCSI Initiator over iSER, v." DRV_VER,
> @@ -556,6 +562,7 @@ static struct scsi_host_template iscsi_iser_sht = {
>   .eh_device_reset_handler= iscsi_eh_device_reset,
>   .eh_host_reset_handler  = iscsi_eh_host_reset,
>   .use_clustering = DISABLE_CLUSTERING,
> + .slave_configure= iscsi_iser_slave_configure,
>   .proc_name  = "iscsi_iser",
>   .this_id= -1,
>  };
>
> >
>   


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-05-02 Thread Pete Wyckoff

[EMAIL PROTECTED] wrote on Mon, 28 Apr 2008 11:20 -0500:
> Erez Zilber wrote:
>> Mike Christie wrote:
>>> Erez Zilber wrote:
>>>   
 This new thread summarizes and continues a discussion that we (Mike, Or
 and myself) had outside the list. This is what we have so far:

 * Having a parent device: commit
   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
   ok, and will solve the DMA mask problem. I think that it's cleaner
   than calling slave_alloc etc. However, this code cannot be used
   outside the bnx2i branch. I think that we need to create another
   patch (based on this one) to submit upstream. Mike - what do you
   think?
 
>>> I am going to push that code soon. Since it is not critical for 2.6.26 I 
>>> am waiting to push it for .27.
>>>   
>>
>> I agree - the DMA mask is not so urgent. However, as you said, Pete is
>> waiting with his patch. Do you want to post something in linux-scsi?
>
> Yeah, will do. I ccd him here too, so we could talk about the dma alignment 
> patch.

Sorry for the delay.  This commit id has changed, and I'm not
finding what you mean.  Perhaps this is the endpoint stuff,
which is good.

 * iSER alignment issue: I'm not sure if we can force our
   restrictions through scsi_host_template. Again, the restrictions are:
   o The 1st element must end at the page boundary.
   o The last element must start at the page boundary.
   o All other elements must be page aligned (i.e. start at the
 beginning of a page and end at the page boundary).

Can it be done using blk_queue_dma_alignment? pad_mask?
 
>>
>> I understand that IB's requirements are too difficult to force. I guess
>> that we will do something like:
>>
>> * If the scatterlist is aligned, just use it as is.
>> * If some elements at the beginning/end of the scatterlist are
>>   unaligned, we can copy only them (and use the rest of the
>>   scatterlist as is). We saw that sometimes (e.g. GFS), only the 1st
>>   element is unaligned, so copying the whole data is unnecessary.
>> * Else, copy the whole scatterlist to buffers that iSER allocates.
>>   This is what we do today with unaligned buffers.
>>
>
> Pete, we are discissing the dma_alignment patch you sent. Above are the dma 
> restrictions the iser driver has right now. For the 3rd requirement does 
> that mean we want to set the alignment to be page aligned right?

Erez's alignment rules leave out one case:  when the sg list
has only one element, there are no restrictions on start/end
alignment.

The iser alignment requirements are too complex to express with the
q->dma_alignment approach.  In iser, as Erez suggests, bouncing can
be done.  His second (*) would be a nice addition that would
definitely help some of our apps.

My patch "iscsi iser: remove DMA alignment restriction" changes
iser to make the alignment 0, rather than the default 512, to avoid
the bounce buffering in __blk_rq_map_user().  Else confirming
single-element or multi-element sg lists will be bounced
unnecessarily.  As bouncing already exists in iser, anything that
the block layer might do is purely duplicative, so it is best to
turn it off.  This can be done now.

Here it is for reference.  I updated the commentary; hopefully
it makes sense.  Expect some fuzz as this is against an older
2.6.25-rc4.

-- Pete

commit 47eaf146cde1ddd0a80aa45a1f50514f6a05b928
Author: Pete Wyckoff <[EMAIL PROTECTED]>
Date:   Fri May 2 10:31:58 2008 -0400

iscsi iser: remove block-layer DMA alignment restriction

iser has rather complex rules for data alignment that come from
its particular use of memory mapping hardware on IB.  iser uses
its own bouncing layer to satisfy these requirements.

This patch removes the 512-byte DMA alignment requirement that
is imposed by default at the block layer.  Without this change,
IO from userspace (through sg or bsg, for example) would be
bounced at the block layer even if that is not required by iser.

Signed-off-by: Pete Wyckoff <[EMAIL PROTECTED]>

diff --git a/drivers/infiniband/ulp/iser/iscsi_iser.c 
b/drivers/infiniband/ulp/iser/iscsi_iser.c
index be1b9fb..313f102 100644
--- a/drivers/infiniband/ulp/iser/iscsi_iser.c
+++ b/drivers/infiniband/ulp/iser/iscsi_iser.c
@@ -543,6 +543,12 @@ iscsi_iser_ep_disconnect(__u64 ep_handle)
iser_conn_terminate(ib_conn);
 }
 
+static int iscsi_iser_slave_configure(struct scsi_device *sdev)
+{
+   blk_queue_dma_alignment(sdev->request_queue, 0);
+   return 0;
+}
+
 static struct scsi_host_template iscsi_iser_sht = {
.module = THIS_MODULE,
.name   = "iSCSI Initiator over iSER, v." DRV_VER,
@@ -556,6 +562,7 @@ static struct scsi_host_template iscsi_iser_sht = {
.eh_device_reset_handler= iscsi_eh_device

Re: Publishing iSER limitations

2008-04-28 Thread Mike Christie

Erez Zilber wrote:
> Mike Christie wrote:
>> Erez Zilber wrote:
>>   
>>> This new thread summarizes and continues a discussion that we (Mike, Or
>>> and myself) had outside the list. This is what we have so far:
>>>
>>> * Having a parent device: commit
>>>   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
>>>   ok, and will solve the DMA mask problem. I think that it's cleaner
>>>   than calling slave_alloc etc. However, this code cannot be used
>>>   outside the bnx2i branch. I think that we need to create another
>>>   patch (based on this one) to submit upstream. Mike - what do you
>>>   think?
>>> 
>> I am going to push that code soon. Since it is not critical for 2.6.26 I 
>> am waiting to push it for .27.
>>   
> 
> I agree - the DMA mask is not so urgent. However, as you said, Pete is
> waiting with his patch. Do you want to post something in linux-scsi?

Yeah, will do. I ccd him here too, so we could talk about the dma 
alignment patch.


> 
>>   
>>> * iSER alignment issue: I'm not sure if we can force our
>>>   restrictions through scsi_host_template. Again, the restrictions are:
>>>   o The 1st element must end at the page boundary.
>>>   o The last element must start at the page boundary.
>>>   o All other elements must be page aligned (i.e. start at the
>>> beginning of a page and end at the page boundary).
>>>
>>>Can it be done using blk_queue_dma_alignment? pad_mask?
>>> 
> 
> I understand that IB's requirements are too difficult to force. I guess
> that we will do something like:
> 
> * If the scatterlist is aligned, just use it as is.
> * If some elements at the beginning/end of the scatterlist are
>   unaligned, we can copy only them (and use the rest of the
>   scatterlist as is). We saw that sometimes (e.g. GFS), only the 1st
>   element is unaligned, so copying the whole data is unnecessary.
> * Else, copy the whole scatterlist to buffers that iSER allocates.
>   This is what we do today with unaligned buffers.
> 

Pete, we are discissing the dma_alignment patch you sent. Above are the 
dma restrictions the iser driver has right now. For the 3rd requirement 
does that mean we want to set the alignment to be page aligned right?

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-04-28 Thread Erez Zilber


>>> * Host per session or host per IB device: I agree with Or about the
>>>   need to have a host per session. I understand that the main
>>>   problem that commit 7e8e8af6511afafff33ef7eb0f519bf8702b78ed tries
>>>   to solve is what happens if we failover from one IB device to
>>>   another, right? We prefer to continue using a host per session,
>>>   
>> It is not related to this.
>>
>> It deals with being able to set some limits at a level higher than per 
>> session. If we had X sessions on one HCA port then we do not want to 
>> always to push X * session->can_queue IOs onto the port because the port 
>> may not be able to take that much IO. The commands will time out and the 
>> sessions will be stopped, and the scsi eh will run. The problem we face 
>> is that the ib_device is per HCA and not per port like we would see with 
>> normal scsi pci drivers or iscsi network drivers.
>>
>> It deals with aligning data structures to objects that can be removed 
>> and handling refcounting in a common way, and aligning this with how we 
>> normally do it in the scsi layer. With a normal scsi hba and driver, we 
>> have a pci device and allocate a scsi host for it (we actually get a pci 
>> resource for each port on the hba and allocate a scsi_host per port). We 
>> implement the pci driver's remove and probe callouts which get called at 
>> those times.
>>
>> Unlike software iscsi for infinniband we have something close to this. 
>> We have the ib_client which makes allocating a host per ib_device really 
>> nice when having to handle resource limits and handling removal of the 
>> host object and the objects accessing it and doing it in a standard way. 
>> For example if I remove a broadcom card I will remove the iscsi/scsi 
>> host for each port and that will cause each session running on that card 
>> to be removed. For iser if I do a host per ib_device and you did 
>> something like rmmod the HCA's module then each devices's remove callout 
>> gets called. We can then remove the iscsi/scsi host for the ib_device 
>> and the iscsi layer will remove the sessions referencing it. And it is 
>> all a common code path and there is no special casing for different 
>> drivers of this class, and the driver does not need to maintain its own 
>> struct to represent what other driver's represent with the scsi_host.
>>
>> 
>
> Oh yeah, I am not sure if you saw the end of that thread about this 
> item, but I had said I was ok with leaving it as is for now. Because we 
> allocate a ib_device per HCA, we run into problems with doing a 
> scsi_host per ib_device (when I did the patch I thought we were getting 
> a device per port). I am not going to push that patch because of this, 
> so we do not have to waste any time on this issue. At this time if you 
> are not complaining then I am not either :)
>   
Yeah, I saw that you said that we have several options. Let's not change
it for now.

Thanks,
Erez

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-04-28 Thread Erez Zilber

Mike Christie wrote:
> Erez Zilber wrote:
>   
>> This new thread summarizes and continues a discussion that we (Mike, Or
>> and myself) had outside the list. This is what we have so far:
>>
>> * Having a parent device: commit
>>   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
>>   ok, and will solve the DMA mask problem. I think that it's cleaner
>>   than calling slave_alloc etc. However, this code cannot be used
>>   outside the bnx2i branch. I think that we need to create another
>>   patch (based on this one) to submit upstream. Mike - what do you
>>   think?
>> 
>
> I am going to push that code soon. Since it is not critical for 2.6.26 I 
> am waiting to push it for .27.
>   

I agree - the DMA mask is not so urgent. However, as you said, Pete is
waiting with his patch. Do you want to post something in linux-scsi?

>   
>> * iSER alignment issue: I'm not sure if we can force our
>>   restrictions through scsi_host_template. Again, the restrictions are:
>>   o The 1st element must end at the page boundary.
>>   o The last element must start at the page boundary.
>>   o All other elements must be page aligned (i.e. start at the
>> beginning of a page and end at the page boundary).
>>
>>Can it be done using blk_queue_dma_alignment? pad_mask?
>> 

I understand that IB's requirements are too difficult to force. I guess
that we will do something like:

* If the scatterlist is aligned, just use it as is.
* If some elements at the beginning/end of the scatterlist are
  unaligned, we can copy only them (and use the rest of the
  scatterlist as is). We saw that sometimes (e.g. GFS), only the 1st
  element is unaligned, so copying the whole data is unnecessary.
* Else, copy the whole scatterlist to buffers that iSER allocates.
  This is what we do today with unaligned buffers.

Erez


--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-04-27 Thread Mike Christie

Mike Christie wrote:
> Erez Zilber wrote:
>> This new thread summarizes and continues a discussion that we (Mike, Or
>> and myself) had outside the list. This is what we have so far:
>>
>> * Having a parent device: commit
>>   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
>>   ok, and will solve the DMA mask problem. I think that it's cleaner
>>   than calling slave_alloc etc. However, this code cannot be used
>>   outside the bnx2i branch. I think that we need to create another
>>   patch (based on this one) to submit upstream. Mike - what do you
>>   think?
> 
> I am going to push that code soon. Since it is not critical for 2.6.26 I 
> am waiting to push it for .27.
> 
>> * iSER alignment issue: I'm not sure if we can force our
>>   restrictions through scsi_host_template. Again, the restrictions are:
>>   o The 1st element must end at the page boundary.
>>   o The last element must start at the page boundary.
>>   o All other elements must be page aligned (i.e. start at the
>> beginning of a page and end at the page boundary).
>>
>>Can it be done using blk_queue_dma_alignment? pad_mask?
>>
>> * Host per session or host per IB device: I agree with Or about the
>>   need to have a host per session. I understand that the main
>>   problem that commit 7e8e8af6511afafff33ef7eb0f519bf8702b78ed tries
>>   to solve is what happens if we failover from one IB device to
>>   another, right? We prefer to continue using a host per session,
> 
> It is not related to this.
> 
> It deals with being able to set some limits at a level higher than per 
> session. If we had X sessions on one HCA port then we do not want to 
> always to push X * session->can_queue IOs onto the port because the port 
> may not be able to take that much IO. The commands will time out and the 
> sessions will be stopped, and the scsi eh will run. The problem we face 
> is that the ib_device is per HCA and not per port like we would see with 
> normal scsi pci drivers or iscsi network drivers.
> 
> It deals with aligning data structures to objects that can be removed 
> and handling refcounting in a common way, and aligning this with how we 
> normally do it in the scsi layer. With a normal scsi hba and driver, we 
> have a pci device and allocate a scsi host for it (we actually get a pci 
> resource for each port on the hba and allocate a scsi_host per port). We 
> implement the pci driver's remove and probe callouts which get called at 
> those times.
> 
> Unlike software iscsi for infinniband we have something close to this. 
> We have the ib_client which makes allocating a host per ib_device really 
> nice when having to handle resource limits and handling removal of the 
> host object and the objects accessing it and doing it in a standard way. 
> For example if I remove a broadcom card I will remove the iscsi/scsi 
> host for each port and that will cause each session running on that card 
> to be removed. For iser if I do a host per ib_device and you did 
> something like rmmod the HCA's module then each devices's remove callout 
> gets called. We can then remove the iscsi/scsi host for the ib_device 
> and the iscsi layer will remove the sessions referencing it. And it is 
> all a common code path and there is no special casing for different 
> drivers of this class, and the driver does not need to maintain its own 
> struct to represent what other driver's represent with the scsi_host.
> 

Oh yeah, I am not sure if you saw the end of that thread about this 
item, but I had said I was ok with leaving it as is for now. Because we 
allocate a ib_device per HCA, we run into problems with doing a 
scsi_host per ib_device (when I did the patch I thought we were getting 
a device per port). I am not going to push that patch because of this, 
so we do not have to waste any time on this issue. At this time if you 
are not complaining then I am not either :)

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-04-27 Thread Mike Christie

Erez Zilber wrote:
> This new thread summarizes and continues a discussion that we (Mike, Or
> and myself) had outside the list. This is what we have so far:
> 
> * Having a parent device: commit
>   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
>   ok, and will solve the DMA mask problem. I think that it's cleaner
>   than calling slave_alloc etc. However, this code cannot be used
>   outside the bnx2i branch. I think that we need to create another
>   patch (based on this one) to submit upstream. Mike - what do you
>   think?

Oh yeah, you will want to reclone my tree. Some patches were broken or 
diffd incorrectly, so I rebuilt the tree. I made the endpoint code more 
generic for iser, bnx2i and one day chelsio. You want several patches in 
there related to the iscsi_endpoint stuff. Commit 
0d199fdee8740bcb1dbc3e33bbe5b03996f7be2b was the initial endpoint code 
and a2212ba43dc9db30e203d0a2d244b5b2569c72c8 was the iser conversion and 
fix to iser to set the host's parent based on the endpoint's parent. I 
also added some compat code so that older tools would continue to work 
with newer iser modules.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---



Re: Publishing iSER limitations

2008-04-27 Thread Mike Christie

Erez Zilber wrote:
> This new thread summarizes and continues a discussion that we (Mike, Or
> and myself) had outside the list. This is what we have so far:
> 
> * Having a parent device: commit
>   62786b526687db54c6dc22a1786d6df8b03da3f3 in the bnx2i branch looks
>   ok, and will solve the DMA mask problem. I think that it's cleaner
>   than calling slave_alloc etc. However, this code cannot be used
>   outside the bnx2i branch. I think that we need to create another
>   patch (based on this one) to submit upstream. Mike - what do you
>   think?

I am going to push that code soon. Since it is not critical for 2.6.26 I 
am waiting to push it for .27.

> * iSER alignment issue: I'm not sure if we can force our
>   restrictions through scsi_host_template. Again, the restrictions are:
>   o The 1st element must end at the page boundary.
>   o The last element must start at the page boundary.
>   o All other elements must be page aligned (i.e. start at the
> beginning of a page and end at the page boundary).
> 
>Can it be done using blk_queue_dma_alignment? pad_mask?
> 
> * Host per session or host per IB device: I agree with Or about the
>   need to have a host per session. I understand that the main
>   problem that commit 7e8e8af6511afafff33ef7eb0f519bf8702b78ed tries
>   to solve is what happens if we failover from one IB device to
>   another, right? We prefer to continue using a host per session,

It is not related to this.

It deals with being able to set some limits at a level higher than per 
session. If we had X sessions on one HCA port then we do not want to 
always to push X * session->can_queue IOs onto the port because the port 
may not be able to take that much IO. The commands will time out and the 
sessions will be stopped, and the scsi eh will run. The problem we face 
is that the ib_device is per HCA and not per port like we would see with 
normal scsi pci drivers or iscsi network drivers.

It deals with aligning data structures to objects that can be removed 
and handling refcounting in a common way, and aligning this with how we 
normally do it in the scsi layer. With a normal scsi hba and driver, we 
have a pci device and allocate a scsi host for it (we actually get a pci 
resource for each port on the hba and allocate a scsi_host per port). We 
implement the pci driver's remove and probe callouts which get called at 
those times.

Unlike software iscsi for infinniband we have something close to this. 
We have the ib_client which makes allocating a host per ib_device really 
nice when having to handle resource limits and handling removal of the 
host object and the objects accessing it and doing it in a standard way. 
For example if I remove a broadcom card I will remove the iscsi/scsi 
host for each port and that will cause each session running on that card 
to be removed. For iser if I do a host per ib_device and you did 
something like rmmod the HCA's module then each devices's remove callout 
gets called. We can then remove the iscsi/scsi host for the ib_device 
and the iscsi layer will remove the sessions referencing it. And it is 
all a common code path and there is no special casing for different 
drivers of this class, and the driver does not need to maintain its own 
struct to represent what other driver's represent with the scsi_host.

--~--~-~--~~~---~--~~
You received this message because you are subscribed to the Google Groups 
"open-iscsi" group.
To post to this group, send email to open-iscsi@googlegroups.com
To unsubscribe from this group, send email to [EMAIL PROTECTED]
For more options, visit this group at http://groups.google.com/group/open-iscsi
-~--~~~~--~~--~--~---