Re: [PATCH 2/4] char: xillybus: replace __get_free_pages() with kmalloc()

[Mike Rapoport <rppt@kernel.org>]

Hi Eli,

On Fri, Sep 04, 2026 at 05:29:16PM +0200, Eli Billauer wrote:
> On 03/09/2026 16:28, Vlastimil Babka (SUSE) wrote:
> > > Replacing it with a kmalloc() is confusing in my opinion, and requires
> > > that the reader is aware that kmalloc() falls back to __get_free_pages()
> > Why? The reader has only to know that kmalloc() will provide such a buffer
> > (up to sizes that the page allocator would) and whether it falls back to the
> > page allocator or not is an implementation detail.
> 
> When I see __get_free_pages(), I automatically assume there is some ugly
> low-level memory management going on, which is exactly what fifo_init()
> does.

However you allocate memory, there will be some ugly low-level memory
management underneath :)

> kmalloc() feels like something you use more for allocating memory for
> a struct.

For that we have k[mz]malloc_obj() now...

> There is no such rule, of course, but this is my subjective view on these
> two functions.

... and using it to allocate struct is becoming a rule pretty much.

> As I wrote earlier, this is a matter of taste. Maybe it's only me
> thinking like that.

Let's agree to disagree :)
I think it's more about using the right tool for a job.

For your usecase I really think vmalloc() should work. Presuming you can
spare 256MB for fifo buffers, the driver can only run on quite recent and
decent hardware and memory rates there are way higher than 400MB/s.

Even the slowest DDR3 is above 6GB/s, so taking a fault to sync page tables
on the first access is really a non-issue.
 
> And as I'm not the one deciding whether this patch is applied or not, it
> doesn't matter so much what I think about this matter. I've humbly voiced my
> opinion, and that's about as much as I can do.
> 
> Regards,
>    Eli

-- 
Sincerely yours,
Mike.