Fix incorrect kunmap_atomic in pktcdvd
PATCH 1/1 Fix incorrect kunmap_atomic in pktcdvd
The pktcdvd driver uses kunmap_atomic() incorrectly. The function is supposed to take an address as the first parameter, but the pktcdvd driver passed a page pointer. Thanks to Douglas Gilbert and Jens Axboe for discovering this.
---
linux-petero/drivers/block/pktcdvd.c | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-)
drivers/block/pktcdvd.c renamed from drivers/block/pktcdvd.c~packet-kmap-fix +2 -2
@@ -621,7 +621,7 @@ static void pkt_copy_bio_data(struct bioBUG_ON (len <0 );memcpy (vto, vfrom, len); -kunmap_atomic (src_bvl->bv_page,KM_USER0 ); +kunmap_atomic (vfrom,KM_USER0 ); seg++; offs =0 ;
@@ -649,7 +649,7 @@ static void pkt_make_local_copy(struct pvoid *vfrom =kmap_atomic (pages[f],KM_USER0 ) + offsets[f];void *vto =page_address (pkt->pages[p]) + offs;memcpy (vto, vfrom,CD_FRAMESIZE ); -kunmap_atomic (pages[f],KM_USER0 ); +kunmap_atomic (vfrom,KM_USER0 ); pages[f] = pkt->pages[p]; offsets[f] = offs; }else {
_
Signature · 2 lines
Peter Osterlund - [email protected] http://w1.894.telia.com/~u89404340
Quoted 5 lines · Peter Osterlund
The pktcdvd driver uses
kunmap_atomic()incorrectly. The function is supposed to take an address as the first parameter, but the pktcdvd driver passed a page pointer. Thanks to Douglas Gilbert and Jens Axboe for discovering this.
You're about the 7,000th person to make that mistake. We really should catch it via typechecking but the code's really lame and nobody ever got around to rotorooting it.
Quoted 10 lines · Andrew Morton
Quoted 5 lines · Peter Osterlund
The pktcdvd driver uses
kunmap_atomic()incorrectly. The function is supposed to take an address as the first parameter, but the pktcdvd driver passed a page pointer. Thanks to Douglas Gilbert and Jens Axboe for discovering this.You're about the 7,000th person to make that mistake. We really should catch it via typechecking but the code's really lame and nobody ever got around to rotorooting it.
Why was the interface made different from kmap()/kunmap() in the first place? Wouldn't it have made more sense to let kunmap_atomic() take a page pointer as the first parameter?
Signature · 2 lines
Peter Osterlund - [email protected] http://w1.894.telia.com/~u89404340
Quoted 10 lines · Peter Osterlund
Quoted 7 lines · Andrew Morton
Quoted 5 lines · Peter Osterlund
The pktcdvd driver uses
kunmap_atomic()incorrectly. The function is supposed to take an address as the first parameter, but the pktcdvd driver passed a page pointer. Thanks to Douglas Gilbert and Jens Axboe for discovering this.
Quoted 7 lines
Quoted 3 lines
You're about the 7,000th person to make that mistake. We really should catch it via typechecking but the code's really lame and nobody ever got around to rotorooting it.
Why was the interface made different from kmap()/kunmap() in the first place? Wouldn't it have made more sense to let
kunmap_atomic()take a page pointer as the first parameter?
No, kmap-atomic() maps a single page into the CPU's address space by making a pte point at the page. To unmap that page we need to get at the pte, not at the page. If kmap_atomic() were to take a pageframe address we'd need to search the whole fixmap space for the corresponding page - a reverse lookup.
Quoted 3 lines · Peter Osterlund
Quoted 3 lines
Why was the interface made different from kmap()/kunmap() in the first place? Wouldn't it have made more sense to let
kunmap_atomic()take a page pointer as the first parameter?
Quoted 5 lines · Andrew Morton
No, kmap-atomic() maps a single page into the CPU's address space by making a pte point at the page. To unmap that page we need to get at the pte, not at the page. If
kmap_atomic()were to take a pageframe address we'd need to search the whole fixmap space for the corresponding page - a reverse lookup.
I don't recall anything truly ancient, but fixmap indices should be enough to recover the virtual address and pte. I think the virtual address is primarily for checking purposes. The same kind of check could be done by checking the pfn derived from the page structure against the contents of the pte at the fixmap index, but I suspect more damage would ensue from changing the calling convention than aligning it with common expectations.
-- wli