Re: [PATCH v2 5/8] drm/amdgpu: Refactor sysfs removal

From: Andrey Grodzovsky <Andrey.Grodzovsky@amd.com>
To: Greg KH <gregkh@linuxfoundation.org>
Cc: daniel.vetter@ffwll.ch, michel@daenzer.net,
	dri-devel@lists.freedesktop.org, ppaalanen@gmail.com,
	amd-gfx@lists.freedesktop.org, Daniel Vetter <daniel@ffwll.ch>,
	ckoenig.leichtzumerken@gmail.com, alexdeucher@gmail.com
Subject: Re: [PATCH v2 5/8] drm/amdgpu: Refactor sysfs removal
Date: Wed, 11 Nov 2020 10:45:53 -0500	[thread overview]
Message-ID: <dc348560-907c-1b7e-a836-7dea4017d4e4@amd.com> (raw)
In-Reply-To: <X6wEbtSDm69gzFbR@kroah.com>

On 11/11/20 10:34 AM, Greg KH wrote:
> On Wed, Nov 11, 2020 at 10:13:13AM -0500, Andrey Grodzovsky wrote:
>> On 11/10/20 12:59 PM, Greg KH wrote:
>>> On Tue, Nov 10, 2020 at 12:54:21PM -0500, Andrey Grodzovsky wrote:
>>>> Hi, back to this after a long context switch for some higher priority stuff.
>>>>
>>>> So here I was able eventually to drop all this code and this change here https://nam11.safelinks.protection.outlook.com/?url=https:%2F%2Fcgit.freedesktop.org%2F~agrodzov%2Flinux%2Fcommit%2F%3Fh%3Damd-staging-drm-next-device-unplug%26id%3D61852c8a59b4dd89d637693552c73175b9f2ccd6&amp;data=04%7C01%7CAndrey.Grodzovsky%40amd.com%7C9fbfecac94a340dfb68408d886571609%7C3dd8961fe4884e608e11a82d994e183d%7C0%7C0%7C637407055896651058%7CUnknown%7CTWFpbGZsb3d8eyJWIjoiMC4wLjAwMDAiLCJQIjoiV2luMzIiLCJBTiI6Ik1haWwiLCJXVCI6Mn0%3D%7C1000&amp;sdata=Ye8HJR1vidppcOBnlOgVu5GwKD2%2Bb5ztHbiI%2BubKKT0%3D&amp;reserved=0
>>>> was enough for me. Seems like while device_remove_file can handle the use
>>>> case where the file and the parent directory already gone,
>>>> sysfs_remove_group goes down in flames in that case
>>>> due to kobj->sd being unset on device removal.
>>> A driver shouldn't ever have to remove individual sysfs groups, the
>>> driver core/bus logic should do it for them automatically.
>>>
>>> And whenever a driver calls a sysfs_* call, that's a hint that something
>>> is not working properly.
>>
>>
>> Do you mean that while the driver creates the groups and files explicitly
>> from it's different subsystems it should not explicitly remove each
>> one of them because all of them should be removed at once (and
>> recursively) when the device is being removed ?
> Individual drivers should never add groups/files in sysfs, the driver
> core should do it properly for you if you have everything set up
> properly.  And yes, the driver core will automatically remove them as
> well.
>
> Please use the default groups attribute for your bus/subsystem and this
> will happen automagically.

Googling for default groups attributes i found this - 
https://www.linuxfoundation.org/blog/2013/06/how-to-create-a-sysfs-file-correctly/
Would this be what you suggest for us ? Specifically for our case the struct 
device's  groups  seems the right solution as different devices
might have slightly diffreent sysfs attributes.

Andrey

>
> thanks,
>
> greg k-h
_______________________________________________
amd-gfx mailing list
amd-gfx@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/amd-gfx