Skip site navigation (1)Skip section navigation (2)
Date:      Sun, 18 Feb 2018 05:26:23 -0800
From:      Gleb Smirnoff <glebius@FreeBSD.org>
To:        Andriy Gapon <avg@FreeBSD.org>
Cc:        Andrew Reilly <areilly@bigpond.net.au>, kib@FreeBSD.org, current@FreeBSD.org
Subject:   Re: Since last week (today) current on my Ryzen box is unstable
Message-ID:  <20180218132623.GF93303@FreeBSD.org>
In-Reply-To: <431f3e00-c66a-8e2e-6c61-a315a6353d1d@FreeBSD.org>
References:  <0CEA9D55-D488-42EC-BBDE-D0B7CE58BAEA@bigpond.net.au> <cc3ae685-5f0e-d968-7b08-60a4836093e1@FreeBSD.org> <20180218023545.GE93303@FreeBSD.org> <431f3e00-c66a-8e2e-6c61-a315a6353d1d@FreeBSD.org>

next in thread | previous in thread | raw e-mail | index | archive | help
On Sun, Feb 18, 2018 at 09:28:30AM +0200, Andriy Gapon wrote:
A> > A> vnode_pager_getpages_async() at vnode_pager_getpages_async+0x81/frame
A> > A> 0xfffffe00b3c36650
A> > A> vn_sendfile() at vn_sendfile+0xe70/frame 0xfffffe00b3c368e0
A> > A> sendfile() at sendfile+0x149/frame 0xfffffe00b3c36980
A> > A> amd64_syscall() at amd64_syscall+0x79b/frame 0xfffffe00b3c36ab0
A> > A> fast_syscall_common() at fast_syscall_common+0x101/frame 0x7fffffffdb00
A> > A> 
A> > A> I looked at sendfile_swapin() code and it seems that it uses the pager API in an
A> > A> undocumented way.  Specifically, it inserts bogus_page into the array of
A> > A> requested pages.  For starters, bogus_page is not busied and VOP_GETPAGES is
A> > A> documented to have all requested pages exclusively busied.  Second, I always had
A> > A> an impression that bogus_page is an implementation detail of the unified buffer
A> > A> / page cache and that other code need not be aware of it.
A> > A> 
A> > A> So, my opinion is that the sendfile code uses a "clever hack" that happens to
A> > A> work with the buffer cache based filesystems, but that that hack is a bug.
A> > A> So, I'd prefer that the problem is fixed in that code.
A> > A> But I am open to being convinced that all VOP_GETPAGES implementations,
A> > A> including that in ZFS, must be made aware of bogus_page.  Or, at least, that
A> > A> they should not verify that the requested pages are busied.
A> > 
A> > This is optimization that improves throughput when file memory cache is
A> > fragmented. Why don't you like adding the code to zfs_freebsd_getpages()?
A> 
A> I cited two reasons above and expected to hear some counter-points rather than
A> them being ignored :-)
A> If we settle upon allowing bogus_page to be used in ma[], then that will
A> obviously need to be documented.

My only point is that it is a performance improvement. IMHO that's enough :)
If you can't suggest a more elegant way of doing that improvement, then all
I can suggest is to document it and add its support to ZFS.

-- 
Gleb Smirnoff



Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20180218132623.GF93303>