Re: [openib-general] [PATCH 2.6.21-rc1 4/5] ehca: replace yield() by wait_for_completion()
> @@ -332,7 +333,7 @@ int ehca_destroy_cq(struct ib_cq *cq)
> spin_lock_irqsave(&ehca_cq_idr_lock, flags);
> while (my_cq->nr_callbacks) {
> spin_unlock_irqrestore(&ehca_cq_idr_lock, flags);
> - yield();
> + wait_for_completion(&my_cq->zero_callbacks);
> spin_lock_irqsave(&ehca_cq_idr_lock, flags);
> }
A while loop around wait_for_completion doesn't make all that much sense.
I suspect a simple
if (my_cq->nr_callbacks)
wait_for_completion(&my_cq->zero_callbacks);
Is what you need.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 2.6.21-rc1 1/5] ehca: reworked irq handler to avoid/reduce missed irq events
On Wed, Feb 14, 2007 at 05:40:47PM +0100, Hoang-Nam Nguyen wrote: > Hi, > here is a patch for ehca with the reworked irq handler. > Thanks > Nam This looks okay to me (and sorry for new replying earlier to you private mail) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 2.6.21 1/4] ehca: fix improper use of yield with spinlock held
On Wed, Jan 24, 2007 at 12:10:36AM +0100, Hoang-Nam Nguyen wrote: > Here is a patch for ehca_cq.c that fixes improper use of yield > with spinlock held. Btw, please don't forget to replace the yield call with a proper condition for 2.6.21. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21] ehca: ehca_uverbs.c: refactor ehca_mmap() for better readability
On Thu, Jan 18, 2007 at 10:56:01AM -0800, Roland Dreier wrote: > I've kind of lost the plot here. How does this patch fit in with the > previous series of patches you posted? Does it replace them or go on > top of them? It's a cleanup ontop of the actual fix. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21 3/5] ehca: completion queue: remove use of do_mmap()
On Thu, Jan 11, 2007 at 01:40:54PM -0600, Nathan Lynch wrote: > Christoph Hellwig wrote: > > On Thu, Jan 11, 2007 at 08:08:36PM +0100, Hoang-Nam Nguyen wrote: > > > > > spin_lock_irqsave(&ehca_cq_idr_lock, flags); > > > while (my_cq->nr_callbacks) > > > yield(); > > > > Calling yield is a very bad idea in general. You should probably > > add a waitqueue that gets woken when nr_callbacks reaches zero to > > sleep effectively here. > > Isn't that code outright buggy? Calling into the scheduler with a > spinlock held and local interrupts disabled... Umm, yes - of course. I missed the spin_lock_irqsave line just above. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21 2/5] ehca: ehca_uverbs.c: "proper" use of mmap
On Thu, Jan 11, 2007 at 08:08:15PM +0100, Hoang-Nam Nguyen wrote:
> +static void mm_open(struct vm_area_struct *vma)
This should be name ehca_vma_open, dito for mm_close/ehca_vma_close and
vm_ops/ehca_vm_ops.
> + u32 *count = (u32*)vma->vm_private_data;
No need for the cast here (both in the open and close routine)
> + for (ofs = 0; ofs < queue->queue_length; ofs += PAGE_SIZE) {
> + u64 virt_addr = (u64)ipz_qeit_calc(queue, ofs);
> + page = virt_to_page(virt_addr);
> + rc = vm_insert_page(vma, start, page);
> + if (unlikely(rc)) {
> + ehca_gen_err("vm_insert_page() failed rc=%x", rc);
> + return rc;
> + }
> + start += PAGE_SIZE;
Not required for now, but long term you really should rework your
whole queue abstraction to operate on an array of struct pages, that
makes things like this and various other bits in ipz_pt_fn.[ch] a lot
simpler.
> int ehca_mmap(struct ib_ucontext *context, struct vm_area_struct *vma)
> {
Can you split this monster routine into individual functions for
each type of mmap please? With two helpers to get and verify the cq/qp
shared by the individual sub-variants, that would also help to get rid
of all those magic offsets.
Actually, this routine directly comes from ib_device.mmap - Roland,
can you shed some light on what's going on here?
Also after applying this patch I have a prototype and various callers
for ehca_mmap_nopage but no actual implementation. Could it be that
there are some bits missing?
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21 1/5] ehca: declaration of queue structures
On Thu, Jan 11, 2007 at 11:17:21AM -0800, Roland Dreier wrote: > > This indentation changes moves away from the preffered form. > > I will fix when I merge it -- no need to resend. > > > Except for that the patch looks fine. > > Christoph, did you look over all 5 or just this one so far? I've looked over all briefly, but I need a few more minutes to understand everything that's going on in patch 2. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21 4/5] ehca: queue pair: remove use of do_mmap()
On Thu, Jan 11, 2007 at 08:09:08PM +0100, Hoang-Nam Nguyen wrote: > Hello Roland and Christoph H.! > This is a patch for ehca_qp.c. It removes all direct calls of > do_mmap()/munmap() > when creating and destroying a queue pair respectively. Looks good. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21 3/5] ehca: completion queue: remove use of do_mmap()
On Thu, Jan 11, 2007 at 08:08:36PM +0100, Hoang-Nam Nguyen wrote:
> Hello Roland and Christoph H.!
> This is a patch for ehca_cq.c. It removes all direct calls of
> do_mmap()/munmap()
> when creating and destroying a completion queue respectively.
> Thanks
> Nam
This patch looks good, but there are some issues with the surrounding code:
> + if (my_cq->ownpid != cur_pid) {
> + ehca_err(device, "Invalid caller pid=%x ownpid=%x "
> + "cq_num=%x",
> + cur_pid, my_cq->ownpid, my_cq->cq_number);
> + return -EINVAL;
> + }
(for other reviewers: this is not new code, just moved around)
Owner tracking by pid is really dangerous. File descriptors can be
passed around by unix sockets, a single process can have files open
more than once, etc..
It seems ehca wants to prevent threads other than the creating one
from performing most operations. Can you explain the reason for this?
> spin_lock_irqsave(&ehca_cq_idr_lock, flags);
> while (my_cq->nr_callbacks)
> yield();
Calling yield is a very bad idea in general. You should probably
add a waitqueue that gets woken when nr_callbacks reaches zero to
sleep effectively here.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH/RFC 2.6.21 1/5] ehca: declaration of queue structures
On Thu, Jan 11, 2007 at 08:07:49PM +0100, Hoang-Nam Nguyen wrote: > -#define ehca_alloc_fw_ctrlblock(flags) ((void *) get_zeroed_page(flags)) > +#define ehca_alloc_fw_ctrlblock(flags) ((void*) get_zeroed_page(flags)) This indentation changes moves away from the preffered form. Except for that the patch looks fine. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/7] IB/core - Add DMA mapping functions to allow device drivers to interpose
On Sun, Nov 05, 2006 at 11:25:27PM +0200, Or Gerlitz wrote: > On 11/5/06, Roland Dreier <[EMAIL PROTECTED]> wrote: > > > I have mentioned this to Ralph in the past, just want to get ack/nak > > > on that from you: also on 64bit arch a block driver (eg SCSI LLD eg > > > SRP/iSER/etc) might get from higher level an SG whose pages are > > > **not** mapped into the kernel virtual address space. For example this > > > can happen with Direct I/O. > > > >No, I don't see how that could happen. Aren't all pages always mapped > >by the the kernel direct mapping on 64-bit architectures? > > I don't know exactly how this happens, but one of the comments i've > got from Christoph > on the iser code, is that one can't assume page_address(sg[i].page) > will not be NULL for SG passed to a SCSI LLD, i think Direct I/O is > one flow where this might happen. That statement is indeed true. Only for GFP_KERNEL allocations you can assume page_address is valid, and the scatterlist passed to a SCSI LLDD can contain any type of pages. Currently on all 64bit architectures page_address works on all pages, but that's an implementation detail that could change any time and that you should not rely on. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 0/6] Tranport Neutral Verbs Proposal.
On Mon, Jul 31, 2006 at 11:28:46AM -0700, Greg Lindahl wrote: > On Mon, Jul 31, 2006 at 11:18:16AM -0700, Roland Dreier wrote: > > > My gut reaction is negative. The whole idea of "verbs" is a bit of > > technical jargon that makes no sense unless you've lived in the RDMA > > world for a while, > > Given the way you are defining RDMA, I'm not surprised at the > conclusion you are coming to. We have been calling these the > transport neutral verbs, btw. > > How about ofabric_ ? No way. This subsystem is about doing rdma-type operation so call it something that includes rdma. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 0/6] Tranport Neutral Verbs Proposal.
On Mon, Jul 31, 2006 at 10:45:39AM -0700, Roland Dreier wrote: > > That's much better than rdma_, but do you really think the Linux folks > > are going to be happy about OpenFabrics calls with a prefix that > > doesn't look anything like "Open Fabrics"? > > I don't think Linux folks care about Open Fabrics at all. > > No other drivers have a brand name and it's pretty silly trying to > brand IB/iWARP/RDMA/whatever drivers. Exactly. Please don't even try to put brand names (especially if they're as stupid as this) in. We don't call our wireless stack centrino just because intel contributed to it either. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 21 of 53] ipath - use phys_to_virt instead of bus_to_virt
On Mon, May 15, 2006 at 02:21:21PM -0700, Bryan O'Sullivan wrote: > On Mon, 2006-05-15 at 08:50 -0700, Roland Dreier wrote: > > > Actually I NAK'ed this patch. It compiles the same thing on x86_64 > > but makes the source code wrong -- dma_map_single() returns a bus > > address, not a physical address. > > As Segher mentioned, bus_to_virt is unportable, so it's definitely the > wrong thing to use. phys_to_virt is as bad. please fix your code to do the right thing, that is to stop pretending to be able to map back from a bus to a virtual address. The only way to get at the virtual address from a bus one is to store it away at the time you call the dma mapping function. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 07/16] ehca: interrupt handling routines
> +#include > +#include > +#include Please don't use directly ever. Always include ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 5 of 13] ipath - use proper address translation routine
On Tue, May 02, 2006 at 07:24:18AM -0700, Roland Dreier wrote: > Christoph> Or stop doing the dma mapping in the IB upper level > Christoph> drivers. I told you that we'll get broken hardware > Christoph> that doesn't want dma mapping in the upper level > Christoph> driver, and pathscale created exactly that :) > > But see my earlier mail to Arjan about RDMA -- what address can a > protocol (eg SRP initiator) put in a message that the other side will > use to initiate a remote DMA operation? It seems to me it has to be a > bus address, and that means that the protocol has to do the DMA mapping. Then we're back to the discussion on why RDMA is a fundamentally flawed approach, but we already knew that. The usual workaround is to only allow RDMA operations to registered memory windows for which we can use the normal dma operation. There's also the *dac* pci dma operations that can avoid iommu overhead if you support 64bit addressing. But for all this to work dma mapping fundamentally needs to be handled by the low level driver. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 5 of 13] ipath - use proper address translation routine
On Mon, May 01, 2006 at 12:00:00PM -0700, Roland Dreier wrote: > Arjan> do you really NEED the vaddr? (most of the time linux > Arjan> drivers don't need it, while other OSes do) If you really > Arjan> need it you should grab it at dma_map time ... (and > Arjan> realize that it's not kernel addressable per se ;) > > Yes, they need some kind of vaddr. > > It's kind of a layering problem. The IB stack assumes that IB devices > have a DMA engine that deals with bus addresses. But the ipath driver > has to simulate this by using a memcpy on the CPU to move data to the > PCI device. > > I really don't know what the right solution is. Maybe having some way > to override the dma mapping operations so that the ipath driver can > keep the info it needs? Or stop doing the dma mapping in the IB upper level drivers. I told you that we'll get broken hardware that doesn't want dma mapping in the upper level driver, and pathscale created exactly that :) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 05/16] ehca: InfiniBand query and multicast functionality
On Thu, Apr 27, 2006 at 02:05:36PM +0200, Arnd Bergmann wrote: > On Thursday 27 April 2006 13:41, J?rn Engel wrote: > > On Thu, 27 April 2006 12:48:29 +0200, Heiko J Schick wrote: > > > > > > + * ?This source code is distributed under a dual license of GPL v2.0 and > > > OpenIB > > > > Line wrap. ?You might want to check your mailer or switch to a > > different one. > > > > Looks correct here. Maybe you need to check yours ;-) It's linewrapped here, too. And the mailer on this box hasn't changed for more than three years. OTOH I got strangely looking mails from you recently :) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 03/16] ehca: structure definitions
On Thu, Apr 27, 2006 at 01:57:49PM +0200, J?rn Engel wrote: > On Thu, 27 April 2006 12:48:13 +0200, Heiko J Schick wrote: > > + > > +#ifdef CONFIG_PPC64 > > +#include "ehca_classes_pSeries.h" > > +#endif > > Is the #ifdef necessary? Such conditions around header includes often > indicate problems with the included header. If that is the case here, > you should fix the header in question in a seperate patch. The real question is what is that ifdef for at all? The code subitted isn't built on anything but ppc64. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] creative file descriptor abuse in uverbs
I'd like to get rid of get_empty_filp midterm. Because of that's I've looked at all the users, and the only modular and most creative one is the uverbs code. Everything else only every returns new fds from syscalls, which is a good thing. Trying to show-horn fds into ->write otohg creates various problems. Any chance you could change this to creating new files in an uversfs or something when doing the next ABI revision for uverbs? (I suspect there'll be a major change for iWarp integration?) I plan to send a patch to mark get_empty_filp deprecated for modules to akpm soon, but it will be a few month at least before it can go, and not before we've found a solution for uverbs. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 3/6] [RFC] iser initiator
On Mon, Apr 03, 2006 at 02:59:03PM +0300, Or Gerlitz wrote: > Haven't heard from you re the patch you have supplied me which removes > at least this SCSI IOCTL issuing a non SG SCSI command. As i wrote you i > have patched 2.6.16 and tested it, works great. Is it queued for 2.6.17? It's in scsi-misc and will hopefully still go in before 2.6.17 ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] updated InfiniBand 2.6.17 merge plans
On Thu, Mar 30, 2006 at 03:11:17PM -0800, Roland Dreier wrote: > Oh yeah, one other thing I plan to merge unless someone objects: > > * Get rid of option for building IPoIB and mthca debug output unless > EMBEDDED=y NACK. Just add a FOO_DEBUG config option, this has no?hing to do with EMBEDDED. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 3/6] [RFC] iser initiator
On Thu, Mar 09, 2006 at 01:51:46PM +0200, Or Gerlitz wrote: > >>I have placed below a printout with some details re all the SCSI > >>commands issued during the ML discovery and setting of the drive, you > >>can see that an INQUIRY command reading 36 bytes is sent down with zero > >>use_sg (other zero use_sg commands don't read/write anything so there's > >>nothing to worry). > > >looks like you have a testcase for SCSI_IOCTL_SEND_COMMAND, nice. > >Could you test the patch below, which should make this remaning user > >of non-SG commands go away? > > thanks for the patch, I will be able to test later next week, i guess it > goes with 2.6.16-rc5 ? did you get a chance to test it? ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 3/6] [RFC] iser initiator
On Thu, Mar 09, 2006 at 01:51:46PM +0200, Or Gerlitz wrote: > >looks like you have a testcase for SCSI_IOCTL_SEND_COMMAND, nice. > >Could you test the patch below, which should make this remaning user > >of non-SG commands go away? > > thanks for the patch, I will be able to test later next week, i guess it > goes with 2.6.16-rc5 ? Should work. I generated it against Linus' git tree from a few days ago, but in that area nothing has changed lately IIRC. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 3/6] [RFC] iser initiator
On Mon, Mar 06, 2006 at 10:01:17AM +0200, Or Gerlitz wrote:
> > program hwscan is using a deprecated SCSI ioctl, pls convert to SG_IO
> > itt 17 op 01 flags c1 cdb[0-3] 12 01 80 00 use_sg 0 edtl 36
>
> I have placed below a printout with some details re all the SCSI
> commands issued during the ML discovery and setting of the drive, you
> can see that an INQUIRY command reading 36 bytes is sent down with zero
> use_sg (other zero use_sg commands don't read/write anything so there's
> nothing to worry).
looks like you have a testcase for SCSI_IOCTL_SEND_COMMAND, nice.
Could you test the patch below, which should make this remaning user
of non-SG commands go away?
Index: linux-2.6/block/scsi_ioctl.c
===
--- linux-2.6.orig/block/scsi_ioctl.c 2006-02-13 21:44:00.0 +0100
+++ linux-2.6/block/scsi_ioctl.c2006-03-08 15:13:47.0 +0100
@@ -350,16 +350,51 @@
return ret;
}
+/**
+ * sg_scsi_ioctl -- handle deprecated SCSI_IOCTL_SEND_COMMAND ioctl
+ * @file: file this ioctl operates on (optional)
+ * @q: request queue to send scsi commands down
+ * @disk: gendisk to operate on (option)
+ * @sic: userspace structure describing the command to perform
+ *
+ * Send down the scsi command described by @sic to the device below
+ * the request queue @q. If @file is non-NULL it's used to perform
+ * fine-grained permission checks that allow users to send down
+ * non-destructive SCSI commands. If the caller has a struct gendisk
+ * available it should be passed in as @disk to allow the low level
+ * driver to use the information contained in it. A non-NULL @disk
+ * is only allowed if the caller knows that the low level driver doesn't
+ * need it (e.g. in the scsi subsystem).
+ *
+ * Notes:
+ * - This interface is deprecated - users should use the SG_IO
+ * interface instead, as this is a more flexible approach to
+ * performing SCSI commands on a device.
+ * - The SCSI command length is determined by examining the 1st byte
+ * of the given command. There is no way to override this.
+ * - Data transfers are limited to PAGE_SIZE
+ * - The length (x + y) must be at least OMAX_SB_LEN bytes long to
+ * accommodate the sense buffer when an error occurs.
+ * The sense buffer is truncated to OMAX_SB_LEN (16) bytes so that
+ * old code will not be surprised.
+ * - If a Unix error occurs (e.g. ENOMEM) then the user will receive
+ * a negative return and the Unix error code in 'errno'.
+ * If the SCSI command succeeds then 0 is returned.
+ * Positive numbers returned are the compacted SCSI error codes (4
+ * bytes in one int) where the lowest byte is the SCSI status.
+ */
#define OMAX_SB_LEN 16 /* For backward compatibility */
-
-static int sg_scsi_ioctl(struct file *file, request_queue_t *q,
-struct gendisk *bd_disk, Scsi_Ioctl_Command __user
*sic)
+int sg_scsi_ioctl(struct file *file, struct request_queue *q,
+ struct gendisk *disk, struct scsi_ioctl_command __user *sic)
{
struct request *rq;
int err;
unsigned int in_len, out_len, bytes, opcode, cmdlen;
char *buffer = NULL, sense[SCSI_SENSE_BUFFERSIZE];
+ if (!sic)
+ return -EINVAL;
+
/*
* get in an out lengths, verify they don't exceed a page worth of data
*/
@@ -393,45 +428,53 @@
if (copy_from_user(rq->cmd, sic->data, cmdlen))
goto error;
- if (copy_from_user(buffer, sic->data + cmdlen, in_len))
+ if (in_len && copy_from_user(buffer, sic->data + cmdlen, in_len))
goto error;
err = verify_command(file, rq->cmd);
if (err)
goto error;
+ /* default. possible overriden later */
+ rq->retries = 5;
+
switch (opcode) {
- case SEND_DIAGNOSTIC:
- case FORMAT_UNIT:
- rq->timeout = FORMAT_UNIT_TIMEOUT;
- break;
- case START_STOP:
- rq->timeout = START_STOP_TIMEOUT;
- break;
- case MOVE_MEDIUM:
- rq->timeout = MOVE_MEDIUM_TIMEOUT;
- break;
- case READ_ELEMENT_STATUS:
- rq->timeout = READ_ELEMENT_STATUS_TIMEOUT;
- break;
- case READ_DEFECT_DATA:
- rq->timeout = READ_DEFECT_DATA_TIMEOUT;
- break;
- default:
- rq->timeout = BLK_DEFAULT_TIMEOUT;
- break;
+ case SEND_DIAGNOSTIC:
+ case FORMAT_UNIT:
+ rq->timeout = FORMAT_UNIT_TIMEOUT;
+ rq->retries = 1;
+ break;
+ case START_STOP:
+ rq->timeout = START_STOP_TIMEOUT;
+ break;
Re: [openib-general] [PATCH 5/6] [RFC] iser handling of memory for RDMA
On Tue, Feb 28, 2006 at 03:05:16PM +0200, Or Gerlitz wrote: > Christoph Hellwig wrote: > > > use kmap_atomic instead of page_address in the code copying from/to SG > > > which is unaligned for rdma > > > >this isn't entirely correct I think. iser_finalize_rdma_unaligned_sg > >is called from a tasklist, which is softirq context, so you can't use > >KM_USER0 there. KM_SOFTIRQ0 should probably work. > > This is exactly the case, iser_finalize_rdma_unaligned_sg runs in > tasklet context and its code is using KM_SOFTIRQ0 and > iser_start_rdma_unaligned_sg runs in kernel thread or user process > context and its code uses KM_USER0 I'm sorry, I misread the code for some reason. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 02/17] ehca: module infrastructure
> > + rblock = kmalloc(PAGE_SIZE, GFP_KERNEL);
> > + if (rblock == NULL) {
> > + EDEB_ERR(4, "Cannot allocate rblock memory.");
> > + ret = -ENOMEM;
> > + goto num_ports0;
> > + }
> > +
> > + memset(rblock, 0, PAGE_SIZE);
>
> Use kzalloc instead (this appears a quite a few places).
for a page-sized allocation __get_zeroed_page sounds better anyway.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 5/6] [RFC] iser handling of memory for RDMA
> use kmap_atomic instead of page_address in the code copying from/to SG > which is unaligned for rdma this isn't entirely correct I think. iser_finalize_rdma_unaligned_sg is called from a tasklist, which is softirq context, so you can't use KM_USER0 there. KM_SOFTIRQ0 should probably work. Otoh tasklets are not very scalable because tasklets of a type a serialized against running at multiple cpus, so maybe you should switch to a different mechanisms. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RFC: SDP plans
On Mon, Feb 27, 2006 at 07:34:59PM +0200, Michael S. Tsirkin wrote: > I started preparing a stable linux SDP implementation, > with the eye towards mainline inclusion. > > The idea is to get to a drastically simple code base and get this admitted in > mainline, then add enhancements. > > The plan (as compared to existing SDP implementation) includes: > - Use CMA API > - Reuse generic code from sock.c > SO_SNDBUF/SO_RCVBUF should work properly > - Use sock_lock_t and simple spin_lock_bh for socket locking > - Use skbuff and standard skbuff queues (in struct sock) > for incoming/outgoing messages > - Implement transport-level queues by simple circular buffer, > attach BSDH by s/g > - Set socket bits to signal the need for control messages > - Single CQ, perform all CQ polling from interrupt context > - Code must be sparse-clean, keep network data in __beXX structures > - Proper use of DMA API > - Use sysfs for statistics, entry per socket this one sounds a bit fishy. with lots of sockets you'll eat up far too much memory I susect. But let's look at the code once it's there. else this list sounds like where sdp needs to head for. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Suggested components to support in 1.0
On Fri, Feb 24, 2006 at 01:42:05PM -0800, Sean Hefty wrote: > Christoph Hellwig wrote: > >>IB protocols: > >> > >> * IPoIB > >> * RC > > > >I haven't seen any RC code for the gen2 stack yet and thus highly doubt > >it'll be ready in time. > > I think he just means reliable connections, which is there. The > reliability is handled by the hardware; and the IB CM or RDMA CM > establishes the connections. sorry, I was thinking of the silverstorm RDS stuff. But for RC then the same is true as with the other features below that aren't protocols. > >> * SRQ > >> * UC > >> * UD > > > >What upper layer protocols do you refer to with these acronyms? > > shared receive queues, unreliable connections, and unreliable datagrams But that's not IB protocols.. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Suggested components to support in 1.0
On Fri, Feb 24, 2006 at 09:16:14AM -0800, Bryan O'Sullivan wrote: > Here is a first cut at the set of components (protocols, drivers, > userspace bits) that I think we should be supporting in 1.0. Please > look over it and let me know if I'm missing anything. > > HCA support (both kernel driver and userspace verbs components): > > * ehca > * ipath > * mthca If fear the first two want be ready in time unless the progress rate increases a lot. > IB protocols: > > * IPoIB > * RC I haven't seen any RC code for the gen2 stack yet and thus highly doubt it'll be ready in time. > * SDP There's various political problems involved here. And besides that the code needs some serious work which is only happening really really slowly. > * SRQ > * UC > * UD What upper layer protocols do you refer to with these acronyms? > Components that I don't know what to do about, and will likely want to > drop unless someone can vouch for them: > > * iSER this has been making some nice progress and could make it into 2.6.17 > * SRP srp has been in mainline for quite a while and is a really simple protocol. no need not to support it. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 6/6] [RFC] iser socket
On Thu, Feb 23, 2006 at 03:58:27PM +0200, Alexander Nezhinsky wrote: > We don't upgrade iSCSI stream connection but start with > an RDMA connection right away. > The iSER code is going to be one of open-iscsi transports, > and open-iscsi opens connections using sockets from user space, > which is only natural with tcp. > The iSER RC connection should be open from kernel, so this special > socket gives us an opportunity to do so, while leaving intact the > entire mechanism of connection establishment and user-kernel handover. > We don't really need to implement read/write primitives because > they are initiated either from within kernel transport module > itself or through a special user-kernel interface bypassing > the socket. I think the iscsi userspace common code should be fixed to handle that case. If we really need a dummy handle it could at least be a pipe or something else that doesn't require writing a new stub. (Sorry for beeing so vague, it's been a while I looked at the iscsi code last) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 5/6] [RFC] iser handling of memory for RDMA
On Thu, Feb 23, 2006 at 11:52:05AM +0200, Or Gerlitz wrote:
> Can you educate me here a little... basically what i was thinking about
> dma mapping is that it maps from kernel virtual address to the bus
> address related to the device and SG sent down to a LLD from the
> midlayer can be supplied to dma_map_sg.
>
> Since that was my thought i assumed using page_address(sg->page) is fine
>
> So what you say here is that there are cases (eg highmem) where
> dma_map_sg does not assume such mapping currently exist? nor the LLD can
> assume this.
dma_map_page/dma_map_sg map from page frames to bus addresses. There
is no need for the pages to mapped into kernel virtual memory at all.
E.g. the simple non-iommu implementation of dma_map_page on i386 does
the following:
static inline dma_addr_t
dma_map_page(struct device *dev, struct page *page, unsigned long offset,
size_t size, enum dma_data_direction direction)
{
BUG_ON(direction == DMA_NONE);
return page_to_phys(page) + offset;
}
it doesn't involve kernel virtual addresses at all, just a struct page
and it's physical address. For more complex schemes the physical
address needs to be translated to a bus address, but there's not
requirement for the page to be mapped into kva. For example direct I/O
on filesystems or block devices will send down pages not mapped into KVA
to the scsi subsystem.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 3/6] [RFC] iser initiator
On Thu, Feb 23, 2006 at 12:27:05PM +0200, Or Gerlitz wrote:
> >>I'd say kill the non-SG case. We're in the progress of removing non-SG
> >>commands in the scsi midlayer, and I'm pretty sure they won't exist
> >>anymore before the iser code merged.
>
> >I wonder what would be the simplest patch to support it, does it make
> >sense to
> >use virt_to_page on sc->request_buffer to compose one entry SG on the
> fly
> >and use it down the code?
>
> Specifically, does something like makes sense?
>
> struct scatterlist my_sg; /* somewhere, but iser_send_command stack */
>
> if(!sc->use_sg) {
> my_sg.page = virt_to_page(sc->request_buffer);
> my_sg.length = sc->request_bufflen;
> my_sg.offset = 0;
> }
>
> now continue as ususal to process my_sg (it can't be on the stack
Yes, that makes sense for now. There's even a sg_init_one helper
to do the legwork for you. But before iser support goes into mainline
that'll be onbsolete already and can be removed again.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 6/6] [RFC] iser socket
On Wed, Feb 22, 2006 at 04:37:24PM +0200, Or Gerlitz wrote: > + note that data is never moved on the socket via send/recv but > only by calls from iscsi_iser.c to iser_send_control/command/dataout > > + data originting/resuling in user space (eg login request/respose) > is moved down/up by open iscsi using netlink So what do the iser sockets do? They look like noop stubs to me. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 5/6] [RFC] iser handling of memory for RDMA
> + if (cmd_dir == ISER_DIR_OUT) {
> + /* copy the unaligned sg the buffer which is used for RDMA */
> + struct scatterlist *p_sg = (struct scatterlist *)p_mem->p_buf;
> + int i;
> + char *p;
> +
> + for (p = mem, i = 0; i < p_mem->size; i++) {
> + memcpy(p,
> +page_address(p_sg[i].page) + p_sg[i].offset,
> +p_sg[i].length);
> + p += p_sg[i].length;
pages you get sent down in a sg list don't have to be kernel mapped,
you need to use kmap or kmap_atomic to access them.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 3/6] [RFC] iser initiator
> +/* Constant PDU lengths calculations */
> +#define ISER_HDR_LENsizeof (struct iser_hdr)
> +#define ISER_PDU_BHS_LENGTH sizeof (struct iscsi_hdr)
these two macros are just use in ISER_TOTAL_HEADERS_LEN below,
just kill them.
> +#define USE_OFFSET(offset) (offset)
> +#define USE_NO_OFFSET 0
> +#define USE_SIZE(size) (size)
> +#define USE_ENTIRE_SIZE 0
please kill these macros.
> +/* iser_dto_add_regd_buff - increments the reference count for *
> + * the registered buffer & adds it to the DTO object */
> +static void iser_dto_add_regd_buff(struct iser_dto *p_dto,
> +struct iser_regd_buf *p_regd_buf,
> +unsigned long use_offset,
> +unsigned long use_size)
> +{
> + int add_idx;
> +
> + atomic_inc(&p_regd_buf->ref_count);
Please kill the p_ prefix for pointer types all over the code.
> +static int iser_dma_map_task_data(struct iscsi_iser_cmd_task *p_iser_task,
> + struct iser_data_buf *p_data,
> + enum iser_data_dir iser_dir,
> + enum dma_data_direction dma_dir)
> +{
> + struct device *dma_device;
> + dma_addr_t dma_addr;
> + intdma_nents;
> +
> + p_iser_task->dir[iser_dir] = 1;
> + dma_device = p_iser_task->conn->ib_conn->p_adaptor->device->dma_device;
> +
> + if (p_data->type == ISER_BUF_TYPE_SINGLE) {
> + p_iser_task->data_len[iser_dir] = p_data->size;
> + dma_addr = dma_map_single(dma_device,p_data->p_buf,
> p_data->size,
> + dma_dir);
> + if (dma_mapping_error(dma_addr)) {
> + iser_err("dma_map_single failed at %p\n",
> p_data->p_buf);
> + return -EINVAL;
> + }
> + p_data->dma_addr = dma_addr;
> + } else {
I'd say kill the non-SG case. We're in the progress of removing non-SG
commands in the scsi midlayer, and I'm pretty sure they won't exist
anymore before the iser code merged.
> +static int iser_post_receive_control(struct iscsi_iser_conn *p_iser_conn)
> +{
> + struct iser_desc *rx_desc;
> + struct iser_regd_buf *p_regd_hdr;
> + struct iser_regd_buf *p_regd_data;
> + struct iser_dto *p_recv_dto = NULL;
> + struct iser_adaptor *p_iser_adaptor = p_iser_conn->ib_conn->p_adaptor;
> + int rx_data_size, err = 0;
> +
> + rx_desc = kmem_cache_alloc(ig.desc_cache,
> + GFP_KERNEL | __GFP_NOFAIL);
__GFP_NOFAIL doesn't work for slab (kmem_cache_alloc/kmalloc/kzalloc/kcalloc)
allocations
> +send_data_out_error:
> + if (p_send_dto != NULL)
> + iser_dto_buffs_release(p_send_dto);
> + if (tx_desc != NULL)
> + kmem_cache_free(ig.desc_cache, tx_desc);
could you please do the same goto-unwinding style we use elsewhere
in the kernel? That is one label before each unwind step and jump
directly to that instead of adding tons of conditionals in the error path.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 1/6] [RFC] iscsi_iser header file
On Wed, Feb 22, 2006 at 04:25:49PM +0200, Or Gerlitz wrote: > + some of the defines here replicate thos in drivers/scsi/iscsi_tcp.h so > merging them is possible > > + a cleanup we plan is to reduce the usage of the iser dbg/err/bug macros, > convert the remaining iser_bug calls into standard BUG() calls. > > + the wire structures are iser_hdr (below) and variuos iscsi_hdr > (from scsi/iscsi_proto.h) iser_adaptor is misspelled ;-) seriously, I think iser_device might be a better name for this. Please don't use volatile but an atomic_t or bitops for the session state field. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] NFS/RDMA client release for Linux 2.6.15
On Sun, Feb 19, 2006 at 09:56:18AM -0500, Talpey, Thomas wrote: > > - please don't build the rdma transport unconditional, but make it > >a user-visible config option > > It's an option, but it's located in fs/Kconfig not net/. This is the way > SUNRPC is selected, so we simply followed that. BTW, Chuck's transport > switch doesn't support dynamically loading modules yet so there is a > dependency to work out until that's in place. Right now it's an option, but not a user-selectable one: --- snip --- +config SUNRPC_XPRT_RDMA + depends on INFINIBAND + tristate --- snip --- to make it user-visible you need to add an option description after the tristate, e.g. tristate "RDMA transport for sunrpc" not strictly required but very useful is an additional help text using the help verb of the kconfig language. In the end form the select on config SUNRPC shouldn't be there either. > > - the CONFIG_HIGHMEM ifdef block in RPC_SEND_COPY is wrong. Please > >always use kmap, it does the right thing for non-highmem aswell. > >The PageHighMem check and using kmap_high directly is always > >wrong, they are internal implementation details. I'd also suggest > >evaluating kmap_atomic because it scales much better on SMP systems. > > Yes, there are some issues here which we're still working out. In fact, we > can't use kunmap() in the context you mention because in 2.6.14 (or is it > .15) it started to check for being invoked in interrupt context. There is > one configuration in which we do call it in bh context. The call won't block > but the kernel BUG_ON's. This is something on our list to address. That's one more reason to use kmap_atomic/kunmap_atomic which is fine from interrupt context. You'll have to carefully check whether you can use an existing KM_ slot or allocate a new one, though. > > - structures like rpcrdma_msg that are on the wire should use __be* > >for endianess annotations, and the cpu_to_be*/be*_to_cpu accessor > >functions instead of hton?/ntoh?. Please verify that these annotations > >are correct using sparse -D__CHECK_ENDIAN__=1 > > Hmm, okay but existing RPC and NFS code don't do this. I'm reluctant to > differ from the style of the governing subsystem. I'll check w/Trond. The nfs code is in the process of beeing converted currently. > > - in transport.c please don't use CamelCase variable names. > > This is just for module parameters? These are going away but we don't have > the new NFS mount API yet. There is a comment to that effect but maybe > it doesn't mention the module stuff. It's for all variables, but afaik you only use mixed case for the module paramaters. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] NFS/RDMA client release for Linux 2.6.15
On Wed, Feb 08, 2006 at 03:58:56PM -0500, Talpey, Thomas wrote: > We have released an updated NFS/RDMA client for Linux at > the project's Sourceforge site: Thanks, this looks much better than the previous patch. Comments: - please don't build the rdma transport unconditional, but make it a user-visible config option - please use the kernel u*/s* types instead of (u)int*_t - please include your local headers after the headers, and keep all the includes at the beginning of the files, just after the licence comment block - chunktype shouldn't be a typedef but a pure enum, and the names look a bit too generic, please add an rdma_ prefix - please kill the XDR_TARGET and pos0 macros, maybe RPC_SEND_SEG0 and RPC_SEND_LEN0, too - RPC_SEND_VECS should become an inline functions and be spelled lowercase - RPC_SEND_COPY is probably too large to be inlined and should be spelled lowercase - the CONFIG_HIGHMEM ifdef block in RPC_SEND_COPY is wrong. Please always use kmap, it does the right thing for non-highmem aswell. The PageHighMem check and using kmap_high directly is always wrong, they are internal implementation details. I'd also suggest evaluating kmap_atomic because it scales much better on SMP systems. - RPC_RECV_VECS should be an inline and spelled lowercase - RPC_RECV_SEG0 and PC_RECV_LEN0 should probably go away. - RPC_RECV_COPY is probably too large to be inlined and should be spelled lowercase - RPC_RECV_COPY same comment about highmem and kmap as in RPC_SEND_COPY - please try to avoid file-scope forward-prototypes but try to order the code in the natural flow where they aren't required - structures like rpcrdma_msg that are on the wire should use __be* for endianess annotations, and the cpu_to_be*/be*_to_cpu accessor functions instead of hton?/ntoh?. Please verify that these annotations are correct using sparse -D__CHECK_ENDIAN__=1 - rdma_convert_physiov/rdma_convert_phys are completely broken. page_to_phys can't be used by driver/fs code. RDMA only deals with bus addresses, not physical addresses. You must use the dma mapping API instead. Also coalescing decisions are made by the dma layer, because they are platform dependent and much more complex then what the code in this patch does. - transport.c is missing a GPL license statement - in transport.c please don't use CamelCase variable names. - MODULE_PARM shouldn't be used in new code, but module_param instead. - please don't use the (void) function() style, it just obsfucates the code without benefit. - try_module_get(THIS_MODULE) is always wrong. Reference counting should happen from the calling module. - please initialize global or file-scope spinlocks with DEFINE_SPINLOCK(). - the traditional name for the second argument to spin_lock_irqsave is just flags, not lock_flags. This doesn't really matter, but following such conventions makes it easier to understand the code for kernel hackers that just occasionally drop into your code. - no need to case the return value from kmalloc/kzalloc/etc. They return void * which can be directly assigned to any pointer type. - please avoid typedes for structure types, like struct rdma_ia, struct rdma_ep, etc.. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 02/22] Firmware interface code for IB device.
On Sat, Feb 18, 2006 at 10:15:09AM -0800, Greg KH wrote: > On Sat, Feb 18, 2006 at 08:32:28AM -0800, Roland Dreier wrote: > > Arjan> The bigger issue is: if people can't be bothered to do > > Arjan> those steps, why would they be bothered to do this for > > Arjan> maintenance and bugfixes etc etc? Basically it's now > > Arjan> already a de-facto unmaintained driver > > > > I don't think that's really a fair statement. The IBM people have > > been active and responsive in maintaining their driving in the > > openib.org svn tree. However, they asked me to post their driver for > > review because it would be difficult for them to do it. > > Checking stuff into a private svn tree is vastly different from posting > to lkml in public. In fact, it looks like the svn tree is so far ahead > of the in-kernel stuff, that most people are just using it instead of > the in-kernel code. > > I know at least one company has asked a distro to just accept the svn > snapshot over the in-kernel IB code, which makes me wonder if the > in-kernel stuff is even useful to people? Why have it, if companies > insist on using the out-of-tree stuff instead? The openib tree isn't private. It's mostly just a staging area for development. Any company that wants it included into a distro release is completely clueless. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH 08/22] Generic ehca headers
On Fri, Feb 17, 2006 at 04:57:23PM -0800, Roland Dreier wrote: > From: Roland Dreier <[EMAIL PROTECTED]> > > The defines of TRUE and FALSE look rather useless. Why are they needed? > > What is struct ehca_cache for? It doesn't seem to be used anywhere. > > ehca_kv_to_g() looks completely horrible. The whole idea of using > vmalloc()ed kernel memory to do DMA seems unacceptable to me. When you want to do scatter-gather dma on kernel-virtual contingous areas allocate the pages individually and map them into kva using vmap(). Then dma can be performed using dma_map_page, or in case you have lots of pages dma_map_sg after creating an S/G list. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 02/22] Firmware interface code for IB device.
On Sat, Feb 18, 2006 at 02:26:31PM +0200, Muli Ben-Yehuda wrote: > I don't speak for IBM or the authors, but there are perfectly > reasonable reasons to ask someone else to post a patch on your behalf > - including but not limited to to only being able to use Lotus Notes > with one's IBM email. I'm sure you've all seen the travesties that > Notes inflicts on inline patches. sure. and there's free webmail accounts that take about 10 minutes to setup as well as various people offering shell access to linux machines if you ask nicely. so this really is not an issue. I think this is more about ibm politics (espeically in boeblingen) sometimes making it pretty hard to post things. But that doesn't mean it's impossible, it just means they didn't try hard enough. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 02/22] Firmware interface code for IB device.
On Fri, Feb 17, 2006 at 06:04:56PM -0800, Roland Dreier wrote: > Greg> Roland, your comments are fine, but what about the original > Greg> author's descriptions of what each patch are? > > This is actually me breaking up a giant driver into pieces small > enough to post to lkml without hitting the 100 KB limit. > > This is just an RFC -- I assume the driver is going to get merged in > the end as one big git changeset with a changelog like "add driver for > IBM eHCA InfiniBand adapters". > > Greg> Come on, IBM allows developers to post code to lkml, just > Greg> look at the archives for proof. For them to use a proxy > Greg> like this is very strange, and also, there is no > Greg> Signed-off-by: record from the original authors, which is > Greg> not ok. > > Well, the eHCA guys tell me that they can't post patches to lkml. Then they lie. And not posting to lkml is a good reason not to merge an otherwise perfect driver. (which this one is far from) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 03/22] pHype specific stuff
> +u64 hipz_galpa_load(struct h_galpa galpa, u32 offset)
> +{
> + u64 addr = galpa.fw_handle + offset;
> + u64 out;
> + EDEB_EN(7, "addr=%lx offset=%x ", addr, offset);
> + out = *(u64 *) addr;
why does this cast an u64 to a pointer?
> +#ifndef EHCA_USERDRIVER
> +inline static int hcall_map_page(u64 physaddr, u64 * mapaddr)
> +{
> + *mapaddr = (u64)(ioremap(physaddr, 4096));
> +
> + EDEB(7, "ioremap physaddr=%lx mapaddr=%lx", physaddr, *mapaddr);
> + return 0;
ioremap returns void __iomem * and casting that to any integer type is
wrong.
> +inline static int hcall_unmap_page(u64 mapaddr)
> +{
> + EDEB(7, "mapaddr=%lx", mapaddr);
> + iounmap((void *)(mapaddr));
> + return 0;
dito for iounmap and casting back.
guys, please run this driver through sparse, thanks.
> + /* if phype returns LongBusyXXX,
> + * we retry several times, but not forever */
> + for (i = 0; i < 5; i++) {
> + __asm__ __volatile__("mr 3,%10\n"
> + "mr 4,%11\n"
> + "mr 5,%12\n"
assembly code under drivers/ is not acceptable. please create
and for it or something similar.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 02/22] Firmware interface code for IB device.
On Fri, Feb 17, 2006 at 04:57:07PM -0800, Roland Dreier wrote: > From: Roland Dreier <[EMAIL PROTECTED]> > > This is a very large file with way too much code for a .h file. > The functions look too big to be inlined also. Is there any way > for this code to move to a .c file? > --- > > drivers/infiniband/hw/ehca/hcp_if.h | 2022 > +++ > +#include "ehca_tools.h" > +#include "hipz_structs.h" > +#include "ehca_classes.h" > + > +#ifndef EHCA_USE_HCALL > +#include "hcz_queue.h" > +#include "hcz_mrmw.h" > +#include "hcz_emmio.h" > +#include "sim_prom.h" > +#endif > +#include "hipz_fns.h" > +#include "hcp_sense.h" > +#include "ehca_irq.h" > + > +#ifndef CONFIG_PPC64 > +#ifndef Z_SERIES > +#warning "included with wrong target, this is a p file" > +#endif > +#endif > + > +#ifdef EHCA_USE_HCALL > + > +#ifndef EHCA_USERDRIVER > +#include "hcp_phyp.h" > +#else > +#include "testbench/hcallbridge.h" > +#endif > +#endif the ifdefs should all go away and the build system should make sure it's only built for the right platforms. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 01/22] Add powerpc-specific clear_cacheline(), which just compiles to "dcbz".
On Fri, Feb 17, 2006 at 04:57:04PM -0800, Roland Dreier wrote: > From: Roland Dreier <[EMAIL PROTECTED]> > > This is horribly non-portable. Yes. If this is needed it should go to an asm/ header, not in a driver. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 0 of 20] [RFC] ipath - PathScale InfiniPath driver
On Mon, Jan 02, 2006 at 01:05:43PM -0300, Horst von Brand wrote: > > > Problem with that is that if everybody and Aunt Tillie does the same, > > > the kernel as a whole gets to be a mess. > > > ALSA does the exact same thing for the exact same reason. Maybe an > > indication that the kernel's i2c layer is too heavy? > > That would mean that the respective teams should put their heads together > and (re)design it to their needs... Exactly. We got quite a few developers to help adjusting the i2c stack for their needs and improve it. The i2c stack started out beeing used only for hardware monitoring chips and then later multimedia devices. Help to make it more useful for other users is always appreciated. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] PathScale license
On Tue, Dec 27, 2005 at 06:02:55PM -0800, Johann George wrote: > We have heard the issues that have been raised regarding the PathScale > license. PathScale's intent is solely to protect its hardware IP and not to > limit use of the software in any way. > > PathScale's use of this language is not original. SGI has used, and perhaps > originated, the additional language. It currently appears in several files > in the Linux kernel. As an example, see fs/xfs/linux-2.6/kmem.c XFS has been switched to a normal short GPL boilerplate exactly because this wording is not okay. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] PathScale license
On Sat, Dec 24, 2005 at 06:34:38PM -0700, Ronald G. Minnich wrote: > > Hi, > > > > The PathScale OpenIB license includes the following which > > is beyond the normal OpenIB license: > > > > * Patent licenses, if any, provided herein do not apply to > > * combinations of this program with other software, or any other > > * product whatsoever. > > ??? What the heck could this mean? This kind of comment, lacking any real > explanation, can cause trouble for a vendor -- it causes worries for > customers. Customer worries can lead to customers looking to other > vendors. I had hoped we had gotten past this 'one wrong move and I kill > myself' aspect of the IB vendor community. Yepp. Second problem is that this is not GPL-compatible so we definitly couldn't out the code into the kernel tree with this license. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 03/13] [RFC] ipath copy routines
On Fri, Dec 16, 2005 at 03:48:54PM -0800, Roland Dreier wrote: > Copy routines for ipath driver NACK, assembler copy routines don't belong into drivers. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 00/13] [RFC] IB: PathScale InfiniPath driver
On Fri, Dec 16, 2005 at 03:48:54PM -0800, Roland Dreier wrote: > having sysctls that set values also settable through module parameters > under /sys/module, code inside #ifndef __KERNEL__ so include files can > be shared with other PathScale code, code in ipath_i2c.c that might be > simplified by using drivers/i2c, etc. I'd like to try to get a sense > of whether I'm being too picky or whether PathScale really does need > to fix these up before the driver is merged. Yes, please fix this stuff before. The current driver looks like a horrible mess. Is there some political plot going where pathscale folks are forcing you to send this out in this scheme? Otherwise I couldn't explain the code quality magnitudes lower than normally expected from your merges. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH 01/13] [RFC] ipath basic headers
> + * $Id: ipath_common.h 4491 2005-12-15 22:20:31Z rjwalsh $
please remove RCSIDs everywhere.
> +#ifdef __KERNEL__
> +#include
> +#include
> +#include
> +#else/* !__KERNEL__; user mode */
> +#include
> +#include
> +#include
> +#include
> +
> +/* these aren't implemented for user mode, which is OK until we multi-thread
> */
> +typedef struct _atomic {
> + uint32_t counter;
> +} atomic_t; /* no atomic_t type in user-land */
> +#define atomic_set(a,v) ((a)->counter = (v))
> +#define atomic_inc_return(a) (++(a)->counter)
> +#define likely(x) (x)
> +#define unlikely(x) (x)
> +
> +#define yield() sched_yield()
Please push this out. It's fine if they reuse kernel-code in userspace
this way, but please move the compat wrappers to a separate file that's
not in the kernel tree.
> +typedef uint8_t ipath_type;
totally meaningless typedef
> +#ifndef _BITS_PER_BYTE
> +#define _BITS_PER_BYTE 8
> +#endif
WTF?
> +
> +static __inline__ void ipath_shortcopy(void *dest, void *src, uint32_t cnt)
> +__attribute__ ((always_inline));
> +/*
> + * this is used for very short copies, usually 1 - 8 bytes,
> + * *NEVER* to the PIO buffers!!! use ipath_dwordcpy for longer
> + * copies, or any copy to the PIO buffers. Works for 32 and 64 bit
> + * gcc and pathcc
> + */
> +static __inline__ void ipath_shortcopy(void *dest, void *src, uint32_t cnt)
in kernel land we __inline__ includes always_inline. Also no need for
a separate prototype for a just following inline function.
> +{
> + void *ssv, *dsv;
> + uint32_t csv;
> + __asm__ __volatile__("cld\n\trep\n\tmovsb":"=&c"(csv), "=&D"(dsv),
> + "=&S"(ssv)
> + :"0"(cnt), "1"(dest), "2"(src)
> + :"memory");
> +}
No way we're gonna put assembler code into such a driver.
> +struct ipath_int_vec {
> + int long long addr;
> + uint32_t info;
> +};
please always used fixes-size types for user communication. also please
avoid ioctls like the rest of the IB codebase.
> +/* Similarly, this is the kernel version going back to the user. It's
> slightly
> + * different, in that we want to tell if the driver was built as part of a
> + * PathScale release, or from the driver from the OpenIB, kernel.org, or a
> + * standard distribution, for support reasons. The high bit is 0 for
> + * non-PathScale, and 1 for PathScale-built/supplied. That bit is defined
> + * in Makefiles, rather than this file.
> + *
> + * It's returned by the driver to the user code during initialization
> + * in the spi_sw_version field of ipath_base_info, so the user code can
> + * in turn check for compatibility with the kernel.
> +*/
> +#define IPATH_KERN_SWVERSION ((IPATH_KERN_TYPE<<31) | IPATH_USER_SWVERSION)
NACK, there's no way we're gonna put in a way to identify an "official"
version. The official version is the last one in mainline always.
> +#ifndef PCI_VENDOR_ID_PATHSCALE /* not in pci.ids yet */
> +#define PCI_VENDOR_ID_PATHSCALE 0x1fc1
> +#define PCI_DEVICE_ID_PATHSCALE_INFINIPATH1 0xa
> +#define PCI_DEVICE_ID_PATHSCALE_INFINIPATH2 0xd
> +#endif
so move it there?
> +typedef struct _ipath_portdata {
please avoid typedefs for struct types.
> +/*
> + * these should be somewhat dynamic someday, although they are fixed
> + * for all users of the device on any given load.
> + *
> + * NOTE: There is a VM bug in the 2.4 Kernels similar to the one Dave
> + * fixed in the 2.6 Kernel. When using large or discontinuous memory,
> + * we get random kernel oops. So, in 2.4, we are just going to stick
> + * with 4k chunks instead of 64k chunks.
> + */
No one cares about 2.4 kernels here.
> + * these function similarly to the mlock/munlock system calls.
> + * ipath_mlock() is used to pin an address range (if not already pinned),
> + * and optionally return the list of physical addresses
> + * ipath_munlock() does the obvious, and ipath_mlock() cleans up all
> + * private memory, used at driver unload.
> + * ipath_mlock_nocopy() is similar to mlock, but only one page, and marks
> + * the vm so the page isn't taken away on a fork.
> + */
> +int ipath_mlock(unsigned long, size_t, struct page **);
> +int ipath_mlock_nocopy(unsigned long, struct page **);
this kind of thing definitly doesn't belong into an LLDD. or maybe
it's just stale prototypes?
> +#ifdef IPATH_COSIM
> +extern __u32 sim_readl(const volatile void __iomem * addr);
> +extern __u64 sim_readq(const volatile void __iomem * addr);
> +extern void sim_writel(__u32 val, volatile void __iomem * addr);
> +extern void sim_writeq(__u64 val, volatile void __iomem * addr);
> +#define ipath_readl(addr) sim_readl(addr)
> +#define ipath_readq(addr) sim_readq(addr)
> +#define ipath_writel(val, addr) sim_writel(val, addr)
> +#define ipath_writeq(val, addr) sim_writeq(val, addr)
> +#else
> +#define ipath_readl(addr) readl(addr)
> +#define ipath_readq(addr) readq(addr)
> +#define ipath_
Re: [openib-general] assumptions on page mapping (was High memory)
On Thu, Dec 08, 2005 at 09:17:15AM +0200, Or Gerlitz wrote: > Roland Dreier wrote: > >The right way to use the MR from get_dma_mr() is to use "bus > >addresses" from the DMA mapping API. For highmem, the right way to > >get those addresses is with dma_map_sg() or dma_map_page(). > > Looking on the kernel x86_64 code, both dma_map_sg and dma_map_page seem > to assume that the page is already mapped, since they call > page_address(page). x86_64 doesn't have highmem, so page_address(page) is valid on every page. > Specifically is it safe in a SCSI LLD (eg SRP and iSER which is among > other things such) to call dma_map_sg on a SG which comes with a SCSI yes, this is definitly safe. > command, so the SCSI Mid-Layer always makes sure the pages are mapped? no, it doesn't. in fact pages don't need to be mapped at all for dma normally. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] compilation platform dependencies
On Thu, Nov 03, 2005 at 08:52:52AM -0800, Grant Grundler wrote: > > I'm not sure if __u64 driver_data[0]; forces alignment to an 8-byte > > boundary on i386... does it? > > I'm now convinced it doesn't on x86. > See output below. Yes, alignment rules for x86 are different for every other architecture in that respect. It causes a lot of problems with ioctl translations for x86 binaries on ia64/x86_64. For the private data I'd suggest you copy the network driver layer approach, see alloc_netdev and netdev_priv for details. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: [PATCH/RFC] IB: Add SCSI RDMA Protocol (SRP) initiator
On Mon, Oct 31, 2005 at 09:03:35PM -0800, Roland Dreier wrote: > Christoph> No. Bitfields for accessing hardware/wire > Christoph> datastructures are wrong and will always break in some > Christoph> circumstances. Your header is much better. > > OK, that's my feeling as well. > > Would it make sense for me to split the pure SRP spec structures and > so on into a separate file and put it in include/scsi/srp.h? Then we > can move ibmvscsi towards using that file. Sounds like a good idea, yes. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: [PATCH/RFC] IB: Add SCSI RDMA Protocol (SRP) initiator
On Mon, Oct 31, 2005 at 08:55:23PM -0800, Roland Dreier wrote: > Anyway, looking at drivers/scsi/ibmvscsi/srp.h, the main problem I see > is that the file has a bunch of bitfields that are big-endian only > (which makes sense because the driver can only be compiled for pSeries > or iSeries anyway). > > But I have no objection to moving the file to include/scsi/srp.h, > adding a bunch of > > #if defined(__LITTLE_ENDIAN_BITFIELD) > #elif defined(__BIG_ENDIAN_BITFIELD) > #endif > > and adding a few missing defines, and then converting ib_srp to use > the same file. > > Does that seem like the right thing to do? No. Bitfields for accessing hardware/wire datastructures are wrong and will always break in some circumstances. Your header is much better. > > Thanks, > Roland > ___ > openib-general mailing list > openib-general@openib.org > http://openib.org/mailman/listinfo/openib-general > > To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general ---end quoted text--- ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] FW: Open letter to OpenIB membership re-SC05 andresponse to last board meeting
On Mon, Oct 17, 2005 at 05:25:03PM +0100, Paul Baxter wrote: > I suggest you back up your opinions with action and avoid future postings > to the OpenIB mailing lists. > > I, for one, am sick of your constant 'holier than thou' attitude. > > OpenIB may have a few faults but it has accomplished a lot in this niche > market. As I said I have lots of respect for people like Roland to actually get all the work done. The political bullshit on the other hand is a constant annoyance. There's a reason the other linux subsystems don't have a trade organization with a political agenda behind them. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] FW: Open letter to OpenIB membership re-SC05 and response to last board meeting
On Mon, Oct 17, 2005 at 05:44:26PM +0200, Christoph Hellwig wrote: > On Mon, Oct 17, 2005 at 07:24:14AM -0700, Ryan, Jim wrote: > > I am cross-posting this email to the general mailing list to bring > > attention to the activities at SC'05 related to InfiniBand. This is > > important work you may not be aware of > > > > Thanks, Jim Ryan > > Who cares? This douns like really horrible bitching. OpenIB Gen2 works > nicely today and everything else fortunately becomes irrelevant. > Whether a slide is slightly wrong or not is something you should talk > to the author about. That beeing said this looks like the typical polciy bullshit once a board and business people are involved. Sounds like it's finally time to kill the unwholy openib organization and let the people who get the really nice work done get their work done without all the bullshit involved. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] FW: Open letter to OpenIB membership re-SC05 and response to last board meeting
On Mon, Oct 17, 2005 at 07:24:14AM -0700, Ryan, Jim wrote: > I am cross-posting this email to the general mailing list to bring > attention to the activities at SC'05 related to InfiniBand. This is > important work you may not be aware of > > Thanks, Jim Ryan Who cares? This douns like really horrible bitching. OpenIB Gen2 works nicely today and everything else fortunately becomes irrelevant. Whether a slide is slightly wrong or not is something you should talk to the author about. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [RFC] IB address translation using ARP
On Fri, Oct 14, 2005 at 03:39:53PM -0700, Grant Grundler wrote: > Open source does NOT ignore legacy applications: > 1) Anyone can continue to update and run on the linux kernel version >they have source code for if they don't want to (or can't) change >the application or newer kernels break the ABI. >Many people are still very happy using 2.4 linux kernels. Actually if your aplication plays by the rules and breaks with a new kernel that's a major bug. We definitly guarantee that applications that use the defined syscall interface work on new kernels indefinitly. That doesn't mean they will get all the new features, though. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [RFC] IB address translation using ARP
On Fri, Oct 14, 2005 at 08:38:18AM -0700, Caitlin Bestler wrote: > I can't think of a better example of something that is truly > brain dead than an application *written* to use Sockets Direct > Protocol. I think you confuse "specificly written to support" with "specificly written to support only". And yes, in the days of getaddrinfo writing an application specific to a protocol instead of IP+Stream or Dgram semantics is pretty bad idea. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] DMA mapping abuses in MAD layer
On Thu, Oct 13, 2005 at 04:17:45PM -0700, Sean Hefty wrote: > Sean Hefty wrote: > >Does anyone else have any other ideas on how to fix this issue? > > The current MAD interface requires the user to have code similar to this: > > send_buf->sge.addr = dma_map_single(mad_agent->device->dma_device, > buf, buf_size, DMA_TO_DEVICE); > pci_unmap_addr_set(send_buf, mapping, send_buf->sge.addr); > > This is consistent with how an ib_send_wr would be formatted for other QPs. > Another possibility, however, is to let the user do: > > send_buf->sge.addr = (unsigned long) buf; > > And then have the MAD layer perform the mapping/unmapping immediately > before and after posting to the QP. This keeps the syntax of the current > interface, but still requires user changes. If you change behaviour you should change the interface, in this case you'd _really_ want to pass down the buffer as void pointer and not cast it to a dma_addr_t - that would in fact break on ppc64 where dma_addr_t is a 32bit data type and a pointer is 64bits wide. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [RFC] IB address translation using ARP
On Thu, Oct 13, 2005 at 10:29:09AM -0700, Michael Krause wrote: > This all comes down to economics which is why some ULP such as SDP are > created. Let's examine SDP for a moment. The purpose of SDP to enable > synchronous and asynchronous Sockets applications to transparently run > unmodified over a RDMA capable interconnect. Unmodified means no source > code changes and no recompile required (this is possible if the Sockets > library is a shared library and dynamically linked). The first part of > unmodified means that the existing address / service resolution API calls > work (further, no change to the address family, etc. is required to make > this work either). Hence, pick any of the get* API calls that are in use > today and they should just work. That's not who SDP is going to work on Linux, though. Where not into your crude hacks to let broken applications work with new technology business. Applications will have to use SDP directly or via getaddrinfo and we will never put in a broken sockets switch. And can you _please_ stop all thise time to market and similar business crap? That simply doesn't matter when designing something properly. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: [PATCH] [CMA] RDMA CM abstraction module
On Mon, Oct 10, 2005 at 12:53:29PM -0700, Michael Krause wrote: > standards. There are also the new standard Sockets extension API available > today that might be extended sometime in the future to include explicit which is never going to get into linux. one more of these braindead standards people masturbating in a dark room and coming up with a frankenstein bastard cases. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] mthca: check for illegal acl when registering an mr
On Sun, Oct 02, 2005 at 03:25:52PM +0200, Jack Morgenstein wrote: > Now check in kernel space for illegal combination of acl parameters > (per IB Spec 11.2.8.2). The check should be in ib_uverbs_reg_mr(), not in every driver. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general][PATCH][SRP] bug fixes & fmr supported,
On Tue, Sep 20, 2005 at 05:46:06PM -0700, Roland Dreier wrote: > Vu> OK we can do the same way that you handled max_sectors. SRP > Vu> targets may prefer a specific cmds_per_lun to reach max > Vu> sequential performance. max_targets == max_id > > Does max_id matter at all for SRP? We only use scsi_scan_target() to > find targets, so I'm not sure where the SCSI midlayer even looks at > max_id for our case. It shouldn't. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general][PATCH][SRP] bug fixes & fmr supported,
On Tue, Sep 20, 2005 at 04:52:54PM -0700, Roland Dreier wrote: > Thanks, I haven't read all the FMR stuff through yet, but a few quick > comments: > > > + support more than default 8 luns per target. Should we have max_luns > > as module param? How about cmds_per_lun, max_sectors, max_targets as > > module params as well > > I think it makes more sense to handle this the same way I handled > max_sectors: make it a per-target parameter passed in when connecting > to the target. We could make cmds_per_lun a similar parameter, but > are there likely to be any SRP targets that need this to be limited? > Also, what is max_targets? Why do we need a limited max_luns at all? I hope all SRP targets propetly support REPORT_LUNS in which case we couldn't care less about a maximum LUN limit. And even if they don't I hope they handle scanning the first non-existant LUN gracefully. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general][PATCH][SRP] bug fixes & fmr supported,
> + if ((dma_addr & (PAGE_SIZE - 1)) ||
> + ((dma_addr + dma_len) & (PAGE_SIZE - 1)) ||
> + ((i == (sg_cnt - 1)) && !unaligned)) {
> + srp_fmr->io_addr = dma_addr & PAGE_MASK;
> + ++unaligned;
> + }
> +
> + if (unaligned <= 1) {
> + cur_len += dma_len;
> + for (base_addr = dma_addr;
> + (dma_addr & PAGE_MASK) <=
> + ((base_addr + dma_len - 1) & PAGE_MASK);
> + dma_addr += PAGE_SIZE)
> + dma_pages[page_cnt++] = dma_addr & PAGE_MASK;
> + }
> +
> + if ((unaligned > 1) || (i == (sg_cnt - 1))) {
this is definitly completely broken. dma_addr_ts are opaqueue handles,
some platforms use high bits in them for iommu flags.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Managing SRP devices via iSCSI ?
On Mon, Sep 19, 2005 at 10:01:51PM -0400, Rick Frank wrote: > One key argument I've heard in favor of iSER vs SRP is that iSCSI (top level > iSER driver) has a very strong management infrastructure - as it is fairly > mature. > > However, iSER seems to be just gaining steam in terms of direct attached > storage supporting this protocol .vs. SRP. > > Would it not be possible to implement some glue between SRP and iSCSI to > allow for the discovery and management of SRP devices ? No. It's also pretty pointless because the iSCSI code doesn't really offer anything fine over SRP. Just use SRP and report anything lacking in management, we'll try to address it. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: Opensm - casting issues #2
On Tue, Sep 13, 2005 at 09:26:31AM -0700, Sean Hefty wrote: > Christoph Hellwig wrote: > >Why does the windows port needs a separate repository? Please just > >check all windows code (not just opensm) into the openib repository. > > My understanding is that the labs, who control the OpenIB servers, refused > to host any Windows related code, forcing it to have a separate repository. It shouldn't be difficult to find someone to host it. I could maybe ask if such a repo could be put at the lst.de servers. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: Opensm - casting issues #2
On Tue, Sep 13, 2005 at 04:16:01PM +0300, Eitan Zahavi wrote: > The answer here is a little more complex: > The WinIB repository will be used to keep the windows code. > However, we are looking for a way to automatically update it > with changes from the OpenIB trunk. So we are looking for > having all the "core" of OpenSM be maintained only in OpenIB and > be automatically "copied" into the WinIB. Why does the windows port needs a separate repository? Please just check all windows code (not just opensm) into the openib repository. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] [verbs] add node_guid to device structure
On Sun, Sep 11, 2005 at 08:34:32AM -0700, Roland Dreier wrote: > Christoph> The PAGE_SIZE slab cache doesn't make sense - just use > Christoph> alloc_page to get a page or merge it into the other > Christoph> slab cache. Probably the former makes more sense. > > I'm confused. I couldn't find any use of PAGE_SIZE in the patch > you're replying to. More details please... Sorry, looks like I replied to a totally unrelated mail for some reason. I'll resend it in the right thread. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] [verbs] add node_guid to device structure
On Wed, Sep 07, 2005 at 04:24:56PM -0700, Sean Hefty wrote: > This patch adds the node_guid to struct ib_device to avoid ULPs needing > to query for it. > > It will also make it possible to give users the attributes of a device > as part of their add_device routine. > > If this patch is okay with everyone, I will submit patches to remove > the device attribute queries in the CM, SRP, and sysfs. The PAGE_SIZE slab cache doesn't make sense - just use alloc_page to get a page or merge it into the other slab cache. Probably the former makes more sense. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] sdp: kill sdp buff pool
On Thu, Sep 08, 2005 at 02:41:56PM +0300, Michael S. Tsirkin wrote: > SDP seems to have a re-implementation of kmem_cache in sdp_buff. > Killing it actually results in a small bandwidth increase > for both bcopy and zcopy versions, so I plan to put this in. > > Note that sdp_buff_pool_chain_link is now same as sdp_buff_pool_put, > and sdp_buff_pool_chain_put is now a nop, but I decided to > keep it in for now, just in case there's another use for it. Btw, the whole sdb_buff code looks a little like it's duplicating the network layers sk_buff code. Anyone looking into this higher-level thing? ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [git pull] InfiniBand updates
On Sat, Sep 10, 2005 at 08:03:45PM -0700, Chris Wedgwood wrote: > On Sat, Sep 10, 2005 at 04:02:12PM -0700, Roland Dreier wrote: > > > include/rdma/ib_cm.h |1 > > include/rdma/ib_mad.h | 21 ++ > > include/rdma/ib_sa.h | 31 +++ > > include/rdma/ib_user_cm.h | 72 +++ > > include/rdma/ib_user_verbs.h | 21 ++ > > Do these really need to be here? if we really must merge RDMA can we > not hide these headers in drivers/inifiniband for now? No. They've been there before, but it's just wrong. This stuff is kernel-wide interfaces and having them under drivers/ was wrong to start with. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RDMA connection and address translation API
On Thu, Aug 25, 2005 at 01:18:06PM -0400, Talpey, Thomas wrote: > At 12:56 PM 8/25/2005, Caitlin Bestler wrote: > >Generic code MUST support both IPv4 and IPv6 addresses. > >I've even seen code that actually does this. > > Let me jump ahead to the root question. How will the NFS layer know > what address to resolve? > > On IB mounts, it will need to resolve a hostname or numeric string to > a GID, in order to provide the address to connect. On TCP/UDP, or > iWARP mounts, it must resolve to IP address. The mount command > has little or no context to perform these lookups, since it does not > know what interface will be used to form the connection. > > In exports, the server must inspect the source network of each > incoming request, in order to match against /etc/exports. If there > are wildcards in the file, a GID-specific algorithm must be applied. > Historically, /etc/exports contains hostnames and IPv4 netmasks/ > addresses. > > In either case, I think it is a red herring to assume that the GID > is actually an IPv6 address. They are not assigned by the sysadmin, > they are not subnetted, and they are quite foreign to many users. > IPv6 support for Linux NFS isn't even submitted yet, btw. > > With an IP address service, we don't have to change a line of > NFS code. I think this shows that using IP addresses in any service over infiniband that isn't actually IP networking is extremly stupid. Just stop living in the illusion that it makes sense and use IB-specific addressing, namely IB and stop all this layering violations into IP, which is much higher up the stack. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RDMA connection and address translation API
On Wed, Aug 24, 2005 at 02:22:31PM -0700, Caitlin Bestler wrote: > Not if the host connects two disjoint networks and does not route > between them. Such a host should/may be configured to reject any > packet that arrives with a destination address that does not match > the expected destination address for the port it arrives upon. While you can configure a Linux system to reject such request through a bunch of crude hacks, the default and fully RFC compliant behaviour is to always reply to ARP requests for any IP address assigned to the system. RDMA CM implementations must work the same. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RDMA connection and address translation API
On Wed, Aug 24, 2005 at 02:15:09PM -0700, Roland Dreier wrote: > Roland> Well, that's not what I would expect. Suppose I have a > Roland> device configured with local addresses 192.168.11.12 and > Roland> 192.168.98.99 and I > > Christoph> You never configure a device with local addresses. IP > Christoph> addresses are always a per-host attribute in Linux. > > I don't think this is really true. In some ways Linux behaves as if > IP addresses are per-host (eg ARP responses can go out any interface) > but really IP addresses are attached to an interface. Every struct > net_device has a struct in_device, and every struct in_device has a > list of struct in_ifaddrs for the device's IP addresses. This is correct, but the user-visible effect is what I said above. When you do an ARP query for any of the IP addresses of a linux box you'll get a responce even if that interface isn't on the network. Even if you don't think that's enough you can assign any number of IP and other networking addresses to a given device even formally, rendering the notation of an IP address <-> network device relation rather mood. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RDMA connection and address translation API
On Wed, Aug 24, 2005 at 11:14:08AM -0700, Caitlin Bestler wrote: > The concensus when this issue was debated in the DAT Collaborative was > that there was no transport neutral way to specify a set of addresses to > listen > on other than "all addresses supported by this device". That doesn't make any sense at all for iWarp as that uses IP addressing which in Linux is host-, not device-based. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RDMA connection and address translation API
On Wed, Aug 24, 2005 at 09:26:42AM -0700, Roland Dreier wrote: > Tom> I think I understand, but the purpose of specifying the IP > Tom> address in the listen is not to filter incoming connect > Tom> requests, but rather to determine which devices I listen > Tom> on. I think this works for the IB case as well. So the > Tom> utility of the IP address specified in the listen is only to > Tom> determine which devices the sid is created on. Does this make > Tom> sense or am I missing something? > > Well, that's not what I would expect. Suppose I have a device > configured with local addresses 192.168.11.12 and 192.168.98.99 and I You never configure a device with local addresses. IP addresses are always a per-host attribute in Linux. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] ISER cleanup
On Tue, Aug 23, 2005 at 07:11:18PM +0300, Dan Bar Dov wrote:
> > iser_api.h
> > Should iSCSI be providiing the jump table definitions?
> > struct iser_api_t
> > struct iser_api_cb_t
> >
> > iser_ext_api.h
> > typedef void * iser_conn_request_t;
> > Delete stuff like this - it just obscures what is going on.
> OK
>
> >
> > I'm not sure what this file is doing.
> > I was expecting iSCSI framework to define the data structures
> > it needs to talk to a service provider.
> This is an "extended API". The ISER spec defines an ISER API, but it does not
> consider implementation.
> We chose to implement the extra api out of the iser_api structute and in the
> iser_ext_api struct.
> iSCSI is still not part of the kernel so we had first modified and added the
> datamover framework to
> linux-iscsi and now to open-iscsi. Once open-iscsi is in the kernel we'll use
> it as the framework.
Note that we care very little about specifications for in-kernel APIs.
so the distinction between a standard API and extension make little
sense, in the end we'll probably only have more or less non-standard
APIs.
> > iser_types.h
> > delete typdef void * iser_api_handle_t.
> > replace usage of iser_api_handle_t with "void *".
> > Ditto for all "void *" typedefs in that file.
> OK
>
> >
> > Kernel already defines scatter-gather lists type.
> The iser_data_buf struct can point to a scatterlist array but can also be
> used to point at a single buffer.
> It does not replicate scatterlist but allows us to deal with two types of
> registrations - single buffer and scatter lists.
We're planning to get rid of non-S/G list data transfer in the scsi
subsystem for 2.6.14+. See the tree at
http://www.kernel.org/git/gitweb.cgi?p=linux/kernel/git/jejb/scsi-block-2.6.git;a=summary
in there everything but the sg,st and osst drivers doesn't submit
non-S/G requests anymore, and these last drivers is beeing worked on
already.
> > > There are many other things to be done, including both coding style
> > > and substance, we'll proceed addressing all the technical
> > issues that
> > > were commented on.
> >
> > great!
> We are going to simplify the local memory registrations by registering all
> memory like in the SRP driver.
> We do not understand some of the substance issues - for example, dma related
> comments - are taken care of by iscsi,
> not the transport. The io_mmu comment, we completely do not understand -
> there was some platform specific code, but its all gone now.
Basically all use of virt_to_{phys,bus} / {phys,bus}_to_virt is wrong.
You must use proper dma mapping routines. See Documentation/DMA-API.txt
in the kernel tree for details.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] OpenSM Coding style
Just allow new modules to be written in a readable style so people can understand at least parts of the mess that opensm is. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISERinitiator
On Fri, Aug 19, 2005 at 08:22:23AM +0300, Yaron Haviv wrote: > Not sure how you do your LOC counting or what's included in it > In any case a protocol that is generalized to multiple transports, has > built in discovery, error-recovery, global routing/naming, > authentication, built-in multi-pathing, multi-connection per session, > optimizations for small messages, comprehensive management and > configuration with industry standard APIs, etc' is a total mess and nothing you'd want to run in a production enviroment. Point taken ;-) > The important things is how many LOC are on the command path and how > optimized it the protocol, this code runs SCSI at 850-900MB/s and on the > same time provides the most comprehensive set of features, and is > managed out of the box with industry standard tools Which is slower than plain iscsi over 10Gige.. > A variation of that code runs today on PPC, so I assume it's not an > issue to make sure it runs over PPC Maybe on toy ppc processors like the 4xx. The code won't run on any non-toy platform with proper iommus without a major rework. That beeing said any driver that makes plattform assumptions at all has no business in the openib.org or mainline kernel tree. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISERinitiator
On Thu, Aug 18, 2005 at 09:24:24PM -0700, Roland Dreier wrote: > Yaron> Not every one wants to keep on doing target discovery with > Yaron> Python scripts, > > Come on, this is just a stupid statement. The whole point of putting > device management in userspace is so that everybody has the > flexibility to use whatever discovery mechanism they want. And just FYI. If you ever want an iSER implementation merged it will have to work the same way. Look at how the open-iscsi TCP initator does it. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISERinitiator
On Thu, Aug 18, 2005 at 04:41:20PM -0700, Grant Grundler wrote: > I've understood that the openib.org Verbs API can be changed to make > it "transport neutral" - ie support RNICs. RNIC vendors don't seem > to be interested in submitting patches for that. Ammasso is working on that. All the other RNIC vendors seems to be involved in the openrdma.org spec masturbation fest. Looks like we have a winner for iWarp HCAs on Linux.. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISERinitiator
> If kDAPL for any reason doesn't get pushed upstream to kernel.org, > we effectively don't have iSER or NFS/RDMA in linux. > Since I think without them, linux won't be competitive in the > commercial market place. iser doesn't matter at all in the marketplace. nfs/rdma matters and even if netapp/citi keeps beeing ignorant I will port it over to the infiniband/rdma layer myself. I'll hopefully have some iwarp cards soon. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISER initiator
On Thu, Aug 18, 2005 at 11:18:23AM -0700, Grant Grundler wrote: > On Thu, Aug 18, 2005 at 07:43:17PM +0200, Christoph Hellwig wrote: > ... > > > The same as last time, the code didn't change at all. It's still > > > totally ignorant about such essential things as dma mapping, has > > > creative new abuse for struct iovec, it's still based on iovecs, > > > > "... still based on kdapl" of course > > Yeah, I was wondering about that. When I was off on vacation > in July (and OLS), kDAPL was committed to the svn repository. > Has anyone reviewed that? We agreed that that James can commit it, but that doesn't mean it'll go anywhere. kDAPL is a dead end except for maybe out of tree vendor code. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [PATCH] srp tidyups
On Thu, Aug 18, 2005 at 10:27:49AM -0700, Roland Dreier wrote: > By the way, what's your feeling about upstream inclusion for the > driver in its current state? Except for the lack of error handling it looks nice. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISER initiator
On Thu, Aug 18, 2005 at 07:42:22PM +0200, Christoph Hellwig wrote: > On Thu, Aug 18, 2005 at 10:43:05AM -0700, Grant Grundler wrote: > > On Thu, Aug 18, 2005 at 02:36:05PM +0200, Christoph Hellwig wrote: > > > > All the iSCSI features including device management are available > > > > seamlessly with the iSCSI/ISER initiator. ISER simply puts iSCSI > > > > on steroids. > > > > > > > > The ISER implementation makes use of the openIB/kDAPL. Please note > > > > that several kDAPL patches that were submitted to the list are > > > > necessary for this implementation to work. > > > > > > The code is complete crap, please remove it again. > > > > Christoph, > > While I agree with you, that's not a very constructive approach. > > Can you pick 5 things that are brain damaged and point them out? > > The same as last time, the code didn't change at all. It's still > totally ignorant about such essential things as dma mapping, has > creative new abuse for struct iovec, it's still based on iovecs, "... still based on kdapl" of course ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISER initiator
On Thu, Aug 18, 2005 at 10:43:05AM -0700, Grant Grundler wrote: > On Thu, Aug 18, 2005 at 02:36:05PM +0200, Christoph Hellwig wrote: > > > All the iSCSI features including device management are available > > > seamlessly with the iSCSI/ISER initiator. ISER simply puts iSCSI > > > on steroids. > > > > > > The ISER implementation makes use of the openIB/kDAPL. Please note > > > that several kDAPL patches that were submitted to the list are > > > necessary for this implementation to work. > > > > The code is complete crap, please remove it again. > > Christoph, > While I agree with you, that's not a very constructive approach. > Can you pick 5 things that are brain damaged and point them out? The same as last time, the code didn't change at all. It's still totally ignorant about such essential things as dma mapping, has creative new abuse for struct iovec, it's still based on iovecs, actually missing the iscsi initiator integration, duplicating iscsi defines and scsi debugging code all over, adding tons of layers of useless abstraction. Not to mention that the code looks like a cat vomiting over the keyboard. In short the code should be thrown a way, and someone with a clue (aka not a Voltaire person) needs to start over again. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] [PATCH] srp tidyups
- you must not call the scsi_done callback in the EH routines.
Will there be real error handling for srp one day?
- given that you have on scsi host per srp target there's no need
for the idr lookup, you can just use the host private data
- no need to fill out unique_id, that's a leftover for ISA HBAs
- and some small cleanups I couldn't resist
Index: ulp/srp/ib_srp.c
===
--- ulp/srp/ib_srp.c(revision 3129)
+++ ulp/srp/ib_srp.c(working copy)
@@ -34,14 +34,11 @@
#include
#include
-
#include
#include
#include
-#include
#include
#include
-
#include
#include
@@ -69,11 +66,6 @@
static const u8 topspin_oui[3] = { 0x00, 0x05, 0xad };
-static atomic_t srp_uid;
-
-static rwlock_t idr_lock;
-static DEFINE_IDR(target_idr);
-
static void srp_add_one(struct ib_device *device)et
static void srp_remove_one(struct ib_device *device);
@@ -101,27 +93,29 @@
iu = kmalloc(sizeof *iu, gfp_mask);
if (!iu)
- return NULL;
+ goto out;
iu->buf = kmalloc(size, gfp_mask);
- if (!iu->buf) {
- kfree(iu);
- return NULL;
- }
+ if (!iu->buf)
+ goto out_free_iu;
memset(iu->buf, 0, size);
iu->dma = dma_map_single(host->dev->dma_device, iu->buf, size,
direction);
- if (dma_mapping_error(iu->dma)) {
- kfree(iu->buf);
- kfree(iu);
- return NULL;
- }
+ if (dma_mapping_error(iu->dma))
+ goto out_free_buf;
iu->size = size;
iu->direction = direction;
return iu;
+
+ out_free_buf:
+ kfree(iu->buf);
+ out_free_iu:
+ kfree(iu);
+ out:
+ return NULL;
}
static void srp_free_iu(struct srp_host *host, struct srp_iu *iu)
@@ -142,7 +136,7 @@
u8 fmt;
if (!scmnd->request_buffer || scmnd->sc_data_direction == DMA_NONE)
- return sizeof (struct srp_cmd);
+ return sizeof(struct srp_cmd);
if (scmnd->sc_data_direction != DMA_FROM_DEVICE &&
scmnd->sc_data_direction != DMA_TO_DEVICE) {
@@ -456,31 +450,11 @@
static int srp_queuecommand(struct scsi_cmnd *scmnd,
void (*done)(struct scsi_cmnd *))
{
- struct srp_target_port *target;
- struct srp_iu *iu;
+ struct srp_target_port *target = host_to_target(scmnd->device->host);
+ struct srp_iu *iu = target->tx_ring[target->tx_head & SRP_SQ_SIZE];
struct srp_cmd *cmd;
- unsigned long flags;
int len;
- read_lock_irqsave(&idr_lock, flags);
- target = idr_find(&target_idr, scmnd->device->id);
- read_unlock_irqrestore(&idr_lock, flags);
-
- if (!target) {
- printk(KERN_ERR PFX "queuecommand for unknown device id %d\n",
- scmnd->device->id);
- scmnd->result = DID_ERROR << 16;
- done(scmnd);
- return 0;
- }
-
- if (0) {
- printk(KERN_ERR PFX "command for %u: ", scmnd->device->id);
- scsi_print_command(scmnd);
- }
-
- iu = target->tx_ring[target->tx_head & SRP_SQ_SIZE];
-
dma_sync_single_for_cpu(target->srp_host->dev->dma_device, iu->dma,
SRP_MAX_IU_LEN, DMA_TO_DEVICE);
@@ -499,7 +473,7 @@
len = srp_map_data(scmnd, target, iu);
if (len < 0) {
printk(KERN_ERR PFX "Failed to map data\n");
- goto err;
+ goto err_free_iu;
}
if (srp_post_recv(target, GFP_ATOMIC)) {
@@ -519,8 +493,7 @@
err_unmap:
srp_unmap_data(scmnd, target, cmd);
-
-err:
+err_free_iu:
return SCSI_MLQUEUE_HOST_BUSY;
}
@@ -528,9 +501,6 @@
{
printk(KERN_ERR PFX "srp_abort called\n");
-scmnd->result = DID_ABORT << 16;
- scmnd->scsi_done(scmnd);
-
return SUCCESS;
}
@@ -538,9 +508,6 @@
{
printk(KERN_ERR PFX "srp_reset called\n");
-scmnd->result = DID_ABORT << 16;
- scmnd->scsi_done(scmnd);
-
return SUCCESS;
}
@@ -892,10 +859,7 @@
static void srp_release_target(struct srp_target_port *target)
{
int i;
- unsigned long flags;
- /* XXX should send SRP_I_LOGOUT request */
-
init_completion(&target->done);
ib_send_cm_dreq(target->cm_id, NULL, 0);
wait_for_completion(&target->done);
@@ -907,10 +871,6 @@
srp_free_iu(target->srp_host, target->rx_ring[i]);
for (i = 0; i < SRP_SQ_SIZE + 1; ++i)
srp_free_iu(target->srp_host, target->tx_ring[i]);
-
- write_lock_irqsave(&idr_lock, flags);
- idr_remove(&target_idr, target->scsi_id);
- write_unlock_irqrestore(&idr_lock, flags);
}
static struct scsi_host_template srp_template = {
@@ -933,21 +893,8 @@
unsigned long flags;
int ret;
- do
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISER initiator
On Thu, Aug 18, 2005 at 02:36:05PM +0200, Christoph Hellwig wrote: > > I just checked in a first version of iSCSI Extensions for RDMA > > Protocol (ISER) initiator under infiniband/ulp/iser. This > > implements the ISER datamover, a transport layer alternative to > > TCP/IP usable by iSCSI. This ISER transport has been tested with > > the open-iscsi opensource project, and against the Voltaire > > Fibre-Channel Router (FCR) and Voltaire's Native-IB storage kit. While we're at it can the admins please remove check in permissions for Dan? Someone who writes such code with zero understanding of kernel intenrals and checks in without asking can cause far too much damage. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [ANNOUNCE] Initial trunk checkin of ISER initiator
On Thu, Aug 18, 2005 at 03:14:05PM +0300, Dan Bar Dov wrote: > I just checked in a first version of iSCSI Extensions for RDMA > Protocol (ISER) initiator under infiniband/ulp/iser. This > implements the ISER datamover, a transport layer alternative to > TCP/IP usable by iSCSI. This ISER transport has been tested with > the open-iscsi opensource project, and against the Voltaire > Fibre-Channel Router (FCR) and Voltaire's Native-IB storage kit. > > All the iSCSI features including device management are available > seamlessly with the iSCSI/ISER initiator. ISER simply puts iSCSI > on steroids. > > The ISER implementation makes use of the openIB/kDAPL. Please note > that several kDAPL patches that were submitted to the list are > necessary for this implementation to work. The code is complete crap, please remove it again. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] Re: [PATCH] sockaddr_ll change for IPoIB interface
On Fri, Aug 12, 2005 at 09:21:29AM -0700, Tom Duffy wrote: > I don't want to get into a big debate about this. If a good solution > can be had that will both maintain compatibility and allow for IB, I > would welcome that. On the other hand, most of the interesting apps > have broken on Linux in the past few years. Some examples: > > - Loki games > - Word Perfect 8 > - Crossover office/plugin > - java s/interesting apps/crappy shit that relies on undefined behaviour/ I can still run most of some really old a.out slackware, including the original doom port on my only x86 box (binfmt_aout complains a lot about some missing alignment and stuff, though ;)) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] RE: [PATCH] amso1100: use standard byteorder macros
On Thu, Aug 11, 2005 at 04:13:24PM -0700, Roland Dreier wrote: > Tom> Christoph: This is great stuff, but would be even better if > Tom> we just globally replaced thinks like "cpu_to_wr64" with > Tom> "cpu_to_be64" and removed the cc_byteorder.h file altogether? > > Yes, if your byte order is definitely frozen to big-endian, then you > should definitely killall the cpu_to_wrXX wrappers. Agreed. This was just the most trivial obvious correct patch to get things to mostly compile on ppc. I'll do a new round of patches when I find time. ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] [PATCH] amso1100: kill memcpy4 and memcpy8
memcpy8 isn't used at all, and for memcpy4 we should just rely on the
compiler to do the right optimizations for us.
Signed-off-by: Christoph Hellwig <[EMAIL PROTECTED]>
Index: cc_qp_common.c
===
--- cc_qp_common.c (revision 3058)
+++ cc_qp_common.c (working copy)
@@ -135,139 +135,6 @@
/*
- * Function: cc_memcpy8
- *
- * Description:
- * Just like memcpy, but does 16 and 8 bytes at a time.
- *
- * IN:
- * dest- ptr destination
- * src - ptr source
- * len - The len, in bytes
- *
- * OUT: none
- *
- * Return: none
- */
-void
-cc_memcpy8( u64 *dest, u64 *src, s32 len)
-{
-#ifdef CCDEBUG
- assert((len & 0x03) == 0);
- assert(((s32)dest & 0x03) == 0);
- assert(((s32)src & 0x03) == 0);
-#endif
-
-#if (defined(X86_32) || defined(X86_64))
-
-#define MINSIZE 16
- /* unaligned data copy, 16 bytes at a time */
- while(len >= MINSIZE) {
- /* printf("%p --> %p 16B unaligned copy,len=%d \n", src,
dest,len); */
- asm volatile("movdqu 0(%1), %%xmm0\n" \
-"movdqu %%xmm0, 0(%0)\n" \
-:: "r"(dest), "r"(src) : "memory");
- src += 2;
- dest += 2;
- len -= 16;
- }
-
- /* At this point, we'll have fewer than 16 bytes left.
-* But, we only allow 8 byte copies. So, we do 8 byte copies now.
-* If our len happens to be 4 or 12, we will copy 8 or 16 bytes,
-* respectively. This is not a problem, since
-* all msg_sizes in all WR queues are padded up to 8 bytes
-* (see fw/clustercore/cc_qp.c, the function ccwr_qp_create()).
-*/
- while(len >= 0) {
- /* printf("%p --> %p 8B copy,len=%d \n", src, dest,len); */
- asm volatile("movq 0(%1), %%xmm0\n" \
-"movq %%xmm0, 0(%0)\n" \
-:: "r"(dest), "r"(src) : "memory");
- src += 1;
- dest += 1;
- len -= 8;
- }
-
-#elif
- #error "You need to define your platform, or add optimized"
- #error "cc_memcpy8 support for your platform."
-
-#endif /*(defined(X86_64) || defined(X86_32)) */
-
-}
-
-/*
- * Function: memcpy4
- *
- * Description:
- * Just like memcpy, but assumes all args are 4 byte aligned already.
- *
- * IN:
- * dest- ptr destination
- * src - ptr source
- * len - The len, in bytes
- *
- * OUT: none
- *
- * Return: none
- */
-static __inline__ void
-memcpy4(u64 *dest, u64 *src, u32 len)
-{
-#ifdef __KERNEL__
- unsigned long flags;
-#endif /* #ifdef __KERNEL__ */
-
- u64 xmm_regs[16]; /* Reserve space for 8, though only use 1 now. */
-
-#ifdef CCDEBUG
- ASSERT((len & 0x03) == 0);
- ASSERT(((long)dest & 0x03) == 0);
- ASSERT(((long)src & 0x03) == 0);
-#endif
-
- /* We must save and restor xmm0.
-* Failure to do so messes up the application code.
-*/
- asm volatile("movdqu %%xmm0, 0(%0)\n" :: "r"(xmm_regs) : "memory");
-
-#ifdef __KERNEL__
- /* Further, in the kernel version, we must disable local interupts.
-* This is because ISRs do not save & restore xmm0. So, if
-* we are interrupted between the first movdqu and the second,
-* then xmm0 may be modified, and we will write garbage to the adapter.
-*/
- local_irq_save(flags);
-#endif /* #ifdef __KERNEL__ */
-
-#define MINSIZE 16
- /* unaligned data copy */
- while(len >= MINSIZE) {
- asm volatile("movdqu 0(%1), %%xmm0\n" \
-"movdqu %%xmm0, 0(%0)\n" \
-:: "r"(dest), "r"(src) : "memory");
- src += 2;
- dest += 2;
- len -= 16;
- }
-
-#ifdef __KERNEL__
- /* Restore interrupts and registers */
- local_irq_restore(flags);
- asm volatile("movdqu 0(%0), %%xmm0\n" :: "r"(xmm_regs) : "memory");
-#endif /* #ifdef __KERNEL__ */
-
- while (len >= 4) {
- *((u32 *)dest) = *((u32 *)src);
- dest = (u64*)((unsigned long)dest + 4);
- src = (u64*)((unsigned long)src + 4);
- len -= 4;
- }
-}
-
-
-/*
* Function: qp_wr_post
*
* Description:
@@ -308,7 +175,7 @@
/*
* Copy the wr down to the adapter
*/
- memcpy4((void *)msg, (void *)wr, size);
+ memcpy(msg, wr, size);
cc_mq_pro
[openib-general] [PATCH] amso1100: use standard byteorder macros
Signed-off-by: Christoph Hellwig <[EMAIL PROTECTED]>
Index: cc_byteorder.h
===
--- cc_byteorder.h (revision 3058)
+++ cc_byteorder.h (working copy)
@@ -1,113 +1,30 @@
#ifndef _CC_BYTEORDER_H_
#define _CC_BYTEORDER_H_
+#include
#include "cc_types.h"
-static inline const u64 cc_arch_swap64(u64 x)
-{
- union {
- struct { u32 a,b; } s;
- u64 u;
- } v;
-
- v.u = x;
-
- asm("bswap %0\n\t"
-"bswap %1\n\t"
-"xchgl %0,%1\n"
- : "=r" (v.s.a), "=r" (v.s.b)
- : "0" (v.s.a), "1" (v.s.b));
-
- return v.u;
-}
-
-static inline const u32 cc_arch_swap32(u32 x)
-{
- asm("bswap %0" : "=r" (x) : "0" (x));
- return x;
-}
-
-#define cc_swap16(x) \
-({ \
- u16 __x = (x); \
- ((u16)( \
- (((u16)(__x) & (u16)0x00ffU) << 8) | \
- (((u16)(__x) & (u16)0xff00U) >> 8) )); \
-})
-
-#define cc_swap32(x) \
-({ \
- u32 __x = (x); \
- ((u32)( \
- (((u32)(__x) & (u32)0x00ffUL) << 24) | \
- (((u32)(__x) & (u32)0xff00UL) << 8) | \
- (((u32)(__x) & (u32)0x00ffUL) >> 8) | \
- (((u32)(__x) & (u32)0xff00UL) >> 24) )); \
-})
-
-#define cc_swap64(x) \
-({ \
- u64 __x = (x); \
- ((u64)( \
- (u64)(((u64)(__x) & (u64)0x00ffULL) << 56) | \
- (u64)(((u64)(__x) & (u64)0xff00ULL) << 40) | \
- (u64)(((u64)(__x) & (u64)0x00ffULL) << 24) | \
- (u64)(((u64)(__x) & (u64)0xff00ULL) << 8) | \
- (u64)(((u64)(__x) & (u64)0x00ffULL) >> 8) | \
- (u64)(((u64)(__x) & (u64)0xff00ULL) >> 24) | \
- (u64)(((u64)(__x) & (u64)0x00ffULL) >> 40) | \
- (u64)(((u64)(__x) & (u64)0xff00ULL) >> 56) )); \
-})
-
-/* This section defines what it means to swap a word into the byte
- order of the current CPU. For example, x86-32 and x86-64 are
- little-endian platforms, so swapping a big-endian number to the
- cpu means the bytes need to be rearranged. However, swapping a
- little-endian number to the cpu means that nothing should be done.
-*/
-
-#define X86_32
-#if defined(X86_32) || defined (X86_64)
-
-#define cc_be64_to_cpu(x) (__builtin_constant_p((u64)(x)) ? cc_swap64(x) :
cc_arch_swap64(x))
-#define cc_be32_to_cpu(x) (__builtin_constant_p((u32)(x)) ? cc_swap32(x) :
cc_arch_swap32(x))
-#define cc_be16_to_cpu(x) cc_swap16(x)
-#define cc_cpu_to_be64(x) cc_be64_to_cpu(x)
-#define cc_cpu_to_be32(x) cc_be32_to_cpu(x)
-#define cc_cpu_to_be16(x) cc_be16_to_cpu(x)
-
-#define cc_le64_to_cpu(x) ((u64)(x))
-#define cc_le32_to_cpu(x) ((u32)(x))
-#define cc_le16_to_cpu(x) ((u16)(x))
-#define cc_cpu_to_le64(x) ((u64)(x))
-#define cc_cpu_to_le32(x) ((u32)(x))
-#define cc_cpu_to_le16(x) ((u16)(x))
-
-#else
-#error Byte swapping functions not defined for this platform
-#endif
-
/* Here we define the adapter-to-cpu and cpu-to-adapter byte order functions
based on whether the adapter is big-endian or little-endian.
*/
#if defined(WR_BYTE_ORDER_BIG_ENDIAN)
-#define wr64_to_cpu cc_be64_to_cpu
-#define wr32_to_cpu cc_be32_to_cpu
-#define wr16_to_cpu cc_be16_to_cpu
-#define cpu_to_wr64 cc_cpu_to_be64
-#define cpu_to_wr32 cc_cpu_to_be32
-#define cpu_to_wr16 cc_cpu_to_be16
+#define wr64_to_cpu be64_to_cpu
+#define wr32_to_cpu be32_to_cpu
+#define wr16_to_cpu be16_to_cpu
+#define cpu_to_wr64 cpu_to_be64
+#define cpu_to_wr32 cpu_to_be32
+#define cpu_to_wr16 cpu_to_be16
#elif defined (WR_BYTE_ORDER_LITTLE_ENDIAN)
-#define wr64_to_cpu cc_le64_to_cpu
-#define wr32_to_cpu cc_le32_to_cpu
-#define wr16_to_cpu cc_le16_to_cpu
-#define cpu_to_wr64 cc_cpu_to_le64
-#define cpu_to_wr32 cc_cpu_to_le32
-#define cpu_to_wr16 cc_cpu_to_le16
+#define wr64_to_cpu le64_to_cpu
+#define wr32_to_cpu le32_to_cpu
+#define wr16_to_cpu le16_to_cpu
+#define cpu_to_wr64 cpu_to_le64
+#define cpu_to_wr32 cpu_to_le32
+#define cpu_to_wr16 cpu_to_le16
#else
#error Work request (WR) byte order is not defined.
___
openib-general mailing list
openib-general@openib.org
http://openib.org/mailman/listinfo/openib-general
To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
Re: [openib-general] [PATCH] Fix amso1100/Kconfig
On Tue, Aug 09, 2005 at 10:58:58PM -0500, Tom Tucker wrote: > oof -- sorry guys. i thought we did that. > > Can you give me some quick guidance on the accepted test-build > procedure. 64, 32, both. minimum test targets are normally: i386 - currently most used platform, little endian and 32bit ppc64 - big endian and 64bit, used quite a lot aswell if you're really eager throw in sparc64 aswell for some odd alignment restrictions (more a runtime than compile-time thing) ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
[openib-general] Re: [RFC] Move InfiniBand .h files
On Thu, Aug 04, 2005 at 10:32:14AM -0700, Roland Dreier wrote: > I would like to get people's reactions to moving the InfiniBand .h > files from their current location in drivers/infiniband/include/ to > include/linux/rdma/. If we agree that this is a good idea then I'll > push this change as soon as 2.6.14 starts. include/rmda, please. not need for the linux/ component. > > The advantages of doing this are: > ___ openib-general mailing list openib-general@openib.org http://openib.org/mailman/listinfo/openib-general To unsubscribe, please visit http://openib.org/mailman/listinfo/openib-general
