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=-0.9 required=3.0 tests=DKIM_SIGNED,DKIM_VALID, DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE, SPF_PASS 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 CBD59C47254 for ; Mon, 4 May 2020 14:26:04 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 8DAD720752 for ; Mon, 4 May 2020 14:26:04 +0000 (UTC) Authentication-Results: mail.kernel.org; dkim=pass (2048-bit key) header.d=linaro.org header.i=@linaro.org header.b="z1iuPCF+" DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 8DAD720752 Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=linaro.org Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 278438E001E; Mon, 4 May 2020 10:26:04 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 24F828E0003; Mon, 4 May 2020 10:26:04 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 165798E001E; Mon, 4 May 2020 10:26:04 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0041.hostedemail.com [216.40.44.41]) by kanga.kvack.org (Postfix) with ESMTP id EA2388E0003 for ; Mon, 4 May 2020 10:26:03 -0400 (EDT) Received: from smtpin24.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay05.hostedemail.com (Postfix) with ESMTP id A63B8181AEF0B for ; Mon, 4 May 2020 14:26:03 +0000 (UTC) X-FDA: 76779261006.24.rose26_34dbfe3eac024 X-HE-Tag: rose26_34dbfe3eac024 X-Filterd-Recvd-Size: 5616 Received: from mail-wr1-f68.google.com (mail-wr1-f68.google.com [209.85.221.68]) by imf45.hostedemail.com (Postfix) with ESMTP for ; Mon, 4 May 2020 14:26:03 +0000 (UTC) Received: by mail-wr1-f68.google.com with SMTP id h9so10859534wrt.0 for ; Mon, 04 May 2020 07:26:02 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=linaro.org; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:in-reply-to; bh=F/z0V3mcBGUp+syXehq8FDmPP6eWJmRKpReZL7dY1ho=; b=z1iuPCF+taVGKa11c2yjWNjOpHuDo1RB1E7mbiG1WoVCEMCNh5P8g+itjpP3FUb4gs mCE66WIAwk3XxYtzkPIlMa2cgsSYqPT/sHmkzg7ZxXM+z96IIV31FcsFHFSq4qIafaMB 24lM1ATRxsNNtieYoZwGa57aXFJ3ATXi47XR5U3U8SyqHQfieuZVpCa6NuqP10uVhaer GlXWtziftzxxoED82b0x2bP5555Q3FS+QQfHvURtC/EZAjTaNVZME8nZQsu/VQSLGBQV 3URfU7Lb1juS31WTwwRasil1iZl8t0N/SPOSiqBJE6kdXsxH3add7ofJV2qxRW9qCi7T V46g== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:in-reply-to; bh=F/z0V3mcBGUp+syXehq8FDmPP6eWJmRKpReZL7dY1ho=; b=C0Ywhr7dt7+GFGY4suTFjbqviVTHHfUPMkFlOsJSOg9tqief6s5yGwfXTkOshdKkuz VHH9s+rYgpcxVqgI/Z+clwN2yxUZGVPJ3iGRgaqd6AkXwSaxG8gHj4b4W6aF2LedvwS8 uEG9OIrQrKUE9WDPXn24i7MeaHutnZdgYsrls3wPXYwMSz5pJlo7pG62f/Vj2wswe7Ry LgkRi0qsqFX8BtJctPLDtxhVo9z++LLZTmp9de37y7+Jqsnn/5jNJBsqCWwY1NkJ5lWL ++vXWq4hBN6oXJwQVquRo59qSlFC2IBn7JJLbihiU3Bu0CbhBBAAdrjvuA6MpyyOFQQb ikEA== X-Gm-Message-State: AGi0PubGo41Sxg/MRnM9Uxli52v/UM4U2JfzhzaTBv1UN+kZLROPQrn0 ypOJpY6KI3lyxaorXWf3uq3/xg== X-Google-Smtp-Source: APiQypJvR/uVsrLrYsoae/U5bNYqj69oeGktdTlceIG72vmnH3x2R6717uW94oIHTrpwNOfPUuH1bw== X-Received: by 2002:adf:e745:: with SMTP id c5mr9977426wrn.263.1588602361937; Mon, 04 May 2020 07:26:01 -0700 (PDT) Received: from myrica ([2001:171b:226e:c200:c43b:ef78:d083:b355]) by smtp.gmail.com with ESMTPSA id a205sm14484714wmh.29.2020.05.04.07.25.59 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 04 May 2020 07:26:01 -0700 (PDT) Date: Mon, 4 May 2020 16:25:48 +0200 From: Jean-Philippe Brucker To: Jacob Pan Cc: iommu@lists.linux-foundation.org, devicetree@vger.kernel.org, linux-arm-kernel@lists.infradead.org, linux-pci@vger.kernel.org, linux-mm@kvack.org, joro@8bytes.org, catalin.marinas@arm.com, will@kernel.org, robin.murphy@arm.com, kevin.tian@intel.com, baolu.lu@linux.intel.com, Jonathan.Cameron@huawei.com, christian.koenig@amd.com, felix.kuehling@amd.com, zhangfei.gao@linaro.org, jgg@ziepe.ca, xuzaibo@huawei.com, fenghua.yu@intel.com, hch@infradead.org Subject: Re: [PATCH v6 02/25] iommu/ioasid: Add ioasid references Message-ID: <20200504142548.GB170104@myrica> References: <20200430143424.2787566-1-jean-philippe@linaro.org> <20200430143424.2787566-3-jean-philippe@linaro.org> <20200430113931.0fbf7a37@jacob-builder> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200430113931.0fbf7a37@jacob-builder> X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Thu, Apr 30, 2020 at 11:39:31AM -0700, Jacob Pan wrote: > > +/** > > + * ioasid_get - obtain a reference to the IOASID > > + */ > > +void ioasid_get(ioasid_t ioasid) > why void? what if the ioasid is not valid. My intended use was for the caller to get an additional reference when they're already holding one. So this should always succeed and I'd prefer a WARN_ON if the ioasid isn't valid rather than returning an error. But if you intend to add a state to ioasids between dropping refcount and free, then a return value makes sense. Thanks, Jean > > > +{ > > + struct ioasid_data *ioasid_data; > > + > > + spin_lock(&ioasid_allocator_lock); > > + ioasid_data = xa_load(&active_allocator->xa, ioasid); > > + if (ioasid_data) > > + refcount_inc(&ioasid_data->refs); > > + spin_unlock(&ioasid_allocator_lock); > > +} > > +EXPORT_SYMBOL_GPL(ioasid_get); > > + > > /** > > * ioasid_free - Free an IOASID > > * @ioasid: the ID to remove > > + * > > + * Put a reference to the IOASID, free it when the number of > > references drops to > > + * zero. > > + * > > + * Return: %true if the IOASID was freed, %false otherwise. > > */ > > -void ioasid_free(ioasid_t ioasid) > > +bool ioasid_free(ioasid_t ioasid) > > { > > + bool free = false; > > struct ioasid_data *ioasid_data; > > > > spin_lock(&ioasid_allocator_lock); > > @@ -360,6 +383,10 @@ void ioasid_free(ioasid_t ioasid) > > goto exit_unlock; > > } > > > > + free = refcount_dec_and_test(&ioasid_data->refs); > > + if (!free) > > + goto exit_unlock; > > + > Just FYI, we may need to add states for the IOASID, i.g. mark the IOASID > inactive after free. And prohibit ioasid_get() after freed. For VT-d, > this is useful when KVM queries the IOASID. > > > active_allocator->ops->free(ioasid, > > active_allocator->ops->pdata); /* Custom allocator needs additional > > steps to free the xa element */ if (active_allocator->flags & > > IOASID_ALLOCATOR_CUSTOM) { @@ -369,6 +396,7 @@ void > > ioasid_free(ioasid_t ioasid) > > exit_unlock: > > spin_unlock(&ioasid_allocator_lock); > > + return free; > > } > > EXPORT_SYMBOL_GPL(ioasid_free); > > > > [Jacob Pan]