Re: ll_rw_block/submit_bh and request limits

Jens Axboe
2001-02-22T13:57:22Z

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