Re: [PATCH 37/43] hpsa: use block layer tag for command allocation

Christoph Hellwig
2015-02-23T20:25:27Z

> @@ -4691,27 +4701,31 @@ static int hpsa_scsi_queue_command(struct Scsi_Host *sh, struct scsi_cmnd *cmd)
>
> /* Get the ptr to our adapter structure out of cmd->host. */
> h = sdev_to_hba(cmd->device);
> - dev = cmd->device->hostdata;
> - if (!dev) {
> +
> + if (cmd->request->tag < 0) {
> + dev_crit(&h->pdev->dev,
> + "sdev %p scmd %p: bad tag %d -- rejecting\n",
> + cmd->device, cmd, cmd->request->tag);
> cmd->result = DID_NO_CONNECT << 16;
> cmd->scsi_done(cmd);
> return 0;

Shouldn't happen with a modern kernel, but if you really want to
assert this I'd recommend a BUG_ON().

> - if (unlikely(lockup_detected(h))) {
> + dev = cmd->device->hostdata;
> + if (!dev) {
> cmd->result = DID_NO_CONNECT << 16;
> cmd->scsi_done(cmd);
> return 0;
> }

How could this happen (see my earlier question in one of the first
patches).

> @@ -4838,6 +4852,11 @@ static int hpsa_register_scsi(struct ctlr_info *h)
> sh->hostdata[0] = (unsigned long) h;
> sh->irq = h->intr[h->intr_mode];
> sh->unique_id = sh->irq;
> + if (!shost_use_blk_mq(sh)) {
> + error = scsi_init_shared_tag_map(sh, sh->can_queue);
> + if (error)
> + goto fail_host_put;
> + }

scsi_init_shared_tag_map alredy has the blk-mq check, so you can call
it unconditionally.

> +static void hpsa_print_scsi_cmd(struct ctlr_info *h, struct scsi_cmnd *scmd,
> + char *prefix)

Maybe just use scsi_print_command?

> +static void cmd_tagged_free(struct ctlr_info *h, struct CommandList *c)
> +{
> + /*
> + * Release our reference to the block. We don't need to do anything
> + * else to free it, because it is accessed by index. (There's no point
> + * in checking the result of the decrement, since we cannot guarantee
> + * that there isn't a concurrent abort which is also accessing it.)
> + */
> + (void)atomic_dec_and_test(&c->refcount);

Just use atomic_dec().

Any reason you're implementing your own allocator instead of setting
->cmd_size?