Responding to those issues not already addressed by Bryant.
On 6/23/16 9:22 AM, Christoph Hellwig wrote:
<SNIP>
+ vscsi->flags &= (~PROCESSING_MAD);
No need for the braces here. (ditto for many other places later on)
Fixed.
<SNIP>
+static struct ibmvscsis_tport *ibmvscsis_lookup_port(const char *name)
+{
+ struct ibmvscsis_tport *tport = NULL;
+ struct vio_dev *vdev;
+ struct scsi_info *vscsi;
+
+ spin_lock_bh(&ibmvscsis_dev_lock);
+ list_for_each_entry(vscsi, &ibmvscsis_dev_list, list) {
+ vdev = vscsi->dma_dev;
+ if (!strcmp(dev_name(&vdev->dev), name)) {
+ tport = &vscsi->tport;
+ break;
+ }
+ }
+ spin_unlock_bh(&ibmvscsis_dev_lock);
Without grabbing a reference this looks inherently unsafe.
I don't understand your concern. Could you please provide more detail?
+static void ibmvscsis_scheduler(struct work_struct *work)
Odd name for a work item.
Not sure why it seems odd. It schedules work to TCM.
+{
+ struct ibmvscsis_cmd *cmd = container_of(work, struct ibmvscsis_cmd,
+ work);
+ struct scsi_info *vscsi = cmd->adapter;
+
+ spin_lock_bh(&vscsi->intr_lock);
+
+ /* Remove from schedule_q */
+ list_del(&cmd->list);
What do you need the schedule_q for as the workqueue already tracks
the commands?
There are a number of places where we need to see if there is anything
on the work queue, and I am not aware of an existing function to do
this, so we keep our own list of commands which have been queued.
<SNIP>
+static void ibmvscsis_alloc_common_locks(struct scsi_info *vscsi)
+{
+ spin_lock_init(&vscsi->intr_lock);
+}
+
+static void ibmvscsis_free_common_locks(struct scsi_info *vscsi)
+{
+ /* Nothing to do here */
+}
No need for these wrapers.
Removed.
+static irqreturn_t ibmvscsis_interrupt(int dummy, void *data)
+{
+ struct scsi_info *vscsi = data;
+
+ vio_disable_interrupts(vscsi->dma_dev);
+ tasklet_schedule(&vscsi->work_task);
+
+ return IRQ_HANDLED;
+}
Can you explain the need for the tasklet? There shouldn't be a whole
lot of working before passing the command off to the workqueue anyway.
There are some requests, like SRP Login and the various MADs, which can
be handled at the interrupt level, and so we do so.
--
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
