diff mbox series

autofs: make autofs4 and autofs mutually exclusive

Message ID 20180529094702.4092022-1-arnd@arndb.de
State New
Headers show
Series autofs: make autofs4 and autofs mutually exclusive | expand

Commit Message

Arnd Bergmann May 29, 2018, 9:46 a.m. UTC
The autofs4 implementation is just a redirect to autofs now, but that
also means we can't have both built into the same kernel:

fs/autofs/inode.o: In function `autofs_new_ino':
inode.c:(.text+0x1b8): multiple definition of `autofs_new_ino'
fs/autofs/inode.o:inode.c:(.text+0x1b8): first defined here
fs/autofs/inode.o: In function `autofs_clean_ino':
inode.c:(.text+0x288): multiple definition of `autofs_clean_ino'

There is also a problem with trying to build both in parallel, which
leads to two 'make' processes writing to the same fs/autofs/.*.o.cmd
file, causing corruption that manifests like

fs/autofs4/../autofs/.expire.o.cmd:679: *** missing separator.  Stop.

Making AUTOFS4_FS depend on AUTOFS_FS being disabled should avoid all
configurations that run into either issue.

Fixes: mmotm ("autofs: update fs/autofs4/Makefile")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>

---
 fs/autofs4/Kconfig | 1 +
 1 file changed, 1 insertion(+)

-- 
2.9.0

Comments

Ian Kent May 30, 2018, 12:48 a.m. UTC | #1
On Tue, 2018-05-29 at 11:46 +0200, Arnd Bergmann wrote:
> The autofs4 implementation is just a redirect to autofs now, but that

> also means we can't have both built into the same kernel:

> 

> fs/autofs/inode.o: In function `autofs_new_ino':

> inode.c:(.text+0x1b8): multiple definition of `autofs_new_ino'

> fs/autofs/inode.o:inode.c:(.text+0x1b8): first defined here

> fs/autofs/inode.o: In function `autofs_clean_ino':

> inode.c:(.text+0x288): multiple definition of `autofs_clean_ino'

> 

> There is also a problem with trying to build both in parallel, which

> leads to two 'make' processes writing to the same fs/autofs/.*.o.cmd

> file, causing corruption that manifests like

> 

> fs/autofs4/../autofs/.expire.o.cmd:679: *** missing separator.  Stop.

> 

> Making AUTOFS4_FS depend on AUTOFS_FS being disabled should avoid all

> configurations that run into either issue.


Thanks Arnd and this adds support that my analysis of build
problems is accurate.

I posted a similar patch on May 21 which also added a NOTE to
fs/autofs4/Kconfig saying pretty much what you've said.
https://patchwork.kernel.org/patch/10413823/

I recommend using my patch so that anyone that's surprised by the
.config change has a chance of finding an explanation somewhere, ;)

Not only is the change needed, but to preserve bisection it needs
to be folded into the original patch titled:
autofs: create autofs Kconfig and Makefile

otherwise build test robots will still see this problem between
build testing after "autofs: create autofs Kconfig and Makefile"
and before this change is applied.

Folding in the change is my current recommendation to Andrew.
Hopefully that will fix the problem.

Any further thoughts are of course welcome.

> 

> Fixes: mmotm ("autofs: update fs/autofs4/Makefile")

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> ---

>  fs/autofs4/Kconfig | 1 +

>  1 file changed, 1 insertion(+)

> 

> diff --git a/fs/autofs4/Kconfig b/fs/autofs4/Kconfig

> index 53bc592a250d..eccf673c6c8c 100644

> --- a/fs/autofs4/Kconfig

> +++ b/fs/autofs4/Kconfig

> @@ -1,5 +1,6 @@

>  config AUTOFS4_FS

>  	tristate "Kernel automounter version 4 support (also supports v3 and

> v5)"

> +	depends on AUTOFS_FS=n

>  	default n

>  	help

>  	  The automounter is a tool to automatically mount remote file

> systems
Arnd Bergmann May 30, 2018, 8:41 a.m. UTC | #2
On Wed, May 30, 2018 at 2:48 AM, Ian Kent <raven@themaw.net> wrote:
> On Tue, 2018-05-29 at 11:46 +0200, Arnd Bergmann wrote:

