On 06/01/2017 04:05 PM, Jiri Kosina wrote: > On Thu, 1 Jun 2017, Joe Lawrence wrote: > >> This patchset is a simplified livepatch port of kpatch's "shadow" >> variable API [1]. The kpatch project has successfully employed such >> shadow variables to implement patches that have extended data structure >> elements. This API provides livepatch a means of associating new, >> shadow data fields with existing data structures. >> >> See the first patch for the implementation, the second for further >> documentation (API, conccurency notes, use-case code snippets) and the >> third patch for an update to the sample livepatch module using shadow >> variables. > > Thanks a lot for initiating this. > > The only issue I've spotted so far -- is there any reason, why the API > completely ignores task_struct->patch_state, and always returns the 'new' > value? > > This basically offloads the responsibility for deciding between old/new to > each and every caller, and that feels much more error prone compared to > having this automatically done by klp_shadow_get(). >
Hi Jiri, I'm a little confused about the question. Maybe this clarifies a few things: * klp_shadow_get() is only returning a pointer to the shadow data, the additional storage that klp_shadow_attach() has associated with the original data structure. Callers will have to handle this shadow structure accordingly, ie, not through old_struct->new_value, but rather *new_value). * the intention is that only livepatched code will be calling klp_shadow_*, so it can assume that the current task is patched * callers might need to verify klp_shadow_get() is returning non-NULL if it's possible that some data-structures don't have a shadow var attached If you are referring to stacking livepatches ... to be honest I hadn't thought of that scenario. In that case, we might be able to get away with pushing something like this into the hash: klp #1: klp_shadow_attach(ptr, "shadow_var", ...) klp #2: klp_shadow_attach(ptr, "shadow_var_v2", ...) ... but that's just off the top of my head :) I was hoping to handle the easy case first. Maybe I misunderstood the question... if so, I can update the documentation file to better describe what's going on. Regards, -- Joe
