linux-kernel - Re: [PATCH 1/2] spi: axi-spi-engine: remove use of ida for sync id

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: Wed, 7 Feb 2024 08:09:46 -0600
From: David Lechner <dlechner@...libre.com>
To: Christophe JAILLET <christophe.jaillet@...adoo.fr>
Cc: broonie@...nel.org, linux-kernel@...r.kernel.org, 
	linux-spi@...r.kernel.org, michael.hennerich@...log.com, nuno.sa@...log.com
Subject: Re: [PATCH 1/2] spi: axi-spi-engine: remove use of ida for sync id

On Tue, Feb 6, 2024 at 3:50 PM Christophe JAILLET
<christophe.jaillet@...adoo.fr> wrote:
>
> Le 06/02/2024 à 21:31, David Lechner a écrit :
> > Profiling has shown that ida_alloc_range() accounts for about 10% of the
> > time spent in spi_sync() when using the AXI SPI Engine controller. This
> > call is used to create a unique id for each SPI message to match to an
> > IRQ when the message is complete.
> >
> > Since the core SPI code serializes messages in a message queue, we can
> > only have one message in flight at a time, namely host->cur_msg. This
> > means that we can use a fixed value instead of a unique id for each
> > message since there can never be more than one message pending at a
> > time.
> >
> > This patch removes the use of ida...
>
> So, maybe #include <linux/idr.h> can be removed as well?
> (untested)
>

Yes it should be removed.

>
>
> Also, even if unrelated to your changes, spi_engine_prepare_message()
> could use struct_size() in:
>
>         size = sizeof(*p->instructions) * (p_dry.length + 1);
>         p = kzalloc(sizeof(*p) + size, GFP_KERNEL);
>
> -->
>         p = kzalloc(struct_size(p, instructions, p_dry.length + 1, GFP_KERNEL);
>
> which can be a little safer and less verbose.

Thanks for the suggestion. I will consider it for a separate patch in
the future.