[PATCH v2 0/5] printk: add macros to simplify handling struct va_format

[PATCH v2 0/5] printk: add macros to simplify handling struct va_format

[Andrzej Hajda]

This patchset contains patch abstracting out most common bits of va_format uses
into small macro. The macro works as a forwarder of variadic args to va_format
call.
Rough estimates based on dirty cocci script suggests about 180-230 users
could be converted, allowing to remove about 1100-1300 lines of code.
Of course, I do not plan to do it in mass. I just provide few patches to
show the usage and the potential.

Provided helper deals with variadic args to va_format conversion/forwarding.
Similar solution could be proposed also for variadic args to va_list conversion.
Common code:
    va_list args;
    va_start(args, fmt);
    ret = func(..., fmt, args);
    va_end(args);
    return ret;
Could be replaced with:
    ret = va_list_call(fmt, func, ..., fmt);
This is less rewarding (less lines to remove) but still looks promising. Also
separate helper should be created for functions without return value,
unless somebody knows trick how to do it in one macro:
    va_list_void_call(fmt, func, ..., fmt);
Any comments?

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
Changes in v2:
- added two patches simplifying handling of variadic args in drivers/core and mm
  in slightly less obvious cases, this way both mm and drivers/core do not use
  anymore hand rolled variadic -> va_format conversions.
- Link to v1: https://lore.kernel.org/r/20251126-va_format_call-v1-0-8a604a331730@intel.com

---
Andrzej Hajda (5):
      printk: add macros to simplify handling struct va_format
      drivers/core: use va_format_call helper
      drivers/core: simplify variadic args handling
      mm: use va_format_call helper
      mm: simplify variadic args handling

 drivers/base/core.c    | 82 ++++++++------------------------------------------
 include/linux/printk.h | 30 ++++++++++++++++++
 mm/page_alloc.c        | 14 +++------
 mm/slub.c              | 33 +++-----------------
 4 files changed, 52 insertions(+), 107 deletions(-)
---
base-commit: 37a098cc87aafdf4de147851cbf350bb66c51b45
change-id: 20251126-va_format_call-41053ef60fd7

Best regards,
--  
Andrzej Hajda <andrzej.hajda@intel.com>

[PATCH v2 1/5] printk: add macros to simplify handling struct va_format

[Andrzej Hajda]

struct va_format is used to facilitate implementation of
variadic functions. Most of variadic users do not parse
arguments one by one, they just forward them to some
helper via va_format pointer. Introduced helpers simplifies
it more.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 include/linux/printk.h | 30 ++++++++++++++++++++++++++++++
 1 file changed, 30 insertions(+)

diff --git a/include/linux/printk.h b/include/linux/printk.h
index 45c663124c9b..6fd817ce29a6 100644
--- a/include/linux/printk.h
+++ b/include/linux/printk.h
@@ -87,6 +87,36 @@ struct va_format {
 	va_list *va;
 };
 