>> The autofs4 implementation is just a redirect to autofs now, but that

>> also means we can't have both built into the same kernel:

>>

>> fs/autofs/inode.o: In function `autofs_new_ino':

>> inode.c:(.text+0x1b8): multiple definition of `autofs_new_ino'

>> fs/autofs/inode.o:inode.c:(.text+0x1b8): first defined here

>> fs/autofs/inode.o: In function `autofs_clean_ino':

>> inode.c:(.text+0x288): multiple definition of `autofs_clean_ino'

>>

>> There is also a problem with trying to build both in parallel, which

>> leads to two 'make' processes writing to the same fs/autofs/.*.o.cmd

>> file, causing corruption that manifests like

>>

>> fs/autofs4/../autofs/.expire.o.cmd:679: *** missing separator.  Stop.

>>

>> Making AUTOFS4_FS depend on AUTOFS_FS being disabled should avoid all

>> configurations that run into either issue.

>

> Thanks Arnd and this adds support that my analysis of build

> problems is accurate.

>

> I posted a similar patch on May 21 which also added a NOTE to

> fs/autofs4/Kconfig saying pretty much what you've said.

> https://patchwork.kernel.org/patch/10413823/

>

> I recommend using my patch so that anyone that's surprised by the

> .config change has a chance of finding an explanation somewhere, ;)

>

> Not only is the change needed, but to preserve bisection it needs

> to be folded into the original patch titled:

> autofs: create autofs Kconfig and Makefile

>

> otherwise build test robots will still see this problem between

> build testing after "autofs: create autofs Kconfig and Makefile"

> and before this change is applied.

>

> Folding in the change is my current recommendation to Andrew.

> Hopefully that will fix the problem.


Yes, looks good.

> Any further thoughts are of course welcome.


I actually had an alternative approach that I tried out successfully
but discarded as being too different from the original code. Just for
reference, this one would work as well, and allow both to be
compiled together. The version you posted is probably better.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>


diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h
index 9400a9f6318a..5a361f5273a1 100644
--- a/fs/autofs/autofs_i.h
+++ b/fs/autofs/autofs_i.h
@@ -6,6 +6,8 @@
  * the terms of the GNU General Public License, version 2, or at your
  * option, any later version, incorporated herein by reference.
  */
+#ifndef __AUTOFS_H
+#define __AUTOFS_H

 /* Internal header file for autofs */

@@ -271,3 +273,5 @@ static inline void autofs_del_expiring(struct
dentry *dentry)
 }

 void autofs_kill_sb(struct super_block *);
+
+#endif /* __AUTOFS_H */
diff --git a/fs/autofs4/Makefile b/fs/autofs4/Makefile
index 417dd726d9ef..5fc0a28d656d 100644
--- a/fs/autofs4/Makefile
+++ b/fs/autofs4/Makefile
@@ -3,7 +3,3 @@
 #

 obj-$(CONFIG_AUTOFS4_FS) += autofs4.o
-
-autofs4-objs := ../autofs/init.o ../autofs/inode.o ../autofs/root.o \
-       ../autofs/symlink.o ../autofs/waitq.o ../autofs/expire.o \
-       ../autofs/dev-ioctl.o
diff --git a/fs/autofs4/autofs4.c b/fs/autofs4/autofs4.c
new file mode 100644
index 000000000000..a76066aa9f27
--- /dev/null
+++ b/fs/autofs4/autofs4.c
@@ -0,0 +1,7 @@
+#include "../autofs/init.c"
+#include "../autofs/inode.c"
+#include "../autofs/root.c"
+#include "../autofs/symlink.c"
+#include "../autofs/waitq.c"
+#include "../autofs/expire.c"
+#include "../autofs/dev-ioctl.c"
Ian Kent May 30, 2018, 9:18 a.m. UTC | #3
On Wed, 2018-05-30 at 10:41 +0200, Arnd Bergmann wrote:
> On Wed, May 30, 2018 at 2:48 AM, Ian Kent <raven@themaw.net> wrote:

