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.