zfsonlinux port of work by Christopher Siden to address illumos issue 4631

[bprotopopov]

The original commit comments:

4631 zvol_get_stats triggering too many reads
Reviewed by: Adam Leventhal [email protected]
Reviewed by: Matthew Ahrens [email protected]
Reviewed by: Sebastien Roy [email protected]

Original author: George Wilson

From the comments on issue 4631:

"The problem is that when a dataset is last closed (e.g. a filesystem is
unmounted), we need to get rid of all its state in the DMU (dbufs, dnodes,
etc). This needs to be coordinated with the ARC (because the arc bufs have
pointers to these data structures too). We end up evicting all the data out of
the ARC.

This results in very bad performance for things like "zfs get all" if some
filesystems/zvols/snapshots are not mounted (or for zvols, are not open /
exported by comstar), because every snapshot's objset will be opened (to get
the zpl/zvol props), read, and then closed (which will be the last close,
evicting all the data from the ARC). When the next snapshot is opened, it
probably shares all its relevant blocks with the previous snapshot, but we just
explicitly evicted all of those blocks from the ARC. So we end up repeatedly
reading in the same blocks from disk.

The fix is to change the interaction between the DMU and ARC so that when the
DMU is shutting down an objset, we do not evict the data from the ARC. Instead
we simply coordinate the destruction of the DMU's data with the ARC. The only
case where we actually need to explicitly evict from the ARC is when
dbuf_rele_and_unlock() determines that the administrator has requested that it
not be kept in memory, via the primarycache/secondarycache properties. In this
case, we evict the data from the ARC by its blkptr_t, the same way as when a
block is freed we explicitly evict it from the ARC."

The resulting (significant positive) change in performance of zfs list -t all operations
with large number of zvols and snapshots is easily observed.

[behlendorf]

Nice. I'll run it through testing.

[behlendorf]

@bprotopopov Could you refresh this using the version of the patch which was actually merged to Illumos, illumos/illumos-gate@bbfa8ea

[bprotopopov]

Will do

Typos courtesy of my iPhone

On Jul 22, 2014, at 6:57 PM, "Brian Behlendorf" [email protected] wrote:

@bprotopopov Could you refresh this using the version of the patch which was actually merged to Illumos, illumos/illumos-gate@bbfa8ea

—
Reply to this email directly or view it on GitHub.

[bprotopopov]

There is one detail about this port that needs to be made explicit.

This commit depends on Illumos-gate@5d7b4d438c4a51eccc95e77a83a437b4d48380eb commit that, among other things, brings in the embedded-data block pointer support. Which is another very nice feature to have; however, the commit is quite large, and it is significantly more complex than the changes included with this pull request.

So, to simplify testing/control the changed, the refreshed pull request includes a
#define BP_IS_EMBEDDED(bp) (B_FALSE)
definition in spa.h that can be removed when the embedded-data block pointer work in ported as well.

[bprotopopov]

Pull request refreshed from illumos/illumos-gate@bbfa8ea, tested with ztest.

[behlendorf]

This commit depends on Illumos-gate@5d7b4d4 commit that...

The embedded-data block pointer work is being reviewed and tested now. Hopefully, we'll be able to merge it in the next few days, see #2544. It would be ideal if you could refresh this once it merges.

[behlendorf]

@bprotopopov OK, embedded-data block pointer work has been merged to master. Adding BP_IS_EMBEDDED(bp) (B_FALSE) should no longer be needed.

[bprotopopov]

Will rebase/update shortly

Typos courtesy of my iPhone

On Aug 1, 2014, at 7:37 PM, "Brian Behlendorf" [email protected] wrote:

@bprotopopov OK, embedded-data block pointer work has been merged to master. Adding BP_IS_EMBEDDED(bp) (B_FALSE) should no longer be needed.

—
Reply to this email directly or view it on GitHub.

[bprotopopov]

Rebased against zfsonlinux/master latest, BP_IS_EMBEDDED() redefinition is no longer needed.

[behlendorf]

@bprotopopov It's not clear to me which is the refreshed commit. Could you create a new branch off zfsonlinux/master with the commit and open a new pull request. Thanks.

[bprotopopov]

The commit is the last one on the mater branch in my repo - rebased on top of zfsonlinux/master
Yes I can create a feature branch.

Typos courtesy of my iPhone

On Aug 6, 2014, at 2:00 PM, "Brian Behlendorf" [email protected] wrote:

@bprotopopov It's not clear to me which is the refreshed commit. Could you create a new branch off zfsonlinux/master with the commit and open a new pull request. Thanks.

—
Reply to this email directly or view it on GitHub.

[bprotopopov]

Sorry, I could not figure out how to update the existing pull request after moving the commit to a different branch. I will close this one and will create a new pull request