+/* Special macro used only by callers of va_format_call to position va_format argument */
+#define va_format_arg (&vaf)
+
+/**
+ * va_format_call() - forward variable arguments to va_format aware function
+ * @_fmt: format used by @_func function
+ * @_func: function which should be called
+ * @_args: arguments passed to @_func. Exactly one of them must be va_format_arg,
+ *         to indicate position of *va_format type arg in arguments of @_func.
+ *
+ * Many variadic functions just forwards their variadic arguments to some helper
+ * function via va_format pointer. It involves few common steps encapsulated in
+ * this macro. The macro is accompanied by va_format_arg macro, described above.
+ * Sample implementation of common helper:
+ * void dev_err(struct device *dev, const char* fmt, ...)
+ * {
+ *    va_format_call(fmt, pr_err, "%s %s: %pV", dev_driver_string(dev),
+ *                   dev_name(dev), va_format_arg);
+ * }
+ */
+#define va_format_call(_fmt, _func, _args...)			\
+({								\
+	va_list ap;						\
+	struct va_format vaf = { .fmt = _fmt, .va = &ap };	\
+								\
+	va_start(ap, _fmt);					\
+	_func(_args);						\
+	va_end(ap);						\
+})
+
 /*
  * FW_BUG
  * Add this to a message where you are sure the firmware is buggy or behaves

-- 
2.43.0

[PATCH v2 2/5] drivers/core: use va_format_call helper

[Andrzej Hajda]

It simplifies the code and makes it more clear.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 drivers/base/core.c | 32 ++++++--------------------------
 1 file changed, 6 insertions(+), 26 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index f69dc9c85954..513e5ef8a6da 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4943,35 +4943,15 @@ static void __dev_printk(const char *level, const struct device *dev,
 void _dev_printk(const char *level, const struct device *dev,
 		 const char *fmt, ...)
 {
-	struct va_format vaf;
-	va_list args;
-
-	va_start(args, fmt);
-
-	vaf.fmt = fmt;
-	vaf.va = &args;
-
-	__dev_printk(level, dev, &vaf);
-
-	va_end(args);
+	va_format_call(fmt, __dev_printk, level, dev, va_format_arg);
 }
 EXPORT_SYMBOL(_dev_printk);
 
-#define define_dev_printk_level(func, kern_level)		\
-void func(const struct device *dev, const char *fmt, ...)	\
-{								\
-	struct va_format vaf;					\
-	va_list args;						\
-								\
-	va_start(args, fmt);					\
-								\
-	vaf.fmt = fmt;						\
-	vaf.va = &args;						\
-								\
-	__dev_printk(kern_level, dev, &vaf);			\
-								\
-	va_end(args);						\
-}								\
+#define define_dev_printk_level(func, kern_level)				\
+void func(const struct device *dev, const char *fmt, ...)			\
+{										\
+	va_format_call(fmt, __dev_printk, kern_level, dev, va_format_arg);	\
+}										\
 EXPORT_SYMBOL(func);
 
 define_dev_printk_level(_dev_emerg, KERN_EMERG);

-- 
2.43.0

[PATCH v2 3/5] drivers/core: simplify variadic args handling

[Andrzej Hajda]

Changing argument type from va_list to struct va_format * allows
to simplify variadic argument handling with va_format_call helper.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 drivers/base/core.c | 50 +++++++-------------------------------------------
 1 file changed, 7 insertions(+), 43 deletions(-)

diff --git a/drivers/base/core.c b/drivers/base/core.c
index 513e5ef8a6da..4d76b67a87e3 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -4965,30 +4965,12 @@ define_dev_printk_level(_dev_info, KERN_INFO);
 #endif
 
 static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
-			       const char *fmt, va_list vargsp)
+			       const char *fmt, struct va_format *vaf)
 {
-	struct va_format vaf;
-	va_list vargs;
-
-	/*
-	 * On x86_64 and possibly on other architectures, va_list is actually a
-	 * size-1 array containing a structure.  As a result, function parameter
-	 * vargsp decays from T[1] to T*, and &vargsp has type T** rather than
-	 * T(*)[1], which is expected by its assignment to vaf.va below.
-	 *
-	 * One standard way to solve this mess is by creating a copy in a local
-	 * variable of type va_list and then using a pointer to that local copy
-	 * instead, which is the approach employed here.
-	 */
-	va_copy(vargs, vargsp);
-
-	vaf.fmt = fmt;
-	vaf.va = &vargs;
-
 	switch (err) {
 	case -EPROBE_DEFER:
-		device_set_deferred_probe_reason(dev, &vaf);
-		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
+		device_set_deferred_probe_reason(dev, vaf);
+		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), vaf);
 		break;
 
 	case -ENOMEM:
@@ -4998,13 +4980,11 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
 	default:
 		/* Log fatal final failures as errors, otherwise produce warnings */
 		if (fatal)
-			dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
+			dev_err(dev, "error %pe: %pV", ERR_PTR(err), vaf);
 		else
-			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
+			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), vaf);
 		break;
 	}
