Skip to contentSkip to conversation

properly create kobjects in acpi/scan.c

PATCH In review Dmitry Torokhov 4 participants 8 messages Aug 24, 2004 4 messages not in the archive

Subsystems (3)
lore
Share & tools
lore
#0 lore raw

PATCH 1/4 properly create kobjects in acpi/scan.c

Hi,

I have a bunch of patches that I would like you to take a look at:

01-acpi-scan-kobject.patch
- properly create/register kobjects - take reference to the parent since kobject_unregister will try dropping it potentially (if ACPI was ever to release created devices) causing parent be destroyed early) and use kobject_set_name to properly set name.

02-acpi-event-code.patch
- move event (userspace signalling) code from acpi/bus.c to acpi/event.c as too many implementation details were exported, plus some additional fixes to the event handling code.

03-acpi-use-rwsem.patch
- kill brandead usage of acpi_device_lock & list_for_eacj_safe. list_for_each_safe is only safe in the sense that current element can be safely removed by the same thread that is using list_for_each_safe. List still has to be protected from other threads for entire duration of list traversal.
Use acpi_subsys.rwsem instead of acpi_device_lock to protect lists.

Btw, why don't we just convert ACPI to be standard driver core bus with standard drivers and devices? I could cook up something... The /sys/firmware/acpi hierarchy is not populated with any useful data yet so there should be no concern of breakign userspace interface (I am mot talking about removing /proc/acpi part at the moment).

04-acpi-multiple-readers.patch
- allow multiple readrs access /proc/acpi/event, every event is delivered to all readers. Also limit number of pending events to 64 (per reader) so if reader is stuck ACPI does not consume all memory.

Can be user by various daemons, I could see cpufreqd listening for battery insertion/removal events and debugging is much easier too.

The patches are against last night pull from Linus repo.

Signature · 52 lines
Dmitry


===================================================================


[email protected], 2004-08-24 01:09:40-05:00, [email protected]
  ACPI: Properly create kobj when registering acpi_device:
        - use kobject_set_name to initialize name;
        - take teference to parent's kobj as kobject_unregister
          will drop it
        - do not call kobject_init and kobject_add, just use
          kobject_register as everyone else does.
  
  Signed-off-by: Dmitry Torokhov <dtor-JGs/[email protected]>


 scan.c |    8 +++-----
 1 files changed, 3 insertions(+), 5 deletions(-)


===================================================================



diff -Nru a/drivers/acpi/scan.c b/drivers/acpi/scan.c
--- a/drivers/acpi/scan.c	2004-08-24 01:16:25 -05:00
+++ b/drivers/acpi/scan.c	2004-08-24 01:16:25 -05:00
@@ -66,13 +66,11 @@
 		list_add_tail(&device->g_list,&acpi_device_list);
 	spin_unlock(&acpi_device_lock);
 
-	kobject_init(&device->kobj);
-	strlcpy(device->kobj.name,device->pnp.bus_id,KOBJ_NAME_LEN);
+	kobject_set_name(&device->kobj, device->pnp.bus_id);
 	if (parent)
-		device->kobj.parent = &parent->kobj;
-	device->kobj.ktype = &ktype_acpi_ns;
+		device->kobj.parent = kobject_get(&parent->kobj);
 	device->kobj.kset = &acpi_namespace_kset;
-	kobject_add(&device->kobj);
+	kobject_register(&device->kobj);
 }
 
 static int


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

A message that is not in the archive (sent off-list or not yet received): 200408240134.16962.dtor_core-yWtbtysYrB+LZ21kGMrzwg@public.gmane.org

#2 lore raw

PATCH 2/4 event code fixes/cleanup

===================================================================

[email protected], 2004-08-24 01:11:02-05:00, [email protected]
ACPI: Move event handling code from bus.c into event.c to cut down
amount of exported implementation details. Also:

- when closing /proc/acpi/event drop all events that have not been read yet so next time it is opened userspace is not fed old events;
- do not reimplement wait_event_interruptible, just use it;
- do not return -EIO if an event is 'stolen', 0 is proper return code;
- tidy up read procedure (ensure that list is not empty before fetching next event)

 drivers/acpi/Makefile     |    4 -
 drivers/acpi/ac.c         |    4 -
 drivers/acpi/acpi_ksyms.c |    9 +-
 drivers/acpi/asus_acpi.c  |    5 -
 drivers/acpi/battery.c    |    3 
 drivers/acpi/bus.c        |   94 ----------------------------
 drivers/acpi/button.c     |    4 -
 drivers/acpi/event.c      |  152 ++++++++++++++++++++++++++++++++++------------
 drivers/acpi/processor.c  |    7 +-
 drivers/acpi/thermal.c    |    9 +-
 include/acpi/acpi_bus.h   |   16 ----
 include/acpi/acpi_event.h |   47 ++++++++++++++
 12 files changed, 190 insertions(+), 164 deletions(-)

