Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:18 as permitted sender) client-ip=2620:137:e000::1:18;
Message-ID: <96d630b3-c2df-99d5-e057-6787bd107d09@intel.com>
Date:   Tue, 5 Apr 2022 12:56:03 -0700
User-Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:91.0) Gecko/20100101
 Firefox/91.0 Thunderbird/91.7.0
Subject: Re: [PATCH V3 19/30] x86/sgx: Free up EPC pages directly to support
 large page ranges
Content-Language: en-US
To:     Jarkko Sakkinen <jarkko@kernel.org>
CC:     <dave.hansen@linux.intel.com>, <tglx@linutronix.de>,
        <bp@alien8.de>, <luto@kernel.org>, <mingo@redhat.com>,
        <linux-sgx@vger.kernel.org>, <x86@kernel.org>, <seanjc@google.com>,
        <kai.huang@intel.com>, <cathy.zhang@intel.com>,
        <cedric.xing@intel.com>, <haitao.huang@intel.com>,
        <mark.shanahan@intel.com>, <hpa@zytor.com>,
        <linux-kernel@vger.kernel.org>
References: <cover.1648847675.git.reinette.chatre@intel.com>
 <b3a17e1ce7bcd14db3c28903e8a97f094998ae74.1648847675.git.reinette.chatre@intel.com>
 <Ykvrk4hYvBEnNOOl@kernel.org>
 <e118f4c6-7216-15f3-2bda-3a5851b0bcb1@intel.com>
 <ddef013dc9ce496ff5840f42cf0651637060b4f2.camel@kernel.org>