> > On Tue, 2018-05-29 at 11:46 +0200, Arnd Bergmann wrote:

> > > The autofs4 implementation is just a redirect to autofs now, but that

> > > also means we can't have both built into the same kernel:

> > > 

> > > fs/autofs/inode.o: In function `autofs_new_ino':

> > > inode.c:(.text+0x1b8): multiple definition of `autofs_new_ino'

> > > fs/autofs/inode.o:inode.c:(.text+0x1b8): first defined here

> > > fs/autofs/inode.o: In function `autofs_clean_ino':

> > > inode.c:(.text+0x288): multiple definition of `autofs_clean_ino'

> > > 

> > > There is also a problem with trying to build both in parallel, which

> > > leads to two 'make' processes writing to the same fs/autofs/.*.o.cmd

> > > file, causing corruption that manifests like

> > > 

> > > fs/autofs4/../autofs/.expire.o.cmd:679: *** missing separator.  Stop.

> > > 

> > > Making AUTOFS4_FS depend on AUTOFS_FS being disabled should avoid all

> > > configurations that run into either issue.

> > 

> > Thanks Arnd and this adds support that my analysis of build

> > problems is accurate.

> > 

> > I posted a similar patch on May 21 which also added a NOTE to

> > fs/autofs4/Kconfig saying pretty much what you've said.

> > https://patchwork.kernel.org/patch/10413823/

> > 

> > I recommend using my patch so that anyone that's surprised by the

> > .config change has a chance of finding an explanation somewhere, ;)

> > 

> > Not only is the change needed, but to preserve bisection it needs

> > to be folded into the original patch titled:

> > autofs: create autofs Kconfig and Makefile

> > 

> > otherwise build test robots will still see this problem between

> > build testing after "autofs: create autofs Kconfig and Makefile"

> > and before this change is applied.

> > 

> > Folding in the change is my current recommendation to Andrew.

> > Hopefully that will fix the problem.

> 

> Yes, looks good.

> 

> > Any further thoughts are of course welcome.

> 

> I actually had an alternative approach that I tried out successfully

> but discarded as being too different from the original code. Just for

> reference, this one would work as well, and allow both to be

> compiled together. The version you posted is probably better.


It's an attractive option but the problem is both implement the
autofs file system.

I've always thought you can't register the same file system at the
same time from two distinct sources.

If you're careful and compile each only as a module you could do it.

But many configurations have autofs compiled built-in because of the
auto-loading problems that arose back when there was an autofs fs
module as well as an autofs fs module present in the autofs4 directory.

Maybe it would actually work with one winning over the other but
I'd prefer not to go that way.

It will be gone in two subsequent releases if it gets merged and no
changes to the retained code will be needed with this approach.

> 

> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

> 

> diff --git a/fs/autofs/autofs_i.h b/fs/autofs/autofs_i.h

> index 9400a9f6318a..5a361f5273a1 100644

> --- a/fs/autofs/autofs_i.h

> +++ b/fs/autofs/autofs_i.h

> @@ -6,6 +6,8 @@

>   * the terms of the GNU General Public License, version 2, or at your

>   * option, any later version, incorporated herein by reference.

>   */

> +#ifndef __AUTOFS_H

> +#define __AUTOFS_H

> 

>  /* Internal header file for autofs */

> 

> @@ -271,3 +273,5 @@ static inline void autofs_del_expiring(struct

> dentry *dentry)

>  }

> 

>  void autofs_kill_sb(struct super_block *);

> +

> +#endif /* __AUTOFS_H */

> diff --git a/fs/autofs4/Makefile b/fs/autofs4/Makefile

> index 417dd726d9ef..5fc0a28d656d 100644

> --- a/fs/autofs4/Makefile

> +++ b/fs/autofs4/Makefile

> @@ -3,7 +3,3 @@

>  #

> 

>  obj-$(CONFIG_AUTOFS4_FS) += autofs4.o

> -

> -autofs4-objs := ../autofs/init.o ../autofs/inode.o ../autofs/root.o \

> -       ../autofs/symlink.o ../autofs/waitq.o ../autofs/expire.o \

