From: "Haitao Huang" <haitao.huang@linux.intel.com>
To: linux-kernel@vger.kernel.org, linux-sgx@vger.kernel.org,
jarkko@kernel.org, dave.hansen@linux.intel.com,
"Bojun Zhu" <zhubojun.zbj@antgroup.com>
Cc: reinette.chatre@intel.com, "刘双(轩屹)" <ls123674@antgroup.com>
Subject: Re: [RFC PATCH v3 1/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup
Date: Wed, 15 May 2024 16:55:59 -0500 [thread overview]
Message-ID: <op.2nt1vls9wjvjmi@hhuan26-mobl.amr.corp.intel.com> (raw)
In-Reply-To: <20240515065521.67908-2-zhubojun.zbj@antgroup.com>
On Wed, 15 May 2024 01:55:21 -0500, Bojun Zhu <zhubojun.zbj@antgroup.com>
wrote:
> EDMM's ioctl()s support batch operations, which may be
> time-consuming. Try to explicitly give up the CPU as the prefix
> operation at the every begin of "for loop" in
> sgx_enclave_{ modify_types | restrict_permissions | remove_pages}
> to give other tasks a chance to run, and avoid softlockup warning.
>
> Additionally perform pending signals check as the prefix operation,
> and introduce sgx_check_signal_and_resched(),
> which wraps all the checks.
>
> The following has been observed on Linux v6.9-rc5 with kernel
> preemptions disabled(by configuring "PREEMPT_NONE=y"), when kernel
> is requested to restrict page permissions of a large number of EPC pages.
>
> ------------[ cut here ]------------
> watchdog: BUG: soft lockup - CPU#45 stuck for 22s!
> ...
> RIP: 0010:sgx_enclave_restrict_permissions+0xba/0x1f0
> ...
> Call Trace:
> sgx_ioctl
> __x64_sys_ioctl
> x64_sys_call
> do_syscall_64
> entry_SYSCALL_64_after_hwframe
> ------------[ end trace ]------------
>
> Signed-off-by: Bojun Zhu <zhubojun.zbj@antgroup.com>
> ---
> arch/x86/kernel/cpu/sgx/ioctl.c | 40 +++++++++++++++++++++++----------
> 1 file changed, 28 insertions(+), 12 deletions(-)
>
> diff --git a/arch/x86/kernel/cpu/sgx/ioctl.c
> b/arch/x86/kernel/cpu/sgx/ioctl.c
> index b65ab214bdf5..6199f483143e 100644
> --- a/arch/x86/kernel/cpu/sgx/ioctl.c
> +++ b/arch/x86/kernel/cpu/sgx/ioctl.c
> @@ -365,6 +365,20 @@ static int sgx_validate_offset_length(struct
> sgx_encl *encl,
> return 0;
> }
> +/*
> + * Check signals and invoke scheduler. Return true for a pending signal.
> + */
> +static bool sgx_check_signal_and_resched(void)
> +{
> + if (signal_pending(current))
> + return true;
> +
> + if (need_resched())
> + cond_resched();
> +
> + return false;
> +}
> +
> /**
> * sgx_ioc_enclave_add_pages() - The handler for
> %SGX_IOC_ENCLAVE_ADD_PAGES
> * @encl: an enclave pointer
> @@ -409,7 +423,7 @@ static long sgx_ioc_enclave_add_pages(struct
> sgx_encl *encl, void __user *arg)
> struct sgx_enclave_add_pages add_arg;
> struct sgx_secinfo secinfo;
> unsigned long c;
> - int ret;
> + int ret = -ERESTARTSYS;
> if (!test_bit(SGX_ENCL_CREATED, &encl->flags) ||
> test_bit(SGX_ENCL_INITIALIZED, &encl->flags))
> @@ -432,15 +446,8 @@ static long sgx_ioc_enclave_add_pages(struct
> sgx_encl *encl, void __user *arg)
> return -EINVAL;
> for (c = 0 ; c < add_arg.length; c += PAGE_SIZE) {
> - if (signal_pending(current)) {
> - if (!c)
> - ret = -ERESTARTSYS;
> -
> + if (sgx_check_signal_and_resched())
> break;
> - }
ERESTARTSYS is only appropriate if we have not EADDed any pages yet.
If we got interrupted in the middle, we should return 0. User space would
check the 'count' returned and decide to recall this ioctl() with
'offset' reset to the next page, and adjust length.
Ditto for other changes below.
Thanks
Haitao
next prev parent reply other threads:[~2024-05-15 21:56 UTC|newest]
Thread overview: 7+ messages / expand[flat|nested] mbox.gz Atom feed top
2024-05-15 6:55 [RFC PATCH v3 0/1] x86/sgx: Explicitly give up the CPU in EDMM's ioctl() to avoid softlockup Bojun Zhu
2024-05-15 6:55 ` [RFC PATCH v3 1/1] " Bojun Zhu
2024-05-15 12:06 ` Jarkko Sakkinen
2024-05-15 21:55 ` Haitao Huang [this message]
2024-05-15 22:29 ` Haitao Huang
2024-05-16 8:26 ` Jarkko Sakkinen
2024-05-16 8:24 ` Jarkko Sakkinen
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=op.2nt1vls9wjvjmi@hhuan26-mobl.amr.corp.intel.com \
--to=haitao.huang@linux.intel.com \
--cc=dave.hansen@linux.intel.com \
--cc=jarkko@kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-sgx@vger.kernel.org \
--cc=ls123674@antgroup.com \
--cc=reinette.chatre@intel.com \
--cc=zhubojun.zbj@antgroup.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).