From:   Reinette Chatre <reinette.chatre@intel.com>
In-Reply-To: <ddef013dc9ce496ff5840f42cf0651637060b4f2.camel@kernel.org>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: 8bit
MIME-Version: 1.0
X-MS-Exchange-AntiSpam-MessageData-ChunkCount: 1
X-MS-Exchange-AntiSpam-MessageData-0: =?utf-8?B?KzloUklVek54bXdMSnpBZHJwQ053ZXNhTmlSbDgrN3JZZDc5MmwzWlc3eDlW?=
 =?utf-8?B?eUxmU1pJZFRITEZoRGJPOTFENWJaVXdvdlFnQWVSTlBnMmI2bVNhaEd0MlEx?=
 =?utf-8?B?TEtqQnpIcjEyU2FMMFVXM1p1NWo5SkJRTkFRQ1hUL3hWVmpCYy95VnREL3Rn?=
 =?utf-8?B?dHY3RkExVkZLZHhBZ0RtTVRhTUhlVkdwUjQyTHhLYWNWT0FjbTE3bnNjNlJM?=
 =?utf-8?B?ZllUZ3NEMG5pTngrUWZqT3NUM1kvZVRsRXhDUGV5TTdDTG5KdzVaYW45SnBR?=
 =?utf-8?B?Yi84dDNuSHkxUnN1a1NLQk8yYm5DNHB1TWhKa2x1Z1N3a1hxbjJEWDgySjJj?=
 =?utf-8?B?Nlo2NjljTi9iSkd6bVluUTQ2YlMvVXIxUEVnZFllS2trWDYwdWJDUFhlVUlR?=
 =?utf-8?B?a2U1MlpqUm9ybzBCSDVMLzQxaHdONXF4OVpNVW82dDRxd1BVUU9PaCtLSXRU?=
 =?utf-8?B?WVhJWEtYL1NLcURXTlcxayt1QWNPa0M3YUpJT1loQ3NhcHRrVDZ2aTZXaVhI?=
 =?utf-8?B?NTdXVlhEaGNGWDl3MTFoTnZnOHJiL0lWT0g2UUN2UG1NUmExd09RdFY4a2RL?=
 =?utf-8?B?U205TEQ2SEJLSkpsakc0MEp6L04vcytXYmhzRmJaZzYvK0JTN2FvSzY4aitW?=
 =?utf-8?B?R2JKK1pURXo4b0tzdHBITG04Rk4xOFN3QXMrQlAyR2N5MnUrUnNBaG1mZmND?=
 =?utf-8?B?UWYwc09wcGZhdG9nYjBXRWNKZ3ZFdkY1ZUl4VmNydjJ4ZTJjcm5yQWJLWkZS?=
 =?utf-8?B?SVBxaGUybVlHdTF1ZXZIVkI4WjRHUTVIV3pEM2UxczFjWFFxZEVBcHFlZ2xu?=
 =?utf-8?B?bVFFNUJpNXQ3Y2NtY2hCNnk1eFMyTHJkcmw5akE2VDZEb0pVUWNLdk52QkVB?=
 =?utf-8?B?VkZ5WmliUGRjWENzNkE1b2psc1ZZRHl4bnZ5QTcycmc0Y1dROXFEWExWWDFW?=
 =?utf-8?B?NHlQaHJtU1ZJS0s1bEhlYk45YlRldS95NW5UWmx1Z0FDQjBPeHRBZGQzcnV4?=
 =?utf-8?B?N0grd2Evd2hFaTd2YlVVZWh6a0owenNOZUFxS3ZZTjkzQUhYZDBpcjB4Q0NG?=
 =?utf-8?B?VTJocE11MTZHSlU5S0ZkSGUxdFMwQ1kwaGV4aTdybUFwVEZMclRaTEV6cHJW?=
 =?utf-8?B?OXJZZkc2UFZRZzE5TlhMZSs2cFlqWjR3NWdLdGc5UEJsQ1JCeFpRQ3pQK3c5?=
 =?utf-8?B?ZzJGN29Ia2RndDNWWTlreUlLM0krU1c3bldrMk1nRTRMWFluV2RJSTdKQmJF?=
 =?utf-8?B?SkN3UzZvSllVeWppMXhYSUM1Tkp6WTJadlloZ2ZSVGRUek9IMFB1VzZLT3dE?=
 =?utf-8?B?bFNxNTExbkU4L3pVNUVNb2JLcDZUZ2JKRjN2N2dvMnI1M2VpdlI4UFZSRkND?=
 =?utf-8?B?YWw5VlFWZ2VLZ0RrWEx6dUZuS0JEOGw1TzdZQ2p3c2k3Z0RWay9QdHNOYUlH?=
 =?utf-8?B?SGk2NmhXZVkwMGozRmIwVy8zY1VQd2JZZGN4RE9IcDJNNUg2Q01uMytyMmdq?=
 =?utf-8?B?Ynh5Q0gwVzR0Y0lyTFhSZ3NXeHpia0F2VmtPa3RsNEw5c0VMMW5YY1RtcU10?=
 =?utf-8?B?SUtlZ0p2WEwvc2NNYU0rOG5oaEF6d0xvM3hZVXBsMG5ScUpEbE84ZnFVYldU?=
 =?utf-8?B?UnFOaUJvU1lpSVREbFlZVjJlTFQzbDVRZllHbjUwTUgvZU96bkFRc2Ezamdw?=
 =?utf-8?B?Yllxazk3WWNMbXRGRXN0VjRHV200SU4yVWZ1ZDBnZnVQak11MDI0L0YvWFZQ?=
 =?utf-8?B?cnBITURDU1hVeElhYkh2RGdYSjlKbnhPTnVwZVN5WWQ1YzQxNkl5RVpsaEtv?=
 =?utf-8?B?YUxtSVlUdGNZWnVicTZlQmgxaUJ4Nml2UVQrZFliM2tnR2U3U2dXbVFBL0hE?=
 =?utf-8?B?YkVRWkcxcG5zNVN2UUN4S0lYaW8wbzg1MVFkSStoR25qeStzMTBJcHVMYnhk?=
 =?utf-8?B?eUVzZGZMWkJQSzZWUmtvaDR3RVhtNVpzNVdJZ0ROTWxPd2lIVnRGT3VTTmFL?=
 =?utf-8?B?VWVaTDN4dytzMGZSTE1HZG1HRHN5NENMSWRFVkE3eUc1dVluYk1NOWVwNGdi?=
 =?utf-8?B?L2ZWdmY5eVFIcnBVbVBQWFlqL2duYUU1ZVY1a0J6NHI0U081dEhXcE9hZ0xS?=
 =?utf-8?B?V0RvZlVtTWhSampJZTVESG1OS0tESEpxb0hOa3V6akk5VzlKWm5kZFlJTEJj?=
 =?utf-8?B?aC9qMCt4Qk5CMSsxM01NRkJhbjVwYUg4RWVWdzFlSm1EOWlJWU1CSG1ySUwv?=
 =?utf-8?B?Q0JtYjFEeStiMXZiaW9ydnpyU2M3eE0zWjN2OVBnQW1QalVEQXdFQ0lTQ1k2?=
 =?utf-8?B?MHFjbzFCT0U5VEVQSnVSblVIMzJLbERsUFlyQ0YxN1p1VENEeEFLVVZqVCtV?=
 =?utf-8?Q?3xxL03hsVBYgAyu8=3D?=
