From patchwork Fri Jun 16 19:56:15 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jens Axboe X-Patchwork-Id: 9793691 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 0BDDF60231 for ; Fri, 16 Jun 2017 19:56:21 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id F1AE728649 for ; Fri, 16 Jun 2017 19:56:20 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id E4C722865F; Fri, 16 Jun 2017 19:56:20 +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 4844D28649 for ; Fri, 16 Jun 2017 19:56:20 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1750854AbdFPT4T (ORCPT ); Fri, 16 Jun 2017 15:56:19 -0400 Received: from mail-it0-f46.google.com ([209.85.214.46]:36687 "EHLO mail-it0-f46.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750792AbdFPT4S (ORCPT ); Fri, 16 Jun 2017 15:56:18 -0400 Received: by mail-it0-f46.google.com with SMTP id m47so40401870iti.1 for ; Fri, 16 Jun 2017 12:56:18 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=kernel-dk.20150623.gappssmtp.com; s=20150623; h=subject:from:to:cc:references:message-id:date:user-agent :mime-version:in-reply-to:content-language:content-transfer-encoding; bh=le6TVzwoxrvUd90x+/KWh3+laVbL9fWOtjTGkvpjkmw=; b=V+8NzKCCXY5Wz+0usPN6P2lx/Dgorl79sV7yiha0Pa7yx0O2i6qFXa1TKOEfbXYXbx zgTLmkH0J8s0vivqag+9/cl1B9cuLPi+J1oGDwYoceuWG142azOBGIAweJj8ynbBa3Gh EhCOMl19empNOq4X+Pk7bboavwsn/oYK4YR06LVryCyAD3H+DN0CCGQdqKAiRBW/5Dyb dU9F937t96LS6nlYUAQVNSEUnciMZeo1sO05ba7XgJb4TnRdnpQ6o2jrY2nBWLl1uoRd 5lkbwASmXCAWGHxYskelT5sW3EOPCEYZuFZUiXlA2XSkIxidlx73aVlutF0ONZ8cmkbx D9fg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:subject:from:to:cc:references:message-id:date :user-agent:mime-version:in-reply-to:content-language :content-transfer-encoding; bh=le6TVzwoxrvUd90x+/KWh3+laVbL9fWOtjTGkvpjkmw=; b=aVvz1Zxj6MPpIPIUUDWyKkrCSMvJXpyXH65Fy3oVIj/Vysz2KCTH0q09ktfkXP796G nmFRthj53zNQQCxpiaFJ9fYbN1wtPk8ITTTdSW4ShTq8jMX6UI+iFQ+KQfh4W4eyEaD3 EAISeC9fVSh6zThXVc49cZbeEpOyCEK/bk+yyHkaWA89nt7/nCCPGfqOt+WhKKeoNIxa UjbusFjjkbPVrb6dbuX6F8NnYmv0ZE1EjAHH8g3dXfdNytDYEKGPXFrfo3XrAJ9/6gJj xefGX9hFF4+nruVxqdFssPsMfdJ6lXVZB26kNEd7NObtKS8CVKDS/n8BVEpx8sRqQF2e iWMQ== X-Gm-Message-State: AKS2vOy6WDgEF3Ec6UNTmvc9zTs67t0/RZiBr68R+5nCuySFvjfZdNK6 7RIUKeCv1YieySAd X-Received: by 10.36.22.69 with SMTP id a66mr12406041ita.12.1497642977442; Fri, 16 Jun 2017 12:56:17 -0700 (PDT) Received: from [192.168.1.154] ([216.160.245.98]) by smtp.gmail.com with ESMTPSA id u191sm2323847ita.22.2017.06.16.12.56.16 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Fri, 16 Jun 2017 12:56:16 -0700 (PDT) Subject: Re: [PATCH 11/11] nvme: add support for streams and directives From: Jens Axboe To: Christoph Hellwig Cc: linux-fsdevel@vger.kernel.org, linux-block@vger.kernel.org, adilger@dilger.ca, martin.petersen@oracle.com References: <1497633883-21230-1-git-send-email-axboe@kernel.dk> <1497633883-21230-12-git-send-email-axboe@kernel.dk> <20170616180942.GD27996@infradead.org> <9811b106-a954-2481-8a66-afb25df76812@kernel.dk> Message-ID: <7999c62e-6ae2-a9ec-0734-9d35720eaac2@kernel.dk> Date: Fri, 16 Jun 2017 13:56:15 -0600 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:52.0) Gecko/20100101 Thunderbird/52.1.1 MIME-Version: 1.0 In-Reply-To: <9811b106-a954-2481-8a66-afb25df76812@kernel.dk> Content-Language: en-US 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 On 06/16/2017 01:41 PM, Jens Axboe wrote: >> .. and instead pass control and dsmgmt to nvme_get_write_stream by >> reference to isolate the functionality there. And move the >> nvme_configure_streams call into it as well. > > OK, I can make those two changes, fine with me. See below, don't want to spam with a new version. Outside of this, are we in agreeing on that it looks OK now from an interface point of view? Do we want the enum rw_hint more polished and split up? Right now it's carried through untouched. I'd rather put that in the flags in the bio/request for two reasons: 1) We have room for the 4 bits. We can always move it to separate members if we have to for more hints. I'm always reluctant to add to either struct, as adding is much easier than removing. 2) If it's in the flags, we get it automatically all the way through the stack. If it's separate members, we have to touch a bunch of places to propagate it from bio to request, or from bio to bio when we clone and split. diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index 48a5acea5105..d5105eb9dc49 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -422,16 +422,19 @@ static int nvme_streams_deallocate(struct nvme_ns *ns) static void nvme_write_hint_work(struct work_struct *work) { - struct nvme_ns *ns = container_of(work, struct nvme_ns, write_hint_work); int ret, nr_streams; + struct nvme_ns *ns; + ns = container_of(work, struct nvme_ns, write_hint_work); if (ns->nr_streams) return; - nr_streams = streams_per_ns; - if (nr_streams > ns->ctrl->nssa) - nr_streams = ns->ctrl->nssa; - + /* + * For now, always ask for 'streams_per_ns' number of streams. It's + * possible that we will then run out of streams if we have many + * name spaces. Different policies could be implemented. + */ + nr_streams = min_t(unsigned int, streams_per_ns, ns->ctrl->nssa); ret = nvme_streams_allocate(ns, nr_streams); if (ret <= 0) goto err; @@ -457,30 +460,41 @@ static void nvme_configure_streams(struct nvme_ns *ns) schedule_work(&ns->write_hint_work); } -static enum rw_hint nvme_get_write_stream(struct nvme_ns *ns, - struct request *req) +/* + * Check if 'req' has a write hint associated with it. If it does, assign + * a valid namespace stream to the write. If we haven't setup streams yet, + * kick off configuration and ignore the hints until that has completed. + */ +static void nvme_assign_write_stream(struct nvme_ns *ns, struct request *req, + u16 *control, u32 *dsmgmt) { - enum rw_hint streamid = req->cmd_flags & REQ_WRITE_LIFE_MASK; + enum rw_hint streamid; + + streamid = (req->cmd_flags & REQ_WRITE_LIFE_MASK) + >> __REQ_WRITE_HINT_SHIFT; + if (streamid == WRITE_LIFE_NONE) + return; + + if (!ns->nr_streams) { + nvme_configure_streams(ns); + return; + } - if (req_op(req) != REQ_OP_WRITE || - !(ns->ctrl->oacs & NVME_CTRL_OACS_DIRECTIVES)) - return WRITE_LIFE_NONE; + /* for now just round-robin, do something more clever later */ + if (streamid > ns->nr_streams) + streamid = (streamid % ns->nr_streams) + 1; - streamid = req->cmd_flags & REQ_WRITE_LIFE_MASK; if (streamid < ARRAY_SIZE(req->q->write_hints)) req->q->write_hints[streamid] += blk_rq_bytes(req) >> 9; - if (streamid <= ns->nr_streams) - return streamid; - - /* for now just round-robin, do something more clever later */ - return (streamid % ns->nr_streams) + 1; + *control |= NVME_RW_DTYPE_STREAMS; + *dsmgmt |= streamid << 16; } static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req, struct nvme_command *cmnd) { - enum rw_hint stream; + struct nvme_ctrl *ctrl = ns->ctrl; u16 control = 0; u32 dsmgmt = 0; @@ -498,19 +512,9 @@ static inline void nvme_setup_rw(struct nvme_ns *ns, struct request *req, cmnd->rw.slba = cpu_to_le64(nvme_block_nr(ns, blk_rq_pos(req))); cmnd->rw.length = cpu_to_le16((blk_rq_bytes(req) >> ns->lba_shift) - 1); - /* - * If we support streams and this request is a write with a valid - * hint, then flag it as such. If we haven't allocated streams on - * this ns before, do so lazily. - */ - stream = nvme_get_write_stream(ns, req); - if (stream != WRITE_LIFE_NONE) { - if (ns->nr_streams) { - control |= NVME_RW_DTYPE_STREAMS; - dsmgmt |= (stream << 16); - } else - nvme_configure_streams(ns); - } + if (req_op(req) == REQ_OP_WRITE && + (ctrl->oacs & NVME_CTRL_OACS_DIRECTIVES)) + nvme_assign_write_stream(ns, req, &control, &dsmgmt); if (ns->ms) { switch (ns->pi_type) {