2008-10-24 05:43:09

by FUJITA Tomonori

[permalink] [raw]
Subject: [PATCH] x86: remove dead BIO_VMERGE_BOUNDARY definition

The block layer dropped the virtual merge feature
(b8b3e16cfe6435d961f6aaebcfd52a1ff2a988c5). BIO_VMERGE_BOUNDARY
definition is meaningless now.

Signed-off-by: FUJITA Tomonori <[email protected]>
---
arch/x86/include/asm/io.h | 2 --
arch/x86/include/asm/io_64.h | 2 --
arch/x86/kernel/pci-dma.c | 6 ------
3 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 5618a10..2f13dbe 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -59,8 +59,6 @@ build_mmio_write(__writeq, "q", unsigned long, "r", )
#define writeq writeq
#endif

-extern int iommu_bio_merge;
-
#ifdef CONFIG_X86_32
# include "io_32.h"
#else
diff --git a/arch/x86/include/asm/io_64.h b/arch/x86/include/asm/io_64.h
index fea325a..563c162 100644
--- a/arch/x86/include/asm/io_64.h
+++ b/arch/x86/include/asm/io_64.h
@@ -232,8 +232,6 @@ void memset_io(volatile void __iomem *a, int b, size_t c);

#define flush_write_buffers()

-#define BIO_VMERGE_BOUNDARY iommu_bio_merge
-
/*
* Convert a virtual cached pointer to an uncached pointer
*/
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 1972266..d851667 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -28,11 +28,6 @@ int no_iommu __read_mostly;
/* Set this to 1 if there is a HW IOMMU in the system */
int iommu_detected __read_mostly = 0;

-/* This tells the BIO block layer to assume merging. Default to off
- because we cannot guarantee merging later. */
-int iommu_bio_merge __read_mostly = 0;
-EXPORT_SYMBOL(iommu_bio_merge);
-
dma_addr_t bad_dma_address __read_mostly = 0;
EXPORT_SYMBOL(bad_dma_address);

@@ -186,7 +181,6 @@ static __init int iommu_setup(char *p)
}

if (!strncmp(p, "biomerge", 8)) {
- iommu_bio_merge = 4096;
iommu_merge = 1;
force_iommu = 1;
}
--
1.5.4.2


2008-10-24 20:59:19

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE

Define __BIOVEC_PHYS_MERGEABLE as the default implementation of
BIOVEC_PHYS_MERGEABLE, so that its available for reuse within an
arch-specific definition of BIOVEC_PHYS_MERGEABLE.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Cc: Jens Axboe <[email protected]>
---
include/linux/bio.h | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)

===================================================================
--- a/include/linux/bio.h
+++ b/include/linux/bio.h
@@ -221,12 +221,16 @@
#define __BVEC_END(bio) bio_iovec_idx((bio), (bio)->bi_vcnt - 1)
#define __BVEC_START(bio) bio_iovec_idx((bio), (bio)->bi_idx)

+/* Default implementation of BIOVEC_PHYS_MERGEABLE */
+#define __BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
+ ((bvec_to_phys((vec1)) + (vec1)->bv_len) == bvec_to_phys((vec2)))
+
/*
* allow arch override, for eg virtualized architectures (put in asm/io.h)
*/
#ifndef BIOVEC_PHYS_MERGEABLE
#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
- ((bvec_to_phys((vec1)) + (vec1)->bv_len) == bvec_to_phys((vec2)))
+ __BIOVEC_PHYS_MERGEABLE(vec1, vec2)
#endif

#define __BIO_SEG_BOUNDARY(addr1, addr2, mask) \

2008-10-24 20:59:33

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH] x86: remove dead BIO_VMERGE_BOUNDARY definition

FUJITA Tomonori wrote:
> The block layer dropped the virtual merge feature
> (b8b3e16cfe6435d961f6aaebcfd52a1ff2a988c5). BIO_VMERGE_BOUNDARY
> definition is meaningless now.
>

Hm, I think we still need something here, no?

I have a couple of patches to make x86 use BIOVEC_PHYS_MERGEABLE
instead, which would probably apply to other arches.

J

2008-10-24 20:59:46

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

The bio common code no longer uses BIO_VMERGE_BOUNDARY, so replace it
with BIOVEC_PHYS_MERGEABLE. Also make iommu_bio_merge a boolean
rather than a size, as befits its use.

