Anil Ravindranath wrote:
> Hi Brian,
>
>
> On Thu, 3 Sep 2009, Brian King wrote:
>
>> Anil,
>>
>> Taking a quick scan through the driver, I would suggest you audit your
>> logging. Looks like there is a lot of dev_err, where sdev_printk might be
>> better,
>
> I hope I am understanding this correct...
>
> I don't see a difference in using dev_err Vs. sdev_printk. B'coz both call
> dev_printk except the prefix of KERN_ERR. But I guess even if we use
> sdev_printk we will use KERN_ERR anyways. Also both refer "Generic device
> interface".
>
> #define dev_err(dev, format, arg...) \
> dev_printk(KERN_ERR , dev , format , ## arg)
>
> #define sdev_printk(prefix, sdev, fmt, a...) \
> dev_printk(prefix, &(sdev)->sdev_gendev, fmt, ##a)
The difference is the dev pointer that gets passed to dev_printk. When calling
sdev_printk, the dev pointer for the scsi device gets passed, so the printk
ends up getting prefixed with the SCSI location: 0:0:0:0, for example where
you have SCSI Host:Bus:Target:LUN. The dev_err calls I see in the driver
generally pass the dev pointer of the PCI device, resulting in the PCI location
of the adapter being logged. Its just a matter of scoping the printk to the
device.
>
> struct scsi_device {
>
> struct device sdev_gendev;
>
> }
>
> and pmcraid_info where sdev_info might be better, etc.
> Where/what is this sdev_info?
There isn't one... I meant to say dev_info, sdev_printk, scmd_printk, etc...
>>> +
>>> +/**
>>> + * pmcraid_eh_abort_handler - entry point for aborting a single task on errors
>>> + *
>>> + * @scsi_cmd: scsi command struct given by mid-layer. When this is called
>>> + * mid-layer ensures that no other commands are queued. This
>>> + * never gets called under interrupt, but a separate eh thread.
>>> + *
>>> + * Return value:
>>> + * SUCCESS / FAILED
>>> + */
>>> +static int pmcraid_eh_abort_handler(struct scsi_cmnd *scsi_cmd)
>>> +{
>>> + struct pmcraid_instance *pinstance;
>>> + struct pmcraid_cmd *cmd;
>>> + struct pmcraid_resource_entry *res;
>>> + unsigned long host_lock_flags;
>>> + unsigned long pending_lock_flags;
>>> + struct pmcraid_cmd *cancel_cmd = NULL;
>>> + int cmd_found = 0;
>>> + int rc = FAILED;
>>> +
>>> + pinstance =
>>> + (struct pmcraid_instance *)scsi_cmd->device->host->hostdata;
>>> +
>>> + dev_err(&pinstance->pdev->dev,
>>> + "I/O command timed out, aborting it.\n");
This is an example of where using:
sdev_printk(KERN_ERR, scsi_cmd->device, "I/O command timed out, aborting it.\n");
Would help isolate the command timeout issue down to the device level rather than
just the adapter level.
>>> +
>>> + res = scsi_cmd->device->hostdata;
>>> +
>>> + if (res == NULL)
>>> + return rc;
>>> +
>>> + /* If we are currently going through reset/reload, return failed.
>>> + * This will force the mid-layer to eventually call
>>> + * pmcraid_eh_host_reset which will then go to sleep and wait for the
>>> + * reset to complete
>>> + */
>>> + spin_lock_irqsave(pinstance->host->host_lock, host_lock_flags);
>>> +
>>> + if (pinstance->ioa_reset_in_progress ||
>>> + pinstance->ioa_state == IOA_STATE_DEAD) {
>>> + spin_unlock_irqrestore(pinstance->host->host_lock,
>>> + host_lock_flags);
>>> + return rc;
>>> + }
>>> +
>>> + /* loop over pending cmd list to find cmd corresponding to this
>>> + * scsi_cmd. Note that this command might not have been completed
>>> + * already. locking: all pending commands are protected with
>>> + * pending_pool_lock.
>>> + */
>>> + spin_lock_irqsave(&pinstance->pending_pool_lock, pending_lock_flags);
>>> + list_for_each_entry(cmd, &pinstance->pending_cmd_pool, free_list) {
>>> +
>>> + if (cmd->scsi_cmd == scsi_cmd) {
>>> + cmd_found = 1;
>>> + break;
>>> + }
>>> + }
>>> +
>>> + spin_unlock_irqrestore(&pinstance->pending_pool_lock,
>>> + pending_lock_flags);
>>> +
>>> + /* If the command to be aborted was given to IOA and still pending with
>>> + * it, send ABORT_TASK to abort this and wait for its completion
>>> + */
>>> + if (cmd_found)
>>> + cancel_cmd = pmcraid_abort_cmd(cmd);
>>> +
>>> + spin_unlock_irqrestore(pinstance->host->host_lock,
>>> + host_lock_flags);
>>> +
>>> + if (cancel_cmd) {
>>> + cancel_cmd->u.res = cmd->scsi_cmd->device->hostdata;
>>> + rc = pmcraid_abort_complete(cancel_cmd);
>>> + }
>>> +
>>> + return cmd_found ? rc : SUCCESS;
>>> +}
--
Brian King
Linux on Power Virtualization
IBM Linux Technology Center
--
To unsubscribe from this list: send the line "unsubscribe linux-scsi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
