All of lore.kernel.org
 help / color / mirror / Atom feed
* -tip: origin tree build failure
@ 2009-12-17  7:50 Ingo Molnar
  2009-12-17 21:53 ` Myron Stowe
                   ` (2 more replies)
  0 siblings, 3 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-12-17  7:50 UTC (permalink / raw)
  To: linux-kernel, Len Brown, Linus Torvalds; +Cc: Bjorn Helgaas, Myron Stowe

Today's -tip failed to build because commit 
9e368fa011d4e0aa050db348d69514900520e40b ("ipmi: add PNP discovery (ACPI 
namespace via PNPACPI)") from today's upstream kernel causes the following 
build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:

 drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
 drivers/char/ipmi/ipmi_si_intf.c:3208: error: (Each undeclared identifier is reported only once
 drivers/char/ipmi/ipmi_si_intf.c:3208: error: for each function it appears in.)
 drivers/char/ipmi/ipmi_si_intf.c:3334: error: 'ipmi_pnp_driver' undeclared (first use in this function)

The reason is that the ipmi_pnp_driver depends on ACPI facilities and is only 
made available under ACPI - while the registration and unregistration is made 
dependent on CONFIG_PNP:

 #ifdef CONFIG_PNP
 	pnp_register_driver(&ipmi_pnp_driver);
 #endif

The solution is to only register this driver under ACPI. (Also, the CONFIG_PNP 
dependency is not needed because pnp_register_driver() is stubbed out in the 
!CONFIG_PNP case.)

I've applied the patch below to tip:out-of-tree for now.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
index 679cd08..176f175 100644
--- a/drivers/char/ipmi/ipmi_si_intf.c
+++ b/drivers/char/ipmi/ipmi_si_intf.c
@@ -3204,7 +3204,7 @@ static __devinit int init_ipmi_si(void)
 #ifdef CONFIG_ACPI
 	spmi_find_bmc();
 #endif
-#ifdef CONFIG_PNP
+#ifdef CONFIG_ACPI
 	pnp_register_driver(&ipmi_pnp_driver);
 #endif
 
@@ -3330,7 +3330,7 @@ static __exit void cleanup_ipmi_si(void)
 #ifdef CONFIG_PCI
 	pci_unregister_driver(&ipmi_pci_driver);
 #endif
-#ifdef CONFIG_PNP
+#ifdef CONFIG_ACPI
 	pnp_unregister_driver(&ipmi_pnp_driver);
 #endif
 

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17  7:50 -tip: origin tree build failure Ingo Molnar
@ 2009-12-17 21:53 ` Myron Stowe
  2009-12-26 12:36 ` Geert Uytterhoeven
  2009-12-26 19:50 ` Len Brown
  2 siblings, 0 replies; 17+ messages in thread
From: Myron Stowe @ 2009-12-17 21:53 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Len Brown, Linus Torvalds, Bjorn Helgaas

On Thu, 2009-12-17 at 08:50 +0100, Ingo Molnar wrote:
> Today's -tip failed to build because commit 
> 9e368fa011d4e0aa050db348d69514900520e40b ("ipmi: add PNP discovery (ACPI 
> namespace via PNPACPI)") from today's upstream kernel causes the following 
> build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
> 
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: (Each undeclared identifier is reported only once
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: for each function it appears in.)
>  drivers/char/ipmi/ipmi_si_intf.c:3334: error: 'ipmi_pnp_driver' undeclared (first use in this function)
> 
> The reason is that the ipmi_pnp_driver depends on ACPI facilities and is only 
> made available under ACPI - while the registration and unregistration is made 
> dependent on CONFIG_PNP:
> 
>  #ifdef CONFIG_PNP
>  	pnp_register_driver(&ipmi_pnp_driver);
>  #endif
> 
> The solution is to only register this driver under ACPI. (Also, the CONFIG_PNP 
> dependency is not needed because pnp_register_driver() is stubbed out in the 
> !CONFIG_PNP case.)

Yes, sorry we missed this case.
> 
> I've applied the patch below to tip:out-of-tree for now.

Looks good to us.  Thanks!

Myron
> 
> Thanks,
> 
> 	Ingo
> 
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
> 
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 679cd08..176f175 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3204,7 +3204,7 @@ static __devinit int init_ipmi_si(void)
>  #ifdef CONFIG_ACPI
>  	spmi_find_bmc();
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>  	pnp_register_driver(&ipmi_pnp_driver);
>  #endif
>  
> @@ -3330,7 +3330,7 @@ static __exit void cleanup_ipmi_si(void)
>  #ifdef CONFIG_PCI
>  	pci_unregister_driver(&ipmi_pci_driver);
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>  	pnp_unregister_driver(&ipmi_pnp_driver);
>  #endif
>  
> 


-- 
Myron Stowe                             HP Open Source Linux Lab (OSLL)


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17  7:50 -tip: origin tree build failure Ingo Molnar
  2009-12-17 21:53 ` Myron Stowe
@ 2009-12-26 12:36 ` Geert Uytterhoeven
  2009-12-26 19:50 ` Len Brown
  2 siblings, 0 replies; 17+ messages in thread
