From owner-freebsd-current@FreeBSD.ORG Mon Sep 15 06:55:31 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 8330A106564A; Mon, 15 Sep 2008 06:55:31 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from mail.terabit.net.ua (mail.terabit.net.ua [195.137.202.147]) by mx1.freebsd.org (Postfix) with ESMTP id 1B98F8FC19; Mon, 15 Sep 2008 06:55:31 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from skuns.zoral.com.ua ([91.193.166.194] helo=mail.zoral.com.ua) by mail.terabit.net.ua with esmtp (Exim 4.63 (FreeBSD)) (envelope-from ) id 1Kf7zc-000IZ8-U5; Mon, 15 Sep 2008 09:55:29 +0300 Received: from deviant.kiev.zoral.com.ua (root@deviant.kiev.zoral.com.ua [10.1.1.148]) by mail.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m8F6tPnn025319 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Mon, 15 Sep 2008 09:55:25 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: from deviant.kiev.zoral.com.ua (kostik@localhost [127.0.0.1]) by deviant.kiev.zoral.com.ua (8.14.2/8.14.2) with ESMTP id m8F6tP10045494; Mon, 15 Sep 2008 09:55:25 +0300 (EEST) (envelope-from kostikbel@gmail.com) Received: (from kostik@localhost) by deviant.kiev.zoral.com.ua (8.14.3/8.14.3/Submit) id m8F6tPhQ045491; Mon, 15 Sep 2008 09:55:25 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: deviant.kiev.zoral.com.ua: kostik set sender to kostikbel@gmail.com using -f Date: Mon, 15 Sep 2008 09:55:25 +0300 From: Kostik Belousov To: Robert Watson Message-ID: <20080915065525.GD39652@deviant.kiev.zoral.com.ua> References: <20080914174801.GC39652@deviant.kiev.zoral.com.ua> Mime-Version: 1.0 Content-Type: multipart/signed; micalg=pgp-sha1; protocol="application/pgp-signature"; boundary="SlNwLc1tujQOaj7L" Content-Disposition: inline In-Reply-To: User-Agent: Mutt/1.4.2.3i X-Virus-Scanned: ClamAV version 0.93.3, clamav-milter version 0.93.3 on skuns.kiev.zoral.com.ua X-Virus-Status: Clean X-Spam-Status: No, score=-4.4 required=5.0 tests=ALL_TRUSTED,AWL,BAYES_00 autolearn=ham version=3.2.5 X-Spam-Checker-Version: SpamAssassin 3.2.5 (2008-06-10) on skuns.kiev.zoral.com.ua X-Virus-Scanned: mail.terabit.net.ua 1Kf7zc-000IZ8-U5 0ee66110025d9f4fd7716939e3741e0d X-Terabit: YES 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: Mon, 15 Sep 2008 06:55:31 -0000 --SlNwLc1tujQOaj7L Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Sun, Sep 14, 2008 at 08:15:04PM +0100, Robert Watson wrote: >=20 > On Sun, 14 Sep 2008, Kostik Belousov wrote: >=20 > >When implementing cdevpriv, I completeley missed the support for d_mmap(= )=20 > >driver method. This method is called by the kernel (at least) twice: one= =20 > >time to validate the mmaped region and protection mode (see=20 > >vm/device_pager.c:dev_pager_alloc()), and second time to obtain the=20 > >physical address when serving page fault (see dev_pager_getpages()). > > > >Support for cdevpriv for the first call(s) is trivial, and is implemente= d=20 > >by the patch below. Second calls are much harder and essentially require= =20 > >attaching cdevpriv bookkeeping data to the struct vm_map_entry. In fact,= I=20 > >am not sure whether this support for the second time calls is needed at= =20 > >all in real usage. > > > >I Cc:ed people that pointed me to the issue, please evaluate the patch= =20 > >against your needs. I think I will commit it shortly after your feedback= .=20 > >On the other hand, I would think about implementing full support for=20 > >d_mmap only if actually needed. >=20 > I'm not really sure that these changes "make sense" given the way our=20 > device pager works right now. My understanding is that most consumers of= =20 > cdevpriv use it in order to provide session-centric device nodes as a=20 > cleaner alternative to cloning. However, even with your change, it's not= =20 > possible to support session-centric memory mapping of devices as the memo= ry=20 > map and device pager code assumes there is one VM object for each device,= =20 > and hence all pages mapped independently from different file descriptors = on=20 > the same device are from the same set of pages (and hence in the same VM= =20 > object page cache). In order to implement session-centric semantics, I= =20 > think it's actually quite a bit more complicated than just adding=20 > vm_map_entry book-keeping -- we also need to have a different VM object f= or=20 > each session. >=20 > And, arguably, we need a more mature device_pager that understands that= =20 > pages might someday no longer be associated with a device due to that=20 > device being removed... The issues you raised are obviously important, but IMHO they are ortogonal to the cdevpriv KPI working in for _any_ pager. Pager' getpages method is usually called from the context where kernel does not have naturally supplied filedescriptor. For instance, most typical caller if vm_fault(). Thus, whatever pager is used, we have to provide a way for kernel to somehow associate struct file with faulted page (region), and make that file available to the pager. [Hmm, because kernel is allowed to fault too, vm_fault() should save/restore td_fpop.] Another point is that we have important consumers of the existing device pager interface that already want to use cdevpriv. Their usage ATM is limited to authentification, whatever it means. I assume it means checking that the caller was given some token the validation step. The code should be structured approx. like this: dri_mmap(...) { some_dri_data *p; int error; error =3D devfs_get_cdevpriv(&p); if (error =3D=3D 0) { /* authenticate; */ } /* * Auth successfull or error =3D=3D EBADF * Do whatever needed to return phys address */ ... } And, the last issue you raised, the need for the new pager is actually real for GEM/TTM or whatever the newest DRI interface is called. I have an intent to play with it, but more smart thing would be to wait some time more. Hopefully, then DRI folks will finally decide on the (more) stable interface. I am sure that it would be changed several dozen times in the future, but have a hope that it will not be designed from scratch. --SlNwLc1tujQOaj7L Content-Type: application/pgp-signature Content-Disposition: inline -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.9 (FreeBSD) iEYEARECAAYFAkjOBtwACgkQC3+MBN1Mb4iaRwCglRl560YvTHhWavJeI7Ih0C7C MJcAnjX9IubccQVwoYmUamaJ0oPBP3wO =p2bF -----END PGP SIGNATURE----- --SlNwLc1tujQOaj7L--