Re: svn commit: r335066 - in head/sys: kern sys

2018-06-13 Thread Ian Lepore
On Wed, 2018-06-13 at 13:22 -0600, Warner Losh wrote:
> So we implement the elevator algorithm, but limit the number of requests
> that we can put in each 'car'. Elevators themselves are sometimes call
> 'cars' for reasons I have no clue about.

An "elevator" is an entire system for moving something vertically. It
includes a lift mechanism, guide system (rails, etc), control
mechanisms, etc. When the portion of the system that moves is an
enclosed space (not a simple platform or bucket) which carries people
and/or goods, that part is called a "carriage" or "car".

-- Ian (who was oddly fascinated by elevators as a child)
___
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: r335066 - in head/sys: kern sys

2018-06-13 Thread Warner Losh
On Wed, Jun 13, 2018 at 1:12 PM, Ravi Pokala  wrote:

> Hi Warner,
>
> I have many questions...
>
> "Car limit"? I'm not sure what you mean by that?
>

So we implement the elevator algorithm, but limit the number of requests
that we can put in each 'car'. Elevators themselves are sometimes call
'cars' for reasons I have no clue about.

So we only put N requests into the sorting algorithm. We put the next
request at the end, unsorted, and that has the side effect of starting over
so the net request after that is sorted against the last one, but not the
first N.

This is to help bound software queuing latency.


> This change starts tracking the total number of BIOs on the queue, but
> doesn't actually do anything with that value.
>

