ZFS_IOC_RECV should only call zvol_create_minors() on zvols #2388
Conversation
|
Does this solve any particular issue? It should be harmless, although it might be somewhat expensive to call |
|
perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. |
|
@behlendorf I refreshed this with a proper commit message. I was juggling a few things at the time, so the lack of a commit message slipped by me. |
|
@ryao I'm all for optimizing this case but we should be doing it in the My suggestion would be to adopt the FreeBSD code for this. They've already solved exactly this problem for presumably the same reason. Let's avoid doing our own thing if possible. It looks like we could almost use the |
|
There is a related pull request #2479 where zvol_create_minors() is optimized and is made aware of snapshots vs. zvols. In case of receive() creating snapshots, "tofs@tosnap" could be passed to zvol_create_minors() to avoid dmu_objset_find() and to _zvol_create_minor() for the snapshot directly if snapshots are visible (or do nothing if not visible). |
|
@bprotopopov What do you think about starting with the freebsd |
|
@behlendorf that sounds reasonable, let me take a quick look at the freebsd zvol_create_minors(). To be honest, I found that with 10s of thousands of snapshots and thousands of zvols, Linux udev sybsystem is stretched to the max. In extreme cases, I would say, the best thing is to avoid the device framework completely, and integrate block interface consumers with ZFS in a way similar to COMSTAR integration in Solaris. For "medium scale", udev should be OK, and if one makes sure one does not scan datasets unnecessarily in dmu_objset_find(), one should be able to improve scalability significantly, as my testing has shown. |
|
@behlendorf so I took a look at freebsd code at https://github.com/freebsd/freebsd/blob/master/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c (hope this is the right reference). Generally, the code seems similar but a bit less streamlined :) then the zfs code for Linux. Seems like FreeBSD does not have a notion similar to the snapdev property, so they do not need to traverse snapshots of a zvol on change of the property value to create/destroy devices for the snapshots. This would be different for Linux where if zvol_create_minors() gets a name of a zvol, the function would need to go through the snaps, I believe, if snapdev == visible. So, the optimizations here are somewhat limited. Essentially, the FreeBSD code does something very similar to what I did in the pull request, but it uses dmu_snapshot_list_next() instead of dmu_objset_find() to traverse snapshots of a zvol, which could be a good idea, I need to look a bit further. |
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
…e_minors() on zvols. ZFS_IOC_RECV should only call zvol_create_minors() on zvols perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
|
@behlendorf This is a rather nice improvement, but the things that I can do on my own time are limited. I am running through outstanding pull requests in the issue tracker to flush them from my to do list, so I have opted to just move this into zvol_create_minors(). This change could be reverted in the future should someone have time to rebase it on the FreeBSD code. |
|
@ryao I'm not thrilled about this, it would be best to adopt the long term solution. But since we clearly don't have the time to devote to it I'm willing to live with this as a stop gap. |
ryao
force-pushed
the
recv
branch
2 times, most recently
from
9d51f15 to
0de11b3
Compare
perf profiling revealed significant time spent in these functions on a system without zvols during a recv. I know of one company that is explicitly disabling this in their own builds because they found that it wastes CPU time. This is the intermediate approach. Signed-off-by: Richard Yao <[email protected]>
|
@ryao Can you rebase this against master and repush it to get a fresh round of tests from the buildbot. |
|
This patch causes zconfig.sh test 3 to fail in the buildbot. Presumably this is caused by a zvol not being created when it should be. |
|
Replaced by #3830 |
Signed-off-by: Richard Yao [email protected]