Re: 2.4.19pre3aa2

[Jens Axboe <axboe@suse.de>]

On Fri, Mar 15 2002, Jari Ruusu wrote:
> Jens Axboe wrote:
> > On Fri, Mar 15 2002, Jari Ruusu wrote:
> > > - No more illegal sleeping in generic_make_request().
> > 
> > I've told you this before -- sleeping in make_request is not illegal,
> > heck it happens _all the time_. Safely sleeping requires a reserved pool
> > of the units you wish to allocate, of course. In fact I think that would
> > be much nicer than the path you are following here by delaying
> > allocations to the loop thread (and still not using a reserved pool).
> 
> Yes, I know you have told me that before, but I'm being overcareful. See:
> 
> <quote> from device drivers book by Alessandro Rubini, chapter 12, page 331
> The request function has one very important constraint: it must be atomic.
> request is not usually called in direct response to user requests, and it is
> not running in the context of any particular process. It can be called at
> interrupt time, from tasklets, or from any number of other places. Thus, it
> must not sleep while carrying out its tasks.
> </quote>

That's talking about the request_fn funtion, not related to
make_request_fn that I rewrote loop to use. So that's not a valid point.

> > -       if(!bh) return((struct buffer_head *)0);
> > 
> > eww!
> > 
> > - Also, please adher to the style. VaRiAbLe names can hurt the eyes, and
> > stuff like
> > 
> >         if (something) break;
> > 
> >         return(val);
> > 
> > etc don't belong too. Could you fix that up?
> > 
> > That said, thanks for fixing it!
> 
> If there is any chance of being merged to mainline kernel, I will fix these
> "hurt the eyes" formatting issues.

I think there is. At least I can safely say there's no chance it will be
merged if these things aren't fixed. So take your pick :-)

> > BTW, it looks like you are killing LO_FLAGS_BH_REMAP?! Why? This is a
> > very worthwhile optimization.
> 
> Removing it simplified the code a lot. Doing remap direcly from
> loop_make_request() would probably be more effective. Just remap and return

LOTS more effective. Please don't kill this functionality. I don't buy
the simplification argument.

> 1 from loop_make_request() like LVM code does.

And like loop currently does...

-- 
Jens Axboe