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 5/22] W1: list handling cleanup
    Date
    W1: list handling cleanup. Most of the list_for_each_safe users
    don't need *_safe variant, *_entry variant is better suited
    in most places. Also, checking retrieved list element for
    null is a bit pointless...

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

    w1.c | 131 +++++++++++++++++++++++----------------------------------------
    w1.h | 6 +-
    w1_int.c | 23 +++--------
    3 files changed, 58 insertions(+), 102 deletions(-)

    Index: dtor/drivers/w1/w1.c
    ===================================================================
    --- dtor.orig/drivers/w1/w1.c
    +++ dtor/drivers/w1/w1.c
    @@ -184,6 +184,7 @@ static ssize_t w1_master_attribute_show_

    {
    struct w1_master *md = container_of(dev, struct w1_master, dev);
    + struct w1_slave *slave;
    int c = PAGE_SIZE;

    if (down_interruptible(&md->mutex))
    @@ -191,16 +192,9 @@ static ssize_t w1_master_attribute_show_

    if (md->slave_count == 0)
    c -= snprintf(buf + PAGE_SIZE - c, c, "not found.\n");
    - else {
    - struct list_head *ent, *n;
    - struct w1_slave *sl;
    -
    - list_for_each_safe(ent, n, &md->slist) {
    - sl = list_entry(ent, struct w1_slave, w1_slave_entry);
    -
    - c -= snprintf(buf + PAGE_SIZE - c, c, "%s\n", sl->name);
    - }
    - }
    + else
    + list_for_each_entry(slave, &md->slist, node)
    + c -= snprintf(buf + PAGE_SIZE - c, c, "%s\n", slave->name);

    up(&md->mutex);

    @@ -391,7 +385,7 @@ static int __w1_attach_slave_device(stru
    return err;
    }

    - list_add_tail(&sl->w1_slave_entry, &sl->master->slist);
    + list_add_tail(&sl->node, &sl->master->slist);

    return 0;
    }
    @@ -415,7 +409,7 @@ static int w1_attach_slave_device(struct

    sl->owner = THIS_MODULE;
    sl->master = dev;
    - set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags);
    + set_bit(W1_SLAVE_ACTIVE, &sl->flags);

    memcpy(&sl->reg_num, rn, sizeof(sl->reg_num));
    atomic_set(&sl->refcnt, 0);
    @@ -484,29 +478,27 @@ static void w1_slave_detach(struct w1_sl

    static struct w1_master *w1_search_master(unsigned long data)
    {
    - struct w1_master *dev;
    + struct w1_master *master;
    int found = 0;

    spin_lock_irq(&w1_mlock);
    - list_for_each_entry(dev, &w1_masters, w1_master_entry) {
    - if (dev->bus_master->data == data) {
    + list_for_each_entry(master, &w1_masters, node) {
    + if (master->bus_master->data == data) {
    found = 1;
    - atomic_inc(&dev->refcnt);
    + atomic_inc(&master->refcnt);
    break;
    }
    }
    spin_unlock_irq(&w1_mlock);

    - return (found)?dev:NULL;
    + return found ? master : NULL;
    }

    void w1_slave_found(unsigned long data, u64 rn)
    {
    int slave_count;
    - struct w1_slave *sl;
    - struct list_head *ent;
    + struct w1_slave *slave;
    struct w1_reg_num *tmp;
    - int family_found = 0;
    struct w1_master *dev;

    dev = w1_search_master(data);
    @@ -519,20 +511,14 @@ void w1_slave_found(unsigned long data,
    tmp = (struct w1_reg_num *) &rn;

    slave_count = 0;
    - list_for_each(ent, &dev->slist) {
    + list_for_each_entry(slave, &dev->slist, node) {

    - sl = list_entry(ent, struct w1_slave, w1_slave_entry);
    -
    - if (sl->reg_num.family == tmp->family &&
    - sl->reg_num.id == tmp->id &&
    - sl->reg_num.crc == tmp->crc) {
    - set_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags);
    - break;
    - } else if (sl->reg_num.family == tmp->family) {
    - family_found = 1;
    + if (slave->reg_num.family == tmp->family &&
    + slave->reg_num.id == tmp->id &&
    + slave->reg_num.crc == tmp->crc) {
    + set_bit(W1_SLAVE_ACTIVE, &slave->flags);
    break;
    }
    -
    slave_count++;
    }

    @@ -635,9 +621,8 @@ void w1_search(struct w1_master *dev)

    int w1_control(void *data)
    {
    - struct w1_slave *sl;
    - struct w1_master *dev;
    - struct list_head *ent, *ment, *n, *mn;
    + struct w1_slave *slave, *nexts;
    + struct w1_master *master, *nextm;
    int err, have_to_wait = 0;

    daemonize("w1_control");
    @@ -652,52 +637,42 @@ int w1_control(void *data)
    if (signal_pending(current))
    flush_signals(current);

    - list_for_each_safe(ment, mn, &w1_masters) {
    - dev = list_entry(ment, struct w1_master, w1_master_entry);
    + list_for_each_entry_safe(master, nextm, &w1_masters, node) {

    - if (!control_needs_exit && !dev->need_exit)
    + if (!control_needs_exit && !master->need_exit)
    continue;
    /*
    * Little race: we can create thread but not set the flag.
    * Get a chance for external process to set flag up.
    */
    - if (!dev->initialized) {
    + if (!master->initialized) {
    have_to_wait = 1;
    continue;
    }

    spin_lock(&w1_mlock);
    - list_del(&dev->w1_master_entry);
    + list_del(&master->node);
    spin_unlock(&w1_mlock);

    if (control_needs_exit) {
    - dev->need_exit = 1;
    + master->need_exit = 1;

    - err = kill_proc(dev->kpid, SIGTERM, 1);
    + err = kill_proc(master->kpid, SIGTERM, 1);
    if (err)
    - dev_err(&dev->dev,
    + dev_err(&master->dev,
    "Failed to send signal to w1 kernel thread %d.\n",
    - dev->kpid);
    + master->kpid);
    }

    - wait_for_completion(&dev->dev_exited);
    + wait_for_completion(&master->dev_exited);

    - list_for_each_safe(ent, n, &dev->slist) {
    - sl = list_entry(ent, struct w1_slave, w1_slave_entry);
    -
    - if (!sl)
    - dev_warn(&dev->dev,
    - "%s: slave entry is NULL.\n",
    - __func__);
    - else {
    - list_del(&sl->w1_slave_entry);
    -
    - w1_slave_detach(sl);
    - kfree(sl);
    - }
    + list_for_each_entry_safe(slave, nexts, &master->slist, node) {
    + list_del(&slave->node);
    + w1_slave_detach(slave);
    + kfree(slave);
    }
    - w1_destroy_master_attributes(dev);
    - atomic_dec(&dev->refcnt);
    + w1_destroy_master_attributes(master);
    + atomic_dec(&master->refcnt);
    }
    }

    @@ -707,8 +682,7 @@ int w1_control(void *data)
    int w1_process(void *data)
    {
    struct w1_master *dev = (struct w1_master *) data;
    - struct list_head *ent, *n;
    - struct w1_slave *sl;
    + struct w1_slave *slave, *next;

    daemonize("%s", dev->name);
    allow_signal(SIGTERM);
    @@ -729,27 +703,21 @@ int w1_process(void *data)
    if (down_interruptible(&dev->mutex))
    continue;

    - list_for_each_safe(ent, n, &dev->slist) {
    - sl = list_entry(ent, struct w1_slave, w1_slave_entry);
    -
    - if (sl)
    - clear_bit(W1_SLAVE_ACTIVE, (long *)&sl->flags);
    - }
    + list_for_each_entry(slave, &dev->slist, node)
    + clear_bit(W1_SLAVE_ACTIVE, &slave->flags);

    w1_search_devices(dev, w1_slave_found);

    - list_for_each_safe(ent, n, &dev->slist) {
    - sl = list_entry(ent, struct w1_slave, w1_slave_entry);
    -
    - if (sl && !test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags) && !--sl->ttl) {
    - list_del (&sl->w1_slave_entry);
    -
    - w1_slave_detach (sl);
    - kfree (sl);
    + list_for_each_entry_safe(slave, next, &dev->slist, node) {

    + if (test_bit(W1_SLAVE_ACTIVE, &slave->flags))
    + slave->ttl = dev->slave_ttl;
    + else if (!--slave->ttl) {
    + list_del(&slave->node);
    + w1_slave_detach(slave);
    + kfree(slave);
    dev->slave_count--;
    - } else if (test_bit(W1_SLAVE_ACTIVE, (unsigned long *)&sl->flags))
    - sl->ttl = dev->slave_ttl;
    + }
    }
    up(&dev->mutex);
    }
    @@ -802,13 +770,10 @@ err_out_exit_init:

    void w1_fini(void)
    {
    - struct w1_master *dev;
    - struct list_head *ent, *n;
    + struct w1_master *master, *next;

    - list_for_each_safe(ent, n, &w1_masters) {
    - dev = list_entry(ent, struct w1_master, w1_master_entry);
    - __w1_remove_master_device(dev);
    - }
    + list_for_each_entry_safe(master, next, &w1_masters, node)
    + __w1_remove_master_device(master);

    control_needs_exit = 1;

    Index: dtor/drivers/w1/w1.h
    ===================================================================
    --- dtor.orig/drivers/w1/w1.h
    +++ dtor/drivers/w1/w1.h
    @@ -66,11 +66,11 @@ struct w1_slave
    {
    struct module *owner;
    unsigned char name[W1_MAXNAMELEN];
    - struct list_head w1_slave_entry;
    + struct list_head node;
    struct w1_reg_num reg_num;
    atomic_t refcnt;
    u8 rom[9];
    - u32 flags;
    + unsigned long flags;
    int ttl;

    struct w1_master *master;
    @@ -107,7 +107,7 @@ struct w1_bus_master

    struct w1_master
    {
    - struct list_head w1_master_entry;
    + struct list_head node;
    struct module *owner;
    unsigned char name[W1_MAXNAMELEN];
    struct list_head slist;
    Index: dtor/drivers/w1/w1_int.c
    ===================================================================
    --- dtor.orig/drivers/w1/w1_int.c
    +++ dtor/drivers/w1/w1_int.c
    @@ -142,7 +142,7 @@ int w1_add_master_device(struct w1_bus_m
    dev->initialized = 1;

    spin_lock(&w1_mlock);
    - list_add(&dev->w1_master_entry, &w1_masters);
    + list_add(&dev->node, &w1_masters);
    spin_unlock(&w1_mlock);

    msg.id.mst.id = dev->id;
    @@ -196,24 +196,15 @@ void __w1_remove_master_device(struct w1

    void w1_remove_master_device(struct w1_bus_master *bm)
    {
    - struct w1_master *dev = NULL;
    - struct list_head *ent, *n;
    + struct w1_master *dev;

    - list_for_each_safe(ent, n, &w1_masters) {
    - dev = list_entry(ent, struct w1_master, w1_master_entry);
    - if (!dev->initialized)
    - continue;
    -
    - if (dev->bus_master->data == bm->data)
    + list_for_each_entry(dev, &w1_masters, node)
    + if (dev->initialized && dev->bus_master->data == bm->data) {
    + __w1_remove_master_device(dev);
    break;
    - }
    -
    - if (!dev) {
    - printk(KERN_ERR "Device doesn't exist.\n");
    - return;
    - }
    + }

    - __w1_remove_master_device(dev);
    + printk(KERN_ERR "Device doesn't exist.\n");
    }

    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 09:55    [W:5.796 / U:0.432 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site