[PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Byungchul Park]

For now, wait_for_completion() / complete() works with lockdep, add
lock_page() / unlock_page() and its family to lockdep support.

Changes from v1
 - Move lockdep_map_cross outside of page_ext to make it flexible
 - Prevent allocating lockdep_map per page by default
 - Add a boot parameter allowing the allocation for debugging

Byungchul Park (4):
  lockdep: Apply crossrelease to PG_locked locks
  lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
  lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
  lockdep: Add a boot parameter enabling to track page locks using
    lockdep and disable it by default

 Documentation/admin-guide/kernel-parameters.txt |   7 ++
 include/linux/mm_types.h                        |   4 +
 include/linux/page-flags.h                      |  43 +++++++-
 include/linux/pagemap.h                         | 125 ++++++++++++++++++++++--
 lib/Kconfig.debug                               |  11 +++
 mm/filemap.c                                    | 114 ++++++++++++++++++++-
 mm/page_ext.c                                   |   4 +
 7 files changed, 299 insertions(+), 9 deletions(-)

-- 
1.9.1

[PATCH v2 1/4] lockdep: Apply crossrelease to PG_locked locks

[Byungchul Park]

Although lock_page() and its family can cause deadlock, lockdep have not
worked with them, becasue unlock_page() might be called in a different
context from the acquire context, which violated lockdep's assumption.

Now CONFIG_LOCKDEP_CROSSRELEASE has been introduced, lockdep can work
with page locks.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 include/linux/mm_types.h |   8 ++++
 include/linux/pagemap.h  | 101 ++++++++++++++++++++++++++++++++++++++++++++---
 lib/Kconfig.debug        |   7 ++++
 mm/filemap.c             |   4 +-
 mm/page_alloc.c          |   3 ++
 5 files changed, 115 insertions(+), 8 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index c85f11d..263b861 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -17,6 +17,10 @@
 
 #include <asm/mmu.h>
 
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+#include <linux/lockdep.h>
+#endif
+
 #ifndef AT_VECTOR_SIZE_ARCH
 #define AT_VECTOR_SIZE_ARCH 0
 #endif
@@ -218,6 +222,10 @@ struct page {
 #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
 	int _last_cpupid;
 #endif
+
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+	struct lockdep_map_cross map;
+#endif
 }
 /*
  * The struct page can be forced to be double word aligned so that atomic ops
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index e08b533..35b4f67 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -15,6 +15,9 @@
 #include <linux/bitops.h>
 #include <linux/hardirq.h> /* for in_interrupt() */
 #include <linux/hugetlb_inline.h>
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+#include <linux/lockdep.h>
+#endif
 
 /*
  * Bits in mapping->flags.
@@ -457,26 +460,91 @@ static inline pgoff_t linear_page_index(struct vm_area_struct *vma,
 	return pgoff;
 }
 
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+#define lock_page_init(p)						\
+do {									\
+	static struct lock_class_key __key;				\
+	lockdep_init_map_crosslock((struct lockdep_map *)&(p)->map,	\
+			"(PG_locked)" #p, &__key, 0);			\
+} while (0)
+
+static inline void lock_page_acquire(struct page *page, int try)
+{
+	page = compound_head(page);
+	lock_acquire_exclusive((struct lockdep_map *)&page->map, 0,
+			       try, NULL, _RET_IP_);
+}
+
+static inline void lock_page_release(struct page *page)
+{
+	page = compound_head(page);
+	/*
+	 * lock_commit_crosslock() is necessary for crosslocks.
+	 */
+	lock_commit_crosslock((struct lockdep_map *)&page->map);
+	lock_release((struct lockdep_map *)&page->map, 0, _RET_IP_);
+}
+#else
+static inline void lock_page_init(struct page *page) {}
+static inline void lock_page_free(struct page *page) {}
+static inline void lock_page_acquire(struct page *page, int try) {}
+static inline void lock_page_release(struct page *page) {}
+#endif
+
 extern void __lock_page(struct page *page);
 extern int __lock_page_killable(struct page *page);
 extern int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
 				unsigned int flags);
-extern void unlock_page(struct page *page);
+extern void do_raw_unlock_page(struct page *page);
 