-
-	va_end(vargs);
 }
 
 /**
@@ -5042,15 +5022,7 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
  */
 int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
 {
-	va_list vargs;
-
-	va_start(vargs, fmt);
-
-	/* Use dev_err() for logging when err doesn't equal -EPROBE_DEFER */
-	__dev_probe_failed(dev, err, true, fmt, vargs);
-
-	va_end(vargs);
-
+	va_format_call(fmt, __dev_probe_failed, dev, err, true, fmt, va_format_arg);
 	return err;
 }
 EXPORT_SYMBOL_GPL(dev_err_probe);
@@ -5090,15 +5062,7 @@ EXPORT_SYMBOL_GPL(dev_err_probe);
  */
 int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
 {
-	va_list vargs;
-
-	va_start(vargs, fmt);
-
-	/* Use dev_warn() for logging when err doesn't equal -EPROBE_DEFER */
-	__dev_probe_failed(dev, err, false, fmt, vargs);
-
-	va_end(vargs);
-
+	va_format_call(fmt, __dev_probe_failed, dev, err, false, fmt, va_format_arg);
 	return err;
 }
 EXPORT_SYMBOL_GPL(dev_warn_probe);

-- 
2.43.0

Re: [PATCH v2 3/5] drivers/core: simplify variadic args handling

[Petr Mladek]

I am adding Andy and Rasmus into Cc who are active vsprintf-related
code reviewers...

You might see the entire patchset at
https://lore.kernel.org/all/20251201-va_format_call-v2-0-2906f3093b60@intel.com/

On Mon 2025-12-01 10:31:24, Andrzej Hajda wrote:
> Changing argument type from va_list to struct va_format * allows
> to simplify variadic argument handling with va_format_call helper.
> 
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 513e5ef8a6da..4d76b67a87e3 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -4965,30 +4965,12 @@ define_dev_printk_level(_dev_info, KERN_INFO);
>  #endif
>  
>  static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
> -			       const char *fmt, va_list vargsp)
> +			       const char *fmt, struct va_format *vaf)
>  {
> -	struct va_format vaf;
> -	va_list vargs;
> -
> -	/*
> -	 * On x86_64 and possibly on other architectures, va_list is actually a
> -	 * size-1 array containing a structure.  As a result, function parameter
> -	 * vargsp decays from T[1] to T*, and &vargsp has type T** rather than
> -	 * T(*)[1], which is expected by its assignment to vaf.va below.
> -	 *
> -	 * One standard way to solve this mess is by creating a copy in a local
> -	 * variable of type va_list and then using a pointer to that local copy
> -	 * instead, which is the approach employed here.
> -	 */
> -	va_copy(vargs, vargsp);
> -
> -	vaf.fmt = fmt;
> -	vaf.va = &vargs;

I am always a bit lost when using this API.
Why is it safe to remove the va_copy() here, please?

The va_format_call() uses va_start()/va_end() which is replacing
these calls in dev_err_probe() and dev_warn_probe().

It is possible that the original code was actually wrong because
it uses the same copy (&vaf) everywhere, see below.

>  	switch (err) {
>  	case -EPROBE_DEFER:
> -		device_set_deferred_probe_reason(dev, &vaf);

This function processes the arguments via:

  + device_set_deferred_probe_reason()
    + kasprintf()
      + va_start()/va_end()

> -		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);

This function uses the already processed copy of the arguments.
IMHO, it might print a garbage because of this. IMHO, it should use
the original va_list() or might need its own copy.

Note that this call does not modify the va_list because it uses "%pV"
and vsprintf() creates its own copy in this case, see va_format()
in lib/vsprintf.c.

> +		device_set_deferred_probe_reason(dev, vaf);
> +		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), vaf);
>  		break;
>  
>  	case -ENOMEM:
> @@ -4998,13 +4980,11 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
>  	default:
>  		/* Log fatal final failures as errors, otherwise produce warnings */
>  		if (fatal)
> -			dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
> +			dev_err(dev, "error %pe: %pV", ERR_PTR(err), vaf);
>  		else
> -			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
> +			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), vaf);

This should be fine because of using "%pV".

