Re: svn commit: r329814 - head/sys/cam

[Warner Losh]

On Feb 22, 2018 3:57 AM, "Warner Losh"  wrote:



On Thu, Feb 22, 2018 at 12:01 AM, Allan Jude  wrote:

> On 2018-02-22 01:07, O. Hartmann wrote:
> > On Thu, 22 Feb 2018 05:43:50 + (UTC)
> > Warner Losh  wrote:
> >
> >> Author: imp
> >> Date: Thu Feb 22 05:43:50 2018
> >> New Revision: 329814
> >> URL: https://svnweb.freebsd.org/changeset/base/329814
> >>
> >> Log:
> >>   Note when we tick.
> >>
> >>   To help implement a policy of 'queue all trims until next I/O sched
> >>   tick' policy to help coalesce them, note when we tick so we can do
> >>   something special on the first call after the tick to get more work.
> >>
> >>   Sponsored by: Netflix
> >>
> >> Modified:
> >>   head/sys/cam/cam_iosched.c
> >>
> >> Modified: head/sys/cam/cam_iosched.c
> >> 
> ==
> >> --- head/sys/cam/cam_iosched.c   Thu Feb 22 05:43:45 2018
> >> (r329813) +++ head/sys/cam/cam_iosched.c Thu Feb 22 05:43:50
> >> 2018 (r329814) @@ -60,6 +60,19 @@ static MALLOC_DEFINE(M_CAMSCHED,
> >> "CAM I/O Scheduler", "CAM I/O Scheduler buffers");
> >>
> >>  /*
> >> + * Trim or similar currently pending completion. Should only be set for
> >> + * those drivers wishing only one Trim active at a time.
> >> + */
> >> +#define CAM_IOSCHED_FLAG_TRIM_ACTIVE(1ul << 0)
> >> +/* Callout active, and needs to be torn down */
> >> +#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
> >> +/* Timer has just ticked */
> >> +#define CAM_IOSCHED_FLAG_TICK   (1ul << 2)
> >> +
> >> +/* Periph drivers set these flags to indicate work
> */
> >> +#define CAM_IOSCHED_FLAG_WORK_FLAGS ((0xu) << 16)
> >> +
> >> +/*
> >>   * Default I/O scheduler for FreeBSD. This implementation is just a
> >> thin-vineer
> >>   * over the bioq_* interface, with notions of separate calls for
> normal I/O
> >> and
> >>   * for trims.
> >> @@ -574,6 +587,7 @@ cam_iosched_ticker(void *arg)
> >>  cam_iosched_limiter_tick(>write_stats);
> >>  cam_iosched_limiter_tick(>trim_stats);
> >>
> >> +isc->flags |= CAM_IOSCHED_FLAGS_TICK;
> >>  cam_iosched_schedule(isc, isc->periph);
> >>
> >>  /*
> >> @@ -701,17 +715,6 @@ cam_iosched_cl_maybe_steer(struct control_loop
> *clp)
> >>  }
> >>  #endif
> >>
> >> -/*
> >> - * Trim or similar currently pending completion. Should only be set for
> >> - * those drivers wishing only one Trim active at a time.
> >> - */
> >> -#define CAM_IOSCHED_FLAG_TRIM_ACTIVE(1ul << 0)
> >> -/* Callout active, and needs to be torn down */
> >> -#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
> >> -
> >> -/* Periph drivers set these flags to indicate work
> */
> >> -#define CAM_IOSCHED_FLAG_WORK_FLAGS ((0xu) << 16)
> >> -
> >>  #ifdef CAM_IOSCHED_DYNAMIC
> >>  static void
> >>  cam_iosched_io_metric_update(struct cam_iosched_softc *isc,
> >> @@ -1322,6 +1325,10 @@ struct bio *
> >>  cam_iosched_next_bio(struct cam_iosched_softc *isc)
> >>  {
> >>  struct bio *bp;
> >> +bool wastick;
> >> +
> >> +wastick = !!(isc->flags & CAM_IOSCHED_FLAGS_TICK);
> >> +isc->flags &= ~CAM_IOSCHED_FLAGS_TICK;
> >>
> >>  /*
> >>   * See if we have a trim that can be scheduled. We can only send
> one
> >> ___
> >> svn-src-head@freebsd.org mailing list
> >> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> >> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
> >
> > A recent buildworld/buildkernel failed with this error just right now:
> >
> > --- cam_iosched.o ---
> > /usr/src/sys/cam/cam_iosched.c:593:16: error: use of undeclared
> identifier
> > 'CAM_IOSCHED_FLAGS_TICK' isc->flags |= CAM_IOSCHED_FLAGS_TICK;
> >
>
> Looks like a typo, FLAGS_TICK vs FLAG_TICK
>

It is. I'm unsure how, but it appears to have snuck in during my final
rebasing to current... I had a mix in the middle of development and thought
I'd fixed them all...


Something is not right with the series. Backing out and will recommit when
its sorted out.

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

Re: svn commit: r329814 - head/sys/cam

[Warner Losh]

On Thu, Feb 22, 2018 at 12:01 AM, Allan Jude  wrote:

> On 2018-02-22 01:07, O. Hartmann wrote:
> > On Thu, 22 Feb 2018 05:43:50 + (UTC)
> > Warner Losh  wrote:
> >
> >> Author: imp
> >> Date: Thu Feb 22 05:43:50 2018
> >> New Revision: 329814
> >> URL: https://svnweb.freebsd.org/changeset/base/329814
> >>
> >> Log:
> >>   Note when we tick.
> >>
> >>   To help implement a policy of 'queue all trims until next I/O sched
> >>   tick' policy to help coalesce them, note when we tick so we can do
> >>   something special on the first call after the tick to get more work.
> >>
> >>   Sponsored by: Netflix
> >>
> >> Modified:
> >>   head/sys/cam/cam_iosched.c
> >>
> >> Modified: head/sys/cam/cam_iosched.c
> >> 
> ==
> >> --- head/sys/cam/cam_iosched.c   Thu Feb 22 05:43:45 2018
> >> (r329813) +++ head/sys/cam/cam_iosched.c Thu Feb 22 05:43:50
> >> 2018 (r329814) @@ -60,6 +60,19 @@ static MALLOC_DEFINE(M_CAMSCHED,
> >> "CAM I/O Scheduler", "CAM I/O Scheduler buffers");
> >>
> >>  /*
> >> + * Trim or similar currently pending completion. Should only be set for
> >> + * those drivers wishing only one Trim active at a time.
> >> + */
> >> +#define CAM_IOSCHED_FLAG_TRIM_ACTIVE(1ul << 0)
> >> +/* Callout active, and needs to be torn down */
> >> +#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
> >> +/* Timer has just ticked */
> >> +#define CAM_IOSCHED_FLAG_TICK   (1ul << 2)
> >> +
> >> +/* Periph drivers set these flags to indicate work
> */
> >> +#define CAM_IOSCHED_FLAG_WORK_FLAGS ((0xu) << 16)
> >> +
> >> +/*
> >>   * Default I/O scheduler for FreeBSD. This implementation is just a
> >> thin-vineer
> >>   * over the bioq_* interface, with notions of separate calls for
> normal I/O
> >> and
> >>   * for trims.
> >> @@ -574,6 +587,7 @@ cam_iosched_ticker(void *arg)
> >>  cam_iosched_limiter_tick(>write_stats);
> >>  cam_iosched_limiter_tick(>trim_stats);
> >>
> >> +isc->flags |= CAM_IOSCHED_FLAGS_TICK;
> >>  cam_iosched_schedule(isc, isc->periph);
> >>
> >>  /*
> >> @@ -701,17 +715,6 @@ cam_iosched_cl_maybe_steer(struct control_loop
> *clp)
> >>  }
> >>  #endif
> >>
> >> -/*
> >> - * Trim or similar currently pending completion. Should only be set for
> >> - * those drivers wishing only one Trim active at a time.
> >> - */
> >> -#define CAM_IOSCHED_FLAG_TRIM_ACTIVE(1ul << 0)
> >> -/* Callout active, and needs to be torn down */
> >> -#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
> >> -
> >> -/* Periph drivers set these flags to indicate work
> */
> >> -#define CAM_IOSCHED_FLAG_WORK_FLAGS ((0xu) << 16)
> >> -
> >>  #ifdef CAM_IOSCHED_DYNAMIC
> >>  static void
> >>  cam_iosched_io_metric_update(struct cam_iosched_softc *isc,
> >> @@ -1322,6 +1325,10 @@ struct bio *
> >>  cam_iosched_next_bio(struct cam_iosched_softc *isc)
> >>  {
> >>  struct bio *bp;
> >> +bool wastick;
> >> +
> >> +wastick = !!(isc->flags & CAM_IOSCHED_FLAGS_TICK);
> >> +isc->flags &= ~CAM_IOSCHED_FLAGS_TICK;
> >>
> >>  /*
> >>   * See if we have a trim that can be scheduled. We can only send
> one
> >> ___
> >> svn-src-head@freebsd.org mailing list
> >> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> >> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
> >
> > A recent buildworld/buildkernel failed with this error just right now:
> >
> > --- cam_iosched.o ---
> > /usr/src/sys/cam/cam_iosched.c:593:16: error: use of undeclared
> identifier
> > 'CAM_IOSCHED_FLAGS_TICK' isc->flags |= CAM_IOSCHED_FLAGS_TICK;
> >
>
> Looks like a typo, FLAGS_TICK vs FLAG_TICK
>

It is. I'm unsure how, but it appears to have snuck in during my final
rebasing to current... I had a mix in the middle of development and thought
I'd fixed them all...

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

Re: svn commit: r329814 - head/sys/cam

[Allan Jude]

On 2018-02-22 01:07, O. Hartmann wrote:
> On Thu, 22 Feb 2018 05:43:50 + (UTC)
> Warner Losh  wrote:
> 
>> Author: imp
>> Date: Thu Feb 22 05:43:50 2018
>> New Revision: 329814
>> URL: https://svnweb.freebsd.org/changeset/base/329814
>>
>> Log:
>>   Note when we tick.
>>   
>>   To help implement a policy of 'queue all trims until next I/O sched
>>   tick' policy to help coalesce them, note when we tick so we can do
>>   something special on the first call after the tick to get more work.
>>   
>>   Sponsored by: Netflix
>>
>> Modified:
>>   head/sys/cam/cam_iosched.c
>>
>> Modified: head/sys/cam/cam_iosched.c
>> ==
>> --- head/sys/cam/cam_iosched.c   Thu Feb 22 05:43:45 2018
>> (r329813) +++ head/sys/cam/cam_iosched.c Thu Feb 22 05:43:50
>> 2018 (r329814) @@ -60,6 +60,19 @@ static MALLOC_DEFINE(M_CAMSCHED,
>> "CAM I/O Scheduler", "CAM I/O Scheduler buffers");
>>  
>>  /*
>> + * Trim or similar currently pending completion. Should only be set for
>> + * those drivers wishing only one Trim active at a time.
>> + */
>> +#define CAM_IOSCHED_FLAG_TRIM_ACTIVE(1ul << 0)
>> +/* Callout active, and needs to be torn down */
>> +#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
>> +/* Timer has just ticked */
>> +#define CAM_IOSCHED_FLAG_TICK   (1ul << 2)
>> +
>> +/* Periph drivers set these flags to indicate work */
>> +#define CAM_IOSCHED_FLAG_WORK_FLAGS ((0xu) << 16)
>> +
>> +/*
>>   * Default I/O scheduler for FreeBSD. This implementation is just a
>> thin-vineer
>>   * over the bioq_* interface, with notions of separate calls for normal I/O
>> and
>>   * for trims.
>> @@ -574,6 +587,7 @@ cam_iosched_ticker(void *arg)
>>  cam_iosched_limiter_tick(>write_stats);
>>  cam_iosched_limiter_tick(>trim_stats);
>>  
>> +isc->flags |= CAM_IOSCHED_FLAGS_TICK;
>>  cam_iosched_schedule(isc, isc->periph);
>>  
>>  /*
>> @@ -701,17 +715,6 @@ cam_iosched_cl_maybe_steer(struct control_loop *clp)
>>  }
>>  #endif
>>  
>> -/*
>> - * Trim or similar currently pending completion. Should only be set for
>> - * those drivers wishing only one Trim active at a time.
>> - */
>> -#define CAM_IOSCHED_FLAG_TRIM_ACTIVE(1ul << 0)
>> -/* Callout active, and needs to be torn down */
>> -#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
>> -
>> -/* Periph drivers set these flags to indicate work */
>> -#define CAM_IOSCHED_FLAG_WORK_FLAGS ((0xu) << 16)
>> -
>>  #ifdef CAM_IOSCHED_DYNAMIC
>>  static void
>>  cam_iosched_io_metric_update(struct cam_iosched_softc *isc,
>> @@ -1322,6 +1325,10 @@ struct bio *
>>  cam_iosched_next_bio(struct cam_iosched_softc *isc)
>>  {
>>  struct bio *bp;
>> +bool wastick;
>> +
>> +wastick = !!(isc->flags & CAM_IOSCHED_FLAGS_TICK);
>> +isc->flags &= ~CAM_IOSCHED_FLAGS_TICK;
>>  
>>  /*
>>   * See if we have a trim that can be scheduled. We can only send one
>> ___
>> svn-src-head@freebsd.org mailing list
>> https://lists.freebsd.org/mailman/listinfo/svn-src-head
>> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"
> 
> A recent buildworld/buildkernel failed with this error just right now:
> 
> --- cam_iosched.o ---
> /usr/src/sys/cam/cam_iosched.c:593:16: error: use of undeclared identifier
> 'CAM_IOSCHED_FLAGS_TICK' isc->flags |= CAM_IOSCHED_FLAGS_TICK;
> 

Looks like a typo, FLAGS_TICK vs FLAG_TICK

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

Re: svn commit: r329814 - head/sys/cam

[O. Hartmann]

On Thu, 22 Feb 2018 05:43:50 + (UTC)
Warner Losh  wrote:

> Author: imp
> Date: Thu Feb 22 05:43:50 2018
> New Revision: 329814
> URL: https://svnweb.freebsd.org/changeset/base/329814
> 
> Log:
>   Note when we tick.
>   
>   To help implement a policy of 'queue all trims until next I/O sched
>   tick' policy to help coalesce them, note when we tick so we can do
>   something special on the first call after the tick to get more work.
>   
>   Sponsored by: Netflix
> 
> Modified:
>   head/sys/cam/cam_iosched.c
> 
> Modified: head/sys/cam/cam_iosched.c
> ==
> --- head/sys/cam/cam_iosched.cThu Feb 22 05:43:45 2018
> (r329813) +++ head/sys/cam/cam_iosched.c  Thu Feb 22 05:43:50
> 2018  (r329814) @@ -60,6 +60,19 @@ static MALLOC_DEFINE(M_CAMSCHED,
> "CAM I/O Scheduler", "CAM I/O Scheduler buffers");
>  
>  /*
> + * Trim or similar currently pending completion. Should only be set for
> + * those drivers wishing only one Trim active at a time.
> + */
> +#define CAM_IOSCHED_FLAG_TRIM_ACTIVE (1ul << 0)
> + /* Callout active, and needs to be torn down */
> +#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
> + /* Timer has just ticked */
> +#define CAM_IOSCHED_FLAG_TICK(1ul << 2)
> +
> + /* Periph drivers set these flags to indicate work */
> +#define CAM_IOSCHED_FLAG_WORK_FLAGS  ((0xu) << 16)
> +
> +/*
>   * Default I/O scheduler for FreeBSD. This implementation is just a
> thin-vineer
>   * over the bioq_* interface, with notions of separate calls for normal I/O
> and
>   * for trims.
> @@ -574,6 +587,7 @@ cam_iosched_ticker(void *arg)
>   cam_iosched_limiter_tick(>write_stats);
>   cam_iosched_limiter_tick(>trim_stats);
>  
> + isc->flags |= CAM_IOSCHED_FLAGS_TICK;
>   cam_iosched_schedule(isc, isc->periph);
>  
>   /*
> @@ -701,17 +715,6 @@ cam_iosched_cl_maybe_steer(struct control_loop *clp)
>  }
>  #endif
>  
> -/*
> - * Trim or similar currently pending completion. Should only be set for
> - * those drivers wishing only one Trim active at a time.
> - */
> -#define CAM_IOSCHED_FLAG_TRIM_ACTIVE (1ul << 0)
> - /* Callout active, and needs to be torn down */
> -#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
> -
> - /* Periph drivers set these flags to indicate work */
> -#define CAM_IOSCHED_FLAG_WORK_FLAGS  ((0xu) << 16)
> -
>  #ifdef CAM_IOSCHED_DYNAMIC
>  static void
>  cam_iosched_io_metric_update(struct cam_iosched_softc *isc,
> @@ -1322,6 +1325,10 @@ struct bio *
>  cam_iosched_next_bio(struct cam_iosched_softc *isc)
>  {
>   struct bio *bp;
> + bool wastick;
> + 
> + wastick = !!(isc->flags & CAM_IOSCHED_FLAGS_TICK);
> + isc->flags &= ~CAM_IOSCHED_FLAGS_TICK;
>  
>   /*
>* See if we have a trim that can be scheduled. We can only send one
> ___
> svn-src-head@freebsd.org mailing list
> https://lists.freebsd.org/mailman/listinfo/svn-src-head
> To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

A recent buildworld/buildkernel failed with this error just right now:

--- cam_iosched.o ---
/usr/src/sys/cam/cam_iosched.c:593:16: error: use of undeclared identifier
'CAM_IOSCHED_FLAGS_TICK' isc->flags |= CAM_IOSCHED_FLAGS_TICK;
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"

svn commit: r329814 - head/sys/cam

[Warner Losh]

Author: imp
Date: Thu Feb 22 05:43:50 2018
New Revision: 329814
URL: https://svnweb.freebsd.org/changeset/base/329814

Log:
  Note when we tick.
  
  To help implement a policy of 'queue all trims until next I/O sched
  tick' policy to help coalesce them, note when we tick so we can do
  something special on the first call after the tick to get more work.
  
  Sponsored by: Netflix

Modified:
  head/sys/cam/cam_iosched.c

Modified: head/sys/cam/cam_iosched.c
==
--- head/sys/cam/cam_iosched.c  Thu Feb 22 05:43:45 2018(r329813)
+++ head/sys/cam/cam_iosched.c  Thu Feb 22 05:43:50 2018(r329814)
@@ -60,6 +60,19 @@ static MALLOC_DEFINE(M_CAMSCHED, "CAM I/O Scheduler",
 "CAM I/O Scheduler buffers");
 
 /*
+ * Trim or similar currently pending completion. Should only be set for
+ * those drivers wishing only one Trim active at a time.
+ */
+#define CAM_IOSCHED_FLAG_TRIM_ACTIVE   (1ul << 0)
+   /* Callout active, and needs to be torn down */
+#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
+   /* Timer has just ticked */
+#define CAM_IOSCHED_FLAG_TICK  (1ul << 2)
+
+   /* Periph drivers set these flags to indicate work */
+#define CAM_IOSCHED_FLAG_WORK_FLAGS((0xu) << 16)
+
+/*
  * Default I/O scheduler for FreeBSD. This implementation is just a thin-vineer
  * over the bioq_* interface, with notions of separate calls for normal I/O and
  * for trims.
@@ -574,6 +587,7 @@ cam_iosched_ticker(void *arg)
cam_iosched_limiter_tick(>write_stats);
cam_iosched_limiter_tick(>trim_stats);
 
+   isc->flags |= CAM_IOSCHED_FLAGS_TICK;
cam_iosched_schedule(isc, isc->periph);
 
/*
@@ -701,17 +715,6 @@ cam_iosched_cl_maybe_steer(struct control_loop *clp)
 }
 #endif
 
-/*
- * Trim or similar currently pending completion. Should only be set for
- * those drivers wishing only one Trim active at a time.
- */
-#define CAM_IOSCHED_FLAG_TRIM_ACTIVE   (1ul << 0)
-   /* Callout active, and needs to be torn down */
-#define CAM_IOSCHED_FLAG_CALLOUT_ACTIVE (1ul << 1)
-
-   /* Periph drivers set these flags to indicate work */
-#define CAM_IOSCHED_FLAG_WORK_FLAGS((0xu) << 16)
-
 #ifdef CAM_IOSCHED_DYNAMIC
 static void
 cam_iosched_io_metric_update(struct cam_iosched_softc *isc,
@@ -1322,6 +1325,10 @@ struct bio *
 cam_iosched_next_bio(struct cam_iosched_softc *isc)
 {
struct bio *bp;
+   bool wastick;
+   
+   wastick = !!(isc->flags & CAM_IOSCHED_FLAGS_TICK);
+   isc->flags &= ~CAM_IOSCHED_FLAGS_TICK;
 
/*
 * See if we have a trim that can be scheduled. We can only send one
___
svn-src-head@freebsd.org mailing list
https://lists.freebsd.org/mailman/listinfo/svn-src-head
To unsubscribe, send any mail to "svn-src-head-unsubscr...@freebsd.org"