[patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[Thomas Petazzoni]

Hi,

This is a resend of the small patch list I sent on July, 29th. I'm
resending the patches because they didn't make it to vger lists for
some setup problem on my side, and because I've been asked by Adrian
Bunk to resend them in order to get proper review.

However, please note that the patches removing ethtool and IGMP have
both been nack-ed by David Miller. The resend of these patches is not
an intent to workaround these NACKs in any way: I'm resending because
I've been asked to do so.

I apologize for the mess, and hope that this time, the mails will
reach vger lists.

Changes since previous post:

 * Add Matt Mackall's Signed-off-by on all patches
 * Make bonding and bridging select ethtool in the ethtool-related
   patch.

Sincerly,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

[patch 1/4] Configure out AIO support

[Thomas Petazzoni]

[-- Attachment #1: configure-out-aio-support --]
[-- Type: text/plain, Size: 4663 bytes --]

This patchs adds the CONFIG_AIO option which allows to remove support
for asynchronous I/O operations, that are not necessarly used by
applications, particularly on embedded devices. As this is a
size-reduction option, it depends on CONFIG_EMBEDDED. It allows to
save ~7 kilobytes of kernel code/data:

   text	   data	    bss	    dec	    hex	filename
1115067	 119180	 217088	1451335	 162547	vmlinux
1108025	 119048	 217088	1444161	 160941	vmlinux.new
  -7042    -132       0   -7174   -1C06 +/-

This patch has been originally written by Matt Mackall
<mpm@selenic.com>, and is part of the Linux Tiny project.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Matt Mackall <mpm@selenic.com>
Cc: bcrl@kvack.org
Cc: linux-aio@kvack.org
Cc: mpm@selenic.com
Cc: akpm@linux-foundation.org


---
 fs/Makefile         |    3 ++-
 include/linux/aio.h |    9 +++++++++
 init/Kconfig        |    8 ++++++++
 kernel/sys_ni.c     |    5 +++++
 kernel/sysctl.c     |    2 ++
 5 files changed, 26 insertions(+), 1 deletion(-)

Index: linuxdev/fs/Makefile
===================================================================
--- linuxdev.orig/fs/Makefile
+++ linuxdev/fs/Makefile
@@ -8,7 +8,7 @@
 obj-y :=	open.o read_write.o file_table.o super.o \
 		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
 		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
-		attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
+		attr.o bad_inode.o file.o filesystems.o namespace.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o drop_caches.o splice.o sync.o utimes.o \
 		stack.o
@@ -27,6 +27,7 @@
 obj-$(CONFIG_SIGNALFD)		+= signalfd.o
 obj-$(CONFIG_TIMERFD)		+= timerfd.o
 obj-$(CONFIG_EVENTFD)		+= eventfd.o
+obj-$(CONFIG_AIO)               += aio.o
 obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
 
 nfsd-$(CONFIG_NFSD)		:= nfsctl.o
Index: linuxdev/include/linux/aio.h
===================================================================
--- linuxdev.orig/include/linux/aio.h
+++ linuxdev/include/linux/aio.h
@@ -204,12 +204,21 @@
 /* prototypes */
 extern unsigned aio_max_size;
 
+#ifdef CONFIG_AIO
 extern ssize_t wait_on_sync_kiocb(struct kiocb *iocb);
 extern int aio_put_req(struct kiocb *iocb);
 extern void kick_iocb(struct kiocb *iocb);
 extern int aio_complete(struct kiocb *iocb, long res, long res2);
 struct mm_struct;
 extern void exit_aio(struct mm_struct *mm);
+#else
+static inline ssize_t wait_on_sync_kiocb(struct kiocb *iocb) { return 0; }
+static inline int aio_put_req(struct kiocb *iocb) { return 0; }
+static inline void kick_iocb(struct kiocb *iocb) { }
+static inline int aio_complete(struct kiocb *iocb, long res, long res2) { return 0; }
+struct mm_struct;
+static inline void exit_aio(struct mm_struct *mm) { }
+#endif /* CONFIG_AIO */
 
 #define io_wait_to_kiocb(wait) container_of(wait, struct kiocb, ki_wait)
 
Index: linuxdev/init/Kconfig
===================================================================
--- linuxdev.orig/init/Kconfig
+++ linuxdev/init/Kconfig
@@ -724,6 +724,14 @@
 	  option replaces shmem and tmpfs with the much simpler ramfs code,
 	  which may be appropriate on small systems without swap.
 
+config AIO
+	bool "Enable AIO support" if EMBEDDED
+	default y
+	help
+	  This option enables POSIX asynchronous I/O which may by used
+          by some high performance threaded applications. Disabling
+          this option saves about 7k.
+
 config VM_EVENT_COUNTERS
 	default y
 	bool "Enable VM event counters for /proc/vmstat" if EMBEDDED
Index: linuxdev/kernel/sys_ni.c
===================================================================
--- linuxdev.orig/kernel/sys_ni.c
+++ linuxdev/kernel/sys_ni.c
@@ -125,6 +125,11 @@
 cond_syscall(sys_vm86);
 cond_syscall(compat_sys_ipc);
 cond_syscall(compat_sys_sysctl);
+cond_syscall(sys_io_setup);
+cond_syscall(sys_io_destroy);
+cond_syscall(sys_io_submit);
+cond_syscall(sys_io_cancel);
+cond_syscall(sys_io_getevents);
 
 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read);
Index: linuxdev/kernel/sysctl.c
===================================================================
--- linuxdev.orig/kernel/sysctl.c
+++ linuxdev/kernel/sysctl.c
@@ -1290,6 +1290,7 @@
 		.extra1		= &zero,
 		.extra2		= &two,
 	},
+#ifdef CONFIG_AIO
 	{
 		.procname	= "aio-nr",
 		.data		= &aio_nr,
@@ -1304,6 +1305,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_doulongvec_minmax,
 	},
+#endif /* CONFIG_AIO */
 #ifdef CONFIG_INOTIFY_USER
 	{
 		.ctl_name	= FS_INOTIFY,

-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 1/4] Configure out AIO support

[Bernhard Fischer]

On Thu, Jul 31, 2008 at 11:27:04AM +0200, Thomas Petazzoni wrote:
>This patchs adds the CONFIG_AIO option which allows to remove support
>for asynchronous I/O operations, that are not necessarly used by
>applications, particularly on embedded devices. As this is a
>size-reduction option, it depends on CONFIG_EMBEDDED. It allows to
>save ~7 kilobytes of kernel code/data:

Shouldn't this also make sure not to install aio_abi.h or at least an
empty aio_abi.h?

Re: [patch 1/4] Configure out AIO support

[Adrian Bunk]

On Thu, Jul 31, 2008 at 12:09:29PM +0200, Bernhard Fischer wrote:
> On Thu, Jul 31, 2008 at 11:27:04AM +0200, Thomas Petazzoni wrote:
> >This patchs adds the CONFIG_AIO option which allows to remove support
> >for asynchronous I/O operations, that are not necessarly used by
> >applications, particularly on embedded devices. As this is a
> >size-reduction option, it depends on CONFIG_EMBEDDED. It allows to
> >save ~7 kilobytes of kernel code/data:
> 
> Shouldn't this also make sure not to install aio_abi.h or at least an
> empty aio_abi.h?

The userspace headers are independent of any kernel configuration
(except for the architecture).

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 1/4] Configure out AIO support

[Bernhard Fischer]

On Thu, Jul 31, 2008 at 01:12:19PM +0300, Adrian Bunk wrote:
>On Thu, Jul 31, 2008 at 12:09:29PM +0200, Bernhard Fischer wrote:
>> On Thu, Jul 31, 2008 at 11:27:04AM +0200, Thomas Petazzoni wrote:
>> >This patchs adds the CONFIG_AIO option which allows to remove support
>> >for asynchronous I/O operations, that are not necessarly used by
>> >applications, particularly on embedded devices. As this is a
>> >size-reduction option, it depends on CONFIG_EMBEDDED. It allows to
>> >save ~7 kilobytes of kernel code/data:
>> 
>> Shouldn't this also make sure not to install aio_abi.h or at least an
>> empty aio_abi.h?
>
>The userspace headers are independent of any kernel configuration
>(except for the architecture).

I beg to disagree:
internals as exposed by e.g. aio_abi.h are impl dependent. Noone except
the impl and it's users are interrested in it.

If a per package feature, independent of arch, is off, all respective
features stuff should be off for that particular installation.

I.e. if I, for subsequent installations, choose to turn on feature, the
requested feature-stuff will be installed properly. If I, OTOH, choose
to (keep to) turn off feature, then all feature-related stuff should --
and has to be and stay -- turned off.

In the particular case of the kernel exposing unwanted API extensions
that are off, all such API reminiscences should be off, thus not
installed to my staging dir at all since i explicitely asked for doing
away with all of the API.

From a libc POV i'd agree, but from an inherently broken userspace POV
installing willingly broken stuff is just wrong and misleading, imo.

Re: [patch 1/4] Configure out AIO support

[Adrian Bunk]

On Fri, Aug 01, 2008 at 12:42:22AM +0200, Bernhard Fischer wrote:
> On Thu, Jul 31, 2008 at 01:12:19PM +0300, Adrian Bunk wrote:
> >On Thu, Jul 31, 2008 at 12:09:29PM +0200, Bernhard Fischer wrote:
> >> On Thu, Jul 31, 2008 at 11:27:04AM +0200, Thomas Petazzoni wrote:
> >> >This patchs adds the CONFIG_AIO option which allows to remove support
> >> >for asynchronous I/O operations, that are not necessarly used by
> >> >applications, particularly on embedded devices. As this is a
> >> >size-reduction option, it depends on CONFIG_EMBEDDED. It allows to
> >> >save ~7 kilobytes of kernel code/data:
> >> 
> >> Shouldn't this also make sure not to install aio_abi.h or at least an
> >> empty aio_abi.h?
> >
> >The userspace headers are independent of any kernel configuration
> >(except for the architecture).
> 
> I beg to disagree:
> internals as exposed by e.g. aio_abi.h are impl dependent. Noone except
> the impl and it's users are interrested in it.
>...

That's utter bullshit.

The contents of aio_abi.h is a kernel<->userspace ABI that mustn't ever 
change. [1]

cu
Adrian

[1] there are some exceptions like adding stuff (but not in existing 
    structs), but basically the contents of aio_abi.h is cast in stone

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 1/4] Configure out AIO support

[Jamie Lokier]

Adrian Bunk wrote:
> On Fri, Aug 01, 2008 at 12:42:22AM +0200, Bernhard Fischer wrote:
> > On Thu, Jul 31, 2008 at 01:12:19PM +0300, Adrian Bunk wrote:
> > >On Thu, Jul 31, 2008 at 12:09:29PM +0200, Bernhard Fischer wrote:
> > >> On Thu, Jul 31, 2008 at 11:27:04AM +0200, Thomas Petazzoni wrote:
> > >> >This patchs adds the CONFIG_AIO option which allows to remove support
> > >> >for asynchronous I/O operations, that are not necessarly used by
> > >> >applications, particularly on embedded devices. As this is a
> > >> >size-reduction option, it depends on CONFIG_EMBEDDED. It allows to
> > >> >save ~7 kilobytes of kernel code/data:
> > >> 
> > >> Shouldn't this also make sure not to install aio_abi.h or at least an
> > >> empty aio_abi.h?
> > >
> > >The userspace headers are independent of any kernel configuration
> > >(except for the architecture).
> > 
> > I beg to disagree:
> > internals as exposed by e.g. aio_abi.h are impl dependent. Noone except
> > the impl and it's users are interrested in it.
> >...
> 
> That's utter bullshit.
> 
> The contents of aio_abi.h is a kernel<->userspace ABI that mustn't ever 
> change. [1]

Case in point:

I want to be able to compile an application for embedded Linux which
*can use* Linux-AIO, but can also run on a kernel which has Linux-AIO
removed by this patch.

I still want to compile the application with that capability, in case
it's run on another kernel with it enabled.

I shouldn't have to have a separate, special kernel with all options
enabled, just to compile applications that run on multiple kernels and
use run-time features when available.

Just like all the other kernel<->userspace interfaces, the header
files (including their presence) shouldn't depend on kernel
configuration at all.

-- Jamie

Re: [patch 1/4] Configure out AIO support

[Bernhard Fischer]

On Tue, Aug 05, 2008 at 07:26:07PM +0100, Jamie Lokier wrote:

>> > >The userspace headers are independent of any kernel configuration
>> > >(except for the architecture).
>> > 
>> > I beg to disagree:
>> > internals as exposed by e.g. aio_abi.h are impl dependent. Noone except
>> > the impl and it's users are interrested in it.
>> >...
>> 
>> That's utter bullshit.
>> 
>> The contents of aio_abi.h is a kernel<->userspace ABI that mustn't ever 
>> change. [1]
>
>Case in point:
>
>I want to be able to compile an application for embedded Linux which
>*can use* Linux-AIO, but can also run on a kernel which has Linux-AIO
>removed by this patch.
>
>I still want to compile the application with that capability, in case
>it's run on another kernel with it enabled.
>
>I shouldn't have to have a separate, special kernel with all options
>enabled, just to compile applications that run on multiple kernels and
>use run-time features when available.
>
>Just like all the other kernel<->userspace interfaces, the header
>files (including their presence) shouldn't depend on kernel
>configuration at all.

alright, makes perfect sense. I must have been playing too much with
libc recently, i guess.

thanks,

[patch 2/4] Configure out file locking features

[Thomas Petazzoni]

[-- Attachment #1: configure-out-fs-locks-support.patch --]
[-- Type: text/plain, Size: 8797 bytes --]

This patch adds the CONFIG_FILE_LOCKING option which allows to remove
support for advisory locks. With this patch enabled, the flock()
system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
and NFS support are disabled. These features are not necessarly needed
on embedded systems. It allows to save ~11 Kb of kernel code and data:

   text	   data	    bss	    dec	    hex	filename
1125436	 118764	 212992	1457192	 163c28	vmlinux.old
1114299	 118564	 212992	1445855	 160fdf	vmlinux
 -11137    -200       0  -11337   -2C49 +/-

This patch has originally been written by Matt Mackall
<mpm@selenic.com>, and is part of the Linux Tiny project.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Matt Mackall <mpm@selenic.com>
Cc: matthew@wil.cx
Cc: linux-fsdevel@vger.kernel.org
Cc: mpm@selenic.com
Cc: akpm@linux-foundation.org

---
 fs/Kconfig          |    2 +-
 fs/Makefile         |    3 ++-
 fs/proc/proc_misc.c |    4 ++++
 include/linux/fs.h  |   52 +++++++++++++++++++++++++++++++++++++++++++++-------
 init/Kconfig        |    8 ++++++++
 kernel/sys_ni.c     |    1 +
 kernel/sysctl.c     |    6 +++++-
 7 files changed, 66 insertions(+), 10 deletions(-)

Index: linuxdev/fs/Kconfig
===================================================================
--- linuxdev.orig/fs/Kconfig
+++ linuxdev/fs/Kconfig
@@ -1559,7 +1559,7 @@
 
 config NFS_FS
 	tristate "NFS client support"
-	depends on INET
+	depends on INET && FILE_LOCKING
 	select LOCKD
 	select SUNRPC
 	select NFS_ACL_SUPPORT if NFS_V3_ACL
Index: linuxdev/fs/Makefile
===================================================================
--- linuxdev.orig/fs/Makefile
+++ linuxdev/fs/Makefile
@@ -7,7 +7,7 @@
 
 obj-y :=	open.o read_write.o file_table.o super.o \
 		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
-		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
+		ioctl.o readdir.o select.o fifo.o dcache.o inode.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o drop_caches.o splice.o sync.o utimes.o \
@@ -28,6 +28,7 @@
 obj-$(CONFIG_TIMERFD)		+= timerfd.o
 obj-$(CONFIG_EVENTFD)		+= eventfd.o
 obj-$(CONFIG_AIO)               += aio.o
+obj-$(CONFIG_FILE_LOCKING)      += locks.o
 obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
 
 nfsd-$(CONFIG_NFSD)		:= nfsctl.o
Index: linuxdev/fs/proc/proc_misc.c
===================================================================
--- linuxdev.orig/fs/proc/proc_misc.c
+++ linuxdev/fs/proc/proc_misc.c
@@ -677,6 +677,7 @@
 	return proc_calc_metrics(page, start, off, count, eof, len);
 }
 
+#ifdef CONFIG_FILE_LOCKING
 static int locks_open(struct inode *inode, struct file *filp)
 {
 	return seq_open(filp, &locks_seq_operations);
@@ -688,6 +689,7 @@
 	.llseek		= seq_lseek,
 	.release	= seq_release,
 };
+#endif /* CONFIG_FILE_LOCKING */
 
 static int execdomains_read_proc(char *page, char **start, off_t off,
 				 int count, int *eof, void *data)
@@ -881,7 +883,9 @@
 #ifdef CONFIG_PRINTK
 	proc_create("kmsg", S_IRUSR, NULL, &proc_kmsg_operations);
 #endif
+#ifdef CONFIG_FILE_LOCKING
 	proc_create("locks", 0, NULL, &proc_locks_operations);
+#endif
 	proc_create("devices", 0, NULL, &proc_devinfo_operations);
 	proc_create("cpuinfo", 0, NULL, &proc_cpuinfo_operations);
 #ifdef CONFIG_BLOCK
Index: linuxdev/include/linux/fs.h
===================================================================
--- linuxdev.orig/include/linux/fs.h
+++ linuxdev/include/linux/fs.h
@@ -983,6 +983,13 @@
 
 #include <linux/fcntl.h>
 
+extern void send_sigio(struct fown_struct *fown, int fd, int band);
+
+/* fs/sync.c */
+extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
+			loff_t endbyte, unsigned int flags);
+
+#ifdef CONFIG_FILE_LOCKING
 extern int fcntl_getlk(struct file *, struct flock __user *);
 extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
 			struct flock __user *);
@@ -993,14 +1000,9 @@
 			struct flock64 __user *);
 #endif
 
-extern void send_sigio(struct fown_struct *fown, int fd, int band);
 extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
 extern int fcntl_getlease(struct file *filp);
 
-/* fs/sync.c */
-extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
-			loff_t endbyte, unsigned int flags);
-
 /* fs/locks.c */
 extern void locks_init_lock(struct file_lock *);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
@@ -1023,6 +1025,33 @@
 extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
 extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
 extern struct seq_operations locks_seq_operations;
+#else /* !CONFIG_FILE_LOCKING */
+#define fcntl_getlk(a, b) (-EINVAL)
+#define fcntl_setlk(a, b, c, d) (-EACCES)
+#if BITS_PER_LONG == 32
+#define fcntl_getlk64(a, b) (-EINVAL)
+#define fcntl_setlk64(a, b, c, d) (-EACCES)
+#endif
+#define fcntl_setlease(a, b, c) (0)
+#define fcntl_getlease(a) (0)
+#define locks_init_lock(a)
+#define locks_copy_lock(a, b)
+#define locks_remove_posix(a, b)
+#define locks_remove_flock(a)
+#define posix_test_lock(a, b) (0)
+#define posix_lock_file(a, b) (-ENOLCK)
+#define posix_lock_file_wait(a, b) (-ENOLCK)
+#define posix_unblock_lock(a, b) (-ENOENT)
+#define vfs_test_lock(a, b) (0)
+#define vfs_lock_file(a, b, c, d) (-ENOLCK)
+#define vfs_cancel_lock(a, b) (0)
+#define flock_lock_file_wait(a, b) (-ENOLCK)
+#define __break_lease(a, b) (0)
+#define lease_get_mtime(a, b)
+#define lock_may_read(a, b, c) (1)
+#define lock_may_write(a, b, c) (1)
+#endif /* !CONFIG_FILE_LOCKING */
+
 
 struct fasync_struct {
 	int	magic;
@@ -1554,9 +1583,12 @@
 /* /sys/fs */
 extern struct kobject *fs_kobj;
 
+extern int rw_verify_area(int, struct file *, loff_t *, size_t);
+
 #define FLOCK_VERIFY_READ  1
 #define FLOCK_VERIFY_WRITE 2
 
+#ifdef CONFIG_FILE_LOCKING
 extern int locks_mandatory_locked(struct inode *);
 extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
 
@@ -1587,8 +1619,6 @@
 	return 0;
 }
 
-extern int rw_verify_area(int, struct file *, loff_t *, size_t);
-
 static inline int locks_verify_truncate(struct inode *inode,
 				    struct file *filp,
 				    loff_t size)
@@ -1609,6 +1639,14 @@
 		return __break_lease(inode, mode);
 	return 0;
 }
+#else /* !CONFIG_FILE_LOCKING */
+#define locks_mandatory_locked(a) (0)
+#define locks_mandatory_area(a, b, c, d, e) (0)
+#define mandatory_lock(a) (0)
+#define locks_verify_locked(a) (0)
+#define locks_verify_truncate(a, b, c) (0)
+#define break_lease(a, b) (0)
+#endif /* CONFIG_FILE_LOCKING */
 
 /* fs/open.c */
 
