aboutsummaryrefslogtreecommitdiffstats
Commit message (Collapse)AuthorAgeFilesLines
* Add systemd unit file for zedChris Dunlap2014-04-023-2/+20
| | | | | | | | | | This commit adds a systemd unit file for zed.service and integrates it into the zfs.target from commit 881f45c. Signed-off-by: Chris Dunlap <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2108 Issue #2
* Initial implementation of zed (ZFS Event Daemon)Chris Dunlap2014-04-0234-1/+3715
| | | | | | | | | | | | | | | | | | | | | | | zed monitors ZFS events. When a zevent is posted, zed will run any scripts that have been enabled for the corresponding zevent class. Multiple scripts may be invoked for a given zevent. The zevent nvpairs are passed to the scripts as environment variables. Events are processed synchronously by the single thread, and there is no maximum timeout for script execution. Consequently, a misbehaving script can delay (or forever block) the processing of subsequent zevents. Plans are to address this in future commits. Initial scripts have been developed to log events to syslog and send email in response to checksum/data/io errors and resilver.finish/scrub.finish events. By default, email will only be sent if the ZED_EMAIL variable is configured in zed.rc (which is serving as a config file of sorts until a proper configuration file is implemented). Signed-off-by: Chris Dunlap <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2
* Replace zpool_events_next() "block" parm w/ "flags"Chris Dunlap2014-03-314-6/+10
| | | | | | | | | | | | | | | | | | | | | zpool_events_next() can be called in blocking mode by specifying a non-zero value for the "block" parameter. However, the design of the ZFS Event Daemon (zed) requires additional functionality from zpool_events_next(). Instead of adding additional arguments to the function, it makes more sense to use flags that can be bitwise-or'd together. This commit replaces the zpool_events_next() int "block" parameter with an unsigned bitwise "flags" parameter. It also defines ZEVENT_NONE to specify the default behavior. Since non-blocking mode can be specified with the existing ZEVENT_NONBLOCK flag, the default behavior becomes blocking mode. This, in effect, inverts the previous use of the "block" parameter. Existing callers of zpool_events_next() have been modified to check for the ZEVENT_NONBLOCK flag. Signed-off-by: Chris Dunlap <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2
* Add defs for makefile installation dir varsChris Dunlap2014-03-313-0/+11
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Add macro definitions to AM_CPPFLAGS to propagate makefile installation directory variables for libexecdir, runstatedir, sbindir, and sysconfdir. https://www.gnu.org/software/autoconf/manual/autoconf-2.69/html_node/Installation-Directory-Variables.html A corollary is that you should not use these variables except in makefiles. For instance, instead of trying to evaluate datadir in configure and hard-coding it in makefiles using e.g., 'AC_DEFINE_UNQUOTED([DATADIR], ["$datadir"], [Data directory.])', you should add -DDATADIR='$(datadir)' to your makefile's definition of CPPFLAGS (AM_CPPFLAGS if you are also using Automake). The runstatedir directory is for "installing data files which the programs modify while they run, that pertain to one specific machine, and which need not persist longer than the execution of the program". https://www.gnu.org/prep/standards/html_node/Directory-Variables.html It will be defined by autoconf 2.70 or later, and default to "$(localstatedir)/run". http://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=a197431414088a417b407b9b20583b2e8f7363bd Signed-off-by: Chris Dunlap <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2
* Clarify zpool_events_next() commentBrian Behlendorf2014-03-313-16/+16
| | | | | | | | | | | | | | | | | | Due to the very poorly chosen argument name 'cleanup_fd' it was completely unclear that this file descriptor is used to track the current cursor location. When the file descriptor is created by opening ZFS_DEV a private cursor is created in the kernel for the returned file descriptor. Subsequent calls to zpool_events_next() and zpool_events_seek() then require the file descriptor as an argument to reposition the cursor. When the file descriptor is closed the kernel state tracking the cursor is destroyed. This patch contains no functional change, it just changes a few variable names and clarifies the documentation. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Chris Dunlap <[email protected]> Issue #2
* Add zpool_events_seek() functionalityBrian Behlendorf2014-03-317-1/+128
| | | | | | | | | | | | | The ZFS_IOC_EVENTS_SEEK ioctl was added to allow user space callers to seek around the zevent file descriptor by EID. When a specific EID is passed and it exists the cursor will be positioned there. If the EID is no longer cached by the kernel ENOENT is returned. The caller may also pass ZEVENT_SEEK_START or ZEVENT_SEEK_END to seek to those respective locations. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Chris Dunlap <[email protected]> Issue #2
* Add a unique "eid" value to all zeventsBrian Behlendorf2014-03-313-0/+18
| | | | | | | | | | | | Tagging each zevent with a unique monotonically increasing EID (Event IDentifier) provides the required infrastructure for a user space daemon to reliably process zevents. By writing the EID to persistent storage the daemon can safely resume where it left off in the event stream when it's restarted. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Chris Dunlap <[email protected]> Issue #2
* Remount datasets for "zfs inherit".Gunnar Beutner2014-03-241-0/+9
| | | | | | | | | | | | Changing properties with "zfs inherit" should cause the datasets to be remounted. This ensures that the modified property values will be propagated in to the filesystem namespace where they can be enforced. This change is modeled after an identical fix made to zfs_prop_set(). Signed-off-by: Gunnar Beutner <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2201
* Linux 3.13 compat: Handle __must_check bdi_setup_and_registerRichard Yao2014-03-241-2/+3
| | | | | | | | | | | | | | | torvalds/linux@8077c0d983ab276ec5f2700df56a64d671781905 added a __must_check to the bdi_setup_and_register(), which caused our autotools check to break. zfsonlinux/zfs@729210564a5325e190fc4fba22bf17bacf957ace was intended to correct that, but it depended on -Wno-unused-result, which is unrecognized in older GCC versions. That commit has been reverted in favor of a solution that does not require -Wno-unused-result. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2102 Closes #2135
* Revert "Properly ignore bdi_setup_and_register return value"Richard Yao2014-03-241-4/+1
| | | | | | | | | | Older GCC versions do not obey -Wno-unused-result. This reverts commit 729210564a5325e190fc4fba22bf17bacf957ace in favor of a solution that does not require -Wno-unused-result. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #1906
* Illumos #4089 NULL pointer dereference in arc_read()Boris Protopopov2014-03-241-9/+11
| | | | | | | | | | | | | | | | | | 4089 NULL pointer dereference in arc_read() Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: Saso Kiselkov <[email protected]> Reviewed by: Garrett D'Amore <[email protected]> Approved by: Dan McDonald <[email protected]> References: https://www.illumos.org/issues/4089 illumos/illumos-gate@57815f6b95a743697e148327725b7f568e75e6ea Signed-off-by: Brian Behlendorf <[email protected]> Issue #2171 Issue #2165 Closes #2198
* Illumos #3120 zinject hangs in zfsdev_ioctl() due to uninitialized zcYuri Pankov2014-03-241-7/+5
| | | | | | | | | | | | | | | | 3120 zinject hangs in zfsdev_ioctl() due to uninitialized zc Reviewed by: Richard Lowe <[email protected]> Reviewed by: Eric Schrock <[email protected]> Reviewed by: Matthew Ahrens <[email protected]> Approved by: Richard Lowe <[email protected]> References: https://www.illumos.org/issues/3120 illumos/illumos-gate@f4c46b1eda9212fd32ba197043d52239ef5c0a7f Ported-by: Brian Behlendorf <[email protected]> Closes #2152
* config: compile test rather than run testChunwei Chen2014-03-203-3/+3
| | | | | | | | | | When testing compiler flags, we only need to do compile test. Otherwise, configure will fail with "configure: error: cannot run test program while cross compiling" when cross compiling. Signed-off-by: Chunwei Chen <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2191
* Implement -t option to zpool import for temporary pool namesRichard Yao2014-03-204-5/+45
| | | | | | | | | | | | | | | | Originally, users had to handle spa namespace collisions by either exporting the already imported pool or by specifying a new name for the pool with a conflicting name. In the case of root pools from virtual guests, neither approach to collision resolution is reasonable. This is addressed by extending the new name syntax with a -t option to specify that the new name is temporary. When specified, this sets an internal flag that is passed into the kernel to tell it that all label updates should refer to the name used in the original label. Consequently, the original pool name will be retained on export. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2189
* Assert alignment in umem_alloc_alignedRichard Yao2014-03-201-0/+2
| | | | | | | | | | | | | | | | | | | | | | Valgrind suggests that the address we are returning is not properly aligned, so lets add an assertion. ==87740== Address 0x1012a22a is 554 bytes inside a block of size 4,096 alloc'd ==87740== at 0x4C2BBA0: memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==87740== by 0x4C2BCC7: posix_memalign (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==87740== by 0x52FA845: zio_buf_alloc (umem.h:101) ==87740== by 0x52F6226: zil_alloc_lwb (zil.c:463) ==87740== by 0x52F8559: zil_commit (zil.c:566) ==87740== by 0x40611D: ztest_freeze (ztest.c:5909) ==87740== by 0x4066A7: ztest_init (ztest.c:6048) ==87740== by 0x407AF4: main (ztest.c:6226) Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2174
* sighandler() should take 2 argumentsIsaac Huang2014-03-201-6/+2
| | | | | | | | | | | | | Stopping arcstat.py with ^C always ends up with error: TypeError: sighandler() takes no arguments (2 given) Since no special signal handling was done in sighandler(), it's simpler to just set SIGINT handler to SIG_DFL, which terminates the script. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Isaac Huang <[email protected]> Closes #2179
* Revert "sighandler() should take 2 arguments"Brian Behlendorf2014-03-201-1/+1
| | | | | | | | This reverts commit 0bb89b6c594259829556f6dea5a89e722f214fd3 in favor of a cleaner implementation. Signed-off-by: Brian Behlendorf <[email protected]> Issue #2182
* Fix regression introduced in port of Illumos #3744Andrey Vesnovaty2014-03-201-2/+4
| | | | | | | | | | | | | | Remove the redundant call to zfs_unmount_snap() which was being done after char array was freed, This fixes an upstream regression that was introduced in commit zfsonlinux/zfs@d09f25dc66774959499a89bf3680d09c6e541ce8, which ported the Illumos 3744 changes. Signed-off-by: Andrey Vesnovaty <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Richard Yao <[email protected]> Closes #2156
* Remove unused option -r from 'zpool import'Richard Yao2014-03-121-1/+1
| | | | | | Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2178
* Switch ztest mmap(2) ASSERTs to VERIFYsRichard Yao2014-03-121-3/+3
| | | | | | | | | | | This is just a small bit of cleanup to ensure ztest fails early on systems where mmap(2) is not functioning. For the automated testing which is the primary consumer of ztest there is no functional change because debugging is always enabled. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2177
* Free props in ztest_init()Richard Yao2014-03-121-0/+1
| | | | | | | | | Valgrind complained about this and it's absolutely right. The props nvlist was not being freed in ztest_init. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Richard Yao <[email protected]> Closes #2174
* sighandler() should take 2 argumentsIsaac Huang2014-03-121-1/+1
| | | | | | | | | | | | | | Stopping arcstat.py with ^C always ends up with error: TypeError: sighandler() takes no arguments (2 given) This patch corrects the error by updating the signal handler to take the correct number of arguments. Signed-off-by: Isaac Huang <[email protected]> Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2182
* Make arcstat.py default to one header per screenIsaac Huang2014-03-121-0/+13
| | | | | | | | | | | | Today arcstat.py prints one header every hdr_intr (20 by default) lines. It would be more consistent with out utilities like vmstat if hdr_intr defaulted to terminal window size, i.e. one header per screenful of outputs. Signed-off-by: Isaac Huang <[email protected]> Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2183
* Fix NAME section of manpages zhack and fsck.zfs.Turbo Fredriksson2014-03-102-2/+3
| | | | | | | | | | | | | | | | In Debian GNU/Linux a program called 'linitian' is used to make sure that packages conforms to the Debian GNU/Linux packaging guide lines. This fixes the problem reported as: W: zfsutils: manpage-has-bad-whatis-entry usr/share/man/man1/zhack.1.gz W: zfsutils: manpage-has-bad-whatis-entry usr/share/man/man8/fsck.zfs.8.gz Not something that ZoL needs to addhere to, but every other man page have their NAME section in a special way - why not these two as well? Signed-off-by: Turbo Fredriksson <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2161
* Illumos #4088 use after free in arc_release()Boris Protopopov2014-03-101-1/+1
| | | | | | | | | | | | | | | | | | | | | | | | 4088 use after free in arc_release() Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: Garrett D'Amore <[email protected]> Reviewed by: Saso Kiselkov <[email protected]> Approved by: Dan McDonald <[email protected]> References: https://www.illumos.org/issues/4088 illumos/illumos-gate@ccc22e130479b5bd7c0002267fee1e0602d3f772 From the illumos issue: A race-induced use after free occurs in arc_release() where the ARC header is used outside the critical section protected by the hash_lock. Ported by: Tim Chase <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Richard Yao <[email protected]> Closes #2162
* Use KM_PUSHPAGE in spa_add() for spa_label_features.Tim Chase2014-03-101-1/+1
| | | | | | | | | The spa_label_features nvlist is used in the sync context during pool version upgrade. Signed-off-by: Tim Chase <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2168
* Export symbols dsl_sync_task{_nowait}Brian Behlendorf2014-03-071-0/+2
| | | | | | | | These are needed by consumers (i.e. Lustre) who wish to perform a callback in the syncing context. Both a blocking and non-blocking version are available to callers. Signed-off-by: Brian Behlendorf <[email protected]>
* Improve reporting of tx assignment wait timesNed Bass2014-03-041-5/+5
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Some callers of dmu_tx_assign() use the TXG_NOWAIT flag and call dmu_tx_wait() themselves before retrying if the assignment fails. The wait times for such callers are not accounted for in the dmu_tx_assign kstat histogram, because the histogram only records time spent in dmu_tx_assign(). This change moves the histogram update to dmu_tx_wait() to properly account for all time spent there. One downside of this approach is that it is possible to call dmu_tx_wait() multiple times before successfully assigning a transaction, in which case the cumulative wait time would not be recorded. However, this case should not often arise in practice, because most callers currently use one of these forms: dmu_tx_assign(tx, TXG_WAIT); dmu_tx_assign(tx, waited ? TXG_WAITED : TXG_NOWAIT); The first form should make just one call to dmu_tx_delay() inside of dmu_tx_assign(). The second form retries with TXG_WAITED if the first assignment fails and incurs a delay, in which case no further waiting is performed. Therefore transaction delays normally occur in one call to dmu_tx_wait() so the histogram should be fairly accurate. Another possible downside of this approach is that the histogram will no longer record overhead outside of dmu_tx_wait() such as in dmu_tx_try_assign(). While I'm not aware of any reason for concern on this point, it is conceivable that lock contention, long list traversal, etc. could cause assignment delays that would not be reflected in the histogram. Therefore the histogram should strictly be used for visibility in to the normal delay mechanisms and not as a profiling tool for code performance. Signed-off-by: Ned Bass <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #1915
* replace nreserved with ndirty in txgs kstatNed Bass2014-03-043-7/+9
| | | | | | | | | | | | | | | | | | | | | | | | | The nreserved column in the txgs kstat file always contains 0 following the write throttle restructuring of commit e8b96c6007bf97cdf34869c1ffbd0ce753873a3d. Prior to that commit, the nreserved column showed the number of bytes temporarily reserved in the pool by a transaction group at sync time. The new write throttle did away with temporary reservations and uses the amount of dirty data instead. To approximate the old output of the txgs kstat, the number of dirty bytes per-txg was passed in as the nreserved value to spa_txg_history_set_io(). This approach did not work as intended, because the per-txg dirty value is decremented as data is written out to disk, so it is zero by the time we call spa_txg_history_set_io(). To fix this, save the number of dirty bytes before calling spa_sync(), and pass this value in to spa_txg_history_set_io(). Also, since the name "nreserved" is now a misnomer, the column heading is now labeled "ndirty". Signed-off-by: Ned Bass <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #1696
* dmu_tx kstat cleanupNed Bass2014-03-043-4/+2
| | | | | | | | | | | | | | | | | | | | | | A few counters in the dmu_tx kstats are obsolete or no longer bumped properly. - The sync task restructuring commit 13fe019870c8779bf2f5b3ff731b512cf89133ef removed the code that bumpted dmu_tx_quota. The counter is now bumped in two cases, instead of just the one case as before (after the result of dsl_dataset_check_quota call). The second case is where we check the requested reservation against the actual pool size, as this is an implicit quota of sorts. - The write throttle restructuring commit e8b96c6007bf97cdf34869c1ffbd0ce753873a3d makes dmu_tx_how and dmu_tx_inflight obsolete, so they are removed. Signed-off-by: Kohsuke Kawaguchi <[email protected]> Signed-off-by: Ned Bass <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #1914
* Invalidate Linux buffer cache on vdevs upon each flushRichard Yao2014-03-041-0/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Userland tools such as blkid, grub2-probe and zdb will go through the buffer cache. However, ZFS uses on submit_bio() to bypass the buffer cache when performing IO operations on vdevs for efficiency purposes. This permits the on-disk state and buffer cache to fall out of synchronization. That causes seemingly random failures when tools reading stale metadata from the buffer cache try to access references to data that is no longer there. A particularly bad failure this causes involves grub2-probe, which is used by grub2-mkconfig. Ordinarily, a rootfs might be called rpool/ROOT/gentoo. However, when a failure occurs in grub2-probe, grub2-mkconfig will generate a configuration file containing /ROOT/gentoo, which omits the pool name and causes a boot failure. This is avoidable by calling invalidate_bdev() on each flush, which is a simple way to ensure that all non-dirty pages are wiped. Since userland tools rarely access vdevs directly, this should be a fancy noop >99.999% of the time and have little impact on IO. We could have tried a finer grained approach for the rare instances in which the vdevs are accessed frequently by userland. However, that would require consideration of corner cases and it is not worth the effort. Memory-wise, it would have been better to use a Linux kernel API hook to disable the buffer cache on such devices, but it provides us no way of doing that, so we opt for this approach instead. We should revisit that idea in the future when higher priority issues have been tackled. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2150
* Inform OpenRC that ZFS uses mtabRichard Yao2014-03-041-0/+1
| | | | | | | | | | | | p_l in #zfsonlinux reported that he had issues mounting filesystems that were resolved by adding rc_need="mtab" to /etc/init.d/zfs. Closer inspection revealed that we do have a race, but it is not clear how this race caused mounting to fail. What is clear is that this race should be fixed, so lets add the proper `use mtab` line to handle it. Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2148
* Illumos #4574 get_clones_stat does not call zap_count in non-debug kernelAlexander Stetsenko2014-03-041-1/+2
| | | | | | | | | | | | | | | | 4574 get_clones_stat does not call zap_count in non-debug kernel Reviewed by: Matthew Ahrens <[email protected]> Reviewed by: Marcel Telka <[email protected]> Approved by: Gordon Ross <[email protected]> References: https://www.illumos.org/issues/4574 illumos/illumos-gate@03d1795fa6f720eafbee821ad37f4343c391cfe4 Ported-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2147
* Fix zap_lookup() in feature_is_supported().Tim Chase2014-03-041-1/+1
| | | | | | | | | | The length (number of integers) argument passed to zap_lookup was wrong; likely as a result of performing stack-reduction on the function. Signed-off-by: Tim Chase <[email protected]> Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2141
* Include l2asize in arcstatcburroughs2014-03-041-0/+2
| | | | | | | | | | | | | For consistency with upstream pull in the l2asize update after reworking it from Perl to Python. References: https://github.com/mharsch/arcstat/pull/11 https://github.com/mharsch/arcstat/pull/12 Signed-off-by: cburroughs <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2122
* Remove recursion from dsl_dir_willuse_space()Andrew Barnes2014-03-041-11/+17
| | | | | | | | | | | Remove recursion from dsl_dir_willuse_space() to reduce stack usage. Issues with stack overflow were observed in zfs recv of zvols, likelihood of an overflow is proportional to the depth of the dataset as dsl_dir_willuse_space() recurses to parent datasets. Signed-off-by: Andrew Barnes <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2069
* Merge branch 'arc-changes'Brian Behlendorf2014-02-214-83/+165
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | This stack of patches has been empirically shown to drastically improve the hit rate of the ARC for certain workloads. As a result, fewer reads to disk are required, which is generally a good thing and can drastically improve performance if the workload is disk limited. For the impatient, I'll summarize the results of the tests performed: * Test 1 - Creating many empty directories. This test saw 99.9% fewer reads and 12.8% more inodes created when running *with* these changes. * Test 2 - Creating many empty files. This test saw 4% fewer reads and 0% more inodes created when running *with* these changes. * Test 3 - Creating many 4 KiB files. This test saw 96.7% fewer reads and 4.9% more inodes created when running *with* these changes. * Test 4 - Creating many 4096 KiB files. This test saw 99.4% fewer reads and 0% more inodes created (but took 6.9% fewer seconds to complete) when running *with* these changes. * Test 5 - Rsync'ing a dataset with many empty directories. This test saw 36.2% fewer reads and 66.2% more inodes created when running *with* these changes. * Test 6 - Rsync'ing a dataset with many empty files. This test saw 30.9% fewer reads and 0% more inodes created (but took 24.3% fewer seconds to complete) when running *with* these changes. * Test 7 - Rsync'ing a dataset with many 4 KiB files. This test saw 30.8% fewer reads and 173.3% more inodes created when running *with* these changes. For the patient, the following consists of more a more detailed description of the tests performed and the results gathered. All the tests were run using identical machines, each with a pool consisting of 5 mirror pairs with 2TB 7200 RPM disks. Each test was run twice, once *without* this set of patches and again *with* this set of patches to highlight the performance changes introduced. The first four workloads tested were: ** NOTE: None of these tests were run to completion. They ran for a set amount of time and then were terminated or hit ENOSPC. 1. Creating many empty directories: * fdtree -d 10 -l 8 -s 0 -f 0 -C -> 111,111,111 Directories -> 0 Files -> 0 KiB File Data 2. Creating many empty files: * fdtree -d 10 -l 5 -s 0 -f 10000 -C -> 111,111 Directories -> 1,111,110,000 Files -> 0 KiB File Data 3. Creating many 4 KiB files: * fdtree -d 10 -l 5 -s 1 -f 10000 -C -> 111,111 Directories -> 1,111,110,000 Files -> 4,444,440,000 KiB File Data 4. Creating many 4096 KiB files: * fdtree -d 10 -l 5 -s 1024 -f 10000 -C -> 111,111 Directories -> 1,111,110,000 Files -> 4,551,106,560,000 KiB File Data Results for these first four tests are below: | Time (s) | inodes | reads | writes | --+----------+----------+--------+-----------+ Test 1 Before | 65069 | 37845363 | 831975 | 3214646 | Test 1 After | 65069 | 42703608 | 778 | 3327674 | --+----------+----------+--------+-----------+ Test 2 Before | 65073 | 54257583 | 208647 | 2413056 | Test 2 After | 65069 | 54255782 | 200038 | 2533759 | --+----------+----------+--------+-----------+ Test 3 Before | 65068 | 49857744 | 487130 | 5533348 | Test 3 After | 65071 | 52294311 | 16078 | 5648354 | --+----------+----------+--------+-----------+ Test 4 Before | 34854 | 2448329 | 385870 | 162116572 | Test 4 After | 32419 | 2448329 | 2339 | 162175706 | --+----------+----------+--------+-----------+ * "Time (s)" - The run time of the test in seconds * "inodes" - The number of inodes created by the test * "reads" - The number of reads performed by the test * "writes" - The number of writes performed by the test As you can see from the table above, running with this patch stack *significantly* reduced the number of reads performed in 3 out of the 4 tests (due to an improved ARC hit rate). In addition to the tests described above, which specifically targeted creates only, three other workloads were tested. These additional tests were targeting rsync performance against the datasets created in the previous tests. A brief description of the workloads and results for these tests are below: ** NOTE: Aside from (6), these tests didn't run to completion. They ran for a set amount of time and then were terminated. 5. Rsync the dataset created in Test 1 to a new dataset: * rsync -a /tank/test-1 /tank/test-5 6. Rsync the dataset created in Test 2 to a new dataset: * rsync -a /tank/test-2 /tank/test-6 7. Rsync the dataset created in Test 3 to a new dataset: * rsync -a /tank/test-3 /tank/test-7 Results for Test 5, 6, and 7 are below: | Time (s) | inodes | reads | writes | --+----------+----------+----------+---------+ Test 5 Before | 93041 | 17921014 | 47632823 | 4094848 | Test 5 After | 93029 | 29785847 | 30376206 | 4484459 | --+----------+----------+----------+---------+ Test 6 Before | 15290 | 54264474 | 6018331 | 733087 | Test 6 After | 11573 | 54260826 | 4155661 | 617285 | --+----------+----------+----------+---------+ Test 7 Before | 93057 | 10093749 | 41561635 | 3659098 | Test 7 After | 93045 | 27587043 | 28773151 | 5612234 | --+----------+----------+----------+---------+ * "Time (s)" - The run time of the test in seconds * "inodes" - The number of inodes created by the test * "reads" - The number of reads performed by the test * "writes" - The number of writes performed by the test Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Closes #2110
| * Set "arc_meta_limit" to 3/4 arc_c_max by defaultPrakash Surya2014-02-211-2/+2
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Unfortunately, this change is an cheap attempt to work around a pathological workload for the ARC. A "real" solution still needs to be fleshed out, so this patch is intended to alleviate the situation in the meantime. Let me try and describe the problem.. Data buffers residing in the dbuf hash table (dbuf cache) will keep a hold on their respective dnode, this dnode will in turn keep a hold on its backing dbuf (the physical block of the dnode object backing it). Since the dnode has a hold on its backing dbuf, the arc buffer for this dbuf is unevictable. What this essentially boils down to, "data" buffers have the potential to pin "metadata" in the arc (as a result of these dnode object buffers being unevictable). This scenario becomes a real problem when the workload consists of many small files (e.g. creating millions of 4K files). With this workload, the arc's "arc_meta_used" space get filled up with buffers for any resident directories as well as buffers for the objset's dnode object. Once the "arc_meta_limit" is reached, the directory buffers will be evicted and only the unevictable dnode object buffers will reside. If the workload is simply creating new small files, these dnode object buffers will never even be needed again, whereas the directory buffers will be used constantly until the creates move to a new directory. If "arc_c" and "arc_meta_limit" are sized appropriately, this situation wont occur. This is because as the data buffers accumulate, "arc_size" will eventually approach "arc_c" (before "arc_meta_used" reaches "arc_meta_limit"); at that point the data buffers will be evicted, which releases the hold on the dnode, which releases the hold on the dnode object's dbuf, which allows that buffer to be evicted from the arc in preference to more "useful" metadata. So, to side step the issue, we simply need to ensure "arc_size" reaches "arc_c" before "arc_meta_used" reaches "arc_meta_limit". In order to pick a proper limit, we have to do some math. To make things a little easier to follow, it is assumed that there will only be a single data buffer per file (which is probably always the case for "small" files anyways). Based on the current internals of the arc, if N files residing in the dbuf cache all pin a single dnode buffer (i.e. their dnodes all share the same physical dnode object block), then the following amount of "arc_meta_used" space will be consumed: - 16K for the dnode object's block - [ 16384 bytes] - N * sizeof(dnode_t) -------------- [ N * 928 bytes] - (N + 1) * sizeof(arc_buf_t) ------ [(N + 1) * 72 bytes] - (N + 1) * sizeof(arc_buf_hdr_t) -- [(N + 1) * 264 bytes] - (N + 1) * sizeof(dmu_buf_impl_t) - [(N + 1) * 280 bytes] To simplify, these N files will pin the following amount of "arc_meta_used" space as unevictable: Pinned "arc_meta_used" bytes = 16384 + N * 928 + (N + 1) * (72 + 264 + 280) Pinned "arc_meta_used" bytes = 17000 + N * 1544 This pinned space is regardless of the size of the files, and is only dependent on the number of pinned dnodes sharing a physical block (i.e. N). For example, 32 512b files sharing a single dnode object block would consume the same "arc_meta_used" space as 32 4K files sharing a single dnode object block. Now, given a files size of S, we can determine the total amount of space that will be consumed in the arc: Total = 17000 + N * 1544 + S * N ^^^^^^^^^^^^^^^^ ^^^^^ metadata data So, given these formulas, we can generate a table which states the ratio of pinned metadata to total arc (meta + data) using different values of N (number of pinned dnodes per pinned physical dnode block) and S (size of the file). File Sizes (S) | 512 | 1024 | 2048 | 4096 | 8192 | 16384 | ---+----------+----------+----------+----------+----------+----------+ 1 | 0.973132 | 0.947670 | 0.900544 | 0.819081 | 0.693597 | 0.530921 | 2 | 0.951497 | 0.907481 | 0.830632 | 0.710325 | 0.550779 | 0.380051 | N 4 | 0.918807 | 0.849809 | 0.738842 | 0.585844 | 0.414271 | 0.261250 | 8 | 0.877541 | 0.781803 | 0.641770 | 0.472505 | 0.309333 | 0.182965 | 16 | 0.835819 | 0.717945 | 0.559996 | 0.388885 | 0.241376 | 0.137253 | 32 | 0.802106 | 0.669597 | 0.503304 | 0.336277 | 0.202123 | 0.112423 | As you can see, if we wanted to support the absolute worst case of 1 dnode per physical dnode block and 512b files, we would have to set the "arc_meta_limit" to something greater than 97.3132% of "arc_c_max". At that point, it essentially defeats the purpose of having an "arc_meta_limit" at all. This patch changes the default value of "arc_meta_limit" to be 75% of "arc_c_max", which should be good enough for "most" workloads (I think). Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Split "data_size" into "meta" and "data"Prakash Surya2014-02-212-14/+25
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Previously, the "data_size" field in the arcstats kstat contained the amount of cached "metadata" and "data" in the ARC. The problem is this then made it difficult to extract out just the "metadata" size, or just the "data" size. To make it easier to distinguish the two values, "data_size" has been modified to count only buffers of type ARC_BUFC_DATA, and "meta_size" was added to count only buffers of type ARC_BUFC_METADATA. If one wants the old "data_size" value, simply sum the new "data_size" and "meta_size" values. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Prioritize "metadata" in arc_get_data_bufPrakash Surya2014-02-211-22/+40
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | When the arc is at it's size limit and a new buffer is added, data will be evicted (or recycled) from the arc to make room for this new buffer. As far as I can tell, this is to try and keep the arc from over stepping it's bounds (i.e. keep it below the size limitation placed on it). This makes sense conceptually, but there appears to be a subtle flaw in its current implementation, resulting in metadata buffers being throttled. When it evicts from the arc's lists, it also passes in a "type" so as to remove a buffer of the same type that it is adding. The problem with this is that once the size limit is hit, the ratio of "metadata" to "data" contained in the arc essentially becomes fixed. For example, consider the following scenario: * the size of the arc is capped at 10G * the meta_limit is capped at 4G * 9G of the arc contains "data" * 1G of the arc contains "metadata" Now, every time a new "metadata" buffer is created and added to the arc, an older "metadata" buffer(s) will be removed from the arc; preserving the 9G "data" to 1G "metadata" ratio that was in-place when the size limit was reached. This occurs even though the amount of "metadata" is far below the "metadata" limit. This can result in the arc behaving pathologically for certain workloads. To fix this, the arc_get_data_buf function was modified to evict "data" from the arc even when adding a "metadata" buffer; unless it's at the "metadata" limit. In addition, arc_evict now more closely resembles arc_evict_ghost; such that when evicting "data" from the arc, it may make a second pass over the arc lists and evict "metadata" if it cannot meet the eviction size the first time around. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Remove "arc_meta_used" from arc_adjust calculationPrakash Surya2014-02-211-2/+1
| | | | | | | | | | | | | | | | | | | | | | | | | | Using "arc_meta_used" to determine if the arc's mru list is over it's target value of "arc_p" doesn't seem correct. The size of the mru list and the value of "arc_meta_used", although related, are completely independent. Buffers contained in "arc_meta_used" may not even be contained in the arc's mru list. As such, this patch removes "arc_meta_used" from the calculation in arc_adjust. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Prune metadata from ghost lists in arc_adjust_metaPrakash Surya2014-02-213-21/+49
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | To maintain a strict limit on the metadata contained in the arc, while preventing the arc buffer headers from completely consuming the "arc_meta_used" space, we need to evict metadata buffers from the arc's ghost lists along with the regular lists. This change modifies arc_adjust_meta such that it more closely models the adjustments made in arc_adjust. "arc_meta_used" is used similarly to "arc_size", and "arc_meta_limit" is used similarly to "arc_c". Testing metadata intensive workloads (e.g. creating, copying, and removing millions of small files and/or directories) has shown this change to make a dramatic improvement to the hit rate maintained in the arc. While I think there is still room for improvement, this is a big step in the right direction. In addition, zpl_free_cached_objects was made into a no-op as I'm not yet sure how to properly implement that function. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Revert "Return -1 from arc_shrinker_func()"Prakash Surya2014-02-211-1/+3
| | | | | | | | | | | | | | | | | | | | This reverts commit c11a12bc3b2e5ee9a6bd74e26f1a396b6025fbd4. Out of memory events were fixed by reverting this patch. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Disable arc_p adapt dampener by defaultPrakash Surya2014-02-212-2/+23
| | | | | | | | | | | | | | | | | | | | | | It's unclear why adjustments to arc_p need to be dampened as they are in arc_adjust. With that said, it's removal significantly improves the arc's ability to "warm up" to a given workload. Thus, I'm disabling by default until its usefulness is better understood. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Allow "arc_p" to drop to zero or grow to "arc_c"Prakash Surya2014-02-212-24/+4
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | Setting a limit on the minimum value of "arc_p" has been shown to have detrimental effects on the arc hit rate for certain "metadata" intensive workloads. Specifically, this has been exhibited with a workload that constantly dirties new "metadata" but also frequently touches a "small" amount of mfu data (e.g. mkdir's). What is seen is that the new anon data throttles the mfu list to a negligible size (because arc_p > anon + mru in arc_get_data_buf), even though the mfu ghost list receives a constant stream of hits. To remedy this, arc_p is now allowed to drop to zero if the algorithm deems it necessary. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Disable aggressive arc_p growth by defaultPrakash Surya2014-02-212-1/+19
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | For specific workloads consisting mainly of mfu data and new anon data buffers, the aggressive growth of arc_p found in the arc_get_data_buf() function can have detrimental effects on the mfu list size and ghost list hit rate. Running a workload consisting of two processes: * Process 1 is creating many small files * Process 2 is tar'ing a directory consisting of many small files I've seen arc_p and the mru grow to their maximum size, while the mru ghost list receives 100K times fewer hits than the mfu ghost list. Ideally, as the mfu ghost list receives hits, arc_p should be driven down and the size of the mfu should increase. Given the specific workload I was testing with, the mfu list size should grow to a point where almost no mfu ghost list hits would occur. Unfortunately, this does not happen because the newly dirtied anon buffers constancy drive arc_p to its maximum value and keep it there (effectively prioritizing the mru list and starving the mfu list down to a negligible size). The logic to increment arc_p from within the arc_get_data_buf() function was introduced many years ago in this upstream commit: commit 641fbdae3a027d12b3c3dcd18927ccafae6d58bc Author: maybee <none@none> Date: Wed Dec 20 15:46:12 2006 -0800 6505658 target MRU size (arc.p) needs to be adjusted more aggressively and since I don't fully understand the motivation for the change, I am reluctant to completely remove it. As a way to test out how it's removal might affect performance, I've disabled that code by default, but left it tunable via a module option. Thus, if its removal is found to be grossly detrimental for certain workloads, it can be re-enabled on the fly, without a code change. Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Adjust arc_p based on "bytes" in arc_shrinkPrakash Surya2014-02-211-1/+9
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | In an attempt to prevent arc_c from collapsing "too fast", the arc_shrink() function was updated to take a "bytes" parameter by this change: commit 302f753f1657c05a4287226eeda1f53ae431b8a7 Author: Brian Behlendorf <[email protected]> Date: Tue Mar 13 14:29:16 2012 -0700 Integrate ARC more tightly with Linux Unfortunately, that change failed to make a similar change to the way that arc_p was updated. So, there still exists the possibility for arc_p to collapse to near 0 when the kernel start calling the arc's shrinkers. This change attempts to fix this, by decrementing arc_p by the "bytes" parameter in the same way that arc_c is updated. In addition, a minimum value of arc_p is attempted to be maintained, similar to the way a minimum arc_p value is maintained in arc_adapt(). Signed-off-by: Prakash Surya <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2110
| * Set zfs_arc_min to 4MBBrian Behlendorf2014-02-211-6/+3
|/ | | | | | | | | | | | | | | | | | | | | Decrease the mimimum ARC size from 1/32 of total system memory (or 64MB) to a much smaller 4MB. 1) Large systems with over a 1TB of memory are being deployed and reserving 1/32 of this memory (32GB) as the mimimum requirement is overkill. 2) Tiny systems like the raspberry pi may only have 256MB of memory in which case 64MB is far too large. The ARC should be reclaimable if the VFS determines it needs the memory for some other purpose. If you want to ensure the ARC is never completely reclaimed due to memory pressure you may still set a larger value with zfs_arc_min. Signed-off-by: Brian Behlendorf <[email protected]> Signed-off-by: Prakash Surya <[email protected]> Issue #2110
* Merge branch 'issue-2094'Brian Behlendorf2014-02-2115-14/+640
|\ | | | | | | | | | | | | | | | | | | | | | | | | | | | | This patch stack was designed to accomplish the following: * Cleanly address the pool incompatibility which was accidentally introduced post-0.6.2 but pre-0.6.3. This required adding infrastructure which can be used from now on to notify system administrators of errata which affect their pool. * Add additional automated regression testing to ensure this type of compatibility issue is caught prior to the change being merged. Signed-off-by: Brian Behlendorf <[email protected]> Closes #2094
| * Add erratum for issue #2094Richard Yao2014-02-214-0/+65
| | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | | ZoL commit 1421c89 unintentionally changed the disk format in a forward- compatible, but not backward compatible way. This was accomplished by adding an entry to zbookmark_t, which is included in a couple of on-disk structures. That lead to the creation of pools with incorrect dsl_scan_phys_t objects that could only be imported by versions of ZoL containing that commit. Such pools cannot be imported by other versions of ZFS or past versions of ZoL. The additional field has been removed by the previous commit. However, affected pools must be imported and scrubbed using a version of ZoL with this commit applied. This will return the pools to a state in which they may be imported by other implementations. The 'zpool import' or 'zpool status' command can be used to determine if a pool is impacted. A message similar to one of the following means your pool must be scrubbed to restore compatibility. $ zpool import pool: zol-0.6.2-173 id: 1165955789558693437 state: ONLINE status: Errata #1 detected. action: The pool can be imported using its name or numeric identifier, however there is a compatibility issue which should be corrected by running 'zpool scrub' see: http://zfsonlinux.org/msg/ZFS-8000-ER config: ... $ zpool status pool: zol-0.6.2-173 state: ONLINE scan: pool compatibility issue detected. see: https://github.com/zfsonlinux/zfs/issues/2094 action: To correct the issue run 'zpool scrub'. config: ... If there was an async destroy in progress 'zpool import' will prevent the pool from being imported. Further advice on how to proceed will be provided by the error message as follows. $ zpool import pool: zol-0.6.2-173 id: 1165955789558693437 state: ONLINE status: Errata #2 detected. action: The pool can not be imported with this version of ZFS due to an active asynchronous destroy. Revert to an earlier version and allow the destroy to complete before updating. see: http://zfsonlinux.org/msg/ZFS-8000-ER config: ... Pools affected by the damaged dsl_scan_phys_t can be detected prior to an upgrade by running the following command as root: zdb -dddd poolname 1 | grep -P '^\t\tscan = ' | sed -e 's;scan = ;;' | wc -w Note that `poolname` must be replaced with the name of the pool you wish to check. A value of 25 indicates the dsl_scan_phys_t has been damaged. A value of 24 indicates that the dsl_scan_phys_t is normal. A value of 0 indicates that there has never been a scrub run on the pool. The regression caused by the change to zbookmark_t never made it into a tagged release, Gentoo backports, Ubuntu, Debian, Fedora, or EPEL stable respositorys. Only those using the HEAD version directly from Github after the 0.6.2 but before the 0.6.3 tag are affected. This patch does have one limitation that should be mentioned. It will not detect errata #2 on a pool unless errata #1 is also present. It expected this will not be a significant problem because pools impacted by errata #2 have a high probably of being impacted by errata #1. End users can ensure they do no hit this unlikely case by waiting for all asynchronous destroy operations to complete before updating ZoL. The presence of any background destroys on any imported pools can be checked by running `zpool get freeing` as root. This will display a non-zero value for any pool with an active asynchronous destroy. Lastly, it is expected that no user data has been lost as a result of this erratum. Original-patch-by: Tim Chase <[email protected]> Reworked-by: Brian Behlendorf <[email protected]> Signed-off-by: Tim Chase <[email protected]> Signed-off-by: Richard Yao <[email protected]> Signed-off-by: Brian Behlendorf <[email protected]> Issue #2094