From: Geert Uytterhoeven @ 2009-12-26 12:36 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Len Brown, Linus Torvalds, Bjorn Helgaas, Myron Stowe

On Thu, Dec 17, 2009 at 08:50, Ingo Molnar <mingo@elte.hu> wrote:
> Today's -tip failed to build because commit
> 9e368fa011d4e0aa050db348d69514900520e40b ("ipmi: add PNP discovery (ACPI
> namespace via PNPACPI)") from today's upstream kernel causes the following
> build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
>
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: (Each undeclared identifier is reported only once
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: for each function it appears in.)
>  drivers/char/ipmi/ipmi_si_intf.c:3334: error: 'ipmi_pnp_driver' undeclared (first use in this function)
>
> The reason is that the ipmi_pnp_driver depends on ACPI facilities and is only
> made available under ACPI - while the registration and unregistration is made
> dependent on CONFIG_PNP:
>
>  #ifdef CONFIG_PNP
>        pnp_register_driver(&ipmi_pnp_driver);
>  #endif
>
> The solution is to only register this driver under ACPI. (Also, the CONFIG_PNP
> dependency is not needed because pnp_register_driver() is stubbed out in the
> !CONFIG_PNP case.)
>
> I've applied the patch below to tip:out-of-tree for now.

Any chance we can see this fixed in mainline soon, so allmodconfig builds again
on non-ACPI platforms? Thx!

> Thanks,
>
>        Ingo
>
> Signed-off-by: Ingo Molnar <mingo@elte.hu>
>
> diff --git a/drivers/char/ipmi/ipmi_si_intf.c b/drivers/char/ipmi/ipmi_si_intf.c
> index 679cd08..176f175 100644
> --- a/drivers/char/ipmi/ipmi_si_intf.c
> +++ b/drivers/char/ipmi/ipmi_si_intf.c
> @@ -3204,7 +3204,7 @@ static __devinit int init_ipmi_si(void)
>  #ifdef CONFIG_ACPI
>        spmi_find_bmc();
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>        pnp_register_driver(&ipmi_pnp_driver);
>  #endif
>
> @@ -3330,7 +3330,7 @@ static __exit void cleanup_ipmi_si(void)
>  #ifdef CONFIG_PCI
>        pci_unregister_driver(&ipmi_pci_driver);
>  #endif
> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>        pnp_unregister_driver(&ipmi_pnp_driver);
>  #endif
>

Gr{oetje,eeting}s,

						Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org

In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
							    -- Linus Torvalds

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17  7:50 -tip: origin tree build failure Ingo Molnar
  2009-12-17 21:53 ` Myron Stowe
  2009-12-26 12:36 ` Geert Uytterhoeven
@ 2009-12-26 19:50 ` Len Brown
  2009-12-27  0:24   ` Henrique de Moraes Holschuh
  2009-12-28  8:26   ` Ingo Molnar
  2 siblings, 2 replies; 17+ messages in thread
From: Len Brown @ 2009-12-26 19:50 UTC (permalink / raw)
  To: Ingo Molnar; +Cc: linux-kernel, Linus Torvalds, Bjorn Helgaas, Myron Stowe

...
> build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
> 
>  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
...

> -#ifdef CONFIG_PNP
> +#ifdef CONFIG_ACPI
>  	pnp_register_driver(&ipmi_pnp_driver);
>  #endif
...

Hi Ingo,
As my tree inflicted this one and I don't see your patch upstream,
I'll include your fix with my "for -rc2" push.

thanks,
-Len Brown, Intel Open Source Technology Center


^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-26 19:50 ` Len Brown
@ 2009-12-27  0:24   ` Henrique de Moraes Holschuh
  2009-12-28  8:26   ` Ingo Molnar
  1 sibling, 0 replies; 17+ messages in thread
From: Henrique de Moraes Holschuh @ 2009-12-27  0:24 UTC (permalink / raw)
  To: Len Brown
  Cc: Ingo Molnar, linux-kernel, Linus Torvalds, Bjorn Helgaas, Myron Stowe

On Sat, 26 Dec 2009, Len Brown wrote:
> As my tree inflicted this one and I don't see your patch upstream,
> I'll include your fix with my "for -rc2" push.

I will be sending you an alternate patch in a few minutes, anyway.  If
you already sent the pull request, just let me know as that means I need
to adjust my tree.

Instead of depending on ALSA, I made the entire feature that depends on
ALSA optional.

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-26 19:50 ` Len Brown
  2009-12-27  0:24   ` Henrique de Moraes Holschuh
