All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] Missing include in swap.h for some architectures
@ 2003-10-20  5:17 Noah J. Misch
  2003-10-20  9:25 ` Andrew Morton
  0 siblings, 1 reply; 3+ messages in thread
From: Noah J. Misch @ 2003-10-20  5:17 UTC (permalink / raw)
  To: linux-kernel

Greetings All,

This flaw broke kernel builds configured with SWAP=n on some architectures,
particularly alpha and ia64.  Since i386 managed to include pagemap.h
indirectly, this did not crop up there.  See log for details.

This applies to linux-2.5 BK as of 0400 UTC 10/20/2003 and passes all kinds of
ridiculous compile tests, probably including (allyesconfig - broken stuff) on
sparc, sparc64, ia64, alpha, and i386.  Don't know about VAX...

One could place this include within the #ifdef'ed block that contains the
relevant function uses, but that's not generally within the style of this file
or most others, per my recollection.

I didn't know who takes things like this, so if anyone would like to pick this
up, please do so.  Otherwise, I will send it to Linus after a few days.

# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher.
# This patch includes the following deltas:
#	           ChangeSet	1.1367  -> 1.1368
#	include/linux/swap.h	1.79    -> 1.80
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 03/10/16	noah@caltech.edu	1.1368
# The linux/swap.h header uses the page_cache_release and release_pages
# functions declared in linux/pagemap.h when CONFIG_SWAP is disabled.  Add
# an include of linux/pagemap.h so swap.h finds those declarations on
# architectures that don't include pagemap.h indirectly.
# --------------------------------------------
#
diff -Nru a/include/linux/swap.h b/include/linux/swap.h
--- a/include/linux/swap.h	Fri Oct 17 13:40:06 2003
+++ b/include/linux/swap.h	Fri Oct 17 13:40:06 2003
@@ -4,6 +4,7 @@
 #include <linux/config.h>
 #include <linux/spinlock.h>
 #include <linux/linkage.h>
+#include <linux/pagemap.h>
 #include <linux/mmzone.h>
 #include <linux/list.h>
 #include <linux/sched.h>


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

* Re: [PATCH] Missing include in swap.h for some architectures
  2003-10-20  5:17 [PATCH] Missing include in swap.h for some architectures Noah J. Misch
@ 2003-10-20  9:25 ` Andrew Morton
  2003-10-21 13:47   ` Noah J. Misch
  0 siblings, 1 reply; 3+ messages in thread
From: Andrew Morton @ 2003-10-20  9:25 UTC (permalink / raw)
  To: Noah J. Misch; +Cc: linux-kernel

"Noah J. Misch" <noah@caltech.edu> wrote:
>
> # The linux/swap.h header uses the page_cache_release and release_pages
>  # functions declared in linux/pagemap.h when CONFIG_SWAP is disabled.  Add
>  # an include of linux/pagemap.h so swap.h finds those declarations on
>  # architectures that don't include pagemap.h indirectly.

This carries a small risk of breaking things.  I think I'd prefer that the
offending .c files just include pagemap.h please.


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

* Re: [PATCH] Missing include in swap.h for some architectures
  2003-10-20  9:25 ` Andrew Morton
@ 2003-10-21 13:47   ` Noah J. Misch
  0 siblings, 0 replies; 3+ messages in thread
From: Noah J. Misch @ 2003-10-21 13:47 UTC (permalink / raw)
  To: Andrew Morton; +Cc: linux-kernel

On Mon, 20 Oct 2003, Andrew Morton wrote:

> "Noah J. Misch" <noah@caltech.edu> wrote:
> >
> > # The linux/swap.h header uses the page_cache_release and release_pages
> >  # functions declared in linux/pagemap.h when CONFIG_SWAP is disabled.  Add
> >  # an include of linux/pagemap.h so swap.h finds those declarations on
> >  # architectures that don't include pagemap.h indirectly.
>
> This carries a small risk of breaking things.  I think I'd prefer that the
> offending .c files just include pagemap.h please.

I think I understand your argument, but consider this:

Adding includes very rarely breaks code, and when it does, the cause is a real
defect and the symptom is usually an easily fixed compilation error.

Furthermore, it's not, by and large, the .c files that are "offending" - it's
the static inline functions tlb_flush_mmu and tlb_remove page, both defined in
asm/tlb.h.  With the current swap.h, those functions need pagemap.h included to
compile cleanly with SWAP=n.  That means that every .c file that includes tlb.h,
directly or indirectly, _must_ somehow also include pagemap.h, regardless of why
it may be including tlb.h, or it gets slapped with a warning or worse.

If we make the file responsible for the pagemap.h dependency, swap.h, clean up
after itself, we can clear up ancillary breakage now.  MM code that doesn't
compile isn't likely to remain unfixed for long.  If we expect every .c file
that includes tlb.h to include pagemap.h, and we know that they may fail to do
so and build without a hitch on some architectures, that sets us up for fixing
odd breakage here and there over time.


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

end of thread, other threads:[~2003-10-21 13:47 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2003-10-20  5:17 [PATCH] Missing include in swap.h for some architectures Noah J. Misch
2003-10-20  9:25 ` Andrew Morton
2003-10-21 13:47   ` Noah J. Misch

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.