Re: 2.4.19pre3aa2

[Jens Axboe <axboe@suse.de>]

On Wed, Mar 20 2002, Jari Ruusu wrote:
> Jens Axboe wrote:
> > On Fri, Mar 15 2002, Jari Ruusu wrote:
> > > 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 :-)
> 
> OK, I have fixed above mentioned formatting issues. A patch is attached.

Super!

> And, about sleeping in loop_make_request(), I have also changed the code in
> such way that it defaults to code that may sleep in loop_make_request(). But
> non-sleeping code is still present (but not currently used), like this:
> 
> #if 1
>     may-sleep-in-loop_make_request-code-here
> #else
>     non-sleeping-loop_make_request-code-here
> #endif

The solution I prefer is to have loop_make_request() block when we run
out of pre-allocated buffers, ie similar to "normal" block drivers when
they run out of request slots. This provides a nice throttling mechanism
and makes sure that loop doesn't eat into the memory polls too heavily.

In any way, do it one way and remove the other.

> > > > 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.
> 
> This new patch does not remove LO_FLAGS_BH_REMAP optimization.

Good.

> > > 1 from loop_make_request() like LVM code does.
> > 
> > And like loop currently does...
> 
> 2.4.19-pre3 loop code wanders to loop_get_buffer() and transfer function in
> LO_FLAGS_BH_REMAP optimization case.
> 
> My version is simpler than yours:
> 
>         if (lo->lo_flags & LO_FLAGS_BH_REMAP) {
>                 rbh->b_rsector += (lo->lo_offset >> 9);
>                 rbh->b_rdev = lo->lo_device;
>                 return 1;
>         }               

So the 'new' version does exactly the same, but doesn't do it in
loop_get_buffer().

-- 
Jens Axboe