Re: [PATCH 1/2] x86/sgx: Add accounting for tracking overcommit

[Dave Hansen <dave.hansen@intel.com>]

>> +/*
>> + * Limits the amount of normal RAM that SGX can consume for EPC
>> + * overcommit to the total EPC pages * sgx_overcommit_percent / 100
>> + */
>> +static int sgx_overcommit_percent = 100;
>> +module_param_named(overcommit_percent, sgx_overcommit_percent, int, 0440);
>> +MODULE_PARM_DESC(overcommit_percent, "Percentage of overcommit of EPC pages.");
>> +
>> +/* The number of pages that can be allocated globally for backing storage. */
>> +static atomic_long_t sgx_nr_available_backing_pages;
>> +static bool sgx_disable_overcommit_tracking;
> 
> I don't like the use of word tracking as we already have ETRACK.

I don't think anyone is going to confuse "overcommit tracking" with ETRACK.

That said, this *could* be "sgx_disable_overcommit_limits", I guess.

> I'd also shorten the first global as "sgx_nr_backing_pages".
That means something different from the variable, though.

"sgx_nr_backing_pages" would be the name for the current number of
backing pages which currently exist.

> Couldn't you set "sgx_nr_backing_pages" to -1 when capping is disabled, and
> then you would not need that bool in the first place?
> 
>> +
>> +/**
>> + * sgx_charge_mem() - charge for a page used for backing storage
>> + *
> 
> Please remove this empty line:
> 
> https://www.kernel.org/doc/Documentation/kernel-doc-nano-HOWTO.txt

That *might* make sense when there are arguments.  The arguments at
least help visually separate the short function description from the
full text description.