Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Illumos 5960 zfs recv should prefetch indirect blocks #4179

Conversation

kernelOfTruth
Copy link
Contributor

Illumos 5925 zfs receive -o origin=

Reviewed by: Prakash Surya [email protected]
Reviewed by: Matthew Ahrens [email protected]

Reference:

https://www.illumos.org/issues/5960
illumos/illumos-gate@a2cdcdd
https://www.illumos.org/issues/5925
illumos/illumos-gate@a2cdcdd

Porting notes:

depends on #3574 Illumos 5745 zfs set allows only one dataset property to be set at a time
depends on #3611 Illumos 5746 more checksumming in zfs send

diverged code base from Illumos:

[lib/libzfs/libzfs_sendrecv.c]
b8864a2 Fix gcc cast warnings
325f023 Add linux kernel device support
5c3f61e Increase Linux pipe buffer size on 'zfs receive'

[module/zfs/zfs_vnops.c]
3558fd7 Prototype/structure update for Linux
c12e3a5 Restructure zfs_readdir() to fix regressions

[module/zfs/zvol.c]
function
@zvol_map_block(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
is non-existent in ZoL

[module/zfs/dmu.c]
in function
dmu_prefetch(objset_t *os, uint64_t object, uint64_t offset, uint64_t len)
int i
is initialized before the following code block (c90 vs. c99)

[module/zfs/Makefile.in + lib/libzpool/Makefile.am]
47a4a6f Support parallel build trees (VPATH builds)

[module/zfs/dbuf.c]
fc5bb51 Fix stack dbuf_hold_impl()
9b67f60 Illumos 4757, 4913
{4757 ZFS embedded-data block pointers ("zero block compression") ,
4913 zfs release should not be subject to space checks}
{reference} 34229a2 Reduce stack usage for recursive traverse_visitbp()

[module/zfs/dmu_send.c]
b58986e Use large stacks when available
241b541 Illumos 5959 - clean up per-dataset feature count code
{reference} 77aef6f Use vmem_alloc() for nvlists
00b4602 Add linux kernel memory support

[module/zfs/zvol.c]
9965059 Prefetch start and end of volumes

[module/zfs/dmu_send.c, C90 warnings - previous commits, code thus less clear to read]
Illumos 5746 more checksumming in zfs send

[module/zfs/dbuf.c, ISO C90 - mixed declarations and code]
arc_flags_t aflags =
uint64_t nextblkid = dpa->dpa_zb.zb_blkid >>
dmu_buf_impl_t _db = dbuf_find(dn->dn_objset, dn->dn_object,
zio_t *pio = zio_root(dmu_objset_spa(dn->dn_objset), NULL, NULL,
blkptr_t *bp = ((blkptr_t *)abuf->b_data) +
dbuf_prefetch_arg_t *dpa = kmem_zalloc(sizeof (_dpa), KM_SLEEP);
dsl_dataset_t *ds = dn->dn_objset->os_dsl_dataset;

[module/zfs/dmu_send.c, ISO C90 - mixed declarations and code]
dnode_phys_t *blk = abuf->b_data;
uint64_t dnobj = zb->zb_blkid * (blksz >> DNODE_SHIFT);
error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode: for (struct receive_ign_obj_node *n =
struct send_block_record *to_data;
struct receive_ign_obj_node *n;
struct receive_ign_obj_node *last_object;
uint64_t last_objnum = (last_object != NULL ?

FIXME: man/man8/zfs.8
FIXME: different manpage format, currently I don't "get it" yet

Ported-by: kernelOfTruth [email protected]

@kernelOfTruth
Copy link
Contributor Author

Note to self:

🎈 @zvol_map_block(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
in module/zfs/zvol.c is missing - what does it provide in Illumos ?

🎈 module/zfs/zvol.c check whether the parameters really were added correctly

🎈 man pages:

Could someone please post sample output with

zfs receive -o origin=

currently the following two lines don't clearly reveal their difference or meaning by staring at that man page code :(

 .Nm
 .Cm receive
 .Op Fl Fnuv
+.Op Fl o Sy origin Ns = Ns Ar snapshot
 .Ar filesystem Ns | Ns Ar volume Ns | Ns Ar snapshot
 .Nm
 .Cm receive
 .Op Fl Fnuv
 .Op Fl d Ns | Ns Fl e
+.Op Fl o Sy origin Ns = Ns Ar snapshot
 .Ar filesystem
 .Nm

particularly .Op Fl d Ns | Ns Fl e

"our" format probably would result in something like

.LP
.nf
\fBzfs\fR \fBreceive\fR [\fB-o\fR \fIorigin\fR=\fIvalue\fR] ... \fIfilesystem\fR \fIvolume\fR|\fIsnapshot\fR
.fi

... anyway - will take a look at it a different time

Illumos 5925 zfs receive -o origin=

Reviewed by: Prakash Surya <[email protected]>
Reviewed by: Matthew Ahrens <[email protected]>

Reference:

https://www.illumos.org/issues/5960
illumos/illumos-gate@a2cdcdd
https://www.illumos.org/issues/5925
illumos/illumos-gate@a2cdcdd

Porting notes:

depends on openzfs#3574 Illumos 5745 zfs set allows only one dataset property to be set at a time
depends on openzfs#3611 Illumos 5746 more checksumming in zfs send

diverged code base from Illumos:

[lib/libzfs/libzfs_sendrecv.c]
b8864a2 Fix gcc cast warnings
325f023 Add linux kernel device support
5c3f61e Increase Linux pipe buffer size on 'zfs receive'

[module/zfs/zfs_vnops.c]
3558fd7 Prototype/structure update for Linux
c12e3a5 Restructure zfs_readdir() to fix regressions

[module/zfs/zvol.c]
function
@zvol_map_block(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
is non-existent in ZoL

[module/zfs/dmu.c]
in function
dmu_prefetch(objset_t *os, uint64_t object, uint64_t offset, uint64_t len)
int i
is initialized before the following code block (c90 vs. c99)

[module/zfs/Makefile.in + lib/libzpool/Makefile.am]
47a4a6f Support parallel build trees (VPATH builds)

[module/zfs/dbuf.c]
fc5bb51 Fix stack dbuf_hold_impl()
9b67f60 Illumos 4757, 4913
{4757 ZFS embedded-data block pointers ("zero block compression") ,
 4913 zfs release should not be subject to space checks}
{reference} 34229a2 Reduce stack usage for recursive traverse_visitbp()

[module/zfs/dmu_send.c]
b58986e Use large stacks when available
241b541 Illumos 5959 - clean up per-dataset feature count code
{reference} 77aef6f Use vmem_alloc() for nvlists
00b4602 Add linux kernel memory support

[module/zfs/zvol.c]
9965059 Prefetch start and end of volumes

[module/zfs/dmu_send.c, C90 warnings - previous commits, code thus less clear to read]
Illumos 5746 more checksumming in zfs send

[module/zfs/dbuf.c, ISO C90 - mixed declarations and code]
arc_flags_t aflags =
uint64_t nextblkid = dpa->dpa_zb.zb_blkid >>
dmu_buf_impl_t *db = dbuf_find(dn->dn_objset, dn->dn_object,
zio_t *pio = zio_root(dmu_objset_spa(dn->dn_objset), NULL, NULL,
blkptr_t *bp = ((blkptr_t *)abuf->b_data) +
dbuf_prefetch_arg_t *dpa = kmem_zalloc(sizeof (*dpa), KM_SLEEP);
dsl_dataset_t *ds = dn->dn_objset->os_dsl_dataset;

[module/zfs/dmu_send.c, ISO C90 - mixed declarations and code]
dnode_phys_t *blk = abuf->b_data;
uint64_t dnobj = zb->zb_blkid * (blksz >> DNODE_SHIFT);
error: ‘for’ loop initial declarations are only allowed in C99 or C11 mode: for (struct receive_ign_obj_node *n =
struct send_block_record *to_data;
struct receive_ign_obj_node *n;
struct receive_ign_obj_node *last_object;
uint64_t last_objnum = (last_object != NULL ?

FIXME: man/man8/zfs.8
FIXME: different manpage format, currently I don't "get it" yet

Ported-by: kernelOfTruth [email protected]
@behlendorf
Copy link
Contributor

@kernelOfTruth Nice job porting this beast to ZoL. This one definitely had more conflicts than most and you did nice a nice job sorting it all out. The traversal code in particular is tricky. I liked how you linked to the relevant commits which were responsible for the conflicts. I've had a chance to carefully look it over, fix a few things which were missed and do some testing. Unless you have some remaining reservations I'll merge it.

@zvol_map_block(spa_t *spa, zilog_t *zilog, const blkptr_t *bp,
in module/zfs/zvol.c is missing - what does it provide in Illumos

This is require for crash dump support in Illumos, there's no required analog on Linux. Simply drop it.

module/zfs/zvol.c check whether the parameters really were added correctly

Yes, it looks right.

man pages:

I've updated them with the correct syntax. Maybe once we're fully caught up with illumos we could inspect the remaining differences and sync up. And just for your information you can always just run man <man-page-file> to see how something should be formatted. Just point it at the original illumos man page.

non-debug build

The normal production build resulted in a few unused variable compiler warnings I resolved.

The were several clearly working files included in the patch I removed (multiple versions of the man page).

@kernelOfTruth
Copy link
Contributor Author

@behlendorf
awesome !

sometimes it's not immediately clear whether things are needed - but if there's no counterpart in Linux ...

didn't know that one could simply read the man pages in that way - but in retrospect it's logical

okay, great :)

sure, go ahead - I'm glad that the prefetch rework finally can land in master 👍

Thanks, LGTM

@behlendorf
Copy link
Contributor

Merged as:

e9e3d31 Allow 16M send/recv blocks
fcff0f3 Illumos 5960, 5925

While manually testing the patch I encountered one regression which is still present in the upstream Illumos code. I left that fix for that as its own patch so we can feed it upstream easily.

@behlendorf behlendorf closed this Jan 9, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants