On 4/16/2020 12:11 AM, Hannes Reinecke wrote:
...
+ if (efct) {
+ memset(efct, 0, sizeof(*efct));
+ for (i = 0; i < ARRAY_SIZE(efct_devices); i++) {
+ if (!efct_devices[i]) {
+ efct->instance_index = i;
+ efct_devices[i] = efct;
+ break;
+ }
+ }
+
+ if (i == ARRAY_SIZE(efct_devices)) {
+ pr_err("Exceeded max supported devices.\n");
+ kfree(efct);
+ efct = NULL;
+ } else {
+ efct->attached = false;
+ }
Errm. This is wrong.
When we exit the for() loop above, i _will_ equal the array size.
Surely you mean
if (i < ARRAY_SIZE())
right?
No. We want to free the structure if we went through the array and
didn't find an empty slot. It only breaks from the loop if it found an
empty slot and used it.
+ } else if (event->topology == SLI_LINK_TOPO_LOOP) {
+ u8 *buf = NULL;
+
+ efc_log_info(hw->os, "Link Up, LOOP, speed is %d\n",
+ event->speed);
+ dma = &hw->loop_map;
+ dma->size = SLI4_MIN_LOOP_MAP_BYTES;
+ dma->virt = dma_alloc_coherent(&efct->pcidev->dev,
+ dma->size, &dma->phys,
+ GFP_DMA);
+ if (!dma->virt)
+ efc_log_err(hw->os, "efct_dma_alloc_fail\n");
+
+ buf = kmalloc(SLI4_BMBX_SIZE, GFP_ATOMIC);
+ if (!buf)
+ break;
+
+ if (!sli_cmd_read_topology(&hw->sli, buf,
+ SLI4_BMBX_SIZE,
+ &hw->loop_map)) {
+ rc = efct_hw_command(hw, buf, EFCT_CMD_NOWAIT,
+ __efct_read_topology_cb,
+ NULL);
+ }
+
Not sure if this is a good idea; we'll have to allocate extra memory
whenever the loop topology changes.
Which typically happens when there had been a failure somewhere, and
chances are that it'll affect our root fs, making memory allocation
something we really need to look at.
Can't we pre-allocate a buffer here somewhere in the global
initialisation so that we don't have to allocate it every time?
Agree. will look into it.
...
What happened to multiqueue support?
The original lpfc driver did it, so we should regress for the new driver...
For a target driver ?
When something like this is in the tgt infrastructure we'll do something
with it.
-- james