Signed-off-by: Jeremy Fitzhardinge <[email protected]>
Cc: Jens Axboe <[email protected]>
---
arch/x86/include/asm/io.h | 8 +++++++-
arch/x86/include/asm/io_64.h | 2 --
arch/x86/kernel/pci-dma.c | 4 ++--
3 files changed, 9 insertions(+), 5 deletions(-)

===================================================================
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -33,7 +33,7 @@

/* This tells the BIO block layer to assume merging. Default to off
because we cannot guarantee merging later. */
-int iommu_bio_merge __read_mostly = 0;
+bool iommu_bio_merge __read_mostly = false;
EXPORT_SYMBOL(iommu_bio_merge);

dma_addr_t bad_dma_address __read_mostly = 0;
@@ -189,7 +189,7 @@
}

if (!strncmp(p, "biomerge", 8)) {
- iommu_bio_merge = 4096;
+ iommu_bio_merge = true;
iommu_merge = 1;
force_iommu = 1;
}
===================================================================
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -4,6 +4,7 @@
#define ARCH_HAS_IOREMAP_WC

#include <linux/compiler.h>
+#include <linux/types.h>

#define build_mmio_read(name, size, type, reg, barrier) \
static inline type name(const volatile void __iomem *addr) \
@@ -59,12 +60,17 @@
#define writeq writeq
#endif

-extern int iommu_bio_merge;
+extern bool iommu_bio_merge;

#ifdef CONFIG_X86_32
# include "io_32.h"
#else
# include "io_64.h"
+#endif
+
+#ifdef CONFIG_X86_64
+#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) \
+ (iommu_bio_merge && __BIOVEC_PHYS_MERGEABLE(vec1, vec2))
#endif

extern void *xlate_dev_mem_ptr(unsigned long phys);
===================================================================
--- a/arch/x86/include/asm/io_64.h
+++ b/arch/x86/include/asm/io_64.h
@@ -232,8 +232,6 @@

#define flush_write_buffers()

-#define BIO_VMERGE_BOUNDARY iommu_bio_merge
-
/*
* Convert a virtual cached pointer to an uncached pointer
*/

2008-10-27 04:03:18

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Fri, 24 Oct 2008 13:59:06 -0700
Jeremy Fitzhardinge <[email protected]> wrote:

> The bio common code no longer uses BIO_VMERGE_BOUNDARY, so replace it
> with BIOVEC_PHYS_MERGEABLE. Also make iommu_bio_merge a boolean
> rather than a size, as befits its use.
>
> Signed-off-by: Jeremy Fitzhardinge <[email protected]>
> Cc: Jens Axboe <[email protected]>

This doesn't look correct.

The block layer always done the physical merge if possible. We don't
provide any kernel parameter to disable it.

The iommu_bio_merge parameter had been used to enable the virtual
merge. As I wrote, the virtual merge feature was completely
removed. Effectively, the iommu_bio_merge parameter is meaningless
now.

2008-10-27 04:42:33

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH] x86: remove dead BIO_VMERGE_BOUNDARY definition

On Fri, 24 Oct 2008 13:58:58 -0700
Jeremy Fitzhardinge <[email protected]> wrote:

> FUJITA Tomonori wrote:
> > The block layer dropped the virtual merge feature
> > (b8b3e16cfe6435d961f6aaebcfd52a1ff2a988c5). BIO_VMERGE_BOUNDARY
> > definition is meaningless now.
> >
>
> Hm, I think we still need something here, no?

I don't think so.


> I have a couple of patches to make x86 use BIOVEC_PHYS_MERGEABLE
> instead, which would probably apply to other arches.

As I wrote, the patch doesn't look correct (I think that the first
patch is fine if some arch want to use __BIOVEC_PHYS_MERGEABLE but no
one in mainline overrides BIOVEC_PHYS_MERGEABLE):

http://lkml.org/lkml/2008/10/27/1

2008-10-27 08:21:54

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

FUJITA Tomonori wrote:
> The block layer always done the physical merge if possible. We don't
> provide any kernel parameter to disable it.
>
> The iommu_bio_merge parameter had been used to enable the virtual
> merge. As I wrote, the virtual merge feature was completely
> removed. Effectively, the iommu_bio_merge parameter is meaningless
> now.
>

Under Xen, pages which appear to be pseudo-physically adjacent are not
necessarily really physically adjacent. We need to hook
BIOVEC_PHYS_MERGEABLE to prevent the bio layer from inappropriately
merging requests across non-contiguous page boundaries.

