From patchwork Mon Dec 4 23:12:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Omar Sandoval X-Patchwork-Id: 10091739 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id 1CB016035E for ; Mon, 4 Dec 2017 23:12:42 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 0F0BD294CC for ; Mon, 4 Dec 2017 23:12:42 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id 040BE294CE; Mon, 4 Dec 2017 23:12:42 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.9 required=2.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,RCVD_IN_DNSWL_HI autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id 4FA9F294CC for ; Mon, 4 Dec 2017 23:12:41 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752179AbdLDXMf (ORCPT ); Mon, 4 Dec 2017 18:12:35 -0500 Received: from mail-pg0-f65.google.com ([74.125.83.65]:38754 "EHLO mail-pg0-f65.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1752160AbdLDXMe (ORCPT ); Mon, 4 Dec 2017 18:12:34 -0500 Received: by mail-pg0-f65.google.com with SMTP id f12so9313900pgo.5 for ; Mon, 04 Dec 2017 15:12:33 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=osandov-com.20150623.gappssmtp.com; s=20150623; h=from:to:cc:subject:date:message-id; bh=JOsnS9ofQ32gEux6O2kHmExYzjAN+RXFa5URIHE3KYc=; b=ZBJNBCFN2pN6DfTeUtdZWAqvfvLGqBH6r3iKZinV0zlEyC1q9x7kPBpMof2GdvUCS2 ssfQJnyy5Skis8ormfHYNO5dxBoHDwkURUPku4xmIOO/tChx4HepxS3V7RQXkCIVCAZ5 YzQcxypU6dOQGnW4odr/o7JZ0myYR9HGfS92faYj94sLxp4lVMD0JVFnzEUwW5WLoxz9 pj8di/FVfKJB3oiLTRJVkRPsOFAstW4TWUpOkG5UlT6yuu364wmQAFqf1FUVjPxRl2c3 oQwx51RbdqJLtnaw3ITMx2+T6SR6xXWb5+MyxiREw+IGe0e5u6q6iG9kmWnLUQIIatdX fn1Q== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id; bh=JOsnS9ofQ32gEux6O2kHmExYzjAN+RXFa5URIHE3KYc=; b=nZN4PZVOhnC96mYxAHZlFVNEQe9Fx0JgfoIAes53+y17/rRe3/N8oV7xfwh0RKstmZ QOQvUyPQ2XY+z4hP1U5dScZC/WZaqcvr3EMouW0QnEBF8SkpQmwIdnalEKHvKLirJnsc zRqRvvEDLOeKbRr/4V9yJpF7TyGsqz/UQ7j+NhJDJ23LwvZFYcH1w8H3aQazWvMMO1fi s5x4+JK+fRUYLvaddvdbt0JvRKESaPeE4OnDkPbiHkED19NGqtRd/2TAAV8KIW47mShJ IU3OexpA/oDsznxU+JP2V0Sl8Z969k/WEHi4x04PU4Pto/bR5U2F3ww8D8OtY6eZxMjS bm+w== X-Gm-Message-State: AJaThX5abjpKgXU9V83EKtoKzf0RHCeXCydGyp27h2uHePkgh1LAexWZ MLWHjLwSOBSJTqdZliRyFuDdDY3KxZM= X-Google-Smtp-Source: AGs4zMbtaD81R95YadOr3cY4KQE7vn+3wORz7B1F9H/QPYrRCxn0SqMVnWE14xbx8ZPlW85Aj0j/9w== X-Received: by 10.98.156.81 with SMTP id f78mr20778954pfe.211.1512429152766; Mon, 04 Dec 2017 15:12:32 -0800 (PST) Received: from vader.thefacebook.com ([2620:10d:c090:200::4:d21a]) by smtp.gmail.com with ESMTPSA id 76sm25913867pfn.179.2017.12.04.15.12.31 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 04 Dec 2017 15:12:32 -0800 (PST) From: Omar Sandoval To: linux-block@vger.kernel.org Cc: Jens Axboe , kernel-team@fb.com Subject: [PATCH] kyber: fix another domain token wait queue hang Date: Mon, 4 Dec 2017 15:12:15 -0800 Message-Id: X-Mailer: git-send-email 2.15.1 Sender: linux-block-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-block@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP From: Omar Sandoval Commit 8cf466602028 ("kyber: fix hang on domain token wait queue") fixed a hang caused by leaving wait entries on the domain token wait queue after the __sbitmap_queue_get() retry succeeded, making that wait entry a "dud" which won't in turn wake more entries up. However, we can also get a dud entry if kyber_get_domain_token() fails once but is then called again and succeeds. This can happen if the hardware queue is rerun for some other reason, or, more likely, kyber_dispatch_request() tries the same domain twice. The fix is to remove our entry from the wait queue whenever we successfully get a token. The only complication is that we might be on one of many wait queues in the struct sbitmap_queue, but that's easily fixed by remembering which wait queue we were put on. While we're here, only initialize the wait queue entry once instead on every wait, and use spin_lock_irq() instead of spin_lock_irqsave(), since this is always called from process context with irqs enabled. Signed-off-by: Omar Sandoval --- I have another, rarer hang I'm still working out, but I can get this one out of the way. block/kyber-iosched.c | 39 +++++++++++++++++++++++++-------------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/block/kyber-iosched.c b/block/kyber-iosched.c index b4df317c2916..00cf624ce3ed 100644 --- a/block/kyber-iosched.c +++ b/block/kyber-iosched.c @@ -100,9 +100,13 @@ struct kyber_hctx_data { unsigned int cur_domain; unsigned int batching; wait_queue_entry_t domain_wait[KYBER_NUM_DOMAINS]; + struct sbq_wait_state *domain_ws[KYBER_NUM_DOMAINS]; atomic_t wait_index[KYBER_NUM_DOMAINS]; }; +static int kyber_domain_wake(wait_queue_entry_t *wait, unsigned mode, int flags, + void *key); + static int rq_sched_domain(const struct request *rq) { unsigned int op = rq->cmd_flags; @@ -385,6 +389,9 @@ static int kyber_init_hctx(struct blk_mq_hw_ctx *hctx, unsigned int hctx_idx) for (i = 0; i < KYBER_NUM_DOMAINS; i++) { INIT_LIST_HEAD(&khd->rqs[i]); + init_waitqueue_func_entry(&khd->domain_wait[i], + kyber_domain_wake); + khd->domain_wait[i].private = hctx; INIT_LIST_HEAD(&khd->domain_wait[i].entry); atomic_set(&khd->wait_index[i], 0); } @@ -524,35 +531,39 @@ static int kyber_get_domain_token(struct kyber_queue_data *kqd, int nr; nr = __sbitmap_queue_get(domain_tokens); - if (nr >= 0) - return nr; /* * If we failed to get a domain token, make sure the hardware queue is * run when one becomes available. Note that this is serialized on * khd->lock, but we still need to be careful about the waker. */ - if (list_empty_careful(&wait->entry)) { - init_waitqueue_func_entry(wait, kyber_domain_wake); - wait->private = hctx; + if (nr < 0 && list_empty_careful(&wait->entry)) { ws = sbq_wait_ptr(domain_tokens, &khd->wait_index[sched_domain]); - add_wait_queue(&ws->wait, wait); + khd->domain_ws[sched_domain] = ws; + add_wait_queue_exclusive(&ws->wait, wait); /* * Try again in case a token was freed before we got on the wait - * queue. The waker may have already removed the entry from the - * wait queue, but list_del_init() is okay with that. + * queue. */ nr = __sbitmap_queue_get(domain_tokens); - if (nr >= 0) { - unsigned long flags; + } - spin_lock_irqsave(&ws->wait.lock, flags); - list_del_init(&wait->entry); - spin_unlock_irqrestore(&ws->wait.lock, flags); - } + /* + * If we got a token while we were on the wait queue, remove ourselves + * from the wait queue to ensure that all wake ups make forward + * progress. It's possible that the waker already deleted the entry + * between the !list_empty_careful() check and us grabbing the lock, but + * list_del_init() is okay with that. + */ + if (nr >= 0 && !list_empty_careful(&wait->entry)) { + ws = khd->domain_ws[sched_domain]; + spin_lock_irq(&ws->wait.lock); + list_del_init(&wait->entry); + spin_unlock_irq(&ws->wait.lock); } + return nr; }