Index: linuxdev/init/Kconfig
===================================================================
--- linuxdev.orig/init/Kconfig
+++ linuxdev/init/Kconfig
@@ -732,6 +732,14 @@
           by some high performance threaded applications. Disabling
           this option saves about 7k.
 
+config FILE_LOCKING
+	bool "Enable POSIX file locking API" if EMBEDDED
+	default y
+	help
+	  This option enables standard file locking support, required
+          for filesystems like NFS and for the flock() system
+          call. Disabling this option saves about 11k.
+
 config VM_EVENT_COUNTERS
 	default y
 	bool "Enable VM event counters for /proc/vmstat" if EMBEDDED
Index: linuxdev/kernel/sys_ni.c
===================================================================
--- linuxdev.orig/kernel/sys_ni.c
+++ linuxdev/kernel/sys_ni.c
@@ -130,6 +130,7 @@
 cond_syscall(sys_io_submit);
 cond_syscall(sys_io_cancel);
 cond_syscall(sys_io_getevents);
+cond_syscall(sys_flock);
 
 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read);
Index: linuxdev/kernel/sysctl.c
===================================================================
--- linuxdev.orig/kernel/sysctl.c
+++ linuxdev/kernel/sysctl.c
@@ -97,7 +97,7 @@
 static int neg_one = -1;
 #endif
 
-#ifdef CONFIG_MMU
+#if defined(CONFIG_MMU) && defined(CONFIG_FILE_LOCKING)
 static int two = 2;
 #endif
 
@@ -1260,6 +1260,7 @@
 		.extra1		= &minolduid,
 		.extra2		= &maxolduid,
 	},
+#ifdef CONFIG_FILE_LOCKING
 	{
 		.ctl_name	= FS_LEASES,
 		.procname	= "leases-enable",
@@ -1268,6 +1269,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+#endif
 #ifdef CONFIG_DNOTIFY
 	{
 		.ctl_name	= FS_DIR_NOTIFY,
@@ -1279,6 +1281,7 @@
 	},
 #endif
 #ifdef CONFIG_MMU
+#ifdef CONFIG_FILE_LOCKING
 	{
 		.ctl_name	= FS_LEASE_TIME,
 		.procname	= "lease-break-time",
@@ -1290,6 +1293,7 @@
 		.extra1		= &zero,
 		.extra2		= &two,
 	},
+#endif /* CONFIG_FILE_LOCKING */
 #ifdef CONFIG_AIO
 	{
 		.procname	= "aio-nr",

-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 2/4] Configure out file locking features

[Adrian Bunk]

On Thu, Jul 31, 2008 at 11:27:05AM +0200, Thomas Petazzoni wrote:
> This patch adds the CONFIG_FILE_LOCKING option which allows to remove
> support for advisory locks. With this patch enabled, the flock()
> system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
> and NFS support are disabled. These features are not necessarly needed
> on embedded systems. It allows to save ~11 Kb of kernel code and data:
> 
>    text	   data	    bss	    dec	    hex	filename
> 1125436	 118764	 212992	1457192	 163c28	vmlinux.old
> 1114299	 118564	 212992	1445855	 160fdf	vmlinux
>  -11137    -200       0  -11337   -2C49 +/-
> 
> This patch has originally been written by Matt Mackall
> <mpm@selenic.com>, and is part of the Linux Tiny project.
>...

As I've already said in the past I'm personally not a huge fan of these 
patches, but if it brings advantages in real-life situations it's hard
to argue against it.

In which use cases can users safely disable this option, and on what 
devices have you verified that CONFIG_FILE_LOCKING=n kernels actually 
work in practice?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 2/4] Configure out file locking features

[Thomas Petazzoni]

Le Thu, 31 Jul 2008 16:53:19 +0300,
Adrian Bunk <bunk@kernel.org> a écrit :

> As I've already said in the past I'm personally not a huge fan of
> these patches, but if it brings advantages in real-life situations
> it's hard to argue against it.

Yes, I've seen your points about that kind of patches on
linux-embedded, and I understand them. I agree that adding dozens and
dozens of configuration items for small features doesn't look like
something sustainable on the long run. However, the kernel keeps
growing and this isn't sustainable either on the long run for *some*
embedded users. So, what should we do ? (That's a real question)

Some numbers about a bootable x86 allnoconfig kernel with ELF, ext2 and
IDE support :

   text	   data	    bss	    dec	    hex	filename
1110389	 119468	 217088	1446945	 161421	vmlinux.2.6.26
1134606	 118840	 212992	1466438	 166046	vmlinux.2.6.27-rc1
  24217    -628   -4096   19493    4C25 +/-

(The only configuration change between the two kernels is
CONFIG_FW_LOADER n->y, which pulls drivers/base/firmware_class.o, 3k).

> In which use cases can users safely disable this option, and on what 
> devices have you verified that CONFIG_FILE_LOCKING=n kernels actually 
> work in practice?

As long as they don't use NFS (realistic in many production
environments) and that the applications do not rely on advisory locking
(flock() and fnctl() F_GETLK and F_SETLK), file locking can be
disabled. In practice, I only tested a CONFIG_FILE_LOCKING=n kernel
with a basic Busybox under Qemu.

Sincerly,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 2/4] Configure out file locking features

[Adrian Bunk]

On Thu, Jul 31, 2008 at 04:20:07PM +0200, Thomas Petazzoni wrote:
> Le Thu, 31 Jul 2008 16:53:19 +0300,
> Adrian Bunk <bunk@kernel.org> a écrit :
> 
> > As I've already said in the past I'm personally not a huge fan of
> > these patches, but if it brings advantages in real-life situations
> > it's hard to argue against it.
> 
> Yes, I've seen your points about that kind of patches on
> linux-embedded, and I understand them. I agree that adding dozens and
> dozens of configuration items for small features doesn't look like
> something sustainable on the long run. However, the kernel keeps
> growing and this isn't sustainable either on the long run for *some*
> embedded users. So, what should we do ? (That's a real question)

I'm not against making the kernel smaller.

I'm just not a fan of adding config options for each few kB of code - 
we have to maintain them and the more complex the configuration becomes 
the more often it breaks.

> Some numbers about a bootable x86 allnoconfig kernel with ELF, ext2 and
> IDE support :
> 
>    text	   data	    bss	    dec	    hex	filename
> 1110389	 119468	 217088	1446945	 161421	vmlinux.2.6.26
> 1134606	 118840	 212992	1466438	 166046	vmlinux.2.6.27-rc1
>   24217    -628   -4096   19493    4C25 +/-

What became bigger was most likely not related to the patches you sent.

Where and why did the kernel become bigger?

> (The only configuration change between the two kernels is
> CONFIG_FW_LOADER n->y, which pulls drivers/base/firmware_class.o, 3k).

Why did CONFIG_FW_LOADER get enabled?
Due to alnoconfig disabling CONFIG_EMBEDDED?

> > In which use cases can users safely disable this option, and on what 
> > devices have you verified that CONFIG_FILE_LOCKING=n kernels actually 
> > work in practice?
> 
> As long as they don't use NFS (realistic in many production
> environments) and that the applications do not rely on advisory locking
> (flock() and fnctl() F_GETLK and F_SETLK), file locking can be
> disabled.

But what does that mean in practice?

A user will ask:
I'm using $applications with $libraries, can I safely disable this option?

And e.g. according to a quick grep through the sources uClibc's 
updwtmp() seems to cease working without flock().

It costs us maintainance of the option and the #ifdef's and gives users 
one way more to shoot themselves into the foot in nontrivial to detect 
ways.

> In practice, I only tested a CONFIG_FILE_LOCKING=n kernel
> with a basic Busybox under Qemu.
> 
> Sincerly,
> 
> Thomas

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 2/4] Configure out file locking features

[Thomas Petazzoni]

Le Thu, 31 Jul 2008 18:37:57 +0300,
Adrian Bunk <bunk@kernel.org> a écrit :

> I'm just not a fan of adding config options for each few kB of code - 
> we have to maintain them and the more complex the configuration
> becomes the more often it breaks.

I'm not a fan of these too, but are there other solutions ?

> What became bigger was most likely not related to the patches you
> sent.

No, it is not.

> Where and why did the kernel become bigger?

It's not up-to-date with 2.6.26 and 2.6.27-rc1, but Bloatwatch
<http://www.selenic.com/bloatwatch/>, by Matt Mackall, is here to
answer these questions. I haven't made the analysis for
2.6.26->2.6.27-rc1.

> Why did CONFIG_FW_LOADER get enabled?
> Due to alnoconfig disabling CONFIG_EMBEDDED?

I don't know. Haven't made the analysis for now.

> A user will ask:
> I'm using $applications with $libraries, can I safely disable this
> option?

Hard to tell in the general case.

> And e.g. according to a quick grep through the sources uClibc's 
> updwtmp() seems to cease working without flock().

Correct. But on many embedded systems, we don't care about logging past
user logins. We might even not care about logins at all.

> It costs us maintainance of the option and the #ifdef's and gives
> users one way more to shoot themselves into the foot in nontrivial to
> detect ways.

That's correct, and as I said previously, I fully understand the
maintainance problem of all these new configuration options. I must
admit that I do not really have more objective technical arguments that
would help us deciding whether the code size reduction vs. code
maintainance choice should be made in one direction or the other.

Sincerly,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 2/4] Configure out file locking features

[Adrian Bunk]

On Thu, Jul 31, 2008 at 06:26:16PM +0200, Thomas Petazzoni wrote:
> Le Thu, 31 Jul 2008 18:37:57 +0300,
> Adrian Bunk <bunk@kernel.org> a écrit :
> 
> > I'm just not a fan of adding config options for each few kB of code - 
> > we have to maintain them and the more complex the configuration
> > becomes the more often it breaks.
> 
> I'm not a fan of these too, but are there other solutions ?

There are many things that can be done to reduce the kernel size or try 
to minimize the growth of the kernel.

E.g. working on --combine -fwhole-program (where David once had 
preliminary patches for the per-module approach) might be better.

> > What became bigger was most likely not related to the patches you
> > sent.
> 
> No, it is not.
> 
> > Where and why did the kernel become bigger?
> 
> It's not up-to-date with 2.6.26 and 2.6.27-rc1, but Bloatwatch
> <http://www.selenic.com/bloatwatch/>, by Matt Mackall, is here to
> answer these questions. I haven't made the analysis for
> 2.6.26->2.6.27-rc1.

It can only give some hints where to start searching.

But it tracks a defconfig, and e.g. the nearly doubled size between
2.6.18 and 2.6.19 is both expected and not a problem for embedded
systems.

The real work is to figure out in which areas that are relevant for 
embedded systems the kernel became bigger.

> > Why did CONFIG_FW_LOADER get enabled?
> > Due to alnoconfig disabling CONFIG_EMBEDDED?
> 
> I don't know. Haven't made the analysis for now.
> 
> > A user will ask:
> > I'm using $applications with $libraries, can I safely disable this
> > option?
> 
> Hard to tell in the general case.
> 
> > And e.g. according to a quick grep through the sources uClibc's 
> > updwtmp() seems to cease working without flock().
> 
> Correct. But on many embedded systems, we don't care about logging past
> user logins. We might even not care about logins at all.

And for embedded systems with which applications is it 100% safe to 
disable this option?

And don't answer "doesn't use flock()", I want a real-life example of a 
device where you could guarantee a developer that disabling this option 
in his product would be safe.

> > It costs us maintainance of the option and the #ifdef's and gives
> > users one way more to shoot themselves into the foot in nontrivial to
> > detect ways.
> 
> That's correct, and as I said previously, I fully understand the
> maintainance problem of all these new configuration options. I must
> admit that I do not really have more objective technical arguments that
> would help us deciding whether the code size reduction vs. code
> maintainance choice should be made in one direction or the other.

My personal criteron for this patch is still how many real-life systems 
can safely disable it.

> Sincerly,
> 
> Thomas

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

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

Re: [patch 2/4] Configure out file locking features

[David Woodhouse]

On Thu, 2008-07-31 at 19:49 +0300, Adrian Bunk wrote:
> On Thu, Jul 31, 2008 at 06:26:16PM +0200, Thomas Petazzoni wrote:
> > Le Thu, 31 Jul 2008 18:37:57 +0300,
> > Adrian Bunk <bunk@kernel.org> a écrit :
> > 
> > > I'm just not a fan of adding config options for each few kB of code - 
> > > we have to maintain them and the more complex the configuration
> > > becomes the more often it breaks.
> > 
> > I'm not a fan of these too, but are there other solutions ?
> 
> There are many things that can be done to reduce the kernel size or try 
> to minimize the growth of the kernel.
> 
> E.g. working on --combine -fwhole-program (where David once had 
> preliminary patches for the per-module approach) might be better.

Yeah, I'm planning to dig that out again and play with it some time
soon. The Kbuild issues were too scary at the time, but I'm less
frightened of it these days...

Segher has also been looking at it, and reported quite a useful win when
he used it to combine arch/$ARCH/mm and mm/, and arch/$ARCH/kernel and
kernel/.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 2/4] Configure out file locking features

[Tim Bird]

Adrian Bunk wrote:
> And for embedded systems with which applications is it 100% safe to 
> disable this option?

Sony's digital cameras.

This option *is* disabled in the kernel for (most) Sony digital cameras.
Those digital cameras have the kernel, busybox, a custom C library,
and one proprietary application.   The application does not use
flock() (or AIO, or ethtool or multi-cast)

These cameras were heavily tested, and are shipping now.  I can't
make any guarantees for other developers, but those of us who
are careful about our application development would like the option
to eliminate completely unused features from the kernel.  (And
the C library, but that's a different issue.)

> And don't answer "doesn't use flock()", I want a real-life example of a 
> device where you could guarantee a developer that disabling this option 
> in his product would be safe.

I'm not sure why a guarantee is required that other developers
use this option safely.  Maybe this is a point of disconnect between
embedded folks and non-embedded folks.  We're accustomed to making
tradeoff decisions that only affect our product, and which
we take full responsibility for testing.

If warnings or support avoidance for the general population using
such config options is the issue, I think that David Woodhouse's
suggestion that such things could taint the kernel is an interesting
idea. Maybe have we could have an "unsafe-config" taint flag?

I should add that I am sympathetic with the larger issue you raise
about nibbling at the bottom with patches that only address a
few KB of the problem, while the size continues to build (to an
even greater degree) with each release.  My response is that I agree
with you on the nibbling bit, but probably just at a different
level of KB savings.

That is, I presume you'd be OK with something that saved 100K or
even 20K, but balk at bit at these patches, which save 10k or less.
My threshold is lower (probably down to about 5K, so these are
pretty close to the bubble), but even I wouldn't recommend
applying anything much below that.  We've already started
considering to drop some linux-tiny patches that just don't save
enough to warrant continued maintenance.
 -- Tim


=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

Re: [patch 2/4] Configure out file locking features

[Robert Schwebel]

On Thu, Jul 31, 2008 at 10:32:20AM -0700, Tim Bird wrote:
> > And for embedded systems with which applications is it 100% safe to
> > disable this option?
>
> Sony's digital cameras.

We have also several very small automation & measurement devices in the
field which run a very dedicated more or less single application which
can be carefully audited.

In the OSADL safety working group, people are discussing about Linux for
safety critical applications. If we want to achieve such scenarios,
stripped-down systems are an absolute must.

Although increasing processor power even in embedded applications lead
to more and more standard-line kernels plus normal userspace components
(i.e. glibc instead of uclibc), there are still applications where the
final product consists only of the kernel plus the app, whereas during
development time, the system has a full-blown configuration.

rsc
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [patch 2/4] Configure out file locking features

[Adrian Bunk]

On Thu, Jul 31, 2008 at 08:12:14PM +0200, Robert Schwebel wrote:
> On Thu, Jul 31, 2008 at 10:32:20AM -0700, Tim Bird wrote:
> > > And for embedded systems with which applications is it 100% safe to
> > > disable this option?
> >
> > Sony's digital cameras.
> 
> We have also several very small automation & measurement devices in the
> field which run a very dedicated more or less single application which
> can be carefully audited.
> 
> In the OSADL safety working group, people are discussing about Linux for
> safety critical applications. If we want to achieve such scenarios,
> stripped-down systems are an absolute must.
>...

My first reaction is that as soon as you enable CONFIG_EMBEDDED you can 
easily enter codepaths noone else has used for a while and that got 
unnoticed broken.

> rsc

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 2/4] Configure out file locking features

[Robert Schwebel]

On Thu, Jul 31, 2008 at 10:31:26PM +0300, Adrian Bunk wrote:
> > In the OSADL safety working group, people are discussing about Linux
> > for safety critical applications. If we want to achieve such
> > scenarios, stripped-down systems are an absolute must.
> >...
>
> My first reaction is that as soon as you enable CONFIG_EMBEDDED you
> can easily enter codepaths noone else has used for a while and that
> got unnoticed broken.

That may be no problem; if we ever come to a safety kernel, it will have
to be audited and carefully tested anyway.

rsc
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [patch 2/4] Configure out file locking features

[Adrian Bunk]

On Thu, Jul 31, 2008 at 10:32:20AM -0700, Tim Bird wrote:
> Adrian Bunk wrote:
> > And for embedded systems with which applications is it 100% safe to 
> > disable this option?
> 
> Sony's digital cameras.
> 
> This option *is* disabled in the kernel for (most) Sony digital cameras.
> Those digital cameras have the kernel, busybox, a custom C library,
> and one proprietary application.   The application does not use
> flock() (or AIO, or ethtool or multi-cast)
> 
> These cameras were heavily tested, and are shipping now.  I can't
> make any guarantees for other developers, but those of us who
> are careful about our application development would like the option
> to eliminate completely unused features from the kernel.  (And
> the C library, but that's a different issue.)
> 
> > And don't answer "doesn't use flock()", I want a real-life example of a 
> > device where you could guarantee a developer that disabling this option 
> > in his product would be safe.
> 
> I'm not sure why a guarantee is required that other developers
> use this option safely.  Maybe this is a point of disconnect between
> embedded folks and non-embedded folks.  We're accustomed to making
> tradeoff decisions that only affect our product, and which
> we take full responsibility for testing.

Thanks. That's quite different from Thomas' "In practice, I only tested
a CONFIG_FILE_LOCKING=n kernel with a basic Busybox under Qemu." and
addresses my concerns.

In case I didn't express myself clearly:
I was not interested in guarantees for random developers but in seeing 
some reasonable use case for a real device.

> If warnings or support avoidance for the general population using
> such config options is the issue, I think that David Woodhouse's
> suggestion that such things could taint the kernel is an interesting
> idea. Maybe have we could have an "unsafe-config" taint flag?

A CONFIG_EMBEDDED=y flag?

> I should add that I am sympathetic with the larger issue you raise
> about nibbling at the bottom with patches that only address a
> few KB of the problem, while the size continues to build (to an
> even greater degree) with each release.  My response is that I agree
> with you on the nibbling bit, but probably just at a different
> level of KB savings.
> 
> That is, I presume you'd be OK with something that saved 100K or
> even 20K, but balk at bit at these patches, which save 10k or less.
> My threshold is lower (probably down to about 5K, so these are
> pretty close to the bubble), but even I wouldn't recommend
> applying anything much below that.  We've already started
> considering to drop some linux-tiny patches that just don't save
> enough to warrant continued maintenance.

It's not only about limits, I also have a general dislike for the
"add more config options" approach.

I get your point why it brings advantages in some cases, but if you are 
looking for a cheerleader it won't be me.  ;-)

>  -- Tim

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 2/4] Configure out file locking features

[Tim Bird]

Adrian Bunk wrote:
> On Thu, Jul 31, 2008 at 10:32:20AM -0700, Tim Bird wrote:
>> If warnings or support avoidance for the general population using
>> such config options is the issue, I think that David Woodhouse's
>> suggestion that such things could taint the kernel is an interesting
>> idea. Maybe have we could have an "unsafe-config" taint flag?
> 
> A CONFIG_EMBEDDED=y flag?

If this is sufficient, it works for me!

> It's not only about limits, I also have a general dislike for the
> "add more config options" approach.
> 
> I get your point why it brings advantages in some cases, but if you are 
> looking for a cheerleader it won't be me.  ;-)

Understood.  I promise not to send you any pom-poms. ;-)
 -- Tim


=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

Re: [patch 2/4] Configure out file locking features

[J. Bruce Fields]

On Thu, Jul 31, 2008 at 11:27:05AM +0200, Thomas Petazzoni wrote:
> This patch adds the CONFIG_FILE_LOCKING option which allows to remove
> support for advisory locks. With this patch enabled, the flock()
> system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
> and NFS support are disabled. These features are not necessarly needed
> on embedded systems. It allows to save ~11 Kb of kernel code and data:

Out of curiosity, why does the nfs client need disabling, but not nfsd,
gfs2, fuse, etc.?

