From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from foss.arm.com (foss.arm.com [217.140.110.172]) by smtp.subspace.kernel.org (Postfix) with ESMTP id 1F289625 for ; Thu, 7 Jul 2022 09:38:19 +0000 (UTC) Received: from usa-sjc-imap-foss1.foss.arm.com (unknown [10.121.207.14]) by usa-sjc-mx-foss1.foss.arm.com (Postfix) with ESMTP id CBFCA1063; Thu, 7 Jul 2022 02:38:19 -0700 (PDT) Received: from [10.57.85.108] (unknown [10.57.85.108]) by usa-sjc-imap-foss1.foss.arm.com (Postfix) with ESMTPSA id 507723F792; Thu, 7 Jul 2022 02:38:17 -0700 (PDT) Message-ID: Date: Thu, 7 Jul 2022 10:38:12 +0100 Precedence: bulk X-Mailing-List: iommu@lists.linux.dev List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla/5.0 (Windows NT 10.0; rv:91.0) Gecko/20100101 Thunderbird/91.11.0 Subject: Re: [PATCH v3 03/15] iommu: Always register bus notifiers Content-Language: en-GB To: "Tian, Kevin" , Baolu Lu , "joro@8bytes.org" Cc: "will@kernel.org" , "iommu@lists.linux.dev" , "linux-arm-kernel@lists.infradead.org" , "suravee.suthikulpanit@amd.com" , "vasant.hegde@amd.com" , "mjrosato@linux.ibm.com" , "gerald.schaefer@linux.ibm.com" , "schnelle@linux.ibm.com" , "linux-s390@vger.kernel.org" , "linux-kernel@vger.kernel.org" References: <8c380309f264cd0dfc73ba2ec060adc9515af2f2.1657034828.git.robin.murphy@arm.com> <1fab4c8a-7bc5-9a50-d48a-0dc590cac7a6@linux.intel.com> <3d613192-f673-852e-9c52-b8a913d25616@arm.com> <28a58a21-a866-b49c-9977-c8d05b320fbd@linux.intel.com> From: Robin Murphy In-Reply-To: Content-Type: text/plain; charset=UTF-8; format=flowed Content-Transfer-Encoding: 8bit On 2022-07-07 07:34, Tian, Kevin wrote: >> From: Baolu Lu >> Sent: Thursday, July 7, 2022 8:21 AM >> >> On 2022/7/6 21:43, Robin Murphy wrote: >>> On 2022-07-06 02:53, Baolu Lu wrote: >>>> On 2022/7/6 01:08, Robin Murphy wrote: >>>>>   /* >>>>>    * Use a function instead of an array here because the domain-type >>>>> is a >>>>>    * bit-field, so an array would waste memory. >>>>> @@ -152,6 +172,10 @@ static int __init iommu_subsys_init(void) >>>>>               (iommu_cmd_line & IOMMU_CMD_LINE_STRICT) ? >>>>>                   "(set via kernel command line)" : ""); >>>>> +    /* If the system is so broken that this fails, it will WARN >>>>> anyway */ >>>> >>>> Can you please elaborate a bit on this? iommu_bus_init() still return >>>> errors. >>> >>> Indeed, it's commenting on the fact that we don't try to clean up or >>> propagate an error value further even if it did ever manage to return >>> one. I feared that if I strip the error handling out of iommu_bus_init() >>> itself on the same reasoning, we'll just get constant patches from the >>> static checker brigade trying to add it back, so it seemed like the >>> neatest compromise to keep that decision where it's obviously in an >>> early initcall, rather than in the helper function which can be viewed >>> out of context. However, I'm happy to either expand this comment or go >>> the whole way and make iommu_bus_init() return void if you think it's >>> worthwhile. >> >> Thanks for the explanation. It would be helpful if the comment could be >> expanded. In this case, after a long time, people will not consider it >> an oversight. :-) >> > > I'd prefer to making iommu_bus_init() return void plus expanding > the comment otherwise the question arises that if the only caller > is not interested in the return value then why bother returning it > in the first place. 😊 OK, that's fair enough, will do. Thanks, Robin.