> On Fri, Mar 21, 2025 at 02:34:41PM +0200, Elena Reshetova wrote: > > sgx_nr_free_pages is an atomic that is used to keep track of > > free EPC pages and detect whenever page reclaiming should start. > > Since successful execution of ENCLS[EUPDATESVN] requires empty > > EPC and a fast way of checking for this, change this variable > > around to indicate number of used pages instead. The subsequent > > patch that introduces ENCLS[EUPDATESVN] will take use of this change. > > s/subsequent patch//
Ok > > You should rather express how EUPDATESVN trigger will depend on the > state of sgx_nr_used_pages and sgx_nr_free_pages. How about this explanation: "By counting the # of used pages instead of #of free pages, it allows the EPC page allocation path execute without a need to take the lock in all but a single case when the first page is being allocated in EPC. This is achieved via a fast check in atomic_long_inc_not_zero." Also, if you think that it is hard to interpret the patch 2/4 without 4/4 I can also squeeze them together and then it becomes right away clear why the change was done. > > > > > No functional changes intended. > > Not really understanding how I should interpret this sentence. Just as usual: this patch itself doesn’t bring any functional changes to the way as current SGX code works. I only needed this change to implement patch 4/4 in more lockless way. > > The commit message does not mention sgx_nr_used_pages, and neiher it > makes a case why implementing the feature based on sgx_nr_free_pages is > not possible. It is possible to implement it, in fact I did exactly this in the beginning instead, but as mentioned previously this would have resulted in taking a lock for each case the page is being allocated. Best Regards, Elena.
