-
Notifications
You must be signed in to change notification settings - Fork 1.8k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
zvol processing should use struct bio
Internally, zvols are files exposed through the block device API. This is intended to reduce overhead when things require block devices. However, the ZoL zvol code emulates a traditional block device in that it has a top half and a bottom half. This is an unnecessary source of overhead that does not exist on any other OpenZFS platform does this. This patch removes it. Early users of this patch reported double digit performance gains in IOPS on zvols in the range of 50% to 80%. Comments in the code suggest that the current implementation was done to obtain IO merging from Linux's IO elevator. However, the DMU already does write merging while arc_read() should implicitly merge read IOs because only 1 thread is permitted to fetch the buffer into ARC. In addition, commercial ZFSOnLinux distributions report that regular files are more performant than zvols under the current implementation, and the main consumers of zvols are VMs and iSCSI targets, which have their own elevators to merge IOs. Some minor refactoring allows us to register zfs_request() as our ->make_request() handler in place of the generic_make_request() function. This eliminates the layer of code that broke IO requests on zvols into a top half and a bottom half. This has several benefits: 1. No per zvol spinlocks. 2. No redundant IO elevator processing. 3. Interrupts are disabled only when actually necessary. 4. No redispatching of IOs when all taskq threads are busy. 5. Linux's page out routines will properly block. 6. Many autotools checks become obsolete. An unfortunate consequence of eliminating the layer that generic_make_request() is that we no longer calls the instrumentation hooks for block IO accounting. Those hooks are GPL-exported, so we cannot call them ourselves and consequently, we lose the ability to do IO monitoring via iostat. Since zvols are internally files mapped as block devices, this should be okay. Anyone who is willing to accept the performance penalty for the block IO layer's accounting could use the loop device in between the zvol and its consumer. Alternatively, perf and ftrace likely could be used. Also, tools like latencytop will still work. Tools such as latencytop sometimes provide a better view of performance bottlenecks than the traditional block IO accounting tools do. Lastly, if direct reclaim occurs during spacemap loading and swap is on a zvol, this code will deadlock. That deadlock could already occur with sync=always on zvols. Given that swap on zvols is not yet production ready, this is not a blocker. Signed-off-by: Richard Yao <[email protected]>
- Loading branch information
Showing
11 changed files
with
276 additions
and
218 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
dnl # | ||
dnl # Interface for issuing a discard bio: | ||
dnl # 2.6.28-2.6.35: BIO_RW_BARRIER | ||
dnl # 2.6.36-3.x: REQ_BARRIER | ||
dnl # | ||
|
||
dnl # Since REQ_BARRIER is a preprocessor definition, there is no need for an | ||
dnl # autotools check for it. Also, REQ_BARRIER existed in the request layer | ||
dnl # until torvalds/linux@7b6d91daee5cac6402186ff224c3af39d79f4a0e unified the | ||
dnl # request layer and bio layer flags, so it would be wrong to assume that | ||
dnl # the APIs are mutually exclusive contrary to the typical case. | ||
AC_DEFUN([ZFS_AC_KERNEL_BIO_RW_BARRIER], [ | ||
AC_MSG_CHECKING([whether BIO_RW_BARRIER is defined]) | ||
ZFS_LINUX_TRY_COMPILE([ | ||
#include <linux/bio.h> | ||
],[ | ||
int flags __attribute__ ((unused)); | ||
flags = BIO_RW_BARRIER; | ||
],[ | ||
AC_MSG_RESULT(yes) | ||
AC_DEFINE(HAVE_BIO_RW_BARRIER, 1, [BIO_RW_BARRIER is defined]) | ||
],[ | ||
AC_MSG_RESULT(no) | ||
]) | ||
]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,25 @@ | ||
dnl # | ||
dnl # Interface for issuing a discard bio: | ||
dnl # 2.6.28-2.6.35: BIO_RW_DISCARD | ||
dnl # 2.6.36-3.x: REQ_DISCARD | ||
dnl # | ||
|
||
dnl # Since REQ_DISCARD is a preprocessor definition, there is no need for an | ||
dnl # autotools check for it. Also, REQ_DISCARD existed in the request layer | ||
dnl # until torvalds/linux@7b6d91daee5cac6402186ff224c3af39d79f4a0e unified the | ||
dnl # request layer and bio layer flags, so it would be wrong to assume that | ||
dnl # the APIs are mutually exclusive contrary to the typical case. | ||
AC_DEFUN([ZFS_AC_KERNEL_BIO_RW_DISCARD], [ | ||
AC_MSG_CHECKING([whether BIO_RW_DISCARD is defined]) | ||
ZFS_LINUX_TRY_COMPILE([ | ||
#include <linux/bio.h> | ||
],[ | ||
int flags __attribute__ ((unused)); | ||
flags = BIO_RW_DISCARD; | ||
],[ | ||
AC_MSG_RESULT(yes) | ||
AC_DEFINE(HAVE_BIO_RW_DISCARD, 1, [BIO_RW_DISCARD is defined]) | ||
],[ | ||
AC_MSG_RESULT(no) | ||
]) | ||
]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,33 @@ | ||
dnl # | ||
dnl # 2.6.34 API change | ||
dnl # current->bio_tail and current->bio_list were struct bio pointers prior to | ||
dnl # Linux 2.6.34. They were refactored into a struct bio_list pointer called | ||
dnl # current->bio_list in Linux 2.6.34. | ||
dnl # | ||
AC_DEFUN([ZFS_AC_KERNEL_CURRENT_BIO_TAIL], [ | ||
AC_MSG_CHECKING([whether current->bio_tail exists]) | ||
ZFS_LINUX_TRY_COMPILE([ | ||
#include <linux/sched.h> | ||
],[ | ||
current->bio_tail = (struct bio **) NULL; | ||
],[ | ||
AC_MSG_RESULT(yes) | ||
AC_DEFINE(HAVE_CURRENT_BIO_TAIL, 1, | ||
[current->bio_tail exists]) | ||
],[ | ||
AC_MSG_RESULT(no) | ||
AC_MSG_CHECKING([whether current->bio_list exists]) | ||
ZFS_LINUX_TRY_COMPILE([ | ||
#include <linux/sched.h> | ||
],[ | ||
current->bio_list = (struct bio_list *) NULL; | ||
],[ | ||
AC_MSG_RESULT(yes) | ||
AC_DEFINE(HAVE_CURRENT_BIO_LIST, 1, | ||
[current->bio_list exists]) | ||
],[ | ||
AC_MSG_ERROR(no - Please file a bug report at | ||
https://github.com/zfsonlinux/zfs/issues/new) | ||
]) | ||
]) | ||
]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
dnl # | ||
dnl # Linux 3.2 API Change | ||
dnl # make_request_fn returns void instead of int. | ||
dnl # | ||
AC_DEFUN([ZFS_AC_KERNEL_MAKE_REQUEST_FN], [ | ||
AC_MSG_CHECKING([whether make_request_fn() returns int]) | ||
ZFS_LINUX_TRY_COMPILE([ | ||
#include <linux/blkdev.h> | ||
int make_request(struct request_queue *q, struct bio *bio) | ||
{ | ||
return (0); | ||
} | ||
],[ | ||
blk_queue_make_request(NULL, &make_request); | ||
],[ | ||
AC_MSG_RESULT(yes) | ||
AC_DEFINE(MAKE_REQUEST_FN_RET, int, | ||
[make_request_fn() returns int]) | ||
AC_DEFINE(HAVE_MAKE_REQUEST_FN_RET_INT, 1, | ||
[Noting that make_request_fn() returns int]) | ||
],[ | ||
AC_MSG_RESULT(no) | ||
AC_MSG_CHECKING([whether make_request_fn() returns void]) | ||
ZFS_LINUX_TRY_COMPILE([ | ||
#include <linux/blkdev.h> | ||
void make_request(struct request_queue *q, struct bio *bio) | ||
{ | ||
return; | ||
} | ||
],[ | ||
blk_queue_make_request(NULL, &make_request); | ||
],[ | ||
AC_MSG_RESULT(yes) | ||
AC_DEFINE(MAKE_REQUEST_FN_RET, void, | ||
[make_request_fn() returns void]) | ||
],[ | ||
AC_MSG_ERROR(no - Please file a bug report at | ||
https://github.com/zfsonlinux/zfs/issues/new) | ||
]) | ||
]) | ||
]) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.