[SLOF] [PATCH 3/3] fdt: Delete nodes of devices removed between boot and CAS

Alexey Kardashevskiy aik at ozlabs.ru
Mon Feb 10 11:27:43 AEDT 2020



On 07/02/2020 20:09, Greg Kurz wrote:
> On Fri, 7 Feb 2020 14:41:09 +1100
> Alexey Kardashevskiy <aik at ozlabs.ru> wrote:
> 
>>
>>
>> On 06/02/2020 05:21, Greg Kurz wrote:
>>> We recently fixed node creation at CAS in order to support early hotplug of
>>> devices between boot and CAS. Let's handle node removal now to support early
>>> hot *un*plug of devices.
>>>
>>> This is done in three phases:
>>>
>>> 1) all nodes from the boot time FDT are added a "slof,present-at-boot"
>>>    property
>>>
>>> 2) during pass 0 of (fdt-fix-cas-node) all nodes from the CAS FDT are added
>>>    a "slof,present-at-cas" property
>>>
>>> 3) loop on all nodes in the DT and delete the ones that only have the
>>>    "slof,present-at-boot" property, and any alias they might have. In
>>>    the  same time, "slof,present-at-cas" properties are converted into
>>>    "slof,present-at-boot" to allow further node removal in case of a
>>>    subsequent device unplug and CAS sequence before quiesce (linux
>>>    doesn't do that but it isn't explicitely forbidden by the spec).
>>>
>>> Signed-off-by: Greg Kurz <groug at kaod.org>
>>> ---
>>>  board-qemu/slof/fdt.fs |   94 ++++++++++++++++++++++++++++++++++++++++++++++++
>>>  1 file changed, 94 insertions(+)
>>>
>>> diff --git a/board-qemu/slof/fdt.fs b/board-qemu/slof/fdt.fs
>>> index 66f8fe74d9dd..1125f6702435 100644
>>> --- a/board-qemu/slof/fdt.fs
>>> +++ b/board-qemu/slof/fdt.fs
>>> @@ -198,6 +198,8 @@ fdt-check-header
>>>      decode-int dup fdt-create-dec fdt-create-enc 2drop
>>>    THEN
>>>  
>>> +  0 0 s" slof,present-at-boot" property
>>> +
>>>    finish-device  
>>>  ;
>>>  
>>> @@ -514,6 +516,9 @@ r> drop
>>>      swap			( newnode? a1 )
>>>  
>>>      fdt-debug IF ." Current  now: " pwd  get-node ."  = " . cr THEN
>>> +    fdt-cas-pass 0= IF
>>> +	0 0 s" slof,present-at-cas" property
>>> +    THEN
>>
>> I wanted to say this will remove "disk" nodes from SCSI hosts but then I
>> realized that "slof,present-at-boot" protects them. May be put this in
>> the commit log?
> 
> Yeah, the logic is to remove QEMU's FDT originated nodes only, if they have
> "slof,present-at-boot" but not "slof,present-at-cas"... Are you suggesting
> that I should emphasize this 

yup, this.


> or are you thinking to something else I might
> have missed with SCSI hosts ?

nah, it is fine.

