| Message ID | d9d5acba01eb80580e7696c50db4263ded86a86c.1374566394.git.lv.zheng@intel.com (mailing list archive) |
|---|---|
| State | Deferred, archived |
| Headers | show |
On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > This patch adds reference couting for ACPI operation region handlers to fix > races caused by the ACPICA address space callback invocations. > > ACPICA address space callback invocation is not suitable for Linux > CONFIG_MODULE=y execution environment. This patch tries to protect the > address space callbacks by invoking them under a module safe environment. > The IPMI address space handler is also upgraded in this patch. > The acpi_unregister_region() is designed to meet the following > requirements: > 1. It acts as a barrier for operation region callbacks - no callback will > happen after acpi_unregister_region(). > 2. acpi_unregister_region() is safe to be called in moudle->exit() > functions. > Using reference counting rather than module referencing allows > such benefits to be achieved even when acpi_unregister_region() is called > in the environments other than module->exit(). > The header file of include/acpi/acpi_bus.h should contain the declarations > that have references to some ACPICA defined types. > > Signed-off-by: Lv Zheng <lv.zheng@intel.com> > Reviewed-by: Huang Ying <ying.huang@intel.com> > --- > drivers/acpi/acpi_ipmi.c | 16 ++-- > drivers/acpi/osl.c | 224 ++++++++++++++++++++++++++++++++++++++++++++++ > include/acpi/acpi_bus.h | 5 ++ > 3 files changed, 235 insertions(+), 10 deletions(-) > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c > index 5f8f495..2a09156 100644 > --- a/drivers/acpi/acpi_ipmi.c > +++ b/drivers/acpi/acpi_ipmi.c > @@ -539,20 +539,18 @@ out_ref: > static int __init acpi_ipmi_init(void) > { > int result = 0; > - acpi_status status; > > if (acpi_disabled) > return result; > > mutex_init(&driver_data.ipmi_lock); > > - status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, > - ACPI_ADR_SPACE_IPMI, > - &acpi_ipmi_space_handler, > - NULL, NULL); > - if (ACPI_FAILURE(status)) { > + result = acpi_register_region(ACPI_ADR_SPACE_IPMI, > + &acpi_ipmi_space_handler, > + NULL, NULL); > + if (result) { > pr_warn("Can't register IPMI opregion space handle\n"); > - return -EINVAL; > + return result; > } > > result = ipmi_smi_watcher_register(&driver_data.bmc_events); > @@ -596,9 +594,7 @@ static void __exit acpi_ipmi_exit(void) > } > mutex_unlock(&driver_data.ipmi_lock); > > - acpi_remove_address_space_handler(ACPI_ROOT_OBJECT, > - ACPI_ADR_SPACE_IPMI, > - &acpi_ipmi_space_handler); > + acpi_unregister_region(ACPI_ADR_SPACE_IPMI); > } > > module_init(acpi_ipmi_init); > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c > index 6ab2c35..8398e51 100644 > --- a/drivers/acpi/osl.c > +++ b/drivers/acpi/osl.c > @@ -86,6 +86,42 @@ static struct workqueue_struct *kacpid_wq; > static struct workqueue_struct *kacpi_notify_wq; > static struct workqueue_struct *kacpi_hotplug_wq; > > +struct acpi_region { > + unsigned long flags; > +#define ACPI_REGION_DEFAULT 0x01 > +#define ACPI_REGION_INSTALLED 0x02 > +#define ACPI_REGION_REGISTERED 0x04 > +#define ACPI_REGION_UNREGISTERING 0x08 > +#define ACPI_REGION_INSTALLING 0x10 What about (1UL << 1), (1UL << 2) etc.? Also please remove the #defines out of the struct definition. > + /* > + * NOTE: Upgrading All Region Handlers > + * This flag is only used during the period where not all of the > + * region handers are upgraded to the new interfaces. > + */ > +#define ACPI_REGION_MANAGED 0x80 > + acpi_adr_space_handler handler; > + acpi_adr_space_setup setup; > + void *context; > + /* Invoking references */ > + atomic_t refcnt; Actually, why don't you use krefs? > +}; > + > +static struct acpi_region acpi_regions[ACPI_NUM_PREDEFINED_REGIONS] = { > + [ACPI_ADR_SPACE_SYSTEM_MEMORY] = { > + .flags = ACPI_REGION_DEFAULT, > + }, > + [ACPI_ADR_SPACE_SYSTEM_IO] = { > + .flags = ACPI_REGION_DEFAULT, > + }, > + [ACPI_ADR_SPACE_PCI_CONFIG] = { > + .flags = ACPI_REGION_DEFAULT, > + }, > + [ACPI_ADR_SPACE_IPMI] = { > + .flags = ACPI_REGION_MANAGED, > + }, > +}; > +static DEFINE_MUTEX(acpi_mutex_region); > + > /* > * This list of permanent mappings is for memory that may be accessed from > * interrupt context, where we can't do the ioremap(). > @@ -1799,3 +1835,191 @@ void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context, > kfree(hp_work); > } > EXPORT_SYMBOL_GPL(alloc_acpi_hp_work); > + > +static bool acpi_region_managed(struct acpi_region *rgn) > +{ > + /* > + * NOTE: Default and Managed > + * We only need to avoid region management on the regions managed > + * by ACPICA (ACPI_REGION_DEFAULT). Currently, we need additional > + * check as many operation region handlers are not upgraded, so > + * only those known to be safe are managed (ACPI_REGION_MANAGED). > + */ > + return !(rgn->flags & ACPI_REGION_DEFAULT) && > + (rgn->flags & ACPI_REGION_MANAGED); > +} > + > +static bool acpi_region_callable(struct acpi_region *rgn) > +{ > + return (rgn->flags & ACPI_REGION_REGISTERED) && > + !(rgn->flags & ACPI_REGION_UNREGISTERING); > +} > + > +static acpi_status > +acpi_region_default_handler(u32 function, > + acpi_physical_address address, > + u32 bit_width, u64 *value, > + void *handler_context, void *region_context) > +{ > + acpi_adr_space_handler handler; > + struct acpi_region *rgn = (struct acpi_region *)handler_context; > + void *context; > + acpi_status status = AE_NOT_EXIST; > + > + mutex_lock(&acpi_mutex_region); > + if (!acpi_region_callable(rgn) || !rgn->handler) { > + mutex_unlock(&acpi_mutex_region); > + return status; > + } > + > + atomic_inc(&rgn->refcnt); > + handler = rgn->handler; > + context = rgn->context; > + mutex_unlock(&acpi_mutex_region); > + > + status = handler(function, address, bit_width, value, context, > + region_context); Why don't we call the handler under the mutex? What exactly prevents context from becoming NULL before the call above? > + atomic_dec(&rgn->refcnt); > + > + return status; > +} > + > +static acpi_status > +acpi_region_default_setup(acpi_handle handle, u32 function, > + void *handler_context, void **region_context) > +{ > + acpi_adr_space_setup setup; > + struct acpi_region *rgn = (struct acpi_region *)handler_context; > + void *context; > + acpi_status status = AE_OK; > + > + mutex_lock(&acpi_mutex_region); > + if (!acpi_region_callable(rgn) || !rgn->setup) { > + mutex_unlock(&acpi_mutex_region); > + return status; > + } > + > + atomic_inc(&rgn->refcnt); > + setup = rgn->setup; > + context = rgn->context; > + mutex_unlock(&acpi_mutex_region); > + > + status = setup(handle, function, context, region_context); Can setup drop rgn->refcnt ? > + atomic_dec(&rgn->refcnt); > + > + return status; > +} > + > +static int __acpi_install_region(struct acpi_region *rgn, > + acpi_adr_space_type space_id) > +{ > + int res = 0; > + acpi_status status; > + int installing = 0; > + > + mutex_lock(&acpi_mutex_region); > + if (rgn->flags & ACPI_REGION_INSTALLED) > + goto out_lock; > + if (rgn->flags & ACPI_REGION_INSTALLING) { > + res = -EBUSY; > + goto out_lock; > + } > + > + installing = 1; > + rgn->flags |= ACPI_REGION_INSTALLING; > + status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, space_id, > + acpi_region_default_handler, > + acpi_region_default_setup, > + rgn); > + rgn->flags &= ~ACPI_REGION_INSTALLING; > + if (ACPI_FAILURE(status)) > + res = -EINVAL; > + else > + rgn->flags |= ACPI_REGION_INSTALLED; > + > +out_lock: > + mutex_unlock(&acpi_mutex_region); > + if (installing) { > + if (res) > + pr_err("Failed to install region %d\n", space_id); > + else > + pr_info("Region %d installed\n", space_id); > + } > + return res; > +} > + > +int acpi_register_region(acpi_adr_space_type space_id, > + acpi_adr_space_handler handler, > + acpi_adr_space_setup setup, void *context) > +{ > + int res; > + struct acpi_region *rgn; > + > + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) > + return -EINVAL; > + > + rgn = &acpi_regions[space_id]; > + if (!acpi_region_managed(rgn)) > + return -EINVAL; > + > + res = __acpi_install_region(rgn, space_id); > + if (res) > + return res; > + > + mutex_lock(&acpi_mutex_region); > + if (rgn->flags & ACPI_REGION_REGISTERED) { > + mutex_unlock(&acpi_mutex_region); > + return -EBUSY; > + } > + > + rgn->handler = handler; > + rgn->setup = setup; > + rgn->context = context; > + rgn->flags |= ACPI_REGION_REGISTERED; > + atomic_set(&rgn->refcnt, 1); > + mutex_unlock(&acpi_mutex_region); > + > + pr_info("Region %d registered\n", space_id); > + > + return 0; > +} > +EXPORT_SYMBOL_GPL(acpi_register_region); > + > +void acpi_unregister_region(acpi_adr_space_type space_id) > +{ > + struct acpi_region *rgn; > + > + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) > + return; > + > + rgn = &acpi_regions[space_id]; > + if (!acpi_region_managed(rgn)) > + return; > + > + mutex_lock(&acpi_mutex_region); > + if (!(rgn->flags & ACPI_REGION_REGISTERED)) { > + mutex_unlock(&acpi_mutex_region); > + return; > + } > + if (rgn->flags & ACPI_REGION_UNREGISTERING) { > + mutex_unlock(&acpi_mutex_region); > + return; What about if ((rgn->flags & ACPI_REGION_UNREGISTERING) || !(rgn->flags & ACPI_REGION_REGISTERED)) { mutex_unlock(&acpi_mutex_region); return; } > + } > + > + rgn->flags |= ACPI_REGION_UNREGISTERING; > + rgn->handler = NULL; > + rgn->setup = NULL; > + rgn->context = NULL; > + mutex_unlock(&acpi_mutex_region); > + > + while (atomic_read(&rgn->refcnt) > 1) > + schedule_timeout_uninterruptible(usecs_to_jiffies(5)); Wouldn't it be better to use a wait queue here? > + atomic_dec(&rgn->refcnt); > + > + mutex_lock(&acpi_mutex_region); > + rgn->flags &= ~(ACPI_REGION_REGISTERED | ACPI_REGION_UNREGISTERING); > + mutex_unlock(&acpi_mutex_region); > + > + pr_info("Region %d unregistered\n", space_id); > +} > +EXPORT_SYMBOL_GPL(acpi_unregister_region); > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h > index a2c2fbb..15fad0d 100644 > --- a/include/acpi/acpi_bus.h > +++ b/include/acpi/acpi_bus.h > @@ -542,4 +542,9 @@ static inline int unregister_acpi_bus_type(void *bus) { return 0; } > > #endif /* CONFIG_ACPI */ > > +int acpi_register_region(acpi_adr_space_type space_id, > + acpi_adr_space_handler handler, > + acpi_adr_space_setup setup, void *context); > +void acpi_unregister_region(acpi_adr_space_type space_id); > + > #endif /*__ACPI_BUS_H__*/ Thanks, Rafael
On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > This patch adds reference couting for ACPI operation region handlers to fix > races caused by the ACPICA address space callback invocations. > > ACPICA address space callback invocation is not suitable for Linux > CONFIG_MODULE=y execution environment. Actually, can you please explain to me what *exactly* the problem is? Rafael -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
DQoNCj4gRnJvbTogUmFmYWVsIEouIFd5c29ja2kgW21haWx0bzpyandAc2lzay5wbF0NCj4gU2Vu dDogRnJpZGF5LCBKdWx5IDI2LCAyMDEzIDQ6MjcgQU0NCj4gDQo+IE9uIFR1ZXNkYXksIEp1bHkg MjMsIDIwMTMgMDQ6MDk6NDMgUE0gTHYgWmhlbmcgd3JvdGU6DQo+ID4gVGhpcyBwYXRjaCBhZGRz IHJlZmVyZW5jZSBjb3V0aW5nIGZvciBBQ1BJIG9wZXJhdGlvbiByZWdpb24gaGFuZGxlcnMNCj4g PiB0byBmaXggcmFjZXMgY2F1c2VkIGJ5IHRoZSBBQ1BJQ0EgYWRkcmVzcyBzcGFjZSBjYWxsYmFj ayBpbnZvY2F0aW9ucy4NCj4gPg0KPiA+IEFDUElDQSBhZGRyZXNzIHNwYWNlIGNhbGxiYWNrIGlu dm9jYXRpb24gaXMgbm90IHN1aXRhYmxlIGZvciBMaW51eA0KPiA+IENPTkZJR19NT0RVTEU9eSBl eGVjdXRpb24gZW52aXJvbm1lbnQuICBUaGlzIHBhdGNoIHRyaWVzIHRvIHByb3RlY3QNCj4gPiB0 aGUgYWRkcmVzcyBzcGFjZSBjYWxsYmFja3MgYnkgaW52b2tpbmcgdGhlbSB1bmRlciBhIG1vZHVs ZSBzYWZlDQo+IGVudmlyb25tZW50Lg0KPiA+IFRoZSBJUE1JIGFkZHJlc3Mgc3BhY2UgaGFuZGxl ciBpcyBhbHNvIHVwZ3JhZGVkIGluIHRoaXMgcGF0Y2guDQo+ID4gVGhlIGFjcGlfdW5yZWdpc3Rl cl9yZWdpb24oKSBpcyBkZXNpZ25lZCB0byBtZWV0IHRoZSBmb2xsb3dpbmcNCj4gPiByZXF1aXJl bWVudHM6DQo+ID4gMS4gSXQgYWN0cyBhcyBhIGJhcnJpZXIgZm9yIG9wZXJhdGlvbiByZWdpb24g Y2FsbGJhY2tzIC0gbm8gY2FsbGJhY2sgd2lsbA0KPiA+ICAgIGhhcHBlbiBhZnRlciBhY3BpX3Vu cmVnaXN0ZXJfcmVnaW9uKCkuDQo+ID4gMi4gYWNwaV91bnJlZ2lzdGVyX3JlZ2lvbigpIGlzIHNh ZmUgdG8gYmUgY2FsbGVkIGluIG1vdWRsZS0+ZXhpdCgpDQo+ID4gICAgZnVuY3Rpb25zLg0KPiA+ IFVzaW5nIHJlZmVyZW5jZSBjb3VudGluZyByYXRoZXIgdGhhbiBtb2R1bGUgcmVmZXJlbmNpbmcg YWxsb3dzIHN1Y2gNCj4gPiBiZW5lZml0cyB0byBiZSBhY2hpZXZlZCBldmVuIHdoZW4gYWNwaV91 bnJlZ2lzdGVyX3JlZ2lvbigpIGlzIGNhbGxlZA0KPiA+IGluIHRoZSBlbnZpcm9ubWVudHMgb3Ro ZXIgdGhhbiBtb2R1bGUtPmV4aXQoKS4NCj4gPiBUaGUgaGVhZGVyIGZpbGUgb2YgaW5jbHVkZS9h Y3BpL2FjcGlfYnVzLmggc2hvdWxkIGNvbnRhaW4gdGhlDQo+ID4gZGVjbGFyYXRpb25zIHRoYXQg aGF2ZSByZWZlcmVuY2VzIHRvIHNvbWUgQUNQSUNBIGRlZmluZWQgdHlwZXMuDQo+ID4NCj4gPiBT aWduZWQtb2ZmLWJ5OiBMdiBaaGVuZyA8bHYuemhlbmdAaW50ZWwuY29tPg0KPiA+IFJldmlld2Vk LWJ5OiBIdWFuZyBZaW5nIDx5aW5nLmh1YW5nQGludGVsLmNvbT4NCj4gPiAtLS0NCj4gPiAgZHJp dmVycy9hY3BpL2FjcGlfaXBtaS5jIHwgICAxNiArKy0tDQo+ID4gIGRyaXZlcnMvYWNwaS9vc2wu YyAgICAgICB8ICAyMjQNCj4gKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysr KysrKysrKw0KPiA+ICBpbmNsdWRlL2FjcGkvYWNwaV9idXMuaCAgfCAgICA1ICsrDQo+ID4gIDMg ZmlsZXMgY2hhbmdlZCwgMjM1IGluc2VydGlvbnMoKyksIDEwIGRlbGV0aW9ucygtKQ0KPiA+DQo+ ID4gZGlmZiAtLWdpdCBhL2RyaXZlcnMvYWNwaS9hY3BpX2lwbWkuYyBiL2RyaXZlcnMvYWNwaS9h Y3BpX2lwbWkuYyBpbmRleA0KPiA+IDVmOGY0OTUuLjJhMDkxNTYgMTAwNjQ0DQo+ID4gLS0tIGEv ZHJpdmVycy9hY3BpL2FjcGlfaXBtaS5jDQo+ID4gKysrIGIvZHJpdmVycy9hY3BpL2FjcGlfaXBt aS5jDQo+ID4gQEAgLTUzOSwyMCArNTM5LDE4IEBAIG91dF9yZWY6DQo+ID4gIHN0YXRpYyBpbnQg X19pbml0IGFjcGlfaXBtaV9pbml0KHZvaWQpICB7DQo+ID4gIAlpbnQgcmVzdWx0ID0gMDsNCj4g PiAtCWFjcGlfc3RhdHVzIHN0YXR1czsNCj4gPg0KPiA+ICAJaWYgKGFjcGlfZGlzYWJsZWQpDQo+ ID4gIAkJcmV0dXJuIHJlc3VsdDsNCj4gPg0KPiA+ICAJbXV0ZXhfaW5pdCgmZHJpdmVyX2RhdGEu aXBtaV9sb2NrKTsNCj4gPg0KPiA+IC0Jc3RhdHVzID0gYWNwaV9pbnN0YWxsX2FkZHJlc3Nfc3Bh Y2VfaGFuZGxlcihBQ1BJX1JPT1RfT0JKRUNULA0KPiA+IC0JCQkJCQkgICAgQUNQSV9BRFJfU1BB Q0VfSVBNSSwNCj4gPiAtCQkJCQkJICAgICZhY3BpX2lwbWlfc3BhY2VfaGFuZGxlciwNCj4gPiAt CQkJCQkJICAgIE5VTEwsIE5VTEwpOw0KPiA+IC0JaWYgKEFDUElfRkFJTFVSRShzdGF0dXMpKSB7 DQo+ID4gKwlyZXN1bHQgPSBhY3BpX3JlZ2lzdGVyX3JlZ2lvbihBQ1BJX0FEUl9TUEFDRV9JUE1J LA0KPiA+ICsJCQkJICAgICAgJmFjcGlfaXBtaV9zcGFjZV9oYW5kbGVyLA0KPiA+ICsJCQkJICAg ICAgTlVMTCwgTlVMTCk7DQo+ID4gKwlpZiAocmVzdWx0KSB7DQo+ID4gIAkJcHJfd2FybigiQ2Fu J3QgcmVnaXN0ZXIgSVBNSSBvcHJlZ2lvbiBzcGFjZSBoYW5kbGVcbiIpOw0KPiA+IC0JCXJldHVy biAtRUlOVkFMOw0KPiA+ICsJCXJldHVybiByZXN1bHQ7DQo+ID4gIAl9DQo+ID4NCj4gPiAgCXJl c3VsdCA9IGlwbWlfc21pX3dhdGNoZXJfcmVnaXN0ZXIoJmRyaXZlcl9kYXRhLmJtY19ldmVudHMp Ow0KPiA+IEBAIC01OTYsOSArNTk0LDcgQEAgc3RhdGljIHZvaWQgX19leGl0IGFjcGlfaXBtaV9l eGl0KHZvaWQpDQo+ID4gIAl9DQo+ID4gIAltdXRleF91bmxvY2soJmRyaXZlcl9kYXRhLmlwbWlf bG9jayk7DQo+ID4NCj4gPiAtCWFjcGlfcmVtb3ZlX2FkZHJlc3Nfc3BhY2VfaGFuZGxlcihBQ1BJ X1JPT1RfT0JKRUNULA0KPiA+IC0JCQkJCSAgQUNQSV9BRFJfU1BBQ0VfSVBNSSwNCj4gPiAtCQkJ CQkgICZhY3BpX2lwbWlfc3BhY2VfaGFuZGxlcik7DQo+ID4gKwlhY3BpX3VucmVnaXN0ZXJfcmVn aW9uKEFDUElfQURSX1NQQUNFX0lQTUkpOw0KPiA+ICB9DQo+ID4NCj4gPiAgbW9kdWxlX2luaXQo YWNwaV9pcG1pX2luaXQpOw0KPiA+IGRpZmYgLS1naXQgYS9kcml2ZXJzL2FjcGkvb3NsLmMgYi9k cml2ZXJzL2FjcGkvb3NsLmMgaW5kZXgNCj4gPiA2YWIyYzM1Li44Mzk4ZTUxIDEwMDY0NA0KPiA+ IC0tLSBhL2RyaXZlcnMvYWNwaS9vc2wuYw0KPiA+ICsrKyBiL2RyaXZlcnMvYWNwaS9vc2wuYw0K PiA+IEBAIC04Niw2ICs4Niw0MiBAQCBzdGF0aWMgc3RydWN0IHdvcmtxdWV1ZV9zdHJ1Y3QgKmth Y3BpZF93cTsgIHN0YXRpYw0KPiA+IHN0cnVjdCB3b3JrcXVldWVfc3RydWN0ICprYWNwaV9ub3Rp Znlfd3E7ICBzdGF0aWMgc3RydWN0DQo+ID4gd29ya3F1ZXVlX3N0cnVjdCAqa2FjcGlfaG90cGx1 Z193cTsNCj4gPg0KPiA+ICtzdHJ1Y3QgYWNwaV9yZWdpb24gew0KPiA+ICsJdW5zaWduZWQgbG9u ZyBmbGFnczsNCj4gPiArI2RlZmluZSBBQ1BJX1JFR0lPTl9ERUZBVUxUCQkweDAxDQo+ID4gKyNk ZWZpbmUgQUNQSV9SRUdJT05fSU5TVEFMTEVECQkweDAyDQo+ID4gKyNkZWZpbmUgQUNQSV9SRUdJ T05fUkVHSVNURVJFRAkJMHgwNA0KPiA+ICsjZGVmaW5lIEFDUElfUkVHSU9OX1VOUkVHSVNURVJJ TkcJMHgwOA0KPiA+ICsjZGVmaW5lIEFDUElfUkVHSU9OX0lOU1RBTExJTkcJCTB4MTANCj4gDQo+ IFdoYXQgYWJvdXQgKDFVTCA8PCAxKSwgKDFVTCA8PCAyKSBldGMuPw0KPiANCj4gQWxzbyBwbGVh c2UgcmVtb3ZlIHRoZSAjZGVmaW5lcyBvdXQgb2YgdGhlIHN0cnVjdCBkZWZpbml0aW9uLg0KDQpP Sy4NCg0KPiANCj4gPiArCS8qDQo+ID4gKwkgKiBOT1RFOiBVcGdyYWRpbmcgQWxsIFJlZ2lvbiBI YW5kbGVycw0KPiA+ICsJICogVGhpcyBmbGFnIGlzIG9ubHkgdXNlZCBkdXJpbmcgdGhlIHBlcmlv ZCB3aGVyZSBub3QgYWxsIG9mIHRoZQ0KPiA+ICsJICogcmVnaW9uIGhhbmRlcnMgYXJlIHVwZ3Jh ZGVkIHRvIHRoZSBuZXcgaW50ZXJmYWNlcy4NCj4gPiArCSAqLw0KPiA+ICsjZGVmaW5lIEFDUElf UkVHSU9OX01BTkFHRUQJCTB4ODANCj4gPiArCWFjcGlfYWRyX3NwYWNlX2hhbmRsZXIgaGFuZGxl cjsNCj4gPiArCWFjcGlfYWRyX3NwYWNlX3NldHVwIHNldHVwOw0KPiA+ICsJdm9pZCAqY29udGV4 dDsNCj4gPiArCS8qIEludm9raW5nIHJlZmVyZW5jZXMgKi8NCj4gPiArCWF0b21pY190IHJlZmNu dDsNCj4gDQo+IEFjdHVhbGx5LCB3aHkgZG9uJ3QgeW91IHVzZSBrcmVmcz8NCg0KSWYgeW91IHRh a2UgYSBsb29rIGF0IG90aGVyIHBpZWNlIG9mIG15IGNvZGVzLCB5b3UnbGwgZmluZCB0aGVyZSBh cmUgdHdvIHJlYXNvbnM6DQoNCjEuIEknbSB1c2luZyB3aGlsZSAoYXRvbWljX3JlYWQoKSA+IDEp IHRvIGltcGxlbWVudCB0aGUgb2JqZWN0cycgZmx1c2hpbmcgYW5kIHRoZXJlIGlzIG5vIGtyZWYg QVBJIHRvIGRvIHNvLg0KICBJIGp1c3QgdGhpbmsgaXQgaXMgbm90IHN1aXRhYmxlIGZvciBtZSB0 byBpbnRyb2R1Y2Ugc3VjaCBhbiBBUEkgaW50byBrcmVmLmggYW5kIHN0YXJ0IGFub3RoZXIgYXJn dW1lbnQgYXJvdW5kIGtyZWYgZGVzaWducyBpbiB0aGlzIGJ1ZyBmaXggcGF0Y2guIDotKQ0KICBJ J2xsIHN0YXJ0IGEgZGlzY3Vzc2lvbiBhYm91dCBrcmVmIGRlc2lnbiB1c2luZyBhbm90aGVyIHRo cmVhZC4NCjIuIEknbSB1c2luZyBpcG1pX2Rldnxtc2dfcmVsZWFzZSgpIGFzIGEgcGFpciBvZiBp cG1pX2Rldnxtc2dfYWxsb2MoKSwgaXQncyBraW5kIG9mIGF0b21pY190IGNvZGluZyBzdHlsZS4N CiAgSWYgYXRvbWljX3QgaXMgY2hhbmdlZCB0byBzdHJ1Y3Qga3JlZiwgSSB3aWxsIG5lZWQgdG8g aW1wbGVtZW50IHR3byBBUEksIF9faXBtaV9kZXZfcmVsZWFzZSgpIHRvIHRha2UgYSBzdHJ1Y3Qg a3JlZiBhcyBwYXJhbWV0ZXIgYW5kIGNhbGwgaXBtaV9kZXZfcmVsZWFzZSBpbnNpZGUgaXQuDQog IEJ5IG5vdCB1c2luZyBrcmVmLCBJIG5lZWRuJ3Qgd3JpdGUgY29kZXMgdG8gaW1wbGVtZW50IHN1 Y2ggQVBJLg0KDQo+IA0KPiA+ICt9Ow0KPiA+ICsNCj4gPiArc3RhdGljIHN0cnVjdCBhY3BpX3Jl Z2lvbiBhY3BpX3JlZ2lvbnNbQUNQSV9OVU1fUFJFREVGSU5FRF9SRUdJT05TXQ0KPiA9IHsNCj4g PiArCVtBQ1BJX0FEUl9TUEFDRV9TWVNURU1fTUVNT1JZXSA9IHsNCj4gPiArCQkuZmxhZ3MgPSBB Q1BJX1JFR0lPTl9ERUZBVUxULA0KPiA+ICsJfSwNCj4gPiArCVtBQ1BJX0FEUl9TUEFDRV9TWVNU RU1fSU9dID0gew0KPiA+ICsJCS5mbGFncyA9IEFDUElfUkVHSU9OX0RFRkFVTFQsDQo+ID4gKwl9 LA0KPiA+ICsJW0FDUElfQURSX1NQQUNFX1BDSV9DT05GSUddID0gew0KPiA+ICsJCS5mbGFncyA9 IEFDUElfUkVHSU9OX0RFRkFVTFQsDQo+ID4gKwl9LA0KPiA+ICsJW0FDUElfQURSX1NQQUNFX0lQ TUldID0gew0KPiA+ICsJCS5mbGFncyA9IEFDUElfUkVHSU9OX01BTkFHRUQsDQo+ID4gKwl9LA0K PiA+ICt9Ow0KPiA+ICtzdGF0aWMgREVGSU5FX01VVEVYKGFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4g PiArDQo+ID4gIC8qDQo+ID4gICAqIFRoaXMgbGlzdCBvZiBwZXJtYW5lbnQgbWFwcGluZ3MgaXMg Zm9yIG1lbW9yeSB0aGF0IG1heSBiZSBhY2Nlc3NlZA0KPiBmcm9tDQo+ID4gICAqIGludGVycnVw dCBjb250ZXh0LCB3aGVyZSB3ZSBjYW4ndCBkbyB0aGUgaW9yZW1hcCgpLg0KPiA+IEBAIC0xNzk5 LDMgKzE4MzUsMTkxIEBAIHZvaWQgYWxsb2NfYWNwaV9ocF93b3JrKGFjcGlfaGFuZGxlIGhhbmRs ZSwNCj4gdTMyIHR5cGUsIHZvaWQgKmNvbnRleHQsDQo+ID4gIAkJa2ZyZWUoaHBfd29yayk7DQo+ ID4gIH0NCj4gPiAgRVhQT1JUX1NZTUJPTF9HUEwoYWxsb2NfYWNwaV9ocF93b3JrKTsNCj4gPiAr DQo+ID4gK3N0YXRpYyBib29sIGFjcGlfcmVnaW9uX21hbmFnZWQoc3RydWN0IGFjcGlfcmVnaW9u ICpyZ24pIHsNCj4gPiArCS8qDQo+ID4gKwkgKiBOT1RFOiBEZWZhdWx0IGFuZCBNYW5hZ2VkDQo+ ID4gKwkgKiBXZSBvbmx5IG5lZWQgdG8gYXZvaWQgcmVnaW9uIG1hbmFnZW1lbnQgb24gdGhlIHJl Z2lvbnMgbWFuYWdlZA0KPiA+ICsJICogYnkgQUNQSUNBIChBQ1BJX1JFR0lPTl9ERUZBVUxUKS4g IEN1cnJlbnRseSwgd2UgbmVlZCBhZGRpdGlvbmFsDQo+ID4gKwkgKiBjaGVjayBhcyBtYW55IG9w ZXJhdGlvbiByZWdpb24gaGFuZGxlcnMgYXJlIG5vdCB1cGdyYWRlZCwgc28NCj4gPiArCSAqIG9u bHkgdGhvc2Uga25vd24gdG8gYmUgc2FmZSBhcmUgbWFuYWdlZCAoQUNQSV9SRUdJT05fTUFOQUdF RCkuDQo+ID4gKwkgKi8NCj4gPiArCXJldHVybiAhKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9E RUZBVUxUKSAmJg0KPiA+ICsJICAgICAgIChyZ24tPmZsYWdzICYgQUNQSV9SRUdJT05fTUFOQUdF RCk7IH0NCj4gPiArDQo+ID4gK3N0YXRpYyBib29sIGFjcGlfcmVnaW9uX2NhbGxhYmxlKHN0cnVj dCBhY3BpX3JlZ2lvbiAqcmduKSB7DQo+ID4gKwlyZXR1cm4gKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JF R0lPTl9SRUdJU1RFUkVEKSAmJg0KPiA+ICsJICAgICAgICEocmduLT5mbGFncyAmIEFDUElfUkVH SU9OX1VOUkVHSVNURVJJTkcpOyB9DQo+ID4gKw0KPiA+ICtzdGF0aWMgYWNwaV9zdGF0dXMNCj4g PiArYWNwaV9yZWdpb25fZGVmYXVsdF9oYW5kbGVyKHUzMiBmdW5jdGlvbiwNCj4gPiArCQkJICAg IGFjcGlfcGh5c2ljYWxfYWRkcmVzcyBhZGRyZXNzLA0KPiA+ICsJCQkgICAgdTMyIGJpdF93aWR0 aCwgdTY0ICp2YWx1ZSwNCj4gPiArCQkJICAgIHZvaWQgKmhhbmRsZXJfY29udGV4dCwgdm9pZCAq cmVnaW9uX2NvbnRleHQpIHsNCj4gPiArCWFjcGlfYWRyX3NwYWNlX2hhbmRsZXIgaGFuZGxlcjsN Cj4gPiArCXN0cnVjdCBhY3BpX3JlZ2lvbiAqcmduID0gKHN0cnVjdCBhY3BpX3JlZ2lvbiAqKWhh bmRsZXJfY29udGV4dDsNCj4gPiArCXZvaWQgKmNvbnRleHQ7DQo+ID4gKwlhY3BpX3N0YXR1cyBz dGF0dXMgPSBBRV9OT1RfRVhJU1Q7DQo+ID4gKw0KPiA+ICsJbXV0ZXhfbG9jaygmYWNwaV9tdXRl eF9yZWdpb24pOw0KPiA+ICsJaWYgKCFhY3BpX3JlZ2lvbl9jYWxsYWJsZShyZ24pIHx8ICFyZ24t PmhhbmRsZXIpIHsNCj4gPiArCQltdXRleF91bmxvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4g PiArCQlyZXR1cm4gc3RhdHVzOw0KPiA+ICsJfQ0KPiA+ICsNCj4gPiArCWF0b21pY19pbmMoJnJn bi0+cmVmY250KTsNCj4gPiArCWhhbmRsZXIgPSByZ24tPmhhbmRsZXI7DQo+ID4gKwljb250ZXh0 ID0gcmduLT5jb250ZXh0Ow0KPiA+ICsJbXV0ZXhfdW5sb2NrKCZhY3BpX211dGV4X3JlZ2lvbik7 DQo+ID4gKw0KPiA+ICsJc3RhdHVzID0gaGFuZGxlcihmdW5jdGlvbiwgYWRkcmVzcywgYml0X3dp ZHRoLCB2YWx1ZSwgY29udGV4dCwNCj4gPiArCQkJIHJlZ2lvbl9jb250ZXh0KTsNCj4gDQo+IFdo eSBkb24ndCB3ZSBjYWxsIHRoZSBoYW5kbGVyIHVuZGVyIHRoZSBtdXRleD8NCj4gDQo+IFdoYXQg ZXhhY3RseSBwcmV2ZW50cyBjb250ZXh0IGZyb20gYmVjb21pbmcgTlVMTCBiZWZvcmUgdGhlIGNh bGwgYWJvdmU/DQoNCkl0J3MgYSBraW5kIG9mIHByb2dyYW1taW5nIHN0eWxlIHJlbGF0ZWQgY29u Y2Vybi4NCklNTywgdXNpbmcgbG9ja3MgYXJvdW5kIGNhbGxiYWNrIGZ1bmN0aW9uIGlzIGEgYnVn Z3kgcHJvZ3JhbW1pbmcgc3R5bGUgdGhhdCBjb3VsZCBsZWFkIHRvIGRlYWQgbG9ja3MuDQpMZXQg bWUgZXhwbGFpbiB0aGlzIHVzaW5nIGFuIGV4YW1wbGUuDQoNCk9iamVjdCBBIGV4cG9ydHMgYSBy ZWdpc3Rlci91bnJlZ2lzdGVyIEFQSSBmb3Igb3RoZXIgb2JqZWN0cy4NCk9iamVjdCBCIGNhbGxz IEEncyByZWdpc3Rlci91bnJlZ2lzdGVyIEFQSSB0byByZWdpc3Rlci91bnJlZ2lzdGVyIEIncyBj YWxsYmFjay4NCkl0J3MgbGlrZWx5IHRoYXQgb2JqZWN0IEIgd2lsbCBob2xkIGxvY2tfb2ZfQiBh cm91bmQgdW5yZWdpc3Rlci9yZWdpc3RlciB3aGVuIG9iamVjdCBCIGlzIGRlc3Ryb3llZC9jcmVh dGVkLCB0aGUgbG9ja19vZl9CIGlzIGxpa2VseSBhbHNvIHVzZWQgaW5zaWRlIHRoZSBjYWxsYmFj ay4NClNvIHdoZW4gb2JqZWN0IEEgaG9sZHMgdGhlIGxvY2tfb2ZfQSBhcm91bmQgdGhlIGNhbGxi YWNrIGludm9jYXRpb24sIGl0IGxlYWRzIHRvIGRlYWQgbG9jayBzaW5jZToNCjEuIHRoZSBsb2Nr aW5nIG9yZGVyIGZvciB0aGUgcmVnaXN0ZXIvdW5yZWdpc3RlciBzaWRlIHdpbGwgYmU6IGxvY2so bG9ja19vZl9CKSwgbG9jayhsb2NrX29mX0EpDQoyLiB0aGUgbG9ja2luZyBvcmRlciBmb3IgdGhl IGNhbGxiYWNrIHNpZGUgd2lsbCBiZTogbG9jayhsb2NrX29mX0EpLCBsb2NrKGxvY2tfb2ZfQikN ClRoZXkgYXJlIGluIHRoZSByZXZlcnNlZCBvcmRlciENCg0KSU1PLCBMaW51eCBtYXkgbmVlZCB0 byBpbnRyb2R1Y2UgX19jYWxsYmFjaywgX19hcGkgYXMgZGVjZWxlcmF0b3JzIGZvciB0aGUgZnVu Y3Rpb25zLCBhbmQgdXNlIHNwYXJzZSB0byBlbmZvcmNlIHRoaXMgcnVsZSwgc3BhcnNlIGtub3dz IGlmIGEgY2FsbGJhY2sgaXMgaW52b2tlZCB1bmRlciBzb21lIGxvY2tzLg0KDQpJbiB0aGUgY2Fz ZSBvZiBBQ1BJQ0Egc3BhY2VfaGFuZGxlcnMsIGFzIHlvdSBtYXkga25vdywgd2hlbiBhbiBBQ1BJ IG9wZXJhdGlvbiByZWdpb24gaGFuZGxlciBpcyBpbnZva2VkLCB0aGVyZSB3aWxsIGJlIG5vIGxv Y2sgaGVsZCBpbnNpZGUgQUNQSUNBIChpbnRlcnByZXRlciBsb2NrIG11c3QgYmUgZnJlZWQgYmVm b3JlIGV4ZWN1dGluZyBvcGVyYXRpb24gcmVnaW9uIGhhbmRsZXJzKS4NClNvIHRoZSBsaWtlbGlo b29kIG9mIHRoZSBkZWFkIGxvY2sgaXMgcHJldHR5IG11Y2ggaGlnaCBoZXJlIQ0KDQo+IA0KPiA+ ICsJYXRvbWljX2RlYygmcmduLT5yZWZjbnQpOw0KPiA+ICsNCj4gPiArCXJldHVybiBzdGF0dXM7 DQo+ID4gK30NCj4gPiArDQo+ID4gK3N0YXRpYyBhY3BpX3N0YXR1cw0KPiA+ICthY3BpX3JlZ2lv bl9kZWZhdWx0X3NldHVwKGFjcGlfaGFuZGxlIGhhbmRsZSwgdTMyIGZ1bmN0aW9uLA0KPiA+ICsJ CQkgIHZvaWQgKmhhbmRsZXJfY29udGV4dCwgdm9pZCAqKnJlZ2lvbl9jb250ZXh0KSB7DQo+ID4g KwlhY3BpX2Fkcl9zcGFjZV9zZXR1cCBzZXR1cDsNCj4gPiArCXN0cnVjdCBhY3BpX3JlZ2lvbiAq cmduID0gKHN0cnVjdCBhY3BpX3JlZ2lvbiAqKWhhbmRsZXJfY29udGV4dDsNCj4gPiArCXZvaWQg KmNvbnRleHQ7DQo+ID4gKwlhY3BpX3N0YXR1cyBzdGF0dXMgPSBBRV9PSzsNCj4gPiArDQo+ID4g KwltdXRleF9sb2NrKCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4gKwlpZiAoIWFjcGlfcmVnaW9u X2NhbGxhYmxlKHJnbikgfHwgIXJnbi0+c2V0dXApIHsNCj4gPiArCQltdXRleF91bmxvY2soJmFj cGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArCQlyZXR1cm4gc3RhdHVzOw0KPiA+ICsJfQ0KPiA+ICsN Cj4gPiArCWF0b21pY19pbmMoJnJnbi0+cmVmY250KTsNCj4gPiArCXNldHVwID0gcmduLT5zZXR1 cDsNCj4gPiArCWNvbnRleHQgPSByZ24tPmNvbnRleHQ7DQo+ID4gKwltdXRleF91bmxvY2soJmFj cGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArDQo+ID4gKwlzdGF0dXMgPSBzZXR1cChoYW5kbGUsIGZ1 bmN0aW9uLCBjb250ZXh0LCByZWdpb25fY29udGV4dCk7DQo+IA0KPiBDYW4gc2V0dXAgZHJvcCBy Z24tPnJlZmNudCA/DQoNClRoZSByZWFzb24gaXMgc2FtZSBhcyB0aGUgaGFuZGxlciwgYXMgYSBz ZXR1cCBpcyBhbHNvIGEgY2FsbGJhY2suDQoNCj4gDQo+ID4gKwlhdG9taWNfZGVjKCZyZ24tPnJl ZmNudCk7DQo+ID4gKw0KPiA+ICsJcmV0dXJuIHN0YXR1czsNCj4gPiArfQ0KPiA+ICsNCj4gPiAr c3RhdGljIGludCBfX2FjcGlfaW5zdGFsbF9yZWdpb24oc3RydWN0IGFjcGlfcmVnaW9uICpyZ24s DQo+ID4gKwkJCQkgYWNwaV9hZHJfc3BhY2VfdHlwZSBzcGFjZV9pZCkNCj4gPiArew0KPiA+ICsJ aW50IHJlcyA9IDA7DQo+ID4gKwlhY3BpX3N0YXR1cyBzdGF0dXM7DQo+ID4gKwlpbnQgaW5zdGFs bGluZyA9IDA7DQo+ID4gKw0KPiA+ICsJbXV0ZXhfbG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0K PiA+ICsJaWYgKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9JTlNUQUxMRUQpDQo+ID4gKwkJZ290 byBvdXRfbG9jazsNCj4gPiArCWlmIChyZ24tPmZsYWdzICYgQUNQSV9SRUdJT05fSU5TVEFMTElO Rykgew0KPiA+ICsJCXJlcyA9IC1FQlVTWTsNCj4gPiArCQlnb3RvIG91dF9sb2NrOw0KPiA+ICsJ fQ0KPiA+ICsNCj4gPiArCWluc3RhbGxpbmcgPSAxOw0KPiA+ICsJcmduLT5mbGFncyB8PSBBQ1BJ X1JFR0lPTl9JTlNUQUxMSU5HOw0KPiA+ICsJc3RhdHVzID0gYWNwaV9pbnN0YWxsX2FkZHJlc3Nf c3BhY2VfaGFuZGxlcihBQ1BJX1JPT1RfT0JKRUNULA0KPiBzcGFjZV9pZCwNCj4gPiArCQkJCQkJ ICAgIGFjcGlfcmVnaW9uX2RlZmF1bHRfaGFuZGxlciwNCj4gPiArCQkJCQkJICAgIGFjcGlfcmVn aW9uX2RlZmF1bHRfc2V0dXAsDQo+ID4gKwkJCQkJCSAgICByZ24pOw0KPiA+ICsJcmduLT5mbGFn cyAmPSB+QUNQSV9SRUdJT05fSU5TVEFMTElORzsNCj4gPiArCWlmIChBQ1BJX0ZBSUxVUkUoc3Rh dHVzKSkNCj4gPiArCQlyZXMgPSAtRUlOVkFMOw0KPiA+ICsJZWxzZQ0KPiA+ICsJCXJnbi0+Zmxh Z3MgfD0gQUNQSV9SRUdJT05fSU5TVEFMTEVEOw0KPiA+ICsNCj4gPiArb3V0X2xvY2s6DQo+ID4g KwltdXRleF91bmxvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArCWlmIChpbnN0YWxsaW5n KSB7DQo+ID4gKwkJaWYgKHJlcykNCj4gPiArCQkJcHJfZXJyKCJGYWlsZWQgdG8gaW5zdGFsbCBy ZWdpb24gJWRcbiIsIHNwYWNlX2lkKTsNCj4gPiArCQllbHNlDQo+ID4gKwkJCXByX2luZm8oIlJl Z2lvbiAlZCBpbnN0YWxsZWRcbiIsIHNwYWNlX2lkKTsNCj4gPiArCX0NCj4gPiArCXJldHVybiBy ZXM7DQo+ID4gK30NCj4gPiArDQo+ID4gK2ludCBhY3BpX3JlZ2lzdGVyX3JlZ2lvbihhY3BpX2Fk cl9zcGFjZV90eXBlIHNwYWNlX2lkLA0KPiA+ICsJCQkgYWNwaV9hZHJfc3BhY2VfaGFuZGxlciBo YW5kbGVyLA0KPiA+ICsJCQkgYWNwaV9hZHJfc3BhY2Vfc2V0dXAgc2V0dXAsIHZvaWQgKmNvbnRl eHQpIHsNCj4gPiArCWludCByZXM7DQo+ID4gKwlzdHJ1Y3QgYWNwaV9yZWdpb24gKnJnbjsNCj4g PiArDQo+ID4gKwlpZiAoc3BhY2VfaWQgPj0gQUNQSV9OVU1fUFJFREVGSU5FRF9SRUdJT05TKQ0K PiA+ICsJCXJldHVybiAtRUlOVkFMOw0KPiA+ICsNCj4gPiArCXJnbiA9ICZhY3BpX3JlZ2lvbnNb c3BhY2VfaWRdOw0KPiA+ICsJaWYgKCFhY3BpX3JlZ2lvbl9tYW5hZ2VkKHJnbikpDQo+ID4gKwkJ cmV0dXJuIC1FSU5WQUw7DQo+ID4gKw0KPiA+ICsJcmVzID0gX19hY3BpX2luc3RhbGxfcmVnaW9u KHJnbiwgc3BhY2VfaWQpOw0KPiA+ICsJaWYgKHJlcykNCj4gPiArCQlyZXR1cm4gcmVzOw0KPiA+ ICsNCj4gPiArCW11dGV4X2xvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArCWlmIChyZ24t PmZsYWdzICYgQUNQSV9SRUdJT05fUkVHSVNURVJFRCkgew0KPiA+ICsJCW11dGV4X3VubG9jaygm YWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ICsJCXJldHVybiAtRUJVU1k7DQo+ID4gKwl9DQo+ID4g Kw0KPiA+ICsJcmduLT5oYW5kbGVyID0gaGFuZGxlcjsNCj4gPiArCXJnbi0+c2V0dXAgPSBzZXR1 cDsNCj4gPiArCXJnbi0+Y29udGV4dCA9IGNvbnRleHQ7DQo+ID4gKwlyZ24tPmZsYWdzIHw9IEFD UElfUkVHSU9OX1JFR0lTVEVSRUQ7DQo+ID4gKwlhdG9taWNfc2V0KCZyZ24tPnJlZmNudCwgMSk7 DQo+ID4gKwltdXRleF91bmxvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArDQo+ID4gKwlw cl9pbmZvKCJSZWdpb24gJWQgcmVnaXN0ZXJlZFxuIiwgc3BhY2VfaWQpOw0KPiA+ICsNCj4gPiAr CXJldHVybiAwOw0KPiA+ICt9DQo+ID4gK0VYUE9SVF9TWU1CT0xfR1BMKGFjcGlfcmVnaXN0ZXJf cmVnaW9uKTsNCj4gPiArDQo+ID4gK3ZvaWQgYWNwaV91bnJlZ2lzdGVyX3JlZ2lvbihhY3BpX2Fk cl9zcGFjZV90eXBlIHNwYWNlX2lkKSB7DQo+ID4gKwlzdHJ1Y3QgYWNwaV9yZWdpb24gKnJnbjsN Cj4gPiArDQo+ID4gKwlpZiAoc3BhY2VfaWQgPj0gQUNQSV9OVU1fUFJFREVGSU5FRF9SRUdJT05T KQ0KPiA+ICsJCXJldHVybjsNCj4gPiArDQo+ID4gKwlyZ24gPSAmYWNwaV9yZWdpb25zW3NwYWNl X2lkXTsNCj4gPiArCWlmICghYWNwaV9yZWdpb25fbWFuYWdlZChyZ24pKQ0KPiA+ICsJCXJldHVy bjsNCj4gPiArDQo+ID4gKwltdXRleF9sb2NrKCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4gKwlp ZiAoIShyZ24tPmZsYWdzICYgQUNQSV9SRUdJT05fUkVHSVNURVJFRCkpIHsNCj4gPiArCQltdXRl eF91bmxvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArCQlyZXR1cm47DQo+ID4gKwl9DQo+ ID4gKwlpZiAocmduLT5mbGFncyAmIEFDUElfUkVHSU9OX1VOUkVHSVNURVJJTkcpIHsNCj4gPiAr CQltdXRleF91bmxvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiArCQlyZXR1cm47DQo+IA0K PiBXaGF0IGFib3V0DQo+IA0KPiAJaWYgKChyZ24tPmZsYWdzICYgQUNQSV9SRUdJT05fVU5SRUdJ U1RFUklORykNCj4gCSAgICB8fCAhKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9SRUdJU1RFUkVE KSkgew0KPiAJCW11dGV4X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiAJCXJldHVybjsN Cj4gCX0NCj4gDQoNCk9LLg0KDQo+ID4gKwl9DQo+ID4gKw0KPiA+ICsJcmduLT5mbGFncyB8PSBB Q1BJX1JFR0lPTl9VTlJFR0lTVEVSSU5HOw0KPiA+ICsJcmduLT5oYW5kbGVyID0gTlVMTDsNCj4g PiArCXJnbi0+c2V0dXAgPSBOVUxMOw0KPiA+ICsJcmduLT5jb250ZXh0ID0gTlVMTDsNCj4gPiAr CW11dGV4X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ICsNCj4gPiArCXdoaWxlIChh dG9taWNfcmVhZCgmcmduLT5yZWZjbnQpID4gMSkNCj4gPiArCQlzY2hlZHVsZV90aW1lb3V0X3Vu aW50ZXJydXB0aWJsZSh1c2Vjc190b19qaWZmaWVzKDUpKTsNCj4gDQo+IFdvdWxkbid0IGl0IGJl IGJldHRlciB0byB1c2UgYSB3YWl0IHF1ZXVlIGhlcmU/DQoNClllcywgSSdsbCB0cnkuDQoNCj4g DQo+ID4gKwlhdG9taWNfZGVjKCZyZ24tPnJlZmNudCk7DQo+ID4gKw0KPiA+ICsJbXV0ZXhfbG9j aygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ICsJcmduLT5mbGFncyAmPSB+KEFDUElfUkVHSU9O X1JFR0lTVEVSRUQgfA0KPiBBQ1BJX1JFR0lPTl9VTlJFR0lTVEVSSU5HKTsNCj4gPiArCW11dGV4 X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ICsNCj4gPiArCXByX2luZm8oIlJlZ2lv biAlZCB1bnJlZ2lzdGVyZWRcbiIsIHNwYWNlX2lkKTsgfQ0KPiA+ICtFWFBPUlRfU1lNQk9MX0dQ TChhY3BpX3VucmVnaXN0ZXJfcmVnaW9uKTsNCj4gPiBkaWZmIC0tZ2l0IGEvaW5jbHVkZS9hY3Bp L2FjcGlfYnVzLmggYi9pbmNsdWRlL2FjcGkvYWNwaV9idXMuaCBpbmRleA0KPiA+IGEyYzJmYmIu LjE1ZmFkMGQgMTAwNjQ0DQo+ID4gLS0tIGEvaW5jbHVkZS9hY3BpL2FjcGlfYnVzLmgNCj4gPiAr KysgYi9pbmNsdWRlL2FjcGkvYWNwaV9idXMuaA0KPiA+IEBAIC01NDIsNCArNTQyLDkgQEAgc3Rh dGljIGlubGluZSBpbnQgdW5yZWdpc3Rlcl9hY3BpX2J1c190eXBlKHZvaWQNCj4gPiAqYnVzKSB7 IHJldHVybiAwOyB9DQo+ID4NCj4gPiAgI2VuZGlmCQkJCS8qIENPTkZJR19BQ1BJICovDQo+ID4N Cj4gPiAraW50IGFjcGlfcmVnaXN0ZXJfcmVnaW9uKGFjcGlfYWRyX3NwYWNlX3R5cGUgc3BhY2Vf aWQsDQo+ID4gKwkJCSBhY3BpX2Fkcl9zcGFjZV9oYW5kbGVyIGhhbmRsZXIsDQo+ID4gKwkJCSBh Y3BpX2Fkcl9zcGFjZV9zZXR1cCBzZXR1cCwgdm9pZCAqY29udGV4dCk7IHZvaWQNCj4gPiArYWNw aV91bnJlZ2lzdGVyX3JlZ2lvbihhY3BpX2Fkcl9zcGFjZV90eXBlIHNwYWNlX2lkKTsNCj4gPiAr DQo+ID4gICNlbmRpZiAvKl9fQUNQSV9CVVNfSF9fKi8NCj4gDQo+IFRoYW5rcywNCj4gUmFmYWVs DQoNClRoYW5rcw0KLUx2DQoNCj4gDQo+IA0KPiAtLQ0KPiBJIHNwZWFrIG9ubHkgZm9yIG15c2Vs Zi4NCj4gUmFmYWVsIEouIFd5c29ja2ksIEludGVsIE9wZW4gU291cmNlIFRlY2hub2xvZ3kgQ2Vu dGVyLg0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> From: Rafael J. Wysocki [mailto:rjw@sisk.pl] > Sent: Friday, July 26, 2013 5:29 AM > > On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > > This patch adds reference couting for ACPI operation region handlers to fix > > races caused by the ACPICA address space callback invocations. > > > > ACPICA address space callback invocation is not suitable for Linux > > CONFIG_MODULE=y execution environment. > > Actually, can you please explain to me what *exactly* the problem is? OK. I'll add race explanations in the next revision. The problem is there is no "lock" held inside ACPICA for invoking operation region handlers. Thus races happens between the acpi_remove/install_address_space_handler and the handler/setup callbacks. This is correct per ACPI specification. As if there is interpreter locks held for invoking operation region handlers, the timeout implemented inside the operation region handlers will make all locking facilities (Acquire or Sleep,...) timed out. Please refer to ACPI specification "5.5.2 Control Method Execution": Interpretation of a Control Method is not preemptive, but it can block. When a control method does block, OSPM can initiate or continue the execution of a different control method. A control method can only assume that access to global objects is exclusive for any period the control method does not block. So it is pretty much likely that ACPI IO transfers are locked inside the operation region callback implementations. Using locking facility to protect the callback invocation will risk dead locks. Thanks -Lv > Rafael
PiBGcm9tOiBsaW51eC1hY3BpLW93bmVyQHZnZXIua2VybmVsLm9yZw0KPiBbbWFpbHRvOmxpbnV4 LWFjcGktb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgWmhlbmcsIEx2DQo+IFNl bnQ6IEZyaWRheSwgSnVseSAyNiwgMjAxMyA4OjQ4IEFNDQo+IA0KPiANCj4gDQo+ID4gRnJvbTog UmFmYWVsIEouIFd5c29ja2kgW21haWx0bzpyandAc2lzay5wbF0NCj4gPiBTZW50OiBGcmlkYXks IEp1bHkgMjYsIDIwMTMgNDoyNyBBTQ0KPiA+DQo+ID4gT24gVHVlc2RheSwgSnVseSAyMywgMjAx MyAwNDowOTo0MyBQTSBMdiBaaGVuZyB3cm90ZToNCj4gPiA+IFRoaXMgcGF0Y2ggYWRkcyByZWZl cmVuY2UgY291dGluZyBmb3IgQUNQSSBvcGVyYXRpb24gcmVnaW9uIGhhbmRsZXJzDQo+ID4gPiB0 byBmaXggcmFjZXMgY2F1c2VkIGJ5IHRoZSBBQ1BJQ0EgYWRkcmVzcyBzcGFjZSBjYWxsYmFjayBp bnZvY2F0aW9ucy4NCj4gPiA+DQo+ID4gPiBBQ1BJQ0EgYWRkcmVzcyBzcGFjZSBjYWxsYmFjayBp bnZvY2F0aW9uIGlzIG5vdCBzdWl0YWJsZSBmb3IgTGludXgNCj4gPiA+IENPTkZJR19NT0RVTEU9 eSBleGVjdXRpb24gZW52aXJvbm1lbnQuICBUaGlzIHBhdGNoIHRyaWVzIHRvIHByb3RlY3QNCj4g PiA+IHRoZSBhZGRyZXNzIHNwYWNlIGNhbGxiYWNrcyBieSBpbnZva2luZyB0aGVtIHVuZGVyIGEg bW9kdWxlIHNhZmUNCj4gPiBlbnZpcm9ubWVudC4NCj4gPiA+IFRoZSBJUE1JIGFkZHJlc3Mgc3Bh Y2UgaGFuZGxlciBpcyBhbHNvIHVwZ3JhZGVkIGluIHRoaXMgcGF0Y2guDQo+ID4gPiBUaGUgYWNw aV91bnJlZ2lzdGVyX3JlZ2lvbigpIGlzIGRlc2lnbmVkIHRvIG1lZXQgdGhlIGZvbGxvd2luZw0K PiA+ID4gcmVxdWlyZW1lbnRzOg0KPiA+ID4gMS4gSXQgYWN0cyBhcyBhIGJhcnJpZXIgZm9yIG9w ZXJhdGlvbiByZWdpb24gY2FsbGJhY2tzIC0gbm8gY2FsbGJhY2sgd2lsbA0KPiA+ID4gICAgaGFw cGVuIGFmdGVyIGFjcGlfdW5yZWdpc3Rlcl9yZWdpb24oKS4NCj4gPiA+IDIuIGFjcGlfdW5yZWdp c3Rlcl9yZWdpb24oKSBpcyBzYWZlIHRvIGJlIGNhbGxlZCBpbiBtb3VkbGUtPmV4aXQoKQ0KPiA+ ID4gICAgZnVuY3Rpb25zLg0KPiA+ID4gVXNpbmcgcmVmZXJlbmNlIGNvdW50aW5nIHJhdGhlciB0 aGFuIG1vZHVsZSByZWZlcmVuY2luZyBhbGxvd3Mgc3VjaA0KPiA+ID4gYmVuZWZpdHMgdG8gYmUg YWNoaWV2ZWQgZXZlbiB3aGVuIGFjcGlfdW5yZWdpc3Rlcl9yZWdpb24oKSBpcyBjYWxsZWQNCj4g PiA+IGluIHRoZSBlbnZpcm9ubWVudHMgb3RoZXIgdGhhbiBtb2R1bGUtPmV4aXQoKS4NCj4gPiA+ IFRoZSBoZWFkZXIgZmlsZSBvZiBpbmNsdWRlL2FjcGkvYWNwaV9idXMuaCBzaG91bGQgY29udGFp biB0aGUNCj4gPiA+IGRlY2xhcmF0aW9ucyB0aGF0IGhhdmUgcmVmZXJlbmNlcyB0byBzb21lIEFD UElDQSBkZWZpbmVkIHR5cGVzLg0KPiA+ID4NCj4gPiA+IFNpZ25lZC1vZmYtYnk6IEx2IFpoZW5n IDxsdi56aGVuZ0BpbnRlbC5jb20+DQo+ID4gPiBSZXZpZXdlZC1ieTogSHVhbmcgWWluZyA8eWlu Zy5odWFuZ0BpbnRlbC5jb20+DQo+ID4gPiAtLS0NCj4gPiA+ICBkcml2ZXJzL2FjcGkvYWNwaV9p cG1pLmMgfCAgIDE2ICsrLS0NCj4gPiA+ICBkcml2ZXJzL2FjcGkvb3NsLmMgICAgICAgfCAgMjI0 DQo+ID4gKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKysrKw0KPiA+ ID4gIGluY2x1ZGUvYWNwaS9hY3BpX2J1cy5oICB8ICAgIDUgKysNCj4gPiA+ICAzIGZpbGVzIGNo YW5nZWQsIDIzNSBpbnNlcnRpb25zKCspLCAxMCBkZWxldGlvbnMoLSkNCj4gPiA+DQo+ID4gPiBk aWZmIC0tZ2l0IGEvZHJpdmVycy9hY3BpL2FjcGlfaXBtaS5jIGIvZHJpdmVycy9hY3BpL2FjcGlf aXBtaS5jDQo+ID4gPiBpbmRleA0KPiA+ID4gNWY4ZjQ5NS4uMmEwOTE1NiAxMDA2NDQNCj4gPiA+ IC0tLSBhL2RyaXZlcnMvYWNwaS9hY3BpX2lwbWkuYw0KPiA+ID4gKysrIGIvZHJpdmVycy9hY3Bp L2FjcGlfaXBtaS5jDQo+ID4gPiBAQCAtNTM5LDIwICs1MzksMTggQEAgb3V0X3JlZjoNCj4gPiA+ ICBzdGF0aWMgaW50IF9faW5pdCBhY3BpX2lwbWlfaW5pdCh2b2lkKSAgew0KPiA+ID4gIAlpbnQg cmVzdWx0ID0gMDsNCj4gPiA+IC0JYWNwaV9zdGF0dXMgc3RhdHVzOw0KPiA+ID4NCj4gPiA+ICAJ aWYgKGFjcGlfZGlzYWJsZWQpDQo+ID4gPiAgCQlyZXR1cm4gcmVzdWx0Ow0KPiA+ID4NCj4gPiA+ ICAJbXV0ZXhfaW5pdCgmZHJpdmVyX2RhdGEuaXBtaV9sb2NrKTsNCj4gPiA+DQo+ID4gPiAtCXN0 YXR1cyA9IGFjcGlfaW5zdGFsbF9hZGRyZXNzX3NwYWNlX2hhbmRsZXIoQUNQSV9ST09UX09CSkVD VCwNCj4gPiA+IC0JCQkJCQkgICAgQUNQSV9BRFJfU1BBQ0VfSVBNSSwNCj4gPiA+IC0JCQkJCQkg ICAgJmFjcGlfaXBtaV9zcGFjZV9oYW5kbGVyLA0KPiA+ID4gLQkJCQkJCSAgICBOVUxMLCBOVUxM KTsNCj4gPiA+IC0JaWYgKEFDUElfRkFJTFVSRShzdGF0dXMpKSB7DQo+ID4gPiArCXJlc3VsdCA9 IGFjcGlfcmVnaXN0ZXJfcmVnaW9uKEFDUElfQURSX1NQQUNFX0lQTUksDQo+ID4gPiArCQkJCSAg ICAgICZhY3BpX2lwbWlfc3BhY2VfaGFuZGxlciwNCj4gPiA+ICsJCQkJICAgICAgTlVMTCwgTlVM TCk7DQo+ID4gPiArCWlmIChyZXN1bHQpIHsNCj4gPiA+ICAJCXByX3dhcm4oIkNhbid0IHJlZ2lz dGVyIElQTUkgb3ByZWdpb24gc3BhY2UgaGFuZGxlXG4iKTsNCj4gPiA+IC0JCXJldHVybiAtRUlO VkFMOw0KPiA+ID4gKwkJcmV0dXJuIHJlc3VsdDsNCj4gPiA+ICAJfQ0KPiA+ID4NCj4gPiA+ICAJ cmVzdWx0ID0gaXBtaV9zbWlfd2F0Y2hlcl9yZWdpc3RlcigmZHJpdmVyX2RhdGEuYm1jX2V2ZW50 cyk7DQo+ID4gPiBAQCAtNTk2LDkgKzU5NCw3IEBAIHN0YXRpYyB2b2lkIF9fZXhpdCBhY3BpX2lw bWlfZXhpdCh2b2lkKQ0KPiA+ID4gIAl9DQo+ID4gPiAgCW11dGV4X3VubG9jaygmZHJpdmVyX2Rh dGEuaXBtaV9sb2NrKTsNCj4gPiA+DQo+ID4gPiAtCWFjcGlfcmVtb3ZlX2FkZHJlc3Nfc3BhY2Vf aGFuZGxlcihBQ1BJX1JPT1RfT0JKRUNULA0KPiA+ID4gLQkJCQkJICBBQ1BJX0FEUl9TUEFDRV9J UE1JLA0KPiA+ID4gLQkJCQkJICAmYWNwaV9pcG1pX3NwYWNlX2hhbmRsZXIpOw0KPiA+ID4gKwlh Y3BpX3VucmVnaXN0ZXJfcmVnaW9uKEFDUElfQURSX1NQQUNFX0lQTUkpOw0KPiA+ID4gIH0NCj4g PiA+DQo+ID4gPiAgbW9kdWxlX2luaXQoYWNwaV9pcG1pX2luaXQpOw0KPiA+ID4gZGlmZiAtLWdp dCBhL2RyaXZlcnMvYWNwaS9vc2wuYyBiL2RyaXZlcnMvYWNwaS9vc2wuYyBpbmRleA0KPiA+ID4g NmFiMmMzNS4uODM5OGU1MSAxMDA2NDQNCj4gPiA+IC0tLSBhL2RyaXZlcnMvYWNwaS9vc2wuYw0K PiA+ID4gKysrIGIvZHJpdmVycy9hY3BpL29zbC5jDQo+ID4gPiBAQCAtODYsNiArODYsNDIgQEAg c3RhdGljIHN0cnVjdCB3b3JrcXVldWVfc3RydWN0ICprYWNwaWRfd3E7DQo+ID4gPiBzdGF0aWMg c3RydWN0IHdvcmtxdWV1ZV9zdHJ1Y3QgKmthY3BpX25vdGlmeV93cTsgIHN0YXRpYyBzdHJ1Y3QN Cj4gPiA+IHdvcmtxdWV1ZV9zdHJ1Y3QgKmthY3BpX2hvdHBsdWdfd3E7DQo+ID4gPg0KPiA+ID4g K3N0cnVjdCBhY3BpX3JlZ2lvbiB7DQo+ID4gPiArCXVuc2lnbmVkIGxvbmcgZmxhZ3M7DQo+ID4g PiArI2RlZmluZSBBQ1BJX1JFR0lPTl9ERUZBVUxUCQkweDAxDQo+ID4gPiArI2RlZmluZSBBQ1BJ X1JFR0lPTl9JTlNUQUxMRUQJCTB4MDINCj4gPiA+ICsjZGVmaW5lIEFDUElfUkVHSU9OX1JFR0lT VEVSRUQJCTB4MDQNCj4gPiA+ICsjZGVmaW5lIEFDUElfUkVHSU9OX1VOUkVHSVNURVJJTkcJMHgw OA0KPiA+ID4gKyNkZWZpbmUgQUNQSV9SRUdJT05fSU5TVEFMTElORwkJMHgxMA0KPiA+DQo+ID4g V2hhdCBhYm91dCAoMVVMIDw8IDEpLCAoMVVMIDw8IDIpIGV0Yy4/DQo+ID4NCj4gPiBBbHNvIHBs ZWFzZSByZW1vdmUgdGhlICNkZWZpbmVzIG91dCBvZiB0aGUgc3RydWN0IGRlZmluaXRpb24uDQo+ IA0KPiBPSy4NCj4gDQo+ID4NCj4gPiA+ICsJLyoNCj4gPiA+ICsJICogTk9URTogVXBncmFkaW5n IEFsbCBSZWdpb24gSGFuZGxlcnMNCj4gPiA+ICsJICogVGhpcyBmbGFnIGlzIG9ubHkgdXNlZCBk dXJpbmcgdGhlIHBlcmlvZCB3aGVyZSBub3QgYWxsIG9mIHRoZQ0KPiA+ID4gKwkgKiByZWdpb24g aGFuZGVycyBhcmUgdXBncmFkZWQgdG8gdGhlIG5ldyBpbnRlcmZhY2VzLg0KPiA+ID4gKwkgKi8N Cj4gPiA+ICsjZGVmaW5lIEFDUElfUkVHSU9OX01BTkFHRUQJCTB4ODANCj4gPiA+ICsJYWNwaV9h ZHJfc3BhY2VfaGFuZGxlciBoYW5kbGVyOw0KPiA+ID4gKwlhY3BpX2Fkcl9zcGFjZV9zZXR1cCBz ZXR1cDsNCj4gPiA+ICsJdm9pZCAqY29udGV4dDsNCj4gPiA+ICsJLyogSW52b2tpbmcgcmVmZXJl bmNlcyAqLw0KPiA+ID4gKwlhdG9taWNfdCByZWZjbnQ7DQo+ID4NCj4gPiBBY3R1YWxseSwgd2h5 IGRvbid0IHlvdSB1c2Uga3JlZnM/DQo+IA0KPiBJZiB5b3UgdGFrZSBhIGxvb2sgYXQgb3RoZXIg cGllY2Ugb2YgbXkgY29kZXMsIHlvdSdsbCBmaW5kIHRoZXJlIGFyZSB0d28gcmVhc29uczoNCj4g DQo+IDEuIEknbSB1c2luZyB3aGlsZSAoYXRvbWljX3JlYWQoKSA+IDEpIHRvIGltcGxlbWVudCB0 aGUgb2JqZWN0cycgZmx1c2hpbmcgYW5kDQo+IHRoZXJlIGlzIG5vIGtyZWYgQVBJIHRvIGRvIHNv Lg0KPiAgIEkganVzdCB0aGluayBpdCBpcyBub3Qgc3VpdGFibGUgZm9yIG1lIHRvIGludHJvZHVj ZSBzdWNoIGFuIEFQSSBpbnRvIGtyZWYuaCBhbmQNCj4gc3RhcnQgYW5vdGhlciBhcmd1bWVudCBh cm91bmQga3JlZiBkZXNpZ25zIGluIHRoaXMgYnVnIGZpeCBwYXRjaC4gOi0pDQo+ICAgSSdsbCBz dGFydCBhIGRpc2N1c3Npb24gYWJvdXQga3JlZiBkZXNpZ24gdXNpbmcgYW5vdGhlciB0aHJlYWQu DQo+IDIuIEknbSB1c2luZyBpcG1pX2Rldnxtc2dfcmVsZWFzZSgpIGFzIGEgcGFpciBvZiBpcG1p X2Rldnxtc2dfYWxsb2MoKSwgaXQncyBraW5kDQo+IG9mIGF0b21pY190IGNvZGluZyBzdHlsZS4N Cj4gICBJZiBhdG9taWNfdCBpcyBjaGFuZ2VkIHRvIHN0cnVjdCBrcmVmLCBJIHdpbGwgbmVlZCB0 byBpbXBsZW1lbnQgdHdvIEFQSSwNCj4gX19pcG1pX2Rldl9yZWxlYXNlKCkgdG8gdGFrZSBhIHN0 cnVjdCBrcmVmIGFzIHBhcmFtZXRlciBhbmQgY2FsbA0KPiBpcG1pX2Rldl9yZWxlYXNlIGluc2lk ZSBpdC4NCj4gICBCeSBub3QgdXNpbmcga3JlZiwgSSBuZWVkbid0IHdyaXRlIGNvZGVzIHRvIGlt cGxlbWVudCBzdWNoIEFQSS4NCj4gDQo+ID4NCj4gPiA+ICt9Ow0KPiA+ID4gKw0KPiA+ID4gK3N0 YXRpYyBzdHJ1Y3QgYWNwaV9yZWdpb24NCj4gYWNwaV9yZWdpb25zW0FDUElfTlVNX1BSRURFRklO RURfUkVHSU9OU10NCj4gPiA9IHsNCj4gPiA+ICsJW0FDUElfQURSX1NQQUNFX1NZU1RFTV9NRU1P UlldID0gew0KPiA+ID4gKwkJLmZsYWdzID0gQUNQSV9SRUdJT05fREVGQVVMVCwNCj4gPiA+ICsJ fSwNCj4gPiA+ICsJW0FDUElfQURSX1NQQUNFX1NZU1RFTV9JT10gPSB7DQo+ID4gPiArCQkuZmxh Z3MgPSBBQ1BJX1JFR0lPTl9ERUZBVUxULA0KPiA+ID4gKwl9LA0KPiA+ID4gKwlbQUNQSV9BRFJf U1BBQ0VfUENJX0NPTkZJR10gPSB7DQo+ID4gPiArCQkuZmxhZ3MgPSBBQ1BJX1JFR0lPTl9ERUZB VUxULA0KPiA+ID4gKwl9LA0KPiA+ID4gKwlbQUNQSV9BRFJfU1BBQ0VfSVBNSV0gPSB7DQo+ID4g PiArCQkuZmxhZ3MgPSBBQ1BJX1JFR0lPTl9NQU5BR0VELA0KPiA+ID4gKwl9LA0KPiA+ID4gK307 DQo+ID4gPiArc3RhdGljIERFRklORV9NVVRFWChhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4gPiAr DQo+ID4gPiAgLyoNCj4gPiA+ICAgKiBUaGlzIGxpc3Qgb2YgcGVybWFuZW50IG1hcHBpbmdzIGlz IGZvciBtZW1vcnkgdGhhdCBtYXkgYmUNCj4gPiA+IGFjY2Vzc2VkDQo+ID4gZnJvbQ0KPiA+ID4g ICAqIGludGVycnVwdCBjb250ZXh0LCB3aGVyZSB3ZSBjYW4ndCBkbyB0aGUgaW9yZW1hcCgpLg0K PiA+ID4gQEAgLTE3OTksMyArMTgzNSwxOTEgQEAgdm9pZCBhbGxvY19hY3BpX2hwX3dvcmsoYWNw aV9oYW5kbGUgaGFuZGxlLA0KPiA+IHUzMiB0eXBlLCB2b2lkICpjb250ZXh0LA0KPiA+ID4gIAkJ a2ZyZWUoaHBfd29yayk7DQo+ID4gPiAgfQ0KPiA+ID4gIEVYUE9SVF9TWU1CT0xfR1BMKGFsbG9j X2FjcGlfaHBfd29yayk7DQo+ID4gPiArDQo+ID4gPiArc3RhdGljIGJvb2wgYWNwaV9yZWdpb25f bWFuYWdlZChzdHJ1Y3QgYWNwaV9yZWdpb24gKnJnbikgew0KPiA+ID4gKwkvKg0KPiA+ID4gKwkg KiBOT1RFOiBEZWZhdWx0IGFuZCBNYW5hZ2VkDQo+ID4gPiArCSAqIFdlIG9ubHkgbmVlZCB0byBh dm9pZCByZWdpb24gbWFuYWdlbWVudCBvbiB0aGUgcmVnaW9ucw0KPiBtYW5hZ2VkDQo+ID4gPiAr CSAqIGJ5IEFDUElDQSAoQUNQSV9SRUdJT05fREVGQVVMVCkuICBDdXJyZW50bHksIHdlIG5lZWQN Cj4gYWRkaXRpb25hbA0KPiA+ID4gKwkgKiBjaGVjayBhcyBtYW55IG9wZXJhdGlvbiByZWdpb24g aGFuZGxlcnMgYXJlIG5vdCB1cGdyYWRlZCwgc28NCj4gPiA+ICsJICogb25seSB0aG9zZSBrbm93 biB0byBiZSBzYWZlIGFyZSBtYW5hZ2VkDQo+IChBQ1BJX1JFR0lPTl9NQU5BR0VEKS4NCj4gPiA+ ICsJICovDQo+ID4gPiArCXJldHVybiAhKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9ERUZBVUxU KSAmJg0KPiA+ID4gKwkgICAgICAgKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9NQU5BR0VEKTsg fQ0KPiA+ID4gKw0KPiA+ID4gK3N0YXRpYyBib29sIGFjcGlfcmVnaW9uX2NhbGxhYmxlKHN0cnVj dCBhY3BpX3JlZ2lvbiAqcmduKSB7DQo+ID4gPiArCXJldHVybiAocmduLT5mbGFncyAmIEFDUElf UkVHSU9OX1JFR0lTVEVSRUQpICYmDQo+ID4gPiArCSAgICAgICAhKHJnbi0+ZmxhZ3MgJiBBQ1BJ X1JFR0lPTl9VTlJFR0lTVEVSSU5HKTsgfQ0KPiA+ID4gKw0KPiA+ID4gK3N0YXRpYyBhY3BpX3N0 YXR1cw0KPiA+ID4gK2FjcGlfcmVnaW9uX2RlZmF1bHRfaGFuZGxlcih1MzIgZnVuY3Rpb24sDQo+ ID4gPiArCQkJICAgIGFjcGlfcGh5c2ljYWxfYWRkcmVzcyBhZGRyZXNzLA0KPiA+ID4gKwkJCSAg ICB1MzIgYml0X3dpZHRoLCB1NjQgKnZhbHVlLA0KPiA+ID4gKwkJCSAgICB2b2lkICpoYW5kbGVy X2NvbnRleHQsIHZvaWQgKnJlZ2lvbl9jb250ZXh0KSB7DQo+ID4gPiArCWFjcGlfYWRyX3NwYWNl X2hhbmRsZXIgaGFuZGxlcjsNCj4gPiA+ICsJc3RydWN0IGFjcGlfcmVnaW9uICpyZ24gPSAoc3Ry dWN0IGFjcGlfcmVnaW9uICopaGFuZGxlcl9jb250ZXh0Ow0KPiA+ID4gKwl2b2lkICpjb250ZXh0 Ow0KPiA+ID4gKwlhY3BpX3N0YXR1cyBzdGF0dXMgPSBBRV9OT1RfRVhJU1Q7DQo+ID4gPiArDQo+ ID4gPiArCW11dGV4X2xvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiA+ICsJaWYgKCFhY3Bp X3JlZ2lvbl9jYWxsYWJsZShyZ24pIHx8ICFyZ24tPmhhbmRsZXIpIHsNCj4gPiA+ICsJCW11dGV4 X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ID4gKwkJcmV0dXJuIHN0YXR1czsNCj4g PiA+ICsJfQ0KPiA+ID4gKw0KPiA+ID4gKwlhdG9taWNfaW5jKCZyZ24tPnJlZmNudCk7DQo+ID4g PiArCWhhbmRsZXIgPSByZ24tPmhhbmRsZXI7DQo+ID4gPiArCWNvbnRleHQgPSByZ24tPmNvbnRl eHQ7DQo+ID4gPiArCW11dGV4X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ID4gKw0K PiA+ID4gKwlzdGF0dXMgPSBoYW5kbGVyKGZ1bmN0aW9uLCBhZGRyZXNzLCBiaXRfd2lkdGgsIHZh bHVlLCBjb250ZXh0LA0KPiA+ID4gKwkJCSByZWdpb25fY29udGV4dCk7DQo+ID4NCj4gPiBXaHkg ZG9uJ3Qgd2UgY2FsbCB0aGUgaGFuZGxlciB1bmRlciB0aGUgbXV0ZXg/DQoNCkkgdGhpbmsgbXkg cmVwbHkgaXMgYWdhaW5zdCB0aGlzIHF1ZXN0aW9uLCBsZXQgbWUgcmVtb3ZlIGl0IHVwLg0KDQo+ IEl0J3MgYSBraW5kIG9mIHByb2dyYW1taW5nIHN0eWxlIHJlbGF0ZWQgY29uY2Vybi4NCj4gSU1P LCB1c2luZyBsb2NrcyBhcm91bmQgY2FsbGJhY2sgZnVuY3Rpb24gaXMgYSBidWdneSBwcm9ncmFt bWluZyBzdHlsZSB0aGF0DQo+IGNvdWxkIGxlYWQgdG8gZGVhZCBsb2Nrcy4NCj4gTGV0IG1lIGV4 cGxhaW4gdGhpcyB1c2luZyBhbiBleGFtcGxlLg0KPiANCj4gT2JqZWN0IEEgZXhwb3J0cyBhIHJl Z2lzdGVyL3VucmVnaXN0ZXIgQVBJIGZvciBvdGhlciBvYmplY3RzLg0KPiBPYmplY3QgQiBjYWxs cyBBJ3MgcmVnaXN0ZXIvdW5yZWdpc3RlciBBUEkgdG8gcmVnaXN0ZXIvdW5yZWdpc3RlciBCJ3Mg Y2FsbGJhY2suDQoNClNvcnJ5IEkgaGF2ZSB0byB1c2Ugb2JqZWN0IHJhdGhlciB0aGFuIG1vZHVs ZSBoZXJlIGFzIHRoZXJlIG1pZ2h0IGJlIHNldmVyYWwgb2JqZWN0cyBpbnNpZGUgYSBtb2R1bGUg dGhhdCBoYXZlIHRoZSBzYW1lIHNpdHVhdGlvbiBuZWVkIHRvIGJlIGhhbmRsZWQuDQoNCj4gSXQn cyBsaWtlbHkgdGhhdCBvYmplY3QgQiB3aWxsIGhvbGQgbG9ja19vZl9CIGFyb3VuZCB1bnJlZ2lz dGVyL3JlZ2lzdGVyIHdoZW4NCj4gb2JqZWN0IEIgaXMgZGVzdHJveWVkL2NyZWF0ZWQsIHRoZSBs b2NrX29mX0IgaXMgbGlrZWx5IGFsc28gdXNlZCBpbnNpZGUgdGhlDQo+IGNhbGxiYWNrLg0KPiBT byB3aGVuIG9iamVjdCBBIGhvbGRzIHRoZSBsb2NrX29mX0EgYXJvdW5kIHRoZSBjYWxsYmFjayBp bnZvY2F0aW9uLCBpdCBsZWFkcyB0bw0KPiBkZWFkIGxvY2sgc2luY2U6DQo+IDEuIHRoZSBsb2Nr aW5nIG9yZGVyIGZvciB0aGUgcmVnaXN0ZXIvdW5yZWdpc3RlciBzaWRlIHdpbGwgYmU6IGxvY2so bG9ja19vZl9CKSwNCj4gbG9jayhsb2NrX29mX0EpIDIuIHRoZSBsb2NraW5nIG9yZGVyIGZvciB0 aGUgY2FsbGJhY2sgc2lkZSB3aWxsIGJlOiBsb2NrKGxvY2tfb2ZfQSksDQo+IGxvY2sobG9ja19v Zl9CKSBUaGV5IGFyZSBpbiB0aGUgcmV2ZXJzZWQgb3JkZXIhDQoNCkkgdGhpbmsgdGhpcyBleGFt cGxlIGlzIG5vdCBxdWl0ZSBjb3JyZWN0Lg0KVGhlcmUgaXMgYW5vdGhlciBhc3BlY3QgaW4gdW5y ZWdpc3RlciBpbXBsZW1lbnRhdGlvbiB3aGljaCBpcyB0aGUgaW50ZW50IG9mIHRoaXMgcGF0Y2g6 DQpObyBjYWxsYmFjayBydW5uaW5nL2ltaXRhdGVkIGFmdGVyICJ1bnJlZ2lzdGVyIiwgd2UgY2Fu IGNhbGwgdGhpcyBhICJmbHVzaCIgcmVxdWlyZW1lbnQuDQpJbnNpZGUgb2YgdGhlIGNhbGxiYWNr LCBsb2NrX29mX0Igc2hvdWxkIG5vdCBiZSBoZWxkIGlmICJmbHVzaCIgaXMgcmVxdWlyZWQuDQoN Cj4gDQo+IElNTywgTGludXggbWF5IG5lZWQgdG8gaW50cm9kdWNlIF9fY2FsbGJhY2ssIF9fYXBp IGFzIGRlY2VsZXJhdG9ycyBmb3IgdGhlDQo+IGZ1bmN0aW9ucywgYW5kIHVzZSBzcGFyc2UgdG8g ZW5mb3JjZSB0aGlzIHJ1bGUsIHNwYXJzZSBrbm93cyBpZiBhIGNhbGxiYWNrIGlzDQo+IGludm9r ZWQgdW5kZXIgc29tZSBsb2Nrcy4NCj4gDQo+IEluIHRoZSBjYXNlIG9mIEFDUElDQSBzcGFjZV9o YW5kbGVycywgYXMgeW91IG1heSBrbm93LCB3aGVuIGFuIEFDUEkNCj4gb3BlcmF0aW9uIHJlZ2lv biBoYW5kbGVyIGlzIGludm9rZWQsIHRoZXJlIHdpbGwgYmUgbm8gbG9jayBoZWxkIGluc2lkZSBB Q1BJQ0ENCj4gKGludGVycHJldGVyIGxvY2sgbXVzdCBiZSBmcmVlZCBiZWZvcmUgZXhlY3V0aW5n IG9wZXJhdGlvbiByZWdpb24gaGFuZGxlcnMpLg0KPiBTbyB0aGUgbGlrZWxpaG9vZCBvZiB0aGUg ZGVhZCBsb2NrIGlzIHByZXR0eSBtdWNoIGhpZ2ggaGVyZSENCg0KSSBuZWVkIHRvIG1lbnRpb24g YW5vdGhlciByZXF1aXJlbWVudCBvZiB0aGUgb3BlcmF0aW9uIHJlZ2lvbiBoYW5kbGVyLg0KSXQg aXMgcmVxdWlyZWQgdGhhdCBtdWx0aXBsZSBvcGVyYXRpb24gcmVnaW9uIGhhbmRsZXJzIGFyZSBl eGVjdXRlZCBhdCB0aGUgc2FtZSB0aW1lLCBvciB0aGUgSU8gb3BlcmF0aW9ucyBpbnZva2VkIGJ5 IHRoZSBCSU9TIEFTTCBjb2RlcyB3aWxsIGJlIHNlcmlhbGl6ZWQuDQpJTU8sIElPIG9wZXJhdGlv bnMgaW52b2tlZCBieSB0aGUgQklPUyBBU0wgbmVlZCB0byBiZSBwYXJhbGxlbGl6ZWQuDQpUaHVz IG11dGV4IGlzIG5vdCB1c2VmdWwgdG8gaW1wbGVtZW50IGEgcHJvdGVjdGlvbiBoZXJlLg0KDQpT byB0aGUgbXV0ZXggaXMgdW5sb2NrZWQgYmVmb3JlIGV4ZWN1dGluZyB0aGUgaGFuZGxlciwgSU1P LCByZWZlcmVuY2UgY291bnRpbmcgaXMgdXNlZnVsIGhlcmUgdG8gbWVldCB0aGlzIHJlcXVpcmVt ZW50Lg0KDQo+ID4NCj4gPiBXaGF0IGV4YWN0bHkgcHJldmVudHMgY29udGV4dCBmcm9tIGJlY29t aW5nIE5VTEwgYmVmb3JlIHRoZSBjYWxsIGFib3ZlPw0KDQpJIHRoaW5rIG15IGFuc3dlcnMgZGlk IG5vdCBhbnN3ZXIgdGhpcyBxdWVzdGlvbiBkaXJlY3RseS4NCg0KU29ycnkgdGhhdCBJJ20gbm90 IGNsZWFyIHdoYXQgeW91IHdhbnQgdG8gYXNrIGhlcmUuICBMZXQgbWUganVzdCB0cnkgdG8gYmUg cHJhY3RpY2FsLg0KDQpUaGUgY29kZSBpcyBoZXJlOg0KPg0KPiA+ID4gKwltdXRleF9sb2NrKCZh Y3BpX211dGV4X3JlZ2lvbik7DQo+ID4gPiArCWlmICghYWNwaV9yZWdpb25fY2FsbGFibGUocmdu KSB8fCAhcmduLT5oYW5kbGVyKSB7DQoNCj4gPiA+ICsJaGFuZGxlciA9IHJnbi0+aGFuZGxlcjsN Cj4gPiA+ICsJY29udGV4dCA9IHJnbi0+Y29udGV4dDsNCj4gPiA+ICsJbXV0ZXhfdW5sb2NrKCZh Y3BpX211dGV4X3JlZ2lvbik7DQoNClRoZSBoYW5kbGVyIGlzIGVuc3VyZWQgbm90IHRvIGJlIE5V TEwgd2l0aGluIHRoZSBtdXRleCBsb2NrLg0KDQpUaGFua3MgZm9yIGNvbW1lbnRpbmcuDQoNCkJl c3QgcmVnYXJkcw0KLUx2DQoNCj4gPg0KPiA+ID4gKwlhdG9taWNfZGVjKCZyZ24tPnJlZmNudCk7 DQo+ID4gPiArDQo+ID4gPiArCXJldHVybiBzdGF0dXM7DQo+ID4gPiArfQ0KPiA+ID4gKw0KPiA+ ID4gK3N0YXRpYyBhY3BpX3N0YXR1cw0KPiA+ID4gK2FjcGlfcmVnaW9uX2RlZmF1bHRfc2V0dXAo YWNwaV9oYW5kbGUgaGFuZGxlLCB1MzIgZnVuY3Rpb24sDQo+ID4gPiArCQkJICB2b2lkICpoYW5k bGVyX2NvbnRleHQsIHZvaWQgKipyZWdpb25fY29udGV4dCkgew0KPiA+ID4gKwlhY3BpX2Fkcl9z cGFjZV9zZXR1cCBzZXR1cDsNCj4gPiA+ICsJc3RydWN0IGFjcGlfcmVnaW9uICpyZ24gPSAoc3Ry dWN0IGFjcGlfcmVnaW9uICopaGFuZGxlcl9jb250ZXh0Ow0KPiA+ID4gKwl2b2lkICpjb250ZXh0 Ow0KPiA+ID4gKwlhY3BpX3N0YXR1cyBzdGF0dXMgPSBBRV9PSzsNCj4gPiA+ICsNCj4gPiA+ICsJ bXV0ZXhfbG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ID4gKwlpZiAoIWFjcGlfcmVnaW9u X2NhbGxhYmxlKHJnbikgfHwgIXJnbi0+c2V0dXApIHsNCj4gPiA+ICsJCW11dGV4X3VubG9jaygm YWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ID4gKwkJcmV0dXJuIHN0YXR1czsNCj4gPiA+ICsJfQ0K PiA+ID4gKw0KPiA+ID4gKwlhdG9taWNfaW5jKCZyZ24tPnJlZmNudCk7DQo+ID4gPiArCXNldHVw ID0gcmduLT5zZXR1cDsNCj4gPiA+ICsJY29udGV4dCA9IHJnbi0+Y29udGV4dDsNCj4gPiA+ICsJ bXV0ZXhfdW5sb2NrKCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4gPiArDQo+ID4gPiArCXN0YXR1 cyA9IHNldHVwKGhhbmRsZSwgZnVuY3Rpb24sIGNvbnRleHQsIHJlZ2lvbl9jb250ZXh0KTsNCj4g Pg0KPiA+IENhbiBzZXR1cCBkcm9wIHJnbi0+cmVmY250ID8NCj4gDQo+IFRoZSByZWFzb24gaXMg c2FtZSBhcyB0aGUgaGFuZGxlciwgYXMgYSBzZXR1cCBpcyBhbHNvIGEgY2FsbGJhY2suDQo+IA0K PiA+DQo+ID4gPiArCWF0b21pY19kZWMoJnJnbi0+cmVmY250KTsNCj4gPiA+ICsNCj4gPiA+ICsJ cmV0dXJuIHN0YXR1czsNCj4gPiA+ICt9DQo+ID4gPiArDQo+ID4gPiArc3RhdGljIGludCBfX2Fj cGlfaW5zdGFsbF9yZWdpb24oc3RydWN0IGFjcGlfcmVnaW9uICpyZ24sDQo+ID4gPiArCQkJCSBh Y3BpX2Fkcl9zcGFjZV90eXBlIHNwYWNlX2lkKQ0KPiA+ID4gK3sNCj4gPiA+ICsJaW50IHJlcyA9 IDA7DQo+ID4gPiArCWFjcGlfc3RhdHVzIHN0YXR1czsNCj4gPiA+ICsJaW50IGluc3RhbGxpbmcg PSAwOw0KPiA+ID4gKw0KPiA+ID4gKwltdXRleF9sb2NrKCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ ID4gPiArCWlmIChyZ24tPmZsYWdzICYgQUNQSV9SRUdJT05fSU5TVEFMTEVEKQ0KPiA+ID4gKwkJ Z290byBvdXRfbG9jazsNCj4gPiA+ICsJaWYgKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9JTlNU QUxMSU5HKSB7DQo+ID4gPiArCQlyZXMgPSAtRUJVU1k7DQo+ID4gPiArCQlnb3RvIG91dF9sb2Nr Ow0KPiA+ID4gKwl9DQo+ID4gPiArDQo+ID4gPiArCWluc3RhbGxpbmcgPSAxOw0KPiA+ID4gKwly Z24tPmZsYWdzIHw9IEFDUElfUkVHSU9OX0lOU1RBTExJTkc7DQo+ID4gPiArCXN0YXR1cyA9IGFj cGlfaW5zdGFsbF9hZGRyZXNzX3NwYWNlX2hhbmRsZXIoQUNQSV9ST09UX09CSkVDVCwNCj4gPiBz cGFjZV9pZCwNCj4gPiA+ICsJCQkJCQkgICAgYWNwaV9yZWdpb25fZGVmYXVsdF9oYW5kbGVyLA0K PiA+ID4gKwkJCQkJCSAgICBhY3BpX3JlZ2lvbl9kZWZhdWx0X3NldHVwLA0KPiA+ID4gKwkJCQkJ CSAgICByZ24pOw0KPiA+ID4gKwlyZ24tPmZsYWdzICY9IH5BQ1BJX1JFR0lPTl9JTlNUQUxMSU5H Ow0KPiA+ID4gKwlpZiAoQUNQSV9GQUlMVVJFKHN0YXR1cykpDQo+ID4gPiArCQlyZXMgPSAtRUlO VkFMOw0KPiA+ID4gKwllbHNlDQo+ID4gPiArCQlyZ24tPmZsYWdzIHw9IEFDUElfUkVHSU9OX0lO U1RBTExFRDsNCj4gPiA+ICsNCj4gPiA+ICtvdXRfbG9jazoNCj4gPiA+ICsJbXV0ZXhfdW5sb2Nr KCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4gPiArCWlmIChpbnN0YWxsaW5nKSB7DQo+ID4gPiAr CQlpZiAocmVzKQ0KPiA+ID4gKwkJCXByX2VycigiRmFpbGVkIHRvIGluc3RhbGwgcmVnaW9uICVk XG4iLCBzcGFjZV9pZCk7DQo+ID4gPiArCQllbHNlDQo+ID4gPiArCQkJcHJfaW5mbygiUmVnaW9u ICVkIGluc3RhbGxlZFxuIiwgc3BhY2VfaWQpOw0KPiA+ID4gKwl9DQo+ID4gPiArCXJldHVybiBy ZXM7DQo+ID4gPiArfQ0KPiA+ID4gKw0KPiA+ID4gK2ludCBhY3BpX3JlZ2lzdGVyX3JlZ2lvbihh Y3BpX2Fkcl9zcGFjZV90eXBlIHNwYWNlX2lkLA0KPiA+ID4gKwkJCSBhY3BpX2Fkcl9zcGFjZV9o YW5kbGVyIGhhbmRsZXIsDQo+ID4gPiArCQkJIGFjcGlfYWRyX3NwYWNlX3NldHVwIHNldHVwLCB2 b2lkICpjb250ZXh0KSB7DQo+ID4gPiArCWludCByZXM7DQo+ID4gPiArCXN0cnVjdCBhY3BpX3Jl Z2lvbiAqcmduOw0KPiA+ID4gKw0KPiA+ID4gKwlpZiAoc3BhY2VfaWQgPj0gQUNQSV9OVU1fUFJF REVGSU5FRF9SRUdJT05TKQ0KPiA+ID4gKwkJcmV0dXJuIC1FSU5WQUw7DQo+ID4gPiArDQo+ID4g PiArCXJnbiA9ICZhY3BpX3JlZ2lvbnNbc3BhY2VfaWRdOw0KPiA+ID4gKwlpZiAoIWFjcGlfcmVn aW9uX21hbmFnZWQocmduKSkNCj4gPiA+ICsJCXJldHVybiAtRUlOVkFMOw0KPiA+ID4gKw0KPiA+ ID4gKwlyZXMgPSBfX2FjcGlfaW5zdGFsbF9yZWdpb24ocmduLCBzcGFjZV9pZCk7DQo+ID4gPiAr CWlmIChyZXMpDQo+ID4gPiArCQlyZXR1cm4gcmVzOw0KPiA+ID4gKw0KPiA+ID4gKwltdXRleF9s b2NrKCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4gPiArCWlmIChyZ24tPmZsYWdzICYgQUNQSV9S RUdJT05fUkVHSVNURVJFRCkgew0KPiA+ID4gKwkJbXV0ZXhfdW5sb2NrKCZhY3BpX211dGV4X3Jl Z2lvbik7DQo+ID4gPiArCQlyZXR1cm4gLUVCVVNZOw0KPiA+ID4gKwl9DQo+ID4gPiArDQo+ID4g PiArCXJnbi0+aGFuZGxlciA9IGhhbmRsZXI7DQo+ID4gPiArCXJnbi0+c2V0dXAgPSBzZXR1cDsN Cj4gPiA+ICsJcmduLT5jb250ZXh0ID0gY29udGV4dDsNCj4gPiA+ICsJcmduLT5mbGFncyB8PSBB Q1BJX1JFR0lPTl9SRUdJU1RFUkVEOw0KPiA+ID4gKwlhdG9taWNfc2V0KCZyZ24tPnJlZmNudCwg MSk7DQo+ID4gPiArCW11dGV4X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ID4gKw0K PiA+ID4gKwlwcl9pbmZvKCJSZWdpb24gJWQgcmVnaXN0ZXJlZFxuIiwgc3BhY2VfaWQpOw0KPiA+ ID4gKw0KPiA+ID4gKwlyZXR1cm4gMDsNCj4gPiA+ICt9DQo+ID4gPiArRVhQT1JUX1NZTUJPTF9H UEwoYWNwaV9yZWdpc3Rlcl9yZWdpb24pOw0KPiA+ID4gKw0KPiA+ID4gK3ZvaWQgYWNwaV91bnJl Z2lzdGVyX3JlZ2lvbihhY3BpX2Fkcl9zcGFjZV90eXBlIHNwYWNlX2lkKSB7DQo+ID4gPiArCXN0 cnVjdCBhY3BpX3JlZ2lvbiAqcmduOw0KPiA+ID4gKw0KPiA+ID4gKwlpZiAoc3BhY2VfaWQgPj0g QUNQSV9OVU1fUFJFREVGSU5FRF9SRUdJT05TKQ0KPiA+ID4gKwkJcmV0dXJuOw0KPiA+ID4gKw0K PiA+ID4gKwlyZ24gPSAmYWNwaV9yZWdpb25zW3NwYWNlX2lkXTsNCj4gPiA+ICsJaWYgKCFhY3Bp X3JlZ2lvbl9tYW5hZ2VkKHJnbikpDQo+ID4gPiArCQlyZXR1cm47DQo+ID4gPiArDQo+ID4gPiAr CW11dGV4X2xvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiA+ICsJaWYgKCEocmduLT5mbGFn cyAmIEFDUElfUkVHSU9OX1JFR0lTVEVSRUQpKSB7DQo+ID4gPiArCQltdXRleF91bmxvY2soJmFj cGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiA+ICsJCXJldHVybjsNCj4gPiA+ICsJfQ0KPiA+ID4gKwlp ZiAocmduLT5mbGFncyAmIEFDUElfUkVHSU9OX1VOUkVHSVNURVJJTkcpIHsNCj4gPiA+ICsJCW11 dGV4X3VubG9jaygmYWNwaV9tdXRleF9yZWdpb24pOw0KPiA+ID4gKwkJcmV0dXJuOw0KPiA+DQo+ ID4gV2hhdCBhYm91dA0KPiA+DQo+ID4gCWlmICgocmduLT5mbGFncyAmIEFDUElfUkVHSU9OX1VO UkVHSVNURVJJTkcpDQo+ID4gCSAgICB8fCAhKHJnbi0+ZmxhZ3MgJiBBQ1BJX1JFR0lPTl9SRUdJ U1RFUkVEKSkgew0KPiA+IAkJbXV0ZXhfdW5sb2NrKCZhY3BpX211dGV4X3JlZ2lvbik7DQo+ID4g CQlyZXR1cm47DQo+ID4gCX0NCj4gPg0KPiANCj4gT0suDQo+IA0KPiA+ID4gKwl9DQo+ID4gPiAr DQo+ID4gPiArCXJnbi0+ZmxhZ3MgfD0gQUNQSV9SRUdJT05fVU5SRUdJU1RFUklORzsNCj4gPiA+ ICsJcmduLT5oYW5kbGVyID0gTlVMTDsNCj4gPiA+ICsJcmduLT5zZXR1cCA9IE5VTEw7DQo+ID4g PiArCXJnbi0+Y29udGV4dCA9IE5VTEw7DQo+ID4gPiArCW11dGV4X3VubG9jaygmYWNwaV9tdXRl eF9yZWdpb24pOw0KPiA+ID4gKw0KPiA+ID4gKwl3aGlsZSAoYXRvbWljX3JlYWQoJnJnbi0+cmVm Y250KSA+IDEpDQo+ID4gPiArCQlzY2hlZHVsZV90aW1lb3V0X3VuaW50ZXJydXB0aWJsZSh1c2Vj c190b19qaWZmaWVzKDUpKTsNCj4gPg0KPiA+IFdvdWxkbid0IGl0IGJlIGJldHRlciB0byB1c2Ug YSB3YWl0IHF1ZXVlIGhlcmU/DQo+IA0KPiBZZXMsIEknbGwgdHJ5Lg0KPiANCj4gPg0KPiA+ID4g KwlhdG9taWNfZGVjKCZyZ24tPnJlZmNudCk7DQo+ID4gPiArDQo+ID4gPiArCW11dGV4X2xvY2so JmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiA+ICsJcmduLT5mbGFncyAmPSB+KEFDUElfUkVHSU9O X1JFR0lTVEVSRUQgfA0KPiA+IEFDUElfUkVHSU9OX1VOUkVHSVNURVJJTkcpOw0KPiA+ID4gKwlt dXRleF91bmxvY2soJmFjcGlfbXV0ZXhfcmVnaW9uKTsNCj4gPiA+ICsNCj4gPiA+ICsJcHJfaW5m bygiUmVnaW9uICVkIHVucmVnaXN0ZXJlZFxuIiwgc3BhY2VfaWQpOyB9DQo+ID4gPiArRVhQT1JU X1NZTUJPTF9HUEwoYWNwaV91bnJlZ2lzdGVyX3JlZ2lvbik7DQo+ID4gPiBkaWZmIC0tZ2l0IGEv aW5jbHVkZS9hY3BpL2FjcGlfYnVzLmggYi9pbmNsdWRlL2FjcGkvYWNwaV9idXMuaCBpbmRleA0K PiA+ID4gYTJjMmZiYi4uMTVmYWQwZCAxMDA2NDQNCj4gPiA+IC0tLSBhL2luY2x1ZGUvYWNwaS9h Y3BpX2J1cy5oDQo+ID4gPiArKysgYi9pbmNsdWRlL2FjcGkvYWNwaV9idXMuaA0KPiA+ID4gQEAg LTU0Miw0ICs1NDIsOSBAQCBzdGF0aWMgaW5saW5lIGludCB1bnJlZ2lzdGVyX2FjcGlfYnVzX3R5 cGUodm9pZA0KPiA+ID4gKmJ1cykgeyByZXR1cm4gMDsgfQ0KPiA+ID4NCj4gPiA+ICAjZW5kaWYJ CQkJLyogQ09ORklHX0FDUEkgKi8NCj4gPiA+DQo+ID4gPiAraW50IGFjcGlfcmVnaXN0ZXJfcmVn aW9uKGFjcGlfYWRyX3NwYWNlX3R5cGUgc3BhY2VfaWQsDQo+ID4gPiArCQkJIGFjcGlfYWRyX3Nw YWNlX2hhbmRsZXIgaGFuZGxlciwNCj4gPiA+ICsJCQkgYWNwaV9hZHJfc3BhY2Vfc2V0dXAgc2V0 dXAsIHZvaWQgKmNvbnRleHQpOyB2b2lkDQo+ID4gPiArYWNwaV91bnJlZ2lzdGVyX3JlZ2lvbihh Y3BpX2Fkcl9zcGFjZV90eXBlIHNwYWNlX2lkKTsNCj4gPiA+ICsNCj4gPiA+ICAjZW5kaWYgLypf X0FDUElfQlVTX0hfXyovDQo+ID4NCj4gPiBUaGFua3MsDQo+ID4gUmFmYWVsDQo+IA0KPiBUaGFu a3MNCj4gLUx2DQo+IA0KPiA+DQo+ID4NCj4gPiAtLQ0KPiA+IEkgc3BlYWsgb25seSBmb3IgbXlz ZWxmLg0KPiA+IFJhZmFlbCBKLiBXeXNvY2tpLCBJbnRlbCBPcGVuIFNvdXJjZSBUZWNobm9sb2d5 IENlbnRlci4NCj4gTiAgICAgciAgeSAgIGIgWCAgx6d2IF4gKd66ey5uICsgICAgeyBpIGIge2F5 IB3Kh9qZICxqICAgZiAgIGggICB6IB4gdw0KPiBqOit2ICAgdyBqIG0gICAgICAgICB6WisgICAg IN2iaiIgICEgaQ0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
PiBGcm9tOiBsaW51eC1hY3BpLW93bmVyQHZnZXIua2VybmVsLm9yZw0KPiBbbWFpbHRvOmxpbnV4 LWFjcGktb3duZXJAdmdlci5rZXJuZWwub3JnXSBPbiBCZWhhbGYgT2YgWmhlbmcsIEx2DQo+IFNl bnQ6IEZyaWRheSwgSnVseSAyNiwgMjAxMyA5OjU0IEFNDQo+IFRvOiBSYWZhZWwgSi4gV3lzb2Nr aQ0KPiBDYzogV3lzb2NraSwgUmFmYWVsIEo7IEJyb3duLCBMZW47IGxpbnV4LWtlcm5lbEB2Z2Vy Lmtlcm5lbC5vcmc7DQo+IGxpbnV4LWFjcGlAdmdlci5rZXJuZWwub3JnDQo+IFN1YmplY3Q6IFJF OiBbUEFUQ0ggMDYvMTNdIEFDUEkvSVBNSTogQWRkIHJlZmVyZW5jZSBjb3VudGluZyBmb3IgQUNQ SQ0KPiBvcGVyYXRpb24gcmVnaW9uIGhhbmRsZXJzDQo+IA0KPiA+IEZyb206IFJhZmFlbCBKLiBX eXNvY2tpIFttYWlsdG86cmp3QHNpc2sucGxdDQo+ID4gU2VudDogRnJpZGF5LCBKdWx5IDI2LCAy MDEzIDU6MjkgQU0NCj4gPg0KPiA+IE9uIFR1ZXNkYXksIEp1bHkgMjMsIDIwMTMgMDQ6MDk6NDMg UE0gTHYgWmhlbmcgd3JvdGU6DQo+ID4gPiBUaGlzIHBhdGNoIGFkZHMgcmVmZXJlbmNlIGNvdXRp bmcgZm9yIEFDUEkgb3BlcmF0aW9uIHJlZ2lvbiBoYW5kbGVycw0KPiA+ID4gdG8gZml4IHJhY2Vz IGNhdXNlZCBieSB0aGUgQUNQSUNBIGFkZHJlc3Mgc3BhY2UgY2FsbGJhY2sgaW52b2NhdGlvbnMu DQo+ID4gPg0KPiA+ID4gQUNQSUNBIGFkZHJlc3Mgc3BhY2UgY2FsbGJhY2sgaW52b2NhdGlvbiBp cyBub3Qgc3VpdGFibGUgZm9yIExpbnV4DQo+ID4gPiBDT05GSUdfTU9EVUxFPXkgZXhlY3V0aW9u IGVudmlyb25tZW50Lg0KPiA+DQo+ID4gQWN0dWFsbHksIGNhbiB5b3UgcGxlYXNlIGV4cGxhaW4g dG8gbWUgd2hhdCAqZXhhY3RseSogdGhlIHByb2JsZW0gaXM/DQo+IA0KPiBPSy4gIEknbGwgYWRk IHJhY2UgZXhwbGFuYXRpb25zIGluIHRoZSBuZXh0IHJldmlzaW9uLg0KPiANCj4gVGhlIHByb2Js ZW0gaXMgdGhlcmUgaXMgbm8gImxvY2siIGhlbGQgaW5zaWRlIEFDUElDQSBmb3IgaW52b2tpbmcg b3BlcmF0aW9uDQo+IHJlZ2lvbiBoYW5kbGVycy4NCj4gVGh1cyByYWNlcyBoYXBwZW5zIGJldHdl ZW4gdGhlIGFjcGlfcmVtb3ZlL2luc3RhbGxfYWRkcmVzc19zcGFjZV9oYW5kbGVyDQo+IGFuZCB0 aGUgaGFuZGxlci9zZXR1cCBjYWxsYmFja3MuDQoNClRoaXMgc2VlbXMgbm90IGEgZ29vZCBleHBs YW5hdGlvbiBvZiB0aGUgaW50ZW50IG9mIHRoaXMgcGF0Y2guDQpJIHRoaW5rIHRoZSBpbnRlbnQg aXMgaGVyZSBpbiB0aGUgcGF0Y2ggZGVzY3JpcHRpb246DQoNCjEuIEl0IGFjdHMgYXMgYSBiYXJy aWVyIGZvciBvcGVyYXRpb24gcmVnaW9uIGNhbGxiYWNrcyAtIG5vIGNhbGxiYWNrIHdpbGwNCiAg IGhhcHBlbiBhZnRlciBhY3BpX3VucmVnaXN0ZXJfcmVnaW9uKCkuDQoyLiBhY3BpX3VucmVnaXN0 ZXJfcmVnaW9uKCkgaXMgc2FmZSB0byBiZSBjYWxsZWQgaW4gbW91ZGxlLT5leGl0KCkNCiAgIGZ1 bmN0aW9ucy4NCg0KSG1tLCBtYXliZSBJIG5lZWQgdG8gcmUtb3JkZXIgdGhlIHBhdGNoIGRlc2Ny aXB0aW9uIGZvciB0aGlzIHBhdGNoLg0KDQpUaGFua3MgZm9yIGNvbW1lbnRpbmcuDQoNCkJlc3Qg cmVnYXJkcw0KLUx2DQoNCj4gDQo+IFRoaXMgaXMgY29ycmVjdCBwZXIgQUNQSSBzcGVjaWZpY2F0 aW9uLg0KPiBBcyBpZiB0aGVyZSBpcyBpbnRlcnByZXRlciBsb2NrcyBoZWxkIGZvciBpbnZva2lu ZyBvcGVyYXRpb24gcmVnaW9uIGhhbmRsZXJzLCB0aGUNCj4gdGltZW91dCBpbXBsZW1lbnRlZCBp bnNpZGUgdGhlIG9wZXJhdGlvbiByZWdpb24gaGFuZGxlcnMgd2lsbCBtYWtlIGFsbCBsb2NraW5n DQo+IGZhY2lsaXRpZXMgKEFjcXVpcmUgb3IgU2xlZXAsLi4uKSB0aW1lZCBvdXQuDQo+IFBsZWFz ZSByZWZlciB0byBBQ1BJIHNwZWNpZmljYXRpb24gIjUuNS4yIENvbnRyb2wgTWV0aG9kIEV4ZWN1 dGlvbiI6DQo+IEludGVycHJldGF0aW9uIG9mIGEgQ29udHJvbCBNZXRob2QgaXMgbm90IHByZWVt cHRpdmUsIGJ1dCBpdCBjYW4gYmxvY2suIFdoZW4gYQ0KPiBjb250cm9sIG1ldGhvZCBkb2VzIGJs b2NrLCBPU1BNIGNhbiBpbml0aWF0ZSBvciBjb250aW51ZSB0aGUgZXhlY3V0aW9uIG9mIGENCj4g ZGlmZmVyZW50IGNvbnRyb2wgbWV0aG9kLiBBIGNvbnRyb2wgbWV0aG9kIGNhbiBvbmx5IGFzc3Vt ZSB0aGF0IGFjY2VzcyB0bw0KPiBnbG9iYWwgb2JqZWN0cyBpcyBleGNsdXNpdmUgZm9yIGFueSBw ZXJpb2QgdGhlIGNvbnRyb2wgbWV0aG9kIGRvZXMgbm90IGJsb2NrLg0KPiANCj4gU28gaXQgaXMg cHJldHR5IG11Y2ggbGlrZWx5IHRoYXQgQUNQSSBJTyB0cmFuc2ZlcnMgYXJlIGxvY2tlZCBpbnNp ZGUgdGhlIG9wZXJhdGlvbg0KPiByZWdpb24gY2FsbGJhY2sgaW1wbGVtZW50YXRpb25zLg0KPiBV c2luZyBsb2NraW5nIGZhY2lsaXR5IHRvIHByb3RlY3QgdGhlIGNhbGxiYWNrIGludm9jYXRpb24g d2lsbCByaXNrIGRlYWQgbG9ja3MuDQo+IA0KPiBUaGFua3MNCj4gLUx2DQo+IA0KPiA+IFJhZmFl bA0K -- To unsubscribe from this list: send the line "unsubscribe linux-acpi" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Friday, July 26, 2013 12:47:44 AM Zheng, Lv wrote: > > > From: Rafael J. Wysocki [mailto:rjw@sisk.pl] > > Sent: Friday, July 26, 2013 4:27 AM > > > > On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > > > This patch adds reference couting for ACPI operation region handlers > > > to fix races caused by the ACPICA address space callback invocations. > > > > > > ACPICA address space callback invocation is not suitable for Linux > > > CONFIG_MODULE=y execution environment. This patch tries to protect > > > the address space callbacks by invoking them under a module safe > > environment. > > > The IPMI address space handler is also upgraded in this patch. > > > The acpi_unregister_region() is designed to meet the following > > > requirements: > > > 1. It acts as a barrier for operation region callbacks - no callback will > > > happen after acpi_unregister_region(). > > > 2. acpi_unregister_region() is safe to be called in moudle->exit() > > > functions. > > > Using reference counting rather than module referencing allows such > > > benefits to be achieved even when acpi_unregister_region() is called > > > in the environments other than module->exit(). > > > The header file of include/acpi/acpi_bus.h should contain the > > > declarations that have references to some ACPICA defined types. > > > > > > Signed-off-by: Lv Zheng <lv.zheng@intel.com> > > > Reviewed-by: Huang Ying <ying.huang@intel.com> > > > --- > > > drivers/acpi/acpi_ipmi.c | 16 ++-- > > > drivers/acpi/osl.c | 224 > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > include/acpi/acpi_bus.h | 5 ++ > > > 3 files changed, 235 insertions(+), 10 deletions(-) > > > > > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index > > > 5f8f495..2a09156 100644 > > > --- a/drivers/acpi/acpi_ipmi.c > > > +++ b/drivers/acpi/acpi_ipmi.c > > > @@ -539,20 +539,18 @@ out_ref: > > > static int __init acpi_ipmi_init(void) { > > > int result = 0; > > > - acpi_status status; > > > > > > if (acpi_disabled) > > > return result; > > > > > > mutex_init(&driver_data.ipmi_lock); > > > > > > - status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, > > > - ACPI_ADR_SPACE_IPMI, > > > - &acpi_ipmi_space_handler, > > > - NULL, NULL); > > > - if (ACPI_FAILURE(status)) { > > > + result = acpi_register_region(ACPI_ADR_SPACE_IPMI, > > > + &acpi_ipmi_space_handler, > > > + NULL, NULL); > > > + if (result) { > > > pr_warn("Can't register IPMI opregion space handle\n"); > > > - return -EINVAL; > > > + return result; > > > } > > > > > > result = ipmi_smi_watcher_register(&driver_data.bmc_events); > > > @@ -596,9 +594,7 @@ static void __exit acpi_ipmi_exit(void) > > > } > > > mutex_unlock(&driver_data.ipmi_lock); > > > > > > - acpi_remove_address_space_handler(ACPI_ROOT_OBJECT, > > > - ACPI_ADR_SPACE_IPMI, > > > - &acpi_ipmi_space_handler); > > > + acpi_unregister_region(ACPI_ADR_SPACE_IPMI); > > > } > > > > > > module_init(acpi_ipmi_init); > > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index > > > 6ab2c35..8398e51 100644 > > > --- a/drivers/acpi/osl.c > > > +++ b/drivers/acpi/osl.c > > > @@ -86,6 +86,42 @@ static struct workqueue_struct *kacpid_wq; static > > > struct workqueue_struct *kacpi_notify_wq; static struct > > > workqueue_struct *kacpi_hotplug_wq; > > > > > > +struct acpi_region { > > > + unsigned long flags; > > > +#define ACPI_REGION_DEFAULT 0x01 > > > +#define ACPI_REGION_INSTALLED 0x02 > > > +#define ACPI_REGION_REGISTERED 0x04 > > > +#define ACPI_REGION_UNREGISTERING 0x08 > > > +#define ACPI_REGION_INSTALLING 0x10 > > > > What about (1UL << 1), (1UL << 2) etc.? > > > > Also please remove the #defines out of the struct definition. > > OK. > > > > > > + /* > > > + * NOTE: Upgrading All Region Handlers > > > + * This flag is only used during the period where not all of the > > > + * region handers are upgraded to the new interfaces. > > > + */ > > > +#define ACPI_REGION_MANAGED 0x80 > > > + acpi_adr_space_handler handler; > > > + acpi_adr_space_setup setup; > > > + void *context; > > > + /* Invoking references */ > > > + atomic_t refcnt; > > > > Actually, why don't you use krefs? > > If you take a look at other piece of my codes, you'll find there are two reasons: > > 1. I'm using while (atomic_read() > 1) to implement the objects' flushing and there is no kref API to do so. No, there's not any, but you can read kref.refcount directly, can't you? Moreover, it is not entirely clear to me that doing the while (atomic_read() > 1) is actually correct. > I just think it is not suitable for me to introduce such an API into kref.h and start another argument around kref designs in this bug fix patch. :-) > I'll start a discussion about kref design using another thread. You don't need to do that at all. > 2. I'm using ipmi_dev|msg_release() as a pair of ipmi_dev|msg_alloc(), it's kind of atomic_t coding style. > If atomic_t is changed to struct kref, I will need to implement two API, __ipmi_dev_release() to take a struct kref as parameter and call ipmi_dev_release inside it. > By not using kref, I needn't write codes to implement such API. I'm not following you, sorry. Please just use krefs for reference counting, the same way as you use struct list_head for implementing lists. This is the way everyone does that in the kernel and that's for a reason. Unless you do your reference counting under a lock, in which case using atomic_t isn't necessary at all and you can use a non-atomic counter. > > > +}; > > > + > > > +static struct acpi_region acpi_regions[ACPI_NUM_PREDEFINED_REGIONS] > > = { > > > + [ACPI_ADR_SPACE_SYSTEM_MEMORY] = { > > > + .flags = ACPI_REGION_DEFAULT, > > > + }, > > > + [ACPI_ADR_SPACE_SYSTEM_IO] = { > > > + .flags = ACPI_REGION_DEFAULT, > > > + }, > > > + [ACPI_ADR_SPACE_PCI_CONFIG] = { > > > + .flags = ACPI_REGION_DEFAULT, > > > + }, > > > + [ACPI_ADR_SPACE_IPMI] = { > > > + .flags = ACPI_REGION_MANAGED, > > > + }, > > > +}; > > > +static DEFINE_MUTEX(acpi_mutex_region); > > > + > > > /* > > > * This list of permanent mappings is for memory that may be accessed > > from > > > * interrupt context, where we can't do the ioremap(). > > > @@ -1799,3 +1835,191 @@ void alloc_acpi_hp_work(acpi_handle handle, > > u32 type, void *context, > > > kfree(hp_work); > > > } > > > EXPORT_SYMBOL_GPL(alloc_acpi_hp_work); > > > + > > > +static bool acpi_region_managed(struct acpi_region *rgn) { > > > + /* > > > + * NOTE: Default and Managed > > > + * We only need to avoid region management on the regions managed > > > + * by ACPICA (ACPI_REGION_DEFAULT). Currently, we need additional > > > + * check as many operation region handlers are not upgraded, so > > > + * only those known to be safe are managed (ACPI_REGION_MANAGED). > > > + */ > > > + return !(rgn->flags & ACPI_REGION_DEFAULT) && > > > + (rgn->flags & ACPI_REGION_MANAGED); } > > > + > > > +static bool acpi_region_callable(struct acpi_region *rgn) { > > > + return (rgn->flags & ACPI_REGION_REGISTERED) && > > > + !(rgn->flags & ACPI_REGION_UNREGISTERING); } > > > + > > > +static acpi_status > > > +acpi_region_default_handler(u32 function, > > > + acpi_physical_address address, > > > + u32 bit_width, u64 *value, > > > + void *handler_context, void *region_context) { > > > + acpi_adr_space_handler handler; > > > + struct acpi_region *rgn = (struct acpi_region *)handler_context; > > > + void *context; > > > + acpi_status status = AE_NOT_EXIST; > > > + > > > + mutex_lock(&acpi_mutex_region); > > > + if (!acpi_region_callable(rgn) || !rgn->handler) { > > > + mutex_unlock(&acpi_mutex_region); > > > + return status; > > > + } > > > + > > > + atomic_inc(&rgn->refcnt); > > > + handler = rgn->handler; > > > + context = rgn->context; > > > + mutex_unlock(&acpi_mutex_region); > > > + > > > + status = handler(function, address, bit_width, value, context, > > > + region_context); > > > > Why don't we call the handler under the mutex? > > > > What exactly prevents context from becoming NULL before the call above? > > It's a kind of programming style related concern. > IMO, using locks around callback function is a buggy programming style that could lead to dead locks. > Let me explain this using an example. > > Object A exports a register/unregister API for other objects. > Object B calls A's register/unregister API to register/unregister B's callback. > It's likely that object B will hold lock_of_B around unregister/register when object B is destroyed/created, the lock_of_B is likely also used inside the callback. Why is it likely to be used inside the callback? Clearly, if a callback is executed under a lock, that lock can't be acquired by that callback. > So when object A holds the lock_of_A around the callback invocation, it leads to dead lock since: > 1. the locking order for the register/unregister side will be: lock(lock_of_B), lock(lock_of_A) > 2. the locking order for the callback side will be: lock(lock_of_A), lock(lock_of_B) > They are in the reversed order! > > IMO, Linux may need to introduce __callback, __api as decelerators for the functions, and use sparse to enforce this rule, sparse knows if a callback is invoked under some locks. Oh, dear. Yes, sparse knows such things, and so what? > In the case of ACPICA space_handlers, as you may know, when an ACPI operation region handler is invoked, there will be no lock held inside ACPICA (interpreter lock must be freed before executing operation region handlers). > So the likelihood of the dead lock is pretty much high here! Sorry, what are you talking about? Please let me rephrase my question: What *practical* problems would it lead to if we executed this particular callback under this particular mutex? Not *theoretical* in the general theory of everything, *practical* in this particular piece of code. And we are talking about a *global* mutex here, not something object-specific. > > > + atomic_dec(&rgn->refcnt); > > > + > > > + return status; > > > +} > > > + > > > +static acpi_status > > > +acpi_region_default_setup(acpi_handle handle, u32 function, > > > + void *handler_context, void **region_context) { > > > + acpi_adr_space_setup setup; > > > + struct acpi_region *rgn = (struct acpi_region *)handler_context; > > > + void *context; > > > + acpi_status status = AE_OK; > > > + > > > + mutex_lock(&acpi_mutex_region); > > > + if (!acpi_region_callable(rgn) || !rgn->setup) { > > > + mutex_unlock(&acpi_mutex_region); > > > + return status; > > > + } > > > + > > > + atomic_inc(&rgn->refcnt); > > > + setup = rgn->setup; > > > + context = rgn->context; > > > + mutex_unlock(&acpi_mutex_region); > > > + > > > + status = setup(handle, function, context, region_context); > > > > Can setup drop rgn->refcnt ? > > The reason is same as the handler, as a setup is also a callback. Let me rephrase: Is it legitimate for setup to modify rgn->refcnt? If so, then why? > > > > > + atomic_dec(&rgn->refcnt); > > > + > > > + return status; > > > +} > > > + > > > +static int __acpi_install_region(struct acpi_region *rgn, > > > + acpi_adr_space_type space_id) > > > +{ > > > + int res = 0; > > > + acpi_status status; > > > + int installing = 0; > > > + > > > + mutex_lock(&acpi_mutex_region); > > > + if (rgn->flags & ACPI_REGION_INSTALLED) > > > + goto out_lock; > > > + if (rgn->flags & ACPI_REGION_INSTALLING) { > > > + res = -EBUSY; > > > + goto out_lock; > > > + } > > > + > > > + installing = 1; > > > + rgn->flags |= ACPI_REGION_INSTALLING; > > > + status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, > > space_id, > > > + acpi_region_default_handler, > > > + acpi_region_default_setup, > > > + rgn); > > > + rgn->flags &= ~ACPI_REGION_INSTALLING; > > > + if (ACPI_FAILURE(status)) > > > + res = -EINVAL; > > > + else > > > + rgn->flags |= ACPI_REGION_INSTALLED; > > > + > > > +out_lock: > > > + mutex_unlock(&acpi_mutex_region); > > > + if (installing) { > > > + if (res) > > > + pr_err("Failed to install region %d\n", space_id); > > > + else > > > + pr_info("Region %d installed\n", space_id); > > > + } > > > + return res; > > > +} > > > + > > > +int acpi_register_region(acpi_adr_space_type space_id, > > > + acpi_adr_space_handler handler, > > > + acpi_adr_space_setup setup, void *context) { > > > + int res; > > > + struct acpi_region *rgn; > > > + > > > + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) > > > + return -EINVAL; > > > + > > > + rgn = &acpi_regions[space_id]; > > > + if (!acpi_region_managed(rgn)) > > > + return -EINVAL; > > > + > > > + res = __acpi_install_region(rgn, space_id); > > > + if (res) > > > + return res; > > > + > > > + mutex_lock(&acpi_mutex_region); > > > + if (rgn->flags & ACPI_REGION_REGISTERED) { > > > + mutex_unlock(&acpi_mutex_region); > > > + return -EBUSY; > > > + } > > > + > > > + rgn->handler = handler; > > > + rgn->setup = setup; > > > + rgn->context = context; > > > + rgn->flags |= ACPI_REGION_REGISTERED; > > > + atomic_set(&rgn->refcnt, 1); > > > + mutex_unlock(&acpi_mutex_region); > > > + > > > + pr_info("Region %d registered\n", space_id); > > > + > > > + return 0; > > > +} > > > +EXPORT_SYMBOL_GPL(acpi_register_region); > > > + > > > +void acpi_unregister_region(acpi_adr_space_type space_id) { > > > + struct acpi_region *rgn; > > > + > > > + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) > > > + return; > > > + > > > + rgn = &acpi_regions[space_id]; > > > + if (!acpi_region_managed(rgn)) > > > + return; > > > + > > > + mutex_lock(&acpi_mutex_region); > > > + if (!(rgn->flags & ACPI_REGION_REGISTERED)) { > > > + mutex_unlock(&acpi_mutex_region); > > > + return; > > > + } > > > + if (rgn->flags & ACPI_REGION_UNREGISTERING) { > > > + mutex_unlock(&acpi_mutex_region); > > > + return; > > > > What about > > > > if ((rgn->flags & ACPI_REGION_UNREGISTERING) > > || !(rgn->flags & ACPI_REGION_REGISTERED)) { > > mutex_unlock(&acpi_mutex_region); > > return; > > } > > > > OK. > > > > + } > > > + > > > + rgn->flags |= ACPI_REGION_UNREGISTERING; > > > + rgn->handler = NULL; > > > + rgn->setup = NULL; > > > + rgn->context = NULL; > > > + mutex_unlock(&acpi_mutex_region); > > > + > > > + while (atomic_read(&rgn->refcnt) > 1) > > > + schedule_timeout_uninterruptible(usecs_to_jiffies(5)); > > > > Wouldn't it be better to use a wait queue here? > > Yes, I'll try. By the way, we do we need to do that? > > > + atomic_dec(&rgn->refcnt); > > > + > > > + mutex_lock(&acpi_mutex_region); > > > + rgn->flags &= ~(ACPI_REGION_REGISTERED | > > ACPI_REGION_UNREGISTERING); > > > + mutex_unlock(&acpi_mutex_region); > > > + > > > + pr_info("Region %d unregistered\n", space_id); } > > > +EXPORT_SYMBOL_GPL(acpi_unregister_region); > > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index > > > a2c2fbb..15fad0d 100644 > > > --- a/include/acpi/acpi_bus.h > > > +++ b/include/acpi/acpi_bus.h > > > @@ -542,4 +542,9 @@ static inline int unregister_acpi_bus_type(void > > > *bus) { return 0; } > > > > > > #endif /* CONFIG_ACPI */ > > > > > > +int acpi_register_region(acpi_adr_space_type space_id, > > > + acpi_adr_space_handler handler, > > > + acpi_adr_space_setup setup, void *context); void > > > +acpi_unregister_region(acpi_adr_space_type space_id); > > > + > > > #endif /*__ACPI_BUS_H__*/ Thanks, Rafael
On Friday, July 26, 2013 01:54:00 AM Zheng, Lv wrote: > > From: Rafael J. Wysocki [mailto:rjw@sisk.pl] > > Sent: Friday, July 26, 2013 5:29 AM > > > > On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > > > This patch adds reference couting for ACPI operation region handlers to fix > > > races caused by the ACPICA address space callback invocations. > > > > > > ACPICA address space callback invocation is not suitable for Linux > > > CONFIG_MODULE=y execution environment. > > > > Actually, can you please explain to me what *exactly* the problem is? > > OK. I'll add race explanations in the next revision. > > The problem is there is no "lock" held inside ACPICA for invoking operation > region handlers. > Thus races happens between the acpi_remove/install_address_space_handler and > the handler/setup callbacks. I see. Now you're trying to introduce something that would prevent those races from happening, right? > This is correct per ACPI specification. > As if there is interpreter locks held for invoking operation region handlers, > the timeout implemented inside the operation region handlers will make all > locking facilities (Acquire or Sleep,...) timed out. > Please refer to ACPI specification "5.5.2 Control Method Execution": > Interpretation of a Control Method is not preemptive, but it can block. When > a control method does block, OSPM can initiate or continue the execution of > a different control method. A control method can only assume that access to > global objects is exclusive for any period the control method does not block. > > So it is pretty much likely that ACPI IO transfers are locked inside the > operation region callback implementations. > Using locking facility to protect the callback invocation will risk dead locks. No. If you use a single global lock around all invocations of operation region handlers, it won't deadlock, but it will *serialize* things. This means that there won't be two handlers executing in parallel. That may or may not be bad depending on what those handlers actually do. Your concern seems to be that if one address space handler is buggy and it blocks indefinitely, executing it under such a lock would affect the other address space handlers and in my opinion this is a valid concern. So the idea seems to be to add wrappers around acpi_install_address_space_handler() and acpi_remove_address_space_handler (but I don't see where the latter is called after the change?), such that they will know when it is safe to unregister the handler. That is simple enough. However, I'm not sure it is needed in the context of IPMI. Your address space handler's context is NULL, so even it if is executed after acpi_remove_address_space_handler() has been called for it (or in parallel), it doesn't depend on anything passed by the caller, so I don't see why the issue can't be addressed by a proper synchronization between acpi_ipmi_exit() and acpi_ipmi_space_handler(). Clearly, acpi_ipmi_exit() should wait for all already running instances of acpi_ipmi_space_handler() to complete and all acpi_ipmi_space_handler() instances started after acpi_ipmi_exit() has been called must return immediately. I would imagine an algorithm like this: acpi_ipmi_exit() 1. Take "address space handler lock". 2. Set "unregistering address space handler" flag. 3. Check if "count of currently running handlers" is 0. If so, call acpi_remove_address_space_handler(), drop the lock (possibly clear the flag) and return. 4. Otherwise drop the lock and go to sleep in "address space handler wait queue". 5. When woken up, take "address space handler lock" and go to 3. acpi_ipmi_space_handler() 1. Take "address space handler lock". 2. Check "unregistering address space handler" flag. If set, drop the lock and return. 3. Increment "count of currently running handlers". 4. Drop the lock. 5. Do your work. 6. Take "address space handler lock". 7. Decrement "count of currently running handlers" and if 0, signal the tasks waiting on it to wake up. 8. Drop the lock. Thanks, Rafael
> On Friday, July 26, 2013 10:01 PM Rafael J. Wysocki wrote: > > On Friday, July 26, 2013 12:47:44 AM Zheng, Lv wrote: > > > > > On Friday, July 26, 2013 4:27 AM Rafael J. Wysocki wrote: > > > > > > On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > > > > This patch adds reference couting for ACPI operation region handlers > > > > to fix races caused by the ACPICA address space callback invocations. > > > > > > > > ACPICA address space callback invocation is not suitable for Linux > > > > CONFIG_MODULE=y execution environment. This patch tries to protect > > > > the address space callbacks by invoking them under a module safe > > > environment. > > > > The IPMI address space handler is also upgraded in this patch. > > > > The acpi_unregister_region() is designed to meet the following > > > > requirements: > > > > 1. It acts as a barrier for operation region callbacks - no callback will > > > > happen after acpi_unregister_region(). > > > > 2. acpi_unregister_region() is safe to be called in moudle->exit() > > > > functions. > > > > Using reference counting rather than module referencing allows such > > > > benefits to be achieved even when acpi_unregister_region() is called > > > > in the environments other than module->exit(). > > > > The header file of include/acpi/acpi_bus.h should contain the > > > > declarations that have references to some ACPICA defined types. > > > > > > > > Signed-off-by: Lv Zheng <lv.zheng@intel.com> > > > > Reviewed-by: Huang Ying <ying.huang@intel.com> > > > > --- > > > > drivers/acpi/acpi_ipmi.c | 16 ++-- > > > > drivers/acpi/osl.c | 224 > > > ++++++++++++++++++++++++++++++++++++++++++++++ > > > > include/acpi/acpi_bus.h | 5 ++ > > > > 3 files changed, 235 insertions(+), 10 deletions(-) > > > > > > > > diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index > > > > 5f8f495..2a09156 100644 > > > > --- a/drivers/acpi/acpi_ipmi.c > > > > +++ b/drivers/acpi/acpi_ipmi.c > > > > @@ -539,20 +539,18 @@ out_ref: > > > > static int __init acpi_ipmi_init(void) { > > > > int result = 0; > > > > - acpi_status status; > > > > > > > > if (acpi_disabled) > > > > return result; > > > > > > > > mutex_init(&driver_data.ipmi_lock); > > > > > > > > - status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, > > > > - ACPI_ADR_SPACE_IPMI, > > > > - &acpi_ipmi_space_handler, > > > > - NULL, NULL); > > > > - if (ACPI_FAILURE(status)) { > > > > + result = acpi_register_region(ACPI_ADR_SPACE_IPMI, > > > > + &acpi_ipmi_space_handler, > > > > + NULL, NULL); > > > > + if (result) { > > > > pr_warn("Can't register IPMI opregion space handle\n"); > > > > - return -EINVAL; > > > > + return result; > > > > } > > > > > > > > result = ipmi_smi_watcher_register(&driver_data.bmc_events); > > > > @@ -596,9 +594,7 @@ static void __exit acpi_ipmi_exit(void) > > > > } > > > > mutex_unlock(&driver_data.ipmi_lock); > > > > > > > > - acpi_remove_address_space_handler(ACPI_ROOT_OBJECT, > > > > - ACPI_ADR_SPACE_IPMI, > > > > - &acpi_ipmi_space_handler); > > > > + acpi_unregister_region(ACPI_ADR_SPACE_IPMI); > > > > } > > > > > > > > module_init(acpi_ipmi_init); > > > > diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index > > > > 6ab2c35..8398e51 100644 > > > > --- a/drivers/acpi/osl.c > > > > +++ b/drivers/acpi/osl.c > > > > @@ -86,6 +86,42 @@ static struct workqueue_struct *kacpid_wq; > static > > > > struct workqueue_struct *kacpi_notify_wq; static struct > > > > workqueue_struct *kacpi_hotplug_wq; > > > > > > > > +struct acpi_region { > > > > + unsigned long flags; > > > > +#define ACPI_REGION_DEFAULT 0x01 > > > > +#define ACPI_REGION_INSTALLED 0x02 > > > > +#define ACPI_REGION_REGISTERED 0x04 > > > > +#define ACPI_REGION_UNREGISTERING 0x08 > > > > +#define ACPI_REGION_INSTALLING 0x10 > > > > > > What about (1UL << 1), (1UL << 2) etc.? > > > > > > Also please remove the #defines out of the struct definition. > > > > OK. > > > > > > > > > + /* > > > > + * NOTE: Upgrading All Region Handlers > > > > + * This flag is only used during the period where not all of the > > > > + * region handers are upgraded to the new interfaces. > > > > + */ > > > > +#define ACPI_REGION_MANAGED 0x80 > > > > + acpi_adr_space_handler handler; > > > > + acpi_adr_space_setup setup; > > > > + void *context; > > > > + /* Invoking references */ > > > > + atomic_t refcnt; > > > > > > Actually, why don't you use krefs? > > > > If you take a look at other piece of my codes, you'll find there are two > reasons: > > > > 1. I'm using while (atomic_read() > 1) to implement the objects' flushing and > there is no kref API to do so. > > No, there's not any, but you can read kref.refcount directly, can't you? > > Moreover, it is not entirely clear to me that doing the while (atomic_read() > 1) > is actually correct. > > > I just think it is not suitable for me to introduce such an API into kref.h and > start another argument around kref designs in this bug fix patch. :-) > > I'll start a discussion about kref design using another thread. > > You don't need to do that at all. > > > 2. I'm using ipmi_dev|msg_release() as a pair of ipmi_dev|msg_alloc(), it's > kind of atomic_t coding style. > > If atomic_t is changed to struct kref, I will need to implement two API, > __ipmi_dev_release() to take a struct kref as parameter and call > ipmi_dev_release inside it. > > By not using kref, I needn't write codes to implement such API. > > I'm not following you, sorry. > > Please just use krefs for reference counting, the same way as you use > struct list_head for implementing lists. This is the way everyone does > that in the kernel and that's for a reason. > > Unless you do your reference counting under a lock, in which case using > atomic_t isn't necessary at all and you can use a non-atomic counter. I'll follow your suggestion of kref. You can find my concern 2 related stuff in the next revision. It's trivial. > > > > > +}; > > > > + > > > > +static struct acpi_region > acpi_regions[ACPI_NUM_PREDEFINED_REGIONS] > > > = { > > > > + [ACPI_ADR_SPACE_SYSTEM_MEMORY] = { > > > > + .flags = ACPI_REGION_DEFAULT, > > > > + }, > > > > + [ACPI_ADR_SPACE_SYSTEM_IO] = { > > > > + .flags = ACPI_REGION_DEFAULT, > > > > + }, > > > > + [ACPI_ADR_SPACE_PCI_CONFIG] = { > > > > + .flags = ACPI_REGION_DEFAULT, > > > > + }, > > > > + [ACPI_ADR_SPACE_IPMI] = { > > > > + .flags = ACPI_REGION_MANAGED, > > > > + }, > > > > +}; > > > > +static DEFINE_MUTEX(acpi_mutex_region); > > > > + > > > > /* > > > > * This list of permanent mappings is for memory that may be accessed > > > from > > > > * interrupt context, where we can't do the ioremap(). > > > > @@ -1799,3 +1835,191 @@ void alloc_acpi_hp_work(acpi_handle > handle, > > > u32 type, void *context, > > > > kfree(hp_work); > > > > } > > > > EXPORT_SYMBOL_GPL(alloc_acpi_hp_work); > > > > + > > > > +static bool acpi_region_managed(struct acpi_region *rgn) { > > > > + /* > > > > + * NOTE: Default and Managed > > > > + * We only need to avoid region management on the regions > managed > > > > + * by ACPICA (ACPI_REGION_DEFAULT). Currently, we need > additional > > > > + * check as many operation region handlers are not upgraded, so > > > > + * only those known to be safe are managed > (ACPI_REGION_MANAGED). > > > > + */ > > > > + return !(rgn->flags & ACPI_REGION_DEFAULT) && > > > > + (rgn->flags & ACPI_REGION_MANAGED); } > > > > + > > > > +static bool acpi_region_callable(struct acpi_region *rgn) { > > > > + return (rgn->flags & ACPI_REGION_REGISTERED) && > > > > + !(rgn->flags & ACPI_REGION_UNREGISTERING); } > > > > + > > > > +static acpi_status > > > > +acpi_region_default_handler(u32 function, > > > > + acpi_physical_address address, > > > > + u32 bit_width, u64 *value, > > > > + void *handler_context, void *region_context) { > > > > + acpi_adr_space_handler handler; > > > > + struct acpi_region *rgn = (struct acpi_region *)handler_context; > > > > + void *context; > > > > + acpi_status status = AE_NOT_EXIST; > > > > + > > > > + mutex_lock(&acpi_mutex_region); > > > > + if (!acpi_region_callable(rgn) || !rgn->handler) { > > > > + mutex_unlock(&acpi_mutex_region); > > > > + return status; > > > > + } > > > > + > > > > + atomic_inc(&rgn->refcnt); > > > > + handler = rgn->handler; > > > > + context = rgn->context; > > > > + mutex_unlock(&acpi_mutex_region); > > > > + > > > > + status = handler(function, address, bit_width, value, context, > > > > + region_context); > > > > > > Why don't we call the handler under the mutex? > > > > > > What exactly prevents context from becoming NULL before the call above? > > > > It's a kind of programming style related concern. > > IMO, using locks around callback function is a buggy programming style that > could lead to dead locks. > > Let me explain this using an example. > > > > Object A exports a register/unregister API for other objects. > > Object B calls A's register/unregister API to register/unregister B's callback. > > It's likely that object B will hold lock_of_B around unregister/register when > object B is destroyed/created, the lock_of_B is likely also used inside the > callback. > > Why is it likely to be used inside the callback? Clearly, if a callback is > executed under a lock, that lock can't be acquired by that callback. I think this is not related to the real purpose of why we must not hold a lock in this situation. So let's ignore this paragraph. > > > So when object A holds the lock_of_A around the callback invocation, it leads > to dead lock since: > > 1. the locking order for the register/unregister side will be: lock(lock_of_B), > lock(lock_of_A) > > 2. the locking order for the callback side will be: lock(lock_of_A), > lock(lock_of_B) > > They are in the reversed order! > > > > IMO, Linux may need to introduce __callback, __api as decelerators for the > functions, and use sparse to enforce this rule, sparse knows if a callback is > invoked under some locks. > > Oh, dear. Yes, sparse knows such things, and so what? I was thinking sparse can give us warnings on __api marked function invocation where __acquire count is not 0, this might be mandatory for high quality codes. And sparse can also give us warnings on __callback marked function invocations where __acquire count is not 0, this should be optional. But since it is not related to our topic, let's ignore this paragraph. > > > In the case of ACPICA space_handlers, as you may know, when an ACPI > operation region handler is invoked, there will be no lock held inside ACPICA > (interpreter lock must be freed before executing operation region handlers). > > So the likelihood of the dead lock is pretty much high here! > > Sorry, what are you talking about? > > Please let me rephrase my question: What *practical* problems would it lead > to > if we executed this particular callback under this particular mutex? > > Not *theoretical* in the general theory of everything, *practical* in this > particular piece of code. > > And we are talking about a *global* mutex here, not something object-specific. I think you have additional replies on this in another email. Let me reply you there. > > > > > + atomic_dec(&rgn->refcnt); > > > > + > > > > + return status; > > > > +} > > > > + > > > > +static acpi_status > > > > +acpi_region_default_setup(acpi_handle handle, u32 function, > > > > + void *handler_context, void **region_context) { > > > > + acpi_adr_space_setup setup; > > > > + struct acpi_region *rgn = (struct acpi_region *)handler_context; > > > > + void *context; > > > > + acpi_status status = AE_OK; > > > > + > > > > + mutex_lock(&acpi_mutex_region); > > > > + if (!acpi_region_callable(rgn) || !rgn->setup) { > > > > + mutex_unlock(&acpi_mutex_region); > > > > + return status; > > > > + } > > > > + > > > > + atomic_inc(&rgn->refcnt); > > > > + setup = rgn->setup; > > > > + context = rgn->context; > > > > + mutex_unlock(&acpi_mutex_region); > > > > + > > > > + status = setup(handle, function, context, region_context); > > > > > > Can setup drop rgn->refcnt ? > > > > The reason is same as the handler, as a setup is also a callback. > > Let me rephrase: Is it legitimate for setup to modify rgn->refcnt? > If so, then why? Yes, the race is same as the handler. When ACPICA is accessing the text segment of the setup function implementation, the module owns the setup function can also be unloaded as there is no lock hold before invoking setup - note that ExitInter also happens to setup invocations. > > > > > > > > + atomic_dec(&rgn->refcnt); > > > > + > > > > + return status; > > > > +} > > > > + > > > > +static int __acpi_install_region(struct acpi_region *rgn, > > > > + acpi_adr_space_type space_id) > > > > +{ > > > > + int res = 0; > > > > + acpi_status status; > > > > + int installing = 0; > > > > + > > > > + mutex_lock(&acpi_mutex_region); > > > > + if (rgn->flags & ACPI_REGION_INSTALLED) > > > > + goto out_lock; > > > > + if (rgn->flags & ACPI_REGION_INSTALLING) { > > > > + res = -EBUSY; > > > > + goto out_lock; > > > > + } > > > > + > > > > + installing = 1; > > > > + rgn->flags |= ACPI_REGION_INSTALLING; > > > > + status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, > > > space_id, > > > > + acpi_region_default_handler, > > > > + acpi_region_default_setup, > > > > + rgn); > > > > + rgn->flags &= ~ACPI_REGION_INSTALLING; > > > > + if (ACPI_FAILURE(status)) > > > > + res = -EINVAL; > > > > + else > > > > + rgn->flags |= ACPI_REGION_INSTALLED; > > > > + > > > > +out_lock: > > > > + mutex_unlock(&acpi_mutex_region); > > > > + if (installing) { > > > > + if (res) > > > > + pr_err("Failed to install region %d\n", space_id); > > > > + else > > > > + pr_info("Region %d installed\n", space_id); > > > > + } > > > > + return res; > > > > +} > > > > + > > > > +int acpi_register_region(acpi_adr_space_type space_id, > > > > + acpi_adr_space_handler handler, > > > > + acpi_adr_space_setup setup, void *context) { > > > > + int res; > > > > + struct acpi_region *rgn; > > > > + > > > > + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) > > > > + return -EINVAL; > > > > + > > > > + rgn = &acpi_regions[space_id]; > > > > + if (!acpi_region_managed(rgn)) > > > > + return -EINVAL; > > > > + > > > > + res = __acpi_install_region(rgn, space_id); > > > > + if (res) > > > > + return res; > > > > + > > > > + mutex_lock(&acpi_mutex_region); > > > > + if (rgn->flags & ACPI_REGION_REGISTERED) { > > > > + mutex_unlock(&acpi_mutex_region); > > > > + return -EBUSY; > > > > + } > > > > + > > > > + rgn->handler = handler; > > > > + rgn->setup = setup; > > > > + rgn->context = context; > > > > + rgn->flags |= ACPI_REGION_REGISTERED; > > > > + atomic_set(&rgn->refcnt, 1); > > > > + mutex_unlock(&acpi_mutex_region); > > > > + > > > > + pr_info("Region %d registered\n", space_id); > > > > + > > > > + return 0; > > > > +} > > > > +EXPORT_SYMBOL_GPL(acpi_register_region); > > > > + > > > > +void acpi_unregister_region(acpi_adr_space_type space_id) { > > > > + struct acpi_region *rgn; > > > > + > > > > + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) > > > > + return; > > > > + > > > > + rgn = &acpi_regions[space_id]; > > > > + if (!acpi_region_managed(rgn)) > > > > + return; > > > > + > > > > + mutex_lock(&acpi_mutex_region); > > > > + if (!(rgn->flags & ACPI_REGION_REGISTERED)) { > > > > + mutex_unlock(&acpi_mutex_region); > > > > + return; > > > > + } > > > > + if (rgn->flags & ACPI_REGION_UNREGISTERING) { > > > > + mutex_unlock(&acpi_mutex_region); > > > > + return; > > > > > > What about > > > > > > if ((rgn->flags & ACPI_REGION_UNREGISTERING) > > > || !(rgn->flags & ACPI_REGION_REGISTERED)) { > > > mutex_unlock(&acpi_mutex_region); > > > return; > > > } > > > > > > > OK. > > > > > > + } > > > > + > > > > + rgn->flags |= ACPI_REGION_UNREGISTERING; > > > > + rgn->handler = NULL; > > > > + rgn->setup = NULL; > > > > + rgn->context = NULL; > > > > + mutex_unlock(&acpi_mutex_region); > > > > + > > > > + while (atomic_read(&rgn->refcnt) > 1) > > > > + schedule_timeout_uninterruptible(usecs_to_jiffies(5)); > > > > > > Wouldn't it be better to use a wait queue here? > > > > Yes, I'll try. > > By the way, we do we need to do that? I think you have additional replies on this in another email. Let me reply you there. Thanks for commenting. Best regards -Lv > > > > > + atomic_dec(&rgn->refcnt); > > > > + > > > > + mutex_lock(&acpi_mutex_region); > > > > + rgn->flags &= ~(ACPI_REGION_REGISTERED | > > > ACPI_REGION_UNREGISTERING); > > > > + mutex_unlock(&acpi_mutex_region); > > > > + > > > > + pr_info("Region %d unregistered\n", space_id); } > > > > +EXPORT_SYMBOL_GPL(acpi_unregister_region); > > > > diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index > > > > a2c2fbb..15fad0d 100644 > > > > --- a/include/acpi/acpi_bus.h > > > > +++ b/include/acpi/acpi_bus.h > > > > @@ -542,4 +542,9 @@ static inline int unregister_acpi_bus_type(void > > > > *bus) { return 0; } > > > > > > > > #endif /* CONFIG_ACPI */ > > > > > > > > +int acpi_register_region(acpi_adr_space_type space_id, > > > > + acpi_adr_space_handler handler, > > > > + acpi_adr_space_setup setup, void *context); void > > > > +acpi_unregister_region(acpi_adr_space_type space_id); > > > > + > > > > #endif /*__ACPI_BUS_H__*/ > > Thanks, > Rafael > > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center.
> On Friday, July 26, 2013 10:49 PM Rafael J. Wysocki wrote: > > On Friday, July 26, 2013 01:54:00 AM Zheng, Lv wrote: > > > On Friday, July 26, 2013 5:29 AM Rafael J. Wysocki wrote: > > > On Tuesday, July 23, 2013 04:09:43 PM Lv Zheng wrote: > > > > This patch adds reference couting for ACPI operation region > > > > handlers to fix races caused by the ACPICA address space callback > invocations. > > > > > > > > ACPICA address space callback invocation is not suitable for Linux > > > > CONFIG_MODULE=y execution environment. > > > > > > Actually, can you please explain to me what *exactly* the problem is? > > > > OK. I'll add race explanations in the next revision. > > > > The problem is there is no "lock" held inside ACPICA for invoking > > operation region handlers. > > Thus races happens between the > > acpi_remove/install_address_space_handler and the handler/setup > callbacks. > > I see. Now you're trying to introduce something that would prevent those > races from happening, right? Yes. Let me explain this later in this email. > > > This is correct per ACPI specification. > > As if there is interpreter locks held for invoking operation region > > handlers, the timeout implemented inside the operation region handlers > > will make all locking facilities (Acquire or Sleep,...) timed out. > > Please refer to ACPI specification "5.5.2 Control Method Execution": > > Interpretation of a Control Method is not preemptive, but it can > > block. When a control method does block, OSPM can initiate or continue > > the execution of a different control method. A control method can only > > assume that access to global objects is exclusive for any period the control > method does not block. > > > > So it is pretty much likely that ACPI IO transfers are locked inside > > the operation region callback implementations. > > Using locking facility to protect the callback invocation will risk dead locks. > > No. If you use a single global lock around all invocations of operation region > handlers, it won't deadlock, but it will *serialize* things. This means that > there won't be two handlers executing in parallel. That may or may not be > bad depending on what those handlers actually do. > > Your concern seems to be that if one address space handler is buggy and it > blocks indefinitely, executing it under such a lock would affect the other address > space handlers and in my opinion this is a valid concern. It can be expressed in more detailed ways: The interpreter runs control methods in the following style according to the ACPI spec. CM1_Enter -> EnterInter -> CM1_Running -> OpRegion1 -> ExitInter -> EnterInter -> CM1_running -> ExitInter -> CM1_Exit CM2_Enter -> EnterInter -> -> CM2_Running -> OpRegion1 -> ExitInter -> EnterInter -> CM2_running -> ExitInter -> CM2_Exit EnterInter: Enter interpreter lock ExitInter: Leave interpreter lock Let me introduce two situations: 1. If we hold global "mutex" before "EnterInter", then no second control method can be run "NotSerialized". If the CM1 just have some codes waiting for a hardware flag and CM2 can access other hardware IOs to trigger this flag, then nothing can happen any longer. This is a practical bug as what we have already seen in "NotSerialized" marked ACPI control methods behave in the interpreter mode executed in serialized way - kernel parameter "acpi_serialize". 2. If we hold global "mutex" after "EnterInter" and Before OpRegion1 If we do things this way, then all IO accesses are serialized, if we have something in an IPMI operation region failed due to timeout, then any other system IOs that should happen in parallel will just happen after 5 seconds. This is not an acceptable experience. > > So the idea seems to be to add wrappers around > acpi_install_address_space_handler() > and acpi_remove_address_space_handler (but I don't see where the latter is > called after the change?), such that they will know when it is safe to unregister > the handler. That is simple enough. An obvious bug, it should be put between the while (atomic_read() > 1) block and the final atomic_dec(). > However, I'm not sure it is needed in the context of IPMI. I think I do this just because I need a quick fix to test IPMI bug-fix series. The issue is highly related to ACPI interpreter design, and codes should be implemented inside ACPICA. And there is not only ACPI_ROOT_OBJECT based address space handlers, but also non-ACPI_ROOT_OBJECT based address space handlers, this patch can't protect the latter ones. > Your address space > handler's context is NULL, so even it if is executed after > acpi_remove_address_space_handler() has been called for it (or in parallel), it > doesn't depend on anything passed by the caller, so I don't see why the issue > can't be addressed by a proper synchronization between > acpi_ipmi_exit() and acpi_ipmi_space_handler(). > > Clearly, acpi_ipmi_exit() should wait for all already running instances of > acpi_ipmi_space_handler() to complete and all acpi_ipmi_space_handler() > instances started after acpi_ipmi_exit() has been called must return > immediately. > > I would imagine an algorithm like this: > > acpi_ipmi_exit() > 1. Take "address space handler lock". > 2. Set "unregistering address space handler" flag. > 3. Check if "count of currently running handlers" is 0. If so, > call acpi_remove_address_space_handler(), drop the lock (possibly clear > the > flag) and return. > 4. Otherwise drop the lock and go to sleep in "address space handler wait > queue". > 5. When woken up, take "address space handler lock" and go to 3. > > acpi_ipmi_space_handler() > 1. Take "address space handler lock". > 2. Check "unregistering address space handler" flag. If set, drop the lock > and return. > 3. Increment "count of currently running handlers". > 4. Drop the lock. > 5. Do your work. > 6. Take "address space handler lock". > 7. Decrement "count of currently running handlers" and if 0, signal the > tasks waiting on it to wake up. > 8. Drop the lock. Yes, it can also work, but the fix will go inside IPMI. And I agree that the codes should not appear in the IPMI context since the issue is highly ACPI interpreter related. What if we just stop doing any further work on this patch and just mark it as RFC or a test patch for information purpose. It is only useful for the testers. Thanks and best regards -Lv > > Thanks, > Rafael > > > -- > I speak only for myself. > Rafael J. Wysocki, Intel Open Source Technology Center.
diff --git a/drivers/acpi/acpi_ipmi.c b/drivers/acpi/acpi_ipmi.c index 5f8f495..2a09156 100644 --- a/drivers/acpi/acpi_ipmi.c +++ b/drivers/acpi/acpi_ipmi.c @@ -539,20 +539,18 @@ out_ref: static int __init acpi_ipmi_init(void) { int result = 0; - acpi_status status; if (acpi_disabled) return result; mutex_init(&driver_data.ipmi_lock); - status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, - ACPI_ADR_SPACE_IPMI, - &acpi_ipmi_space_handler, - NULL, NULL); - if (ACPI_FAILURE(status)) { + result = acpi_register_region(ACPI_ADR_SPACE_IPMI, + &acpi_ipmi_space_handler, + NULL, NULL); + if (result) { pr_warn("Can't register IPMI opregion space handle\n"); - return -EINVAL; + return result; } result = ipmi_smi_watcher_register(&driver_data.bmc_events); @@ -596,9 +594,7 @@ static void __exit acpi_ipmi_exit(void) } mutex_unlock(&driver_data.ipmi_lock); - acpi_remove_address_space_handler(ACPI_ROOT_OBJECT, - ACPI_ADR_SPACE_IPMI, - &acpi_ipmi_space_handler); + acpi_unregister_region(ACPI_ADR_SPACE_IPMI); } module_init(acpi_ipmi_init); diff --git a/drivers/acpi/osl.c b/drivers/acpi/osl.c index 6ab2c35..8398e51 100644 --- a/drivers/acpi/osl.c +++ b/drivers/acpi/osl.c @@ -86,6 +86,42 @@ static struct workqueue_struct *kacpid_wq; static struct workqueue_struct *kacpi_notify_wq; static struct workqueue_struct *kacpi_hotplug_wq; +struct acpi_region { + unsigned long flags; +#define ACPI_REGION_DEFAULT 0x01 +#define ACPI_REGION_INSTALLED 0x02 +#define ACPI_REGION_REGISTERED 0x04 +#define ACPI_REGION_UNREGISTERING 0x08 +#define ACPI_REGION_INSTALLING 0x10 + /* + * NOTE: Upgrading All Region Handlers + * This flag is only used during the period where not all of the + * region handers are upgraded to the new interfaces. + */ +#define ACPI_REGION_MANAGED 0x80 + acpi_adr_space_handler handler; + acpi_adr_space_setup setup; + void *context; + /* Invoking references */ + atomic_t refcnt; +}; + +static struct acpi_region acpi_regions[ACPI_NUM_PREDEFINED_REGIONS] = { + [ACPI_ADR_SPACE_SYSTEM_MEMORY] = { + .flags = ACPI_REGION_DEFAULT, + }, + [ACPI_ADR_SPACE_SYSTEM_IO] = { + .flags = ACPI_REGION_DEFAULT, + }, + [ACPI_ADR_SPACE_PCI_CONFIG] = { + .flags = ACPI_REGION_DEFAULT, + }, + [ACPI_ADR_SPACE_IPMI] = { + .flags = ACPI_REGION_MANAGED, + }, +}; +static DEFINE_MUTEX(acpi_mutex_region); + /* * This list of permanent mappings is for memory that may be accessed from * interrupt context, where we can't do the ioremap(). @@ -1799,3 +1835,191 @@ void alloc_acpi_hp_work(acpi_handle handle, u32 type, void *context, kfree(hp_work); } EXPORT_SYMBOL_GPL(alloc_acpi_hp_work); + +static bool acpi_region_managed(struct acpi_region *rgn) +{ + /* + * NOTE: Default and Managed + * We only need to avoid region management on the regions managed + * by ACPICA (ACPI_REGION_DEFAULT). Currently, we need additional + * check as many operation region handlers are not upgraded, so + * only those known to be safe are managed (ACPI_REGION_MANAGED). + */ + return !(rgn->flags & ACPI_REGION_DEFAULT) && + (rgn->flags & ACPI_REGION_MANAGED); +} + +static bool acpi_region_callable(struct acpi_region *rgn) +{ + return (rgn->flags & ACPI_REGION_REGISTERED) && + !(rgn->flags & ACPI_REGION_UNREGISTERING); +} + +static acpi_status +acpi_region_default_handler(u32 function, + acpi_physical_address address, + u32 bit_width, u64 *value, + void *handler_context, void *region_context) +{ + acpi_adr_space_handler handler; + struct acpi_region *rgn = (struct acpi_region *)handler_context; + void *context; + acpi_status status = AE_NOT_EXIST; + + mutex_lock(&acpi_mutex_region); + if (!acpi_region_callable(rgn) || !rgn->handler) { + mutex_unlock(&acpi_mutex_region); + return status; + } + + atomic_inc(&rgn->refcnt); + handler = rgn->handler; + context = rgn->context; + mutex_unlock(&acpi_mutex_region); + + status = handler(function, address, bit_width, value, context, + region_context); + atomic_dec(&rgn->refcnt); + + return status; +} + +static acpi_status +acpi_region_default_setup(acpi_handle handle, u32 function, + void *handler_context, void **region_context) +{ + acpi_adr_space_setup setup; + struct acpi_region *rgn = (struct acpi_region *)handler_context; + void *context; + acpi_status status = AE_OK; + + mutex_lock(&acpi_mutex_region); + if (!acpi_region_callable(rgn) || !rgn->setup) { + mutex_unlock(&acpi_mutex_region); + return status; + } + + atomic_inc(&rgn->refcnt); + setup = rgn->setup; + context = rgn->context; + mutex_unlock(&acpi_mutex_region); + + status = setup(handle, function, context, region_context); + atomic_dec(&rgn->refcnt); + + return status; +} + +static int __acpi_install_region(struct acpi_region *rgn, + acpi_adr_space_type space_id) +{ + int res = 0; + acpi_status status; + int installing = 0; + + mutex_lock(&acpi_mutex_region); + if (rgn->flags & ACPI_REGION_INSTALLED) + goto out_lock; + if (rgn->flags & ACPI_REGION_INSTALLING) { + res = -EBUSY; + goto out_lock; + } + + installing = 1; + rgn->flags |= ACPI_REGION_INSTALLING; + status = acpi_install_address_space_handler(ACPI_ROOT_OBJECT, space_id, + acpi_region_default_handler, + acpi_region_default_setup, + rgn); + rgn->flags &= ~ACPI_REGION_INSTALLING; + if (ACPI_FAILURE(status)) + res = -EINVAL; + else + rgn->flags |= ACPI_REGION_INSTALLED; + +out_lock: + mutex_unlock(&acpi_mutex_region); + if (installing) { + if (res) + pr_err("Failed to install region %d\n", space_id); + else + pr_info("Region %d installed\n", space_id); + } + return res; +} + +int acpi_register_region(acpi_adr_space_type space_id, + acpi_adr_space_handler handler, + acpi_adr_space_setup setup, void *context) +{ + int res; + struct acpi_region *rgn; + + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) + return -EINVAL; + + rgn = &acpi_regions[space_id]; + if (!acpi_region_managed(rgn)) + return -EINVAL; + + res = __acpi_install_region(rgn, space_id); + if (res) + return res; + + mutex_lock(&acpi_mutex_region); + if (rgn->flags & ACPI_REGION_REGISTERED) { + mutex_unlock(&acpi_mutex_region); + return -EBUSY; + } + + rgn->handler = handler; + rgn->setup = setup; + rgn->context = context; + rgn->flags |= ACPI_REGION_REGISTERED; + atomic_set(&rgn->refcnt, 1); + mutex_unlock(&acpi_mutex_region); + + pr_info("Region %d registered\n", space_id); + + return 0; +} +EXPORT_SYMBOL_GPL(acpi_register_region); + +void acpi_unregister_region(acpi_adr_space_type space_id) +{ + struct acpi_region *rgn; + + if (space_id >= ACPI_NUM_PREDEFINED_REGIONS) + return; + + rgn = &acpi_regions[space_id]; + if (!acpi_region_managed(rgn)) + return; + + mutex_lock(&acpi_mutex_region); + if (!(rgn->flags & ACPI_REGION_REGISTERED)) { + mutex_unlock(&acpi_mutex_region); + return; + } + if (rgn->flags & ACPI_REGION_UNREGISTERING) { + mutex_unlock(&acpi_mutex_region); + return; + } + + rgn->flags |= ACPI_REGION_UNREGISTERING; + rgn->handler = NULL; + rgn->setup = NULL; + rgn->context = NULL; + mutex_unlock(&acpi_mutex_region); + + while (atomic_read(&rgn->refcnt) > 1) + schedule_timeout_uninterruptible(usecs_to_jiffies(5)); + atomic_dec(&rgn->refcnt); + + mutex_lock(&acpi_mutex_region); + rgn->flags &= ~(ACPI_REGION_REGISTERED | ACPI_REGION_UNREGISTERING); + mutex_unlock(&acpi_mutex_region); + + pr_info("Region %d unregistered\n", space_id); +} +EXPORT_SYMBOL_GPL(acpi_unregister_region); diff --git a/include/acpi/acpi_bus.h b/include/acpi/acpi_bus.h index a2c2fbb..15fad0d 100644 --- a/include/acpi/acpi_bus.h +++ b/include/acpi/acpi_bus.h @@ -542,4 +542,9 @@ static inline int unregister_acpi_bus_type(void *bus) { return 0; } #endif /* CONFIG_ACPI */ +int acpi_register_region(acpi_adr_space_type space_id, + acpi_adr_space_handler handler, + acpi_adr_space_setup setup, void *context); +void acpi_unregister_region(acpi_adr_space_type space_id); + #endif /*__ACPI_BUS_H__*/