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

[Alexey Kardashevskiy]

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?

For example:

0 > $bootdev type  /pci at 800000020000000/ethernet at 1
/pci at 800000020000000/ethernet at 2 disk cdrom net net1  ok


Thanks.


> 
> ---
> 
> qemu-system-ppc64 -nographic -nodefaults -serial stdio -monitor pty -m 2G \
> -device virtio-scsi-pci    \
> `for ((i=2;i<=50;i++)) ; \
> do echo -n "  -drive file=/tmp/storage$i.qcow2,if=none,id=drive$i,format=qcow2 \
> -device
> scsi-hd,drive=drive$i,id=disk$i,channel=0,scsi-id=0,lun=$i,bootindex=$i"; \
> done;` \
> -drive file=guest.disk,if=none,id=drv1,format=qcow2,cache=none \
> -device scsi-hd,drive=drv1,bootindex=1 -boot menu=on,splash-time=3000,strict=on
> ---
>  slof/fs/boot.fs | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-----
>  1 file changed, 48 insertions(+), 5 deletions(-)
> 
> diff --git a/slof/fs/boot.fs b/slof/fs/boot.fs
> index 1fd7439..6d16c54 100644
> --- a/slof/fs/boot.fs
> +++ b/slof/fs/boot.fs
> @@ -15,8 +15,27 @@
>  VARIABLE state-valid false state-valid !
>  CREATE go-args 2 cells allot go-args 2 cells erase
>  
> +4000 CONSTANT bootdev-size
> +0 VALUE bootdev-buf
> +
>  \ \\\\\\\\\\\\\\ Structure/Implementation Dependent Methods
>  
> +: alloc-bootdev-buf ( -- )
> +   bootdev-size alloc-mem ?dup 0= ABORT" Unable to allocate bootdev buffer!"
> +   dup bootdev-size erase
> +   to bootdev-buf
> +;
> +
> +: free-bootdev-buf ( -- )
> +   bootdev-buf bootdev-size free-mem
> +   0 to bootdev-buf
> +;
> +
> +: bootdev-string-cat ( addr1 len1 addr2 len2 -- addr1 len1+len2 )
> +   dup 3 pick + bootdev-size > ABORT" bootdev size too big!"
> +   string-cat
> +;
> +
>  : $bootargs
>     bootargs 2@ ?dup IF
>     ELSE s" diagnostic-mode?" evaluate and IF s" diag-file" evaluate
> @@ -24,14 +43,23 @@ CREATE go-args 2 cells allot go-args 2 cells erase
>  ;
>  
>  : $bootdev ( -- device-name len )
> -   bootdevice 2@ dup IF s"  " $cat THEN
> +   alloc-bootdev-buf
> +   bootdevice 2@ ?dup IF
> +      swap bootdev-buf 2 pick move
> +      bootdev-buf swap s"  " bootdev-string-cat
> +   ELSE
> +      \ use bootdev-buf for concatenating diag mode/boot-device if any
> +      drop bootdev-buf 0
> +   THEN
>     s" diagnostic-mode?" evaluate IF
>        s" diag-device" evaluate
>     ELSE
>        s" boot-device" evaluate
>     THEN
> -   $cat \ prepend bootdevice setting from vpd-bootlist
> +   ( bootdev len str len1 )
> +   bootdev-string-cat \ concatenate both
>     strdup
> +   free-bootdev-buf
>     ?dup 0= IF
>        disable-watchdog
>        drop true ABORT" No boot device!"
> @@ -51,7 +79,14 @@ CREATE go-args 2 cells allot go-args 2 cells erase
>  ' (set-boot-device) to set-boot-device
>  
>  : (add-boot-device) ( str len -- )	\ Concatenate " str" to "bootdevice"
> -   bootdevice 2@ ?dup IF $cat-space ELSE drop THEN set-boot-device
> +   bootdevice 2@ ?dup IF
> +      alloc-bootdev-buf
> +      swap bootdev-buf 2 pick move
> +      bootdev-buf swap s"  " bootdev-string-cat
> +      2swap bootdev-string-cat
> +   ELSE drop THEN
> +   set-boot-device
> +   bootdev-buf 0 <> IF free-bootdev-buf THEN
>  ;
>  
>  ' (add-boot-device) to add-boot-device
> @@ -221,11 +256,19 @@ defer go ( -- )
>     ELSE
>        drop
>     THEN
> -   set-boot-args s" parse-load " $bootdev $cat strdup evaluate
> +   set-boot-args
> +   save-source  -1 to source-id
> +   $bootdev dup #ib ! span ! to ib
> +   0 >in !
> +   ['] parse-load catch restore-source throw
>  ;
>  
>  : load-next ( -- success )	\ Continue after go failed
> -   load-list 2@ ?dup IF s" parse-load " 2swap $cat strdup evaluate
> +   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
>  ;
>  
> 


-- 
Alexey