Timur Tabi
2024-Feb-01 16:20 UTC
[PATCH] drm/nouveau: nouveau_sched_fini() should check for init failure
On Thu, 2024-02-01 at 13:58 +0100, Danilo Krummrich wrote:> I wonder if we should allocate struct nouveau_sched dynamically instead and > just > check for NULL in the corresponding *_fini() functions. > > Actually, in nouveau_abi16_ioctl_channel_alloc() we can omit creating a > scheduler > instance entirely if !nouveau_cli_uvmm().I am all for a better solution if you can think of one. I don't really like the idea of "bool initialized", but I couldn't think of a more reliable test.
Danilo Krummrich
2024-Feb-01 16:35 UTC
[PATCH] drm/nouveau: nouveau_sched_fini() should check for init failure
On 2/1/24 17:20, Timur Tabi wrote:> On Thu, 2024-02-01 at 13:58 +0100, Danilo Krummrich wrote: >> I wonder if we should allocate struct nouveau_sched dynamically instead and >> just >> check for NULL in the corresponding *_fini() functions. >> >> Actually, in nouveau_abi16_ioctl_channel_alloc() we can omit creating a >> scheduler >> instance entirely if !nouveau_cli_uvmm(). > > I am all for a better solution if you can think of one. I don't really like > the idea of "bool initialized", but I couldn't think of a more reliable test.Yeah, it's not really ideal that there is a single error path calling the corresponding *_fini() in nouveau_abi16_ioctl_channel_alloc() and nouveau_cli_init() only. Maybe this can be re-worked to only reverse what has actually been done already. However, I'd probably not do that for -fixes. Maybe we should go with the dynamic allocation and a NULL check for now. Do you want to send another patch? Otherwise I can also come up with something and add your Reported-by. - Danilo> >