Re: svn commit: r324011 - in head: cddl/contrib/opensolaris/cmd/ztest sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys sys/cddl/contrib/opensolaris/uts

2017-09-26 Thread Andriy Gapon
On 26/09/2017 16:48, O. Hartmann wrote:
> Build world/kernel on r324015 fails due to:
> 
> [...]
> ===> lib/libpam/modules/pam_login_access (obj)
> --- cddl/lib__L ---
> --- zil.o ---
> /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c:2333:19: 
> warning:
> implicit declaration of function 'cv_timedwait_sbt' is invalid in C99
> [-Wimplicit-function-declaration] int wait_err = 
> cv_timedwait_sbt(>zcw_cv,
> ^ /usr/src/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c:2335:8: error:
> use of undeclared identifier 'C_ABSOLUTE' C_ABSOLUTE);
> ^
> --- lib__L ---
> --- obj_subdir_lib/libpam/modules/pam_nologin ---
> ===> lib/libpam/modules/pam_nologin (obj)
> --- cddl/lib__L ---
> 1 warning and 1 error generated.

Seems like messed up the commit.
Not only I managed to somehow commit an older version of what I had in the
review request, but I also forget that zil.c is compiled in userland mode as 
well.

A fix is coming shortly.
Apologies for the breakage.

-- 
Andriy Gapon
___
svn-src-all@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"


Re: svn commit: r324011 - in head: cddl/contrib/opensolaris/cmd/ztest sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys sys/cddl/contrib/opensolaris/uts

2017-09-26 Thread O. Hartmann
On Tue, 26 Sep 2017 11:04:08 + (UTC)
Andriy Gapon  wrote:

> Author: avg
> Date: Tue Sep 26 11:04:08 2017
> New Revision: 324011
> URL: https://svnweb.freebsd.org/changeset/base/324011
> 
> Log:
>   MFV r323535: 8585 improve batching done in zil_commit()
>   
>   FreeBSD notes:
>   - this MFV reverts FreeBSD commit r314549 to make the merge easier
>   - at present our emulation of cv_timedwait_hires is rather poor,
> so I elected to use cv_timedwait_sbt directly
>   Please see the differential revision for details.
>   Unfortunately, I did not get any positive reviews, so there could be
>   bugs in the FreeBSD-specific piece of the merge.
>   Hence, the long MFC timeout.
>   
>   illumos/illumos-gate@1271e4b10dfaaed576c08a812f466f6e81370e5e
>   
> https://github.com/illumos/illumos-gate/commit/1271e4b10dfaaed576c08a812f466f6e81370e5e
>   
>   https://www.illumos.org/issues/8585
> The current implementation of zil_commit() can introduce significant
> latency, beyond what is inherent due to the latency of the underlying
> storage. The additional latency comes from two main problems:
> 1. When there's outstanding ZIL blocks being written (i.e. there's
> already a "writer thread" in progress), then any new calls to
> zil_commit() will block waiting for the currently oustanding ZIL
> blocks to complete. The blocks written for each "writer thread" is
> coined a "batch", and there can only ever be a single "batch" being
> written at a time. When a batch is being written, any new ZIL
> transactions will have to wait for the next batch to be written,
> which won't occur until the current batch finishes.
> As a result, the underlying storage may not be used as efficiently
> as possible. While "new" threads enter zil_commit() and are blocked
> waiting for the next batch, it's possible that the underlying
> storage isn't fully utilized by the current batch of ZIL blocks. In
> that case, it'd be better to allow these new threads to generate
> (and issue) a new ZIL block, such that it could be serviced by the
> underlying storage concurrently with the other ZIL blocks that are
> being serviced.
> 2. Any call to zil_commit() must wait for all ZIL blocks in its "batch"
> to complete, prior to zil_commit() returning. The size of any given
> batch is proportional to the number of ZIL transaction in the queue
> at the time that the batch starts processing the queue; which
> doesn't occur until the previous batch completes. Thus, if there's a
> lot of transactions in the queue, the batch could be composed of
> many ZIL blocks, and each call to zil_commit() will have to wait for
> all of these writes to complete (even if the thread calling
> zil_commit() only cared about one of the transactions in the batch).
>   
>   Reviewed by: Brad Lewis 
>   Reviewed by: Matt Ahrens 
>   Reviewed by: George Wilson 
>   Approved by: Dan McDonald 
>   Author: Prakash Surya 
>   
>   MFC after:  1 month
>   Differential Revision:  https://reviews.freebsd.org/D12355
> 
> Modified:
>   head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dmu.h
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zil.h
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zil_impl.h
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/txg.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
>   head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
>   head/sys/cddl/contrib/opensolaris/uts/common/sys/debug.h
> Directory Properties:
>   head/cddl/contrib/opensolaris/   (props changed)
>   head/sys/cddl/contrib/opensolaris/   (props changed)
> 
> Modified: head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
> ==
> --- head/cddl/contrib/opensolaris/cmd/ztest/ztest.c   Tue Sep 26
> 09:34:18 2017 (r324010) +++
> head/cddl/contrib/opensolaris/cmd/ztest/ztest.c   Tue Sep 26 11:04:08
> 2017  (r324011) @@ -1825,13 +1825,14 @@ ztest_get_done(zgd_t *zgd, int
> error) ztest_object_unlock(zd, object); 
>   if (error == 0 && zgd->zgd_bp)
> - zil_add_block(zgd->zgd_zilog, zgd->zgd_bp);
> + zil_lwb_add_block(zgd->zgd_lwb, zgd->zgd_bp);
>  
>   umem_free(zgd, sizeof (*zgd));
>  }
>  
>  static int
> -ztest_get_data(void *arg, lr_write_t *lr, char *buf, zio_t *zio)
> +ztest_get_data(void *arg, 

svn commit: r324011 - in head: cddl/contrib/opensolaris/cmd/ztest sys/cddl/contrib/opensolaris/uts/common/fs/zfs sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys sys/cddl/contrib/opensolaris/uts/...

2017-09-26 Thread Andriy Gapon
Author: avg
Date: Tue Sep 26 11:04:08 2017
New Revision: 324011
URL: https://svnweb.freebsd.org/changeset/base/324011

Log:
  MFV r323535: 8585 improve batching done in zil_commit()
  
  FreeBSD notes:
  - this MFV reverts FreeBSD commit r314549 to make the merge easier
  - at present our emulation of cv_timedwait_hires is rather poor,
so I elected to use cv_timedwait_sbt directly
  Please see the differential revision for details.
  Unfortunately, I did not get any positive reviews, so there could be
  bugs in the FreeBSD-specific piece of the merge.
  Hence, the long MFC timeout.
  
  illumos/illumos-gate@1271e4b10dfaaed576c08a812f466f6e81370e5e
  
https://github.com/illumos/illumos-gate/commit/1271e4b10dfaaed576c08a812f466f6e81370e5e
  
  https://www.illumos.org/issues/8585
The current implementation of zil_commit() can introduce significant
latency, beyond what is inherent due to the latency of the underlying
storage. The additional latency comes from two main problems:
1. When there's outstanding ZIL blocks being written (i.e. there's
already a "writer thread" in progress), then any new calls to
zil_commit() will block waiting for the currently oustanding ZIL
blocks to complete. The blocks written for each "writer thread" is
coined a "batch", and there can only ever be a single "batch" being
written at a time. When a batch is being written, any new ZIL
transactions will have to wait for the next batch to be written,
which won't occur until the current batch finishes.
As a result, the underlying storage may not be used as efficiently
as possible. While "new" threads enter zil_commit() and are blocked
waiting for the next batch, it's possible that the underlying
storage isn't fully utilized by the current batch of ZIL blocks. In
that case, it'd be better to allow these new threads to generate
(and issue) a new ZIL block, such that it could be serviced by the
underlying storage concurrently with the other ZIL blocks that are
being serviced.
2. Any call to zil_commit() must wait for all ZIL blocks in its "batch"
to complete, prior to zil_commit() returning. The size of any given
batch is proportional to the number of ZIL transaction in the queue
at the time that the batch starts processing the queue; which
doesn't occur until the previous batch completes. Thus, if there's a
lot of transactions in the queue, the batch could be composed of
many ZIL blocks, and each call to zil_commit() will have to wait for
all of these writes to complete (even if the thread calling
zil_commit() only cared about one of the transactions in the batch).
  
  Reviewed by: Brad Lewis 
  Reviewed by: Matt Ahrens 
  Reviewed by: George Wilson 
  Approved by: Dan McDonald 
  Author: Prakash Surya 
  
  MFC after:1 month
  Differential Revision:https://reviews.freebsd.org/D12355

