Dave, thanks for the quick review! On Wed, Aug 1, 2018 at 1:10 PM John David Anglin <dave.anglin@xxxxxxxx> wrote: > > On 2018-08-01 2:22 PM, Nick Desaulniers wrote: > > As part of the effort to reduce the code duplication between _THIS_IP_ > > and current_text_addr(), let's consolidate callers of > > current_text_addr() to use _THIS_IP_. > Using the generic _THIS_IP_ results in significantly longer code than > the parisc implementation > of current_text_addr(). It might be worthwhile to file a bug with your compiler vendor. It seems like there may be a better way to generate code for this construct. Also, I'm curious how hot this code is? I assume in general that the C construct may be larger than the inline assembly, but I'm curious if this introduces a performance regression or just makes the code larger? Do you have stats on the size difference and performance differences? What's more important to me is whether the patch is correct... > It also results in a local label in the text. > This breaks the unwind data > for the function with the label in 32-bit kernels. The implementation > of current_text_addr() > doesn't add a label. ... oh, I guess I'm surprised that the label ends up in the code, vs there just being a constant generated. Can you send me the disassembly? Also, I'm curious how does having the label present in the text break the unwinder? (I'm not familiar with how unwinding works beyond following frame pointers). > _THIS_IP_ should be defined using > current_text_addr() on parisc. I'm trying to eliminate current_text_addr() in the kernel, as it only has 4 call sites (3 are arch specific). What are your thoughts on making the current parisc current_text_addr() just a static function (or statement expression that's local to) in arch/parisc/kernel/unwind.c? -- Thanks, ~Nick Desaulniers -- To unsubscribe from this list: send the line "unsubscribe linux-parisc" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