J

2008-10-27 08:38:27

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Mon, 27 Oct 2008 19:21:41 +1100
Jeremy Fitzhardinge <[email protected]> wrote:

> FUJITA Tomonori wrote:
> > The block layer always done the physical merge if possible. We don't
> > provide any kernel parameter to disable it.
> >
> > The iommu_bio_merge parameter had been used to enable the virtual
> > merge. As I wrote, the virtual merge feature was completely
> > removed. Effectively, the iommu_bio_merge parameter is meaningless
> > now.
> >
>
> Under Xen, pages which appear to be pseudo-physically adjacent are not
> necessarily really physically adjacent. We need to hook
> BIOVEC_PHYS_MERGEABLE to prevent the bio layer from inappropriately
> merging requests across non-contiguous page boundaries.

I'm not familiar with what Xen does but why can't Xen just override
BIOVEC_PHYS_MERGEABLE?

Why does Xen need to hook BIOVEC_PHYS_MERGEABLE to the iommu_bio_merge
parameter (as this patch does)? BIOVEC_PHYS_MERGEABLE and the
iommu_bio_merge parameter are not related at all.

2008-10-27 09:19:13

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

FUJITA Tomonori wrote:
> I'm not familiar with what Xen does but why can't Xen just override
> BIOVEC_PHYS_MERGEABLE?
>
> Why does Xen need to hook BIOVEC_PHYS_MERGEABLE to the iommu_bio_merge
> parameter (as this patch does)? BIOVEC_PHYS_MERGEABLE and the
> iommu_bio_merge parameter are not related at all.
>

No, it doesn't. It was convenient to reuse that mechanism, but I can
easily re-add something else (which would be more or less identical).

J

2008-10-27 09:37:17

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Mon, 27 Oct 2008 20:18:44 +1100
Jeremy Fitzhardinge <[email protected]> wrote:

> FUJITA Tomonori wrote:
> > I'm not familiar with what Xen does but why can't Xen just override
> > BIOVEC_PHYS_MERGEABLE?
> >
> > Why does Xen need to hook BIOVEC_PHYS_MERGEABLE to the iommu_bio_merge
> > parameter (as this patch does)? BIOVEC_PHYS_MERGEABLE and the
> > iommu_bio_merge parameter are not related at all.
> >
>
> No, it doesn't. It was convenient to reuse that mechanism, but I can
> easily re-add something else (which would be more or less identical).

I still don't see how Xen needs something like the virtual merge
(sounds that overriding BIOVEC_PHYS_MERGEABLE perfectly works for Xen)
or why Xen needs a new boot parameter for it. The virtual merge just
defines how IOMMUs should work.

2008-10-27 09:42:39

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Mon, Oct 27 2008, FUJITA Tomonori wrote:
> On Mon, 27 Oct 2008 20:18:44 +1100
> Jeremy Fitzhardinge <[email protected]> wrote:
>
> > FUJITA Tomonori wrote:
> > > I'm not familiar with what Xen does but why can't Xen just override
> > > BIOVEC_PHYS_MERGEABLE?
> > >
> > > Why does Xen need to hook BIOVEC_PHYS_MERGEABLE to the iommu_bio_merge
> > > parameter (as this patch does)? BIOVEC_PHYS_MERGEABLE and the
> > > iommu_bio_merge parameter are not related at all.
> > >
> >
> > No, it doesn't. It was convenient to reuse that mechanism, but I can
> > easily re-add something else (which would be more or less identical).
>
> I still don't see how Xen needs something like the virtual merge
> (sounds that overriding BIOVEC_PHYS_MERGEABLE perfectly works for Xen)
> or why Xen needs a new boot parameter for it. The virtual merge just
> defines how IOMMUs should work.

Pretty much baffles me as well, xen should just need to do

#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0

and that should be it.

--
Jens Axboe

2008-10-27 11:44:07

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

Jens Axboe wrote:
> Pretty much baffles me as well, xen should just need to do
>
> #define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0
>

It needs to be a runtime switch, since we only want to do this when
actually running under Xen. Also, its possible that the two pages might
actually be physically contiguous, so they could be merged anyway.

J

2008-10-27 11:58:19

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Mon, Oct 27 2008, Jeremy Fitzhardinge wrote:
> Jens Axboe wrote:
> >Pretty much baffles me as well, xen should just need to do
> >
> >#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0
> >
>
> It needs to be a runtime switch, since we only want to do this when
> actually running under Xen. Also, its possible that the two pages might
> actually be physically contiguous, so they could be merged anyway.

