Re: [RFC PATCH v3 03/27] x86/sgx: Remove a warn from sgx_free_epc_page()

Dave Hansen
2021-01-26T15:41:47Z

On 1/26/21 1:30 AM, Kai Huang wrote:
> Remove SGX_EPC_PAGE_RECLAIMER_TRACKED check and warning. This cannot
> happen, as enclave pages are freed only at the time when encl->refcount
> triggers, i.e. when both VFS and the page reclaimer have given up on
> their references.
>
> Signed-off-by: Jarkko Sakkinen <jarkko@kernel.org>
> Signed-off-by: Kai Huang <kai.huang@intel.com>
> ---
> arch/x86/kernel/cpu/sgx/main.c | 2 --
> 1 file changed, 2 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
> index 8df81a3ed945..f330abdb5bb1 100644
> --- a/arch/x86/kernel/cpu/sgx/main.c
> +++ b/arch/x86/kernel/cpu/sgx/main.c
> @@ -605,8 +605,6 @@ void sgx_free_epc_page(struct sgx_epc_page *page)
> struct sgx_epc_section *section = &sgx_epc_sections[page->section];
> int ret;
>
> - WARN_ON_ONCE(page->flags & SGX_EPC_PAGE_RECLAIMER_TRACKED);

I'm all for cleaning up silly, useless warnings. But, don't we usually
put warnings in for things that we don't expect to be able to happen?

In other words, I'm fine with removing this if it hasn't been a valuable
warning and we don't expect it to become a valuable warning. But, the
changelog doesn't say that. It also doesn't explain what this patch is
doing in this series.

Why is this her?