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>,
	Joerg Roedel <joerg.roedel@amd.com>,
	x86@kernel.org,
	"linux-kernel@vger.kernel.org Mailing List"
	<linux-kernel@vger.kernel.org>,
	FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>,
	"linuxppc-dev@ozlabs.org list" <linuxppc-dev@ozlabs.org>
Subject: Re: [00/15] swiotlb cleanup
Date: Wed, 15 Jul 2009 15:24:33 -0500	[thread overview]
Message-ID: <FF72C10D-2755-4B85-8714-8C35F34FED69@kernel.crashing.org> (raw)
In-Reply-To: <68EFFAF6-EF5B-4148-BC54-70BF2AF2456E@kernel.crashing.org>


On Jul 13, 2009, at 10:13 PM, Becky Bruce wrote:

>
> 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.

Well, I am still testing.  I've observed one unexpected LTP testcase  
failure with these patches applied, but so far have been unable to  
reproduce it.  So these patches are probably OK, but I will look into  
this some more next week.

-Becky

>
>
> -Becky
>
> _______________________________________________
> Linuxppc-dev mailing list
> Linuxppc-dev@lists.ozlabs.org
> https://lists.ozlabs.org/listinfo/linuxppc-dev

      reply	other threads:[~2009-07-15 20:26 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
2009-07-15 20:24     ` Becky Bruce [this message]

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=FF72C10D-2755-4B85-8714-8C35F34FED69@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).