Re: [OpenZFS Developer] Review Request 259: ZFS indirection tables prefetch

2015-10-13 Thread Matthew Ahrens 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.csiden.org/r/259/#review833
---



usr/src/uts/common/fs/zfs/dmu.c (lines 444 - 447)


I don't see that DNODE_IS_CACHEABLE has ever existed in illumos.



usr/src/uts/common/fs/zfs/dmu_zfetch.c (line 52)


can you clarify if this is "bytes of the file" vs "bytes of indirects"?  
i.e. will we use an additional 64MB memory per stream, vs approximately 64MB / 
block size * sizeof(blkptr_t)



usr/src/uts/common/fs/zfs/dmu_zfetch.c (lines 266 - 268)


nice cleanup



usr/src/uts/common/fs/zfs/dmu_zfetch.c (line 269)


cstyle says that if one side of the if/else has braces, both should.



usr/src/uts/common/fs/zfs/dmu_zfetch.c (lines 285 - 286)


I think I understand why you are rounding up the start -- if the start is 
misaligned (not at the beginning of the indirect block) we would have alread 
prefetched this indirect last time.

Why do you round up the end?  It seems like you would want to end on the 
indirect that contains the ending level-0 blkid (zs_ibf_blkid).



usr/src/uts/common/fs/zfs/sys/dmu_zfetch.h (line 46)


it would be helpful to specify that this is the blkid of the level-0 block 
whose level-1 indirect we will prefetch (as opposed to the blkid of the next 
level-1 indirect to prefetch)


- Matthew Ahrens


On Oct. 12, 2015, 4:33 p.m., Alexander Motin wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.csiden.org/r/259/
> ---
> 
> (Updated Oct. 12, 2015, 4:33 p.m.)
> 
> 
> Review request for OpenZFS Developer Mailing List and Christopher Siden.
> 
> 
> Bugs: 6322
> https://www.illumos.org/issues/6322
> 
> 
> Repository: illumos-gate
> 
> 
> Description
> ---
> 
> 6322 ZFS indirection tables prefetch
> 
> 
> Diffs
> -
> 
>   usr/src/uts/common/fs/zfs/dmu_zfetch.c 
> f2cdf863d83c9ecb627a3b566c4c09553e4b9666 
>   usr/src/uts/common/fs/zfs/dmu.c 5b24268af1a813c153c75e8531cb3a6e23baa406 
>   usr/src/uts/common/fs/zfs/dbuf.c 8b0c71db5c86fab2219699f4e3b2eccf00de545c 
>   usr/src/uts/common/fs/zfs/sys/dnode.h 
> 69cc54dc272b143b085f897b956bb9cdd8c09bf3 
>   usr/src/uts/common/fs/zfs/sys/dmu_zfetch.h 
> 6f61198ebc4528f58761f56a11610cb2fbefadd3 
> 
> Diff: https://reviews.csiden.org/r/259/diff/
> 
> 
> Testing
> ---
> 
> Tested on FreeBSD head system.
> 
> 
> Thanks,
> 
> Alexander Motin
> 
>

___
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer

Re: [OpenZFS Developer] Review Request 250: DLPX-37488 exporting a pool while an async destroy is running can leave entries in the deferred tree

2015-10-13 Thread Fabian Keil 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.csiden.org/r/250/#review828
---

Ship it!


Ship It!

- Fabian Keil


On Oct. 7, 2015, 3:02 a.m., Matthew Ahrens wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.csiden.org/r/250/
> ---
> 
> (Updated Oct. 7, 2015, 3:02 a.m.)
> 
> 
> Review request for OpenZFS Developer Mailing List and Christopher Siden.
> 
> 
> Bugs: 6292
> https://www.illumos.org/issues/6292
> 
> 
> Repository: illumos-gate
> 
> 
> Description
> ---
> 
> 6292 exporting a pool while an async destroy is running can leave entries in 
> the deferred tree
> Reviewed by: Paul Dagnelie 
> Reviewed by: Matthew Ahrens 
> 
> Original author: George Wilson
> 
> 
> Diffs
> -
> 
>   usr/src/uts/common/fs/zfs/dsl_scan.c 
> 6ba5cb6a1c831f681fba16dbc8d25fd8b59b13c9 
> 
> Diff: https://reviews.csiden.org/r/250/diff/
> 
> 
> Testing
> ---
> 
> http://jenkins.delphix.com/job/zfs-precommit/3149/
> 
> 
> Thanks,
> 
> Matthew Ahrens
> 
>