>  		break;
>  	}
> -
> -	va_end(vargs);
>  }
>  
>  /**
> @@ -5042,15 +5022,7 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
>   */
>  int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
>  {
> -	va_list vargs;
> -
> -	va_start(vargs, fmt);
> -
> -	/* Use dev_err() for logging when err doesn't equal -EPROBE_DEFER */
> -	__dev_probe_failed(dev, err, true, fmt, vargs);
> -
> -	va_end(vargs);
> -
> +	va_format_call(fmt, __dev_probe_failed, dev, err, true, fmt, va_format_arg);
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(dev_err_probe);
> @@ -5090,15 +5062,7 @@ EXPORT_SYMBOL_GPL(dev_err_probe);
>   */
>  int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
>  {
> -	va_list vargs;
> -
> -	va_start(vargs, fmt);
> -
> -	/* Use dev_warn() for logging when err doesn't equal -EPROBE_DEFER */
> -	__dev_probe_failed(dev, err, false, fmt, vargs);
> -
> -	va_end(vargs);
> -
> +	va_format_call(fmt, __dev_probe_failed, dev, err, false, fmt, va_format_arg);
>  	return err;
>  }
>  EXPORT_SYMBOL_GPL(dev_warn_probe);

Best Regards,
Petr

Re: [PATCH v2 3/5] drivers/core: simplify variadic args handling

[Hajda, Andrzej]

W dniu 02.12.2025 o 16:51, Petr Mladek pisze:
> I am adding Andy and Rasmus into Cc who are active vsprintf-related
> code reviewers...
>
> You might see the entire patchset at
> https://lore.kernel.org/all/20251201-va_format_call-v2-0-2906f3093b60@intel.com/
>
> On Mon 2025-12-01 10:31:24, Andrzej Hajda wrote:
>> Changing argument type from va_list to struct va_format * allows
>> to simplify variadic argument handling with va_format_call helper.
>>
>> diff --git a/drivers/base/core.c b/drivers/base/core.c
>> index 513e5ef8a6da..4d76b67a87e3 100644
>> --- a/drivers/base/core.c
>> +++ b/drivers/base/core.c
>> @@ -4965,30 +4965,12 @@ define_dev_printk_level(_dev_info, KERN_INFO);
>>   #endif
>>   
>>   static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
>> -			       const char *fmt, va_list vargsp)
>> +			       const char *fmt, struct va_format *vaf)
>>   {
>> -	struct va_format vaf;
>> -	va_list vargs;
>> -
>> -	/*
>> -	 * On x86_64 and possibly on other architectures, va_list is actually a
>> -	 * size-1 array containing a structure.  As a result, function parameter
>> -	 * vargsp decays from T[1] to T*, and &vargsp has type T** rather than
>> -	 * T(*)[1], which is expected by its assignment to vaf.va below.
>> -	 *
>> -	 * One standard way to solve this mess is by creating a copy in a local
>> -	 * variable of type va_list and then using a pointer to that local copy
>> -	 * instead, which is the approach employed here.
>> -	 */
>> -	va_copy(vargs, vargsp);
>> -
>> -	vaf.fmt = fmt;
>> -	vaf.va = &vargs;
> I am always a bit lost when using this API.
> Why is it safe to remove the va_copy() here, please?

Not very familiar with this workaround, just my thoughts about it.

It is just va_list is compiler's private implementation, which can be 
anything.

And if it happens to be T[1], it's type decays to T* if it is type of 
argument of the function.

So vargsp is in fact of type T*, and &vargs is of type T** and it does 
not point to va_list anymore.

So in short passing va_list to a function, which takes a pointer to the 
arg is problematic.

va_format_call DOES NOT pass va_list to a function, so it seems to be safe.


> The va_format_call() uses va_start()/va_end() which is replacing
> these calls in dev_err_probe() and dev_warn_probe().
>
> It is possible that the original code was actually wrong because
> it uses the same copy (&vaf) everywhere, see below.
>
>>   	switch (err) {
>>   	case -EPROBE_DEFER:
>> -		device_set_deferred_probe_reason(dev, &vaf);
> This function processes the arguments via:
>
>    + device_set_deferred_probe_reason()
>      + kasprintf()
>        + va_start()/va_end()


This va_start/va_end is for var_args of kasprintf, not for &vaf, I hope 
parsing %pV uses va_copy.


Regards

Andrzej


>
>> -		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
> This function uses the already processed copy of the arguments.
> IMHO, it might print a garbage because of this. IMHO, it should use
> the original va_list() or might need its own copy.
>
> Note that this call does not modify the va_list because it uses "%pV"
> and vsprintf() creates its own copy in this case, see va_format()
> in lib/vsprintf.c.
>
>> +		device_set_deferred_probe_reason(dev, vaf);
>> +		dev_dbg(dev, "error %pe: %pV", ERR_PTR(err), vaf);
>>   		break;
>>   
>>   	case -ENOMEM:
>> @@ -4998,13 +4980,11 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
>>   	default:
>>   		/* Log fatal final failures as errors, otherwise produce warnings */
>>   		if (fatal)
>> -			dev_err(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
>> +			dev_err(dev, "error %pe: %pV", ERR_PTR(err), vaf);
>>   		else
>> -			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), &vaf);
>> +			dev_warn(dev, "error %pe: %pV", ERR_PTR(err), vaf);
> This should be fine because of using "%pV".
>
>>   		break;
>>   	}
>> -
>> -	va_end(vargs);
>>   }
>>   
>>   /**
>> @@ -5042,15 +5022,7 @@ static void __dev_probe_failed(const struct device *dev, int err, bool fatal,
>>    */
>>   int dev_err_probe(const struct device *dev, int err, const char *fmt, ...)
>>   {
>> -	va_list vargs;
>> -
>> -	va_start(vargs, fmt);
>> -
>> -	/* Use dev_err() for logging when err doesn't equal -EPROBE_DEFER */
>> -	__dev_probe_failed(dev, err, true, fmt, vargs);
>> -
>> -	va_end(vargs);
>> -
>> +	va_format_call(fmt, __dev_probe_failed, dev, err, true, fmt, va_format_arg);
>>   	return err;
>>   }
>>   EXPORT_SYMBOL_GPL(dev_err_probe);
>> @@ -5090,15 +5062,7 @@ EXPORT_SYMBOL_GPL(dev_err_probe);
>>    */
>>   int dev_warn_probe(const struct device *dev, int err, const char *fmt, ...)
>>   {
>> -	va_list vargs;
>> -
>> -	va_start(vargs, fmt);
>> -
>> -	/* Use dev_warn() for logging when err doesn't equal -EPROBE_DEFER */
>> -	__dev_probe_failed(dev, err, false, fmt, vargs);
>> -
>> -	va_end(vargs);
>> -
>> +	va_format_call(fmt, __dev_probe_failed, dev, err, false, fmt, va_format_arg);
>>   	return err;
>>   }
>>   EXPORT_SYMBOL_GPL(dev_warn_probe);
> Best Regards,
> Petr

