ext4dev build failure on mips: "empty_zero_page" undefined

ext4dev build failure on mips: "empty_zero_page" undefined

[Martin Michlmayr]

I get the following build failure on MIPS Malta with 2.6.26-rc1:

  MODPOST 1516 modules
ERROR: "empty_zero_page" [fs/ext4/ext4dev.ko] undefined!

Any idea?  A missing export?
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Dmitri Vorobiev]

[-- Attachment #1: Type: text/plain, Size: 352 bytes --]

2008/5/12 Martin Michlmayr <tbm@cyrius.com>:
> I get the following build failure on MIPS Malta with 2.6.26-rc1:
>
>   MODPOST 1516 modules
>  ERROR: "empty_zero_page" [fs/ext4/ext4dev.ko] undefined!
>
>  Any idea?  A missing export?

Yep. The export is missing. Attached patch was build-tested for a
Malta config with ext4 enabled as a module.

Dmitri

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-MIPS-Export-empty_zero_page-as-a-GPL-symbol.patch --]
[-- Type: text/x-patch; name=0001-MIPS-Export-empty_zero_page-as-a-GPL-symbol.patch, Size: 1204 bytes --]

From cb55ed7d958cf4abb58dd1d6e46e09447b5694b0 Mon Sep 17 00:00:00 2001
From: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
Date: Mon, 12 May 2008 17:49:19 +0400
Subject: [PATCH 1/1] [MIPS] Export empty_zero_page as a GPL symbol

The empty_zero_page symbol is needed for the ext4 driver and
should therefore be exported. This fixes the following error
reported by Martin Michlmayr:

>>>>>>>

MODPOST 1516 modules
ERROR: "empty_zero_page" [fs/ext4/ext4dev.ko] undefined!

>>>>>>

Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
---
 arch/mips/mm/init.c |    5 ++++-
 1 files changed, 4 insertions(+), 1 deletions(-)

diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
index ecd562d..618a418 100644
--- a/arch/mips/mm/init.c
+++ b/arch/mips/mm/init.c
@@ -70,7 +70,10 @@ DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
  * any price.  Since page is never written to after the initialization we
  * don't have to care about aliases on other CPUs.
  */