___
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer

Re: [OpenZFS Developer] Review Request 250: DLPX-37488 exporting a pool while an async destroy is running can leave entries in the deferred tree

2015-10-13 Thread George Wilson 


> On Oct. 7, 2015, 5:42 p.m., Justin Gibbs wrote:
> > usr/src/uts/common/fs/zfs/dsl_scan.c, lines 1421-1427
> > 
> >
> > Wouldn't it be better to merge this logic into dsl_scan_active() so 
> > that the policy is contained in one place?  Something like:
> > 
> > ```c
> > if (!dsl_scan_active(scn, INCLUDE_STALLED_SCANS))
> > return;
> > ```
> 
> Justin Gibbs wrote:
> Grr.  Review board posted a dup and I somehow failed to mark the right 
> lines...

I'm not convinced it would be better since proceeding with a scan when a scan 
is not running is not common policy but I don't have a strong opinion either 
way. I think if we did change the interface I would prefer to see a "force" 
flag or something like that so that it's clear that we're asking for an 
exception. Using "include" make it seem like that's the preferred behavior (at 
least to me).


- George


---
This is an automatically generated e-mail. To reply, visit:
https://reviews.csiden.org/r/250/#review818
---


On Oct. 7, 2015, 3:02 a.m., Matthew Ahrens wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.csiden.org/r/250/
> ---
> 
> (Updated Oct. 7, 2015, 3:02 a.m.)
> 
> 
> Review request for OpenZFS Developer Mailing List and Christopher Siden.
> 
> 
> Bugs: 6292
> https://www.illumos.org/issues/6292
> 
> 
> Repository: illumos-gate
> 
> 
> Description
> ---
> 
> 6292 exporting a pool while an async destroy is running can leave entries in 
> the deferred tree
> Reviewed by: Paul Dagnelie 
> Reviewed by: Matthew Ahrens 
> 
> Original author: George Wilson
> 
> 
> Diffs
> -
> 
>   usr/src/uts/common/fs/zfs/dsl_scan.c 
> 6ba5cb6a1c831f681fba16dbc8d25fd8b59b13c9 
> 
> Diff: https://reviews.csiden.org/r/250/diff/
> 
> 
> Testing
> ---
> 
> http://jenkins.delphix.com/job/zfs-precommit/3149/
> 
> 
> Thanks,
> 
> Matthew Ahrens
> 
>

___
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer

[OpenZFS Developer] Review Request 261: DLPX-36432 Fix cstyle errors in zfs codebase

2015-10-13 Thread Matthew Ahrens 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.csiden.org/r/261/
---

Review request for OpenZFS Developer Mailing List and Christopher Siden.


Bugs: 6328
https://www.illumos.org/issues/6328


Repository: illumos-gate


Description
---

6328 Fix cstyle errors in zfs codebase
Reviewed by: Matthew Ahrens 
Reviewed by: Alex Reece 

Original author: Paul Dagnelie


Diffs
-

  usr/src/uts/common/fs/zfs/zfs_ioctl.c 
f813116e4126634e3b01be2ee271a462e1a9a6e4 
  usr/src/uts/common/fs/zfs/zfeature.c 1833e1e27047f46681dddc4f236295f51c995872 
  usr/src/uts/common/fs/zfs/space_reftree.c 
a508092c530e0055023bf9ede71453e46e661bd6 
  usr/src/uts/common/fs/zfs/dmu.c 5b24268af1a813c153c75e8531cb3a6e23baa406 
  usr/src/lib/libzfs/common/libzfs_util.c 
