All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: Martin Dalecki <dalecki@evision-ventures.com>
Cc: Zwane Mwaikambo <zwane@linux.realnet.co.sz>,
	Linux Kernel <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH][2.5] BUG check in elevator.c:237
Date: Mon, 11 Mar 2002 10:44:45 +0100	[thread overview]
Message-ID: <20020311094445.GC31108@suse.de> (raw)
In-Reply-To: <Pine.LNX.4.44.0203081258500.5383-100000@netfinity.realnet.co.sz> <3C88A796.2070301@evision-ventures.com>
In-Reply-To: <3C88A796.2070301@evision-ventures.com>

On Fri, Mar 08 2002, Martin Dalecki wrote:
> After having a look at the oops I think that this may be very well a
> symptom of another problem in the ide-cd.c drivers overall
> way of working. Please let me elaborate a bit.
> 
> In ide.c there is one central interrupt handler, namely:
> 
> void ide_timer_expiry(unsigned long data)
> 
> This function is called upon finish of every command.

Eh? ide_timer_expiry is the timer handler called if a interrupt timeout
occurs.

> However for cd-rom there are commands, which can
> take quite a long time. Therefore there is the possiblity there
> to provide a polling function, which will be engaged after the
> interrupt happens in the above function:
> 
> 	/* continue */
> 				if ((wait = expiry(drive)) != 0) {

[snip]

That's nonsense too. I added the expiry hook to let lower levels decide
what should happen when an interrupt timeout occurs. So there's been
_no_ interrupt if we enter this from the timer handler.

> And plase guess whot? CD-ROM is the only driver which is using
> this facility. Please have a look at the last

Right, it was added to handle long commands like format unit etc.

> argument of ide_set_handler(). The second argument is the
> interrutp handler for a command. The third is supposed to be
> the poll timerout function. But if you look at the
> actual poll function found in ide-cd.c (and only there).
> You may as well feel to try to just execute its commands directly in
> ide_timer_expiry, thus reducing tons of possible races ind the
> overall intr handling found currently there.

I don't know what tangent you are going off on here, I think you should
re-read this code a lot more carefully. There's no polling going on
here.

-- 
Jens Axboe


  parent reply	other threads:[~2002-03-11  9:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2002-03-08 11:00 [PATCH][2.5] BUG check in elevator.c:237 Zwane Mwaikambo
2002-03-08 11:59 ` Martin Dalecki
2002-03-08 11:57   ` Zwane Mwaikambo
2002-03-08 12:19     ` Zwane Mwaikambo
2002-03-08 12:36     ` Martin Dalecki
2002-03-08 12:29       ` Zwane Mwaikambo
2002-03-08 12:49         ` Martin Dalecki
2002-03-08 12:50           ` Zwane Mwaikambo
2002-03-08 13:57             ` Martin Dalecki
2002-03-11  9:44   ` Jens Axboe [this message]
2002-03-11 10:22     ` Martin Dalecki
2002-03-11 11:13       ` Andre Hedrick

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20020311094445.GC31108@suse.de \
    --to=axboe@suse.de \
    --cc=dalecki@evision-ventures.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=zwane@linux.realnet.co.sz \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.