Al Viro [off-list ref] wrote:

On Tue, Jun 22, 2021 at 04:20:40PM +0100, David Howells wrote:

quoted

and wondering if the iov_iter_fault_in_readable() is actually effective.
Yes, it can make sure that the page we're intending to modify is dragged
into the pagecache and marked uptodate so that it can be read from, but is
it possible for the page to then get reclaimed before we get to
iov_iter_copy_from_user_atomic()?  a_ops->write_begin() could potentially
take a long time, say if it has to go and get a lock/lease from a server.

Yes, it is.  So what?  We'll just retry.  You *can't* take faults while
holding some pages locked; not without shitloads of deadlocks.

In that case, can we amend the comment immediately above
iov_iter_fault_in_readable()?

	/*
	 * Bring in the user page that we will copy from _first_.
	 * Otherwise there's a nasty deadlock on copying from the
	 * same page as we're writing to, without it being marked
	 * up-to-date.
	 *
	 * Not only is this an optimisation, but it is also required
	 * to check that the address is actually valid, when atomic
	 * usercopies are used, below.
	 */
	if (unlikely(iov_iter_fault_in_readable(i, bytes))) {

The first part suggests this is for deadlock avoidance.  If that's not true,
then this should perhaps be changed.

David