-static inline int trylock_page(struct page *page)
+static inline void unlock_page(struct page *page)
+{
+	lock_page_release(page);
+	do_raw_unlock_page(page);
+}
+
+static inline int do_raw_trylock_page(struct page *page)
 {
 	page = compound_head(page);
 	return (likely(!test_and_set_bit_lock(PG_locked, &page->flags)));
 }
 
+static inline int trylock_page(struct page *page)
+{
+	if (do_raw_trylock_page(page)) {
+		lock_page_acquire(page, 1);
+		return 1;
+	}
+	return 0;
+}
+
 /*
  * lock_page may only be called if we have the page's inode pinned.
  */
 static inline void lock_page(struct page *page)
 {
 	might_sleep();
-	if (!trylock_page(page))
+
+	if (!do_raw_trylock_page(page))
 		__lock_page(page);
+	/*
+	 * acquire() must be after actual lock operation for crosslocks.
+	 * This way a crosslock and current lock can be ordered like:
+	 *
+	 *	CONTEXT 1		CONTEXT 2
+	 *	---------		---------
+	 *	lock A (cross)
+	 *	acquire A
+	 *	  X = atomic_inc_return(&cross_gen_id)
+	 *	~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+	 *				acquire B
+	 *				  Y = atomic_read_acquire(&cross_gen_id)
+	 *				lock B
+	 *
+	 * so that 'lock A and then lock B' can be seen globally,
+	 * if X <= Y.
+	 */
+	lock_page_acquire(page, 0);
 }
 
 /*
@@ -486,9 +554,20 @@ static inline void lock_page(struct page *page)
  */
 static inline int lock_page_killable(struct page *page)
 {
+	int ret;
+
 	might_sleep();
-	if (!trylock_page(page))
-		return __lock_page_killable(page);
+
+	if (!do_raw_trylock_page(page)) {
+		ret = __lock_page_killable(page);
+		if (ret)
+			return ret;
+	}
+	/*
+	 * acquire() must be after actual lock operation for crosslocks.
+	 * This way a crosslock and other locks can be ordered.
+	 */
+	lock_page_acquire(page, 0);
 	return 0;
 }
 
@@ -503,7 +582,17 @@ static inline int lock_page_or_retry(struct page *page, struct mm_struct *mm,
 				     unsigned int flags)
 {
 	might_sleep();
-	return trylock_page(page) || __lock_page_or_retry(page, mm, flags);
+
+	if (do_raw_trylock_page(page) || __lock_page_or_retry(page, mm, flags)) {
+		/*
+		 * acquire() must be after actual lock operation for crosslocks.
+		 * This way a crosslock and other locks can be ordered.
+		 */
+		lock_page_acquire(page, 0);
+		return 1;
+	}
+
+	return 0;
 }
 
 /*
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2b439a5..2e8c679 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1094,6 +1094,7 @@ config PROVE_LOCKING
 	select DEBUG_LOCK_ALLOC
 	select LOCKDEP_CROSSRELEASE
 	select LOCKDEP_COMPLETIONS
+	select LOCKDEP_PAGELOCK
 	select TRACE_IRQFLAGS
 	default n
 	help
@@ -1179,6 +1180,12 @@ config LOCKDEP_COMPLETIONS
 	 A deadlock caused by wait_for_completion() and complete() can be
 	 detected by lockdep using crossrelease feature.
 
+config LOCKDEP_PAGELOCK
+	bool
+	help
+	 PG_locked lock is a kind of crosslock. Using crossrelease feature,
+	 PG_locked lock can work with lockdep.
+
 config BOOTPARAM_LOCKDEP_CROSSRELEASE_FULLSTACK
 	bool "Enable the boot parameter, crossrelease_fullstack"
 	depends on LOCKDEP_CROSSRELEASE
diff --git a/mm/filemap.c b/mm/filemap.c
index 594d73f..870d442 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1099,7 +1099,7 @@ static inline bool clear_bit_unlock_is_negative_byte(long nr, volatile void *mem
  * portably (architectures that do LL/SC can test any bit, while x86 can
  * test the sign bit).
  */