Re: [PATCH v2 3/5] drivers/core: simplify variadic args handling

[Andy Shevchenko]

On Tue, Dec 02, 2025 at 07:03:37PM +0100, Hajda, Andrzej wrote:
> W dniu 02.12.2025 o 16:51, Petr Mladek pisze:
> > I am adding Andy and Rasmus into Cc who are active vsprintf-related
> > code reviewers...
> > 
> > You might see the entire patchset at
> > https://lore.kernel.org/all/20251201-va_format_call-v2-0-2906f3093b60@intel.com/

TBH, I don't like the result. There are two problems with readability:

1) macro well hides the actual low-level call, hard to parse from its
parameters;

2) sometimes it has va_format_call(fmt, ..., fmt, ...) which is confusing.

Implementation is also doubtful (to me) as GCC extension. Can't it rather
return an error code and use something like do { } while (0) inside? OTOH,
may be this is not feasible in a clean way...

And what is the motivation? Just make less LoCs? I would really like to see
at least vmlinux sizes, the reports that GCC _and_ clang are both happy with
the compilation as of `make W=1` of this on both 32- and 64-bit cases.

Does it solve any issue? Does it bring any consistency or standardisation here?

> > On Mon 2025-12-01 10:31:24, Andrzej Hajda wrote:

...

