Re: [PATCH v3 03/15] iommu: Always register bus notifiers

From: Robin Murphy
Date: Thu Jul 07 2022 - 05:38:26 EST


On 2022-07-07 07:34, Tian, Kevin wrote:
From: Baolu Lu <baolu.lu@xxxxxxxxxxxxxxx>
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.