Re: [LKP] [lkp] [xfs] 68a9f5e700: aim7.jobs-per-min -13.6% regression

From: Linus Torvalds
Date: Thu Aug 11 2016 - 16:35:07 EST


On Thu, Aug 11, 2016 at 1:00 PM, Christoph Hellwig <hch@xxxxxx> wrote:
>
> I can't really think of any reason why the pagefaul_disable() would
> sÑgnificantly change performance.

No, you're right, we prefault the page anyway.

And quite frankly, looking at it, I think the pagefault_disable/enable
is actually *correct*.

The thing is, iov_iter_copy_from_user_atomic() doesn't itself enforce
non-blocking user accesses, it depends on the caller blocking page
faults.

And the reason we want to block page faults is to make sure we don't
deadlock on the page we just locked for writing.

So looking at it, I think the pagefault_disable/enable is actually
needed, and it may in fact be a bug that mm/filemap.c doesn't do that.

However, see commit 00a3d660cbac ("Revert "fs: do not prefault
sys_write() user buffer pages"") about why mm/filemap.c doesn't do the
pagefault_disable().

But depending on the prefaulting actually guaranteeing that we don't
deadlock sounds like a nasty race in theory (ie somebody does mmap
tricks in another thread in between the pre-faulting and the final
copying).

Linus