===================================================================

drivers/acpi/Makefile +2 -2
diff -Nru a/drivers/acpi/Makefile b/drivers/acpi/Makefile
@@ -32,7 +32,7 @@
 # ACPI Bus and Device Drivers
 #
 obj-$(CONFIG_ACPI_BUS)		+= sleep/
-obj-$(CONFIG_ACPI_BUS)		+= bus.o
+obj-$(CONFIG_ACPI_BUS)		+= bus.o event.o
 obj-$(CONFIG_ACPI_AC) 		+= ac.o
 obj-$(CONFIG_ACPI_BATTERY)	+= battery.o
 obj-$(CONFIG_ACPI_BUTTON)	+= button.o
@@ -42,7 +42,7 @@
 obj-$(CONFIG_ACPI_POWER)	+= power.o
 obj-$(CONFIG_ACPI_PROCESSOR)	+= processor.o
 obj-$(CONFIG_ACPI_THERMAL)	+= thermal.o
-obj-$(CONFIG_ACPI_SYSTEM)	+= system.o event.o
+obj-$(CONFIG_ACPI_SYSTEM)	+= system.o
 obj-$(CONFIG_ACPI_DEBUG)	+= debug.o
 obj-$(CONFIG_ACPI_NUMA)		+= numa.o
 obj-$(CONFIG_ACPI_ASUS)		+= asus_acpi.o
drivers/acpi/ac.c +3 -1
diff -Nru a/drivers/acpi/ac.c b/drivers/acpi/ac.c
@@ -29,7 +29,9 @@
 #include <linux/types.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+
 #include <acpi/acpi_bus.h>
+#include <acpi/acpi_event.h>
 #include <acpi/acpi_drivers.h>
 
 
@@ -220,7 +222,7 @@
 	switch (event) {
 	case ACPI_AC_NOTIFY_STATUS:
 		acpi_ac_get_state(ac);
-		acpi_bus_generate_event(device, event, (u32) ac->state);
+		acpi_generate_event(device, event, (u32) ac->state);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
drivers/acpi/acpi_ksyms.c +6 -3
diff -Nru a/drivers/acpi/acpi_ksyms.c b/drivers/acpi/acpi_ksyms.c
@@ -27,6 +27,7 @@
 #include <linux/acpi.h>
 #include <acpi/acpi.h>
 #include <acpi/acpi_bus.h>
+#include <acpi/acpi_event.h>
 
 
 #ifdef CONFIG_ACPI_INTERPRETER
@@ -120,10 +121,11 @@
 #endif /*CONFIG_ACPI_INTERPRETER*/
 
 
-/* ACPI Bus Driver (acpi_bus.c) */
+/* ACPI Bus Driver */
 
 #ifdef CONFIG_ACPI_BUS
 
+/* bus.c */
 EXPORT_SYMBOL(acpi_fadt);
 EXPORT_SYMBOL(acpi_walk_namespace);
 EXPORT_SYMBOL(acpi_root_dir);
@@ -131,10 +133,11 @@
 EXPORT_SYMBOL(acpi_bus_get_status);
 EXPORT_SYMBOL(acpi_bus_get_power);
 EXPORT_SYMBOL(acpi_bus_set_power);
-EXPORT_SYMBOL(acpi_bus_generate_event);
-EXPORT_SYMBOL(acpi_bus_receive_event);
 EXPORT_SYMBOL(acpi_bus_register_driver);
 EXPORT_SYMBOL(acpi_bus_unregister_driver);
+
+/* event.c */
+EXPORT_SYMBOL(acpi_generate_event);
 
 #endif /*CONFIG_ACPI_BUS*/
 
drivers/acpi/asus_acpi.c +3 -2
diff -Nru a/drivers/acpi/asus_acpi.c b/drivers/acpi/asus_acpi.c
@@ -40,6 +40,7 @@
 #include <linux/proc_fs.h>
 #include <acpi/acpi_drivers.h>
 #include <acpi/acpi_bus.h>
+#include <acpi/acpi_event.h>
 #include <asm/uaccess.h>
 
 #define ASUS_ACPI_VERSION "0.28"
@@ -949,8 +950,8 @@
 		hotk->brightness = (event & ~((u32) BR_DOWN));
 	}
 
-	acpi_bus_generate_event(hotk->device, event,
-				hotk->event_count[event % 128]++);
+	acpi_generate_event(hotk->device, event,
+			    hotk->event_count[event % 128]++);
 
 	return;
 }
drivers/acpi/battery.c +2 -1
diff -Nru a/drivers/acpi/battery.c b/drivers/acpi/battery.c
@@ -31,6 +31,7 @@
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_bus.h>
+#include <acpi/acpi_event.h>
 #include <acpi/acpi_drivers.h>
 
 
