On 11/12/19 7:25 AM, Christoph Hellwig wrote:
Shouldn't this go into mm/ instead? It certainly doesn't seem
like a library.
I was following the convention for the other vm test kernel modules.
I see a couple of modules in mm/ but I don't have a personal
preference for where to place it.
Andrew, do you have a preference?
+static int dmirror_bounce_copy_from(struct dmirror_bounce *bounce,
+ unsigned long addr)
+{
+ unsigned long end = addr + bounce->size;
+ char __user *uptr = (void __user *)addr;
+ void *ptr = bounce->ptr;
+
+ for (; addr < end; addr += PAGE_SIZE, ptr += PAGE_SIZE,
+ uptr += PAGE_SIZE) {
+ int ret;
+
+ ret = copy_from_user(ptr, uptr, PAGE_SIZE);
+ if (ret)
+ return ret;
+ }
+
+ return 0;
+}
Why does this iterate in page sized chunks? I don't remember a page
size limit on copy_{from,to}_user.
Good point. I'll fix that.
+static int dmirror_invalidate_range_start(struct mmu_notifier *mn,
+ const struct mmu_notifier_range *update)
+{
+ struct dmirror *dmirror = container_of(mn, struct dmirror, notifier);
+
+ if (mmu_notifier_range_blockable(update))
+ mutex_lock(&dmirror->mutex);
+ else if (!mutex_trylock(&dmirror->mutex))
+ return -EAGAIN;
+
+ dmirror_do_update(dmirror, update->start, update->end);
+ mutex_unlock(&dmirror->mutex);
+ return 0;
+}
Can we adopts this to Jasons new interval tree invalidate?
Well, it would mean registering for the whole process address space.
I'll give it a try.
+static int dmirror_fops_open(struct inode *inode, struct file *filp)
+{
+ struct cdev *cdev = inode->i_cdev;
+ struct dmirror_device *mdevice;
+ struct dmirror *dmirror;
+
+ /* No exclusive opens. */
+ if (filp->f_flags & O_EXCL)
+ return -EINVAL;
Device files usually just ignore O_EXCL, I don't see why this one
would be any different.
OK, I'll remove that test.
+ mdevice = container_of(cdev, struct dmirror_device, cdevice);
+ dmirror = dmirror_new(mdevice);
+ if (!dmirror)
+ return -ENOMEM;
+
+ /* Only the first open registers the address space. */
+ mutex_lock(&mdevice->devmem_lock);
+ if (filp->private_data)
+ goto err_busy;
+ filp->private_data = dmirror;
+ mutex_unlock(&mdevice->devmem_lock);
->open is only called for the first open of a given file structure..
+static int dmirror_fops_release(struct inode *inode, struct file *filp)
+{
+ struct dmirror *dmirror = filp->private_data;
+
+ if (!dmirror)
+ return 0;
This can't happen if your ->open never returns 0 without setting the
private data.
+ filp->private_data = NULL;
The file is feed afterwards, no need to clear the private data.
OK, I'll clean that up.