--b.

> 
>    text	   data	    bss	    dec	    hex	filename
> 1125436	 118764	 212992	1457192	 163c28	vmlinux.old
> 1114299	 118564	 212992	1445855	 160fdf	vmlinux
>  -11137    -200       0  -11337   -2C49 +/-
> 
> This patch has originally been written by Matt Mackall
> <mpm@selenic.com>, and is part of the Linux Tiny project.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Matt Mackall <mpm@selenic.com>
> Cc: matthew@wil.cx
> Cc: linux-fsdevel@vger.kernel.org
> Cc: mpm@selenic.com
> Cc: akpm@linux-foundation.org
> 
> ---
>  fs/Kconfig          |    2 +-
>  fs/Makefile         |    3 ++-
>  fs/proc/proc_misc.c |    4 ++++
>  include/linux/fs.h  |   52 +++++++++++++++++++++++++++++++++++++++++++++-------
>  init/Kconfig        |    8 ++++++++
>  kernel/sys_ni.c     |    1 +
>  kernel/sysctl.c     |    6 +++++-
>  7 files changed, 66 insertions(+), 10 deletions(-)
> 
> Index: linuxdev/fs/Kconfig
> ===================================================================
> --- linuxdev.orig/fs/Kconfig
> +++ linuxdev/fs/Kconfig
> @@ -1559,7 +1559,7 @@
>  
>  config NFS_FS
>  	tristate "NFS client support"
> -	depends on INET
> +	depends on INET && FILE_LOCKING
>  	select LOCKD
>  	select SUNRPC
>  	select NFS_ACL_SUPPORT if NFS_V3_ACL
> Index: linuxdev/fs/Makefile
> ===================================================================
> --- linuxdev.orig/fs/Makefile
> +++ linuxdev/fs/Makefile
> @@ -7,7 +7,7 @@
>  
>  obj-y :=	open.o read_write.o file_table.o super.o \
>  		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
> -		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
> +		ioctl.o readdir.o select.o fifo.o dcache.o inode.o \
>  		attr.o bad_inode.o file.o filesystems.o namespace.o \
>  		seq_file.o xattr.o libfs.o fs-writeback.o \
>  		pnode.o drop_caches.o splice.o sync.o utimes.o \
> @@ -28,6 +28,7 @@
>  obj-$(CONFIG_TIMERFD)		+= timerfd.o
>  obj-$(CONFIG_EVENTFD)		+= eventfd.o
>  obj-$(CONFIG_AIO)               += aio.o
> +obj-$(CONFIG_FILE_LOCKING)      += locks.o
>  obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
>  
>  nfsd-$(CONFIG_NFSD)		:= nfsctl.o
> Index: linuxdev/fs/proc/proc_misc.c
> ===================================================================
> --- linuxdev.orig/fs/proc/proc_misc.c
> +++ linuxdev/fs/proc/proc_misc.c
> @@ -677,6 +677,7 @@
>  	return proc_calc_metrics(page, start, off, count, eof, len);
>  }
>  
> +#ifdef CONFIG_FILE_LOCKING
>  static int locks_open(struct inode *inode, struct file *filp)
>  {
>  	return seq_open(filp, &locks_seq_operations);
> @@ -688,6 +689,7 @@
>  	.llseek		= seq_lseek,
>  	.release	= seq_release,
>  };
> +#endif /* CONFIG_FILE_LOCKING */
>  
>  static int execdomains_read_proc(char *page, char **start, off_t off,
>  				 int count, int *eof, void *data)
> @@ -881,7 +883,9 @@
>  #ifdef CONFIG_PRINTK
>  	proc_create("kmsg", S_IRUSR, NULL, &proc_kmsg_operations);
>  #endif
> +#ifdef CONFIG_FILE_LOCKING
>  	proc_create("locks", 0, NULL, &proc_locks_operations);
> +#endif
>  	proc_create("devices", 0, NULL, &proc_devinfo_operations);
>  	proc_create("cpuinfo", 0, NULL, &proc_cpuinfo_operations);
>  #ifdef CONFIG_BLOCK
> Index: linuxdev/include/linux/fs.h
> ===================================================================
> --- linuxdev.orig/include/linux/fs.h
> +++ linuxdev/include/linux/fs.h
> @@ -983,6 +983,13 @@
>  
>  #include <linux/fcntl.h>
>  
> +extern void send_sigio(struct fown_struct *fown, int fd, int band);
> +
> +/* fs/sync.c */
> +extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
> +			loff_t endbyte, unsigned int flags);
> +
> +#ifdef CONFIG_FILE_LOCKING
>  extern int fcntl_getlk(struct file *, struct flock __user *);
>  extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
>  			struct flock __user *);
> @@ -993,14 +1000,9 @@
>  			struct flock64 __user *);
>  #endif
>  
> -extern void send_sigio(struct fown_struct *fown, int fd, int band);
>  extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
>  extern int fcntl_getlease(struct file *filp);
>  
> -/* fs/sync.c */
> -extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
> -			loff_t endbyte, unsigned int flags);
> -
>  /* fs/locks.c */
>  extern void locks_init_lock(struct file_lock *);
>  extern void locks_copy_lock(struct file_lock *, struct file_lock *);
> @@ -1023,6 +1025,33 @@
>  extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
>  extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
>  extern struct seq_operations locks_seq_operations;
> +#else /* !CONFIG_FILE_LOCKING */
> +#define fcntl_getlk(a, b) (-EINVAL)
> +#define fcntl_setlk(a, b, c, d) (-EACCES)
> +#if BITS_PER_LONG == 32
> +#define fcntl_getlk64(a, b) (-EINVAL)
> +#define fcntl_setlk64(a, b, c, d) (-EACCES)
> +#endif
> +#define fcntl_setlease(a, b, c) (0)
> +#define fcntl_getlease(a) (0)
> +#define locks_init_lock(a)
> +#define locks_copy_lock(a, b)
> +#define locks_remove_posix(a, b)
> +#define locks_remove_flock(a)
> +#define posix_test_lock(a, b) (0)
> +#define posix_lock_file(a, b) (-ENOLCK)
> +#define posix_lock_file_wait(a, b) (-ENOLCK)
> +#define posix_unblock_lock(a, b) (-ENOENT)
> +#define vfs_test_lock(a, b) (0)
> +#define vfs_lock_file(a, b, c, d) (-ENOLCK)
> +#define vfs_cancel_lock(a, b) (0)
> +#define flock_lock_file_wait(a, b) (-ENOLCK)
> +#define __break_lease(a, b) (0)
> +#define lease_get_mtime(a, b)
> +#define lock_may_read(a, b, c) (1)
> +#define lock_may_write(a, b, c) (1)
> +#endif /* !CONFIG_FILE_LOCKING */
> +
>  
>  struct fasync_struct {
>  	int	magic;
> @@ -1554,9 +1583,12 @@
>  /* /sys/fs */
>  extern struct kobject *fs_kobj;
>  
> +extern int rw_verify_area(int, struct file *, loff_t *, size_t);
> +
>  #define FLOCK_VERIFY_READ  1
>  #define FLOCK_VERIFY_WRITE 2
>  
> +#ifdef CONFIG_FILE_LOCKING
>  extern int locks_mandatory_locked(struct inode *);
>  extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
>  
> @@ -1587,8 +1619,6 @@
>  	return 0;
>  }
>  
> -extern int rw_verify_area(int, struct file *, loff_t *, size_t);
> -
>  static inline int locks_verify_truncate(struct inode *inode,
>  				    struct file *filp,
>  				    loff_t size)
> @@ -1609,6 +1639,14 @@
>  		return __break_lease(inode, mode);
>  	return 0;
>  }
> +#else /* !CONFIG_FILE_LOCKING */
> +#define locks_mandatory_locked(a) (0)
> +#define locks_mandatory_area(a, b, c, d, e) (0)
> +#define mandatory_lock(a) (0)
> +#define locks_verify_locked(a) (0)
> +#define locks_verify_truncate(a, b, c) (0)
> +#define break_lease(a, b) (0)
> +#endif /* CONFIG_FILE_LOCKING */
>  
>  /* fs/open.c */
>  
> Index: linuxdev/init/Kconfig
> ===================================================================
> --- linuxdev.orig/init/Kconfig
> +++ linuxdev/init/Kconfig
> @@ -732,6 +732,14 @@
>            by some high performance threaded applications. Disabling
>            this option saves about 7k.
>  
> +config FILE_LOCKING
> +	bool "Enable POSIX file locking API" if EMBEDDED
> +	default y
> +	help
> +	  This option enables standard file locking support, required
> +          for filesystems like NFS and for the flock() system
> +          call. Disabling this option saves about 11k.
> +
>  config VM_EVENT_COUNTERS
>  	default y
>  	bool "Enable VM event counters for /proc/vmstat" if EMBEDDED
> Index: linuxdev/kernel/sys_ni.c
> ===================================================================
> --- linuxdev.orig/kernel/sys_ni.c
> +++ linuxdev/kernel/sys_ni.c
> @@ -130,6 +130,7 @@
>  cond_syscall(sys_io_submit);
>  cond_syscall(sys_io_cancel);
>  cond_syscall(sys_io_getevents);
> +cond_syscall(sys_flock);
>  
>  /* arch-specific weak syscall entries */
>  cond_syscall(sys_pciconfig_read);
> Index: linuxdev/kernel/sysctl.c
> ===================================================================
> --- linuxdev.orig/kernel/sysctl.c
> +++ linuxdev/kernel/sysctl.c
> @@ -97,7 +97,7 @@
>  static int neg_one = -1;
>  #endif
>  
> -#ifdef CONFIG_MMU
> +#if defined(CONFIG_MMU) && defined(CONFIG_FILE_LOCKING)
>  static int two = 2;
>  #endif
>  
> @@ -1260,6 +1260,7 @@
>  		.extra1		= &minolduid,
>  		.extra2		= &maxolduid,
>  	},
> +#ifdef CONFIG_FILE_LOCKING
>  	{
>  		.ctl_name	= FS_LEASES,
>  		.procname	= "leases-enable",
> @@ -1268,6 +1269,7 @@
>  		.mode		= 0644,
>  		.proc_handler	= &proc_dointvec,
>  	},
> +#endif
>  #ifdef CONFIG_DNOTIFY
>  	{
>  		.ctl_name	= FS_DIR_NOTIFY,
> @@ -1279,6 +1281,7 @@
>  	},
>  #endif
>  #ifdef CONFIG_MMU
> +#ifdef CONFIG_FILE_LOCKING
>  	{
>  		.ctl_name	= FS_LEASE_TIME,
>  		.procname	= "lease-break-time",
> @@ -1290,6 +1293,7 @@
>  		.extra1		= &zero,
>  		.extra2		= &two,
>  	},
> +#endif /* CONFIG_FILE_LOCKING */
>  #ifdef CONFIG_AIO
>  	{
>  		.procname	= "aio-nr",
> 
> -- 
> Thomas Petazzoni, Free Electrons
> Kernel, drivers and embedded Linux development,
> consulting, training and support.
> http://free-electrons.com
> --
> To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 2/4] Configure out file locking features

[Thomas Petazzoni]

Le Sat, 2 Aug 2008 12:38:48 -0400,
"J. Bruce Fields" <bfields@fieldses.org> a écrit :

> Out of curiosity, why does the nfs client need disabling, but not
> nfsd, gfs2, fuse, etc.?

Then also need disabling. Updated patch below.

Thanks!

Thomas

---

Configure out file locking features

This patch adds the CONFIG_FILE_LOCKING option which allows to remove
support for advisory locks. With this patch enabled, the flock()
system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
and NFS support are disabled. These features are not necessarly needed
on embedded systems. It allows to save ~11 Kb of kernel code and data:

   text	   data	    bss	    dec	    hex	filename
1125436	 118764	 212992	1457192	 163c28	vmlinux.old
1114299	 118564	 212992	1445855	 160fdf	vmlinux
 -11137    -200       0  -11337   -2C49 +/-

This patch has originally been written by Matt Mackall
<mpm@selenic.com>, and is part of the Linux Tiny project.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Matt Mackall <mpm@selenic.com>
Cc: matthew@wil.cx
Cc: linux-fsdevel@vger.kernel.org
Cc: mpm@selenic.com
Cc: akpm@linux-foundation.org

---
 fs/Kconfig          |   15 ++++++++++++++-
 fs/Makefile         |    3 ++-
 fs/dlm/Kconfig      |    1 +
 fs/gfs2/Kconfig     |    1 +
 fs/proc/proc_misc.c |    4 ++++
 include/linux/fs.h  |   52 +++++++++++++++++++++++++++++++++++++++++++++-------
 kernel/sys_ni.c     |    1 +
 kernel/sysctl.c     |    6 +++++-
 8 files changed, 73 insertions(+), 10 deletions(-)

Index: linuxdev/fs/Kconfig
===================================================================
--- linuxdev.orig/fs/Kconfig
+++ linuxdev/fs/Kconfig
@@ -427,12 +427,21 @@
 	bool
 	default n
 
+config FILE_LOCKING
+	bool "Enable POSIX file locking API" if EMBEDDED
+	default y
+	help
+	  This option enables standard file locking support, required
+          for filesystems like NFS and for the flock() system
+          call. Disabling this option saves about 11k.
+
 source "fs/xfs/Kconfig"
 source "fs/gfs2/Kconfig"
 
 config OCFS2_FS
 	tristate "OCFS2 file system support"
 	depends on NET && SYSFS
+	depends on FILE_LOCKING
 	select CONFIGFS_FS
 	select JBD
 	select CRC32
@@ -642,6 +651,7 @@
 
 config FUSE_FS
 	tristate "Filesystem in Userspace support"
+	depends on FILE_LOCKING
 	help
 	  With FUSE it is possible to implement a fully functional filesystem
 	  in a userspace program.
@@ -1567,7 +1577,7 @@
 
 config NFS_FS
 	tristate "NFS client support"
-	depends on INET
+	depends on INET && FILE_LOCKING
 	select LOCKD
 	select SUNRPC
 	select NFS_ACL_SUPPORT if NFS_V3_ACL
@@ -1735,6 +1745,7 @@
 
 config LOCKD
 	tristate
+	depends on FILE_LOCKING
 
 config LOCKD_V4
 	bool
@@ -1870,6 +1881,7 @@
 config CIFS
 	tristate "CIFS support (advanced network filesystem, SMBFS successor)"
 	depends on INET
+	depends on FILE_LOCKING
 	select NLS
 	help
 	  This is the client VFS module for the Common Internet File System
@@ -2059,6 +2071,7 @@
 config AFS_FS
 	tristate "Andrew File System support (AFS) (EXPERIMENTAL)"
 	depends on INET && EXPERIMENTAL
+	depends on FILE_LOCKING
 	select AF_RXRPC
 	help
 	  If you say Y here, you will get an experimental Andrew File System
Index: linuxdev/fs/Makefile
===================================================================
--- linuxdev.orig/fs/Makefile
+++ linuxdev/fs/Makefile
@@ -7,7 +7,7 @@
 
 obj-y :=	open.o read_write.o file_table.o super.o \
 		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
-		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
+		ioctl.o readdir.o select.o fifo.o dcache.o inode.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o drop_caches.o splice.o sync.o utimes.o \
@@ -28,6 +28,7 @@
 obj-$(CONFIG_TIMERFD)		+= timerfd.o
 obj-$(CONFIG_EVENTFD)		+= eventfd.o
 obj-$(CONFIG_AIO)               += aio.o
+obj-$(CONFIG_FILE_LOCKING)      += locks.o
 obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
 
 nfsd-$(CONFIG_NFSD)		:= nfsctl.o
Index: linuxdev/fs/dlm/Kconfig
===================================================================
--- linuxdev.orig/fs/dlm/Kconfig
+++ linuxdev/fs/dlm/Kconfig
@@ -2,6 +2,7 @@
 	tristate "Distributed Lock Manager (DLM)"
 	depends on EXPERIMENTAL && INET
 	depends on SYSFS && (IPV6 || IPV6=n)
+	depends on FILE_LOCKING
 	select CONFIGFS_FS
 	select IP_SCTP
 	help
Index: linuxdev/fs/gfs2/Kconfig
===================================================================
--- linuxdev.orig/fs/gfs2/Kconfig
+++ linuxdev/fs/gfs2/Kconfig
@@ -1,6 +1,7 @@
 config GFS2_FS
 	tristate "GFS2 file system support"
 	depends on EXPERIMENTAL && (64BIT || (LSF && LBD))
+	depends on FILE_LOCKING
 	select FS_POSIX_ACL
 	select CRC32
 	help
Index: linuxdev/fs/proc/proc_misc.c
===================================================================
--- linuxdev.orig/fs/proc/proc_misc.c
+++ linuxdev/fs/proc/proc_misc.c
@@ -677,6 +677,7 @@
 	return proc_calc_metrics(page, start, off, count, eof, len);
 }
 
+#ifdef CONFIG_FILE_LOCKING
 static int locks_open(struct inode *inode, struct file *filp)
 {
 	return seq_open(filp, &locks_seq_operations);
@@ -688,6 +689,7 @@
 	.llseek		= seq_lseek,
 	.release	= seq_release,
 };
+#endif /* CONFIG_FILE_LOCKING */
 
 static int execdomains_read_proc(char *page, char **start, off_t off,
 				 int count, int *eof, void *data)
@@ -881,7 +883,9 @@
 #ifdef CONFIG_PRINTK
 	proc_create("kmsg", S_IRUSR, NULL, &proc_kmsg_operations);
 #endif
+#ifdef CONFIG_FILE_LOCKING
 	proc_create("locks", 0, NULL, &proc_locks_operations);
+#endif
 	proc_create("devices", 0, NULL, &proc_devinfo_operations);
 	proc_create("cpuinfo", 0, NULL, &proc_cpuinfo_operations);
 #ifdef CONFIG_BLOCK
Index: linuxdev/include/linux/fs.h
===================================================================
--- linuxdev.orig/include/linux/fs.h
+++ linuxdev/include/linux/fs.h
@@ -983,6 +983,13 @@
 
 #include <linux/fcntl.h>
 
+extern void send_sigio(struct fown_struct *fown, int fd, int band);
+
+/* fs/sync.c */
+extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
+			loff_t endbyte, unsigned int flags);
+
+#ifdef CONFIG_FILE_LOCKING
 extern int fcntl_getlk(struct file *, struct flock __user *);
 extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
 			struct flock __user *);
@@ -993,14 +1000,9 @@
 			struct flock64 __user *);
 #endif
 
-extern void send_sigio(struct fown_struct *fown, int fd, int band);
 extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
 extern int fcntl_getlease(struct file *filp);
 
-/* fs/sync.c */
-extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
-			loff_t endbyte, unsigned int flags);
-
 /* fs/locks.c */
 extern void locks_init_lock(struct file_lock *);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
@@ -1023,6 +1025,33 @@
 extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
 extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
 extern struct seq_operations locks_seq_operations;
+#else /* !CONFIG_FILE_LOCKING */
+#define fcntl_getlk(a, b) (-EINVAL)
+#define fcntl_setlk(a, b, c, d) (-EACCES)
+#if BITS_PER_LONG == 32
+#define fcntl_getlk64(a, b) (-EINVAL)
+#define fcntl_setlk64(a, b, c, d) (-EACCES)
+#endif
+#define fcntl_setlease(a, b, c) (0)
+#define fcntl_getlease(a) (0)
+#define locks_init_lock(a)
+#define locks_copy_lock(a, b)
+#define locks_remove_posix(a, b)
+#define locks_remove_flock(a)
+#define posix_test_lock(a, b) (0)
+#define posix_lock_file(a, b) (-ENOLCK)
+#define posix_lock_file_wait(a, b) (-ENOLCK)
+#define posix_unblock_lock(a, b) (-ENOENT)
+#define vfs_test_lock(a, b) (0)
+#define vfs_lock_file(a, b, c, d) (-ENOLCK)
+#define vfs_cancel_lock(a, b) (0)
+#define flock_lock_file_wait(a, b) (-ENOLCK)
+#define __break_lease(a, b) (0)
+#define lease_get_mtime(a, b)
+#define lock_may_read(a, b, c) (1)
+#define lock_may_write(a, b, c) (1)
+#endif /* !CONFIG_FILE_LOCKING */
+
 
 struct fasync_struct {
 	int	magic;
@@ -1554,9 +1583,12 @@
 /* /sys/fs */
 extern struct kobject *fs_kobj;
 
+extern int rw_verify_area(int, struct file *, loff_t *, size_t);
+
 #define FLOCK_VERIFY_READ  1
 #define FLOCK_VERIFY_WRITE 2
 
+#ifdef CONFIG_FILE_LOCKING
 extern int locks_mandatory_locked(struct inode *);
 extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
 
@@ -1587,8 +1619,6 @@
 	return 0;
 }
 
