> From: ira.weiny@xxxxxxxxx <ira.weiny@xxxxxxxxx>
> Sent: Sunday, June 5, 2022 8:51 AM
> ...
> +static void retire_cur_task(struct pci_doe_mb *doe_mb) {
> + spin_lock(&doe_mb->task_lock);
> + doe_mb->cur_task = NULL;
> + spin_unlock(&doe_mb->task_lock);
> + wake_up_interruptible(&doe_mb->wq);
> +}
> +
> +static void doe_statemachine_work(struct work_struct *work) {
> + struct delayed_work *w = to_delayed_work(work);
> + struct pci_doe_mb *doe_mb = container_of(w, struct pci_doe_mb,
> + statemachine);
> + struct pci_dev *pdev = doe_mb->pdev;
> + int offset = doe_mb->cap_offset;
> + enum pci_doe_state prev_state;
> + struct pci_doe_task *task;
> + u32 val;
> + int rc;
> +
> + spin_lock(&doe_mb->task_lock);
> + task = doe_mb->cur_task;
> + spin_unlock(&doe_mb->task_lock);
I don't think it needs the lock protection here.
No matter "task" is !NULL or NULL, it is checked before it's used within this function.
> +
> + if (test_and_clear_bit(PCI_DOE_FLAG_ABORT, &doe_mb->flags)) {
> + /*
> + * Currently only used during init - care needed if
> + * pci_doe_abort() is generally exposed as it would impact
> + * queries in flight.
> + */
> + if (task)
> + pci_err(pdev, "DOE [%x] Aborting with active task!\n",
> + doe_mb->cap_offset);
> + doe_mb->state = DOE_WAIT_ABORT;
> + pci_doe_abort_start(doe_mb);
> + return;
> + }
> +
> + switch (doe_mb->state) {
> + case DOE_IDLE:
> + if (task == NULL)
> + return;
> +
> + rc = pci_doe_send_req(doe_mb, task);
> +
> + /*
> + * The specification does not provide any guidance on how
> long
> + * some other entity could keep the DOE busy, so try for 1
> + * second then fail. Busy handling is best effort only, because
> + * there is no way of avoiding racing against another user of
> + * the DOE.
> + */
> + if (rc == -EBUSY) {
> + doe_mb->busy_retries++;
> + if (doe_mb->busy_retries ==
> PCI_DOE_BUSY_MAX_RETRIES) {
> + /* Long enough, fail this request */
> + pci_warn(pdev,
> + "DOE [%x] busy for too long (> 1
> sec)\n",
> + doe_mb->cap_offset);
> + doe_mb->busy_retries = 0;
> + goto err_busy;
> + }
> + schedule_delayed_work(w, HZ /
> PCI_DOE_BUSY_MAX_RETRIES);
> + return;
> + }
> + if (rc)
> + goto err_abort;
> + doe_mb->busy_retries = 0;
> +
> + doe_mb->state = DOE_WAIT_RESP;
> + doe_mb->timeout_jiffies = jiffies + HZ;
> + /* Now poll or wait for IRQ with timeout */
> + if (doe_mb->irq >= 0)
> + schedule_delayed_work(w, PCI_DOE_TIMEOUT);
> + else
> + schedule_delayed_work(w, PCI_DOE_POLL_INTERVAL);
> + return;
> +
> + case DOE_WAIT_RESP:
> + /* Not possible to get here with NULL task */
> + pci_read_config_dword(pdev, offset + PCI_DOE_STATUS,
> &val);
> + if (FIELD_GET(PCI_DOE_STATUS_ERROR, val)) {
> + rc = -EIO;
> + goto err_abort;
> + }
> +
> + if (!FIELD_GET(PCI_DOE_STATUS_DATA_OBJECT_READY, val))
> {
> + /* If not yet at timeout reschedule otherwise abort
> */
> + if (time_after(jiffies, doe_mb->timeout_jiffies)) {
> + rc = -ETIMEDOUT;
> + goto err_abort;
> + }
> + schedule_delayed_work(w, PCI_DOE_POLL_INTERVAL);
> + return;
> + }
> +
> + rc = pci_doe_recv_resp(doe_mb, task);
> + if (rc < 0)
> + goto err_abort;
> +
> + doe_mb->state = DOE_IDLE;
> +
> + retire_cur_task(doe_mb);
> + /* Set the return value to the length of received payload */
> + signal_task_complete(task, rc);
> +
> + return;
> +
> + case DOE_WAIT_ABORT:
> + case DOE_WAIT_ABORT_ON_ERR:
> + prev_state = doe_mb->state;
> +
> + pci_read_config_dword(pdev, offset + PCI_DOE_STATUS,
> &val);
> +
> + if (!FIELD_GET(PCI_DOE_STATUS_ERROR, val) &&
> + !FIELD_GET(PCI_DOE_STATUS_BUSY, val)) {
> + doe_mb->state = DOE_IDLE;
> + /* Back to normal state - carry on */
> + retire_cur_task(doe_mb);
> + } else if (time_after(jiffies, doe_mb->timeout_jiffies)) {
> + /* Task has timed out and is dead - abort */
> + pci_err(pdev, "DOE [%x] ABORT timed out\n",
> + doe_mb->cap_offset);
> + set_bit(PCI_DOE_FLAG_DEAD, &doe_mb->flags);
> + retire_cur_task(doe_mb);
> + }
> +
> + /*
> + * For deliberately triggered abort, someone is
> + * waiting.
> + */
> + if (prev_state == DOE_WAIT_ABORT) {
> + if (task)
> + signal_task_complete(task, -EFAULT);
> + complete(&doe_mb->abort_c);
> + }
> +
> + return;
> + }
> +
> +err_abort:
> + doe_mb->state = DOE_WAIT_ABORT_ON_ERR;
> + pci_doe_abort_start(doe_mb);
> +err_busy:
> + signal_task_complete(task, rc);
> + if (doe_mb->state == DOE_IDLE)
> + retire_cur_task(doe_mb);
> +}