-void unlock_page(struct page *page)
+void do_raw_unlock_page(struct page *page)
 {
 	BUILD_BUG_ON(PG_waiters != 7);
 	page = compound_head(page);
@@ -1107,7 +1107,7 @@ void unlock_page(struct page *page)
 	if (clear_bit_unlock_is_negative_byte(PG_locked, &page->flags))
 		wake_up_page_bit(page, PG_locked);
 }
-EXPORT_SYMBOL(unlock_page);
+EXPORT_SYMBOL(do_raw_unlock_page);
 
 /**
  * end_page_writeback - end writeback against a page
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 77e4d3c..8436b28 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5371,6 +5371,9 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		} else {
 			__init_single_pfn(pfn, zone, nid);
 		}
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+		lock_page_init(pfn_to_page(pfn));
+#endif
 	}
 }
 
-- 
1.9.1

[PATCH v2 2/4] lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked

[Byungchul Park]

Usually PG_locked bit is updated by lock_page() or unlock_page().
However, it can be also updated through __SetPageLocked() or
__ClearPageLockded(). They have to be considered, to get paired between
acquire and release.

Furthermore, e.g. __SetPageLocked() in add_to_page_cache_lru() is called
frequently. We might miss many chances to check deadlock if we ignore it.
Make __Set(__Clear)PageLockded considered as well.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 include/linux/page-flags.h | 30 +++++++++++++++++++++++++++++-
 1 file changed, 29 insertions(+), 1 deletion(-)

diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 584b14c..108d2dd 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -262,7 +262,6 @@ static __always_inline int PageCompound(struct page *page)
 #define TESTSCFLAG_FALSE(uname)						\
 	TESTSETFLAG_FALSE(uname) TESTCLEARFLAG_FALSE(uname)
 
-__PAGEFLAG(Locked, locked, PF_NO_TAIL)
 PAGEFLAG(Waiters, waiters, PF_ONLY_HEAD) __CLEARPAGEFLAG(Waiters, waiters, PF_ONLY_HEAD)
 PAGEFLAG(Error, error, PF_NO_COMPOUND) TESTCLEARFLAG(Error, error, PF_NO_COMPOUND)
 PAGEFLAG(Referenced, referenced, PF_HEAD)
@@ -374,6 +373,35 @@ static __always_inline int PageSwapCache(struct page *page)
 PAGEFLAG(Idle, idle, PF_ANY)
 #endif
 
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+#include <linux/lockdep.h>
+
+TESTPAGEFLAG(Locked, locked, PF_NO_TAIL)
+
+static __always_inline void __SetPageLocked(struct page *page)
+{
+	__set_bit(PG_locked, &PF_NO_TAIL(page, 1)->flags);
+
+	page = compound_head(page);
+	lock_acquire_exclusive((struct lockdep_map *)&page->map, 0, 1, NULL, _RET_IP_);
+}
+
+static __always_inline void __ClearPageLocked(struct page *page)
+{
+	__clear_bit(PG_locked, &PF_NO_TAIL(page, 1)->flags);
+
+	page = compound_head(page);
+	/*
+	 * lock_commit_crosslock() is necessary for crosslock
+	 * when the lock is released, before lock_release().
+	 */
+	lock_commit_crosslock((struct lockdep_map *)&page->map);
+	lock_release((struct lockdep_map *)&page->map, 0, _RET_IP_);
+}
+#else
+__PAGEFLAG(Locked, locked, PF_NO_TAIL)
+#endif
+
 /*
  * On an anonymous page mapped into a user virtual memory area,
  * page->mapping points to its anon_vma, not to a struct address_space;
-- 
1.9.1

[PATCH v2 3/4] lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext

[Byungchul Park]

CONFIG_LOCKDEP_PAGELOCK needs to keep lockdep_map_cross per page. Since
it's a debug feature, it's preferred to keep it in struct page_ext
rather than struct page. Move it to struct page_ext.

Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 include/linux/mm_types.h   |  4 ---
 include/linux/page-flags.h | 19 ++++++++--
 include/linux/pagemap.h    | 36 +++++++++++++++----
 lib/Kconfig.debug          |  1 +
 mm/filemap.c               | 87 ++++++++++++++++++++++++++++++++++++++++++++++
 mm/page_alloc.c            |  3 --
 mm/page_ext.c              |  4 +++
 7 files changed, 138 insertions(+), 16 deletions(-)

diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h
index 263b861..bc52a4a 100644
--- a/include/linux/mm_types.h
+++ b/include/linux/mm_types.h
@@ -222,10 +222,6 @@ struct page {
 #ifdef LAST_CPUPID_NOT_IN_PAGE_FLAGS
 	int _last_cpupid;
 #endif
-
-#ifdef CONFIG_LOCKDEP_PAGELOCK
-	struct lockdep_map_cross map;
-#endif
 }
 /*
  * The struct page can be forced to be double word aligned so that atomic ops
diff --git a/include/linux/page-flags.h b/include/linux/page-flags.h
index 108d2dd..32ae372 100644
--- a/include/linux/page-flags.h
+++ b/include/linux/page-flags.h
@@ -375,28 +375,41 @@ static __always_inline int PageSwapCache(struct page *page)
 
 #ifdef CONFIG_LOCKDEP_PAGELOCK
 #include <linux/lockdep.h>
+extern struct lockdep_map *get_page_map(struct page *p);
 
 TESTPAGEFLAG(Locked, locked, PF_NO_TAIL)
 
 static __always_inline void __SetPageLocked(struct page *page)
 {
+	struct lockdep_map *m;
+
 	__set_bit(PG_locked, &PF_NO_TAIL(page, 1)->flags);
 
 	page = compound_head(page);
-	lock_acquire_exclusive((struct lockdep_map *)&page->map, 0, 1, NULL, _RET_IP_);
+	m = get_page_map(page);
+	if (unlikely(!m))
+		return;
+
+	lock_acquire_exclusive(m, 0, 1, NULL, _RET_IP_);
 }
 
 static __always_inline void __ClearPageLocked(struct page *page)
 {
+	struct lockdep_map *m;
+
 	__clear_bit(PG_locked, &PF_NO_TAIL(page, 1)->flags);
 
 	page = compound_head(page);
+	m = get_page_map(page);
+	if (unlikely(!m))
+		return;
+
 	/*
 	 * lock_commit_crosslock() is necessary for crosslock
 	 * when the lock is released, before lock_release().
 	 */