-extern int rw_verify_area(int, struct file *, loff_t *, size_t);
-
 static inline int locks_verify_truncate(struct inode *inode,
 				    struct file *filp,
 				    loff_t size)
@@ -1609,6 +1639,14 @@
 		return __break_lease(inode, mode);
 	return 0;
 }
+#else /* !CONFIG_FILE_LOCKING */
+#define locks_mandatory_locked(a) (0)
+#define locks_mandatory_area(a, b, c, d, e) (0)
+#define mandatory_lock(a) (0)
+#define locks_verify_locked(a) (0)
+#define locks_verify_truncate(a, b, c) (0)
+#define break_lease(a, b) (0)
+#endif /* CONFIG_FILE_LOCKING */
 
 /* fs/open.c */
 
Index: linuxdev/kernel/sys_ni.c
===================================================================
--- linuxdev.orig/kernel/sys_ni.c
+++ linuxdev/kernel/sys_ni.c
@@ -130,6 +130,7 @@
 cond_syscall(sys_io_submit);
 cond_syscall(sys_io_cancel);
 cond_syscall(sys_io_getevents);
+cond_syscall(sys_flock);
 
 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read);
Index: linuxdev/kernel/sysctl.c
===================================================================
--- linuxdev.orig/kernel/sysctl.c
+++ linuxdev/kernel/sysctl.c
@@ -97,7 +97,7 @@
 static int neg_one = -1;
 #endif
 
-#ifdef CONFIG_MMU
+#if defined(CONFIG_MMU) && defined(CONFIG_FILE_LOCKING)
 static int two = 2;
 #endif
 
@@ -1260,6 +1260,7 @@
 		.extra1		= &minolduid,
 		.extra2		= &maxolduid,
 	},
