On Wed, Sep 04, 2024 at 10:39:30AM -0700, Nicolin Chen wrote:

On Wed, Sep 04, 2024 at 08:12:19PM +0300, Dan Carpenter wrote:
 

quoted

On Wed, Sep 04, 2024 at 08:17:11AM -0700, Nicolin Chen wrote:

quoted

Hi Dan,

On Wed, Sep 04, 2024 at 10:29:26AM +0300, Dan Carpenter wrote:

quoted

I was reviewing Smatch warnings:

    drivers/iommu/arm/arm-smmu-v3/tegra241-cmdqv.c:616 tegra241_cmdqv_init_vintf()
    error: Calling ida_alloc_max() with a 'max' argument which is a power of 2. -1 missing?

The problem is that we're calling ida_alloc_max() where max is always zero.

quoted

+static int tegra241_cmdqv_init_vintf(struct tegra241_cmdqv *cmdqv, u16 max_idx,
+                                  struct tegra241_vintf *vintf)
+{
+
+     u16 idx;
+     int ret;
+
+     ret = ida_alloc_max(&cmdqv->vintf_ids, max_idx, GFP_KERNEL);
+     if (ret < 0)
+             return ret;
+     idx = ret;

max_idx is always zero so idx is always zero.

There is a followup series adding support for max[1, max_vintf].
And I guess that would make Smatch happy. I'd personally prefer
keep this by ignoring the Smatch warning. But if you think the
common practice is to drop it and add back, I'd be okay with it.

I'm just reviewing static checker warnings so I don't know the back story...
How long are we going to have to wait for the follow on patchset?

There are a couple of dependencies we need to get merged first.
So, it might take a few months I think.

Perhaps I can make a small patch by changing the ida_alloc_max
in the common place here to iad_alloc_range(.., 1, max,..) in
the caller of the followup series. Then the existing caller for
vintf0 wouldn't need an ida_alloc().

Nah, forget about it.  We can wait for a few months.

regards,
dan carpenter