> 
>>
>>
>>
>>>      BEGIN
>>>  	fdt-next-tag dup OF_DT_END_NODE <>
>>>      WHILE
>>> @@ -584,8 +589,97 @@ r> drop
>>>      THEN
>>>  ;
>>>  
>>> +: alias-dev-path ( xt -- dev-path len )
>>> +    link> execute decode-string	2swap 2drop
>>> +;
>>> +
>>> +: alias-name ( xt -- alias-name len )
>>> +    link> >name name>string
>>> +;
>>> +
>>> +: fdt-cas-alias-obsolete? ( node xt -- true|false )
>>> +    alias-dev-path rot node>path str=
>>> +;
>>> +
>>> +: (fdt-cas-delete-obsolete-aliases) ( node xt -- )
>>> +    dup IF
>>> +	dup @ 2 pick swap                 ( node xt node next-xt )
>>> +	recurse                           ( node xt )
>>> +	2dup fdt-cas-alias-obsolete? IF
>>> +	    dup alias-name                ( node xt alias-name len )
>>> +	    fdt-debug IF ."    Deleting alias " 2dup type cr THEN
>>> +	    delete-property               ( node xt )
>>> +	THEN
>>> +    THEN
>>> +    2drop
>>> +;
>>> +
>>> +: fdt-cas-delete-obsolete-aliases ( node -- )
>>> +    s" /aliases" find-node dup IF
>>> +                                          ( node aliases )
>>> +	get-node >r
>>> +	dup set-node
>>> +
>>> +	node>properties @ cell+ @         ( node xt )
>>> +	over swap                         ( node node xt )
>>> +	(fdt-cas-delete-obsolete-aliases) ( node )
>>
>>
>> I'd suggest clearing aliases in the end - simply walk through them and
>> delete those which we fail to "find-path".
>>
> 
> Ok, this will probably simplify the code.
> 
>>
>>> +
>>> +	r> set-node
>>> +    THEN
>>> +    drop
>>> +;
>>> +
>>> +: fdt-cas-node-obsolete? ( node -- true|false)
>>> +    s" slof,present-at-cas" 2 pick get-package-property IF
>>> +	s" slof,present-at-boot" 2 pick get-package-property IF
>>> +	    \ Node not coming from the QEMU
>>> +	    FALSE
>>> +	ELSE
>>> +	    \ Device got removed since last boot or CAS
>>> +	    2drop
>>> +	    TRUE
>>> +	THEN
>>> +    ELSE
>>> +	2drop                                   ( node )
>>> +
>>> +	get-node >r
>>> +	dup set-node
>>> +
>>> +	\ Any device present in the CAS FDT is now considered as if it was
>>> +	\ present at boot, ie. cold plugged. This allows a subsequent CAS
>>> +	\ to remove the node from the DT if the device gets unplugged in
>>> +	\ between).
>>> +	s" slof,present-at-cas" delete-property
>>> +	0 0 s" slof,present-at-boot" property
>>
>> Uff. The "fdt-cas-node-obsolete?" suggests it does checking but the code
>> actually changes the node. For the write only language we should be
>> extra careful :)
>>
> 
> Heh... I must confess I was kinda expecting this comment would pop up.
> 
>> I'd probably make it one property - "slof,from-fdt" - and store "0" for
>> "at-boot" and "1" for "at-cas" and would not bother with switching
>> "at-cas" -> "at-boot". If the property is present, then we preserve it,
>> that is all.
>>
> 
> The "at-cas" -> "at-boot" switching was done so that the deleting logic
> still works if the guest calls CAS again before quiesce. Linux doesn't
> do that but since it isn't explicitly forbidden by spec AFAIK, who knows
> what another OS can decide to do ?
>
> Following your suggestion, maybe increment "slof,from-fdt" at CAS and
> introduce a "slof,fdt-counter" in " /", set to 0 during boot and also
> incremented at CAS. Then we would delete nodes for which "slof,from-fdt"
> is less than "slof,fdt-counter" ? And of course, don't hijack the
> "fdt-cas-node-obsolete?" for that :)


Usually it is called "generation" rather then "counter" but yeah, looks
simpler too, no?

btw I am taking 1/3 and 2/3 now, no need to repost.

Thanks,


> 
>>
>>
>>> +
>>> +	r> set-node
>>> +	FALSE
>>> +    THEN
>>> +    nip
>>> +;
>>> +
>>> +: (fdt-cas-search-obsolete-nodes) ( start node -- )
>>> +    dup IF
>>> +	dup child 2 pick swap recurse
>>> +	dup peer 2 pick swap recurse      ( start node )
>>> +
>>> +	fdt-debug IF dup ." Inspecting node: " dup .node ." = " . cr THEN
>>> +	dup fdt-cas-node-obsolete? IF
>>> +	    fdt-debug IF ."    Deleting node" cr THEN
>>> +	    dup fdt-cas-delete-obsolete-aliases
>>> +	    dup delete-node
>>> +	THEN
>>> +    THEN
>>> +    2drop
>>> +;
>>> +
>>> +: fdt-cas-delete-obsolete-nodes ( start -- )
>>> +    s" /" find-device get-node (fdt-cas-search-obsolete-nodes)
>>> +;
>>> +
>>>  : fdt-fix-cas-node ( start -- )
>>>      0 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Add phandles
>>> +    dup fdt-cas-delete-obsolete-nodes             \ Delete removed devices
>>>      1 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Patch+add other properties
>>>      2 to fdt-cas-pass dup (fdt-fix-cas-node) drop \ Delete phandles from pass 0
>>>      drop
>>>
>>
> 

-- 
Alexey


More information about the SLOF mailing list