Date:   Tue, 7 Mar 2023 04:09:37 +0800
From:   Vernon Yang <vernon2gm@gmail.com>
To:     Linus Torvalds <torvalds@linux-foundation.org>
Cc:     tytso@mit.edu, Jason@zx2c4.com, davem@davemloft.net,
        edumazet@google.com, kuba@kernel.org, pabeni@redhat.com,
        jejb@linux.ibm.com, martin.petersen@oracle.com,
        yury.norov@gmail.com, andriy.shevchenko@linux.intel.com,
        linux@rasmusvillemoes.dk, james.smart@broadcom.com,
        dick.kennedy@broadcom.com, linux-kernel@vger.kernel.org,
        wireguard@lists.zx2c4.com, netdev@vger.kernel.org,
        linux-scsi@vger.kernel.org
Subject: Re: [PATCH 3/5] scsi: lpfc: fix lpfc_cpu_affinity_check() if no
 further cpus set
Message-ID: <ZAZIgcpEBE7HXBuy@vernon-pc>
References: <20230306160651.2016767-1-vernon2gm@gmail.com>
 <20230306160651.2016767-4-vernon2gm@gmail.com>
 <CAHk-=wj73=Os1p=W7D2va=Rd81ZKvjb35yWgXQNgn1hXNRpAbw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <CAHk-=wj73=Os1p=W7D2va=Rd81ZKvjb35yWgXQNgn1hXNRpAbw@mail.gmail.com>
Precedence: bulk

On Mon, Mar 06, 2023 at 10:48:04AM -0800, Linus Torvalds wrote:
> On Mon, Mar 6, 2023 at 8:07 AM Vernon Yang <vernon2gm@gmail.com> wrote:
> >
> > -                               if (new_cpu == nr_cpumask_bits)
> > +                               if (new_cpu >= nr_cpumask_bits)
>
> This all should use "nr_cpu_ids", not "nr_cpumask_bits".
>
> But I really suspect that it should all be rewritten to not do that
> thing over and over, but just use a helper function for it.
>
>   int lpfc_next_present_cpu(int n, int alternate)
>   {
>         n = cpumask_next(n, cpu_present_mask);
>         if (n >= nr_cpu_ids)
>                 n = alternate;
>         return n;
>   }
>
> and then you could just use
>
>         start_cpu = lpfc_next_present_cpu(new_cpu, first_cpu);

OK, thanks you very much.

I'll send a second version shortly

>
> or similar.
>
>               Linus
>
> PS. We "kind of" already have a helper function for this:
> cpumask_next_wrap(). But it's really meant for a different pattern
> entirely, so let's not confuse things.