@ 2009-12-28  8:26   ` Ingo Molnar
  1 sibling, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-12-28  8:26 UTC (permalink / raw)
  To: Len Brown; +Cc: linux-kernel, Linus Torvalds, Bjorn Helgaas, Myron Stowe


* Len Brown <lenb@kernel.org> wrote:

> ...
> > build failure on x86, for CONFIG_ACPI=n && CONFIG_IPMI_SI=y:
> > 
> >  drivers/char/ipmi/ipmi_si_intf.c:3208: error: 'ipmi_pnp_driver' undeclared (first use in this function)
> ...
> 
> > -#ifdef CONFIG_PNP
> > +#ifdef CONFIG_ACPI
> >  	pnp_register_driver(&ipmi_pnp_driver);
> >  #endif
> ...
> 
> Hi Ingo,
> As my tree inflicted this one and I don't see your patch upstream,
> I'll include your fix with my "for -rc2" push.

Thanks!

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2010-10-28 21:39         ` -tip: origin tree build failure Junio C Hamano
@ 2010-10-28 21:50             ` Linus Torvalds
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Torvalds @ 2010-10-28 21:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ingo Molnar, Theodore Ts'o, linux-ext4, linux-kernel

On Thu, Oct 28, 2010 at 2:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> By the way, I noticed that you started sending patches as attachments
> lately.  What made you change your mind?

Nothing. I still hate them. But the tools I use (web interface to
gmail) are broken in this respect. There's no way to include a file,
or specify that an attachement should be inlined. And don't tell me
about IMAP - if I wanted to use IMAP, I'd be living in a padded cell.

I have a deep love/hate relationship with gmail. Many things make it
wonderful, and I'm not regretting the switch (which was initially just
a trial while traveling).

But it has two issues that I absolutely detest:
 (a) the idiotic inability to inline attachements and
 (b) the android gmail app is a total piece of shit and cannot even do
simple text messages. Crazy.
(there are other small annoyances, but they are smallish in comparison
to the above big honking bugs)

Does anybody know anybody who works on the google mail clients and
could raise these as bugs inside google?

                             Linus

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
@ 2010-10-28 21:50             ` Linus Torvalds
  0 siblings, 0 replies; 17+ messages in thread
From: Linus Torvalds @ 2010-10-28 21:50 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: Ingo Molnar, Theodore Ts'o, linux-ext4, linux-kernel

On Thu, Oct 28, 2010 at 2:39 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> By the way, I noticed that you started sending patches as attachments
> lately.  What made you change your mind?

Nothing. I still hate them. But the tools I use (web interface to
gmail) are broken in this respect. There's no way to include a file,
or specify that an attachement should be inlined. And don't tell me
about IMAP - if I wanted to use IMAP, I'd be living in a padded cell.

I have a deep love/hate relationship with gmail. Many things make it
wonderful, and I'm not regretting the switch (which was initially just
a trial while traveling).

But it has two issues that I absolutely detest:
 (a) the idiotic inability to inline attachements and
 (b) the android gmail app is a total piece of shit and cannot even do
simple text messages. Crazy.
(there are other small annoyances, but they are smallish in comparison
to the above big honking bugs)