> -       ../autofs/dev-ioctl.o

> diff --git a/fs/autofs4/autofs4.c b/fs/autofs4/autofs4.c

> new file mode 100644

> index 000000000000..a76066aa9f27

> --- /dev/null

> +++ b/fs/autofs4/autofs4.c

> @@ -0,0 +1,7 @@

> +#include "../autofs/init.c"

> +#include "../autofs/inode.c"

> +#include "../autofs/root.c"

> +#include "../autofs/symlink.c"

> +#include "../autofs/waitq.c"

> +#include "../autofs/expire.c"

> +#include "../autofs/dev-ioctl.c"
Andrew Morton June 1, 2018, 12:13 a.m. UTC | #4
On Wed, 30 May 2018 17:18:55 +0800 Ian Kent <raven@themaw.net> wrote:

> > I actually had an alternative approach that I tried out successfully

> > but discarded as being too different from the original code. Just for

> > reference, this one would work as well, and allow both to be

> > compiled together. The version you posted is probably better.

> 

> It's an attractive option but the problem is both implement the

> autofs file system.

> 

> I've always thought you can't register the same file system at the

> same time from two distinct sources.

> 

> If you're careful and compile each only as a module you could do it.

> 

> But many configurations have autofs compiled built-in because of the

> auto-loading problems that arose back when there was an autofs fs

> module as well as an autofs fs module present in the autofs4 directory.

> 

> Maybe it would actually work with one winning over the other but

> I'd prefer not to go that way.

> 

> It will be gone in two subsequent releases if it gets merged and no

> changes to the retained code will be needed with this approach.


I'm losing the plot here.  Can you please confirm that this is the
patch we want?


From: Ian Kent <raven@themaw.net>

Subject: autofs: update fs/autofs4/Kconfig

Update Kconfig and add a depricated warning.

[raven@themaw.net: make autofs4 Kconfig depend on AUTOFS_FS]
  Link: http://lkml.kernel.org/r/152687649097.8263.7046086367407522029.stgit@pluto.themaw.net
Link: http://lkml.kernel.org/r/152626706133.28589.11994171621899212952.stgit@pluto.themaw.net
Signed-off-by: Ian Kent <raven@themaw.net>

Tested-by: Randy Dunlap <rdunlap@infradead.org>

Cc: Al Viro <viro@ZenIV.linux.org.uk>
Cc: Arnd Bergmann <arnd@arndb.de>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

---

 fs/autofs4/Kconfig |   42 +++++++++++++++++++++++++++++++-----------
 1 file changed, 31 insertions(+), 11 deletions(-)

diff -puN fs/autofs4/Kconfig~autofs-update-fs-autofs4-kconfig fs/autofs4/Kconfig
--- a/fs/autofs4/Kconfig~autofs-update-fs-autofs4-kconfig
+++ a/fs/autofs4/Kconfig
@@ -1,5 +1,7 @@
 config AUTOFS4_FS
-	tristate "Kernel automounter version 4 support (also supports v3)"
+	tristate "Kernel automounter version 4 support (also supports v3 and v5)"
+	default n
+	depends on AUTOFS_FS = n
 	help
 	  The automounter is a tool to automatically mount remote file systems
 	  on demand. This implementation is partially kernel-based to reduce
@@ -7,14 +9,32 @@ config AUTOFS4_FS
 	  automounter (amd), which is a pure user space daemon.
 
 	  To use the automounter you need the user-space tools from
-	  <https://www.kernel.org/pub/linux/daemons/autofs/v4/>; you also
-	  want to answer Y to "NFS file system support", below.
+	  <https://www.kernel.org/pub/linux/daemons/autofs/>; you also want
+	  to answer Y to "NFS file system support", below.
 
