[PATCH v2 2/2] drm/xe: Use the filelist from drm for ccs_mode change

[Lucas De Marchi]

On Tue, Oct 08, 2024 at 03:42:09PM +0000, Matthew Brost wrote:
>On Tue, Oct 08, 2024 at 10:20:15AM -0500, Lucas De Marchi wrote:
>> On Tue, Oct 08, 2024 at 02:54:19PM +0000, Matthew Brost wrote:
>> > On Tue, Oct 08, 2024 at 01:06:28PM +0530, Balasubramani Vivekanandan wrote:
>> > > Drop the exclusive client count tracking and use the filelist from the
>> > > drm to track the active clients. This also ensures the clients created
>> > > internally by the driver won't block changing the ccs mode.
>> > >
>> > > Fixes: ce8c161cbad4 ("drm/xe: Add ref counting for xe_file")
>> >
>> > Is this really fixing anything. As far as I can tell nothing upstream
>> > opens a file internally (i.e. xe_file_open) is never called directly.
>>
>> should fix this case:
>>
>> 	open()
>> 	close()
>> 			<---- race here
>> 	change_ccs_mode()
>>
>> because the close is not completely sync - the cleanup where the
>> previous number of clients is decremented is executed too late and
>> subject to a race fixed here.
>>
>
>Ah, ok. But then IMO just move the clients.count decrement to
>xe_file_close. I try to preach solid locking / layering and this seems
>to go against this idea.

why do you want to track the exact same thing in 2 ways, one in drm and
the other in xe? "Are there clients connected?" is something drm_file
answer and doesn't need to be duplicated in the individual drivers.

At least it's also used for similar reasons in amdgpu and vmwgfx:

$ git grep -l -e "->filelist" -- drivers/gpu/drm/
drivers/gpu/drm/amd/amdgpu/amdgpu_debugfs.c
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c
drivers/gpu/drm/drm_client.c
drivers/gpu/drm/drm_debugfs.c
drivers/gpu/drm/drm_drv.c
drivers/gpu/drm/drm_file.c
drivers/gpu/drm/vmwgfx/vmwgfx_gem.c


Lucas De Marchi