Does anybody know anybody who works on the google mail clients and
could raise these as bugs inside google?

                             Linus
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2010-10-28 17:00       ` Linus Torvalds
@ 2010-10-28 21:39         ` Junio C Hamano
  2010-10-28 21:50             ` Linus Torvalds
  0 siblings, 1 reply; 17+ messages in thread
From: Junio C Hamano @ 2010-10-28 21:39 UTC (permalink / raw)
  To: Linus Torvalds; +Cc: Ingo Molnar, Theodore Ts'o, linux-ext4, linux-kernel

Linus Torvalds <torvalds@linux-foundation.org> writes:

> Yes. Except for the kernel the default git commit abbreviation is
> borderline too short. Seven hex-chars can easily alias with a few more
> pulls from me: git will not give aliases at the time it gives a
> shorthand, but a month or two later the abbreviated commit may no
> longer be unique.
>
> So I suggest using --abbrev=12 or similar.

Would a new configuration to specify how many more letters to ensure the
uniqueness at the time of generation make sense?

By the way, I noticed that you started sending patches as attachments
lately.  What made you change your mind?

 Documentation/config.txt |    9 +++++++++
 cache.h                  |    1 +
 config.c                 |    7 +++++++
 environment.c            |    1 +
 sha1_name.c              |    4 +++-
 5 files changed, 21 insertions(+), 1 deletions(-)

diff --git a/Documentation/config.txt b/Documentation/config.txt
index 538ebb5..6994338 100644
--- a/Documentation/config.txt
+++ b/Documentation/config.txt
@@ -374,6 +374,15 @@ core.warnAmbiguousRefs::
 	If true, git will warn you if the ref name you passed it is ambiguous
 	and might match multiple refs in the .git/refs/ tree. True by default.
 
+core.abbrevguard::
+	Even though git makes sure that it uses enough hexdigits to show
+	an abbreviated object name unambiguously, as more objects are
+	added to the repository over time, a short name that used to be
+	unique will stop being unique.  Git uses this many extra hexdigits
+	that are more than necessary to make the object name currently
+	unique, in the hope that its output will stay unique a bit longer.
+	Defaults to 0.
+
 core.compression::
 	An integer -1..9, indicating a default compression level.
 	-1 is the zlib default. 0 means no compression,
diff --git a/cache.h b/cache.h
index 33decd9..931fb59 100644
--- a/cache.h
+++ b/cache.h
@@ -545,6 +545,7 @@ extern int assume_unchanged;
 extern int prefer_symlink_refs;
 extern int log_all_ref_updates;
 extern int warn_ambiguous_refs;
+extern int unique_abbrev_extra_length;
 extern int shared_repository;
 extern const char *apply_default_whitespace;
 extern const char *apply_default_ignorewhitespace;
diff --git a/config.c b/config.c
index 4b0a820..1aa72c2 100644
--- a/config.c
+++ b/config.c
@@ -489,6 +489,13 @@ static int git_default_core_config(const char *var, const char *value)
 		return 0;
 	}
 
+	if (!strcmp(var, "core.abbrevguard")) {
+		unique_abbrev_extra_length = git_config_int(var, value);
+		if (unique_abbrev_extra_length < 0)
+			unique_abbrev_extra_length = 0;
+		return 0;
+	}
+
 	if (!strcmp(var, "core.bare")) {
 		is_bare_repository_cfg = git_config_bool(var, value);
 		return 0;
diff --git a/environment.c b/environment.c
index de5581f..92e16b1 100644
--- a/environment.c
+++ b/environment.c
@@ -21,6 +21,7 @@ int prefer_symlink_refs;
 int is_bare_repository_cfg = -1; /* unspecified */
 int log_all_ref_updates = -1; /* unspecified */
 int warn_ambiguous_refs = 1;
+int unique_abbrev_extra_length;
 int repository_format_version;
 const char *git_commit_encoding;
 const char *git_log_output_encoding;
diff --git a/sha1_name.c b/sha1_name.c
index 484081d..4a226ad 100644
--- a/sha1_name.c
+++ b/sha1_name.c
@@ -206,7 +206,9 @@ const char *find_unique_abbrev(const unsigned char *sha1, int len)
 		if (exists
 		    ? !status
 		    : status == SHORT_NAME_NOT_FOUND) {
-			hex[len] = 0;
+			int cut_at = len + unique_abbrev_extra_length;
+			cut_at = (cut_at < 40) ? cut_at : 40;
+			hex[cut_at] = 0;
 			return hex;
 		}
 		len++;

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-18 11:23     ` Ingo Molnar
@ 2009-12-18 11:45       ` Andi Kleen
  0 siblings, 0 replies; 17+ messages in thread
From: Andi Kleen @ 2009-12-18 11:45 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: Andi Kleen, linux-kernel, Linus Torvalds, Wu Fengguang,
	Andi Kleen, Peter Zijlstra, Andrew Morton, Matt Mackall,
	Nick Piggin, Christoph Lameter, Fr??d??ric Weisbecker,
	Steven Rostedt, Thomas Gleixner

> No, your patch is not the correct fix. As i said, PROC_PAGE_MONITOR provides 
> the facility and mm/memory-failure.c uses that facility - and your patch does 
> not solve that fundamental dependency issue, it just fudges around the 
> dependencies.

Thanks for the report.

The filter is really part of the injector, even if it happens to be 
in the other file. I first resisted adding an ifdef to it, but it seems
we need it. I think I forgot to set to set CONFIG_EMBEDDED earlier,
that is what made the earlier build test pass even when it shouldn't have.

This updated patch should fix it. I tried to build test all applicable
combinations, if anything is still missing please let me know.

-Andi

---

HWPOISON: Add PROC_FS dependency to hwpoison injector v2

The injector filter requires stable_page_flags() which is supplied
by procfs. So make it dependent on that.

Also add ifdefs around the filter code in memory-failure.c so that
when the filter is disabled due to missing dependencies the whole
code still builds. 

Reported-by: Ingo Molnar 
Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 mm/Kconfig          |    2 +-
 mm/memory-failure.c |    9 +++++++++
 2 files changed, 10 insertions(+), 1 deletion(-)

Index: linux/mm/Kconfig
===================================================================
--- linux.orig/mm/Kconfig
+++ linux/mm/Kconfig
@@ -252,7 +252,7 @@ config MEMORY_FAILURE
 
 config HWPOISON_INJECT
 	tristate "HWPoison pages injector"
-	depends on MEMORY_FAILURE && DEBUG_KERNEL
+	depends on MEMORY_FAILURE && DEBUG_KERNEL && PROC_FS
 	select PROC_PAGE_MONITOR
 
 config NOMMU_INITIAL_TRIM_EXCESS
Index: linux/mm/memory-failure.c
===================================================================
--- linux.orig/mm/memory-failure.c
+++ linux/mm/memory-failure.c
@@ -52,6 +52,8 @@ int sysctl_memory_failure_recovery __rea
 
 atomic_long_t mce_bad_pages __read_mostly = ATOMIC_LONG_INIT(0);
 
+#if defined(CONFIG_HWPOISON_INJECT) || defined(CONFIG_HWPOISON_INJECT_MODULE)
+
 u32 hwpoison_filter_enable = 0;
 u32 hwpoison_filter_dev_major = ~0U;
 u32 hwpoison_filter_dev_minor = ~0U;
@@ -164,6 +166,13 @@ int hwpoison_filter(struct page *p)
 
 	return 0;
 }
+#else
+int hwpoison_filter(struct page *p)
+{
+	return 0;
+}
+#endif
+
 EXPORT_SYMBOL_GPL(hwpoison_filter);
 
 /*

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17 12:55   ` Andi Kleen
@ 2009-12-18 11:23     ` Ingo Molnar
  2009-12-18 11:45       ` Andi Kleen
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2009-12-18 11:23 UTC (permalink / raw)
  To: Andi Kleen
  Cc: linux-kernel, Linus Torvalds, Wu Fengguang, Andi Kleen,
	Peter Zijlstra, Andrew Morton, Matt Mackall, Nick Piggin,
	Christoph Lameter, Fr??d??ric Weisbecker, Steven Rostedt,
	Thomas Gleixner


