On 2/22/21 7:23 AM, Hannes Reinecke wrote:
> -
> -static struct CommandList *cmd_alloc(struct ctlr_info *h)
> +static struct CommandList *cmd_alloc(struct ctlr_info *h, u8 direction)
> {
> + struct scsi_cmnd *scmd;
> struct CommandList *c;
> - int refcount, i;
> - int offset = 0;
> -
> - /*
> - * There is some *extremely* small but non-zero chance that that
> - * multiple threads could get in here, and one thread could
> - * be scanning through the list of bits looking for a free
> - * one, but the free ones are always behind him, and other
> - * threads sneak in behind him and eat them before he can
> - * get to them, so that while there is always a free one, a
> - * very unlucky thread might be starved anyway, never able to
> - * beat the other threads. In reality, this happens so
> - * infrequently as to be indistinguishable from never.
> - *
> - * Note that we start allocating commands before the SCSI host structure
> - * is initialized. Since the search starts at bit zero, this
> - * all works, since we have at least one command structure available;
> - * however, it means that the structures with the low indexes have to be
> - * reserved for driver-initiated requests, while requests from the block
> - * layer will use the higher indexes.
> - */
> -
> - for (;;) {
> - i = find_next_zero_bit(h->cmd_pool_bits,
> - HPSA_NRESERVED_CMDS,
> - offset);
> - if (unlikely(i >= HPSA_NRESERVED_CMDS)) {
> - offset = 0;
> - continue;
> - }
> - c = h->cmd_pool + i;
> - refcount = atomic_inc_return(&c->refcount);
> - if (unlikely(refcount > 1)) {
> - cmd_free(h, c); /* already in use */
> - offset = (i + 1) % HPSA_NRESERVED_CMDS;
> - continue;
> - }
> - set_bit(i & (BITS_PER_LONG - 1),
> - h->cmd_pool_bits + (i / BITS_PER_LONG));
> - break; /* it's ours now. */
> + int idx;
> +
> + scmd = scsi_get_internal_cmd(h->raid_ctrl_sdev,
> + (direction & XFER_WRITE) ?
> + DMA_TO_DEVICE : DMA_FROM_DEVICE,
> + REQ_NOWAIT);
> + if (!scmd) {
> + dev_warn(&h->pdev->dev, "failed to allocate reserved cmd\n");
> + return NULL;
I think in the orig code cmd_alloc would always return a non null pointer.
It looks like we would always just keep looping.
Now, it looks like we could fail from the above code where we return NULL.
I was not sure if it's maybe impossible to hit the "return NULL" becuase we
only call this function when we know there will be a cmd availale. If we
can fail then the cmd_alloc callers should check for NULL now I think.