Alright, then add a xen_biovec_phys_mergeable(vec1, vec2) in the xen
code that actually checks this for real. You can add your switch there
as well. Then put the BIOVEC_PHYS_MERGEABLE() in the xen arch includes,
done.

What Tomo is saying is that this has nothing to do with virtual merging,
and he's right.

--
Jens Axboe

2008-10-28 23:06:52

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Mon, 27 Oct 2008 12:57:00 +0100
Jens Axboe <[email protected]> wrote:

> On Mon, Oct 27 2008, Jeremy Fitzhardinge wrote:
> > Jens Axboe wrote:
> > >Pretty much baffles me as well, xen should just need to do
> > >
> > >#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0
> > >
> >
> > It needs to be a runtime switch, since we only want to do this when
> > actually running under Xen. Also, its possible that the two pages might
> > actually be physically contiguous, so they could be merged anyway.
>
> Alright, then add a xen_biovec_phys_mergeable(vec1, vec2) in the xen
> code that actually checks this for real. You can add your switch there
> as well. Then put the BIOVEC_PHYS_MERGEABLE() in the xen arch includes,
> done.
>
> What Tomo is saying is that this has nothing to do with virtual merging,
> and he's right.

Yeah, overriding BIOVEC_PHYS_MERGEABLE perfectly works for Xen. And it
is not related with BIO_VMERGE_BOUNDARY at all.

Ingo, please put this patch into your tree:

http://marc.info/?l=linux-kernel&m=122482703716620&w=2


Thanks,

2008-10-29 08:37:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE


* FUJITA Tomonori <[email protected]> wrote:

> On Mon, 27 Oct 2008 12:57:00 +0100
> Jens Axboe <[email protected]> wrote:
>
> > On Mon, Oct 27 2008, Jeremy Fitzhardinge wrote:
> > > Jens Axboe wrote:
> > > >Pretty much baffles me as well, xen should just need to do
> > > >
> > > >#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0
> > > >
> > >
> > > It needs to be a runtime switch, since we only want to do this when
> > > actually running under Xen. Also, its possible that the two pages might
> > > actually be physically contiguous, so they could be merged anyway.
> >
> > Alright, then add a xen_biovec_phys_mergeable(vec1, vec2) in the xen
> > code that actually checks this for real. You can add your switch there
> > as well. Then put the BIOVEC_PHYS_MERGEABLE() in the xen arch includes,
> > done.
> >
> > What Tomo is saying is that this has nothing to do with virtual merging,
> > and he's right.
>
> Yeah, overriding BIOVEC_PHYS_MERGEABLE perfectly works for Xen. And it
> is not related with BIO_VMERGE_BOUNDARY at all.
>
> Ingo, please put this patch into your tree:
>
> http://marc.info/?l=linux-kernel&m=122482703716620&w=2

does it have any dependency on:

Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE

?

Ingo

2008-10-29 09:25:20

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

Ingo Molnar wrote:
> * FUJITA Tomonori <[email protected]> wrote:
>
>
>> On Mon, 27 Oct 2008 12:57:00 +0100
>> Jens Axboe <[email protected]> wrote:
>>
>>
>>> On Mon, Oct 27 2008, Jeremy Fitzhardinge wrote:
>>>
>>>> Jens Axboe wrote:
>>>>
>>>>> Pretty much baffles me as well, xen should just need to do
>>>>>
>>>>> #define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0
>>>>>
>>>>>
>>>> It needs to be a runtime switch, since we only want to do this when
>>>> actually running under Xen. Also, its possible that the two pages might
>>>> actually be physically contiguous, so they could be merged anyway.
>>>>
>>> Alright, then add a xen_biovec_phys_mergeable(vec1, vec2) in the xen
>>> code that actually checks this for real. You can add your switch there
>>> as well. Then put the BIOVEC_PHYS_MERGEABLE() in the xen arch includes,
>>> done.
>>>
>>> What Tomo is saying is that this has nothing to do with virtual merging,
>>> and he's right.
>>>
>> Yeah, overriding BIOVEC_PHYS_MERGEABLE perfectly works for Xen. And it
>> is not related with BIO_VMERGE_BOUNDARY at all.
>>
>> Ingo, please put this patch into your tree:
>>
>> http://marc.info/?l=linux-kernel&m=122482703716620&w=2
>>
>
> does it have any dependency on:
>
> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
>

