Currently in brw_kiovec, iobuf->io_count is being incremented as each
bh is submitted, and decremented in the bh->b_end_io(). This means
io_count can go to zero before all the bhs have been submitted,
especially during a large request. This causes the end_kio_request()
to be called before all of the io is complete.
This suggested patch against 2.4.2 sets io_count to the total amount
before the bhs are submitted, although there is probably a better way
to determine the io_count than this.
robert
diff -ru linux/fs/buffer.c linux-rm/fs/buffer.c
--- linux/fs/buffer.c Mon Jan 15 12:42:32 2001
+++ linux-rm/fs/buffer.c Tue Jan 30 11:41:57 2001
@@ -2085,6 +2085,7 @@
offset = iobuf->offset;
length = iobuf->length;
iobuf->errno = 0;
+ atomic_set(&iobuf->io_count, length/size);
for (pageind = 0; pageind < iobuf->nr_pages; pageind++) {
map = iobuf->maplist[pageind];
@@ -2119,8 +2120,6 @@
bh[bhind++] = tmp;
length -= size;
offset += size;
-
- atomic_inc(&iobuf->io_count);
submit_bh(rw, tmp);
/*
On Tue, 27 Feb 2001, Robert Read wrote:
> Currently in brw_kiovec, iobuf->io_count is being incremented as each
> bh is submitted, and decremented in the bh->b_end_io(). This means
> io_count can go to zero before all the bhs have been submitted,
> especially during a large request. This causes the end_kio_request()
> to be called before all of the io is complete.
>
> This suggested patch against 2.4.2 sets io_count to the total amount
> before the bhs are submitted, although there is probably a better way
> to determine the io_count than this.
>
> robert
>
> diff -ru linux/fs/buffer.c linux-rm/fs/buffer.c
> --- linux/fs/buffer.c Mon Jan 15 12:42:32 2001
> +++ linux-rm/fs/buffer.c Tue Jan 30 11:41:57 2001
> @@ -2085,6 +2085,7 @@
> offset = iobuf->offset;
> length = iobuf->length;
> iobuf->errno = 0;
> + atomic_set(&iobuf->io_count, length/size);
>
> for (pageind = 0; pageind < iobuf->nr_pages; pageind++) {
> map = iobuf->maplist[pageind];
> @@ -2119,8 +2120,6 @@
> bh[bhind++] = tmp;
> length -= size;
> offset += size;
> -
> - atomic_inc(&iobuf->io_count);
>
> submit_bh(rw, tmp);
> /*
> -
It seems your patch breaks bh allocation failure handling. If
get_unused_buffer_head() fails, iobuf->io_count never reaches 0, so
processes waiting on kiobuf_wait_for_io() will block forever.
On Tue, Feb 27, 2001 at 10:50:54PM -0300, Marcelo Tosatti wrote:
>
>
> It seems your patch breaks bh allocation failure handling. If
> get_unused_buffer_head() fails, iobuf->io_count never reaches 0, so
> processes waiting on kiobuf_wait_for_io() will block forever.
>
This is true, but it looks like the brw_kiovec allocation failure
handling is broken already; it's calling __put_unused_buffer_head on
bhs without waiting for them to complete first. Also, the err won't
be returned if the previous batch of bhs finished ok. It looks like
brw_kiovec needs some work, but I'm going to need some coffee first...
robert
On Tue, Feb 27, 2001 at 04:22:22PM -0800, Robert Read wrote:
> Currently in brw_kiovec, iobuf->io_count is being incremented as each
> bh is submitted, and decremented in the bh->b_end_io(). This means
> io_count can go to zero before all the bhs have been submitted,
> especially during a large request. This causes the end_kio_request()
> to be called before all of the io is complete.
brw_kiovec is currently entirely synchronous, so end_kio_request()
calling is probably not a big deal right now. It would be much more
important for an async version.
--Stephen
Hi,
On Wed, Feb 28, 2001 at 09:18:59AM -0800, Robert Read wrote:
> On Tue, Feb 27, 2001 at 10:50:54PM -0300, Marcelo Tosatti wrote:
> This is true, but it looks like the brw_kiovec allocation failure
> handling is broken already; it's calling __put_unused_buffer_head on
> bhs without waiting for them to complete first. Also, the err won't
> be returned if the previous batch of bhs finished ok. It looks like
> brw_kiovec needs some work, but I'm going to need some coffee first...
Right, looks like this happened when somebody was changing the bh
submission mechanism to use submit_bh(). I'll fix it.
--Stephen