Message ID | 169199901230.1782217.9803098171993981037.stgit@dwillia2-xfh.jf.intel.com |
---|---|
State | New |
Headers |
Return-Path: <linux-kernel-owner@vger.kernel.org> Delivered-To: ouuuleilei@gmail.com Received: by 2002:a59:b824:0:b0:3f2:4152:657d with SMTP id z4csp2607050vqi; Mon, 14 Aug 2023 01:49:00 -0700 (PDT) X-Google-Smtp-Source: AGHT+IF+j70lb5Ydy8JL8/aZCJe3haoE6ZOFRtQCqEQtYWL/jOr5fd+HziEgE2367XT/dhgButZk X-Received: by 2002:a05:6808:20a5:b0:3a7:5309:f24 with SMTP id s37-20020a05680820a500b003a753090f24mr11430464oiw.49.1692002939814; Mon, 14 Aug 2023 01:48:59 -0700 (PDT) ARC-Seal: i=1; a=rsa-sha256; t=1692002939; cv=none; d=google.com; s=arc-20160816; b=f7KOJQ2YO7Q450egamNCdhXTWljcXozW7ePAaHwE/1aFSK1CuDlrIzSuoSjTsAhPcE M9hPMaOBr0dUukX6sV+7XXST7AGYNUzaXdpEjD1TNtQjEfkS4mOieO6y2LFlo/I9rgja 7fS4GZUF9yaAzUllr/5R6yPdTazSVyLk+30PQ1wSwN1SqpKXpDYbde93USCetkcDml32 i1r39+KRZJmDjyDsd0LKBCHM5bJpDsuvk1GQ9LNetbCJF9Ng1odJ6yR20gCWfCO8T7N8 TvJiLqK1cBJbTPd7lioa7vOERNA+McaKYbMQKiB0zd47RUuW3ekY1FWhZgjqj7tyOxUq s+qA== ARC-Message-Signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=google.com; s=arc-20160816; h=list-id:precedence:content-transfer-encoding:mime-version :user-agent:references:in-reply-to:message-id:date:cc:to:from :subject:dkim-signature; bh=EMMaKjcmf0w6Qeck97k95pB3VR1xSDdwNYw3ZCcMyNg=; fh=PAI56yEu0AiuX8TT+Q3eaKO8S7fFZxUvPBCA6bNffeU=; b=gqkQRNRjdqtMFn7aSGDc5ZyuZRtwDOY/cIOAu3XaQ3lWQJHc1smlqv+yQUec0GokZ0 TUJJ6qy73bb1R9BU9iS2UwikVjDwSxqf7ovHiblIbfqfSumijqCSB/HFb7BYzCsyN5or RSPS2fqR5I7X/+r+Wc0464nhhvhHsXPrwdUdnahouvmhC6Gb3Z+Z/MgPMXzNDVkEK4Yv dOwQj4lV1J15G1xyak+c5M3C7MK+tY8F2QLNhdXemRlD+ruJV51at9btymFF45nXY4MG 75WxaUHt03mFzANPMGijgNdjEyJl2NWmeHU5bFQM1cSGxyPO3SYun21JxbZvg4bYJdQy qACA== ARC-Authentication-Results: i=1; mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=Y2um6b59; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: from out1.vger.email (out1.vger.email. [2620:137:e000::1:20]) by mx.google.com with ESMTP id c6-20020a6566c6000000b00530b3b98fc5si7971040pgw.417.2023.08.14.01.48.47; Mon, 14 Aug 2023 01:48:59 -0700 (PDT) Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; Authentication-Results: mx.google.com; dkim=pass header.i=@intel.com header.s=Intel header.b=Y2um6b59; spf=pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) smtp.mailfrom=linux-kernel-owner@vger.kernel.org; dmarc=pass (p=NONE sp=NONE dis=NONE) header.from=intel.com Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S234324AbjHNHnx (ORCPT <rfc822;274620705z@gmail.com> + 99 others); Mon, 14 Aug 2023 03:43:53 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:42150 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S234260AbjHNHne (ORCPT <rfc822;linux-kernel@vger.kernel.org>); Mon, 14 Aug 2023 03:43:34 -0400 Received: from mgamail.intel.com (mgamail.intel.com [134.134.136.100]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id A6A8294 for <linux-kernel@vger.kernel.org>; Mon, 14 Aug 2023 00:43:33 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=intel.com; i=@intel.com; q=dns/txt; s=Intel; t=1691999013; x=1723535013; h=subject:from:to:cc:date:message-id:in-reply-to: references:mime-version:content-transfer-encoding; bh=RReoo07Z8COfyQe4vCo2LnNBVJrUhMSozbnkyWWcdFM=; b=Y2um6b59bFJq2QJ28RbMGjgW5fghkFwKnxV4/IGQ93qnHukybI3wKcJ9 pNtfAfMDH26fb2byq/CsMn0nj8mhp8fMkvBZ9EVX0/Ty8zw7jmycQGa2L U5HdBj1HyshJiw2SEcwF6TY61yW+TZqJxi2HyNFebdpawMNkhbS5G+YlH Vx1n5PLDqAB8uVnZt/SutaZV/2hzuuOp5sl3DHJV5UzM0j3Y9TTLIvXUJ 7MTtP3/WEy0Q3rg8SwblLnE46YYCQ85spJv6rQsivlzrjW1FfE2tKBuMb BH6a0VSvw/vUAabdryqxCDmfbAzmkNmY4OQEAVss5KRIsF7Wi1OKSmIr8 Q==; X-IronPort-AV: E=McAfee;i="6600,9927,10801"; a="438314744" X-IronPort-AV: E=Sophos;i="6.01,172,1684825200"; d="scan'208";a="438314744" Received: from fmsmga004.fm.intel.com ([10.253.24.48]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Aug 2023 00:43:33 -0700 X-ExtLoop1: 1 X-IronPort-AV: E=McAfee;i="6600,9927,10801"; a="803372565" X-IronPort-AV: E=Sophos;i="6.01,172,1684825200"; d="scan'208";a="803372565" Received: from navanban-mobl.amr.corp.intel.com (HELO dwillia2-xfh.jf.intel.com) ([10.209.127.25]) by fmsmga004-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 14 Aug 2023 00:43:32 -0700 Subject: [PATCH v2 4/5] mm/slab: Add __free() support for kvfree From: Dan Williams <dan.j.williams@intel.com> To: linux-coco@lists.linux.dev Cc: Andrew Morton <akpm@linux-foundation.org>, Peter Zijlstra <peterz@infradead.org>, Greg Kroah-Hartman <gregkh@linuxfoundation.org>, x86@kernel.org, linux-kernel@vger.kernel.org Date: Mon, 14 Aug 2023 00:43:32 -0700 Message-ID: <169199901230.1782217.9803098171993981037.stgit@dwillia2-xfh.jf.intel.com> In-Reply-To: <169199898909.1782217.10899362240465838600.stgit@dwillia2-xfh.jf.intel.com> References: <169199898909.1782217.10899362240465838600.stgit@dwillia2-xfh.jf.intel.com> User-Agent: StGit/0.18-3-g996c MIME-Version: 1.0 Content-Type: text/plain; charset="utf-8" Content-Transfer-Encoding: 7bit X-Spam-Status: No, score=-4.4 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH, DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,RCVD_IN_DNSWL_MED, SPF_HELO_NONE,SPF_NONE autolearn=ham autolearn_force=no version=3.4.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on lindbergh.monkeyblade.net Precedence: bulk List-ID: <linux-kernel.vger.kernel.org> X-Mailing-List: linux-kernel@vger.kernel.org X-getmail-retrieved-from-mailbox: INBOX X-GMAIL-THRID: 1774193674284446808 X-GMAIL-MSGID: 1774193674284446808 |
Series |
tsm: Attestation Report ABI
|
|
Commit Message
Dan Williams
Aug. 14, 2023, 7:43 a.m. UTC
Allow for the declaration of variables that trigger kvfree() when they
go out of scope.
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
Signed-off-by: Dan Williams <dan.j.williams@intel.com>
---
include/linux/slab.h | 2 ++
1 file changed, 2 insertions(+)
Comments
On Mon, Aug 14, 2023 at 12:43:32AM -0700, Dan Williams wrote: > Allow for the declaration of variables that trigger kvfree() when they > go out of scope. > > Cc: Andrew Morton <akpm@linux-foundation.org> > Cc: Peter Zijlstra <peterz@infradead.org> > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > --- > include/linux/slab.h | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/include/linux/slab.h b/include/linux/slab.h > index 848c7c82ad5a..241025367943 100644 > --- a/include/linux/slab.h > +++ b/include/linux/slab.h > @@ -746,6 +746,8 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t size, gfp_t fla > extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t flags) > __realloc_size(3); > extern void kvfree(const void *addr); > +DEFINE_FREE(kvfree, void *, if (_T) kvfree(_T)) No need to check _T before calling this, right (as was also pointed out earlier). thanks, greg k-h
On Mon, Aug 14, 2023 at 06:17:31PM +0200, Peter Zijlstra wrote: > On Mon, Aug 14, 2023 at 05:31:27PM +0200, Greg Kroah-Hartman wrote: > > On Mon, Aug 14, 2023 at 12:43:32AM -0700, Dan Williams wrote: > > > Allow for the declaration of variables that trigger kvfree() when they > > > go out of scope. > > > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > Cc: Peter Zijlstra <peterz@infradead.org> > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > --- > > > include/linux/slab.h | 2 ++ > > > 1 file changed, 2 insertions(+) > > > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > > index 848c7c82ad5a..241025367943 100644 > > > --- a/include/linux/slab.h > > > +++ b/include/linux/slab.h > > > @@ -746,6 +746,8 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t size, gfp_t fla > > > extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t flags) > > > __realloc_size(3); > > > extern void kvfree(const void *addr); > > > +DEFINE_FREE(kvfree, void *, if (_T) kvfree(_T)) > > > > No need to check _T before calling this, right (as was also pointed out > > earlier). > > Well, that does mean you get an unconditional call to kvfree() in the > success case. Linus argued against this. > > This way the compiler sees: > > buf = NULL; > if (buf) > kvfree(buf); > > and goes: 'let me clean that up for you'. And all is well. Have you actually verified that assumption in the generated Assembler code? The kernel is compiled with -fno-delete-null-pointer-checks since commit a3ca86aea507 ("Add '-fno-delete-null-pointer-checks' to gcc CFLAGS"). So NULL pointer checks are *not* optimized away even if the compiler knows that a pointer is NULL. Background story: https://lwn.net/Articles/342330/ Thanks, Lukas
Lukas Wunner wrote: > On Mon, Aug 14, 2023 at 06:17:31PM +0200, Peter Zijlstra wrote: > > On Mon, Aug 14, 2023 at 05:31:27PM +0200, Greg Kroah-Hartman wrote: > > > On Mon, Aug 14, 2023 at 12:43:32AM -0700, Dan Williams wrote: > > > > Allow for the declaration of variables that trigger kvfree() when they > > > > go out of scope. > > > > > > > > Cc: Andrew Morton <akpm@linux-foundation.org> > > > > Cc: Peter Zijlstra <peterz@infradead.org> > > > > Cc: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > > Signed-off-by: Dan Williams <dan.j.williams@intel.com> > > > > --- > > > > include/linux/slab.h | 2 ++ > > > > 1 file changed, 2 insertions(+) > > > > > > > > diff --git a/include/linux/slab.h b/include/linux/slab.h > > > > index 848c7c82ad5a..241025367943 100644 > > > > --- a/include/linux/slab.h > > > > +++ b/include/linux/slab.h > > > > @@ -746,6 +746,8 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t size, gfp_t fla > > > > extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t flags) > > > > __realloc_size(3); > > > > extern void kvfree(const void *addr); > > > > +DEFINE_FREE(kvfree, void *, if (_T) kvfree(_T)) > > > > > > No need to check _T before calling this, right (as was also pointed out > > > earlier). > > > > Well, that does mean you get an unconditional call to kvfree() in the > > success case. Linus argued against this. > > > > This way the compiler sees: > > > > buf = NULL; > > if (buf) > > kvfree(buf); > > > > and goes: 'let me clean that up for you'. And all is well. > > Have you actually verified that assumption in the generated Assembler code? > > The kernel is compiled with -fno-delete-null-pointer-checks since commit > a3ca86aea507 ("Add '-fno-delete-null-pointer-checks' to gcc CFLAGS"). > > So NULL pointer checks are *not* optimized away even if the compiler > knows that a pointer is NULL. Interesting, I am not sure how -fno-delete-null-pointer-checks plays into this, but I can confirm that Peter's expectations are being met in a routine with: DEFINE_FREE(pci_dev_put, struct pci_dev *, if (_T) pci_dev_put(_T)) ...without that conditional the assembly is: 0xffffffff819ad129 <+41>: call 0xffffffff81800840 <pci_get_domain_bus_and_slot> 0xffffffff819ad12e <+46>: mov %rax,%r12 0xffffffff819ad131 <+49>: test %rax,%rax 0xffffffff819ad134 <+52>: je 0xffffffff819ad154 <cxl_cper_event_call+84> 0xffffffff819ad136 <+54>: mov %rax,%rdi 0xffffffff819ad139 <+57>: call 0xffffffff817f5f10 <pci_dev_lock> 0xffffffff819ad13e <+62>: cmpq $0xffffffff82c681c0,0x80(%r12) 0xffffffff819ad14a <+74>: je 0xffffffff819ad160 <cxl_cper_event_call+96> 0xffffffff819ad14c <+76>: mov %r12,%rdi 0xffffffff819ad14f <+79>: call 0xffffffff817f5fa0 <pci_dev_unlock> 0xffffffff819ad154 <+84>: pop %rbx 0xffffffff819ad155 <+85>: mov %r12,%rdi 0xffffffff819ad158 <+88>: pop %rbp 0xffffffff819ad159 <+89>: pop %r12 0xffffffff819ad15b <+91>: jmp 0xffffffff817fe1e0 <pci_dev_put> ...i.e. the check for NULL at 0xffffffff819ad134 jumps to do an unnecessary pci_dev_put(). With the conditional in the macro the sequence is: 0xffffffff819ad129 <+41>: call 0xffffffff81800840 <pci_get_domain_bus_and_slot> 0xffffffff819ad12e <+46>: test %rax,%rax 0xffffffff819ad131 <+49>: je 0xffffffff819ad18c <cxl_cper_event_call+140> 0xffffffff819ad133 <+51>: mov %rax,%r12 0xffffffff819ad136 <+54>: mov %rax,%rdi 0xffffffff819ad139 <+57>: call 0xffffffff817f5f10 <pci_dev_lock> 0xffffffff819ad13e <+62>: cmpq $0xffffffff82c681c0,0x80(%r12) 0xffffffff819ad14a <+74>: je 0xffffffff819ad160 <cxl_cper_event_call+96> 0xffffffff819ad14c <+76>: mov %r12,%rdi 0xffffffff819ad14f <+79>: call 0xffffffff817f5fa0 <pci_dev_unlock> ... 0xffffffff819ad18c <+140>: pop %rbx 0xffffffff819ad18d <+141>: pop %rbp 0xffffffff819ad18e <+142>: pop %r12 0xffffffff819ad190 <+144>: jmp 0xffffffff81efc6a0 <__x86_return_thunk> ...i.e. optimize away the pci_dev_put() and return directly when @pdev is already known to be NULL. So empirically -fno-delete-null-pointer-checks still allows for redundant NULL checks to be optimized.
diff --git a/include/linux/slab.h b/include/linux/slab.h index 848c7c82ad5a..241025367943 100644 --- a/include/linux/slab.h +++ b/include/linux/slab.h @@ -746,6 +746,8 @@ static inline __alloc_size(1, 2) void *kvcalloc(size_t n, size_t size, gfp_t fla extern void *kvrealloc(const void *p, size_t oldsize, size_t newsize, gfp_t flags) __realloc_size(3); extern void kvfree(const void *addr); +DEFINE_FREE(kvfree, void *, if (_T) kvfree(_T)) + extern void kvfree_sensitive(const void *addr, size_t len); unsigned int kmem_cache_size(struct kmem_cache *s);