From 3a3a5fece6f28c14d3d05c74fb7696412e53a067 Mon Sep 17 00:00:00 2001 From: Deepa Dinamani Date: Mon, 22 Feb 2016 07:17:53 -0800 Subject: fs: kernfs: Replace CURRENT_TIME by current_fs_time() This is in preparation for the series that transitions filesystem timestamps to use 64 bit time and hence make them y2038 safe. CURRENT_TIME macro will be deleted before merging the aforementioned series. Use current_fs_time() instead of CURRENT_TIME for inode timestamps. struct kernfs_node is associated with a sysfs file/ directory. Truncate the values to appropriate time granularity when writing to inode timestamps of the files. ktime_get_real_ts() is used to obtain times for struct kernfs_iattrs. Since these times are later assigned to inode times using timespec_truncate() for all filesystem based operations, we can save the supers list traversal time here by using ktime_get_real_ts() directly. Signed-off-by: Deepa Dinamani Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/dir.c | 8 +++++--- fs/kernfs/inode.c | 15 ++++++++++----- 2 files changed, 15 insertions(+), 8 deletions(-) (limited to 'fs/kernfs') diff --git a/fs/kernfs/dir.c b/fs/kernfs/dir.c index 03b688d19f69..eb2c58732bcf 100644 --- a/fs/kernfs/dir.c +++ b/fs/kernfs/dir.c @@ -753,7 +753,8 @@ int kernfs_add_one(struct kernfs_node *kn) ps_iattr = parent->iattr; if (ps_iattr) { struct iattr *ps_iattrs = &ps_iattr->ia_iattr; - ps_iattrs->ia_ctime = ps_iattrs->ia_mtime = CURRENT_TIME; + ktime_get_real_ts(&ps_iattrs->ia_ctime); + ps_iattrs->ia_mtime = ps_iattrs->ia_ctime; } mutex_unlock(&kernfs_mutex); @@ -1279,8 +1280,9 @@ static void __kernfs_remove(struct kernfs_node *kn) /* update timestamps on the parent */ if (ps_iattr) { - ps_iattr->ia_iattr.ia_ctime = CURRENT_TIME; - ps_iattr->ia_iattr.ia_mtime = CURRENT_TIME; + ktime_get_real_ts(&ps_iattr->ia_iattr.ia_ctime); + ps_iattr->ia_iattr.ia_mtime = + ps_iattr->ia_iattr.ia_ctime; } kernfs_put(pos); diff --git a/fs/kernfs/inode.c b/fs/kernfs/inode.c index 16405ae88d2d..1ac1dbae5aba 100644 --- a/fs/kernfs/inode.c +++ b/fs/kernfs/inode.c @@ -54,7 +54,10 @@ static struct kernfs_iattrs *kernfs_iattrs(struct kernfs_node *kn) iattrs->ia_mode = kn->mode; iattrs->ia_uid = GLOBAL_ROOT_UID; iattrs->ia_gid = GLOBAL_ROOT_GID; - iattrs->ia_atime = iattrs->ia_mtime = iattrs->ia_ctime = CURRENT_TIME; + + ktime_get_real_ts(&iattrs->ia_atime); + iattrs->ia_mtime = iattrs->ia_atime; + iattrs->ia_ctime = iattrs->ia_atime; simple_xattrs_init(&kn->iattr->xattrs); out_unlock: @@ -236,16 +239,18 @@ ssize_t kernfs_iop_listxattr(struct dentry *dentry, char *buf, size_t size) static inline void set_default_inode_attr(struct inode *inode, umode_t mode) { inode->i_mode = mode; - inode->i_atime = inode->i_mtime = inode->i_ctime = CURRENT_TIME; + inode->i_atime = inode->i_mtime = + inode->i_ctime = current_fs_time(inode->i_sb); } static inline void set_inode_attr(struct inode *inode, struct iattr *iattr) { + struct super_block *sb = inode->i_sb; inode->i_uid = iattr->ia_uid; inode->i_gid = iattr->ia_gid; - inode->i_atime = iattr->ia_atime; - inode->i_mtime = iattr->ia_mtime; - inode->i_ctime = iattr->ia_ctime; + inode->i_atime = timespec_trunc(iattr->ia_atime, sb->s_time_gran); + inode->i_mtime = timespec_trunc(iattr->ia_mtime, sb->s_time_gran); + inode->i_ctime = timespec_trunc(iattr->ia_ctime, sb->s_time_gran); } static void kernfs_refresh_inode(struct kernfs_node *kn, struct inode *inode) -- cgit v1.2.3 From e4234a1fc343ca35f852bc527fae56fade879d4a Mon Sep 17 00:00:00 2001 From: Chris Wilson Date: Thu, 31 Mar 2016 11:45:06 +0100 Subject: kernfs: Move faulting copy_user operations outside of the mutex MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A fault in a user provided buffer may lead anywhere, and lockdep warns that we have a potential deadlock between the mm->mmap_sem and the kernfs file mutex: [ 82.811702] ====================================================== [ 82.811705] [ INFO: possible circular locking dependency detected ] [ 82.811709] 4.5.0-rc4-gfxbench+ #1 Not tainted [ 82.811711] ------------------------------------------------------- [ 82.811714] kms_setmode/5859 is trying to acquire lock: [ 82.811717] (&dev->struct_mutex){+.+.+.}, at: [] drm_gem_mmap+0x1a1/0x270 [ 82.811731] but task is already holding lock: [ 82.811734] (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x44/0xa0 [ 82.811745] which lock already depends on the new lock. [ 82.811749] the existing dependency chain (in reverse order) is: [ 82.811752] -> #3 (&mm->mmap_sem){++++++}: [ 82.811761] [] lock_acquire+0xc3/0x1d0 [ 82.811766] [] __might_fault+0x75/0xa0 [ 82.811771] [] kernfs_fop_write+0x8a/0x180 [ 82.811787] [] __vfs_write+0x23/0xe0 [ 82.811792] [] vfs_write+0xa4/0x190 [ 82.811797] [] SyS_write+0x44/0xb0 [ 82.811801] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.811807] -> #2 (s_active#6){++++.+}: [ 82.811814] [] lock_acquire+0xc3/0x1d0 [ 82.811819] [] __kernfs_remove+0x210/0x2f0 [ 82.811823] [] kernfs_remove_by_name_ns+0x40/0xa0 [ 82.811828] [] sysfs_remove_file_ns+0x10/0x20 [ 82.811832] [] device_del+0x124/0x250 [ 82.811837] [] device_unregister+0x19/0x60 [ 82.811841] [] cpu_cache_sysfs_exit+0x51/0xb0 [ 82.811846] [] cacheinfo_cpu_callback+0x38/0x70 [ 82.811851] [] notifier_call_chain+0x39/0xa0 [ 82.811856] [] __raw_notifier_call_chain+0x9/0x10 [ 82.811860] [] cpu_notify+0x1e/0x40 [ 82.811865] [] cpu_notify_nofail+0x9/0x20 [ 82.811869] [] _cpu_down+0x233/0x340 [ 82.811874] [] disable_nonboot_cpus+0xc9/0x350 [ 82.811878] [] suspend_devices_and_enter+0x5a1/0xb50 [ 82.811883] [] pm_suspend+0x543/0x8d0 [ 82.811888] [] state_store+0x77/0xe0 [ 82.811892] [] kobj_attr_store+0xf/0x20 [ 82.811897] [] sysfs_kf_write+0x40/0x50 [ 82.811902] [] kernfs_fop_write+0x13c/0x180 [ 82.811906] [] __vfs_write+0x23/0xe0 [ 82.811910] [] vfs_write+0xa4/0x190 [ 82.811914] [] SyS_write+0x44/0xb0 [ 82.811918] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.811923] -> #1 (cpu_hotplug.lock){+.+.+.}: [ 82.811929] [] lock_acquire+0xc3/0x1d0 [ 82.811933] [] mutex_lock_nested+0x62/0x3b0 [ 82.811940] [] get_online_cpus+0x61/0x80 [ 82.811944] [] stop_machine+0x1b/0xe0 [ 82.811949] [] gen8_ggtt_insert_entries__BKL+0x2d/0x30 [i915] [ 82.812009] [] ggtt_bind_vma+0x46/0x70 [i915] [ 82.812045] [] i915_vma_bind+0x140/0x290 [i915] [ 82.812081] [] i915_gem_object_do_pin+0x899/0xb00 [i915] [ 82.812117] [] i915_gem_object_pin+0x35/0x40 [i915] [ 82.812154] [] intel_init_pipe_control+0xbe/0x210 [i915] [ 82.812192] [] intel_logical_rings_init+0xe2/0xde0 [i915] [ 82.812232] [] i915_gem_init+0xf3/0x130 [i915] [ 82.812278] [] i915_driver_load+0xf2d/0x1770 [i915] [ 82.812318] [] drm_dev_register+0xa4/0xb0 [ 82.812323] [] drm_get_pci_dev+0xce/0x1e0 [ 82.812328] [] i915_pci_probe+0x2f/0x50 [i915] [ 82.812360] [] pci_device_probe+0x87/0xf0 [ 82.812366] [] driver_probe_device+0x229/0x450 [ 82.812371] [] __driver_attach+0x83/0x90 [ 82.812375] [] bus_for_each_dev+0x61/0xa0 [ 82.812380] [] driver_attach+0x19/0x20 [ 82.812384] [] bus_add_driver+0x1ef/0x290 [ 82.812388] [] driver_register+0x5b/0xe0 [ 82.812393] [] __pci_register_driver+0x5b/0x60 [ 82.812398] [] drm_pci_init+0xd6/0x100 [ 82.812402] [] 0xffffffffa027c094 [ 82.812406] [] do_one_initcall+0xae/0x1d0 [ 82.812412] [] do_init_module+0x5b/0x1cb [ 82.812417] [] load_module+0x1c20/0x2480 [ 82.812422] [] SyS_finit_module+0x7e/0xa0 [ 82.812428] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.812433] -> #0 (&dev->struct_mutex){+.+.+.}: [ 82.812439] [] __lock_acquire+0x1fc9/0x20f0 [ 82.812443] [] lock_acquire+0xc3/0x1d0 [ 82.812456] [] drm_gem_mmap+0x1c7/0x270 [ 82.812460] [] mmap_region+0x334/0x580 [ 82.812466] [] do_mmap+0x364/0x410 [ 82.812470] [] vm_mmap_pgoff+0x6d/0xa0 [ 82.812474] [] SyS_mmap_pgoff+0x184/0x220 [ 82.812479] [] SyS_mmap+0x1d/0x20 [ 82.812484] [] entry_SYSCALL_64_fastpath+0x16/0x73 [ 82.812489] other info that might help us debug this: [ 82.812493] Chain exists of: &dev->struct_mutex --> s_active#6 --> &mm->mmap_sem [ 82.812502] Possible unsafe locking scenario: [ 82.812506] CPU0 CPU1 [ 82.812508] ---- ---- [ 82.812510] lock(&mm->mmap_sem); [ 82.812514] lock(s_active#6); [ 82.812519] lock(&mm->mmap_sem); [ 82.812522] lock(&dev->struct_mutex); [ 82.812526] *** DEADLOCK *** [ 82.812531] 1 lock held by kms_setmode/5859: [ 82.812533] #0: (&mm->mmap_sem){++++++}, at: [] vm_mmap_pgoff+0x44/0xa0 [ 82.812541] stack backtrace: [ 82.812547] CPU: 0 PID: 5859 Comm: kms_setmode Not tainted 4.5.0-rc4-gfxbench+ #1 [ 82.812550] Hardware name: /NUC5CPYB, BIOS PYBSWCEL.86A.0040.2015.0814.1353 08/14/2015 [ 82.812553] 0000000000000000 ffff880079407bf0 ffffffff813f8505 ffffffff825fb270 [ 82.812560] ffffffff825c4190 ffff880079407c30 ffffffff810c84ac ffff880079407c90 [ 82.812566] ffff8800797ed328 ffff8800797ecb00 0000000000000001 ffff8800797ed350 [ 82.812573] Call Trace: [ 82.812578] [] dump_stack+0x67/0x92 [ 82.812582] [] print_circular_bug+0x1fc/0x310 [ 82.812586] [] __lock_acquire+0x1fc9/0x20f0 [ 82.812590] [] lock_acquire+0xc3/0x1d0 [ 82.812594] [] ? drm_gem_mmap+0x1a1/0x270 [ 82.812599] [] drm_gem_mmap+0x1c7/0x270 [ 82.812603] [] ? drm_gem_mmap+0x1a1/0x270 [ 82.812608] [] mmap_region+0x334/0x580 [ 82.812612] [] do_mmap+0x364/0x410 [ 82.812616] [] vm_mmap_pgoff+0x6d/0xa0 [ 82.812629] [] SyS_mmap_pgoff+0x184/0x220 [ 82.812633] [] SyS_mmap+0x1d/0x20 [ 82.812637] [] entry_SYSCALL_64_fastpath+0x16/0x73 Highly unlikely though this scenario is, we can avoid the issue entirely by moving the copy operation from out under the kernfs_get_active() tracking by assigning the preallocated buffer its own mutex. The temporary buffer allocation doesn't require mutex locking as it is entirely local. The locked section was extended by the addition of the preallocated buf to speed up md user operations in commit 2b75869bba676c248d8d25ae6d2bd9221dfffdb6 Author: NeilBrown Date: Mon Oct 13 16:41:28 2014 +1100 sysfs/kernfs: allow attributes to request write buffer be pre-allocated. Reported-by: Ville Syrjälä Bugzilla: https://bugs.freedesktop.org/show_bug.cgi?id=94350 Signed-off-by: Chris Wilson Reviewed-by: Joonas Lahtinen Cc: Ville Syrjälä Cc: Joonas Lahtinen Cc: NeilBrown Acked-by: Tejun Heo Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 51 +++++++++++++++++++++++++++++---------------------- 1 file changed, 29 insertions(+), 22 deletions(-) (limited to 'fs/kernfs') diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index 7247252ee9b1..e1574008adc9 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -190,15 +190,16 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of, char *buf; buf = of->prealloc_buf; - if (!buf) + if (buf) + mutex_lock(&of->prealloc_mutex); + else buf = kmalloc(len, GFP_KERNEL); if (!buf) return -ENOMEM; /* * @of->mutex nests outside active ref and is used both to ensure that - * the ops aren't called concurrently for the same open file, and - * to provide exclusive access to ->prealloc_buf (when that exists). + * the ops aren't called concurrently for the same open file. */ mutex_lock(&of->mutex); if (!kernfs_get_active(of->kn)) { @@ -214,21 +215,23 @@ static ssize_t kernfs_file_direct_read(struct kernfs_open_file *of, else len = -EINVAL; + kernfs_put_active(of->kn); + mutex_unlock(&of->mutex); + if (len < 0) - goto out_unlock; + goto out_free; if (copy_to_user(user_buf, buf, len)) { len = -EFAULT; - goto out_unlock; + goto out_free; } *ppos += len; - out_unlock: - kernfs_put_active(of->kn); - mutex_unlock(&of->mutex); out_free: - if (buf != of->prealloc_buf) + if (buf == of->prealloc_buf) + mutex_unlock(&of->prealloc_mutex); + else kfree(buf); return len; } @@ -284,15 +287,22 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf, } buf = of->prealloc_buf; - if (!buf) + if (buf) + mutex_lock(&of->prealloc_mutex); + else buf = kmalloc(len + 1, GFP_KERNEL); if (!buf) return -ENOMEM; + if (copy_from_user(buf, user_buf, len)) { + len = -EFAULT; + goto out_free; + } + buf[len] = '\0'; /* guarantee string termination */ + /* * @of->mutex nests outside active ref and is used both to ensure that - * the ops aren't called concurrently for the same open file, and - * to provide exclusive access to ->prealloc_buf (when that exists). + * the ops aren't called concurrently for the same open file. */ mutex_lock(&of->mutex); if (!kernfs_get_active(of->kn)) { @@ -301,26 +311,22 @@ static ssize_t kernfs_fop_write(struct file *file, const char __user *user_buf, goto out_free; } - if (copy_from_user(buf, user_buf, len)) { - len = -EFAULT; - goto out_unlock; - } - buf[len] = '\0'; /* guarantee string termination */ - ops = kernfs_ops(of->kn); if (ops->write) len = ops->write(of, buf, len, *ppos); else len = -EINVAL; + kernfs_put_active(of->kn); + mutex_unlock(&of->mutex); + if (len > 0) *ppos += len; -out_unlock: - kernfs_put_active(of->kn); - mutex_unlock(&of->mutex); out_free: - if (buf != of->prealloc_buf) + if (buf == of->prealloc_buf) + mutex_unlock(&of->prealloc_mutex); + else kfree(buf); return len; } @@ -687,6 +693,7 @@ static int kernfs_fop_open(struct inode *inode, struct file *file) error = -ENOMEM; if (!of->prealloc_buf) goto err_free; + mutex_init(&of->prealloc_mutex); } /* -- cgit v1.2.3