[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20120825182546.GC18320@konrad-lan.dumpdata.com>
Date: Sat, 25 Aug 2012 14:25:47 -0400
From: Konrad Rzeszutek Wilk <konrad.wilk@...cle.com>
To: Shuah Khan <shuah.khan@...com>
Cc: fujita.tomonori@....ntt.co.jp, akpm@...ux-foundation.org,
paul.gortmaker@...driver.com, bhelgaas@...gle.com,
amwang@...hat.com, LKML <linux-kernel@...r.kernel.org>,
shuahkhan@...il.com, linux-doc@...r.kernel.org, rob@...dley.net
Subject: Re: [PATCH] swiotlb: Add support to disable swiotlb overflow buffer
with deprecation warning
On Fri, Aug 24, 2012 at 03:14:05PM -0600, Shuah Khan wrote:
> Add support for disabling swiotlb overflow buffer using zero size swiotlb
> overflow buffer to help test disable overflow scenarios to find drivers that
> don't check dma mapping errors. Add kernel error message to track overflow
> buffer triggers to understand how often overflow buffer gets used. Add
> deprecation notice warning message and deprecation schedule documentation, as
> a first step towards removing overflow support, to be consistent with other
> iommu implementations and return DMA_ERROR_CODE. Once drivers are fixed
> overflow support can be removed.
This looks like the previous patch. I thought we discussed this and
decided that the overflow buffer support should not be disabled outright
(meaning by default it is ON) - but we can provide a knob to turn it
off.
And alongside you were going to look at DMA API debug code to see if you
can come up with a mechanism to track that the users of DMA API call
dma_mapping_error after they have called various variants of 'map'?
>
> Tested on x86-64 and compile tested on IA64.
>
> Signed-off-by: Shuah Khan <shuah.khan@...com>
> ---
> Documentation/feature-removal-schedule.txt | 15 ++++++
> lib/swiotlb.c | 73 ++++++++++++++++++++--------
> 2 files changed, 69 insertions(+), 19 deletions(-)
>
> diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
> index a52924e..b5938ca 100644
> --- a/Documentation/feature-removal-schedule.txt
> +++ b/Documentation/feature-removal-schedule.txt
> @@ -646,3 +646,18 @@ Who: Russell King <linux@....linux.org.uk>,
> Santosh Shilimkar <santosh.shilimkar@...com>
>
> ----------------------------
> +
> +What: SWIOTLB overflow buffer support.
> +When: 3.8
> +Why: Remove SWIOTLB overflow buffer support and return DMA_ERROR_CODE
> + (a value of zero) to make it consistent with iommu implementation
> + on Intel, AMD, and swiotlb-xen. In 3.7, add logging to track swiotlb
> + buffer overflow triggers to understand how often overflow buffer
> + gets used. The next step is to fix drivers that don't call
> + dma_mapping_error to check for errors returned by the mapping
> + interfaces. Once drivers are fixed overflow support can be removed.
> + If you see any problems related to disabling SWIOTLB overflow buffer,
> + please report to us!
> + E-mail us at: linux-kernel@...r.kernel.org
> +Who: Shuah Khan <shuah.khan@...com> <shuahkhan@...il.com>
> +
> diff --git a/lib/swiotlb.c b/lib/swiotlb.c
> index 45bc1f8..658f35a 100644
> --- a/lib/swiotlb.c
> +++ b/lib/swiotlb.c
> @@ -92,6 +92,24 @@ static DEFINE_SPINLOCK(io_tlb_lock);
>
> static int late_alloc;
>
> +static void swiotlb_print_overflow_deprecation_notice(void)
> +{
> + if (io_tlb_overflow) {
> + pr_warn("SWIOTLB overflow buffer will be deprecated.\n"
> + " If you have a driver that depends on this feature\n"
> + " please Email us at: linux-kernel@...r.kernel.org,\n"
> + " Shuah Khan (shuahkhan@...il.com), and\n"
> + " Konrad Wilk (konrad.wilk@...cle.com)\n");
> + } else {
> + pr_warn("SWIOTLB overflow buffer is disabled and will be\n"
> + " deprecated. Please report problems related to\n"
> + " disabling overflow buffer to\n"
> + " linux-kernel@...r.kernel.org,\n"
> + " Shuah Khan (shuahkhan@...il.com), and\n"
> + " Konrad Wilk (konrad.wilk@...cle.com)\n");
> + }
> +}
> +
> static int __init
> setup_io_tlb_npages(char *str)
> {
> @@ -108,7 +126,6 @@ setup_io_tlb_npages(char *str)
> return 1;
> }
> __setup("swiotlb=", setup_io_tlb_npages);
> -/* make io_tlb_overflow tunable too? */
>
> unsigned long swiotlb_nr_tbl(void)
> {
> @@ -156,12 +173,18 @@ void __init swiotlb_init_with_tbl(char *tlb, unsigned long nslabs, int verbose)
> io_tlb_index = 0;
> io_tlb_orig_addr = alloc_bootmem_pages(PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
>
> - /*
> - * Get the overflow emergency buffer
> - */
> - io_tlb_overflow_buffer = alloc_bootmem_low_pages(PAGE_ALIGN(io_tlb_overflow));
> - if (!io_tlb_overflow_buffer)
> - panic("Cannot allocate SWIOTLB overflow buffer!\n");
> + if (io_tlb_overflow) {
> + /*
> + * Get the overflow emergency buffer
> + */
> + io_tlb_overflow_buffer = alloc_bootmem_low_pages(
> + PAGE_ALIGN(io_tlb_overflow));
> + if (!io_tlb_overflow_buffer)
> + panic("Cannot allocate SWIOTLB overflow buffer!\n");
> + }
> +
> + swiotlb_print_overflow_deprecation_notice();
> +
> if (verbose)
> swiotlb_print_info();
> }
> @@ -264,14 +287,17 @@ swiotlb_late_init_with_default_size(size_t default_size)
>
> memset(io_tlb_orig_addr, 0, io_tlb_nslabs * sizeof(phys_addr_t));
>
> - /*
> - * Get the overflow emergency buffer
> - */
> - io_tlb_overflow_buffer = (void *)__get_free_pages(GFP_DMA,
> - get_order(io_tlb_overflow));
> - if (!io_tlb_overflow_buffer)
> - goto cleanup4;
> + if (io_tlb_overflow) {
> + /*
> + * Get the overflow emergency buffer
> + */
> + io_tlb_overflow_buffer = (void *)
> + __get_free_pages(GFP_DMA, get_order(io_tlb_overflow));
> + if (!io_tlb_overflow_buffer)
> + goto cleanup4;
> + }
>
> + swiotlb_print_overflow_deprecation_notice();
> swiotlb_print_info();
>
> late_alloc = 1;
> @@ -297,12 +323,13 @@ cleanup1:
>
> void __init swiotlb_free(void)
> {
> - if (!io_tlb_overflow_buffer)
> + if (!io_tlb_orig_addr)
> return;
>
> if (late_alloc) {
> - free_pages((unsigned long)io_tlb_overflow_buffer,
> - get_order(io_tlb_overflow));
> + if (io_tlb_overflow_buffer)
> + free_pages((unsigned long)io_tlb_overflow_buffer,
> + get_order(io_tlb_overflow));
> free_pages((unsigned long)io_tlb_orig_addr,
> get_order(io_tlb_nslabs * sizeof(phys_addr_t)));
> free_pages((unsigned long)io_tlb_list, get_order(io_tlb_nslabs *
> @@ -310,8 +337,9 @@ void __init swiotlb_free(void)
> free_pages((unsigned long)io_tlb_start,
> get_order(io_tlb_nslabs << IO_TLB_SHIFT));
> } else {
> - free_bootmem_late(__pa(io_tlb_overflow_buffer),
> - PAGE_ALIGN(io_tlb_overflow));
> + if (io_tlb_overflow_buffer)
> + free_bootmem_late(__pa(io_tlb_overflow_buffer),
> + PAGE_ALIGN(io_tlb_overflow));
> free_bootmem_late(__pa(io_tlb_orig_addr),
> PAGE_ALIGN(io_tlb_nslabs * sizeof(phys_addr_t)));
> free_bootmem_late(__pa(io_tlb_list),
> @@ -681,7 +709,10 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
> map = map_single(dev, phys, size, dir);
> if (!map) {
> swiotlb_full(dev, size, dir, 1);
> + if (!io_tlb_overflow)
> + return DMA_ERROR_CODE;
> map = io_tlb_overflow_buffer;
> + dev_err(dev, "SWIOTLB is full. Mapping overflow buffer.\n");
> }
>
> dev_addr = swiotlb_virt_to_bus(dev, map);
> @@ -691,6 +722,8 @@ dma_addr_t swiotlb_map_page(struct device *dev, struct page *page,
> */
> if (!dma_capable(dev, dev_addr, size)) {
> swiotlb_tbl_unmap_single(dev, map, size, dir);
> + if (!io_tlb_overflow)
> + return DMA_ERROR_CODE;
> dev_addr = swiotlb_virt_to_bus(dev, io_tlb_overflow_buffer);
> }
>
> @@ -910,6 +943,8 @@ EXPORT_SYMBOL(swiotlb_sync_sg_for_device);
> int
> swiotlb_dma_mapping_error(struct device *hwdev, dma_addr_t dma_addr)
> {
> + if (!io_tlb_overflow)
> + return DMA_ERROR_CODE;
> return (dma_addr == swiotlb_virt_to_bus(hwdev, io_tlb_overflow_buffer));
> }
> EXPORT_SYMBOL(swiotlb_dma_mapping_error);
> --
> 1.7.9.5
>
>
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists