From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.2 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id A44F5C433F5 for ; Wed, 8 Sep 2021 06:15:36 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 816C26113C for ; Wed, 8 Sep 2021 06:15:36 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1347663AbhIHGQm (ORCPT ); Wed, 8 Sep 2021 02:16:42 -0400 Received: from verein.lst.de ([213.95.11.211]:38046 "EHLO verein.lst.de" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232146AbhIHGQm (ORCPT ); Wed, 8 Sep 2021 02:16:42 -0400 Received: by verein.lst.de (Postfix, from userid 2407) id C1A8D67373; Wed, 8 Sep 2021 08:15:30 +0200 (CEST) Date: Wed, 8 Sep 2021 08:15:30 +0200 From: Christoph Hellwig To: Kanchan Joshi Cc: Christoph Hellwig , Kanchan Joshi , Jens Axboe , Keith Busch , io-uring@vger.kernel.org, linux-nvme@lists.infradead.org, anuj20.g@samsung.com, Javier Gonzalez , hare@suse.de Subject: Re: [RFC PATCH 2/6] nvme: wire-up support for async-passthru on char-device. Message-ID: <20210908061530.GA28505@lst.de> References: <20210805125539.66958-1-joshi.k@samsung.com> <20210805125539.66958-3-joshi.k@samsung.com> <20210907074650.GB29874@lst.de> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) Precedence: bulk List-ID: X-Mailing-List: io-uring@vger.kernel.org On Tue, Sep 07, 2021 at 09:50:27PM +0530, Kanchan Joshi wrote: > > A few other notes: > > > > - I suspect the ioctl_cmd really should move into the core using_cmd > > infrastructure > > Yes, that seems possible by creating that field outside by combining > "op" and "unused" below. > +struct io_uring_cmd { > + struct file *file; > + __u16 op; > + __u16 unused; > + __u32 len; > + __u64 pdu[5]; /* 40 bytes available inline for free use */ > +}; Two different issues here: - the idea of having a two layer indirection with op and a cmd doesn't really make much sense - if we want to avoid conflicts using 32-bit probably makes sense So I'd turn op and unused into a single cmd field, use the ioctl encoding macros for it (but preferably pick different numbers than the existing ioctls). > > - that whole mix of user space interface and internal data in the > > ->pdu field is a mess. What is the problem with deferring the > > request freeing into the user context, which would clean up > > quite a bit of that, especially if io_uring_cmd grows a private > > field. > > That mix isn't great but the attempt was to save the allocation. > And I was not very sure if it'd be fine to defer freeing the request > until task-work fires up. What would be the problem with the delaying? > Even if we take that route, we would still need a place to store bio > pointer (hopefully meta pointer can be extracted out of bio). > Do you see it differently? We don't need the bio pointer at all. The old passthrough code needed it when we still used block layer bonuce buffering for it. But that bounce buffering for passthrough commands got removed a while ago, and even before nvme never used it. From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.6 required=3.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI, SPF_HELO_NONE,SPF_PASS,USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id B5344C433F5 for ; Wed, 8 Sep 2021 06:21:45 +0000 (UTC) Received: from bombadil.infradead.org (bombadil.infradead.org [198.137.202.133]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 842C1610A3 for ; Wed, 8 Sep 2021 06:21:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.4.1 mail.kernel.org 842C1610A3 Authentication-Results: mail.kernel.org; dmarc=none (p=none dis=none) header.from=lst.de Authentication-Results: mail.kernel.org; spf=none smtp.mailfrom=lists.infradead.org DKIM-Signature: v=1; a=rsa-sha256; q=dns/txt; c=relaxed/relaxed; d=lists.infradead.org; s=bombadil.20210309; h=Sender: Content-Transfer-Encoding:Content-Type:List-Subscribe:List-Help:List-Post: List-Archive:List-Unsubscribe:List-Id:In-Reply-To:MIME-Version:References: Message-ID:Subject:Cc:To:From:Date:Reply-To:Content-ID:Content-Description: Resent-Date:Resent-From:Resent-Sender:Resent-To:Resent-Cc:Resent-Message-ID: List-Owner; bh=SLhXuJgpxadg4QgKmVQrtbOfmxNNCi5PPq7ABxnK4KU=; b=E2G7w6w7Rwuann 5UPYsDpMam6+Zr2xYClvvros5+FTsNK+7fSQUVentrsqyvIrd07eelKROlzIv+VBHzYrHNG0CBaNa vrrnoTN65PQcHHqXOZ3OqHQJQL6jBkf0bXq/tnX98gPj24vaD9VLGUBCDyxOZHkctSH2qI6FGqBOz 90FF8uakjcH+APsDBi5pjy4uEm+5Yj1kIa8xBrz+uYYUC3TErEQC+MoxUVo5KA2ywtkNLRgK2eo21 sgL7tvDGEEdscJAEKrwleDFjzPlj6iYILFlgevyyJTP/a84m4qvMjr+QZUQPby/e1q4k5U+jq3xht sXVSlyD8dc+UAXEvMqeg==; Received: from localhost ([::1] helo=bombadil.infradead.org) by bombadil.infradead.org with esmtp (Exim 4.94.2 #2 (Red Hat Linux)) id 1mNqxL-005Z2n-Sr; Wed, 08 Sep 2021 06:21:12 +0000 Received: from verein.lst.de ([213.95.11.211]) by bombadil.infradead.org with esmtps (Exim 4.94.2 #2 (Red Hat Linux)) id 1mNqry-005WNb-2R for linux-nvme@lists.infradead.org; Wed, 08 Sep 2021 06:15:42 +0000 Received: by verein.lst.de (Postfix, from userid 2407) id C1A8D67373; Wed, 8 Sep 2021 08:15:30 +0200 (CEST) Date: Wed, 8 Sep 2021 08:15:30 +0200 From: Christoph Hellwig To: Kanchan Joshi Cc: Christoph Hellwig , Kanchan Joshi , Jens Axboe , Keith Busch , io-uring@vger.kernel.org, linux-nvme@lists.infradead.org, anuj20.g@samsung.com, Javier Gonzalez , hare@suse.de Subject: Re: [RFC PATCH 2/6] nvme: wire-up support for async-passthru on char-device. Message-ID: <20210908061530.GA28505@lst.de> References: <20210805125539.66958-1-joshi.k@samsung.com> <20210805125539.66958-3-joshi.k@samsung.com> <20210907074650.GB29874@lst.de> MIME-Version: 1.0 Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.5.17 (2007-11-01) X-CRM114-Version: 20100106-BlameMichelson ( TRE 0.8.0 (BSD) ) MR-646709E3 X-CRM114-CacheID: sfid-20210907_231538_340703_ED378889 X-CRM114-Status: GOOD ( 24.21 ) X-BeenThere: linux-nvme@lists.infradead.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Content-Type: text/plain; charset="us-ascii" Content-Transfer-Encoding: 7bit Sender: "Linux-nvme" Errors-To: linux-nvme-bounces+linux-nvme=archiver.kernel.org@lists.infradead.org On Tue, Sep 07, 2021 at 09:50:27PM +0530, Kanchan Joshi wrote: > > A few other notes: > > > > - I suspect the ioctl_cmd really should move into the core using_cmd > > infrastructure > > Yes, that seems possible by creating that field outside by combining > "op" and "unused" below. > +struct io_uring_cmd { > + struct file *file; > + __u16 op; > + __u16 unused; > + __u32 len; > + __u64 pdu[5]; /* 40 bytes available inline for free use */ > +}; Two different issues here: - the idea of having a two layer indirection with op and a cmd doesn't really make much sense - if we want to avoid conflicts using 32-bit probably makes sense So I'd turn op and unused into a single cmd field, use the ioctl encoding macros for it (but preferably pick different numbers than the existing ioctls). > > - that whole mix of user space interface and internal data in the > > ->pdu field is a mess. What is the problem with deferring the > > request freeing into the user context, which would clean up > > quite a bit of that, especially if io_uring_cmd grows a private > > field. > > That mix isn't great but the attempt was to save the allocation. > And I was not very sure if it'd be fine to defer freeing the request > until task-work fires up. What would be the problem with the delaying? > Even if we take that route, we would still need a place to store bio > pointer (hopefully meta pointer can be extracted out of bio). > Do you see it differently? We don't need the bio pointer at all. The old passthrough code needed it when we still used block layer bonuce buffering for it. But that bounce buffering for passthrough commands got removed a while ago, and even before nvme never used it. _______________________________________________ Linux-nvme mailing list Linux-nvme@lists.infradead.org http://lists.infradead.org/mailman/listinfo/linux-nvme