[dm-devel] [RFC, PATCH] remove signal handling from dm-io.c, sync_io()

[Dave Olien]

Sorry, I've been in a meeting for the last hour...

I agree on both your counts.

I've been wondering about the multiple region stuff.  I don't
think these routines are ever used to do multiple region IO.
Is there any plan to use them that way?  I could see maybe with
multi-way mirroring, there might be some use. But I don't think
that's done yet.

the do_region() function in particular seems broken to me.

If we eliminate the multi-region support entirely, this code can
all be simplified a lot.  The allocation and initialization could
of bio's and bvecs could just use the defined code in fs/bio.c

How about I write up this file to eliminate multi-region IO?

On Thu, Jun 10, 2004 at 05:30:31PM +0000, Kevin Corry wrote:
> On Thursday 10 June 2004 5:18 pm, Kevin Corry wrote:
> > On Thursday 10 June 2004 9:53 pm, Dave Olien wrote:
> > > This patch removes the signal handling case from the sync_io() routine
> > > in dm-io.c.  This seems appropriate for several reasons.
> >
> > Couldn't we also just convert that whole loop to a wait_event() call? The
> > only real difference would be that the loop currently uses io_schedule(),
> > whereas wait_event() would use regular schedule().  But perhaps that's a
> > distinction we need to maintain.
> >
> > Here's the patch anyways for review.
> 
> And while we're in there hacking up that routine, we should probably add 
> proper error handling instead of just using BUG_ON() in the event someone 
> tries to read from multiple regions.
> 
> -- 
> Kevin Corry
> kevcorry at us.ibm.com
> http://evms.sourceforge.net/
> 
> 
> --- diff/drivers/md/dm-io.c	2004-06-10 17:28:19.113922632 +0000
> +++ source/drivers/md/dm-io.c	2004-06-10 17:28:13.772734616 +0000
> @@ -538,7 +538,9 @@
>  	DECLARE_WAIT_QUEUE_HEAD(wq);
>  	struct io io;
>  
> -	BUG_ON(num_regions > 1 && rw != WRITE);
> +	if (num_regions > 1 && rw != WRITE) {
> +		return -EIO;
> +	}
>  
>  	io.error = 0;
>  	atomic_set(&io.count, 1); /* see dispatch_io() */