Messages in this thread | | | Date | Fri, 18 Jun 2004 18:45:36 -0400 | From | Jeff Garzik <> | Subject | Re: [PATCH] HPET driver |
| |
Andrew Morton wrote: > Jeff Garzik <jgarzik@pobox.com> 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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |