Messages in this thread | | | Date | Tue, 03 Mar 2009 10:49:15 -0600 | From | Mike Christie <> | Subject | Re: [PATCH] hpsa: SCSI driver for HP Smart Array controllers |
| |
scameron@beardog.cca.cpqcorp.net wrote: > [...] >>> +/* >>> + * For operations that cannot sleep, a command block is allocated at init, >>> + * and managed by cmd_alloc() and cmd_free() using a simple bitmap to track >>> + * which ones are free or in use. Lock must be held when calling this. >>> + * cmd_free() is the complement. >>> + */ >>> +static struct CommandList_struct *cmd_alloc(struct ctlr_info *h) >>> +{ >>> + struct CommandList_struct *c; >>> + int i; >>> + union u64bit temp64; >>> + dma_addr_t cmd_dma_handle, err_dma_handle; >>> + >>> + do { >>> + i = find_first_zero_bit(h->cmd_pool_bits, h->nr_cmds); >>> + if (i == h->nr_cmds) >>> + return NULL; >>> + } while (test_and_set_bit >>> + (i & (BITS_PER_LONG - 1), >>> + h->cmd_pool_bits + (i / BITS_PER_LONG)) != 0); >> Using bitmap to manage free commands looks too complicated a bit to >> me. Can we just use lists for command management? > > Hmm, this doesn't seem all that complicated to me, and this code snippet > has been pretty stable for about 10 years. it's nearly identical to what's in > cpqarray in the 2.2.13 kernel from 1999: > > do { > i = find_first_zero_bit(h->cmd_pool_bits, NR_CMDS); > if (i == NR_CMDS) > return NULL; > } while(test_and_set_bit(i%32, h->cmd_pool_bits+(i/32)) != 0) > > It's fast, works well, and has needed very little maintenance over the > years. Without knowing what you have in mind specifically, I don't see a > big need to change this. >
Other drivers have had to convert to and modify the host tagging to get merged. They too had stable and fast code, and we complained and fought against changing it :) I have had to convert or help convert libfc and qla4xxx and I will now convert iscsi, so I feel your pain :)
To create the map call scsi_init_shared_tag_map after you allocate the scsi host and before you add it. Then in slave_alloc set the sdev->tag_supported = 1 and call scsi_activate_tcq. Then replace your bitmap with:
for scsi commands:
c = h->cmd_pool + scsi_cmnd->request->tag
And for the reset path I was thinking you could use my patch in the other mail and do
tag = blk_map_start_tag(scsi_host->bqt, NULL, 0); if (tag < 0) goto fail; c = h->cmd_pool + tag; c->cmdindex = tag;
In the completion path you need to do a blk_map_end_tag(scsi_host->bqt, c->cmdindex);. The scsi/block layer will free the tag for scsi commadns.
| |