507a72ce02630f23b4143076b02a6ca7903b22ef 
  usr/src/common/zfs/zfeature_common.c f75894b44deefd22f3b56152b7ed73ea6cbb74ab 
  usr/src/common/net/wanboot/p12aux.h 76c71c9cb63efab4f9bb11e9497bdfbf4f92afb6 
  usr/src/cmd/zpool/zpool_main.c 982057d1b52f945aaca2359f7ce6cb4651e7a999 
  usr/src/cmd/zoneadmd/vplat.c b9954b81b3375d3c0954671acb38d89b64a8e554 
  usr/src/cmd/zoneadm/zoneadm.c 6d80fcd8c3fa0bc55edf7c016bebe3eb9642da35 
  usr/src/uts/common/fs/zfs/zrlock.c 22151843e03054d5c12450237d88e556f3b0161e 
  usr/src/uts/common/fs/zfs/zio_checksum.c 
b471ad904749700031499147d64a9314a9491cc1 
  usr/src/uts/common/fs/zfs/zio.c 7fa795ea8cadafa0aa7bc241b0735b2fd4ce2593 
  usr/src/uts/common/fs/zfs/zfs_vnops.c 
fe740a5d28c1d68526d0eb84bc6090219627ba32 
  usr/src/uts/common/fs/zfs/zfs_replay.c 
3f98aaed7939cd6b398605f45309d018bb6ae0d6 
  usr/src/uts/common/fs/zfs/zfs_log.c 47d32a45c39d5a22291048ce4a39234d24c70557 
  usr/src/uts/common/fs/zfs/zfs_dir.c bd7424b55b5ef875d5f041dff567195c13119d38 
  usr/src/uts/common/fs/zfs/zap_leaf.c 96358f7bd80f22f8c17589cd4dfcc740bc13d726 
  usr/src/uts/common/fs/zfs/vdev_label.c 
7bbd7f2bde69a9aa059796cfeb13b8ce9e7b805a 
  usr/src/uts/common/fs/zfs/sys/zrlock.h 
dcd63f7b5b91f506449776b43df5ba94a58812a9 
  usr/src/uts/common/fs/zfs/dsl_dataset.c 
b06369ec13430d63b355a4e706dbc2e25bc5f2d2 
  usr/src/lib/libzpool/common/kernel.c a74276e95e4d76517d429a39bd8208e3b71ba967 
  usr/src/lib/libzfs/common/libzfs_pool.c 
0cc3ce4e58228a9fa88b83b874eb689c866f467b 
  usr/src/lib/libzfs/common/libzfs_iter.c 
5fdfe1d591acf9d9c550e7ac3d892b7db894213d 
  usr/src/grub/grub-0.97/stage2/fsys_zfs.c 
8c0d137e427c61a5b37919813c923f8280b11254 

Diff: https://reviews.csiden.org/r/261/diff/


Testing
---

ztest, zfs test suite
http://jenkins.delphix.com/job/os-build/4756/console


Thanks,

Matthew Ahrens

___
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer

Re: [OpenZFS Developer] Review Request 261: DLPX-36432 Fix cstyle errors in zfs codebase

2015-10-13 Thread Richard Elling 

---
This is an automatically generated e-mail. To reply, visit:
https://reviews.csiden.org/r/261/#review831
---

Ship it!


Ship It!

- Richard Elling