-	  To compile this support as a module, choose M here: the module will be
-	  called autofs4.  You will need to add "alias autofs autofs4" to your
-	  modules configuration file.
-
-	  If you are not a part of a fairly large, distributed network or
-	  don't have a laptop which needs to dynamically reconfigure to the
-	  local network, you probably do not need an automounter, and can say
-	  N here.
+	  This module is in the process of being renamed from autofs4 to
+	  autofs. Since autofs is now the only module that provides the
+	  autofs file system the module is not version 4 specific.
+
+	  The autofs4 module is now built from the source located in
+	  fs/autofs. The autofs4 directory and its configuration entry
+	  will be removed two kernel versions from the inclusion of this
+	  change.
+
+	  Changes that will need to be made should be limited to:
+	  - source include statments should be changed from autofs_fs4.h to
+	    autofs_fs.h since these two header files have been merged.
+	  - user space scripts that manually load autofs4.ko should be
+	    changed to load autofs.ko. But since the module directory name
+	    and the module name are the same as the file system name there
+	    is no need to manually load module.
+	  - any "alias autofs autofs4" will need to be removed.
+
+	  Please configure AUTOFS_FS instead of AUTOFS4_FS from now on.
+
+	  NOTE: Since the modules autofs and autofs4 use the same file system
+		type name of "autofs" only one can be built. The "depends"
+		above will result in AUTOFS4_FS not appearing in .config for
+		any setting of AUTOFS_FS other than n and AUTOFS4_FS will
+		appear under the AUTOFS_FS entry otherwise which is intended
+		to draw attention to the module rename change.
Ian Kent June 1, 2018, 1:35 a.m. UTC | #5
On Thu, 2018-05-31 at 17:13 -0700, Andrew Morton wrote:
> On Wed, 30 May 2018 17:18:55 +0800 Ian Kent <raven@themaw.net> wrote:

> 

> > > I actually had an alternative approach that I tried out successfully

> > > but discarded as being too different from the original code. Just for

> > > reference, this one would work as well, and allow both to be

> > > compiled together. The version you posted is probably better.

> > 

> > It's an attractive option but the problem is both implement the

> > autofs file system.

> > 

> > I've always thought you can't register the same file system at the

> > same time from two distinct sources.

> > 

> > If you're careful and compile each only as a module you could do it.

> > 

> > But many configurations have autofs compiled built-in because of the

> > auto-loading problems that arose back when there was an autofs fs

> > module as well as an autofs fs module present in the autofs4 directory.

> > 

> > Maybe it would actually work with one winning over the other but

> > I'd prefer not to go that way.

> > 

> > It will be gone in two subsequent releases if it gets merged and no

> > changes to the retained code will be needed with this approach.

> 

> I'm losing the plot here.  Can you please confirm that this is the

> patch we want?


Understandable.

This wasn't quite what I did and at the risk of confusing matters
further I'll try and explain what I did and why.

I folded the change into the patch which created fs/autofs/Kconfig
(autofs-create-autofs-kconfig-and-makefile.patch).

However doing what you're doing here should have the same effect as
long as Kbuild is smart enough to work out that
"depends on AUTOFS_FS = n" doesn't apply when fs/autofs/Kconfig hasn't
yet been created (by autofs-create-autofs-kconfig-and-makefile.patch).

The problem is that AUTOFS_FS=y might be still be present in .config
(surviving after many years) causing the bisection problem.

That's why I thought it best to add the depends in fs/autofs4/Kconfig
at the time fs/autofs/Kconfig is created rather than before that in
autofs-update-fs-autofs4-makefile.patch, as is done here.

Let me check if Kbuild will do the right thing and get back to you.

> 

> 

> From: Ian Kent <raven@themaw.net>

> Subject: autofs: update fs/autofs4/Kconfig

> 

> Update Kconfig and add a depricated warning.

> 

> [raven@themaw.net: make autofs4 Kconfig depend on AUTOFS_FS]

>   Link: http://lkml.kernel.org/r/152687649097.8263.7046086367407522029.stgit@p

> luto.themaw.net

> Link: http://lkml.kernel.org/r/152626706133.28589.11994171621899212952.stgit@p

> luto.themaw.net

> Signed-off-by: Ian Kent <raven@themaw.net>

> Tested-by: Randy Dunlap <rdunlap@infradead.org>

> Cc: Al Viro <viro@ZenIV.linux.org.uk>

> Cc: Arnd Bergmann <arnd@arndb.de>

> Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

> ---

> 

>  fs/autofs4/Kconfig |   42 +++++++++++++++++++++++++++++++-----------

>  1 file changed, 31 insertions(+), 11 deletions(-)

> 

> diff -puN fs/autofs4/Kconfig~autofs-update-fs-autofs4-kconfig

> fs/autofs4/Kconfig

> --- a/fs/autofs4/Kconfig~autofs-update-fs-autofs4-kconfig

> +++ a/fs/autofs4/Kconfig

> @@ -1,5 +1,7 @@

>  config AUTOFS4_FS

> -	tristate "Kernel automounter version 4 support (also supports v3)"

> +	tristate "Kernel automounter version 4 support (also supports v3 and

> v5)"

> +	default n

> +	depends on AUTOFS_FS = n

>  	help

>  	  The automounter is a tool to automatically mount remote file

> systems

>  	  on demand. This implementation is partially kernel-based to reduce

> @@ -7,14 +9,32 @@ config AUTOFS4_FS

>  	  automounter (amd), which is a pure user space daemon.

>  

>  	  To use the automounter you need the user-space tools from

> -	  <https://www.kernel.org/pub/linux/daemons/autofs/v4/>; you also

> -	  want to answer Y to "NFS file system support", below.

> +	  <https://www.kernel.org/pub/linux/daemons/autofs/>; you also want

> +	  to answer Y to "NFS file system support", below.

>  

> -	  To compile this support as a module, choose M here: the module will

> be

> -	  called autofs4.  You will need to add "alias autofs autofs4" to

> your

> -	  modules configuration file.

> -

> -	  If you are not a part of a fairly large, distributed network or

> -	  don't have a laptop which needs to dynamically reconfigure to the

> -	  local network, you probably do not need an automounter, and can say

> -	  N here.

> +	  This module is in the process of being renamed from autofs4 to

> +	  autofs. Since autofs is now the only module that provides the

> +	  autofs file system the module is not version 4 specific.

> +

> +	  The autofs4 module is now built from the source located in

> +	  fs/autofs. The autofs4 directory and its configuration entry

> +	  will be removed two kernel versions from the inclusion of this

> +	  change.

> +

> +	  Changes that will need to be made should be limited to:

> +	  - source include statments should be changed from autofs_fs4.h to

> +	    autofs_fs.h since these two header files have been merged.

> +	  - user space scripts that manually load autofs4.ko should be

> +	    changed to load autofs.ko. But since the module directory name

> +	    and the module name are the same as the file system name there

> +	    is no need to manually load module.

> +	  - any "alias autofs autofs4" will need to be removed.

> +

> +	  Please configure AUTOFS_FS instead of AUTOFS4_FS from now on.

> +

> +	  NOTE: Since the modules autofs and autofs4 use the same file system

> +		type name of "autofs" only one can be built. The "depends"

> +		above will result in AUTOFS4_FS not appearing in .config for

> +		any setting of AUTOFS_FS other than n and AUTOFS4_FS will

> +		appear under the AUTOFS_FS entry otherwise which is intended

> +		to draw attention to the module rename change.

> _

>
Ian Kent June 1, 2018, 8:42 a.m. UTC | #6
On Fri, 2018-06-01 at 09:35 +0800, Ian Kent wrote:
> On Thu, 2018-05-31 at 17:13 -0700, Andrew Morton wrote:

> > On Wed, 30 May 2018 17:18:55 +0800 Ian Kent <raven@themaw.net> wrote:

> > 

> > > > I actually had an alternative approach that I tried out successfully

> > > > but discarded as being too different from the original code. Just for

> > > > reference, this one would work as well, and allow both to be

> > > > compiled together. The version you posted is probably better.

> > > 

> > > It's an attractive option but the problem is both implement the

> > > autofs file system.

> > > 

> > > I've always thought you can't register the same file system at the

> > > same time from two distinct sources.

> > > 

> > > If you're careful and compile each only as a module you could do it.

> > > 

> > > But many configurations have autofs compiled built-in because of the

> > > auto-loading problems that arose back when there was an autofs fs