* Andi Kleen <andi@firstfloor.org> wrote:

> Ingo Molnar <mingo@elte.hu> writes:
> 
> > * Ingo Molnar <mingo@elte.hu> wrote:
> >
> >>  config MEMORY_FAILURE
> >>  	depends on MMU
> >>  	depends on ARCH_SUPPORTS_MEMORY_FAILURE
> >> +	select PROC_PAGE_MONITOR
> >>  	bool "Enable recovery from hardware memory errors"
> >
> > It also needs to depend on PROC_FS in that case - as per the updated patch 
> > below.
> 
> Thanks for the report.
> 
> MEMORY_FAILURE itself doesn't depend on the page flags, just the injector 
> which has a separate config option. It already has a select 
> PROC_PAGE_MONITOR, but the proc dependency is indeed missing.
> 
> I think the correct fix is the appended patch.

No, your patch is not the correct fix. As i said, PROC_PAGE_MONITOR provides 
the facility and mm/memory-failure.c uses that facility - and your patch does 
not solve that fundamental dependency issue, it just fudges around the 
dependencies.

So your patch can still produce the same kind of build failure (with other 
Kconfig variations):

 mm/built-in.o: In function `hwpoison_filter':
 (.text+0x430ba): undefined reference to `stable_page_flags'

This should be cleaned up for real - there's no reason why stable_page_flags() 
(which is an MM facility) should live in fs/proc/. That's the root of the 
problem.

Also a few more (very small) nits:

> HWPOISON: Add PROC_FS dependency to hwpoison injector

It would be really useful if hwpoison commit titles were in lower case, like 
most other commits in the kernel do? Upper-case is used in special cases - IMO 
this case does not qualify.

> The injector filter requires stable_page_flags() which is supplied by 
> procfs. So make it dependent on that.
> 
> Reported by Ingo Molnar

FYI, we have a Reported-by tag that can be (and should be) used in such cases.

Thanks,

	Ingo

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17 12:23 ` Ingo Molnar
@ 2009-12-17 12:55   ` Andi Kleen
  2009-12-18 11:23     ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Andi Kleen @ 2009-12-17 12:55 UTC (permalink / raw)
  To: Ingo Molnar
  Cc: linux-kernel, Linus Torvalds, Wu Fengguang, Andi Kleen,
	Peter Zijlstra, Andrew Morton, Matt Mackall, Nick Piggin,
	Christoph Lameter, Fr??d??ric Weisbecker, Steven Rostedt,
	Thomas Gleixner

Ingo Molnar <mingo@elte.hu> writes:

> * Ingo Molnar <mingo@elte.hu> wrote:
>
>>  config MEMORY_FAILURE
>>  	depends on MMU
>>  	depends on ARCH_SUPPORTS_MEMORY_FAILURE
>> +	select PROC_PAGE_MONITOR
>>  	bool "Enable recovery from hardware memory errors"
>
> It also needs to depend on PROC_FS in that case - as per the updated patch 
> below.

Thanks for the report.

MEMORY_FAILURE itself doesn't depend on the page flags, just the
injector which has a separate config option. It already has a select
PROC_PAGE_MONITOR, but the proc dependency is indeed missing.

I think the correct fix is the appended patch.

-Andi

---

HWPOISON: Add PROC_FS dependency to hwpoison injector

The injector filter requires stable_page_flags() which is supplied
by procfs. So make it dependent on that.

Reported by Ingo Molnar

Signed-off-by: Andi Kleen <ak@linux.intel.com>

---
 mm/Kconfig |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Index: linux/mm/Kconfig
===================================================================
--- linux.orig/mm/Kconfig
+++ linux/mm/Kconfig
@@ -252,7 +252,7 @@ config MEMORY_FAILURE
 
 config HWPOISON_INJECT
 	tristate "HWPoison pages injector"
-	depends on MEMORY_FAILURE && DEBUG_KERNEL
+	depends on MEMORY_FAILURE && DEBUG_KERNEL && PROC_FS
 	select PROC_PAGE_MONITOR
 
 config NOMMU_INITIAL_TRIM_EXCESS

^ permalink raw reply	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17  9:40 Ingo Molnar
@ 2009-12-17 12:23 ` Ingo Molnar
  2009-12-17 12:55   ` Andi Kleen
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2009-12-17 12:23 UTC (permalink / raw)
  To: linux-kernel, Linus Torvalds, Wu Fengguang, Andi Kleen
  Cc: Peter Zijlstra, Andrew Morton, Matt Mackall, Nick Piggin,
	Christoph Lameter, Fr??d??ric Weisbecker, Steven Rostedt,
	Thomas Gleixner


