From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mail-wm1-f54.google.com (mail-wm1-f54.google.com [209.85.128.54]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 9BCA51FD4 for ; Tue, 2 Dec 2025 15:51:06 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=209.85.128.54 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764690670; cv=none; b=GKvW5AGxJddiDVDBXA8lduI47w+tQJ+DPwl3bwYN9NgGZ7jvuMHwl4iMVfgUaCF5OpXClP8wGTnXpFM5bHmch4Rv4WGeEzr5sgehDAYol09iiX8kDS3HdtbePJebSE8A866YB0CdRDzB1W8aWFlaUJ9VqkfVzSDdToXliSIGxlU= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764690670; c=relaxed/simple; bh=2LTEy9VWYVRd6FnFCLOXWKFMvUd8g04QfIYCJ6OJpXI=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=jhie0aQNBDhCVurdQtXWh/gxO6pa8cMljTbRUBmLkrgeTcU9SLMr/2KuodqQqSkGQzoMUYIXbrY3snNfmO63102Jlqp5fSYF4DQacPn7q2oBY0aD1s8cdqA9jkJJQbMi8fXz2zZmlCet6JXYHcitCWy7uxs6nBCu1pecFNbmrXc= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com; spf=pass smtp.mailfrom=suse.com; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b=Mm1Lp+wr; arc=none smtp.client-ip=209.85.128.54 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=quarantine dis=none) header.from=suse.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=suse.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=suse.com header.i=@suse.com header.b="Mm1Lp+wr" Received: by mail-wm1-f54.google.com with SMTP id 5b1f17b1804b1-4779cb0a33fso59677375e9.0 for ; Tue, 02 Dec 2025 07:51:06 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=suse.com; s=google; t=1764690665; x=1765295465; darn=vger.kernel.org; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=B0jF0fLRJNMBnQ1rT5kegL2zQL7JDmOAYD7gzv1mgMQ=; b=Mm1Lp+wrtIrP0Uu+Rj1bBk0EWR8vOeGySN/RMz86ljEzd1ZcEM1PTxjLfJFnFsVsHo kjKAIgLrcxJvFmK+kF5qBIH42JIvU3qkUxvrMyCrZhXziZfKoMi5A3Daq+RPSoLXbH00 2rkD9LCZxnpeoTVj0GxFZkKo6USVYZtkYLb2K4vpaF07J8cfFbjBHE+fjCFYNoQQn325 Mydo9zQE7yQAT+eU7eJ47H4+YpZy2sOwUoqOrqHsUOvkEzC7NLKvuXST+QOf3ejKMjDS pJPRoioV5bDiCLldCIn5DtG9ocWY8j3wUaz1eov8PCITOT3HfSsR4F5M/ajrsuYkpfwJ VpHg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1764690665; x=1765295465; h=in-reply-to:content-disposition:mime-version:references:message-id :subject:cc:to:from:date:x-gm-gg:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=B0jF0fLRJNMBnQ1rT5kegL2zQL7JDmOAYD7gzv1mgMQ=; b=jNQDELa7ZW4gVVn8lfz2HWJDrWuQJ6PRDlHnv8GjsWECmYPey+uqULl5JpjaEcL+io Wpw4k23jyenAK4UHRcq63T75H8OjmOcq67UaSLIr7fxtQfzGaIwrT9mzyhtq0GgpTT+2 KntmMpR4jmM83g+lGKea7Utxa3IL+EHlovUlXGVUFhG+vMnn92qCqjH8JxYhYqZ0enlF mwdZTGv6f4g0gNVR+tc3H4sKp7343ysmXB4lz7VFDIS7m/odfJdfW4uqgHh2zVEHPE7d WF+3WIg2FNKll28FDb3yviM2xFyMsXTUY212P0wkxAGAUyxh9FIZlDvt0RIoL2hO9fg4 c3gw== X-Forwarded-Encrypted: i=1; AJvYcCWHiTc11NUfLO+jW4a1K5QAxb9Q1ZUZFFQW7zuZ6AGbZQ076ImUWYSS0Qy+4QRrpIxogOOanoibA1BmCCQ=@vger.kernel.org X-Gm-Message-State: AOJu0YwMdv93EhPy4IJkrKHZTw9L9JEg96ZEfaLt4cbXVDVmVcIFmC9N tDlhGRX54E41Iimbh84UgwWY+xeXP7Zq4netdvaU7InI/sC2y9ClWPl+jN+pxlWhQt0AafVBHFE vjxp9 X-Gm-Gg: ASbGncu+okFdtflvm0GaoON1K+OI6iTNfRBOqCgh31DjdWMA8/h+RfSl30TsnOM4G/x AJyAtlt+gAopDGgtJJeWb4gDcnfB1VjorEXfmS/EvDTHn+x81UKSEZ1U3inMP47yyY2UjOC+FFO WGVveyoZ2KcVMZ4xW3pE5s4WWrEDuE/FvX5sLo1iFNMFDSWBqUOl6qXYRNyQqZCfII4TrnEWzZW qd+Vuay2fO2CvYsSAuqYQVe/3rmSlrUNvg6gSTN+TxQZ9FJoUG4ooAOneS1FO26sj4oEYUyCh3U 7v+oiNo1Z6Drwz44IM02DSTURvbYbkpL4uQwIw9uoclH76xys061Oac8ITqp9xSMODp8Y/bWhPd gBi1021rG3DvvDgYvlKcQnMPLslZ3EkFR15p2g918MN4urbFHFqqRqLy0wnFB4KAgO+WPIoiHSm SQOAUTxBe41Tq8aA== X-Google-Smtp-Source: AGHT+IHebbf5Vw9lZaq6IFuFTBYRPHyOz2I2Hu7QCHyEQlP3wRlC/78XfKOVh0LrRV4BfxEgcaUNNw== X-Received: by 2002:a05:600c:4e88:b0:46e:7247:cbc0 with SMTP id 5b1f17b1804b1-477c01d4af2mr422946075e9.18.1764690664742; Tue, 02 Dec 2025 07:51:04 -0800 (PST) Received: from pathway.suse.cz ([176.114.240.130]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-479279d930dsm18024735e9.6.2025.12.02.07.51.02 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Tue, 02 Dec 2025 07:51:04 -0800 (PST) Date: Tue, 2 Dec 2025 16:51:00 +0100 From: Petr Mladek To: Andrzej Hajda Cc: Steven Rostedt , John Ogness , Sergey Senozhatsky , Greg Kroah-Hartman , "Rafael J. Wysocki" , Danilo Krummrich , Andrew Morton , Vlastimil Babka , Suren Baghdasaryan , Michal Hocko , Brendan Jackman , Johannes Weiner , Zi Yan , Christoph Lameter , David Rientjes , Roman Gushchin , Harry Yoo , linux-kernel@vger.kernel.org, linux-mm@kvack.org, Andy Shevchenko , Rasmus Villemoes Subject: Re: [PATCH v2 3/5] drivers/core: simplify variadic args handling Message-ID: References: <20251201-va_format_call-v2-0-2906f3093b60@intel.com> <20251201-va_format_call-v2-3-2906f3093b60@intel.com> Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20251201-va_format_call-v2-3-2906f3093b60@intel.com> 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