lkml.org 
[lkml]   [2000]   [Aug]   [25]   [last100]   RSS Feed
Views: [more markup]  [less markup]  [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
DateFri, 25 Aug 2000 11:01:35 -0400
FromDavid Mansfield <>
SubjectRe: ll_rw_blk.c fails to merge requests. Help!
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 MansfieldIndex: 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 {
\
 
 \ /
  Last update: 2005-03-22 12:38    [from the cache]
©2003-2008