Messages in this thread | | | Date | Sat, 27 Dec 2008 04:36:10 +0100 | From | Johannes Weiner <> | Subject | Re: [PATCH] 2.6.28, vmalloc.c, vmap_page_range |
| |
On Fri, Dec 26, 2008 at 04:39:46PM -0800, Andrew Morton wrote: > On Thu, 25 Dec 2008 22:02:35 +0100 Adam Lackorzynski <adam@os.inf.tu-dresden.de> wrote: > > > Hi, > > > > in 2.6.28, the flush_cache_vmap in vmap_page_range() is called with the end of > > the range twice. The following patch fixes this for me. > > > > Did this bug have any observeable runtime effects? If so, what were > they? > > > --- > > vmalloc.c | 4 ++-- > > 1 file changed, 2 insertions(+), 2 deletions(-) > > > > --- linux-2.6.28/mm/vmalloc.c 2008-12-25 00:26:37.000000000 +0100 > > +++ linux-2.6.28.a/mm/vmalloc.c 2008-12-25 21:45:43.118725744 +0100 > > @@ -155,7 +155,7 @@ > > pgprot_t prot, struct page **pages) > > { > > pgd_t *pgd; > > - unsigned long next; > > + unsigned long next, start = addr; > > int err = 0; > > int nr = 0; > > > > @@ -167,7 +167,7 @@ > > if (err) > > break; > > } while (pgd++, addr = next, addr != end); > > - flush_cache_vmap(addr, end); > > + flush_cache_vmap(start, end); > > > > if (unlikely(err)) > > return err; > > Well yeah. This is what happens when functions modify their incoming > arguments. It's a bad programming practice which leads directly to > exactly this sort of bug. > > How about we fix that? > > > --- a/mm/vmalloc.c~vmallocc-fix-flushing-in-vmap_page_range > +++ a/mm/vmalloc.c > @@ -151,11 +151,12 @@ static int vmap_pud_range(pgd_t *pgd, un > * > * Ie. pte at addr+N*PAGE_SIZE shall point to pfn corresponding to pages[N] > */ > -static int vmap_page_range(unsigned long addr, unsigned long end, > +static int vmap_page_range(unsigned long start_addr, unsigned long end, > pgprot_t prot, struct page **pages) > { > pgd_t *pgd; > unsigned long next; > + unsigned long addr = start_addr;
Ugh, start_addr is an awful name. How about start? I know it doesn't hold the same amount of information but it's a local API, the pgd_offset_k() should make the unit unambiguous, it goes better with the end parameter and it's unique enough for this short function.
Hannes
| |