From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by smtp.lore.kernel.org (Postfix) with ESMTP id 3B49CC4829A for ; Tue, 13 Feb 2024 06:18:29 +0000 (UTC) Received: by kanga.kvack.org (Postfix) id 6DBE36B0071; Tue, 13 Feb 2024 01:18:28 -0500 (EST) Received: by kanga.kvack.org (Postfix, from userid 40) id 664036B0074; Tue, 13 Feb 2024 01:18:28 -0500 (EST) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 504286B0075; Tue, 13 Feb 2024 01:18:28 -0500 (EST) X-Delivered-To: linux-mm@kvack.org Received: from relay.hostedemail.com (smtprelay0015.hostedemail.com [216.40.44.15]) by kanga.kvack.org (Postfix) with ESMTP id 39EAD6B0071 for ; Tue, 13 Feb 2024 01:18:28 -0500 (EST) Received: from smtpin03.hostedemail.com (a10.router.float.18 [10.200.18.1]) by unirelay03.hostedemail.com (Postfix) with ESMTP id CDACEA037D for ; Tue, 13 Feb 2024 06:18:27 +0000 (UTC) X-FDA: 81785776254.03.563B427 Received: from bmailout3.hostsharing.net (bmailout3.hostsharing.net [176.9.242.62]) by imf18.hostedemail.com (Postfix) with ESMTP id DFEDE1C0008 for ; Tue, 13 Feb 2024 06:18:25 +0000 (UTC) Authentication-Results: imf18.hostedemail.com; dkim=none; dmarc=none; spf=none (imf18.hostedemail.com: domain of foo00@h08.hostsharing.net has no SPF policy when checking 176.9.242.62) smtp.mailfrom=foo00@h08.hostsharing.net ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=hostedemail.com; s=arc-20220608; t=1707805106; h=from:from:sender:reply-to:subject:subject:date:date: message-id:message-id:to:to:cc:cc:mime-version:mime-version: content-type:content-type:content-transfer-encoding: in-reply-to:in-reply-to:references:references; bh=p3HZLf+xgHEQoSr3VwfNWJQnLyp5HWNwBRq3fjNEhfA=; b=enMfq9NrVWjmiUlSDXO794YnX0W+L2i6ZtmpcQBwtaWbX16mZOPqGS+pUay5kb/Khroa/g XzymVRg2mJ9d6IivJpCqQvvNJ35eypNh4yPK1yMz8wLuVoGKmnDqBZLUrz3fjHczUaopTP 66TloZx/zHFm9JWuP3vFOGwJCZOKA4A= ARC-Authentication-Results: i=1; imf18.hostedemail.com; dkim=none; dmarc=none; spf=none (imf18.hostedemail.com: domain of foo00@h08.hostsharing.net has no SPF policy when checking 176.9.242.62) smtp.mailfrom=foo00@h08.hostsharing.net ARC-Seal: i=1; s=arc-20220608; d=hostedemail.com; t=1707805106; a=rsa-sha256; cv=none; b=bfhGcKGsWc06E2TaNyaULRqYAjj0VtzMb5vT2H5YHKAJTtZxU0n2YsQaM1FSSKwgPNc7W0 EhmVDRV7O6pq/wNRcqwPF8ld1mS+mH2ZDQbL1cN8Q5dVQQNYGfgAAg772j0006y2Io68o/ hlDUeiFikNtoGlBuoJML118LJOH+zBY= Received: from h08.hostsharing.net (h08.hostsharing.net [83.223.95.28]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) key-exchange X25519 server-signature RSA-PSS (4096 bits) server-digest SHA256 client-signature RSA-PSS (4096 bits) client-digest SHA256) (Client CN "*.hostsharing.net", Issuer "RapidSSL TLS RSA CA G1" (verified OK)) by bmailout3.hostsharing.net (Postfix) with ESMTPS id A88B0100DECB7; Tue, 13 Feb 2024 07:18:23 +0100 (CET) Received: by h08.hostsharing.net (Postfix, from userid 100393) id 78FE7473ECA; Tue, 13 Feb 2024 07:18:23 +0100 (CET) Date: Tue, 13 Feb 2024 07:18:23 +0100 From: Lukas Wunner To: Dan Williams Cc: Linus Torvalds , Mathieu Desnoyers , Arnd Bergmann , Dave Chinner , linux-kernel@vger.kernel.org, Andrew Morton , Vishal Verma , Dave Jiang , Matthew Wilcox , Russell King , linux-arch@vger.kernel.org, linux-cxl@vger.kernel.org, linux-fsdevel@vger.kernel.org, linux-mm@kvack.org, linux-xfs@vger.kernel.org, dm-devel@lists.linux.dev, nvdimm@lists.linux.dev, linux-s390@vger.kernel.org, Alasdair Kergon , Mike Snitzer , Mikulas Patocka Subject: Re: [PATCH v5 5/8] virtio: Treat alloc_dax() -EOPNOTSUPP failure as non-fatal Message-ID: <20240213061823.GB27995@wunner.de> References: <20240212163101.19614-1-mathieu.desnoyers@efficios.com> <20240212163101.19614-6-mathieu.desnoyers@efficios.com> <65ca95d086dfd_d2d429470@dwillia2-xfh.jf.intel.com.notmuch> <65caa3966caa_5a7f294cf@dwillia2-xfh.jf.intel.com.notmuch> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <65caa3966caa_5a7f294cf@dwillia2-xfh.jf.intel.com.notmuch> User-Agent: Mutt/1.10.1 (2018-07-13) X-Rspam-User: X-Stat-Signature: z9sw4ozs5ficorykm6e6tribqcppw937 X-Rspamd-Server: rspam07 X-Rspamd-Queue-Id: DFEDE1C0008 X-HE-Tag: 1707805105-742433 X-HE-Meta: U2FsdGVkX18X3VU9Oi7Ml0gWFJ0l9EmR6EJxkmWG6spZEEEsTS0RcWsKwtXDtmp2Yzy7enCSGQC5Z9HLhnzQbhGEvOcdujrK3krp27zwN2Op4YOt4aEziFgDYRF01+2zCOBoK42vYsSEWhZlXYc/vEMzmy08gGnhvDEd8GEljXM+KkG/kcMjX2NpjWhC87lYc98kN8RL2fNNf+jsLMIRgh4gjXtuIaqrF1st0Qhzr/swfH0EM3gLSMAtSwLM8V0583iLQfQsXLe4sP+YM1qMT5IcKDtL5BpuuY69K//WluXdNOC48JK39dVdMhpR8RFZk966s9PtgLtL+hjX2fGXD3sD1cKWfRSUTbm7L12Zs+tHOq7c7PfoUWMmX3gHJiAXIo2XZ3NBB5eMWjckEIo3Xdzct60XF5s2kkpVEq93QGZ+4Pg3R2IHm19oxBX+8k8qDtIDOFLdG49EiA3j5e0SvAq7qxx6A4bIsyoA8zcvFz1ENFKYMC0rgXzePxzEEEcMk1MrXjvHhF+/vFSwz378/ePEgtGY9BPLDo9vMgVzLjxwejtlu44fZdUZZLAAQ5WkT4nqMxOGmxTLTMYvrrsAQzyzK323Y79yOR7vKVUbPiS8mWPlQBUJOEyzEfcTp/l21tEZB9uA0QWBdoZKV0pbo+vLRvOJ+pv4iTkL0vcnOoXPe8OKRREMPhLaMv+EtvQGbbQX/M64i3Q887RCSnf7VtB7tR6az4pQJ5BRkUDuyZ5AhHwMh+LwkDOtaiEu5gasQc3pNvjmpKA6191LHoPxrYbZzEVqJQ92rjl3TuZ+LSQAzy292kpBOIBAHO3/SdawqXey5mJfoFY716fNeOM8v34MaeEk0Ssf4oviGVvjMRcw5xQmexHEI94JXvFWGr5kk4Vu4Y8Q601HGjHjFUL/8BdQHCaj3EOyq/z9fwBs4mxNYWU9en1bX+L1FAlVJIzv15djaAbHKuKTft1vU6L xZB0VE2Q xalAhUjQ6/iQRe/8= X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: List-Subscribe: List-Unsubscribe: On Mon, Feb 12, 2024 at 03:02:46PM -0800, Dan Williams wrote: > However, Lukas, I think Linus is right, your DEFINE_FREE() should use > IS_ERR_OR_NULL(). Uh... that's a negative, sir. ;) IS_ERR_OR_NULL() results in... * a superfluous NULL pointer check in x509_key_preparse() and * a superfluous IS_ERR check in x509_cert_parse(). IS_ERR() results *only* in... * a superfluous IS_ERR check in x509_cert_parse(). I can get rid of the IS_ERR() check by using assume(). I can *not* get rid of the NULL pointer check because the compiler is compiled with -fno-delete-null-pointer-checks. (The compiler seems to ignore __attribute__((returns_nonnull)) due to that.) > I.e. the problem is trying to use > __free(x509_free_certificate) in x509_cert_parse(). > > > --- a/crypto/asymmetric_keys/x509_cert_parser.c > > +++ b/crypto/asymmetric_keys/x509_cert_parser.c > > @@ -60,24 +60,24 @@ void x509_free_certificate(struct x509_certificate *cert) > > */ > > struct x509_certificate *x509_cert_parse(const void *data, size_t datalen) > > { > > - struct x509_certificate *cert; > > - struct x509_parse_context *ctx; > > + struct x509_certificate *cert __free(x509_free_certificate); > > ...make this: > > struct x509_certificate *cert __free(kfree); That doesn't work I'm afraid. x509_cert_parse() needs x509_free_certificate() to be called in the error path, not kfree(). See the existing code in current mainline: x509_cert_parse() populates three sub-allocations in struct x509_certificate (pub, sig, id) and two sub-sub-allocations (pub->key, pub->params). So I'd have to add five additional local variables which get freed by __cleanup(). One of them (pub->key) requires kfree_sensitive() instead of kfree(), so I'd need an extra DEFINE_FREE() for that. I haven't tried it but I suspect the result would look terrible and David Howells wouldn't like it. > ...and Mathieu, this should be IS_ERR_OR_NULL() to skip an unnecessary > call to virtio_fs_cleanup_dax() at function exit that the compiler > should elide. My recommendation is to check for !IS_ERR() in the DEFINE_FREE() clause and amend virtio_fs_cleanup_dax() with a "if (!dax_dev) return;" for defensiveness in case someone calls it with a NULL pointer. That's the best solution I could come up with for the x509_certificate conversion. Note that even with superfluous checks avoided, __cleanup() causes gcc-12 to always generate two return paths. It's very visible in the generated code that all the stack unwinding code gets duplicated in every function using __cleanup(). The existing Assembler code of x509_key_preparse() and x509_cert_parse(), without __cleanup() invocation, has only a single return path. So __cleanup() bloats the code regardless of superfluous checks, but future gcc versions might avoid that. clang-15 generates much more compact code (vmlinux is a couple hundred kBytes smaller), but does weird things such as inlining x509_free_certificate() in x509_cert_parse(). As you may have guessed, I've spent an inordinate amount of time down that rabbit hole. ;( Thanks, Lukas