Linux-ARM-Kernel Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Eli Billauer <eli.billauer@gmail.com>
To: Mike Rapoport <rppt@kernel.org>,
	David Laight <david.laight.linux@gmail.com>
Cc: Arnd Bergmann <arnd@arndb.de>,
	Brad Warrum <bwarrum@linux.ibm.com>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Michal Simek <michal.simek@amd.com>,
	Ritu Agarwal <rituagar@linux.ibm.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	David Hildenbrand <david@kernel.org>,
	Matthew Wilcox <willy@infradead.org>,
	Vlastimil Babka <vbabka@kernel.org>,
	linux-arm-kernel@lists.infradead.org,
	linux-kernel@vger.kernel.org, linux-mm@kvack.org,
	linuxppc-dev@lists.ozlabs.org
Subject: Re: [PATCH 2/4] char: xillybus: replace __get_free_pages() with kmalloc()
Date: Tue, 1 Sep 2026 10:44:20 +0200	[thread overview]
Message-ID: <1729fb06-6beb-f8ef-3e71-47fc3f00345d@outbound.gmail.com> (raw)
In-Reply-To: <apaF1HUtJnstuUN4@kernel.org>

On 01/09/2026 9:59, Mike Rapoport wrote:
>> Would it really make sense to allocate the four buffers separately?
>> And/or use vmalloc().
> My understanding is that the buffers don't need to be physically
> contiguous and vmalloc()ing the entire fifo->mem in one go should work.

vmalloc() is an interesting point.

fifo_init(), fifo_write(), fifo_read() and fifo_mem_release() implement 
a FIFO in software that the XillyUSB driver uses internally.

The memory for this FIFO is allocated in fifo_init() by calling 
__get_free_pages() with requests for up to 64 kiB. With the maximal 
total buffer size of 256 MiB, we have a possibility of 4096 allocations 
into an array of buffers. And if __get_free_pages() fails, the size of 
each buffer is halved in the following attempt, which tries to allocate 
8192 buffers, each 32 kiB, in this example. And so on.

This mechanism with an array of buffers complicates the implementation 
of the other functions as well.

So why not replace this with a single call to vmalloc(), possibly asking 
for 256 MiB in one call? That would mean simplifying all four functions.

When I wrote this driver back in 2020, I avoided vmalloc() because Linus 
wrote "vmalloc() is NOT SOMETHING YOU SHOULD EVER USE!". (See [1]). He 
also noted that vmalloc() is a restricted resource. But that's from 
2003, so maybe things have changed since?

Questions that arise in this context:

* Does vmalloc() guarantee that non-pageable physical RAM is allocated 
when it returns?
* Can copy_to/from_user() be used with memory allocated with vmalloc().
* Is vmalloc() guaranteed to successfully allocate memory in the same 
situation that __get_free_pages() could have been used to obtain the 
same amount of memory (in smaller chunks, as with fifo_init() )? Maybe 
they allocate memory from separate memory pools?

And most important: In what way, if at all, is memory obtained with 
vmalloc() practically different from memory allocated by 
__get_free_pages(), if it's never used for DMA?

Does the API offer clear answers to these questions?

Thanks in advance,
    Eli

[1] https://lwn.net/Articles/57804/


  parent reply	other threads:[~2026-09-01  8:44 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-08-30  7:47 [PATCH 0/4] char/misc: replace page allocator calls with k[mz]alloc() Mike Rapoport (Microsoft)
2026-08-30  7:47 ` [PATCH 1/4] char: xilinx_hwicap: " Mike Rapoport (Microsoft)
2026-08-30  7:47 ` [PATCH 2/4] char: xillybus: replace __get_free_pages() with kmalloc() Mike Rapoport (Microsoft)
2026-08-31 10:13   ` Eli Billauer
2026-08-31 11:32     ` Mike Rapoport
2026-09-01  8:42       ` Eli Billauer
2026-09-01 17:24         ` Mike Rapoport
2026-08-31 11:39     ` David Laight
2026-09-01  7:59       ` Mike Rapoport
2026-09-01  8:37         ` David Laight
2026-09-01  8:44         ` Eli Billauer [this message]
2026-09-01 17:46           ` Mike Rapoport
2026-08-30  7:48 ` [PATCH 3/4] misc: ibmvmc: replace get_zeroed_page() with kzalloc() Mike Rapoport (Microsoft)
2026-08-30  7:48 ` [PATCH 4/4] platform: goldfish: pipe: replace __get_free_page() with kmalloc() Mike Rapoport (Microsoft)

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1729fb06-6beb-f8ef-3e71-47fc3f00345d@outbound.gmail.com \
    --to=eli.billauer@gmail.com \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=bwarrum@linux.ibm.com \
    --cc=david.laight.linux@gmail.com \
    --cc=david@kernel.org \
    --cc=gregkh@linuxfoundation.org \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=linuxppc-dev@lists.ozlabs.org \
    --cc=michal.simek@amd.com \
    --cc=rituagar@linux.ibm.com \
    --cc=rppt@kernel.org \
    --cc=vbabka@kernel.org \
    --cc=willy@infradead.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox