Sorry for another mail on same patch. I missed to add one response on previous comments <snip> > > +/** > > + * init_bios_attributes() - Initialize all attributes for a type > > + * @attr_type: The attribute type to initialize > > + * @guid: The WMI GUID associated with this type to initialize > > + * > > + * Initialiaze all 4 types of attributes enumeration, integer, string and > password object. > > + * Populates each attrbute typ's respective properties under sysfs files > > + **/ > > +static int init_bios_attributes(int attr_type, const char *guid) > > +{ > > + struct kobject *attr_name_kobj; //individual attribute names > > + union acpi_object *obj = NULL; > > + union acpi_object *elements; > > + struct kset *tmp_set; > > + > > + /* instance_id needs to be reset for each type GUID > > + * also, instance IDs are unique within GUID but not across > > + */ > > + int instance_id = 0; > > + int retval = 0; > > + > > + retval = alloc_attributes_data(attr_type); > > + if (retval) > > + return retval; > > + /* need to use specific instance_id and guid combination to get right > data */ > > + obj = get_wmiobj_pointer(instance_id, guid); > > + if (!obj) > > + return -ENODEV; > > + elements = obj->package.elements; > > + > > + mutex_lock(&wmi_priv.mutex); > > + while (elements) { > > + /* sanity checking */ > > + if (strlen(elements[ATTR_NAME].string.pointer) == 0) { > > + pr_debug("empty attribute found\n"); > > + goto nextobj; > > + } > > + if (attr_type == PO) > > + tmp_set = wmi_priv.authentication_dir_kset; > > + else > > + tmp_set = wmi_priv.main_dir_kset; > > + > > + if (kset_find_obj(tmp_set, > elements[ATTR_NAME].string.pointer)) { > > + pr_debug("duplicate attribute name found - %s\n", > > + elements[ATTR_NAME].string.pointer); > > + goto nextobj; > > + } > > + > > + /* build attribute */ > > + attr_name_kobj = kzalloc(sizeof(*attr_name_kobj), > GFP_KERNEL); > > + if (!attr_name_kobj) > > + goto err_attr_init; > > + > > + attr_name_kobj->kset = tmp_set; > > + > > + retval = kobject_init_and_add(attr_name_kobj, > &attr_name_ktype, NULL, "%s", > > + > elements[ATTR_NAME].string.pointer); > > + if (retval) { > > + kobject_put(attr_name_kobj); > > + goto err_attr_init; > > + } > > + > > + /* enumerate all of this attribute */ > > + switch (attr_type) { > > + case ENUM: > > + retval = populate_enum_data(elements, instance_id, > attr_name_kobj); > > + break; > > + case INT: > > + retval = populate_int_data(elements, instance_id, > attr_name_kobj); > > + break; > > + case STR: > > + retval = populate_str_data(elements, instance_id, > attr_name_kobj); > > + break; > > + case PO: > > + retval = populate_po_data(elements, instance_id, > attr_name_kobj); > > + break; > > + default: > > + break; > > + } > > The show functions don't take the mutex and can be called as soon as > kobject_init_and_add() is called, so it would be better to first populate the > data for the current instance_id and only then call kobject_init_and_add() > Populate functions called here for each type of attribute uses attribute_koject which helps in attribute group cleanup. Regards, Divya