On Oct. 13, 2015, 5:26 p.m., Matthew Ahrens wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.csiden.org/r/261/
> ---
> 
> (Updated Oct. 13, 2015, 5:26 p.m.)
> 
> 
> Review request for OpenZFS Developer Mailing List and Christopher Siden.
> 
> 
> Bugs: 6328
> https://www.illumos.org/issues/6328
> 
> 
> Repository: illumos-gate
> 
> 
> Description
> ---
> 
> 6328 Fix cstyle errors in zfs codebase
> Reviewed by: Matthew Ahrens 
> Reviewed by: Alex Reece 
> 
> Original author: Paul Dagnelie
> 
> 
> Diffs
> -
> 
>   usr/src/uts/common/fs/zfs/sys/zrlock.h 
> dcd63f7b5b91f506449776b43df5ba94a58812a9 
>   usr/src/uts/common/fs/zfs/zio_checksum.c 
> b471ad904749700031499147d64a9314a9491cc1 
>   usr/src/uts/common/fs/zfs/zfs_replay.c 
> 3f98aaed7939cd6b398605f45309d018bb6ae0d6 
>   usr/src/uts/common/fs/zfs/zfs_ioctl.c 
> f813116e4126634e3b01be2ee271a462e1a9a6e4 
>   usr/src/uts/common/fs/zfs/zfeature.c 
> 1833e1e27047f46681dddc4f236295f51c995872 
>   usr/src/uts/common/fs/zfs/space_reftree.c 
> a508092c530e0055023bf9ede71453e46e661bd6 
>   usr/src/uts/common/fs/zfs/dmu.c 5b24268af1a813c153c75e8531cb3a6e23baa406 
>   usr/src/lib/libzfs/common/libzfs_util.c 
> 507a72ce02630f23b4143076b02a6ca7903b22ef 
>   usr/src/common/zfs/zfeature_common.c 
> f75894b44deefd22f3b56152b7ed73ea6cbb74ab 
>   usr/src/common/net/wanboot/p12aux.h 
> 76c71c9cb63efab4f9bb11e9497bdfbf4f92afb6 
>   usr/src/cmd/zpool/zpool_main.c 982057d1b52f945aaca2359f7ce6cb4651e7a999 
>   usr/src/cmd/zoneadmd/vplat.c b9954b81b3375d3c0954671acb38d89b64a8e554 
>   usr/src/cmd/zoneadm/zoneadm.c 6d80fcd8c3fa0bc55edf7c016bebe3eb9642da35 
>   usr/src/uts/common/fs/zfs/zrlock.c 22151843e03054d5c12450237d88e556f3b0161e 
>   usr/src/uts/common/fs/zfs/zio.c 7fa795ea8cadafa0aa7bc241b0735b2fd4ce2593 
>   usr/src/uts/common/fs/zfs/zfs_vnops.c 
> fe740a5d28c1d68526d0eb84bc6090219627ba32 
>   usr/src/uts/common/fs/zfs/zfs_log.c 
> 47d32a45c39d5a22291048ce4a39234d24c70557 
>   usr/src/uts/common/fs/zfs/zfs_dir.c 
> bd7424b55b5ef875d5f041dff567195c13119d38 
>   usr/src/uts/common/fs/zfs/zap_leaf.c 
> 96358f7bd80f22f8c17589cd4dfcc740bc13d726 
>   usr/src/uts/common/fs/zfs/vdev_label.c 
> 7bbd7f2bde69a9aa059796cfeb13b8ce9e7b805a 
>   usr/src/uts/common/fs/zfs/dsl_dataset.c 
> b06369ec13430d63b355a4e706dbc2e25bc5f2d2 
>   usr/src/lib/libzpool/common/kernel.c 
> a74276e95e4d76517d429a39bd8208e3b71ba967 
>   usr/src/lib/libzfs/common/libzfs_pool.c 
> 0cc3ce4e58228a9fa88b83b874eb689c866f467b 
>   usr/src/lib/libzfs/common/libzfs_iter.c 
> 5fdfe1d591acf9d9c550e7ac3d892b7db894213d 
>   usr/src/grub/grub-0.97/stage2/fsys_zfs.c 
> 8c0d137e427c61a5b37919813c923f8280b11254 
> 
> Diff: https://reviews.csiden.org/r/261/diff/
> 
> 
> Testing
> ---
> 
> ztest, zfs test suite
> http://jenkins.delphix.com/job/os-build/4756/console
> 
> 
> Thanks,
> 
> Matthew Ahrens
> 
>

___
developer mailing list
developer@open-zfs.org
http://lists.open-zfs.org/mailman/listinfo/developer