diff mbox series

mm: workingset: replace IRQ-off check with a lockdep assert.

Message ID 20190211095724.nmflaigqlcipbxtk@linutronix.de (mailing list archive)
State New, archived
Headers show
Series mm: workingset: replace IRQ-off check with a lockdep assert. | expand

Commit Message

Sebastian Andrzej Siewior Feb. 11, 2019, 9:57 a.m. UTC
Commit

  68d48e6a2df57 ("mm: workingset: add vmstat counter for shadow nodes")

introduced an IRQ-off check to ensure that a lock is held which also
disabled interrupts. This does not work the same way on -RT because none
of the locks, that are held, disable interrupts.
Replace this check with a lockdep assert which ensures that the lock is
held.

Signed-off-by: Sebastian Andrzej Siewior <bigeasy@linutronix.de>
---
 mm/workingset.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

kernel test robot Feb. 11, 2019, 5:07 p.m. UTC | #1
Hi Sebastian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190211]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Andrzej-Siewior/mm-workingset-replace-IRQ-off-check-with-a-lockdep-assert/20190212-001418
config: openrisc-or1ksim_defconfig (attached as .config)
compiler: or1k-linux-gcc (GCC) 6.0.0 20160327 (experimental)
reproduce:
        wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
        chmod +x ~/bin/make.cross
        # save the attached .config to linux build tree
        make.cross ARCH=openrisc 

All errors (new ones prefixed by >>):

   mm/workingset.c: In function 'workingset_update_node':
>> mm/workingset.c:382:2: error: implicit declaration of function 'lockdep_is_held' [-Werror=implicit-function-declaration]
     lockdep_is_held(&mapping->i_pages.xa_lock);
     ^~~~~~~~~~~~~~~
   cc1: some warnings being treated as errors

vim +/lockdep_is_held +382 mm/workingset.c

   368	
   369	void workingset_update_node(struct xa_node *node)
   370	{
   371		struct address_space *mapping;
   372	
   373		/*
   374		 * Track non-empty nodes that contain only shadow entries;
   375		 * unlink those that contain pages or are being freed.
   376		 *
   377		 * Avoid acquiring the list_lru lock when the nodes are
   378		 * already where they should be. The list_empty() test is safe
   379		 * as node->private_list is protected by the i_pages lock.
   380		 */
   381		mapping = container_of(node->array, struct address_space, i_pages);
 > 382		lockdep_is_held(&mapping->i_pages.xa_lock);
   383	
   384		if (node->count && node->count == node->nr_values) {
   385			if (list_empty(&node->private_list)) {
   386				list_lru_add(&shadow_nodes, &node->private_list);
   387				__inc_lruvec_page_state(virt_to_page(node),
   388							WORKINGSET_NODES);
   389			}
   390		} else {
   391			if (!list_empty(&node->private_list)) {
   392				list_lru_del(&shadow_nodes, &node->private_list);
   393				__dec_lruvec_page_state(virt_to_page(node),
   394							WORKINGSET_NODES);
   395			}
   396		}
   397	}
   398	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
kernel test robot Feb. 11, 2019, 5:37 p.m. UTC | #2
Hi Sebastian,

Thank you for the patch! Yet something to improve:

[auto build test ERROR on linus/master]
[also build test ERROR on v5.0-rc4 next-20190211]
[if your patch is applied to the wrong git tree, please drop us a note to help improve the system]

url:    https://github.com/0day-ci/linux/commits/Sebastian-Andrzej-Siewior/mm-workingset-replace-IRQ-off-check-with-a-lockdep-assert/20190212-001418
config: i386-randconfig-x076-201906 (attached as .config)
compiler: gcc-8 (Debian 8.2.0-20) 8.2.0
reproduce:
        # save the attached .config to linux build tree
        make ARCH=i386 

All errors (new ones prefixed by >>):

   mm/workingset.c: In function 'workingset_update_node':
>> mm/workingset.c:382:2: error: implicit declaration of function 'lockdep_is_held'; did you mean 'lockdep_is_held_type'? [-Werror=implicit-function-declaration]
     lockdep_is_held(&mapping->i_pages.xa_lock);
     ^~~~~~~~~~~~~~~
     lockdep_is_held_type
   cc1: some warnings being treated as errors

vim +382 mm/workingset.c

   368	
   369	void workingset_update_node(struct xa_node *node)
   370	{
   371		struct address_space *mapping;
   372	
   373		/*
   374		 * Track non-empty nodes that contain only shadow entries;
   375		 * unlink those that contain pages or are being freed.
   376		 *
   377		 * Avoid acquiring the list_lru lock when the nodes are
   378		 * already where they should be. The list_empty() test is safe
   379		 * as node->private_list is protected by the i_pages lock.
   380		 */
   381		mapping = container_of(node->array, struct address_space, i_pages);
 > 382		lockdep_is_held(&mapping->i_pages.xa_lock);
   383	
   384		if (node->count && node->count == node->nr_values) {
   385			if (list_empty(&node->private_list)) {
   386				list_lru_add(&shadow_nodes, &node->private_list);
   387				__inc_lruvec_page_state(virt_to_page(node),
   388							WORKINGSET_NODES);
   389			}
   390		} else {
   391			if (!list_empty(&node->private_list)) {
   392				list_lru_del(&shadow_nodes, &node->private_list);
   393				__dec_lruvec_page_state(virt_to_page(node),
   394							WORKINGSET_NODES);
   395			}
   396		}
   397	}
   398	

---
0-DAY kernel test infrastructure                Open Source Technology Center
https://lists.01.org/pipermail/kbuild-all                   Intel Corporation
diff mbox series

Patch

diff --git a/mm/workingset.c b/mm/workingset.c
index dcb994f2acc2e..c75d10d48be16 100644
--- a/mm/workingset.c
+++ b/mm/workingset.c
@@ -368,6 +368,8 @@  static struct list_lru shadow_nodes;
 
 void workingset_update_node(struct xa_node *node)
 {
+	struct address_space *mapping;
+
 	/*
 	 * Track non-empty nodes that contain only shadow entries;
 	 * unlink those that contain pages or are being freed.
@@ -376,7 +378,8 @@  void workingset_update_node(struct xa_node *node)
 	 * already where they should be. The list_empty() test is safe
 	 * as node->private_list is protected by the i_pages lock.
 	 */
-	VM_WARN_ON_ONCE(!irqs_disabled());  /* For __inc_lruvec_page_state */
+	mapping = container_of(node->array, struct address_space, i_pages);
+	lockdep_is_held(&mapping->i_pages.xa_lock);
 
 	if (node->count && node->count == node->nr_values) {
 		if (list_empty(&node->private_list)) {