Re: [PATCH v2 5/5] Input: evdev - Switch to bitmap_zalloc()

[Andy Shevchenko]

On Mon, Jun 18, 2018 at 6:49 PM, Joe Perches <joe@xxxxxxxxxxx> wrote:
> On Mon, 2018-06-18 at 15:02 +0300, Andy Shevchenko wrote:
>> On Sat, 2018-06-16 at 12:16 -0700, Joe Perches wrote:
>> > On Sat, 2018-06-16 at 21:45 +0300, Andy Shevchenko wrote:
>> > > On Sat, Jun 16, 2018 at 12:46 AM Yury Norov <ynorov@xxxxxxxxxxxxxxxx
>> > > om> wrote:
>> > > > On Fri, Jun 15, 2018 at 04:20:17PM +0300, Andy Shevchenko wrote:
>> > > > > Switch to bitmap_zalloc() to show clearly what we are
>> > > > > allocating.
>> > > > > Besides that it returns pointer of bitmap type instead of opaque
>> > > > > void *.
>> > >
>> > >
>> > > > > +       mem = bitmap_alloc(maxbit, GFP_KERNEL);
>> > > > >         if (!mem)
>> > > > >                 return -ENOMEM;
>> > > >
>> > > > But in commit message you say you switch to bitmap_zalloc(). IIUC
>> > > > bitmap_alloc() is OK here. But could you please update comment to
>> > > > avoid confusing.
>> > >
>> > > There are two places, one with alloc, another with zalloc.
>> > > I will clarify this in commit message of next version.
>> > >
>> > > > > +       mask = bitmap_zalloc(cnt, GFP_KERNEL);
>> > > > >         if (!mask)
>> > > > >                 return -ENOMEM;
>> > > > >
>> > > > >         error = bits_from_user(mask, cnt - 1, codes_size, codes,
>> > > > > compat);
>> > > >
>> > > > If my understanding of bits_from_user() correct, here you can also
>> > > > use
>> > > > bitmap_alloc(), true?
>> >
>> > Also it might be useful to have a separate bitmap_from_user
>> > to alloc and copy.
>>
>> Maybe. I didn't check if there are such users except this driver.
>>
>> Anyway, it's out of scope of the series.
>
> That seems incorrect as you are introducing alloc/free helpers.
>
> Perhaps bitmap_dup_user [or some better name] could or should
> be one of the helpers.

Can you help with estimation how many existing users need this kind of
functionality? One of them evdev, which has an open coded variant.

Also, pay attention to that fact we have already bitmap_parse_user()
and bitmap_parlelist_user() which are called by several users.

If the estimation will show something like 3+, it would definitely
make sense, otherwise, I wouldn't like spend time on it.

-- 
With Best Regards,
Andy Shevchenko