From: FUJITA Tomonori <fujita.tomonori@lab.ntt.co.jp>
To: mingo@elte.hu
Cc: fujita.tomonori@lab.ntt.co.jp, jeremy@goop.org,
tony.luck@intel.com, linux-ia64@vger.kernel.org, x86@kernel.org,
linux-kernel@vger.kernel.org, Ian.Campbell@eu.citrix.com,
linuxppc-dev@ozlabs.org, joerg.roedel@amd.com
Subject: Re: [00/15] swiotlb cleanup
Date: Mon, 13 Jul 2009 13:20:22 +0900 [thread overview]
Message-ID: <20090713131157V.fujita.tomonori@lab.ntt.co.jp> (raw)
In-Reply-To: <20090710141248.GE26264@elte.hu>
On Fri, 10 Jul 2009 16:12:48 +0200
Ingo Molnar <mingo@elte.hu> wrote:
> > functionality and reimplemented the surrounding infrastructure in
> > terms of that (and incorporating our additional requirements). I
> > prototyped this (it is currently unworking, in fact it seems to
> > have developed rather a taste for filesystems :-() but the
> > diffstat of my WIP patch is:
> >
> > arch/x86/kernel/pci-swiotlb.c | 6
> > arch/x86/xen/pci-swiotlb.c | 2
> > drivers/pci/xen-iommu.c | 385 ++++++++++++++++++++++++++++++++++++++++--
> > include/linux/swiotlb.h | 12 +
> > lib/swiotlb.c | 10 -
> > 5 files changed, 385 insertions(+), 30 deletions(-)
> >
> > where a fair number of the lines in xen-iommu.c are copies of
> > functions from swiotlb.c with minor modifications. As I say it
> > doesn't work yet but I think it's roughly indicative of what such
> > an approach would look like. I don't like it much but am happy to
> > run with it if it looks to be the most acceptable approach. [...]
>
> +400 lines of code to avoid much fewer lines of generic code impact
> on the lib/swiotlb.c side sounds like a bad technical choice to me.
The amount of code is not the point. The way to impact on the
lib/swiotlb.c is totally wrong from the perspective of the kernel
design; it uses architecture code in the very original (xen) way.
> It makes the swiotlb code less useful and basically forks a random
> implementation of it in drivers/pci/xen-iommu.c.
I don't think so. We always have the reasonable amount of duplication
rather than integration in a dirty way (at least about IOMMU
code).
> Fujita-san, can you think of a solution that avoids the whole-sale
> copying of hundreds of lines of code?
Ian didn't give a pointer to his code in a public place so I'm not
sure his claim is valid. But if his code does the right thing and
the duplication is less than 400 lines, it doesn't sound that bad to
me.
next prev parent reply other threads:[~2009-07-13 4:21 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 [this message]
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
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=20090713131157V.fujita.tomonori@lab.ntt.co.jp \
--to=fujita.tomonori@lab.ntt.co.jp \
--cc=Ian.Campbell@eu.citrix.com \
--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).