On Fri, Nov 04, 2022 at 11:25:38AM +0100, Petr Mladek wrote: > > I get the feeling the latter would be easier to implement (no reference > > counting; also maybe can be auto-detected with THIS_MODULE?) and harder > > for the patch author to mess up (by accidentally omitting an object > > which uses it). > > I am not sure how you mean it. I guess that you suggest to store > the name of the livepatch module into the shadow variable. > And use the variable only when the livepatch module is still loaded. Actually I was thinking the klp_patch could have references to all the shadow variables (or shadow variable types?) it owns. Instead of reference counting, the livepatch atomic replace code could just migrate any previously owned shadow variables to the new klp_patch. This of course adds the restriction that such garbage-collected shadow variables couldn't be shared across non-replace livepatches. But I wouldn't expect that to be much of a problem. Additionally, I was wondering if "which klp_patch owns which shadow variables" could be auto-detected somehow. For example: 1) add 'struct module *owner' or 'struct klp_patch *owner' to klp_shadow 2) add klp_shadow_alloc_gc() and klp_shadow_get_or_alloc_gc(), which are similar to their non-gc counterparts, with a few additional arguments: the klp module owner (THIS_MODULE for the caller); and a destructor to be used later for the garbage collection 3) When atomic replacing a patch, iterate through the klp_shadow_hash and, for each klp_shadow which previously had an owner, change it to be owned by the new patch 4) When unloading/freeing a patch, free all its associated klp_shadows (also calling destructors where applicable) I'm thinking this would be easier for the patch author, and also simpler overall. I could work up a patch. -- Josh
