On 27 October 2015 at 10:50, samuel.pitoiset <samuel.pitoi...@gmail.com> wrote: > On 27/10/2015 11:37, Emil Velikov wrote: >> >> On 22 October 2015 at 00:16, Julien Isorce <julien.iso...@gmail.com> >> wrote: >>> >>> The real fix is in nouveau_drm_winsys.c by setting dev to 0. >>> Which means dev's ownership has been passed to previous call. >>> Other changes are there to be consistent with what the >>> screen_create functions already do on errors. >>> >>> Encountered this crash because nvc0_screen_create sometimes fails with: >>> nvc0_screen_create:717 - Error allocating PGRAPH context for M2MF: -16 >>> Also see: https://bugs.freedesktop.org/show_bug.cgi?id=70354 >>> >>> Signed-off-by: Julien Isorce <j.iso...@samsung.com> >>> --- >>> src/gallium/drivers/nouveau/nv30/nv30_screen.c | 5 ++++- >>> src/gallium/drivers/nouveau/nv50/nv50_screen.c | 4 +++- >>> src/gallium/winsys/nouveau/drm/nouveau_drm_winsys.c | 2 ++ >>> 3 files changed, 9 insertions(+), 2 deletions(-) >>> >>> diff --git a/src/gallium/drivers/nouveau/nv30/nv30_screen.c >>> b/src/gallium/drivers/nouveau/nv30/nv30_screen.c >>> index 0330164..9b8ddac 100644 >>> --- a/src/gallium/drivers/nouveau/nv30/nv30_screen.c >>> +++ b/src/gallium/drivers/nouveau/nv30/nv30_screen.c >>> @@ -425,8 +425,10 @@ nv30_screen_create(struct nouveau_device *dev) >>> unsigned oclass = 0; >>> int ret, i; >>> >>> - if (!screen) >>> + if (!screen) { >>> + nouveau_device_del(&dev); >>> return NULL; >>> + } >>> >> Imho having these in screen_create() seems like the wrong 'layer'. >> Shouldn't one call nouveau_device_dev() from within >> nouveau_drm_screen_unref >> and explicitly call the latter if the calloc() (here and in nv50/nvc0) >> fails ? > > > We can't do that because nouveau_drm_screen_unref() needs a valid > nouveau_screen > object and in this case it is NULL. > Ouch I was under the impression that we've brought back the concept of winsys in nouveau with the hash_table patches. Seems like we haven't :(
If we are to do so (split things just like the radeon/amdgpu winsys) then we can kill two birds with one stone. The missing device_del() on calloc failure as well as other error paths in nvxx_screen_create(). > I agree that it's not really an elegant fix but we don't really have the > choice actually. > In my opinion, this is not that bad. > I never said it's "bad" just the wrong place for the fix. Or in other words - if we're to fix things might as well do it properly :-) -Emil _______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org http://lists.freedesktop.org/mailman/listinfo/mesa-dev
