From owner-freebsd-current@FreeBSD.ORG Sun Sep 14 19:15:05 2008 Return-Path: Delivered-To: current@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 74410106567C; Sun, 14 Sep 2008 19:15:05 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from cyrus.watson.org (unknown [65.122.17.42]) by mx1.freebsd.org (Postfix) with ESMTP id 453F48FC13; Sun, 14 Sep 2008 19:15:05 +0000 (UTC) (envelope-from rwatson@FreeBSD.org) Received: from fledge.watson.org (fledge.watson.org [65.122.17.41]) by cyrus.watson.org (Postfix) with ESMTP id DEC4346C46; Sun, 14 Sep 2008 15:15:04 -0400 (EDT) Date: Sun, 14 Sep 2008 20:15:04 +0100 (BST) From: Robert Watson X-X-Sender: robert@fledge.watson.org To: Kostik Belousov In-Reply-To: <20080914174801.GC39652@deviant.kiev.zoral.com.ua> Message-ID: References: <20080914174801.GC39652@deviant.kiev.zoral.com.ua> User-Agent: Alpine 1.10 (BSF 962 2008-03-14) MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: rnoland@freebsd.org, current@freebsd.org, "Yair K." , vehemens Subject: Re: cdevpriv and mmap(2) X-BeenThere: freebsd-current@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Discussions about the use of FreeBSD-current List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 14 Sep 2008 19:15:05 -0000 On Sun, 14 Sep 2008, Kostik Belousov wrote: > When implementing cdevpriv, I completeley missed the support for d_mmap() > driver method. This method is called by the kernel (at least) twice: one > time to validate the mmaped region and protection mode (see > vm/device_pager.c:dev_pager_alloc()), and second time to obtain the physical > address when serving page fault (see dev_pager_getpages()). > > Support for cdevpriv for the first call(s) is trivial, and is implemented by > the patch below. Second calls are much harder and essentially require > attaching cdevpriv bookkeeping data to the struct vm_map_entry. In fact, I > am not sure whether this support for the second time calls is needed at all > in real usage. > > I Cc:ed people that pointed me to the issue, please evaluate the patch > against your needs. I think I will commit it shortly after your feedback. On > the other hand, I would think about implementing full support for d_mmap > only if actually needed. I'm not really sure that these changes "make sense" given the way our device pager works right now. My understanding is that most consumers of cdevpriv use it in order to provide session-centric device nodes as a cleaner alternative to cloning. However, even with your change, it's not possible to support session-centric memory mapping of devices as the memory map and device pager code assumes there is one VM object for each device, and hence all pages mapped independently from different file descriptors on the same device are from the same set of pages (and hence in the same VM object page cache). In order to implement session-centric semantics, I think it's actually quite a bit more complicated than just adding vm_map_entry book-keeping -- we also need to have a different VM object for each session. And, arguably, we need a more mature device_pager that understands that pages might someday no longer be associated with a device due to that device being removed... Robert N M Watson Computer Laboratory University of Cambridge > > Thanks ! > > diff --git a/sys/vm/vm_mmap.c b/sys/vm/vm_mmap.c > index 7e6b04f..c3f08b0 100644 > --- a/sys/vm/vm_mmap.c > +++ b/sys/vm/vm_mmap.c > @@ -391,8 +391,10 @@ map: > goto done; > } > > + td->td_fpop = fp; > error = vm_mmap(&vms->vm_map, &addr, size, prot, maxprot, > flags, handle_type, handle, pos); > + td->td_fpop = NULL; > #ifdef HWPMC_HOOKS > /* inform hwpmc(4) if an executable is being mapped */ > if (error == 0 && handle_type == OBJT_VNODE && >