Re: [PATCH 5.15 53/77] platform/x86: think-lmi: Fix sysfs group cleanup

[Sasha Levin <sashal@kernel.org>]

On Tue, Jul 15, 2025 at 04:22:22PM -0300, Kurt Borja wrote:
>Hi Sasha,
>
>On Tue Jul 15, 2025 at 4:04 PM -03, Sasha Levin wrote:
>> On Tue, Jul 15, 2025 at 03:38:58PM -0300, Kurt Borja wrote:
>>>Hi Greg,
>>>
>>>On Tue Jul 15, 2025 at 10:13 AM -03, Greg Kroah-Hartman wrote:
>>>> 5.15-stable review patch.  If anyone has any objections, please let me know.
>>>>
>>>> ------------------
>>>>
>>>> From: Kurt Borja <kuurtb@gmail.com>
>>>>
>>>> [ Upstream commit 4f30f946f27b7f044cf8f3f1f353dee1dcd3517a ]
>>>>
>>>> Many error paths in tlmi_sysfs_init() lead to sysfs groups being removed
>>>> when they were not even created.
>>>>
>>>> Fix this by letting the kobject core manage these groups through their
>>>> kobj_type's defult_groups.
>>>>
>>>> Fixes: a40cd7ef22fb ("platform/x86: think-lmi: Add WMI interface support on Lenovo platforms")
>>>> Cc: stable@vger.kernel.org
>>>> Reviewed-by: Mark Pearson <mpearson-lenovo@squebb.ca>
>>>> Reviewed-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>>> Signed-off-by: Kurt Borja <kuurtb@gmail.com>
>>>> Link: https://lore.kernel.org/r/20250630-lmi-fix-v3-3-ce4f81c9c481@gmail.com
>>>> Signed-off-by: Ilpo Järvinen <ilpo.jarvinen@linux.intel.com>
>>>> Signed-off-by: Sasha Levin <sashal@kernel.org>
>>>> ---
>>>>  drivers/platform/x86/think-lmi.c | 35 +++++++++-----------------------
>>>>  1 file changed, 10 insertions(+), 25 deletions(-)
>>>>
>>>> diff --git a/drivers/platform/x86/think-lmi.c b/drivers/platform/x86/think-lmi.c
>>>> index 36ff64a7b6847..cc46aa5f1da2c 100644
>>>> --- a/drivers/platform/x86/think-lmi.c
>>>> +++ b/drivers/platform/x86/think-lmi.c
>>>> @@ -491,6 +491,7 @@ static struct attribute *auth_attrs[] = {
>>>>  static const struct attribute_group auth_attr_group = {
>>>>  	.attrs = auth_attrs,
>>>>  };
>>>> +__ATTRIBUTE_GROUPS(auth_attr);
>>>>
>>>>  /* ---- Attributes sysfs --------------------------------------------------------- */
>>>>  static ssize_t display_name_show(struct kobject *kobj, struct kobj_attribute *attr,
>>>> @@ -643,6 +644,7 @@ static const struct attribute_group tlmi_attr_group = {
>>>>  	.is_visible = attr_is_visible,
>>>>  	.attrs = tlmi_attrs,
>>>>  };
>>>> +__ATTRIBUTE_GROUPS(tlmi_attr);
>>>>
>>>>  static ssize_t tlmi_attr_show(struct kobject *kobj, struct attribute *attr,
>>>>  				    char *buf)
>>>> @@ -688,12 +690,14 @@ static void tlmi_pwd_setting_release(struct kobject *kobj)
>>>>
>>>>  static struct kobj_type tlmi_attr_setting_ktype = {
>>>>  	.release        = &tlmi_attr_setting_release,
>>>> -	.sysfs_ops	= &tlmi_kobj_sysfs_ops,
>>>> +	.sysfs_ops	= &kobj_sysfs_ops,
>>>> +	.default_groups = tlmi_attr_groups,
>>>
>>>I did *not* author this change and it utterly *breaks* the driver.
>>>
>>>This patch should be dropped ASAP.
>>
>> Right sorry about that - I accidently left that extra line of context
>> you've pointed out. Dropped now along with the other patch you've
>> pointed out.
>
>Thank you for the quick response!
>
>May I suggest informing authors and maintainers about manual conflict
>resolution in the email's subject?
>
>This resolution was not acked by anyone and I don't appreciate that.

In general we'd note a manual resolution and wait for an ack if any
significant changes were made to a commit.

In this case I messed up the context when resolving the conflict, and
didn't consider it meaningful.

However, yes, just noting that along with my signoff wouldn't have hurt.
I'll start doing that more often.

>In the past couple of weeks and I got like 50 stable related emails. I
>simply cannot check each and every one for things like this.

Is there something we can do on our end to make it more managable?

-- 
Thanks,
Sasha