-unsigned long empty_zero_page, zero_page_mask;
+unsigned long empty_zero_page;
+EXPORT_SYMBOL_GPL(empty_zero_page);
+
+unsigned long zero_page_mask;
 
 /*
  * Not static inline because used by IP27 special magic initialization code
-- 
1.5.3

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Theodore Tso]

On Mon, May 12, 2008 at 05:54:24PM +0400, Dmitri Vorobiev wrote:
> 
> Yep. The export is missing. Attached patch was build-tested for a
> Malta config with ext4 enabled as a module.

Thanks, Dmitri!  

What is the Linux-mips' team preference for feeding this patch to
Linus?  This technically isn't a regression, since it was broken in
2.6.25, but it would be nice to get this to Linus sooner rather than
later.  Should I push it with a batch of ext4 fixes, or do you want to
push it via the mips tree?  (Davem asked me to push the sparc export
via ext4, while the ppc arch, it went via the ppc tree.  So whichever
is your preference; I'm easy.  :-)

   	     	     		     	    - Ted


> From cb55ed7d958cf4abb58dd1d6e46e09447b5694b0 Mon Sep 17 00:00:00 2001
> From: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
> Date: Mon, 12 May 2008 17:49:19 +0400
> Subject: [PATCH 1/1] [MIPS] Export empty_zero_page as a GPL symbol
> 
> The empty_zero_page symbol is needed for the ext4 driver and
> should therefore be exported. This fixes the following error
> reported by Martin Michlmayr:
> 
> >>>>>>>
> 
> MODPOST 1516 modules
> ERROR: "empty_zero_page" [fs/ext4/ext4dev.ko] undefined!
> 
> >>>>>>
> 
> Signed-off-by: Dmitri Vorobiev <dmitri.vorobiev@gmail.com>
> ---
>  arch/mips/mm/init.c |    5 ++++-
>  1 files changed, 4 insertions(+), 1 deletions(-)
> 
> diff --git a/arch/mips/mm/init.c b/arch/mips/mm/init.c
> index ecd562d..618a418 100644
> --- a/arch/mips/mm/init.c
> +++ b/arch/mips/mm/init.c
> @@ -70,7 +70,10 @@ DEFINE_PER_CPU(struct mmu_gather, mmu_gathers);
>   * any price.  Since page is never written to after the initialization we
>   * don't have to care about aliases on other CPUs.
>   */
> -unsigned long empty_zero_page, zero_page_mask;
> +unsigned long empty_zero_page;
> +EXPORT_SYMBOL_GPL(empty_zero_page);
> +
> +unsigned long zero_page_mask;
>  
>  /*
>   * Not static inline because used by IP27 special magic initialization code
> -- 
> 1.5.3
> 

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Dmitri Vorobiev]

2008/5/12 Theodore Tso <tytso@mit.edu>:
> On Mon, May 12, 2008 at 05:54:24PM +0400, Dmitri Vorobiev wrote:
>  >
>  > Yep. The export is missing. Attached patch was build-tested for a
>  > Malta config with ext4 enabled as a module.
>
>  Thanks, Dmitri!
>
>  What is the Linux-mips' team preference for feeding this patch to
>  Linus?  This technically isn't a regression, since it was broken in
>  2.6.25, but it would be nice to get this to Linus sooner rather than
>  later.  Should I push it with a batch of ext4 fixes, or do you want to
>  push it via the mips tree?  (Davem asked me to push the sparc export
>  via ext4, while the ppc arch, it went via the ppc tree.  So whichever
>  is your preference; I'm easy.  :-)
>
>                                             - Ted

Hi Ted,

Normally I push my patches via the mips tree, and now I'm Cc:ing Ralf for that.

Hopefully Ralf will react quickly. :)

Dmitri

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Ralf Baechle]

On Mon, May 12, 2008 at 06:46:21PM +0400, Dmitri Vorobiev wrote:

> >  What is the Linux-mips' team preference for feeding this patch to
> >  Linus?  This technically isn't a regression, since it was broken in
> >  2.6.25, but it would be nice to get this to Linus sooner rather than
> >  later.  Should I push it with a batch of ext4 fixes, or do you want to
> >  push it via the mips tree?  (Davem asked me to push the sparc export
> >  via ext4, while the ppc arch, it went via the ppc tree.  So whichever
> >  is your preference; I'm easy.  :-)
> >
> >                                             - Ted
> 
> Hi Ted,
> 
> Normally I push my patches via the mips tree, and now I'm Cc:ing Ralf for that.
> 
> Hopefully Ralf will react quickly. :)

I prefer to do it myself so I can apply it at the same time to the MIPS
-stable branches.

I'm a little irriated that this thread seems to be only about
empty_zero_page but apparently not zero_page_mask?  empty_zero_page is
actualy an array of pages on MIPS and ZERO_PAGE() will pick the right one
for a particular user space mapping based on the virtual address but
ZERO_PAGE() also references zero_page_mask.  So I sense more brokenness
here.

  Ralf

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Ralf Baechle]

On Tue, May 13, 2008 at 05:50:29AM +0100, Ralf Baechle wrote:

> > Normally I push my patches via the mips tree, and now I'm Cc:ing Ralf for that.
> > 
> > Hopefully Ralf will react quickly. :)
> 
> I prefer to do it myself so I can apply it at the same time to the MIPS
> -stable branches.
> 
> I'm a little irriated that this thread seems to be only about
> empty_zero_page but apparently not zero_page_mask?  empty_zero_page is
> actualy an array of pages on MIPS and ZERO_PAGE() will pick the right one
> for a particular user space mapping based on the virtual address but
> ZERO_PAGE() also references zero_page_mask.  So I sense more brokenness
> here.

The ZERO_PAGE(0) call in ext4_ext_zeroout is the culprit.  Using a zero
argument allows the compiler to eleminate the reference to zero_page_mask.

Am I reading this right that ZERO_PAGE() is being used without any
mappings to userspace being involved?

  Ralf

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Theodore Tso]

On Tue, May 13, 2008 at 06:12:52AM +0100, Ralf Baechle wrote:
> The ZERO_PAGE(0) call in ext4_ext_zeroout is the culprit.  Using a zero
> argument allows the compiler to eleminate the reference to zero_page_mask.
> 
> Am I reading this right that ZERO_PAGE() is being used without any
> mappings to userspace being involved?

Correct.  Ext4 supports unitialized extents; this is useful in DVR's,
for example, where there is a desire to allocate contiguous blocks
for, say, a 60 minute TV show, without having to pay the cost of
zero'ing the blocks.  But in some cases where someone seeks a few
blocks ahead, and writes into the middle of the unitialized region,
instead of splitting the unitialized extent into 3 pieces, for short
regions we will simply zero out a few blocks and then write the
requested block.  

This is better in the case of binutils, for example, where it will
write out blocks in a random order using a few close-range seeks, and
it's more efficient to do this than to bloat out the extent tree only
to have to recombine extents later.

Anyway, yes, we just need to use the zero page without any user
mapping being involved.

						- Ted

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Martin Michlmayr]

* Ralf Baechle <ralf@linux-mips.org> [2008-05-13 05:50]:
> I prefer to do it myself so I can apply it at the same time to the MIPS
> -stable branches.
> 
> I'm a little irriated that this thread seems to be only about
> empty_zero_page but apparently not zero_page_mask?  empty_zero_page is
> actualy an array of pages on MIPS and ZERO_PAGE() will pick the right one
> for a particular user space mapping based on the virtual address but
> ZERO_PAGE() also references zero_page_mask.  So I sense more brokenness
> here.

Just as a reminder, this issue is still there (at least with rc4).
-- 
Martin Michlmayr
http://www.cyrius.com/

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Martin Michlmayr]

* Martin Michlmayr <tbm@cyrius.com> [2008-05-28 09:06]:
> * Ralf Baechle <ralf@linux-mips.org> [2008-05-13 05:50]:
> > I prefer to do it myself so I can apply it at the same time to the MIPS
> > -stable branches.
> > 
> > I'm a little irriated that this thread seems to be only about
> > empty_zero_page but apparently not zero_page_mask?  empty_zero_page is
> > actualy an array of pages on MIPS and ZERO_PAGE() will pick the right one
> > for a particular user space mapping based on the virtual address but
> > ZERO_PAGE() also references zero_page_mask.  So I sense more brokenness
> > here.
> 
> Just as a reminder, this issue is still there (at least with rc4).

Still present in rc5.

-- 
Martin Michlmayr
http://www.cyrius.com/

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Dmitri Vorobiev]

On Thu, 2008-06-05 at 13:11 +0200, Martin Michlmayr wrote:
> * Martin Michlmayr <tbm@cyrius.com> [2008-05-28 09:06]:
> > * Ralf Baechle <ralf@linux-mips.org> [2008-05-13 05:50]:
> > > I prefer to do it myself so I can apply it at the same time to the MIPS
> > > -stable branches.
> > > 
> > > I'm a little irriated that this thread seems to be only about
> > > empty_zero_page but apparently not zero_page_mask?  empty_zero_page is
> > > actualy an array of pages on MIPS and ZERO_PAGE() will pick the right one
> > > for a particular user space mapping based on the virtual address but
> > > ZERO_PAGE() also references zero_page_mask.  So I sense more brokenness
> > > here.
> > 
> > Just as a reminder, this issue is still there (at least with rc4).
> 
> Still present in rc5.

It looks like the discussion related to this issue has faded out. Ralf
seemed to have some objections to using ZERO_PAGE() outside of the
context of getting a user-mapped page, but I think that ext4 driver is
still doing that.

Ralf, will it be possible to use the patch I sent earlier as a temporary
solution? Just to make sure the kernel builds?

Thanks,
Dmitri
> 

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Theodore Tso]

On Thu, Jun 05, 2008 at 02:22:57PM +0300, Dmitri Vorobiev wrote:
> 
> It looks like the discussion related to this issue has faded out. Ralf
> seemed to have some objections to using ZERO_PAGE() outside of the
> context of getting a user-mapped page, but I think that ext4 driver is
> still doing that.
> 

I thought I had sent a reply indicating that yes, we are using
ZERO_PAGE outside of getting a user-mapped page, because this is
happening when we need to write zero's to directly to a filesystem
block.  This case arises when we have a file which contains
preallocated space that has not yet been initialized, and the user
program seeks into the middle of the unitialized extent range, and
writes into the middle of that space.

In some cases, it is more efficient to zero out a small range of
blocks on disk rather than splitting the extent in the middle.  We
could explicitly allocate a page, and zero it out, and use it to write
zeros from the ext4 filesystem, code, but that seems silly, given that
ZERO_PAGE exists and is available on all other architectures.

Cristoph Hellwig had complained about the use of ZERO_PAGE, but when I
gave him the above explanation, he agreed that this was indeed
probably the best way to do things.

If you really insist I suppose we could have a MIPS specific patch
where we allocate a 4k page and zero it, so we can use it from our
kernel code because you don't want to export and make available the
ZERO_PAGE that gets used by the rest of the kernel, but that seems
awfully silly, and would be a waste of 4k of memory.....  Someone from
MIPS land would have to test it, as well, as I dont think any of the
ext4 developers have access to a MIPS platform.

						- Ted

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Vorobiev Dmitri]

Theodore Tso wrote:
>
> If you really insist I suppose we could have a MIPS specific patch
> where we allocate a 4k page and zero it, so we can use it from our
> kernel code because you don't want to export and make available the
> ZERO_PAGE that gets used by the rest of the kernel, but that seems
> awfully silly, and would be a waste of 4k of memory.....  Someone from
> MIPS land would have to test it, as well, as I dont think any of the
> ext4 developers have access to a MIPS platform.

Ted, Ralf seems to be unwilling to accept the ZERO_PAGE() export. If you
send the MIPS-specific patch, I can do the testing for you as I have a
MIPS Malta board at my disposal.

Thanks,
Dmitri

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Thiemo Seufer]

Vorobiev Dmitri wrote:
> Theodore Tso wrote:
> >
> > If you really insist I suppose we could have a MIPS specific patch
> > where we allocate a 4k page and zero it, so we can use it from our
> > kernel code because you don't want to export and make available the
> > ZERO_PAGE that gets used by the rest of the kernel, but that seems
> > awfully silly, and would be a waste of 4k of memory.....  Someone from
> > MIPS land would have to test it, as well, as I dont think any of the
> > ext4 developers have access to a MIPS platform.
> 
> Ted, Ralf seems to be unwilling to accept the ZERO_PAGE() export. If you
> send the MIPS-specific patch, I can do the testing for you as I have a
> MIPS Malta board at my disposal.

AFAIU the problematic case are systems with R4000/R4400 SC/MC CPUs
since they use 8 zero pages of different color. Have a look at
arch/mips/mm/init.c:setup_zero_pages.


Thiemo

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Dmitri Vorobiev]

Thiemo Seufer wrote:
> Vorobiev Dmitri wrote:
>> Theodore Tso wrote:
>>> If you really insist I suppose we could have a MIPS specific patch
>>> where we allocate a 4k page and zero it, so we can use it from our
>>> kernel code because you don't want to export and make available the
>>> ZERO_PAGE that gets used by the rest of the kernel, but that seems
>>> awfully silly, and would be a waste of 4k of memory.....  Someone from
>>> MIPS land would have to test it, as well, as I dont think any of the
>>> ext4 developers have access to a MIPS platform.
>> Ted, Ralf seems to be unwilling to accept the ZERO_PAGE() export. If you
>> send the MIPS-specific patch, I can do the testing for you as I have a
>> MIPS Malta board at my disposal.
> 
> AFAIU the problematic case are systems with R4000/R4400 SC/MC CPUs
> since they use 8 zero pages of different color. Have a look at
> arch/mips/mm/init.c:setup_zero_pages.

OK, thanks for the info. However, I won't be able to tackle into the
issue during this and the next week as I'm having a business trip.
Therefore, if the issue is urgent, I'd be grateful if someone could take
over.

Thanks,
Dmitri

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Theodore Tso]

On Thu, Jun 05, 2008 at 10:51:52PM +0100, Thiemo Seufer wrote:
> > Ted, Ralf seems to be unwilling to accept the ZERO_PAGE() export. If you
> > send the MIPS-specific patch, I can do the testing for you as I have a
> > MIPS Malta board at my disposal.

Ralf sent me a private note saying he would take care of this, but
that he had been very busy with real-world life items.

> AFAIU the problematic case are systems with R4000/R4400 SC/MC CPUs
> since they use 8 zero pages of different color. Have a look at
> arch/mips/mm/init.c:setup_zero_pages.

Right.  So we're not actually going to ever write to the page, but we
are going to add the page to a bio and submit for writing.  I assume
that's not going to cause VECI/VCED exceptions, right?  If the act of
DMA'ing, or worse yet, for older devices/drivers which have to use PIO
to write out a block device is going to cause "thousand and thousands
of exceptions", it sounds like MIPS CPU designers need to be slapped
silly....

					- Ted

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Martin Michlmayr]

* Theodore Tso <tytso@mit.edu> [2008-05-12 10:34]:
> What is the Linux-mips' team preference for feeding this patch to
> Linus?  This technically isn't a regression, since it was broken in
> 2.6.25, but it would be nice to get this to Linus sooner rather than

Actually, it built just fine with 2.6.25.

-- 
Martin Michlmayr
http://www.cyrius.com/

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Dmitri Vorobiev]

2008/5/12 Martin Michlmayr <tbm@cyrius.com>:
> * Theodore Tso <tytso@mit.edu> [2008-05-12 10:34]:
>
> > What is the Linux-mips' team preference for feeding this patch to
>  > Linus?  This technically isn't a regression, since it was broken in
>  > 2.6.25, but it would be nice to get this to Linus sooner rather than
>
>  Actually, it built just fine with 2.6.25.

It was April 29th 2008 when commit
093a088b76352e0a6fdca84eb78b3aa65fbe6dd1 where the ext4_ext_zeroout()
routine was introduced (it is this routine that makes use of the
ZERO_PAGE macro expanding to an expression that contains a reference
to the empty_zero_page global symbol). After this commit, the patches
similar to the one found in this thread were made for several
architectures. Apparently, MIPS has been overlooked.

Dmitri

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Theodore Tso]

On Mon, May 12, 2008 at 07:14:23PM +0400, Dmitri Vorobiev wrote:
> 
> It was April 29th 2008 when commit
> 093a088b76352e0a6fdca84eb78b3aa65fbe6dd1 where the ext4_ext_zeroout()
> routine was introduced (it is this routine that makes use of the
> ZERO_PAGE macro expanding to an expression that contains a reference
> to the empty_zero_page global symbol). After this commit, the patches
> similar to the one found in this thread were made for several
> architectures. Apparently, MIPS has been overlooked.
> 

Yeah, I guess no one is currently doing test builds of linux-next on
the mips architecture.  It's likely Stephen Rothwell doesn't have
access to one.

(The patch itself has been around for a while, both in the -mm tree,
the ext4 patches, and in Linux-next.)

						- Ted

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Dmitri Vorobiev]

>
>  Yeah, I guess no one is currently doing test builds of linux-next on
>  the mips architecture.  It's likely Stephen Rothwell doesn't have
>  access to one.
>

I do that sometimes. Obviously, not frequently enough. Sorry.

If Stephen wants, I, in principle, could set up a MIPS toolchain for
him, similar to the one I use myself in my small test farm. Or assist
in doing that somehow. Stephen, what do you think of that?

Dmitri

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 1042 bytes --]

Hi Dimitri,

On Mon, 12 May 2008 23:37:46 +0400 "Dmitri Vorobiev" <dmitri.vorobiev@gmail.com> wrote:
>
> >  Yeah, I guess no one is currently doing test builds of linux-next on
> >  the mips architecture.  It's likely Stephen Rothwell doesn't have
> >  access to one.
> 
> I do that sometimes. Obviously, not frequently enough. Sorry.
> 
> If Stephen wants, I, in principle, could set up a MIPS toolchain for
> him, similar to the one I use myself in my small test farm. Or assist
> in doing that somehow. Stephen, what do you think of that?

That would be great.  However, let us try with crosstool first. We
haven't actually tried using crosstool to build a mips cross compiler.
For a start, do we need mips or mipsel?  (My tame minion is trying to
build both now.  :-))  Our host arch is powerpc64.

Of course, we have much more incentive to do mips builds if the mips tree
is in linux-next.  HINT! HINT! :-)

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Stephen Rothwell]

[-- Attachment #1: Type: text/plain, Size: 603 bytes --]

On Tue, 13 May 2008 10:55:49 +1000 Stephen Rothwell <sfr@canb.auug.org.au> wrote:
>
> That would be great.  However, let us try with crosstool first. We
> haven't actually tried using crosstool to build a mips cross compiler.
> For a start, do we need mips or mipsel?  (My tame minion is trying to
> build both now.  :-))  Our host arch is powerpc64.

Also, the only compiler version that crosstool seems to know about for
mips is 3.4.5.  Is this version ok for building mips kernels?

-- 
Cheers,
Stephen Rothwell                    sfr@canb.auug.org.au
http://www.canb.auug.org.au/~sfr/

[-- Attachment #2: Type: application/pgp-signature, Size: 189 bytes --]

Re: ext4dev build failure on mips: "empty_zero_page" undefined

[Ralf Baechle]

On Tue, May 13, 2008 at 11:42:44AM +1000, Stephen Rothwell wrote:

> Also, the only compiler version that crosstool seems to know about for
> mips is 3.4.5.  Is this version ok for building mips kernels?

Minimum is 3.2 for building a 32-bit kernel and 3.3 for a 64-bit kernel
but I really wouldn't recommend anything older than 3.4.

There are crosscompiler rpms on ftp.linux-mips.org in
/pub/linux/mips/crossdev for various host / target combinations.  They
come without libraries so just good enough for building kernels - but that's
the only purpose I build them for.  Also I don't have access to a powerpc
box so no power rpms.

  Ralf