From: Steven Rostedt The only user of FAULT_FLAG_RETRY_NOWAIT also sets the FAULT_FLAG_ALLOW_RETRY flag. This makes the check in the __lock_page_or_retry redundant as it checks the RETRY_NOWAIT just after checking ALLOW_RETRY and then returns if it is set. The FAULT_FLAG_ALLOW_RETRY does not make any other difference in this path. Setting both and then ignoring one is quite confusing, especially since this code has very subtle locking issues when it comes to the mmap_sem. Only set the RETRY_WAIT flag and have that do the necessary work instead of confusing reviewers of this code by setting ALLOW_RETRY and not releasing the mmap_sem. Cc: Linus Torvalds Cc: Hugh Dickins Cc: Rik van Riel Cc: Michel Lespinasse Cc: Avi Kivity Cc: Marcelo Tosatti Cc: Andrew Morton Signed-off-by: Steven Rostedt --- include/linux/mm.h | 4 ++-- mm/filemap.c | 14 +++++++------- mm/memory.c | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/include/linux/mm.h b/include/linux/mm.h index 9670f71..2ec71d9 100644 --- a/include/linux/mm.h +++ b/include/linux/mm.h @@ -151,8 +151,8 @@ extern pgprot_t protection_map[16]; #define FAULT_FLAG_WRITE 0x01 /* Fault was a write access */ #define FAULT_FLAG_NONLINEAR 0x02 /* Fault was via a nonlinear mapping */ #define FAULT_FLAG_MKWRITE 0x04 /* Fault was mkwrite of existing pte */ -#define FAULT_FLAG_ALLOW_RETRY 0x08 /* Retry fault if blocking */ -#define FAULT_FLAG_RETRY_NOWAIT 0x10 /* Don't drop mmap_sem and wait when retrying */ +#define FAULT_FLAG_ALLOW_RETRY 0x08 /* Retry fault if blocking (drops mmap_sem) */ +#define FAULT_FLAG_RETRY_NOWAIT 0x10 /* Wait when retrying (don't drop mmap_sem) */ #define FAULT_FLAG_KILLABLE 0x20 /* The fault task is in SIGKILL killable region */ /* diff --git a/mm/filemap.c b/mm/filemap.c index a8251a8..bc9978f 100644 --- a/mm/filemap.c +++ b/mm/filemap.c @@ -665,14 +665,14 @@ EXPORT_SYMBOL_GPL(__lock_page_killable); int __lock_page_or_retry(struct page *page, struct mm_struct *mm, unsigned int flags) { - if (flags & FAULT_FLAG_ALLOW_RETRY) { - /* - * CAUTION! In this case, mmap_sem is not released - * even though return 0. - */ - if (flags & FAULT_FLAG_RETRY_NOWAIT) - return 0; + /* + * Don't drop mmap_sem if FAULT_FLAG_RETRY_NOWAIT is + * set, even if FAULT_FLAG_ALLOW_RETRY is set. + */ + if (flags & FAULT_FLAG_RETRY_NOWAIT) + return 0; + if (flags & FAULT_FLAG_ALLOW_RETRY) { up_read(&mm->mmap_sem); if (flags & FAULT_FLAG_KILLABLE) wait_on_page_locked_killable(page); diff --git a/mm/memory.c b/mm/memory.c index 40b7531..5371b5e 100644 --- a/mm/memory.c +++ b/mm/memory.c @@ -1742,7 +1742,7 @@ int __get_user_pages(struct task_struct *tsk, struct mm_struct *mm, if (nonblocking) fault_flags |= FAULT_FLAG_ALLOW_RETRY; if (foll_flags & FOLL_NOWAIT) - fault_flags |= (FAULT_FLAG_ALLOW_RETRY | FAULT_FLAG_RETRY_NOWAIT); + fault_flags |= FAULT_FLAG_RETRY_NOWAIT; ret = handle_mm_fault(mm, vma, start, fault_flags); -- 1.7.5.4 -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/