On Thu, Feb 22 2001, Marcelo Tosatti wrote:
> The following piece of code in ll_rw_block() aims to limit the number of
> locked buffers by making processes throttle on IO if the number of on
> flight requests is bigger than a high watermaker. IO will only start
> again if we're under a low watermark.
> 
>                 if (atomic_read(&queued_sectors) >= high_queued_sectors) {
>                         run_task_queue(&tq_disk);
>                         wait_event(blk_buffers_wait,
>                               atomic_read(&queued_sectors) < low_queued_sectors);
>                 }
> 
> 
> However, if submit_bh() is used to queue IO (which is used by ->readpage()
> for ext2, for example), no throttling happens.
> 
> It looks like ll_rw_block() users (writes, metadata reads) can be starved
> by submit_bh() (data reads). 
> 
> If I'm not missing something, the watermark check should be moved to
> submit_bh(). 

We might as well put it there, the idea was to not lock this one
buffer either but I doubt this would make any different in reality :-)

Linus, could you apply?

--- /opt/kernel/linux-2.4.2/drivers/block/ll_rw_blk.c   Thu Feb 22 14:55:22 2001
+++ drivers/block/ll_rw_blk.c   Thu Feb 22 14:53:07 2001
@@ -957,6 +959,20 @@
        if (!test_bit(BH_Lock, &bh->b_state))
                BUG();
 
+       /*
+        * don't lock any more buffers if we are above the high
+        * water mark. instead start I/O on the queued stuff.
+        */
+       if (atomic_read(&queued_sectors) >= high_queued_sectors) {
+               run_task_queue(&tq_disk);
+               if (rw == READA) {
+                       bh->b_end_io(bh, test_bit(BH_Uptodate, &bh->b_state));
+                       return;
+               }
+               wait_event(blk_buffers_wait,
+                       atomic_read(&queued_sectors) < low_queued_sectors);
+       }
+
        set_bit(BH_Req, &bh->b_state);
 
        /*
@@ -1057,16 +1073,6 @@
 
        for (i = 0; i < nr; i++) {
                struct buffer_head *bh = bhs[i];
-
-               /*
-                * don't lock any more buffers if we are above the high
-                * water mark. instead start I/O on the queued stuff.
-                */
-               if (atomic_read(&queued_sectors) >= high_queued_sectors) {
-                       run_task_queue(&tq_disk);
-                       wait_event(blk_buffers_wait,
-                        atomic_read(&queued_sectors) < low_queued_sectors);
-               }
 
                /* Only one thread can actually submit the I/O. */
                if (test_and_set_bit(BH_Lock, &bh->b_state))

-- 
Jens Axboe

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

Reply via email to