> > > -	/*
> > > -	 * On x86_64 and possibly on other architectures, va_list is actually a
> > > -	 * size-1 array containing a structure.  As a result, function parameter
> > > -	 * vargsp decays from T[1] to T*, and &vargsp has type T** rather than
> > > -	 * T(*)[1], which is expected by its assignment to vaf.va below.
> > > -	 *
> > > -	 * One standard way to solve this mess is by creating a copy in a local
> > > -	 * variable of type va_list and then using a pointer to that local copy
> > > -	 * instead, which is the approach employed here.
> > > -	 */
> > > -	va_copy(vargs, vargsp);
> > > -
> > > -	vaf.fmt = fmt;
> > > -	vaf.va = &vargs;

> > I am always a bit lost when using this API.
> > Why is it safe to remove the va_copy() here, please?
> 
> Not very familiar with this workaround, just my thoughts about it.
> 
> It is just va_list is compiler's private implementation, which can be
> anything.
> 
> And if it happens to be T[1], it's type decays to T* if it is type of
> argument of the function.
> 
> So vargsp is in fact of type T*, and &vargs is of type T** and it does not
> point to va_list anymore.
> 
> So in short passing va_list to a function, which takes a pointer to the arg
> is problematic.
> 
> va_format_call DOES NOT pass va_list to a function, so it seems to be safe.

I'm sorry, I can't be helpful here, as I am not well familiar
with va_*() stuff. The idea is interesting, nevertheless, but
see above.

> > The va_format_call() uses va_start()/va_end() which is replacing
> > these calls in dev_err_probe() and dev_warn_probe().
> > 
> > It is possible that the original code was actually wrong because
> > it uses the same copy (&vaf) everywhere, see below.
> > 
> > >   	switch (err) {
> > >   	case -EPROBE_DEFER:
> > > -		device_set_deferred_probe_reason(dev, &vaf);
> > This function processes the arguments via:
> > 
> >    + device_set_deferred_probe_reason()
> >      + kasprintf()
> >        + va_start()/va_end()
> 
> This va_start/va_end is for var_args of kasprintf, not for &vaf, I hope
> parsing %pV uses va_copy.

Yes, it does call va_copy().

-- 
With Best Regards,
Andy Shevchenko

Re: [PATCH v2 3/5] drivers/core: simplify variadic args handling

[Hajda, Andrzej]

W dniu 03.12.2025 o 15:47, Andy Shevchenko pisze:
> On Tue, Dec 02, 2025 at 07:03:37PM +0100, Hajda, Andrzej wrote:
>> W dniu 02.12.2025 o 16:51, Petr Mladek pisze:
>>> I am adding Andy and Rasmus into Cc who are active vsprintf-related
>>> code reviewers...
>>>
>>> You might see the entire patchset at
>>> https://lore.kernel.org/all/20251201-va_format_call-v2-0-2906f3093b60@intel.com/
> TBH, I don't like the result. There are two problems with readability:
>
> 1) macro well hides the actual low-level call, hard to parse from its
> parameters;

I hope 'va_format_call' is a strong suggestion that sth is to be called.

>
> 2) sometimes it has va_format_call(fmt, ..., fmt, ...) which is confusing.

Many functions sometimes takes the same argument twice.

1st argument is to indicate beginning of the '...' in caller function 
args list, quite natural.

2nd argument is function name, and the rest are it's arguments, quite 
natural.

Since C23 standard, we could omit the 1st argument [1], but this is 
potential future.

[1]: https://en.cppreference.com/w/c/variadic/va_start


>
> Implementation is also doubtful (to me) as GCC extension. Can't it rather
> return an error code and use something like do { } while (0) inside? OTOH,
> may be this is not feasible in a clean way...

???, expression statement "({ ...})" is present even in macro samples in 
kernel coding-style [2].

[2] https://www.kernel.org/doc/html/latest/process/coding-style.html

