From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from mgamail.intel.com (mgamail.intel.com [198.175.65.19]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 279A21FE451 for ; Wed, 3 Dec 2025 14:47:50 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=198.175.65.19 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764773273; cv=none; b=FRoFbJEkugJNHIuMWyQ2AduQyIeuFjx0pZoFbOOxKdI1BT6dECv3Aop0kgM9I4vJ9yjDD5b4a5YXDNT4ckUbDLYhEE6BxDdsqZDjrN4TDo5Zu9IWTkFL+/kQywIsc+NHnBYro9XgsAhc6f83vCMkalB2bstT+GMIVKz5XnryrkA= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1764773273; c=relaxed/simple; bh=ONLKaMVDzvuVK/VrJwxFR2Jg+soODJ+V+Md8LMMJBUU=; h=Date:From:To:Cc:Subject:Message-ID:References:MIME-Version: Content-Type:Content-Disposition:In-Reply-To; b=JCletn1TbVXfoKrPvuCajQV9K+zeG/eh1EP5pITalt6MYT5BhZsiCBopt3I7VjeVhBSKLUP1L3B0G5p8STOzYxAhvkftuyzo8y1hY9OkKzntz3V353vvL8Vb+JhRLCMj9oTVNNo8jMi0rxI/hjgav00sANII6C1YnKgBM8rZNcg= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com; spf=pass smtp.mailfrom=linux.intel.com; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b=BPOX6Bdq; arc=none smtp.client-ip=198.175.65.19 Authentication-Results: smtp.subspace.kernel.org; dmarc=pass (p=none dis=none) header.from=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; spf=pass smtp.mailfrom=linux.intel.com Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=intel.com header.i=@intel.com header.b="BPOX6Bdq" DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1764773271; x=1796309271; h=date:from:to:cc:subject:message-id:references: mime-version:content-transfer-encoding:in-reply-to; bh=ONLKaMVDzvuVK/VrJwxFR2Jg+soODJ+V+Md8LMMJBUU=; b=BPOX6BdqEHNdYYblIT9BlSBkvWeZH4Bh4fBod5xu9YOsWq4m0FZySPci TqH7c1xI1YmNVCXp5By6lIxD0x0XrXk/SCFPSH4EAqaZi8AJHM0cFVNg2 Zg10U5DHxAt0BiNrHPRxzXCv83KrxFH1VPPSslqExtwQhhdW+YtOGqtk0 cxWeZ6cBMrvTHOa0NyyT7SOBgf28Y/STo09Onz+GrZplggbk4LFgBSApL eUOHCFA+5ZJ6Iuga+Ab1zpx/BIlcgCxFmhaJITWAe1SQ+5EZnmlw9hqgP 2X5aj6QLrTnFc20zIoOqRJ7vLQq7APcrQeVpeADbJPRYhUqGM7RVvTjKP A==; X-CSE-ConnectionGUID: WFcLfHORRzGzpAkZ3rC1/g== X-CSE-MsgGUID: nLQcNRfPSCuygMrxUIPSNQ== X-IronPort-AV: E=McAfee;i="6800,10657,11631"; a="66654788" X-IronPort-AV: E=Sophos;i="6.20,246,1758610800"; d="scan'208";a="66654788" Received: from orviesa007.jf.intel.com ([10.64.159.147]) by orvoesa111.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Dec 2025 06:47:51 -0800 X-CSE-ConnectionGUID: U8l/okhdSQK3xKnNblGeaw== X-CSE-MsgGUID: XhAC6MyXRxe1UNFYMc0CFQ== X-ExtLoop1: 1 X-IronPort-AV: E=Sophos;i="6.20,246,1758610800"; d="scan'208";a="194733512" Received: from dhhellew-desk2.ger.corp.intel.com (HELO localhost) ([10.245.245.81]) by orviesa007-auth.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 03 Dec 2025 06:47:45 -0800 Date: Wed, 3 Dec 2025 16:47:43 +0200 From: Andy Shevchenko To: "Hajda, Andrzej" Cc: Petr Mladek , 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, 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> <94c02fb2-3407-4efc-a80f-305140e64b94@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=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <94c02fb2-3407-4efc-a80f-305140e64b94@intel.com> Organization: Intel Finland Oy - BIC 0357606-4 - c/o Alberga Business Park, 6 krs, Bertel Jungin Aukio 5, 02600 Espoo 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