| Message ID | 20231220015106.16732-3-warthog618@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | gpiolib: cdev: guard tidying | expand |
On Wed, Dec 20, 2023 at 09:51:04AM +0800, Kent Gibson wrote: > The size of struct linereq may exceed a page, so allocate space for > it using kvzalloc() instead of kzalloc(). It might be this needs a bit of elaboration. The kmalloc() tries to allocate a contiguous (in physical address space) chunk of memory and with fragmented memory it might be not possible. So the above issue might (rarely) happen. In most cases the call to kmalloc() will succeed.
On Wed, Dec 20, 2023 at 04:30:24PM +0200, Andy Shevchenko wrote: > On Wed, Dec 20, 2023 at 09:51:04AM +0800, Kent Gibson wrote: > > The size of struct linereq may exceed a page, so allocate space for > > it using kvzalloc() instead of kzalloc(). > > It might be this needs a bit of elaboration. The kmalloc() tries to allocate > a contiguous (in physical address space) chunk of memory and with fragmented > memory it might be not possible. So the above issue might (rarely) happen. > In most cases the call to kmalloc() will succeed. > For sure, the kzalloc() generally works - or we wouldn't've gotten this far as tests with MAX_LINES would've been failing. We are targetting a very niche failure mode here. The size allocated can only be determined at runtime, may be more or less than a page, and we don't care whether the physical memory allocated is contiguous. As such kvzalloc() is the appropriate allocator. Are you suggesting repeating the relevant sections of the kmalloc/vmalloc() documentation or Memory Allocation Guide as part of the checkin comment? Cheers, Kent.
On Wed, Dec 20, 2023 at 10:53:07PM +0800, Kent Gibson wrote: > On Wed, Dec 20, 2023 at 04:30:24PM +0200, Andy Shevchenko wrote: > > On Wed, Dec 20, 2023 at 09:51:04AM +0800, Kent Gibson wrote: > > > The size of struct linereq may exceed a page, so allocate space for > > > it using kvzalloc() instead of kzalloc(). > > > > It might be this needs a bit of elaboration. The kmalloc() tries to allocate > > a contiguous (in physical address space) chunk of memory and with fragmented > > memory it might be not possible. So the above issue might (rarely) happen. > > In most cases the call to kmalloc() will succeed. > > For sure, the kzalloc() generally works - or we wouldn't've gotten this > far as tests with MAX_LINES would've been failing. > We are targetting a very niche failure mode here. > > The size allocated can only be determined at runtime, may be more or > less than a page, and we don't care whether the physical memory allocated > is contiguous. > As such kvzalloc() is the appropriate allocator. > > Are you suggesting repeating the relevant sections of the > kmalloc/vmalloc() documentation or Memory Allocation Guide as part of the > checkin comment? I suggesting to make clear in the commit message that: - there is no bug per se with the code logic (i.o.w. there is no issue to have allocations bigger than one page) - this is very rarely case when it might be a problem You can also put a reference to the documentation, if you wish. This should be harmless and adding not more than a line into the commit message (or even as a Link: tag to the HTML variant of it).
diff --git a/drivers/gpio/gpiolib-cdev.c b/drivers/gpio/gpiolib-cdev.c index 44d864f63130..6fec793f5513 100644 --- a/drivers/gpio/gpiolib-cdev.c +++ b/drivers/gpio/gpiolib-cdev.c @@ -1723,7 +1723,7 @@ static void linereq_free(struct linereq *lr) kfifo_free(&lr->events); kfree(lr->label); gpio_device_put(lr->gdev); - kfree(lr); + kvfree(lr); } static int linereq_release(struct inode *inode, struct file *file) @@ -1788,7 +1788,7 @@ static int linereq_create(struct gpio_device *gdev, void __user *ip) if (ret) return ret; - lr = kzalloc(struct_size(lr, lines, ulr.num_lines), GFP_KERNEL); + lr = kvzalloc(struct_size(lr, lines, ulr.num_lines), GFP_KERNEL); if (!lr) return -ENOMEM; lr->num_lines = ulr.num_lines;
The size of struct linereq may exceed a page, so allocate space for it using kvzalloc() instead of kzalloc(). Signed-off-by: Kent Gibson <warthog618@gmail.com> --- drivers/gpio/gpiolib-cdev.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)