[developer] Re: [openzfs/openzfs] 9647 Introduce ZFS Read/Write kstats (#664)

[Serapheim Dimitropoulos]

This is not ready to go yet.

I still haven't updated the code with the restructuring from ZoL. There is a 
kstat memory leak that I need to narrow down in the new code before updating 
this. Sorry for the delay.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/664#issuecomment-426351265
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Ta167a88bbab5c009-Me25b2d91d7d659ccb8d2c920
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Matthew Ahrens]

ahrens commented on this pull request.



> @@ -92,124 +93,146 @@
  * growth all other writers and readers must be excluded.

sure thing.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/680#discussion_r222039193
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-Me2d9439091c8d4aa8c280192
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Matthew Ahrens]

ahrens commented on this pull request.



> -  */
-   if (new->r_type == RL_APPEND)
-   new->r_off = zp->z_size;
-
-   /*
-* If we need to grow the block size then grab the whole
-* file range. This is also done under z_range_lock to
-* avoid races.
-*/
-   end_size = MAX(zp->z_size, new->r_off + len);
-   if (end_size > zp->z_blksz && (!ISP2(zp->z_blksz) ||
-   zp->z_blksz < zp->z_zfsvfs->z_max_blksz)) {
-   new->r_off = 0;
-   new->r_len = UINT64_MAX;
-   }
+   if (rl->rl_cb != NULL) {

OK.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/680#discussion_r222039239
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-Md76db1d71a963a8c04c5d330
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Matthew Ahrens]

@ahrens pushed 1 commit.

ce2975a  add comments


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/680/files/0aef34ea20590747caece45b2aa12c3e71a9d2a4..ce2975a02af44d8eb198703b1978f123a4af4bd0