No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
useful for the Xen changes I need to make in the wake of Tomo's patch.

J

2008-10-29 09:28:54

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Wed, Oct 29 2008, FUJITA Tomonori wrote:
> On Mon, 27 Oct 2008 12:57:00 +0100
> Jens Axboe <[email protected]> wrote:
>
> > On Mon, Oct 27 2008, Jeremy Fitzhardinge wrote:
> > > Jens Axboe wrote:
> > > >Pretty much baffles me as well, xen should just need to do
> > > >
> > > >#define BIOVEC_PHYS_MERGEABLE(vec1, vec2) 0
> > > >
> > >
> > > It needs to be a runtime switch, since we only want to do this when
> > > actually running under Xen. Also, its possible that the two pages might
> > > actually be physically contiguous, so they could be merged anyway.
> >
> > Alright, then add a xen_biovec_phys_mergeable(vec1, vec2) in the xen
> > code that actually checks this for real. You can add your switch there
> > as well. Then put the BIOVEC_PHYS_MERGEABLE() in the xen arch includes,
> > done.
> >
> > What Tomo is saying is that this has nothing to do with virtual merging,
> > and he's right.
>
> Yeah, overriding BIOVEC_PHYS_MERGEABLE perfectly works for Xen. And it
> is not related with BIO_VMERGE_BOUNDARY at all.
>
> Ingo, please put this patch into your tree:
>
> http://marc.info/?l=linux-kernel&m=122482703716620&w=2

You can add my Signed-off-by: as well.

--
Jens Axboe

2008-10-29 10:51:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE


* Jeremy Fitzhardinge <[email protected]> wrote:

>> does it have any dependency on:
>>
>> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
>
> No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
> useful for the Xen changes I need to make in the wake of Tomo's
> patch.

ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
Acked-by)

Ingo

2008-10-29 11:25:34

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE


* Ingo Molnar <[email protected]> wrote:

>
> * Jeremy Fitzhardinge <[email protected]> wrote:
>
> >> does it have any dependency on:
> >>
> >> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
> >
> > No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
> > useful for the Xen changes I need to make in the wake of Tomo's
> > patch.
>
> ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
> Acked-by)

build failure, zapped it:

fs/bio.c: In function '__bio_add_page':
fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'

config attached.

Ingo


Attachments:
(No filename) (610.00 B)
config (61.37 kB)
Download all attachments

2008-10-29 11:29:10

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Wed, Oct 29 2008, Ingo Molnar wrote:
>
> * Ingo Molnar <[email protected]> wrote:
>
> >
> > * Jeremy Fitzhardinge <[email protected]> wrote:
> >
> > >> does it have any dependency on:
> > >>
> > >> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
> > >
> > > No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
> > > useful for the Xen changes I need to make in the wake of Tomo's
> > > patch.
> >
> > ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
> > Acked-by)
>
> build failure, zapped it:
>
> fs/bio.c: In function '__bio_add_page':
> fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'

What patch(es) did you apply?

--
Jens Axboe

2008-10-29 11:30:36

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE


* Jens Axboe <[email protected]> wrote:

> On Wed, Oct 29 2008, Ingo Molnar wrote:
> >
> > * Ingo Molnar <[email protected]> wrote:
> >
> > >
> > > * Jeremy Fitzhardinge <[email protected]> wrote:
> > >
> > > >> does it have any dependency on:
> > > >>
> > > >> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
> > > >
> > > > No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
> > > > useful for the Xen changes I need to make in the wake of Tomo's
> > > > patch.
> > >
> > > ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
> > > Acked-by)
> >
> > build failure, zapped it:
> >
> > fs/bio.c: In function '__bio_add_page':
> > fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'
>
> What patch(es) did you apply?

the one i replied to (2/2), and which one Jeremy said was independent
of 1/2. Not so much it appears.

Ingo

