Date: Wed, 29 Aug 2012 06:55:26 +0200 From: Luigi Rizzo <rizzo@iet.unipi.it> To: Konstantin Belousov <kostikbel@gmail.com> Cc: Andriy Gapon <avg@freebsd.org>, freebsd-current@freebsd.org, current@freebsd.org Subject: Re: per file descriptor device callbacks ? Message-ID: <20120829045526.GA75216@onelab2.iet.unipi.it> In-Reply-To: <20120829041240.GT33100@deviant.kiev.zoral.com.ua> References: <20120827073403.GA49223@onelab2.iet.unipi.it> <201208271227.54785.jhb@freebsd.org> <20120828155025.GA66068@onelab2.iet.unipi.it> <201208281240.29612.jhb@freebsd.org> <20120828172606.GR33100@deviant.kiev.zoral.com.ua> <20120828184226.GB68683@onelab2.iet.unipi.it> <20120829041240.GT33100@deviant.kiev.zoral.com.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
On Wed, Aug 29, 2012 at 07:12:40AM +0300, Konstantin Belousov wrote: > On Tue, Aug 28, 2012 at 08:42:26PM +0200, Luigi Rizzo wrote: > > On Tue, Aug 28, 2012 at 08:26:06PM +0300, Konstantin Belousov wrote: > > ... > > > > dev_clone() is rather gross and a lot harder to use than > > > > devfs_set_cdevpriv(). If you are fine with the inherent problems > > > > of the device pager (you can't ever make mappings go away), you can > > > > just assign each client a unique offset into your shared object's > > > > memory space. However, if you are exporting shared memory buffers, > > > > then a better model might be to let your clients use > > > > shm_open(SHM_ANON) to create buffers, then pass them into your driver > > > > via an ioctl() and use shm_map() to map them into the kernel. > > > > > > Well, there is a new OBJT_MGTDEVICE pager, which together with > > > d_mmap_single() allows to have even per-mapping data. i915kms uses it. > > > You do not need cdevpriv for the per-mapping data. > > > > > > Also, MGTDEVICE does track the mappings of the pages, so you can clean > > > up on device destruction. > > > > interesting, thanks for the pointer, i'll look it up in i915kms. > > Does this exist also in stable/9 ? > > It would be a shame otherwise... > Yes, it was merged. > > > > > > Normal callbacks of the device pager allows to execute ctr/dtr methods > > > at the time of mapping creation and tear down. > > > > what would be a good way to install my own ctr/dtr methods ? > > I only found out a rather crude one, and it only works for > > the destructor: > See below. > > > > > static struct cdev_pager_ops saved_cdev_pager_ops; > > static struct cdev_pager_ops netmap_cdev_pager_ops; > > > > static void > > netmap_dev_pager_dtor(void *handle) > > { > > saved_cdev_pager_ops.cdev_pg_dtor(handle); > > // my code here > > D("ready to release memory for %p", handle); > > } > > > > > > static int > > netmap_mmap_single(struct cdev *cdev, vm_ooffset_t *foff, > > vm_size_t objsize, vm_object_t *objp, int prot) > > { > > vm_object_t obj; > > > > /* XXX check size etc. */ > > obj = vm_pager_allocate(OBJT_DEVICE, cdev, objsize, prot, *foff, > > curthread->td_ucred); > Use cdev_pager_allocate(). > > > if (obj == NULL) > > return EINVAL; > > if (saved_cdev_pager_ops.cdev_pg_fault == NULL) { > I do not understand what are you trying to accomplish with the > check and reinitialization, but I assume that cdev_pager_allocate() > would take care of it. First and foremost, I am trying to do things without requiring kernel modifications. I am trying to reuse the constructor and destructor of the standard device pager, and around those add my own calls. Those methods are declared static in sys/vm/device_pager.c so i cannot invoke directly cdev_pager_allocate(). I could indeed rewrite the body of those three methods (ctor, dtor, fault) in my own code. I will look at this today. Perhaps I could even try to install all mappings at mmap() time so I never need to fault. Thanks again for the suggestions cheers luigi > > D("initialize cdev_pager_ops"); > > saved_cdev_pager_ops = *(obj->un_pager.devp.ops); > > netmap_cdev_pager_ops = *(obj->un_pager.devp.ops); > > netmap_cdev_pager_ops.cdev_pg_dtor = netmap_dev_pager_dtor; > > }; > > obj->un_pager.devp.ops = &netmap_cdev_pager_ops; > > *objp = obj; > > /* XXX perhaps do something more here, such as install > > * mappings for the pages so we have no faults later. > > */ > > return 0; > > } > > > > static struct cdevsw netmap_cdevsw = { > > .d_version = D_VERSION, > > .d_name = "netmap", > > .d_open = netmap_open, > > .d_mmap = netmap_mmap, > > .d_mmap_single = netmap_mmap_single, > > .d_ioctl = netmap_ioctl, > > .d_poll = netmap_poll, > > .d_close = netmap_close, > > }; > > > > cheers > > luigi
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20120829045526.GA75216>