From patchwork Wed Oct 4 09:50:07 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Sebastian Andrzej Siewior X-Patchwork-Id: 821226 Return-Path: X-Original-To: incoming@patchwork.ozlabs.org Delivered-To: patchwork-incoming@bilbo.ozlabs.org Authentication-Results: ozlabs.org; spf=none (mailfrom) smtp.mailfrom=vger.kernel.org (client-ip=209.132.180.67; helo=vger.kernel.org; envelope-from=linux-pci-owner@vger.kernel.org; receiver=) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by ozlabs.org (Postfix) with ESMTP id 3y6WNZ0fZ9z9sxR for ; Wed, 4 Oct 2017 20:50:22 +1100 (AEDT) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751208AbdJDJuU (ORCPT ); Wed, 4 Oct 2017 05:50:20 -0400 Received: from Galois.linutronix.de ([146.0.238.70]:56487 "EHLO Galois.linutronix.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751198AbdJDJuT (ORCPT ); Wed, 4 Oct 2017 05:50:19 -0400 Received: from localhost ([127.0.0.1] helo=bazinga.breakpoint.cc) by Galois.linutronix.de with esmtp (Exim 4.80) (envelope-from ) id 1dzgJh-0007gl-8X; Wed, 04 Oct 2017 11:50:13 +0200 From: Sebastian Andrzej Siewior To: linux-pci@vger.kernel.org Cc: tglx@linutronix.de, Sebastian Andrzej Siewior , Kurt Schwemmer , Stephen Bates , Logan Gunthorpe Subject: [PATCH] pci/switchtec: Don't use completion's wait queue Date: Wed, 4 Oct 2017 11:50:07 +0200 Message-Id: <20171004095007.19148-1-bigeasy@linutronix.de> X-Mailer: git-send-email 2.14.2 MIME-Version: 1.0 Sender: linux-pci-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org The poll callback is using completion's wait_queue_head_t member and puts it in poll_wait() so the poll() caller gets a wakeup after command completed. This does not work on RT because we don't have a wait_queue_head_t in our completion implementation. Nobody in tree does like that in tree so this is the only driver that breaks. From reading the code, the completion is used to signal to the waiter that a command completed. I tried to wait_queue_head_t cmd_comp instead using the completion for that. There is a cmd_cnt which is incremented after each command completed and the user "copies" the value once the read was successful. I don't have the HW so I have no idea if it works as expected, so please test it. It looks like both wait queues (event_wq and cmd_comp) could be merged into one. The events can be distinguished and the "command complete" should be the event with the higher frequency. event_wq is only used on poll. Cc: Kurt Schwemmer Cc: Stephen Bates Cc: Logan Gunthorpe Signed-off-by: Sebastian Andrzej Siewior --- drivers/pci/switch/switchtec.c | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/drivers/pci/switch/switchtec.c b/drivers/pci/switch/switchtec.c index af81b2dec42e..879330eb77bd 100644 --- a/drivers/pci/switch/switchtec.c +++ b/drivers/pci/switch/switchtec.c @@ -286,7 +286,9 @@ struct switchtec_dev { bool alive; wait_queue_head_t event_wq; + wait_queue_head_t cmd_comp; atomic_t event_cnt; + atomic_t cmd_cnt; }; static struct switchtec_dev *to_stdev(struct device *dev) @@ -306,7 +308,6 @@ struct switchtec_user { enum mrpc_state state; - struct completion comp; struct kref kref; struct list_head list; @@ -317,6 +318,7 @@ struct switchtec_user { size_t read_len; unsigned char data[SWITCHTEC_MRPC_PAYLOAD_SIZE]; int event_cnt; + int cmd_cnt; }; static struct switchtec_user *stuser_create(struct switchtec_dev *stdev) @@ -331,8 +333,8 @@ static struct switchtec_user *stuser_create(struct switchtec_dev *stdev) stuser->stdev = stdev; kref_init(&stuser->kref); INIT_LIST_HEAD(&stuser->list); - init_completion(&stuser->comp); stuser->event_cnt = atomic_read(&stdev->event_cnt); + stuser->cmd_cnt = atomic_read(&stdev->cmd_cnt); dev_dbg(&stdev->dev, "%s: %p\n", __func__, stuser); @@ -414,7 +416,6 @@ static int mrpc_queue_cmd(struct switchtec_user *stuser) kref_get(&stuser->kref); stuser->read_len = sizeof(stuser->data); stuser_set_state(stuser, MRPC_QUEUED); - init_completion(&stuser->comp); list_add_tail(&stuser->list, &stdev->mrpc_queue); mrpc_cmd_submit(stdev); @@ -451,7 +452,8 @@ static void mrpc_complete_cmd(struct switchtec_dev *stdev) stuser->read_len); out: - complete_all(&stuser->comp); + atomic_inc(&stdev->cmd_cnt); + wake_up_interruptible(&stdev->cmd_comp); list_del_init(&stuser->list); stuser_put(stuser); stdev->mrpc_busy = 0; @@ -721,10 +723,11 @@ static ssize_t switchtec_dev_read(struct file *filp, char __user *data, mutex_unlock(&stdev->mrpc_mutex); if (filp->f_flags & O_NONBLOCK) { - if (!try_wait_for_completion(&stuser->comp)) + if (stuser->cmd_cnt == atomic_read(&stdev->cmd_cnt)) return -EAGAIN; } else { - rc = wait_for_completion_interruptible(&stuser->comp); + rc = wait_event_interruptible(stdev->cmd_comp, + stuser->cmd_cnt != atomic_read(&stdev->cmd_cnt)); if (rc < 0) return rc; } @@ -752,7 +755,7 @@ static ssize_t switchtec_dev_read(struct file *filp, char __user *data, rc = -EFAULT; goto out; } - + stuser->cmd_cnt = atomic_read(&stdev->cmd_cnt); stuser_set_state(stuser, MRPC_IDLE); out: @@ -772,7 +775,7 @@ static unsigned int switchtec_dev_poll(struct file *filp, poll_table *wait) struct switchtec_dev *stdev = stuser->stdev; int ret = 0; - poll_wait(filp, &stuser->comp.wait, wait); + poll_wait(filp, &stdev->cmd_comp, wait); poll_wait(filp, &stdev->event_wq, wait); if (lock_mutex_and_test_alive(stdev)) @@ -780,7 +783,7 @@ static unsigned int switchtec_dev_poll(struct file *filp, poll_table *wait) mutex_unlock(&stdev->mrpc_mutex); - if (try_wait_for_completion(&stuser->comp)) + if (stuser->cmd_cnt != atomic_read(&stdev->cmd_cnt)) ret |= POLLIN | POLLRDNORM; if (stuser->event_cnt != atomic_read(&stdev->event_cnt)) @@ -1255,7 +1258,6 @@ static void stdev_kill(struct switchtec_dev *stdev) /* Wake up and kill any users waiting on an MRPC request */ list_for_each_entry_safe(stuser, tmpuser, &stdev->mrpc_queue, list) { - complete_all(&stuser->comp); list_del_init(&stuser->list); stuser_put(stuser); } @@ -1264,6 +1266,7 @@ static void stdev_kill(struct switchtec_dev *stdev) /* Wake up any users waiting on event_wq */ wake_up_interruptible(&stdev->event_wq); + wake_up_interruptible(&stdev->cmd_comp); } static struct switchtec_dev *stdev_create(struct pci_dev *pdev) @@ -1287,7 +1290,9 @@ static struct switchtec_dev *stdev_create(struct pci_dev *pdev) INIT_WORK(&stdev->mrpc_work, mrpc_event_work); INIT_DELAYED_WORK(&stdev->mrpc_timeout, mrpc_timeout_work); init_waitqueue_head(&stdev->event_wq); + init_waitqueue_head(&stdev->cmd_comp); atomic_set(&stdev->event_cnt, 0); + atomic_set(&stdev->cmd_cnt, 0); dev = &stdev->dev; device_initialize(dev);