--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-M88e38b39ed0ec126791baad7
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 8589 zfs send | recv crashes receiving system (#685)

[waikontse]

@waikontse pushed 1 commit.

c36b758  Update code formatting and comments


-- 
You are receiving this because you are subscribed to this thread.
View it on GitHub:
https://github.com/openzfs/openzfs/pull/685/files/bbc5f9c450496aacd33228c59a5e9c3addaa216c..c36b7580a0a0875ad8f7548099b47898cdfbf891

--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tcf25f566d2c2fe2d-M95e3564d6c59ec89489c459c
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 8589 zfs send | recv crashes receiving system (#685)

[Matthew Ahrens]

ahrens approved this pull request.





-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/685#pullrequestreview-160900425
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tcf25f566d2c2fe2d-M3a7e1e5be2ca45bcb048e34d
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] First OpenZFS Leadership Meeting

[Matthew Ahrens]

At this year's OpenZFS Developer Conference, we decided we should have more
regular meetings to coordinate activities across all the platforms.  The
first meeting will be held *October 9th, 1:00-1:30pm Pacific time*, and we
plan to hold this every 4 weeks thereafter.  Everyone is welcome to attend
and participate, and we will try to keep the meeting on agenda and on
time.  The meetings will be held online via Zoom
, and recorded and posted to the
website and YouTube after the meeting.

The agenda for the first meeting will be a discussion of *feature
availability on each platform*, with a goal of finding owners for
everything in the "integrated to at least one platform" section.

For more information and details on how to attend, please see the agenda
document:

https://docs.google.com/document/d/1w2jv2XVYFmBVvG1EGf-9A5HBVsjAYoLIFZAnWHhV-BM/edit

--matt

p.s. Videos from the OpenZFS Developer Summit are now available:
http://www.open-zfs.org/wiki/OpenZFS_Developer_Summit_2018

--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tb3df73bfa8694d0d-M222caa5ea300461f1412af0e
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9861 Harden ZFS receive against out-of-bounds property values (#630)

[Allan Jude]

allanjude commented on this pull request.



> @@ -2669,6 +2678,18 @@ receive_read_record(struct receive_arg *ra)
case DRR_WRITE:
{
struct drr_write *drrw = >rrd->header.drr_u.drr_write;
+   if (drrw->drr_compressiontype >= ZIO_COMPRESS_FUNCTIONS)
+   return (SET_ERROR(ENOTSUP));
+   if (drrw->drr_checksumtype >= ZIO_CHECKSUM_FUNCTIONS)
+   return (SET_ERROR(ENOTSUP));
+   if (P2PHASE(drrw->drrw->drr_logical_size, SPA_MINBLOCKSIZE))

fixed

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/630#discussion_r222130752
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tc3a0a7ad59c999df-Mcae0516813734976761cd86b
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9861 Harden ZFS receive against out-of-bounds property values (#630)

[Allan Jude]

Sorry for the build error, it seemed to have crept in when I refactored this

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/630#issuecomment-426450196
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tc3a0a7ad59c999df-M81ee3672e0a9c139f041416e
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Matthew Ahrens]

Thanks @behlendorf.  I think the potential issue with cv_destroy() is out of 
scope for this change, which isn't really touching that (just renaming the cv 
that it's operating on).  I agree that if conflicting changes were made in ZoL, 
you'll want to keep the ZoL version and just change the names.  IIRC 
@pcd1193182 did some research a while back on the issue of accidentally using a 
CV after destroying it, he may be able to comment on if the same problem is 
possible on illumos.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/680#issuecomment-426450765
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-Ma26240ffd4911388de562c5f
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Brian Behlendorf]

behlendorf approved this pull request.

Looks good.  While reviewing I took the opportunity to [port this to 
Linux](https://github.com/zfsonlinux/zfs/pull/7980).

My only concern is the use of the `cv_broadcast(); cv_destroy();` construct.  
This was always dodgy because once `cv_destroy()` was called no process could 
safely access the cv except to wake up and exit `cv_wait()`.  `cv_destroy()` 
does block for all current waiters to exit so there's no use-after-free issue, 
but no new waiters are allowed, and according to the commit logs we ran in to 
some deadlocks with the VFS due to the waiting.  I believe @lundman ran in to 
the same issue, I don't think this ever a problem for illumos of FreeBSD simply 
due to VFS differences.

This is all to say there's a good chance that at least on Linux we're going to 
need to add back in our fix for this issue which was to defer the 
`cv_destroy()` until the end of `zfs_range_unlock()`.



-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/680#pullrequestreview-160924430
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-M50c5b4189022d5338b133155
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Matthew Ahrens]

internal tests: 
http://platform.jenkins.delphix.com/job/devops-gate/job/master/job/zfs-precommit/3784/flowGraphTable/

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/680#issuecomment-426497318
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-M7e9a390cb10e970d8ca20f18
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9485 Optimize possible split block search space (#625)

[Matthew Ahrens]

I've updated the code to pull in 
https://github.com/zfsonlinux/zfs/commit/1258bd778e8279a4cd051543827f333fe2daed76.
  @sdimitro could you take another look at this?

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/625#issuecomment-426506105
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tf29308eb1ca3d052-M323d031556b2e435ac9a51d6
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9485 Optimize possible split block search space (#625)

[Serapheim Dimitropoulos]

sdimitro approved this pull request.

I just found one minor nit. Code LGTM.

Are there any plans of doing any extra testing for this in illumos?

> - * majority of segment copies are good. This allows all the segment copies to
- * participate fairly in the reconstruction and prevents the repeated use of
- * one bad copy.
+ * If an indirect split block contains more than this many possible unique
+ * combinations when being reconstructed, consider it too computationally
+ * expensive to check them all. Instead, try at most 100 randomly-selected
+ * combinations each time the block is accessed.  This allows all segment
+ * copies to participate fairly in the reconstruction when all combinations
+ * cannot be checked and prevents repeated use of one bad copy.
+ */
+int zfs_reconstruct_indirect_combinations_max = 256;
+
+
+/*
+ * Enable to simulate damaged segments and validate reconstruction.  This
+ * is intentionally not exposed as a module parameter.

We don't have "module parameters" in illumos so maybe we should take out this 
sentence just to avoid confusion.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/625#pullrequestreview-161002747
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tf29308eb1ca3d052-M9fe82ccb9076625963e36ad9
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 9689 zfs range lock code should not be zpl-specific (#680)

[Matthew Ahrens]

@brad-lewis I've addressed the code review feedback and rebased this.  If you 
need to update your branch, be sure to pull down the new version first.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/680#issuecomment-426497233
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/T0ada5cf1c6037d04-Me581d6f530dbcd98932da375
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription

[developer] Re: [openzfs/openzfs] 8589 zfs send | recv crashes receiving system (#685)

[waikontse]

waikontse commented on this pull request.



> @@ -392,6 +392,22 @@ static int
 dump_freeobjects(dmu_sendarg_t *dsp, uint64_t firstobj, uint64_t numobjs)
 {
struct drr_freeobjects *drrfo = &(dsp->dsa_drr->drr_u.drr_freeobjects);
+   uint64_t maxobj = DNODES_PER_BLOCK *
+   (DMU_META_DNODE(dsp->dsa_os)->dn_maxblkid + 1);
+
+   /*
+* Older versions of OpenZFS (since 7104) does not handle large 
+ * FREEOBJECTS records correctly, leading to zfs recv never 
completing. 

Hi @ahrens, thanks for the guidance on the commenting.
I need to get used to tab spaces, I usually use spaces. 

You're right, what I mean is that versions "less recent"/"older than".  I'll 
update the formatting with the comment later today.

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/openzfs/openzfs/pull/685#discussion_r221905306
--
openzfs: openzfs-developer
Permalink: 
https://openzfs.topicbox.com/groups/developer/Tcf25f566d2c2fe2d-Meb3fb3a23b754da194914588
Delivery options: https://openzfs.topicbox.com/groups/developer/subscription