> > > module as well as an autofs fs module present in the autofs4 directory.

> > > 

> > > Maybe it would actually work with one winning over the other but

> > > I'd prefer not to go that way.

> > > 

> > > It will be gone in two subsequent releases if it gets merged and no

> > > changes to the retained code will be needed with this approach.

> > 

> > I'm losing the plot here.  Can you please confirm that this is the

> > patch we want?

> 

> Understandable.

> 

> This wasn't quite what I did and at the risk of confusing matters

> further I'll try and explain what I did and why.

> 

> I folded the change into the patch which created fs/autofs/Kconfig

> (autofs-create-autofs-kconfig-and-makefile.patch).

> 

> However doing what you're doing here should have the same effect as

> long as Kbuild is smart enough to work out that

> "depends on AUTOFS_FS = n" doesn't apply when fs/autofs/Kconfig hasn't

> yet been created (by autofs-create-autofs-kconfig-and-makefile.patch).

> 

> The problem is that AUTOFS_FS=y might be still be present in .config

> (surviving after many years) causing the bisection problem.

> 

> That's why I thought it best to add the depends in fs/autofs4/Kconfig

> at the time fs/autofs/Kconfig is created rather than before that in

> autofs-update-fs-autofs4-makefile.patch, as is done here.

> 

> Let me check if Kbuild will do the right thing and get back to you.


It appears that folding the "depends" update into the earlier
patch will result in any CONFIG_AUTOFS* entries being silently
removed from .config until the later patch that creates the new
fs/autofs/Kconfig is applied.

Loosing the CONFIG_AUTOFS* settings could surprise someone doing
a bisect.

To try and clear up the confusion I've taken the series from
http://www.ozlabs.org/~akpm/mmotm/series

#NEXT_PATCHES_START autofs
autofs4-merge-auto_fsh-and-auto_fs4h.patch
autofs4-use-autofs-instead-of-autofs4-everywhere.patch
autofs-copy-autofs4-to-autofs.patch
autofs-update-fs-autofs4-kconfig.patch
autofs-update-fs-autofs4-kconfig-fix.patch <--the update was folded into this
autofs-update-fs-autofs4-makefile.patch
autofs-delete-fs-autofs4-source-files.patch
autofs-create-autofs-kconfig-and-makefile.patch <-- when it should have been
                                                <-- folded into this one
autofs-rename-autofs-documentation-files.patch
autofs-use-autofs-instead-of-autofs4-in-documentation.patch
autofs-update-maintainers-entry-for-autofs.patch
#
autofs-comment-on-selinux-changes-needed-for-module-autoload.patch
#
autofs-clean-up-includes.patch
#NEXT_PATCHES_END

And the patches from http://www.ozlabs.org/~akpm/mmotm/broken-out/,
folded "autofs-update-fs-autofs4-kconfig-fix.patch" into
"autofs-create-autofs-kconfig-and-makefile.patch", and dropped the
former fix patch.

I've uploaded the resulting quilt series to
http://people.redhat.com/~ikent/akpm-autofs-rename-series/
so you can check.

The resulting "autofs-create-autofs-kconfig-and-makefile.patch" should
now look like (ha, of course that "Link:" is now wrong ...):

From: Ian Kent <raven@themaw.net>

Subject: autofs: create autofs Kconfig and Makefile

Create Makefile and Kconfig for autofs module.

Link: http://lkml.kernel.org/r/152626705591.28589.356365986974038383.stgit@pluto.themaw.net
Signed-off-by: Ian Kent <raven@themaw.net>

Cc: Al Viro <viro@ZenIV.linux.org.uk>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>

---

 fs/Kconfig         |    1 +
 fs/Makefile        |    1 +
 fs/autofs/Kconfig  |   20 ++++++++++++++++++++
 fs/autofs/Makefile |    7 +++++++
 4 files changed, 29 insertions(+)

