Re: ll_rw_blk.c fails to merge requests. Help!

From: David Mansfield (lkml@dm.ultramaster.com)
Date: Fri Aug 25 2000 - 10:01:35 EST


Linus Torvalds wrote:
>
> On Thu, 24 Aug 2000, Jens Axboe wrote:
> >
> > Heh, but it's not that simple. The requests that are currently on
> > the pending list, are not necessarily added to the same freelist. So
> > we have to scan the pending list, and look at each request individually.
>
> Why not just have a pending list for each free list, no?
>
> Linus
>

As a beginning hacker (and having send two or three emails already to
clarify some things), I thought I'd give the 'pending list for each free
list' a try, which means two pending lists instead of one the way I read
it. This patch is based on the previous ones you sent. In fact I
haven't looked ever at what the add_wait_queue_exclusive even is, but I
copied it anyway :-)

This patch is against 2.4.0-test7 vanilla. In bonnie tests it seems to
have very little change (maybe increase?) in the sequential read/write
cases (which BTW are way down from 2.2 :-( almost 25% lower) but make a
HUGE increase in the 'random seeks' test, which is 3 processes running
I/O against each other. Huge is from 160 seeks/sec to 225 seeks/sec.

DISCLAIMER: I make no claim that this is fit for the real kernel. It
wasn't my idea, I just coded it :-)

David Mansfield

Index: drivers/block/ll_rw_blk.c
===================================================================
RCS file: /home/kernel/cvs_master/linux/drivers/block/ll_rw_blk.c,v
retrieving revision 1.2
diff -u -u -r1.2 ll_rw_blk.c
--- drivers/block/ll_rw_blk.c 2000/08/17 16:38:25 1.2
+++ drivers/block/ll_rw_blk.c 2000/08/25 14:39:51
@@ -172,6 +172,24 @@
         return i;
 }
 
+static inline void refill_free_lists(request_queue_t * q)
+{
+ if (q->refill_length == 0)
+ return;
+
+ if (!list_empty(&q->refill_list[READ])) {
+ list_splice(&q->refill_list[READ], &q->request_freelist[READ]);
+ INIT_LIST_HEAD(&q->refill_list[READ]);
+ }
+
+ if (!list_empty(&q->refill_list[WRITE])) {
+ list_splice(&q->refill_list[WRITE], &q->request_freelist[WRITE]);
+ INIT_LIST_HEAD(&q->refill_list[WRITE]);
+ }
+
+ q->refill_length = 0;
+}
+
 /**
  * blk_cleanup_queue: - release a &request_queue_t when it is no longer needed
  * @q: the request queue to be released
@@ -189,6 +207,9 @@
 {
         int count = QUEUE_NR_REQUESTS;
 
+ /* move the unused requests that may be waiting for a batch refill */
+ refill_free_lists(q);
+
         count -= __blk_cleanup_queue(&q->request_freelist[READ]);
         count -= __blk_cleanup_queue(&q->request_freelist[WRITE]);
 
@@ -423,6 +444,8 @@
         INIT_LIST_HEAD(&q->queue_head);
         INIT_LIST_HEAD(&q->request_freelist[READ]);
         INIT_LIST_HEAD(&q->request_freelist[WRITE]);
+ INIT_LIST_HEAD(&q->refill_list[READ]);
+ INIT_LIST_HEAD(&q->refill_list[WRITE]);
         elevator_init(&q->elevator, ELEVATOR_LINUS);
         blk_init_free_list(q);
         q->request_fn = rfn;
@@ -442,6 +465,7 @@
          */
         q->plug_device_fn = generic_plug_device;
         q->head_active = 1;
+ q->refill_length = 0;
 }
 
 
@@ -494,9 +518,9 @@
         register struct request *rq;
         DECLARE_WAITQUEUE(wait, current);
 
- add_wait_queue_exclusive(&q->wait_for_request, &wait);
+ add_wait_queue(&q->wait_for_request, &wait);
         for (;;) {
- __set_current_state(TASK_UNINTERRUPTIBLE | TASK_EXCLUSIVE);
+ __set_current_state(TASK_UNINTERRUPTIBLE);
                 spin_lock_irq(&io_request_lock);
                 rq = get_request(q, rw);
                 spin_unlock_irq(&io_request_lock);
@@ -609,15 +633,36 @@
  */
 void inline blkdev_release_request(struct request *req)
 {
+ struct request_queue * q = req->q;
+ int rw;
+
         req->rq_status = RQ_INACTIVE;
 
         /*
          * Request may not have originated from ll_rw_blk
+ */
+ if (req->free_list == NULL)
+ return;
+
+ /*
+ * no one waiting for free requests
+ */
+ if (!waitqueue_active(&q->wait_for_request)) {
+ refill_free_lists(q);
+ list_add(&req->table, req->free_list);
+ req->free_list = NULL;
+ return;
+ }
+
+ /*
+ * batch request freeing
          */
- if (req->free_list) {
- list_add(&req->table, req->free_list);
- req->free_list = NULL;
- wake_up(&req->q->wait_for_request);
+ rw = (req->free_list == &q->request_freelist[READ]) ? READ : WRITE;
+ list_add(&req->table, &q->refill_list[rw]);
+ req->free_list = NULL;
+ if (++q->refill_length > 64) {
+ refill_free_lists(q);
+ wake_up(&q->wait_for_request);
         }
 }
 
Index: include/linux/blkdev.h
===================================================================
RCS file: /home/kernel/cvs_master/linux/include/linux/blkdev.h,v
retrieving revision 1.1.1.1
diff -u -u -r1.1.1.1 blkdev.h
--- include/linux/blkdev.h 2000/08/17 16:30:13 1.1.1.1
+++ include/linux/blkdev.h 2000/08/25 14:51:52
@@ -77,6 +77,7 @@
          * the queue request freelist, one for reads and one for writes
          */
         struct list_head request_freelist[2];
+ struct list_head refill_list[2];
 
         /*
          * Together with queue_head for cacheline sharing
@@ -122,6 +123,7 @@
          * Tasks wait here for free request
          */
         wait_queue_head_t wait_for_request;
+ int refill_length;
 };
 
 struct blk_dev_struct {

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
Please read the FAQ at http://www.tux.org/lkml/



This archive was generated by hypermail 2b29 : Thu Aug 31 2000 - 21:00:16 EST