Re: [PATCH 10/11] LED: Add IDE disk activity LED trigger

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jan 31 2006, Bartlomiej Zolnierkiewicz wrote:
> Hi,
> 
> On 1/31/06, Richard Purdie <[email protected]> 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

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [email protected]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[Index of Archives]     [Kernel Newbies]     [Netfilter]     [Bugtraq]     [Photo]     [Stuff]     [Gimp]     [Yosemite News]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Video 4 Linux]     [Linux for the blind]     [Linux Resources]
  Powered by Linux