Re: [PATCH] HPET driver
From: Jeff Garzik
Date: Fri Jun 18 2004 - 17:58:53 EST
Andrew Morton wrote:
Jeff Garzik <jgarzik@xxxxxxxxx> wrote:
[PATCH] HPET driver
Was this posted on lkml, or simply snuck in?
Was posted on lkml, was fairly widely reviewed, had comments from hch and
others, had several fixes from myself and from Robert and a long discussion
wrt the readq() implementation.
I'm surprised it was reviewed, but I apologize for my harsh words in any
case.
wrt the readq() implementation: I reverted the generic implementation based
on concerns raised on lkml by Eric Biederman. As a generic readq/writeq
implementation seems to be a new R&D project I decided to leave the
implementation private to the HPET driver until someone takes all of this
on.
All the readq/writeq users at the moment don't give a crap about atomicity.
wrt the hpets list locking: yeah, I noticed that, mentioned it to Robert
wrt the request_irq() bug: yipes. Robert, please fix.
wrt the new miscdev minor: yes, devices.txt should be updated. When the
And:
1) a merge issue, we shouldn't be merging new procfs stuff
2) build breaks if CONFIG_ACPI is not set, but this driver is selected
3) shared interrupt causes very incorrect behavior, look at the last few
lines of hpet_interrupt().
4) return EINVAL in open(2) if FMODE_WRITE isn't set. Yes, vfs_write()
will return EINVAL if you actually attempt to write(2), but other areas
of the kernel check FMODE_WRITE. I consider this a security bug, if the
driver does not support writing, but does not prevent FMODE_WRITE from
being set. I do not know for sure, but I strongly suspect you can use
this to cause incorrect behavior _somewhere_.
5) use of "__set_current_state" _and_ "current->state ="
6) race:
spin-lock
set HPET_IE
spin-unlock
doh! we shouldn't have set HPET_IE
spin-lock
clear HPET_IE
spin-unlock
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/