+#ifdef CONFIG_FILE_LOCKING
 	{
 		.ctl_name	= FS_LEASES,
 		.procname	= "leases-enable",
@@ -1268,6 +1269,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+#endif
 #ifdef CONFIG_DNOTIFY
 	{
 		.ctl_name	= FS_DIR_NOTIFY,
@@ -1279,6 +1281,7 @@
 	},
 #endif
 #ifdef CONFIG_MMU
+#ifdef CONFIG_FILE_LOCKING
 	{
 		.ctl_name	= FS_LEASE_TIME,
 		.procname	= "lease-break-time",
@@ -1290,6 +1293,7 @@
 		.extra1		= &zero,
 		.extra2		= &two,
 	},
+#endif /* CONFIG_FILE_LOCKING */
 #ifdef CONFIG_AIO
 	{
 		.procname	= "aio-nr",


-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 2/4] Configure out file locking features

[J. Bruce Fields]

On Mon, Aug 04, 2008 at 03:52:37PM +0200, Thomas Petazzoni wrote:
> Le Sat, 2 Aug 2008 12:38:48 -0400,
> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
> 
> > Out of curiosity, why does the nfs client need disabling, but not
> > nfsd, gfs2, fuse, etc.?
> 
> Then also need disabling.

OK by me, but again, why exactly?  Since you're replacing the locking
calls they used by stubs that just return errors, in theory nfs, nfsd,
gfs2, and the rest should still compile and run, just without locking
support, right?

I don't have a strong opinion either way, just curious.

--b.

> Updated patch below.
> 
> Thanks!
> 
> Thomas
> 
> ---
> 
> Configure out file locking features
> 
> This patch adds the CONFIG_FILE_LOCKING option which allows to remove
> support for advisory locks. With this patch enabled, the flock()
> system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
> and NFS support are disabled. These features are not necessarly needed
> on embedded systems. It allows to save ~11 Kb of kernel code and data:
> 
>    text	   data	    bss	    dec	    hex	filename
> 1125436	 118764	 212992	1457192	 163c28	vmlinux.old
> 1114299	 118564	 212992	1445855	 160fdf	vmlinux
>  -11137    -200       0  -11337   -2C49 +/-
> 
> This patch has originally been written by Matt Mackall
> <mpm@selenic.com>, and is part of the Linux Tiny project.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Matt Mackall <mpm@selenic.com>
> Cc: matthew@wil.cx
> Cc: linux-fsdevel@vger.kernel.org
> Cc: mpm@selenic.com
> Cc: akpm@linux-foundation.org
> 
> ---
>  fs/Kconfig          |   15 ++++++++++++++-
>  fs/Makefile         |    3 ++-
>  fs/dlm/Kconfig      |    1 +
>  fs/gfs2/Kconfig     |    1 +
>  fs/proc/proc_misc.c |    4 ++++
>  include/linux/fs.h  |   52 +++++++++++++++++++++++++++++++++++++++++++++-------
>  kernel/sys_ni.c     |    1 +
>  kernel/sysctl.c     |    6 +++++-
>  8 files changed, 73 insertions(+), 10 deletions(-)
> 
> Index: linuxdev/fs/Kconfig
> ===================================================================
> --- linuxdev.orig/fs/Kconfig
> +++ linuxdev/fs/Kconfig
> @@ -427,12 +427,21 @@
>  	bool
>  	default n
>  
> +config FILE_LOCKING
> +	bool "Enable POSIX file locking API" if EMBEDDED
> +	default y
> +	help
> +	  This option enables standard file locking support, required
> +          for filesystems like NFS and for the flock() system
> +          call. Disabling this option saves about 11k.
> +
>  source "fs/xfs/Kconfig"
>  source "fs/gfs2/Kconfig"
>  
>  config OCFS2_FS
>  	tristate "OCFS2 file system support"
>  	depends on NET && SYSFS
> +	depends on FILE_LOCKING
>  	select CONFIGFS_FS
>  	select JBD
>  	select CRC32
> @@ -642,6 +651,7 @@
>  
>  config FUSE_FS
>  	tristate "Filesystem in Userspace support"
> +	depends on FILE_LOCKING
>  	help
>  	  With FUSE it is possible to implement a fully functional filesystem
>  	  in a userspace program.
> @@ -1567,7 +1577,7 @@
>  
>  config NFS_FS
>  	tristate "NFS client support"
> -	depends on INET
> +	depends on INET && FILE_LOCKING
>  	select LOCKD
>  	select SUNRPC
>  	select NFS_ACL_SUPPORT if NFS_V3_ACL
> @@ -1735,6 +1745,7 @@
>  
>  config LOCKD
>  	tristate
> +	depends on FILE_LOCKING
>  
>  config LOCKD_V4
>  	bool
> @@ -1870,6 +1881,7 @@
>  config CIFS
>  	tristate "CIFS support (advanced network filesystem, SMBFS successor)"
>  	depends on INET
> +	depends on FILE_LOCKING
>  	select NLS
>  	help
>  	  This is the client VFS module for the Common Internet File System
> @@ -2059,6 +2071,7 @@
>  config AFS_FS
>  	tristate "Andrew File System support (AFS) (EXPERIMENTAL)"
>  	depends on INET && EXPERIMENTAL
> +	depends on FILE_LOCKING
>  	select AF_RXRPC
>  	help
>  	  If you say Y here, you will get an experimental Andrew File System
> Index: linuxdev/fs/Makefile
> ===================================================================
> --- linuxdev.orig/fs/Makefile
> +++ linuxdev/fs/Makefile
> @@ -7,7 +7,7 @@
>  
>  obj-y :=	open.o read_write.o file_table.o super.o \
>  		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
> -		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
> +		ioctl.o readdir.o select.o fifo.o dcache.o inode.o \
>  		attr.o bad_inode.o file.o filesystems.o namespace.o \
>  		seq_file.o xattr.o libfs.o fs-writeback.o \
>  		pnode.o drop_caches.o splice.o sync.o utimes.o \
> @@ -28,6 +28,7 @@
>  obj-$(CONFIG_TIMERFD)		+= timerfd.o
>  obj-$(CONFIG_EVENTFD)		+= eventfd.o
>  obj-$(CONFIG_AIO)               += aio.o
> +obj-$(CONFIG_FILE_LOCKING)      += locks.o
>  obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
>  
>  nfsd-$(CONFIG_NFSD)		:= nfsctl.o
> Index: linuxdev/fs/dlm/Kconfig
> ===================================================================
> --- linuxdev.orig/fs/dlm/Kconfig
> +++ linuxdev/fs/dlm/Kconfig
> @@ -2,6 +2,7 @@
>  	tristate "Distributed Lock Manager (DLM)"
>  	depends on EXPERIMENTAL && INET
>  	depends on SYSFS && (IPV6 || IPV6=n)
> +	depends on FILE_LOCKING
>  	select CONFIGFS_FS
>  	select IP_SCTP
>  	help
> Index: linuxdev/fs/gfs2/Kconfig
> ===================================================================
> --- linuxdev.orig/fs/gfs2/Kconfig
> +++ linuxdev/fs/gfs2/Kconfig
> @@ -1,6 +1,7 @@
>  config GFS2_FS
>  	tristate "GFS2 file system support"
>  	depends on EXPERIMENTAL && (64BIT || (LSF && LBD))
> +	depends on FILE_LOCKING
>  	select FS_POSIX_ACL
>  	select CRC32
>  	help
> Index: linuxdev/fs/proc/proc_misc.c
> ===================================================================
> --- linuxdev.orig/fs/proc/proc_misc.c
> +++ linuxdev/fs/proc/proc_misc.c
> @@ -677,6 +677,7 @@
>  	return proc_calc_metrics(page, start, off, count, eof, len);
>  }
>  
> +#ifdef CONFIG_FILE_LOCKING
>  static int locks_open(struct inode *inode, struct file *filp)
>  {
>  	return seq_open(filp, &locks_seq_operations);
> @@ -688,6 +689,7 @@
>  	.llseek		= seq_lseek,
>  	.release	= seq_release,
>  };
> +#endif /* CONFIG_FILE_LOCKING */
>  
>  static int execdomains_read_proc(char *page, char **start, off_t off,
>  				 int count, int *eof, void *data)
> @@ -881,7 +883,9 @@
>  #ifdef CONFIG_PRINTK
>  	proc_create("kmsg", S_IRUSR, NULL, &proc_kmsg_operations);
>  #endif
> +#ifdef CONFIG_FILE_LOCKING
>  	proc_create("locks", 0, NULL, &proc_locks_operations);
> +#endif
>  	proc_create("devices", 0, NULL, &proc_devinfo_operations);
>  	proc_create("cpuinfo", 0, NULL, &proc_cpuinfo_operations);
>  #ifdef CONFIG_BLOCK
> Index: linuxdev/include/linux/fs.h
> ===================================================================
> --- linuxdev.orig/include/linux/fs.h
> +++ linuxdev/include/linux/fs.h
> @@ -983,6 +983,13 @@
>  
>  #include <linux/fcntl.h>
>  
> +extern void send_sigio(struct fown_struct *fown, int fd, int band);
> +
> +/* fs/sync.c */
> +extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
> +			loff_t endbyte, unsigned int flags);
> +
> +#ifdef CONFIG_FILE_LOCKING
>  extern int fcntl_getlk(struct file *, struct flock __user *);
>  extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
>  			struct flock __user *);
> @@ -993,14 +1000,9 @@
>  			struct flock64 __user *);
>  #endif
>  
> -extern void send_sigio(struct fown_struct *fown, int fd, int band);
>  extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
>  extern int fcntl_getlease(struct file *filp);
>  
> -/* fs/sync.c */
> -extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
> -			loff_t endbyte, unsigned int flags);
> -
>  /* fs/locks.c */
>  extern void locks_init_lock(struct file_lock *);
>  extern void locks_copy_lock(struct file_lock *, struct file_lock *);
> @@ -1023,6 +1025,33 @@
>  extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
>  extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
>  extern struct seq_operations locks_seq_operations;
> +#else /* !CONFIG_FILE_LOCKING */
> +#define fcntl_getlk(a, b) (-EINVAL)
> +#define fcntl_setlk(a, b, c, d) (-EACCES)
> +#if BITS_PER_LONG == 32
> +#define fcntl_getlk64(a, b) (-EINVAL)
> +#define fcntl_setlk64(a, b, c, d) (-EACCES)
> +#endif
> +#define fcntl_setlease(a, b, c) (0)
> +#define fcntl_getlease(a) (0)
> +#define locks_init_lock(a)
> +#define locks_copy_lock(a, b)
> +#define locks_remove_posix(a, b)
> +#define locks_remove_flock(a)
> +#define posix_test_lock(a, b) (0)
> +#define posix_lock_file(a, b) (-ENOLCK)
> +#define posix_lock_file_wait(a, b) (-ENOLCK)
> +#define posix_unblock_lock(a, b) (-ENOENT)
> +#define vfs_test_lock(a, b) (0)
> +#define vfs_lock_file(a, b, c, d) (-ENOLCK)
> +#define vfs_cancel_lock(a, b) (0)
> +#define flock_lock_file_wait(a, b) (-ENOLCK)
> +#define __break_lease(a, b) (0)
> +#define lease_get_mtime(a, b)
> +#define lock_may_read(a, b, c) (1)
> +#define lock_may_write(a, b, c) (1)
> +#endif /* !CONFIG_FILE_LOCKING */
> +
>  
>  struct fasync_struct {
>  	int	magic;
> @@ -1554,9 +1583,12 @@
>  /* /sys/fs */
>  extern struct kobject *fs_kobj;
>  
> +extern int rw_verify_area(int, struct file *, loff_t *, size_t);
> +
>  #define FLOCK_VERIFY_READ  1
>  #define FLOCK_VERIFY_WRITE 2
>  
> +#ifdef CONFIG_FILE_LOCKING
>  extern int locks_mandatory_locked(struct inode *);
>  extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
>  
> @@ -1587,8 +1619,6 @@
>  	return 0;
>  }
>  
> -extern int rw_verify_area(int, struct file *, loff_t *, size_t);
> -
>  static inline int locks_verify_truncate(struct inode *inode,
>  				    struct file *filp,
>  				    loff_t size)
> @@ -1609,6 +1639,14 @@
>  		return __break_lease(inode, mode);
>  	return 0;
>  }
> +#else /* !CONFIG_FILE_LOCKING */
> +#define locks_mandatory_locked(a) (0)
> +#define locks_mandatory_area(a, b, c, d, e) (0)
> +#define mandatory_lock(a) (0)
> +#define locks_verify_locked(a) (0)
> +#define locks_verify_truncate(a, b, c) (0)
> +#define break_lease(a, b) (0)
> +#endif /* CONFIG_FILE_LOCKING */
>  
>  /* fs/open.c */
>  
> Index: linuxdev/kernel/sys_ni.c
> ===================================================================
> --- linuxdev.orig/kernel/sys_ni.c
> +++ linuxdev/kernel/sys_ni.c
> @@ -130,6 +130,7 @@
>  cond_syscall(sys_io_submit);
>  cond_syscall(sys_io_cancel);
>  cond_syscall(sys_io_getevents);
> +cond_syscall(sys_flock);
>  
>  /* arch-specific weak syscall entries */
>  cond_syscall(sys_pciconfig_read);
> Index: linuxdev/kernel/sysctl.c
> ===================================================================
> --- linuxdev.orig/kernel/sysctl.c
> +++ linuxdev/kernel/sysctl.c
> @@ -97,7 +97,7 @@
>  static int neg_one = -1;
>  #endif
>  
> -#ifdef CONFIG_MMU
> +#if defined(CONFIG_MMU) && defined(CONFIG_FILE_LOCKING)
>  static int two = 2;
>  #endif
>  
> @@ -1260,6 +1260,7 @@
>  		.extra1		= &minolduid,
>  		.extra2		= &maxolduid,
>  	},
> +#ifdef CONFIG_FILE_LOCKING
>  	{
>  		.ctl_name	= FS_LEASES,
>  		.procname	= "leases-enable",
> @@ -1268,6 +1269,7 @@
>  		.mode		= 0644,
>  		.proc_handler	= &proc_dointvec,
>  	},
> +#endif
>  #ifdef CONFIG_DNOTIFY
>  	{
>  		.ctl_name	= FS_DIR_NOTIFY,
> @@ -1279,6 +1281,7 @@
>  	},
>  #endif
>  #ifdef CONFIG_MMU
> +#ifdef CONFIG_FILE_LOCKING
>  	{
>  		.ctl_name	= FS_LEASE_TIME,
>  		.procname	= "lease-break-time",
> @@ -1290,6 +1293,7 @@
>  		.extra1		= &zero,
>  		.extra2		= &two,
>  	},
> +#endif /* CONFIG_FILE_LOCKING */
>  #ifdef CONFIG_AIO
>  	{
>  		.procname	= "aio-nr",
> 
> 
> -- 
> Thomas Petazzoni, Free Electrons
> Kernel, drivers and embedded Linux development,
> consulting, training and support.
> http://free-electrons.com
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Re: [patch 2/4] Configure out file locking features

[Tim Bird]

J. Bruce Fields wrote:
> On Mon, Aug 04, 2008 at 03:52:37PM +0200, Thomas Petazzoni wrote:
>> Le Sat, 2 Aug 2008 12:38:48 -0400,
>> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
>>
>>> Out of curiosity, why does the nfs client need disabling, but not
>>> nfsd, gfs2, fuse, etc.?
>> Then also need disabling.
> 
> OK by me, but again, why exactly?  Since you're replacing the locking
> calls they used by stubs that just return errors, in theory nfs, nfsd,
> gfs2, and the rest should still compile and run, just without locking
> support, right?

I think so, but haven't tested this myself.

However, I would still be inclined to NOT add the extra config
dependencies.  Just my 2 cents.
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

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

Re: [patch 2/4] Configure out file locking features

[J. Bruce Fields]

On Mon, Aug 04, 2008 at 11:24:51AM -0700, Tim Bird wrote:
> J. Bruce Fields wrote:
> > On Mon, Aug 04, 2008 at 03:52:37PM +0200, Thomas Petazzoni wrote:
> >> Le Sat, 2 Aug 2008 12:38:48 -0400,
> >> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
> >>
> >>> Out of curiosity, why does the nfs client need disabling, but not
> >>> nfsd, gfs2, fuse, etc.?
> >> Then also need disabling.
> > 
> > OK by me, but again, why exactly?  Since you're replacing the locking
> > calls they used by stubs that just return errors, in theory nfs, nfsd,
> > gfs2, and the rest should still compile and run, just without locking
> > support, right?
> 
> I think so, but haven't tested this myself.
> 
> However, I would still be inclined to NOT add the extra config
> dependencies.  Just my 2 cents.

OK.  My fear was that there was some good reason that the nfs dependency
was added in the first place, and that it's since been lost....

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

Re: [patch 2/4] Configure out file locking features

[Matt Mackall]

On Mon, 2008-08-04 at 14:25 -0400, J. Bruce Fields wrote:
> On Mon, Aug 04, 2008 at 11:24:51AM -0700, Tim Bird wrote:
> > J. Bruce Fields wrote:
> > > On Mon, Aug 04, 2008 at 03:52:37PM +0200, Thomas Petazzoni wrote:
> > >> Le Sat, 2 Aug 2008 12:38:48 -0400,
> > >> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
> > >>
> > >>> Out of curiosity, why does the nfs client need disabling, but not
> > >>> nfsd, gfs2, fuse, etc.?
> > >> Then also need disabling.
> > > 
> > > OK by me, but again, why exactly?  Since you're replacing the locking
> > > calls they used by stubs that just return errors, in theory nfs, nfsd,
> > > gfs2, and the rest should still compile and run, just without locking
> > > support, right?
> > 
> > I think so, but haven't tested this myself.
> > 
> > However, I would still be inclined to NOT add the extra config
> > dependencies.  Just my 2 cents.
> 
> OK.  My fear was that there was some good reason that the nfs dependency
> was added in the first place, and that it's since been lost....

I vaguely remember there was some compile issue here, but that would
have been back in the 2.6.10 era.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [patch 2/4] Configure out file locking features

[J. Bruce Fields]

On Mon, Aug 04, 2008 at 01:54:01PM -0500, Matt Mackall wrote:
> 
> On Mon, 2008-08-04 at 14:25 -0400, J. Bruce Fields wrote:
> > On Mon, Aug 04, 2008 at 11:24:51AM -0700, Tim Bird wrote:
> > > J. Bruce Fields wrote:
> > > > On Mon, Aug 04, 2008 at 03:52:37PM +0200, Thomas Petazzoni wrote:
> > > >> Le Sat, 2 Aug 2008 12:38:48 -0400,
> > > >> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
> > > >>
> > > >>> Out of curiosity, why does the nfs client need disabling, but not
> > > >>> nfsd, gfs2, fuse, etc.?
> > > >> Then also need disabling.
> > > > 
> > > > OK by me, but again, why exactly?  Since you're replacing the locking
> > > > calls they used by stubs that just return errors, in theory nfs, nfsd,
> > > > gfs2, and the rest should still compile and run, just without locking
> > > > support, right?
> > > 
> > > I think so, but haven't tested this myself.
> > > 
> > > However, I would still be inclined to NOT add the extra config
> > > dependencies.  Just my 2 cents.
> > 
> > OK.  My fear was that there was some good reason that the nfs dependency
> > was added in the first place, and that it's since been lost....
> 
> I vaguely remember there was some compile issue here, but that would
> have been back in the 2.6.10 era.

Sounds plausible.  I've got no objection to the patch either way, but if
we could at least just add a comment documenting the issue (if it
exists), that might be helpful.

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

Re: [patch 2/4] Configure out file locking features

[Tim Bird]

J. Bruce Fields wrote:
> On Mon, Aug 04, 2008 at 11:24:51AM -0700, Tim Bird wrote:
>> J. Bruce Fields wrote:
>>> On Mon, Aug 04, 2008 at 03:52:37PM +0200, Thomas Petazzoni wrote:
>>>> Le Sat, 2 Aug 2008 12:38:48 -0400,
>>>> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
>>>>
>>>>> Out of curiosity, why does the nfs client need disabling, but not
>>>>> nfsd, gfs2, fuse, etc.?
>>>> Then also need disabling.
>>> OK by me, but again, why exactly?  Since you're replacing the locking
>>> calls they used by stubs that just return errors, in theory nfs, nfsd,
>>> gfs2, and the rest should still compile and run, just without locking
>>> support, right?
>> I think so, but haven't tested this myself.
>>
>> However, I would still be inclined to NOT add the extra config
>> dependencies.  Just my 2 cents.
> 
> OK.  My fear was that there was some good reason that the nfs dependency
> was added in the first place, and that it's since been lost....

For general information,  I just ran a test with the NFS dependency
on file locking turned off, and with the NFS auto-enable of
CONFIG_LOCKD removed.

It got me about a 16K savings, and I was able to successfully boot
a target with an NFS-mounted root file system.  I haven't run extensive
tests, so I don't know what apps might break in this configuration,
but at least in this test, it ran well enough to boot the machine.

Below is the patch I used, which is only provided for information.
This should NOT be applied - it consists of quick hacks to test this
configuration only (that is, to test NFS with CONFIG_FILE_LOCKING=n).

BTW - this is on a 2.6.23 kernel, so even the regular FILE_LOCKING
bits may be different from the current no-file-locking patch.

---
 fs/Kconfig         |    3 +--
 fs/nfs/client.c    |    6 ++++++
 fs/nfs/nfs3proc.c  |    4 ++++
 fs/nfs/proc.c      |    4 ++++
 fs/read_write.c    |    2 ++
 include/linux/fs.h |   13 +++++++------
 6 files changed, 24 insertions(+), 8 deletions(-)

--- a/fs/Kconfig
+++ b/fs/Kconfig
@@ -1701,8 +1701,7 @@ menu "Network File Systems"

 config NFS_FS
 	tristate "NFS file system support"
-	depends on INET && FILE_LOCKING
-	select LOCKD
+	depends on INET
 	select SUNRPC
 	select NFS_ACL_SUPPORT if NFS_V3_ACL
 	help
--- a/fs/nfs/client.c
+++ b/fs/nfs/client.c
@@ -414,6 +414,7 @@ static void nfs_destroy_server(struct nf
 		lockd_down();	/* release rpc.lockd */
 }

+#ifdef LOCKD
 /*
  * Version 2 or 3 lockd setup
  */
@@ -434,6 +435,7 @@ static int nfs_start_lockd(struct nfs_se
 out:
 	return error;
 }
+#endif

 /*
  * Initialise an NFSv3 ACL client connection
@@ -577,9 +579,11 @@ static int nfs_init_server(struct nfs_se
 	server->acdirmax = data->acdirmax * HZ;

 	/* Start lockd here, before we might error out */
+#ifdef LOCKD
 	error = nfs_start_lockd(server);
 	if (error < 0)
 		goto error;
+#endif

 	error = nfs_init_server_rpcclient(server, data->pseudoflavor);
 	if (error < 0)
@@ -1128,9 +1132,11 @@ struct nfs_server *nfs_clone_server(stru
 		(unsigned long long) server->fsid.major,
 		(unsigned long long) server->fsid.minor);

+#ifdef LOCKD
 	error = nfs_start_lockd(server);
 	if (error < 0)
 		goto out_free_server;
+#endif

 	spin_lock(&nfs_client_lock);
 	list_add_tail(&server->client_link, &server->nfs_client->cl_superblocks);
--- a/fs/nfs/nfs3proc.c
+++ b/fs/nfs/nfs3proc.c
@@ -795,7 +795,11 @@ static void nfs3_proc_commit_setup(struc
 static int
 nfs3_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
 {
+#ifdef LOCKD
 	return nlmclnt_proc(filp->f_path.dentry->d_inode, cmd, fl);
+#else
+	return -ENOLCK;
+#endif
 }

 const struct nfs_rpc_ops nfs_v3_clientops = {
--- a/fs/nfs/proc.c
+++ b/fs/nfs/proc.c
@@ -605,7 +605,11 @@ nfs_proc_commit_setup(struct nfs_write_d
 static int
 nfs_proc_lock(struct file *filp, int cmd, struct file_lock *fl)
 {
+#ifdef LOCKD
 	return nlmclnt_proc(filp->f_path.dentry->d_inode, cmd, fl);
+#else
+	return -ENOLCK;
+#endif
 }


--- a/fs/read_write.c
+++ b/fs/read_write.c
@@ -211,6 +211,7 @@ int rw_verify_area(int read_write, struc
 	if (unlikely((pos < 0) || (loff_t) (pos + count) < 0))
 		goto Einval;

+#ifdef FILE_LOCKING
 	if (unlikely(inode->i_flock && MANDATORY_LOCK(inode))) {
 		int retval = locks_mandatory_area(
 			read_write == READ ? FLOCK_VERIFY_READ : FLOCK_VERIFY_WRITE,
@@ -218,6 +219,7 @@ int rw_verify_area(int read_write, struc
 		if (retval < 0)
 			return retval;
 	}
+#endif
 	return count > MAX_RW_COUNT ? MAX_RW_COUNT : count;

 Einval:
--- a/include/linux/fs.h
+++ b/include/linux/fs.h
@@ -863,10 +863,6 @@ extern int fcntl_setlk64(unsigned int, s
 extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
 extern int fcntl_getlease(struct file *filp);

-/* fs/sync.c */
-extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
-			loff_t endbyte, unsigned int flags);
-
 /* fs/locks.c */
 extern void locks_init_lock(struct file_lock *);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
@@ -918,6 +914,10 @@ extern int lock_may_write(struct inode *
 #define steal_locks(a)
 #endif /* !CONFIG_FILE_LOCKING */

+/* fs/sync.c */
+extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
+			loff_t endbyte, unsigned int flags);
+
 struct fasync_struct {
 	int	magic;
 	int	fa_fd;
@@ -1424,8 +1424,6 @@ static inline int locks_verify_locked(st
 	return 0;
 }

-extern int rw_verify_area(int, struct file *, loff_t *, size_t);
-
 static inline int locks_verify_truncate(struct inode *inode,
 				    struct file *filp,
 				    loff_t size)
@@ -1459,6 +1457,9 @@ static inline int break_lease(struct ino

 #endif /* !CONFIG_FILE_LOCKING */

+extern int rw_verify_area(int, struct file *, loff_t *, size_t);
+
+
 /* fs/open.c */

 extern int do_truncate(struct dentry *, loff_t start, unsigned int time_attrs,



=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

Re: [patch 2/4] Configure out file locking features

[Thomas Petazzoni]

Le Mon, 4 Aug 2008 14:16:41 -0400,
"J. Bruce Fields" <bfields@fieldses.org> a écrit :

> OK by me, but again, why exactly?  Since you're replacing the locking
> calls they used by stubs that just return errors, in theory nfs, nfsd,
> gfs2, and the rest should still compile and run, just without locking
> support, right?

You're right, that was stupid. Either should I add the Kconfig
dependencies *OR* add the function stubs, not both. The following patch
implements the second solution. I've checked that NFS, CIFS, GFS2,
OCFS2, AFS and FUSE compile correctly. I've tested NFS only, though.

Here is the new patch. Comments and suggestions welcome.

Sincerly,

Thomas

---

Configure out file locking features

This patch adds the CONFIG_FILE_LOCKING option which allows to remove
support for advisory locks. With this patch enabled, the flock()
system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
and NFS support are disabled. These features are not necessarly needed
on embedded systems. It allows to save ~11 Kb of kernel code and data:

   text	   data	    bss	    dec	    hex	filename
1125436	 118764	 212992	1457192	 163c28	vmlinux.old
1114299	 118564	 212992	1445855	 160fdf	vmlinux
 -11137    -200       0  -11337   -2C49 +/-

This patch has originally been written by Matt Mackall
<mpm@selenic.com>, and is part of the Linux Tiny project.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Matt Mackall <mpm@selenic.com>
Cc: matthew@wil.cx
Cc: linux-fsdevel@vger.kernel.org
Cc: mpm@selenic.com
Cc: akpm@linux-foundation.org

---
 fs/Kconfig          |    8 +++++++
 fs/Makefile         |    3 +-
 fs/proc/proc_misc.c |    4 +++
 include/linux/fs.h  |   55 +++++++++++++++++++++++++++++++++++++++++++++-------
 kernel/sys_ni.c     |    1 
 kernel/sysctl.c     |    6 ++++-
 6 files changed, 68 insertions(+), 9 deletions(-)

Index: linuxdev/fs/Kconfig
===================================================================
--- linuxdev.orig/fs/Kconfig
+++ linuxdev/fs/Kconfig
@@ -419,6 +419,14 @@
 	bool
 	default n
 
+config FILE_LOCKING
+	bool "Enable POSIX file locking API" if EMBEDDED
+	default y
+	help
+	  This option enables standard file locking support, required
+          for filesystems like NFS and for the flock() system
+          call. Disabling this option saves about 11k.
+
 source "fs/xfs/Kconfig"
 source "fs/gfs2/Kconfig"
 
Index: linuxdev/fs/Makefile
===================================================================
--- linuxdev.orig/fs/Makefile
+++ linuxdev/fs/Makefile
@@ -7,7 +7,7 @@
 
 obj-y :=	open.o read_write.o file_table.o super.o \
 		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
-		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
+		ioctl.o readdir.o select.o fifo.o dcache.o inode.o \
 		attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
 		seq_file.o xattr.o libfs.o fs-writeback.o \
 		pnode.o drop_caches.o splice.o sync.o utimes.o \
@@ -27,6 +27,7 @@
 obj-$(CONFIG_SIGNALFD)		+= signalfd.o
 obj-$(CONFIG_TIMERFD)		+= timerfd.o
 obj-$(CONFIG_EVENTFD)		+= eventfd.o
+obj-$(CONFIG_FILE_LOCKING)      += locks.o
 obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
 
 nfsd-$(CONFIG_NFSD)		:= nfsctl.o
Index: linuxdev/fs/proc/proc_misc.c
===================================================================
--- linuxdev.orig/fs/proc/proc_misc.c
+++ linuxdev/fs/proc/proc_misc.c
@@ -677,6 +677,7 @@
 	return proc_calc_metrics(page, start, off, count, eof, len);
 }
 
+#ifdef CONFIG_FILE_LOCKING
 static int locks_open(struct inode *inode, struct file *filp)
 {
 	return seq_open(filp, &locks_seq_operations);
@@ -688,6 +689,7 @@
 	.llseek		= seq_lseek,
 	.release	= seq_release,
 };
+#endif /* CONFIG_FILE_LOCKING */
 
 static int execdomains_read_proc(char *page, char **start, off_t off,
 				 int count, int *eof, void *data)
@@ -881,7 +883,9 @@
 #ifdef CONFIG_PRINTK
 	proc_create("kmsg", S_IRUSR, NULL, &proc_kmsg_operations);
 #endif
+#ifdef CONFIG_FILE_LOCKING
 	proc_create("locks", 0, NULL, &proc_locks_operations);
+#endif
 	proc_create("devices", 0, NULL, &proc_devinfo_operations);
 	proc_create("cpuinfo", 0, NULL, &proc_cpuinfo_operations);
 #ifdef CONFIG_BLOCK
Index: linuxdev/include/linux/fs.h
===================================================================
--- linuxdev.orig/include/linux/fs.h
+++ linuxdev/include/linux/fs.h
@@ -983,6 +983,13 @@
 
 #include <linux/fcntl.h>
 
+extern void send_sigio(struct fown_struct *fown, int fd, int band);
+
+/* fs/sync.c */
+extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
+			loff_t endbyte, unsigned int flags);
+
+#ifdef CONFIG_FILE_LOCKING
 extern int fcntl_getlk(struct file *, struct flock __user *);
 extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
 			struct flock __user *);
@@ -993,14 +1000,9 @@
 			struct flock64 __user *);
 #endif
 
-extern void send_sigio(struct fown_struct *fown, int fd, int band);
 extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
 extern int fcntl_getlease(struct file *filp);
 
-/* fs/sync.c */
-extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
-			loff_t endbyte, unsigned int flags);
-
 /* fs/locks.c */
 extern void locks_init_lock(struct file_lock *);
 extern void locks_copy_lock(struct file_lock *, struct file_lock *);
@@ -1023,6 +1025,35 @@
 extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
 extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
 extern struct seq_operations locks_seq_operations;
+#else /* !CONFIG_FILE_LOCKING */
+#define fcntl_getlk(a, b) ({ -EINVAL; })
+#define fcntl_setlk(a, b, c, d) ({ -EACCES; })
+#if BITS_PER_LONG == 32
+#define fcntl_getlk64(a, b) ({ -EINVAL; })
+#define fcntl_setlk64(a, b, c, d) ({ -EACCES; })
+#endif
+#define fcntl_setlease(a, b, c) ({ 0; })
+#define fcntl_getlease(a) ({ 0; })
+#define locks_init_lock(a) ({ })
+#define __locks_copy_lock(a, b) ({ })
+#define locks_copy_lock(a, b) ({ })
+#define locks_remove_posix(a, b) ({ })
+#define locks_remove_flock(a) ({ })
+#define posix_test_lock(a, b) ({ 0; })
+#define posix_lock_file(a, b, c) ({ -ENOLCK; })
+#define posix_lock_file_wait(a, b) ({ -ENOLCK; })
+#define posix_unblock_lock(a, b) (-ENOENT)
+#define vfs_test_lock(a, b) ({ 0; })
+#define vfs_lock_file(a, b, c, d) (-ENOLCK)
+#define vfs_cancel_lock(a, b) ({ 0; })
+#define flock_lock_file_wait(a, b) ({ -ENOLCK; })
+#define __break_lease(a, b) ({ 0; })
+#define lease_get_mtime(a, b) ({ })
+#define generic_setlease(a, b, c) ({ -EINVAL; })
+#define lock_may_read(a, b, c) ({ 1; })
+#define lock_may_write(a, b, c) ({ 1; })
+#endif /* !CONFIG_FILE_LOCKING */
+
 
 struct fasync_struct {
 	int	magic;
@@ -1554,9 +1585,12 @@
 /* /sys/fs */
 extern struct kobject *fs_kobj;
 
+extern int rw_verify_area(int, struct file *, loff_t *, size_t);
+
 #define FLOCK_VERIFY_READ  1
 #define FLOCK_VERIFY_WRITE 2
 
+#ifdef CONFIG_FILE_LOCKING
 extern int locks_mandatory_locked(struct inode *);
 extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
 
@@ -1587,8 +1621,6 @@
 	return 0;
 }
 
-extern int rw_verify_area(int, struct file *, loff_t *, size_t);
-
 static inline int locks_verify_truncate(struct inode *inode,
 				    struct file *filp,
 				    loff_t size)
@@ -1609,6 +1641,15 @@
 		return __break_lease(inode, mode);
 	return 0;
 }
+#else /* !CONFIG_FILE_LOCKING */
+#define locks_mandatory_locked(a) ({ 0; })
+#define locks_mandatory_area(a, b, c, d, e) ({ 0; })
+#define __mandatory_lock(a) ({ 0; })
+#define mandatory_lock(a) ({ 0; })
+#define locks_verify_locked(a) ({ 0; })
+#define locks_verify_truncate(a, b, c) ({ 0; })
+#define break_lease(a, b) ({ 0; })
+#endif /* CONFIG_FILE_LOCKING */
 
 /* fs/open.c */
 
Index: linuxdev/kernel/sys_ni.c
===================================================================
--- linuxdev.orig/kernel/sys_ni.c
+++ linuxdev/kernel/sys_ni.c
@@ -125,6 +125,7 @@
 cond_syscall(sys_vm86);
 cond_syscall(compat_sys_ipc);
 cond_syscall(compat_sys_sysctl);
+cond_syscall(sys_flock);
 
 /* arch-specific weak syscall entries */
 cond_syscall(sys_pciconfig_read);
Index: linuxdev/kernel/sysctl.c
===================================================================
--- linuxdev.orig/kernel/sysctl.c
+++ linuxdev/kernel/sysctl.c
@@ -97,7 +97,7 @@
 static int neg_one = -1;
 #endif
 
-#ifdef CONFIG_MMU
+#if defined(CONFIG_MMU) && defined(CONFIG_FILE_LOCKING)
 static int two = 2;
 #endif
 
@@ -1260,6 +1260,7 @@
 		.extra1		= &minolduid,
 		.extra2		= &maxolduid,
 	},
+#ifdef CONFIG_FILE_LOCKING
 	{
 		.ctl_name	= FS_LEASES,
 		.procname	= "leases-enable",
@@ -1268,6 +1269,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec,
 	},
+#endif
 #ifdef CONFIG_DNOTIFY
 	{
 		.ctl_name	= FS_DIR_NOTIFY,
@@ -1279,6 +1281,7 @@
 	},
 #endif
 #ifdef CONFIG_MMU
+#ifdef CONFIG_FILE_LOCKING
 	{
 		.ctl_name	= FS_LEASE_TIME,
 		.procname	= "lease-break-time",
@@ -1290,6 +1293,7 @@
 		.extra1		= &zero,
 		.extra2		= &two,
 	},
+#endif
 	{
 		.procname	= "aio-nr",
 		.data		= &aio_nr,


-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 2/4] Configure out file locking features

[J. Bruce Fields]

On Wed, Aug 06, 2008 at 03:12:22PM +0200, Thomas Petazzoni wrote:
> Le Mon, 4 Aug 2008 14:16:41 -0400,
> "J. Bruce Fields" <bfields@fieldses.org> a écrit :
> 
> > OK by me, but again, why exactly?  Since you're replacing the locking
> > calls they used by stubs that just return errors, in theory nfs, nfsd,
> > gfs2, and the rest should still compile and run, just without locking
> > support, right?
> 
> You're right, that was stupid. Either should I add the Kconfig
> dependencies *OR* add the function stubs, not both. The following patch
> implements the second solution. I've checked that NFS, CIFS, GFS2,
> OCFS2, AFS and FUSE compile correctly. I've tested NFS only, though.
> 
> Here is the new patch. Comments and suggestions welcome.

Seems fine to me, thanks!  I can queue it up in my tree for 2.6.28 if
there's no objections.

(But, basic mail question: so I give this message to git-am and it
complains because of all this =3D=20 stuff, which google tells me is
quoted-printable encoding.  Fine, I fixed it up by hand, but a) there
must be some less boring way to deal with it, and b) there must have
been some easy way to avoid it in the first place, either on my side or
yours?  Enlightenment welcome.)

--b.

> 
> Sincerly,
> 
> Thomas
> 
> ---
> 
> Configure out file locking features
> 
> This patch adds the CONFIG_FILE_LOCKING option which allows to remove
> support for advisory locks. With this patch enabled, the flock()
> system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
> and NFS support are disabled. These features are not necessarly needed
> on embedded systems. It allows to save ~11 Kb of kernel code and data:
> 
>    text	   data	    bss	    dec	    hex	filename
> 1125436	 118764	 212992	1457192	 163c28	vmlinux.old
> 1114299	 118564	 212992	1445855	 160fdf	vmlinux
>  -11137    -200       0  -11337   -2C49 +/-
> 
> This patch has originally been written by Matt Mackall
> <mpm@selenic.com>, and is part of the Linux Tiny project.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Signed-off-by: Matt Mackall <mpm@selenic.com>
> Cc: matthew@wil.cx
> Cc: linux-fsdevel@vger.kernel.org
> Cc: mpm@selenic.com
> Cc: akpm@linux-foundation.org
> 
> ---
>  fs/Kconfig          |    8 +++++++
>  fs/Makefile         |    3 +-
>  fs/proc/proc_misc.c |    4 +++
>  include/linux/fs.h  |   55 +++++++++++++++++++++++++++++++++++++++++++++-------
>  kernel/sys_ni.c     |    1 
>  kernel/sysctl.c     |    6 ++++-
>  6 files changed, 68 insertions(+), 9 deletions(-)
> 
> Index: linuxdev/fs/Kconfig
> ===================================================================
> --- linuxdev.orig/fs/Kconfig
> +++ linuxdev/fs/Kconfig
> @@ -419,6 +419,14 @@
>  	bool
>  	default n
>  
> +config FILE_LOCKING
> +	bool "Enable POSIX file locking API" if EMBEDDED
> +	default y
> +	help
> +	  This option enables standard file locking support, required
> +          for filesystems like NFS and for the flock() system
> +          call. Disabling this option saves about 11k.
> +
>  source "fs/xfs/Kconfig"
>  source "fs/gfs2/Kconfig"
>  
> Index: linuxdev/fs/Makefile
> ===================================================================
> --- linuxdev.orig/fs/Makefile
> +++ linuxdev/fs/Makefile
> @@ -7,7 +7,7 @@
>  
>  obj-y :=	open.o read_write.o file_table.o super.o \
>  		char_dev.o stat.o exec.o pipe.o namei.o fcntl.o \
> -		ioctl.o readdir.o select.o fifo.o locks.o dcache.o inode.o \
> +		ioctl.o readdir.o select.o fifo.o dcache.o inode.o \
>  		attr.o bad_inode.o file.o filesystems.o namespace.o aio.o \
>  		seq_file.o xattr.o libfs.o fs-writeback.o \
>  		pnode.o drop_caches.o splice.o sync.o utimes.o \
> @@ -27,6 +27,7 @@
>  obj-$(CONFIG_SIGNALFD)		+= signalfd.o
>  obj-$(CONFIG_TIMERFD)		+= timerfd.o
>  obj-$(CONFIG_EVENTFD)		+= eventfd.o
> +obj-$(CONFIG_FILE_LOCKING)      += locks.o
>  obj-$(CONFIG_COMPAT)		+= compat.o compat_ioctl.o
>  
>  nfsd-$(CONFIG_NFSD)		:= nfsctl.o
> Index: linuxdev/fs/proc/proc_misc.c
> ===================================================================
> --- linuxdev.orig/fs/proc/proc_misc.c
> +++ linuxdev/fs/proc/proc_misc.c
> @@ -677,6 +677,7 @@
>  	return proc_calc_metrics(page, start, off, count, eof, len);
>  }
>  
> +#ifdef CONFIG_FILE_LOCKING
>  static int locks_open(struct inode *inode, struct file *filp)
>  {
>  	return seq_open(filp, &locks_seq_operations);
> @@ -688,6 +689,7 @@
>  	.llseek		= seq_lseek,
>  	.release	= seq_release,
>  };
> +#endif /* CONFIG_FILE_LOCKING */
>  
>  static int execdomains_read_proc(char *page, char **start, off_t off,
>  				 int count, int *eof, void *data)
> @@ -881,7 +883,9 @@
>  #ifdef CONFIG_PRINTK
>  	proc_create("kmsg", S_IRUSR, NULL, &proc_kmsg_operations);
>  #endif
> +#ifdef CONFIG_FILE_LOCKING
>  	proc_create("locks", 0, NULL, &proc_locks_operations);
> +#endif
>  	proc_create("devices", 0, NULL, &proc_devinfo_operations);
>  	proc_create("cpuinfo", 0, NULL, &proc_cpuinfo_operations);
>  #ifdef CONFIG_BLOCK
> Index: linuxdev/include/linux/fs.h
> ===================================================================
> --- linuxdev.orig/include/linux/fs.h
> +++ linuxdev/include/linux/fs.h
> @@ -983,6 +983,13 @@
>  
>  #include <linux/fcntl.h>
>  
> +extern void send_sigio(struct fown_struct *fown, int fd, int band);
> +
> +/* fs/sync.c */
> +extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
> +			loff_t endbyte, unsigned int flags);
> +
> +#ifdef CONFIG_FILE_LOCKING
>  extern int fcntl_getlk(struct file *, struct flock __user *);
>  extern int fcntl_setlk(unsigned int, struct file *, unsigned int,
>  			struct flock __user *);
> @@ -993,14 +1000,9 @@
>  			struct flock64 __user *);
>  #endif
>  
> -extern void send_sigio(struct fown_struct *fown, int fd, int band);
>  extern int fcntl_setlease(unsigned int fd, struct file *filp, long arg);
>  extern int fcntl_getlease(struct file *filp);
>  
> -/* fs/sync.c */
> -extern int do_sync_mapping_range(struct address_space *mapping, loff_t offset,
> -			loff_t endbyte, unsigned int flags);
> -
>  /* fs/locks.c */
>  extern void locks_init_lock(struct file_lock *);
>  extern void locks_copy_lock(struct file_lock *, struct file_lock *);
> @@ -1023,6 +1025,35 @@
>  extern int lock_may_read(struct inode *, loff_t start, unsigned long count);
>  extern int lock_may_write(struct inode *, loff_t start, unsigned long count);
>  extern struct seq_operations locks_seq_operations;
> +#else /* !CONFIG_FILE_LOCKING */
> +#define fcntl_getlk(a, b) ({ -EINVAL; })
> +#define fcntl_setlk(a, b, c, d) ({ -EACCES; })
> +#if BITS_PER_LONG == 32
> +#define fcntl_getlk64(a, b) ({ -EINVAL; })
> +#define fcntl_setlk64(a, b, c, d) ({ -EACCES; })
> +#endif
> +#define fcntl_setlease(a, b, c) ({ 0; })
> +#define fcntl_getlease(a) ({ 0; })
> +#define locks_init_lock(a) ({ })
> +#define __locks_copy_lock(a, b) ({ })
> +#define locks_copy_lock(a, b) ({ })
> +#define locks_remove_posix(a, b) ({ })
> +#define locks_remove_flock(a) ({ })
> +#define posix_test_lock(a, b) ({ 0; })
> +#define posix_lock_file(a, b, c) ({ -ENOLCK; })
> +#define posix_lock_file_wait(a, b) ({ -ENOLCK; })
> +#define posix_unblock_lock(a, b) (-ENOENT)
> +#define vfs_test_lock(a, b) ({ 0; })
> +#define vfs_lock_file(a, b, c, d) (-ENOLCK)
> +#define vfs_cancel_lock(a, b) ({ 0; })
> +#define flock_lock_file_wait(a, b) ({ -ENOLCK; })
> +#define __break_lease(a, b) ({ 0; })
> +#define lease_get_mtime(a, b) ({ })
> +#define generic_setlease(a, b, c) ({ -EINVAL; })
> +#define lock_may_read(a, b, c) ({ 1; })
> +#define lock_may_write(a, b, c) ({ 1; })
> +#endif /* !CONFIG_FILE_LOCKING */
> +
>  
>  struct fasync_struct {
>  	int	magic;
> @@ -1554,9 +1585,12 @@
>  /* /sys/fs */
>  extern struct kobject *fs_kobj;
>  
> +extern int rw_verify_area(int, struct file *, loff_t *, size_t);
> +
>  #define FLOCK_VERIFY_READ  1
>  #define FLOCK_VERIFY_WRITE 2
>  
> +#ifdef CONFIG_FILE_LOCKING
>  extern int locks_mandatory_locked(struct inode *);
>  extern int locks_mandatory_area(int, struct inode *, struct file *, loff_t, size_t);
>  
> @@ -1587,8 +1621,6 @@
>  	return 0;
>  }
>  
> -extern int rw_verify_area(int, struct file *, loff_t *, size_t);
> -
>  static inline int locks_verify_truncate(struct inode *inode,
>  				    struct file *filp,
>  				    loff_t size)
> @@ -1609,6 +1641,15 @@
>  		return __break_lease(inode, mode);
>  	return 0;
>  }
> +#else /* !CONFIG_FILE_LOCKING */
> +#define locks_mandatory_locked(a) ({ 0; })
> +#define locks_mandatory_area(a, b, c, d, e) ({ 0; })
> +#define __mandatory_lock(a) ({ 0; })
> +#define mandatory_lock(a) ({ 0; })
> +#define locks_verify_locked(a) ({ 0; })
> +#define locks_verify_truncate(a, b, c) ({ 0; })
> +#define break_lease(a, b) ({ 0; })
> +#endif /* CONFIG_FILE_LOCKING */
>  
>  /* fs/open.c */
>  
> Index: linuxdev/kernel/sys_ni.c
> ===================================================================
> --- linuxdev.orig/kernel/sys_ni.c
> +++ linuxdev/kernel/sys_ni.c
> @@ -125,6 +125,7 @@
>  cond_syscall(sys_vm86);
>  cond_syscall(compat_sys_ipc);
>  cond_syscall(compat_sys_sysctl);
> +cond_syscall(sys_flock);
>  
>  /* arch-specific weak syscall entries */
>  cond_syscall(sys_pciconfig_read);
> Index: linuxdev/kernel/sysctl.c
> ===================================================================
> --- linuxdev.orig/kernel/sysctl.c
> +++ linuxdev/kernel/sysctl.c
> @@ -97,7 +97,7 @@
>  static int neg_one = -1;
>  #endif
>  
> -#ifdef CONFIG_MMU
> +#if defined(CONFIG_MMU) && defined(CONFIG_FILE_LOCKING)
>  static int two = 2;
>  #endif
>  
> @@ -1260,6 +1260,7 @@
>  		.extra1		= &minolduid,
>  		.extra2		= &maxolduid,
>  	},
> +#ifdef CONFIG_FILE_LOCKING
>  	{
>  		.ctl_name	= FS_LEASES,
>  		.procname	= "leases-enable",
> @@ -1268,6 +1269,7 @@
>  		.mode		= 0644,
>  		.proc_handler	= &proc_dointvec,
>  	},
> +#endif
>  #ifdef CONFIG_DNOTIFY
>  	{
>  		.ctl_name	= FS_DIR_NOTIFY,
> @@ -1279,6 +1281,7 @@
>  	},
>  #endif
>  #ifdef CONFIG_MMU
> +#ifdef CONFIG_FILE_LOCKING
>  	{
>  		.ctl_name	= FS_LEASE_TIME,
>  		.procname	= "lease-break-time",
> @@ -1290,6 +1293,7 @@
>  		.extra1		= &zero,
>  		.extra2		= &two,
>  	},
> +#endif
>  	{
>  		.procname	= "aio-nr",
>  		.data		= &aio_nr,
> 
> 
> -- 
> Thomas Petazzoni, Free Electrons
> Kernel, drivers and embedded Linux development,
> consulting, training and support.
> http://free-electrons.com

[patch 3/4] Configure out ethtool support

[Thomas Petazzoni]

[-- Attachment #1: configure-out-ethtool-support --]
[-- Type: text/plain, Size: 5790 bytes --]

This patchs add the CONFIG_ETHTOOL option which allows to remove
support for ethtool, not necessarly used on embedded systems. As this
is a size-reduction option, it depends on CONFIG_EMBEDDED. It allows
to save ~6 kilobytes of kernel code:

   text	   data	    bss	    dec	    hex	filename
1258447	 123592	 212992	1595031	 185697	vmlinux
1252147	 123592	 212992	1588731	 183dfb	vmlinux.new
  -6300       0       0   -6300   -189C +/-

Bonding and bridging both depends on Ethtool functionnality, so
ETHTOOL is selected automatically when either bonding and bridging are
selected.

Question: should we also remove ethtool-related functions from all
network drivers ?

This patch has been originally written by Matt Mackall
<mpm@selenic.com>, and is part of the Linux Tiny project.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Matt Mackall <mpm@selenic.com>
Cc: jgarzik@pobox.com
Cc: netdev@vger.kernel.org
Cc: davem@davemloft.net
Cc: mpm@selenic.com
Cc: akpm@linux-foundation.org

---
 drivers/net/Kconfig     |    1 +
 include/linux/ethtool.h |   16 ++++++++++++++++
 init/Kconfig            |    8 ++++++++
 net/bridge/Kconfig      |    1 +
 net/core/Makefile       |    3 ++-
 net/core/dev.c          |    4 ++++
 6 files changed, 32 insertions(+), 1 deletion(-)

Index: linuxdev/drivers/net/Kconfig
===================================================================
--- linuxdev.orig/drivers/net/Kconfig
+++ linuxdev/drivers/net/Kconfig
@@ -61,6 +61,7 @@
 config BONDING
 	tristate "Bonding driver support"
 	depends on INET
+	select ETHTOOL
 	---help---
 	  Say 'Y' or 'M' if you wish to be able to 'bond' multiple Ethernet
 	  Channels together. This is called 'Etherchannel' by Cisco,
Index: linuxdev/include/linux/ethtool.h
===================================================================
--- linuxdev.orig/include/linux/ethtool.h
+++ linuxdev/include/linux/ethtool.h
@@ -283,6 +283,7 @@
 struct net_device;
 
 /* Some generic methods drivers may use in their ethtool_ops */
+#ifdef CONFIG_ETHTOOL
 u32 ethtool_op_get_link(struct net_device *dev);
 u32 ethtool_op_get_tx_csum(struct net_device *dev);
 int ethtool_op_set_tx_csum(struct net_device *dev, u32 data);
@@ -296,6 +297,21 @@
 int ethtool_op_set_ufo(struct net_device *dev, u32 data);
 u32 ethtool_op_get_flags(struct net_device *dev);
 int ethtool_op_set_flags(struct net_device *dev, u32 data);
+#else
+static inline u32 ethtool_op_get_link(struct net_device *dev) { return 0; }
+static inline u32 ethtool_op_get_tx_csum(struct net_device *dev) { return 0; }
+static inline int ethtool_op_set_tx_csum(struct net_device *dev, u32 data) { return 0; }
+static inline int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data) { return 0; }
+static inline int ethtool_op_set_tx_ipv6_csum(struct net_device *dev, u32 data) { return 0; }
+static inline u32 ethtool_op_get_sg(struct net_device *dev) { return 0; }
+static inline int ethtool_op_set_sg(struct net_device *dev, u32 data) { return 0; }
+static inline u32 ethtool_op_get_tso(struct net_device *dev) { return 0; }
+static inline int ethtool_op_set_tso(struct net_device *dev, u32 data) { return 0; }
+static inline u32 ethtool_op_get_ufo(struct net_device *dev) { return 0; }
+static inline int ethtool_op_set_ufo(struct net_device *dev, u32 data) { return 0; }
+static inline u32 ethtool_op_get_flags(struct net_device *dev) { return 0; }
+static inline int ethtool_op_set_flags(struct net_device *dev, u32 data) { return 0; }
+#endif
 
 /**
  * &ethtool_ops - Alter and report network device settings
Index: linuxdev/init/Kconfig
===================================================================
--- linuxdev.orig/init/Kconfig
+++ linuxdev/init/Kconfig
@@ -740,6 +740,14 @@
           for filesystems like NFS and for the flock() system
           call. Disabling this option saves about 11k.
 
+config ETHTOOL
+	bool "Enable ethtool support" if EMBEDDED
+	depends on NET
+	default y
+	help
+	  Disabling this option removes support for configuring
+	  ethernet device features via ethtool. Saves about 6k.
+
 config VM_EVENT_COUNTERS
 	default y
 	bool "Enable VM event counters for /proc/vmstat" if EMBEDDED
Index: linuxdev/net/bridge/Kconfig
===================================================================
--- linuxdev.orig/net/bridge/Kconfig
+++ linuxdev/net/bridge/Kconfig
@@ -6,6 +6,7 @@
 	tristate "802.1d Ethernet Bridging"
 	select LLC
 	select STP
+	select ETHTOOL
 	---help---
 	  If you say Y here, then your Linux box will be able to act as an
 	  Ethernet bridge, which means that the different Ethernet segments it
Index: linuxdev/net/core/Makefile
===================================================================
--- linuxdev.orig/net/core/Makefile
+++ linuxdev/net/core/Makefile
@@ -7,10 +7,11 @@
 
 obj-$(CONFIG_SYSCTL) += sysctl_net_core.o
 
-obj-y		     += dev.o ethtool.o dev_mcast.o dst.o netevent.o \
+obj-y		     += dev.o dev_mcast.o dst.o netevent.o \
 			neighbour.o rtnetlink.o utils.o link_watch.o filter.o
 
 obj-$(CONFIG_XFRM) += flow.o
+obj-$(CONFIG_ETHTOOL) += ethtool.o
 obj-y += net-sysfs.o
 obj-$(CONFIG_NET_PKTGEN) += pktgen.o
 obj-$(CONFIG_NETPOLL) += netpoll.o
Index: linuxdev/net/core/dev.c
===================================================================
--- linuxdev.orig/net/core/dev.c
+++ linuxdev/net/core/dev.c
@@ -3669,6 +3669,7 @@
 			return ret;
 
 		case SIOCETHTOOL:
+#ifdef CONFIG_ETHTOOL
 			dev_load(net, ifr.ifr_name);
 			rtnl_lock();
 			ret = dev_ethtool(net, &ifr);
@@ -3681,6 +3682,9 @@
 					ret = -EFAULT;
 			}
 			return ret;
+#else
+			return -EINVAL;
+#endif
 
 		/*
 		 *	These ioctl calls:

-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 3/4] Configure out ethtool support

[Ben Hutchings]

Thomas Petazzoni wrote:
[...]
> --- linuxdev.orig/include/linux/ethtool.h
> +++ linuxdev/include/linux/ethtool.h
> @@ -283,6 +283,7 @@
>  struct net_device;
>  
>  /* Some generic methods drivers may use in their ethtool_ops */
> +#ifdef CONFIG_ETHTOOL
>  u32 ethtool_op_get_link(struct net_device *dev);
>  u32 ethtool_op_get_tx_csum(struct net_device *dev);
>  int ethtool_op_set_tx_csum(struct net_device *dev, u32 data);
> @@ -296,6 +297,21 @@
>  int ethtool_op_set_ufo(struct net_device *dev, u32 data);
>  u32 ethtool_op_get_flags(struct net_device *dev);
>  int ethtool_op_set_flags(struct net_device *dev, u32 data);
> +#else
> +static inline u32 ethtool_op_get_link(struct net_device *dev) { return 0; }
> +static inline u32 ethtool_op_get_tx_csum(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_tx_csum(struct net_device *dev, u32 data) { return 0; }
> +static inline int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data) { return 0; }
> +static inline int ethtool_op_set_tx_ipv6_csum(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_sg(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_sg(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_tso(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_tso(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_ufo(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_ufo(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_flags(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_flags(struct net_device *dev, u32 data) { return 0; }

The dummy setter functions should return -EOPNOTSUPP.  The getter functions
just read device feature flags and could be made inline.  They have no way
of returning failure.

[...]
> ===================================================================
> --- linuxdev.orig/net/core/dev.c
> +++ linuxdev/net/core/dev.c
> @@ -3669,6 +3669,7 @@
>  			return ret;
>  
>  		case SIOCETHTOOL:
> +#ifdef CONFIG_ETHTOOL
>  			dev_load(net, ifr.ifr_name);
>  			rtnl_lock();
>  			ret = dev_ethtool(net, &ifr);
> @@ -3681,6 +3682,9 @@
>  					ret = -EFAULT;
>  			}
>  			return ret;
> +#else
> +			return -EINVAL;
> +#endif
>  
>  		/*
>  		 *	These ioctl calls:

You also need to conditionalise dev_disable_lro().

Ben.

-- 
Ben Hutchings, Senior Software Engineer, Solarflare Communications
Not speaking for my employer; that's the marketing department's job.
They asked us to note that Solarflare product names are trademarked.

Re: [patch 3/4] Configure out ethtool support

[David Miller]

From: Ben Hutchings <bhutchings@solarflare.com>
Date: Thu, 31 Jul 2008 11:40:05 +0100

> You also need to conditionalise dev_disable_lro().

That can only be done once the CONFIG_ETHTOOL select statement
is added for CONFIG_INET.

Which basically makes this CONFIG_ETHTOOL thing completely pointless.

Re: [patch 3/4] Configure out ethtool support

[David Woodhouse]

On Thu, 2008-07-31 at 03:49 -0700, David Miller wrote:
> From: Ben Hutchings <bhutchings@solarflare.com>
> Date: Thu, 31 Jul 2008 11:40:05 +0100
> 
> > You also need to conditionalise dev_disable_lro().
> 
> That can only be done once the CONFIG_ETHTOOL select statement
> is added for CONFIG_INET.
> 
> Which basically makes this CONFIG_ETHTOOL thing completely pointless.

Other potential approaches include not enabling LRO by default if 
!CONFIG_ETHTOOL. Or having the driver(s) which _do_ enable LRO by
default 'select ETHTOOL'.

-- 
dwmw2

Re: [patch 3/4] Configure out ethtool support

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 11:54:24 +0100

> Other potential approaches include not enabling LRO by default if 
> !CONFIG_ETHTOOL. Or having the driver(s) which _do_ enable LRO by
> default 'select ETHTOOL'.

It is possible for us to generically enable LRO for every device,
since it is a software algorithm.  And likely we will do something
like this in the not too distant future.

Re: [patch 3/4] Configure out ethtool support

[David Woodhouse]

On Thu, 2008-07-31 at 11:27 +0200, Thomas Petazzoni wrote:
> 
> +#else
> +static inline u32 ethtool_op_get_link(struct net_device *dev) { return 0; }
> +static inline u32 ethtool_op_get_tx_csum(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_tx_csum(struct net_device *dev, u32 data) { return 0; }
> +static inline int ethtool_op_set_tx_hw_csum(struct net_device *dev, u32 data) { return 0; }
> +static inline int ethtool_op_set_tx_ipv6_csum(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_sg(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_sg(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_tso(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_tso(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_ufo(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_ufo(struct net_device *dev, u32 data) { return 0; }
> +static inline u32 ethtool_op_get_flags(struct net_device *dev) { return 0; }
> +static inline int ethtool_op_set_flags(struct net_device *dev, u32 data) { return 0; }
> +#endif

It's alleged that these functions are called from 'core' network code in
some places, although I can't actually see any evidence of that anywhere
in Linus' tree except for vlans and bridging.

If that's actually the case, perhaps it makes sense to add a
WARN_ON_ONCE() to these empty functions, so that a developer who
disables CONFIG_ETHTOOL when they shouldn't will see a nasty warning
about it rather than a silent failure?

Btw, I see you've made bridging 'select ETHTOOL'; did you do the same
for vlan support?

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 3/4] Configure out ethtool support

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 11:42:47 +0100

> It's alleged that these functions are called from 'core' network code in
> some places, although I can't actually see any evidence of that anywhere
> in Linus' tree except for vlans and bridging.
> 
> If that's actually the case, perhaps it makes sense to add a
> WARN_ON_ONCE() to these empty functions, so that a developer who
> disables CONFIG_ETHTOOL when they shouldn't will see a nasty warning
> about it rather than a silent failure?
> 
> Btw, I see you've made bridging 'select ETHTOOL'; did you do the same
> for vlan support?

CONFIG_INET needs it too.

dev_disable_lro() calls the ethtool ops directly.

But it still needs to be conditional, because as I said what I see
happening next is this CONFIG_ETHTOOL thing getting jammed into each
and every network driver.  (in fact, ethtool support code in a single
driver probably drarfs the 6K savings this initial patch is giving
us).

And at which point you'll have a broken system for drivers that
enable LRO and the user enables forwarding.

Re: [patch 3/4] Configure out ethtool support

[David Woodhouse]

On Thu, 2008-07-31 at 03:51 -0700, David Miller wrote:
> CONFIG_INET needs it too.
> 
> dev_disable_lro() calls the ethtool ops directly.

Ah, right. That's why it didn't show up in my grep.

There are solutions to that, as I said. Either we could 'select ETHTOOL'
in those drivers which enable LRO by default, or we could just make sure
they _don't_ enable LRO by default when CONFIG_ETHTOOL isn't set.

And if we end up doing LRO generically in software where the hardware
doesn't handle it, presumably we can do that without having to use
ethtool to change the hardware behaviour?

> But it still needs to be conditional, because as I said what I see
> happening next is this CONFIG_ETHTOOL thing getting jammed into each
> and every network driver.  (in fact, ethtool support code in a single
> driver probably drarfs the 6K savings this initial patch is giving
> us).

I think we can avoid that. If the actual functions and the struct
ethtool_ops are static, and if we have something like...

#ifdef CONFIG_ETHTOOL
#define dev_set_ethtool_ops(dev, ops) dev->ethtool_ops = ops
#else
#define dev_set_ethtool_ops(dev, ops) (void)ops
#endif

... then it should all fall out silently. (Yeah, those definitions would
need 'hardening' but they're a proof of concept).

> And at which point you'll have a broken system for drivers that
> enable LRO and the user enables forwarding.

Obviously that needs avoiding. Thanks for the technical feedback.

After an offline discussion, I understand that if we can sort out the
actual technical issues, you'll carry this in the net tree. Thanks.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 3/4] Configure out ethtool support

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 12:29:30 +0100

> After an offline discussion, I understand that if we can sort out the
> actual technical issues, you'll carry this in the net tree. Thanks.

I will, but only because you threatened to bypass me and send them
directly to Linus.  And frankly fighting someone willing to do things
like that is simply not worth my time, so I'll just merge them blindly.
You just let me know when you think they are ready for inclusion.

Re: [patch 3/4] Configure out ethtool support

[David Woodhouse]

On Thu, 2008-07-31 at 04:33 -0700, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Thu, 31 Jul 2008 12:29:30 +0100
> 
> > After an offline discussion, I understand that if we can sort out the
> > actual technical issues, you'll carry this in the net tree. Thanks.
> 
> I will, but only because you threatened to bypass me and send them
> directly to Linus.  And frankly fighting someone willing to do things
> like that is simply not worth my time, so I'll just merge them
> blindly.

That doesn't make a lot of sense, Dave. Just because I _submit_ them to
Linus, that doesn't mean he automatically takes them.

I only said I'd submit them directly to Linus because I _think_ he'd
agree with Andrew and I, and take them despite your objections. And
because I think that's the right thing for him to do.

I wasn't going to hack hera and just put them into his tree by myself.

But don't let me talk you out of taking the patches... :)

> You just let me know when you think they are ready for inclusion.

I'll do that; thanks.

-- 
dwmw2

Re: [patch 3/4] Configure out ethtool support

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 12:46:41 +0100

> I only said I'd submit them directly to Linus because I _think_ he'd
> agree with Andrew and I, and take them despite your objections. And
> because I think that's the right thing for him to do.

I guess Linus is unable to participate in the discussion, voice his
opinion, and sort this out with the rest of us unless you submit the
patches directly to him for inclusion, right?

Can you at least begin to see why your doing that might irritate me?

> > You just let me know when you think they are ready for inclusion.
> 
> I'll do that; thanks.

No problem.

Re: [patch 3/4] Configure out ethtool support

[Adrian Bunk]

On Thu, Jul 31, 2008 at 12:46:41PM +0100, David Woodhouse wrote:
> On Thu, 2008-07-31 at 04:33 -0700, David Miller wrote:
> > From: David Woodhouse <dwmw2@infradead.org>
> > Date: Thu, 31 Jul 2008 12:29:30 +0100
> > 
> > > After an offline discussion, I understand that if we can sort out the
> > > actual technical issues, you'll carry this in the net tree. Thanks.
> > 
> > I will, but only because you threatened to bypass me and send them
> > directly to Linus.  And frankly fighting someone willing to do things
> > like that is simply not worth my time, so I'll just merge them
> > blindly.
> 
> That doesn't make a lot of sense, Dave. Just because I _submit_ them to
> Linus, that doesn't mean he automatically takes them.
> 
> I only said I'd submit them directly to Linus because I _think_ he'd
> agree with Andrew and I, and take them despite your objections. And
> because I think that's the right thing for him to do.
>...

I'm sure we can find simpler and less controversial ways to save 6 kB in 
the network stack. E.g. Ilpo's past work on making inline functions in 
the network stack out-of-line had far bigger effects than the patch in 
this discussion. And as a bonus, his work brings benefits to everyone.

It might have also made more sense to spend some of the energy used in 
this discussion instead on checking where the global 24 kB size increase 
Thomas reported for 2.6.27-rc1 compared to 2.6.26 comes from.

It's kinda silly to spend time on creating and arguing about non-trivial 
patches that might save a few kB for very few people while noone seems 
to work on reducing the continuous size increase of the kernel that 
affects everyone...

> dwmw2

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 3/4] Configure out ethtool support

[Thomas Petazzoni]

Le Thu, 31 Jul 2008 18:58:20 +0300,
Adrian Bunk <bunk@kernel.org> a écrit :

> It might have also made more sense to spend some of the energy used
> in this discussion instead on checking where the global 24 kB size
> increase Thomas reported for 2.6.27-rc1 compared to 2.6.26 comes from.

I agree. This is something I'm working on. I hope to have results soon.
But I *fear* that the results will be: the size growth is 10 bytes
here, 22 bytes there, 35 bytes here, spread over hundreds of patches.
Something we can do anything against.

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

[patch 4/4] Configure out IGMP support

[Thomas Petazzoni]

[-- Attachment #1: configure-out-igmp-support --]
[-- Type: text/plain, Size: 5718 bytes --]

This patchs adds the CONFIG_IGMP option which allows to remove support
for the Internet Group Management Protocol, used in
multicast. Multicast is not necessarly used by applications,
particularly on embedded devices. As this is a size-reduction option,
it depends on CONFIG_EMBEDDED. It allows to save ~10 kilobytes of
kernel code/data:

   text	   data	    bss	    dec	    hex	filename
1718857	 143672	 221184	2083713	 1fcb81	vmlinux
1708838	 143640	 221184	2073662	 1fa43e	vmlinux.new
 -10019     -32       0  -10051   -2743 +/-

This patch has been originally written by Matt Mackall
<mpm@selenic.com>, and is part of the Linux Tiny project.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Signed-off-by: Matt Mackall <mpm@selenic.com>
Cc: netdev@vger.kernel.org
Cc: davem@davemloft.net
Cc: mpm@selenic.com
Cc: akpm@linux-foundation.org

---
 include/linux/igmp.h       |   20 ++++++++++++++++++++
 init/Kconfig               |    8 ++++++++
 net/ipv4/Makefile          |    3 ++-
 net/ipv4/af_inet.c         |    2 --
 net/ipv4/ip_sockglue.c     |    4 ++++
 net/ipv4/sysctl_net_ipv4.c |    2 ++
 6 files changed, 36 insertions(+), 3 deletions(-)

Index: linuxdev/include/linux/igmp.h
===================================================================
--- linuxdev.orig/include/linux/igmp.h
+++ linuxdev/include/linux/igmp.h
@@ -215,6 +215,7 @@
 #define IGMPV3_QQIC(value) IGMPV3_EXP(0x80, 4, 3, value)
 #define IGMPV3_MRC(value) IGMPV3_EXP(0x80, 4, 3, value)
 
+#ifdef CONFIG_IGMP
 extern int ip_check_mc(struct in_device *dev, __be32 mc_addr, __be32 src_addr, u16 proto);
 extern int igmp_rcv(struct sk_buff *);
 extern int ip_mc_join_group(struct sock *sk, struct ip_mreqn *imr);
@@ -235,6 +236,25 @@
 extern void ip_mc_dec_group(struct in_device *in_dev, __be32 addr);
 extern void ip_mc_inc_group(struct in_device *in_dev, __be32 addr);
 extern void ip_mc_rejoin_group(struct ip_mc_list *im);
+#else /* !CONFIG_IGMP */
+#define ip_check_mc(a, b, c, d) (0)
+#define igmp_rcv(a) (0)
+#define ip_mc_join_group(a, b) (0)
+#define ip_mc_leave_group(a, b) (0)
+#define ip_mc_drop_socket(a)
+#define ip_mc_source(a, b, c, d, e) (0)
+#define ip_mc_msfilter(a, b, c) (0)
+#define ip_mc_msfget(a, b, c, d) (0)
+#define ip_mc_gsfget(a, b, c, d) (0)
+#define ip_mc_sf_allow(a, b, c, d) (0)
+#define ip_mc_init_dev(a)
+#define ip_mc_destroy_dev(a)
+#define ip_mc_up(a)
+#define ip_mc_down(a)
+#define ip_mc_dec_group(a)
+#define ip_mc_inc_group(a)
+#define ip_mc_rejoin_group(a)
+#endif /* CONFIG_IGMP */
 
 #endif
 #endif
Index: linuxdev/init/Kconfig
===================================================================
--- linuxdev.orig/init/Kconfig
+++ linuxdev/init/Kconfig
@@ -748,6 +748,14 @@
 	  Disabling this option removes support for configuring
 	  ethernet device features via ethtool. Saves about 6k.
 
+config IGMP
+	depends on INET
+	bool "Enable IGMP support" if EMBEDDED && !IP_MULTICAST
+	default y
+	help
+	  Disabling this option removes support for the Internet group
+          management protocol, used for multicast. Saves about 10k.
+
 config VM_EVENT_COUNTERS
 	default y
 	bool "Enable VM event counters for /proc/vmstat" if EMBEDDED
Index: linuxdev/net/ipv4/Makefile
===================================================================
--- linuxdev.orig/net/ipv4/Makefile
+++ linuxdev/net/ipv4/Makefile
@@ -9,13 +9,14 @@
 	     tcp.o tcp_input.o tcp_output.o tcp_timer.o tcp_ipv4.o \
 	     tcp_minisocks.o tcp_cong.o \
 	     datagram.o raw.o udp.o udplite.o \
-	     arp.o icmp.o devinet.o af_inet.o  igmp.o \
+	     arp.o icmp.o devinet.o af_inet.o \
 	     fib_frontend.o fib_semantics.o \
 	     inet_fragment.o
 
 obj-$(CONFIG_SYSCTL) += sysctl_net_ipv4.o
 obj-$(CONFIG_IP_FIB_HASH) += fib_hash.o
 obj-$(CONFIG_IP_FIB_TRIE) += fib_trie.o
+obj-$(CONFIG_IGMP) += igmp.o
 obj-$(CONFIG_PROC_FS) += proc.o
 obj-$(CONFIG_IP_MULTIPLE_TABLES) += fib_rules.o
 obj-$(CONFIG_IP_MROUTE) += ipmr.o
Index: linuxdev/net/ipv4/af_inet.c
===================================================================
--- linuxdev.orig/net/ipv4/af_inet.c
+++ linuxdev/net/ipv4/af_inet.c
@@ -115,8 +115,6 @@
 #include <linux/mroute.h>
 #endif
 
-extern void ip_mc_drop_socket(struct sock *sk);
-
 /* The inetsw table contains everything that inet_create needs to
  * build a new socket.
  */
Index: linuxdev/net/ipv4/ip_sockglue.c
===================================================================
--- linuxdev.orig/net/ipv4/ip_sockglue.c
+++ linuxdev/net/ipv4/ip_sockglue.c
@@ -640,6 +640,7 @@
 			err = ip_mc_leave_group(sk, &mreq);
 		break;
 	}
+#ifdef CONFIG_IGMP
 	case IP_MSFILTER:
 	{
 		extern int sysctl_igmp_max_msf;
@@ -677,6 +678,7 @@
 		kfree(msf);
 		break;
 	}
+#endif
 	case IP_BLOCK_SOURCE:
 	case IP_UNBLOCK_SOURCE:
 	case IP_ADD_SOURCE_MEMBERSHIP:
@@ -794,6 +796,7 @@
 				   greqs.gsr_interface);
 		break;
 	}
+#ifdef CONFIG_IGMP
 	case MCAST_MSFILTER:
 	{
 		extern int sysctl_igmp_max_msf;
@@ -860,6 +863,7 @@
 		kfree(gsf);
 		break;
 	}
+#endif
 	case IP_ROUTER_ALERT:
 		err = ip_ra_control(sk, val ? 1 : 0, NULL);
 		break;
Index: linuxdev/net/ipv4/sysctl_net_ipv4.c
===================================================================
--- linuxdev.orig/net/ipv4/sysctl_net_ipv4.c
+++ linuxdev/net/ipv4/sysctl_net_ipv4.c
@@ -412,6 +412,7 @@
 	},
 
 #endif
+#ifdef CONFIG_IGMP
 	{
 		.ctl_name	= NET_IPV4_IGMP_MAX_MSF,
 		.procname	= "igmp_max_msf",
@@ -420,6 +421,7 @@
 		.mode		= 0644,
 		.proc_handler	= &proc_dointvec
 	},
+#endif
 	{
 		.ctl_name	= NET_IPV4_INET_PEER_THRESHOLD,
 		.procname	= "inet_peer_threshold",

-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 4/4] Configure out IGMP support

[David Woodhouse]

On Thu, 2008-07-31 at 11:27 +0200, Thomas Petazzoni wrote:
> This patchs adds the CONFIG_IGMP option which allows to remove support
> for the Internet Group Management Protocol, used in
> multicast. Multicast is not necessarly used by applications,
> particularly on embedded devices. As this is a size-reduction option,
> it depends on CONFIG_EMBEDDED. It allows to save ~10 kilobytes of
> kernel code/data:

The config option probably lives in net/Kconfig, not init/Kconfig.

And please could you make it clear how this interacts with IP_MULTICAST?

We already have a CONFIG_IP_MULTICAST option, for which the help text
says "For more people, it's safe to say N'. And I think it defaults to
that too. What more does CONFIG_IGMP remove? It's not made clear by the
help text.

-- 
dwmw2

Re: [patch 4/4] Configure out IGMP support

[Thomas Petazzoni]

Le Fri, 01 Aug 2008 20:41:55 +0100,
David Woodhouse <dwmw2@infradead.org> a écrit :

> The config option probably lives in net/Kconfig, not init/Kconfig.

Yes, it could. But AFAIK, until now, all CONFIG_EMBEDDED-related
options have been put in init/Kconfig. But if it's preferred, I can of
course change the patch to move the config option to net/Kconfig.

> And please could you make it clear how this interacts with
> IP_MULTICAST?
> 
> We already have a CONFIG_IP_MULTICAST option, for which the help text
> says "For more people, it's safe to say N'. And I think it defaults to
> that too. What more does CONFIG_IGMP remove? It's not made clear by
> the help text.

The interaction of IGMP support with CONFIG_IP_MULTICAST is fairly
unclear to me.

A large portion of igmp.c is already under #ifdef CONFIG_IP_MULTICAST:
all the igmp_*() functions, amongst which is igmp_rcv(), referenced in
igmp_protocol in net/ipv4/af_inet.c, which is compiled-out
when !CONFIG_IP_MULTICAST.

All the proc-related code at the end of the file is only conditionnaly
compiled on CONFIG_PROC_FS, but seems to in fact be only used if both
CONFIG_IP_MULTICAST and CONFIG_PROC_FS are selected:
igmp_mc_proc_init() in net/ipv4/ip_output.c is only called when
CONFIG_IP_MULTICAST and CONFIG_PROC_FS are selected.

Besides that, it's unclear to me why the ip_mc_*() functions are useful
when !CONFIG_IP_MULTICAST, but I'm probably missing something. They are
used to implement setsockopt-operations related to multicast, hooks for
the routing code to handle multicast-related traffic, etc.

Sincerly,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

Re: [patch 4/4] Configure out IGMP support

[Adrian Bunk]

On Mon, Aug 04, 2008 at 02:48:07PM +0200, Thomas Petazzoni wrote:
> Le Fri, 01 Aug 2008 20:41:55 +0100,
> David Woodhouse <dwmw2@infradead.org> a écrit :
> 
> > The config option probably lives in net/Kconfig, not init/Kconfig.
> 
> Yes, it could. But AFAIK, until now, all CONFIG_EMBEDDED-related
> options have been put in init/Kconfig.
>...

No, the vast majority is actually outside of init/Kconfig.

> Sincerly,
> 
> Thomas

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 4/4] Configure out IGMP support

[David Woodhouse]

On Mon, 2008-08-04 at 14:48 +0200, Thomas Petazzoni wrote:
> Le Fri, 01 Aug 2008 20:41:55 +0100,
> David Woodhouse <dwmw2@infradead.org> a écrit :
> 
> > The config option probably lives in net/Kconfig, not init/Kconfig.
> 
> Yes, it could. But AFAIK, until now, all CONFIG_EMBEDDED-related
> options have been put in init/Kconfig. But if it's preferred, I can of
> course change the patch to move the config option to net/Kconfig.

It clearly lives in net/Kconfig.

> > And please could you make it clear how this interacts with
> > IP_MULTICAST?
> > 
> > We already have a CONFIG_IP_MULTICAST option, for which the help text
> > says "For more people, it's safe to say N'. And I think it defaults to
> > that too. What more does CONFIG_IGMP remove? It's not made clear by
> > the help text.
> 
> The interaction of IGMP support with CONFIG_IP_MULTICAST is fairly
> unclear to me.
> 
> A large portion of igmp.c is already under #ifdef CONFIG_IP_MULTICAST:
> all the igmp_*() functions, amongst which is igmp_rcv(), referenced in
> igmp_protocol in net/ipv4/af_inet.c, which is compiled-out
> when !CONFIG_IP_MULTICAST.
> 
> All the proc-related code at the end of the file is only conditionnaly
> compiled on CONFIG_PROC_FS, but seems to in fact be only used if both
> CONFIG_IP_MULTICAST and CONFIG_PROC_FS are selected:
> igmp_mc_proc_init() in net/ipv4/ip_output.c is only called when
> CONFIG_IP_MULTICAST and CONFIG_PROC_FS are selected.
> 
> Besides that, it's unclear to me why the ip_mc_*() functions are useful
> when !CONFIG_IP_MULTICAST, but I'm probably missing something. 

Most of them aren't, as far as I can tell. 

> They are used to implement setsockopt-operations related to multicast,
> hooks for the routing code to handle multicast-related traffic, etc.

I wonder if those options should return errors now, rather than silently
failing but returning zero. Or maybe that _would_ cause a stock build of
ntpd to fail? Not that it really matters if it _does_, though.

It sounds like 'CONFIG_IGMP' is a bad name for the option, too -- and
the help text is similarly misleading. I think you need to work out how
it all fits together with CONFIG_IP_MULTICAST, fix it up, and resubmit
it.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Miller]

From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
Date: Thu, 31 Jul 2008 11:27:03 +0200

> Changes since previous post:
> 
>  * Add Matt Mackall's Signed-off-by on all patches
>  * Make bonding and bridging select ethtool in the ethtool-related
>    patch.

The ethtool config option needs to be selected by CONFIG_INET as well,
as per the things I described today.  Which erodes it's usefulness
tremendously.

I also am keeping my stance that I have no current intention of
applying these patches.

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Woodhouse]

On Thu, 2008-07-31 at 02:40 -0700, David Miller wrote:
> From: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> Date: Thu, 31 Jul 2008 11:27:03 +0200
> 
> > Changes since previous post:
> > 
> >  * Add Matt Mackall's Signed-off-by on all patches
> >  * Make bonding and bridging select ethtool in the ethtool-related
> >    patch.
> 
> The ethtool config option needs to be selected by CONFIG_INET as well,
> as per the things I described today.  Which erodes it's usefulness
> tremendously.
> 
> I also am keeping my stance that I have no current intention of
> applying these patches.

I would very much like you to reconsider, Dave.

We can make them default to 'y' and hide them behind CONFIG_EMBEDDED,
and put in a scary help text that tells people _never_ to turn it off --
and hell, you can even make a taint flag for it if you like. But there
are a lot of people who really don't need these features and really want
the option of leaving them out.

Perhaps we should add a warning printk when one of these features is
'required' but absent. That would help to make it clear when someone has
disabled a feature which they shouldn't have disabled.

Refusing to apply the patches just means that either those people will
get them from elsewhere, or that their kernel will be more bloated than
it needs to be.

-- 
dwmw2

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 10:51:52 +0100

> But there are a lot of people who really don't need these features
> and really want the option of leaving them out.

I'll say it one last time.

If you have ipv4 enabled, you need ETHTOOL.

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Woodhouse]

