From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1758614AbcG0FiL (ORCPT ); Wed, 27 Jul 2016 01:38:11 -0400 Received: from anholt.net ([50.246.234.109]:40281 "EHLO anholt.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1750868AbcG0FiE (ORCPT ); Wed, 27 Jul 2016 01:38:04 -0400 From: Eric Anholt To: Rob Clark Cc: "dri-devel\@lists.freedesktop.org" , Linux Kernel Mailing List , stable Subject: Re: [PATCH 5/6] drm/vc4: Fix overflow mem unreferencing when the binner runs dry. In-Reply-To: References: <1469566035-22006-1-git-send-email-eric@anholt.net> <1469566035-22006-6-git-send-email-eric@anholt.net> <87bn1kov0b.fsf@eliezer.anholt.net> User-Agent: Notmuch/0.22.1 (http://notmuchmail.org) Emacs/24.5.1 (x86_64-pc-linux-gnu) Date: Tue, 26 Jul 2016 22:37:59 -0700 Message-ID: <87k2g7vdxk.fsf@eliezer.anholt.net> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha512; protocol="application/pgp-signature" Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org --=-=-= Content-Type: text/plain Rob Clark writes: > On Tue, Jul 26, 2016 at 7:11 PM, Eric Anholt wrote: >> Rob Clark writes: >> >>> On Tue, Jul 26, 2016 at 4:47 PM, Eric Anholt wrote: >>>> Overflow memory handling is tricky: While it's still referenced by the >>>> BPO registers, we want to keep it from being freed. When we are >>>> putting a new set of overflow memory in the registers, we need to >>>> assign the old one to the last rendering job using it. >>>> >>>> We were looking at "what's currently running in the binner", but since >>>> the bin/render submission split, we may end up with the binner >>>> completing and having no new job while the renderer is still >>>> processing. So, if we don't find a bin job at all, look at the >>>> highest-seqno (last) render job to attach our overflow to. >>> >>> so, drive-by comment.. but can you allocate gem bo's without backing >>> them immediately with pages? If so, just always allocate the bo >>> up-front and attach it as a dependency of the batch, and only pin it >>> to actual pages when you have to overflow? >> >> The amount of overflow for a given CL is arbitrary, depending on the >> geometry submitted, and the overflow pool just gets streamed into by the >> hardware as you submit bin jobs. You'll end up allocating [0,n] new >> overflows per bin job. I don't see where "allocate gem BOs without >> backing them immediately with pages" idea would fit into this. > > well, even not knowing the size up front shouldn't really be a > show-stopper, unless you had to mmap it to userspace, perhaps.. > normally backing pages aren't allocated until drm_gem_get_pages() so > allocating the gem bo as placeholder to track dependencies of the > batch/submit shouldn't be an issue. But I noticed you don't use > drm_gem_get_pages().. maybe w/ cma helpers it is harder to decouple > allocation of the drm_gem_object from the backing store. There's no period of time between "I need to allocate an overflow BO" and "I need pages in the BO", though. I could have a different setup that allocated a massive (all of CMA?), fresh overflow BO per CL and populated page ranges in it as I overflow, but with CMA you really need to never do new allocations in the hot path because you get to stop and wait approximately forever. So you'd want to chunk it up so you could cache the groups of contiguous pages of overflow, and it turns out we already have a thing for this in the form of GEM BOs. Anyway, doing that that means you're losing out on the rest of the last overflow BO for the new CL, expanding the working set in your precious 256MB CMA area. Well, OK, actually I *do* allocate a fresh overflow BO per CL today, because of leftover bringup code that I think I could just delete at this point. I'm not doing that in a -fixes commit, though. --=-=-= Content-Type: application/pgp-signature; name="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1 iQIcBAEBCgAGBQJXmEi3AAoJELXWKTbR/J7omRcP/1YBmWtse4b9xgoN5OthYqrR fXQAIRiCb/L5hUDNUncTCIFTS2pqvbPllS2ugsacEkr02LQLtBRCFg8jSm8nkz9Q u9LAIb+20TN4z55d5A4YLuHlCpjS6kJkwZiykzwhBph/CynVkA2povaA0HI9cfJU LcIKQVstWCb68qoRXgE2bI9iiRFQSZ+8NNXIdyWT2KdyvTSoDFSQ0glSg1Ot21U0 0uTbaX2Xxdnvhadyzu8nqvIXPAaEM/nxGtNgVbDqzEav5sm0XKyazEXFt4g34P2k gwm4eqVx0pkHytEdkIc3ZmlplVUhKk4YbcpSFLzNQL+qGYYrfe+WFPEEr4fZSh5U 1lpX4DW83qq9dVBraI8cJZPkascY8+Cx1Ye/CTzXmMmi2jiQfrNSy3jQ+6n/TRaT yj9EbicrnpIjIxByJW7y6g6qZCdZdMlCPzhPDtNIkrGl/ueRh38AyjEy6wSYTeY8 cWle7tp2v08+VFxdIcKgnNHPN7hs1I8jb/iVrlPnNOKeWNTO1eyZXWRXl4408soH JyC/47Vu0nJQtRUvxymc29Z86S1rVU5famH389Pm2B796D1o3fyohxXGhqhGvFAO 22UZWFDorm1M+DH6D6lS/trU84Az7YsxGP8nKGqD4qcEfjeM6KqTPTPfO8fKw+um CS3CbFo6sbRpWB9WIm18 =7s+J -----END PGP SIGNATURE----- --=-=-=--