Re: [PATCH] swiotlb: enlarge iotlb buffer on demand

[Konrad Rzeszutek Wilk]

I took your patch and was trying to fit it over the
stable/swiotlb-0.8.4 branch and when I did so a found couple of things..


> > @@ -215,14 +222,14 @@ swiotlb_late_init_with_default_size(size_t default_size)
> >  	bytes = io_tlb_nslabs << IO_TLB_SHIFT;

You should also initialize the __io_tlb_start array first:

	__io_tlb_start = __get_free_pages(GFP_KERNEL,
                get_order((io_tlb_nslabs / IO_TLB_SEGSIZE) * sizeof(char *)));
        if (!__io_tlb_start)
                goto cleanup1;

> >  
> >  	while ((SLABS_PER_PAGE << order) > IO_TLB_MIN_SLABS) {
> > -		io_tlb_start = (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN,
> > -							order);
> > -		if (io_tlb_start)
> > +		__io_tlb_start[0] = (void *)__get_free_pages(GFP_DMA | __GFP_NOWARN,
> > +							     order);

Otherwise this will be a NULL pointer exception.
> > +		if (__io_tlb_start[0])
> >  			break;
> >  		order--;
> >  	}
> >  
> > -	if (!io_tlb_start)
> > +	if (!__io_tlb_start[0])
> >  		goto cleanup1;
> >  
> >  	if (order != get_order(bytes)) {
> > @@ -231,13 +238,12 @@ swiotlb_late_init_with_default_size(size_t default_size)
> >  		io_tlb_nslabs = SLABS_PER_PAGE << order;
> >  		bytes = io_tlb_nslabs << IO_TLB_SHIFT;
> >  	}
> > -	io_tlb_end = io_tlb_start + bytes;
> > -	memset(io_tlb_start, 0, bytes);
> > +	memset(__io_tlb_start[0], 0, bytes);
> >  
> >  	/*
> >  	 * Allocate and initialize the free list array.  This array is used
> >  	 * to find contiguous free memory regions of size up to IO_TLB_SEGSIZE
> > -	 * between io_tlb_start and io_tlb_end.
> > +	 * between io_tlb_start.
> >  	 */
> >  	io_tlb_list = (unsigned int *)__get_free_pages(GFP_KERNEL,
> >  	                              get_order(io_tlb_nslabs * sizeof(int)));
> > @@ -280,9 +286,8 @@ cleanup3:
> >  	                                                 sizeof(int)));
> >  	io_tlb_list = NULL;
> >  cleanup2:
> > -	io_tlb_end = NULL;
> > -	free_pages((unsigned long)io_tlb_start, order);
> > -	io_tlb_start = NULL;
> > +	free_pages((unsigned long)__io_tlb_start[0], order);
> > +	__io_tlb_start[0] = NULL;
> >  cleanup1:
> >  	io_tlb_nslabs = req_nslabs;
> >  	return -ENOMEM;
> > @@ -300,7 +305,7 @@ void __init swiotlb_free(void)
> >  			   get_order(io_tlb_nslabs * sizeof(phys_addr_t)));
> >  		free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs *
> >  								 sizeof(int)));
> > -		free_pages((unsigned long)io_tlb_start,
> > +		free_pages((unsigned long)__io_tlb_start[0],
> >  			   get_order(io_tlb_nslabs << IO_TLB_SHIFT));

That isn't exactly right I think. You are de-allocating the first array,
which size is determined by 'order'. Probably 10. And you not freeing
the array, just the first entry.

> >  	} else {
> >  		free_bootmem_late(__pa(io_tlb_overflow_buffer),
> > @@ -309,15 +314,36 @@ void __init swiotlb_free(void)
> >  				  io_tlb_nslabs * sizeof(phys_addr_t));
> >  		free_bootmem_late(__pa(io_tlb_list),
> >  				  io_tlb_nslabs * sizeof(int));
> > -		free_bootmem_late(__pa(io_tlb_start),
> > +		free_bootmem_late(__pa(__io_tlb_start[0]),
> >  				  io_tlb_nslabs << IO_TLB_SHIFT);

I think you need this:

                free_bootmem_late(__pa(__io_tlb_start[0]),
                                  IO_TLB_SEGSIZE << IO_TLB_SHIFT);
                free_bootmem_late(__pa(__io_tlb_start),
                                  (io_tlb_nslabs / IO_TLB_SEGSIZE) * sizeof(char *));
--
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/