2008-10-29 11:35:54

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Wed, Oct 29 2008, Ingo Molnar wrote:
>
> * Jens Axboe <[email protected]> wrote:
>
> > On Wed, Oct 29 2008, Ingo Molnar wrote:
> > >
> > > * Ingo Molnar <[email protected]> wrote:
> > >
> > > >
> > > > * Jeremy Fitzhardinge <[email protected]> wrote:
> > > >
> > > > >> does it have any dependency on:
> > > > >>
> > > > >> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
> > > > >
> > > > > No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
> > > > > useful for the Xen changes I need to make in the wake of Tomo's
> > > > > patch.
> > > >
> > > > ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
> > > > Acked-by)
> > >
> > > build failure, zapped it:
> > >
> > > fs/bio.c: In function '__bio_add_page':
> > > fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'
> >
> > What patch(es) did you apply?
>
> the one i replied to (2/2), and which one Jeremy said was independent
> of 1/2. Not so much it appears.

OK, those two are definitely connected, since the first one defines
__BIOVEC_PHYS_MERGEABLE :-)

The one I acked was in the mail I replied to, Tomos patch.

--
Jens Axboe

2008-10-29 11:56:00

by FUJITA Tomonori

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Wed, 29 Oct 2008 12:34:34 +0100
Jens Axboe <[email protected]> wrote:

> On Wed, Oct 29 2008, Ingo Molnar wrote:
> >
> > * Jens Axboe <[email protected]> wrote:
> >
> > > On Wed, Oct 29 2008, Ingo Molnar wrote:
> > > >
> > > > * Ingo Molnar <[email protected]> wrote:
> > > >
> > > > >
> > > > > * Jeremy Fitzhardinge <[email protected]> wrote:
> > > > >
> > > > > >> does it have any dependency on:
> > > > > >>
> > > > > >> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
> > > > > >
> > > > > > No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
> > > > > > useful for the Xen changes I need to make in the wake of Tomo's
> > > > > > patch.
> > > > >
> > > > > ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
> > > > > Acked-by)
> > > >
> > > > build failure, zapped it:
> > > >
> > > > fs/bio.c: In function '__bio_add_page':
> > > > fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'
> > >
> > > What patch(es) did you apply?
> >
> > the one i replied to (2/2), and which one Jeremy said was independent
> > of 1/2. Not so much it appears.
>
> OK, those two are definitely connected, since the first one defines
> __BIOVEC_PHYS_MERGEABLE :-)

And [2/2] is wrong (that's what Jens and I explained to
Jeremy). Please don't apply it.


> The one I acked was in the mail I replied to, Tomos patch.

For your convenience, here's a repost (with Jens' ack).

=
From: FUJITA Tomonori <[email protected]>
Subject: [PATCH] x86: remove dead BIO_VMERGE_BOUNDARY definition

The block layer dropped the virtual merge feature
(b8b3e16cfe6435d961f6aaebcfd52a1ff2a988c5). BIO_VMERGE_BOUNDARY
definition is meaningless now.

Signed-off-by: FUJITA Tomonori <[email protected]>
Acked-by: Jens Axboe <[email protected]>
---
arch/x86/include/asm/io.h | 2 --
arch/x86/include/asm/io_64.h | 2 --
arch/x86/kernel/pci-dma.c | 6 ------
3 files changed, 0 insertions(+), 10 deletions(-)

diff --git a/arch/x86/include/asm/io.h b/arch/x86/include/asm/io.h
index 5618a10..2f13dbe 100644
--- a/arch/x86/include/asm/io.h
+++ b/arch/x86/include/asm/io.h
@@ -59,8 +59,6 @@ build_mmio_write(__writeq, "q", unsigned long, "r", )
#define writeq writeq
#endif

-extern int iommu_bio_merge;
-
#ifdef CONFIG_X86_32
# include "io_32.h"
#else
diff --git a/arch/x86/include/asm/io_64.h b/arch/x86/include/asm/io_64.h
index fea325a..563c162 100644
--- a/arch/x86/include/asm/io_64.h
+++ b/arch/x86/include/asm/io_64.h
@@ -232,8 +232,6 @@ void memset_io(volatile void __iomem *a, int b, size_t c);

#define flush_write_buffers()

-#define BIO_VMERGE_BOUNDARY iommu_bio_merge
-
/*
* Convert a virtual cached pointer to an uncached pointer
*/
diff --git a/arch/x86/kernel/pci-dma.c b/arch/x86/kernel/pci-dma.c
index 1972266..d851667 100644
--- a/arch/x86/kernel/pci-dma.c
+++ b/arch/x86/kernel/pci-dma.c
@@ -28,11 +28,6 @@ int no_iommu __read_mostly;
/* Set this to 1 if there is a HW IOMMU in the system */
int iommu_detected __read_mostly = 0;