@@ -719,7 +720,7 @@
 	case ACPI_BATTERY_NOTIFY_STATUS:
 	case ACPI_BATTERY_NOTIFY_INFO:
 		acpi_battery_check(battery);
-		acpi_bus_generate_event(device, event, battery->flags.present);
+		acpi_generate_event(device, event, battery->flags.present);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
drivers/acpi/bus.c +0 -94

100 lines not on this page · raw message

drivers/acpi/button.c +3 -1
diff -Nru a/drivers/acpi/button.c b/drivers/acpi/button.c
@@ -29,7 +29,9 @@
 #include <linux/types.h>
 #include <linux/proc_fs.h>
 #include <linux/seq_file.h>
+
 #include <acpi/acpi_bus.h>
+#include <acpi/acpi_event.h>
 #include <acpi/acpi_drivers.h>
 
 
@@ -290,7 +292,7 @@
 
 	switch (event) {
 	case ACPI_BUTTON_NOTIFY_STATUS:
-		acpi_bus_generate_event(button->device, event, ++button->pushed);
+		acpi_generate_event(button->device, event, ++button->pushed);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
drivers/acpi/event.c +115 -37

212 lines not on this page · raw message

drivers/acpi/processor.c +4 -3
diff -Nru a/drivers/acpi/processor.c b/drivers/acpi/processor.c
@@ -50,6 +50,7 @@
 
 #include <acpi/acpi_bus.h>
 #include <acpi/acpi_drivers.h>
+#include <acpi/acpi_event.h>
 #include <acpi/processor.h>
 
 
@@ -2312,12 +2313,12 @@
 	switch (event) {
 	case ACPI_PROCESSOR_NOTIFY_PERFORMANCE:
 		acpi_processor_ppc_has_changed(pr);
-		acpi_bus_generate_event(device, event, 
-			pr->performance_platform_limit);
+		acpi_generate_event(device, event,
+				    pr->performance_platform_limit);
 		break;
 	case ACPI_PROCESSOR_NOTIFY_POWER:
 		/* TBD */
-		acpi_bus_generate_event(device, event, 0);
+		acpi_generate_event(device, event, 0);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
drivers/acpi/thermal.c +5 -4
diff -Nru a/drivers/acpi/thermal.c b/drivers/acpi/thermal.c
@@ -42,6 +42,7 @@
 #include <asm/uaccess.h>
 
 #include <acpi/acpi_bus.h>
+#include <acpi/acpi_event.h>
 #include <acpi/acpi_drivers.h>
 
 #define ACPI_THERMAL_COMPONENT		0x04000000
@@ -469,7 +470,7 @@
 		return_VALUE(result);
 
 	printk(KERN_EMERG "Critical temperature reached (%ld C), shutting down.\n", KELVIN_TO_CELSIUS(tz->temperature));
-	acpi_bus_generate_event(device, ACPI_THERMAL_NOTIFY_CRITICAL, tz->trips.critical.flags.enabled);
+	acpi_generate_event(device, ACPI_THERMAL_NOTIFY_CRITICAL, tz->trips.critical.flags.enabled);
 
 	acpi_thermal_call_usermode(ACPI_THERMAL_PATH_POWEROFF);
 
@@ -500,7 +501,7 @@
 	if (result)
 		return_VALUE(result);
 
-	acpi_bus_generate_event(device, ACPI_THERMAL_NOTIFY_HOT, tz->trips.hot.flags.enabled);
+	acpi_generate_event(device, ACPI_THERMAL_NOTIFY_HOT, tz->trips.hot.flags.enabled);
 
 	/* TBD: Call user-mode "sleep(S4)" function */
 
@@ -1199,12 +1200,12 @@
 	case ACPI_THERMAL_NOTIFY_THRESHOLDS:
 		acpi_thermal_get_trip_points(tz);
 		acpi_thermal_check(tz);
-		acpi_bus_generate_event(device, event, 0);
+		acpi_generate_event(device, event, 0);
 		break;
 	case ACPI_THERMAL_NOTIFY_DEVICES:
 		if (tz->flags.devices)
 			acpi_thermal_get_devices(tz);
-		acpi_bus_generate_event(device, event, 0);
+		acpi_generate_event(device, event, 0);
 		break;
 	default:
 		ACPI_DEBUG_PRINT((ACPI_DB_INFO,
include/acpi/acpi_bus.h +0 -16
diff -Nru a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h
@@ -293,20 +293,6 @@
 
 #define acpi_driver_data(d)	((d)->driver_data)
 
-
-/*
- * Events
- * ------
- */
-
-struct acpi_bus_event {
-	struct list_head	node;
-	acpi_device_class	device_class;
-	acpi_bus_id		bus_id;
-	u32			type;
-	u32			data;
-};
-
 extern struct subsystem acpi_subsys;
 
 /*
@@ -317,8 +303,6 @@
 int acpi_bus_get_status (struct acpi_device *device);
 int acpi_bus_get_power (acpi_handle handle, int *state);
 int acpi_bus_set_power (acpi_handle handle, int state);
-int acpi_bus_generate_event (struct acpi_device *device, u8 type, int data);
-int acpi_bus_receive_event (struct acpi_bus_event *event);
 int acpi_bus_register_driver (struct acpi_driver *driver);
 int acpi_bus_unregister_driver (struct acpi_driver *driver);
 
include/acpi/acpi_event.h new file +47 -0

47 lines not on this page · raw message

-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media 100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33 Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

A message that is not in the archive (sent off-list or not yet received): 200408240136.46113.dtor_core-yWtbtysYrB+LZ21kGMrzwg@public.gmane.org

#4 lore raw

PATCH 3/4 use acpi_subsys.rwsem in acpi/scan.c

===================================================================

[email protected], 2004-08-24 01:11:37-05:00, [email protected]
ACPI: Do not use list_for_each_safe as it does not protect from
changes done by other thread, take acpi_subsys.rwsem instead.

 bus.c          |    6 --
 scan.c         |  139 ++++++++++++++++++++++++++-------------------------------
 sleep/main.c   |    2 
 sleep/proc.c   |   34 ++++++-------
 sleep/wakeup.c |  126 ++++++++++++++++++++++-----------------------------
 5 files changed, 139 insertions(+), 168 deletions(-)

===================================================================

drivers/acpi/bus.c +0 -6
diff -Nru a/drivers/acpi/bus.c b/drivers/acpi/bus.c
@@ -27,7 +27,6 @@
 #include <linux/list.h>
 #include <linux/sched.h>
 #include <linux/pm.h>
-#include <linux/device.h>
 #include <linux/proc_fs.h>
 #ifdef CONFIG_X86
 #include <asm/mpspec.h>
@@ -629,8 +628,6 @@
 	return_VALUE(-ENODEV);
 }
 
-decl_subsys(acpi,NULL,NULL);
-
 static int __init acpi_init (void)
 {
 	int			result = 0;
@@ -648,10 +645,7 @@
 		return -ENODEV;
 	}
 
-	firmware_register(&acpi_subsys);
-
 	result = acpi_bus_init();
-
 	if (!result) {
 #ifdef CONFIG_PM
 		if (!PM_IS_ACTIVE())
drivers/acpi/scan.c +65 -74

272 lines not on this page · raw message

drivers/acpi/sleep/main.c +1 -1
diff -Nru a/drivers/acpi/sleep/main.c b/drivers/acpi/sleep/main.c
@@ -89,9 +89,9 @@
 			return error;
 	}
 
+	acpi_enable_wakeup_device(acpi_state);
 
 	local_irq_save(flags);
-	acpi_enable_wakeup_device(acpi_state);
 	switch (pm_state)
 	{
 	case PM_SUSPEND_STANDBY:
drivers/acpi/sleep/proc.c +17 -17
diff -Nru a/drivers/acpi/sleep/proc.c b/drivers/acpi/sleep/proc.c
@@ -354,22 +354,20 @@
 }
 
 extern struct list_head	acpi_wakeup_device_list;
-extern spinlock_t acpi_device_lock;
 
 static int
 acpi_system_wakeup_device_seq_show(struct seq_file *seq, void *offset)
 {
-	struct list_head * node, * next;
+	struct acpi_device *dev;
 
 	seq_printf(seq, "Device	Sleep state	Status\n");
 
-	spin_lock(&acpi_device_lock);
-	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
-		struct acpi_device * dev = container_of(node, struct acpi_device, wakeup_list);
+	down_read(&acpi_subsys.rwsem);
+	list_for_each_entry(dev, &acpi_wakeup_device_list, wakeup_list) {
 
 		if (!dev->wakeup.flags.valid)
 			continue;
-		spin_unlock(&acpi_device_lock);
+
 		if (dev->wakeup.flags.run_wake)
 			seq_printf(seq, "%4s	%4d		%8s\n",
 				dev->pnp.bus_id, (u32) dev->wakeup.sleep_state,
@@ -378,9 +376,9 @@
 			seq_printf(seq, "%4s	%4d		%8s\n",
 				dev->pnp.bus_id, (u32) dev->wakeup.sleep_state,
 				dev->wakeup.state.enabled ? "enabled" : "disabled");
-		spin_lock(&acpi_device_lock);
 	}
-	spin_unlock(&acpi_device_lock);
+	up_read(&acpi_subsys.rwsem);
+
 	return 0;
 }
 
@@ -391,30 +389,32 @@
 	size_t			count,
 	loff_t			*ppos)
 {
-	struct list_head * node, * next;
-	char		strbuf[5];
-	char		str[5] = "";
-	int 		len = count;
+	struct acpi_device	*dev;
+	char			strbuf[5];
+	char			str[5] = "";
+	int 			len = count;
 
 	if (len > 4) len = 4;
 
 	if (copy_from_user(strbuf, buffer, len))
 		return -EFAULT;
+
 	strbuf[len] = '\0';
 	sscanf(strbuf, "%s", str);
 
-	spin_lock(&acpi_device_lock);
-	list_for_each_safe(node, next, &acpi_wakeup_device_list) {
-		struct acpi_device * dev = container_of(node, struct acpi_device, wakeup_list);
+	down_write(&acpi_subsys.rwsem);
+	list_for_each_entry(dev, &acpi_wakeup_device_list, wakeup_list) {
+
 		if (!dev->wakeup.flags.valid)
 			continue;
 
 		if (!strncmp(dev->pnp.bus_id, str, 4)) {
-			dev->wakeup.state.enabled = dev->wakeup.state.enabled ? 0:1;
+			dev->wakeup.state.enabled = dev->wakeup.state.enabled ? 0 : 1;
 			break;
 		}
 	}
-	spin_unlock(&acpi_device_lock);
+	up_write(&acpi_subsys.rwsem);
+
 	return count;
 }
 
drivers/acpi/sleep/wakeup.c +56 -70

206 lines not on this page · raw message

-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media 100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33 Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

A message that is not in the archive (sent off-list or not yet received): 200408240137.55691.dtor_core-yWtbtysYrB+LZ21kGMrzwg@public.gmane.org

#6 lore raw

PATCH 4/4 allow multiple /proc/acpi/event readers

===================================================================

[email protected], 2004-08-24 01:12:17-05:00, [email protected]
ACPI: allow multiple readers for /proc/acpi/event, limit number
of pending events to 64 so if reader is stuck ACPI won't
consume all memory.

 drivers/acpi/event.c      |  148 +++++++++++++++++++++-------------------------
 include/acpi/acpi_event.h |    1 
 2 files changed, 69 insertions(+), 80 deletions(-)

===================================================================

drivers/acpi/event.c +69 -79
diff -Nru a/drivers/acpi/event.c b/drivers/acpi/event.c
@@ -16,10 +16,20 @@
 #define _COMPONENT		ACPI_SYSTEM_COMPONENT
 ACPI_MODULE_NAME		("event")
 
+#define ACPI_EVENT_BUFFER_SIZE	64
+struct acpi_event_reader {
+	struct acpi_bus_event	buffer[ACPI_EVENT_BUFFER_SIZE];
+	int			head, tail;
+	char			str[ACPI_MAX_STRING];
+	int			chars_remaining;
+	char			*ptr;
+	struct list_head	node;
+};
+
 DECLARE_WAIT_QUEUE_HEAD(acpi_event_queue);
-LIST_HEAD(acpi_event_list);
+static LIST_HEAD(acpi_event_reader_list);
 static spinlock_t	acpi_event_lock = SPIN_LOCK_UNLOCKED;
-static int		event_is_open;
+static int		reader_count;
 
 int
 acpi_generate_event(
@@ -27,9 +37,9 @@
 	u8			type,
 	int			data)
 {
-	struct acpi_bus_event	*event;
-	unsigned long		flags;
-	int			result = 0;
+	struct acpi_bus_event		event;
+	struct acpi_event_reader	*reader;
+	unsigned long			flags;
 
 	ACPI_FUNCTION_TRACE("acpi_generate_event");
 
@@ -38,75 +48,66 @@
 
 	spin_lock_irqsave(&acpi_event_lock, flags);
 
-	/* drop event on the floor if no one's listening */
-	if (!event_is_open)
-		goto out;
-
-	event = kmalloc(sizeof(struct acpi_bus_event), GFP_ATOMIC);
-	if (!event) {
-		result = -ENOMEM;
-		goto out;
+	strcpy(event.device_class, device->pnp.device_class);
+	strcpy(event.bus_id, device->pnp.bus_id);
+	event.type = type;
+	event.data = data;
+
+	list_for_each_entry(reader, &acpi_event_reader_list, node) {
+		reader->buffer[reader->head] = event;
+		reader->head = (reader->head + 1) & (ACPI_EVENT_BUFFER_SIZE - 1);
 	}
+	spin_unlock_irqrestore(&acpi_event_lock, flags);
 
-	strcpy(event->device_class, device->pnp.device_class);
-	strcpy(event->bus_id, device->pnp.bus_id);
-	event->type = type;
-	event->data = data;
-
-	list_add_tail(&event->node, &acpi_event_list);
 	wake_up_interruptible(&acpi_event_queue);
 
-out:
-	spin_unlock_irqrestore(&acpi_event_lock, flags);
-	return_VALUE(result);
+	return_VALUE(0);
 }
 
-static struct acpi_bus_event *acpi_fetch_event(void)
+static void acpi_fetch_event(struct acpi_event_reader *reader)
 {
-	struct acpi_bus_event	*event = NULL;
+	struct acpi_bus_event	*event;
 	unsigned long		flags;
 
 	ACPI_FUNCTION_TRACE("acpi_fetch_event");
 
 	spin_lock_irqsave(&acpi_event_lock, flags);
 
-	if (!list_empty(&acpi_event_list)) {
-		event = list_entry(acpi_event_list.next,
-				   struct acpi_bus_event, node);
-		list_del(&event->node);
+	if (reader->head != reader->tail) {
+		event = &reader->buffer[reader->tail];
+		reader->tail = (reader->tail + 1) & (ACPI_EVENT_BUFFER_SIZE - 1);
+
+		reader->chars_remaining =
+			snprintf(reader->str, ACPI_MAX_STRING,
+				"%s %s %08x %08x\n",
+				event->device_class ?
+					event->device_class : "<unknown>",
+				event->bus_id ? event->bus_id : "<unknown>",
+				event->type, event->data);
+
 	}
 
 	spin_unlock_irqrestore(&acpi_event_lock, flags);
 
-	return_PTR(event);
+	return_VOID;
 }
 
 static int
 acpi_system_open_event(struct inode *inode, struct file *file)
 {
-	int rc = 0;
-
-	spin_lock_irq(&acpi_event_lock);
+	struct acpi_event_reader	*reader;
 
-	if (event_is_open) {
-		rc = -EBUSY;
-		goto out;
-	}
+	if (!(reader = kmalloc(sizeof(struct acpi_event_reader), GFP_KERNEL)))
+		return -ENOMEM;
 
-	event_is_open = 1;
+	memset(reader, 0, sizeof(struct acpi_event_reader));
+	file->private_data = reader;
 
-out:
+	spin_lock_irq(&acpi_event_lock);
+	list_add_tail(&reader->node, &acpi_event_reader_list);
+	reader_count++;
 	spin_unlock_irq(&acpi_event_lock);
-	return rc;
-}
-
-static inline int
-event_to_string(struct acpi_bus_event *event, char *buffer)
-{
-	return sprintf(buffer, "%s %s %08x %08x\n",
-		       event->device_class ? event->device_class : "<unknown>",
-		       event->bus_id ? event->bus_id : "<unknown>",
-		       event->type, event->data);
+	return 0;
 }
 
 static ssize_t
@@ -116,42 +117,33 @@
 	size_t			count,
 	loff_t			*ppos)
 {
-	int			result;
-	struct acpi_bus_event	*event;
-	static char		str[ACPI_MAX_STRING];
-	static int		chars_remaining;
-	static char		*ptr;
-
+	struct acpi_event_reader	*reader = file->private_data;
+	int				result;
 
 	ACPI_FUNCTION_TRACE("acpi_system_read_event");
 
-	if (!chars_remaining) {
-		if (list_empty(&acpi_event_list) &&
-		    (file->f_flags & O_NONBLOCK))
+	if (reader->chars_remaining == 0) {
+		if (reader->head == reader->tail && (file->f_flags & O_NONBLOCK))
 			return_VALUE(-EAGAIN);
 
 		result = wait_event_interruptible(acpi_event_queue,
-					!list_empty(&acpi_event_list));
+						  reader->head != reader->tail);
 		if (result)
 			return_VALUE(result);
 
-		if ((event = acpi_fetch_event()) == NULL)
-			return_VALUE(0);
-
-		chars_remaining = event_to_string(event, str);
-		ptr = str;
-		kfree(event);
+		acpi_fetch_event(reader);
+		reader->ptr = reader->str;
 	}
 
-	if (chars_remaining < count)
-		count = chars_remaining;
+	if (reader->chars_remaining < count)
+		count = reader->chars_remaining;
 
-	if (copy_to_user(buffer, ptr, count))
+	if (copy_to_user(buffer, reader->ptr, count))
 		return_VALUE(-EFAULT);
 
 	*ppos += count;
-	chars_remaining -= count;
-	ptr += count;
+	reader->chars_remaining -= count;
+	reader->ptr += count;
 
 	return_VALUE(count);
 }
@@ -159,19 +151,15 @@
 static int
 acpi_system_close_event(struct inode *inode, struct file *file)
 {
-	struct acpi_bus_event *event, *next;
+	struct acpi_event_reader *reader = file->private_data;
 
 	spin_lock_irq(&acpi_event_lock);
+	list_del(&reader->node);
+	reader_count--;
+	spin_unlock_irq(&acpi_event_lock);
 
-	/* We won't be interested in old events, will we? */
-	list_for_each_entry_safe(event, next, &acpi_event_list, node) {
-		list_del(&event->node);
-		kfree(event);
-	}
-
-	event_is_open = 0;
+	kfree(reader);
 
-	spin_unlock_irq(&acpi_event_lock);
 	return 0;
 }
 
@@ -180,8 +168,10 @@
 	struct file		*file,
 	poll_table		*wait)
 {
+	struct acpi_event_reader *reader = file->private_data;
+
 	poll_wait(file, &acpi_event_queue, wait);
-	if (!list_empty(&acpi_event_list))
+	if (reader->head != reader->tail)
 		return POLLIN | POLLRDNORM;
 	return 0;
 }
include/acpi/acpi_event.h +0 -1
diff -Nru a/include/acpi/acpi_event.h b/include/acpi/acpi_event.h
@@ -31,7 +31,6 @@
 #include <acpi/acpi_bus.h>
 
 struct acpi_bus_event {
-	struct list_head	node;
 	acpi_device_class	device_class;
 	acpi_bus_id		bus_id;
 	u32			type;

-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media 100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33 Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

#7 lore raw
In reply to a message not in the archive

Hi,

At Tue, 24 Aug 2004 01:34:15 -0500,

Quoted 9 lines · Dmitry Torokhov

04-acpi-multiple-readers.patch
- allow multiple readrs access /proc/acpi/event, every event is delivered to all readers. Also limit number of pending events to 64 (per reader) so if reader is stuck ACPI does not consume all memory.

Can be user by various daemons, I could see cpufreqd listening for battery insertion/removal events and debugging is much easier too.

'acpid' has soket interface for other daemon/utillities.

from man acpid
In addition to rule files, acpid also accepts connections on a UNIX domain socket (/var/run/acpid.socket by default). Any application may connect to this socket. Once connected, acpid will send the text of all ACPI events to the client. The client has the responsibility of filtering for messages about which it cares. acpid will not close the client socket except in the case of a SIGHUP or acpid exiting.

I think it is enough.

If allow multiple readers, acpid and other tool need some lock facility to avoid unwanted multiple event handling.

Signature · 11 lines
Hiroshi Miura  --- http://www.da-cha.org/  --- [email protected]
NTTDATA Corp. OpenSource Software Center. --- [email protected] 
NTTDATA Intellilink Corp. OpenSource Engineering Dev. -- miurahr-w0OK63jvRlAuJ+9fw/[email protected]
Key fingerprint = 9117 9407 5684 FBF1 4063  15B4 401D D077 04AB 8617


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285

A message that is not in the archive (sent off-list or not yet received): 87zn4ky2zr.wl%[email protected]

#9 lore raw
Quoted 19 lines · Hiroshi Miura

Hi,

At Tue, 24 Aug 2004 01:34:15 -0500,

Quoted 9 lines · Dmitry Torokhov

04-acpi-multiple-readers.patch
- allow multiple readrs access /proc/acpi/event, every event is delivered to all readers. Also limit number of pending events to 64 (per reader) so if reader is stuck ACPI does not consume all memory.

Can be user by various daemons, I could see cpufreqd listening for battery insertion/removal events and debugging is much easier too.

'acpid' has soket interface for other daemon/utillities.

from man acpid
In addition to rule files, acpid also accepts connections on a UNIX domain socket (/var/run/acpid.socket by default). Any application may

[...]

Quoted 1 line

I think it is enough.

but relying on the availability of acpid is not a good approach. And why can acpid hijack /proc/acpi/event?
Allowing multiple readers is much better for every daemon that needs to get acpi events.

just my 0.2 :)

Signature · 9 lines
mattia
:wq!


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285
#10 lore raw
In reply to a message not in the archive
Quoted 24 lines · Hiroshi Miura

Hi,

At Tue, 24 Aug 2004 01:34:15 -0500,

Quoted 9 lines · Dmitry Torokhov

04-acpi-multiple-readers.patch
- allow multiple readrs access /proc/acpi/event, every event is delivered to all readers. Also limit number of pending events to 64 (per reader) so if reader is stuck ACPI does not consume all memory.

Can be user by various daemons, I could see cpufreqd listening for battery insertion/removal events and debugging is much easier too.

'acpid' has soket interface for other daemon/utillities.

from man acpid
In addition to rule files, acpid also accepts connections on a UNIX domain socket (/var/run/acpid.socket by default). Any application may connect to this socket. Once connected, acpid will send the text of all ACPI events to the client. The client has the responsibility of filtering for messages about which it cares. acpid will not close the client socket except in the case of a SIGHUP or acpid exiting.

Quoted 1 line

I think it is enough.

I do not think that present implementation of acpid has to be a prerequisite for all setups. Plus, tunneling events through acpid introduces unwanted (IMHO)
dependencies: acpid has to be started first, restarting it can mess up other processes reading from it's socket, etc, etc...

Given how cheap multiple readers implementation is and the fact that exclusive only access only causes troubles (think psaux in older kernels) I think it should go in.

Quoted 3 lines

If allow multiple readers, acpid and other tool need some lock facility to avoid unwanted multiple event handling.

Well, since acpid manifests that it will pass _all_ events to its readers the same issue exists right now.

Signature · 8 lines
Dmitry


-------------------------------------------------------
SF.Net email is sponsored by Shop4tech.com-Lowest price on Blank Media
100pk Sonic DVD-R 4x for only $29 -100pk Sonic DVD+R for only $33
Save 50% off Retail on Ink & Toner - Free Shipping and Free Gift.
http://www.shop4tech.com/z/Inkjet_Cartridges/9_108_r285
#11 lore raw
In reply to a message not in the archive
Quoted 9 lines · Dmitry Torokhov

Hi,

I have a bunch of patches that I would like you to take a look at:

01-acpi-scan-kobject.patch
- properly create/register kobjects - take reference to the parent since kobject_unregister will try dropping it potentially (if ACPI was ever to release created devices) causing parent be destroyed early) and use kobject_set_name to properly set name.

I'll take a look.

Quoted 4 lines

02-acpi-event-code.patch
- move event (userspace signalling) code from acpi/bus.c to acpi/event.c as too many implementation details were exported, plus some additional fixes to the event handling code.

I'll take a look.

Quoted 8 lines

03-acpi-use-rwsem.patch
- kill brandead usage of acpi_device_lock & list_for_eacj_safe. list_for_each_safe is only safe in the sense that current element can be safely removed by the same thread that is using list_for_each_safe. List still has to be protected from other threads for entire duration of list traversal.
Use acpi_subsys.rwsem instead of acpi_device_lock to protect lists.

sounds like a bug.

Quoted 5 lines

Btw, why don't we just convert ACPI to be standard driver core bus with standard drivers and devices? I could cook up something... The /sys/firmware/acpi hierarchy is not populated with any useful data yet so there should be no concern of breakign userspace interface (I am mot talking about removing /proc/acpi part at the moment).

Funny you should mention it.

My current opinion is that /sys/firmware/acpi should probably not exist at all, and that there should be a single /sys/devices including both ACPI-enumerated and pci-enumerated devices. Adam and I have been ganging up on Patrick on this, but we don't have him convinced yet...

Quoted 9 lines

04-acpi-multiple-readers.patch
- allow multiple readrs access /proc/acpi/event, every event is delivered to all readers. Also limit number of pending events to 64 (per reader) so if reader is stuck ACPI does not consume all memory.

Can be user by various daemons, I could see cpufreqd listening for battery insertion/removal events and debugging is much easier too.

others have been frustrated by this also, so I tend to agree that we should probably address it. Somebody can still contact acpid if they want to... At some point, of course, we're going to have to move this out of /proc...

Quoted 1 line

The patches are against last night pull from Linus repo.

Note that if you did individual file comments, then I can preserve your comments if you export with bjorn's bkexport script (attached).

thanks,
-Len

#!/bin/sh

# Import a bitkeeper patch, preserving author and per-file comments

_import() {
    PATCH=$1

    FROM=`grep '^#### AUTHOR' $PATCH`
    export BK_USER=`echo $FROM | sed -e 's/^#### AUTHOR //' -e 's/@.*//'`
    export BK_HOST=`echo $FROM | sed -e 's/.*@//'`

    echo Importing $PATCH from $BK_USER@$BK_HOST

    #bk import -tpatch -qR $PATCH .
    bk import -tpatch -vR $PATCH .
    if [ $? -ne 0 ]; then
	echo $PATCH: import failed
	exit 0
    fi
    cat $PATCH |\
	awk "/#### COMMENT START/ {p=1; next}; /#### COMMENT END/ {exit}; p==1" |\
	bk comments -
}

for P in $@; do
_import $P
done

#!/bin/sh

# Export a bitkeeper patch, adding author and per-file comments

if [ $# -gt 1 ]; then

	CSET=`bk r2c $*`
	CSET="-r$CSET"
else
	CSET=$1

fi

echo -n "#### AUTHOR "

bk cset $CSET |\
    awk -F\| "{printf \"bk prs -r%s -h -d '\$if(:DPN:=ChangeSet){:USER:@:HOST:\\n}' %s\n\", \$2, \$1}" |\
    sh
echo "#### COMMENT START"
bk cset $CSET |\
    awk -F\| "{printf \"bk prs -r%s -h -d '### Comments for :DPN:\\n\$each(:C:){(:C:)\\n}' %s\n\", \$2, \$1}" |\
    sh

echo "#### COMMENT END"
echo

bk export -tpatch -du $CSET