X-MS-Exchange-CrossTenant-Network-Message-Id: f8eb0edf-a48f-4379-1742-08da173e5362
X-MS-Exchange-CrossTenant-AuthSource: BN0PR11MB5744.namprd11.prod.outlook.com
X-MS-Exchange-CrossTenant-AuthAs: Internal
X-MS-Exchange-CrossTenant-OriginalArrivalTime: 05 Apr 2022 19:56:08.3038
 (UTC)
X-MS-Exchange-CrossTenant-FromEntityHeader: Hosted
X-MS-Exchange-CrossTenant-Id: 46c98d88-e344-4ed4-8496-4ed7712e255d
X-MS-Exchange-CrossTenant-MailboxType: HOSTED
X-MS-Exchange-CrossTenant-UserPrincipalName: 9h1mhAZK7xQ8iX3EqpFksNk6pohC1pF7DcLQzSWcHVQ2jUakdnWpHBxJBj57VMxG/QyXBmMM7nsbvHFs6miPUlf8WvzKod9U3RWSTlvuBiY=
X-MS-Exchange-Transport-CrossTenantHeadersStamped: DM6PR11MB4578
X-OriginatorOrg: intel.com
X-Spam-Status: No, score=-4.8 required=5.0 tests=BAYES_00,DKIMWL_WL_HIGH,
	DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,HEADER_FROM_DIFFERENT_DOMAINS,
	MAILING_LIST_MULTI,NICE_REPLY_A,RDNS_NONE,SPF_HELO_NONE,
	T_SCC_BODY_TEXT_LINE autolearn=unavailable autolearn_force=no
	version=3.4.6
X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on
	lindbergh.monkeyblade.net
Precedence: bulk
List-ID: <linux-kernel.vger.kernel.org>
X-Mailing-List: linux-kernel@vger.kernel.org

Hi Jarkko,

On 4/5/2022 11:42 AM, Jarkko Sakkinen wrote:
> On Tue, 2022-04-05 at 10:13 -0700, Reinette Chatre wrote:
>> On 4/5/2022 12:11 AM, Jarkko Sakkinen wrote:
>>> On Mon, Apr 04, 2022 at 09:49:27AM -0700, Reinette Chatre wrote:

...

>>>> diff --git a/arch/x86/kernel/cpu/sgx/main.c b/arch/x86/kernel/cpu/sgx/main.c
>>>> index 6e2cb7564080..545da16bb3ea 100644
>>>> --- a/arch/x86/kernel/cpu/sgx/main.c
>>>> +++ b/arch/x86/kernel/cpu/sgx/main.c
>>>> @@ -370,6 +370,12 @@ static bool sgx_should_reclaim(unsigned long watermark)
>>>>                !list_empty(&sgx_active_page_list);
>>>>  }
>>>>  
>>>> +void sgx_direct_reclaim(void)
>>>> +{
>>>> +       if (sgx_should_reclaim(SGX_NR_LOW_PAGES))
>>>> +               sgx_reclaim_pages();
>>>> +}
>>>
>>> Please, instead open code this to both locations - not enough redundancy
>>> to be worth of new function. Causes only unnecessary cross-referencing
>>> when maintaining. Otherwise, I agree with the idea.
>>>
>>
>> hmmm, that means the heart of the reclaimer (sgx_reclaim_pages()) would be
>> made available for direct use from everywhere in the driver. I will look into this.
>>
>> Reinette
>>
> 
> It's a valid enough point. Let's keep it as it is :-)

Will do. I plan to add Dave's suggested comments to sgx_direct_reclaim() that is
introduced in this patch.

> 
> Acked-by: Jarkko Sakkinen <jarkko@kernel.org>

Thank you very much.

Reinette