Date:   Tue, 21 Feb 2023 04:30:26 +0000
From:   Matthew Wilcox <willy@infradead.org>
To:     Hugh Dickins <hughd@google.com>
Cc:     "Huang, Ying" <ying.huang@intel.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Jan Kara <jack@suse.cz>, linux-mm@kvack.org,
        linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org,
        Zi Yan <ziy@nvidia.com>, Yang Shi <shy828301@gmail.com>,
        Baolin Wang <baolin.wang@linux.alibaba.com>,
        Oscar Salvador <osalvador@suse.de>,
        Bharata B Rao <bharata@amd.com>,
        Alistair Popple <apopple@nvidia.com>,
        Xin Hao <xhao@linux.alibaba.com>,
        Minchan Kim <minchan@kernel.org>,
        Mike Kravetz <mike.kravetz@oracle.com>,
        Hyeonggon Yoo <42.hyeyoo@gmail.com>,
        "Xu, Pengfei" <pengfei.xu@intel.com>,
        Christoph Hellwig <hch@lst.de>,
        Stefan Roesch <shr@devkernel.io>, Tejun Heo <tj@kernel.org>
Subject: Re: [PATCH -v5 0/9] migrate_pages(): batch TLB flushing
Message-ID: <Y/RI4s45PZ6Bv2ZR@casper.infradead.org>
References: <20230213123444.155149-1-ying.huang@intel.com>
 <87a6c8c-c5c1-67dc-1e32-eb30831d6e3d@google.com>
 <874jrg7kke.fsf@yhuang6-desk2.ccr.corp.intel.com>
 <2ab4b33e-f570-a6ff-6315-7d5a4614a7bd@google.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <2ab4b33e-f570-a6ff-6315-7d5a4614a7bd@google.com>
Precedence: bulk

On Mon, Feb 20, 2023 at 06:48:38PM -0800, Hugh Dickins wrote:
> Yes, that's a good principle, that we should avoid to lock/wait
> synchronously once we have locked one folio (hmm, above you say
> "more than one": I think we mean the same thing, we're just
> stating it differently, given how the code runs at present).

I suspect the migrate page code is disobeying the locking ordering
rules for multiple folios.  if two folios belong to the same file,
they must be locked by folio->index order, low to high.  If two folios
belong to different files, they must be ordered by folio->mapping, the
mapping lowest in memory first.  You can see this locking rule embedded
in vfs_lock_two_folios() in fs/remap_range.c.

I don't know what the locking rules are for two folios which are not file
folios, or for two folios when one is anonymous and the other belongs
to a file.  Maybe it's the same; you can lock them ordered by ->mapping
first, then by ->index.

Or you can just trylock multiple folios and skip the ones that don't work.