From: dtor_core@ameritech.net (Dmitry Torokhov)
To: sensors@Stimpy.netroedge.com, LKML <linux-kernel@vger.kernel.org>,
Greg KH <gregkh@suse.de>, Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Subject: [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes
Date: Thu, 19 May 2005 06:25:53 +0000 [thread overview]
Message-ID: <200504210207.02421.dtor_core@ameritech.net> (raw)
Hi,
I happened to take a look into drivers/w1 and found there bunch of thigs
that IMO should be changed:
- custom-made refcounting is racy
- lifetime rules need to be better enforced
- family framework is insufficient for many advanced w1 devices
- custom-made hotplug notification over netlink should be removed in favor
of standard hotplug notification
- sysfs attributes have unnecessary prefixes (like w1_master) or not needed
at all (w1_master_pointer)
Please consider series of patches below. Unfortunately I do not have any W1
equipment so it was only compile-tested. Please also note that lifetime and
locking rules were changed on object-by-object base so mid-series some stuff
may appear broken but as far as I can see the end result shoudl work pretty
well.
w1-whitespace.patch
Whitespace fixes.
w1-formatting.patch
Some formatting changes to bring the code in line with CodingStyle
guidelines.
w1-master-attr-group.patch
Use attribute_group to create master device attributes to guarantee
proper cleanup in case of failure. Also, hide most of attribute define
ugliness in macros.
w1-slave-attr-group.patc
Add 2 default attributes "family" and "serial" to slave devices, every
1-Wire slave has them. Use attribute_group to handle. The rest of slave
attributes are left as is - will be dealt with later.
w1-lists-cleanup.patch
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...
w1-drop-owner.patch
Drop owner field from w1_master and w1_slave structures. Just having it
there does not magically fixes lifetime rules.
w1-bus-ops.patch
Cleanup bus operations code:
- have bus operatiions accept w1_master instead of unsigned long and
drop data field from w1_bus_master so the structure can be statically
allocated by driver implementing it;
- rename w1_bus_master to w1_bus_ops to avoid confusion with w1_master;
- separate master registering and allocation so drivers can setup proper
link between private data and master and set useable master's name.
w1-fold-w1-int.patch
Fold w1_int.c into w1.c - there is no point in artificially separating
code for master devices between 2 files.
w1-drop-netlink.patch
Drop custom-made hotplug over netlink notification from w1 core.
Standard hotplug mechanism should work just fine (patch will follow).
w1-drop-control-thread.patch
Drop control thread from w1 core, whatever it does can also be done in
the context of w1_remove_master_device. Also, pin the module when
registering new master device to make sure that w1 core is not unloaded
until last device is gone. This simplifies logic a lot.
w1-move-search-to-io.patch
Move w1_search function to w1_io.c to be with the rest of IO code.
w1-master-drop-attrs.patch
Get rid of unneeded master device attributes:
- 'pointer' and 'attempts' are meaningless for userspace;
- information provided by 'slaves' and 'slave_count' can be gathered
from other sysfs bits;
- w1_slave_found has to be rearranged now that slave_count field is gone.
w1-master-attr-cleanup.patch
Clean-up master attribute implementation:
- drop unnecessary "w1_master" prefix from attribute names;
- do not acquire master->mutex when accessing attributes;
- move attribute code "closer" to the rest of master code.
w1-master-scan-interval.patch
More master attributes changes:
- rename timeout parameter/attribute to scan_interval to better
reflect its purpose;
- make scan_timeout be a per-device attribute and allow changing
it from userspace via sysfs;
- allow changing max_slave_count it from userspace as well.
w1-master-add-ttl-attr.patch
Add slave_ttl attribute to w1 masters.
w1-master-cleanup.patch
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.
w1-slave-cleanup.patch
Clean-up slave device implementation:
- get rid of separate refcount, rely on driver model to enforce
lifetime rules;
- pin w1 module until slave device is registered with sysfs to make
sure W1 core stays loaded.
- drop 'name' attribute as we already have it in bus_id.
w1-family-cleanup.patch
Clean-up family implementation:
- get rid of w1_family_ops and template attributes in w1_slave
structure and have family drivers create necessary attributes
themselves. There are too many different devices using 1-Wire
interface and it is impossible to fit them all into single
attribute model. If interface unification is needed it can be
done by building cross-bus class hierarchy.
- rename w1_smem to w1_sernum because devices are called Silicon
serial numbers, they have address (ID) but don't have memory
in regular sense.
- rename w1_therm to w1_thermal.
w1-family-is-driver.patch
Convert family into proper device-model drivers:
- embed driver structure into w1_family and register with the
driver core;
- do not try to manually bind slaves to familes, leave it to
the driver core;
- fold w1_family.c into w1.c
w1-device-id.patch
Support for automatic family drivers loading via hotplug:
- allow family drivers support list of families;
- export supported families through MODULE_DEVICE_TABLE.
w1-hotplug.patch
Implement W1 bus hotplug handler. Slave devices will define
FID=%02x (family ID) end SN=%024llX environment variables.
w1-module-attrs.patch
Allow changing w1 module parameters through sysfs, add parameter
descriptions and document them in Documentation/kernel-parameters.txt
Thanks!
--
Dmitry
WARNING: multiple messages have this Message-ID (diff)
From: Dmitry Torokhov <dtor_core@ameritech.net>
To: sensors@Stimpy.netroedge.com, LKML <linux-kernel@vger.kernel.org>,
Greg KH <gregkh@suse.de>, Evgeniy Polyakov <johnpol@2ka.mipt.ru>
Subject: [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes
Date: Thu, 21 Apr 2005 02:07:02 -0500 [thread overview]
Message-ID: <200504210207.02421.dtor_core@ameritech.net> (raw)
Hi,
I happened to take a look into drivers/w1 and found there bunch of thigs
that IMO should be changed:
- custom-made refcounting is racy
- lifetime rules need to be better enforced
- family framework is insufficient for many advanced w1 devices
- custom-made hotplug notification over netlink should be removed in favor
of standard hotplug notification
- sysfs attributes have unnecessary prefixes (like w1_master) or not needed
at all (w1_master_pointer)
Please consider series of patches below. Unfortunately I do not have any W1
equipment so it was only compile-tested. Please also note that lifetime and
locking rules were changed on object-by-object base so mid-series some stuff
may appear broken but as far as I can see the end result shoudl work pretty
well.
w1-whitespace.patch
Whitespace fixes.
w1-formatting.patch
Some formatting changes to bring the code in line with CodingStyle
guidelines.
w1-master-attr-group.patch
Use attribute_group to create master device attributes to guarantee
proper cleanup in case of failure. Also, hide most of attribute define
ugliness in macros.
w1-slave-attr-group.patc
Add 2 default attributes "family" and "serial" to slave devices, every
1-Wire slave has them. Use attribute_group to handle. The rest of slave
attributes are left as is - will be dealt with later.
w1-lists-cleanup.patch
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...
w1-drop-owner.patch
Drop owner field from w1_master and w1_slave structures. Just having it
there does not magically fixes lifetime rules.
w1-bus-ops.patch
Cleanup bus operations code:
- have bus operatiions accept w1_master instead of unsigned long and
drop data field from w1_bus_master so the structure can be statically
allocated by driver implementing it;
- rename w1_bus_master to w1_bus_ops to avoid confusion with w1_master;
- separate master registering and allocation so drivers can setup proper
link between private data and master and set useable master's name.
w1-fold-w1-int.patch
Fold w1_int.c into w1.c - there is no point in artificially separating
code for master devices between 2 files.
w1-drop-netlink.patch
Drop custom-made hotplug over netlink notification from w1 core.
Standard hotplug mechanism should work just fine (patch will follow).
w1-drop-control-thread.patch
Drop control thread from w1 core, whatever it does can also be done in
the context of w1_remove_master_device. Also, pin the module when
registering new master device to make sure that w1 core is not unloaded
until last device is gone. This simplifies logic a lot.
w1-move-search-to-io.patch
Move w1_search function to w1_io.c to be with the rest of IO code.
w1-master-drop-attrs.patch
Get rid of unneeded master device attributes:
- 'pointer' and 'attempts' are meaningless for userspace;
- information provided by 'slaves' and 'slave_count' can be gathered
from other sysfs bits;
- w1_slave_found has to be rearranged now that slave_count field is gone.
w1-master-attr-cleanup.patch
Clean-up master attribute implementation:
- drop unnecessary "w1_master" prefix from attribute names;
- do not acquire master->mutex when accessing attributes;
- move attribute code "closer" to the rest of master code.
w1-master-scan-interval.patch
More master attributes changes:
- rename timeout parameter/attribute to scan_interval to better
reflect its purpose;
- make scan_timeout be a per-device attribute and allow changing
it from userspace via sysfs;
- allow changing max_slave_count it from userspace as well.
w1-master-add-ttl-attr.patch
Add slave_ttl attribute to w1 masters.
w1-master-cleanup.patch
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.
w1-slave-cleanup.patch
Clean-up slave device implementation:
- get rid of separate refcount, rely on driver model to enforce
lifetime rules;
- pin w1 module until slave device is registered with sysfs to make
sure W1 core stays loaded.
- drop 'name' attribute as we already have it in bus_id.
w1-family-cleanup.patch
Clean-up family implementation:
- get rid of w1_family_ops and template attributes in w1_slave
structure and have family drivers create necessary attributes
themselves. There are too many different devices using 1-Wire
interface and it is impossible to fit them all into single
attribute model. If interface unification is needed it can be
done by building cross-bus class hierarchy.
- rename w1_smem to w1_sernum because devices are called Silicon
serial numbers, they have address (ID) but don't have memory
in regular sense.
- rename w1_therm to w1_thermal.
w1-family-is-driver.patch
Convert family into proper device-model drivers:
- embed driver structure into w1_family and register with the
driver core;
- do not try to manually bind slaves to familes, leave it to
the driver core;
- fold w1_family.c into w1.c
w1-device-id.patch
Support for automatic family drivers loading via hotplug:
- allow family drivers support list of families;
- export supported families through MODULE_DEVICE_TABLE.
w1-hotplug.patch
Implement W1 bus hotplug handler. Slave devices will define
FID=%02x (family ID) end SN=%024llX environment variables.
w1-module-attrs.patch
Allow changing w1 module parameters through sysfs, add parameter
descriptions and document them in Documentation/kernel-parameters.txt
Thanks!
--
Dmitry
next reply other threads:[~2005-05-19 6:25 UTC|newest]
Thread overview: 88+ messages / expand[flat|nested] mbox.gz Atom feed top
2005-04-21 7:07 Dmitry Torokhov [this message]
2005-05-19 6:25 ` [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes Dmitry Torokhov
2005-04-21 7:08 ` [RFC/PATCH 1/22] W1: whitespace fixes Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:08 ` [RFC/PATCH 2/22] W1: formatting fixes Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:09 ` [RFC/PATCH 3/22] W1: use attribute group for master's attributes Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:10 ` [RFC/PATCH 4/22] W1: use attribute group for slave's attributes Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:11 ` [RFC/PATCH 5/22] W1: list handling cleanup Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:13 ` [RFC/PATCH 6/22] W1: drop owner field from master and slave structures Dmitry Torokhov
2005-05-19 6:25 ` [RFC/PATCH 6/22] W1: drop owner field from master and slave Dmitry Torokhov
2005-04-21 7:13 ` [RFC/PATCH 7/22] W1: bus operations cleanup Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:15 ` [RFC/PATCH 8/22] W1: merge master code into one file Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:16 ` [RFC/PATCH 9/22] W1: drop custom hotplug over netlink notification Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:17 ` [RFC/PATCH 10/22] W1: drop main control thread Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:18 ` [RFC/PATCH 11/22] W1: move w1_search to the rest of IO code Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:19 ` [RFC/PATCH 12/22] W1: drop unneeded master attributes Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:20 ` [RFC/PATCH 13/22] W1: cleanup master attributes handling Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:21 ` [RFC/PATCH 14/22] W1: rename timeout to scan_interval Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:22 ` [RFC/PATCH 15/22] W1: add slave_ttl master attribute Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:23 ` [RFC/PATCH 16/22] W1: cleanup masters refcounting & more Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:23 ` [RFC/PATCH 17/22] W1: cleanup slave " Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:25 ` [RFC/PATCH 18/22] W1: cleanup family implementation Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:26 ` [RFC/PATCH 19/22] W1: convert families to be proper sysfs rivers Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:27 ` [RFC/PATCH 20/22] W1: add w1_device_id/MODULE_DEVICE_TABLE for automatic driver loading Dmitry Torokhov
2005-05-19 6:25 ` [RFC/PATCH 20/22] W1: add w1_device_id/MODULE_DEVICE_TABLE for Dmitry Torokhov
2005-04-21 7:36 ` [RFC/PATCH 21/22] W1: implement standard hotplug handler Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 7:38 ` [RFC/PATCH 22/22] W1: expose module parameters in sysfs Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-21 13:18 ` [RFC/PATCH 0/22] W1: sysfs, lifetime and other fixes Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-21 14:31 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-25 9:08 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-25 16:32 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-25 19:26 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-25 21:32 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-26 7:19 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-25 20:15 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-25 20:22 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-26 6:43 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-26 6:50 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-26 7:06 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-26 7:16 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-26 7:35 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-26 7:00 ` Greg KH
2005-05-19 6:25 ` Greg KH
2005-04-26 7:17 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-26 6:58 ` Greg KH
2005-05-19 6:25 ` Greg KH
2005-04-21 16:09 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-25 9:11 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
2005-04-25 16:36 ` Dmitry Torokhov
2005-05-19 6:25 ` Dmitry Torokhov
2005-04-25 19:32 ` Evgeniy Polyakov
2005-05-19 6:25 ` Evgeniy Polyakov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=200504210207.02421.dtor_core@ameritech.net \
--to=dtor_core@ameritech.net \
--cc=gregkh@suse.de \
--cc=johnpol@2ka.mipt.ru \
--cc=linux-kernel@vger.kernel.org \
--cc=sensors@Stimpy.netroedge.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.