linux-kernel - Re: [PATCH 0/4] iio: introduce IIO_DECLARE_BUFFER_WITH

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] [day] [month] [year] [list]

Message-ID: <CAHp75VdcMoxoBU+fKQ5ex28N7YJNcEe96dOuq6hWFxpnn7UYyQ@mail.gmail.com>
Date: Sun, 20 Apr 2025 07:36:18 +0300
From: Andy Shevchenko <andy.shevchenko@...il.com>
To: David Lechner <dlechner@...libre.com>
Cc: Jonathan Cameron <jic23@...nel.org>, Nuno Sá <nuno.sa@...log.com>, 
	Andy Shevchenko <andy@...nel.org>, Lars-Peter Clausen <lars@...afoo.de>, 
	Michael Hennerich <Michael.Hennerich@...log.com>, linux-iio@...r.kernel.org, 
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH 0/4] iio: introduce IIO_DECLARE_BUFFER_WITH_TS

On Sat, Apr 19, 2025 at 1:59 AM David Lechner <dlechner@...libre.com> wrote:
>
> Creating a buffer of the proper size and correct alignment for use with
> iio_push_to_buffers_with_ts() is commonly used and not easy to get
> right (as seen by a number of recent fixes on the mailing list).
>
> In general, we prefer to use this pattern for creating such buffers:
>
> struct {
>     u16 data[2];
>     aligned_s64 timestamp;
> } buffer;
>
> However, there are many cases where a driver may have a large number of
> channels that can be optionally enabled or disabled in a scan or the
> driver might support a range of chips that have different numbers of
> channels or different storage sizes for the data.  In these cases, the
> timestamp may not always be at the same place relative to the data. We
> just allocate a buffer large enough for the largest possible case and
> don't care exactly where the timestamp ends up in the buffer.
>
> For these cases, we propose to introduce a new macro to make it easier
> it easier for both the authors to get it right and for readers of the
> code to not have to do all of the math to verify that it is correct.
>
> I have just included a few examples of drivers that can make use of this
> new macro, but there are dozens more.

I'm going to answer here as the summary of my view to this series and
macro after your replies.

So, first of all, the macro embeds alignment which is used only once
in practice and the alignment used in most of the cases is DMA one.
Having two alignments in a row seems a bit weird to me. Second one, if
we drop alignment, it means each of the users will need it. That
significantly increases the line size and with high probability will
require two LoCs to occupy. And third, based on the examples, the
macro doesn't help much if we don't convert drivers to properly handle
what they are using instead of plain u8 in all of the cases. Yes, it
might require quite an invasive change to a driver, but this is how I
see it should go.

That said, it feels like this series took a half road.

I leave it to Jonothan, but I don't like it to be merged in this form.

-- 
With Best Regards,
Andy Shevchenko