All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create
@ 2021-01-18 13:33 Tianjia Zhang
  2021-01-19 20:29 ` Sean Christopherson
  2021-01-20 14:39 ` Jarkko Sakkinen
  0 siblings, 2 replies; 3+ messages in thread
From: Tianjia Zhang @ 2021-01-18 13:33 UTC (permalink / raw)
  To: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Andrew Morton, Shuah Khan, haitao.huang,
	Kai Huang, x86, linux-sgx, linux-kernel, linux-mm,
	linux-kselftest, Jia Zhang
  Cc: Tianjia Zhang

In this scenario, there is no case where va_page is NULL, and
the error has been checked. The if condition statement here is
redundant, so remove the condition detection.

Reported-by: Jia Zhang <zhang.jia@linux.alibaba.com>
Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
---
 arch/x86/kernel/cpu/sgx/ioctl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
index 90a5caf76939..f45957c05f69 100644
--- a/arch/x86/kernel/cpu/sgx/ioctl.c
+++ b/arch/x86/kernel/cpu/sgx/ioctl.c
@@ -66,9 +66,8 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
 	va_page = sgx_encl_grow(encl);
 	if (IS_ERR(va_page))
 		return PTR_ERR(va_page);
-	else if (va_page)
-		list_add(&va_page->list, &encl->va_pages);
-	/* else the tail page of the VA page list had free slots. */
+
+	list_add(&va_page->list, &encl->va_pages);
 
 	/* The extra page goes to SECS. */
 	encl_size = secs->size + PAGE_SIZE;
-- 
2.19.1.3.ge56e4f7


^ permalink raw reply related	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create
  2021-01-18 13:33 [PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
@ 2021-01-19 20:29 ` Sean Christopherson
  2021-01-20 14:39 ` Jarkko Sakkinen
  1 sibling, 0 replies; 3+ messages in thread
From: Sean Christopherson @ 2021-01-19 20:29 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Jarkko Sakkinen, Thomas Gleixner, Ingo Molnar, Borislav Petkov,
	H. Peter Anvin, Andrew Morton, Shuah Khan, haitao.huang,
	Kai Huang, x86, linux-sgx, linux-kernel, linux-mm,
	linux-kselftest, Jia Zhang

On Mon, Jan 18, 2021, Tianjia Zhang wrote:
> In this scenario, there is no case where va_page is NULL, and
> the error has been checked. The if condition statement here is
> redundant, so remove the condition detection.
> 
> Reported-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>
> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 90a5caf76939..f45957c05f69 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -66,9 +66,8 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
>  	va_page = sgx_encl_grow(encl);
>  	if (IS_ERR(va_page))
>  		return PTR_ERR(va_page);
> -	else if (va_page)
> -		list_add(&va_page->list, &encl->va_pages);
> -	/* else the tail page of the VA page list had free slots. */
> +
> +	list_add(&va_page->list, &encl->va_pages);

Maybe add a WARN_ONCE to document/enforce the behavior?

	va_page = sgx_encl_grow(encl);
	if (IS_ERR(va_page))
		return PTR_ERR(va_page);

	if (WARN_ONCE(!va_page, "clever comment goes here))
		return -EIO;

	list_add(&va_page->list, &encl->va_pages);

>  
>  	/* The extra page goes to SECS. */
>  	encl_size = secs->size + PAGE_SIZE;
> -- 
> 2.19.1.3.ge56e4f7
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: [PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create
  2021-01-18 13:33 [PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
  2021-01-19 20:29 ` Sean Christopherson
@ 2021-01-20 14:39 ` Jarkko Sakkinen
  1 sibling, 0 replies; 3+ messages in thread
From: Jarkko Sakkinen @ 2021-01-20 14:39 UTC (permalink / raw)
  To: Tianjia Zhang
  Cc: Thomas Gleixner, Ingo Molnar, Borislav Petkov, H. Peter Anvin,
	Andrew Morton, Shuah Khan, haitao.huang, Kai Huang, x86,
	linux-sgx, linux-kernel, linux-mm, linux-kselftest, Jia Zhang

On Mon, Jan 18, 2021 at 09:33:58PM +0800, Tianjia Zhang wrote:
> In this scenario, there is no case where va_page is NULL, and
> the error has been checked. The if condition statement here is
> redundant, so remove the condition detection.
> 
> Reported-by: Jia Zhang <zhang.jia@linux.alibaba.com>
> Signed-off-by: Tianjia Zhang <tianjia.zhang@linux.alibaba.com>

What do we gain with this change and why is it reported as a
regression?

/Jarkko

> ---
>  arch/x86/kernel/cpu/sgx/ioctl.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c b/arch/x86/kernel/cpu/sgx/ioctl.c
> index 90a5caf76939..f45957c05f69 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -66,9 +66,8 @@ static int sgx_encl_create(struct sgx_encl *encl, struct sgx_secs *secs)
>  	va_page = sgx_encl_grow(encl);
>  	if (IS_ERR(va_page))
>  		return PTR_ERR(va_page);
> -	else if (va_page)
> -		list_add(&va_page->list, &encl->va_pages);
> -	/* else the tail page of the VA page list had free slots. */
> +
> +	list_add(&va_page->list, &encl->va_pages);
>  
>  	/* The extra page goes to SECS. */
>  	encl_size = secs->size + PAGE_SIZE;
> -- 
> 2.19.1.3.ge56e4f7
> 
> 

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2021-01-20 14:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-01-18 13:33 [PATCH] x86/sgx: Remove redundant if conditions in sgx_encl_create Tianjia Zhang
2021-01-19 20:29 ` Sean Christopherson
2021-01-20 14:39 ` Jarkko Sakkinen

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.