From d11808b5c6b032de4284281ed2ff77ae697a4ebd Mon Sep 17 00:00:00 2001 From: Christopher Kenna Date: Sun, 9 Jan 2011 19:33:49 -0500 Subject: Feather-Trace: dynamic memory allocation and clean exit This patch changes Feather-Trace to allocate memory for the minor devices dynamically, which addresses a long-standing FIXME. It also provides clean module exit and error conditions for Feather-Trace. --- include/litmus/ftdev.h | 10 ++-- litmus/ftdev.c | 131 ++++++++++++++++++++++++++++++++-------------- litmus/sched_task_trace.c | 35 ++++++++++--- litmus/trace.c | 27 ++++++++-- 4 files changed, 148 insertions(+), 55 deletions(-) diff --git a/include/litmus/ftdev.h b/include/litmus/ftdev.h index efb2a5c9a9b0..348387e9adf9 100644 --- a/include/litmus/ftdev.h +++ b/include/litmus/ftdev.h @@ -6,8 +6,6 @@ #include #include -#define MAX_FTDEV_MINORS NR_CPUS - #define FTDEV_ENABLE_CMD 0 #define FTDEV_DISABLE_CMD 1 @@ -32,11 +30,11 @@ struct ftdev_minor { }; struct ftdev { + dev_t major; struct cdev cdev; struct class* class; const char* name; - /* FIXME: don't waste memory, allocate dynamically */ - struct ftdev_minor minor[MAX_FTDEV_MINORS]; + struct ftdev_minor* minor; unsigned int minor_cnt; ftdev_alloc_t alloc; ftdev_free_t free; @@ -46,7 +44,9 @@ struct ftdev { struct ft_buffer* alloc_ft_buffer(unsigned int count, size_t size); void free_ft_buffer(struct ft_buffer* buf); -void ftdev_init(struct ftdev* ftdev, struct module* owner, const char* name); +int ftdev_init( struct ftdev* ftdev, struct module* owner, + const int minor_cnt, const char* name); +void ftdev_exit(struct ftdev* ftdev); int register_ftdev(struct ftdev* ftdev); #endif diff --git a/litmus/ftdev.c b/litmus/ftdev.c index d68d05c6d7dc..4a4b2e3e56c2 100644 --- a/litmus/ftdev.c +++ b/litmus/ftdev.c @@ -310,80 +310,131 @@ struct file_operations ftdev_fops = { .read = ftdev_read, }; - -void ftdev_init(struct ftdev* ftdev, struct module* owner, const char* name) +int ftdev_init( struct ftdev* ftdev, struct module* owner, + const int minor_cnt, const char* name) { - int i, error; + int i, err; + + BUG_ON(minor_cnt < 1); + cdev_init(&ftdev->cdev, &ftdev_fops); ftdev->name = name; + ftdev->minor_cnt = minor_cnt; ftdev->cdev.owner = owner; ftdev->cdev.ops = &ftdev_fops; - ftdev->minor_cnt = 0; - for (i = 0; i < MAX_FTDEV_MINORS; i++) { + ftdev->alloc = NULL; + ftdev->free = NULL; + ftdev->can_open = NULL; + + ftdev->minor = kcalloc(ftdev->minor_cnt, sizeof(*ftdev->minor), + GFP_KERNEL); + if (!ftdev->minor) { + printk(KERN_WARNING "ftdev(%s): Could not allocate memory\n", + ftdev->name); + err = -ENOMEM; + goto err_out; + } + + for (i = 0; i < ftdev->minor_cnt; i++) { mutex_init(&ftdev->minor[i].lock); ftdev->minor[i].readers = 0; ftdev->minor[i].buf = NULL; ftdev->minor[i].events = NULL; } - ftdev->alloc = NULL; - ftdev->free = NULL; - ftdev->can_open = NULL; ftdev->class = class_create(owner, ftdev->name); if (IS_ERR(ftdev->class)) { - error = PTR_ERR(ftdev->class); + err = PTR_ERR(ftdev->class); printk(KERN_WARNING "ftdev(%s): " - "Could not create device class.\n", - name); + "Could not create device class.\n", ftdev->name); + goto err_dealloc; } + + return 0; + +err_dealloc: + kfree(ftdev->minor); +err_out: + return err; +} + +/* + * Destroy minor devices up to, but not including, up_to. + */ +static void ftdev_device_destroy(struct ftdev* ftdev, unsigned int up_to) +{ + dev_t minor_cntr; + + if (up_to < 1) + up_to = (ftdev->minor_cnt < 1) ? 0 : ftdev->minor_cnt; + + for (minor_cntr = 0; minor_cntr < up_to; ++minor_cntr) + device_destroy(ftdev->class, MKDEV(ftdev->major, minor_cntr)); +} + +void ftdev_exit(struct ftdev* ftdev) +{ + printk("ftdev(%s): Exiting\n", ftdev->name); + ftdev_device_destroy(ftdev, -1); + cdev_del(&ftdev->cdev); + unregister_chrdev_region(MKDEV(ftdev->major, 0), ftdev->minor_cnt); + class_destroy(ftdev->class); + kfree(ftdev->minor); } int register_ftdev(struct ftdev* ftdev) { struct device **device; - dev_t trace_dev; - int error = 0, major, i; + dev_t trace_dev_tmp, minor_cntr; + int err; - error = alloc_chrdev_region(&trace_dev, 0, ftdev->minor_cnt, + err = alloc_chrdev_region(&trace_dev_tmp, 0, ftdev->minor_cnt, ftdev->name); - major = MAJOR(trace_dev); - if (error) - { + if (err) { printk(KERN_WARNING "ftdev(%s): " - "Could not register major/minor number %d/%u\n", - ftdev->name, major, ftdev->minor_cnt); - goto out; + "Could not allocate char. device region (%d minors)\n", + ftdev->name, ftdev->minor_cnt); + goto err_out; } - error = cdev_add(&ftdev->cdev, trace_dev, ftdev->minor_cnt); - if (error) { + + ftdev->major = MAJOR(trace_dev_tmp); + + err = cdev_add(&ftdev->cdev, trace_dev_tmp, ftdev->minor_cnt); + if (err) { printk(KERN_WARNING "ftdev(%s): " - "Could not add cdev for major/minor = %d/%u.\n", - ftdev->name, major, ftdev->minor_cnt); - goto out; + "Could not add cdev for major %u with %u minor(s).\n", + ftdev->name, ftdev->major, ftdev->minor_cnt); + goto err_unregister; } - /* - * create all the minor devices - */ - for (i = 0; i < ftdev->minor_cnt; ++i) + /* create the minor device(s) */ + for (minor_cntr = 0; minor_cntr < ftdev->minor_cnt; ++minor_cntr) { - trace_dev = MKDEV(major, i); - device = &(ftdev->minor[i].device); + trace_dev_tmp = MKDEV(ftdev->major, minor_cntr); + device = &ftdev->minor[minor_cntr].device; - *device = device_create(ftdev->class, NULL, trace_dev, NULL, - "%s%d", ftdev->name, i); + *device = device_create(ftdev->class, NULL, trace_dev_tmp, NULL, + "litmus/%s%d", ftdev->name, minor_cntr); if (IS_ERR(*device)) { - error = PTR_ERR(*device); + err = PTR_ERR(*device); printk(KERN_WARNING "ftdev(%s): " "Could not create device major/minor number " - "%d/%d\n", ftdev->name, major, i); + "%u/%u\n", ftdev->name, ftdev->major, + minor_cntr); printk(KERN_WARNING "ftdev(%s): " - "Will not continue creating devices. Tracing " - "may be in an inconsistent state.\n", + "will attempt deletion of allocated devices.\n", ftdev->name); - goto out; + goto err_minors; } } -out: - return error; + + return 0; + +err_minors: + ftdev_device_destroy(ftdev, minor_cntr); + cdev_del(&ftdev->cdev); +err_unregister: + unregister_chrdev_region(MKDEV(ftdev->major, 0), ftdev->minor_cnt); +err_out: + return err; } diff --git a/litmus/sched_task_trace.c b/litmus/sched_task_trace.c index 7ef7b2889b42..a15b25d21a89 100644 --- a/litmus/sched_task_trace.c +++ b/litmus/sched_task_trace.c @@ -38,12 +38,17 @@ static int st_dev_can_open(struct ftdev *dev, unsigned int cpu) static int __init init_sched_task_trace(void) { struct local_buffer* buf; - int i, ok = 0; + int i, ok = 0, err; printk("Allocated %u sched_trace_xxx() events per CPU " "(buffer size: %d bytes)\n", NO_EVENTS, (int) sizeof(struct local_buffer)); - ftdev_init(&st_dev, THIS_MODULE, "sched_trace"); - for (i = 0; i < NR_CPUS; i++) { + + err = ftdev_init(&st_dev, THIS_MODULE, + num_online_cpus(), "sched_trace"); + if (err) + goto err_out; + + for (i = 0; i < st_dev.minor_cnt; i++) { buf = &per_cpu(st_event_buffer, i); ok += init_ft_buffer(&buf->ftbuf, NO_EVENTS, sizeof(struct st_event_record), @@ -51,16 +56,32 @@ static int __init init_sched_task_trace(void) buf->record); st_dev.minor[i].buf = &buf->ftbuf; } - if (ok == NR_CPUS) { - st_dev.minor_cnt = NR_CPUS; + if (ok == st_dev.minor_cnt) { st_dev.can_open = st_dev_can_open; - return register_ftdev(&st_dev); + err = register_ftdev(&st_dev); + if (err) + goto err_dealloc; } else { - return -EINVAL; + err = -EINVAL; + goto err_dealloc; } + + return 0; + +err_dealloc: + ftdev_exit(&st_dev); +err_out: + printk(KERN_WARNING "Could not register sched_trace module\n"); + return err; +} + +static void __exit exit_sched_task_trace(void) +{ + ftdev_exit(&st_dev); } module_init(init_sched_task_trace); +module_exit(exit_sched_task_trace); static inline struct st_event_record* get_record(u8 type, struct task_struct* t) diff --git a/litmus/trace.c b/litmus/trace.c index da650dfe7f4d..e7ea1c2ab3e4 100644 --- a/litmus/trace.c +++ b/litmus/trace.c @@ -90,12 +90,33 @@ static void free_timestamp_buffer(struct ftdev* ftdev, unsigned int idx) static int __init init_ft_overhead_trace(void) { + int err; + printk("Initializing Feather-Trace overhead tracing device.\n"); - ftdev_init(&overhead_dev, THIS_MODULE, "ft_trace"); - overhead_dev.minor_cnt = 1; /* only one buffer */ + err = ftdev_init(&overhead_dev, THIS_MODULE, 1, "ft_trace"); + if (err) + goto err_out; + overhead_dev.alloc = alloc_timestamp_buffer; overhead_dev.free = free_timestamp_buffer; - return register_ftdev(&overhead_dev); + + err = register_ftdev(&overhead_dev); + if (err) + goto err_dealloc; + + return 0; + +err_dealloc: + ftdev_exit(&overhead_dev); +err_out: + printk(KERN_WARNING "Could not register ft_trace module.\n"); + return err; +} + +static void __exit exit_ft_overhead_trace(void) +{ + ftdev_exit(&overhead_dev); } module_init(init_ft_overhead_trace); +module_exit(exit_ft_overhead_trace); -- cgit v1.2.2