On Fri, Feb 19, 2016 at 1:48 PM, Luis R. Rodriguez <mcgrof@xxxxxxxxxx> wrote:
> On Fri, Feb 19, 2016 at 12:25:55PM -0800, H. Peter Anvin wrote:
>> On 02/19/2016 05:45 AM, Luis R. Rodriguez wrote:
>> > +
>> > +/**
>> > + * DOC: Regular linker linker table constructors
>> > + *
>> > + * Regular constructors are expected to be used for valid linker table entries.
>> > + * Valid uses of weak entries other than the beginning and is currently
>> > + * untested but should in theory work.
>> > + */
>> > +
>> > +/**
>> > + * LINKTABLE_TEXT - Declares a linker table entry for execution
>> > + *
>> > + * @name: linker table name
>> > + * @level: order level
>> > + *
>> > + * Declares a linker table to be used for execution.
>> > + */
>> > +#define LINKTABLE_TEXT(name, level) \
>> > + __typeof__(name[0]) \
>> > + __attribute__((used, \
>> > + __aligned__(LINKTABLE_ALIGNMENT(name)), \
>> > + section(SECTION_TBL(SECTION_TEXT, name, level))))
>>
>> I'm really confused by this. Text should obviously be readonly,
>
> So this uses SECTION_TEXT, so we just pegged the linker table entry right below
> the standard SECTION_TEXT:
OK yes I see the issue now. I've modified this to use const, and
retested the kprobe patch and it works well still. kprobe would not
use LINKTABLE_TEXT, instead it uses its own macro, however users of
LINKTABLE_TEXT would then have const declared. The implications are
that you *can* declare structs so long as everything is const.
Folks may at times need to modify the structural definitions -- for
that LINKTABLE_DATA() is more appropriate, and as per my testing it
still allows execution of callbacks. I can document this.
Do we want .init.text to match this? I'd port my "struct x86_init_fn"
to use LINKTABLE_INIT_DATA() then. FWIW below is a paste of a simple
test program that demos what I mean.
#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
#include <linux/kernel.h>
#include <linux/module.h>
#include <linux/workqueue.h>
#include <linux/tables.h>
static void stuff_todo(struct work_struct *work);
static DECLARE_WORK(stuff_work, stuff_todo);
struct stuff {
int a;
int b;
void (*print_a)(struct stuff *);
void (*print_b)(struct stuff *);
void (*print_a_ro)(const struct stuff *);
void (*print_b_ro)(const struct stuff *);
};
void print_a(struct stuff *s)
{
pr_info("print_a a: %d\n", s->a);
}
void print_a_ro(const struct stuff *s)
{
pr_info("print_a a: %d\n", s->a);
}
void print_b(struct stuff *s)
{
pr_info("print_b b: %d\n", s->b);
}
void print_b_ro(const struct stuff *s)
{
pr_info("print_b b: %d\n", s->b);
}
DEFINE_LINKTABLE_TEXT(struct stuff, my_stuff_fns_ro);
DEFINE_LINKTABLE_DATA(struct stuff, my_stuff_fns);
static LINKTABLE_TEXT(my_stuff_fns_ro, 0000) stuff_a_ro = {
.a = 1,
.b = 1,
.print_a_ro = print_a_ro,
.print_b_ro = print_b_ro,
};
static LINKTABLE_TEXT(my_stuff_fns_ro, 0000) stuff_b_ro = {
.a = 2,
.b = 2,
.print_a_ro = print_a_ro,
.print_b_ro = print_b_ro,
};
static LINKTABLE_TEXT(my_stuff_fns_ro, 0000) stuff_c_ro = {
.a = 3,
.b = 3,
.print_a_ro = print_a_ro,
.print_b_ro = print_b_ro,
};
static LINKTABLE_DATA(my_stuff_fns, 0000) stuff_a = {
.a = 1,
.b = 1,
.print_a = print_a,
.print_b = print_b,
};
static LINKTABLE_DATA(my_stuff_fns, 0000) stuff_b = {
.a = 2,
.b = 2,
.print_a = print_a,
.print_b = print_b,
};
static void stuff_todo(struct work_struct *work)
{
struct stuff *s;
const struct stuff *s_ro;
unsigned int i = 0;
pr_info("Looping over my_stuff_fns_ro\n");
LINKTABLE_FOR_EACH(s_ro, my_stuff_fns_ro) {
pr_info("Looping on s ro %d\n", i++);
s_ro->print_a_ro(s_ro);
s_ro->print_b_ro(s_ro);
}
i=0;
pr_info("Looping over my_stuff_fns\n");
LINKTABLE_FOR_EACH(s, my_stuff_fns) {
pr_info("Looping on s %d\n", i++);
s->print_a(s);
s->print_b(s);
}
i=0;
pr_info("Looping over my_stuff_fns and creating modifications\n");
LINKTABLE_FOR_EACH(s, my_stuff_fns) {
s->a = 10;
s->b = 10;
s->print_a(s);
s->print_b(s);
}
}
static int __init stuff_init(void)
{
/* get out of __init context */
schedule_work(&stuff_work);
return 0;
}
static void __exit stuff_exit(void)
{
cancel_work_sync(&stuff_work);
}
module_init(stuff_init)
module_exit(stuff_exit)
MODULE_LICENSE("GPL");
Luis
--
To unsubscribe from this list: send the line "unsubscribe linux-arch" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
