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

[Alexey Kardashevskiy]

On 11/12/17 19:57, Nikunj A Dadhania wrote:
> 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.


This must be some different concatenation then as the patch uses new
bootdev-buf for concatenation.


>> 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.

Ufff. I am lost now. $bootdev adds something to the dictionary (via
strdup), and so does (set-boot-device), and "bootdevice" points to what?


Since you understand this stuff, can you please outline what is called what
and where it is all stored?

(add-boot-device) is called when devices are discovered, the result is in
the dictionary, "bootdevice" points to a concatenated string. What happens
then?


> 
> 
>> And there is also a load-list list which is what for? :)
> 
> Not sure what you are referring to.


slof/fs/boot.fs  :

: load-next ( -- success ). \ Continue after go failed
   load-list 2@ ?dup IF
      save-source  -1 to source-id
      dup #ib ! span ! to ib
      0 >in !
      ['] parse-load catch restore-source throw
   ELSE drop false THEN
;


This uses "load-list" - what is that for?


I would have pushed this out already if they were 2 patches - one replacing
$cat with bootdev-string-cat and the other doing this open coded thing but
I do not know if these parts are independent and where is the line :)


-- 
Alexey