Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   David Laight <David.Laight@ACULAB.COM>
To:     'Baoquan He' <bhe@redhat.com>, David Hildenbrand <david@redhat.com>
CC:     Lorenzo Stoakes <lstoakes@gmail.com>,
        "linux-mm@kvack.org" <linux-mm@kvack.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "linux-fsdevel@vger.kernel.org" <linux-fsdevel@vger.kernel.org>,
        Andrew Morton <akpm@linux-foundation.org>,
        Uladzislau Rezki <urezki@gmail.com>,
        Matthew Wilcox <willy@infradead.org>,
        Liu Shixin <liushixin2@huawei.com>,
        Jiri Olsa <jolsa@kernel.org>, Jens Axboe <axboe@kernel.dk>,
        Alexander Viro <viro@zeniv.linux.org.uk>
Subject: RE: [PATCH v7 4/4] mm: vmalloc: convert vread() to vread_iter()
Thread-Topic: [PATCH v7 4/4] mm: vmalloc: convert vread() to vread_iter()
Thread-Index: AQHZXYvy5HgRBBUkm0qsuDBYF9y6k68ND8zA
Date:   Sun, 26 Mar 2023 13:26:57 +0000
Message-ID: <0cff573c3a344504b1b1b77486b4d853@AcuMS.aculab.com>
References: <cover.1679511146.git.lstoakes@gmail.com>
 <941f88bc5ab928e6656e1e2593b91bf0f8c81e1b.1679511146.git.lstoakes@gmail.com>
 <ZBu+2cPCQvvFF/FY@MiWiFi-R3L-srv>
 <ff630c2e-42ff-42ec-9abb-38922d5107ec@lucifer.local>
 <ZBwroYh22pEqJYhv@MiWiFi-R3L-srv>
 <7aee68e9-6e31-925f-68bc-73557c032a42@redhat.com>
 <ZBxUvBFHcQvsl0r9@MiWiFi-R3L-srv>
In-Reply-To: <ZBxUvBFHcQvsl0r9@MiWiFi-R3L-srv>
Accept-Language: en-GB, en-US
MIME-Version: 1.0
Content-Language: en-US
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8BIT
Precedence: bulk

From: Baoquan He
> Sent: 23 March 2023 13:32
...
> > > > If this fails, then we fault in, and try again. We loop because there could
> > > > be some extremely unfortunate timing with a race on e.g. swapping out or
> > > > migrating pages between faulting in and trying to write out again.
> > > >
> > > > This is extremely unlikely, but to avoid any chance of breaking userland we
> > > > repeat the operation until it completes. In nearly all real-world
> > > > situations it'll either work immediately or loop once.
> > >
> > > Thanks a lot for these helpful details with patience. I got it now. I was
> > > mainly confused by the while(true) loop in KCORE_VMALLOC case of read_kcore_iter.
> > >
> > > Now is there any chance that the faulted in memory is swapped out or
> > > migrated again before vread_iter()? fault_in_iov_iter_writeable() will
> > > pin the memory? I didn't find it from code and document. Seems it only
> > > falults in memory. If yes, there's window between faluting in and
> > > copy_to_user_nofault().
> > >
> >
> > See the documentation of fault_in_safe_writeable():
> >
> > "Note that we don't pin or otherwise hold the pages referenced that we fault
> > in.  There's no guarantee that they'll stay in memory for any duration of
> > time."
> 
> Thanks for the info. Then swapping out/migration could happen again, so
> that's why while(true) loop is meaningful.

One of the problems is that is the system is under severe memory
pressure and you try to fault in (say) 20 pages, the first page
might get unmapped in order to map the last one in.

So it is quite likely better to retry 'one page at a time'.

There have also been cases where the instruction to copy data
has faulted for reasons other than 'page fault'.
ISTR an infinite loop being caused by misaligned accesses failing
due to 'bad instruction choice' in the copy code.
While this is rally a bug, an infinite retry in a file read/write
didn't make it easy to spot.

So maybe there are cases where a dropping back to a 'bounce buffer'
may be necessary.

	David

-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)