Modified:
  head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/dmu.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/dmu.h
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zil.h
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zil_impl.h
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/sys/zio.h
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/txg.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zfs_vnops.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zil.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zio.c
  head/sys/cddl/contrib/opensolaris/uts/common/fs/zfs/zvol.c
  head/sys/cddl/contrib/opensolaris/uts/common/sys/debug.h
Directory Properties:
  head/cddl/contrib/opensolaris/   (props changed)
  head/sys/cddl/contrib/opensolaris/   (props changed)

Modified: head/cddl/contrib/opensolaris/cmd/ztest/ztest.c
==
--- head/cddl/contrib/opensolaris/cmd/ztest/ztest.c Tue Sep 26 09:34:18 
2017(r324010)
+++ head/cddl/contrib/opensolaris/cmd/ztest/ztest.c Tue Sep 26 11:04:08 
2017(r324011)
@@ -1825,13 +1825,14 @@ ztest_get_done(zgd_t *zgd, int error)
ztest_object_unlock(zd, object);
 
if (error == 0 && zgd->zgd_bp)
-   zil_add_block(zgd->zgd_zilog, zgd->zgd_bp);
+   zil_lwb_add_block(zgd->zgd_lwb, zgd->zgd_bp);
 
umem_free(zgd, sizeof (*zgd));
 }
 
 static int
-ztest_get_data(void *arg, lr_write_t *lr, char *buf, zio_t *zio)
+ztest_get_data(void *arg, lr_write_t *lr, char *buf, struct lwb *lwb,
+zio_t *zio)
 {
ztest_ds_t *zd = arg;
objset_t *os = zd->zd_os;
@@ -1845,6 +1846,10 @@ ztest_get_data(void *arg, lr_write_t *lr, char *buf, z
zgd_t *zgd;