-	lock_commit_crosslock((struct lockdep_map *)&page->map);
-	lock_release((struct lockdep_map *)&page->map, 0, _RET_IP_);
+	lock_commit_crosslock(m);
+	lock_release(m, 0, _RET_IP_);
 }
 #else
 __PAGEFLAG(Locked, locked, PF_NO_TAIL)
diff --git a/include/linux/pagemap.h b/include/linux/pagemap.h
index 35b4f67..6722ef7 100644
--- a/include/linux/pagemap.h
+++ b/include/linux/pagemap.h
@@ -17,6 +17,7 @@
 #include <linux/hugetlb_inline.h>
 #ifdef CONFIG_LOCKDEP_PAGELOCK
 #include <linux/lockdep.h>
+#include <linux/page_ext.h>
 #endif
 
 /*
@@ -461,34 +462,57 @@ static inline pgoff_t linear_page_index(struct vm_area_struct *vma,
 }
 
 #ifdef CONFIG_LOCKDEP_PAGELOCK
+extern struct page_ext_operations lockdep_pagelock_ops;
+extern struct lockdep_map *get_page_map(struct page *page);
+
 #define lock_page_init(p)						\
 do {									\
 	static struct lock_class_key __key;				\
-	lockdep_init_map_crosslock((struct lockdep_map *)&(p)->map,	\
-			"(PG_locked)" #p, &__key, 0);			\
+	struct lockdep_map *m = get_page_map(p);			\
+									\
+	if (unlikely(!m))						\
+		break;							\
+									\
+	lockdep_init_map_crosslock(m, "(PG_locked)" #p, &__key, 0);	\
 } while (0)
 
 static inline void lock_page_acquire(struct page *page, int try)
 {
+	struct lockdep_map *m;
+
 	page = compound_head(page);
-	lock_acquire_exclusive((struct lockdep_map *)&page->map, 0,
-			       try, NULL, _RET_IP_);
+	m = get_page_map(page);
+	if (unlikely(!m))
+		return;
+
+	lock_acquire_exclusive(m, 0, try, NULL, _RET_IP_);
 }
 
 static inline void lock_page_release(struct page *page)
 {
+	struct lockdep_map *m;
+
 	page = compound_head(page);
+	m = get_page_map(page);
+	if (unlikely(!m))
+		return;
+
 	/*
 	 * lock_commit_crosslock() is necessary for crosslocks.
 	 */
