2023-01-13 19:18:58

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] iommu/ipmmu-vmsa: Remove ipmmu_utlb_disable()

On Fri, Jan 13, 2023 at 07:56:40PM +0100, Joerg Roedel wrote:
> From: Joerg Roedel <[email protected]>
>
> The function is unused after commit 1b932ceddd19 ("iommu:
> Remove detach_dev callbacks") and so compilation fails with
>
> drivers/iommu/ipmmu-vmsa.c:305:13: error: ‘ipmmu_utlb_disable’ defined but not used [-Werror=unused-function]
> 305 | static void ipmmu_utlb_disable(struct ipmmu_vmsa_domain *domain,
> | ^~~~~~~~~~~~~~~~~~
>
> Remove the function to fix the compile error.
>
> Fixes: 1b932ceddd19 ("iommu: Remove detach_dev callbacks")
> Signed-off-by: Joerg Roedel <[email protected]>
> ---
> drivers/iommu/ipmmu-vmsa.c | 12 ------------
> 1 file changed, 12 deletions(-)

I'm surprised the 0-day bot didn't notice?

Reviewed-by: Jason Gunthorpe <[email protected]>

Jason


2023-01-13 19:32:44

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH] iommu/ipmmu-vmsa: Remove ipmmu_utlb_disable()

On Fri, Jan 13, 2023 at 03:12:21PM -0400, Jason Gunthorpe wrote:
> I'm surprised the 0-day bot didn't notice?

Well, I think 0-day does not spend that much time on iommu patch-sets,
especially doing randconfigs or allyes/modconfigs.

In general it is a good idea to at least compile-test every file that is
changed in a patch-set before sending it out and not rely on 0-day bot
for that.

Regards,

Joerg

2023-01-13 19:51:30

by Jason Gunthorpe

[permalink] [raw]
Subject: Re: [PATCH] iommu/ipmmu-vmsa: Remove ipmmu_utlb_disable()

On Fri, Jan 13, 2023 at 08:25:17PM +0100, Joerg Roedel wrote:
> On Fri, Jan 13, 2023 at 03:12:21PM -0400, Jason Gunthorpe wrote:
> > I'm surprised the 0-day bot didn't notice?
>
> Well, I think 0-day does not spend that much time on iommu patch-sets,
> especially doing randconfigs or allyes/modconfigs.

Intel folks, can you check on this with the 0-day team? Perhaps since
the list was moved it is not properly subscribed.

> In general it is a good idea to at least compile-test every file that is
> changed in a patch-set before sending it out and not rely on 0-day bot
> for that.

Against every arch combination? This is why we have automation bots :(

Jason

2023-01-13 21:40:36

by Joerg Roedel

[permalink] [raw]
Subject: Re: [PATCH] iommu/ipmmu-vmsa: Remove ipmmu_utlb_disable()

On Fri, Jan 13, 2023 at 03:45:46PM -0400, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2023 at 08:25:17PM +0100, Joerg Roedel wrote:
> > In general it is a good idea to at least compile-test every file that is
> > changed in a patch-set before sending it out and not rely on 0-day bot
> > for that.
>
> Against every arch combination? This is why we have automation bots :(

No, not every combination. But if possible please compile-test each
changed file with a .config that pulls that source file in. Lots of
drivers can be enabled just with COMPILE_TEST on x86 or be catched with
a generic ARM/ARM64 config which enables all IOMMU drivers. PAMU is a
bit more difficult as it requires a PPC-32 bit config, but that is the
exception.

A full kernel build is usually also not necessary, often a 'make
drivers/iommu/' with a given config is enough.

That is also how I compile-test the IOMMU tree before I push changes
out. There are per-arch configurations which select all IOMMU drivers on
that arch. Only for X86 I do the full allnoconfig, defconfig,
allmodconfig and allyesconfig cycle (each for 32 and 64 bit).

That certainly does not catch everything, but a lot of compile issues can be
found that way. And for patch-sets only touching, for example, VT-d it
is still enough to only compile-test on x86. A patch-set touching that
much drivers is rather the exception.

Regards,

Joerg

2023-01-14 07:06:00

by Lu Baolu

[permalink] [raw]
Subject: Re: [PATCH] iommu/ipmmu-vmsa: Remove ipmmu_utlb_disable()

On 2023/1/14 3:45, Jason Gunthorpe wrote:
> On Fri, Jan 13, 2023 at 08:25:17PM +0100, Joerg Roedel wrote:
>> On Fri, Jan 13, 2023 at 03:12:21PM -0400, Jason Gunthorpe wrote:
>>> I'm surprised the 0-day bot didn't notice?
>>
>> Well, I think 0-day does not spend that much time on iommu patch-sets,
>> especially doing randconfigs or allyes/modconfigs.
>
> Intel folks, can you check on this with the 0-day team? Perhaps since
> the list was moved it is not properly subscribed.

I've forwarded this thread to the Intel 0-day team.

>
>> In general it is a good idea to at least compile-test every file that is
>> changed in a patch-set before sending it out and not rely on 0-day bot
>> for that.
>
> Against every arch combination? This is why we have automation bots :(
>
> Jason
>

--
Best regards,
baolu

2023-01-14 07:11:32

by Lu Baolu

[permalink] [raw]
Subject: Re: [PATCH] iommu/ipmmu-vmsa: Remove ipmmu_utlb_disable()

On 2023/1/14 5:29, Joerg Roedel wrote:
> On Fri, Jan 13, 2023 at 03:45:46PM -0400, Jason Gunthorpe wrote:
>> On Fri, Jan 13, 2023 at 08:25:17PM +0100, Joerg Roedel wrote:
>>> In general it is a good idea to at least compile-test every file that is
>>> changed in a patch-set before sending it out and not rely on 0-day bot
>>> for that.
>> Against every arch combination? This is why we have automation bots ????
> No, not every combination. But if possible please compile-test each
> changed file with a .config that pulls that source file in. Lots of
> drivers can be enabled just with COMPILE_TEST on x86 or be catched with
> a generic ARM/ARM64 config which enables all IOMMU drivers. PAMU is a
> bit more difficult as it requires a PPC-32 bit config, but that is the
> exception.
>
> A full kernel build is usually also not necessary, often a 'make
> drivers/iommu/' with a given config is enough.
>
> That is also how I compile-test the IOMMU tree before I push changes
> out. There are per-arch configurations which select all IOMMU drivers on
> that arch. Only for X86 I do the full allnoconfig, defconfig,
> allmodconfig and allyesconfig cycle (each for 32 and 64 bit).
>
> That certainly does not catch everything, but a lot of compile issues can be
> found that way. And for patch-sets only touching, for example, VT-d it
> is still enough to only compile-test on x86. A patch-set touching that
> much drivers is rather the exception.

This one as well. Thank you for your feedback on 0-day.

--
Best regards,
baolu