linuxppc-dev.lists.ozlabs.org archive mirror
 help / color / mirror / Atom feed
From: Becky Bruce <beckyb@kernel.crashing.org>
To: Ingo Molnar <mingo@elte.hu>
Cc: Jeremy Fitzhardinge <jeremy@goop.org>,
	tony.luck@intel.com, linux-ia64@vger.kernel.org,
	Ian Campbell <Ian.Campbell@citrix.com>,
	x86@kernel.org, linux-kernel@vger.kernel.org,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	linuxppc-dev@ozlabs.org, Joerg Roedel <joerg.roedel@amd.com>
Subject: Re: [00/15] swiotlb cleanup
Date: Mon, 13 Jul 2009 22:13:56 -0500	[thread overview]
Message-ID: <68EFFAF6-EF5B-4148-BC54-70BF2AF2456E@kernel.crashing.org> (raw)
In-Reply-To: <20090710051236.GA22218@elte.hu>


On Jul 10, 2009, at 12:12 AM, Ingo Molnar wrote:

>
> * FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp> wrote:
>
>> - removes unused (and unnecessary) hooks in swiotlb.
>>
>> - adds dma_capable() and converts swiotlb to use it. It can be used  
>> to
>> know if a memory area is dma capable or not. I added
>> is_buffer_dma_capable() for the same purpose long ago but it turned
>> out that the function doesn't work on POWERPC.
>>
>> This can be applied cleanly to linux-next, -mm, and mainline. This
>> patchset touches multiple architectures (ia64, powerpc, x86) so I
>> guess that -mm is appropriate for this patchset (I don't care much
>> what tree would merge this though).
>>
>> This is tested on x86 but only compile tested on POWERPC and IA64.
>>
>> Thanks,
>>
>> =
>> arch/ia64/include/asm/dma-mapping.h    |   18 ++++++
>> arch/powerpc/include/asm/dma-mapping.h |   23 +++++++
>> arch/powerpc/kernel/dma-swiotlb.c      |   48 +---------------
>> arch/x86/include/asm/dma-mapping.h     |   18 ++++++
>> arch/x86/kernel/pci-dma.c              |    2 +-
>> arch/x86/kernel/pci-gart_64.c          |    5 +-
>> arch/x86/kernel/pci-nommu.c            |    2 +-
>> arch/x86/kernel/pci-swiotlb.c          |   25 --------
>> include/linux/dma-mapping.h            |    5 --
>> include/linux/swiotlb.h                |   11 ----
>> lib/swiotlb.c                          |  102 ++++++++ 
>> +-----------------------
>> 11 files changed, 92 insertions(+), 167 deletions(-)
>
> Hm, the functions and facilities you remove here were added as part
> of preparatory patches for Xen guest support. You were aware of
> them, you were involved in discussions about those aspects with Ian
> and Jeremy but still you chose not to Cc: either of them and you
> failed to address that aspect in the changelogs.
>
> I'd like the Xen code to become cleaner more than anyone else here i
> guess, but patch submission methods like this are not really
> helpful. A far better method is to be open about such disagreements,
> to declare them, to Cc: everyone who disagrees, and to line out the
> arguments in the changelogs as well - instead of just curtly
> declaring those APIs 'unused' and failing to Cc: involved parties.
>
> Alas, on the technical level the cleanups themselves look mostly
> fine to me. Ian, Jeremy, the changes will alter Xen's use of
> swiotlb, but can the Xen side still live with these new methods - in
> particular is dma_capable() sufficient as a mechanism and can the
> Xen side filter out DMA allocations to make them physically
> continuous?
>
> Ben, Tony, Becky, any objections wrt. the PowerPC / IA64 impact? If
> everyone agrees i can apply them to the IOMMU tree, test it and push
> it out to -next, etc.
>

Ingo,

With the exception of the patch I commented on, I think these look OK  
from the powerpc point of view.  I've successfully booted one of my  
test platforms with the entire series applied and will run some more  
extensive (i.e. not "Whee!  A prompt!") tests tomorrow.

-Becky

  parent reply	other threads:[~2009-07-14  3:15 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-07-10  1:04 [00/15] swiotlb cleanup FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 01/15] swiotlb: remove unused swiotlb_alloc_boot() FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 02/15] swiotlb: remove unused swiotlb_alloc() FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 03/15] swiotlb: remove swiotlb_arch_range_needs_mapping FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 04/15] swiotlb: remove unnecessary swiotlb_bus_to_virt FUJITA Tomonori
2009-07-14  2:17   ` Becky Bruce
2009-07-14  5:08     ` FUJITA Tomonori
2009-07-16  3:40     ` Benjamin Herrenschmidt
2009-07-10  1:04 ` [PATCH 05/15] x86: add dma_capable() to replace is_buffer_dma_capable() FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 06/15] x86: replace is_buffer_dma_capable() with dma_capable FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 07/15] ia64: add dma_capable() to replace is_buffer_dma_capable() FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 08/15] powerpc: " FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 09/15] swiotlb: use dma_capable() FUJITA Tomonori
2009-07-10  1:04 ` [PATCH 10/15] powerpc: remove unncesary swiotlb_arch_address_needs_mapping FUJITA Tomonori
2009-07-10  1:05 ` [PATCH 11/15] remove is_buffer_dma_capable() FUJITA Tomonori
2009-07-10  1:05 ` [PATCH 12/15] x86, IA64, powerpc: add phys_to_dma() and dma_to_phys() FUJITA Tomonori
2009-07-10  1:05 ` [PATCH 13/15] swiotlb: use phys_to_dma and dma_to_phys FUJITA Tomonori
2009-07-10  1:05 ` [PATCH 14/15] powerpc: remove unused swiotlb_phys_to_bus() and swiotlb_bus_to_phys() FUJITA Tomonori
2009-07-10  1:05 ` [PATCH 15/15] x86: " FUJITA Tomonori
2009-07-10  5:12 ` [00/15] swiotlb cleanup Ingo Molnar
2009-07-10  5:35   ` FUJITA Tomonori
2009-07-10 14:02     ` Ian Campbell
2009-07-13  4:20       ` FUJITA Tomonori
2009-07-13  9:40         ` Ian Campbell
2009-07-13  9:53           ` FUJITA Tomonori
2009-07-13 10:05             ` Ian Campbell
2009-07-10 14:01   ` Ian Campbell
2009-07-10 14:12     ` Ingo Molnar
2009-07-13  4:20       ` FUJITA Tomonori
2009-07-13  9:16         ` FUJITA Tomonori
2009-07-18 10:41           ` Ingo Molnar
2009-07-14  3:13   ` Becky Bruce [this message]
2009-07-15 20:24     ` Becky Bruce

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=68EFFAF6-EF5B-4148-BC54-70BF2AF2456E@kernel.crashing.org \
    --to=beckyb@kernel.crashing.org \
    --cc=Ian.Campbell@citrix.com \
    --cc=fujita.tomonori@lab.ntt.co.jp \
    --cc=jeremy@goop.org \
    --cc=joerg.roedel@amd.com \
    --cc=linux-ia64@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=mingo@elte.hu \
    --cc=tony.luck@intel.com \
    --cc=x86@kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).