--- /dev/null
+++ linux.git/fs/autofs/Kconfig
@@ -0,0 +1,20 @@
+config AUTOFS_FS
+	tristate "Kernel automounter support (supports v3, v4 and v5)"
+	default n
+	help
+	   The automounter is a tool to automatically mount remote file systems
+	   on demand. This implementation is partially kernel-based to reduce
+	   overhead in the already-mounted case; this is unlike the BSD
+	   automounter (amd), which is a pure user space daemon.
+
+	   To use the automounter you need the user-space tools from
+	   <https://www.kernel.org/pub/linux/daemons/autofs/>; you also want
+	   to answer Y to "NFS file system support", below.
+
+	   To compile this support as a module, choose M here: the module will be
+	   called autofs.
+
+	   If you are not a part of a fairly large, distributed network or
+	   don't have a laptop which needs to dynamically reconfigure to the
+	   local network, you probably do not need an automounter, and can say
+	   N here.
--- /dev/null
+++ linux.git/fs/autofs/Makefile
@@ -0,0 +1,7 @@
+#
+# Makefile for the linux autofs-filesystem routines.
+#
+
+obj-$(CONFIG_AUTOFS_FS) += autofs.o
+
+autofs-objs := init.o inode.o root.o symlink.o waitq.o expire.o dev-ioctl.o
--- linux.git.orig/fs/Kconfig
+++ linux.git/fs/Kconfig
@@ -108,6 +108,7 @@ source "fs/notify/Kconfig"
 
 source "fs/quota/Kconfig"
 
+source "fs/autofs/Kconfig"
 source "fs/autofs4/Kconfig"
 source "fs/fuse/Kconfig"
 source "fs/overlayfs/Kconfig"
--- linux.git.orig/fs/Makefile
+++ linux.git/fs/Makefile
@@ -102,6 +102,7 @@ obj-$(CONFIG_AFFS_FS)		+= affs/
 obj-$(CONFIG_ROMFS_FS)		+= romfs/
 obj-$(CONFIG_QNX4FS_FS)		+= qnx4/
 obj-$(CONFIG_QNX6FS_FS)		+= qnx6/
+obj-$(CONFIG_AUTOFS_FS)		+= autofs/
 obj-$(CONFIG_AUTOFS4_FS)	+= autofs4/
 obj-$(CONFIG_ADFS_FS)		+= adfs/
 obj-$(CONFIG_FUSE_FS)		+= fuse/
--- linux.git.orig/fs/autofs4/Kconfig
+++ linux.git/fs/autofs4/Kconfig
@@ -1,6 +1,7 @@
 config AUTOFS4_FS
 	tristate "Kernel automounter version 4 support (also supports v3 and v5)"
 	default n
+	depends on AUTOFS_FS = n
 	help
 	  The automounter is a tool to automatically mount remote file systems
 	  on demand. This implementation is partially kernel-based to reduce
@@ -30,3 +31,10 @@ config AUTOFS4_FS
 	  - any "alias autofs autofs4" will need to be removed.
 
 	  Please configure AUTOFS_FS instead of AUTOFS4_FS from now on.
+
+	  NOTE: Since the modules autofs and autofs4 use the same file system
+		type name of "autofs" only one can be built. The "depends"
+		above will result in AUTOFS4_FS not appearing in .config for
+		any setting of AUTOFS_FS other than n and AUTOFS4_FS will
+		appear under the AUTOFS_FS entry otherwise which is intended
+		to draw attention to the module rename change.
Andrew Morton June 1, 2018, 11:12 p.m. UTC | #7
On Fri, 01 Jun 2018 16:42:36 +0800 Ian Kent <raven@themaw.net> wrote:

> The resulting "autofs-create-autofs-kconfig-and-makefile.patch" should

> now look like


Got it, thanks ;)
diff mbox series

Patch

diff --git a/fs/autofs4/Kconfig b/fs/autofs4/Kconfig
index 53bc592a250d..eccf673c6c8c 100644
--- a/fs/autofs4/Kconfig
+++ b/fs/autofs4/Kconfig
@@ -1,5 +1,6 @@ 
 config AUTOFS4_FS
 	tristate "Kernel automounter version 4 support (also supports v3 and v5)"
+	depends on AUTOFS_FS=n
 	default n
 	help
 	  The automounter is a tool to automatically mount remote file systems