From owner-freebsd-hackers@freebsd.org Mon Oct 31 18:11:00 2016 Return-Path: Delivered-To: freebsd-hackers@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:1900:2254:206a::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 44868C16A98 for ; Mon, 31 Oct 2016 18:11:00 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: from mail-pf0-x22e.google.com (mail-pf0-x22e.google.com [IPv6:2607:f8b0:400e:c00::22e]) (using TLSv1.2 with cipher ECDHE-RSA-AES128-GCM-SHA256 (128/128 bits)) (Client CN "smtp.gmail.com", Issuer "Google Internet Authority G2" (verified OK)) by mx1.freebsd.org (Postfix) with ESMTPS id 107BB1ABD; Mon, 31 Oct 2016 18:11:00 +0000 (UTC) (envelope-from markjdb@gmail.com) Received: by mail-pf0-x22e.google.com with SMTP id d2so5337498pfd.0; Mon, 31 Oct 2016 11:11:00 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=sender:date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to :user-agent; bh=4Kkwknp2SqMXfmu0LrLf2PKTkALmF8V20p/8d1fsOPQ=; b=nkr9NQl3jEdkTc6Mbh58Bg686lvI1TANw4OzT8mUXwrgLvHd7JV5hOkNwAouyiq/Fa WETVwFz32B6swLql38xsP+Cd12HAiwMzULYDZQflAHojp0PH8p/NKMp885iwKVHySb/5 /goY43XjWIs/ak5kOIpGN/XkZxgEEQMprRF34Du7wefDZ2428uE0hlsgQkQtIhi56vyA 9zIMG3GVxSfZtG5KCf2+MN06TJI/pL33GLrQQotuWXu6jpawOH4XbF+7jzlYgsu4Nv5D Z01oMkABPh2yIFxxKdec0vj6F8muHVcf3YqdxKTuF1TXNi09uGiZ3LvjJhOGyLRlvgrt 5yGg== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20130820; h=x-gm-message-state:sender:date:from:to:cc:subject:message-id :references:mime-version:content-disposition :content-transfer-encoding:in-reply-to:user-agent; bh=4Kkwknp2SqMXfmu0LrLf2PKTkALmF8V20p/8d1fsOPQ=; b=aDRdaQXVLSdo9ivjIEHNs7kzajFI7l1kKZLkkz0/izU5XMgaIpiKPBPhBWJN0VEhBo 9Lzh1npzGhNb+57MHrMHm2pAq1QKX1DBiJvIEHAFb4lCMd3Ejgq0s7SslIdV4hDuZyz6 J2pTSWnCqLEVfzNJb23idKxlYG3kHeVjPMFJlnQB8nJIx9+e7x8S3Pcr37PyshTbzMiA 44UEEZb4IKQ66oLpeV+pRiKbMgjsK1OjBCwCkymRVyWmZ577I4k2K591F4DzgDmqO4m/ lGHKLocjuDD7opwOQs/7dZeIu1vSKJz5gtZUC56knSObqEnwxtZpc/SdxFSLfehJS5ok inaQ== X-Gm-Message-State: ABUngvdf+yVfs7KK8qXgxxxyp1lCxHGv/LcmeHgkzhhThJJgYx2EMYvXoXkQ5PC/ormZkA== X-Received: by 10.98.211.24 with SMTP id q24mr51503524pfg.69.1477937459375; Mon, 31 Oct 2016 11:10:59 -0700 (PDT) Received: from wkstn-mjohnston.west.isilon.com (c-76-104-201-218.hsd1.wa.comcast.net. [76.104.201.218]) by smtp.gmail.com with ESMTPSA id t5sm37006750pfb.58.2016.10.31.11.10.57 (version=TLS1_2 cipher=ECDHE-RSA-AES128-GCM-SHA256 bits=128/128); Mon, 31 Oct 2016 11:10:58 -0700 (PDT) Sender: Mark Johnston Date: Mon, 31 Oct 2016 11:16:57 -0700 From: Mark Johnston To: =?iso-8859-1?Q?Jean-S=E9bastien_P=E9dron?= Cc: "freebsd-hackers@freebsd.org" Subject: Re: Linux' struct address_space & FreeBSD's vm_object Message-ID: <20161031181657.GB92661@wkstn-mjohnston.west.isilon.com> References: MIME-Version: 1.0 Content-Type: text/plain; charset=iso-8859-1 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: User-Agent: Mutt/1.7.1 (2016-10-04) X-BeenThere: freebsd-hackers@freebsd.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: Technical Discussions relating to FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 31 Oct 2016 18:11:00 -0000 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.