On 2017/06/07 03:46PM, Balbir Singh wrote: > On Wed, 2017-06-07 at 00:42 +0530, Naveen N. Rao wrote: > > On 2017/06/06 02:29PM, Balbir Singh wrote: > > > With text moving to read-only migrate optprobes to using > > > the patch_instruction infrastructure. Without this optprobes > > > will fail and complain. > > > > <snip> > > > + /* We can optimize this via patch_instruction_window later */ > > > > This probably needs a TODO just so it's clear. I do think this would be > > good to add since we copy many instructions while setting up the > > optprobe, so this is quite slow as it exists today. > > I made it read like a TODO, with a TOOD:
Yes, just a nit that it would be good to have an explicit 'TODO:' there, without which it looks like any other comment... But, thinking about this more, we could probably simplify this by having optprobes set things up in a local buffer before copying it into the instruction slot. That will also make it easier for the subsequent support to patch many instructions at once. > > > > > > + size = (TMPL_END_IDX * sizeof(kprobe_opcode_t)) / sizeof(int); > > > > That's just TMPL_END_IDX. > > > > The entire kprobe_opcode_t and int types thing is a bit messy. The size Right - it's an artifact of kprobes generic infrastructure. For powerpc, it's always an integer. > calculation assumes nothing for now in terms of sizes, but I guess the > patch_instruction does. Another TODO? Not sure I follow... - Naveen