Hi Tao, Thank you for the review and comments. > The TRANSITION mark indicating a live patch is activating or > deactivating or either? I mean, when people see the TRANSITION flag, > do they know the exact operation that is on the system, like it is > patching, or unpatching, or it doesn't distinguish the operation at > all? [TRANSITION] now explicitly means “a livepatch transition is in progress,” without distinguishing enable vs. disable. The intent is to surface that the system is in a transient state.

> Your above code is 1) check if transition_patch is null. 2) check if > transition_patch is within the klp_patches list. > > But I see in the kernel source, there are only lists to be added into > the klp_patches list, no one is to be deleted [1]. So my assumption > is, transition_patch will always be found within klp_patches. Because > for all livepatches, they must go through klp_enable_patch() -> > klp_init_patch() -> list_add_tail(&patch->list, &klp_patches), and no > one to be deleted from klp_patches afterwards, so transition_patch > must be one of those. If my assumption is correct, is step 2) > necessary? Step 2 is still necessary. A patch can be removed from the klp_patches list by klp_free_patch_async(), which can be reached via the path: enabled_store() -> __klp_disable_patch(). Although it is unlikely that klp_transition_patch is not in klp_patches, this is not guaranteed, so it is safer to keep the additional list membership check.

> And also for the list iteration in step 2), I suggest using the > do_list() function of crash utility, you can refer to > memory.c:dump_vmap_area() to get an example of do_list() usage. That's a good idea! I agree with you. I wasn’t aware of this helper in the crash utility, so I will update the code accordingly and send a v2 patch. Best regards, Motomasa Suzuki