All of lore.kernel.org
 help / color / mirror / Atom feed
From: matteomartelli3@gmail.com
To: linux-kernel@vger.kernel.org
Cc: Peter Zijlstra <peterz@infradead.org>,
	Jonathan Cameron <jic23@kernel.org>,
	Marc Gonzalez <marc.w.gonzalez@free.fr>,
	Peter Rosin <peda@axentia.se>,
	Greg Kroah-Hartman <gregkh@linuxfoundation.org>,
	Joe Perches <joe@perches.com>,
	Rafael J. Wysocki <rafael@kernel.org>,
	linux-iio@vger.kernel.org
Subject: iio, syfs, devres: devm_kmalloc not aligned to pow2 size argument
Date: Mon, 28 Oct 2024 13:04:10 +0100	[thread overview]
Message-ID: <c486a1cf98a8b9ad093270543e8d2007@gmail.com> (raw)

Hi everyone,

I found an issue that might interest iio, sysfs and devres, about a
particular usage of devm_kmalloc() for buffers that later pass through
sysfs_emit() or sysfs_emit_at(). These sysfs helpers require the output
buffer to be PAGE_SIZE aligned since commit 2efc459d06f1 ("sysfs: Add
sysfs_emit and sysfs_emit_at to format sysfs output"). Such requirement
is satisfied when kmalloc(PAGE_SIZE, ...) is used but not when
devm_kmalloc(PAGE_SIZE,...) is used as it actually returns a pointer to
a buffer located after the devres metadata and thus aligned to
PAGE_SIZE+sizeof(struct devres).

Specifically, I came across this issue during some testing of the
pac1921 iio driver together with the iio-mux iio consumer driver, which
allocates a page sized buffer to copy the ext_info of the producer
pac1921 iio producer driver. To fill the buffer, the latter calls
iio_format_value(), and so sysfs_emit_at() which fails due to the buffer
not being page aligned. This pattern seems common for many iio drivers
which fill the ext_info attributes through sysfs_emit*() helpers, likely
necessary as they are exposed on sysfs.

I could reproduce the same error behavior with a minimal dummy char
device driver completely unrelated to iio. I will share the entire dummy
driver code if needed but essentially this is the only interesting part:

	data->info_buf = devm_kzalloc(data->dev, PAGE_SIZE, GFP_KERNEL);
	if (!data->info_buf)
		return -ENOMEM;

	if (offset_in_page(data->info_buf))
		pr_err("dummy_test: buf not page algined\n");

When running this, the error message is printed out for the reason above.

I am not sure whether this should be addressed in the users of
devm_kmalloc() or in the devres implementation itself. I would say that
it would be more clear if devm_kmalloc() would return the pointer to the
size aligned buffer, as it would also comply to the following kmalloc
requirement (introduced in [1]):

The address of a chunk allocated with `kmalloc` is aligned to at least
ARCH_KMALLOC_MINALIGN bytes. For sizes of power of two bytes, the
alignment is also guaranteed to be at least to the respective size.

To do so I was thinking to try to move the devres metadata after the
data buffer, so that the latter would directly correspond to pointer
returned by kmalloc. I then found out that it had been already suggested
previously to address a memory optimization [2]. Thus I am reporting the
issue before submitting any patch as some discussions might be helpful
first.

I am sending this to who I think might be interested based on previous
related activity. Feel free to extend the cc list if needed.

[1]: https://lore.kernel.org/all/20190826111627.7505-3-vbabka@suse.cz/
[2]: https://lore.kernel.org/all/20191220140655.GN2827@hirez.programming.kicks-ass.net/

Best regard,
Matteo Martelli

             reply	other threads:[~2024-10-28 12:04 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-10-28 12:04 matteomartelli3 [this message]
2024-11-08  9:04 ` iio, syfs, devres: devm_kmalloc not aligned to pow2 size argument Matteo Martelli
2024-11-09  9:29   ` Greg Kroah-Hartman
2024-11-09 15:51     ` Jonathan Cameron
2024-11-09 16:57       ` Greg Kroah-Hartman
2024-11-09 21:10   ` Zijun Hu
2024-11-14 11:29     ` Matteo Martelli
2024-11-14 12:25       ` Zijun Hu
2024-11-14 16:09         ` Matteo Martelli

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=c486a1cf98a8b9ad093270543e8d2007@gmail.com \
    --to=matteomartelli3@gmail.com \
    --cc=gregkh@linuxfoundation.org \
    --cc=jic23@kernel.org \
    --cc=joe@perches.com \
    --cc=linux-iio@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=marc.w.gonzalez@free.fr \
    --cc=peda@axentia.se \
    --cc=peterz@infradead.org \
    --cc=rafael@kernel.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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.