From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-5.3 required=3.0 tests=BAYES_00, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS, USER_AGENT_SANE_1 autolearn=no autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 6EE69C2D0A8 for ; Tue, 29 Sep 2020 01:14:46 +0000 (UTC) Received: from kanga.kvack.org (kanga.kvack.org [205.233.56.17]) by mail.kernel.org (Postfix) with ESMTP id 06FA22083B for ; Tue, 29 Sep 2020 01:14:45 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 06FA22083B Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=intel.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=owner-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix) id 345C66B005C; Mon, 28 Sep 2020 21:14:45 -0400 (EDT) Received: by kanga.kvack.org (Postfix, from userid 40) id 2F70F6B005D; Mon, 28 Sep 2020 21:14:45 -0400 (EDT) X-Delivered-To: int-list-linux-mm@kvack.org Received: by kanga.kvack.org (Postfix, from userid 63042) id 1E6846B0062; Mon, 28 Sep 2020 21:14:45 -0400 (EDT) X-Delivered-To: linux-mm@kvack.org Received: from forelay.hostedemail.com (smtprelay0165.hostedemail.com [216.40.44.165]) by kanga.kvack.org (Postfix) with ESMTP id 05C616B005C for ; Mon, 28 Sep 2020 21:14:45 -0400 (EDT) Received: from smtpin12.hostedemail.com (10.5.19.251.rfc1918.com [10.5.19.251]) by forelay02.hostedemail.com (Postfix) with ESMTP id C2D0B4DC8 for ; Tue, 29 Sep 2020 01:14:44 +0000 (UTC) X-FDA: 77314329288.12.cord03_1517e2a27186 Received: from filter.hostedemail.com (10.5.16.251.rfc1918.com [10.5.16.251]) by smtpin12.hostedemail.com (Postfix) with ESMTP id 9830518014B86 for ; Tue, 29 Sep 2020 01:14:44 +0000 (UTC) X-HE-Tag: cord03_1517e2a27186 X-Filterd-Recvd-Size: 5381 Received: from mga07.intel.com (mga07.intel.com [134.134.136.100]) by imf45.hostedemail.com (Postfix) with ESMTP for ; Tue, 29 Sep 2020 01:14:43 +0000 (UTC) IronPort-SDR: Od7YsVe6K3cQ8HM7KFdbzBBjL+vfmc0lCv+Z5UYMRYnuHRiPdbeLar55wM944gFwEmdzaBc8r8 sHYYdvrKpblQ== X-IronPort-AV: E=McAfee;i="6000,8403,9758"; a="226234021" X-IronPort-AV: E=Sophos;i="5.77,316,1596524400"; d="scan'208";a="226234021" X-Amp-Result: SKIPPED(no attachment in message) X-Amp-File-Uploaded: False Received: from fmsmga006.fm.intel.com ([10.253.24.20]) by orsmga105.jf.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Sep 2020 18:14:42 -0700 IronPort-SDR: pgtpNA+jtuvGoogfUXxiqFnRu841hblJguoLlGbttUi+kgiYHcNtBAOTadUTHJP2gwNzlDQWAZ MIKAy04r32Pw== X-IronPort-AV: E=Sophos;i="5.77,316,1596524400"; d="scan'208";a="513656253" Received: from sjchrist-coffee.jf.intel.com (HELO linux.intel.com) ([10.54.74.160]) by fmsmga006-auth.fm.intel.com with ESMTP/TLS/ECDHE-RSA-AES256-GCM-SHA384; 28 Sep 2020 18:14:41 -0700 Date: Mon, 28 Sep 2020 18:14:39 -0700 From: Sean Christopherson To: Jarkko Sakkinen Cc: Borislav Petkov , x86@kernel.org, linux-sgx@vger.kernel.org, linux-kernel@vger.kernel.org, linux-mm@kvack.org, Jethro Beekman , Jordan Hand , Nathaniel McCallum , Chunyang Hui , Seth Moore , akpm@linux-foundation.org, andriy.shevchenko@linux.intel.com, asapek@google.com, cedric.xing@intel.com, chenalexchen@google.com, conradparker@google.com, cyhanish@google.com, dave.hansen@intel.com, haitao.huang@intel.com, josh@joshtriplett.org, kai.huang@intel.com, kai.svahn@intel.com, kmoy@google.com, ludloff@google.com, luto@kernel.org, nhorman@redhat.com, puiterwijk@redhat.com, rientjes@google.com, tglx@linutronix.de, yaozhangx@google.com Subject: Re: [PATCH v38 16/24] x86/sgx: Add a page reclaimer Message-ID: <20200929011438.GA31167@linux.intel.com> References: <20200915112842.897265-1-jarkko.sakkinen@linux.intel.com> <20200915112842.897265-17-jarkko.sakkinen@linux.intel.com> <20200922104538.GE22660@zn.tnic> <20200922140314.GA164527@linux.intel.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20200922140314.GA164527@linux.intel.com> User-Agent: Mutt/1.5.24 (2015-08-30) X-Bogosity: Ham, tests=bogofilter, spamicity=0.000000, version=1.2.4 Sender: owner-linux-mm@kvack.org Precedence: bulk X-Loop: owner-majordomo@kvack.org List-ID: On Tue, Sep 22, 2020 at 05:03:23PM +0300, Jarkko Sakkinen wrote: > On Tue, Sep 22, 2020 at 12:45:38PM +0200, Borislav Petkov wrote: > > > + spin_lock(&sgx_active_page_list_lock); > > > + for (i = 0; i < SGX_NR_TO_SCAN; i++) { > > > + if (list_empty(&sgx_active_page_list)) > > > > Isn't it enough to do this once, i.e., not in the loop? You're holding > > sgx_active_page_list_lock... Argh, I missed this until I looked at Jarkko's updated tree. The reason for checking list_empty() on every iteration is that the loop is greedy, i.e. it tries to grab and reclaim up to 16 (SGX_NR_TO_SCAN) EPC pages at a time. > I think that would make sense. Distantly analogous to the EINIT > discussion. Too complex code for yet to be known problem workloads I'd > say. Nooooo. Please no. This will most definitely impact workloads running a single large enclave, or a process running multiple enclaves, as we'll lose the batching of ETRACK and IPIs. ETRACK isn't a big deal, but the IPIs are painful and could thrash the system. E.g. in the current code, reclaiming 10 pages from an enclave with actively running threads will generate one round of IPIs to CPUs associated with the enclave (based on the mm struct). Going to per-page reclaim will likely result in 10 rounds of IPIs as threads will reenter the enclave immediately after each IPI wave. Having to grab the section spinlock for every page is also (very) problematic. Reclaiming one page at a time makes it nigh impossible for the reclaimer to keep up as every allocation attempt acquires the spinlock. I.e. the reclaimer has to contend with every CPU that is faulting in an EPC page or is adding a page to the enclave. In my experiments with the EPC cgroup, even batching 16 pages may be insufficient to make "forward progress". That's not an apples to apples comparison, but it's a rough equivalent of what will happen with the reclaim thread versus EPC page faults. We can (and should) play with scaling the number of reclaim threads, but at this stage, that's an exercise best left to post-upstreaming. I can't do A/B testing to provide numbers, because the changes in Jarkko's tree break basic reclaim. Which is a nice segue into my last point: this is by far the most subtle code in the SGX code base; I really, really don't want to be making last minute changes in this area unless they are absolutely necessary or obviously benign. Even if/when we get the basic reclaim functional again, and assuming it doesn't have performance issues, I wouldn't be comfortable merging the code without hammering it with the EPC cgroup tests for multiple days (on top of the 1+ days to rebased the EPC cgroup). Past testing with the cgroup found multiple bugs with races between CPUs that required hitting a window that was open for less than 10 instructions.