Re: drm: Branch 'master' - 2 commits
On Mon, 05 Dec 2011 14:23:55 -0800, Eric Anholt wrote: > On Mon, 5 Dec 2011 02:31:58 -0800 (PST), [email protected] (Chris > Wilson) wrote: > > configure.ac |2 +- > > intel/intel_bufmgr_gem.c | 27 +-- > > 2 files changed, 22 insertions(+), 7 deletions(-) > > > > New commits: > > commit e73161a02b604742e3da3bca8f13cff81276de43 > > Author: Chris Wilson > > Date: Mon Dec 5 10:30:52 2011 + > > > > configure: Bump version to 2.4.28 > > > > So that we can pull a couple of Intel bug fixes into xf86-video-intel. > > > > Signed-off-by: Chris Wilson > > Performance before: > [ 0] glfirefox-talos-gfx 17.866 17.915 0.14%4/4 > after: > [ 0] glfirefox-talos-gfx 22.173 22.251 0.20%4/4 > > There's a pretty obvious opportunity to keep the performance win of the > userspace caching that you've broken here, but you gave none of us a > chance to review it before you pushed the patch *and shipped a release > with it*. This is not acceptable. Please revert and bump the release, > and fix it correctly. No, what was unacceptable was a vma leak impacting a critical system application, and for the library API to be broken by design. The onus is to improve performance without regressing system stability, not the other way around. -Chris -- Chris Wilson, Intel Open Source Technology Centre -- Cloud Services Checklist: Pricing and Packaging Optimization This white paper is intended to serve as a reference, checklist and point of discussion for anyone considering optimizing the pricing and packaging model of a cloud services business. Read Now! http://www.accelacomm.com/jaw/sfnl/114/51491232/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
On Mon, 5 Dec 2011 02:31:58 -0800 (PST), [email protected] (Chris Wilson) wrote: > configure.ac |2 +- > intel/intel_bufmgr_gem.c | 27 +-- > 2 files changed, 22 insertions(+), 7 deletions(-) > > New commits: > commit e73161a02b604742e3da3bca8f13cff81276de43 > Author: Chris Wilson > Date: Mon Dec 5 10:30:52 2011 + > > configure: Bump version to 2.4.28 > > So that we can pull a couple of Intel bug fixes into xf86-video-intel. > > Signed-off-by: Chris Wilson Performance before: [ 0] glfirefox-talos-gfx 17.866 17.915 0.14%4/4 after: [ 0] glfirefox-talos-gfx 22.173 22.251 0.20%4/4 There's a pretty obvious opportunity to keep the performance win of the userspace caching that you've broken here, but you gave none of us a chance to review it before you pushed the patch *and shipped a release with it*. This is not acceptable. Please revert and bump the release, and fix it correctly. pgpAEuNpy2UG9.pgp Description: PGP signature -- All the data continuously generated in your IT infrastructure contains a definitive record of customers, application performance, security threats, fraudulent activity, and more. Splunk takes this data and makes sense of it. IT sense. And common sense. http://p.sf.net/sfu/splunk-novd2d-- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
Dave Airlie wrote: >>> >>> >> Yes, I think so. One reason to lock is to make sure the GTT page table >> is really repopulated after a resume, but that can be handled in the DRM >> driver resume function, and we could perhaps >> change the user-space lock_mm() to ignore kernel_bos. >> > > It defintely should all be handled in-kernel, putting functionality like > that under user space control is a throw back, the lock-mm should maybe > only apply to the bos allocated to that user who locks.. or really I'm not > sure we really need a lock function if we have a proper kernel driver.. > > I'm not sure that we need that either. Currently it has two purposes: 1) Make sure the X server doesn't use any VRAM or TT resources when it's VT switched. With TTM I don't think we really need this, since if an fb app or whatever needs space, it will evict the X server and DRI buffers. 2) Make dri clients sleep while VT is switched away. If we decide that 1) is not important with kernel-created mms, the problem more or less goes away, and I haven't really read up enough on the new drm design to know how 2 is supposed to be handled. /Thomas - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
> > > Yes, I think so. One reason to lock is to make sure the GTT page table > is really repopulated after a resume, but that can be handled in the DRM > driver resume function, and we could perhaps > change the user-space lock_mm() to ignore kernel_bos. It defintely should all be handled in-kernel, putting functionality like that under user space control is a throw back, the lock-mm should maybe only apply to the bos allocated to that user who locks.. or really I'm not sure we really need a lock function if we have a proper kernel driver.. > > We also have a another possibility where an ioctl makes the drm fb evict > its scanout buffers and one that brings them back. This preserves > aperture space when X is doing its allocation, but it's abit > error-prone since it requires that fb can live with updated offsets and > kernel virtual addresses. It apparently can, but there's no mutex > generally protecting these variables so it seems quite fragile. Again we can't do this with an ioctl as it leaves us open to userspace whims, however with modesetting branch I'm hoping to come up with something to allow this, myself and Jakob have discussed a driver specific ioctl, which can swap fbs out of the aperture on the fly, so the driver can swap in X's fb and when X exits swap in the fb fb.. Yes fb is a bit messy, it doesn't have a prepare/commit type functionality, and we may have to come up with a map/unmap interface with a mutex for it to use to update the screen... Dave. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
Dave Airlie wrote: >> So, over to the somewhat different problem I was referring to, namely >> potential buffer eviction on vt switch where the X server run >> drm_bo_lock_mm(), and will evict any fb scanout BOs. Only the fb layer >> doesn't really notice as long as the BOs sit in VRAM... >> > > Yes I get around this currently by not having the X server call it on > modesetting systems. Clearly I need a better answer which may be that we > ignore it in the kernel, unless it comes from a control node process.. > > when does locking the mm make actual sense? just VT switching? > > Yes, I think so. One reason to lock is to make sure the GTT page table is really repopulated after a resume, but that can be handled in the DRM driver resume function, and we could perhaps change the user-space lock_mm() to ignore kernel_bos. We also have a another possibility where an ioctl makes the drm fb evict its scanout buffers and one that brings them back. This preserves aperture space when X is doing its allocation, but it's abit error-prone since it requires that fb can live with updated offsets and kernel virtual addresses. It apparently can, but there's no mutex generally protecting these variables so it seems quite fragile. /Thomas > Dave. > - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
> So, over to the somewhat different problem I was referring to, namely > potential buffer eviction on vt switch where the X server run > drm_bo_lock_mm(), and will evict any fb scanout BOs. Only the fb layer > doesn't really notice as long as the BOs sit in VRAM... Yes I get around this currently by not having the X server call it on modesetting systems. Clearly I need a better answer which may be that we ignore it in the kernel, unless it comes from a control node process.. when does locking the mm make actual sense? just VT switching? Dave. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
Dave Airlie wrote: >>> >>> this adds something to say the kernel initialised the memory region not >>> the userspace. and blocks userspace from deallocating kernel areas >>> >>> >> Dave, >> I guess this commit is a fix for the fact that the X server will try to >> evict fb scanout buffers when leaving VT. We've solved this issue so far >> by having the fb layer evict its scanouts when the X server starts >> before it goes on allocating its own buffers. This was done using >> device-private IOCTLS. >> > > Nope its to do with the fact that if the kernel sets up a memory managed > area, userspace shuoldn't be allowed to just tear it down... so in > modesetting the kernel sets up the areas, running an old X driver then > nukes my memory manager that I'm still using... > > What I'd like to do actually is make the memory managed areas configured > from userspace attach to the master fd as well, so when you nuke > userspace, it self destructs any memory managed areas it setup, while this > has little long term value (as really you need to create the memory areas > in-kernel already) I can see it being useful for a lot of short-term > stuff. > > OK. Hmm, I was confusing drm_bo_clean_mm with drm_bo_lock_mm. > > >> While this is probably desirable to save GTT / VRAM space, I agree it's >> not desirable that the X server or master has the power to evict kernel >> buffers. >> >> What are your thoughts about this? It'd be good to have a common >> practice for upcoming drivers. >> Do we need a mechanism for the X server to clean out it's own buffers >> and the DRI client buffers, or should we just leave them where they are? >> If we want to implement something like that we might have >> drm_bo_clean_mm leave bo_kernel buffers alone instead of the whole >> memory area. >> >> > > So really when the X server exits, all buffers allocated by it need to go > away, or at least get dereferenced, however if it has create memory > managed areas these also need to go away... it should not be destroying > memory manager areas. > I agree. So, over to the somewhat different problem I was referring to, namely potential buffer eviction on vt switch where the X server run drm_bo_lock_mm(), and will evict any fb scanout BOs. Only the fb layer doesn't really notice as long as the BOs sit in VRAM... This needs to be addressed in one way or another. /Thomas > Dave. > - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
> > > > this adds something to say the kernel initialised the memory region not > > the userspace. and blocks userspace from deallocating kernel areas > > > Dave, > I guess this commit is a fix for the fact that the X server will try to > evict fb scanout buffers when leaving VT. We've solved this issue so far > by having the fb layer evict its scanouts when the X server starts > before it goes on allocating its own buffers. This was done using > device-private IOCTLS. Nope its to do with the fact that if the kernel sets up a memory managed area, userspace shuoldn't be allowed to just tear it down... so in modesetting the kernel sets up the areas, running an old X driver then nukes my memory manager that I'm still using... What I'd like to do actually is make the memory managed areas configured from userspace attach to the master fd as well, so when you nuke userspace, it self destructs any memory managed areas it setup, while this has little long term value (as really you need to create the memory areas in-kernel already) I can see it being useful for a lot of short-term stuff. > While this is probably desirable to save GTT / VRAM space, I agree it's > not desirable that the X server or master has the power to evict kernel > buffers. > > What are your thoughts about this? It'd be good to have a common > practice for upcoming drivers. > Do we need a mechanism for the X server to clean out it's own buffers > and the DRI client buffers, or should we just leave them where they are? > If we want to implement something like that we might have > drm_bo_clean_mm leave bo_kernel buffers alone instead of the whole > memory area. > So really when the X server exits, all buffers allocated by it need to go away, or at least get dereferenced, however if it has create memory managed areas these also need to go away... it should not be destroying memory manager areas. Dave. - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
Re: drm: Branch 'master' - 2 commits
Dave Airlie wrote: > libdrm/xf86drm.c | 15 ++ > libdrm/xf86mm.h |1 > linux-core/drm_bo.c | 67 > +++--- > linux-core/drm_drv.c |2 + > linux-core/drm_objects.h |8 - > shared-core/drm.h |6 > shared-core/nouveau_mem.c |6 ++-- > 7 files changed, 90 insertions(+), 15 deletions(-) > > New commits: > commit d5c0101252e9f48ef1b59f48c05fea7007df97f0 > Author: Dave Airlie <[EMAIL PROTECTED]> > Date: Mon Feb 18 10:39:21 2008 +1000 > > ttm: make sure userspace can't destroy kernel create memory managers > > this adds something to say the kernel initialised the memory region not > the userspace. and blocks userspace from deallocating kernel areas > Dave, I guess this commit is a fix for the fact that the X server will try to evict fb scanout buffers when leaving VT. We've solved this issue so far by having the fb layer evict its scanouts when the X server starts before it goes on allocating its own buffers. This was done using device-private IOCTLS. While this is probably desirable to save GTT / VRAM space, I agree it's not desirable that the X server or master has the power to evict kernel buffers. What are your thoughts about this? It'd be good to have a common practice for upcoming drivers. Do we need a mechanism for the X server to clean out it's own buffers and the DRI client buffers, or should we just leave them where they are? If we want to implement something like that we might have drm_bo_clean_mm leave bo_kernel buffers alone instead of the whole memory area. /Thomas - This SF.net email is sponsored by: Microsoft Defy all challenges. Microsoft(R) Visual Studio 2008. http://clk.atdmt.com/MRT/go/vse012070mrt/direct/01/ -- ___ Dri-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/dri-devel
