Re: [PATCH] backing dev unplugging

[Jens Axboe <axboe@suse.de>]

On Thu, Mar 11 2004, Nathan Scott wrote:
> Hi Jens,
> 
> On Wed, Mar 10, 2004 at 01:45:07PM +0100, Jens Axboe wrote:
> > ...[snip]...
> > diff -ur -X /home/axboe/cdrom/exclude /opt/kernel/linux-2.6.4-rc2-mm1/fs/xfs/linux/xfs_buf.c linux-2.6.4-rc2-mm1-plug/fs/xfs/linux/xfs_buf.c
> > --- /opt/kernel/linux-2.6.4-rc2-mm1/fs/xfs/linux/xfs_buf.c	2004-03-09 13:08:30.000000000 +0100
> > +++ linux-2.6.4-rc2-mm1-plug/fs/xfs/linux/xfs_buf.c	2004-03-10 13:13:49.000000000 +0100
> > @@ -1013,7 +1013,7 @@
> >  {
> >  	PB_TRACE(pb, "lock", 0);
> >  	if (atomic_read(&pb->pb_io_remaining))
> > -		blk_run_queues();
> > +		blk_run_address_space(pb->pb_target->pbr_mapping);
> >  	down(&pb->pb_sema);
> >  	PB_SET_OWNER(pb);
> >  	PB_TRACE(pb, "locked", 0);
> > @@ -1109,7 +1109,7 @@
> >  		if (atomic_read(&pb->pb_pin_count) == 0)
> >  			break;
> >  		if (atomic_read(&pb->pb_io_remaining))
> > -			blk_run_queues();
> > +			blk_run_address_space(pb->pb_target->pbr_mapping);
> >  		schedule();
> >  	}
> >  	remove_wait_queue(&pb->pb_waiters, &wait);
> > @@ -1407,7 +1407,7 @@
> >  	if (pb->pb_flags & PBF_RUN_QUEUES) {
> >  		pb->pb_flags &= ~PBF_RUN_QUEUES;
> >  		if (atomic_read(&pb->pb_io_remaining) > 1)
> > -			blk_run_queues();
> > +			blk_run_address_space(pb->pb_target->pbr_mapping);
> >  	}
> >  }
> >  
> > @@ -1471,7 +1471,7 @@
> >  {
> >  	PB_TRACE(pb, "iowait", 0);
> >  	if (atomic_read(&pb->pb_io_remaining))
> > -		blk_run_queues();
> > +		blk_run_address_space(pb->pb_target->pbr_mapping);
> >  	down(&pb->pb_iodonesema);
> >  	PB_TRACE(pb, "iowaited", (long)pb->pb_error);
> >  	return pb->pb_error;
> > @@ -1617,7 +1617,6 @@
> >  pagebuf_daemon(
> >  	void			*data)
> >  {
> > -	int			count;
> >  	page_buf_t		*pb;
> >  	struct list_head	*curr, *next, tmp;
> >  
> > @@ -1640,7 +1639,6 @@
> >  
> >  		spin_lock(&pbd_delwrite_lock);
> >  
> > -		count = 0;
> >  		list_for_each_safe(curr, next, &pbd_delwrite_queue) {
> >  			pb = list_entry(curr, page_buf_t, pb_list);
> >  
> > @@ -1657,7 +1655,7 @@
> >  				pb->pb_flags &= ~PBF_DELWRI;
> >  				pb->pb_flags |= PBF_WRITE;
> >  				list_move(&pb->pb_list, &tmp);
> > -				count++;
> > +				blk_run_address_space(pb->pb_target->pbr_mapping);
> >  			}
> 
> This moves the blk_run_address_space to before we submit the
> I/O (this bit of code is moving buffers off the delwri queue
> onto a temporary queue, buffers on the temporary queue are
> then submitted a little further down) - I suspect we need to
> move this new blk_run_address_space call down into the temp
> list processing, just after pagebuf_iostrategy.

I'm not surprised, the XFS 'conversion' was done quickly and is expected
to be half-assed :)

Thanks a lot for taking a look at this.

> >  		}
> >  
> > @@ -1671,8 +1669,6 @@
> >  
> >  		if (as_list_len > 0)
> >  			purge_addresses();
> > -		if (count)
> > -			blk_run_queues();
> >  
> >  		force_flush = 0;
> >  	} while (pagebuf_daemon_active);
> > @@ -1734,13 +1730,11 @@
> >  		pagebuf_lock(pb);
> >  		pagebuf_iostrategy(pb);
> >  		if (++flush_cnt > 32) {
> > -			blk_run_queues();
> > +			blk_run_address_space(pb->pb_target->pbr_mapping);
> >  			flush_cnt = 0;
> >  		}
> >  	}
> >  
> > -	blk_run_queues();
> > -
> >  	while (!list_empty(&tmp)) {
> >  		pb = list_entry(tmp.next, page_buf_t, pb_list);
> >  
> 
> For this second one, we probably just want to ditch the flush_cnt
> there (this change is doing blk_run_address_space on every 32nd
> buffer target, and not the intervening ones).  We will be doing a
> bunch more blk_run_address_space calls than we probably need to,
> not sure if thats going to become an issue or not, let me prod
> some of the other XFS folks for more insight there...

If any of you could send me a replacement xfs_buf bit, I'd much
appreciate it.

-- 
Jens Axboe