linux-kernel - Re: [PATCH 3/6] bitops: bitmap helper to set variable length values

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: <20220721170539.57z5kcxtgxuh3qma@basti-XPS-13-9310>
Date:   Thu, 21 Jul 2022 19:05:39 +0200
From:   Sebastian Fricke <sebastian.fricke@...labora.com>
To:     Andy Shevchenko <andy.shevchenko@...il.com>
Cc:     Yury Norov <yury.norov@...il.com>,
        Linux Media Mailing List <linux-media@...r.kernel.org>,
        Jernej Skrabec <jernej.skrabec@...il.com>, knaerzche@...il.com,
        Collabora Kernel ML <kernel@...labora.com>,
        bob.beckett@...labora.com,
        Ezequiel Garcia <ezequiel@...guardiasur.com.ar>,
        Mauro Carvalho Chehab <mchehab@...nel.org>,
        Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        "open list:ARM/Rockchip SoC..." <linux-rockchip@...ts.infradead.org>,
        linux-staging@...ts.linux.dev, nicolas.dufresne@...labora.com,
        andrzej.p@...labora.com,
        Rasmus Villemoes <linux@...musvillemoes.dk>
Subject: Re: [PATCH 3/6] bitops: bitmap helper to set variable length values

Hey Yury and Andy,

thanks for you review and discussion, after some further communication
with Nicolas Dufresne and Andrzej Pietrasiewicz, we came to the
conclusion that I will prepare a more general bit-writer API, which fits
better to the use-case we have on a lot of multimedia hardware.

I'll probably reuse the bitmap API as the backbone of that
implementation and use the `*_set8` to write the content back to memory,
but as we have a few more potential users for such an API, I'll strive
towards creating a new kernel API instead of extending an existing API
that isn't designed for our use-case.

Thanks a lot for your input and ideas.

Greetings,
Sebastian

On 14.07.2022 14:24, Andy Shevchenko wrote:
>On Wed, Jul 13, 2022 at 01:42:17PM -0700, Yury Norov wrote:
>> On Wed, Jul 13, 2022 at 10:14:24PM +0200, Andy Shevchenko wrote:
>> > On Wed, Jul 13, 2022 at 9:44 PM Yury Norov <yury.norov@...il.com> wrote:
>> > > On Wed, Jul 13, 2022 at 09:10:33PM +0200, Andy Shevchenko wrote:
>> > > > On Wed, Jul 13, 2022 at 8:56 PM Yury Norov <yury.norov@...il.com> wrote:
>> > > > > On Wed, Jul 13, 2022 at 06:31:59PM +0200, Sebastian Fricke wrote:
>
>...
>
>> > > > > I'd suggest you to try implementing
>> > > > >         bitmap_copy_from(dst, src, dst_off, len)
>> > > > > or even
>> > > > >         bitmap_copy_from(dst, dst_off, src, src_off, len)
>> > > > > if you expect that you'll need more flexibility in the future.
>> > > >
>> > > > Do you think it would be useful?
>> > > >
>> > > > We have bitmap_replace() & bitmap_remap(). Wouldn't that be enough?
>> > >
>> > > bitmap_replace and bitmap_remap have no an 'offset' parameter.
>> >
>> > True.
>> >
>> > But then it's a bit too generic to have this src_off, no?
>>
>> That's why I said:
>>
>> > > > > if you expect that you'll need more flexibility in the future.
>>
>> My preferred option is bitmap_copy_from(dst, src, dst_off, len).
>>
>> > I would rather expect for asymmetrical bitmaps that the other side
>> > will be either one of the fixed width types (it makes sense to have
>> > for 32- or 64-bit arguments.
>>
>> Look at patch #6 - it copies 1,4,5,9,10,32,37... - pretty much a random
>> number number of bits.
>
>It's too poor randomness, as u64 covers all what in patch 6.
>
>> > When you have a source bitmap of x bits and  you would like to copy it
>> > into a y-bit one, I would think that either you have a small amount of
>> > bits in x anyway, or x is a full-sized bitmap (same order as y).
>>
>> It sounds like a speculation to me. Why shouldn't we let people to
>> copy with an offset any number of bits?
>
>Because it's a common case. You have a value in the register / variable, which
>naturally is one of the POD types. Now you want to inject this into bitmap at
>the arbitrary offset. Value itself also needs to be variadic size in bits.
>
>Basically what he is trying to achieve is something like bitfield.h API over
>bitmaps. Dunno, if actually bitfield.h in the certain driver wouldn't be
>enough.
>
>> > Also
>> > keep in mind that granularity is long, so less than long it makes no
>> > sense.
>> >
>> >   bitmap_copy_from_T(unsigned long *map, start, len, T src),
>> >
>> > where T is type, start is the offset in map, len is the amount of bits
>> > from src starting from 0. That's what is required in most of the cases
>> > I believe.
>>
>> But not in Sebastian's case, according to patch #6.
>
>I think it's a case, see above.
>
>-- 
>With Best Regards,
>Andy Shevchenko
>
>