lkml.org 
[lkml]   [2005]   [Apr]   [21]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    Patch in this message
    /
    From
    Subject[RFC/PATCH 16/22] W1: cleanup masters refcounting & more
    Date
    W1: clean-up master device implementation:
    - get rid of separate refcount, rely on driver model to
    enforce lifetime rules;
    - use atomic to generate unique master IDs;
    - drop unused fields.

    Signed-off-by: Dmitry Torokhov <dtor@mail.ru>
    ---

    w1.c | 263 ++++++++++++++++++++++++++-----------------------------------------
    w1.h | 17 +---
    2 files changed, 108 insertions(+), 172 deletions(-)

    Index: dtor/drivers/w1/w1.h
    ===================================================================
    --- dtor.orig/drivers/w1/w1.h
    +++ dtor/drivers/w1/w1.h
    @@ -103,30 +103,21 @@ struct w1_bus_ops

    struct w1_master
    {
    + void *private;
    +
    unsigned char name[W1_MAXNAMELEN];
    struct list_head slist;
    int max_slave_count;
    int slave_ttl;
    int scan_interval;
    - int initialized;
    - u32 id;
    -
    - atomic_t refcnt;
    -
    - void *private;

    - int need_exit;
    pid_t kpid;
    - struct semaphore mutex;

    - struct device_driver *driver;
    struct device dev;
    - struct completion dev_released;
    - struct completion dev_exited;
    + struct completion thread_exited;
    + struct semaphore mutex;

    struct w1_bus_ops *bus_ops;
    -
    - u32 seq, groups;
    };
    #define to_w1_master(dev) container_of((dev), struct w1_master, dev)

    Index: dtor/drivers/w1/w1.c
    ===================================================================
    --- dtor.orig/drivers/w1/w1.c
    +++ dtor/drivers/w1/w1.c
    @@ -50,49 +50,19 @@ module_param_named(scan_interval, w1_sca
    module_param_named(max_slave_count, w1_max_slave_count, int, 0);
    module_param_named(slave_ttl, w1_max_slave_ttl, int, 0);

    -static u32 w1_ids = 1;
    -
    static int w1_master_match(struct device *dev, struct device_driver *drv)
    {
    return 1;
    }

    -static int w1_master_probe(struct device *dev)
    -{
    - return -ENODEV;
    -}
    -
    -static int w1_master_remove(struct device *dev)
    -{
    - return 0;
    -}
    -
    -static void w1_master_release(struct device *dev)
    -{
    - struct w1_master *md = container_of(dev, struct w1_master, dev);
    -
    - complete(&md->dev_released);
    -}
    -
    -
    static struct bus_type w1_bus_type = {
    .name = "w1",
    .match = w1_master_match,
    };

    -struct device_driver w1_driver = {
    - .name = "w1_driver",
    - .bus = &w1_bus_type,
    - .probe = w1_master_probe,
    - .remove = w1_master_remove,
    -};
    -
    -struct device w1_device = {
    - .parent = NULL,
    +struct device_driver w1_master_driver = {
    + .name = "master",
    .bus = &w1_bus_type,
    - .bus_id = "w1 bus master",
    - .driver = &w1_driver,
    - .release = &w1_master_release
    };

    static ssize_t w1_slave_attribute_show_family(struct device *dev, char *buf)
    @@ -166,7 +136,6 @@ static int __w1_attach_slave_device(stru
    int err;

    sl->dev.parent = &sl->master->dev;
    - sl->dev.driver = sl->master->driver;
    sl->dev.bus = &w1_bus_type;
    sl->dev.release = &w1_slave_release;

    @@ -346,52 +315,44 @@ static void w1_slave_found(struct w1_mas
    w1_attach_slave_device(dev, reg_num);
    }

    -
    -static int w1_process(void *data)
    +static void w1_master_scan_slaves(struct w1_master *master)
    {
    - struct w1_master *dev = (struct w1_master *) data;
    struct w1_slave *slave, *next;

    - daemonize("%s", dev->name);
    - allow_signal(SIGTERM);
    -
    - while (!dev->need_exit) {
    - try_to_freeze(PF_FREEZE);
    - msleep_interruptible(dev->scan_interval * 1000);
    -
    - if (signal_pending(current))
    - flush_signals(current);
    -
    - if (dev->need_exit)
    - break;
    -
    - if (!dev->initialized)
    - continue;
    -
    - if (down_interruptible(&dev->mutex))
    - continue;
    + if (down_interruptible(&master->mutex))
    + return;

    - list_for_each_entry(slave, &dev->slist, node)
    - clear_bit(W1_SLAVE_ACTIVE, &slave->flags);
    + list_for_each_entry(slave, &master->slist, node)
    + clear_bit(W1_SLAVE_ACTIVE, &slave->flags);

    - w1_search_devices(dev, w1_slave_found);
    + w1_search_devices(master, w1_slave_found);

    - list_for_each_entry_safe(slave, next, &dev->slist, node) {
    + list_for_each_entry_safe(slave, next, &master->slist, node) {

    - if (!test_bit(W1_SLAVE_ACTIVE, &slave->flags) &&
    - !--slave->ttl) {
    - list_del(&slave->node);
    - w1_slave_detach(slave);
    - kfree(slave);
    - }
    + if (!test_bit(W1_SLAVE_ACTIVE, &slave->flags) &&
    + !--slave->ttl) {
    + list_del(&slave->node);
    + w1_slave_detach(slave);
    + kfree(slave);
    }
    - up(&dev->mutex);
    }
    + up(&master->mutex);
    +}
    +
    +static int w1_process(void *data)
    +{
    + struct w1_master *master = (struct w1_master *) data;

    - atomic_dec(&dev->refcnt);
    - complete_and_exit(&dev->dev_exited, 0);
    + daemonize("%s", master->dev.bus_id);
    + allow_signal(SIGTERM);

    - return 0;
    + do {
    + w1_master_scan_slaves(master);
    + msleep_interruptible(master->scan_interval * 1000);
    + try_to_freeze(PF_FREEZE);
    + } while (!signal_pending(current));
    +
    + complete_and_exit(&master->thread_exited, 0);
    }

    static ssize_t w1_master_attribute_show_name(struct device *dev, char *buf)
    @@ -497,128 +458,110 @@ static struct attribute_group w1_master_
    .attrs = w1_master_default_attrs,
    };

    +static void w1_release_master_device(struct device *dev)
    +{
    + struct w1_master *master = to_w1_master(dev);
    +
    + kfree(master);
    + module_put(THIS_MODULE);
    +}
    +
    struct w1_master *w1_allocate_master_device(void)
    {
    - struct w1_master *dev;
    + struct w1_master *master;

    - /*
    - * We are in process context(kernel thread), so can sleep.
    - */
    - dev = kcalloc(1, sizeof(struct w1_master), GFP_KERNEL);
    - if (!dev) {
    + master = kcalloc(1, sizeof(struct w1_master), GFP_KERNEL);
    + if (!master) {
    printk(KERN_ERR
    - "Failed to allocate %zd bytes for new w1 device.\n",
    - sizeof(struct w1_master));
    + "w1: Failed to allocate %zd bytes for new w1 device.\n",
    + sizeof(struct w1_master));
    return NULL;
    }

    - dev->max_slave_count = w1_max_slave_count;
    - dev->kpid = -1;
    - dev->id = w1_ids++;
    - dev->slave_ttl = w1_max_slave_ttl;
    -
    - atomic_set(&dev->refcnt, 2);
    -
    - INIT_LIST_HEAD(&dev->slist);
    - init_MUTEX(&dev->mutex);
    -
    - init_completion(&dev->dev_released);
    - init_completion(&dev->dev_exited);
    -
    - dev->dev = w1_device;
    - snprintf(dev->dev.bus_id, sizeof(dev->dev.bus_id),
    - "w1_bus_master%u", dev->id);
    + master->max_slave_count = w1_max_slave_count;
    + master->slave_ttl = w1_max_slave_ttl;
    + master->scan_interval = w1_scan_interval;

    - dev->driver = &w1_driver;
    -
    - dev->groups = 23;
    - dev->seq = 1;
    -
    - return dev;
    -}
    -
    -static void w1_free_master_dev(struct w1_master *dev)
    -{
    - device_unregister(&dev->dev);
    - kfree(dev);
    -}
    + INIT_LIST_HEAD(&master->slist);
    + init_MUTEX(&master->mutex);
    + init_completion(&master->thread_exited);

    -static void w1_stop_master_device(struct w1_master *dev)
    -{
    - dev->need_exit = 1;
    - if (kill_proc(dev->kpid, SIGTERM, 1))
    - dev_err(&dev->dev,
    - "Failed to send signal to w1 kernel thread %d.\n",
    - dev->kpid);
    - wait_for_completion(&dev->dev_exited);
    + return master;
    }

    -int w1_add_master_device(struct w1_master *dev)
    +int w1_add_master_device(struct w1_master *master)
    {
    + static atomic_t master_no = ATOMIC_INIT(0);
    int error;

    - error = device_register(&dev->dev);
    + snprintf(master->dev.bus_id, sizeof(master->dev.bus_id),
    + "master%lu", (unsigned long)atomic_inc_return(&master_no));
    + master->dev.bus = &w1_bus_type;
    + master->dev.release = w1_release_master_device;
    + master->dev.driver = &w1_master_driver;
    +
    + error = device_register(&master->dev);
    if (error) {
    - printk(KERN_ERR "Failed to register master device. err=%d\n", error);
    + printk(KERN_ERR
    + "w1: Failed to register master device. err=%d\n",
    + error);
    return error;
    }

    - dev->kpid = kernel_thread(&w1_process, dev, 0);
    - if (dev->kpid < 0) {
    - dev_err(&dev->dev,
    - "Failed to create new kernel thread. err=%d\n",
    - dev->kpid);
    - error = dev->kpid;
    - goto err_out_free_dev;
    - }
    -
    - error = sysfs_create_group(&dev->dev.kobj, &w1_master_defattr_group);
    - if (error)
    - goto err_out_kill_thread;
    -
    - dev->initialized = 1;
    -
    __module_get(THIS_MODULE);

    - return 0;
    + error = sysfs_create_group(&master->dev.kobj, &w1_master_defattr_group);
    + if (error) {
    + printk(KERN_ERR
    + "w1: Failed to create master attributes, err=%d\n",
    + error);
    + goto err_out_unregister;
    + }

    -err_out_kill_thread:
    - w1_stop_master_device(dev);
    + master->kpid = kernel_thread(&w1_process, master, 0);
    + if (master->kpid < 0) {
    + printk(KERN_ERR
    + "w1: Failed to create new kernel thread, err=%d\n",
    + master->kpid);
    + error = master->kpid;
    + goto err_out_remove_attrs;
    + }

    -err_out_free_dev:
    - w1_free_master_dev(dev);
    + return 0;

    +err_out_remove_attrs:
    + sysfs_remove_group(&master->dev.kobj, &w1_master_defattr_group);
    +err_out_unregister:
    + device_unregister(&master->dev);
    return error;
    }

    -void w1_remove_master_device(struct w1_master *dev)
    +static void w1_stop_master_device(struct w1_master *master)
    +{
    + if (kill_proc(master->kpid, SIGTERM, 1))
    + printk(KERN_ERR
    + "w1: Failed to send signal to w1 kernel thread %d.\n",
    + master->kpid);
    + wait_for_completion(&master->thread_exited);
    +}
    +
    +void w1_remove_master_device(struct w1_master *master)
    {
    struct w1_slave *slave, *next;

    - w1_stop_master_device(dev);
    + w1_stop_master_device(master);

    - list_for_each_entry_safe(slave, next, &dev->slist, node) {
    + list_for_each_entry_safe(slave, next, &master->slist, node) {
    list_del(&slave->node);
    w1_slave_detach(slave);
    kfree(slave);
    }

    - sysfs_remove_group(&dev->dev.kobj, &w1_master_defattr_group);
    -
    - while (atomic_read(&dev->refcnt)) {
    - printk(KERN_INFO "Waiting for %s to become free: refcnt=%d.\n",
    - dev->name, atomic_read(&dev->refcnt));
    -
    - if (msleep_interruptible(1000))
    - flush_signals(current);
    - }
    -
    - w1_free_master_dev(dev);
    - module_put(THIS_MODULE);
    + sysfs_remove_group(&master->dev.kobj, &w1_master_defattr_group);
    + device_unregister(&master->dev);
    }

    -
    -int w1_init(void)
    +static int w1_init(void)
    {
    int error;

    @@ -626,14 +569,16 @@ int w1_init(void)

    error = bus_register(&w1_bus_type);
    if (error) {
    - printk(KERN_ERR "Failed to register bus. err=%d.\n", error);
    + printk(KERN_ERR "w1: Failed to register bus. err=%d.\n",
    + error);
    return error;
    }

    - error = driver_register(&w1_driver);
    + error = driver_register(&w1_master_driver);
    if (error) {
    printk(KERN_ERR
    - "Failed to register master driver. err=%d.\n", error);
    + "w1: Failed to register master driver. err=%d.\n",
    + error);
    bus_unregister(&w1_bus_type);
    return error;
    }
    @@ -641,14 +586,14 @@ int w1_init(void)
    return 0;
    }

    -void w1_fini(void)
    +static void w1_exit(void)
    {
    - driver_unregister(&w1_driver);
    + driver_unregister(&w1_master_driver);
    bus_unregister(&w1_bus_type);
    }

    module_init(w1_init);
    -module_exit(w1_fini);
    +module_exit(w1_exit);

    EXPORT_SYMBOL(w1_allocate_master_device);
    EXPORT_SYMBOL(w1_add_master_device);
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/
    \
     
     \ /
      Last update: 2005-04-21 10:20    [W:0.050 / U:0.092 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site