This series includes:
- remove checks in the code which are not required
- the re-org, which I had originally posted separately

Based on v6.0-rc1

John Garry (3):
  iova: Remove some magazine pointer NULL checks
  iova: Remove magazine BUG_ON() checks
  iova: Re-order code to remove forward declarations

 drivers/iommu/iova.c | 1074 +++++++++++++++++++++---------------------
 include/linux/iova.h |   60 +--
 2 files changed, 559 insertions(+), 575 deletions(-)

-- 
2.35.3

On 17/08/2022 11:09, John Garry wrote:
> This series includes:
> - remove checks in the code which are not required
> - the re-org, which I had originally posted separately

BTW, Can we drop the !IOMMU_IOVA stubs in iova.h? I could not find a 
kernel config which actually exercises that code (so testing changes 
there is difficult).

> 
> Based on v6.0-rc1
> 
> John Garry (3):
>    iova: Remove some magazine pointer NULL checks
>    iova: Remove magazine BUG_ON() checks
>    iova: Re-order code to remove forward declarations
> 
>   drivers/iommu/iova.c | 1074 +++++++++++++++++++++---------------------
>   include/linux/iova.h |   60 +--
>   2 files changed, 559 insertions(+), 575 deletions(-)
>

On 17/08/2022 11:24, John Garry wrote:
> On 17/08/2022 11:09, John Garry wrote:
>> This series includes:
>> - remove checks in the code which are not required
>> - the re-org, which I had originally posted separately
> 
> BTW, Can we drop the !IOMMU_IOVA stubs in iova.h? I could not find a 
> kernel config which actually exercises that code (so testing changes 
> there is difficult).

Any thoughts on this? Since I got no review of patch #3 I assume that it 
is not keenly welcome either.

Now I think that I'll just post a new version with patches #1 and #2 so 
that they don't get lost in limbo.

Thanks,
John

On 2022-09-02 13:18, John Garry wrote:
> On 17/08/2022 11:24, John Garry wrote:
>> On 17/08/2022 11:09, John Garry wrote:
>>> This series includes:
>>> - remove checks in the code which are not required
>>> - the re-org, which I had originally posted separately
>>
>> BTW, Can we drop the !IOMMU_IOVA stubs in iova.h? I could not find a 
>> kernel config which actually exercises that code (so testing changes 
>> there is difficult).
> 
> Any thoughts on this? Since I got no review of patch #3 I assume that it 
> is not keenly welcome either.

Yeah, I applied patch #3 to have a look at the result, but couldn't 
really convince myself either way - there are certainly a few functions 
in weirdly incongruous places at the moment, but afterwards we end up 
with certain other things in rather contrived order for the sake of 
avoiding declarations, so overall it just didn't feel objectively better 
to me. Plus the fact that rewriting nearly 2/3 of the file stands to 
make backporting tweaks or fixes unnecessarily painful is hard to 
overlook. Hence I guess I'm leaning towards "worth trying to see how it 
looked, but let's not".

As for the stubs, it seems that they're currently unused due to linkage 
issues with IOMMU_IOVA=m - if we want better compile-test coverage, I 
wonder if we couldn't replace the IS_ENABLED() with IS_REACHABLE() and 
restore some of the previously-conditional selects?

Robin.

On 05/09/2022 16:51, Robin Murphy wrote:
>>
>> Any thoughts on this? Since I got no review of patch #3 I assume that 
>> it is not keenly welcome either.
> 
> Yeah, I applied patch #3 to have a look at the result, but couldn't 
> really convince myself either way - there are certainly a few functions 
> in weirdly incongruous places at the moment, but afterwards we end up 
> with certain other things in rather contrived order for the sake of 
> avoiding declarations, so overall it just didn't feel objectively better 
> to me. Plus the fact that rewriting nearly 2/3 of the file stands to 
> make backporting tweaks or fixes unnecessarily painful is hard to 
> overlook. 

Yeah, that was my main concern. But if it is going to be done, then now 
is as good a time as ever...

> Hence I guess I'm leaning towards "worth trying to see how it 
> looked, but let's not".
> 

ok, fine. But I do still feel that iova.c does need tidying to some 
extent along these lines.

> As for the stubs, it seems that they're currently unused due to linkage 
> issues with IOMMU_IOVA=m - if we want better compile-test coverage, I 
> wonder if we couldn't replace the IS_ENABLED() with IS_REACHABLE() and 
> restore some of the previously-conditional selects?

Sorry, but I am not familiar - what were some examples of 
previously-conditional selects?

Thanks,
John

On 2022-09-06 12:59, John Garry wrote:
> On 05/09/2022 16:51, Robin Murphy wrote:
>>>
>>> Any thoughts on this? Since I got no review of patch #3 I assume that 
>>> it is not keenly welcome either.
>>
>> Yeah, I applied patch #3 to have a look at the result, but couldn't 
>> really convince myself either way - there are certainly a few 
>> functions in weirdly incongruous places at the moment, but afterwards 
>> we end up with certain other things in rather contrived order for the 
>> sake of avoiding declarations, so overall it just didn't feel 
>> objectively better to me. Plus the fact that rewriting nearly 2/3 of 
>> the file stands to make backporting tweaks or fixes unnecessarily 
>> painful is hard to overlook. 
> 
> Yeah, that was my main concern. But if it is going to be done, then now 
> is as good a time as ever...
> 
>> Hence I guess I'm leaning towards "worth trying to see how it looked, 
>> but let's not".
>>
> 
> ok, fine. But I do still feel that iova.c does need tidying to some 
> extent along these lines.
> 
>> As for the stubs, it seems that they're currently unused due to 
>> linkage issues with IOMMU_IOVA=m - if we want better compile-test 
>> coverage, I wonder if we couldn't replace the IS_ENABLED() with 
>> IS_REACHABLE() and restore some of the previously-conditional selects?
> 
> Sorry, but I am not familiar - what were some examples of 
> previously-conditional selects?

Commits 84db889e6d82 and c8a203647488 were the ones that most readily 
stood out from the current "select IOMMU_IOVA" lines.

Cheers,
Robin.