On 1/7/25 4:49 AM, John Garry wrote:
On 07/01/2025 01:24, Matthew Sakai wrote:
Hi John,
Hi Matt,
I'm all for getting rid of warnings where we can, but I'm not
convinced this is the right approach here.
First, we use snprintf here because we really do want to limit the
string to 15 bytes.
Out of curiosity, why is that?
A dm-vdo target creates several threads to do its work. The
thread_name_prefix is intended to be a short string at the start of the
thread name that allows users to easily distinguish these dm-vdo threads
from all others on the system. But it should not be so long that the
dm-vdo threads cannot be distinguished from each other.
Now that we're talking about it, I think it's probably even better not
to use the full module name, but instead use a short static string like
"vdo".
BTW, as I understand, for CONFIG_DM_VDO=m, the module name is "dm_vdo".
So why different MODULE_NAME if MODULE is not defined ("dm-vdo")?
That is a historical artifact and it should just get removed. The module
name should just be a constant string. I could write up a patch to do
this if you don't want to. It might take me a couple of days though
because I'm still dealing with some holiday backlog.
Even after we do this, I still think the prefix string should no longer
rely on the module name.
Making this constant larger doesn't maintain that. This change also
increases the size of struct vdo (in vdo.h) by about 50 bytes. While
that's not a lot, it does seem unnecessary.
In practice, we know that the our module name is only about 6 bytes,
and the instance number will never be more than 3, so snprintf will
never actually truncate anything here. Would checking for a non-zero
return value suppress the error?
I'm also confused because I don't understand the difference between
this line and the similar use of snprintf at drivers/md/dm-vdo/funnel-
workqueue.c:430, which I presume does not trigger a warning?
For that snprintf usage, the compiler just doesn't know the possible
length of @name, as it is a pointer. That is why it is a good reason to
use snprintf.
Anyway, I'd be happy with a version that suppresses the warning
without changing the size of any structures.
Matt Sakai
Thanks,
John
Matt
