From aa1057b3dec478b20c77bad07442318ae36d893c Mon Sep 17 00:00:00 2001 From: Ryan Ding Date: Fri, 4 Sep 2015 15:42:36 -0700 Subject: ocfs2: direct write will call ocfs2_rw_unlock() twice when doing aio+dio ocfs2_file_write_iter() is usng the wrong return value ('written'). This will cause ocfs2_rw_unlock() be called both in write_iter & end_io, triggering a BUG_ON. This issue was introduced by commit 7da839c47589 ("ocfs2: use __generic_file_write_iter()"). Orabug: 21612107 Fixes: 7da839c47589 ("ocfs2: use __generic_file_write_iter()") Signed-off-by: Ryan Ding Reviewed-by: Junxiao Bi Cc: Al Viro Cc: Mark Fasheh Cc: Joel Becker Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/file.c | 28 ++++++++++++++-------------- 1 file changed, 14 insertions(+), 14 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 7210583b472f..2eb11363b1f7 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -2378,6 +2378,20 @@ relock: /* buffered aio wouldn't have proper lock coverage today */ BUG_ON(written == -EIOCBQUEUED && !(iocb->ki_flags & IOCB_DIRECT)); + /* + * deep in g_f_a_w_n()->ocfs2_direct_IO we pass in a ocfs2_dio_end_io + * function pointer which is called when o_direct io completes so that + * it can unlock our rw lock. + * Unfortunately there are error cases which call end_io and others + * that don't. so we don't have to unlock the rw_lock if either an + * async dio is going to do it in the future or an end_io after an + * error has already done it. + */ + if ((written == -EIOCBQUEUED) || (!ocfs2_iocb_is_rw_locked(iocb))) { + rw_level = -1; + unaligned_dio = 0; + } + if (unlikely(written <= 0)) goto no_sync; @@ -2402,20 +2416,6 @@ relock: } no_sync: - /* - * deep in g_f_a_w_n()->ocfs2_direct_IO we pass in a ocfs2_dio_end_io - * function pointer which is called when o_direct io completes so that - * it can unlock our rw lock. - * Unfortunately there are error cases which call end_io and others - * that don't. so we don't have to unlock the rw_lock if either an - * async dio is going to do it in the future or an end_io after an - * error has already done it. - */ - if ((ret == -EIOCBQUEUED) || (!ocfs2_iocb_is_rw_locked(iocb))) { - rw_level = -1; - unaligned_dio = 0; - } - if (unaligned_dio) { ocfs2_iocb_clear_unaligned_aio(iocb); mutex_unlock(&OCFS2_I(inode)->ip_unaligned_aio); -- cgit v1.2.1 From 58319057b7847667f0c9585b9de0e8932b0fdb08 Mon Sep 17 00:00:00 2001 From: Andy Lutomirski Date: Fri, 4 Sep 2015 15:42:45 -0700 Subject: capabilities: ambient capabilities Credit where credit is due: this idea comes from Christoph Lameter with a lot of valuable input from Serge Hallyn. This patch is heavily based on Christoph's patch. ===== The status quo ===== On Linux, there are a number of capabilities defined by the kernel. To perform various privileged tasks, processes can wield capabilities that they hold. Each task has four capability masks: effective (pE), permitted (pP), inheritable (pI), and a bounding set (X). When the kernel checks for a capability, it checks pE. The other capability masks serve to modify what capabilities can be in pE. Any task can remove capabilities from pE, pP, or pI at any time. If a task has a capability in pP, it can add that capability to pE and/or pI. If a task has CAP_SETPCAP, then it can add any capability to pI, and it can remove capabilities from X. Tasks are not the only things that can have capabilities; files can also have capabilities. A file can have no capabilty information at all [1]. If a file has capability information, then it has a permitted mask (fP) and an inheritable mask (fI) as well as a single effective bit (fE) [2]. File capabilities modify the capabilities of tasks that execve(2) them. A task that successfully calls execve has its capabilities modified for the file ultimately being excecuted (i.e. the binary itself if that binary is ELF or for the interpreter if the binary is a script.) [3] In the capability evolution rules, for each mask Z, pZ represents the old value and pZ' represents the new value. The rules are: pP' = (X & fP) | (pI & fI) pI' = pI pE' = (fE ? pP' : 0) X is unchanged For setuid binaries, fP, fI, and fE are modified by a moderately complicated set of rules that emulate POSIX behavior. Similarly, if euid == 0 or ruid == 0, then fP, fI, and fE are modified differently (primary, fP and fI usually end up being the full set). For nonroot users executing binaries with neither setuid nor file caps, fI and fP are empty and fE is false. As an extra complication, if you execute a process as nonroot and fE is set, then the "secure exec" rules are in effect: AT_SECURE gets set, LD_PRELOAD doesn't work, etc. This is rather messy. We've learned that making any changes is dangerous, though: if a new kernel version allows an unprivileged program to change its security state in a way that persists cross execution of a setuid program or a program with file caps, this persistent state is surprisingly likely to allow setuid or file-capped programs to be exploited for privilege escalation. ===== The problem ===== Capability inheritance is basically useless. If you aren't root and you execute an ordinary binary, fI is zero, so your capabilities have no effect whatsoever on pP'. This means that you can't usefully execute a helper process or a shell command with elevated capabilities if you aren't root. On current kernels, you can sort of work around this by setting fI to the full set for most or all non-setuid executable files. This causes pP' = pI for nonroot, and inheritance works. No one does this because it's a PITA and it isn't even supported on most filesystems. If you try this, you'll discover that every nonroot program ends up with secure exec rules, breaking many things. This is a problem that has bitten many people who have tried to use capabilities for anything useful. ===== The proposed change ===== This patch adds a fifth capability mask called the ambient mask (pA). pA does what most people expect pI to do. pA obeys the invariant that no bit can ever be set in pA if it is not set in both pP and pI. Dropping a bit from pP or pI drops that bit from pA. This ensures that existing programs that try to drop capabilities still do so, with a complication. Because capability inheritance is so broken, setting KEEPCAPS, using setresuid to switch to nonroot uids, and then calling execve effectively drops capabilities. Therefore, setresuid from root to nonroot conditionally clears pA unless SECBIT_NO_SETUID_FIXUP is set. Processes that don't like this can re-add bits to pA afterwards. The capability evolution rules are changed: pA' = (file caps or setuid or setgid ? 0 : pA) pP' = (X & fP) | (pI & fI) | pA' pI' = pI pE' = (fE ? pP' : pA') X is unchanged If you are nonroot but you have a capability, you can add it to pA. If you do so, your children get that capability in pA, pP, and pE. For example, you can set pA = CAP_NET_BIND_SERVICE, and your children can automatically bind low-numbered ports. Hallelujah! Unprivileged users can create user namespaces, map themselves to a nonzero uid, and create both privileged (relative to their namespace) and unprivileged process trees. This is currently more or less impossible. Hallelujah! You cannot use pA to try to subvert a setuid, setgid, or file-capped program: if you execute any such program, pA gets cleared and the resulting evolution rules are unchanged by this patch. Users with nonzero pA are unlikely to unintentionally leak that capability. If they run programs that try to drop privileges, dropping privileges will still work. It's worth noting that the degree of paranoia in this patch could possibly be reduced without causing serious problems. Specifically, if we allowed pA to persist across executing non-pA-aware setuid binaries and across setresuid, then, naively, the only capabilities that could leak as a result would be the capabilities in pA, and any attacker *already* has those capabilities. This would make me nervous, though -- setuid binaries that tried to privilege-separate might fail to do so, and putting CAP_DAC_READ_SEARCH or CAP_DAC_OVERRIDE into pA could have unexpected side effects. (Whether these unexpected side effects would be exploitable is an open question.) I've therefore taken the more paranoid route. We can revisit this later. An alternative would be to require PR_SET_NO_NEW_PRIVS before setting ambient capabilities. I think that this would be annoying and would make granting otherwise unprivileged users minor ambient capabilities (CAP_NET_BIND_SERVICE or CAP_NET_RAW for example) much less useful than it is with this patch. ===== Footnotes ===== [1] Files that are missing the "security.capability" xattr or that have unrecognized values for that xattr end up with has_cap set to false. The code that does that appears to be complicated for no good reason. [2] The libcap capability mask parsers and formatters are dangerously misleading and the documentation is flat-out wrong. fE is *not* a mask; it's a single bit. This has probably confused every single person who has tried to use file capabilities. [3] Linux very confusingly processes both the script and the interpreter if applicable, for reasons that elude me. The results from thinking about a script's file capabilities and/or setuid bits are mostly discarded. Preliminary userspace code is here, but it needs updating: https://git.kernel.org/cgit/linux/kernel/git/luto/util-linux-playground.git/commit/?h=cap_ambient&id=7f5afbd175d2 Here is a test program that can be used to verify the functionality (from Christoph): /* * Test program for the ambient capabilities. This program spawns a shell * that allows running processes with a defined set of capabilities. * * (C) 2015 Christoph Lameter * Released under: GPL v3 or later. * * * Compile using: * * gcc -o ambient_test ambient_test.o -lcap-ng * * This program must have the following capabilities to run properly: * Permissions for CAP_NET_RAW, CAP_NET_ADMIN, CAP_SYS_NICE * * A command to equip the binary with the right caps is: * * setcap cap_net_raw,cap_net_admin,cap_sys_nice+p ambient_test * * * To get a shell with additional caps that can be inherited by other processes: * * ./ambient_test /bin/bash * * * Verifying that it works: * * From the bash spawed by ambient_test run * * cat /proc/$$/status * * and have a look at the capabilities. */ #include #include #include #include #include #include /* * Definitions from the kernel header files. These are going to be removed * when the /usr/include files have these defined. */ #define PR_CAP_AMBIENT 47 #define PR_CAP_AMBIENT_IS_SET 1 #define PR_CAP_AMBIENT_RAISE 2 #define PR_CAP_AMBIENT_LOWER 3 #define PR_CAP_AMBIENT_CLEAR_ALL 4 static void set_ambient_cap(int cap) { int rc; capng_get_caps_process(); rc = capng_update(CAPNG_ADD, CAPNG_INHERITABLE, cap); if (rc) { printf("Cannot add inheritable cap\n"); exit(2); } capng_apply(CAPNG_SELECT_CAPS); /* Note the two 0s at the end. Kernel checks for these */ if (prctl(PR_CAP_AMBIENT, PR_CAP_AMBIENT_RAISE, cap, 0, 0)) { perror("Cannot set cap"); exit(1); } } int main(int argc, char **argv) { int rc; set_ambient_cap(CAP_NET_RAW); set_ambient_cap(CAP_NET_ADMIN); set_ambient_cap(CAP_SYS_NICE); printf("Ambient_test forking shell\n"); if (execv(argv[1], argv + 1)) perror("Cannot exec"); return 0; } Signed-off-by: Christoph Lameter # Original author Signed-off-by: Andy Lutomirski Acked-by: Serge E. Hallyn Acked-by: Kees Cook Cc: Jonathan Corbet Cc: Aaron Jones Cc: Ted Ts'o Cc: Andrew G. Morgan Cc: Mimi Zohar Cc: Austin S Hemmelgarn Cc: Markku Savela Cc: Jarkko Sakkinen Cc: Michael Kerrisk Cc: James Morris Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/proc/array.c | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/proc/array.c b/fs/proc/array.c index ce065cf3104f..f60f0121e331 100644 --- a/fs/proc/array.c +++ b/fs/proc/array.c @@ -308,7 +308,8 @@ static void render_cap_t(struct seq_file *m, const char *header, static inline void task_cap(struct seq_file *m, struct task_struct *p) { const struct cred *cred; - kernel_cap_t cap_inheritable, cap_permitted, cap_effective, cap_bset; + kernel_cap_t cap_inheritable, cap_permitted, cap_effective, + cap_bset, cap_ambient; rcu_read_lock(); cred = __task_cred(p); @@ -316,12 +317,14 @@ static inline void task_cap(struct seq_file *m, struct task_struct *p) cap_permitted = cred->cap_permitted; cap_effective = cred->cap_effective; cap_bset = cred->cap_bset; + cap_ambient = cred->cap_ambient; rcu_read_unlock(); render_cap_t(m, "CapInh:\t", &cap_inheritable); render_cap_t(m, "CapPrm:\t", &cap_permitted); render_cap_t(m, "CapEff:\t", &cap_effective); render_cap_t(m, "CapBnd:\t", &cap_bset); + render_cap_t(m, "CapAmb:\t", &cap_ambient); } static inline void task_seccomp(struct seq_file *m, struct task_struct *p) -- cgit v1.2.1 From 7c49b8616460ebb12ee56d80d1abfbc20b6f3cbb Mon Sep 17 00:00:00 2001 From: Dave Hansen Date: Fri, 4 Sep 2015 15:43:01 -0700 Subject: fs/notify: optimize inotify/fsnotify code for unwatched files I have a _tiny_ microbenchmark that sits in a loop and writes single bytes to a file. Writing one byte to a tmpfs file is around 2x slower than reading one byte from a file, which is a _bit_ more than I expecte. This is a dumb benchmark, but I think it's hard to deny that write() is a hot path and we should avoid unnecessary overhead there. I did a 'perf record' of 30-second samples of read and write. The top item in a diffprofile is srcu_read_lock() from fsnotify(). There are active inotify fd's from systemd, but nothing is actually listening to the file or its part of the filesystem. I *think* we can avoid taking the srcu_read_lock() for the common case where there are no actual marks on the file. This means that there will both be nothing to notify for *and* implies that there is no need for clearing the ignore mask. This patch gave a 13.1% speedup in writes/second on my test, which is an improvement from the 10.8% that I saw with the last version. Signed-off-by: Dave Hansen Reviewed-by: Jan Kara Cc: Al Viro Cc: Eric Paris Cc: John McCutchan Cc: Robert Love Cc: Andi Kleen Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/fsnotify.c | 10 ++++++++++ 1 file changed, 10 insertions(+) (limited to 'fs') diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index dd3fb0b17be7..d675e76251d3 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -204,6 +204,16 @@ int fsnotify(struct inode *to_tell, __u32 mask, void *data, int data_is, else mnt = NULL; + /* + * Optimization: srcu_read_lock() has a memory barrier which can + * be expensive. It protects walking the *_fsnotify_marks lists. + * However, if we do not walk the lists, we do not have to do + * SRCU because we have no references to any objects and do not + * need SRCU to keep them "alive". + */ + if (hlist_empty(&to_tell->i_fsnotify_marks) && + (!mnt || hlist_empty(&mnt->mnt_fsnotify_marks))) + return 0; /* * if this is a modify event we may need to clear the ignored masks * otherwise return if neither the inode nor the vfsmount care about -- cgit v1.2.1 From 3c53e514212455db9923c203694a72007558b48f Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 4 Sep 2015 15:43:03 -0700 Subject: fsnotify: fix check in inotify fdinfo printing A check in inotify_fdinfo() checking whether mark is valid was always true due to a bug. Luckily we can never get to invalidated marks since we hold mark_mutex and invalidated marks get removed from the group list when they are invalidated under that mutex. Anyway fix the check to make code more future proof. Signed-off-by: Jan Kara Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/fdinfo.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/notify/fdinfo.c b/fs/notify/fdinfo.c index 58b7cdb63da9..6b6f0d472ae8 100644 --- a/fs/notify/fdinfo.c +++ b/fs/notify/fdinfo.c @@ -76,7 +76,8 @@ static void inotify_fdinfo(struct seq_file *m, struct fsnotify_mark *mark) struct inotify_inode_mark *inode_mark; struct inode *inode; - if (!(mark->flags & (FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_INODE))) + if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE) || + !(mark->flags & FSNOTIFY_MARK_FLAG_INODE)) return; inode_mark = container_of(mark, struct inotify_inode_mark, fsn_mark); -- cgit v1.2.1 From 925d1132a03e33cb8f29a0057300d023b4f1be23 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 4 Sep 2015 15:43:09 -0700 Subject: fsnotify: remove mark->free_list Free list is used when all marks on given inode / mount should be destroyed when inode / mount is going away. However we can free all of the marks without using a special list with some care. Signed-off-by: Jan Kara Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/fsnotify.c | 1 - fs/notify/fsnotify.h | 21 +++++++++++++++------ fs/notify/inode_mark.c | 20 -------------------- fs/notify/mark.c | 40 +++++++++++++++++++++++++--------------- fs/notify/vfsmount_mark.c | 19 ------------------- 5 files changed, 40 insertions(+), 61 deletions(-) (limited to 'fs') diff --git a/fs/notify/fsnotify.c b/fs/notify/fsnotify.c index d675e76251d3..db39de2dd4cb 100644 --- a/fs/notify/fsnotify.c +++ b/fs/notify/fsnotify.c @@ -26,7 +26,6 @@ #include #include "fsnotify.h" -#include "../mount.h" /* * Clear all of the marks on an inode when it is being evicted from core diff --git a/fs/notify/fsnotify.h b/fs/notify/fsnotify.h index 13a00be516d2..b44c68a857e7 100644 --- a/fs/notify/fsnotify.h +++ b/fs/notify/fsnotify.h @@ -6,6 +6,8 @@ #include #include +#include "../mount.h" + /* destroy all events sitting in this groups notification queue */ extern void fsnotify_flush_notify(struct fsnotify_group *group); @@ -38,15 +40,22 @@ extern int fsnotify_add_vfsmount_mark(struct fsnotify_mark *mark, extern void fsnotify_destroy_vfsmount_mark(struct fsnotify_mark *mark); /* inode specific destruction of a mark */ extern void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark); -/* Destroy all marks in the given list */ -extern void fsnotify_destroy_marks(struct list_head *to_free); /* Find mark belonging to given group in the list of marks */ extern struct fsnotify_mark *fsnotify_find_mark(struct hlist_head *head, struct fsnotify_group *group); -/* run the list of all marks associated with inode and flag them to be freed */ -extern void fsnotify_clear_marks_by_inode(struct inode *inode); -/* run the list of all marks associated with vfsmount and flag them to be freed */ -extern void fsnotify_clear_marks_by_mount(struct vfsmount *mnt); +/* Destroy all marks in the given list protected by 'lock' */ +extern void fsnotify_destroy_marks(struct hlist_head *head, spinlock_t *lock); +/* run the list of all marks associated with inode and destroy them */ +static inline void fsnotify_clear_marks_by_inode(struct inode *inode) +{ + fsnotify_destroy_marks(&inode->i_fsnotify_marks, &inode->i_lock); +} +/* run the list of all marks associated with vfsmount and destroy them */ +static inline void fsnotify_clear_marks_by_mount(struct vfsmount *mnt) +{ + fsnotify_destroy_marks(&real_mount(mnt)->mnt_fsnotify_marks, + &mnt->mnt_root->d_lock); +} /* * update the dentry->d_flags of all of inode's children to indicate if inode cares * about events that happen to its children. diff --git a/fs/notify/inode_mark.c b/fs/notify/inode_mark.c index 3daf513ee99e..474a3ce1b5e1 100644 --- a/fs/notify/inode_mark.c +++ b/fs/notify/inode_mark.c @@ -64,26 +64,6 @@ void fsnotify_destroy_inode_mark(struct fsnotify_mark *mark) spin_unlock(&inode->i_lock); } -/* - * Given an inode, destroy all of the marks associated with that inode. - */ -void fsnotify_clear_marks_by_inode(struct inode *inode) -{ - struct fsnotify_mark *mark; - struct hlist_node *n; - LIST_HEAD(free_list); - - spin_lock(&inode->i_lock); - hlist_for_each_entry_safe(mark, n, &inode->i_fsnotify_marks, obj_list) { - list_add(&mark->free_list, &free_list); - hlist_del_init_rcu(&mark->obj_list); - fsnotify_get_mark(mark); - } - spin_unlock(&inode->i_lock); - - fsnotify_destroy_marks(&free_list); -} - /* * Given a group clear all of the inode marks associated with that group. */ diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 39ddcaf0918f..3b2d1ba41e7b 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -203,24 +203,34 @@ void fsnotify_destroy_mark(struct fsnotify_mark *mark, mutex_unlock(&group->mark_mutex); } -/* - * Destroy all marks in the given list. The marks must be already detached from - * the original inode / vfsmount. - */ -void fsnotify_destroy_marks(struct list_head *to_free) +void fsnotify_destroy_marks(struct hlist_head *head, spinlock_t *lock) { - struct fsnotify_mark *mark, *lmark; - struct fsnotify_group *group; - - list_for_each_entry_safe(mark, lmark, to_free, free_list) { - spin_lock(&mark->lock); - fsnotify_get_group(mark->group); - group = mark->group; - spin_unlock(&mark->lock); + struct fsnotify_mark *mark; - fsnotify_destroy_mark(mark, group); + while (1) { + /* + * We have to be careful since we can race with e.g. + * fsnotify_clear_marks_by_group() and once we drop 'lock', + * mark can get removed from the obj_list and destroyed. But + * we are holding mark reference so mark cannot be freed and + * calling fsnotify_destroy_mark() more than once is fine. + */ + spin_lock(lock); + if (hlist_empty(head)) { + spin_unlock(lock); + break; + } + mark = hlist_entry(head->first, struct fsnotify_mark, obj_list); + /* + * We don't update i_fsnotify_mask / mnt_fsnotify_mask here + * since inode / mount is going away anyway. So just remove + * mark from the list. + */ + hlist_del_init_rcu(&mark->obj_list); + fsnotify_get_mark(mark); + spin_unlock(lock); + fsnotify_destroy_mark(mark, mark->group); fsnotify_put_mark(mark); - fsnotify_put_group(group); } } diff --git a/fs/notify/vfsmount_mark.c b/fs/notify/vfsmount_mark.c index 326b148e623c..a8fcab68faef 100644 --- a/fs/notify/vfsmount_mark.c +++ b/fs/notify/vfsmount_mark.c @@ -28,25 +28,6 @@ #include #include "fsnotify.h" -#include "../mount.h" - -void fsnotify_clear_marks_by_mount(struct vfsmount *mnt) -{ - struct fsnotify_mark *mark; - struct hlist_node *n; - struct mount *m = real_mount(mnt); - LIST_HEAD(free_list); - - spin_lock(&mnt->mnt_root->d_lock); - hlist_for_each_entry_safe(mark, n, &m->mnt_fsnotify_marks, obj_list) { - list_add(&mark->free_list, &free_list); - hlist_del_init_rcu(&mark->obj_list); - fsnotify_get_mark(mark); - } - spin_unlock(&mnt->mnt_root->d_lock); - - fsnotify_destroy_marks(&free_list); -} void fsnotify_clear_vfsmount_marks_by_group(struct fsnotify_group *group) { -- cgit v1.2.1 From 4712e722f91457e60723b9cef6265a74290efba9 Mon Sep 17 00:00:00 2001 From: Jan Kara Date: Fri, 4 Sep 2015 15:43:12 -0700 Subject: fsnotify: get rid of fsnotify_destroy_mark_locked() fsnotify_destroy_mark_locked() is subtle to use because it temporarily releases group->mark_mutex. To avoid future problems with this function, split it into two. fsnotify_detach_mark() is the part that needs group->mark_mutex and fsnotify_free_mark() is the part that must be called outside of group->mark_mutex. This way it's much clearer what's going on and we also avoid some pointless acquisitions of group->mark_mutex. Signed-off-by: Jan Kara Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/notify/dnotify/dnotify.c | 14 +++++--- fs/notify/fanotify/fanotify_user.c | 8 +++-- fs/notify/mark.c | 73 +++++++++++++++++++++----------------- 3 files changed, 56 insertions(+), 39 deletions(-) (limited to 'fs') diff --git a/fs/notify/dnotify/dnotify.c b/fs/notify/dnotify/dnotify.c index 44523f4a6084..6faaf710e563 100644 --- a/fs/notify/dnotify/dnotify.c +++ b/fs/notify/dnotify/dnotify.c @@ -154,6 +154,7 @@ void dnotify_flush(struct file *filp, fl_owner_t id) struct dnotify_struct *dn; struct dnotify_struct **prev; struct inode *inode; + bool free = false; inode = file_inode(filp); if (!S_ISDIR(inode->i_mode)) @@ -182,11 +183,15 @@ void dnotify_flush(struct file *filp, fl_owner_t id) /* nothing else could have found us thanks to the dnotify_groups mark_mutex */ - if (dn_mark->dn == NULL) - fsnotify_destroy_mark_locked(fsn_mark, dnotify_group); + if (dn_mark->dn == NULL) { + fsnotify_detach_mark(fsn_mark); + free = true; + } mutex_unlock(&dnotify_group->mark_mutex); + if (free) + fsnotify_free_mark(fsn_mark); fsnotify_put_mark(fsn_mark); } @@ -362,9 +367,10 @@ out: spin_unlock(&fsn_mark->lock); if (destroy) - fsnotify_destroy_mark_locked(fsn_mark, dnotify_group); - + fsnotify_detach_mark(fsn_mark); mutex_unlock(&dnotify_group->mark_mutex); + if (destroy) + fsnotify_free_mark(fsn_mark); fsnotify_put_mark(fsn_mark); out_err: if (new_fsn_mark) diff --git a/fs/notify/fanotify/fanotify_user.c b/fs/notify/fanotify/fanotify_user.c index cf275500a665..8e8e6bcd1d43 100644 --- a/fs/notify/fanotify/fanotify_user.c +++ b/fs/notify/fanotify/fanotify_user.c @@ -529,8 +529,10 @@ static int fanotify_remove_vfsmount_mark(struct fsnotify_group *group, removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags, &destroy_mark); if (destroy_mark) - fsnotify_destroy_mark_locked(fsn_mark, group); + fsnotify_detach_mark(fsn_mark); mutex_unlock(&group->mark_mutex); + if (destroy_mark) + fsnotify_free_mark(fsn_mark); fsnotify_put_mark(fsn_mark); if (removed & real_mount(mnt)->mnt_fsnotify_mask) @@ -557,8 +559,10 @@ static int fanotify_remove_inode_mark(struct fsnotify_group *group, removed = fanotify_mark_remove_from_mask(fsn_mark, mask, flags, &destroy_mark); if (destroy_mark) - fsnotify_destroy_mark_locked(fsn_mark, group); + fsnotify_detach_mark(fsn_mark); mutex_unlock(&group->mark_mutex); + if (destroy_mark) + fsnotify_free_mark(fsn_mark); /* matches the fsnotify_find_inode_mark() */ fsnotify_put_mark(fsn_mark); diff --git a/fs/notify/mark.c b/fs/notify/mark.c index 3b2d1ba41e7b..fc0df4442f7b 100644 --- a/fs/notify/mark.c +++ b/fs/notify/mark.c @@ -122,26 +122,27 @@ u32 fsnotify_recalc_mask(struct hlist_head *head) } /* - * Any time a mark is getting freed we end up here. - * The caller had better be holding a reference to this mark so we don't actually - * do the final put under the mark->lock + * Remove mark from inode / vfsmount list, group list, drop inode reference + * if we got one. + * + * Must be called with group->mark_mutex held. */ -void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, - struct fsnotify_group *group) +void fsnotify_detach_mark(struct fsnotify_mark *mark) { struct inode *inode = NULL; + struct fsnotify_group *group = mark->group; BUG_ON(!mutex_is_locked(&group->mark_mutex)); spin_lock(&mark->lock); /* something else already called this function on this mark */ - if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { + if (!(mark->flags & FSNOTIFY_MARK_FLAG_ATTACHED)) { spin_unlock(&mark->lock); return; } - mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; + mark->flags &= ~FSNOTIFY_MARK_FLAG_ATTACHED; if (mark->flags & FSNOTIFY_MARK_FLAG_INODE) { inode = mark->inode; @@ -150,6 +151,12 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, fsnotify_destroy_vfsmount_mark(mark); else BUG(); + /* + * Note that we didn't update flags telling whether inode cares about + * what's happening with children. We update these flags from + * __fsnotify_parent() lazily when next event happens on one of our + * children. + */ list_del_init(&mark->g_list); @@ -157,18 +164,32 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, if (inode && (mark->flags & FSNOTIFY_MARK_FLAG_OBJECT_PINNED)) iput(inode); - /* release lock temporarily */ - mutex_unlock(&group->mark_mutex); + + atomic_dec(&group->num_marks); +} + +/* + * Free fsnotify mark. The freeing is actually happening from a kthread which + * first waits for srcu period end. Caller must have a reference to the mark + * or be protected by fsnotify_mark_srcu. + */ +void fsnotify_free_mark(struct fsnotify_mark *mark) +{ + struct fsnotify_group *group = mark->group; + + spin_lock(&mark->lock); + /* something else already called this function on this mark */ + if (!(mark->flags & FSNOTIFY_MARK_FLAG_ALIVE)) { + spin_unlock(&mark->lock); + return; + } + mark->flags &= ~FSNOTIFY_MARK_FLAG_ALIVE; + spin_unlock(&mark->lock); spin_lock(&destroy_lock); list_add(&mark->g_list, &destroy_list); spin_unlock(&destroy_lock); wake_up(&destroy_waitq); - /* - * We don't necessarily have a ref on mark from caller so the above destroy - * may have actually freed it, unless this group provides a 'freeing_mark' - * function which must be holding a reference. - */ /* * Some groups like to know that marks are being freed. This is a @@ -177,30 +198,15 @@ void fsnotify_destroy_mark_locked(struct fsnotify_mark *mark, */ if (group->ops->freeing_mark) group->ops->freeing_mark(mark, group); - - /* - * __fsnotify_update_child_dentry_flags(inode); - * - * I really want to call that, but we can't, we have no idea if the inode - * still exists the second we drop the mark->lock. - * - * The next time an event arrive to this inode from one of it's children - * __fsnotify_parent will see that the inode doesn't care about it's - * children and will update all of these flags then. So really this - * is just a lazy update (and could be a perf win...) - */ - - atomic_dec(&group->num_marks); - - mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING); } void fsnotify_destroy_mark(struct fsnotify_mark *mark, struct fsnotify_group *group) { mutex_lock_nested(&group->mark_mutex, SINGLE_DEPTH_NESTING); - fsnotify_destroy_mark_locked(mark, group); + fsnotify_detach_mark(mark); mutex_unlock(&group->mark_mutex); + fsnotify_free_mark(mark); } void fsnotify_destroy_marks(struct hlist_head *head, spinlock_t *lock) @@ -342,7 +348,7 @@ int fsnotify_add_mark_locked(struct fsnotify_mark *mark, * inode->i_lock */ spin_lock(&mark->lock); - mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE; + mark->flags |= FSNOTIFY_MARK_FLAG_ALIVE | FSNOTIFY_MARK_FLAG_ATTACHED; fsnotify_get_group(group); mark->group = group; @@ -448,8 +454,9 @@ void fsnotify_clear_marks_by_group_flags(struct fsnotify_group *group, } mark = list_first_entry(&to_free, struct fsnotify_mark, g_list); fsnotify_get_mark(mark); - fsnotify_destroy_mark_locked(mark, group); + fsnotify_detach_mark(mark); mutex_unlock(&group->mark_mutex); + fsnotify_free_mark(mark); fsnotify_put_mark(mark); } } -- cgit v1.2.1 From 917520e100e1db5e8dd546dd94fef070a31652a5 Mon Sep 17 00:00:00 2001 From: SF Markus Elfring Date: Fri, 4 Sep 2015 15:43:32 -0700 Subject: ntfs: delete unnecessary checks before calling iput() iput() tests whether its argument is NULL and then returns immediately. Thus the test around the call is not needed. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring Cc: Julia Lawall Reviewed-by: Anton Altaparmakov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ntfs/super.c | 21 +++++++-------------- 1 file changed, 7 insertions(+), 14 deletions(-) (limited to 'fs') diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c index c1128bcbeb5e..d1a853585b53 100644 --- a/fs/ntfs/super.c +++ b/fs/ntfs/super.c @@ -2204,17 +2204,12 @@ get_ctx_vol_failed: return true; #ifdef NTFS_RW iput_usnjrnl_err_out: - if (vol->usnjrnl_j_ino) - iput(vol->usnjrnl_j_ino); - if (vol->usnjrnl_max_ino) - iput(vol->usnjrnl_max_ino); - if (vol->usnjrnl_ino) - iput(vol->usnjrnl_ino); + iput(vol->usnjrnl_j_ino); + iput(vol->usnjrnl_max_ino); + iput(vol->usnjrnl_ino); iput_quota_err_out: - if (vol->quota_q_ino) - iput(vol->quota_q_ino); - if (vol->quota_ino) - iput(vol->quota_ino); + iput(vol->quota_q_ino); + iput(vol->quota_ino); iput(vol->extend_ino); #endif /* NTFS_RW */ iput_sec_err_out: @@ -2223,8 +2218,7 @@ iput_root_err_out: iput(vol->root_ino); iput_logfile_err_out: #ifdef NTFS_RW - if (vol->logfile_ino) - iput(vol->logfile_ino); + iput(vol->logfile_ino); iput_vol_err_out: #endif /* NTFS_RW */ iput(vol->vol_ino); @@ -2254,8 +2248,7 @@ iput_mftbmp_err_out: iput(vol->mftbmp_ino); iput_mirr_err_out: #ifdef NTFS_RW - if (vol->mftmirr_ino) - iput(vol->mftmirr_ino); + iput(vol->mftmirr_ino); #endif /* NTFS_RW */ return false; } -- cgit v1.2.1 From 512f62acbdf1ee81ce4882c85835f5420a1c304c Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:37 -0700 Subject: ocfs2: fix race between dio and recover orphan During direct io the inode will be added to orphan first and then deleted from orphan. There is a race window that the orphan entry will be deleted twice and thus trigger the BUG when validating OCFS2_DIO_ORPHANED_FL in ocfs2_del_inode_from_orphan. ocfs2_direct_IO_write ... ocfs2_add_inode_to_orphan >>>>>>>> race window. 1) another node may rm the file and then down, this node take care of orphan recovery and clear flag OCFS2_DIO_ORPHANED_FL. 2) since rw lock is unlocked, it may race with another orphan recovery and append dio. ocfs2_del_inode_from_orphan So take inode mutex lock when recovering orphans and make rw unlock at the end of aio write in case of append dio. Signed-off-by: Joseph Qi Reported-by: Yiwen Jiang Cc: Weiwei Wang Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/aops.c | 9 ++++++--- fs/ocfs2/file.c | 2 +- fs/ocfs2/inode.h | 2 -- fs/ocfs2/journal.c | 8 ++++---- fs/ocfs2/namei.c | 42 +++++++++++++----------------------------- fs/ocfs2/super.c | 2 -- 6 files changed, 24 insertions(+), 41 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index 0f5fd9db8194..1e88ff483702 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -627,10 +627,13 @@ static void ocfs2_dio_end_io(struct kiocb *iocb, mutex_unlock(&OCFS2_I(inode)->ip_unaligned_aio); } - ocfs2_iocb_clear_rw_locked(iocb); + /* Let rw unlock to be done later to protect append direct io write */ + if (offset + bytes <= i_size_read(inode)) { + ocfs2_iocb_clear_rw_locked(iocb); - level = ocfs2_iocb_rw_locked_level(iocb); - ocfs2_rw_unlock(inode, level); + level = ocfs2_iocb_rw_locked_level(iocb); + ocfs2_rw_unlock(inode, level); + } } static int ocfs2_releasepage(struct page *page, gfp_t wait) diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 2eb11363b1f7..5d384a6cd696 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -2416,7 +2416,7 @@ relock: } no_sync: - if (unaligned_dio) { + if (unaligned_dio && ocfs2_iocb_is_unaligned_aio(iocb)) { ocfs2_iocb_clear_unaligned_aio(iocb); mutex_unlock(&OCFS2_I(inode)->ip_unaligned_aio); } diff --git a/fs/ocfs2/inode.h b/fs/ocfs2/inode.h index 5e86b247c821..ca3431ee7f24 100644 --- a/fs/ocfs2/inode.h +++ b/fs/ocfs2/inode.h @@ -81,8 +81,6 @@ struct ocfs2_inode_info tid_t i_sync_tid; tid_t i_datasync_tid; - wait_queue_head_t append_dio_wq; - struct dquot *i_dquot[MAXQUOTAS]; }; diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index 7c099f7032fd..5e5626884433 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -2170,6 +2170,7 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb, iter = oi->ip_next_orphan; oi->ip_next_orphan = NULL; + mutex_lock(&inode->i_mutex); ret = ocfs2_rw_lock(inode, 1); if (ret < 0) { mlog_errno(ret); @@ -2206,17 +2207,16 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb, ret = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0); if (ret) mlog_errno(ret); - - wake_up(&OCFS2_I(inode)->append_dio_wq); } /* else if ORPHAN_NO_NEED_TRUNCATE, do nothing */ unlock_inode: ocfs2_inode_unlock(inode, 1); + brelse(di_bh); + di_bh = NULL; unlock_rw: ocfs2_rw_unlock(inode, 1); next: + mutex_unlock(&inode->i_mutex); iput(inode); - brelse(di_bh); - di_bh = NULL; inode = iter; } diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 948681e37cfd..e9ea7f23da12 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -2601,27 +2601,6 @@ leave: return status; } -static int ocfs2_dio_orphan_recovered(struct inode *inode) -{ - int ret; - struct buffer_head *di_bh = NULL; - struct ocfs2_dinode *di = NULL; - - ret = ocfs2_inode_lock(inode, &di_bh, 1); - if (ret < 0) { - mlog_errno(ret); - return 0; - } - - di = (struct ocfs2_dinode *) di_bh->b_data; - ret = !(di->i_flags & cpu_to_le32(OCFS2_DIO_ORPHANED_FL)); - ocfs2_inode_unlock(inode, 1); - brelse(di_bh); - - return ret; -} - -#define OCFS2_DIO_ORPHANED_FL_CHECK_INTERVAL 10000 int ocfs2_add_inode_to_orphan(struct ocfs2_super *osb, struct inode *inode) { @@ -2633,7 +2612,6 @@ int ocfs2_add_inode_to_orphan(struct ocfs2_super *osb, handle_t *handle = NULL; struct ocfs2_dinode *di = NULL; -restart: status = ocfs2_inode_lock(inode, &di_bh, 1); if (status < 0) { mlog_errno(status); @@ -2643,15 +2621,21 @@ restart: di = (struct ocfs2_dinode *) di_bh->b_data; /* * Another append dio crashed? - * If so, wait for recovery first. + * If so, manually recover it first. */ if (unlikely(di->i_flags & cpu_to_le32(OCFS2_DIO_ORPHANED_FL))) { - ocfs2_inode_unlock(inode, 1); - brelse(di_bh); - wait_event_interruptible_timeout(OCFS2_I(inode)->append_dio_wq, - ocfs2_dio_orphan_recovered(inode), - msecs_to_jiffies(OCFS2_DIO_ORPHANED_FL_CHECK_INTERVAL)); - goto restart; + status = ocfs2_truncate_file(inode, di_bh, i_size_read(inode)); + if (status < 0) { + if (status != -ENOSPC) + mlog_errno(status); + goto bail_unlock_inode; + } + + status = ocfs2_del_inode_from_orphan(osb, inode, di_bh, 0, 0); + if (status < 0) { + mlog_errno(status); + goto bail_unlock_inode; + } } status = ocfs2_prepare_orphan_dir(osb, &orphan_dir_inode, diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index 403c5660b306..4474ef2bbc96 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -1746,8 +1746,6 @@ static void ocfs2_inode_init_once(void *data) ocfs2_lock_res_init_once(&oi->ip_inode_lockres); ocfs2_lock_res_init_once(&oi->ip_open_lockres); - init_waitqueue_head(&oi->append_dio_wq); - ocfs2_metadata_cache_init(INODE_CACHE(&oi->vfs_inode), &ocfs2_inode_caching_ops); -- cgit v1.2.1 From faaebf18f831c1546bdc65ff8f49d2a73e675ded Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:40 -0700 Subject: ocfs2: fix several issues of append dio 1) Take rw EX lock in case of append dio. 2) Explicitly treat the error code -EIOCBQUEUED as normal. 3) Set di_bh to NULL after brelse if it may be used again later. Signed-off-by: Joseph Qi Cc: Yiwen Jiang Cc: Weiwei Wang Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/aops.c | 7 ++++++- fs/ocfs2/file.c | 5 ++++- 2 files changed, 10 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index 1e88ff483702..b36dcad3a140 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -860,7 +860,8 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb, written = __blockdev_direct_IO(iocb, inode, inode->i_sb->s_bdev, iter, offset, ocfs2_direct_IO_get_blocks, ocfs2_dio_end_io, NULL, 0); - if (unlikely(written < 0)) { + /* overwrite aio may return -EIOCBQUEUED, and it is not an error */ + if ((written < 0) && (written != -EIOCBQUEUED)) { loff_t i_size = i_size_read(inode); if (offset + count > i_size) { @@ -879,12 +880,14 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb, ocfs2_inode_unlock(inode, 1); brelse(di_bh); + di_bh = NULL; goto clean_orphan; } } ocfs2_inode_unlock(inode, 1); brelse(di_bh); + di_bh = NULL; ret = jbd2_journal_force_commit(journal); if (ret < 0) @@ -939,10 +942,12 @@ clean_orphan: if (tmp_ret < 0) { ret = tmp_ret; mlog_errno(ret); + brelse(di_bh); goto out; } ocfs2_inode_unlock(inode, 1); + brelse(di_bh); tmp_ret = jbd2_journal_force_commit(journal); if (tmp_ret < 0) { diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 5d384a6cd696..38fc33922832 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -2271,6 +2271,8 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb, OCFS2_MOUNT_COHERENCY_BUFFERED); int unaligned_dio = 0; int dropped_dio = 0; + int append_write = ((iocb->ki_pos + count) >= + i_size_read(inode) ? 1 : 0); trace_ocfs2_file_aio_write(inode, file, file->f_path.dentry, (unsigned long long)OCFS2_I(inode)->ip_blkno, @@ -2290,8 +2292,9 @@ relock: /* * Concurrent O_DIRECT writes are allowed with * mount_option "coherency=buffered". + * For append write, we must take rw EX. */ - rw_level = (!direct_io || full_coherency); + rw_level = (!direct_io || full_coherency || append_write); ret = ocfs2_rw_lock(inode, rw_level); if (ret < 0) { -- cgit v1.2.1 From acf8fdbe6afb084666df347602fe4258f1cf5fd5 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:43 -0700 Subject: ocfs2: do not BUG if buffer not uptodate in __ocfs2_journal_access When storage network is unstable, it may trigger the BUG in __ocfs2_journal_access because of buffer not uptodate. We can retry the write in this case or return error instead of BUG. Signed-off-by: Joseph Qi Reported-by: Zhangguanghui Tested-by: Zhangguanghui Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/journal.c | 18 +++++++++++++++++- 1 file changed, 17 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index 5e5626884433..3bfd36a23e40 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -668,7 +668,23 @@ static int __ocfs2_journal_access(handle_t *handle, mlog(ML_ERROR, "giving me a buffer that's not uptodate!\n"); mlog(ML_ERROR, "b_blocknr=%llu\n", (unsigned long long)bh->b_blocknr); - BUG(); + + lock_buffer(bh); + /* + * A previous attempt to write this buffer head failed. + * Nothing we can do but to retry the write and hope for + * the best. + */ + if (buffer_write_io_error(bh) && !buffer_uptodate(bh)) { + clear_buffer_write_io_error(bh); + set_buffer_uptodate(bh); + } + + if (!buffer_uptodate(bh)) { + unlock_buffer(bh); + return -EIO; + } + unlock_buffer(bh); } /* Set the current transaction information on the ci so -- cgit v1.2.1 From 372a447c4bb8271d128def5f93e3365d5d06b4d8 Mon Sep 17 00:00:00 2001 From: Christophe JAILLET Date: Fri, 4 Sep 2015 15:43:46 -0700 Subject: ocfs2: do not log twice error messages 'o2hb_map_slot_data' and 'o2hb_populate_slot_data' are called from only one place, in 'o2hb_region_dev_write'. Return value is checked and 'mlog_errno' is called to log a message if it is not 0. So there is no need to call 'mlog_errno' directly within these functions. This would result on logging the message twice. Signed-off-by: Christophe JAILLET Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/cluster/heartbeat.c | 20 +++++--------------- 1 file changed, 5 insertions(+), 15 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index 140de3c93d2e..f97306453a0b 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -1619,17 +1619,13 @@ static int o2hb_map_slot_data(struct o2hb_region *reg) struct o2hb_disk_slot *slot; reg->hr_tmp_block = kmalloc(reg->hr_block_bytes, GFP_KERNEL); - if (reg->hr_tmp_block == NULL) { - mlog_errno(-ENOMEM); + if (reg->hr_tmp_block == NULL) return -ENOMEM; - } reg->hr_slots = kcalloc(reg->hr_blocks, sizeof(struct o2hb_disk_slot), GFP_KERNEL); - if (reg->hr_slots == NULL) { - mlog_errno(-ENOMEM); + if (reg->hr_slots == NULL) return -ENOMEM; - } for(i = 0; i < reg->hr_blocks; i++) { slot = ®->hr_slots[i]; @@ -1645,17 +1641,13 @@ static int o2hb_map_slot_data(struct o2hb_region *reg) reg->hr_slot_data = kcalloc(reg->hr_num_pages, sizeof(struct page *), GFP_KERNEL); - if (!reg->hr_slot_data) { - mlog_errno(-ENOMEM); + if (!reg->hr_slot_data) return -ENOMEM; - } for(i = 0; i < reg->hr_num_pages; i++) { page = alloc_page(GFP_KERNEL); - if (!page) { - mlog_errno(-ENOMEM); + if (!page) return -ENOMEM; - } reg->hr_slot_data[i] = page; @@ -1687,10 +1679,8 @@ static int o2hb_populate_slot_data(struct o2hb_region *reg) struct o2hb_disk_heartbeat_block *hb_block; ret = o2hb_read_slots(reg, reg->hr_blocks); - if (ret) { - mlog_errno(ret); + if (ret) goto out; - } /* We only want to get an idea of the values initially in each * slot, so we do no verification - o2hb_check_slot will -- cgit v1.2.1 From bf59e6623a3a92a2bf428f2d6592c81aae6317e1 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:49 -0700 Subject: ocfs2: clean up unused local variables in ocfs2_file_write_iter Since commit 86b9c6f3f891 ("ocfs2: remove filesize checks for sync I/O journal commit") removes filesize checks for sync I/O journal commit, variables old_size and old_clusters are not actually used any more. So clean them up. Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/file.c | 9 --------- 1 file changed, 9 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index 38fc33922832..c4a99fb61c3e 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -2262,8 +2262,6 @@ static ssize_t ocfs2_file_write_iter(struct kiocb *iocb, ssize_t written = 0; ssize_t ret; size_t count = iov_iter_count(from), orig_count; - loff_t old_size; - u32 old_clusters; struct file *file = iocb->ki_filp; struct inode *inode = file_inode(file); struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); @@ -2367,13 +2365,6 @@ relock: ocfs2_iocb_set_unaligned_aio(iocb); } - /* - * To later detect whether a journal commit for sync writes is - * necessary, we sample i_size, and cluster count here. - */ - old_size = i_size_read(inode); - old_clusters = OCFS2_I(inode)->ip_clusters; - /* communicate with ocfs2_dio_end_io */ ocfs2_iocb_set_rw_locked(iocb, rw_level); -- cgit v1.2.1 From 3cb2ec43f63c42412a18620f1226eb4aa434a7a8 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:52 -0700 Subject: ocfs2: adjust code to match locking/unlocking order Unlocking order in ocfs2_unlink and ocfs2_rename mismatches the corresponding locking order, although it won't cause issues, adjust the code so that it looks more reasonable. Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/namei.c | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index e9ea7f23da12..97c47d71efa7 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -1035,11 +1035,6 @@ leave: if (handle) ocfs2_commit_trans(osb, handle); - if (child_locked) - ocfs2_inode_unlock(inode, 1); - - ocfs2_inode_unlock(dir, 1); - if (orphan_dir) { /* This was locked for us in ocfs2_prepare_orphan_dir() */ ocfs2_inode_unlock(orphan_dir, 1); @@ -1047,6 +1042,11 @@ leave: iput(orphan_dir); } + if (child_locked) + ocfs2_inode_unlock(inode, 1); + + ocfs2_inode_unlock(dir, 1); + brelse(fe_bh); brelse(parent_node_bh); @@ -1633,21 +1633,9 @@ static int ocfs2_rename(struct inode *old_dir, ocfs2_dentry_move(old_dentry, new_dentry, old_dir, new_dir); status = 0; bail: - if (rename_lock) - ocfs2_rename_unlock(osb); - if (handle) ocfs2_commit_trans(osb, handle); - if (parents_locked) - ocfs2_double_unlock(old_dir, new_dir); - - if (old_child_locked) - ocfs2_inode_unlock(old_inode, 1); - - if (new_child_locked) - ocfs2_inode_unlock(new_inode, 1); - if (orphan_dir) { /* This was locked for us in ocfs2_prepare_orphan_dir() */ ocfs2_inode_unlock(orphan_dir, 1); @@ -1655,6 +1643,18 @@ bail: iput(orphan_dir); } + if (new_child_locked) + ocfs2_inode_unlock(new_inode, 1); + + if (old_child_locked) + ocfs2_inode_unlock(old_inode, 1); + + if (parents_locked) + ocfs2_double_unlock(old_dir, new_dir); + + if (rename_lock) + ocfs2_rename_unlock(osb); + if (new_inode) sync_mapping_buffers(old_inode->i_mapping); -- cgit v1.2.1 From 914a9b74295774b92409fbc3e0abcfa9185d9469 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:54 -0700 Subject: ocfs2: remove unneeded code in ocfs2_dlm_init status is already initialized and it will only be 0 or negatives in the code flow. So remove the unneeded assignment after the lable 'local'. Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/dlmglue.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/dlmglue.c b/fs/ocfs2/dlmglue.c index 23157e40dd74..1c91103c1333 100644 --- a/fs/ocfs2/dlmglue.c +++ b/fs/ocfs2/dlmglue.c @@ -3035,8 +3035,6 @@ local: ocfs2_orphan_scan_lock_res_init(&osb->osb_orphan_scan.os_lockres, osb); osb->cconn = conn; - - status = 0; bail: if (status < 0) { ocfs2_dlm_shutdown_debug(osb); -- cgit v1.2.1 From cdd09f49cb271d95cbe69ef886459e0490040e98 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:43:57 -0700 Subject: ocfs2: fix BUG when o2hb_register_callback fails In dlm_register_domain_handlers, if o2hb_register_callback fails, it will call dlm_unregister_domain_handlers to unregister. This will trigger the BUG_ON in o2hb_unregister_callback because hc_magic is 0. So we should call o2hb_setup_callback to initialize hc first. Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/dlm/dlmdomain.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 7df88a6dd626..4f750701bd9a 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1725,12 +1725,13 @@ static int dlm_register_domain_handlers(struct dlm_ctxt *dlm) o2hb_setup_callback(&dlm->dlm_hb_down, O2HB_NODE_DOWN_CB, dlm_hb_node_down_cb, dlm, DLM_HB_NODE_DOWN_PRI); + o2hb_setup_callback(&dlm->dlm_hb_up, O2HB_NODE_UP_CB, + dlm_hb_node_up_cb, dlm, DLM_HB_NODE_UP_PRI); + status = o2hb_register_callback(dlm->name, &dlm->dlm_hb_down); if (status) goto bail; - o2hb_setup_callback(&dlm->dlm_hb_up, O2HB_NODE_UP_CB, - dlm_hb_node_up_cb, dlm, DLM_HB_NODE_UP_PRI); status = o2hb_register_callback(dlm->name, &dlm->dlm_hb_up); if (status) goto bail; -- cgit v1.2.1 From 0e3d9eafb86183a33efc42f0beff5afceebbafba Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:44:00 -0700 Subject: ocfs2: remove unneeded code in dlm_register_domain_handlers The last goto statement is unneeded, so remove it. Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/dlm/dlmdomain.c | 2 -- 1 file changed, 2 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 4f750701bd9a..019459b20aeb 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1846,8 +1846,6 @@ static int dlm_register_domain_handlers(struct dlm_ctxt *dlm) sizeof(struct dlm_exit_domain), dlm_begin_exit_domain_handler, dlm, NULL, &dlm->dlm_domain_handlers); - if (status) - goto bail; bail: if (status) -- cgit v1.2.1 From f83c7b5e9fd633fe91128af116e6472a8c4d29a5 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:44:03 -0700 Subject: ocfs2/dlm: use list_for_each_entry instead of list_for_each Use list_for_each_entry instead of list_for_each to simplify code. Signed-off-by: Joseph Qi Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/dlm/dlmrecovery.c | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/dlm/dlmrecovery.c b/fs/ocfs2/dlm/dlmrecovery.c index ce12e0b1a31f..d0e436dc6437 100644 --- a/fs/ocfs2/dlm/dlmrecovery.c +++ b/fs/ocfs2/dlm/dlmrecovery.c @@ -1776,7 +1776,7 @@ static int dlm_process_recovery_data(struct dlm_ctxt *dlm, struct dlm_migratable_lockres *mres) { struct dlm_migratable_lock *ml; - struct list_head *queue, *iter; + struct list_head *queue; struct list_head *tmpq = NULL; struct dlm_lock *newlock = NULL; struct dlm_lockstatus *lksb = NULL; @@ -1821,9 +1821,7 @@ static int dlm_process_recovery_data(struct dlm_ctxt *dlm, spin_lock(&res->spinlock); for (j = DLM_GRANTED_LIST; j <= DLM_BLOCKED_LIST; j++) { tmpq = dlm_list_idx_to_ptr(res, j); - list_for_each(iter, tmpq) { - lock = list_entry(iter, - struct dlm_lock, list); + list_for_each_entry(lock, tmpq, list) { if (lock->ml.cookie == ml->cookie) break; lock = NULL; -- cgit v1.2.1 From 807a7907114c7c703017ed7a96477a2eeb0d08e0 Mon Sep 17 00:00:00 2001 From: jiangyiwen Date: Fri, 4 Sep 2015 15:44:06 -0700 Subject: ocfs2: set filesytem read-only when ocfs2_delete_entry failed. In ocfs2_rename, it will lead to an inode with two entried(old and new) if ocfs2_delete_entry(old) failed. Thus, filesystem will be inconsistent. The case is described below: ocfs2_rename -> ocfs2_start_trans -> ocfs2_add_entry(new) -> ocfs2_delete_entry(old) -> __ocfs2_journal_access *failed* because of -ENOMEM -> ocfs2_commit_trans So filesystem should be set to read-only at the moment. Signed-off-by: Yiwen Jiang Cc: Joseph Qi Cc: Joel Becker Reviewed-by: Mark Fasheh Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/namei.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 97c47d71efa7..1c43993e81b0 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -1569,12 +1569,25 @@ static int ocfs2_rename(struct inode *old_dir, status = ocfs2_find_entry(old_dentry->d_name.name, old_dentry->d_name.len, old_dir, &old_entry_lookup); - if (status) + if (status) { + if (!is_journal_aborted(osb->journal->j_journal)) { + ocfs2_error(osb->sb, "new entry %.*s is added, but old entry %.*s " + "is not deleted.", + new_dentry->d_name.len, new_dentry->d_name.name, + old_dentry->d_name.len, old_dentry->d_name.name); + } goto bail; + } status = ocfs2_delete_entry(handle, old_dir, &old_entry_lookup); if (status < 0) { mlog_errno(status); + if (!is_journal_aborted(osb->journal->j_journal)) { + ocfs2_error(osb->sb, "new entry %.*s is added, but old entry %.*s " + "is not deleted.", + new_dentry->d_name.len, new_dentry->d_name.name, + old_dentry->d_name.len, old_dentry->d_name.name); + } goto bail; } -- cgit v1.2.1 From 0f5e7b41f91814447defc34e915fc5d6e52266d9 Mon Sep 17 00:00:00 2001 From: Sanidhya Kashyap Date: Fri, 4 Sep 2015 15:44:08 -0700 Subject: ocfs2: trusted xattr missing CAP_SYS_ADMIN check The trusted extended attributes are only visible to the process which hvae CAP_SYS_ADMIN capability but the check is missing in ocfs2 xattr_handler trusted list. The check is important because this will be used for implementing mechanisms in the userspace for which other ordinary processes should not have access to. Signed-off-by: Sanidhya Kashyap Reviewed-by: Mark Fasheh Cc: Joel Becker Cc: Taesoo kim Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/xattr.c | 3 +++ 1 file changed, 3 insertions(+) (limited to 'fs') diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 889f3796a0d7..a24f264b2fc4 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -7334,6 +7334,9 @@ static size_t ocfs2_xattr_trusted_list(struct dentry *dentry, char *list, const size_t prefix_len = XATTR_TRUSTED_PREFIX_LEN; const size_t total_len = prefix_len + name_len + 1; + if (!capable(CAP_SYS_ADMIN)) + return 0; + if (list && total_len <= list_size) { memcpy(list, XATTR_TRUSTED_PREFIX, prefix_len); memcpy(list + prefix_len, name, name_len); -- cgit v1.2.1 From 513e2dae9422223072ed3887e91efebec2fc0a01 Mon Sep 17 00:00:00 2001 From: Xue jiufei Date: Fri, 4 Sep 2015 15:44:11 -0700 Subject: ocfs2: flush inode data to disk and free inode when i_count becomes zero Disk inode deletion may be heavily delayed when one node unlink a file after the same dentry is freed on another node(say N1) because of memory shrink but inode is left in memory. This inode can only be freed while N1 doing the orphan scan work. However, N1 may skip orphan scan for several times because other nodes may do the work earlier. In our tests, it may take 1 hour on 4 nodes cluster and it hurts the user experience. So we think the inode should be freed after the data flushed to disk when i_count becomes zero to avoid such circumstances. Signed-off-by: Joyce.xue Cc: Joel Becker Reviewed-by: Mark Fasheh Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/inode.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c index b254416dc8d9..4e69f3cbc5f1 100644 --- a/fs/ocfs2/inode.c +++ b/fs/ocfs2/inode.c @@ -1191,17 +1191,19 @@ void ocfs2_evict_inode(struct inode *inode) int ocfs2_drop_inode(struct inode *inode) { struct ocfs2_inode_info *oi = OCFS2_I(inode); - int res; trace_ocfs2_drop_inode((unsigned long long)oi->ip_blkno, inode->i_nlink, oi->ip_flags); - if (oi->ip_flags & OCFS2_INODE_MAYBE_ORPHANED) - res = 1; - else - res = generic_drop_inode(inode); + assert_spin_locked(&inode->i_lock); + inode->i_state |= I_WILL_FREE; + spin_unlock(&inode->i_lock); + write_inode_now(inode, 1); + spin_lock(&inode->i_lock); + WARN_ON(inode->i_state & I_NEW); + inode->i_state &= ~I_WILL_FREE; - return res; + return 1; } /* -- cgit v1.2.1 From 7d0fb9148ab6f52006de7cce18860227594ba872 Mon Sep 17 00:00:00 2001 From: Goldwyn Rodrigues Date: Fri, 4 Sep 2015 15:44:11 -0700 Subject: ocfs2: add errors=continue OCFS2 is often used in high-availaibility systems. However, ocfs2 converts the filesystem to read-only at the drop of the hat. This may not be necessary, since turning the filesystem read-only would affect other running processes as well, decreasing availability. This attempt is to add errors=continue, which would return the EIO to the calling process and terminate furhter processing so that the filesystem is not corrupted further. However, the filesystem is not converted to read-only. As a future plan, I intend to create a small utility or extend fsck.ocfs2 to fix small errors such as in the inode. The input to the utility such as the inode can come from the kernel logs so we don't have to schedule a downtime for fixing small-enough errors. The patch changes the ocfs2_error to return an error. The error returned depends on the mount option set. If none is set, the default is to turn the filesystem read-only. Perhaps errors=continue is not the best option name. Historically it is used for making an attempt to progress in the current process itself. Should we call it errors=eio? or errors=killproc? Suggestions/Comments welcome. Sources are available at: https://github.com/goldwynr/linux/tree/error-cont Signed-off-by: Goldwyn Rodrigues Signed-off-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/ocfs2.h | 2 ++ fs/ocfs2/super.c | 63 +++++++++++++++++++++++++++++++++++++++----------------- fs/ocfs2/super.h | 2 +- 3 files changed, 47 insertions(+), 20 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/ocfs2.h b/fs/ocfs2/ocfs2.h index 690ddc60189b..7a0126267847 100644 --- a/fs/ocfs2/ocfs2.h +++ b/fs/ocfs2/ocfs2.h @@ -286,6 +286,8 @@ enum ocfs2_mount_options OCFS2_MOUNT_HB_GLOBAL = 1 << 14, /* Global heartbeat */ OCFS2_MOUNT_JOURNAL_ASYNC_COMMIT = 1 << 15, /* Journal Async Commit */ + OCFS2_MOUNT_ERRORS_CONT = 1 << 16, /* Return EIO to the calling process on error */ + OCFS2_MOUNT_ERRORS_ROFS = 1 << 17, /* Change filesystem to read-only on error */ }; #define OCFS2_OSB_SOFT_RO 0x0001 diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index 4474ef2bbc96..e79058ecfb4b 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -192,6 +192,7 @@ enum { Opt_resv_level, Opt_dir_resv_level, Opt_journal_async_commit, + Opt_err_cont, Opt_err, }; @@ -224,6 +225,7 @@ static const match_table_t tokens = { {Opt_resv_level, "resv_level=%u"}, {Opt_dir_resv_level, "dir_resv_level=%u"}, {Opt_journal_async_commit, "journal_async_commit"}, + {Opt_err_cont, "errors=continue"}, {Opt_err, NULL} }; @@ -1330,10 +1332,19 @@ static int ocfs2_parse_options(struct super_block *sb, mopt->mount_opt |= OCFS2_MOUNT_NOINTR; break; case Opt_err_panic: + mopt->mount_opt &= ~OCFS2_MOUNT_ERRORS_CONT; + mopt->mount_opt &= ~OCFS2_MOUNT_ERRORS_ROFS; mopt->mount_opt |= OCFS2_MOUNT_ERRORS_PANIC; break; case Opt_err_ro: + mopt->mount_opt &= ~OCFS2_MOUNT_ERRORS_CONT; mopt->mount_opt &= ~OCFS2_MOUNT_ERRORS_PANIC; + mopt->mount_opt |= OCFS2_MOUNT_ERRORS_ROFS; + break; + case Opt_err_cont: + mopt->mount_opt &= ~OCFS2_MOUNT_ERRORS_ROFS; + mopt->mount_opt &= ~OCFS2_MOUNT_ERRORS_PANIC; + mopt->mount_opt |= OCFS2_MOUNT_ERRORS_CONT; break; case Opt_data_ordered: mopt->mount_opt &= ~OCFS2_MOUNT_DATA_WRITEBACK; @@ -1530,6 +1541,8 @@ static int ocfs2_show_options(struct seq_file *s, struct dentry *root) if (opts & OCFS2_MOUNT_ERRORS_PANIC) seq_printf(s, ",errors=panic"); + else if (opts & OCFS2_MOUNT_ERRORS_CONT) + seq_printf(s, ",errors=continue"); else seq_printf(s, ",errors=remount-ro"); @@ -2539,31 +2552,43 @@ static void ocfs2_delete_osb(struct ocfs2_super *osb) memset(osb, 0, sizeof(struct ocfs2_super)); } -/* Put OCFS2 into a readonly state, or (if the user specifies it), - * panic(). We do not support continue-on-error operation. */ -static void ocfs2_handle_error(struct super_block *sb) +/* Depending on the mount option passed, perform one of the following: + * Put OCFS2 into a readonly state (default) + * Return EIO so that only the process errs + * Fix the error as if fsck.ocfs2 -y + * panic + */ +static int ocfs2_handle_error(struct super_block *sb) { struct ocfs2_super *osb = OCFS2_SB(sb); - - if (osb->s_mount_opt & OCFS2_MOUNT_ERRORS_PANIC) - panic("OCFS2: (device %s): panic forced after error\n", - sb->s_id); + int rv = 0; ocfs2_set_osb_flag(osb, OCFS2_OSB_ERROR_FS); + pr_crit("On-disk corruption discovered. " + "Please run fsck.ocfs2 once the filesystem is unmounted.\n"); - if (sb->s_flags & MS_RDONLY && - (ocfs2_is_soft_readonly(osb) || - ocfs2_is_hard_readonly(osb))) - return; - - printk(KERN_CRIT "File system is now read-only due to the potential " - "of on-disk corruption. Please run fsck.ocfs2 once the file " - "system is unmounted.\n"); - sb->s_flags |= MS_RDONLY; - ocfs2_set_ro_flag(osb, 0); + if (osb->s_mount_opt & OCFS2_MOUNT_ERRORS_PANIC) { + panic("OCFS2: (device %s): panic forced after error\n", + sb->s_id); + } else if (osb->s_mount_opt & OCFS2_MOUNT_ERRORS_CONT) { + pr_crit("OCFS2: Returning error to the calling process.\n"); + rv = -EIO; + } else { /* default option */ + rv = -EROFS; + if (sb->s_flags & MS_RDONLY && + (ocfs2_is_soft_readonly(osb) || + ocfs2_is_hard_readonly(osb))) + return rv; + + pr_crit("OCFS2: File system is now read-only.\n"); + sb->s_flags |= MS_RDONLY; + ocfs2_set_ro_flag(osb, 0); + } + + return rv; } -void __ocfs2_error(struct super_block *sb, const char *function, +int __ocfs2_error(struct super_block *sb, const char *function, const char *fmt, ...) { struct va_format vaf; @@ -2580,7 +2605,7 @@ void __ocfs2_error(struct super_block *sb, const char *function, va_end(args); - ocfs2_handle_error(sb); + return ocfs2_handle_error(sb); } /* Handle critical errors. This is intentionally more drastic than diff --git a/fs/ocfs2/super.h b/fs/ocfs2/super.h index 74ff74cf78fe..c1c87d90542c 100644 --- a/fs/ocfs2/super.h +++ b/fs/ocfs2/super.h @@ -32,7 +32,7 @@ int ocfs2_publish_get_mount_state(struct ocfs2_super *osb, int node_num); __printf(3, 4) -void __ocfs2_error(struct super_block *sb, const char *function, +int __ocfs2_error(struct super_block *sb, const char *function, const char *fmt, ...); #define ocfs2_error(sb, fmt, args...) __ocfs2_error(sb, __PRETTY_FUNCTION__, fmt, ##args) -- cgit v1.2.1 From 17a5b9ab32fe0464e7f556e28a2b49d2023fb533 Mon Sep 17 00:00:00 2001 From: Goldwyn Rodrigues Date: Fri, 4 Sep 2015 15:44:17 -0700 Subject: ocfs2: acknowledge return value of ocfs2_error() Caveat: This may return -EROFS for a read case, which seems wrong. This is happening even without this patch series though. Should we convert EROFS to EIO? Signed-off-by: Goldwyn Rodrigues Cc: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/alloc.c | 16 ++++++++-------- fs/ocfs2/dir.c | 25 +++++++++---------------- fs/ocfs2/inode.c | 8 ++++---- fs/ocfs2/move_extents.c | 3 +-- fs/ocfs2/refcounttree.c | 42 ++++++++++++++++++------------------------ fs/ocfs2/suballoc.c | 25 ++++++------------------- fs/ocfs2/xattr.c | 15 +++++---------- 7 files changed, 51 insertions(+), 83 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c index 5997c00a1515..9a0fd494fe74 100644 --- a/fs/ocfs2/alloc.c +++ b/fs/ocfs2/alloc.c @@ -908,32 +908,32 @@ static int ocfs2_validate_extent_block(struct super_block *sb, */ if (!OCFS2_IS_VALID_EXTENT_BLOCK(eb)) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Extent block #%llu has bad signature %.*s", (unsigned long long)bh->b_blocknr, 7, eb->h_signature); - return -EINVAL; + goto bail; } if (le64_to_cpu(eb->h_blkno) != bh->b_blocknr) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Extent block #%llu has an invalid h_blkno " "of %llu", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(eb->h_blkno)); - return -EINVAL; + goto bail; } if (le32_to_cpu(eb->h_fs_generation) != OCFS2_SB(sb)->fs_generation) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Extent block #%llu has an invalid " "h_fs_generation of #%u", (unsigned long long)bh->b_blocknr, le32_to_cpu(eb->h_fs_generation)); - return -EINVAL; + goto bail; } - - return 0; +bail: + return rc; } int ocfs2_read_extent_block(struct ocfs2_caching_info *ci, u64 eb_blkno, diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c index 02878a83f0b4..25f03af09237 100644 --- a/fs/ocfs2/dir.c +++ b/fs/ocfs2/dir.c @@ -480,8 +480,7 @@ static int ocfs2_check_dir_trailer(struct inode *dir, struct buffer_head *bh) trailer = ocfs2_trailer_from_bh(bh, dir->i_sb); if (!OCFS2_IS_VALID_DIR_TRAILER(trailer)) { - rc = -EINVAL; - ocfs2_error(dir->i_sb, + rc = ocfs2_error(dir->i_sb, "Invalid dirblock #%llu: " "signature = %.*s\n", (unsigned long long)bh->b_blocknr, 7, @@ -489,8 +488,7 @@ static int ocfs2_check_dir_trailer(struct inode *dir, struct buffer_head *bh) goto out; } if (le64_to_cpu(trailer->db_blkno) != bh->b_blocknr) { - rc = -EINVAL; - ocfs2_error(dir->i_sb, + rc = ocfs2_error(dir->i_sb, "Directory block #%llu has an invalid " "db_blkno of %llu", (unsigned long long)bh->b_blocknr, @@ -499,8 +497,7 @@ static int ocfs2_check_dir_trailer(struct inode *dir, struct buffer_head *bh) } if (le64_to_cpu(trailer->db_parent_dinode) != OCFS2_I(dir)->ip_blkno) { - rc = -EINVAL; - ocfs2_error(dir->i_sb, + rc = ocfs2_error(dir->i_sb, "Directory block #%llu on dinode " "#%llu has an invalid parent_dinode " "of %llu", @@ -604,14 +601,13 @@ static int ocfs2_validate_dx_root(struct super_block *sb, } if (!OCFS2_IS_VALID_DX_ROOT(dx_root)) { - ocfs2_error(sb, + ret = ocfs2_error(sb, "Dir Index Root # %llu has bad signature %.*s", (unsigned long long)le64_to_cpu(dx_root->dr_blkno), 7, dx_root->dr_signature); - return -EINVAL; } - return 0; + return ret; } static int ocfs2_read_dx_root(struct inode *dir, struct ocfs2_dinode *di, @@ -648,12 +644,11 @@ static int ocfs2_validate_dx_leaf(struct super_block *sb, } if (!OCFS2_IS_VALID_DX_LEAF(dx_leaf)) { - ocfs2_error(sb, "Dir Index Leaf has bad signature %.*s", + ret = ocfs2_error(sb, "Dir Index Leaf has bad signature %.*s", 7, dx_leaf->dl_signature); - return -EROFS; } - return 0; + return ret; } static int ocfs2_read_dx_leaf(struct inode *dir, u64 blkno, @@ -812,11 +807,10 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode, el = &eb->h_list; if (el->l_tree_depth) { - ocfs2_error(inode->i_sb, + ret = ocfs2_error(inode->i_sb, "Inode %lu has non zero tree depth in " "btree tree block %llu\n", inode->i_ino, (unsigned long long)eb_bh->b_blocknr); - ret = -EROFS; goto out; } } @@ -832,11 +826,10 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode, } if (!found) { - ocfs2_error(inode->i_sb, "Inode %lu has bad extent " + ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent " "record (%u, %u, 0) in btree", inode->i_ino, le32_to_cpu(rec->e_cpos), ocfs2_rec_clusters(el, rec)); - ret = -EROFS; goto out; } diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c index 4e69f3cbc5f1..7868f7e7c455 100644 --- a/fs/ocfs2/inode.c +++ b/fs/ocfs2/inode.c @@ -1352,21 +1352,21 @@ int ocfs2_validate_inode_block(struct super_block *sb, rc = -EINVAL; if (!OCFS2_IS_VALID_DINODE(di)) { - ocfs2_error(sb, "Invalid dinode #%llu: signature = %.*s\n", + rc = ocfs2_error(sb, "Invalid dinode #%llu: signature = %.*s\n", (unsigned long long)bh->b_blocknr, 7, di->i_signature); goto bail; } if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) { - ocfs2_error(sb, "Invalid dinode #%llu: i_blkno is %llu\n", + rc = ocfs2_error(sb, "Invalid dinode #%llu: i_blkno is %llu\n", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(di->i_blkno)); goto bail; } if (!(di->i_flags & cpu_to_le32(OCFS2_VALID_FL))) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Invalid dinode #%llu: OCFS2_VALID_FL not set\n", (unsigned long long)bh->b_blocknr); goto bail; @@ -1374,7 +1374,7 @@ int ocfs2_validate_inode_block(struct super_block *sb, if (le32_to_cpu(di->i_fs_generation) != OCFS2_SB(sb)->fs_generation) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Invalid dinode #%llu: fs_generation is %u\n", (unsigned long long)bh->b_blocknr, le32_to_cpu(di->i_fs_generation)); diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c index 56a768d06aa6..70dd0ec7b7e9 100644 --- a/fs/ocfs2/move_extents.c +++ b/fs/ocfs2/move_extents.c @@ -99,11 +99,10 @@ static int __ocfs2_move_extent(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { - ocfs2_error(inode->i_sb, + ret = ocfs2_error(inode->i_sb, "Inode %llu has an extent at cpos %u which can no " "longer be found.\n", (unsigned long long)ino, cpos); - ret = -EROFS; goto out; } diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c index 7dc818b87cd8..b404dbde3fe4 100644 --- a/fs/ocfs2/refcounttree.c +++ b/fs/ocfs2/refcounttree.c @@ -102,32 +102,32 @@ static int ocfs2_validate_refcount_block(struct super_block *sb, if (!OCFS2_IS_VALID_REFCOUNT_BLOCK(rb)) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Refcount block #%llu has bad signature %.*s", (unsigned long long)bh->b_blocknr, 7, rb->rf_signature); - return -EINVAL; + goto out; } if (le64_to_cpu(rb->rf_blkno) != bh->b_blocknr) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Refcount block #%llu has an invalid rf_blkno " "of %llu", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(rb->rf_blkno)); - return -EINVAL; + goto out; } if (le32_to_cpu(rb->rf_fs_generation) != OCFS2_SB(sb)->fs_generation) { - ocfs2_error(sb, + rc = ocfs2_error(sb, "Refcount block #%llu has an invalid " "rf_fs_generation of #%u", (unsigned long long)bh->b_blocknr, le32_to_cpu(rb->rf_fs_generation)); - return -EINVAL; + goto out; } - - return 0; +out: + return rc; } static int ocfs2_read_refcount_block(struct ocfs2_caching_info *ci, @@ -1102,12 +1102,11 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci, el = &eb->h_list; if (el->l_tree_depth) { - ocfs2_error(sb, - "refcount tree %llu has non zero tree " - "depth in leaf btree tree block %llu\n", - (unsigned long long)ocfs2_metadata_cache_owner(ci), - (unsigned long long)eb_bh->b_blocknr); - ret = -EROFS; + ret = ocfs2_error(sb, + "refcount tree %llu has non zero tree " + "depth in leaf btree tree block %llu\n", + (unsigned long long)ocfs2_metadata_cache_owner(ci), + (unsigned long long)eb_bh->b_blocknr); goto out; } } @@ -2359,10 +2358,9 @@ static int ocfs2_mark_extent_refcounted(struct inode *inode, cpos, len, phys); if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) { - ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " + ret = ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " "tree, but the feature bit is not set in the " "super block.", inode->i_ino); - ret = -EROFS; goto out; } @@ -2545,10 +2543,9 @@ int ocfs2_prepare_refcount_change_for_del(struct inode *inode, u64 start_cpos = ocfs2_blocks_to_clusters(inode->i_sb, phys_blkno); if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) { - ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " + ret = ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " "tree, but the feature bit is not set in the " "super block.", inode->i_ino); - ret = -EROFS; goto out; } @@ -2672,11 +2669,10 @@ static int ocfs2_refcount_cal_cow_clusters(struct inode *inode, el = &eb->h_list; if (el->l_tree_depth) { - ocfs2_error(inode->i_sb, + ret = ocfs2_error(inode->i_sb, "Inode %lu has non zero tree depth in " "leaf block %llu\n", inode->i_ino, (unsigned long long)eb_bh->b_blocknr); - ret = -EROFS; goto out; } } @@ -3106,11 +3102,10 @@ static int ocfs2_clear_ext_refcount(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { - ocfs2_error(sb, + ret = ocfs2_error(sb, "Inode %llu has an extent at cpos %u which can no " "longer be found.\n", (unsigned long long)ino, cpos); - ret = -EROFS; goto out; } @@ -3376,10 +3371,9 @@ static int ocfs2_replace_cow(struct ocfs2_cow_context *context) struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) { - ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " + return ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " "tree, but the feature bit is not set in the " "super block.", inode->i_ino); - return -EROFS; } ocfs2_init_dealloc_ctxt(&context->dealloc); diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c index 4479029630bb..e4bb00110e91 100644 --- a/fs/ocfs2/suballoc.c +++ b/fs/ocfs2/suballoc.c @@ -171,7 +171,7 @@ static u32 ocfs2_bits_per_group(struct ocfs2_chain_list *cl) if (resize) \ mlog(ML_ERROR, fmt "\n", ##__VA_ARGS__); \ else \ - ocfs2_error(sb, fmt, ##__VA_ARGS__); \ + return ocfs2_error(sb, fmt, ##__VA_ARGS__); \ } while (0) static int ocfs2_validate_gd_self(struct super_block *sb, @@ -184,7 +184,6 @@ static int ocfs2_validate_gd_self(struct super_block *sb, do_error("Group descriptor #%llu has bad signature %.*s", (unsigned long long)bh->b_blocknr, 7, gd->bg_signature); - return -EINVAL; } if (le64_to_cpu(gd->bg_blkno) != bh->b_blocknr) { @@ -192,7 +191,6 @@ static int ocfs2_validate_gd_self(struct super_block *sb, "of %llu", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(gd->bg_blkno)); - return -EINVAL; } if (le32_to_cpu(gd->bg_generation) != OCFS2_SB(sb)->fs_generation) { @@ -200,7 +198,6 @@ static int ocfs2_validate_gd_self(struct super_block *sb, "fs_generation of #%u", (unsigned long long)bh->b_blocknr, le32_to_cpu(gd->bg_generation)); - return -EINVAL; } if (le16_to_cpu(gd->bg_free_bits_count) > le16_to_cpu(gd->bg_bits)) { @@ -209,7 +206,6 @@ static int ocfs2_validate_gd_self(struct super_block *sb, (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_bits), le16_to_cpu(gd->bg_free_bits_count)); - return -EINVAL; } if (le16_to_cpu(gd->bg_bits) > (8 * le16_to_cpu(gd->bg_size))) { @@ -218,7 +214,6 @@ static int ocfs2_validate_gd_self(struct super_block *sb, (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_bits), 8 * le16_to_cpu(gd->bg_size)); - return -EINVAL; } return 0; @@ -238,7 +233,6 @@ static int ocfs2_validate_gd_parent(struct super_block *sb, (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(gd->bg_parent_dinode), (unsigned long long)le64_to_cpu(di->i_blkno)); - return -EINVAL; } max_bits = le16_to_cpu(di->id2.i_chain.cl_cpg) * le16_to_cpu(di->id2.i_chain.cl_bpc); @@ -246,7 +240,6 @@ static int ocfs2_validate_gd_parent(struct super_block *sb, do_error("Group descriptor #%llu has bit count of %u", (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_bits)); - return -EINVAL; } /* In resize, we may meet the case bg_chain == cl_next_free_rec. */ @@ -257,7 +250,6 @@ static int ocfs2_validate_gd_parent(struct super_block *sb, do_error("Group descriptor #%llu has bad chain %u", (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_chain)); - return -EINVAL; } return 0; @@ -384,11 +376,10 @@ static int ocfs2_block_group_fill(handle_t *handle, struct super_block * sb = alloc_inode->i_sb; if (((unsigned long long) bg_bh->b_blocknr) != group_blkno) { - ocfs2_error(alloc_inode->i_sb, "group block (%llu) != " + status = ocfs2_error(alloc_inode->i_sb, "group block (%llu) != " "b_blocknr (%llu)", (unsigned long long)group_blkno, (unsigned long long) bg_bh->b_blocknr); - status = -EIO; goto bail; } @@ -834,9 +825,8 @@ static int ocfs2_reserve_suballoc_bits(struct ocfs2_super *osb, BUG_ON(!OCFS2_IS_VALID_DINODE(fe)); if (!(fe->i_flags & cpu_to_le32(OCFS2_CHAIN_FL))) { - ocfs2_error(alloc_inode->i_sb, "Invalid chain allocator %llu", + status = ocfs2_error(alloc_inode->i_sb, "Invalid chain allocator %llu", (unsigned long long)le64_to_cpu(fe->i_blkno)); - status = -EIO; goto bail; } @@ -1370,12 +1360,11 @@ int ocfs2_block_group_set_bits(handle_t *handle, le16_add_cpu(&bg->bg_free_bits_count, -num_bits); if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { - ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" + return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" " count %u but claims %u are freed. num_bits %d", (unsigned long long)le64_to_cpu(bg->bg_blkno), le16_to_cpu(bg->bg_bits), le16_to_cpu(bg->bg_free_bits_count), num_bits); - return -EROFS; } while(num_bits--) ocfs2_set_bit(bit_off++, bitmap); @@ -1905,13 +1894,12 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_alloc_context *ac, if (le32_to_cpu(fe->id1.bitmap1.i_used) >= le32_to_cpu(fe->id1.bitmap1.i_total)) { - ocfs2_error(ac->ac_inode->i_sb, + status = ocfs2_error(ac->ac_inode->i_sb, "Chain allocator dinode %llu has %u used " "bits but only %u total.", (unsigned long long)le64_to_cpu(fe->i_blkno), le32_to_cpu(fe->id1.bitmap1.i_used), le32_to_cpu(fe->id1.bitmap1.i_total)); - status = -EIO; goto bail; } @@ -2429,12 +2417,11 @@ static int ocfs2_block_group_clear_bits(handle_t *handle, } le16_add_cpu(&bg->bg_free_bits_count, num_bits); if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { - ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" + return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" " count %u but claims %u are freed. num_bits %d", (unsigned long long)le64_to_cpu(bg->bg_blkno), le16_to_cpu(bg->bg_bits), le16_to_cpu(bg->bg_free_bits_count), num_bits); - return -EROFS; } if (undo_fn) diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index a24f264b2fc4..5944a311bb94 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -499,30 +499,27 @@ static int ocfs2_validate_xattr_block(struct super_block *sb, */ if (!OCFS2_IS_VALID_XATTR_BLOCK(xb)) { - ocfs2_error(sb, + return ocfs2_error(sb, "Extended attribute block #%llu has bad " "signature %.*s", (unsigned long long)bh->b_blocknr, 7, xb->xb_signature); - return -EINVAL; } if (le64_to_cpu(xb->xb_blkno) != bh->b_blocknr) { - ocfs2_error(sb, + return ocfs2_error(sb, "Extended attribute block #%llu has an " "invalid xb_blkno of %llu", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(xb->xb_blkno)); - return -EINVAL; } if (le32_to_cpu(xb->xb_fs_generation) != OCFS2_SB(sb)->fs_generation) { - ocfs2_error(sb, + return ocfs2_error(sb, "Extended attribute block #%llu has an invalid " "xb_fs_generation of #%u", (unsigned long long)bh->b_blocknr, le32_to_cpu(xb->xb_fs_generation)); - return -EINVAL; } return 0; @@ -3694,11 +3691,10 @@ static int ocfs2_xattr_get_rec(struct inode *inode, el = &eb->h_list; if (el->l_tree_depth) { - ocfs2_error(inode->i_sb, + ret = ocfs2_error(inode->i_sb, "Inode %lu has non zero tree depth in " "xattr tree block %llu\n", inode->i_ino, (unsigned long long)eb_bh->b_blocknr); - ret = -EROFS; goto out; } } @@ -3713,11 +3709,10 @@ static int ocfs2_xattr_get_rec(struct inode *inode, } if (!e_blkno) { - ocfs2_error(inode->i_sb, "Inode %lu has bad extent " + ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent " "record (%u, %u, 0) in xattr", inode->i_ino, le32_to_cpu(rec->e_cpos), ocfs2_rec_clusters(el, rec)); - ret = -EROFS; goto out; } -- cgit v1.2.1 From 34237681e02ad1617138926f437d0a147249ec13 Mon Sep 17 00:00:00 2001 From: Goldwyn Rodrigues Date: Fri, 4 Sep 2015 15:44:20 -0700 Subject: ocfs2: clear the rest of the buffers on error In case a validation fails, clear the rest of the buffers and return the error to the calling function. This also facilitates bubbling up the error originating from ocfs2_error to calling functions. Signed-off-by: Goldwyn Rodrigues Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/buffer_head_io.c | 6 ++++++ 1 file changed, 6 insertions(+) (limited to 'fs') diff --git a/fs/ocfs2/buffer_head_io.c b/fs/ocfs2/buffer_head_io.c index 1edcb141f639..fe50ded1b4ce 100644 --- a/fs/ocfs2/buffer_head_io.c +++ b/fs/ocfs2/buffer_head_io.c @@ -316,6 +316,12 @@ int ocfs2_read_blocks(struct ocfs2_caching_info *ci, u64 block, int nr, bh = bhs[i]; if (!(flags & OCFS2_BH_READAHEAD)) { + if (status) { + /* Clear the rest of the buffers on error */ + put_bh(bh); + bhs[i] = NULL; + continue; + } /* We know this can't have changed as we hold the * owner sem. Avoid doing any work on the bh if the * journal has it. */ -- cgit v1.2.1 From 6ab855a99b735c227ad1e0deda636833f41c5b87 Mon Sep 17 00:00:00 2001 From: WeiWei Wang Date: Fri, 4 Sep 2015 15:44:23 -0700 Subject: ocfs2: add ip_alloc_sem in direct IO to protect allocation changes In ocfs2, ip_alloc_sem is used to protect allocation changes on the node. In direct IO, we add ip_alloc_sem to protect date consistent between direct-io and ocfs2_truncate_file race (buffer io use ip_alloc_sem already). Although inode->i_mutex lock is used to avoid concurrency of above situation, i think ip_alloc_sem is still needed because protect allocation changes is significant. Other filesystem like ext4 also uses rw_semaphore to protect data consistent between get_block-vs-truncate race by other means, So ip_alloc_sem in ocfs2 direct io is needed. Signed-off-by: Weiwei Wang Signed-off-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/aops.c | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index b36dcad3a140..a7ab145e2901 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -533,10 +533,14 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, inode_blocks = ocfs2_blocks_for_bytes(inode->i_sb, i_size_read(inode)); + down_read(&OCFS2_I(inode)->ip_alloc_sem); + /* This figures out the size of the next contiguous block, and * our logical offset */ ret = ocfs2_extent_map_get_blocks(inode, iblock, &p_blkno, &contig_blocks, &ext_flags); + up_read(&OCFS2_I(inode)->ip_alloc_sem); + if (ret) { mlog(ML_ERROR, "get_blocks() failed iblock=%llu\n", (unsigned long long)iblock); @@ -557,6 +561,8 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, alloc_locked = 1; + down_write(&OCFS2_I(inode)->ip_alloc_sem); + /* fill hole, allocate blocks can't be larger than the size * of the hole */ clusters_to_alloc = ocfs2_clusters_for_bytes(inode->i_sb, len); @@ -569,6 +575,7 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, ret = ocfs2_extend_allocation(inode, cpos, clusters_to_alloc, 0); if (ret < 0) { + up_write(&OCFS2_I(inode)->ip_alloc_sem); mlog_errno(ret); goto bail; } @@ -576,11 +583,13 @@ static int ocfs2_direct_IO_get_blocks(struct inode *inode, sector_t iblock, ret = ocfs2_extent_map_get_blocks(inode, iblock, &p_blkno, &contig_blocks, &ext_flags); if (ret < 0) { + up_write(&OCFS2_I(inode)->ip_alloc_sem); mlog(ML_ERROR, "get_blocks() failed iblock=%llu\n", (unsigned long long)iblock); ret = -EIO; goto bail; } + up_write(&OCFS2_I(inode)->ip_alloc_sem); } /* @@ -835,12 +844,17 @@ static ssize_t ocfs2_direct_IO_write(struct kiocb *iocb, /* zeroing out the previously allocated cluster tail * that but not zeroed */ - if (ocfs2_sparse_alloc(OCFS2_SB(inode->i_sb))) + if (ocfs2_sparse_alloc(OCFS2_SB(inode->i_sb))) { + down_read(&OCFS2_I(inode)->ip_alloc_sem); ret = ocfs2_direct_IO_zero_extend(osb, inode, offset, zero_len_tail, cluster_align_tail); - else + up_read(&OCFS2_I(inode)->ip_alloc_sem); + } else { + down_write(&OCFS2_I(inode)->ip_alloc_sem); ret = ocfs2_direct_IO_extend_no_holes(osb, inode, offset); + up_write(&OCFS2_I(inode)->ip_alloc_sem); + } if (ret < 0) { mlog_errno(ret); ocfs2_inode_unlock(inode, 1); -- cgit v1.2.1 From 928dda1f9433f024ac48c3d97ae683bf83dd0e42 Mon Sep 17 00:00:00 2001 From: Yiwen Jiang Date: Fri, 4 Sep 2015 15:44:25 -0700 Subject: ocfs2: fix a tiny case that inode can not removed When running dirop_fileop_racer we found a case that inode can not removed. Two nodes, say Node A and Node B, mount the same ocfs2 volume. Create two dirs /race/1/ and /race/2/ in the filesystem. Node A Node B rm -r /race/2/ mv /race/1/ /race/2/ call ocfs2_unlink(), get the EX mode of /race/2/ wait for B unlock /race/2/ decrease i_nlink of /race/2/ to 0, and add inode of /race/2/ into orphan dir, unlock /race/2/ got EX mode of /race/2/. because /race/1/ is dir, so inc i_nlink of /race/2/ and update into disk, unlock /race/2/ because i_nlink of /race/2/ is not zero, this inode will always remain in orphan dir This patch fixes this case by test whether i_nlink of new dir is zero. Signed-off-by: Yiwen Jiang Reviewed-by: Mark Fasheh Cc: Joel Becker Cc: Joseph Qi Cc: Xue jiufei Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/namei.c | 5 +++++ 1 file changed, 5 insertions(+) (limited to 'fs') diff --git a/fs/ocfs2/namei.c b/fs/ocfs2/namei.c index 1c43993e81b0..b7dfac226b1e 100644 --- a/fs/ocfs2/namei.c +++ b/fs/ocfs2/namei.c @@ -1309,6 +1309,11 @@ static int ocfs2_rename(struct inode *old_dir, } parents_locked = 1; + if (!new_dir->i_nlink) { + status = -EACCES; + goto bail; + } + /* make sure both dirs have bhs * get an extra ref on old_dir_bh if old==new */ if (!new_dir_bh) { -- cgit v1.2.1 From 72f6fe1fe5a386225cdc30f025681830a63a117e Mon Sep 17 00:00:00 2001 From: "Norton.Zhu" Date: Fri, 4 Sep 2015 15:44:28 -0700 Subject: ocfs2: optimize error handling in dlm_request_join Currently error handling in dlm_request_join is a little obscure, so optimize it to promote readability. If packet.code is invalid, reset it to JOIN_DISALLOW to keep it meaningful. It only influences the log printing. Signed-off-by: Norton.Zhu Cc: Srinivas Eeda Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/dlm/dlmdomain.c | 71 ++++++++++++++++++++++++++---------------------- 1 file changed, 39 insertions(+), 32 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/dlm/dlmdomain.c b/fs/ocfs2/dlm/dlmdomain.c index 019459b20aeb..6918f30d02cd 100644 --- a/fs/ocfs2/dlm/dlmdomain.c +++ b/fs/ocfs2/dlm/dlmdomain.c @@ -1465,39 +1465,46 @@ static int dlm_request_join(struct dlm_ctxt *dlm, if (status == -ENOPROTOOPT) { status = 0; *response = JOIN_OK_NO_MAP; - } else if (packet.code == JOIN_DISALLOW || - packet.code == JOIN_OK_NO_MAP) { - *response = packet.code; - } else if (packet.code == JOIN_PROTOCOL_MISMATCH) { - mlog(ML_NOTICE, - "This node requested DLM locking protocol %u.%u and " - "filesystem locking protocol %u.%u. At least one of " - "the protocol versions on node %d is not compatible, " - "disconnecting\n", - dlm->dlm_locking_proto.pv_major, - dlm->dlm_locking_proto.pv_minor, - dlm->fs_locking_proto.pv_major, - dlm->fs_locking_proto.pv_minor, - node); - status = -EPROTO; - *response = packet.code; - } else if (packet.code == JOIN_OK) { - *response = packet.code; - /* Use the same locking protocol as the remote node */ - dlm->dlm_locking_proto.pv_minor = packet.dlm_minor; - dlm->fs_locking_proto.pv_minor = packet.fs_minor; - mlog(0, - "Node %d responds JOIN_OK with DLM locking protocol " - "%u.%u and fs locking protocol %u.%u\n", - node, - dlm->dlm_locking_proto.pv_major, - dlm->dlm_locking_proto.pv_minor, - dlm->fs_locking_proto.pv_major, - dlm->fs_locking_proto.pv_minor); } else { - status = -EINVAL; - mlog(ML_ERROR, "invalid response %d from node %u\n", - packet.code, node); + *response = packet.code; + switch (packet.code) { + case JOIN_DISALLOW: + case JOIN_OK_NO_MAP: + break; + case JOIN_PROTOCOL_MISMATCH: + mlog(ML_NOTICE, + "This node requested DLM locking protocol %u.%u and " + "filesystem locking protocol %u.%u. At least one of " + "the protocol versions on node %d is not compatible, " + "disconnecting\n", + dlm->dlm_locking_proto.pv_major, + dlm->dlm_locking_proto.pv_minor, + dlm->fs_locking_proto.pv_major, + dlm->fs_locking_proto.pv_minor, + node); + status = -EPROTO; + break; + case JOIN_OK: + /* Use the same locking protocol as the remote node */ + dlm->dlm_locking_proto.pv_minor = packet.dlm_minor; + dlm->fs_locking_proto.pv_minor = packet.fs_minor; + mlog(0, + "Node %d responds JOIN_OK with DLM locking protocol " + "%u.%u and fs locking protocol %u.%u\n", + node, + dlm->dlm_locking_proto.pv_major, + dlm->dlm_locking_proto.pv_minor, + dlm->fs_locking_proto.pv_major, + dlm->fs_locking_proto.pv_minor); + break; + default: + status = -EINVAL; + mlog(ML_ERROR, "invalid response %d from node %u\n", + packet.code, node); + /* Reset response to JOIN_DISALLOW */ + *response = JOIN_DISALLOW; + break; + } } mlog(0, "status %d, node %d response is %d\n", status, node, -- cgit v1.2.1 From 3d46a44a0c01b15d385ccaae24b56f619613c256 Mon Sep 17 00:00:00 2001 From: Tariq Saeed Date: Fri, 4 Sep 2015 15:44:31 -0700 Subject: ocfs2: fix BUG_ON() in ocfs2_ci_checkpointed() PID: 614 TASK: ffff882a739da580 CPU: 3 COMMAND: "ocfs2dc" #0 [ffff882ecc3759b0] machine_kexec at ffffffff8103b35d #1 [ffff882ecc375a20] crash_kexec at ffffffff810b95b5 #2 [ffff882ecc375af0] oops_end at ffffffff815091d8 #3 [ffff882ecc375b20] die at ffffffff8101868b #4 [ffff882ecc375b50] do_trap at ffffffff81508bb0 #5 [ffff882ecc375ba0] do_invalid_op at ffffffff810165e5 #6 [ffff882ecc375c40] invalid_op at ffffffff815116fb [exception RIP: ocfs2_ci_checkpointed+208] RIP: ffffffffa0a7e940 RSP: ffff882ecc375cf0 RFLAGS: 00010002 RAX: 0000000000000001 RBX: 000000000000654b RCX: ffff8812dc83f1f8 RDX: 00000000000017d9 RSI: ffff8812dc83f1f8 RDI: ffffffffa0b2c318 RBP: ffff882ecc375d20 R8: ffff882ef6ecfa60 R9: ffff88301f272200 R10: 0000000000000000 R11: 0000000000000000 R12: ffffffffffffffff R13: ffff8812dc83f4f0 R14: 0000000000000000 R15: ffff8812dc83f1f8 ORIG_RAX: ffffffffffffffff CS: 0010 SS: 0018 #7 [ffff882ecc375d28] ocfs2_check_meta_downconvert at ffffffffa0a7edbd [ocfs2] #8 [ffff882ecc375d38] ocfs2_unblock_lock at ffffffffa0a84af8 [ocfs2] #9 [ffff882ecc375dc8] ocfs2_process_blocked_lock at ffffffffa0a85285 [ocfs2] #10 [ffff882ecc375e18] ocfs2_downconvert_thread_do_work at ffffffffa0a85445 [ocfs2] #11 [ffff882ecc375e68] ocfs2_downconvert_thread at ffffffffa0a854de [ocfs2] #12 [ffff882ecc375ee8] kthread at ffffffff81090da7 #13 [ffff882ecc375f48] kernel_thread_helper at ffffffff81511884 assert is tripped because the tran is not checkpointed and the lock level is PR. Some time ago, chmod command had been executed. As result, the following call chain left the inode cluster lock in PR state, latter on causing the assert. system_call_fastpath -> my_chmod -> sys_chmod -> sys_fchmodat -> notify_change -> ocfs2_setattr -> posix_acl_chmod -> ocfs2_iop_set_acl -> ocfs2_set_acl -> ocfs2_acl_set_mode Here is how. 1119 int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) 1120 { 1247 ocfs2_inode_unlock(inode, 1); <<< WRONG thing to do. .. 1258 if (!status && attr->ia_valid & ATTR_MODE) { 1259 status = posix_acl_chmod(inode, inode->i_mode); 519 posix_acl_chmod(struct inode *inode, umode_t mode) 520 { .. 539 ret = inode->i_op->set_acl(inode, acl, ACL_TYPE_ACCESS); 287 int ocfs2_iop_set_acl(struct inode *inode, struct posix_acl *acl, ... 288 { 289 return ocfs2_set_acl(NULL, inode, NULL, type, acl, NULL, NULL); 224 int ocfs2_set_acl(handle_t *handle, 225 struct inode *inode, ... 231 { .. 252 ret = ocfs2_acl_set_mode(inode, di_bh, 253 handle, mode); 168 static int ocfs2_acl_set_mode(struct inode *inode, struct buffer_head ... 170 { 183 if (handle == NULL) { >>> BUG: inode lock not held in ex at this point <<< 184 handle = ocfs2_start_trans(OCFS2_SB(inode->i_sb), 185 OCFS2_INODE_UPDATE_CREDITS); ocfs2_setattr.#1247 we unlock and at #1259 call posix_acl_chmod. When we reach ocfs2_acl_set_mode.#181 and do trans, the inode cluster lock is not held in EX mode (it should be). How this could have happended? We are the lock master, were holding lock EX and have released it in ocfs2_setattr.#1247. Note that there are no holders of this lock at this point. Another node needs the lock in PR, and we downconvert from EX to PR. So the inode lock is PR when do the trans in ocfs2_acl_set_mode.#184. The trans stays in core (not flushed to disc). Now another node want the lock in EX, downconvert thread gets kicked (the one that tripped assert abovt), finds an unflushed trans but the lock is not EX (it is PR). If the lock was at EX, it would have flushed the trans ocfs2_ci_checkpointed -> ocfs2_start_checkpoint before downconverting (to NULL) for the request. ocfs2_setattr must not drop inode lock ex in this code path. If it does, takes it again before the trans, say in ocfs2_set_acl, another cluster node can get in between, execute another setattr, overwriting the one in progress on this node, resulting in a mode acl size combo that is a mix of the two. Orabug: 20189959 Signed-off-by: Tariq Saeed Reviewed-by: Mark Fasheh Cc: Joel Becker Cc: Joseph Qi Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/file.c | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ocfs2/file.c b/fs/ocfs2/file.c index c4a99fb61c3e..0e5b4515f92e 100644 --- a/fs/ocfs2/file.c +++ b/fs/ocfs2/file.c @@ -1130,6 +1130,7 @@ out: int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) { int status = 0, size_change; + int inode_locked = 0; struct inode *inode = d_inode(dentry); struct super_block *sb = inode->i_sb; struct ocfs2_super *osb = OCFS2_SB(sb); @@ -1178,6 +1179,7 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) mlog_errno(status); goto bail_unlock_rw; } + inode_locked = 1; if (size_change) { status = inode_newsize_ok(inode, attr->ia_size); @@ -1258,7 +1260,10 @@ int ocfs2_setattr(struct dentry *dentry, struct iattr *attr) bail_commit: ocfs2_commit_trans(osb, handle); bail_unlock: - ocfs2_inode_unlock(inode, 1); + if (status) { + ocfs2_inode_unlock(inode, 1); + inode_locked = 0; + } bail_unlock_rw: if (size_change) ocfs2_rw_unlock(inode, 1); @@ -1274,6 +1279,8 @@ bail: if (status < 0) mlog_errno(status); } + if (inode_locked) + ocfs2_inode_unlock(inode, 1); return status; } -- cgit v1.2.1 From 743b5f1434f57a147226c747fe228cadeb7b05ed Mon Sep 17 00:00:00 2001 From: Tariq Saeed Date: Fri, 4 Sep 2015 15:44:34 -0700 Subject: ocfs2: take inode lock in ocfs2_iop_set/get_acl() This bug in mainline code is pointed out by Mark Fasheh. When ocfs2_iop_set_acl() and ocfs2_iop_get_acl() are entered from VFS layer, inode lock is not held. This seems to be regression from older kernels. The patch is to fix that. Orabug: 20189959 Signed-off-by: Tariq Saeed Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/acl.c | 26 ++++++++++++++++++++------ 1 file changed, 20 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/acl.c b/fs/ocfs2/acl.c index c58a1bcfda0f..0cdf497c91ef 100644 --- a/fs/ocfs2/acl.c +++ b/fs/ocfs2/acl.c @@ -284,7 +284,19 @@ int ocfs2_set_acl(handle_t *handle, int ocfs2_iop_set_acl(struct inode *inode, struct posix_acl *acl, int type) { - return ocfs2_set_acl(NULL, inode, NULL, type, acl, NULL, NULL); + struct buffer_head *bh = NULL; + int status = 0; + + status = ocfs2_inode_lock(inode, &bh, 1); + if (status < 0) { + if (status != -ENOENT) + mlog_errno(status); + return status; + } + status = ocfs2_set_acl(NULL, inode, bh, type, acl, NULL, NULL); + ocfs2_inode_unlock(inode, 1); + brelse(bh); + return status; } struct posix_acl *ocfs2_iop_get_acl(struct inode *inode, int type) @@ -292,19 +304,21 @@ struct posix_acl *ocfs2_iop_get_acl(struct inode *inode, int type) struct ocfs2_super *osb; struct buffer_head *di_bh = NULL; struct posix_acl *acl; - int ret = -EAGAIN; + int ret; osb = OCFS2_SB(inode->i_sb); if (!(osb->s_mount_opt & OCFS2_MOUNT_POSIX_ACL)) return NULL; - - ret = ocfs2_read_inode_block(inode, &di_bh); - if (ret < 0) + ret = ocfs2_inode_lock(inode, &di_bh, 0); + if (ret < 0) { + if (ret != -ENOENT) + mlog_errno(ret); return ERR_PTR(ret); + } acl = ocfs2_get_acl_nolock(inode, type, di_bh); + ocfs2_inode_unlock(inode, 0); brelse(di_bh); - return acl; } -- cgit v1.2.1 From f57a22ddecd6f26040a67e2c12880f98f88b6e00 Mon Sep 17 00:00:00 2001 From: Yiwen Jiang Date: Fri, 4 Sep 2015 15:44:37 -0700 Subject: ocfs2: avoid access invalid address when read o2dlm debug messages The following case will lead to a lockres is freed but is still in use. cat /sys/kernel/debug/o2dlm/locking_state dlm_thread lockres_seq_start -> lock dlm->track_lock -> get resA resA->refs decrease to 0, call dlm_lockres_release, and wait for "cat" unlock. Although resA->refs is already set to 0, increase resA->refs, and then unlock lock dlm->track_lock -> list_del_init() -> unlock -> free resA In such a race case, invalid address access may occurs. So we should delete list res->tracking before resA->refs decrease to 0. Signed-off-by: Yiwen Jiang Reviewed-by: Joseph Qi Cc: Joel Becker Signed-off-by: Mark Fasheh Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/dlm/dlmmaster.c | 22 +++++++++++----------- fs/ocfs2/dlm/dlmthread.c | 10 ++++++++++ 2 files changed, 21 insertions(+), 11 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/dlm/dlmmaster.c b/fs/ocfs2/dlm/dlmmaster.c index fdf4b41d0609..46b8b2bbc95a 100644 --- a/fs/ocfs2/dlm/dlmmaster.c +++ b/fs/ocfs2/dlm/dlmmaster.c @@ -498,16 +498,6 @@ static void dlm_lockres_release(struct kref *kref) mlog(0, "destroying lockres %.*s\n", res->lockname.len, res->lockname.name); - spin_lock(&dlm->track_lock); - if (!list_empty(&res->tracking)) - list_del_init(&res->tracking); - else { - mlog(ML_ERROR, "Resource %.*s not on the Tracking list\n", - res->lockname.len, res->lockname.name); - dlm_print_one_lock_resource(res); - } - spin_unlock(&dlm->track_lock); - atomic_dec(&dlm->res_cur_count); if (!hlist_unhashed(&res->hash_node) || @@ -795,8 +785,18 @@ lookup: dlm_lockres_grab_inflight_ref(dlm, tmpres); spin_unlock(&tmpres->spinlock); - if (res) + if (res) { + spin_lock(&dlm->track_lock); + if (!list_empty(&res->tracking)) + list_del_init(&res->tracking); + else + mlog(ML_ERROR, "Resource %.*s not " + "on the Tracking list\n", + res->lockname.len, + res->lockname.name); + spin_unlock(&dlm->track_lock); dlm_lockres_put(res); + } res = tmpres; goto leave; } diff --git a/fs/ocfs2/dlm/dlmthread.c b/fs/ocfs2/dlm/dlmthread.c index 69aac6f088ad..2e5e6d5fffe8 100644 --- a/fs/ocfs2/dlm/dlmthread.c +++ b/fs/ocfs2/dlm/dlmthread.c @@ -211,6 +211,16 @@ static void dlm_purge_lockres(struct dlm_ctxt *dlm, __dlm_unhash_lockres(dlm, res); + spin_lock(&dlm->track_lock); + if (!list_empty(&res->tracking)) + list_del_init(&res->tracking); + else { + mlog(ML_ERROR, "Resource %.*s not on the Tracking list\n", + res->lockname.len, res->lockname.name); + __dlm_print_one_lock_resource(res); + } + spin_unlock(&dlm->track_lock); + /* lockres is not in the hash now. drop the flag and wake up * any processes waiting in dlm_get_lock_resource. */ if (!master) { -- cgit v1.2.1 From ad694821224634d46b6571f0161e85ac2e397396 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:44:40 -0700 Subject: ocfs2: fix race between crashed dio and rm There is a race case between crashed dio and rm, which will lead to OCFS2_VALID_FL not set read-only. N1 N2 ------------------------------------------------------------------------ dd with direct flag rm file crashed with an dio entry left in orphan dir clear OCFS2_VALID_FL in ocfs2_remove_inode recover N1 and read the corrupted inode, and set filesystem read-only So we skip the inode deletion this time and wait for dio entry recovered first. Signed-off-by: Joseph Qi Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/inode.c | 9 +++++++++ fs/ocfs2/journal.c | 4 +++- 2 files changed, 12 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c index 7868f7e7c455..fe4b3f7db245 100644 --- a/fs/ocfs2/inode.c +++ b/fs/ocfs2/inode.c @@ -971,6 +971,7 @@ static void ocfs2_delete_inode(struct inode *inode) int wipe, status; sigset_t oldset; struct buffer_head *di_bh = NULL; + struct ocfs2_dinode *di = NULL; trace_ocfs2_delete_inode(inode->i_ino, (unsigned long long)OCFS2_I(inode)->ip_blkno, @@ -1025,6 +1026,14 @@ static void ocfs2_delete_inode(struct inode *inode) goto bail_unlock_nfs_sync; } + di = (struct ocfs2_dinode *)di_bh->b_data; + /* Skip inode deletion and wait for dio orphan entry recovered + * first */ + if (unlikely(di->i_flags & cpu_to_le32(OCFS2_DIO_ORPHANED_FL))) { + ocfs2_cleanup_delete_inode(inode, 0); + goto bail_unlock_inode; + } + /* Query the cluster. This will be the final decision made * before we go ahead and wipe the inode. */ status = ocfs2_query_inode_wipe(inode, di_bh, &wipe); diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index 3bfd36a23e40..52948af646b6 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -2210,7 +2210,9 @@ static int ocfs2_recover_orphans(struct ocfs2_super *osb, * ocfs2_delete_inode. */ oi->ip_flags |= OCFS2_INODE_MAYBE_ORPHANED; spin_unlock(&oi->ip_lock); - } else if ((orphan_reco_type == ORPHAN_NEED_TRUNCATE) && + } + + if ((orphan_reco_type == ORPHAN_NEED_TRUNCATE) && (di->i_flags & cpu_to_le32(OCFS2_DIO_ORPHANED_FL))) { ret = ocfs2_truncate_file(inode, di_bh, i_size_read(inode)); -- cgit v1.2.1 From 40476b8294466d40e7db57b4cbf69a831a4486b8 Mon Sep 17 00:00:00 2001 From: Tina Ruchandani Date: Fri, 4 Sep 2015 15:44:43 -0700 Subject: ocfs2: use 64bit variables to track heartbeat time o2hb_elapsed_msecs computes the time taken for a disk heartbeat. 'struct timeval' variables are used to store start and end times. On 32-bit systems, the 'tv_sec' component of 'struct timeval' will overflow in year 2038 and beyond. This patch solves the overflow with the following: 1. Replace o2hb_elapsed_msecs using 'ktime_t' values to measure start and end time, and built-in function 'ktime_ms_delta' to compute the elapsed time. ktime_get_real() is used since the code prints out the wallclock time. 2. Changes format string to print time as a single 64-bit nanoseconds value ("%lld") instead of seconds and microseconds. This simplifies the code since converting ktime_t to that format would need expensive computation. However, the debug log string is less readable than the previous format. Signed-off-by: Tina Ruchandani Suggested by: Arnd Bergmann Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/cluster/heartbeat.c | 49 ++++++++------------------------------------ 1 file changed, 9 insertions(+), 40 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/cluster/heartbeat.c b/fs/ocfs2/cluster/heartbeat.c index f97306453a0b..fa15debcc02b 100644 --- a/fs/ocfs2/cluster/heartbeat.c +++ b/fs/ocfs2/cluster/heartbeat.c @@ -36,7 +36,7 @@ #include #include #include - +#include #include "heartbeat.h" #include "tcp.h" #include "nodemanager.h" @@ -1060,37 +1060,6 @@ bail: return ret; } -/* Subtract b from a, storing the result in a. a *must* have a larger - * value than b. */ -static void o2hb_tv_subtract(struct timeval *a, - struct timeval *b) -{ - /* just return 0 when a is after b */ - if (a->tv_sec < b->tv_sec || - (a->tv_sec == b->tv_sec && a->tv_usec < b->tv_usec)) { - a->tv_sec = 0; - a->tv_usec = 0; - return; - } - - a->tv_sec -= b->tv_sec; - a->tv_usec -= b->tv_usec; - while ( a->tv_usec < 0 ) { - a->tv_sec--; - a->tv_usec += 1000000; - } -} - -static unsigned int o2hb_elapsed_msecs(struct timeval *start, - struct timeval *end) -{ - struct timeval res = *end; - - o2hb_tv_subtract(&res, start); - - return res.tv_sec * 1000 + res.tv_usec / 1000; -} - /* * we ride the region ref that the region dir holds. before the region * dir is removed and drops it ref it will wait to tear down this @@ -1101,7 +1070,7 @@ static int o2hb_thread(void *data) int i, ret; struct o2hb_region *reg = data; struct o2hb_bio_wait_ctxt write_wc; - struct timeval before_hb, after_hb; + ktime_t before_hb, after_hb; unsigned int elapsed_msec; mlog(ML_HEARTBEAT|ML_KTHREAD, "hb thread running\n"); @@ -1118,18 +1087,18 @@ static int o2hb_thread(void *data) * hr_timeout_ms between disk writes. On busy systems * this should result in a heartbeat which is less * likely to time itself out. */ - do_gettimeofday(&before_hb); + before_hb = ktime_get_real(); ret = o2hb_do_disk_heartbeat(reg); - do_gettimeofday(&after_hb); - elapsed_msec = o2hb_elapsed_msecs(&before_hb, &after_hb); + after_hb = ktime_get_real(); + + elapsed_msec = (unsigned int) + ktime_ms_delta(after_hb, before_hb); mlog(ML_HEARTBEAT, - "start = %lu.%lu, end = %lu.%lu, msec = %u, ret = %d\n", - before_hb.tv_sec, (unsigned long) before_hb.tv_usec, - after_hb.tv_sec, (unsigned long) after_hb.tv_usec, - elapsed_msec, ret); + "start = %lld, end = %lld, msec = %u, ret = %d\n", + before_hb.tv64, after_hb.tv64, elapsed_msec, ret); if (!kthread_should_stop() && elapsed_msec < reg->hr_timeout_ms) { -- cgit v1.2.1 From 7f27ec978b0ef37391262bbf15c587fd8526e268 Mon Sep 17 00:00:00 2001 From: yangwenfang Date: Fri, 4 Sep 2015 15:44:45 -0700 Subject: ocfs2: call ocfs2_journal_access_di() before ocfs2_journal_dirty() in ocfs2_write_end_nolock() 1: After we call ocfs2_journal_access_di() in ocfs2_write_begin(), jbd2_journal_restart() may also be called, in this function transaction A's t_updates-- and obtains a new transaction B. If jbd2_journal_commit_transaction() is happened to commit transaction A, when t_updates==0, it will continue to complete commit and unfile buffer. So when jbd2_journal_dirty_metadata(), the handle is pointed a new transaction B, and the buffer head's journal head is already freed, jh->b_transaction == NULL, jh->b_next_transaction == NULL, it returns EINVAL, So it triggers the BUG_ON(status). thread 1 jbd2 ocfs2_write_begin jbd2_journal_commit_transaction ocfs2_write_begin_nolock ocfs2_start_trans jbd2__journal_start(t_updates+1, transaction A) ocfs2_journal_access_di ocfs2_write_cluster_by_desc ocfs2_mark_extent_written ocfs2_change_extent_flag ocfs2_split_extent ocfs2_extend_rotate_transaction jbd2_journal_restart (t_updates-1,transaction B) t_updates==0 __jbd2_journal_refile_buffer (jh->b_transaction = NULL) ocfs2_write_end ocfs2_write_end_nolock ocfs2_journal_dirty jbd2_journal_dirty_metadata(bug) ocfs2_commit_trans 2. In ext4, I found that: jbd2_journal_get_write_access() called by ext4_write_end. ext4_write_begin ext4_journal_start __ext4_journal_start_sb ext4_journal_check_start jbd2__journal_start ext4_write_end ext4_mark_inode_dirty ext4_reserve_inode_write ext4_journal_get_write_access jbd2_journal_get_write_access ext4_mark_iloc_dirty ext4_do_update_inode ext4_handle_dirty_metadata jbd2_journal_dirty_metadata 3. So I think we should put ocfs2_journal_access_di before ocfs2_journal_dirty in the ocfs2_write_end. and it works well after my modification. Signed-off-by: vicky Reviewed-by: Mark Fasheh Cc: Joel Becker Cc: Zhangguanghui Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/aops.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index a7ab145e2901..faf36a96cd19 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -2207,10 +2207,7 @@ try_again: if (ret) goto out_commit; } - /* - * We don't want this to fail in ocfs2_write_end(), so do it - * here. - */ + ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), wc->w_di_bh, OCFS2_JOURNAL_ACCESS_WRITE); if (ret) { @@ -2367,7 +2364,7 @@ int ocfs2_write_end_nolock(struct address_space *mapping, loff_t pos, unsigned len, unsigned copied, struct page *page, void *fsdata) { - int i; + int i, ret; unsigned from, to, start = pos & (PAGE_CACHE_SIZE - 1); struct inode *inode = mapping->host; struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); @@ -2376,6 +2373,14 @@ int ocfs2_write_end_nolock(struct address_space *mapping, handle_t *handle = wc->w_handle; struct page *tmppage; + ret = ocfs2_journal_access_di(handle, INODE_CACHE(inode), wc->w_di_bh, + OCFS2_JOURNAL_ACCESS_WRITE); + if (ret) { + copied = ret; + mlog_errno(ret); + goto out; + } + if (OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL) { ocfs2_write_end_inline(inode, pos, len, &copied, di, wc); goto out_write_size; @@ -2431,6 +2436,7 @@ out_write_size: ocfs2_update_inode_fsync_trans(handle, inode, 1); ocfs2_journal_dirty(handle, wc->w_di_bh); +out: /* unlock pages before dealloc since it needs acquiring j_trans_barrier * lock, or it will cause a deadlock since journal commit threads holds * this lock and will ask for the page lock when flushing the data. -- cgit v1.2.1 From d0c97d52f5e1de125394d748be7bd5763fd9ed9e Mon Sep 17 00:00:00 2001 From: Xue jiufei Date: Fri, 4 Sep 2015 15:44:48 -0700 Subject: ocfs2: do not set fs read-only if rec[0] is empty while committing truncate While appending an extent to a file, it will call these functions: ocfs2_insert_extent -> call ocfs2_grow_tree() if there's no free rec -> ocfs2_add_branch add a new branch to extent tree, now rec[0] in the leaf of rightmost path is empty -> ocfs2_do_insert_extent -> ocfs2_rotate_tree_right -> ocfs2_extend_rotate_transaction -> jbd2_journal_restart if jbd2_journal_extend fail -> ocfs2_insert_path -> ocfs2_extend_trans -> jbd2_journal_restart if jbd2_journal_extend fail -> ocfs2_insert_at_leaf -> ocfs2_et_update_clusters Function jbd2_journal_restart() may be called and it may happened that buffers dirtied in ocfs2_add_branch() are committed while buffers dirtied in ocfs2_insert_at_leaf() and ocfs2_et_update_clusters() are not. So an empty rec[0] is left in rightmost path which will cause read-only filesystem when call ocfs2_commit_truncate() with the error message: "Inode %lu has an empty extent record". This is not a serious problem, so remove the rightmost path when call ocfs2_commit_truncate(). Signed-off-by: joyce.xue Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/alloc.c | 44 ++++++++++++++++++++++++++++++++++++++------ 1 file changed, 38 insertions(+), 6 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c index 9a0fd494fe74..77cbd1e3c950 100644 --- a/fs/ocfs2/alloc.c +++ b/fs/ocfs2/alloc.c @@ -3131,6 +3131,30 @@ out: return ret; } +static int ocfs2_remove_rightmost_empty_extent(struct ocfs2_super *osb, + struct ocfs2_extent_tree *et, + struct ocfs2_path *path, + struct ocfs2_cached_dealloc_ctxt *dealloc) +{ + handle_t *handle; + int ret; + int credits = path->p_tree_depth * 2 + 1; + + handle = ocfs2_start_trans(osb, credits); + if (IS_ERR(handle)) { + ret = PTR_ERR(handle); + mlog_errno(ret); + return ret; + } + + ret = ocfs2_remove_rightmost_path(handle, et, path, dealloc); + if (ret) + mlog_errno(ret); + + ocfs2_commit_trans(osb, handle); + return ret; +} + /* * Left rotation of btree records. * @@ -7108,15 +7132,23 @@ start: * to check it up here before changing the tree. */ if (root_el->l_tree_depth && rec->e_int_clusters == 0) { - ocfs2_error(inode->i_sb, "Inode %lu has an empty " + mlog(ML_ERROR, "Inode %lu has an empty " "extent record, depth %u\n", inode->i_ino, le16_to_cpu(root_el->l_tree_depth)); - status = -EROFS; - goto bail; + status = ocfs2_remove_rightmost_empty_extent(osb, + &et, path, &dealloc); + if (status) { + mlog_errno(status); + goto bail; + } + + ocfs2_reinit_path(path, 1); + goto start; + } else { + trunc_cpos = le32_to_cpu(rec->e_cpos); + trunc_len = 0; + blkno = 0; } - trunc_cpos = le32_to_cpu(rec->e_cpos); - trunc_len = 0; - blkno = 0; } else if (le32_to_cpu(rec->e_cpos) >= new_highest_cpos) { /* * Truncate entire record. -- cgit v1.2.1 From 7ecef14ab1db961545354fa443749aeda2ea1b75 Mon Sep 17 00:00:00 2001 From: Joe Perches Date: Fri, 4 Sep 2015 15:44:51 -0700 Subject: ocfs2: neaten do_error, ocfs2_error and ocfs2_abort These uses sometimes do and sometimes don't have '\n' terminations. Make the uses consistently use '\n' terminations and remove the newline from the functions. Miscellanea: o Coalesce formats o Realign arguments Signed-off-by: Joe Perches Reviewed-by: Mark Fasheh Cc: Joel Becker Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/alloc.c | 86 ++++++++++++++++++------------------------------- fs/ocfs2/aops.c | 4 +-- fs/ocfs2/dir.c | 49 +++++++++++++--------------- fs/ocfs2/extent_map.c | 22 +++++++------ fs/ocfs2/inode.c | 18 +++++------ fs/ocfs2/journal.c | 2 +- fs/ocfs2/localalloc.c | 3 +- fs/ocfs2/move_extents.c | 5 ++- fs/ocfs2/quota_local.c | 3 +- fs/ocfs2/refcounttree.c | 53 +++++++++++++----------------- fs/ocfs2/suballoc.c | 75 ++++++++++++++++++++---------------------- fs/ocfs2/super.c | 4 +-- fs/ocfs2/super.h | 6 ++-- fs/ocfs2/xattr.c | 35 +++++++++----------- 14 files changed, 163 insertions(+), 202 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c index 77cbd1e3c950..b20706e8a4d1 100644 --- a/fs/ocfs2/alloc.c +++ b/fs/ocfs2/alloc.c @@ -909,27 +909,25 @@ static int ocfs2_validate_extent_block(struct super_block *sb, if (!OCFS2_IS_VALID_EXTENT_BLOCK(eb)) { rc = ocfs2_error(sb, - "Extent block #%llu has bad signature %.*s", - (unsigned long long)bh->b_blocknr, 7, - eb->h_signature); + "Extent block #%llu has bad signature %.*s\n", + (unsigned long long)bh->b_blocknr, 7, + eb->h_signature); goto bail; } if (le64_to_cpu(eb->h_blkno) != bh->b_blocknr) { rc = ocfs2_error(sb, - "Extent block #%llu has an invalid h_blkno " - "of %llu", - (unsigned long long)bh->b_blocknr, - (unsigned long long)le64_to_cpu(eb->h_blkno)); + "Extent block #%llu has an invalid h_blkno of %llu\n", + (unsigned long long)bh->b_blocknr, + (unsigned long long)le64_to_cpu(eb->h_blkno)); goto bail; } if (le32_to_cpu(eb->h_fs_generation) != OCFS2_SB(sb)->fs_generation) { rc = ocfs2_error(sb, - "Extent block #%llu has an invalid " - "h_fs_generation of #%u", - (unsigned long long)bh->b_blocknr, - le32_to_cpu(eb->h_fs_generation)); + "Extent block #%llu has an invalid h_fs_generation of #%u\n", + (unsigned long long)bh->b_blocknr, + le32_to_cpu(eb->h_fs_generation)); goto bail; } bail: @@ -1446,8 +1444,7 @@ static int ocfs2_find_branch_target(struct ocfs2_extent_tree *et, while(le16_to_cpu(el->l_tree_depth) > 1) { if (le16_to_cpu(el->l_next_free_rec) == 0) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu has empty " - "extent list (next_free_rec == 0)", + "Owner %llu has empty extent list (next_free_rec == 0)\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci)); status = -EIO; goto bail; @@ -1456,9 +1453,7 @@ static int ocfs2_find_branch_target(struct ocfs2_extent_tree *et, blkno = le64_to_cpu(el->l_recs[i].e_blkno); if (!blkno) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu has extent " - "list where extent # %d has no physical " - "block start", + "Owner %llu has extent list where extent # %d has no physical block start\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), i); status = -EIO; goto bail; @@ -1788,8 +1783,7 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci, while (el->l_tree_depth) { if (le16_to_cpu(el->l_next_free_rec) == 0) { ocfs2_error(ocfs2_metadata_cache_get_super(ci), - "Owner %llu has empty extent list at " - "depth %u\n", + "Owner %llu has empty extent list at depth %u\n", (unsigned long long)ocfs2_metadata_cache_owner(ci), le16_to_cpu(el->l_tree_depth)); ret = -EROFS; @@ -1814,8 +1808,7 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci, blkno = le64_to_cpu(el->l_recs[i].e_blkno); if (blkno == 0) { ocfs2_error(ocfs2_metadata_cache_get_super(ci), - "Owner %llu has bad blkno in extent list " - "at depth %u (index %d)\n", + "Owner %llu has bad blkno in extent list at depth %u (index %d)\n", (unsigned long long)ocfs2_metadata_cache_owner(ci), le16_to_cpu(el->l_tree_depth), i); ret = -EROFS; @@ -1836,8 +1829,7 @@ static int __ocfs2_find_path(struct ocfs2_caching_info *ci, if (le16_to_cpu(el->l_next_free_rec) > le16_to_cpu(el->l_count)) { ocfs2_error(ocfs2_metadata_cache_get_super(ci), - "Owner %llu has bad count in extent list " - "at block %llu (next free=%u, count=%u)\n", + "Owner %llu has bad count in extent list at block %llu (next free=%u, count=%u)\n", (unsigned long long)ocfs2_metadata_cache_owner(ci), (unsigned long long)bh->b_blocknr, le16_to_cpu(el->l_next_free_rec), @@ -2116,8 +2108,7 @@ static int ocfs2_rotate_subtree_right(handle_t *handle, if (left_el->l_next_free_rec != left_el->l_count) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Inode %llu has non-full interior leaf node %llu" - "(next free = %u)", + "Inode %llu has non-full interior leaf node %llu (next free = %u)\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), (unsigned long long)left_leaf_bh->b_blocknr, le16_to_cpu(left_el->l_next_free_rec)); @@ -2256,8 +2247,7 @@ int ocfs2_find_cpos_for_left_leaf(struct super_block *sb, * If we got here, we never found a valid node where * the tree indicated one should be. */ - ocfs2_error(sb, - "Invalid extent tree at extent block %llu\n", + ocfs2_error(sb, "Invalid extent tree at extent block %llu\n", (unsigned long long)blkno); ret = -EROFS; goto out; @@ -2872,8 +2862,7 @@ int ocfs2_find_cpos_for_right_leaf(struct super_block *sb, * If we got here, we never found a valid node where * the tree indicated one should be. */ - ocfs2_error(sb, - "Invalid extent tree at extent block %llu\n", + ocfs2_error(sb, "Invalid extent tree at extent block %llu\n", (unsigned long long)blkno); ret = -EROFS; goto out; @@ -3224,7 +3213,7 @@ rightmost_no_delete: if (le16_to_cpu(el->l_next_free_rec) == 0) { ret = -EIO; ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu has empty extent block at %llu", + "Owner %llu has empty extent block at %llu\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), (unsigned long long)le64_to_cpu(eb->h_blkno)); goto out; @@ -3954,7 +3943,7 @@ static void ocfs2_adjust_rightmost_records(handle_t *handle, next_free = le16_to_cpu(el->l_next_free_rec); if (next_free == 0) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu has a bad extent list", + "Owner %llu has a bad extent list\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci)); ret = -EIO; return; @@ -4379,10 +4368,7 @@ static int ocfs2_figure_merge_contig_type(struct ocfs2_extent_tree *et, bh = path_leaf_bh(left_path); eb = (struct ocfs2_extent_block *)bh->b_data; ocfs2_error(sb, - "Extent block #%llu has an " - "invalid l_next_free_rec of " - "%d. It should have " - "matched the l_count of %d", + "Extent block #%llu has an invalid l_next_free_rec of %d. It should have matched the l_count of %d\n", (unsigned long long)le64_to_cpu(eb->h_blkno), le16_to_cpu(new_el->l_next_free_rec), le16_to_cpu(new_el->l_count)); @@ -4437,8 +4423,7 @@ static int ocfs2_figure_merge_contig_type(struct ocfs2_extent_tree *et, bh = path_leaf_bh(right_path); eb = (struct ocfs2_extent_block *)bh->b_data; ocfs2_error(sb, - "Extent block #%llu has an " - "invalid l_next_free_rec of %d", + "Extent block #%llu has an invalid l_next_free_rec of %d\n", (unsigned long long)le64_to_cpu(eb->h_blkno), le16_to_cpu(new_el->l_next_free_rec)); status = -EINVAL; @@ -4994,10 +4979,9 @@ leftright: split_index = ocfs2_search_extent_list(el, cpos); if (split_index == -1) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu has an extent at cpos %u " - "which can no longer be found.\n", - (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), - cpos); + "Owner %llu has an extent at cpos %u which can no longer be found\n", + (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), + cpos); ret = -EROFS; goto out; } @@ -5182,10 +5166,9 @@ int ocfs2_change_extent_flag(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { ocfs2_error(sb, - "Owner %llu has an extent at cpos %u which can no " - "longer be found.\n", - (unsigned long long) - ocfs2_metadata_cache_owner(et->et_ci), cpos); + "Owner %llu has an extent at cpos %u which can no longer be found\n", + (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), + cpos); ret = -EROFS; goto out; } @@ -5252,9 +5235,7 @@ int ocfs2_mark_extent_written(struct inode *inode, cpos, len, phys); if (!ocfs2_writes_unwritten_extents(OCFS2_SB(inode->i_sb))) { - ocfs2_error(inode->i_sb, "Inode %llu has unwritten extents " - "that are being written to, but the feature bit " - "is not set in the super block.", + ocfs2_error(inode->i_sb, "Inode %llu has unwritten extents that are being written to, but the feature bit is not set in the super block\n", (unsigned long long)OCFS2_I(inode)->ip_blkno); ret = -EROFS; goto out; @@ -5538,8 +5519,7 @@ int ocfs2_remove_extent(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu has an extent at cpos %u which can no " - "longer be found.\n", + "Owner %llu has an extent at cpos %u which can no longer be found\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), cpos); ret = -EROFS; @@ -5604,7 +5584,7 @@ int ocfs2_remove_extent(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu: split at cpos %u lost record.", + "Owner %llu: split at cpos %u lost record\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), cpos); ret = -EROFS; @@ -5620,8 +5600,7 @@ int ocfs2_remove_extent(handle_t *handle, ocfs2_rec_clusters(el, rec); if (rec_range != trunc_range) { ocfs2_error(ocfs2_metadata_cache_get_super(et->et_ci), - "Owner %llu: error after split at cpos %u" - "trunc len %u, existing record is (%u,%u)", + "Owner %llu: error after split at cpos %u trunc len %u, existing record is (%u,%u)\n", (unsigned long long)ocfs2_metadata_cache_owner(et->et_ci), cpos, len, le32_to_cpu(rec->e_cpos), ocfs2_rec_clusters(el, rec)); @@ -7236,8 +7215,7 @@ int ocfs2_truncate_inline(struct inode *inode, struct buffer_head *di_bh, !(le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL) || !ocfs2_supports_inline_data(osb)) { ocfs2_error(inode->i_sb, - "Inline data flags for inode %llu don't agree! " - "Disk: 0x%x, Memory: 0x%x, Superblock: 0x%x\n", + "Inline data flags for inode %llu don't agree! Disk: 0x%x, Memory: 0x%x, Superblock: 0x%x\n", (unsigned long long)OCFS2_I(inode)->ip_blkno, le16_to_cpu(di->i_dyn_features), OCFS2_I(inode)->ip_dyn_features, diff --git a/fs/ocfs2/aops.c b/fs/ocfs2/aops.c index faf36a96cd19..64b11d90eca6 100644 --- a/fs/ocfs2/aops.c +++ b/fs/ocfs2/aops.c @@ -227,7 +227,7 @@ int ocfs2_read_inline_data(struct inode *inode, struct page *page, struct ocfs2_dinode *di = (struct ocfs2_dinode *)di_bh->b_data; if (!(le16_to_cpu(di->i_dyn_features) & OCFS2_INLINE_DATA_FL)) { - ocfs2_error(inode->i_sb, "Inode %llu lost inline data flag", + ocfs2_error(inode->i_sb, "Inode %llu lost inline data flag\n", (unsigned long long)OCFS2_I(inode)->ip_blkno); return -EROFS; } @@ -237,7 +237,7 @@ int ocfs2_read_inline_data(struct inode *inode, struct page *page, if (size > PAGE_CACHE_SIZE || size > ocfs2_max_inline_data_with_xattr(inode->i_sb, di)) { ocfs2_error(inode->i_sb, - "Inode %llu has with inline data has bad size: %Lu", + "Inode %llu has with inline data has bad size: %Lu\n", (unsigned long long)OCFS2_I(inode)->ip_blkno, (unsigned long long)size); return -EROFS; diff --git a/fs/ocfs2/dir.c b/fs/ocfs2/dir.c index 25f03af09237..ffecf89c8c1c 100644 --- a/fs/ocfs2/dir.c +++ b/fs/ocfs2/dir.c @@ -481,29 +481,25 @@ static int ocfs2_check_dir_trailer(struct inode *dir, struct buffer_head *bh) trailer = ocfs2_trailer_from_bh(bh, dir->i_sb); if (!OCFS2_IS_VALID_DIR_TRAILER(trailer)) { rc = ocfs2_error(dir->i_sb, - "Invalid dirblock #%llu: " - "signature = %.*s\n", - (unsigned long long)bh->b_blocknr, 7, - trailer->db_signature); + "Invalid dirblock #%llu: signature = %.*s\n", + (unsigned long long)bh->b_blocknr, 7, + trailer->db_signature); goto out; } if (le64_to_cpu(trailer->db_blkno) != bh->b_blocknr) { rc = ocfs2_error(dir->i_sb, - "Directory block #%llu has an invalid " - "db_blkno of %llu", - (unsigned long long)bh->b_blocknr, - (unsigned long long)le64_to_cpu(trailer->db_blkno)); + "Directory block #%llu has an invalid db_blkno of %llu\n", + (unsigned long long)bh->b_blocknr, + (unsigned long long)le64_to_cpu(trailer->db_blkno)); goto out; } if (le64_to_cpu(trailer->db_parent_dinode) != OCFS2_I(dir)->ip_blkno) { rc = ocfs2_error(dir->i_sb, - "Directory block #%llu on dinode " - "#%llu has an invalid parent_dinode " - "of %llu", - (unsigned long long)bh->b_blocknr, - (unsigned long long)OCFS2_I(dir)->ip_blkno, - (unsigned long long)le64_to_cpu(trailer->db_blkno)); + "Directory block #%llu on dinode #%llu has an invalid parent_dinode of %llu\n", + (unsigned long long)bh->b_blocknr, + (unsigned long long)OCFS2_I(dir)->ip_blkno, + (unsigned long long)le64_to_cpu(trailer->db_blkno)); goto out; } out: @@ -602,9 +598,9 @@ static int ocfs2_validate_dx_root(struct super_block *sb, if (!OCFS2_IS_VALID_DX_ROOT(dx_root)) { ret = ocfs2_error(sb, - "Dir Index Root # %llu has bad signature %.*s", - (unsigned long long)le64_to_cpu(dx_root->dr_blkno), - 7, dx_root->dr_signature); + "Dir Index Root # %llu has bad signature %.*s\n", + (unsigned long long)le64_to_cpu(dx_root->dr_blkno), + 7, dx_root->dr_signature); } return ret; @@ -644,8 +640,8 @@ static int ocfs2_validate_dx_leaf(struct super_block *sb, } if (!OCFS2_IS_VALID_DX_LEAF(dx_leaf)) { - ret = ocfs2_error(sb, "Dir Index Leaf has bad signature %.*s", - 7, dx_leaf->dl_signature); + ret = ocfs2_error(sb, "Dir Index Leaf has bad signature %.*s\n", + 7, dx_leaf->dl_signature); } return ret; @@ -808,9 +804,9 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode, if (el->l_tree_depth) { ret = ocfs2_error(inode->i_sb, - "Inode %lu has non zero tree depth in " - "btree tree block %llu\n", inode->i_ino, - (unsigned long long)eb_bh->b_blocknr); + "Inode %lu has non zero tree depth in btree tree block %llu\n", + inode->i_ino, + (unsigned long long)eb_bh->b_blocknr); goto out; } } @@ -826,10 +822,11 @@ static int ocfs2_dx_dir_lookup_rec(struct inode *inode, } if (!found) { - ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent " - "record (%u, %u, 0) in btree", inode->i_ino, - le32_to_cpu(rec->e_cpos), - ocfs2_rec_clusters(el, rec)); + ret = ocfs2_error(inode->i_sb, + "Inode %lu has bad extent record (%u, %u, 0) in btree\n", + inode->i_ino, + le32_to_cpu(rec->e_cpos), + ocfs2_rec_clusters(el, rec)); goto out; } diff --git a/fs/ocfs2/extent_map.c b/fs/ocfs2/extent_map.c index 767370b656ca..e4719e0a3f99 100644 --- a/fs/ocfs2/extent_map.c +++ b/fs/ocfs2/extent_map.c @@ -305,8 +305,8 @@ static int ocfs2_last_eb_is_empty(struct inode *inode, if (el->l_tree_depth) { ocfs2_error(inode->i_sb, - "Inode %lu has non zero tree depth in " - "leaf block %llu\n", inode->i_ino, + "Inode %lu has non zero tree depth in leaf block %llu\n", + inode->i_ino, (unsigned long long)eb_bh->b_blocknr); ret = -EROFS; goto out; @@ -441,8 +441,8 @@ static int ocfs2_get_clusters_nocache(struct inode *inode, if (el->l_tree_depth) { ocfs2_error(inode->i_sb, - "Inode %lu has non zero tree depth in " - "leaf block %llu\n", inode->i_ino, + "Inode %lu has non zero tree depth in leaf block %llu\n", + inode->i_ino, (unsigned long long)eb_bh->b_blocknr); ret = -EROFS; goto out; @@ -475,8 +475,9 @@ static int ocfs2_get_clusters_nocache(struct inode *inode, BUG_ON(v_cluster < le32_to_cpu(rec->e_cpos)); if (!rec->e_blkno) { - ocfs2_error(inode->i_sb, "Inode %lu has bad extent " - "record (%u, %u, 0)", inode->i_ino, + ocfs2_error(inode->i_sb, + "Inode %lu has bad extent record (%u, %u, 0)\n", + inode->i_ino, le32_to_cpu(rec->e_cpos), ocfs2_rec_clusters(el, rec)); ret = -EROFS; @@ -564,8 +565,8 @@ int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster, if (el->l_tree_depth) { ocfs2_error(inode->i_sb, - "Inode %lu has non zero tree depth in " - "xattr leaf block %llu\n", inode->i_ino, + "Inode %lu has non zero tree depth in xattr leaf block %llu\n", + inode->i_ino, (unsigned long long)eb_bh->b_blocknr); ret = -EROFS; goto out; @@ -582,8 +583,9 @@ int ocfs2_xattr_get_clusters(struct inode *inode, u32 v_cluster, BUG_ON(v_cluster < le32_to_cpu(rec->e_cpos)); if (!rec->e_blkno) { - ocfs2_error(inode->i_sb, "Inode %lu has bad extent " - "record (%u, %u, 0) in xattr", inode->i_ino, + ocfs2_error(inode->i_sb, + "Inode %lu has bad extent record (%u, %u, 0) in xattr\n", + inode->i_ino, le32_to_cpu(rec->e_cpos), ocfs2_rec_clusters(el, rec)); ret = -EROFS; diff --git a/fs/ocfs2/inode.c b/fs/ocfs2/inode.c index fe4b3f7db245..8f87e05ee25d 100644 --- a/fs/ocfs2/inode.c +++ b/fs/ocfs2/inode.c @@ -1362,31 +1362,31 @@ int ocfs2_validate_inode_block(struct super_block *sb, if (!OCFS2_IS_VALID_DINODE(di)) { rc = ocfs2_error(sb, "Invalid dinode #%llu: signature = %.*s\n", - (unsigned long long)bh->b_blocknr, 7, - di->i_signature); + (unsigned long long)bh->b_blocknr, 7, + di->i_signature); goto bail; } if (le64_to_cpu(di->i_blkno) != bh->b_blocknr) { rc = ocfs2_error(sb, "Invalid dinode #%llu: i_blkno is %llu\n", - (unsigned long long)bh->b_blocknr, - (unsigned long long)le64_to_cpu(di->i_blkno)); + (unsigned long long)bh->b_blocknr, + (unsigned long long)le64_to_cpu(di->i_blkno)); goto bail; } if (!(di->i_flags & cpu_to_le32(OCFS2_VALID_FL))) { rc = ocfs2_error(sb, - "Invalid dinode #%llu: OCFS2_VALID_FL not set\n", - (unsigned long long)bh->b_blocknr); + "Invalid dinode #%llu: OCFS2_VALID_FL not set\n", + (unsigned long long)bh->b_blocknr); goto bail; } if (le32_to_cpu(di->i_fs_generation) != OCFS2_SB(sb)->fs_generation) { rc = ocfs2_error(sb, - "Invalid dinode #%llu: fs_generation is %u\n", - (unsigned long long)bh->b_blocknr, - le32_to_cpu(di->i_fs_generation)); + "Invalid dinode #%llu: fs_generation is %u\n", + (unsigned long long)bh->b_blocknr, + le32_to_cpu(di->i_fs_generation)); goto bail; } diff --git a/fs/ocfs2/journal.c b/fs/ocfs2/journal.c index 52948af646b6..ff82b28462a6 100644 --- a/fs/ocfs2/journal.c +++ b/fs/ocfs2/journal.c @@ -374,7 +374,7 @@ handle_t *ocfs2_start_trans(struct ocfs2_super *osb, int max_buffs) mlog_errno(PTR_ERR(handle)); if (is_journal_aborted(journal)) { - ocfs2_abort(osb->sb, "Detected aborted journal"); + ocfs2_abort(osb->sb, "Detected aborted journal\n"); handle = ERR_PTR(-EROFS); } } else { diff --git a/fs/ocfs2/localalloc.c b/fs/ocfs2/localalloc.c index 857bbbcd39f3..0a4457fb0711 100644 --- a/fs/ocfs2/localalloc.c +++ b/fs/ocfs2/localalloc.c @@ -665,8 +665,7 @@ int ocfs2_reserve_local_alloc_bits(struct ocfs2_super *osb, #ifdef CONFIG_OCFS2_DEBUG_FS if (le32_to_cpu(alloc->id1.bitmap1.i_used) != ocfs2_local_alloc_count_bits(alloc)) { - ocfs2_error(osb->sb, "local alloc inode %llu says it has " - "%u used bits, but a count shows %u", + ocfs2_error(osb->sb, "local alloc inode %llu says it has %u used bits, but a count shows %u\n", (unsigned long long)le64_to_cpu(alloc->i_blkno), le32_to_cpu(alloc->id1.bitmap1.i_used), ocfs2_local_alloc_count_bits(alloc)); diff --git a/fs/ocfs2/move_extents.c b/fs/ocfs2/move_extents.c index 70dd0ec7b7e9..124471d26a73 100644 --- a/fs/ocfs2/move_extents.c +++ b/fs/ocfs2/move_extents.c @@ -100,9 +100,8 @@ static int __ocfs2_move_extent(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { ret = ocfs2_error(inode->i_sb, - "Inode %llu has an extent at cpos %u which can no " - "longer be found.\n", - (unsigned long long)ino, cpos); + "Inode %llu has an extent at cpos %u which can no longer be found\n", + (unsigned long long)ino, cpos); goto out; } diff --git a/fs/ocfs2/quota_local.c b/fs/ocfs2/quota_local.c index bb07004df72a..8a54fd8a4fa5 100644 --- a/fs/ocfs2/quota_local.c +++ b/fs/ocfs2/quota_local.c @@ -138,8 +138,7 @@ static int ocfs2_read_quota_block(struct inode *inode, u64 v_block, if (i_size_read(inode) >> inode->i_sb->s_blocksize_bits <= v_block) { ocfs2_error(inode->i_sb, - "Quota file %llu is probably corrupted! Requested " - "to read block %Lu but file has size only %Lu\n", + "Quota file %llu is probably corrupted! Requested to read block %Lu but file has size only %Lu\n", (unsigned long long)OCFS2_I(inode)->ip_blkno, (unsigned long long)v_block, (unsigned long long)i_size_read(inode)); diff --git a/fs/ocfs2/refcounttree.c b/fs/ocfs2/refcounttree.c index b404dbde3fe4..e5d57cd32505 100644 --- a/fs/ocfs2/refcounttree.c +++ b/fs/ocfs2/refcounttree.c @@ -103,27 +103,25 @@ static int ocfs2_validate_refcount_block(struct super_block *sb, if (!OCFS2_IS_VALID_REFCOUNT_BLOCK(rb)) { rc = ocfs2_error(sb, - "Refcount block #%llu has bad signature %.*s", - (unsigned long long)bh->b_blocknr, 7, - rb->rf_signature); + "Refcount block #%llu has bad signature %.*s\n", + (unsigned long long)bh->b_blocknr, 7, + rb->rf_signature); goto out; } if (le64_to_cpu(rb->rf_blkno) != bh->b_blocknr) { rc = ocfs2_error(sb, - "Refcount block #%llu has an invalid rf_blkno " - "of %llu", - (unsigned long long)bh->b_blocknr, - (unsigned long long)le64_to_cpu(rb->rf_blkno)); + "Refcount block #%llu has an invalid rf_blkno of %llu\n", + (unsigned long long)bh->b_blocknr, + (unsigned long long)le64_to_cpu(rb->rf_blkno)); goto out; } if (le32_to_cpu(rb->rf_fs_generation) != OCFS2_SB(sb)->fs_generation) { rc = ocfs2_error(sb, - "Refcount block #%llu has an invalid " - "rf_fs_generation of #%u", - (unsigned long long)bh->b_blocknr, - le32_to_cpu(rb->rf_fs_generation)); + "Refcount block #%llu has an invalid rf_fs_generation of #%u\n", + (unsigned long long)bh->b_blocknr, + le32_to_cpu(rb->rf_fs_generation)); goto out; } out: @@ -1103,10 +1101,9 @@ static int ocfs2_get_refcount_rec(struct ocfs2_caching_info *ci, if (el->l_tree_depth) { ret = ocfs2_error(sb, - "refcount tree %llu has non zero tree " - "depth in leaf btree tree block %llu\n", - (unsigned long long)ocfs2_metadata_cache_owner(ci), - (unsigned long long)eb_bh->b_blocknr); + "refcount tree %llu has non zero tree depth in leaf btree tree block %llu\n", + (unsigned long long)ocfs2_metadata_cache_owner(ci), + (unsigned long long)eb_bh->b_blocknr); goto out; } } @@ -2358,9 +2355,8 @@ static int ocfs2_mark_extent_refcounted(struct inode *inode, cpos, len, phys); if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) { - ret = ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " - "tree, but the feature bit is not set in the " - "super block.", inode->i_ino); + ret = ocfs2_error(inode->i_sb, "Inode %lu want to use refcount tree, but the feature bit is not set in the super block\n", + inode->i_ino); goto out; } @@ -2543,9 +2539,8 @@ int ocfs2_prepare_refcount_change_for_del(struct inode *inode, u64 start_cpos = ocfs2_blocks_to_clusters(inode->i_sb, phys_blkno); if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) { - ret = ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " - "tree, but the feature bit is not set in the " - "super block.", inode->i_ino); + ret = ocfs2_error(inode->i_sb, "Inode %lu want to use refcount tree, but the feature bit is not set in the super block\n", + inode->i_ino); goto out; } @@ -2670,9 +2665,9 @@ static int ocfs2_refcount_cal_cow_clusters(struct inode *inode, if (el->l_tree_depth) { ret = ocfs2_error(inode->i_sb, - "Inode %lu has non zero tree depth in " - "leaf block %llu\n", inode->i_ino, - (unsigned long long)eb_bh->b_blocknr); + "Inode %lu has non zero tree depth in leaf block %llu\n", + inode->i_ino, + (unsigned long long)eb_bh->b_blocknr); goto out; } } @@ -3103,9 +3098,8 @@ static int ocfs2_clear_ext_refcount(handle_t *handle, index = ocfs2_search_extent_list(el, cpos); if (index == -1) { ret = ocfs2_error(sb, - "Inode %llu has an extent at cpos %u which can no " - "longer be found.\n", - (unsigned long long)ino, cpos); + "Inode %llu has an extent at cpos %u which can no longer be found\n", + (unsigned long long)ino, cpos); goto out; } @@ -3371,9 +3365,8 @@ static int ocfs2_replace_cow(struct ocfs2_cow_context *context) struct ocfs2_super *osb = OCFS2_SB(inode->i_sb); if (!ocfs2_refcount_tree(OCFS2_SB(inode->i_sb))) { - return ocfs2_error(inode->i_sb, "Inode %lu want to use refcount " - "tree, but the feature bit is not set in the " - "super block.", inode->i_ino); + return ocfs2_error(inode->i_sb, "Inode %lu want to use refcount tree, but the feature bit is not set in the super block\n", + inode->i_ino); } ocfs2_init_dealloc_ctxt(&context->dealloc); diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c index e4bb00110e91..0456ae399bf7 100644 --- a/fs/ocfs2/suballoc.c +++ b/fs/ocfs2/suballoc.c @@ -167,12 +167,12 @@ static u32 ocfs2_bits_per_group(struct ocfs2_chain_list *cl) } #define do_error(fmt, ...) \ - do{ \ - if (resize) \ - mlog(ML_ERROR, fmt "\n", ##__VA_ARGS__); \ - else \ - return ocfs2_error(sb, fmt, ##__VA_ARGS__); \ - } while (0) +do { \ + if (resize) \ + mlog(ML_ERROR, fmt, ##__VA_ARGS__); \ + else \ + return ocfs2_error(sb, fmt, ##__VA_ARGS__); \ +} while (0) static int ocfs2_validate_gd_self(struct super_block *sb, struct buffer_head *bh, @@ -181,36 +181,32 @@ static int ocfs2_validate_gd_self(struct super_block *sb, struct ocfs2_group_desc *gd = (struct ocfs2_group_desc *)bh->b_data; if (!OCFS2_IS_VALID_GROUP_DESC(gd)) { - do_error("Group descriptor #%llu has bad signature %.*s", + do_error("Group descriptor #%llu has bad signature %.*s\n", (unsigned long long)bh->b_blocknr, 7, gd->bg_signature); } if (le64_to_cpu(gd->bg_blkno) != bh->b_blocknr) { - do_error("Group descriptor #%llu has an invalid bg_blkno " - "of %llu", + do_error("Group descriptor #%llu has an invalid bg_blkno of %llu\n", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(gd->bg_blkno)); } if (le32_to_cpu(gd->bg_generation) != OCFS2_SB(sb)->fs_generation) { - do_error("Group descriptor #%llu has an invalid " - "fs_generation of #%u", + do_error("Group descriptor #%llu has an invalid fs_generation of #%u\n", (unsigned long long)bh->b_blocknr, le32_to_cpu(gd->bg_generation)); } if (le16_to_cpu(gd->bg_free_bits_count) > le16_to_cpu(gd->bg_bits)) { - do_error("Group descriptor #%llu has bit count %u but " - "claims that %u are free", + do_error("Group descriptor #%llu has bit count %u but claims that %u are free\n", (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_bits), le16_to_cpu(gd->bg_free_bits_count)); } if (le16_to_cpu(gd->bg_bits) > (8 * le16_to_cpu(gd->bg_size))) { - do_error("Group descriptor #%llu has bit count %u but " - "max bitmap bits of %u", + do_error("Group descriptor #%llu has bit count %u but max bitmap bits of %u\n", (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_bits), 8 * le16_to_cpu(gd->bg_size)); @@ -228,8 +224,7 @@ static int ocfs2_validate_gd_parent(struct super_block *sb, struct ocfs2_group_desc *gd = (struct ocfs2_group_desc *)bh->b_data; if (di->i_blkno != gd->bg_parent_dinode) { - do_error("Group descriptor #%llu has bad parent " - "pointer (%llu, expected %llu)", + do_error("Group descriptor #%llu has bad parent pointer (%llu, expected %llu)\n", (unsigned long long)bh->b_blocknr, (unsigned long long)le64_to_cpu(gd->bg_parent_dinode), (unsigned long long)le64_to_cpu(di->i_blkno)); @@ -237,7 +232,7 @@ static int ocfs2_validate_gd_parent(struct super_block *sb, max_bits = le16_to_cpu(di->id2.i_chain.cl_cpg) * le16_to_cpu(di->id2.i_chain.cl_bpc); if (le16_to_cpu(gd->bg_bits) > max_bits) { - do_error("Group descriptor #%llu has bit count of %u", + do_error("Group descriptor #%llu has bit count of %u\n", (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_bits)); } @@ -247,7 +242,7 @@ static int ocfs2_validate_gd_parent(struct super_block *sb, le16_to_cpu(di->id2.i_chain.cl_next_free_rec)) || ((le16_to_cpu(gd->bg_chain) == le16_to_cpu(di->id2.i_chain.cl_next_free_rec)) && !resize)) { - do_error("Group descriptor #%llu has bad chain %u", + do_error("Group descriptor #%llu has bad chain %u\n", (unsigned long long)bh->b_blocknr, le16_to_cpu(gd->bg_chain)); } @@ -376,10 +371,10 @@ static int ocfs2_block_group_fill(handle_t *handle, struct super_block * sb = alloc_inode->i_sb; if (((unsigned long long) bg_bh->b_blocknr) != group_blkno) { - status = ocfs2_error(alloc_inode->i_sb, "group block (%llu) != " - "b_blocknr (%llu)", - (unsigned long long)group_blkno, - (unsigned long long) bg_bh->b_blocknr); + status = ocfs2_error(alloc_inode->i_sb, + "group block (%llu) != b_blocknr (%llu)\n", + (unsigned long long)group_blkno, + (unsigned long long) bg_bh->b_blocknr); goto bail; } @@ -825,8 +820,9 @@ static int ocfs2_reserve_suballoc_bits(struct ocfs2_super *osb, BUG_ON(!OCFS2_IS_VALID_DINODE(fe)); if (!(fe->i_flags & cpu_to_le32(OCFS2_CHAIN_FL))) { - status = ocfs2_error(alloc_inode->i_sb, "Invalid chain allocator %llu", - (unsigned long long)le64_to_cpu(fe->i_blkno)); + status = ocfs2_error(alloc_inode->i_sb, + "Invalid chain allocator %llu\n", + (unsigned long long)le64_to_cpu(fe->i_blkno)); goto bail; } @@ -1360,11 +1356,11 @@ int ocfs2_block_group_set_bits(handle_t *handle, le16_add_cpu(&bg->bg_free_bits_count, -num_bits); if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { - return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" - " count %u but claims %u are freed. num_bits %d", - (unsigned long long)le64_to_cpu(bg->bg_blkno), - le16_to_cpu(bg->bg_bits), - le16_to_cpu(bg->bg_free_bits_count), num_bits); + return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit count %u but claims %u are freed. num_bits %d\n", + (unsigned long long)le64_to_cpu(bg->bg_blkno), + le16_to_cpu(bg->bg_bits), + le16_to_cpu(bg->bg_free_bits_count), + num_bits); } while(num_bits--) ocfs2_set_bit(bit_off++, bitmap); @@ -1895,11 +1891,10 @@ static int ocfs2_claim_suballoc_bits(struct ocfs2_alloc_context *ac, if (le32_to_cpu(fe->id1.bitmap1.i_used) >= le32_to_cpu(fe->id1.bitmap1.i_total)) { status = ocfs2_error(ac->ac_inode->i_sb, - "Chain allocator dinode %llu has %u used " - "bits but only %u total.", - (unsigned long long)le64_to_cpu(fe->i_blkno), - le32_to_cpu(fe->id1.bitmap1.i_used), - le32_to_cpu(fe->id1.bitmap1.i_total)); + "Chain allocator dinode %llu has %u used bits but only %u total\n", + (unsigned long long)le64_to_cpu(fe->i_blkno), + le32_to_cpu(fe->id1.bitmap1.i_used), + le32_to_cpu(fe->id1.bitmap1.i_total)); goto bail; } @@ -2417,11 +2412,11 @@ static int ocfs2_block_group_clear_bits(handle_t *handle, } le16_add_cpu(&bg->bg_free_bits_count, num_bits); if (le16_to_cpu(bg->bg_free_bits_count) > le16_to_cpu(bg->bg_bits)) { - return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit" - " count %u but claims %u are freed. num_bits %d", - (unsigned long long)le64_to_cpu(bg->bg_blkno), - le16_to_cpu(bg->bg_bits), - le16_to_cpu(bg->bg_free_bits_count), num_bits); + return ocfs2_error(alloc_inode->i_sb, "Group descriptor # %llu has bit count %u but claims %u are freed. num_bits %d\n", + (unsigned long long)le64_to_cpu(bg->bg_blkno), + le16_to_cpu(bg->bg_bits), + le16_to_cpu(bg->bg_free_bits_count), + num_bits); } if (undo_fn) diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index e79058ecfb4b..3a9a1af39ad7 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -2600,7 +2600,7 @@ int __ocfs2_error(struct super_block *sb, const char *function, /* Not using mlog here because we want to show the actual * function the error came from. */ - printk(KERN_CRIT "OCFS2: ERROR (device %s): %s: %pV\n", + printk(KERN_CRIT "OCFS2: ERROR (device %s): %s: %pV", sb->s_id, function, &vaf); va_end(args); @@ -2622,7 +2622,7 @@ void __ocfs2_abort(struct super_block *sb, const char *function, vaf.fmt = fmt; vaf.va = &args; - printk(KERN_CRIT "OCFS2: abort (device %s): %s: %pV\n", + printk(KERN_CRIT "OCFS2: abort (device %s): %s: %pV", sb->s_id, function, &vaf); va_end(args); diff --git a/fs/ocfs2/super.h b/fs/ocfs2/super.h index c1c87d90542c..b477d0b1c7b6 100644 --- a/fs/ocfs2/super.h +++ b/fs/ocfs2/super.h @@ -35,13 +35,15 @@ __printf(3, 4) int __ocfs2_error(struct super_block *sb, const char *function, const char *fmt, ...); -#define ocfs2_error(sb, fmt, args...) __ocfs2_error(sb, __PRETTY_FUNCTION__, fmt, ##args) +#define ocfs2_error(sb, fmt, ...) \ + __ocfs2_error(sb, __PRETTY_FUNCTION__, fmt, ##__VA_ARGS__) __printf(3, 4) void __ocfs2_abort(struct super_block *sb, const char *function, const char *fmt, ...); -#define ocfs2_abort(sb, fmt, args...) __ocfs2_abort(sb, __PRETTY_FUNCTION__, fmt, ##args) +#define ocfs2_abort(sb, fmt, ...) \ + __ocfs2_abort(sb, __PRETTY_FUNCTION__, fmt, ##__VA_ARGS__) /* * Void signal blockers, because in-kernel sigprocmask() only fails diff --git a/fs/ocfs2/xattr.c b/fs/ocfs2/xattr.c index 5944a311bb94..ebfdea78659b 100644 --- a/fs/ocfs2/xattr.c +++ b/fs/ocfs2/xattr.c @@ -500,26 +500,23 @@ static int ocfs2_validate_xattr_block(struct super_block *sb, if (!OCFS2_IS_VALID_XATTR_BLOCK(xb)) { return ocfs2_error(sb, - "Extended attribute block #%llu has bad " - "signature %.*s", - (unsigned long long)bh->b_blocknr, 7, - xb->xb_signature); + "Extended attribute block #%llu has bad signature %.*s\n", + (unsigned long long)bh->b_blocknr, 7, + xb->xb_signature); } if (le64_to_cpu(xb->xb_blkno) != bh->b_blocknr) { return ocfs2_error(sb, - "Extended attribute block #%llu has an " - "invalid xb_blkno of %llu", - (unsigned long long)bh->b_blocknr, - (unsigned long long)le64_to_cpu(xb->xb_blkno)); + "Extended attribute block #%llu has an invalid xb_blkno of %llu\n", + (unsigned long long)bh->b_blocknr, + (unsigned long long)le64_to_cpu(xb->xb_blkno)); } if (le32_to_cpu(xb->xb_fs_generation) != OCFS2_SB(sb)->fs_generation) { return ocfs2_error(sb, - "Extended attribute block #%llu has an invalid " - "xb_fs_generation of #%u", - (unsigned long long)bh->b_blocknr, - le32_to_cpu(xb->xb_fs_generation)); + "Extended attribute block #%llu has an invalid xb_fs_generation of #%u\n", + (unsigned long long)bh->b_blocknr, + le32_to_cpu(xb->xb_fs_generation)); } return 0; @@ -3692,9 +3689,9 @@ static int ocfs2_xattr_get_rec(struct inode *inode, if (el->l_tree_depth) { ret = ocfs2_error(inode->i_sb, - "Inode %lu has non zero tree depth in " - "xattr tree block %llu\n", inode->i_ino, - (unsigned long long)eb_bh->b_blocknr); + "Inode %lu has non zero tree depth in xattr tree block %llu\n", + inode->i_ino, + (unsigned long long)eb_bh->b_blocknr); goto out; } } @@ -3709,10 +3706,10 @@ static int ocfs2_xattr_get_rec(struct inode *inode, } if (!e_blkno) { - ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent " - "record (%u, %u, 0) in xattr", inode->i_ino, - le32_to_cpu(rec->e_cpos), - ocfs2_rec_clusters(el, rec)); + ret = ocfs2_error(inode->i_sb, "Inode %lu has bad extent record (%u, %u, 0) in xattr\n", + inode->i_ino, + le32_to_cpu(rec->e_cpos), + ocfs2_rec_clusters(el, rec)); goto out; } -- cgit v1.2.1 From 46359295a352e01a5a017297c70b7ee0c5da6de6 Mon Sep 17 00:00:00 2001 From: Joseph Qi Date: Fri, 4 Sep 2015 15:44:54 -0700 Subject: ocfs2: clean up redundant NULL checks before kfree NULL check before kfree is redundant and so clean them up. Signed-off-by: Joseph Qi Reviewed-by: Mark Fasheh Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ocfs2/alloc.c | 2 +- fs/ocfs2/suballoc.c | 6 ++---- 2 files changed, 3 insertions(+), 5 deletions(-) (limited to 'fs') diff --git a/fs/ocfs2/alloc.c b/fs/ocfs2/alloc.c index b20706e8a4d1..86181d6526dc 100644 --- a/fs/ocfs2/alloc.c +++ b/fs/ocfs2/alloc.c @@ -6178,7 +6178,7 @@ bail: iput(tl_inode); brelse(tl_bh); - if (status < 0 && (*tl_copy)) { + if (status < 0) { kfree(*tl_copy); *tl_copy = NULL; mlog_errno(status); diff --git a/fs/ocfs2/suballoc.c b/fs/ocfs2/suballoc.c index 0456ae399bf7..d83d2602cf2b 100644 --- a/fs/ocfs2/suballoc.c +++ b/fs/ocfs2/suballoc.c @@ -149,10 +149,8 @@ void ocfs2_free_ac_resource(struct ocfs2_alloc_context *ac) brelse(ac->ac_bh); ac->ac_bh = NULL; ac->ac_resv = NULL; - if (ac->ac_find_loc_priv) { - kfree(ac->ac_find_loc_priv); - ac->ac_find_loc_priv = NULL; - } + kfree(ac->ac_find_loc_priv); + ac->ac_find_loc_priv = NULL; } void ocfs2_free_alloc_context(struct ocfs2_alloc_context *ac) -- cgit v1.2.1 From a068acf2ee77693e0bf39d6e07139ba704f461c3 Mon Sep 17 00:00:00 2001 From: Kees Cook Date: Fri, 4 Sep 2015 15:44:57 -0700 Subject: fs: create and use seq_show_option for escaping Many file systems that implement the show_options hook fail to correctly escape their output which could lead to unescaped characters (e.g. new lines) leaking into /proc/mounts and /proc/[pid]/mountinfo files. This could lead to confusion, spoofed entries (resulting in things like systemd issuing false d-bus "mount" notifications), and who knows what else. This looks like it would only be the root user stepping on themselves, but it's possible weird things could happen in containers or in other situations with delegated mount privileges. Here's an example using overlay with setuid fusermount trusting the contents of /proc/mounts (via the /etc/mtab symlink). Imagine the use of "sudo" is something more sneaky: $ BASE="ovl" $ MNT="$BASE/mnt" $ LOW="$BASE/lower" $ UP="$BASE/upper" $ WORK="$BASE/work/ 0 0 none /proc fuse.pwn user_id=1000" $ mkdir -p "$LOW" "$UP" "$WORK" $ sudo mount -t overlay -o "lowerdir=$LOW,upperdir=$UP,workdir=$WORK" none /mnt $ cat /proc/mounts none /root/ovl/mnt overlay rw,relatime,lowerdir=ovl/lower,upperdir=ovl/upper,workdir=ovl/work/ 0 0 none /proc fuse.pwn user_id=1000 0 0 $ fusermount -u /proc $ cat /proc/mounts cat: /proc/mounts: No such file or directory This fixes the problem by adding new seq_show_option and seq_show_option_n helpers, and updating the vulnerable show_option handlers to use them as needed. Some, like SELinux, need to be open coded due to unusual existing escape mechanisms. [akpm@linux-foundation.org: add lost chunk, per Kees] [keescook@chromium.org: seq_show_option should be using const parameters] Signed-off-by: Kees Cook Acked-by: Serge Hallyn Acked-by: Jan Kara Acked-by: Paul Moore Cc: J. R. Okajima Signed-off-by: Kees Cook Cc: Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/ceph/super.c | 2 +- fs/cifs/cifsfs.c | 6 +++--- fs/ext4/super.c | 4 ++-- fs/gfs2/super.c | 6 +++--- fs/hfs/super.c | 4 ++-- fs/hfsplus/options.c | 4 ++-- fs/hostfs/hostfs_kern.c | 2 +- fs/ocfs2/super.c | 4 ++-- fs/overlayfs/super.c | 6 +++--- fs/reiserfs/super.c | 8 +++++--- fs/xfs/xfs_super.c | 4 ++-- 11 files changed, 26 insertions(+), 24 deletions(-) (limited to 'fs') diff --git a/fs/ceph/super.c b/fs/ceph/super.c index d1c833c321b9..7b6bfcbf801c 100644 --- a/fs/ceph/super.c +++ b/fs/ceph/super.c @@ -479,7 +479,7 @@ static int ceph_show_options(struct seq_file *m, struct dentry *root) if (fsopt->max_readdir_bytes != CEPH_MAX_READDIR_BYTES_DEFAULT) seq_printf(m, ",readdir_max_bytes=%d", fsopt->max_readdir_bytes); if (strcmp(fsopt->snapdir_name, CEPH_SNAPDIRNAME_DEFAULT)) - seq_printf(m, ",snapdirname=%s", fsopt->snapdir_name); + seq_show_option(m, "snapdirname", fsopt->snapdir_name); return 0; } diff --git a/fs/cifs/cifsfs.c b/fs/cifs/cifsfs.c index 0a9fb6b53126..6a1119e87fbb 100644 --- a/fs/cifs/cifsfs.c +++ b/fs/cifs/cifsfs.c @@ -394,17 +394,17 @@ cifs_show_options(struct seq_file *s, struct dentry *root) struct sockaddr *srcaddr; srcaddr = (struct sockaddr *)&tcon->ses->server->srcaddr; - seq_printf(s, ",vers=%s", tcon->ses->server->vals->version_string); + seq_show_option(s, "vers", tcon->ses->server->vals->version_string); cifs_show_security(s, tcon->ses); cifs_show_cache_flavor(s, cifs_sb); if (cifs_sb->mnt_cifs_flags & CIFS_MOUNT_MULTIUSER) seq_puts(s, ",multiuser"); else if (tcon->ses->user_name) - seq_printf(s, ",username=%s", tcon->ses->user_name); + seq_show_option(s, "username", tcon->ses->user_name); if (tcon->ses->domainName) - seq_printf(s, ",domain=%s", tcon->ses->domainName); + seq_show_option(s, "domain", tcon->ses->domainName); if (srcaddr->sa_family != AF_UNSPEC) { struct sockaddr_in *saddr4; diff --git a/fs/ext4/super.c b/fs/ext4/super.c index ee3878262a49..a63c7b0a10cf 100644 --- a/fs/ext4/super.c +++ b/fs/ext4/super.c @@ -1776,10 +1776,10 @@ static inline void ext4_show_quota_options(struct seq_file *seq, } if (sbi->s_qf_names[USRQUOTA]) - seq_printf(seq, ",usrjquota=%s", sbi->s_qf_names[USRQUOTA]); + seq_show_option(seq, "usrjquota", sbi->s_qf_names[USRQUOTA]); if (sbi->s_qf_names[GRPQUOTA]) - seq_printf(seq, ",grpjquota=%s", sbi->s_qf_names[GRPQUOTA]); + seq_show_option(seq, "grpjquota", sbi->s_qf_names[GRPQUOTA]); #endif } diff --git a/fs/gfs2/super.c b/fs/gfs2/super.c index 2982445947e1..894fb01a91da 100644 --- a/fs/gfs2/super.c +++ b/fs/gfs2/super.c @@ -1334,11 +1334,11 @@ static int gfs2_show_options(struct seq_file *s, struct dentry *root) if (is_ancestor(root, sdp->sd_master_dir)) seq_puts(s, ",meta"); if (args->ar_lockproto[0]) - seq_printf(s, ",lockproto=%s", args->ar_lockproto); + seq_show_option(s, "lockproto", args->ar_lockproto); if (args->ar_locktable[0]) - seq_printf(s, ",locktable=%s", args->ar_locktable); + seq_show_option(s, "locktable", args->ar_locktable); if (args->ar_hostdata[0]) - seq_printf(s, ",hostdata=%s", args->ar_hostdata); + seq_show_option(s, "hostdata", args->ar_hostdata); if (args->ar_spectator) seq_puts(s, ",spectator"); if (args->ar_localflocks) diff --git a/fs/hfs/super.c b/fs/hfs/super.c index 55c03b9e9070..4574fdd3d421 100644 --- a/fs/hfs/super.c +++ b/fs/hfs/super.c @@ -136,9 +136,9 @@ static int hfs_show_options(struct seq_file *seq, struct dentry *root) struct hfs_sb_info *sbi = HFS_SB(root->d_sb); if (sbi->s_creator != cpu_to_be32(0x3f3f3f3f)) - seq_printf(seq, ",creator=%.4s", (char *)&sbi->s_creator); + seq_show_option_n(seq, "creator", (char *)&sbi->s_creator, 4); if (sbi->s_type != cpu_to_be32(0x3f3f3f3f)) - seq_printf(seq, ",type=%.4s", (char *)&sbi->s_type); + seq_show_option_n(seq, "type", (char *)&sbi->s_type, 4); seq_printf(seq, ",uid=%u,gid=%u", from_kuid_munged(&init_user_ns, sbi->s_uid), from_kgid_munged(&init_user_ns, sbi->s_gid)); diff --git a/fs/hfsplus/options.c b/fs/hfsplus/options.c index c90b72ee676d..bb806e58c977 100644 --- a/fs/hfsplus/options.c +++ b/fs/hfsplus/options.c @@ -218,9 +218,9 @@ int hfsplus_show_options(struct seq_file *seq, struct dentry *root) struct hfsplus_sb_info *sbi = HFSPLUS_SB(root->d_sb); if (sbi->creator != HFSPLUS_DEF_CR_TYPE) - seq_printf(seq, ",creator=%.4s", (char *)&sbi->creator); + seq_show_option_n(seq, "creator", (char *)&sbi->creator, 4); if (sbi->type != HFSPLUS_DEF_CR_TYPE) - seq_printf(seq, ",type=%.4s", (char *)&sbi->type); + seq_show_option_n(seq, "type", (char *)&sbi->type, 4); seq_printf(seq, ",umask=%o,uid=%u,gid=%u", sbi->umask, from_kuid_munged(&init_user_ns, sbi->uid), from_kgid_munged(&init_user_ns, sbi->gid)); diff --git a/fs/hostfs/hostfs_kern.c b/fs/hostfs/hostfs_kern.c index 059597b23f67..2ac99db3750e 100644 --- a/fs/hostfs/hostfs_kern.c +++ b/fs/hostfs/hostfs_kern.c @@ -260,7 +260,7 @@ static int hostfs_show_options(struct seq_file *seq, struct dentry *root) size_t offset = strlen(root_ino) + 1; if (strlen(root_path) > offset) - seq_printf(seq, ",%s", root_path + offset); + seq_show_option(seq, root_path + offset, NULL); if (append) seq_puts(seq, ",append"); diff --git a/fs/ocfs2/super.c b/fs/ocfs2/super.c index 3a9a1af39ad7..2de4c8a9340c 100644 --- a/fs/ocfs2/super.c +++ b/fs/ocfs2/super.c @@ -1563,8 +1563,8 @@ static int ocfs2_show_options(struct seq_file *s, struct dentry *root) seq_printf(s, ",localflocks,"); if (osb->osb_cluster_stack[0]) - seq_printf(s, ",cluster_stack=%.*s", OCFS2_STACK_LABEL_LEN, - osb->osb_cluster_stack); + seq_show_option_n(s, "cluster_stack", osb->osb_cluster_stack, + OCFS2_STACK_LABEL_LEN); if (opts & OCFS2_MOUNT_USRQUOTA) seq_printf(s, ",usrquota"); if (opts & OCFS2_MOUNT_GRPQUOTA) diff --git a/fs/overlayfs/super.c b/fs/overlayfs/super.c index 7466ff339c66..79073d68b475 100644 --- a/fs/overlayfs/super.c +++ b/fs/overlayfs/super.c @@ -588,10 +588,10 @@ static int ovl_show_options(struct seq_file *m, struct dentry *dentry) struct super_block *sb = dentry->d_sb; struct ovl_fs *ufs = sb->s_fs_info; - seq_printf(m, ",lowerdir=%s", ufs->config.lowerdir); + seq_show_option(m, "lowerdir", ufs->config.lowerdir); if (ufs->config.upperdir) { - seq_printf(m, ",upperdir=%s", ufs->config.upperdir); - seq_printf(m, ",workdir=%s", ufs->config.workdir); + seq_show_option(m, "upperdir", ufs->config.upperdir); + seq_show_option(m, "workdir", ufs->config.workdir); } return 0; } diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c index 0e4cf728126f..4a62fe8cc3bf 100644 --- a/fs/reiserfs/super.c +++ b/fs/reiserfs/super.c @@ -714,18 +714,20 @@ static int reiserfs_show_options(struct seq_file *seq, struct dentry *root) seq_puts(seq, ",acl"); if (REISERFS_SB(s)->s_jdev) - seq_printf(seq, ",jdev=%s", REISERFS_SB(s)->s_jdev); + seq_show_option(seq, "jdev", REISERFS_SB(s)->s_jdev); if (journal->j_max_commit_age != journal->j_default_max_commit_age) seq_printf(seq, ",commit=%d", journal->j_max_commit_age); #ifdef CONFIG_QUOTA if (REISERFS_SB(s)->s_qf_names[USRQUOTA]) - seq_printf(seq, ",usrjquota=%s", REISERFS_SB(s)->s_qf_names[USRQUOTA]); + seq_show_option(seq, "usrjquota", + REISERFS_SB(s)->s_qf_names[USRQUOTA]); else if (opts & (1 << REISERFS_USRQUOTA)) seq_puts(seq, ",usrquota"); if (REISERFS_SB(s)->s_qf_names[GRPQUOTA]) - seq_printf(seq, ",grpjquota=%s", REISERFS_SB(s)->s_qf_names[GRPQUOTA]); + seq_show_option(seq, "grpjquota", + REISERFS_SB(s)->s_qf_names[GRPQUOTA]); else if (opts & (1 << REISERFS_GRPQUOTA)) seq_puts(seq, ",grpquota"); if (REISERFS_SB(s)->s_jquota_fmt) { diff --git a/fs/xfs/xfs_super.c b/fs/xfs/xfs_super.c index 1fb16562c159..bbd9b1f10ffb 100644 --- a/fs/xfs/xfs_super.c +++ b/fs/xfs/xfs_super.c @@ -511,9 +511,9 @@ xfs_showargs( seq_printf(m, "," MNTOPT_LOGBSIZE "=%dk", mp->m_logbsize >> 10); if (mp->m_logname) - seq_printf(m, "," MNTOPT_LOGDEV "=%s", mp->m_logname); + seq_show_option(m, MNTOPT_LOGDEV, mp->m_logname); if (mp->m_rtname) - seq_printf(m, "," MNTOPT_RTDEV "=%s", mp->m_rtname); + seq_show_option(m, MNTOPT_RTDEV, mp->m_rtname); if (mp->m_dalign > 0) seq_printf(m, "," MNTOPT_SUNIT "=%d", -- cgit v1.2.1 From 16ba6f811dfe44bc14f7946a4b257b85476fc16e Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:17 -0700 Subject: userfaultfd: add VM_UFFD_MISSING and VM_UFFD_WP These two flags gets set in vma->vm_flags to tell the VM common code if the userfaultfd is armed and in which mode (only tracking missing faults, only tracking wrprotect faults or both). If neither flags is set it means the userfaultfd is not armed on the vma. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/proc/task_mmu.c | 2 ++ 1 file changed, 2 insertions(+) (limited to 'fs') diff --git a/fs/proc/task_mmu.c b/fs/proc/task_mmu.c index ca1e091881d4..3b4d8255e806 100644 --- a/fs/proc/task_mmu.c +++ b/fs/proc/task_mmu.c @@ -597,6 +597,8 @@ static void show_smap_vma_flags(struct seq_file *m, struct vm_area_struct *vma) [ilog2(VM_HUGEPAGE)] = "hg", [ilog2(VM_NOHUGEPAGE)] = "nh", [ilog2(VM_MERGEABLE)] = "mg", + [ilog2(VM_UFFD_MISSING)]= "um", + [ilog2(VM_UFFD_WP)] = "uw", }; size_t i; -- cgit v1.2.1 From 86039bd3b4e6a1129318cbfed4e0a6e001656635 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:31 -0700 Subject: userfaultfd: add new syscall to provide memory externalization Once an userfaultfd has been created and certain region of the process virtual address space have been registered into it, the thread responsible for doing the memory externalization can manage the page faults in userland by talking to the kernel using the userfaultfd protocol. poll() can be used to know when there are new pending userfaults to be read (POLLIN). Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Cc: Dan Carpenter Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 1036 ++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 1036 insertions(+) create mode 100644 fs/userfaultfd.c (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c new file mode 100644 index 000000000000..9bc256d1a143 --- /dev/null +++ b/fs/userfaultfd.c @@ -0,0 +1,1036 @@ +/* + * fs/userfaultfd.c + * + * Copyright (C) 2007 Davide Libenzi + * Copyright (C) 2008-2009 Red Hat, Inc. + * Copyright (C) 2015 Red Hat, Inc. + * + * This work is licensed under the terms of the GNU GPL, version 2. See + * the COPYING file in the top-level directory. + * + * Some part derived from fs/eventfd.c (anon inode setup) and + * mm/ksm.c (mm hashing). + */ + +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include +#include + +enum userfaultfd_state { + UFFD_STATE_WAIT_API, + UFFD_STATE_RUNNING, +}; + +struct userfaultfd_ctx { + /* pseudo fd refcounting */ + atomic_t refcount; + /* waitqueue head for the userfaultfd page faults */ + wait_queue_head_t fault_wqh; + /* waitqueue head for the pseudo fd to wakeup poll/read */ + wait_queue_head_t fd_wqh; + /* userfaultfd syscall flags */ + unsigned int flags; + /* state machine */ + enum userfaultfd_state state; + /* released */ + bool released; + /* mm with one ore more vmas attached to this userfaultfd_ctx */ + struct mm_struct *mm; +}; + +struct userfaultfd_wait_queue { + unsigned long address; + wait_queue_t wq; + bool pending; + struct userfaultfd_ctx *ctx; +}; + +struct userfaultfd_wake_range { + unsigned long start; + unsigned long len; +}; + +static int userfaultfd_wake_function(wait_queue_t *wq, unsigned mode, + int wake_flags, void *key) +{ + struct userfaultfd_wake_range *range = key; + int ret; + struct userfaultfd_wait_queue *uwq; + unsigned long start, len; + + uwq = container_of(wq, struct userfaultfd_wait_queue, wq); + ret = 0; + /* don't wake the pending ones to avoid reads to block */ + if (uwq->pending && !ACCESS_ONCE(uwq->ctx->released)) + goto out; + /* len == 0 means wake all */ + start = range->start; + len = range->len; + if (len && (start > uwq->address || start + len <= uwq->address)) + goto out; + ret = wake_up_state(wq->private, mode); + if (ret) + /* + * Wake only once, autoremove behavior. + * + * After the effect of list_del_init is visible to the + * other CPUs, the waitqueue may disappear from under + * us, see the !list_empty_careful() in + * handle_userfault(). try_to_wake_up() has an + * implicit smp_mb__before_spinlock, and the + * wq->private is read before calling the extern + * function "wake_up_state" (which in turns calls + * try_to_wake_up). While the spin_lock;spin_unlock; + * wouldn't be enough, the smp_mb__before_spinlock is + * enough to avoid an explicit smp_mb() here. + */ + list_del_init(&wq->task_list); +out: + return ret; +} + +/** + * userfaultfd_ctx_get - Acquires a reference to the internal userfaultfd + * context. + * @ctx: [in] Pointer to the userfaultfd context. + * + * Returns: In case of success, returns not zero. + */ +static void userfaultfd_ctx_get(struct userfaultfd_ctx *ctx) +{ + if (!atomic_inc_not_zero(&ctx->refcount)) + BUG(); +} + +/** + * userfaultfd_ctx_put - Releases a reference to the internal userfaultfd + * context. + * @ctx: [in] Pointer to userfaultfd context. + * + * The userfaultfd context reference must have been previously acquired either + * with userfaultfd_ctx_get() or userfaultfd_ctx_fdget(). + */ +static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx) +{ + if (atomic_dec_and_test(&ctx->refcount)) { + VM_BUG_ON(spin_is_locked(&ctx->fault_pending_wqh.lock)); + VM_BUG_ON(waitqueue_active(&ctx->fault_pending_wqh)); + VM_BUG_ON(spin_is_locked(&ctx->fault_wqh.lock)); + VM_BUG_ON(waitqueue_active(&ctx->fault_wqh)); + VM_BUG_ON(spin_is_locked(&ctx->fd_wqh.lock)); + VM_BUG_ON(waitqueue_active(&ctx->fd_wqh)); + mmput(ctx->mm); + kfree(ctx); + } +} + +static inline unsigned long userfault_address(unsigned long address, + unsigned int flags, + unsigned long reason) +{ + BUILD_BUG_ON(PAGE_SHIFT < UFFD_BITS); + address &= PAGE_MASK; + if (flags & FAULT_FLAG_WRITE) + /* + * Encode "write" fault information in the LSB of the + * address read by userland, without depending on + * FAULT_FLAG_WRITE kernel internal value. + */ + address |= UFFD_BIT_WRITE; + if (reason & VM_UFFD_WP) + /* + * Encode "reason" fault information as bit number 1 + * in the address read by userland. If bit number 1 is + * clear it means the reason is a VM_FAULT_MISSING + * fault. + */ + address |= UFFD_BIT_WP; + return address; +} + +/* + * The locking rules involved in returning VM_FAULT_RETRY depending on + * FAULT_FLAG_ALLOW_RETRY, FAULT_FLAG_RETRY_NOWAIT and + * FAULT_FLAG_KILLABLE are not straightforward. The "Caution" + * recommendation in __lock_page_or_retry is not an understatement. + * + * If FAULT_FLAG_ALLOW_RETRY is set, the mmap_sem must be released + * before returning VM_FAULT_RETRY only if FAULT_FLAG_RETRY_NOWAIT is + * not set. + * + * If FAULT_FLAG_ALLOW_RETRY is set but FAULT_FLAG_KILLABLE is not + * set, VM_FAULT_RETRY can still be returned if and only if there are + * fatal_signal_pending()s, and the mmap_sem must be released before + * returning it. + */ +int handle_userfault(struct vm_area_struct *vma, unsigned long address, + unsigned int flags, unsigned long reason) +{ + struct mm_struct *mm = vma->vm_mm; + struct userfaultfd_ctx *ctx; + struct userfaultfd_wait_queue uwq; + + BUG_ON(!rwsem_is_locked(&mm->mmap_sem)); + + ctx = vma->vm_userfaultfd_ctx.ctx; + if (!ctx) + return VM_FAULT_SIGBUS; + + BUG_ON(ctx->mm != mm); + + VM_BUG_ON(reason & ~(VM_UFFD_MISSING|VM_UFFD_WP)); + VM_BUG_ON(!(reason & VM_UFFD_MISSING) ^ !!(reason & VM_UFFD_WP)); + + /* + * If it's already released don't get it. This avoids to loop + * in __get_user_pages if userfaultfd_release waits on the + * caller of handle_userfault to release the mmap_sem. + */ + if (unlikely(ACCESS_ONCE(ctx->released))) + return VM_FAULT_SIGBUS; + + /* + * Check that we can return VM_FAULT_RETRY. + * + * NOTE: it should become possible to return VM_FAULT_RETRY + * even if FAULT_FLAG_TRIED is set without leading to gup() + * -EBUSY failures, if the userfaultfd is to be extended for + * VM_UFFD_WP tracking and we intend to arm the userfault + * without first stopping userland access to the memory. For + * VM_UFFD_MISSING userfaults this is enough for now. + */ + if (unlikely(!(flags & FAULT_FLAG_ALLOW_RETRY))) { + /* + * Validate the invariant that nowait must allow retry + * to be sure not to return SIGBUS erroneously on + * nowait invocations. + */ + BUG_ON(flags & FAULT_FLAG_RETRY_NOWAIT); +#ifdef CONFIG_DEBUG_VM + if (printk_ratelimit()) { + printk(KERN_WARNING + "FAULT_FLAG_ALLOW_RETRY missing %x\n", flags); + dump_stack(); + } +#endif + return VM_FAULT_SIGBUS; + } + + /* + * Handle nowait, not much to do other than tell it to retry + * and wait. + */ + if (flags & FAULT_FLAG_RETRY_NOWAIT) + return VM_FAULT_RETRY; + + /* take the reference before dropping the mmap_sem */ + userfaultfd_ctx_get(ctx); + + /* be gentle and immediately relinquish the mmap_sem */ + up_read(&mm->mmap_sem); + + init_waitqueue_func_entry(&uwq.wq, userfaultfd_wake_function); + uwq.wq.private = current; + uwq.address = userfault_address(address, flags, reason); + uwq.pending = true; + uwq.ctx = ctx; + + spin_lock(&ctx->fault_wqh.lock); + /* + * After the __add_wait_queue the uwq is visible to userland + * through poll/read(). + */ + __add_wait_queue(&ctx->fault_wqh, &uwq.wq); + for (;;) { + set_current_state(TASK_KILLABLE); + if (!uwq.pending || ACCESS_ONCE(ctx->released) || + fatal_signal_pending(current)) + break; + spin_unlock(&ctx->fault_wqh.lock); + + wake_up_poll(&ctx->fd_wqh, POLLIN); + schedule(); + + spin_lock(&ctx->fault_wqh.lock); + } + __remove_wait_queue(&ctx->fault_wqh, &uwq.wq); + __set_current_state(TASK_RUNNING); + spin_unlock(&ctx->fault_wqh.lock); + + /* + * ctx may go away after this if the userfault pseudo fd is + * already released. + */ + userfaultfd_ctx_put(ctx); + + return VM_FAULT_RETRY; +} + +static int userfaultfd_release(struct inode *inode, struct file *file) +{ + struct userfaultfd_ctx *ctx = file->private_data; + struct mm_struct *mm = ctx->mm; + struct vm_area_struct *vma, *prev; + /* len == 0 means wake all */ + struct userfaultfd_wake_range range = { .len = 0, }; + unsigned long new_flags; + + ACCESS_ONCE(ctx->released) = true; + + /* + * Flush page faults out of all CPUs. NOTE: all page faults + * must be retried without returning VM_FAULT_SIGBUS if + * userfaultfd_ctx_get() succeeds but vma->vma_userfault_ctx + * changes while handle_userfault released the mmap_sem. So + * it's critical that released is set to true (above), before + * taking the mmap_sem for writing. + */ + down_write(&mm->mmap_sem); + prev = NULL; + for (vma = mm->mmap; vma; vma = vma->vm_next) { + cond_resched(); + BUG_ON(!!vma->vm_userfaultfd_ctx.ctx ^ + !!(vma->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP))); + if (vma->vm_userfaultfd_ctx.ctx != ctx) { + prev = vma; + continue; + } + new_flags = vma->vm_flags & ~(VM_UFFD_MISSING | VM_UFFD_WP); + prev = vma_merge(mm, prev, vma->vm_start, vma->vm_end, + new_flags, vma->anon_vma, + vma->vm_file, vma->vm_pgoff, + vma_policy(vma), + NULL_VM_UFFD_CTX); + if (prev) + vma = prev; + else + prev = vma; + vma->vm_flags = new_flags; + vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX; + } + up_write(&mm->mmap_sem); + + /* + * After no new page faults can wait on this fault_wqh, flush + * the last page faults that may have been already waiting on + * the fault_wqh. + */ + spin_lock(&ctx->fault_wqh.lock); + __wake_up_locked_key(&ctx->fault_wqh, TASK_NORMAL, 0, &range); + spin_unlock(&ctx->fault_wqh.lock); + + wake_up_poll(&ctx->fd_wqh, POLLHUP); + userfaultfd_ctx_put(ctx); + return 0; +} + +/* fault_wqh.lock must be hold by the caller */ +static inline unsigned int find_userfault(struct userfaultfd_ctx *ctx, + struct userfaultfd_wait_queue **uwq) +{ + wait_queue_t *wq; + struct userfaultfd_wait_queue *_uwq; + unsigned int ret = 0; + + VM_BUG_ON(!spin_is_locked(&ctx->fault_wqh.lock)); + + list_for_each_entry(wq, &ctx->fault_wqh.task_list, task_list) { + _uwq = container_of(wq, struct userfaultfd_wait_queue, wq); + if (_uwq->pending) { + ret = POLLIN; + if (!uwq) + /* + * If there's at least a pending and + * we don't care which one it is, + * break immediately and leverage the + * efficiency of the LIFO walk. + */ + break; + /* + * If we need to find which one was pending we + * keep walking until we find the first not + * pending one, so we read() them in FIFO order. + */ + *uwq = _uwq; + } else + /* + * break the loop at the first not pending + * one, there cannot be pending userfaults + * after the first not pending one, because + * all new pending ones are inserted at the + * head and we walk it in LIFO. + */ + break; + } + + return ret; +} + +static unsigned int userfaultfd_poll(struct file *file, poll_table *wait) +{ + struct userfaultfd_ctx *ctx = file->private_data; + unsigned int ret; + + poll_wait(file, &ctx->fd_wqh, wait); + + switch (ctx->state) { + case UFFD_STATE_WAIT_API: + return POLLERR; + case UFFD_STATE_RUNNING: + spin_lock(&ctx->fault_wqh.lock); + ret = find_userfault(ctx, NULL); + spin_unlock(&ctx->fault_wqh.lock); + return ret; + default: + BUG(); + } +} + +static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, + __u64 *addr) +{ + ssize_t ret; + DECLARE_WAITQUEUE(wait, current); + struct userfaultfd_wait_queue *uwq = NULL; + + /* always take the fd_wqh lock before the fault_wqh lock */ + spin_lock(&ctx->fd_wqh.lock); + __add_wait_queue(&ctx->fd_wqh, &wait); + for (;;) { + set_current_state(TASK_INTERRUPTIBLE); + spin_lock(&ctx->fault_wqh.lock); + if (find_userfault(ctx, &uwq)) { + /* + * The fault_wqh.lock prevents the uwq to + * disappear from under us. + */ + uwq->pending = false; + /* careful to always initialize addr if ret == 0 */ + *addr = uwq->address; + spin_unlock(&ctx->fault_wqh.lock); + ret = 0; + break; + } + spin_unlock(&ctx->fault_wqh.lock); + if (signal_pending(current)) { + ret = -ERESTARTSYS; + break; + } + if (no_wait) { + ret = -EAGAIN; + break; + } + spin_unlock(&ctx->fd_wqh.lock); + schedule(); + spin_lock(&ctx->fd_wqh.lock); + } + __remove_wait_queue(&ctx->fd_wqh, &wait); + __set_current_state(TASK_RUNNING); + spin_unlock(&ctx->fd_wqh.lock); + + return ret; +} + +static ssize_t userfaultfd_read(struct file *file, char __user *buf, + size_t count, loff_t *ppos) +{ + struct userfaultfd_ctx *ctx = file->private_data; + ssize_t _ret, ret = 0; + /* careful to always initialize addr if ret == 0 */ + __u64 uninitialized_var(addr); + int no_wait = file->f_flags & O_NONBLOCK; + + if (ctx->state == UFFD_STATE_WAIT_API) + return -EINVAL; + BUG_ON(ctx->state != UFFD_STATE_RUNNING); + + for (;;) { + if (count < sizeof(addr)) + return ret ? ret : -EINVAL; + _ret = userfaultfd_ctx_read(ctx, no_wait, &addr); + if (_ret < 0) + return ret ? ret : _ret; + if (put_user(addr, (__u64 __user *) buf)) + return ret ? ret : -EFAULT; + ret += sizeof(addr); + buf += sizeof(addr); + count -= sizeof(addr); + /* + * Allow to read more than one fault at time but only + * block if waiting for the very first one. + */ + no_wait = O_NONBLOCK; + } +} + +static void __wake_userfault(struct userfaultfd_ctx *ctx, + struct userfaultfd_wake_range *range) +{ + unsigned long start, end; + + start = range->start; + end = range->start + range->len; + + spin_lock(&ctx->fault_wqh.lock); + /* wake all in the range and autoremove */ + __wake_up_locked_key(&ctx->fault_wqh, TASK_NORMAL, 0, range); + spin_unlock(&ctx->fault_wqh.lock); +} + +static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx, + struct userfaultfd_wake_range *range) +{ + /* + * To be sure waitqueue_active() is not reordered by the CPU + * before the pagetable update, use an explicit SMP memory + * barrier here. PT lock release or up_read(mmap_sem) still + * have release semantics that can allow the + * waitqueue_active() to be reordered before the pte update. + */ + smp_mb(); + + /* + * Use waitqueue_active because it's very frequent to + * change the address space atomically even if there are no + * userfaults yet. So we take the spinlock only when we're + * sure we've userfaults to wake. + */ + if (waitqueue_active(&ctx->fault_wqh)) + __wake_userfault(ctx, range); +} + +static __always_inline int validate_range(struct mm_struct *mm, + __u64 start, __u64 len) +{ + __u64 task_size = mm->task_size; + + if (start & ~PAGE_MASK) + return -EINVAL; + if (len & ~PAGE_MASK) + return -EINVAL; + if (!len) + return -EINVAL; + if (start < mmap_min_addr) + return -EINVAL; + if (start >= task_size) + return -EINVAL; + if (len > task_size - start) + return -EINVAL; + return 0; +} + +static int userfaultfd_register(struct userfaultfd_ctx *ctx, + unsigned long arg) +{ + struct mm_struct *mm = ctx->mm; + struct vm_area_struct *vma, *prev, *cur; + int ret; + struct uffdio_register uffdio_register; + struct uffdio_register __user *user_uffdio_register; + unsigned long vm_flags, new_flags; + bool found; + unsigned long start, end, vma_end; + + user_uffdio_register = (struct uffdio_register __user *) arg; + + ret = -EFAULT; + if (copy_from_user(&uffdio_register, user_uffdio_register, + sizeof(uffdio_register)-sizeof(__u64))) + goto out; + + ret = -EINVAL; + if (!uffdio_register.mode) + goto out; + if (uffdio_register.mode & ~(UFFDIO_REGISTER_MODE_MISSING| + UFFDIO_REGISTER_MODE_WP)) + goto out; + vm_flags = 0; + if (uffdio_register.mode & UFFDIO_REGISTER_MODE_MISSING) + vm_flags |= VM_UFFD_MISSING; + if (uffdio_register.mode & UFFDIO_REGISTER_MODE_WP) { + vm_flags |= VM_UFFD_WP; + /* + * FIXME: remove the below error constraint by + * implementing the wprotect tracking mode. + */ + ret = -EINVAL; + goto out; + } + + ret = validate_range(mm, uffdio_register.range.start, + uffdio_register.range.len); + if (ret) + goto out; + + start = uffdio_register.range.start; + end = start + uffdio_register.range.len; + + down_write(&mm->mmap_sem); + vma = find_vma_prev(mm, start, &prev); + + ret = -ENOMEM; + if (!vma) + goto out_unlock; + + /* check that there's at least one vma in the range */ + ret = -EINVAL; + if (vma->vm_start >= end) + goto out_unlock; + + /* + * Search for not compatible vmas. + * + * FIXME: this shall be relaxed later so that it doesn't fail + * on tmpfs backed vmas (in addition to the current allowance + * on anonymous vmas). + */ + found = false; + for (cur = vma; cur && cur->vm_start < end; cur = cur->vm_next) { + cond_resched(); + + BUG_ON(!!cur->vm_userfaultfd_ctx.ctx ^ + !!(cur->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP))); + + /* check not compatible vmas */ + ret = -EINVAL; + if (cur->vm_ops) + goto out_unlock; + + /* + * Check that this vma isn't already owned by a + * different userfaultfd. We can't allow more than one + * userfaultfd to own a single vma simultaneously or we + * wouldn't know which one to deliver the userfaults to. + */ + ret = -EBUSY; + if (cur->vm_userfaultfd_ctx.ctx && + cur->vm_userfaultfd_ctx.ctx != ctx) + goto out_unlock; + + found = true; + } + BUG_ON(!found); + + if (vma->vm_start < start) + prev = vma; + + ret = 0; + do { + cond_resched(); + + BUG_ON(vma->vm_ops); + BUG_ON(vma->vm_userfaultfd_ctx.ctx && + vma->vm_userfaultfd_ctx.ctx != ctx); + + /* + * Nothing to do: this vma is already registered into this + * userfaultfd and with the right tracking mode too. + */ + if (vma->vm_userfaultfd_ctx.ctx == ctx && + (vma->vm_flags & vm_flags) == vm_flags) + goto skip; + + if (vma->vm_start > start) + start = vma->vm_start; + vma_end = min(end, vma->vm_end); + + new_flags = (vma->vm_flags & ~vm_flags) | vm_flags; + prev = vma_merge(mm, prev, start, vma_end, new_flags, + vma->anon_vma, vma->vm_file, vma->vm_pgoff, + vma_policy(vma), + ((struct vm_userfaultfd_ctx){ ctx })); + if (prev) { + vma = prev; + goto next; + } + if (vma->vm_start < start) { + ret = split_vma(mm, vma, start, 1); + if (ret) + break; + } + if (vma->vm_end > end) { + ret = split_vma(mm, vma, end, 0); + if (ret) + break; + } + next: + /* + * In the vma_merge() successful mprotect-like case 8: + * the next vma was merged into the current one and + * the current one has not been updated yet. + */ + vma->vm_flags = new_flags; + vma->vm_userfaultfd_ctx.ctx = ctx; + + skip: + prev = vma; + start = vma->vm_end; + vma = vma->vm_next; + } while (vma && vma->vm_start < end); +out_unlock: + up_write(&mm->mmap_sem); + if (!ret) { + /* + * Now that we scanned all vmas we can already tell + * userland which ioctls methods are guaranteed to + * succeed on this range. + */ + if (put_user(UFFD_API_RANGE_IOCTLS, + &user_uffdio_register->ioctls)) + ret = -EFAULT; + } +out: + return ret; +} + +static int userfaultfd_unregister(struct userfaultfd_ctx *ctx, + unsigned long arg) +{ + struct mm_struct *mm = ctx->mm; + struct vm_area_struct *vma, *prev, *cur; + int ret; + struct uffdio_range uffdio_unregister; + unsigned long new_flags; + bool found; + unsigned long start, end, vma_end; + const void __user *buf = (void __user *)arg; + + ret = -EFAULT; + if (copy_from_user(&uffdio_unregister, buf, sizeof(uffdio_unregister))) + goto out; + + ret = validate_range(mm, uffdio_unregister.start, + uffdio_unregister.len); + if (ret) + goto out; + + start = uffdio_unregister.start; + end = start + uffdio_unregister.len; + + down_write(&mm->mmap_sem); + vma = find_vma_prev(mm, start, &prev); + + ret = -ENOMEM; + if (!vma) + goto out_unlock; + + /* check that there's at least one vma in the range */ + ret = -EINVAL; + if (vma->vm_start >= end) + goto out_unlock; + + /* + * Search for not compatible vmas. + * + * FIXME: this shall be relaxed later so that it doesn't fail + * on tmpfs backed vmas (in addition to the current allowance + * on anonymous vmas). + */ + found = false; + ret = -EINVAL; + for (cur = vma; cur && cur->vm_start < end; cur = cur->vm_next) { + cond_resched(); + + BUG_ON(!!cur->vm_userfaultfd_ctx.ctx ^ + !!(cur->vm_flags & (VM_UFFD_MISSING | VM_UFFD_WP))); + + /* + * Check not compatible vmas, not strictly required + * here as not compatible vmas cannot have an + * userfaultfd_ctx registered on them, but this + * provides for more strict behavior to notice + * unregistration errors. + */ + if (cur->vm_ops) + goto out_unlock; + + found = true; + } + BUG_ON(!found); + + if (vma->vm_start < start) + prev = vma; + + ret = 0; + do { + cond_resched(); + + BUG_ON(vma->vm_ops); + + /* + * Nothing to do: this vma is already registered into this + * userfaultfd and with the right tracking mode too. + */ + if (!vma->vm_userfaultfd_ctx.ctx) + goto skip; + + if (vma->vm_start > start) + start = vma->vm_start; + vma_end = min(end, vma->vm_end); + + new_flags = vma->vm_flags & ~(VM_UFFD_MISSING | VM_UFFD_WP); + prev = vma_merge(mm, prev, start, vma_end, new_flags, + vma->anon_vma, vma->vm_file, vma->vm_pgoff, + vma_policy(vma), + NULL_VM_UFFD_CTX); + if (prev) { + vma = prev; + goto next; + } + if (vma->vm_start < start) { + ret = split_vma(mm, vma, start, 1); + if (ret) + break; + } + if (vma->vm_end > end) { + ret = split_vma(mm, vma, end, 0); + if (ret) + break; + } + next: + /* + * In the vma_merge() successful mprotect-like case 8: + * the next vma was merged into the current one and + * the current one has not been updated yet. + */ + vma->vm_flags = new_flags; + vma->vm_userfaultfd_ctx = NULL_VM_UFFD_CTX; + + skip: + prev = vma; + start = vma->vm_end; + vma = vma->vm_next; + } while (vma && vma->vm_start < end); +out_unlock: + up_write(&mm->mmap_sem); +out: + return ret; +} + +/* + * This is mostly needed to re-wakeup those userfaults that were still + * pending when userland wake them up the first time. We don't wake + * the pending one to avoid blocking reads to block, or non blocking + * read to return -EAGAIN, if used with POLLIN, to avoid userland + * doubts on why POLLIN wasn't reliable. + */ +static int userfaultfd_wake(struct userfaultfd_ctx *ctx, + unsigned long arg) +{ + int ret; + struct uffdio_range uffdio_wake; + struct userfaultfd_wake_range range; + const void __user *buf = (void __user *)arg; + + ret = -EFAULT; + if (copy_from_user(&uffdio_wake, buf, sizeof(uffdio_wake))) + goto out; + + ret = validate_range(ctx->mm, uffdio_wake.start, uffdio_wake.len); + if (ret) + goto out; + + range.start = uffdio_wake.start; + range.len = uffdio_wake.len; + + /* + * len == 0 means wake all and we don't want to wake all here, + * so check it again to be sure. + */ + VM_BUG_ON(!range.len); + + wake_userfault(ctx, &range); + ret = 0; + +out: + return ret; +} + +/* + * userland asks for a certain API version and we return which bits + * and ioctl commands are implemented in this kernel for such API + * version or -EINVAL if unknown. + */ +static int userfaultfd_api(struct userfaultfd_ctx *ctx, + unsigned long arg) +{ + struct uffdio_api uffdio_api; + void __user *buf = (void __user *)arg; + int ret; + + ret = -EINVAL; + if (ctx->state != UFFD_STATE_WAIT_API) + goto out; + ret = -EFAULT; + if (copy_from_user(&uffdio_api, buf, sizeof(__u64))) + goto out; + if (uffdio_api.api != UFFD_API) { + /* careful not to leak info, we only read the first 8 bytes */ + memset(&uffdio_api, 0, sizeof(uffdio_api)); + if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api))) + goto out; + ret = -EINVAL; + goto out; + } + /* careful not to leak info, we only read the first 8 bytes */ + uffdio_api.bits = UFFD_API_BITS; + uffdio_api.ioctls = UFFD_API_IOCTLS; + ret = -EFAULT; + if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api))) + goto out; + ctx->state = UFFD_STATE_RUNNING; + ret = 0; +out: + return ret; +} + +static long userfaultfd_ioctl(struct file *file, unsigned cmd, + unsigned long arg) +{ + int ret = -EINVAL; + struct userfaultfd_ctx *ctx = file->private_data; + + switch(cmd) { + case UFFDIO_API: + ret = userfaultfd_api(ctx, arg); + break; + case UFFDIO_REGISTER: + ret = userfaultfd_register(ctx, arg); + break; + case UFFDIO_UNREGISTER: + ret = userfaultfd_unregister(ctx, arg); + break; + case UFFDIO_WAKE: + ret = userfaultfd_wake(ctx, arg); + break; + } + return ret; +} + +#ifdef CONFIG_PROC_FS +static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f) +{ + struct userfaultfd_ctx *ctx = f->private_data; + wait_queue_t *wq; + struct userfaultfd_wait_queue *uwq; + unsigned long pending = 0, total = 0; + + spin_lock(&ctx->fault_wqh.lock); + list_for_each_entry(wq, &ctx->fault_wqh.task_list, task_list) { + uwq = container_of(wq, struct userfaultfd_wait_queue, wq); + if (uwq->pending) + pending++; + total++; + } + spin_unlock(&ctx->fault_wqh.lock); + + /* + * If more protocols will be added, there will be all shown + * separated by a space. Like this: + * protocols: aa:... bb:... + */ + seq_printf(m, "pending:\t%lu\ntotal:\t%lu\nAPI:\t%Lx:%x:%Lx\n", + pending, total, UFFD_API, UFFD_API_BITS, + UFFD_API_IOCTLS|UFFD_API_RANGE_IOCTLS); +} +#endif + +static const struct file_operations userfaultfd_fops = { +#ifdef CONFIG_PROC_FS + .show_fdinfo = userfaultfd_show_fdinfo, +#endif + .release = userfaultfd_release, + .poll = userfaultfd_poll, + .read = userfaultfd_read, + .unlocked_ioctl = userfaultfd_ioctl, + .compat_ioctl = userfaultfd_ioctl, + .llseek = noop_llseek, +}; + +/** + * userfaultfd_file_create - Creates an userfaultfd file pointer. + * @flags: Flags for the userfaultfd file. + * + * This function creates an userfaultfd file pointer, w/out installing + * it into the fd table. This is useful when the userfaultfd file is + * used during the initialization of data structures that require + * extra setup after the userfaultfd creation. So the userfaultfd + * creation is split into the file pointer creation phase, and the + * file descriptor installation phase. In this way races with + * userspace closing the newly installed file descriptor can be + * avoided. Returns an userfaultfd file pointer, or a proper error + * pointer. + */ +static struct file *userfaultfd_file_create(int flags) +{ + struct file *file; + struct userfaultfd_ctx *ctx; + + BUG_ON(!current->mm); + + /* Check the UFFD_* constants for consistency. */ + BUILD_BUG_ON(UFFD_CLOEXEC != O_CLOEXEC); + BUILD_BUG_ON(UFFD_NONBLOCK != O_NONBLOCK); + + file = ERR_PTR(-EINVAL); + if (flags & ~UFFD_SHARED_FCNTL_FLAGS) + goto out; + + file = ERR_PTR(-ENOMEM); + ctx = kmalloc(sizeof(*ctx), GFP_KERNEL); + if (!ctx) + goto out; + + atomic_set(&ctx->refcount, 1); + init_waitqueue_head(&ctx->fault_wqh); + init_waitqueue_head(&ctx->fd_wqh); + ctx->flags = flags; + ctx->state = UFFD_STATE_WAIT_API; + ctx->released = false; + ctx->mm = current->mm; + /* prevent the mm struct to be freed */ + atomic_inc(&ctx->mm->mm_users); + + file = anon_inode_getfile("[userfaultfd]", &userfaultfd_fops, ctx, + O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS)); + if (IS_ERR(file)) + kfree(ctx); +out: + return file; +} + +SYSCALL_DEFINE1(userfaultfd, int, flags) +{ + int fd, error; + struct file *file; + + error = get_unused_fd_flags(flags & UFFD_SHARED_FCNTL_FLAGS); + if (error < 0) + return error; + fd = error; + + file = userfaultfd_file_create(flags); + if (IS_ERR(file)) { + error = PTR_ERR(file); + goto err_put_unused_fd; + } + fd_install(fd, file); + + return fd; + +err_put_unused_fd: + put_unused_fd(fd); + + return error; +} -- cgit v1.2.1 From 3f602d2724b1f7d2d27ddcd7963a040a5890fd16 Mon Sep 17 00:00:00 2001 From: Pavel Emelyanov Date: Fri, 4 Sep 2015 15:46:34 -0700 Subject: userfaultfd: Rename uffd_api.bits into .features This is (seems to be) the minimal thing that is required to unblock standard uffd usage from the non-cooperative one. Now more bits can be added to the features field indicating e.g. UFFD_FEATURE_FORK and others needed for the latter use-case. Signed-off-by: Pavel Emelyanov Signed-off-by: Andrea Arcangeli Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 9bc256d1a143..0756d97b0666 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -884,7 +884,7 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, goto out; } /* careful not to leak info, we only read the first 8 bytes */ - uffdio_api.bits = UFFD_API_BITS; + uffdio_api.features = UFFD_API_FEATURES; uffdio_api.ioctls = UFFD_API_IOCTLS; ret = -EFAULT; if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api))) @@ -941,7 +941,7 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f) * protocols: aa:... bb:... */ seq_printf(m, "pending:\t%lu\ntotal:\t%lu\nAPI:\t%Lx:%x:%Lx\n", - pending, total, UFFD_API, UFFD_API_BITS, + pending, total, UFFD_API, UFFD_API_FEATURES, UFFD_API_IOCTLS|UFFD_API_RANGE_IOCTLS); } #endif -- cgit v1.2.1 From a9b85f9415fd9e529d03299e5335433f614ec1fb Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:37 -0700 Subject: userfaultfd: change the read API to return a uffd_msg I had requests to return the full address (not the page aligned one) to userland. It's not entirely clear how the page offset could be relevant because userfaults aren't like SIGBUS that can sigjump to a different place and it actually skip resolving the fault depending on a page offset. There's currently no real way to skip the fault especially because after a UFFDIO_COPY|ZEROPAGE, the fault is optimized to be retried within the kernel without having to return to userland first (not even self modifying code replacing the .text that touched the faulting address would prevent the fault to be repeated). Userland cannot skip repeating the fault even more so if the fault was triggered by a KVM secondary page fault or any get_user_pages or any copy-user inside some syscall which will return to kernel code. The second time FAULT_FLAG_RETRY_NOWAIT won't be set leading to a SIGBUS being raised because the userfault can't wait if it cannot release the mmap_map first (and FAULT_FLAG_RETRY_NOWAIT is required for that). Still returning userland a proper structure during the read() on the uffd, can allow to use the current UFFD_API for the future non-cooperative extensions too and it looks cleaner as well. Once we get additional fields there's no point to return the fault address page aligned anymore to reuse the bits below PAGE_SHIFT. The only downside is that the read() syscall will read 32bytes instead of 8bytes but that's not going to be measurable overhead. The total number of new events that can be extended or of new future bits for already shipped events, is limited to 64 by the features field of the uffdio_api structure. If more will be needed a bump of UFFD_API will be required. [akpm@linux-foundation.org: use __packed] Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 79 +++++++++++++++++++++++++++++++++----------------------- 1 file changed, 46 insertions(+), 33 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 0756d97b0666..1f2ddaaf3c03 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -50,7 +50,7 @@ struct userfaultfd_ctx { }; struct userfaultfd_wait_queue { - unsigned long address; + struct uffd_msg msg; wait_queue_t wq; bool pending; struct userfaultfd_ctx *ctx; @@ -77,7 +77,8 @@ static int userfaultfd_wake_function(wait_queue_t *wq, unsigned mode, /* len == 0 means wake all */ start = range->start; len = range->len; - if (len && (start > uwq->address || start + len <= uwq->address)) + if (len && (start > uwq->msg.arg.pagefault.address || + start + len <= uwq->msg.arg.pagefault.address)) goto out; ret = wake_up_state(wq->private, mode); if (ret) @@ -135,28 +136,43 @@ static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx) } } -static inline unsigned long userfault_address(unsigned long address, - unsigned int flags, - unsigned long reason) +static inline void msg_init(struct uffd_msg *msg) { - BUILD_BUG_ON(PAGE_SHIFT < UFFD_BITS); - address &= PAGE_MASK; + BUILD_BUG_ON(sizeof(struct uffd_msg) != 32); + /* + * Must use memset to zero out the paddings or kernel data is + * leaked to userland. + */ + memset(msg, 0, sizeof(struct uffd_msg)); +} + +static inline struct uffd_msg userfault_msg(unsigned long address, + unsigned int flags, + unsigned long reason) +{ + struct uffd_msg msg; + msg_init(&msg); + msg.event = UFFD_EVENT_PAGEFAULT; + msg.arg.pagefault.address = address; if (flags & FAULT_FLAG_WRITE) /* - * Encode "write" fault information in the LSB of the - * address read by userland, without depending on - * FAULT_FLAG_WRITE kernel internal value. + * If UFFD_FEATURE_PAGEFAULT_FLAG_WRITE was set in the + * uffdio_api.features and UFFD_PAGEFAULT_FLAG_WRITE + * was not set in a UFFD_EVENT_PAGEFAULT, it means it + * was a read fault, otherwise if set it means it's + * a write fault. */ - address |= UFFD_BIT_WRITE; + msg.arg.pagefault.flags |= UFFD_PAGEFAULT_FLAG_WRITE; if (reason & VM_UFFD_WP) /* - * Encode "reason" fault information as bit number 1 - * in the address read by userland. If bit number 1 is - * clear it means the reason is a VM_FAULT_MISSING - * fault. + * If UFFD_FEATURE_PAGEFAULT_FLAG_WP was set in the + * uffdio_api.features and UFFD_PAGEFAULT_FLAG_WP was + * not set in a UFFD_EVENT_PAGEFAULT, it means it was + * a missing fault, otherwise if set it means it's a + * write protect fault. */ - address |= UFFD_BIT_WP; - return address; + msg.arg.pagefault.flags |= UFFD_PAGEFAULT_FLAG_WP; + return msg; } /* @@ -242,7 +258,7 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, init_waitqueue_func_entry(&uwq.wq, userfaultfd_wake_function); uwq.wq.private = current; - uwq.address = userfault_address(address, flags, reason); + uwq.msg = userfault_msg(address, flags, reason); uwq.pending = true; uwq.ctx = ctx; @@ -398,7 +414,7 @@ static unsigned int userfaultfd_poll(struct file *file, poll_table *wait) } static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, - __u64 *addr) + struct uffd_msg *msg) { ssize_t ret; DECLARE_WAITQUEUE(wait, current); @@ -416,8 +432,8 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, * disappear from under us. */ uwq->pending = false; - /* careful to always initialize addr if ret == 0 */ - *addr = uwq->address; + /* careful to always initialize msg if ret == 0 */ + *msg = uwq->msg; spin_unlock(&ctx->fault_wqh.lock); ret = 0; break; @@ -447,8 +463,7 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf, { struct userfaultfd_ctx *ctx = file->private_data; ssize_t _ret, ret = 0; - /* careful to always initialize addr if ret == 0 */ - __u64 uninitialized_var(addr); + struct uffd_msg msg; int no_wait = file->f_flags & O_NONBLOCK; if (ctx->state == UFFD_STATE_WAIT_API) @@ -456,16 +471,16 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf, BUG_ON(ctx->state != UFFD_STATE_RUNNING); for (;;) { - if (count < sizeof(addr)) + if (count < sizeof(msg)) return ret ? ret : -EINVAL; - _ret = userfaultfd_ctx_read(ctx, no_wait, &addr); + _ret = userfaultfd_ctx_read(ctx, no_wait, &msg); if (_ret < 0) return ret ? ret : _ret; - if (put_user(addr, (__u64 __user *) buf)) + if (copy_to_user((__u64 __user *) buf, &msg, sizeof(msg))) return ret ? ret : -EFAULT; - ret += sizeof(addr); - buf += sizeof(addr); - count -= sizeof(addr); + ret += sizeof(msg); + buf += sizeof(msg); + count -= sizeof(msg); /* * Allow to read more than one fault at time but only * block if waiting for the very first one. @@ -873,17 +888,15 @@ static int userfaultfd_api(struct userfaultfd_ctx *ctx, if (ctx->state != UFFD_STATE_WAIT_API) goto out; ret = -EFAULT; - if (copy_from_user(&uffdio_api, buf, sizeof(__u64))) + if (copy_from_user(&uffdio_api, buf, sizeof(uffdio_api))) goto out; - if (uffdio_api.api != UFFD_API) { - /* careful not to leak info, we only read the first 8 bytes */ + if (uffdio_api.api != UFFD_API || uffdio_api.features) { memset(&uffdio_api, 0, sizeof(uffdio_api)); if (copy_to_user(buf, &uffdio_api, sizeof(uffdio_api))) goto out; ret = -EINVAL; goto out; } - /* careful not to leak info, we only read the first 8 bytes */ uffdio_api.features = UFFD_API_FEATURES; uffdio_api.ioctls = UFFD_API_IOCTLS; ret = -EFAULT; -- cgit v1.2.1 From ba85c702e4b247393ffe9e3fbc13d8aee7b02059 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:41 -0700 Subject: userfaultfd: wake pending userfaults This is an optimization but it's a userland visible one and it affects the API. The downside of this optimization is that if you call poll() and you get POLLIN, read(ufd) may still return -EAGAIN. The blocked userfault may be waken by a different thread, before read(ufd) comes around. This in short means that poll() isn't really usable if the userfaultfd is opened in blocking mode. userfaults won't wait in "pending" state to be read anymore and any UFFDIO_WAKE or similar operations that has the objective of waking userfaults after their resolution, will wake all blocked userfaults for the resolved range, including those that haven't been read() by userland yet. The behavior of poll() becomes not standard, but this obviates the need of "spurious" UFFDIO_WAKE and it lets the userland threads to restart immediately without requiring an UFFDIO_WAKE. This is even more significant in case of repeated faults on the same address from multiple threads. This optimization is justified by the measurement that the number of spurious UFFDIO_WAKE accounts for 5% and 10% of the total userfaults for heavy workloads, so it's worth optimizing those away. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 65 +++++++++++++++++++++++++++++++++++++------------------- 1 file changed, 43 insertions(+), 22 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 1f2ddaaf3c03..0877222dfa47 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -52,6 +52,10 @@ struct userfaultfd_ctx { struct userfaultfd_wait_queue { struct uffd_msg msg; wait_queue_t wq; + /* + * Only relevant when queued in fault_wqh and only used by the + * read operation to avoid reading the same userfault twice. + */ bool pending; struct userfaultfd_ctx *ctx; }; @@ -71,9 +75,6 @@ static int userfaultfd_wake_function(wait_queue_t *wq, unsigned mode, uwq = container_of(wq, struct userfaultfd_wait_queue, wq); ret = 0; - /* don't wake the pending ones to avoid reads to block */ - if (uwq->pending && !ACCESS_ONCE(uwq->ctx->released)) - goto out; /* len == 0 means wake all */ start = range->start; len = range->len; @@ -196,12 +197,14 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, struct mm_struct *mm = vma->vm_mm; struct userfaultfd_ctx *ctx; struct userfaultfd_wait_queue uwq; + int ret; BUG_ON(!rwsem_is_locked(&mm->mmap_sem)); + ret = VM_FAULT_SIGBUS; ctx = vma->vm_userfaultfd_ctx.ctx; if (!ctx) - return VM_FAULT_SIGBUS; + goto out; BUG_ON(ctx->mm != mm); @@ -214,7 +217,7 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, * caller of handle_userfault to release the mmap_sem. */ if (unlikely(ACCESS_ONCE(ctx->released))) - return VM_FAULT_SIGBUS; + goto out; /* * Check that we can return VM_FAULT_RETRY. @@ -240,15 +243,16 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, dump_stack(); } #endif - return VM_FAULT_SIGBUS; + goto out; } /* * Handle nowait, not much to do other than tell it to retry * and wait. */ + ret = VM_FAULT_RETRY; if (flags & FAULT_FLAG_RETRY_NOWAIT) - return VM_FAULT_RETRY; + goto out; /* take the reference before dropping the mmap_sem */ userfaultfd_ctx_get(ctx); @@ -268,21 +272,23 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, * through poll/read(). */ __add_wait_queue(&ctx->fault_wqh, &uwq.wq); - for (;;) { - set_current_state(TASK_KILLABLE); - if (!uwq.pending || ACCESS_ONCE(ctx->released) || - fatal_signal_pending(current)) - break; - spin_unlock(&ctx->fault_wqh.lock); + set_current_state(TASK_KILLABLE); + spin_unlock(&ctx->fault_wqh.lock); + if (likely(!ACCESS_ONCE(ctx->released) && + !fatal_signal_pending(current))) { wake_up_poll(&ctx->fd_wqh, POLLIN); schedule(); + ret |= VM_FAULT_MAJOR; + } + __set_current_state(TASK_RUNNING); + /* see finish_wait() comment for why list_empty_careful() */ + if (!list_empty_careful(&uwq.wq.task_list)) { spin_lock(&ctx->fault_wqh.lock); + list_del_init(&uwq.wq.task_list); + spin_unlock(&ctx->fault_wqh.lock); } - __remove_wait_queue(&ctx->fault_wqh, &uwq.wq); - __set_current_state(TASK_RUNNING); - spin_unlock(&ctx->fault_wqh.lock); /* * ctx may go away after this if the userfault pseudo fd is @@ -290,7 +296,8 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, */ userfaultfd_ctx_put(ctx); - return VM_FAULT_RETRY; +out: + return ret; } static int userfaultfd_release(struct inode *inode, struct file *file) @@ -404,6 +411,12 @@ static unsigned int userfaultfd_poll(struct file *file, poll_table *wait) case UFFD_STATE_WAIT_API: return POLLERR; case UFFD_STATE_RUNNING: + /* + * poll() never guarantees that read won't block. + * userfaults can be waken before they're read(). + */ + if (unlikely(!(file->f_flags & O_NONBLOCK))) + return POLLERR; spin_lock(&ctx->fault_wqh.lock); ret = find_userfault(ctx, NULL); spin_unlock(&ctx->fault_wqh.lock); @@ -834,11 +847,19 @@ out: } /* - * This is mostly needed to re-wakeup those userfaults that were still - * pending when userland wake them up the first time. We don't wake - * the pending one to avoid blocking reads to block, or non blocking - * read to return -EAGAIN, if used with POLLIN, to avoid userland - * doubts on why POLLIN wasn't reliable. + * userfaultfd_wake is needed in case an userfault is in flight by the + * time a UFFDIO_COPY (or other ioctl variants) completes. The page + * may be well get mapped and the page fault if repeated wouldn't lead + * to a userfault anymore, but before scheduling in TASK_KILLABLE mode + * handle_userfault() doesn't recheck the pagetables and it doesn't + * serialize against UFFDO_COPY (or other ioctl variants). Ultimately + * the knowledge of which pages are mapped is left to userland who is + * responsible for handling the race between read() userfaults and + * background UFFDIO_COPY (or other ioctl variants), if done by + * separate concurrent threads. + * + * userfaultfd_wake may be used in combination with the + * UFFDIO_*_MODE_DONTWAKE to wakeup userfaults in batches. */ static int userfaultfd_wake(struct userfaultfd_ctx *ctx, unsigned long arg) -- cgit v1.2.1 From 15b726ef048b31a24b3fefb6863083a25fe34800 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:44 -0700 Subject: userfaultfd: optimize read() and poll() to be O(1) This makes read O(1) and poll that was already O(1) becomes lockless. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 185 +++++++++++++++++++++++++++++++++---------------------- 1 file changed, 111 insertions(+), 74 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 0877222dfa47..232cbf37c59f 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -35,7 +35,9 @@ enum userfaultfd_state { struct userfaultfd_ctx { /* pseudo fd refcounting */ atomic_t refcount; - /* waitqueue head for the userfaultfd page faults */ + /* waitqueue head for the pending (i.e. not read) userfaults */ + wait_queue_head_t fault_pending_wqh; + /* waitqueue head for the userfaults */ wait_queue_head_t fault_wqh; /* waitqueue head for the pseudo fd to wakeup poll/read */ wait_queue_head_t fd_wqh; @@ -52,11 +54,6 @@ struct userfaultfd_ctx { struct userfaultfd_wait_queue { struct uffd_msg msg; wait_queue_t wq; - /* - * Only relevant when queued in fault_wqh and only used by the - * read operation to avoid reading the same userfault twice. - */ - bool pending; struct userfaultfd_ctx *ctx; }; @@ -263,17 +260,21 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, init_waitqueue_func_entry(&uwq.wq, userfaultfd_wake_function); uwq.wq.private = current; uwq.msg = userfault_msg(address, flags, reason); - uwq.pending = true; uwq.ctx = ctx; - spin_lock(&ctx->fault_wqh.lock); + spin_lock(&ctx->fault_pending_wqh.lock); /* * After the __add_wait_queue the uwq is visible to userland * through poll/read(). */ - __add_wait_queue(&ctx->fault_wqh, &uwq.wq); + __add_wait_queue(&ctx->fault_pending_wqh, &uwq.wq); + /* + * The smp_mb() after __set_current_state prevents the reads + * following the spin_unlock to happen before the list_add in + * __add_wait_queue. + */ set_current_state(TASK_KILLABLE); - spin_unlock(&ctx->fault_wqh.lock); + spin_unlock(&ctx->fault_pending_wqh.lock); if (likely(!ACCESS_ONCE(ctx->released) && !fatal_signal_pending(current))) { @@ -283,11 +284,28 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, } __set_current_state(TASK_RUNNING); - /* see finish_wait() comment for why list_empty_careful() */ + + /* + * Here we race with the list_del; list_add in + * userfaultfd_ctx_read(), however because we don't ever run + * list_del_init() to refile across the two lists, the prev + * and next pointers will never point to self. list_add also + * would never let any of the two pointers to point to + * self. So list_empty_careful won't risk to see both pointers + * pointing to self at any time during the list refile. The + * only case where list_del_init() is called is the full + * removal in the wake function and there we don't re-list_add + * and it's fine not to block on the spinlock. The uwq on this + * kernel stack can be released after the list_del_init. + */ if (!list_empty_careful(&uwq.wq.task_list)) { - spin_lock(&ctx->fault_wqh.lock); - list_del_init(&uwq.wq.task_list); - spin_unlock(&ctx->fault_wqh.lock); + spin_lock(&ctx->fault_pending_wqh.lock); + /* + * No need of list_del_init(), the uwq on the stack + * will be freed shortly anyway. + */ + list_del(&uwq.wq.task_list); + spin_unlock(&ctx->fault_pending_wqh.lock); } /* @@ -345,59 +363,38 @@ static int userfaultfd_release(struct inode *inode, struct file *file) up_write(&mm->mmap_sem); /* - * After no new page faults can wait on this fault_wqh, flush + * After no new page faults can wait on this fault_*wqh, flush * the last page faults that may have been already waiting on - * the fault_wqh. + * the fault_*wqh. */ - spin_lock(&ctx->fault_wqh.lock); + spin_lock(&ctx->fault_pending_wqh.lock); + __wake_up_locked_key(&ctx->fault_pending_wqh, TASK_NORMAL, 0, &range); __wake_up_locked_key(&ctx->fault_wqh, TASK_NORMAL, 0, &range); - spin_unlock(&ctx->fault_wqh.lock); + spin_unlock(&ctx->fault_pending_wqh.lock); wake_up_poll(&ctx->fd_wqh, POLLHUP); userfaultfd_ctx_put(ctx); return 0; } -/* fault_wqh.lock must be hold by the caller */ -static inline unsigned int find_userfault(struct userfaultfd_ctx *ctx, - struct userfaultfd_wait_queue **uwq) +/* fault_pending_wqh.lock must be hold by the caller */ +static inline struct userfaultfd_wait_queue *find_userfault( + struct userfaultfd_ctx *ctx) { wait_queue_t *wq; - struct userfaultfd_wait_queue *_uwq; - unsigned int ret = 0; - - VM_BUG_ON(!spin_is_locked(&ctx->fault_wqh.lock)); + struct userfaultfd_wait_queue *uwq; - list_for_each_entry(wq, &ctx->fault_wqh.task_list, task_list) { - _uwq = container_of(wq, struct userfaultfd_wait_queue, wq); - if (_uwq->pending) { - ret = POLLIN; - if (!uwq) - /* - * If there's at least a pending and - * we don't care which one it is, - * break immediately and leverage the - * efficiency of the LIFO walk. - */ - break; - /* - * If we need to find which one was pending we - * keep walking until we find the first not - * pending one, so we read() them in FIFO order. - */ - *uwq = _uwq; - } else - /* - * break the loop at the first not pending - * one, there cannot be pending userfaults - * after the first not pending one, because - * all new pending ones are inserted at the - * head and we walk it in LIFO. - */ - break; - } + VM_BUG_ON(!spin_is_locked(&ctx->fault_pending_wqh.lock)); - return ret; + uwq = NULL; + if (!waitqueue_active(&ctx->fault_pending_wqh)) + goto out; + /* walk in reverse to provide FIFO behavior to read userfaults */ + wq = list_last_entry(&ctx->fault_pending_wqh.task_list, + typeof(*wq), task_list); + uwq = container_of(wq, struct userfaultfd_wait_queue, wq); +out: + return uwq; } static unsigned int userfaultfd_poll(struct file *file, poll_table *wait) @@ -417,9 +414,20 @@ static unsigned int userfaultfd_poll(struct file *file, poll_table *wait) */ if (unlikely(!(file->f_flags & O_NONBLOCK))) return POLLERR; - spin_lock(&ctx->fault_wqh.lock); - ret = find_userfault(ctx, NULL); - spin_unlock(&ctx->fault_wqh.lock); + /* + * lockless access to see if there are pending faults + * __pollwait last action is the add_wait_queue but + * the spin_unlock would allow the waitqueue_active to + * pass above the actual list_add inside + * add_wait_queue critical section. So use a full + * memory barrier to serialize the list_add write of + * add_wait_queue() with the waitqueue_active read + * below. + */ + ret = 0; + smp_mb(); + if (waitqueue_active(&ctx->fault_pending_wqh)) + ret = POLLIN; return ret; default: BUG(); @@ -431,27 +439,47 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, { ssize_t ret; DECLARE_WAITQUEUE(wait, current); - struct userfaultfd_wait_queue *uwq = NULL; + struct userfaultfd_wait_queue *uwq; - /* always take the fd_wqh lock before the fault_wqh lock */ + /* always take the fd_wqh lock before the fault_pending_wqh lock */ spin_lock(&ctx->fd_wqh.lock); __add_wait_queue(&ctx->fd_wqh, &wait); for (;;) { set_current_state(TASK_INTERRUPTIBLE); - spin_lock(&ctx->fault_wqh.lock); - if (find_userfault(ctx, &uwq)) { + spin_lock(&ctx->fault_pending_wqh.lock); + uwq = find_userfault(ctx); + if (uwq) { /* - * The fault_wqh.lock prevents the uwq to - * disappear from under us. + * The fault_pending_wqh.lock prevents the uwq + * to disappear from under us. + * + * Refile this userfault from + * fault_pending_wqh to fault_wqh, it's not + * pending anymore after we read it. + * + * Use list_del() by hand (as + * userfaultfd_wake_function also uses + * list_del_init() by hand) to be sure nobody + * changes __remove_wait_queue() to use + * list_del_init() in turn breaking the + * !list_empty_careful() check in + * handle_userfault(). The uwq->wq.task_list + * must never be empty at any time during the + * refile, or the waitqueue could disappear + * from under us. The "wait_queue_head_t" + * parameter of __remove_wait_queue() is unused + * anyway. */ - uwq->pending = false; + list_del(&uwq->wq.task_list); + __add_wait_queue(&ctx->fault_wqh, &uwq->wq); + /* careful to always initialize msg if ret == 0 */ *msg = uwq->msg; - spin_unlock(&ctx->fault_wqh.lock); + spin_unlock(&ctx->fault_pending_wqh.lock); ret = 0; break; } - spin_unlock(&ctx->fault_wqh.lock); + spin_unlock(&ctx->fault_pending_wqh.lock); if (signal_pending(current)) { ret = -ERESTARTSYS; break; @@ -510,10 +538,14 @@ static void __wake_userfault(struct userfaultfd_ctx *ctx, start = range->start; end = range->start + range->len; - spin_lock(&ctx->fault_wqh.lock); + spin_lock(&ctx->fault_pending_wqh.lock); /* wake all in the range and autoremove */ - __wake_up_locked_key(&ctx->fault_wqh, TASK_NORMAL, 0, range); - spin_unlock(&ctx->fault_wqh.lock); + if (waitqueue_active(&ctx->fault_pending_wqh)) + __wake_up_locked_key(&ctx->fault_pending_wqh, TASK_NORMAL, 0, + range); + if (waitqueue_active(&ctx->fault_wqh)) + __wake_up_locked_key(&ctx->fault_wqh, TASK_NORMAL, 0, range); + spin_unlock(&ctx->fault_pending_wqh.lock); } static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx, @@ -534,7 +566,8 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx, * userfaults yet. So we take the spinlock only when we're * sure we've userfaults to wake. */ - if (waitqueue_active(&ctx->fault_wqh)) + if (waitqueue_active(&ctx->fault_pending_wqh) || + waitqueue_active(&ctx->fault_wqh)) __wake_userfault(ctx, range); } @@ -960,14 +993,17 @@ static void userfaultfd_show_fdinfo(struct seq_file *m, struct file *f) struct userfaultfd_wait_queue *uwq; unsigned long pending = 0, total = 0; - spin_lock(&ctx->fault_wqh.lock); + spin_lock(&ctx->fault_pending_wqh.lock); + list_for_each_entry(wq, &ctx->fault_pending_wqh.task_list, task_list) { + uwq = container_of(wq, struct userfaultfd_wait_queue, wq); + pending++; + total++; + } list_for_each_entry(wq, &ctx->fault_wqh.task_list, task_list) { uwq = container_of(wq, struct userfaultfd_wait_queue, wq); - if (uwq->pending) - pending++; total++; } - spin_unlock(&ctx->fault_wqh.lock); + spin_unlock(&ctx->fault_pending_wqh.lock); /* * If more protocols will be added, there will be all shown @@ -1027,6 +1063,7 @@ static struct file *userfaultfd_file_create(int flags) goto out; atomic_set(&ctx->refcount, 1); + init_waitqueue_head(&ctx->fault_pending_wqh); init_waitqueue_head(&ctx->fault_wqh); init_waitqueue_head(&ctx->fd_wqh); ctx->flags = flags; -- cgit v1.2.1 From 3004ec9cabf49f43fae2b2bd1855a4720f1def7a Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:48 -0700 Subject: userfaultfd: allocate the userfaultfd_ctx cacheline aligned Use proper slab to guarantee alignment. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 39 +++++++++++++++++++++++++++++++-------- 1 file changed, 31 insertions(+), 8 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 232cbf37c59f..8977a4e8a7f8 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -27,20 +27,26 @@ #include #include +static struct kmem_cache *userfaultfd_ctx_cachep __read_mostly; + enum userfaultfd_state { UFFD_STATE_WAIT_API, UFFD_STATE_RUNNING, }; +/* + * Start with fault_pending_wqh and fault_wqh so they're more likely + * to be in the same cacheline. + */ struct userfaultfd_ctx { - /* pseudo fd refcounting */ - atomic_t refcount; /* waitqueue head for the pending (i.e. not read) userfaults */ wait_queue_head_t fault_pending_wqh; /* waitqueue head for the userfaults */ wait_queue_head_t fault_wqh; /* waitqueue head for the pseudo fd to wakeup poll/read */ wait_queue_head_t fd_wqh; + /* pseudo fd refcounting */ + atomic_t refcount; /* userfaultfd syscall flags */ unsigned int flags; /* state machine */ @@ -130,7 +136,7 @@ static void userfaultfd_ctx_put(struct userfaultfd_ctx *ctx) VM_BUG_ON(spin_is_locked(&ctx->fd_wqh.lock)); VM_BUG_ON(waitqueue_active(&ctx->fd_wqh)); mmput(ctx->mm); - kfree(ctx); + kmem_cache_free(userfaultfd_ctx_cachep, ctx); } } @@ -1028,6 +1034,15 @@ static const struct file_operations userfaultfd_fops = { .llseek = noop_llseek, }; +static void init_once_userfaultfd_ctx(void *mem) +{ + struct userfaultfd_ctx *ctx = (struct userfaultfd_ctx *) mem; + + init_waitqueue_head(&ctx->fault_pending_wqh); + init_waitqueue_head(&ctx->fault_wqh); + init_waitqueue_head(&ctx->fd_wqh); +} + /** * userfaultfd_file_create - Creates an userfaultfd file pointer. * @flags: Flags for the userfaultfd file. @@ -1058,14 +1073,11 @@ static struct file *userfaultfd_file_create(int flags) goto out; file = ERR_PTR(-ENOMEM); - ctx = kmalloc(sizeof(*ctx), GFP_KERNEL); + ctx = kmem_cache_alloc(userfaultfd_ctx_cachep, GFP_KERNEL); if (!ctx) goto out; atomic_set(&ctx->refcount, 1); - init_waitqueue_head(&ctx->fault_pending_wqh); - init_waitqueue_head(&ctx->fault_wqh); - init_waitqueue_head(&ctx->fd_wqh); ctx->flags = flags; ctx->state = UFFD_STATE_WAIT_API; ctx->released = false; @@ -1076,7 +1088,7 @@ static struct file *userfaultfd_file_create(int flags) file = anon_inode_getfile("[userfaultfd]", &userfaultfd_fops, ctx, O_RDWR | (flags & UFFD_SHARED_FCNTL_FLAGS)); if (IS_ERR(file)) - kfree(ctx); + kmem_cache_free(userfaultfd_ctx_cachep, ctx); out: return file; } @@ -1105,3 +1117,14 @@ err_put_unused_fd: return error; } + +static int __init userfaultfd_init(void) +{ + userfaultfd_ctx_cachep = kmem_cache_create("userfaultfd_ctx_cache", + sizeof(struct userfaultfd_ctx), + 0, + SLAB_HWCACHE_ALIGN|SLAB_PANIC, + init_once_userfaultfd_ctx); + return 0; +} +__initcall(userfaultfd_init); -- cgit v1.2.1 From 8d2afd96c20316d112e04d935d9e09150e988397 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:51 -0700 Subject: userfaultfd: solve the race between UFFDIO_COPY|ZEROPAGE and read Solve in-kernel the race between UFFDIO_COPY|ZEROPAGE and userfaultfd_read if they are run on different threads simultaneously. Until now qemu solved the race in userland: the race was explicitly and intentionally left for userland to solve. However we can also solve it in kernel. Requiring all users to solve this race if they use two threads (one for the background transfer and one for the userfault reads) isn't very attractive from an API prospective, furthermore this allows to remove a whole bunch of mutex and bitmap code from qemu, making it faster. The cost of __get_user_pages_fast should be insignificant considering it scales perfectly and the pagetables are already hot in the CPU cache, compared to the overhead in userland to maintain those structures. Applying this patch is backwards compatible with respect to the userfaultfd userland API, however reverting this change wouldn't be backwards compatible anymore. Without this patch qemu in the background transfer thread, has to read the old state, and do UFFDIO_WAKE if old_state is missing but it become REQUESTED by the time it tries to set it to RECEIVED (signaling the other side received an userfault). vcpu background_thr userfault_thr ----- ----- ----- vcpu0 handle_mm_fault() postcopy_place_page read old_state -> MISSING UFFDIO_COPY 0x7fb76a139000 (no wakeup, still pending) vcpu0 fault at 0x7fb76a139000 enters handle_userfault poll() is kicked poll() -> POLLIN read() -> 0x7fb76a139000 postcopy_pmi_change_state(MISSING, REQUESTED) -> REQUESTED tmp_state = postcopy_pmi_change_state(old_state, RECEIVED) -> REQUESTED /* check that no userfault raced with UFFDIO_COPY */ if (old_state == MISSING && tmp_state == REQUESTED) UFFDIO_WAKE from background thread And a second case where a UFFDIO_WAKE would be needed is in the userfault thread: vcpu background_thr userfault_thr ----- ----- ----- vcpu0 handle_mm_fault() postcopy_place_page read old_state -> MISSING UFFDIO_COPY 0x7fb76a139000 (no wakeup, still pending) tmp_state = postcopy_pmi_change_state(old_state, RECEIVED) -> RECEIVED vcpu0 fault at 0x7fb76a139000 enters handle_userfault poll() is kicked poll() -> POLLIN read() -> 0x7fb76a139000 if (postcopy_pmi_change_state(MISSING, REQUESTED) == RECEIVED) UFFDIO_WAKE from userfault thread This patch removes the need of both UFFDIO_WAKE and of the associated per-page tristate as well. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 81 +++++++++++++++++++++++++++++++++++++++++++++----------- 1 file changed, 66 insertions(+), 15 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 8977a4e8a7f8..febbd2b165df 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -179,6 +179,67 @@ static inline struct uffd_msg userfault_msg(unsigned long address, return msg; } +/* + * Verify the pagetables are still not ok after having reigstered into + * the fault_pending_wqh to avoid userland having to UFFDIO_WAKE any + * userfault that has already been resolved, if userfaultfd_read and + * UFFDIO_COPY|ZEROPAGE are being run simultaneously on two different + * threads. + */ +static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx, + unsigned long address, + unsigned long flags, + unsigned long reason) +{ + struct mm_struct *mm = ctx->mm; + pgd_t *pgd; + pud_t *pud; + pmd_t *pmd, _pmd; + pte_t *pte; + bool ret = true; + + VM_BUG_ON(!rwsem_is_locked(&mm->mmap_sem)); + + pgd = pgd_offset(mm, address); + if (!pgd_present(*pgd)) + goto out; + pud = pud_offset(pgd, address); + if (!pud_present(*pud)) + goto out; + pmd = pmd_offset(pud, address); + /* + * READ_ONCE must function as a barrier with narrower scope + * and it must be equivalent to: + * _pmd = *pmd; barrier(); + * + * This is to deal with the instability (as in + * pmd_trans_unstable) of the pmd. + */ + _pmd = READ_ONCE(*pmd); + if (!pmd_present(_pmd)) + goto out; + + ret = false; + if (pmd_trans_huge(_pmd)) + goto out; + + /* + * the pmd is stable (as in !pmd_trans_unstable) so we can re-read it + * and use the standard pte_offset_map() instead of parsing _pmd. + */ + pte = pte_offset_map(pmd, address); + /* + * Lockless access: we're in a wait_event so it's ok if it + * changes under us. + */ + if (pte_none(*pte)) + ret = true; + pte_unmap(pte); + +out: + return ret; +} + /* * The locking rules involved in returning VM_FAULT_RETRY depending on * FAULT_FLAG_ALLOW_RETRY, FAULT_FLAG_RETRY_NOWAIT and @@ -201,6 +262,7 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, struct userfaultfd_ctx *ctx; struct userfaultfd_wait_queue uwq; int ret; + bool must_wait; BUG_ON(!rwsem_is_locked(&mm->mmap_sem)); @@ -260,9 +322,6 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, /* take the reference before dropping the mmap_sem */ userfaultfd_ctx_get(ctx); - /* be gentle and immediately relinquish the mmap_sem */ - up_read(&mm->mmap_sem); - init_waitqueue_func_entry(&uwq.wq, userfaultfd_wake_function); uwq.wq.private = current; uwq.msg = userfault_msg(address, flags, reason); @@ -282,7 +341,10 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, set_current_state(TASK_KILLABLE); spin_unlock(&ctx->fault_pending_wqh.lock); - if (likely(!ACCESS_ONCE(ctx->released) && + must_wait = userfaultfd_must_wait(ctx, address, flags, reason); + up_read(&mm->mmap_sem); + + if (likely(must_wait && !ACCESS_ONCE(ctx->released) && !fatal_signal_pending(current))) { wake_up_poll(&ctx->fd_wqh, POLLIN); schedule(); @@ -886,17 +948,6 @@ out: } /* - * userfaultfd_wake is needed in case an userfault is in flight by the - * time a UFFDIO_COPY (or other ioctl variants) completes. The page - * may be well get mapped and the page fault if repeated wouldn't lead - * to a userfault anymore, but before scheduling in TASK_KILLABLE mode - * handle_userfault() doesn't recheck the pagetables and it doesn't - * serialize against UFFDO_COPY (or other ioctl variants). Ultimately - * the knowledge of which pages are mapped is left to userland who is - * responsible for handling the race between read() userfaults and - * background UFFDIO_COPY (or other ioctl variants), if done by - * separate concurrent threads. - * * userfaultfd_wake may be used in combination with the * UFFDIO_*_MODE_DONTWAKE to wakeup userfaults in batches. */ -- cgit v1.2.1 From a14c151e567cb2c3e62611da808a8bdab86fdee5 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:46:54 -0700 Subject: userfaultfd: buildsystem activation This allows to select the userfaultfd during configuration to build it. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/Makefile | 1 + 1 file changed, 1 insertion(+) (limited to 'fs') diff --git a/fs/Makefile b/fs/Makefile index 09e051fefc5b..f79cf4043e60 100644 --- a/fs/Makefile +++ b/fs/Makefile @@ -27,6 +27,7 @@ obj-$(CONFIG_ANON_INODES) += anon_inodes.o obj-$(CONFIG_SIGNALFD) += signalfd.o obj-$(CONFIG_TIMERFD) += timerfd.o obj-$(CONFIG_EVENTFD) += eventfd.o +obj-$(CONFIG_USERFAULTFD) += userfaultfd.o obj-$(CONFIG_AIO) += aio.o obj-$(CONFIG_FS_DAX) += dax.o obj-$(CONFIG_FILE_LOCKING) += locks.o -- cgit v1.2.1 From ad465cae96b456b48d26c96f27a0577ba443472a Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:47:11 -0700 Subject: userfaultfd: UFFDIO_COPY and UFFDIO_ZEROPAGE These two ioctl allows to either atomically copy or to map zeropages into the virtual address space. This is used by the thread that opened the userfaultfd to resolve the userfaults. Signed-off-by: Andrea Arcangeli Acked-by: Pavel Emelyanov Cc: Sanidhya Kashyap Cc: zhang.zhanghailiang@huawei.com Cc: "Kirill A. Shutemov" Cc: Andres Lagar-Cavilla Cc: Dave Hansen Cc: Paolo Bonzini Cc: Rik van Riel Cc: Mel Gorman Cc: Andy Lutomirski Cc: Hugh Dickins Cc: Peter Feiner Cc: "Dr. David Alan Gilbert" Cc: Johannes Weiner Cc: "Huangpeng (Peter)" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 96 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index febbd2b165df..5f11678907d5 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -983,6 +983,96 @@ out: return ret; } +static int userfaultfd_copy(struct userfaultfd_ctx *ctx, + unsigned long arg) +{ + __s64 ret; + struct uffdio_copy uffdio_copy; + struct uffdio_copy __user *user_uffdio_copy; + struct userfaultfd_wake_range range; + + user_uffdio_copy = (struct uffdio_copy __user *) arg; + + ret = -EFAULT; + if (copy_from_user(&uffdio_copy, user_uffdio_copy, + /* don't copy "copy" last field */ + sizeof(uffdio_copy)-sizeof(__s64))) + goto out; + + ret = validate_range(ctx->mm, uffdio_copy.dst, uffdio_copy.len); + if (ret) + goto out; + /* + * double check for wraparound just in case. copy_from_user() + * will later check uffdio_copy.src + uffdio_copy.len to fit + * in the userland range. + */ + ret = -EINVAL; + if (uffdio_copy.src + uffdio_copy.len <= uffdio_copy.src) + goto out; + if (uffdio_copy.mode & ~UFFDIO_COPY_MODE_DONTWAKE) + goto out; + + ret = mcopy_atomic(ctx->mm, uffdio_copy.dst, uffdio_copy.src, + uffdio_copy.len); + if (unlikely(put_user(ret, &user_uffdio_copy->copy))) + return -EFAULT; + if (ret < 0) + goto out; + BUG_ON(!ret); + /* len == 0 would wake all */ + range.len = ret; + if (!(uffdio_copy.mode & UFFDIO_COPY_MODE_DONTWAKE)) { + range.start = uffdio_copy.dst; + wake_userfault(ctx, &range); + } + ret = range.len == uffdio_copy.len ? 0 : -EAGAIN; +out: + return ret; +} + +static int userfaultfd_zeropage(struct userfaultfd_ctx *ctx, + unsigned long arg) +{ + __s64 ret; + struct uffdio_zeropage uffdio_zeropage; + struct uffdio_zeropage __user *user_uffdio_zeropage; + struct userfaultfd_wake_range range; + + user_uffdio_zeropage = (struct uffdio_zeropage __user *) arg; + + ret = -EFAULT; + if (copy_from_user(&uffdio_zeropage, user_uffdio_zeropage, + /* don't copy "zeropage" last field */ + sizeof(uffdio_zeropage)-sizeof(__s64))) + goto out; + + ret = validate_range(ctx->mm, uffdio_zeropage.range.start, + uffdio_zeropage.range.len); + if (ret) + goto out; + ret = -EINVAL; + if (uffdio_zeropage.mode & ~UFFDIO_ZEROPAGE_MODE_DONTWAKE) + goto out; + + ret = mfill_zeropage(ctx->mm, uffdio_zeropage.range.start, + uffdio_zeropage.range.len); + if (unlikely(put_user(ret, &user_uffdio_zeropage->zeropage))) + return -EFAULT; + if (ret < 0) + goto out; + /* len == 0 would wake all */ + BUG_ON(!ret); + range.len = ret; + if (!(uffdio_zeropage.mode & UFFDIO_ZEROPAGE_MODE_DONTWAKE)) { + range.start = uffdio_zeropage.range.start; + wake_userfault(ctx, &range); + } + ret = range.len == uffdio_zeropage.range.len ? 0 : -EAGAIN; +out: + return ret; +} + /* * userland asks for a certain API version and we return which bits * and ioctl commands are implemented in this kernel for such API @@ -1038,6 +1128,12 @@ static long userfaultfd_ioctl(struct file *file, unsigned cmd, case UFFDIO_WAKE: ret = userfaultfd_wake(ctx, arg); break; + case UFFDIO_COPY: + ret = userfaultfd_copy(ctx, arg); + break; + case UFFDIO_ZEROPAGE: + ret = userfaultfd_zeropage(ctx, arg); + break; } return ret; } -- cgit v1.2.1 From e6485a47b758cae04a496764a1095961ee3249e4 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:47:15 -0700 Subject: userfaultfd: require UFFDIO_API before other ioctls UFFDIO_API was already forced before read/poll could work. This makes the code more strict to force it also for all other ioctls. All users would already have been required to call UFFDIO_API before invoking other ioctls but this makes it more explicit. This will ensure we can change all ioctls (all but UFFDIO_API/struct uffdio_api) with a bump of uffdio_api.api. There's no actual plan or need to change the API or the ioctl, the current API already should cover fine even the non cooperative usage, but this is just for the longer term future just in case. Signed-off-by: Andrea Arcangeli Cc: Pavel Emelyanov Cc: Dave Hansen Cc: Linus Torvalds Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index 5f11678907d5..af88ef6fffff 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -577,7 +577,6 @@ static ssize_t userfaultfd_read(struct file *file, char __user *buf, if (ctx->state == UFFD_STATE_WAIT_API) return -EINVAL; - BUG_ON(ctx->state != UFFD_STATE_RUNNING); for (;;) { if (count < sizeof(msg)) @@ -1115,6 +1114,9 @@ static long userfaultfd_ioctl(struct file *file, unsigned cmd, int ret = -EINVAL; struct userfaultfd_ctx *ctx = file->private_data; + if (cmd != UFFDIO_API && ctx->state == UFFD_STATE_WAIT_API) + return -EINVAL; + switch(cmd) { case UFFDIO_API: ret = userfaultfd_api(ctx, arg); -- cgit v1.2.1 From dfa37dc3fc1f6f81a6900d0e561c02362f4817f6 Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:47:18 -0700 Subject: userfaultfd: allow signals to interrupt a userfault This is only simple to achieve if the userfault is going to return to userland (not to the kernel) because we can avoid returning VM_FAULT_RETRY despite we temporarily released the mmap_sem. The fault would just be retried by userland then. This is safe at least on x86 and powerpc (the two archs with the syscall implemented so far). Hint to verify for which archs this is safe: after handle_mm_fault returns, no access to data structures protected by the mmap_sem must be done by the fault code in arch/*/mm/fault.c until up_read(&mm->mmap_sem) is called. This has two main benefits: signals can run with lower latency in production (signals aren't blocked by userfaults and userfaults are immediately repeated after signal processing) and gdb can then trivially debug the threads blocked in this kind of userfaults coming directly from userland. On a side note: while gdb has a need to get signal processed, coredumps always worked perfectly with userfaults, no matter if the userfault is triggered by GUP a kernel copy_user or directly from userland. Signed-off-by: Andrea Arcangeli Cc: Pavel Emelyanov Cc: Dave Hansen Cc: Linus Torvalds Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 35 ++++++++++++++++++++++++++++++++--- 1 file changed, 32 insertions(+), 3 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index af88ef6fffff..a14d63e945f4 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -262,7 +262,7 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, struct userfaultfd_ctx *ctx; struct userfaultfd_wait_queue uwq; int ret; - bool must_wait; + bool must_wait, return_to_userland; BUG_ON(!rwsem_is_locked(&mm->mmap_sem)); @@ -327,6 +327,9 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, uwq.msg = userfault_msg(address, flags, reason); uwq.ctx = ctx; + return_to_userland = (flags & (FAULT_FLAG_USER|FAULT_FLAG_KILLABLE)) == + (FAULT_FLAG_USER|FAULT_FLAG_KILLABLE); + spin_lock(&ctx->fault_pending_wqh.lock); /* * After the __add_wait_queue the uwq is visible to userland @@ -338,14 +341,16 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, * following the spin_unlock to happen before the list_add in * __add_wait_queue. */ - set_current_state(TASK_KILLABLE); + set_current_state(return_to_userland ? TASK_INTERRUPTIBLE : + TASK_KILLABLE); spin_unlock(&ctx->fault_pending_wqh.lock); must_wait = userfaultfd_must_wait(ctx, address, flags, reason); up_read(&mm->mmap_sem); if (likely(must_wait && !ACCESS_ONCE(ctx->released) && - !fatal_signal_pending(current))) { + (return_to_userland ? !signal_pending(current) : + !fatal_signal_pending(current)))) { wake_up_poll(&ctx->fd_wqh, POLLIN); schedule(); ret |= VM_FAULT_MAJOR; @@ -353,6 +358,30 @@ int handle_userfault(struct vm_area_struct *vma, unsigned long address, __set_current_state(TASK_RUNNING); + if (return_to_userland) { + if (signal_pending(current) && + !fatal_signal_pending(current)) { + /* + * If we got a SIGSTOP or SIGCONT and this is + * a normal userland page fault, just let + * userland return so the signal will be + * handled and gdb debugging works. The page + * fault code immediately after we return from + * this function is going to release the + * mmap_sem and it's not depending on it + * (unlike gup would if we were not to return + * VM_FAULT_RETRY). + * + * If a fatal signal is pending we still take + * the streamlined VM_FAULT_RETRY failure path + * and there's no need to retake the mmap_sem + * in such case. + */ + down_read(&mm->mmap_sem); + ret = 0; + } + } + /* * Here we race with the list_del; list_add in * userfaultfd_ctx_read(), however because we don't ever run -- cgit v1.2.1 From 2c5b7e1be74ff0175dedbbd325abe9f0dbbb09ae Mon Sep 17 00:00:00 2001 From: Andrea Arcangeli Date: Fri, 4 Sep 2015 15:47:23 -0700 Subject: userfaultfd: avoid missing wakeups during refile in userfaultfd_read During the refile in userfaultfd_read both waitqueues could look empty to the lockless wake_userfault(). Use a seqcount to prevent this false negative that could leave an userfault blocked. Signed-off-by: Andrea Arcangeli Cc: Pavel Emelyanov Cc: Dave Hansen Cc: Linus Torvalds Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/userfaultfd.c | 26 ++++++++++++++++++++++++-- 1 file changed, 24 insertions(+), 2 deletions(-) (limited to 'fs') diff --git a/fs/userfaultfd.c b/fs/userfaultfd.c index a14d63e945f4..634e676072cb 100644 --- a/fs/userfaultfd.c +++ b/fs/userfaultfd.c @@ -45,6 +45,8 @@ struct userfaultfd_ctx { wait_queue_head_t fault_wqh; /* waitqueue head for the pseudo fd to wakeup poll/read */ wait_queue_head_t fd_wqh; + /* a refile sequence protected by fault_pending_wqh lock */ + struct seqcount refile_seq; /* pseudo fd refcounting */ atomic_t refcount; /* userfaultfd syscall flags */ @@ -546,6 +548,15 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, spin_lock(&ctx->fault_pending_wqh.lock); uwq = find_userfault(ctx); if (uwq) { + /* + * Use a seqcount to repeat the lockless check + * in wake_userfault() to avoid missing + * wakeups because during the refile both + * waitqueue could become empty if this is the + * only userfault. + */ + write_seqcount_begin(&ctx->refile_seq); + /* * The fault_pending_wqh.lock prevents the uwq * to disappear from under us. @@ -570,6 +581,8 @@ static ssize_t userfaultfd_ctx_read(struct userfaultfd_ctx *ctx, int no_wait, list_del(&uwq->wq.task_list); __add_wait_queue(&ctx->fault_wqh, &uwq->wq); + write_seqcount_end(&ctx->refile_seq); + /* careful to always initialize msg if ret == 0 */ *msg = uwq->msg; spin_unlock(&ctx->fault_pending_wqh.lock); @@ -647,6 +660,9 @@ static void __wake_userfault(struct userfaultfd_ctx *ctx, static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx, struct userfaultfd_wake_range *range) { + unsigned seq; + bool need_wakeup; + /* * To be sure waitqueue_active() is not reordered by the CPU * before the pagetable update, use an explicit SMP memory @@ -662,8 +678,13 @@ static __always_inline void wake_userfault(struct userfaultfd_ctx *ctx, * userfaults yet. So we take the spinlock only when we're * sure we've userfaults to wake. */ - if (waitqueue_active(&ctx->fault_pending_wqh) || - waitqueue_active(&ctx->fault_wqh)) + do { + seq = read_seqcount_begin(&ctx->refile_seq); + need_wakeup = waitqueue_active(&ctx->fault_pending_wqh) || + waitqueue_active(&ctx->fault_wqh); + cond_resched(); + } while (read_seqcount_retry(&ctx->refile_seq, seq)); + if (need_wakeup) __wake_userfault(ctx, range); } @@ -1219,6 +1240,7 @@ static void init_once_userfaultfd_ctx(void *mem) init_waitqueue_head(&ctx->fault_pending_wqh); init_waitqueue_head(&ctx->fault_wqh); init_waitqueue_head(&ctx->fd_wqh); + seqcount_init(&ctx->refile_seq); } /** -- cgit v1.2.1 From 5477e70a6420a6b7ca96c8e21413ee1c96a84260 Mon Sep 17 00:00:00 2001 From: Oleg Nesterov Date: Fri, 4 Sep 2015 15:48:04 -0700 Subject: mm: move ->mremap() from file_operations to vm_operations_struct vma->vm_ops->mremap() looks more natural and clean in move_vma(), and this way ->mremap() can have more users. Say, vdso. While at it, s/aio_ring_remap/aio_ring_mremap/. Note: this is the minimal change before ->mremap() finds another user in file_operations; this method should have more arguments, and it can be used to kill arch_remap(). Signed-off-by: Oleg Nesterov Acked-by: Pavel Emelyanov Acked-by: Kirill A. Shutemov Cc: David Rientjes Cc: Benjamin LaHaise Cc: Hugh Dickins Cc: Jeff Moyer Cc: Laurent Dufour Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- fs/aio.c | 27 ++++++++++++++++++--------- 1 file changed, 18 insertions(+), 9 deletions(-) (limited to 'fs') diff --git a/fs/aio.c b/fs/aio.c index 480440f4701f..155f84253f33 100644 --- a/fs/aio.c +++ b/fs/aio.c @@ -308,15 +308,9 @@ static void aio_free_ring(struct kioctx *ctx) } } -static int aio_ring_mmap(struct file *file, struct vm_area_struct *vma) -{ - vma->vm_flags |= VM_DONTEXPAND; - vma->vm_ops = &generic_file_vm_ops; - return 0; -} - -static int aio_ring_remap(struct file *file, struct vm_area_struct *vma) +static int aio_ring_mremap(struct vm_area_struct *vma) { + struct file *file = vma->vm_file; struct mm_struct *mm = vma->vm_mm; struct kioctx_table *table; int i, res = -EINVAL; @@ -342,9 +336,24 @@ static int aio_ring_remap(struct file *file, struct vm_area_struct *vma) return res; } +static const struct vm_operations_struct aio_ring_vm_ops = { + .mremap = aio_ring_mremap, +#if IS_ENABLED(CONFIG_MMU) + .fault = filemap_fault, + .map_pages = filemap_map_pages, + .page_mkwrite = filemap_page_mkwrite, +#endif +}; + +static int aio_ring_mmap(struct file *file, struct vm_area_struct *vma) +{ + vma->vm_flags |= VM_DONTEXPAND; + vma->vm_ops = &aio_ring_vm_ops; + return 0; +} + static const struct file_operations aio_ring_fops = { .mmap = aio_ring_mmap, - .mremap = aio_ring_remap, }; #if IS_ENABLED(CONFIG_MIGRATION) -- cgit v1.2.1