[SLOF] [PATCH v3] boot: do not use catpad to concatenate strings

[Nikunj A Dadhania]

Alexey Kardashevskiy <aik at ozlabs.ru> writes:

> On 11/12/17 17:30, Nikunj A Dadhania wrote:
>> Hi Alexey,
>> 
>> Alexey Kardashevskiy <aik at ozlabs.ru> writes:
>> 
>>> On 08/12/17 16:32, Nikunj A Dadhania wrote:
>>>> The catpad size is 1K size, which can be hit easily hit with around 20 devices
>>>> with bootindex.
>>>>
>>>> Open code EVALUATE such that concatenation is not required. Replace usage of
>>>> $cat with a dynamically allocated buffer(16K) here.
>>>>
>>>> Reported here: https://github.com/qemu/SLOF/issues/3
>>>>
>>>> Signed-off-by: Nikunj A Dadhania <nikunj at linux.vnet.ibm.com>
>>>
>>> I've read the v1..v3 of this patch and (to my embarrassment) I do not
>>> understand how this works at all :)
>>>
>>> Above you said "concatenation is not required" but there is
>>> bootdev-string-cat, how is that not concat? We did cat to bootdevice, now -
>>> to bootdev-string-cat, but this is not just it?
>> 
>> Earlier the concatenation was done using a common buffer $catpad
>> 
>> slof/fs/base.fs:CREATE $catpad 400 allot
>> 
>> Which had a limitation of 1K, my first patch was to increase the size of
>> $catpad and Segher said that $catpad was kept small on purpose for
>> concatenating quickly without much overhead.
>> 
>> Version-2 patch did following things:
>> 
>>     1) introduced a similar static buffer for bootdev concatenation without
>>        using $cat.
>
> This part I understood :)
>
>
>>     2) Replaced concatenation in LOAD and LOAD-NEXT by open coding EVALUATE word.
>
> This part I do not understand - why is this change needed or how does it
> make it better?

Because we were  concatenating and then evaluating during runtime, newer
code plays directly with args and populates the stack and calls
parse-load, so concatenation is not required anymore.

> There is a global list of boot devices - bootdevice; and for some reason
> now there is another one - bootdev-buf, both are strings...

No, bootdev-buf is a temporary buffer, its not replacing bootdevice.


> And there is also a load-list list which is what for? :)

Not sure what you are referring to.

Regards
Nikunj