* Ingo Molnar <mingo@elte.hu> wrote:

>  config MEMORY_FAILURE
>  	depends on MMU
>  	depends on ARCH_SUPPORTS_MEMORY_FAILURE
> +	select PROC_PAGE_MONITOR
>  	bool "Enable recovery from hardware memory errors"

It also needs to depend on PROC_FS in that case - as per the updated patch 
below.

	Ingo

----------------->
>From 896358c0554c56398d40191dda24da8c4b8029c4 Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 17 Dec 2009 10:40:41 +0100
Subject: [PATCH] hwpoison: fix build failure

Today's -tip fails to build due to upstream commit
1a9b5b7fe0c5dad8a635288882d36785dea742f9 ("mm: export stable
page flags")  (authored and merged yesterday) causing the
following build failure on x86  when CONFIG_PROC_PAGE_MONITOR is
disabled:

 mm/built-in.o: In function `hwpoison_filter':
  (.text+0x39fbf): undefined reference to `stable_page_flags'

The bug is that the stable_page_flags() API is only available
under  CONFIG_PROC_PAGE_MONITOR, but utilized in
mm/memory-failure.c unconditionally.

I've applied the patch below to -tip for now, which expresses
this dependency  in the Kconfig. (Eventually a cleaner solution
would be to factor such ABI  details out of procfs, they dont
belong there.)

Cc: Peter Zijlstra <a.p.zijlstra@chello.nl>
Cc: Matt Mackall <mpm@selenic.com>
Cc: Nick Piggin <npiggin@suse.de>
Cc: Christoph Lameter <cl@linux-foundation.org>
Cc: Fr??d??ric Weisbecker <fweisbec@gmail.com>
Cc: Steven Rostedt <rostedt@goodmis.org>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Cc: Wu Fengguang <fengguang.wu@intel.com>
Cc: Andi Kleen <ak@linux.intel.com>
LKML-Reference: <20091217094041.GA24708@elte.hu>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 mm/Kconfig |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/mm/Kconfig b/mm/Kconfig
index 43ea8c3..021bc12 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -243,6 +243,8 @@ config ARCH_SUPPORTS_MEMORY_FAILURE
 config MEMORY_FAILURE
 	depends on MMU
 	depends on ARCH_SUPPORTS_MEMORY_FAILURE
+	depends on PROC_FS
+	select PROC_PAGE_MONITOR
 	bool "Enable recovery from hardware memory errors"
 	help
 	  Enables code to recover from some memory failures on systems

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17  6:17 Ingo Molnar
  2009-12-17 10:04 ` Henrique de Moraes Holschuh
@ 2009-12-17 12:16 ` Ingo Molnar
  1 sibling, 0 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-12-17 12:16 UTC (permalink / raw)
  To: linux-kernel, Len Brown, Linus Torvalds
  Cc: Henrique de Moraes Holschuh, Lorne Applebaum, Matthew Garrett


* Ingo Molnar <mingo@elte.hu> wrote:

> Add a Kconfig dependency on SOUND to remedy this.

ok, that should have been CONFIG_SND. Updated patch below.

	Ingo

------------------->
>From f9bf1de0fcb6dddf0145d472b13a21ab8e7723cd Mon Sep 17 00:00:00 2001
From: Ingo Molnar <mingo@elte.hu>
Date: Thu, 17 Dec 2009 07:08:12 +0100
Subject: [PATCH] acpi: Build CONFIG_SOUND=m build failure in CONFIG_THINKPAD_ACPI

-tip testing found that commit 0d204c34e85d1d63e5fdd3e3192747daf0ee7ec1
("thinkpad-acpi: basic ALSA mixer support (v2)") from today's upstream
kernel causes the following build failure on x86, for
CONFIG_THINKPAD_ACPI=y && CONFIG_SOUND=m:

  drivers/built-in.o: In function `volume_alsa_notify_change':
  thinkpad_acpi.c:(.text+0x2d5c2d): undefined reference to `snd_ctl_notify'
  thinkpad_acpi.c:(.text+0x2d5c47): undefined reference to `snd_ctl_notify'

This build fails because thinkpad_acpi.c uses sound facilities
unconditionally, without expressing its dependency on the sound
subsystem.

Add a Kconfig dependency on SOUND to remedy this.

Cc: Len Brown <len.brown@intel.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Lorne Applebaum <lorne.applebaum@gmail.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/platform/x86/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f8bec62..dbd4ef1 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -216,6 +216,7 @@ config THINKPAD_ACPI
 	depends on ACPI
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
+	depends on SND
 	select BACKLIGHT_LCD_SUPPORT
 	select BACKLIGHT_CLASS_DEVICE
 	select HWMON

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* Re: -tip: origin tree build failure
  2009-12-17  6:17 Ingo Molnar
@ 2009-12-17 10:04 ` Henrique de Moraes Holschuh
  2009-12-17 12:16 ` Ingo Molnar
  1 sibling, 0 replies; 17+ messages in thread
From: Henrique de Moraes Holschuh @ 2009-12-17 10:04 UTC (permalink / raw)
  To: Ingo Molnar, linux-kernel, Len Brown, Linus Torvalds
  Cc: Lorne Applebaum, Matthew Garrett

Thank you!

Acked-by: Henrique de Moraes Holschuh <hmh@hmh.eng.br>

On Thu, 17 Dec 2009 07:17 +0100, "Ingo Molnar" <mingo@elte.hu> wrote:
> Today's -tip failed to build because commit 
> 0d204c34e85d1d63e5fdd3e3192747daf0ee7ec1 ("thinkpad-acpi: basic ALSA
> mixer 
> support (v2)") from today's upstream kernel causes the following build
> failure 
> on x86, for CONFIG_THINKPAD_ACPI=y && CONFIG_SOUND=m:
...

>  	depends on RFKILL || RFKILL = n
> +       depends on SOUND
>  	select BACKLIGHT_LCD_SUPPORT
>  	select BACKLIGHT_CLASS_DEVICE
>  	select HWMON

-- 
  "One disk to rule them all, One disk to find them. One disk to bring
  them all and in the darkness grind them. In the Land of Redmond
  where the shadows lie." -- The Silicon Valley Tarot
  Henrique Holschuh


^ permalink raw reply	[flat|nested] 17+ messages in thread

* -tip: origin tree build failure
@ 2009-12-17  9:40 Ingo Molnar
  2009-12-17 12:23 ` Ingo Molnar
  0 siblings, 1 reply; 17+ messages in thread
From: Ingo Molnar @ 2009-12-17  9:40 UTC (permalink / raw)
  To: linux-kernel, Linus Torvalds, Wu Fengguang, Andi Kleen
  Cc: Peter Zijlstra, Andrew Morton, Matt Mackall, Nick Piggin,
	Christoph Lameter, Fr??d??ric Weisbecker, Steven Rostedt,
	Thomas Gleixner

Today's -tip fails to build due to upstream commit 
1a9b5b7fe0c5dad8a635288882d36785dea742f9 ("mm: export stable page flags") 
(authored and merged yesterday) causing the following build failure on x86 
when CONFIG_PROC_PAGE_MONITOR is disabled:

 mm/built-in.o: In function `hwpoison_filter':
  (.text+0x39fbf): undefined reference to `stable_page_flags'

The bug is that the stable_page_flags() API is only available under 
CONFIG_PROC_PAGE_MONITOR, but utilized in mm/memory-failure.c unconditionally.

I've applied the patch below to -tip for now, which expresses this dependency 
in the Kconfig. (Eventually a cleaner solution would be to factor such ABI 
details out of procfs, they dont belong there.)

A technical critique: as i pointed it out in the past on lkml, this whole 
ad-hoc exporting of MM details via /proc is ill advised. It's a limiting 
interface for instrumentation (we could do so much more and could integrate it 
so much better with a similar amount of intrusion), and it also creates 
needless ABI dependencies with user-space tooling.

This should be prototyped in debugfs and sufficiently integrated with other 
instrumentation frameworks in the kernel. I made a few suggestions and we even 
wrote patches in the past to help that out:

 3383e37: tracing, page-allocator: Add a postprocessing script for page-allocator-related ftrace events
 c33b359: tracing, page-allocator: Add trace event for page traffic related to the buddy lists
 0d524fb: tracing, mm: Add trace events for anti-fragmentation falling back to other migratetypes
 b9a2817: tracing, page-allocator: Add trace events for page allocation and page freeing
 eb46710: tracing/mm: rename 'trigger' file to 'dump_range'
 1487a7a: tracing/mm: fix mapcount trace record field
 dcac8cd: tracing/mm: add page frame snapshot trace

Unfortunately nothing of that was pursued from the MM and hwpoison side and 
now this inferior version goes upstream, and as two separate ABIs straight 
away. (the /proc ABI and the memory-failure filter ABI) Not very well designed 
and not very helpful IMO, and this will come back to haunt us in the future.

We know how to do this correctly, why not embrace, utilize and expand those 
instrumentation facilities?

Also, a workflow observation - i tried to figure out the history of the bug 
but the Git timestamps show this weirdness:

 commit 1a9b5b7fe0c5dad8a635288882d36785dea742f9
 Author:     Wu Fengguang <fengguang.wu@intel.com>
 AuthorDate: Wed Dec 16 12:19:59 2009 +0100
 Commit:     Andi Kleen <ak@linux.intel.com>
 CommitDate: Wed Dec 16 12:19:59 2009 +0100

that's precisely the same author and commit date, but author and committer are 
different - how is that possible? No combination of git-am, git-rebase and git 
commit --amend can achieve this kind of timestamp.

I checked the other 'HWPOISON' commits as well that were merged yesterday - 
they have a similarly damaged history.

Thanks,

	Ingo

Signed-off-by: Ingo Molnar <mingo@elte.hu>

diff --git a/mm/Kconfig b/mm/Kconfig
index 43ea8c3..d7bd560 100644
--- a/mm/Kconfig
+++ b/mm/Kconfig
@@ -243,6 +243,7 @@ config ARCH_SUPPORTS_MEMORY_FAILURE
 config MEMORY_FAILURE
 	depends on MMU
 	depends on ARCH_SUPPORTS_MEMORY_FAILURE
+	select PROC_PAGE_MONITOR
 	bool "Enable recovery from hardware memory errors"
 	help
 	  Enables code to recover from some memory failures on systems

^ permalink raw reply related	[flat|nested] 17+ messages in thread

* -tip: origin tree build failure
@ 2009-12-17  6:17 Ingo Molnar
  2009-12-17 10:04 ` Henrique de Moraes Holschuh
  2009-12-17 12:16 ` Ingo Molnar
  0 siblings, 2 replies; 17+ messages in thread
From: Ingo Molnar @ 2009-12-17  6:17 UTC (permalink / raw)
  To: linux-kernel, Len Brown, Linus Torvalds
  Cc: Henrique de Moraes Holschuh, Lorne Applebaum, Matthew Garrett


Today's -tip failed to build because commit 
0d204c34e85d1d63e5fdd3e3192747daf0ee7ec1 ("thinkpad-acpi: basic ALSA mixer 
support (v2)") from today's upstream kernel causes the following build failure 
on x86, for CONFIG_THINKPAD_ACPI=y && CONFIG_SOUND=m:

  drivers/built-in.o: In function `volume_alsa_notify_change':
  thinkpad_acpi.c:(.text+0x2d5c2d): undefined reference to `snd_ctl_notify'
  thinkpad_acpi.c:(.text+0x2d5c47): undefined reference to `snd_ctl_notify'

I've applied the patch below to tip:out-of-tree.

Thanks,

	Ingo

----------------->
acpi: Build CONFIG_SOUND=m build failure in CONFIG_THINKPAD_ACPI=y

Today's -tip failed to build because thinkpad_acpi.c uses sound facilities 
unconditionally, without expressing its dependency on the sound subsystem:

  drivers/built-in.o: In function `volume_alsa_notify_change':
  thinkpad_acpi.c:(.text+0x2d5c2d): undefined reference to `snd_ctl_notify'
  thinkpad_acpi.c:(.text+0x2d5c47): undefined reference to `snd_ctl_notify'

Add a Kconfig dependency on SOUND to remedy this.

Cc: Len Brown <len.brown@intel.com>
Cc: Henrique de Moraes Holschuh <hmh@hmh.eng.br>
Cc: Lorne Applebaum <lorne.applebaum@gmail.com>
Cc: Matthew Garrett <mjg@redhat.com>
Cc: Linus Torvalds <torvalds@linux-foundation.org>
Signed-off-by: Ingo Molnar <mingo@elte.hu>
---
 drivers/platform/x86/Kconfig |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/drivers/platform/x86/Kconfig b/drivers/platform/x86/Kconfig
index f8bec62..60083e9 100644
--- a/drivers/platform/x86/Kconfig
+++ b/drivers/platform/x86/Kconfig
@@ -216,6 +216,7 @@ config THINKPAD_ACPI
 	depends on ACPI
 	depends on INPUT
 	depends on RFKILL || RFKILL = n
+	depends on SOUND
 	select BACKLIGHT_LCD_SUPPORT
 	select BACKLIGHT_CLASS_DEVICE
 	select HWMON

^ permalink raw reply related	[flat|nested] 17+ messages in thread

end of thread, other threads:[~2010-10-28 21:55 UTC | newest]

Thread overview: 17+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2009-12-17  7:50 -tip: origin tree build failure Ingo Molnar
2009-12-17 21:53 ` Myron Stowe
2009-12-26 12:36 ` Geert Uytterhoeven
2009-12-26 19:50 ` Len Brown
2009-12-27  0:24   ` Henrique de Moraes Holschuh
2009-12-28  8:26   ` Ingo Molnar
  -- strict thread matches above, loose matches on Subject: below --
2010-10-28  4:52 [GIT PULL] ext4 update for 2.6.37 Theodore Ts'o
2010-10-28  7:56 ` -tip: origin tree build failure (was: [GIT PULL] ext4 update for 2.6.37) Ingo Molnar
2010-10-28 16:30   ` Linus Torvalds
2010-10-28 16:38     ` Ingo Molnar
2010-10-28 17:00       ` Linus Torvalds
2010-10-28 21:39         ` -tip: origin tree build failure Junio C Hamano
2010-10-28 21:50           ` Linus Torvalds
2010-10-28 21:50             ` Linus Torvalds
2009-12-17  9:40 Ingo Molnar
2009-12-17 12:23 ` Ingo Molnar
2009-12-17 12:55   ` Andi Kleen
2009-12-18 11:23     ` Ingo Molnar
2009-12-18 11:45       ` Andi Kleen
2009-12-17  6:17 Ingo Molnar
2009-12-17 10:04 ` Henrique de Moraes Holschuh
2009-12-17 12:16 ` Ingo Molnar

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.