Date: Mon, 31 Oct 2016 11:16:57 -0700 From: Mark Johnston <markj@FreeBSD.org> To: =?iso-8859-1?Q?Jean-S=E9bastien_P=E9dron?= <dumbbell@FreeBSD.org> Cc: "freebsd-hackers@freebsd.org" <freebsd-hackers@freebsd.org> Subject: Re: Linux' struct address_space & FreeBSD's vm_object Message-ID: <20161031181657.GB92661@wkstn-mjohnston.west.isilon.com> In-Reply-To: <b5fa5297-cff0-1f20-633e-95facf801bae@FreeBSD.org> References: <b5fa5297-cff0-1f20-633e-95facf801bae@FreeBSD.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Oct 29, 2016 at 11:37:13PM +0200, Jean-Sébastien Pédron wrote: > Hi! > > I'm tracking a memory leak in the drm-next-4.7 branch [1]. I found the > issue, however, I'm unsure of the solution for now. Let me sum up what I > understand (or what I think I understand :): > > In Linux, they use a `struct vm_area_struct` to represent a mapping of > an object. It holds the callback functions (open, close and fault) of > the device driver and the private data to be used with those callbacks. > > All `struct vm_area_struct` are stored in a tree in another structure > called `struct address_space` which belongs to the owner of the resource > (an inode in the case of DRM). This structure holds references to pages > loaded from the inode, so it acts as a page cache. > > So: > struct inode > `-- struct address_space > |-- tree of pages > `-- tree of struct vm_area_struct > > In DRM, there is a `struct vm_area_struct` for each mapping of each > graphics object. But those mapping are all stored in the same `struct > address_space` belonging to an "anonymous inode" attached to the device. > Furthermore, a DRM driver creates three character devices in /dev for > each real device, and all three character devices use this same > anonymous inode. > > Therefore, if I understand correctly, all mappings for all three > character devices use the same list of pages. Thus the memory is shared. > > In DRM, when a mapping must be released, eg. i915_gem_release_mmap() > indirectly calls unmap_mapping_range() with the anonymous inode's > `struct address_space`. This function removes all mappings of a given > graphics object, thus removes all `struct vm_area_struct` from `struct > address` which are covered by the specified range. > > Currently, on FreeBSD, `struct address_space` is replaced by the > vm_object returned by cdev_pager_allocate(). The first issue is that we > never create the equivalent of `struct address_space` for the global > anonymous inode. Therefore the code responsible for removing mappings > does nothing and mappings & pages are leaked. Anyway, the d_mmap_single > implementation doesn't even try to fill the equivalent of `struct > address_space`. > > So that's my understanding of the issue. First, I'm not 100% sure of > what I described and second, I don't see how to implement the same > shared page cache in FreeBSD because a device pager vm_object can't be > shared by multiple mappings (or can it?). I don't see a reason that an OBJT_MGTDEVICE object can't be mapped into multiple address spaces. Am I missing something? I'm wondering if linux_dev_mmap_single() could give individual drivers a chance to return an object via the mmap method in struct file_operations, and only fall back to allocating an OBJT_SG object in the default case. Then the code for the three cdevs could be made to return the same OBJT_MGTDEVICE object, rather than the situation today where linux_dev_mmap_single() itself allocates objects for each cdev.
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20161031181657.GB92661>