[Xen-devel] Re: [PATCH] remove fs/Kconfig from sparse tree in favor of a 1 line patch

[Sean Dague <japh@xxxxxxxxxx>]

On Fri, Jun 09, 2006 at 04:04:47PM -0700, Chris Wright wrote:
> * Sean Dague (japh@xxxxxxxxxx) wrote:
> > This patch removes fs/Kconfig, which is currently only different from the
> > linux-2.6.16.13 version by 1 line.  It adds that change back in as a patch
> > in the patches directory.
> 
> I don't think this is a great candidate for patches/.  Good things there
> are bits that are ready to go upstream or fixes to base Linux that are
> transient by version (backport of a fix that's in the -rc for the next
> kernel or somesuch).

I'm not sure that that reasoning holds with things like xenoprof-generic.patch
included in the tree, but even if it does, it would be really good to have
somewhere in the xen tree where something besides just backports and
fully upstream content.

> > This makes it possible to use additional filesystems in xen by just adding
> > them to the patches directory.  Without this patch you must also change the
> > sparse tree as all filesystem options get clobbered by the Kconfig from the
> > sparse tree.
> 
> Heh, tht just depends on when you add the filesystem ;-)

Given that patches goes first, and sparse clobbers it, the patches
infrastructure is blocked from being able to affect a number of parts of the
tree.  So when should the filesystem be added? ;)

The most egregious instances of sparse tree clobber are the following:

linux-2.6-xen-sparse/drivers/firmware/Kconfig
linux-2.6-xen-sparse/drivers/pci/Kconfig
linux-2.6-xen-sparse/drivers/serial/Kconfig
linux-2.6-xen-sparse/drivers/video/Kconfig
linux-2.6-xen-sparse/fs/Kconfig

Where the sparse tree contains only a single file in each directory.  In
each case a full sparse file is used to turn off a _single_ feature for a
directory.  This has the really negative side effect of clobbering any other
additions or changes.

The complete list of diffs follows, just to drive home what we are really
talking about.

dargo:~/xen-unstable> diff -u
pristine-linux-2.6.16.13/drivers/firmware/Kconfig
linux-2.6-xen-sparse/drivers/firmware/Kconfig
--- pristine-linux-2.6.16.13/drivers/firmware/Kconfig   2006-05-02
17:38:44.000000000 -0400
+++ linux-2.6-xen-sparse/drivers/firmware/Kconfig       2006-06-09
21:40:20.000000000 -0400
@@ -8,7 +8,7 @@
 config EDD
        tristate "BIOS Enhanced Disk Drive calls determine boot disk 
(EXPERIMENTAL)"
        depends on EXPERIMENTAL
-       depends on !IA64
+       depends on !IA64 && !XEN
        help
          Say Y or M here if you want to enable BIOS Enhanced Disk Drive
          Services real mode BIOS calls to determine which disk


dargo:~/xen-unstable> diff -u pristine-linux-2.6.16.13/drivers/pci/Kconfig
linux-2.6-xen-sparse/drivers/pci/Kconfig
--- pristine-linux-2.6.16.13/drivers/pci/Kconfig        2006-05-02
17:38:44.000000000 -0400
+++ linux-2.6-xen-sparse/drivers/pci/Kconfig    2006-06-09
21:40:20.000000000 -0400
@@ -5,6 +5,7 @@
        bool "Message Signaled Interrupts (MSI and MSI-X)"
        depends on PCI
        depends on (X86_LOCAL_APIC && X86_IO_APIC) || IA64
+       depends on !XEN
        help
           This allows device drivers to enable MSI (Message Signaled
           Interrupts).  Message Signaled Interrupts enable a device to


dargo:~/xen-unstable> diff -u
pristine-linux-2.6.16.13/drivers/serial/Kconfig
linux-2.6-xen-sparse/drivers/serial/Kconfig
--- pristine-linux-2.6.16.13/drivers/serial/Kconfig     2006-05-02
17:38:44.000000000 -0400
+++ linux-2.6-xen-sparse/drivers/serial/Kconfig 2006-06-09
21:40:20.000000000 -0400
@@ -11,6 +11,7 @@
 config SERIAL_8250
        tristate "8250/16550 and compatible serial support"
        depends on (BROKEN || !SPARC)
+       depends on !XEN_DISABLE_SERIAL
        select SERIAL_CORE
        ---help---
          This selects whether you want to include the driver for the standard


dargo:~/xen-unstable> diff -u pristine-linux-2.6.16.13/drivers/video/Kconfig
linux-2.6-xen-sparse/drivers/video/Kconfig
--- pristine-linux-2.6.16.13/drivers/video/Kconfig      2006-05-02
17:38:44.000000000 -0400
+++ linux-2.6-xen-sparse/drivers/video/Kconfig  2006-06-09
21:40:20.000000000 -0400
@@ -495,7 +495,7 @@
 
 config VIDEO_SELECT
        bool
-       depends on (FB = y) && X86
+       depends on (FB = y) && X86 && !XEN
        default y
 
 config FB_SGIVW



dargo:~/xen-unstable> diff -u pristine-linux-2.6.16.13/fs/Kconfig
linux-2.6-xen-sparse/fs/Kconfig           
--- pristine-linux-2.6.16.13/fs/Kconfig 2006-05-02 17:38:44.000000000 -0400
+++ linux-2.6-xen-sparse/fs/Kconfig     2006-06-09 21:40:21.000000000 -0400
@@ -841,6 +841,7 @@
 config HUGETLBFS
        bool "HugeTLB file system support"
        depends X86 || IA64 || PPC64 || SPARC64 || SUPERH || BROKEN
+       depends !XEN
 
 config HUGETLB_PAGE
        def_bool HUGETLBFS


For those 7 lines of change, Xen currently includes 4356 lines of code:

dargo:~/xen-unstable> wc -l linux-2.6-xen-sparse/fs/Kconfig
linux-2.6-xen-sparse/drivers/video/Kconfig
linux-2.6-xen-sparse/drivers/serial/Kconfig
linux-2.6-xen-sparse/drivers/pci/Kconfig
linux-2.6-xen-sparse/drivers/firmware/Kconfig
  1834 linux-2.6-xen-sparse/fs/Kconfig
  1462 linux-2.6-xen-sparse/drivers/video/Kconfig
   929 linux-2.6-xen-sparse/drivers/serial/Kconfig
    43 linux-2.6-xen-sparse/drivers/pci/Kconfig
    88 linux-2.6-xen-sparse/drivers/firmware/Kconfig
  4356 total


What makes it even worse, is that these changes are *ideal* as patches,
because given the amount of context, and the area patched, these are
reasonbly resilient to future changes in these files.

> > It also just reduces the size of the sparse tree, which I think is just a
> > Good Thing (tm). :)
> 
> In general, I completely agree, just not so much on this particular
> patch.

I'd really argue that while the sparse tree is needed for some of the
extensive code changes, using it to turn off single features in Kconfig in
directories where no code is changed, is verging on abuse. ;)

This is specifically causing me pain trying to apply add squashfs support,
which otherwise would have been dropping 1 patch in place, and tweaking my
build config.  In the current infrastructure it requires a patch + sparse
tree changes, which is really unfortunate, and I think completely
unnessesary, given the very small changes that Xen needs in fs/Kconfig.

        -Sean

-- 
Sean Dague
IBM Linux Technology Center                     email: japh@xxxxxxxxxx
Open Hypervisor Team                           alt: sldague@xxxxxxxxxx

Attachment: pgp96KsKTzqBo.pgp
Description: PGP signature

_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel