diff mbox series

KVM: X86: Remove unnecessary GFP_KERNEL_ACCOUNT for temporary variables

Message ID 20240624012016.46133-1-flyingpeng@tencent.com (mailing list archive)
State New, archived
Headers show
Series KVM: X86: Remove unnecessary GFP_KERNEL_ACCOUNT for temporary variables | expand

Commit Message

Hao Peng June 24, 2024, 1:20 a.m. UTC 
From: Peng Hao <flyingpeng@tencent.com>

Some variables allocated in kvm_arch_vcpu_ioctl are released when
the function exits, so there is no need to set GFP_KERNEL_ACCOUNT.

Signed-off-by: Peng Hao <flyingpeng@tencent.com>
---
 arch/x86/kvm/x86.c | 8 ++++----
 1 file changed, 4 insertions(+), 4 deletions(-)

Comments

Sean Christopherson June 28, 2024, 10:55 p.m. UTC | #1 
On Mon, 24 Jun 2024 09:20:16 +0800, flyingpenghao@gmail.com wrote:
> Some variables allocated in kvm_arch_vcpu_ioctl are released when
> the function exits, so there is no need to set GFP_KERNEL_ACCOUNT.

Applied to kvm-x86 misc, thanks!

[1/1] KVM: X86: Remove unnecessary GFP_KERNEL_ACCOUNT for temporary variables
      https://github.com/kvm-x86/linux/commit/dd103407ca31

--
https://github.com/kvm-x86/linux/tree/next
Huang, Kai June 29, 2024, 2:41 a.m. UTC | #2 
On Fri, 2024-06-28 at 15:55 -0700, Sean Christopherson wrote:
> On Mon, 24 Jun 2024 09:20:16 +0800, flyingpenghao@gmail.com wrote:
> > Some variables allocated in kvm_arch_vcpu_ioctl are released when
> > the function exits, so there is no need to set GFP_KERNEL_ACCOUNT.
> 
> Applied to kvm-x86 misc, thanks!
> 
> [1/1] KVM: X86: Remove unnecessary GFP_KERNEL_ACCOUNT for temporary variables
>       https://github.com/kvm-x86/linux/commit/dd103407ca31
> 
> --
> https://github.com/kvm-x86/linux/tree/next
> 

Hi Sean,

I thought we should use _ACCOUNT even for temporary variables.

Should I send a patch to do below? :-)

--- a/arch/x86/kvm/vmx/sgx.c
+++ b/arch/x86/kvm/vmx/sgx.c
@@ -274,7 +274,7 @@ static int handle_encls_ecreate(struct kvm_vcpu *vcpu)
         * simultaneously set SGX_ATTR_PROVISIONKEY to bypass the check to
         * enforce restriction of access to the PROVISIONKEY.
         */
-       contents = (struct sgx_secs *)__get_free_page(GFP_KERNEL_ACCOUNT);
+       contents = (struct sgx_secs *)__get_free_page(GFP_KERNEL);
        if (!contents)
                return -ENOMEM;
Sean Christopherson July 29, 2024, 3:31 p.m. UTC | #3 
On Sat, Jun 29, 2024, Kai Huang wrote:
> On Fri, 2024-06-28 at 15:55 -0700, Sean Christopherson wrote:
> > On Mon, 24 Jun 2024 09:20:16 +0800, flyingpenghao@gmail.com wrote:
> > > Some variables allocated in kvm_arch_vcpu_ioctl are released when
> > > the function exits, so there is no need to set GFP_KERNEL_ACCOUNT.
> > 
> > Applied to kvm-x86 misc, thanks!
> > 
> > [1/1] KVM: X86: Remove unnecessary GFP_KERNEL_ACCOUNT for temporary variables
> >       https://github.com/kvm-x86/linux/commit/dd103407ca31
> > 
> > --
> > https://github.com/kvm-x86/linux/tree/next
> > 
> 
> Hi Sean,

Sorry, lost this at the bottom of my inbox.

> I thought we should use _ACCOUNT even for temporary variables.

Heh, that's what I thought too.

[*] https://lore.kernel.org/all/c0122f66-f428-417e-a360-b25fc0f154a0@p183
Huang, Kai July 30, 2024, 12:51 a.m. UTC | #4 
On 30/07/2024 3:31 am, Sean Christopherson wrote:
> On Sat, Jun 29, 2024, Kai Huang wrote:
>> On Fri, 2024-06-28 at 15:55 -0700, Sean Christopherson wrote:
>>> On Mon, 24 Jun 2024 09:20:16 +0800, flyingpenghao@gmail.com wrote:
>>>> Some variables allocated in kvm_arch_vcpu_ioctl are released when
>>>> the function exits, so there is no need to set GFP_KERNEL_ACCOUNT.
>>>
>>> Applied to kvm-x86 misc, thanks!
>>>
>>> [1/1] KVM: X86: Remove unnecessary GFP_KERNEL_ACCOUNT for temporary variables
>>>        https://github.com/kvm-x86/linux/commit/dd103407ca31
>>>
>>> --
>>> https://github.com/kvm-x86/linux/tree/next
>>>
>>
>> Hi Sean,
> 
> Sorry, lost this at the bottom of my inbox.

Thanks for getting back to this. :-)

> 
>> I thought we should use _ACCOUNT even for temporary variables.
> 
> Heh, that's what I thought too.
> 
> [*] https://lore.kernel.org/all/c0122f66-f428-417e-a360-b25fc0f154a0@p183
> 

Thanks for the info.

Looking at Paolo's reply and by applying this patch, it seems we agreed 
we can just use GFP_KERNEL (e.g., GFP_USER isn't needed too)?

Anyway I've sent a patch to change handle_encls_ecreate():

https://lore.kernel.org/kvm/20240715101224.90958-1-kai.huang@intel.com/T/
diff mbox series

Patch

diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 0763a0f72a06..bfddc04eae40 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -5880,7 +5880,7 @@  long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (!lapic_in_kernel(vcpu))
 			goto out;
 		u.lapic = kzalloc(sizeof(struct kvm_lapic_state),
-				GFP_KERNEL_ACCOUNT);
+				GFP_KERNEL);
 
 		r = -ENOMEM;
 		if (!u.lapic)
@@ -6073,7 +6073,7 @@  long kvm_arch_vcpu_ioctl(struct file *filp,
 		if (vcpu->arch.guest_fpu.uabi_size > sizeof(struct kvm_xsave))
 			break;
 
-		u.xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL_ACCOUNT);
+		u.xsave = kzalloc(sizeof(struct kvm_xsave), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!u.xsave)
 			break;
@@ -6104,7 +6104,7 @@  long kvm_arch_vcpu_ioctl(struct file *filp,
 	case KVM_GET_XSAVE2: {
 		int size = vcpu->arch.guest_fpu.uabi_size;
 
-		u.xsave = kzalloc(size, GFP_KERNEL_ACCOUNT);
+		u.xsave = kzalloc(size, GFP_KERNEL);
 		r = -ENOMEM;
 		if (!u.xsave)
 			break;
@@ -6122,7 +6122,7 @@  long kvm_arch_vcpu_ioctl(struct file *filp,
 	}
 
 	case KVM_GET_XCRS: {
-		u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL_ACCOUNT);
+		u.xcrs = kzalloc(sizeof(struct kvm_xcrs), GFP_KERNEL);
 		r = -ENOMEM;
 		if (!u.xcrs)
 			break;