linux-kernel - Re: [PATCH v1 1/4] spidev: Do not use atomic bit operations when allocating minor

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 25 Mar 2022 13:09:36 +0200
From:   Andy Shevchenko <andy.shevchenko@...il.com>
To:     Mark Brown <broonie@...nel.org>, Yury Norov <yury.norov@...il.com>
Cc:     Andy Shevchenko <andriy.shevchenko@...ux.intel.com>,
        linux-spi <linux-spi@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v1 1/4] spidev: Do not use atomic bit operations when
 allocating minor

On Thu, Mar 24, 2022 at 10:48 PM Mark Brown <broonie@...nel.org> wrote:
> On Thu, Mar 24, 2022 at 11:24:26AM +0200, Andy Shevchenko wrote:
> > On Wed, Mar 23, 2022 at 07:06:25PM +0000, Mark Brown wrote:
>
> > > Yes, it's not needed but what meaningful harm does it do?
>
> > There are basically two points:
>
> > 1) in one driver the additional lock may not be influential, but
> >    if many drivers will do the same, it will block CPUs for no
> >    purpose;
>
> > 2) derived from the above, if one copies'n'pastes the code, esp.
> >    using spin locks, it may become an unneeded code and performance
> >    degradation.
>
> I think if these are serious issues they need to be addressed in the API
> so that code doing the fancy unlocked stuff that needs atomicity is the
> code that has the __ and looks like it's doing something tricky and
> peering into internals.

I believe the issue you mainly pointed out is the __ in the name of
the APIs, since it's case by case when you need one or the other. In
case of spidev we need non-atomic versions, in case of, e.g.,
drivers/dma/dw/core.c we need atomic, because spin locks used there do
not (and IIRC may not) cover some cases where the bit operations are
used against same bitmap.

Perhaps we might add the aliases as clear_bit_nonatomic() et al. Yury,
what do you think?

-- 
With Best Regards,
Andy Shevchenko