-/* This tells the BIO block layer to assume merging. Default to off
- because we cannot guarantee merging later. */
-int iommu_bio_merge __read_mostly = 0;
-EXPORT_SYMBOL(iommu_bio_merge);
-
dma_addr_t bad_dma_address __read_mostly = 0;
EXPORT_SYMBOL(bad_dma_address);

@@ -186,7 +181,6 @@ static __init int iommu_setup(char *p)
}

if (!strncmp(p, "biomerge", 8)) {
- iommu_bio_merge = 4096;
iommu_merge = 1;
force_iommu = 1;
}
--
1.5.5.GIT

2008-10-29 12:12:57

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

Ingo Molnar wrote:
> * Jens Axboe <[email protected]> wrote:
>
>
>> On Wed, Oct 29 2008, Ingo Molnar wrote:
>>
>>> * Ingo Molnar <[email protected]> wrote:
>>>
>>>
>>>> * Jeremy Fitzhardinge <[email protected]> wrote:
>>>>
>>>>
>>>>>> does it have any dependency on:
>>>>>>
>>>>>> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
>>>>>>
>>>>> No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will be
>>>>> useful for the Xen changes I need to make in the wake of Tomo's
>>>>> patch.
>>>>>
>>>> ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
>>>> Acked-by)
>>>>
>>> build failure, zapped it:
>>>
>>> fs/bio.c: In function '__bio_add_page':
>>> fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'
>>>
>> What patch(es) did you apply?
>>
>
> the one i replied to (2/2), and which one Jeremy said was independent
> of 1/2. Not so much it appears.
>

Sorry, miscommunication. I meant you should apply 1/2, and drop 2/2.

J
> Ingo
>

2008-10-29 13:04:07

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE


* Jeremy Fitzhardinge <[email protected]> wrote:

> Ingo Molnar wrote:
>> * Jens Axboe <[email protected]> wrote:
>>
>>
>>> On Wed, Oct 29 2008, Ingo Molnar wrote:
>>>
>>>> * Ingo Molnar <[email protected]> wrote:
>>>>
>>>>
>>>>> * Jeremy Fitzhardinge <[email protected]> wrote:
>>>>>
>>>>>
>>>>>>> does it have any dependency on:
>>>>>>>
>>>>>>> Subject: [PATCH 1/2] bio: define __BIOVEC_PHYS_MERGEABLE
>>>>>>>
>>>>>> No, they're independent. Defining __BIOVEC_PHYS_MERGEABLE will
>>>>>> be useful for the Xen changes I need to make in the wake of
>>>>>> Tomo's patch.
>>>>>>
>>>>> ok - applied to tip/x86/xen, thanks Jeremy! (also added Jens's
>>>>> Acked-by)
>>>>>
>>>> build failure, zapped it:
>>>>
>>>> fs/bio.c: In function '__bio_add_page':
>>>> fs/bio.c:443: error: implicit declaration of function '__BIOVEC_PHYS_MERGEABLE'
>>>>
>>> What patch(es) did you apply?
>>>
>>
>> the one i replied to (2/2), and which one Jeremy said was independent
>> of 1/2. Not so much it appears.
>>
>
> Sorry, miscommunication. I meant you should apply 1/2, and drop 2/2.

uhm, 1/2 changes bio.h. I'll let both patches alone for now ;-) Please
resend once the dependencies are sorted out.

Ingo

2008-10-29 13:07:17

by Jeremy Fitzhardinge

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

Ingo Molnar wrote:
> uhm, 1/2 changes bio.h. I'll let both patches alone for now ;-)

OK, 1/2 should be applied one way or another. It's not critical, but it
avoids duplicating code if/when an arch/io.h wants to override
BIOVEC_PHYS_MERGEABLE.

J

2008-10-29 13:15:28

by Jens Axboe

[permalink] [raw]
Subject: Re: [PATCH 2/2] x86: replace BIO_VMERGE_BOUNDARY with BIOVEC_PHYS_MERGEABLE

On Thu, Oct 30 2008, Jeremy Fitzhardinge wrote:
> Ingo Molnar wrote:
> >uhm, 1/2 changes bio.h. I'll let both patches alone for now ;-)
>
> OK, 1/2 should be applied one way or another. It's not critical, but it
> avoids duplicating code if/when an arch/io.h wants to override
> BIOVEC_PHYS_MERGEABLE.

I'll get it upstream.

--
Jens Axboe