* Re: [PATCH 05/12] mm: trylock_page
2007-09-28 7:42 ` [PATCH 05/12] mm: trylock_page Peter Zijlstra
@ 2007-09-28 3:11 ` Nick Piggin
2007-09-29 15:01 ` Peter Zijlstra
0 siblings, 1 reply; 21+ messages in thread
From: Nick Piggin @ 2007-09-28 3:11 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
On Friday 28 September 2007 17:42, Peter Zijlstra wrote:
> Replace raw TestSetPageLocked() usage with trylock_page()
I have such a thing queued too, for the lock bitops patches for when 2.6.24
opens, Andrew promises me :).
I guess they should be identical, except I don't like doing trylock_page in
place of SetPageLocked, for memory ordering performance and aesthetic
reasons... I've got an init_page_locked (or set_page_locked... I can't
remember, the patch is at home).
Fine idea to lockdep the page lock, anyway. Does it show up any of the
buffered write deadlock possibilities? :)
buffer lock is another notable bit-mutex that might be converted (I have
the patch to do the similar nice !tas->trylock conversion for that too). I
think it is used widely enough by tricky code that it would be useful to
annotate as well.
Unfortunately we can't convert bit_spinlock.h easily, I guess?
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 00/12] various lockdep patches
@ 2007-09-28 7:42 Peter Zijlstra
2007-09-28 7:42 ` [PATCH 01/12] lockdep: syscall exit check Peter Zijlstra
` (12 more replies)
0 siblings, 13 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
The first 3 patches provide a new lockdep check, it verifies we don't hold any
locks when returning to userspace.
lockdep-sys_exit.patch
lockdep-i386-sys_exit.patch
lockdep-x86_64-sys_exit.patch
Could the various arch maintainers that have support for lockdep help out
with placing this hook?
A journal_start annotation:
jbd-lock.patch
Up until this point stuff should be safe to merge (assuming there are no
objections and bugs.. :-)
The rest of the series annotates lock_page():
rework lock_page() API:
trylock_page.patch
lock_page.patch
lockdep-page_lock-hooks.patch
actuall lockdep annotation:
lockdep-depth.patch
lockdep-fs-page.patch
lockdep-page-async.patch
set_page_mapping.patch
lockdep-lock_page.patch
Part of that is re-working the lock_page() API a bit. It introduces
trylock_page(), and removes all the raw *PageLock() usage.
I've compile and boot tested this series on x86_64 (with and without lockdep).
And it completes an LTP run with lockdep enabled.
Andrew, can we start by pusing the lock_page() API rework into -mm?
[ this series is against -linus, but if you're willing I'll do some
patches against -mm ]
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 01/12] lockdep: syscall exit check
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 12:03 ` Heiko Carstens
2007-09-28 7:42 ` [PATCH 02/12] lockdep: i386: connect the sysexit hook Peter Zijlstra
` (11 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-sys_exit.patch --]
[-- Type: text/plain, Size: 1850 bytes --]
Provide a check to validate that we do not hold any locks when switching
back to user-space.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/lockdep.h | 2 ++
kernel/lockdep.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+)
Index: linux-2.6/include/linux/lockdep.h
===================================================================
--- linux-2.6.orig/include/linux/lockdep.h
+++ linux-2.6/include/linux/lockdep.h
@@ -238,6 +238,7 @@ extern void lockdep_info(void);
extern void lockdep_reset(void);
extern void lockdep_reset_lock(struct lockdep_map *lock);
extern void lockdep_free_key_range(void *start, unsigned long size);
+extern void lockdep_sys_exit(void);
extern void lockdep_off(void);
extern void lockdep_on(void);
@@ -317,6 +318,7 @@ static inline void lockdep_on(void)
# define INIT_LOCKDEP
# define lockdep_reset() do { debug_locks = 1; } while (0)
# define lockdep_free_key_range(start, size) do { } while (0)
+# define lockdep_sys_exit() do { } while (0)
/*
* The class key takes no space if lockdep is disabled:
*/
Index: linux-2.6/kernel/lockdep.c
===================================================================
--- linux-2.6.orig/kernel/lockdep.c
+++ linux-2.6/kernel/lockdep.c
@@ -3199,3 +3199,19 @@ void debug_show_held_locks(struct task_s
}
EXPORT_SYMBOL_GPL(debug_show_held_locks);
+
+void lockdep_sys_exit(void)
+{
+ struct task_struct *curr = current;
+
+ if (unlikely(curr->lockdep_depth)) {
+ if (!debug_locks_off())
+ return;
+ printk("\n========================================\n");
+ printk( "[ BUG: lock held at syscall exit time! ]\n");
+ printk( "----------------------------------------\n");
+ printk("%s/%d is leaving the kernel with locks still held!\n",
+ curr->comm, curr->pid);
+ lockdep_print_held_locks(curr);
+ }
+}
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 02/12] lockdep: i386: connect the sysexit hook
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
2007-09-28 7:42 ` [PATCH 01/12] lockdep: syscall exit check Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 03/12] lockdep: x86_64: " Peter Zijlstra
` (10 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-i386-sys_exit.patch --]
[-- Type: text/plain, Size: 1687 bytes --]
Run the lockdep_sys_exit hook after all other C code on the syscall
return path.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
arch/i386/kernel/entry.S | 3 +++
include/asm-i386/irqflags.h | 13 +++++++++++++
2 files changed, 16 insertions(+)
Index: linux-2.6/arch/i386/kernel/entry.S
===================================================================
--- linux-2.6.orig/arch/i386/kernel/entry.S
+++ linux-2.6/arch/i386/kernel/entry.S
@@ -338,6 +338,7 @@ sysenter_past_esp:
jae syscall_badsys
call *sys_call_table(,%eax,4)
movl %eax,PT_EAX(%esp)
+ LOCKDEP_SYS_EXIT
DISABLE_INTERRUPTS(CLBR_ANY)
TRACE_IRQS_OFF
movl TI_flags(%ebp), %ecx
@@ -376,6 +377,7 @@ ENTRY(system_call)
syscall_call:
call *sys_call_table(,%eax,4)
movl %eax,PT_EAX(%esp) # store the return value
+ LOCKDEP_SYS_EXIT
syscall_exit:
DISABLE_INTERRUPTS(CLBR_ANY) # make sure we don't miss an interrupt
# setting need_resched or sigpending
@@ -532,6 +534,7 @@ syscall_exit_work:
movl %esp, %eax
movl $1, %edx
call do_syscall_trace
+ LOCKDEP_SYS_EXIT
jmp resume_userspace
END(syscall_exit_work)
CFI_ENDPROC
Index: linux-2.6/include/asm-i386/irqflags.h
===================================================================
--- linux-2.6.orig/include/asm-i386/irqflags.h
+++ linux-2.6/include/asm-i386/irqflags.h
@@ -160,4 +160,17 @@ static inline int raw_irqs_disabled(void
# define TRACE_IRQS_OFF
#endif
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+# define LOCKDEP_SYS_EXIT \
+ pushl %eax; \
+ pushl %ecx; \
+ pushl %edx; \
+ call lockdep_sys_exit; \
+ popl %edx; \
+ popl %ecx; \
+ popl %eax;
+#else
+# define LOCKDEP_SYS_EXIT
+#endif
+
#endif
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 03/12] lockdep: x86_64: connect the sysexit hook
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
2007-09-28 7:42 ` [PATCH 01/12] lockdep: syscall exit check Peter Zijlstra
2007-09-28 7:42 ` [PATCH 02/12] lockdep: i386: connect the sysexit hook Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 04/12] lockdep: annotate journal_start() Peter Zijlstra
` (9 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-x86_64-sys_exit.patch --]
[-- Type: text/plain, Size: 2205 bytes --]
Run the lockdep_sys_exit hook after all other C code on the syscall
return path.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
arch/x86_64/kernel/entry.S | 3 +++
arch/x86_64/lib/thunk.S | 4 ++++
include/asm-x86_64/irqflags.h | 9 +++++++++
3 files changed, 16 insertions(+)
Index: linux-2.6/arch/x86_64/kernel/entry.S
===================================================================
--- linux-2.6.orig/arch/x86_64/kernel/entry.S
+++ linux-2.6/arch/x86_64/kernel/entry.S
@@ -164,6 +164,7 @@ ENTRY(ret_from_fork)
testl $(_TIF_SYSCALL_TRACE|_TIF_SYSCALL_AUDIT),threadinfo_flags(%rcx)
jnz rff_trace
rff_action:
+ LOCKDEP_SYS_EXIT
RESTORE_REST
testl $3,CS-ARGOFFSET(%rsp) # from kernel_thread?
je int_ret_from_sys_call
@@ -326,6 +327,7 @@ tracesys:
*/
.globl int_ret_from_sys_call
int_ret_from_sys_call:
+ LOCKDEP_SYS_EXIT_REST
cli
TRACE_IRQS_OFF
testl $3,CS-ARGOFFSET(%rsp)
@@ -382,6 +384,7 @@ int_signal:
call do_notify_resume
1: movl $_TIF_NEED_RESCHED,%edi
int_restore_rest:
+ LOCKDEP_SYS_EXIT
RESTORE_REST
cli
TRACE_IRQS_OFF
Index: linux-2.6/arch/x86_64/lib/thunk.S
===================================================================
--- linux-2.6.orig/arch/x86_64/lib/thunk.S
+++ linux-2.6/arch/x86_64/lib/thunk.S
@@ -50,6 +50,10 @@
thunk trace_hardirqs_on_thunk,trace_hardirqs_on
thunk trace_hardirqs_off_thunk,trace_hardirqs_off
#endif
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ thunk lockdep_sys_exit_thunk,lockdep_sys_exit
+#endif
/* SAVE_ARGS below is used only for the .cfi directives it contains. */
CFI_STARTPROC
Index: linux-2.6/include/asm-x86_64/irqflags.h
===================================================================
--- linux-2.6.orig/include/asm-x86_64/irqflags.h
+++ linux-2.6/include/asm-x86_64/irqflags.h
@@ -137,6 +137,15 @@ static inline void halt(void)
# define TRACE_IRQS_ON
# define TRACE_IRQS_OFF
# endif
+# ifdef CONFIG_DEBUG_LOCK_ALLOC
+# define LOCKDEP_SYS_EXIT call lockdep_sys_exit_thunk
+# define LOCKDEP_SYS_EXIT_REST SAVE_REST; \
+ LOCKDEP_SYS_EXIT; \
+ RESTORE_REST;
+# else
+# define LOCKDEP_SYS_EXIT
+# define LOCKDEP_SYS_EXIT_REST
+# endif
#endif
#endif
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 04/12] lockdep: annotate journal_start()
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (2 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 03/12] lockdep: x86_64: " Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 05/12] mm: trylock_page Peter Zijlstra
` (8 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: jbd-lock.patch --]
[-- Type: text/plain, Size: 1999 bytes --]
On Fri, 2007-07-13 at 02:05 -0700, Andrew Morton wrote:
> Except lockdep doesn't know about journal_start(), which has ranking
> requirements similar to a semaphore.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
fs/jbd/transaction.c | 9 +++++++++
include/linux/jbd.h | 5 +++++
2 files changed, 14 insertions(+)
Index: linux-2.6/fs/jbd/transaction.c
===================================================================
--- linux-2.6.orig/fs/jbd/transaction.c
+++ linux-2.6/fs/jbd/transaction.c
@@ -233,6 +233,8 @@ out:
return ret;
}
+static struct lock_class_key jbd_handle_key;
+
/* Allocate a new handle. This should probably be in a slab... */
static handle_t *new_handle(int nblocks)
{
@@ -243,6 +245,8 @@ static handle_t *new_handle(int nblocks)
handle->h_buffer_credits = nblocks;
handle->h_ref = 1;
+ lockdep_init_map(&handle->h_lockdep_map, "jbd_handle", &jbd_handle_key, 0);
+
return handle;
}
@@ -286,6 +290,9 @@ handle_t *journal_start(journal_t *journ
current->journal_info = NULL;
handle = ERR_PTR(err);
}
+
+ lock_acquire(&handle->h_lockdep_map, 0, 0, 0, 2, _THIS_IP_);
+
return handle;
}
@@ -1411,6 +1418,8 @@ int journal_stop(handle_t *handle)
spin_unlock(&journal->j_state_lock);
}
+ lock_release(&handle->h_lockdep_map, 1, _THIS_IP_);
+
jbd_free_handle(handle);
return err;
}
Index: linux-2.6/include/linux/jbd.h
===================================================================
--- linux-2.6.orig/include/linux/jbd.h
+++ linux-2.6/include/linux/jbd.h
@@ -30,6 +30,7 @@
#include <linux/bit_spinlock.h>
#include <linux/mutex.h>
#include <linux/timer.h>
+#include <linux/lockdep.h>
#include <asm/semaphore.h>
#endif
@@ -396,6 +397,10 @@ struct handle_s
unsigned int h_sync: 1; /* sync-on-close */
unsigned int h_jdata: 1; /* force data journaling */
unsigned int h_aborted: 1; /* fatal error on handle */
+
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ struct lockdep_map h_lockdep_map;
+#endif
};
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 05/12] mm: trylock_page
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (3 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 04/12] lockdep: annotate journal_start() Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 3:11 ` Nick Piggin
2007-09-28 7:42 ` [PATCH 06/12] mm: remove raw SetPageLocked() usage Peter Zijlstra
` (7 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: trylock_page.patch --]
[-- Type: text/plain, Size: 9860 bytes --]
Replace raw TestSetPageLocked() usage with trylock_page()
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
fs/afs/write.c | 2 +-
fs/cifs/file.c | 2 +-
fs/jbd/commit.c | 2 +-
fs/jbd2/commit.c | 2 +-
fs/splice.c | 2 +-
fs/xfs/linux-2.6/xfs_aops.c | 4 ++--
include/linux/pagemap.h | 5 +++++
mm/filemap.c | 4 ++--
mm/memory.c | 2 +-
mm/migrate.c | 4 ++--
mm/rmap.c | 2 +-
mm/shmem.c | 4 ++--
mm/swap.c | 2 +-
mm/swap_state.c | 2 +-
mm/swapfile.c | 2 +-
mm/truncate.c | 4 ++--
mm/vmscan.c | 4 ++--
17 files changed, 27 insertions(+), 22 deletions(-)
Index: linux-2.6/fs/afs/write.c
===================================================================
--- linux-2.6.orig/fs/afs/write.c
+++ linux-2.6/fs/afs/write.c
@@ -404,7 +404,7 @@ static int afs_write_back_from_locked_pa
page = pages[loop];
if (page->index > wb->last)
break;
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
break;
if (!PageDirty(page) ||
page_private(page) != (unsigned long) wb) {
Index: linux-2.6/fs/cifs/file.c
===================================================================
--- linux-2.6.orig/fs/cifs/file.c
+++ linux-2.6/fs/cifs/file.c
@@ -1205,7 +1205,7 @@ retry:
if (first < 0)
lock_page(page);
- else if (TestSetPageLocked(page))
+ else if (!trylock_page(page))
break;
if (unlikely(page->mapping != mapping)) {
Index: linux-2.6/fs/jbd/commit.c
===================================================================
--- linux-2.6.orig/fs/jbd/commit.c
+++ linux-2.6/fs/jbd/commit.c
@@ -63,7 +63,7 @@ static void release_buffer_page(struct b
goto nope;
/* OK, it's a truncated page */
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
goto nope;
page_cache_get(page);
Index: linux-2.6/fs/jbd2/commit.c
===================================================================
--- linux-2.6.orig/fs/jbd2/commit.c
+++ linux-2.6/fs/jbd2/commit.c
@@ -63,7 +63,7 @@ static void release_buffer_page(struct b
goto nope;
/* OK, it's a truncated page */
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
goto nope;
page_cache_get(page);
Index: linux-2.6/fs/splice.c
===================================================================
--- linux-2.6.orig/fs/splice.c
+++ linux-2.6/fs/splice.c
@@ -364,7 +364,7 @@ __generic_file_splice_read(struct file *
* for an in-flight io page
*/
if (flags & SPLICE_F_NONBLOCK) {
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
break;
} else
lock_page(page);
Index: linux-2.6/fs/xfs/linux-2.6/xfs_aops.c
===================================================================
--- linux-2.6.orig/fs/xfs/linux-2.6/xfs_aops.c
+++ linux-2.6/fs/xfs/linux-2.6/xfs_aops.c
@@ -665,7 +665,7 @@ xfs_probe_cluster(
} else
pg_offset = PAGE_CACHE_SIZE;
- if (page->index == tindex && !TestSetPageLocked(page)) {
+ if (page->index == tindex && trylock_page(page)) {
pg_len = xfs_probe_page(page, pg_offset, mapped);
unlock_page(page);
}
@@ -749,7 +749,7 @@ xfs_convert_page(
if (page->index != tindex)
goto fail;
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
goto fail;
if (PageWriteback(page))
goto fail_unlock_page;
Index: linux-2.6/include/linux/pagemap.h
===================================================================
--- linux-2.6.orig/include/linux/pagemap.h
+++ linux-2.6/include/linux/pagemap.h
@@ -167,6 +167,11 @@ static inline void lock_page(struct page
__lock_page(page);
}
+static inline int trylock_page(struct page *page)
+{
+ return !TestSetPageLocked(page);
+}
+
/*
* lock_page_nosync should only be used if we can't pin the page's inode.
* Doesn't play quite so well with block device plugging.
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -626,7 +626,7 @@ repeat:
page = radix_tree_lookup(&mapping->page_tree, offset);
if (page) {
page_cache_get(page);
- if (TestSetPageLocked(page)) {
+ if (!trylock_page(page)) {
read_unlock_irq(&mapping->tree_lock);
__lock_page(page);
read_lock_irq(&mapping->tree_lock);
@@ -802,7 +802,7 @@ grab_cache_page_nowait(struct address_sp
struct page *page = find_get_page(mapping, index);
if (page) {
- if (!TestSetPageLocked(page))
+ if (trylock_page(page))
return page;
page_cache_release(page);
return NULL;
Index: linux-2.6/mm/memory.c
===================================================================
--- linux-2.6.orig/mm/memory.c
+++ linux-2.6/mm/memory.c
@@ -1650,7 +1650,7 @@ static int do_wp_page(struct mm_struct *
* not dirty accountable.
*/
if (PageAnon(old_page)) {
- if (!TestSetPageLocked(old_page)) {
+ if (trylock_page(old_page)) {
reuse = can_share_swap_page(old_page);
unlock_page(old_page);
}
Index: linux-2.6/mm/migrate.c
===================================================================
--- linux-2.6.orig/mm/migrate.c
+++ linux-2.6/mm/migrate.c
@@ -568,7 +568,7 @@ static int move_to_new_page(struct page
* establishing additional references. We are the only one
* holding a reference to the new page at this point.
*/
- if (TestSetPageLocked(newpage))
+ if (!trylock_page(newpage))
BUG();
/* Prepare mapping for the new page.*/
@@ -621,7 +621,7 @@ static int unmap_and_move(new_page_t get
goto move_newpage;
rc = -EAGAIN;
- if (TestSetPageLocked(page)) {
+ if (!trylock_page(page)) {
if (!force)
goto move_newpage;
lock_page(page);
Index: linux-2.6/mm/rmap.c
===================================================================
--- linux-2.6.orig/mm/rmap.c
+++ linux-2.6/mm/rmap.c
@@ -401,7 +401,7 @@ int page_referenced(struct page *page, i
referenced += page_referenced_anon(page);
else if (is_locked)
referenced += page_referenced_file(page);
- else if (TestSetPageLocked(page))
+ else if (!trylock_page(page))
referenced++;
else {
if (page->mapping)
Index: linux-2.6/mm/shmem.c
===================================================================
--- linux-2.6.orig/mm/shmem.c
+++ linux-2.6/mm/shmem.c
@@ -1165,7 +1165,7 @@ repeat:
}
/* We have to do this with page locked to prevent races */
- if (TestSetPageLocked(swappage)) {
+ if (!trylock_page(swappage)) {
shmem_swp_unmap(entry);
spin_unlock(&info->lock);
wait_on_page_locked(swappage);
@@ -1224,7 +1224,7 @@ repeat:
shmem_swp_unmap(entry);
filepage = find_get_page(mapping, idx);
if (filepage &&
- (!PageUptodate(filepage) || TestSetPageLocked(filepage))) {
+ (!PageUptodate(filepage) || !trylock_page(filepage))) {
spin_unlock(&info->lock);
wait_on_page_locked(filepage);
page_cache_release(filepage);
Index: linux-2.6/mm/swap.c
===================================================================
--- linux-2.6.orig/mm/swap.c
+++ linux-2.6/mm/swap.c
@@ -412,7 +412,7 @@ void pagevec_strip(struct pagevec *pvec)
for (i = 0; i < pagevec_count(pvec); i++) {
struct page *page = pvec->pages[i];
- if (PagePrivate(page) && !TestSetPageLocked(page)) {
+ if (PagePrivate(page) && trylock_page(page)) {
if (PagePrivate(page))
try_to_release_page(page, 0);
unlock_page(page);
Index: linux-2.6/mm/swap_state.c
===================================================================
--- linux-2.6.orig/mm/swap_state.c
+++ linux-2.6/mm/swap_state.c
@@ -252,7 +252,7 @@ int move_from_swap_cache(struct page *pa
*/
static inline void free_swap_cache(struct page *page)
{
- if (PageSwapCache(page) && !TestSetPageLocked(page)) {
+ if (PageSwapCache(page) && trylock_page(page)) {
remove_exclusive_swap_page(page);
unlock_page(page);
}
Index: linux-2.6/mm/swapfile.c
===================================================================
--- linux-2.6.orig/mm/swapfile.c
+++ linux-2.6/mm/swapfile.c
@@ -401,7 +401,7 @@ void free_swap_and_cache(swp_entry_t ent
if (p) {
if (swap_entry_free(p, swp_offset(entry)) == 1) {
page = find_get_page(&swapper_space, entry.val);
- if (page && unlikely(TestSetPageLocked(page))) {
+ if (page && unlikely(!trylock_page(page))) {
page_cache_release(page);
page = NULL;
}
Index: linux-2.6/mm/truncate.c
===================================================================
--- linux-2.6.orig/mm/truncate.c
+++ linux-2.6/mm/truncate.c
@@ -186,7 +186,7 @@ void truncate_inode_pages_range(struct a
if (page_index > next)
next = page_index;
next++;
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
continue;
if (PageWriteback(page)) {
unlock_page(page);
@@ -279,7 +279,7 @@ unsigned long __invalidate_mapping_pages
pgoff_t index;
int lock_failed;
- lock_failed = TestSetPageLocked(page);
+ lock_failed = !trylock_page(page);
/*
* We really shouldn't be looking at the ->index of an
Index: linux-2.6/mm/vmscan.c
===================================================================
--- linux-2.6.orig/mm/vmscan.c
+++ linux-2.6/mm/vmscan.c
@@ -461,7 +461,7 @@ static unsigned long shrink_page_list(st
page = lru_to_page(page_list);
list_del(&page->lru);
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
goto keep;
VM_BUG_ON(PageActive(page));
@@ -547,7 +547,7 @@ static unsigned long shrink_page_list(st
* A synchronous write - probably a ramdisk. Go
* ahead and try to reclaim the page.
*/
- if (TestSetPageLocked(page))
+ if (!trylock_page(page))
goto keep;
if (PageDirty(page) || PageWriteback(page))
goto keep_locked;
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 06/12] mm: remove raw SetPageLocked() usage
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (4 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 05/12] mm: trylock_page Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 8:22 ` Christoph Hellwig
2007-09-28 7:42 ` [PATCH 07/12] lockdep: page lock hooks Peter Zijlstra
` (6 subsequent siblings)
12 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lock_page.patch --]
[-- Type: text/plain, Size: 1284 bytes --]
Remove the last few SetPageLocked() instances.
use trylock_page() instead of SetPageLocked() because we hold tree_lock
so we cannot suffer the might_sleep(). Also at least add_to_page_cache() can
be handed a locked page.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
mm/filemap.c | 2 +-
mm/swap_state.c | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -445,7 +445,7 @@ int add_to_page_cache(struct page *page,
error = radix_tree_insert(&mapping->page_tree, offset, page);
if (!error) {
page_cache_get(page);
- SetPageLocked(page);
+ trylock_page(page);
page->mapping = mapping;
page->index = offset;
mapping->nrpages++;
Index: linux-2.6/mm/swap_state.c
===================================================================
--- linux-2.6.orig/mm/swap_state.c
+++ linux-2.6/mm/swap_state.c
@@ -83,7 +83,7 @@ static int __add_to_swap_cache(struct pa
entry.val, page);
if (!error) {
page_cache_get(page);
- SetPageLocked(page);
+ trylock_page(page);
SetPageSwapCache(page);
set_page_private(page, entry.val);
total_swapcache_pages++;
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 07/12] lockdep: page lock hooks
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (5 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 06/12] mm: remove raw SetPageLocked() usage Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 08/12] lockdep: increase MAX_LOCK_DEPTH Peter Zijlstra
` (5 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-page_lock-hooks.patch --]
[-- Type: text/plain, Size: 4269 bytes --]
rework the now complete page lock API to provide acquisition and release hooks.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/pagemap.h | 40 +++++++++++++++++++++++++++++++---------
mm/filemap.c | 13 +++++++------
2 files changed, 38 insertions(+), 15 deletions(-)
Index: linux-2.6/include/linux/pagemap.h
===================================================================
--- linux-2.6.orig/include/linux/pagemap.h
+++ linux-2.6/include/linux/pagemap.h
@@ -153,23 +153,45 @@ static inline pgoff_t linear_page_index(
return pgoff >> (PAGE_CACHE_SHIFT - PAGE_SHIFT);
}
-extern void FASTCALL(__lock_page(struct page *page));
-extern void FASTCALL(__lock_page_nosync(struct page *page));
-extern void FASTCALL(unlock_page(struct page *page));
+extern void FASTCALL(__lock_page(struct page *page, unsigned long ip));
+extern void FASTCALL(__lock_page_nosync(struct page *page, unsigned long ip));
+extern void FASTCALL(unlock_page_nocheck(struct page *page));
+
+static inline void __lock_page_check(struct page *page, int try, unsigned long ip)
+{
+}
+
+static inline void __unlock_page_check(struct page *page, unsigned long ip)
+{
+}
/*
* lock_page may only be called if we have the page's inode pinned.
*/
+static inline int __trylock_page(struct page *page, int try)
+{
+ int ret = !TestSetPageLocked(page);
+ if (ret)
+ __lock_page_check(page, try, _RET_IP_);
+ return ret;
+}
+
+static inline int trylock_page(struct page *page)
+{
+ return __trylock_page(page, 1);
+}
+
static inline void lock_page(struct page *page)
{
might_sleep();
- if (TestSetPageLocked(page))
- __lock_page(page);
+ if (!__trylock_page(page, 0))
+ __lock_page(page, _RET_IP_);
}
-static inline int trylock_page(struct page *page)
+static inline void unlock_page(struct page *page)
{
- return !TestSetPageLocked(page);
+ __unlock_page_check(page, _RET_IP_);
+ unlock_page_nocheck(page);
}
/*
@@ -179,8 +201,8 @@ static inline int trylock_page(struct pa
static inline void lock_page_nosync(struct page *page)
{
might_sleep();
- if (TestSetPageLocked(page))
- __lock_page_nosync(page);
+ if (!__trylock_page(page, 0))
+ __lock_page_nosync(page, _RET_IP_);
}
/*
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -531,7 +531,7 @@ EXPORT_SYMBOL(wait_on_page_bit);
* the clear_bit and the read of the waitqueue (to avoid SMP races with a
* parallel wait_on_page_locked()).
*/
-void fastcall unlock_page(struct page *page)
+void fastcall unlock_page_nocheck(struct page *page)
{
smp_mb__before_clear_bit();
if (!TestClearPageLocked(page))
@@ -539,7 +539,7 @@ void fastcall unlock_page(struct page *p
smp_mb__after_clear_bit();
wake_up_page(page, PG_locked);
}
-EXPORT_SYMBOL(unlock_page);
+EXPORT_SYMBOL(unlock_page_nocheck);
/**
* end_page_writeback - end writeback against a page
@@ -565,12 +565,12 @@ EXPORT_SYMBOL(end_page_writeback);
* chances are that on the second loop, the block layer's plug list is empty,
* so sync_page() will then return in state TASK_UNINTERRUPTIBLE.
*/
-void fastcall __lock_page(struct page *page)
+void fastcall __lock_page(struct page *page, unsigned long ip)
{
DEFINE_WAIT_BIT(wait, &page->flags, PG_locked);
-
__wait_on_bit_lock(page_waitqueue(page), &wait, sync_page,
TASK_UNINTERRUPTIBLE);
+ __lock_page_check(page, 0, ip);
}
EXPORT_SYMBOL(__lock_page);
@@ -578,11 +578,12 @@ EXPORT_SYMBOL(__lock_page);
* Variant of lock_page that does not require the caller to hold a reference
* on the page's mapping.
*/
-void fastcall __lock_page_nosync(struct page *page)
+void fastcall __lock_page_nosync(struct page *page, unsigned long ip)
{
DEFINE_WAIT_BIT(wait, &page->flags, PG_locked);
__wait_on_bit_lock(page_waitqueue(page), &wait, __sleep_on_page_lock,
TASK_UNINTERRUPTIBLE);
+ __lock_page_check(page, 0, ip);
}
/**
@@ -628,7 +629,7 @@ repeat:
page_cache_get(page);
if (!trylock_page(page)) {
read_unlock_irq(&mapping->tree_lock);
- __lock_page(page);
+ __lock_page(page, _THIS_IP_);
read_lock_irq(&mapping->tree_lock);
/* Has the page been truncated while we slept? */
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 08/12] lockdep: increase MAX_LOCK_DEPTH
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (6 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 07/12] lockdep: page lock hooks Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 09/12] lockdep: add a page lock class per filesystem type Peter Zijlstra
` (4 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-depth.patch --]
[-- Type: text/plain, Size: 638 bytes --]
we can easily hold PAGE_VEC (and more in places) page locks.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/sched.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
Index: linux-2.6/include/linux/sched.h
===================================================================
--- linux-2.6.orig/include/linux/sched.h
+++ linux-2.6/include/linux/sched.h
@@ -1246,7 +1246,7 @@ struct task_struct {
int softirq_context;
#endif
#ifdef CONFIG_LOCKDEP
-# define MAX_LOCK_DEPTH 30UL
+# define MAX_LOCK_DEPTH 48UL
u64 curr_chain_key;
int lockdep_depth;
struct held_lock held_locks[MAX_LOCK_DEPTH];
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 09/12] lockdep: add a page lock class per filesystem type
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (7 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 08/12] lockdep: increase MAX_LOCK_DEPTH Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 10/12] lockdep: lock_page: handle IO-completions Peter Zijlstra
` (3 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-fs-page.patch --]
[-- Type: text/plain, Size: 1316 bytes --]
Provide a different page lock class per filesystem, so we can properly
express the neseting relations between filesystems.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
fs/filesystems.c | 1 +
include/linux/fs.h | 5 +++++
2 files changed, 6 insertions(+)
Index: linux-2.6/fs/filesystems.c
===================================================================
--- linux-2.6.orig/fs/filesystems.c
+++ linux-2.6/fs/filesystems.c
@@ -73,6 +73,7 @@ int register_filesystem(struct file_syst
if (fs->next)
return -EBUSY;
INIT_LIST_HEAD(&fs->fs_supers);
+ lockdep_init_map(&fs->s_mapping_map, fs->name, &fs->s_mapping_key, 0);
write_lock(&file_systems_lock);
p = find_filesystem(fs->name, strlen(fs->name));
if (*p)
Index: linux-2.6/include/linux/fs.h
===================================================================
--- linux-2.6.orig/include/linux/fs.h
+++ linux-2.6/include/linux/fs.h
@@ -1302,8 +1302,13 @@ struct file_system_type {
struct module *owner;
struct file_system_type * next;
struct list_head fs_supers;
+
struct lock_class_key s_lock_key;
struct lock_class_key s_umount_key;
+ struct lock_class_key s_mapping_key;
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+ struct lockdep_map s_mapping_map;
+#endif
};
extern int get_sb_bdev(struct file_system_type *fs_type,
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 10/12] lockdep: lock_page: handle IO-completions
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (8 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 09/12] lockdep: add a page lock class per filesystem type Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 11/12] mm: set_page_mapping() Peter Zijlstra
` (2 subsequent siblings)
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-page-async.patch --]
[-- Type: text/plain, Size: 3964 bytes --]
IO-completions unlock the page in interrupt context, which is not the same
context as that locked the page.
Avoid this problem by doing the unlock tracking when submitting the page for
IO.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
fs/buffer.c | 4 +++-
fs/mpage.c | 19 +++++++++++++++----
fs/nfs/read.c | 4 +++-
include/linux/pagemap.h | 5 +++++
mm/page_io.c | 3 ++-
5 files changed, 28 insertions(+), 7 deletions(-)
Index: linux-2.6/fs/buffer.c
===================================================================
--- linux-2.6.orig/fs/buffer.c
+++ linux-2.6/fs/buffer.c
@@ -413,7 +413,7 @@ static void end_buffer_async_read(struct
*/
if (page_uptodate && !PageError(page))
SetPageUptodate(page);
- unlock_page(page);
+ unlock_page_nocheck(page);
return;
still_busy:
@@ -1988,6 +1988,8 @@ int block_read_full_page(struct page *pa
mark_buffer_async_read(bh);
}
+ unlock_page_check(page);
+
/*
* Stage 3: start the IO. Check for uptodateness
* inside the buffer lock in case another process reading
Index: linux-2.6/fs/mpage.c
===================================================================
--- linux-2.6.orig/fs/mpage.c
+++ linux-2.6/fs/mpage.c
@@ -59,7 +59,7 @@ static int mpage_end_io_read(struct bio
ClearPageUptodate(page);
SetPageError(page);
}
- unlock_page(page);
+ unlock_page_nocheck(page);
} while (bvec >= bio->bi_io_vec);
bio_put(bio);
return 0;
@@ -92,9 +92,20 @@ static int mpage_end_io_write(struct bio
static struct bio *mpage_bio_submit(int rw, struct bio *bio)
{
- bio->bi_end_io = mpage_end_io_read;
- if (rw == WRITE)
- bio->bi_end_io = mpage_end_io_write;
+ bio->bi_end_io = mpage_end_io_write;
+ if (rw == READ) {
+ struct bio_vec *bvec = bio->bi_io_vec;
+ int i;
+
+ bio->bi_end_io = mpage_end_io_read;
+
+ for (i = 0; i < bio->bi_vcnt; i++) {
+ struct page *page = bvec[i].bv_page;
+
+ if (page)
+ unlock_page_check(page);
+ }
+ }
submit_bio(rw, bio);
return NULL;
}
Index: linux-2.6/fs/nfs/read.c
===================================================================
--- linux-2.6.orig/fs/nfs/read.c
+++ linux-2.6/fs/nfs/read.c
@@ -137,12 +137,13 @@ static int nfs_readpage_async(struct nfs
nfs_pagein_multi(inode, &one_request, 1, len, 0);
else
nfs_pagein_one(inode, &one_request, 1, len, 0);
+ unlock_page_check(page);
return 0;
}
static void nfs_readpage_release(struct nfs_page *req)
{
- unlock_page(req->wb_page);
+ unlock_page_nocheck(req->wb_page);
dprintk("NFS: read done (%s/%Ld %d@%Ld)\n",
req->wb_context->path.dentry->d_inode->i_sb->s_id,
@@ -540,6 +541,7 @@ readpage_async_filler(void *data, struct
if (len < PAGE_CACHE_SIZE)
zero_user_page(page, len, PAGE_CACHE_SIZE - len, KM_USER0);
nfs_pageio_add_request(desc->pgio, new);
+ unlock_page_check(page);
return 0;
out_error:
error = PTR_ERR(new);
Index: linux-2.6/mm/page_io.c
===================================================================
--- linux-2.6.orig/mm/page_io.c
+++ linux-2.6/mm/page_io.c
@@ -92,7 +92,7 @@ int end_swap_bio_read(struct bio *bio, u
} else {
SetPageUptodate(page);
}
- unlock_page(page);
+ unlock_page_nocheck(page);
bio_put(bio);
return 0;
}
@@ -144,6 +144,7 @@ int swap_readpage(struct file *file, str
}
count_vm_event(PSWPIN);
submit_bio(READ, bio);
+ unlock_page_check(page);
out:
return ret;
}
Index: linux-2.6/include/linux/pagemap.h
===================================================================
--- linux-2.6.orig/include/linux/pagemap.h
+++ linux-2.6/include/linux/pagemap.h
@@ -189,6 +189,11 @@ static inline void unlock_page(struct pa
unlock_page_nocheck(page);
}
+static inline void unlock_page_check(struct page *page)
+{
+ __unlock_page_check(page, _RET_IP_);
+}
+
/*
* lock_page_nosync should only be used if we can't pin the page's inode.
* Doesn't play quite so well with block device plugging.
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 11/12] mm: set_page_mapping()
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (9 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 10/12] lockdep: lock_page: handle IO-completions Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 7:42 ` [PATCH 12/12] lockdep: enable lock_page lockdep annotation Peter Zijlstra
2007-09-28 11:49 ` [PATCH 00/12] various lockdep patches Heiko Carstens
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: set_page_mapping.patch --]
[-- Type: text/plain, Size: 2693 bytes --]
wrap:
page->mapping = some_mapping;
because the lock class is dependent on page_mapping()
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/pagemap.h | 9 +++++++++
mm/filemap.c | 4 ++--
mm/migrate.c | 6 +++---
3 files changed, 14 insertions(+), 5 deletions(-)
Index: linux-2.6/include/linux/pagemap.h
===================================================================
--- linux-2.6.orig/include/linux/pagemap.h
+++ linux-2.6/include/linux/pagemap.h
@@ -199,6 +199,15 @@ static inline void unlock_page_check(str
__unlock_page_check(page, _RET_IP_);
}
+static inline struct address_space *set_page_mapping(struct page *page,
+ struct address_space *mapping)
+{
+ __unlock_page_check(page, _RET_IP_);
+ page->mapping = mapping;
+ __lock_page_check(page, 0, _RET_IP_); /* XXX we lost trylock */
+ return mapping;
+}
+
/*
* lock_page_nosync should only be used if we can't pin the page's inode.
* Doesn't play quite so well with block device plugging.
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -117,7 +117,7 @@ void __remove_from_page_cache(struct pag
struct address_space *mapping = page->mapping;
radix_tree_delete(&mapping->page_tree, page->index);
- page->mapping = NULL;
+ set_page_mapping(page, NULL);
mapping->nrpages--;
__dec_zone_page_state(page, NR_FILE_PAGES);
BUG_ON(page_mapped(page));
@@ -446,7 +446,7 @@ int add_to_page_cache(struct page *page,
if (!error) {
page_cache_get(page);
trylock_page(page);
- page->mapping = mapping;
+ set_page_mapping(page, mapping);
page->index = offset;
mapping->nrpages++;
__inc_zone_page_state(page, NR_FILE_PAGES);
Index: linux-2.6/mm/migrate.c
===================================================================
--- linux-2.6.orig/mm/migrate.c
+++ linux-2.6/mm/migrate.c
@@ -381,7 +381,7 @@ static void migrate_page_copy(struct pag
ClearPageActive(page);
ClearPagePrivate(page);
set_page_private(page, 0);
- page->mapping = NULL;
+ set_page_mapping(page, NULL);
/*
* If any waiters have accumulated on the new page then
@@ -573,7 +573,7 @@ static int move_to_new_page(struct page
/* Prepare mapping for the new page.*/
newpage->index = page->index;
- newpage->mapping = page->mapping;
+ set_page_mapping(newpage, page->mapping);
mapping = page_mapping(page);
if (!mapping)
@@ -594,7 +594,7 @@ static int move_to_new_page(struct page
if (!rc)
remove_migration_ptes(page, newpage);
else
- newpage->mapping = NULL;
+ set_page_mapping(newpage, NULL);
unlock_page(newpage);
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* [PATCH 12/12] lockdep: enable lock_page lockdep annotation
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (10 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 11/12] mm: set_page_mapping() Peter Zijlstra
@ 2007-09-28 7:42 ` Peter Zijlstra
2007-09-28 11:49 ` [PATCH 00/12] various lockdep patches Heiko Carstens
12 siblings, 0 replies; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 7:42 UTC (permalink / raw)
To: lkml, linux-arch; +Cc: Zach Brown, Ingo Molnar, akpm, Peter Zijlstra
[-- Attachment #1: lockdep-lock_page.patch --]
[-- Type: text/plain, Size: 3081 bytes --]
With everything ready, connect the dots.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/lockdep.h | 7 +++++++
include/linux/pagemap.h | 10 ++++++++++
mm/filemap.c | 35 +++++++++++++++++++++++++++++++++++
3 files changed, 52 insertions(+)
Index: linux-2.6/include/linux/lockdep.h
===================================================================
--- linux-2.6.orig/include/linux/lockdep.h
+++ linux-2.6/include/linux/lockdep.h
@@ -253,6 +253,13 @@ extern void lockdep_init_map(struct lock
struct lock_class_key *key, int subclass);
/*
+ * To initialize a lockdep_map statically use this macro.
+ * Note that _name must not be NULL.
+ */
+#define STATIC_LOCKDEP_MAP_INIT(_name, _key) \
+ { .name = (_name), .key = (void *)(_key), }
+
+/*
* Reinitialize a lock key - for cases where there is special locking or
* special initialization of locks so that the validator gets the scope
* of dependencies wrong: they are either too broad (they need a class-split)
Index: linux-2.6/include/linux/pagemap.h
===================================================================
--- linux-2.6.orig/include/linux/pagemap.h
+++ linux-2.6/include/linux/pagemap.h
@@ -157,12 +157,22 @@ extern void FASTCALL(__lock_page(struct
extern void FASTCALL(__lock_page_nosync(struct page *page, unsigned long ip));
extern void FASTCALL(unlock_page_nocheck(struct page *page));
+extern struct lockdep_map *page_lock_map(struct page *page);
+
static inline void __lock_page_check(struct page *page, int try, unsigned long ip)
{
+ /*
+ * use recursive read locks to annotate the page lock.
+ * this allows us to have a single lock instance per filesystem.
+ * obviously this reduces the usefulness of the lock checker
+ * but it also keeps struct page from bloating.
+ */
+ lock_acquire(page_lock_map(page), 0, try, 2, 2, ip);
}
static inline void __unlock_page_check(struct page *page, unsigned long ip)
{
+ lock_release(page_lock_map(page), 1, ip);
}
/*
Index: linux-2.6/mm/filemap.c
===================================================================
--- linux-2.6.orig/mm/filemap.c
+++ linux-2.6/mm/filemap.c
@@ -556,6 +556,41 @@ void end_page_writeback(struct page *pag
}
EXPORT_SYMBOL(end_page_writeback);
+#ifdef CONFIG_DEBUG_LOCK_ALLOC
+static struct lock_class_key lock_page_key;
+static struct lockdep_map lock_page_map =
+ STATIC_LOCKDEP_MAP_INIT("lock_page", &lock_page_key);
+
+struct lockdep_map *page_lock_map(struct page *page)
+{
+ struct address_space *mapping;
+ struct inode *inode;
+ struct super_block *sb;
+ struct file_system_type *type;
+ struct lockdep_map *map = &lock_page_map;
+
+ mapping = page_mapping(page);
+ if (!mapping)
+ goto out;
+
+ inode = mapping->host;
+ if (!inode)
+ goto out;
+
+ sb = inode->i_sb;
+ if (!sb)
+ goto out;
+
+ type = sb->s_type;
+ if (type)
+ map = &type->s_mapping_map;
+
+out:
+ return map;
+}
+EXPORT_SYMBOL(page_lock_map);
+#endif
+
/**
* __lock_page - get a lock on the page, assuming we need to sleep to get it
* @page: the page to lock
--
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 06/12] mm: remove raw SetPageLocked() usage
2007-09-28 7:42 ` [PATCH 06/12] mm: remove raw SetPageLocked() usage Peter Zijlstra
@ 2007-09-28 8:22 ` Christoph Hellwig
0 siblings, 0 replies; 21+ messages in thread
From: Christoph Hellwig @ 2007-09-28 8:22 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
\x12 error = radix_tree_insert(&mapping->page_tree, offset, page);
> if (!error) {
> page_cache_get(page);
I don't think we want an unchecked trylock_page without a comment explaining
it. I'd go as far as saying trylock_page should get a __must_check
attribute, and we'll need a dummy variable to actually ignore it.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 00/12] various lockdep patches
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
` (11 preceding siblings ...)
2007-09-28 7:42 ` [PATCH 12/12] lockdep: enable lock_page lockdep annotation Peter Zijlstra
@ 2007-09-28 11:49 ` Heiko Carstens
12 siblings, 0 replies; 21+ messages in thread
From: Heiko Carstens @ 2007-09-28 11:49 UTC (permalink / raw)
To: Peter Zijlstra
Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm,
Martin Schwidefsky
On Fri, Sep 28, 2007 at 09:42:00AM +0200, Peter Zijlstra wrote:
> The first 3 patches provide a new lockdep check, it verifies we don't hold any
> locks when returning to userspace.
>
> lockdep-sys_exit.patch
> lockdep-i386-sys_exit.patch
> lockdep-x86_64-sys_exit.patch
>
> Could the various arch maintainers that have support for lockdep help out
> with placing this hook?
Subject: [PATCH] lockdep: s390: connect the sysexit hook
From: Heiko Carstens <heiko.carstens@de.ibm.com>
Run the lockdep_sys_exit hook before returning to user space.
Reviewed-by: Martin Schwidefsky <schwidefsky@de.ibm.com>
Signed-off-by: Heiko Carstens <heiko.carstens@de.ibm.com>
---
arch/s390/kernel/entry.S | 12 ++++++++++++
arch/s390/kernel/entry64.S | 9 +++++++++
2 files changed, 21 insertions(+)
Index: linux-2.6/arch/s390/kernel/entry64.S
===================================================================
--- linux-2.6.orig/arch/s390/kernel/entry64.S
+++ linux-2.6/arch/s390/kernel/entry64.S
@@ -66,9 +66,14 @@ _TIF_WORK_INT = (_TIF_SIGPENDING | _TIF_
.macro TRACE_IRQS_OFF
brasl %r14,trace_hardirqs_off
.endm
+
+ .macro LOCKDEP_SYS_EXIT
+ brasl %r14,lockdep_sys_exit
+ .endm
#else
#define TRACE_IRQS_ON
#define TRACE_IRQS_OFF
+#define LOCKDEP_SYS_EXIT
#endif
.macro STORE_TIMER lc_offset
@@ -255,6 +260,7 @@ sysc_return:
jno sysc_leave
tm __TI_flags+7(%r9),_TIF_WORK_SVC
jnz sysc_work # there is work to do (signals etc.)
+ LOCKDEP_SYS_EXIT
sysc_leave:
RESTORE_ALL __LC_RETURN_PSW,1
@@ -278,6 +284,7 @@ sysc_work:
jo sysc_restart
tm __TI_flags+7(%r9),_TIF_SINGLE_STEP
jo sysc_singlestep
+ LOCKDEP_SYS_EXIT
j sysc_leave
#
@@ -558,6 +565,7 @@ io_return:
#endif
tm __TI_flags+7(%r9),_TIF_WORK_INT
jnz io_work # there is work to do (signals etc.)
+ LOCKDEP_SYS_EXIT
io_leave:
RESTORE_ALL __LC_RETURN_PSW,0
io_done:
@@ -605,6 +613,7 @@ io_work_loop:
jo io_reschedule
tm __TI_flags+7(%r9),(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK)
jnz io_sigpending
+ LOCKDEP_SYS_EXIT
j io_leave
#
Index: linux-2.6/arch/s390/kernel/entry.S
===================================================================
--- linux-2.6.orig/arch/s390/kernel/entry.S
+++ linux-2.6/arch/s390/kernel/entry.S
@@ -68,9 +68,15 @@ STACK_SIZE = 1 << STACK_SHIFT
l %r1,BASED(.Ltrace_irq_off)
basr %r14,%r1
.endm
+
+ .macro LOCKDEP_SYS_EXIT
+ l %r1,BASED(.Llockdep_sys_exit)
+ basr %r14,%r1
+ .endm
#else
#define TRACE_IRQS_ON
#define TRACE_IRQS_OFF
+#define LOCKDEP_SYS_EXIT
#endif
/*
@@ -260,6 +266,7 @@ sysc_return:
bno BASED(sysc_leave)
tm __TI_flags+3(%r9),_TIF_WORK_SVC
bnz BASED(sysc_work) # there is work to do (signals etc.)
+ LOCKDEP_SYS_EXIT
sysc_leave:
RESTORE_ALL __LC_RETURN_PSW,1
@@ -283,6 +290,7 @@ sysc_work:
bo BASED(sysc_restart)
tm __TI_flags+3(%r9),_TIF_SINGLE_STEP
bo BASED(sysc_singlestep)
+ LOCKDEP_SYS_EXIT
b BASED(sysc_leave)
#
@@ -572,6 +580,7 @@ io_return:
#endif
tm __TI_flags+3(%r9),_TIF_WORK_INT
bnz BASED(io_work) # there is work to do (signals etc.)
+ LOCKDEP_SYS_EXIT
io_leave:
RESTORE_ALL __LC_RETURN_PSW,0
io_done:
@@ -618,6 +627,7 @@ io_work_loop:
bo BASED(io_reschedule)
tm __TI_flags+3(%r9),(_TIF_SIGPENDING | _TIF_RESTORE_SIGMASK)
bnz BASED(io_sigpending)
+ LOCKDEP_SYS_EXIT
b BASED(io_leave)
#
@@ -1040,6 +1050,8 @@ cleanup_io_leave_insn:
.Ltrace_irq_on: .long trace_hardirqs_on
.Ltrace_irq_off:
.long trace_hardirqs_off
+.Llockdep_sys_exit:
+ .long lockdep_sys_exit
#endif
.Lcritical_start:
.long __critical_start + 0x80000000
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 01/12] lockdep: syscall exit check
2007-09-28 7:42 ` [PATCH 01/12] lockdep: syscall exit check Peter Zijlstra
@ 2007-09-28 12:03 ` Heiko Carstens
2007-09-28 12:14 ` Peter Zijlstra
0 siblings, 1 reply; 21+ messages in thread
From: Heiko Carstens @ 2007-09-28 12:03 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
> +void lockdep_sys_exit(void)
> +{
> + struct task_struct *curr = current;
> +
> + if (unlikely(curr->lockdep_depth)) {
> + if (!debug_locks_off())
> + return;
> + printk("\n========================================\n");
> + printk( "[ BUG: lock held at syscall exit time! ]\n");
> + printk( "----------------------------------------\n");
> + printk("%s/%d is leaving the kernel with locks still held!\n",
> + curr->comm, curr->pid);
> + lockdep_print_held_locks(curr);
> + }
> +}
By the way, the s390 patch I just posted also checks if we hold any locks
when returning from interrupt context to user space. Maybe the above text
could be changed to "lock held when returning to user space" ?
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 01/12] lockdep: syscall exit check
2007-09-28 12:03 ` Heiko Carstens
@ 2007-09-28 12:14 ` Peter Zijlstra
2007-09-28 12:21 ` Heiko Carstens
0 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-28 12:14 UTC (permalink / raw)
To: Heiko Carstens; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
[-- Attachment #1: Type: text/plain, Size: 2905 bytes --]
On Fri, 2007-09-28 at 14:03 +0200, Heiko Carstens wrote:
> > +void lockdep_sys_exit(void)
> > +{
> > + struct task_struct *curr = current;
> > +
> > + if (unlikely(curr->lockdep_depth)) {
> > + if (!debug_locks_off())
> > + return;
> > + printk("\n========================================\n");
> > + printk( "[ BUG: lock held at syscall exit time! ]\n");
> > + printk( "----------------------------------------\n");
> > + printk("%s/%d is leaving the kernel with locks still held!\n",
> > + curr->comm, curr->pid);
> > + lockdep_print_held_locks(curr);
> > + }
> > +}
>
> By the way, the s390 patch I just posted also checks if we hold any locks
> when returning from interrupt context to user space. Maybe the above text
> could be changed to "lock held when returning to user space" ?
Good idea, I'll look at doing the same for i386/x86_64. Traps (page
faults) would also make sense I guess.
---
Subject: lockdep: syscall exit check
Provide a check to validate that we do not hold any locks when switching
back to user-space.
Signed-off-by: Peter Zijlstra <a.p.zijlstra@chello.nl>
---
include/linux/lockdep.h | 2 ++
kernel/lockdep.c | 16 ++++++++++++++++
2 files changed, 18 insertions(+)
Index: linux-2.6/include/linux/lockdep.h
===================================================================
--- linux-2.6.orig/include/linux/lockdep.h
+++ linux-2.6/include/linux/lockdep.h
@@ -238,6 +238,7 @@ extern void lockdep_info(void);
extern void lockdep_reset(void);
extern void lockdep_reset_lock(struct lockdep_map *lock);
extern void lockdep_free_key_range(void *start, unsigned long size);
+extern void lockdep_sys_exit(void);
extern void lockdep_off(void);
extern void lockdep_on(void);
@@ -317,6 +318,7 @@ static inline void lockdep_on(void)
# define INIT_LOCKDEP
# define lockdep_reset() do { debug_locks = 1; } while (0)
# define lockdep_free_key_range(start, size) do { } while (0)
+# define lockdep_sys_exit() do { } while (0)
/*
* The class key takes no space if lockdep is disabled:
*/
Index: linux-2.6/kernel/lockdep.c
===================================================================
--- linux-2.6.orig/kernel/lockdep.c
+++ linux-2.6/kernel/lockdep.c
@@ -3199,3 +3199,19 @@ void debug_show_held_locks(struct task_s
}
EXPORT_SYMBOL_GPL(debug_show_held_locks);
+
+void lockdep_sys_exit(void)
+{
+ struct task_struct *curr = current;
+
+ if (unlikely(curr->lockdep_depth)) {
+ if (!debug_locks_off())
+ return;
+ printk("\n================================================\n");
+ printk( "[ BUG: lock held when returning to user space! ]\n");
+ printk( "------------------------------------------------\n");
+ printk("%s/%d is leaving the kernel with locks still held!\n",
+ curr->comm, curr->pid);
+ lockdep_print_held_locks(curr);
+ }
+}
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 189 bytes --]
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 01/12] lockdep: syscall exit check
2007-09-28 12:14 ` Peter Zijlstra
@ 2007-09-28 12:21 ` Heiko Carstens
0 siblings, 0 replies; 21+ messages in thread
From: Heiko Carstens @ 2007-09-28 12:21 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
On Fri, Sep 28, 2007 at 02:14:11PM +0200, Peter Zijlstra wrote:
> On Fri, 2007-09-28 at 14:03 +0200, Heiko Carstens wrote:
> > > +void lockdep_sys_exit(void)
> > > +{
> > > + struct task_struct *curr = current;
> > > +
> > > + if (unlikely(curr->lockdep_depth)) {
> > > + if (!debug_locks_off())
> > > + return;
> > > + printk("\n========================================\n");
> > > + printk( "[ BUG: lock held at syscall exit time! ]\n");
> > > + printk( "----------------------------------------\n");
> > > + printk("%s/%d is leaving the kernel with locks still held!\n",
> > > + curr->comm, curr->pid);
> > > + lockdep_print_held_locks(curr);
> > > + }
> > > +}
> >
> > By the way, the s390 patch I just posted also checks if we hold any locks
> > when returning from interrupt context to user space. Maybe the above text
> > could be changed to "lock held when returning to user space" ?
>
> Good idea, I'll look at doing the same for i386/x86_64. Traps (page
> faults) would also make sense I guess.
Yes, traps and syscalls have the same exit path on s390. So we do that
alreasy as well.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 05/12] mm: trylock_page
2007-09-28 3:11 ` Nick Piggin
@ 2007-09-29 15:01 ` Peter Zijlstra
2007-10-02 8:44 ` Nick Piggin
0 siblings, 1 reply; 21+ messages in thread
From: Peter Zijlstra @ 2007-09-29 15:01 UTC (permalink / raw)
To: Nick Piggin; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
On Fri, 2007-09-28 at 13:11 +1000, Nick Piggin wrote:
> On Friday 28 September 2007 17:42, Peter Zijlstra wrote:
> > Replace raw TestSetPageLocked() usage with trylock_page()
>
> I have such a thing queued too, for the lock bitops patches for when 2.6.24
> opens, Andrew promises me :).
>
> I guess they should be identical, except I don't like doing trylock_page in
> place of SetPageLocked, for memory ordering performance and aesthetic
> reasons... I've got an init_page_locked (or set_page_locked... I can't
> remember, the patch is at home).
Sure, that might work, or we could just make it so that add_to_*_cache
is never passed an unlocked page. But sure...
> Fine idea to lockdep the page lock, anyway. Does it show up any of the
> buffered write deadlock possibilities? :)
Not yet, it might just be that the concessions done to annotate this
type of lock were too severe.
What I basically did was treat all the page locks as a single recursive
lock.
> buffer lock is another notable bit-mutex that might be converted (I have
> the patch to do the similar nice !tas->trylock conversion for that too). I
> think it is used widely enough by tricky code that it would be useful to
> annotate as well.
Not at all familiar with that lock, but yeah, we could have a look at
doing that too.
> Unfortunately we can't convert bit_spinlock.h easily, I guess?
Yeah, the space constraints make that rather hard. Each of these locks
needs some form of external meta-data.
For the page lock I used one lock instance per file system type.
^ permalink raw reply [flat|nested] 21+ messages in thread
* Re: [PATCH 05/12] mm: trylock_page
2007-09-29 15:01 ` Peter Zijlstra
@ 2007-10-02 8:44 ` Nick Piggin
0 siblings, 0 replies; 21+ messages in thread
From: Nick Piggin @ 2007-10-02 8:44 UTC (permalink / raw)
To: Peter Zijlstra; +Cc: lkml, linux-arch, Zach Brown, Ingo Molnar, akpm
On Sunday 30 September 2007 01:01, Peter Zijlstra wrote:
> On Fri, 2007-09-28 at 13:11 +1000, Nick Piggin wrote:
> > On Friday 28 September 2007 17:42, Peter Zijlstra wrote:
> > > Replace raw TestSetPageLocked() usage with trylock_page()
> >
> > I have such a thing queued too, for the lock bitops patches for when
> > 2.6.24 opens, Andrew promises me :).
> >
> > I guess they should be identical, except I don't like doing trylock_page
> > in place of SetPageLocked, for memory ordering performance and aesthetic
> > reasons... I've got an init_page_locked (or set_page_locked... I can't
> > remember, the patch is at home).
>
> Sure, that might work, or we could just make it so that add_to_*_cache
> is never passed an unlocked page. But sure...
It does kind of make sense to have it there (because you want the page
to be locked iff it gets inserted into pagecache). And wherever you lock
the page, we'll still want an init_page_locked to be able to lock it while we
are the only owner of it, for the same performance reason.
> > Fine idea to lockdep the page lock, anyway. Does it show up any of the
> > buffered write deadlock possibilities? :)
>
> Not yet, it might just be that the concessions done to annotate this
> type of lock were too severe.
>
> What I basically did was treat all the page locks as a single recursive
> lock.
Hmm, OK. There are a couple of page lock deadlocks there that wouldn't
be picked up, but the page lock vs mmap_sem one probably should be.
> > buffer lock is another notable bit-mutex that might be converted (I have
> > the patch to do the similar nice !tas->trylock conversion for that too).
> > I think it is used widely enough by tricky code that it would be useful
> > to annotate as well.
>
> Not at all familiar with that lock, but yeah, we could have a look at
> doing that too.
Should be pretty well identical to the page lock. I'll cc you on the patch
to do this equivalent API conversion, if you'd like.
> > Unfortunately we can't convert bit_spinlock.h easily, I guess?
>
> Yeah, the space constraints make that rather hard. Each of these locks
> needs some form of external meta-data.
Yeah.
> For the page lock I used one lock instance per file system type.
Seems OK.
^ permalink raw reply [flat|nested] 21+ messages in thread
end of thread, other threads:[~2007-10-03 1:22 UTC | newest]
Thread overview: 21+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2007-09-28 7:42 [PATCH 00/12] various lockdep patches Peter Zijlstra
2007-09-28 7:42 ` [PATCH 01/12] lockdep: syscall exit check Peter Zijlstra
2007-09-28 12:03 ` Heiko Carstens
2007-09-28 12:14 ` Peter Zijlstra
2007-09-28 12:21 ` Heiko Carstens
2007-09-28 7:42 ` [PATCH 02/12] lockdep: i386: connect the sysexit hook Peter Zijlstra
2007-09-28 7:42 ` [PATCH 03/12] lockdep: x86_64: " Peter Zijlstra
2007-09-28 7:42 ` [PATCH 04/12] lockdep: annotate journal_start() Peter Zijlstra
2007-09-28 7:42 ` [PATCH 05/12] mm: trylock_page Peter Zijlstra
2007-09-28 3:11 ` Nick Piggin
2007-09-29 15:01 ` Peter Zijlstra
2007-10-02 8:44 ` Nick Piggin
2007-09-28 7:42 ` [PATCH 06/12] mm: remove raw SetPageLocked() usage Peter Zijlstra
2007-09-28 8:22 ` Christoph Hellwig
2007-09-28 7:42 ` [PATCH 07/12] lockdep: page lock hooks Peter Zijlstra
2007-09-28 7:42 ` [PATCH 08/12] lockdep: increase MAX_LOCK_DEPTH Peter Zijlstra
2007-09-28 7:42 ` [PATCH 09/12] lockdep: add a page lock class per filesystem type Peter Zijlstra
2007-09-28 7:42 ` [PATCH 10/12] lockdep: lock_page: handle IO-completions Peter Zijlstra
2007-09-28 7:42 ` [PATCH 11/12] mm: set_page_mapping() Peter Zijlstra
2007-09-28 7:42 ` [PATCH 12/12] lockdep: enable lock_page lockdep annotation Peter Zijlstra
2007-09-28 11:49 ` [PATCH 00/12] various lockdep patches Heiko Carstens
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).