On Thu, 2008-07-31 at 02:55 -0700, David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Thu, 31 Jul 2008 10:51:52 +0100
> 
> > But there are a lot of people who really don't need these features
> > and really want the option of leaving them out.
> 
> I'll say it one last time.
> 
> If you have ipv4 enabled, you need ETHTOOL.

I have drivers which don't even _have_ ethtool support, and they seem to
work fine. But leaving aside the debate on that point, your statement
also seemed to be covering the other patches, such as the IGMP one and
others which haven't been seen (or perhaps even imagined) yet.

-- 
dwmw2

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 10:59:15 +0100

> I have drivers which don't even _have_ ethtool support, and they seem to
> work fine. But leaving aside the debate on that point, your statement
> also seemed to be covering the other patches, such as the IGMP one and
> others which haven't been seen (or perhaps even imagined) yet.

I explained why I didn't want to apply the IGMP one too.

Andrew didn't like my objections, but that doesn't mean I
need to defend my position further.

Show me a size reduction patch for networking that actually
makes real sense and I'll apply it.

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Woodhouse]

On Thu, 2008-07-31 at 03:02 -0700, David Miller wrote:
> I explained why I didn't want to apply the IGMP one too.
>
> Andrew didn't like my objections, but that doesn't mean I
> need to defend my position further.

