Re: [PATCH] Missing include in swap.h for some architectures

From: Noah J. Misch
Date: Tue Oct 21 2003 - 08:49:06 EST


On Mon, 20 Oct 2003, Andrew Morton wrote:

> "Noah J. Misch" <noah@xxxxxxxxxxx> 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.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/