Re: [PATCH] 2.5.30 IDE 115

[Jens Axboe <axboe@suse.de>]

On Fri, Aug 09 2002, Marcin Dalecki wrote:
> Jens Axboe wrote:
> >On Fri, Aug 09 2002, Marcin Dalecki wrote:
> >
> >>- Fix small typo introduced in 113, which prevented CD-ROMs from
> >> working altogether.
> >
> >
> >Have you fixed the sense reporting issue I told you about months ago?
> 
> Well at least ide-116 will start to unify the corresponding code.
> But please don't expecty anything "revolutionary" yet... Just for
> example using GPCMD_ constants throughout the code and a unified error
> dissection function. One of the issues involved is rq->buffer in
> ide-floppy versus rq->special in ide-cd.c

Well I consider the sense decoding a somewhat important feature, hence
it's really bad it has been broken for months now. It's impossible to
diagnose problems in ide-cd code without it.

> >>- Eliminate block_ioctl(). This code can't be shared in the way
> >> proposed by this file. We will port it to the proper
> >> blk_insert_request() soon. This will eliminate the _elv_add_request()
> >> "layering violation".
> >
> >
> >What are you talking about?
> 
> Hmm, so apparently you where not the one who "inventid" it?

? I added block_ioct.c, yes.

> Anyway I talk about the block_ioctl.c file, which was supposed
> to contain the two eject ioctl functions for "generic" packet code.

It _did_ contain two eject ioctl as a "here's what it's supposed to do"
proof of concept type thing.

> But since we don't have any kind of "generic" packet commands this
> didn't make much sense.

What are you talking about?!

> It was inventing a function called blk_do_rq(), which was using
> elv_add_request(). You called this not a long time ago a "layering
> violation" yourself. And I simply intend to replace it in one of the
> forthcomming patches  with the recently inventid blk_insert_request() 
> function.

Sigh... Martin, for fscks sake please stop always just assuming and get
your facts straight. This is why you are repeatedly pissing me (and
others) off. blk_do_rq() means "insert request and execute it, return
when it's done". It probably should have been in ll_rw_blk.c itself,
sinoce it's that sort of helper.

Using elv_add_request() is not a layering violation, that's the exported
interface... The layering violation is using __elv_add_request() since
it exposes the internal queue lists, which may not be appropriate for
all io schedulers.

> Oh, I realize I didn't express myself properly. I certinaly don't intend
> to eliminate elv_add_request() itself any time soon ;-).

No, I would appreciate it if you would keep your hands out of the block
code.

-- 
Jens Axboe