On Fri, Nov 07, 2014 at 07:21:03PM +0100, Petr Mladek wrote: > On Thu 2014-11-06 10:57:48, Seth Jennings wrote: > > On Thu, Nov 06, 2014 at 04:51:02PM +0100, Jiri Slaby wrote: > > > On 11/06/2014, 03:39 PM, Seth Jennings wrote: > > > > +/************************************* > > > > + * Core structures > > > > + ************************************/ > > > > + > > > > +/* > > > > + * lp_ structs vs lpc_ structs > > > > + * > > > > + * For each element (patch, object, func) in the live-patching code, > > > > + * there are two types with two different prefixes: lp_ and lpc_. > > > > + * > > > > + * Structures used by the live-patch modules to register with this core module > > > > + * are prefixed with lp_ (live patching). These structures are part of the > > > > + * registration API and are defined in livepatch.h. The structures used > > > > + * internally by this core module are prefixed with lpc_ (live patching core). > > > > + */ > > > > > > I am not sure if the separation and the allocations/kobj handling are > > > worth it. It makes the code really less understandable. Can we have just > > > struct lip_function (don't unnecessarily abbreviate), lip_objectfile > > > (object is too generic, like Java object) and lip_patch containing all > > > the needed information? It would clean up the code a lot. (Yes, we would > > > have profited from c++ here.) > > > > I looked at doing this and this is actually what we did in kpatch. We > > made one structure that had "private" members that the user wasn't > > suppose to access that were only used in the core. This was messy > > though. Every time you wanted to add a "private" field to the struct so > > the core could do something new, you were changing the API to the patch > > modules as well. While copying the data into an internal structure does > > add code and opportunity for errors, that functionality is localized > > into functions that are specifically tasked with taking care of that. > > So the risk is minimized and we gain flexibility within the core and > > more self-documenting API structures. > > I am not sure if the modified API is really such a big limit. The > modules initialize the needed members using ".member = value". > Also we do not need to take care of API/ABI backward compatibility because > there is very strict dependency between patches and the patched > kernel. Our patch module generation tool (kpatch-build) relies on the API as well, so we should try to keep the API as stable as possible. At least until we can put kpatch-build (or something like it) into the kernel tree. -- Josh -- To unsubscribe from this list: send the line "unsubscribe live-patching" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
