linux-kernel - Re: [PATCH] mm/migrate.c: Rework migration_entry

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [day] [month] [year] [list]
Date:   Mon, 8 Nov 2021 18:21:20 +1100
From:   Alistair Popple <apopple@...dia.com>
To:     "akpm@...ux-foundation.org" <akpm@...ux-foundation.org>,
        kernel test robot <lkp@...el.com>
CC:     "llvm@...ts.linux.dev" <llvm@...ts.linux.dev>,
        "kbuild-all@...ts.01.org" <kbuild-all@...ts.01.org>,
        "willy@...radead.org" <willy@...radead.org>,
        "dhowells@...hat.com" <dhowells@...hat.com>,
        "hughd@...gle.com" <hughd@...gle.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "jglisse@...hat.com" <jglisse@...hat.com>,
        Jason Gunthorpe <jgg@...dia.com>,
        Ralph Campbell <rcampbell@...dia.com>,
        John Hubbard <jhubbard@...dia.com>
Subject: Re: [PATCH] mm/migrate.c: Rework migration_entry_wait() to not take a pageref

Got this after sending v2 but it will have the same problem. This occurs for
CONFIG_MMU=n which seems to be broken anyway due to other arch build errors (at
least for Arm SA1100 based builds). Fixing this is easy enough though - only
defining migration_entry_wait_on_locked() when CONFIG_MIGRATION=y fixes this
and is probably a good idea anyway.

I will wait a bit for feedback on v2 before sending v3 with this fix.

 - Alistair

On Friday, 5 November 2021 8:50:08 PM AEDT kernel test robot wrote:
> Hi Alistair,
> 
> Thank you for the patch! Yet something to improve:
> 
> [auto build test ERROR on linux/master]
> [also build test ERROR on v5.15]
> [cannot apply to hnaz-mm/master linus/master next-20211105]
> [If your patch is applied to the wrong git tree, kindly drop us a note.
> And when submitting patch, we suggest to use '--base' as documented in
> https://git-scm.com/docs/git-format-patch]
> 
> url:    https://github.com/0day-ci/linux/commits/Alistair-Popple/mm-migrate-c-Rework-migration_entry_wait-to-not-take-a-pageref/20211104-183442
> base:   https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 2f111a6fd5b5297b4e92f53798ca086f7c7d33a4
> config: arm-randconfig-r026-20211105 (attached as .config)
> compiler: clang version 14.0.0 (https://github.com/llvm/llvm-project 847a6807332b13f43704327c2d30103ec0347c77)
> reproduce (this is a W=1 build):
>         wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross
>         chmod +x ~/bin/make.cross
>         # install arm cross compiling tool for clang build
>         # apt-get install binutils-arm-linux-gnueabi
>         # https://github.com/0day-ci/linux/commit/e9447498f8f8758741f3dae044c3e4593130595c
>         git remote add linux-review https://github.com/0day-ci/linux
>         git fetch --no-tags linux-review Alistair-Popple/mm-migrate-c-Rework-migration_entry_wait-to-not-take-a-pageref/20211104-183442
>         git checkout e9447498f8f8758741f3dae044c3e4593130595c
>         # save the attached .config to linux build tree
>         COMPILER_INSTALL_PATH=$HOME/0day COMPILER=clang make.cross W=1 ARCH=arm 
> 
> If you fix the issue, kindly add following tag as appropriate
> Reported-by: kernel test robot <lkp@...el.com>
> 
> All errors (new ones prefixed by >>):
> 
> >> mm/filemap.c:1468:2: error: implicit declaration of function 'pte_unmap' [-Werror,-Wimplicit-function-declaration]
>            pte_unmap_unlock(ptep, ptl);
>            ^
>    include/linux/mm.h:2275:2: note: expanded from macro 'pte_unmap_unlock'
>            pte_unmap(pte);                                 \
>            ^
>    1 error generated.
> 
> 
> vim +/pte_unmap +1468 mm/filemap.c
> 
>   1413	
>   1414	/**
>   1415	 * migration_entry_wait_on_locked - Wait for a migration entry to be removed
>   1416	 * @page: page referenced by the migration entry.
>   1417	 * @ptep: mapped pte pointer. This function will return with the ptep unmapped.
>   1418	 * @ptl: already locked ptl. This function will drop the lock.
>   1419	 *
>   1420	 * Wait for a migration entry referencing the given page to be removed. This is
>   1421	 * equivalent to put_and_wait_on_page_locked(page, TASK_UNINTERRUPTIBLE) except
>   1422	 * this can be called without taking a reference on the page. Instead this
>   1423	 * should be called while holding the ptl for the migration entry referencing
>   1424	 * the page.
>   1425	 *
>   1426	 * Returns after unmapping and unlocking the pte/ptl with pte_unmap_unlock().
>   1427	 *
>   1428	 * This follows the same logic as wait_on_page_bit_common() so see the comments
>   1429	 * there.
>   1430	 */
>   1431	void migration_entry_wait_on_locked(struct page *page, pte_t *ptep,
>   1432					spinlock_t *ptl)
>   1433	{
>   1434		struct wait_page_queue wait_page;
>   1435		wait_queue_entry_t *wait = &wait_page.wait;
>   1436		bool thrashing = false;
>   1437		bool delayacct = false;
>   1438		unsigned long pflags;
>   1439		wait_queue_head_t *q;
>   1440	
>   1441		q = page_waitqueue(page);
>   1442		if (!PageUptodate(page) && PageWorkingset(page)) {
>   1443			if (!PageSwapBacked(page)) {
>   1444				delayacct_thrashing_start();
>   1445				delayacct = true;
>   1446			}
>   1447			psi_memstall_enter(&pflags);
>   1448			thrashing = true;
>   1449		}
>   1450	
>   1451		init_wait(wait);
>   1452		wait->func = wake_page_function;
>   1453		wait_page.page = page;
>   1454		wait_page.bit_nr = PG_locked;
>   1455		wait->flags = 0;
>   1456	
>   1457		spin_lock_irq(&q->lock);
>   1458		SetPageWaiters(page);
>   1459		if (!trylock_page_bit_common(page, PG_locked, wait))
>   1460			__add_wait_queue_entry_tail(q, wait);
>   1461		spin_unlock_irq(&q->lock);
>   1462	
>   1463		/*
>   1464		 * If a migration entry exists for the page the migration path must hold
>   1465		 * a valid reference to the page, and it must take the ptl to remove the
>   1466		 * migration entry. So the page is valid until the ptl is dropped.
>   1467		 */
> > 1468		pte_unmap_unlock(ptep, ptl);
>   1469	
>   1470		for (;;) {
>   1471			unsigned int flags;
>   1472	
>   1473			set_current_state(TASK_UNINTERRUPTIBLE);
>   1474	
>   1475			/* Loop until we've been woken or interrupted */
>   1476			flags = smp_load_acquire(&wait->flags);
>   1477			if (!(flags & WQ_FLAG_WOKEN)) {
>   1478				if (signal_pending_state(TASK_UNINTERRUPTIBLE, current))
>   1479					break;
>   1480	
>   1481				io_schedule();
>   1482				continue;
>   1483			}
>   1484			break;
>   1485		}
>   1486	
>   1487		finish_wait(q, wait);
>   1488	
>   1489		if (thrashing) {
>   1490			if (delayacct)
>   1491				delayacct_thrashing_end();
>   1492			psi_memstall_leave(&pflags);
>   1493		}
>   1494	}
>   1495	
> 
> ---
> 0-DAY CI Kernel Test Service, Intel Corporation
> https://lists.01.org/hyperkitty/list/kbuild-all@lists.01.org
>