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=-2.7 required=3.0 tests=BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, URIBL_BLOCKED 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 BA233C433E6 for ; Fri, 12 Mar 2021 17:00:23 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by mail.kernel.org (Postfix) with ESMTP id 8FF7065020 for ; Fri, 12 Mar 2021 17:00:23 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S232458AbhCLQ7v (ORCPT ); Fri, 12 Mar 2021 11:59:51 -0500 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:35258 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S232398AbhCLQ7t (ORCPT ); Fri, 12 Mar 2021 11:59:49 -0500 Received: from mail-il1-x12b.google.com (mail-il1-x12b.google.com [IPv6:2607:f8b0:4864:20::12b]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id C4C5AC061574; Fri, 12 Mar 2021 08:59:49 -0800 (PST) Received: by mail-il1-x12b.google.com with SMTP id c10so3177269ilo.8; Fri, 12 Mar 2021 08:59:49 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=mime-version:references:in-reply-to:from:date:message-id:subject:to :cc; bh=8lyG9mI2+ZJ9STRsO49iQYUUEtp1pPpZxebMdUf5OrU=; b=lOa/pXAUadrrdSUfTfsnb3u9ozA2I3n4dpJYV1nAZQ3IftoPv6nv0MKopf8+HxrPsJ DLugt+E+IRC3GDqzvl6dAtuBVuzGeR2reElxkayv+VhzxT1Eya4mvuxiYjyg6XVbT6iQ 1Wwh7YoXz8Rrj7S4jKcthXJ3/qBE+y3kZbccxBs9Ef4XMoILwwvfUSxHi4G+bBspAgXs KYSjQpgYS+l0aseN1WIMVYMChTtZu44Sj/v7IGNhJTtPzLMElIN4V1T9xnXOqr8oT2j4 DDBK2GTkN9R4dd1TLAhuhSylTzjop8abB1LeIHc+C8EITCLC/A5hVraS02E4I9gDGb7g +hww== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:references:in-reply-to:from:date :message-id:subject:to:cc; bh=8lyG9mI2+ZJ9STRsO49iQYUUEtp1pPpZxebMdUf5OrU=; b=QSlgiCkBzddeOUH3dXbB8khWS9RNs2vQ9M3aW5L9/WYtNfrlLjFnE4nf1tUeM2Q28g kGCLTc0V0+gegsiid/Hmid5y96zywyO0VGLy8rmGBq5XlBTY5emMDqw9tvWWCgceFhx8 WYYSjZG/8bJeE3N+9aWBJOOZiNDWQyBdcq/LRtF+eeWVnRwf4hsDiA3m87FD5l9IsA7h jSytbKHFpp1KrCwM7zzvRw5x8I4HkN23NO9jEEsgUxriz+0CWJgN1MaPwfwDau7XQouo 5Y9Ek++N8FpRe1wRNm3Dt70KXaT9TKj8uPqklG21J8gLQ5VX4IF4LRfzEin/6KpG+JVb BwhQ== X-Gm-Message-State: AOAM5321/bfXJmqj1NWmMzj+fWFu6szHRNBshBckk69ojsC5l53bI6Bd e3gutdqvGjl2LZERO0/MTuobN+ZtQIwP8kJFfCA= X-Google-Smtp-Source: ABdhPJyxhAZqqHgpN7WIcSRLeNGYeqqplMBUXvARRawTxYw/AyadyFWdybe5WkPRPdMigwFmCN6SbgSFFU26kRoUOeI= X-Received: by 2002:a92:d18c:: with SMTP id z12mr3132686ilz.95.1615568389141; Fri, 12 Mar 2021 08:59:49 -0800 (PST) MIME-Version: 1.0 References: <20210311181729.GA2148230@bjorn-Precision-5520> <20210311201929.GN2356281@nvidia.com> <20210311232059.GR2356281@nvidia.com> In-Reply-To: From: Alexander Duyck Date: Fri, 12 Mar 2021 08:59:38 -0800 Message-ID: Subject: Re: [PATCH mlx5-next v7 0/4] Dynamically assign MSI-X vectors count To: Leon Romanovsky Cc: Jason Gunthorpe , Bjorn Helgaas , Bjorn Helgaas , Saeed Mahameed , Jakub Kicinski , linux-pci , linux-rdma@vger.kernel.org, Netdev , Don Dutile , Alex Williamson , "David S . Miller" , Greg Kroah-Hartman Content-Type: text/plain; charset="UTF-8" Precedence: bulk List-ID: X-Mailing-List: linux-pci@vger.kernel.org On Thu, Mar 11, 2021 at 10:32 PM Leon Romanovsky wrote: > > On Thu, Mar 11, 2021 at 06:53:16PM -0800, Alexander Duyck wrote: > > On Thu, Mar 11, 2021 at 3:21 PM Jason Gunthorpe wrote: > > > > > > On Thu, Mar 11, 2021 at 01:49:24PM -0800, Alexander Duyck wrote: > > > > > We don't need to invent new locks and new complexity for something > > > > > that is trivially solved already. > > > > > > > > I am not wanting a new lock. What I am wanting is a way to mark the VF > > > > as being stale/offline while we are performing the update. With that > > > > we would be able to apply similar logic to any changes in the future. > > > > > > I think we should hold off doing this until someone comes up with HW > > > that needs it. The response time here is microseconds, it is not worth > > > any complexity > > <...> > > > Another way to think of this is that we are essentially pulling a > > device back after we have already allocated the VFs and we are > > reconfiguring it before pushing it back out for usage. Having a flag > > that we could set on the VF device to say it is "under > > construction"/modification/"not ready for use" would be quite useful I > > would think. > > It is not simple flag change, but change of PCI state machine, which is > far more complex than holding two locks or call to sysfs_create_file in > the loop that made Bjorn nervous. > > I want to remind again that the suggestion here has nothing to do with > the real use case of SR-IOV capable devices in the Linux. > > The flow is: > 1. Disable SR-IOV driver autoprobe > 2. Create as much as possible VFs > 3. Wait for request from the user to get VM > 4. Change MSI-X table according to requested in item #3 > 5. Bind ready to go VF to VM > 6. Inform user about VM readiness > > The destroy flow includes VM destroy and unbind. > > Let's focus on solutions for real problems instead of trying to solve theoretical > cases that are not going to be tested and deployed. > > Thanks So part of the problem with this all along has been that you are only focused on how you are going to use this and don't think about how somebody else might need to use or implement it. In addition there are a number of half measures even within your own flow. In reality if we are thinking we are going to have to reconfigure every device it might make sense to simply block the driver from being able to load until you have configured it. Then the SR-IOV autoprobe would be redundant since you could use something like the "offline" flag to avoid that. If you are okay with step 1 where you are setting a flag to prevent driver auto probing why is it so much more overhead to set a bit blocking drivers from loading entirely while you are changing the config space? Sitting on two locks and assuming a synchronous operation is assuming a lot about the hardware and how this is going to be used. In addition it seems like the logic is that step 4 will always succeed. What happens if for example you send the message to the firmware and you don't get a response? Do you just say the request failed let the VF be used anyway? This is another reason why I would be much more comfortable with the option to offline the device and then tinker with it rather than hope that your operation can somehow do everything in one shot. In my mind step 4 really should be 4 steps. 1. Offline VF to reserve it for modification 2. Submit request for modification 3. Verify modification has occurred, reset if needed. 4. Online VF Doing it in that order allows for handling many more scenarios including those where perhaps step 2 actually consists of several changes to support any future extensions that are needed. Splitting step 2 and 3 allows for an asynchronous event where you can wait if firmware takes an excessively long time, or if step 2 somehow fails you can then repeat or revert it to get back to a consistent state. Lastly by splitting out the onlining step you can avoid potentially releasing a broken VF to be reserved if there is some sort of unrecoverable error between steps 2 and 3.