-	lock_commit_crosslock((struct lockdep_map *)&page->map);
-	lock_release((struct lockdep_map *)&page->map, 0, _RET_IP_);
+	lock_commit_crosslock(m);
+	lock_release(m, 0, _RET_IP_);
 }
 #else
 static inline void lock_page_init(struct page *page) {}
 static inline void lock_page_free(struct page *page) {}
 static inline void lock_page_acquire(struct page *page, int try) {}
 static inline void lock_page_release(struct page *page) {}
+
+static inline struct lockdep_map *get_page_map(struct page *page)
+{
+	return NULL;
+}
 #endif
 
 extern void __lock_page(struct page *page);
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 2e8c679..45fdb3a 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1182,6 +1182,7 @@ config LOCKDEP_COMPLETIONS
 
 config LOCKDEP_PAGELOCK
 	bool
+	select PAGE_EXTENSION
 	help
 	 PG_locked lock is a kind of crosslock. Using crossrelease feature,
 	 PG_locked lock can work with lockdep.
diff --git a/mm/filemap.c b/mm/filemap.c
index 870d442..34251fb 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -36,6 +36,9 @@
 #include <linux/memcontrol.h>
 #include <linux/cleancache.h>
 #include <linux/rmap.h>
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+#include <linux/page_ext.h>
+#endif
 #include "internal.h"
 
 #define CREATE_TRACE_POINTS