You said that it was part of the core BSD socket API and "Like TCP and
UDP, multicast capabilities are something applications can always depend
upon being available".

Andrew's response was that embedded devices have full control over their
userspace and that they are in a position to _know_ that they never use
multicast, so your argument is bogus. If they _know_ they don't want
multicast, it makes a lot of sense for them to turn it off.

While I agree with Andrew's observation, I'd also respectfully submit
that your argument is more fundamentally bogus than that. TCP and UDP
are _not_ universally available. They go away if you set CONFIG_INET=n.

-- 
dwmw2

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[David Miller]

From: David Woodhouse <dwmw2@infradead.org>
Date: Thu, 31 Jul 2008 11:15:16 +0100

> While I agree with Andrew's observation, I'd also respectfully submit
> that your argument is more fundamentally bogus than that. TCP and UDP
> are _not_ universally available. They go away if you set CONFIG_INET=n.

Like I said, people can locally patch their systems if they really
want to rip out fundamental things like multicast support.

Some folks might find it instructive to do a google code search
or similar on the multicast socket options this things dikes out
of the tree.

Even simple things like NTP will spew failures with this CONFIG_IGMP
thing turned off.  And that's just the tip of the iceberg.

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[Tim Bird]

David Miller wrote:
> Some folks might find it instructive to do a google code search
> or similar on the multicast socket options this things dikes out
> of the tree.
> 
> Even simple things like NTP will spew failures with this CONFIG_IGMP
> thing turned off.  And that's just the tip of the iceberg.

