Re: Questions answered by Neil Brown

"A month of sundays ago Neil Brown wrote:"

On Monday February 24, Paul.Clements@SteelEye.com wrote:

quoted

 
So I think this means that we really need to duplicate the buffer memory
(bh->b_page, bh->b_data) and point the mirror I/Os to the duplicated
buffer, so we can allow the original (master_bh) to be freed before all
the I/Os complete. We then free the duplicate when the last mirror I/O
completes, right?

Yes, that is right.

It's early in the morning (for me!), but I believe that page data at
least can be encouraged to stay around by hiking a reference counter
or equivalent.

It really depends how that b_end_io() got hold of its buffer and what
it does with it on completion. The end_buffer_io() fn in buffer.c
will lock the page and walk through the bh chain hanging off
bh->b_this_page (->b_this_page ...) to see if those other bh's have
completed before marking the page up to date and unlocking it.

So it might be enough to chain all the mirror bh's through
bh->b_this_page.

I believe that currently this field is just set to "1" in
raid1_make_request().

Peter

"Peter T. Breuer" wrote:

"A month of sundays ago Neil Brown wrote:"

quoted

On Monday February 24, Paul.Clements@SteelEye.com wrote:

So it might be enough to chain all the mirror bh's through
bh->b_this_page.

That's an interesting idea. I looked through the code and I have some
questions:

What if the user is waiting on a page and not a buffer (not sure if that
can/will happen). In that case, we'd be artificially causing him to wait
when it wasn't necessary. Suppose all the I/O for a page really was
complete, but we kept the user waiting until all the mirror I/Os
(including ones to backup devices) for that page had completed. 

Another thing I'm not sure about is whether it's safe for raid1 to
modify the b_this_page field (for a buffer that was passed in from
above)...we'd at least have to insert our values into the existing list.
Is it safe to modify the list without any locks held?

 

I believe that currently this field is just set to "1" in
raid1_make_request().

Yeah, I sure wish I knew who did that and why. I wonder if someone had a
clever plan to use that field at some point, but never got around to it.
Setting that field to something besides a real address sure does seem
odd...and I can't see that it's ever used anywhere.

--
Paul