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.2 required=3.0 tests=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 E8376C282DD for ; Fri, 10 Jan 2020 18:57:30 +0000 (UTC) Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.kernel.org (Postfix) with ESMTP id BC3CB2072A for ; Fri, 10 Jan 2020 18:57:30 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1728478AbgAJS53 (ORCPT ); Fri, 10 Jan 2020 13:57:29 -0500 Received: from mga02.intel.com ([134.134.136.20]:5801 "EHLO mga02.intel.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1728023AbgAJS53 (ORCPT ); Fri, 10 Jan 2020 13:57:29 -0500 X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from orsmga003.jf.intel.com ([10.7.209.27]) by orsmga101.jf.intel.com with ESMTP/TLS/DHE-RSA-AES256-GCM-SHA384; 10 Jan 2020 10:57:29 -0800 X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="5.69,418,1571727600"; d="scan'208";a="224279706" Received: from jekeller-mobl.amr.corp.intel.com (HELO [134.134.177.84]) ([134.134.177.84]) by orsmga003.jf.intel.com with ESMTP; 10 Jan 2020 10:57:29 -0800 Subject: Re: [PATCH v2 0/3] devlink region trigger support To: Jakub Kicinski Cc: Jiri Pirko , netdev@vger.kernel.org, valex@mellanox.com References: <20200109193311.1352330-1-jacob.e.keller@intel.com> <20200110094027.GL2235@nanopsycho.orion> <20200110104719.2035572d@cakuba.netronome.com> From: Jacob Keller Organization: Intel Corporation Message-ID: Date: Fri, 10 Jan 2020 10:57:29 -0800 User-Agent: Mozilla/5.0 (Windows NT 10.0; WOW64; rv:68.0) Gecko/20100101 Thunderbird/68.3.1 MIME-Version: 1.0 In-Reply-To: <20200110104719.2035572d@cakuba.netronome.com> Content-Type: text/plain; charset=utf-8 Content-Language: en-US Content-Transfer-Encoding: 7bit Sender: netdev-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: netdev@vger.kernel.org On 1/10/2020 10:47 AM, Jakub Kicinski wrote: > On Fri, 10 Jan 2020 09:54:20 -0800, Jacob Keller wrote: >> Mostly I wanted to make sure the direction was looking good earlier than >> the time frame for completing that work. > > As Jiri said, quite hard to tell without seeing the real user. > Fair. > I presume you take some lock to ensure the contents of the snapshot are > atomic? Otherwise I wonder if you wouldn't be better served by just > allowing region read to operate directly on the data rather then going > through the snapshot create -> read -> snapshot remove cycle. Not sure > Jiri would agree, it require more code. > Right. I saw the original devlink region commit messages indicated possible plans to support writing to regions. There is also the question of handling data that might want to be read sparsely, rather than reading an entire snapshot at once. Hmm. > For the patches themselves we may want to move the callbacks into some > ops structure in the region. And I don't think you need to make the > trigger command NO_LOCK. > So the reason it was made NO_LOCK right now is because the trigger ends up calling devlink_region_snapshot_id_get and devlink_region_snapshot_create which both take the devlink lock. I suppose this could be simplified by having the snapshot callback take similar parameters as the devlink_region_snapshot_create, and assume the devlink core code does such things as generating an id and adding the snapshot to the list all while holding the lock... I will sort out these questions into what I think makes sense for the use case I envision, and will submit the core devlink patches at the same time as the driver changes. Thanks, Jake