lkml.org 
[lkml]   [2012]   [Aug]   [6]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: [PATCH RESEND] swiotlb: Disable swiotlb overflow support when CONFIG_ISA is disabled
    From
    Date
    On Tue, 2012-07-24 at 20:48 -0400, Konrad Rzeszutek Wilk wrote:
    > On Tue, Jul 24, 2012 at 01:46:27PM -0600, Shuah Khan wrote:
    > > Disable iotlb overflow support when CONFIG_ISA is disabled. Add deprecation
    >
    > You need to check one more thing. In the email I mentioned that the bulk
    > of the drivers that utilize this are ISA, but there are also some that
    > are PCI dependent.
    >
    > So I grepped for anything that does 'dma_map_page' and some of them are
    > even PCIe, for example: drivers/net/ethernet/intel/ixgbevf/ixgbevf_main.c
    >
    > Which means that the check for CONFIG_ISA is not sufficient.

    Thanks for the review and comments. Got back from a two week vacation
    and going to start work on this. I started sending fixes to drivers I
    can find that don't check dma mapping errors as well.

    -- Shuah
    >
    > > notice warning message and deprecation schedule documentation. This is the
    > > first step towards removing overflow support, to be consistent with other
    > > iommu implementations and return DMA_ERROR_CODE. This disabling step is for
    > > finding drivers that don't call dma_mapping_error to check for errors returned
    > > by the mapping interface. Once drivers are fixed overflow support can be
    > > removed.
    > >
    > > Signed-off-by: Shuah Khan <shuah.khan@hp.com>
    >
    > Did you do a cross compile on IA64 just to double check? Comments below.
    >
    > > ---
    > > Documentation/feature-removal-schedule.txt | 22 ++++++--
    > > lib/swiotlb.c | 79 +++++++++++++++++++++-------
    > > 2 files changed, 79 insertions(+), 22 deletions(-)
    > >
    > > diff --git a/Documentation/feature-removal-schedule.txt b/Documentation/feature-removal-schedule.txt
    > > index 5979c3e..ce6c0ae 100644
    > > --- a/Documentation/feature-removal-schedule.txt
    > > +++ b/Documentation/feature-removal-schedule.txt
    > > @@ -35,7 +35,7 @@ What: x86_32 "no-hlt" cmdline param
    > > When: 2012
    > > Why: remove a branch from idle path, simplify code used by everybody.
    > > This option disabled the use of HLT in idle and machine_halt()
    > > - for hardware that was flakey 15-years ago. Today we have
    > > + for hardware that was flaky 15-years ago. Today we have
    >
    > Ummm, that is not part of what this patch should be doing.
    >
    > > "idle=poll" that removed HLT from idle, and so if such a machine
    > > is still running the upstream kernel, "idle=poll" is likely sufficient.
    > > Who: Len Brown <len.brown@intel.com>
    > > @@ -160,7 +160,7 @@ Files: arch/*/kernel/*_ksyms.c
    > > Check: kernel_thread
    > > Why: kernel_thread is a low-level implementation detail. Drivers should
    > > use the <linux/kthread.h> API instead which shields them from
    > > - implementation details and provides a higherlevel interface that
    > > + implementation details and provides a higher level interface that
    >
    > Neither is this.
    > > prevents bugs and code duplication
    > > Who: Christoph Hellwig <hch@lst.de>
    > >
    > > @@ -236,7 +236,7 @@ Who: David Brownell <dbrownell@users.sourceforge.net>
    > >
    > > What: b43 support for firmware revision < 410
    > > When: The schedule was July 2008, but it was decided that we are going to keep the
    > > - code as long as there are no major maintanance headaches.
    > > + code as long as there are no major maintenance headaches.
    >
    > Or this.
    >
    > > So it _could_ be removed _any_ time now, if it conflicts with something new.
    > > Why: The support code for the old firmware hurts code readability/maintainability
    > > and slightly hurts runtime performance. Bugfixes for the old firmware
    > > @@ -608,3 +608,19 @@ When: June 2013
    > > Why: Unsupported/unmaintained/unused since 2.6
    > >
    > > ----------------------------
    > > +
    > > +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.6, Disable iotlb overflow
    > > + support when CONFIG_ISA is disabled with the intent to find drivers
    > > + that don't call dma_mapping_error to check for errors returned by the
    > > + mapping interface. 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@vger.kernel.org
    > > +Who: Shuah Khan <shuah.khan@hp.com> <shuahkhan@gmail.com>
    > > +
    > > +----------------------------
    > > diff --git a/lib/swiotlb.c b/lib/swiotlb.c
    > > index 45bc1f8..0123bb8 100644
    > > --- a/lib/swiotlb.c
    > > +++ b/lib/swiotlb.c
    > > @@ -15,6 +15,9 @@
    > > * 05/09/10 linville Add support for syncing ranges, support syncing for
    > > * DMA_BIDIRECTIONAL mappings, miscellaneous cleanup.
    > > * 08/12/11 beckyb Add highmem support
    > > + * 07/2012 shuahkhan Disable iotlb overflow support when CONFIG_ISA
    > > + * is enabled. Remove it for all configs when drivers
    > > + * that don't check for mapping errors are fixed.
    >
    > Don't bother. The changelog is now in the git commit.
    >
    > > */
    > >
    > > #include <linux/cache.h>
    > > @@ -68,7 +71,11 @@ static unsigned long io_tlb_nslabs;
    > > /*
    > > * When the IOMMU overflows we return a fallback buffer. This sets the size.
    > > */
    > > +#if defined(CONFIG_ISA)
    > > static unsigned long io_tlb_overflow = 32*1024;
    > > +#else
    > > +static unsigned long io_tlb_overflow;
    > > +#endif
    > >
    > > static void *io_tlb_overflow_buffer;
    > >
    > > @@ -92,6 +99,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@vger.kernel.org,\n"
    > > + " Shuah Khan (shuahkhan@gmail.com), and\n"
    > > + " Konrad Wilk (konrad.wilk@oracle.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@vger.kernel.org,\n"
    > > + " Shuah Khan (shuahkhan@gmail.com), and\n"
    > > + " Konrad Wilk (konrad.wilk@oracle.com)\n");
    > > + }
    > > +}
    > > +
    > > static int __init
    > > setup_io_tlb_npages(char *str)
    > > {
    > > @@ -108,7 +133,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 +180,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 +294,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 +330,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 +344,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,6 +716,8 @@ 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;
    > > }
    > >
    > > @@ -691,6 +728,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 +949,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@vger.kernel.org
    > > More majordomo info at http://vger.kernel.org/majordomo-info.html
    > > Please read the FAQ at http://www.tux.org/lkml/




    \
     
     \ /
      Last update: 2012-08-07 02:21    [W:0.052 / U:30.156 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site