From f1ee616214cb22410e939d963bbb2349c2570f02 Mon Sep 17 00:00:00 2001 From: NeilBrown Date: Thu, 21 Dec 2017 09:45:40 +1100 Subject: VFS: don't keep disconnected dentries on d_anon The original purpose of the per-superblock d_anon list was to keep disconnected dentries in the cache between consecutive requests to the NFS server. Dentries can be disconnected if a client holds a file open and repeatedly performs IO on it, and if the server drops the dentry, whether due to memory pressure, server restart, or "echo 3 > /proc/sys/vm/drop_caches". This purpose was thwarted by commit 75a6f82a0d10 ("freeing unlinked file indefinitely delayed") which caused disconnected dentries to be freed as soon as their refcount reached zero. This means that, when a dentry being used by nfsd gets disconnected, a new one needs to be allocated for every request (unless requests overlap). As the dentry has no name, no parent, and no children, there is little of value to cache. As small memory allocations are typically fast (from per-cpu free lists) this likely has little cost. This means that the original purpose of s_anon is no longer relevant: there is no longer any need to keep disconnected dentries on a list so they appear to be hashed. However, s_anon now has a new use. When you mount an NFS filesystem, the dentry stored in s_root is just a placebo. The "real" root dentry is allocated using d_obtain_root() and so it kept on the s_anon list. I don't know the reason for this, but suspect it related to NFSv4 where a mount of "server:/some/path" require NFS to look up the root filehandle on the server, then walk down "/some" and "/path" to get the filehandle to mount. Whatever the reason, NFS depends on the s_anon list and on shrink_dcache_for_umount() pruning all dentries on this list. So we cannot simply remove s_anon. We could just leave the code unchanged, but apart from that being potentially confusing, the (unfair) bit-spin-lock which protects s_anon can become a bottle neck when lots of disconnected dentries are being created. So this patch renames s_anon to s_roots, and stops storing disconnected dentries on the list. Only dentries obtained with d_obtain_root() are now stored on this list. There are many fewer of these (only NFS and NILFS2 use the call, and only during filesystem mount) so contention on the bit-lock will not be a problem. Possibly an alternate solution should be found for NFS and NILFS2, but that would require understanding their needs first. Signed-off-by: NeilBrown Signed-off-by: Al Viro --- drivers/staging/lustre/lustre/llite/llite_internal.h | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) (limited to 'drivers') diff --git a/drivers/staging/lustre/lustre/llite/llite_internal.h b/drivers/staging/lustre/lustre/llite/llite_internal.h index b133fd00c08c..0d62fcf016dc 100644 --- a/drivers/staging/lustre/lustre/llite/llite_internal.h +++ b/drivers/staging/lustre/lustre/llite/llite_internal.h @@ -1296,15 +1296,7 @@ static inline void d_lustre_invalidate(struct dentry *dentry, int nested) spin_lock_nested(&dentry->d_lock, nested ? DENTRY_D_LOCK_NESTED : DENTRY_D_LOCK_NORMAL); ll_d2d(dentry)->lld_invalid = 1; - /* - * We should be careful about dentries created by d_obtain_alias(). - * These dentries are not put in the dentry tree, instead they are - * linked to sb->s_anon through dentry->d_hash. - * shrink_dcache_for_umount() shrinks the tree and sb->s_anon list. - * If we unhashed such a dentry, unmount would not be able to find - * it and busy inodes would be reported. - */ - if (d_count(dentry) == 0 && !(dentry->d_flags & DCACHE_DISCONNECTED)) + if (d_count(dentry) == 0) __d_drop(dentry); spin_unlock(&dentry->d_lock); } -- cgit v1.2.3 From 3d26759c09da7242db6ae18ce9a0f6b09bfeafcc Mon Sep 17 00:00:00 2001 From: Al Viro Date: Wed, 27 Dec 2017 19:00:09 -0500 Subject: r128: don't open-code memdup_user() Signed-off-by: Al Viro --- drivers/gpu/drm/r128/r128_state.c | 23 ++++++----------------- 1 file changed, 6 insertions(+), 17 deletions(-) (limited to 'drivers') diff --git a/drivers/gpu/drm/r128/r128_state.c b/drivers/gpu/drm/r128/r128_state.c index 8fdc56c1c953..b9bfa806d346 100644 --- a/drivers/gpu/drm/r128/r128_state.c +++ b/drivers/gpu/drm/r128/r128_state.c @@ -982,25 +982,14 @@ static int r128_cce_dispatch_write_pixels(struct drm_device *dev, xbuf_size = count * sizeof(*x); ybuf_size = count * sizeof(*y); - x = kmalloc(xbuf_size, GFP_KERNEL); - if (x == NULL) - return -ENOMEM; - y = kmalloc(ybuf_size, GFP_KERNEL); - if (y == NULL) { - kfree(x); - return -ENOMEM; - } - if (copy_from_user(x, depth->x, xbuf_size)) { - kfree(x); - kfree(y); - return -EFAULT; - } - if (copy_from_user(y, depth->y, xbuf_size)) { + x = memdup_user(depth->x, xbuf_size); + if (IS_ERR(x)) + return PTR_ERR(x); + y = memdup_user(depth->y, ybuf_size); + if (IS_ERR(y)) { kfree(x); - kfree(y); - return -EFAULT; + return PTR_ERR(y); } - buffer_size = depth->n * sizeof(u32); buffer = memdup_user(depth->buffer, buffer_size); if (IS_ERR(buffer)) { -- cgit v1.2.3 From 14544d7690f674ce2b7aa9dc6531c244cc861d19 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 29 Dec 2017 16:12:28 -0500 Subject: vme_user: don't use __copy_..._user() Saving access_ok() is not worth the trouble; yes, the callers of ->read() and ->write() will have done the right checks, but it's much too long (and varied) call chains to rely upon. Signed-off-by: Al Viro --- drivers/staging/vme/devices/vme_user.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) (limited to 'drivers') diff --git a/drivers/staging/vme/devices/vme_user.c b/drivers/staging/vme/devices/vme_user.c index a3d4610fbdbe..4c8c6fa0a79f 100644 --- a/drivers/staging/vme/devices/vme_user.c +++ b/drivers/staging/vme/devices/vme_user.c @@ -134,7 +134,7 @@ static ssize_t resource_to_user(int minor, char __user *buf, size_t count, if (copied < 0) return (int)copied; - if (__copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied)) + if (copy_to_user(buf, image[minor].kern_buf, (unsigned long)copied)) return -EFAULT; return copied; @@ -146,7 +146,7 @@ static ssize_t resource_from_user(unsigned int minor, const char __user *buf, if (count > image[minor].size_buf) count = image[minor].size_buf; - if (__copy_from_user(image[minor].kern_buf, buf, (unsigned long)count)) + if (copy_from_user(image[minor].kern_buf, buf, (unsigned long)count)) return -EFAULT; return vme_master_write(image[minor].resource, image[minor].kern_buf, @@ -159,7 +159,7 @@ static ssize_t buffer_to_user(unsigned int minor, char __user *buf, void *image_ptr; image_ptr = image[minor].kern_buf + *ppos; - if (__copy_to_user(buf, image_ptr, (unsigned long)count)) + if (copy_to_user(buf, image_ptr, (unsigned long)count)) return -EFAULT; return count; @@ -171,7 +171,7 @@ static ssize_t buffer_from_user(unsigned int minor, const char __user *buf, void *image_ptr; image_ptr = image[minor].kern_buf + *ppos; - if (__copy_from_user(image_ptr, buf, (unsigned long)count)) + if (copy_from_user(image_ptr, buf, (unsigned long)count)) return -EFAULT; return count; -- cgit v1.2.3 From 18e2ea5cd00c35f4a3e7ba3ef261c38643ef2af3 Mon Sep 17 00:00:00 2001 From: Al Viro Date: Fri, 29 Dec 2017 16:25:30 -0500 Subject: uvc_v4l2: clean copyin/copyout up Signed-off-by: Al Viro --- drivers/media/usb/uvc/uvc_v4l2.c | 55 ++++++++++++++++------------------------ 1 file changed, 22 insertions(+), 33 deletions(-) (limited to 'drivers') diff --git a/drivers/media/usb/uvc/uvc_v4l2.c b/drivers/media/usb/uvc/uvc_v4l2.c index 3e7e283a44a8..329a967382dd 100644 --- a/drivers/media/usb/uvc/uvc_v4l2.c +++ b/drivers/media/usb/uvc/uvc_v4l2.c @@ -1284,36 +1284,30 @@ struct uvc_xu_control_mapping32 { static int uvc_v4l2_get_xu_mapping(struct uvc_xu_control_mapping *kp, const struct uvc_xu_control_mapping32 __user *up) { - compat_caddr_t p; + struct uvc_xu_control_mapping32 *p = (void *)kp; + compat_caddr_t info; + u32 count; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - __copy_from_user(kp, up, offsetof(typeof(*up), menu_info)) || - __get_user(kp->menu_count, &up->menu_count)) + if (copy_from_user(p, up, sizeof(*p))) return -EFAULT; - memset(kp->reserved, 0, sizeof(kp->reserved)); - - if (kp->menu_count == 0) { - kp->menu_info = NULL; - return 0; - } - - if (__get_user(p, &up->menu_info)) - return -EFAULT; - kp->menu_info = compat_ptr(p); + count = p->menu_count; + info = p->menu_info; + memset(kp->reserved, 0, sizeof(kp->reserved)); + kp->menu_info = count ? compat_ptr(info) : NULL; + kp->menu_count = count; return 0; } static int uvc_v4l2_put_xu_mapping(const struct uvc_xu_control_mapping *kp, struct uvc_xu_control_mapping32 __user *up) { - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - __copy_to_user(up, kp, offsetof(typeof(*up), menu_info)) || - __put_user(kp->menu_count, &up->menu_count)) + if (copy_to_user(up, kp, offsetof(typeof(*up), menu_info)) || + put_user(kp->menu_count, &up->menu_count)) return -EFAULT; - if (__clear_user(up->reserved, sizeof(up->reserved))) + if (clear_user(up->reserved, sizeof(up->reserved))) return -EFAULT; return 0; @@ -1330,31 +1324,26 @@ struct uvc_xu_control_query32 { static int uvc_v4l2_get_xu_query(struct uvc_xu_control_query *kp, const struct uvc_xu_control_query32 __user *up) { - compat_caddr_t p; + struct uvc_xu_control_query32 v; - if (!access_ok(VERIFY_READ, up, sizeof(*up)) || - __copy_from_user(kp, up, offsetof(typeof(*up), data))) + if (copy_from_user(&v, up, sizeof(v))) return -EFAULT; - if (kp->size == 0) { - kp->data = NULL; - return 0; - } - - if (__get_user(p, &up->data)) - return -EFAULT; - kp->data = compat_ptr(p); - + *kp = (struct uvc_xu_control_query){ + .unit = v.unit, + .selector = v.selector, + .query = v.query, + .size = v.size, + .data = v.size ? compat_ptr(v.data) : NULL + }; return 0; } static int uvc_v4l2_put_xu_query(const struct uvc_xu_control_query *kp, struct uvc_xu_control_query32 __user *up) { - if (!access_ok(VERIFY_WRITE, up, sizeof(*up)) || - __copy_to_user(up, kp, offsetof(typeof(*up), data))) + if (copy_to_user(up, kp, offsetof(typeof(*up), data))) return -EFAULT; - return 0; } -- cgit v1.2.3