Oh, you're right. We have a sysctl that counts them, I think, in Netflix's
code base (or did, I can't find it in the current sources)... I should
remove that from here. I'm surprised I didn't notice that before.


> When there are more requests batched than the limit, your comment say you
> "start over", and the code says you bioq_insert_tail(). Why does the latter
> imply "start(ing) over"?
>

Because that also sets a new sorting point for inserting sorted.

Warner



> Thanks,
>
> Ravi (rpokala@)
>
> -Original Message-
> From:  on behalf of Warner Losh
> 
> Date: 2018-06-13, Wednesday at 12:48
> To: , , <
> svn-src-h...@freebsd.org>
> Subject: svn commit: r335066 - in head/sys: kern sys
>
> Author: imp
> Date: Wed Jun 13 16:48:07 2018
> New Revision: 335066
> URL: https://svnweb.freebsd.org/changeset/base/335066
>
> Log:
>   Implement a 'car limit' for bioq.
>
>   Allow one to implement a 'car limit' for
>   bioq_disksort. debug.bioq_batchsize sets the size of car limit. Every
>   time we queue that many requests, we start over so that we limit the
>   latency for requests when the software queue depths are large. A value
>   of '0', the default, means to revert to the old behavior.
>
>   Sponsored by: Netflix
>
> Modified:
>   head/sys/kern/subr_disk.c
>   head/sys/sys/bio.h
>
> Modified: head/sys/kern/subr_disk.c
> 
> ==
> --- head/sys/kern/subr_disk.c   Wed Jun 13 15:58:33 2018(r335065)
> +++ head/sys/kern/subr_disk.c   Wed Jun 13 16:48:07 2018(r335066)
> @@ -23,8 +23,13 @@ __FBSDID("$FreeBSD$");
>  #include 
>  #include 
>  #include 
> +#include 
>  #include 
>
> +static int bioq_batchsize = 0;
> +SYSCTL_INT(_debug, OID_AUTO, bioq_batchsize, CTLFLAG_RW,
> +_batchsize, 0, "BIOQ batch size");
> +
>  /*-
>   * Disk error is the preface to plaintive error messages
>   * about failing disk transfers.  It prints messages of the form
> @@ -152,6 +157,8 @@ bioq_init(struct bio_queue_head *head)
> TAILQ_INIT(>queue);
> head->last_offset = 0;
> head->insert_point = NULL;
> +   head->total = 0;
> +   head->batched = 0;
>  }
>
>  void
> @@ -165,6 +172,7 @@ bioq_remove(struct bio_queue_head *head, struct bio *b
> head->insert_point = NULL;
>
> TAILQ_REMOVE(>queue, bp, bio_queue);
> +   head->total--;
>  }
>
>  void
> @@ -183,6 +191,8 @@ bioq_insert_head(struct bio_queue_head *head, struct b
> if (head->insert_point == NULL)
> head->last_offset = bp->bio_offset;
> TAILQ_INSERT_HEAD(>queue, bp, bio_queue);
> +   head->total++;
> +   head->batched = 0;
>  }
>
>  void
> @@ -190,6 +200,7 @@ bioq_insert_tail(struct bio_queue_head *head, struct b
>  {
>
> TAILQ_INSERT_TAIL(>queue, bp, bio_queue);
> +   head->total++;
> head->insert_point = bp;
> head->last_offset = bp->bio_offset;
>  }
> @@ -248,6 +259,11 @@ bioq_disksort(struct bio_queue_head *head, struct bio
> return;
> }
>
> +   if (bioq_batchsize > 0 && head->batched > bioq_batchsize) {
> +   bioq_insert_tail(head, bp);
> +   return;
> +   }
> +
> prev = NULL;
> key = bioq_bio_key(head, bp);
> cur = TAILQ_FIRST(>queue);
> @@ -266,4 +282,6 @@ bioq_disksort(struct bio_queue_head *head, struct bio
> TAILQ_INSERT_HEAD(>queue, bp, bio_queue);
> else
> TAILQ_INSERT_AFTER(>queue, prev, bp, bio_queue);
> +   head->total++;
> +   head->batched++;
>  }
>
> Modified: head/sys/sys/bio.h
> 
> ==
> --- head/sys/sys/bio.h

Re: svn commit: r335066 - in head/sys: kern sys

2018-06-13 Thread Ravi Pokala
Hi Warner,

I have many questions...

"Car limit"? I'm not sure what you mean by that?

This change starts tracking the total number of BIOs on the queue, but doesn't 
actually do anything with that value.

When there are more requests batched than the limit, your comment say you 
"start over", and the code says you bioq_insert_tail(). Why does the latter 
imply "start(ing) over"?

Thanks,

Ravi (rpokala@)

-Original Message-
From:  on behalf of Warner Losh 

Date: 2018-06-13, Wednesday at 12:48
To: , , 

Subject: svn commit: r335066 - in head/sys: kern sys

Author: imp
Date: Wed Jun 13 16:48:07 2018
New Revision: 335066
URL: https://svnweb.freebsd.org/changeset/base/335066

Log:
  Implement a 'car limit' for bioq.
  
  Allow one to implement a 'car limit' for
  bioq_disksort. debug.bioq_batchsize sets the size of car limit. Every
  time we queue that many requests, we start over so that we limit the
  latency for requests when the software queue depths are large. A value
  of '0', the default, means to revert to the old behavior.
  
  Sponsored by: Netflix

Modified:
  head/sys/kern/subr_disk.c
  head/sys/sys/bio.h

Modified: head/sys/kern/subr_disk.c
==
--- head/sys/kern/subr_disk.c   Wed Jun 13 15:58:33 2018(r335065)
+++ head/sys/kern/subr_disk.c   Wed Jun 13 16:48:07 2018(r335066)
@@ -23,8 +23,13 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 
 #include 
 
+static int bioq_batchsize = 0;
+SYSCTL_INT(_debug, OID_AUTO, bioq_batchsize, CTLFLAG_RW,
+_batchsize, 0, "BIOQ batch size");
+
 /*-
  * Disk error is the preface to plaintive error messages
  * about failing disk transfers.  It prints messages of the form
@@ -152,6 +157,8 @@ bioq_init(struct bio_queue_head *head)
TAILQ_INIT(>queue);
head->last_offset = 0;
head->insert_point = NULL;
+   head->total = 0;
+   head->batched = 0;
 }
 
 void
@@ -165,6 +172,7 @@ bioq_remove(struct bio_queue_head *head, struct bio *b
head->insert_point = NULL;
 
TAILQ_REMOVE(>queue, bp, bio_queue);
+   head->total--;
 }
 
 void
@@ -183,6 +191,8 @@ bioq_insert_head(struct bio_queue_head *head, struct b
if (head->insert_point == NULL)
head->last_offset = bp->bio_offset;
TAILQ_INSERT_HEAD(>queue, bp, bio_queue);
+   head->total++;
+   head->batched = 0;
 }
 
 void
@@ -190,6 +200,7 @@ bioq_insert_tail(struct bio_queue_head *head, struct b
 {
 
TAILQ_INSERT_TAIL(>queue, bp, bio_queue);
+   head->total++;
head->insert_point = bp;
head->last_offset = bp->bio_offset;
 }
@@ -248,6 +259,11 @@ bioq_disksort(struct bio_queue_head *head, struct bio 
return;
}
 
+   if (bioq_batchsize > 0 && head->batched > bioq_batchsize) {
+   bioq_insert_tail(head, bp);
+   return;
+   }
+
prev = NULL;
key = bioq_bio_key(head, bp);
cur = TAILQ_FIRST(>queue);
@@ -266,4 +282,6 @@ bioq_disksort(struct bio_queue_head *head, struct bio 
TAILQ_INSERT_HEAD(>queue, bp, bio_queue);
else
TAILQ_INSERT_AFTER(>queue, prev, bp, bio_queue);
+   head->total++;
+   head->batched++;
 }

Modified: head/sys/sys/bio.h
==
--- head/sys/sys/bio.h  Wed Jun 13 15:58:33 2018(r335065)
+++ head/sys/sys/bio.h  Wed Jun 13 16:48:07 2018(r335066)
@@ -138,6 +138,8 @@ struct bio_queue_head {
TAILQ_HEAD(bio_queue, bio) queue;
off_t last_offset;
struct  bio *insert_point;
+   int total;
+   int batched;
 };
 
 extern struct vm_map *bio_transient_map;



___
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"


svn commit: r335066 - in head/sys: kern sys

2018-06-13 Thread Warner Losh
Author: imp
Date: Wed Jun 13 16:48:07 2018
New Revision: 335066
URL: https://svnweb.freebsd.org/changeset/base/335066

Log:
  Implement a 'car limit' for bioq.
  
  Allow one to implement a 'car limit' for
  bioq_disksort. debug.bioq_batchsize sets the size of car limit. Every
  time we queue that many requests, we start over so that we limit the
  latency for requests when the software queue depths are large. A value
  of '0', the default, means to revert to the old behavior.
  
  Sponsored by: Netflix

Modified:
  head/sys/kern/subr_disk.c
  head/sys/sys/bio.h

Modified: head/sys/kern/subr_disk.c
==
--- head/sys/kern/subr_disk.c   Wed Jun 13 15:58:33 2018(r335065)
+++ head/sys/kern/subr_disk.c   Wed Jun 13 16:48:07 2018(r335066)
@@ -23,8 +23,13 @@ __FBSDID("$FreeBSD$");
 #include 
 #include 
 #include 
+#include 
 #include 
 
+static int bioq_batchsize = 0;
+SYSCTL_INT(_debug, OID_AUTO, bioq_batchsize, CTLFLAG_RW,
+_batchsize, 0, "BIOQ batch size");
+
 /*-
  * Disk error is the preface to plaintive error messages
  * about failing disk transfers.  It prints messages of the form
@@ -152,6 +157,8 @@ bioq_init(struct bio_queue_head *head)
TAILQ_INIT(>queue);
head->last_offset = 0;
head->insert_point = NULL;
+   head->total = 0;
+   head->batched = 0;
 }
 
 void
@@ -165,6 +172,7 @@ bioq_remove(struct bio_queue_head *head, struct bio *b
head->insert_point = NULL;
 
TAILQ_REMOVE(>queue, bp, bio_queue);
+   head->total--;
 }
 
 void
@@ -183,6 +191,8 @@ bioq_insert_head(struct bio_queue_head *head, struct b
if (head->insert_point == NULL)
head->last_offset = bp->bio_offset;
TAILQ_INSERT_HEAD(>queue, bp, bio_queue);
+   head->total++;
+   head->batched = 0;
 }
 
 void
@@ -190,6 +200,7 @@ bioq_insert_tail(struct bio_queue_head *head, struct b
 {
 
TAILQ_INSERT_TAIL(>queue, bp, bio_queue);
+   head->total++;
head->insert_point = bp;
head->last_offset = bp->bio_offset;
 }
@@ -248,6 +259,11 @@ bioq_disksort(struct bio_queue_head *head, struct bio 
return;
}
 
+   if (bioq_batchsize > 0 && head->batched > bioq_batchsize) {
+   bioq_insert_tail(head, bp);
+   return;
+   }
+
prev = NULL;
key = bioq_bio_key(head, bp);
cur = TAILQ_FIRST(>queue);
@@ -266,4 +282,6 @@ bioq_disksort(struct bio_queue_head *head, struct bio 
TAILQ_INSERT_HEAD(>queue, bp, bio_queue);
else
TAILQ_INSERT_AFTER(>queue, prev, bp, bio_queue);
+   head->total++;
+   head->batched++;
 }

Modified: head/sys/sys/bio.h
==
--- head/sys/sys/bio.h  Wed Jun 13 15:58:33 2018(r335065)
+++ head/sys/sys/bio.h  Wed Jun 13 16:48:07 2018(r335066)
@@ -138,6 +138,8 @@ struct bio_queue_head {
TAILQ_HEAD(bio_queue, bio) queue;
off_t last_offset;
struct  bio *insert_point;
+   int total;
+   int batched;
 };
 
 extern struct vm_map *bio_transient_map;
___
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"