>
> And what is the motivation? Just make less LoCs?

Also, but first:

- encapsulate common repeatable pattern into one macro, with clear 
purpose - forward variable args to va_format aware function.

- simplify handling variable arguments: it is easier to write single 
line instead of ten, usually interleaved by other declarations and code.

Is it so hard to see it? Have you seen other patches - quite good examples.

>   I would really like to see at least vmlinux sizes,

What for? This not about binary size optimisation.

> the reports that GCC _and_ clang are both happy with
> the compilation as of `make W=1` of this on both 32- and 64-bit cases.


I do not expect any problems here, but will check :)


>
> Does it solve any issue? Does it bring any consistency or standardisation here?

No. Yes.


Regards

Andrzej


>
>>> On Mon 2025-12-01 10:31:24, Andrzej Hajda wrote:
> ...
>
>>>> -	/*
>>>> -	 * On x86_64 and possibly on other architectures, va_list is actually a
>>>> -	 * size-1 array containing a structure.  As a result, function parameter
>>>> -	 * vargsp decays from T[1] to T*, and &vargsp has type T** rather than
>>>> -	 * T(*)[1], which is expected by its assignment to vaf.va below.
>>>> -	 *
>>>> -	 * One standard way to solve this mess is by creating a copy in a local
>>>> -	 * variable of type va_list and then using a pointer to that local copy
>>>> -	 * instead, which is the approach employed here.
>>>> -	 */
>>>> -	va_copy(vargs, vargsp);
>>>> -
>>>> -	vaf.fmt = fmt;
>>>> -	vaf.va = &vargs;
>>> I am always a bit lost when using this API.
>>> Why is it safe to remove the va_copy() here, please?
>> Not very familiar with this workaround, just my thoughts about it.
>>
>> It is just va_list is compiler's private implementation, which can be
>> anything.
>>
>> And if it happens to be T[1], it's type decays to T* if it is type of
>> argument of the function.
>>
>> So vargsp is in fact of type T*, and &vargs is of type T** and it does not
>> point to va_list anymore.
>>
>> So in short passing va_list to a function, which takes a pointer to the arg
>> is problematic.
>>
>> va_format_call DOES NOT pass va_list to a function, so it seems to be safe.
> I'm sorry, I can't be helpful here, as I am not well familiar
> with va_*() stuff. The idea is interesting, nevertheless, but
> see above.
>
>>> The va_format_call() uses va_start()/va_end() which is replacing
>>> these calls in dev_err_probe() and dev_warn_probe().
>>>
>>> It is possible that the original code was actually wrong because
>>> it uses the same copy (&vaf) everywhere, see below.
>>>
>>>>    	switch (err) {
>>>>    	case -EPROBE_DEFER:
>>>> -		device_set_deferred_probe_reason(dev, &vaf);
>>> This function processes the arguments via:
>>>
>>>     + device_set_deferred_probe_reason()
>>>       + kasprintf()
>>>         + va_start()/va_end()
>> This va_start/va_end is for var_args of kasprintf, not for &vaf, I hope
>> parsing %pV uses va_copy.
> Yes, it does call va_copy().
>

[PATCH v2 4/5] mm: use va_format_call helper

[Andrzej Hajda]

It simplifies the code and makes it more clear.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 mm/page_alloc.c | 14 ++++----------
 mm/slub.c       | 10 +---------
 2 files changed, 5 insertions(+), 19 deletions(-)

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ed82ee55e66a..cd0b17ea4de0 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3940,22 +3940,16 @@ static void warn_alloc_show_mem(gfp_t gfp_mask, nodemask_t *nodemask)
 
 void warn_alloc(gfp_t gfp_mask, nodemask_t *nodemask, const char *fmt, ...)
 {
-	struct va_format vaf;
-	va_list args;
 	static DEFINE_RATELIMIT_STATE(nopage_rs, 10*HZ, 1);
 
 	if ((gfp_mask & __GFP_NOWARN) ||
 	     !__ratelimit(&nopage_rs) ||
 	     ((gfp_mask & __GFP_DMA) && !has_managed_dma()))
 		return;
-
-	va_start(args, fmt);
-	vaf.fmt = fmt;
-	vaf.va = &args;
-	pr_warn("%s: %pV, mode:%#x(%pGg), nodemask=%*pbl",
-			current->comm, &vaf, gfp_mask, &gfp_mask,
-			nodemask_pr_args(nodemask));
-	va_end(args);
+	va_format_call(fmt, pr_warn,
+		       "%s: %pV, mode:%#x(%pGg), nodemask=%*pbl",
+		       current->comm, va_format_arg, gfp_mask, &gfp_mask,
+		       nodemask_pr_args(nodemask));
 
 	cpuset_print_current_mems_allowed();
 	pr_cont("\n");
diff --git a/mm/slub.c b/mm/slub.c
index 1a5fc3429042..786c5a4195d4 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1161,17 +1161,9 @@ static void slab_bug(struct kmem_cache *s, const char *fmt, ...)
 __printf(2, 3)
 static void slab_fix(struct kmem_cache *s, const char *fmt, ...)
 {
-	struct va_format vaf;
-	va_list args;
-
 	if (slab_add_kunit_errors())
 		return;
-
-	va_start(args, fmt);
-	vaf.fmt = fmt;
-	vaf.va = &args;
-	pr_err("FIX %s: %pV\n", s->name, &vaf);
-	va_end(args);
+	va_format_call(fmt, pr_err, "FIX %s: %pV\n", s->name, va_format_arg);
 }
 
 static void print_trailer(struct kmem_cache *s, struct slab *slab, u8 *p)

-- 
2.43.0

[PATCH v2 5/5] mm: simplify variadic args handling

[Andrzej Hajda]

Changing argument type from va_list to struct va_format * allows
to simplify variadic argument handling with va_format_call helper.

Signed-off-by: Andrzej Hajda <andrzej.hajda@intel.com>
---
 mm/slub.c | 23 ++++-------------------
 1 file changed, 4 insertions(+), 19 deletions(-)

diff --git a/mm/slub.c b/mm/slub.c
index 786c5a4195d4..12d8ec97eac5 100644
--- a/mm/slub.c
+++ b/mm/slub.c
@@ -1135,27 +1135,16 @@ void skip_orig_size_check(struct kmem_cache *s, const void *object)
 	set_orig_size(s, (void *)object, s->object_size);
 }
 
-static void __slab_bug(struct kmem_cache *s, const char *fmt, va_list argsp)
+static void __slab_bug(struct kmem_cache *s, const char *fmt, struct va_format *vaf)
 {
-	struct va_format vaf;
-	va_list args;
-
-	va_copy(args, argsp);
-	vaf.fmt = fmt;
-	vaf.va = &args;
 	pr_err("=============================================================================\n");
-	pr_err("BUG %s (%s): %pV\n", s ? s->name : "<unknown>", print_tainted(), &vaf);
+	pr_err("BUG %s (%s): %pV\n", s ? s->name : "<unknown>", print_tainted(), vaf);
 	pr_err("-----------------------------------------------------------------------------\n\n");
-	va_end(args);
 }
 
 static void slab_bug(struct kmem_cache *s, const char *fmt, ...)
 {
-	va_list args;
-
-	va_start(args, fmt);
-	__slab_bug(s, fmt, args);
-	va_end(args);
+	va_format_call(fmt, __slab_bug, s, fmt, va_format_arg);
 }
 
 __printf(2, 3)
@@ -1252,14 +1241,10 @@ static void __slab_err(struct slab *slab)
 static __printf(3, 4) void slab_err(struct kmem_cache *s, struct slab *slab,
 			const char *fmt, ...)
 {
-	va_list args;
-
 	if (slab_add_kunit_errors())
 		return;
 
-	va_start(args, fmt);
-	__slab_bug(s, fmt, args);
-	va_end(args);
+	va_format_call(fmt, __slab_bug, s, fmt, va_format_arg);
 
 	__slab_err(slab);
 }

-- 
2.43.0