All of lore.kernel.org
 help / color / mirror / Atom feed
From: Jens Axboe <axboe@suse.de>
To: Bartlomiej Zolnierkiewicz <bzolnier@gmail.com>
Cc: Richard Purdie <rpurdie@rpsys.net>,
	LKML <linux-kernel@vger.kernel.org>,
	Benjamin Herrenschmidt <benh@kernel.crashing.org>,
	Linux-ide <linux-ide@vger.kernel.org>
Subject: Re: [PATCH 10/11] LED: Add IDE disk activity LED trigger
Date: Tue, 31 Jan 2006 21:29:45 +0100	[thread overview]
Message-ID: <20060131202944.GE4215@suse.de> (raw)
In-Reply-To: <58cb370e0601310944l421174f8j1802d94f1ae93a01@mail.gmail.com>

On Tue, Jan 31 2006, Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> On 1/31/06, Richard Purdie <rpurdie@rpsys.net> wrote:
> > Hi,
> >
> > On Tue, 2006-01-31 at 15:46 +0100, Bartlomiej Zolnierkiewicz wrote:
> > >
> > > Why cannot existing block layer hook be used for this?
> >
> > The trigger is supposed to be reflecting actual hardware activity, not
> > block layer activity.
> 
> Ben, code in pmac.c (+ block layer) seems to be doing something
> different then Kconfig help entry states ("Blink laptop LED on drive
> activity")?

I doubt it really matters a lot, since either the activity will be done
right after (the LED will likely still be on), or the drive is already
busy doing stuff (in which case the LED is on anyways). So while the
trigger point might not be at the instant we start drive activity, it's
really close.

You could move the block layer trigger from add_request() to
elevator.c:elv_next_request() instead, right where it sets REQ_STARTED
to improve the start trigger point. Since that can happen at irq time
(whereas the add_request() cannot), it's likely more expensive.

The goal of the activity led for powerbook was not really to be 100%
accurate, but be able to tell whether the drive was doing io or not.
It's nice feedback to have for the user.

That said, the LED stuff should be able to handle pmac as well, so why
not add it generically instead of clamping it into the ide layer in odd
places?

-- 
Jens Axboe


  reply	other threads:[~2006-01-31 20:27 UTC|newest]

Thread overview: 16+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-01-31 13:41 [PATCH 10/11] LED: Add IDE disk activity LED trigger Richard Purdie
2006-01-31 14:46 ` Bartlomiej Zolnierkiewicz
2006-01-31 16:21   ` Richard Purdie
2006-01-31 17:44     ` Bartlomiej Zolnierkiewicz
2006-01-31 20:29       ` Jens Axboe [this message]
2006-01-31 22:08       ` Benjamin Herrenschmidt
2006-01-31 23:39         ` Richard Purdie
2006-02-04 15:29       ` Richard Purdie
2006-01-31 20:35     ` Jens Axboe
2006-01-31 21:22       ` Jordan Crouse
2006-02-01  0:12         ` Matt Reimer
2006-02-02  1:32         ` Adrian Bunk
2006-01-31 22:03       ` [PATCH 10/11] " Richard Purdie
2006-02-02 11:07       ` Pavel Machek
2006-02-01 16:09 ` Jan Engelhardt
2006-02-04  9:54   ` Jan-Benedict Glaw

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=20060131202944.GE4215@suse.de \
    --to=axboe@suse.de \
    --cc=benh@kernel.crashing.org \
    --cc=bzolnier@gmail.com \
    --cc=linux-ide@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=rpurdie@rpsys.net \
    /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.