On Sun, Oct 16, 2016 at 10:40 AM, Joel Fernandes <joe...@google.com> wrote:
> Hi Kees,
>
> On Mon, Oct 10, 2016 at 4:59 PM, Kees Cook <keesc...@chromium.org> wrote:
>> On Sun, Oct 9, 2016 at 10:15 AM, Joel Fernandes <joe...@google.com> wrote:
>>> On Fri, Oct 7, 2016 at 10:28 PM, Joel Fernandes <joe...@google.com> wrote:
>>>> If FTRACE_PER_CPU flag is passed to ramoops pdata, split the space into
>>>> multiple zones depending on the number of CPUs.
>>>>
>>>> This speeds up the performance of function tracing by about 280% in my
>>>> tests as
>>>> we avoid the locking. The trade off being lesser space available per CPU.
>>>> Let
>>>> the ramoops user decide which option they want based on pdata flag.
>>>>
>>>> Signed-off-by: Joel Fernandes <joe...@google.com>
>>>> ---
>>>> fs/pstore/ram.c | 70
>>>> +++++++++++++++++++++++++++++++++++-----------
>>>> include/linux/pstore_ram.h | 3 ++
>>>> 2 files changed, 56 insertions(+), 17 deletions(-)
>>>>
>>>> diff --git a/fs/pstore/ram.c b/fs/pstore/ram.c
>>> [..]
>>>> @@ -391,14 +418,21 @@ static void ramoops_free_przs(struct ramoops_context
>>>> *cxt)
>>>> {
>>>> int i;
>>>>
>>>> - if (!cxt->przs)
>>>> - return;
>>>> + /* Free dump PRZs */
>>>> + if (cxt->przs) {
>>>> + for (i = 0; i < cxt->max_dump_cnt; i++)
>>>> + persistent_ram_free(cxt->przs[i]);
>>>>
>>>> - for (i = 0; i < cxt->max_dump_cnt; i++)
>>>> - persistent_ram_free(cxt->przs[i]);
>>>> + kfree(cxt->przs);
>>>> + cxt->max_dump_cnt = 0;
>>>> + }
>>>>
>>>> - kfree(cxt->przs);
>>>> - cxt->max_dump_cnt = 0;
>>>> + /* Free ftrace PRZs */
>>>> + if (cxt->fprzs) {
>>>> + for (i = 0; i < nr_cpu_ids; i++)
>>>> + persistent_ram_free(cxt->przs[i]);
>>>
>>> I am supposed to free fprzs[i] here, instead of przs[i]. Also need to
>>> check flag and free correct number of zones. Will fix for v2, sorry
>>> about that.
>>
>> I think the cpu id needs to be bounds-checked against the size of the
>> allocation. In theory, CPU hot-plug could grow the number of CPUs
>> after pstore is initialized.
>
> nr_cpu_ids is fixed to the number of possible CPUs regardless of if
> hotplug is being used or not. I did a hotplug test as well to confirm
> this. So if I boot on 4 CPU machine with only 2 CPUs online, then
> nr_cpu_ids is 4.
Ah-ha, okay. I wasn't sure if there was some way to grow nr_cpu_ids after boot.
Thanks for checking!
-Kees
--
Kees Cook
Nexus Security