@@ -1226,6 +1229,90 @@ int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
 	}
 }
 
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+
+static bool need_lockdep_pagelock(void)
+{
+	return true;
+}
+
+static void init_pages_in_zone(pg_data_t *pgdat, struct zone *zone)
+{
+	struct page *page;
+	unsigned long pfn = zone->zone_start_pfn;
+	unsigned long end_pfn = pfn + zone->spanned_pages;
+	unsigned long count = 0;
+
+	for (; pfn < end_pfn; pfn++) {
+		if (!pfn_valid(pfn)) {
+			pfn = ALIGN(pfn + 1, MAX_ORDER_NR_PAGES);
+			continue;
+		}
+
+		if (!pfn_valid_within(pfn))
+			continue;
+
+		page = pfn_to_page(pfn);
+
+		if (page_zone(page) != zone)
+			continue;
+
+		lock_page_init(page);
+
+		if (get_page_map(page))
+			count++;
+	}
+
+	pr_info("Node %d, zone %8s: lockdep pagelock found early allocated %lu pages\n",
+		pgdat->node_id, zone->name, count);
+}
+
+static void init_zones_in_node(pg_data_t *pgdat)
+{
+	struct zone *zone;
+	struct zone *node_zones = pgdat->node_zones;
+	unsigned long flags;
+
+	for (zone = node_zones; zone - node_zones < MAX_NR_ZONES; ++zone) {
+		if (!populated_zone(zone))
+			continue;
+
+		spin_lock_irqsave(&zone->lock, flags);
+		init_pages_in_zone(pgdat, zone);
+		spin_unlock_irqrestore(&zone->lock, flags);
+	}
+}
+
+static void init_lockdep_pagelock(void)
+{
+	pg_data_t *pgdat;
+	for_each_online_pgdat(pgdat)
+		init_zones_in_node(pgdat);
+}
+
+struct page_ext_operations lockdep_pagelock_ops = {
+	.size = sizeof(struct lockdep_map_cross),
+	.need = need_lockdep_pagelock,
+	.init = init_lockdep_pagelock,
+};
+
+/*
+ * Even though we reserved a space sized of struct lockdep_map_cross,
+ * we only return it as struct lockdep_map, because a full instance of
+ * lockdep_map_cross is only for lockdep cross-release internal.
+ */
+struct lockdep_map *get_page_map(struct page *p)
+{
+	struct page_ext *e;
+
+	e = lookup_page_ext(p);
+	if (!e)
+		return NULL;
+
+	return (void *)e + lockdep_pagelock_ops.offset;
+}
+#endif
+
 /**
  * page_cache_next_hole - find the next hole (not-present entry)
  * @mapping: mapping
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8436b28..77e4d3c 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -5371,9 +5371,6 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone,
 		} else {
 			__init_single_pfn(pfn, zone, nid);
 		}
-#ifdef CONFIG_LOCKDEP_PAGELOCK
-		lock_page_init(pfn_to_page(pfn));
-#endif
 	}
 }
 
diff --git a/mm/page_ext.c b/mm/page_ext.c
index 4f0367d..63ae336 100644
--- a/mm/page_ext.c
+++ b/mm/page_ext.c
@@ -8,6 +8,7 @@
 #include <linux/kmemleak.h>
 #include <linux/page_owner.h>
 #include <linux/page_idle.h>
+#include <linux/pagemap.h>
 
 /*
  * struct page extension
@@ -66,6 +67,9 @@
 #if defined(CONFIG_IDLE_PAGE_TRACKING) && !defined(CONFIG_64BIT)
 	&page_idle_ops,
 #endif
+#ifdef CONFIG_LOCKDEP_PAGELOCK
+	&lockdep_pagelock_ops,
+#endif
 };
 
 static unsigned long total_usage;
-- 
1.9.1

[PATCH v2 4/4] lockdep: Add a boot parameter enabling to track page locks using lockdep and disable it by default

[Byungchul Park]

To track page locks using lockdep, we need a huge memory space for
lockdep_map per page. So, it would be better to make it disabled by
default and provide a boot parameter to turn it on. Do it.

Suggested-by: Michal Hocko <mhocko@kernel.org>
Signed-off-by: Byungchul Park <byungchul.park@lge.com>
---
 Documentation/admin-guide/kernel-parameters.txt |  7 +++++++
 lib/Kconfig.debug                               |  5 ++++-
 mm/filemap.c                                    | 23 +++++++++++++++++++++++
 3 files changed, 34 insertions(+), 1 deletion(-)

diff --git a/Documentation/admin-guide/kernel-parameters.txt b/Documentation/admin-guide/kernel-parameters.txt
index f20ed5e..5e8d15d 100644
--- a/Documentation/admin-guide/kernel-parameters.txt
+++ b/Documentation/admin-guide/kernel-parameters.txt
@@ -712,6 +712,13 @@
 	crossrelease_fullstack
 			[KNL] Allow to record full stack trace in cross-release
 
+	lockdep_pagelock=
+			[KNL] Boot-time lockdep_pagelock enabling option.
+			Storage of lockdep_map per page to track lock_page()/
+			unlock_page() is disabled by default. With this switch,
+			we can turn it on.
+			on: enable the feature
+
 	cryptomgr.notests
                         [KNL] Disable crypto self-tests
 
diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 45fdb3a..c609e97 100644
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -1185,7 +1185,10 @@ config LOCKDEP_PAGELOCK
 	select PAGE_EXTENSION
 	help
 	 PG_locked lock is a kind of crosslock. Using crossrelease feature,
-	 PG_locked lock can work with lockdep.
+	 PG_locked lock can work with lockdep. Even if you include this
+	 feature on your build, it is disabled in default. You should pass
+	 "lockdep_pagelock=on" to boot parameter in order to enable it. It
+	 consumes a fair amount of memory if enabled.
 
 config BOOTPARAM_LOCKDEP_CROSSRELEASE_FULLSTACK
 	bool "Enable the boot parameter, crossrelease_fullstack"
diff --git a/mm/filemap.c b/mm/filemap.c
index 34251fb..cb7b20b 100644
--- a/mm/filemap.c
+++ b/mm/filemap.c
@@ -1231,8 +1231,24 @@ int __lock_page_or_retry(struct page *page, struct mm_struct *mm,
 
 #ifdef CONFIG_LOCKDEP_PAGELOCK
 
+static int lockdep_pagelock;
+static int __init allow_lockdep_pagelock(char *str)
+{
+	if (!str)
+		return -EINVAL;
+
+	if (!strcmp(str, "on"))
+		lockdep_pagelock = 1;
+
+	return 0;
+}
+early_param("lockdep_pagelock", allow_lockdep_pagelock);
+
 static bool need_lockdep_pagelock(void)
 {
+	if (!lockdep_pagelock)
+		return false;
+
 	return true;
 }
 
@@ -1286,6 +1302,10 @@ static void init_zones_in_node(pg_data_t *pgdat)
 static void init_lockdep_pagelock(void)
 {
 	pg_data_t *pgdat;
+
+	if (!lockdep_pagelock)
+		return;
+
 	for_each_online_pgdat(pgdat)
 		init_zones_in_node(pgdat);
 }
@@ -1305,6 +1325,9 @@ struct lockdep_map *get_page_map(struct page *p)
 {
 	struct page_ext *e;
 
+	if (!lockdep_pagelock)
+		return NULL;
+
 	e = lookup_page_ext(p);
 	if (!e)
 		return NULL;
-- 
1.9.1

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Matthew Wilcox]

On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:
> For now, wait_for_completion() / complete() works with lockdep, add
> lock_page() / unlock_page() and its family to lockdep support.
> 
> Changes from v1
>  - Move lockdep_map_cross outside of page_ext to make it flexible
>  - Prevent allocating lockdep_map per page by default
>  - Add a boot parameter allowing the allocation for debugging
> 
> Byungchul Park (4):
>   lockdep: Apply crossrelease to PG_locked locks
>   lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
>   lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
>   lockdep: Add a boot parameter enabling to track page locks using
>     lockdep and disable it by default

I don't like the way you've structured this patch series; first adding
the lockdep map to struct page, then moving it to page_ext.

I also don't like it that you've made CONFIG_LOCKDEP_PAGELOCK not
individually selectable.  I might well want a kernel with crosslock
support, but only for completions.

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Byungchul Park]

On 12/5/2017 2:30 PM, Matthew Wilcox wrote:
> On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:
>> For now, wait_for_completion() / complete() works with lockdep, add
>> lock_page() / unlock_page() and its family to lockdep support.
>>
>> Changes from v1
>>   - Move lockdep_map_cross outside of page_ext to make it flexible
>>   - Prevent allocating lockdep_map per page by default
>>   - Add a boot parameter allowing the allocation for debugging
>>
>> Byungchul Park (4):
>>    lockdep: Apply crossrelease to PG_locked locks
>>    lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
>>    lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
>>    lockdep: Add a boot parameter enabling to track page locks using
>>      lockdep and disable it by default
> 
> I don't like the way you've structured this patch series; first adding
> the lockdep map to struct page, then moving it to page_ext.

Hello,

I will make them into one patch.

> I also don't like it that you've made CONFIG_LOCKDEP_PAGELOCK not
> individually selectable.  I might well want a kernel with crosslock
> support, but only for completions.

OK then, I will make it individually selectable.

I want to know others' opinions as well.

Thank you for the opinions. I will apply yours next spin.

-- 
Thanks,
Byungchul

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Byungchul Park]

On 12/5/2017 2:46 PM, Byungchul Park wrote:
> On 12/5/2017 2:30 PM, Matthew Wilcox wrote:
>> On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:
>>> For now, wait_for_completion() / complete() works with lockdep, add
>>> lock_page() / unlock_page() and its family to lockdep support.
>>>
>>> Changes from v1
>>>   - Move lockdep_map_cross outside of page_ext to make it flexible
>>>   - Prevent allocating lockdep_map per page by default
>>>   - Add a boot parameter allowing the allocation for debugging
>>>
>>> Byungchul Park (4):
>>>    lockdep: Apply crossrelease to PG_locked locks
>>>    lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
>>>    lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
>>>    lockdep: Add a boot parameter enabling to track page locks using
>>>      lockdep and disable it by default
>>
>> I don't like the way you've structured this patch series; first adding
>> the lockdep map to struct page, then moving it to page_ext.
> 
> Hello,
> 
> I will make them into one patch.

I've thought it more.

Actually I did it because I thought I'd better make it into two
patches since it makes reviewers easier to review. It doesn't matter
which one I choose, but I prefer to split it.

But, if you are strongly against it, then I will follow you.

-- 
Thanks,
Byungchul

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Matthew Wilcox]

On Tue, Dec 05, 2017 at 03:19:46PM +0900, Byungchul Park wrote:
> On 12/5/2017 2:46 PM, Byungchul Park wrote:
> > On 12/5/2017 2:30 PM, Matthew Wilcox wrote:
> > > On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:
> > > > For now, wait_for_completion() / complete() works with lockdep, add
> > > > lock_page() / unlock_page() and its family to lockdep support.
> > > > 
> > > > Changes from v1
> > > >   - Move lockdep_map_cross outside of page_ext to make it flexible
> > > >   - Prevent allocating lockdep_map per page by default
> > > >   - Add a boot parameter allowing the allocation for debugging
> > > > 
> > > > Byungchul Park (4):
> > > >    lockdep: Apply crossrelease to PG_locked locks
> > > >    lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
> > > >    lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
> > > >    lockdep: Add a boot parameter enabling to track page locks using
> > > >      lockdep and disable it by default
> > > 
> > > I don't like the way you've structured this patch series; first adding
> > > the lockdep map to struct page, then moving it to page_ext.
> > 
> > Hello,
> > 
> > I will make them into one patch.
> 
> I've thought it more.
> 
> Actually I did it because I thought I'd better make it into two
> patches since it makes reviewers easier to review. It doesn't matter
> which one I choose, but I prefer to split it.

I don't know whether it's better to make it all one patch or split it
into multiple patches.  But it makes no sense to introduce it in struct
page, then move it to struct page_ext.

Re: [PATCH v2 0/4] lockdep/crossrelease: Apply crossrelease to page locks

[Michal Hocko]

On Mon 04-12-17 23:45:17, Matthew Wilcox wrote:
> On Tue, Dec 05, 2017 at 03:19:46PM +0900, Byungchul Park wrote:
> > On 12/5/2017 2:46 PM, Byungchul Park wrote:
> > > On 12/5/2017 2:30 PM, Matthew Wilcox wrote:
> > > > On Mon, Dec 04, 2017 at 02:16:19PM +0900, Byungchul Park wrote:
> > > > > For now, wait_for_completion() / complete() works with lockdep, add
> > > > > lock_page() / unlock_page() and its family to lockdep support.
> > > > > 
> > > > > Changes from v1
> > > > >   - Move lockdep_map_cross outside of page_ext to make it flexible
> > > > >   - Prevent allocating lockdep_map per page by default
> > > > >   - Add a boot parameter allowing the allocation for debugging
> > > > > 
> > > > > Byungchul Park (4):
> > > > >    lockdep: Apply crossrelease to PG_locked locks
> > > > >    lockdep: Apply lock_acquire(release) on __Set(__Clear)PageLocked
> > > > >    lockdep: Move data of CONFIG_LOCKDEP_PAGELOCK from page to page_ext
> > > > >    lockdep: Add a boot parameter enabling to track page locks using
> > > > >      lockdep and disable it by default
> > > > 
> > > > I don't like the way you've structured this patch series; first adding
> > > > the lockdep map to struct page, then moving it to page_ext.
> > > 
> > > Hello,
> > > 
> > > I will make them into one patch.
> > 
> > I've thought it more.
> > 
> > Actually I did it because I thought I'd better make it into two
> > patches since it makes reviewers easier to review. It doesn't matter
> > which one I choose, but I prefer to split it.
> 
> I don't know whether it's better to make it all one patch or split it
> into multiple patches.  But it makes no sense to introduce it in struct
> page, then move it to struct page_ext.

I would tend to agree. It is not like anybody would like to apply only
the first part alone. Adding the necessary infrastructure to page_ext is
not such a big deal.

-- 
Michal Hocko
SUSE Labs