I don't know of any embedded products that ship with NTP turned
on.  It's best to assume, with embedded, that we're not shipping
ANY of the desktop or server applications you are familiar with.
Absent those, does something break in the kernel with multicast
support when IGMP is turned off?
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

Re: [patch 0/4] [resend] Add configuration options to disable features

[Ulrich Teichert]

Hi,

>I don't know of any embedded products that ship with NTP turned
>on.

Well, I do. To be exact, I've developed parts of it. But it's numbers
are only into the thousands, so that makes it insignificant ;-)

>It's best to assume, with embedded, that we're not shipping
>ANY of the desktop or server applications you are familiar with.
>Absent those, does something break in the kernel with multicast
>support when IGMP is turned off?

I do not think of NTP as desktop or server application, but that's
probably just me,

CU,
Uli
-- 
Dipl. Inf. Ulrich Teichert|e-mail: Ulrich.Teichert@gmx.de | Listening to:
Stormweg 24               |Pale Bride (The Von Bondies), No Time (Statues),
24539 Neumuenster, Germany|Am Strand (Smoke Blow), Sacred Decay (The Estranged)

Re: [patch 0/4] [resend] Add configuration options to disable features

[Josh Boyer]

On Thu, 2008-07-31 at 20:50 +0200, Ulrich Teichert wrote:
> Hi,
> 
> >I don't know of any embedded products that ship with NTP turned
> >on.
> 
> Well, I do. To be exact, I've developed parts of it. But it's numbers
> are only into the thousands, so that makes it insignificant ;-)
> 
> >It's best to assume, with embedded, that we're not shipping
> >ANY of the desktop or server applications you are familiar with.
> >Absent those, does something break in the kernel with multicast
> >support when IGMP is turned off?
> 
> I do not think of NTP as desktop or server application, but that's
> probably just me,

No, it's not just you.  NTP is useful in cases where things do care
about time but hardware designers were too cheap to put an RTC on the
board.

I will admit that it's use in embedded products is probably very limited
though.

josh

Re: [patch 0/4] [resend] Add configuration options to disable features

[David Woodhouse]

On Thu, 2008-07-31 at 15:46 -0400, Josh Boyer wrote:
> On Thu, 2008-07-31 at 20:50 +0200, Ulrich Teichert wrote:
> > Hi,
> > 
> > >I don't know of any embedded products that ship with NTP turned
> > >on.
> > 
> > Well, I do. To be exact, I've developed parts of it. But it's numbers
> > are only into the thousands, so that makes it insignificant ;-)
> > 
> > >It's best to assume, with embedded, that we're not shipping
> > >ANY of the desktop or server applications you are familiar with.
> > >Absent those, does something break in the kernel with multicast
> > >support when IGMP is turned off?
> > 
> > I do not think of NTP as desktop or server application, but that's
> > probably just me,
> 
> No, it's not just you.  NTP is useful in cases where things do care
> about time but hardware designers were too cheap to put an RTC on the
> board.
> 
> I will admit that it's use in embedded products is probably very limited
> though.

NTP is a red herring. It has a check for multicast support in its
configure script and wraps it all in #ifdef MCAST anyway.

So even if it _does_ crap out when I build my standard distro kernel
with !CONFIG_IGMP and use the standard distro build of ntpd, that still
isn't particularly relevant to the kind of application where someone
would built a kernel without IGMP support and build their own ntpd to
match.

-- 
dwmw2

Re: [patch 0/4] [resend] Add configuration options to disable features

[Robert Schwebel]

On Thu, Jul 31, 2008 at 03:46:28PM -0400, Josh Boyer wrote:
> > I do not think of NTP as desktop or server application, but that's
> > probably just me,
>
> No, it's not just you. NTP is useful in cases where things do care
> about time but hardware designers were too cheap to put an RTC on the
> board.

Yes, we also have such customer systems, i.e. one which is used in a
Telemetry application where you can have hundrets of PXA270 data
concentrators that collect data from FPGAs and push them to a PC via
ethernet. As the system does not work without the PC, the hardware
designers decided that they can save hundrets of RTCs.

Or another autonomous data collection system where only one system of
several tens has an RTC...

We used chrony in these cases, as the standard ntputils seem to be
optimized for scenarios where you have permanent network connection,
which is often not the case in embedded applications.

rsc
-- 
 Dipl.-Ing. Robert Schwebel | http://www.pengutronix.de
 Pengutronix - Linux Solutions for Science and Industry
   Handelsregister:  Amtsgericht Hildesheim, HRA 2686
     Hannoversche Str. 2, 31134 Hildesheim, Germany
   Phone: +49-5121-206917-0 |  Fax: +49-5121-206917-9

Re: [patch 0/4] [resend] Add configuration options to disable features

[Linus Torvalds]

On Thu, 31 Jul 2008, Josh Boyer wrote:
> On Thu, 2008-07-31 at 20:50 +0200, Ulrich Teichert wrote:
> > 
> > I do not think of NTP as desktop or server application, but that's
> > probably just me,
> 
> No, it's not just you.  NTP is useful in cases where things do care
> about time but hardware designers were too cheap to put an RTC on the
> board.

In fact, didn't one of the netgear firewall/switch/routers end up being 
famous for overloading some NTP service exactly because all the _millions_ 
of routers ended up using the same (incorrect) NTP host?

So NTP is very definitely an embedded thing too.

			Linus

Re: [patch 0/4] [resend] Add configuration options to disable features

[David Woodhouse]

On Fri, 2008-08-01 at 12:15 -0700, Linus Torvalds wrote:
> 
> On Thu, 31 Jul 2008, Josh Boyer wrote:
> > On Thu, 2008-07-31 at 20:50 +0200, Ulrich Teichert wrote:
> > > 
> > > I do not think of NTP as desktop or server application, but that's
> > > probably just me,
> > 
> > No, it's not just you.  NTP is useful in cases where things do care
> > about time but hardware designers were too cheap to put an RTC on the
> > board.
> 
> In fact, didn't one of the netgear firewall/switch/routers end up being 
> famous for overloading some NTP service exactly because all the _millions_ 
> of routers ended up using the same (incorrect) NTP host?
> 
> So NTP is very definitely an embedded thing too.

And it's _still_ a red herring.

I just booted a !CONFIG_IGMP kernel on my workstation and NTP is running
just fine (as is IPv6).

Even though ntpd has a configure test for multicast and can be built
without multicast support at all, it isn't even necessary to build it
that way -- the stock Fedora build of it works just fine.

(Actually, I never managed to get ntpd to work _with_ multicast, but
that's a different issue... :)

-- 
dwmw2

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[Tim Bird]

David Miller wrote:
> From: David Woodhouse <dwmw2@infradead.org>
> Date: Thu, 31 Jul 2008 10:51:52 +0100
> 
>> But there are a lot of people who really don't need these features
>> and really want the option of leaving them out.
> 
> I'll say it one last time.
> 
> If you have ipv4 enabled, you need ETHTOOL.

I've been using and administering Linux for 16 years,
and using it successfully in embedded projects for 10.
Until I stumbled upon this patch in Linux-tiny, I had
never heard of ethtool.  Sony has shipped hundreds
of thousands of products with ETHTOOL turned off.

It sounds like you don't want to talk about it any more,
but could you please give me the 30-second overview
of why ETHTOOL is required for proper ipv4 operation?

If Sony is shipping network-buggy products I certainly
want to know about it.

Sorry if I missed an earlier explanation.

Thanks,
 -- Tim

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

Re: [patch 0/4] [resend] Add configuration options to disable features not needed on embedded devices

[Tim Bird]

Tim Bird wrote:
> It sounds like you don't want to talk about it any more,
> but could you please give me the 30-second overview
> of why ETHTOOL is required for proper ipv4 operation?
...
> Sorry if I missed an earlier explanation.

Never mind.  I hadn't read the whole thread yet.

=============================
Tim Bird
Architecture Group Chair, CE Linux Forum
Senior Staff Engineer, Sony Corporation of America
=============================

Re: [patch 2/4] Configure out file locking features

[Matthew Wilcox]

On Tue, Jul 29, 2008 at 05:45:22PM +0200, Thomas Petazzoni wrote:
> This patch adds the CONFIG_FILE_LOCKING option which allows to remove
> support for advisory locks. With this patch enabled, the flock()
> system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
> and NFS support are disabled. These features are not necessarly needed
> on embedded systems. It allows to save ~11 Kb of kernel code and data:
> 
>    text	   data	    bss	    dec	    hex	filename
> 1125436	 118764	 212992	1457192	 163c28	vmlinux.old
> 1114299	 118564	 212992	1445855	 160fdf	vmlinux
>  -11137    -200       0  -11337   -2C49 +/-
> 
> This patch has originally been written by Matt Mackall
> <mpm@selenic.com>, and is part of the Linux Tiny project.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

In principle, I think this is a great idea.

>  config NFS_FS
>  	tristate "NFS client support"
> -	depends on INET
> +	depends on INET && FILE_LOCKING
>  	select LOCKD
>  	select SUNRPC
>  	select NFS_ACL_SUPPORT if NFS_V3_ACL

I think this part is a little lazy.  It should be possible to support
NFS without file locking.  I suspect that's really not in-scope for the
linux-tiny tree as currently envisaged with the focus on embedded
devices that probably don't use NFS anyway.  Do we want to care about
the situation of a machine with fixed workload, that doesn't need file
locking, but does use NFS?

-- 
Intel are signing my paycheques ... these opinions are still mine
"Bill, look, we understand that you're interested in selling us this
operating system, but compare it to ours.  We can't possibly take such
a retrograde step."

Re: [patch 2/4] Configure out file locking features

[Matt Mackall]

On Tue, 2008-07-29 at 12:17 -0600, Matthew Wilcox wrote:
> On Tue, Jul 29, 2008 at 05:45:22PM +0200, Thomas Petazzoni wrote:
> > This patch adds the CONFIG_FILE_LOCKING option which allows to remove
> > support for advisory locks. With this patch enabled, the flock()
> > system call, the F_GETLK, F_SETLK and F_SETLKW operations of fcntl()
> > and NFS support are disabled. These features are not necessarly needed
> > on embedded systems. It allows to save ~11 Kb of kernel code and data:
> > 
> >    text	   data	    bss	    dec	    hex	filename
> > 1125436	 118764	 212992	1457192	 163c28	vmlinux.old
> > 1114299	 118564	 212992	1445855	 160fdf	vmlinux
> >  -11137    -200       0  -11337   -2C49 +/-
> > 
> > This patch has originally been written by Matt Mackall
> > <mpm@selenic.com>, and is part of the Linux Tiny project.
> > 
> > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
> 
> In principle, I think this is a great idea.
> 
> >  config NFS_FS
> >  	tristate "NFS client support"
> > -	depends on INET
> > +	depends on INET && FILE_LOCKING
> >  	select LOCKD
> >  	select SUNRPC
> >  	select NFS_ACL_SUPPORT if NFS_V3_ACL
> 
> I think this part is a little lazy.  It should be possible to support
> NFS without file locking.  I suspect that's really not in-scope for the
> linux-tiny tree as currently envisaged with the focus on embedded
> devices that probably don't use NFS anyway.  Do we want to care about
> the situation of a machine with fixed workload, that doesn't need file
> locking, but does use NFS?

I would lean towards no, but if someone comes along who cares, they're
welcome to try it. This stuff all has to strike a balance between
savings and effort/complexity/maintainability, so any time the submitter
is too lazy to cover a less common use case, it's probably a good sign
they're approaching that tipping point. 

On the other hand, if you think it's trivial to do a locking-ectomy on
NFS, I'd be happy to see it.

The typical embedded NFS-based devices are NAS servers and media players
and are going to be more concerned about things like page cache
balancing.

-- 
Mathematics is the supreme nostalgia of our time.

Re: [patch 2/4] Configure out file locking features

[Jamie Lokier]

Matt Mackall wrote:
> The typical embedded NFS-based devices are NAS servers and media players
> and are going to be more concerned about things like page cache
> balancing.

Oh, those.

It would be really annoying to buy a home NAS and find it doesn't
support NFS locks or SMB oplocks.  NASes are vaguely useful for more
than one computer in the house at the same time.

That said, I bought a big, expensive one, found it far too slow for my
needs, send it back for a refund and bought a portable cheap USB disk
which had *so* much higher performance.  The convenience of serving
multiple machines just wasn't worth the lousy performance.

-- Jamie

Re: [patch 2/4] Configure out file locking features

[Adrian Bunk]

It seems the emails containing the patches never made it to the vger 
lists, which makes it a bit hard to comment on them.

Thomas, can you try to figure out what went wrong and resend them then?

cu
Adrian

-- 

       "Is there not promise of rain?" Ling Tan asked suddenly out
        of the darkness. There had been need of rain for many days.
       "Only a promise," Lao Er said.
                                       Pearl S. Buck - Dragon Seed

Re: [patch 2/4] Configure out file locking features

[Thomas Petazzoni]

[-- Attachment #1: Type: text/plain, Size: 2070 bytes --]

Le Wed, 30 Jul 2008 17:27:54 +0300,
Adrian Bunk <bunk@kernel.org> a écrit :

> It seems the emails containing the patches never made it to the vger 
> lists, which makes it a bit hard to comment on them.

Yes, they didn't make it to the lists, for some reason. Maybe it's
because I sent them using "quilt mail --send", which uses my local
exim4 mail server, and for some reason, vger doesn't like that kind of
e-mails. However, my exim4 is configured to sent the e-mails through by
ISP SMTP. From my exim4 mainlog:

2008-07-29 17:47:57 1KNrQS-0007kt-Kg => linux-kernel@vger.kernel.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> linux-embedded@vger.kernel.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> netdev@vger.kernel.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> davem@davemloft.net R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> mpm@selenic.com R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> akpm@linux-foundation.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> thomas.petazzoni@free-electrons.com R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> michael@free-electrons.com R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
2008-07-29 17:47:57 1KNrQS-0007kt-Kg Completed

So from my side, everything seemed to work well. Does anybody has a
clue ?

> Thomas, can you try to figure out what went wrong and resend them
> then?

I will send them again through my normal MUA, using quilt mail --mbox.
Hopefully it'll work.

Sincerly,

Thomas
-- 
Thomas Petazzoni, Free Electrons
Kernel, drivers and embedded Linux development,
consulting, training and support.
http://free-electrons.com

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: [patch 2/4] Configure out file locking features

[Uwe Kleine-König]

Hello Thomas,

Thomas Petazzoni wrote:
> Le Wed, 30 Jul 2008 17:27:54 +0300,
> Adrian Bunk <bunk@kernel.org> a écrit :
> 
> > It seems the emails containing the patches never made it to the vger 
> > lists, which makes it a bit hard to comment on them.
> 
> Yes, they didn't make it to the lists, for some reason. Maybe it's
> because I sent them using "quilt mail --send", which uses my local
> exim4 mail server, and for some reason, vger doesn't like that kind of
> e-mails. However, my exim4 is configured to sent the e-mails through by
> ISP SMTP. From my exim4 mainlog:
> 
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg => linux-kernel@vger.kernel.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> linux-embedded@vger.kernel.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> netdev@vger.kernel.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> davem@davemloft.net R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> mpm@selenic.com R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> akpm@linux-foundation.org R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> thomas.petazzoni@free-electrons.com R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg -> michael@free-electrons.com R=smarthost T=remote_smtp_smarthost H=smtp.free.fr [212.27.48.4]*
> 2008-07-29 17:47:57 1KNrQS-0007kt-Kg Completed
> 
> So from my side, everything seemed to work well. Does anybody has a
> clue ?
Don't know if this is the problem, but I had problems getting mails
through some time ago, too.  For me the problem was that the source
address was myusername@mymachine.intranet.$mycompany.$tld and
vger.kernel.org didn't want to take these because the host name had no
DNS entry.

I fixed that by rewriting the From line in outgoing mails on my host.
I'm using postfix (from Debian) so I had to add an entry to
/etc/postfix/sender_canonical.

Best regards
Uwe

-- 
Uwe Kleine-König, Software Engineer
Digi International GmbH Branch Breisach, Küferstrasse 8, 79206 Breisach, Germany
Tax: 315/5781/0242 / VAT: DE153662976 / Reg. Amtsgericht Dortmund HRB 13962
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html