[PATCH] Allow to disable shmem.o

[PATCH] Allow to disable shmem.o

[Andi Kleen]

This patch allows to not compile mm/shmem.o in. It is now dependent on
CONFIG_TMPFS, which is the main user. This allows shrinking the kernel
a bit more when needed. Disabling TMPFS disables the mmap
MAP_ANONYMOUS|MAP_SHARED functionality now. That's ok, because Linux
2.2 also didn't have it iirc. To make sure this doesn't happen by
accident I made the TMPFS configuration dependent on CONFIG_EMBEDDED.
This means a normal configuration enables TMPFS, which is ok 
since it is only minor additional overhead to shmem.o and also quite
useful for shared memory.

Patch for 2.6.7, but applies with some offsets to 2.6.8rc3

-Andi

diff -u linux-2.6.7-boot64/drivers/char/mem.c-SHMEM linux-2.6.7-boot64/drivers/char/mem.c
diff -u linux-2.6.7-boot64/include/linux/mm.h-SHMEM linux-2.6.7-boot64/include/linux/mm.h
--- linux-2.6.7-boot64/include/linux/mm.h-SHMEM	2004-06-16 12:23:40.000000000 +0200
+++ linux-2.6.7-boot64/include/linux/mm.h	2004-08-08 00:06:04.000000000 +0200
@@ -496,7 +496,15 @@
 					unsigned long addr);
 struct file *shmem_file_setup(char * name, loff_t size, unsigned long flags);
 void shmem_lock(struct file * file, int lock);
+
+#ifdef CONFIG_TMPFS
 int shmem_zero_setup(struct vm_area_struct *);
+#else
+static inline int shmem_zero_setup(struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+#endif
 
 /*
  * Parameter block passed down to zap_pte_range in exceptional cases.
diff -u linux-2.6.7-boot64/include/linux/swap.h-SHMEM linux-2.6.7-boot64/include/linux/swap.h
--- linux-2.6.7-boot64/include/linux/swap.h-SHMEM	2004-06-16 12:23:41.000000000 +0200
+++ linux-2.6.7-boot64/include/linux/swap.h	2004-08-08 00:07:23.000000000 +0200
@@ -177,8 +177,15 @@
 extern int vm_swappiness;
 
 #ifdef CONFIG_MMU
+#ifdef CONFIG_TMPFS
 /* linux/mm/shmem.c */
 extern int shmem_unuse(swp_entry_t entry, struct page *page);
+#else
+static inline int shmem_unuse(swp_entry_t entry, struct page *page)
+{
+	return 0;
+}
+#endif
 #endif /* CONFIG_MMU */
 
 extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
diff -u linux-2.6.7-boot64/fs/Kconfig-SHMEM linux-2.6.7-boot64/fs/Kconfig
--- linux-2.6.7-boot64/fs/Kconfig-SHMEM	2004-06-16 12:23:18.000000000 +0200
+++ linux-2.6.7-boot64/fs/Kconfig	2004-08-08 00:07:46.000000000 +0200
@@ -914,7 +914,8 @@
 	  extended attributes for file security labels, say N.
 
 config TMPFS
-	bool "Virtual memory file system support (former shm fs)"
+	bool "Virtual memory file system support (former shm fs)" if EMBEDDED
+	default y
 	help
 	  Tmpfs is a file system which keeps all files in virtual memory.
 
diff -u linux-2.6.7-boot64/mm/Makefile-SHMEM linux-2.6.7-boot64/mm/Makefile
--- linux-2.6.7-boot64/mm/Makefile-SHMEM	2004-06-16 12:23:42.000000000 +0200
+++ linux-2.6.7-boot64/mm/Makefile	2004-08-07 23:57:49.000000000 +0200
@@ -5,13 +5,14 @@
 mmu-y			:= nommu.o
 mmu-$(CONFIG_MMU)	:= fremap.o highmem.o madvise.o memory.o mincore.o \
 			   mlock.o mmap.o mprotect.o mremap.o msync.o rmap.o \
-			   shmem.o vmalloc.o
+			   vmalloc.o
 
 obj-y			:= bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \
 			   page_alloc.o page-writeback.o pdflush.o prio_tree.o \
 			   readahead.o slab.o swap.o truncate.o vmscan.o \
 			   $(mmu-y)
 
+obj-$(CONFIG_TMPFS)	+= shmem.o
 obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o
 obj-$(CONFIG_HUGETLBFS)	+= hugetlb.o
 obj-$(CONFIG_NUMA) 	+= mempolicy.o

Re: [PATCH] Allow to disable shmem.o

[Hugh Dickins]

Please no, not this half-baked patch anyway.

On Sun, 8 Aug 2004, Andi Kleen wrote:
> 
> This patch allows to not compile mm/shmem.o in. It is now dependent on
> CONFIG_TMPFS, which is the main user.

Well, one of its users, yes.

> This allows shrinking the kernel a bit more when needed.

Good.

> Disabling TMPFS disables the mmap
> MAP_ANONYMOUS|MAP_SHARED functionality now. That's ok, because Linux
> 2.2 also didn't have it iirc.

That's not an argument for throwing out support for POSIX/SuS
functionality which 2.4 has supported all along.  But never mind,
embedded may not need it, and your defaults are the right way round.

> To make sure this doesn't happen by
> accident I made the TMPFS configuration dependent on CONFIG_EMBEDDED.
> This means a normal configuration enables TMPFS, which is ok 
> since it is only minor additional overhead to shmem.o and also quite
> useful for shared memory.

At the least you need to add a dependency on CONFIG_SYSVIPC.
And you shouldn't disable standard functionality without
saying so in the Kconfig help.

You're completely changing the meaning and configurability of
CONFIG_TMPFS.  Perhaps it's not a useful config option any more
(too often needed "y", too little saved by "n"), and yours is
more useful.  But if you want to go this way, you must change
its name and Documentation and #ifdefs too.

I do agree that mm/shmem.o is larger (did someone at the back say
"bloated"?) than we'd wish, that embedded in particular would like
it smaller, and embedded might have no need to support SysV IPC
and Posix SHM and shared anonymous and tmpfs.

But I prefer Matt's tiny tiny-shmem.c which does support all those,
using ramfs instead, and says in Kconfig what it's doing.
But perhaps you're wanting to avoid ramfs too?

A lot (but less than I once imagined) of the overhead in mm/shmem.o
does come from its support for swap.  I've not been eager to pepper
it with #ifdef CONFIG_SWAPs; but if there's a real demand for that
I could give it a try (while abstracting away the #ifdefs as much
as possible).

Hugh

Re: [PATCH] Allow to disable shmem.o

[Matt Mackall]

On Sun, Aug 08, 2004 at 02:03:09PM +0100, Hugh Dickins wrote:
> 
> But I prefer Matt's tiny tiny-shmem.c which does support all those,
> using ramfs instead, and says in Kconfig what it's doing.
> But perhaps you're wanting to avoid ramfs too?

Ramfs is hard to avoid as it's used internally for / at boot and so
on. My patch for comparison. Comments appreciated.

configurable tiny shmem/tmpfs support


 tiny-mpm/fs/ramfs/inode.c   |    8 +-
 tiny-mpm/include/linux/mm.h |   10 +++
 tiny-mpm/init/Kconfig       |   14 ++++
 tiny-mpm/mm/Makefile        |    4 +
 tiny-mpm/mm/filemap.c       |    2 
 tiny-mpm/mm/tiny-shmem.c    |  128 ++++++++++++++++++++++++++++++++++++++++++++
 6 files changed, 159 insertions(+), 7 deletions(-)

Index: tiny/mm/Makefile
===================================================================
--- tiny.orig/mm/Makefile	2004-07-19 13:14:45.000000000 -0500
+++ tiny/mm/Makefile	2004-07-19 13:43:35.000000000 -0500
@@ -5,7 +5,7 @@
 mmu-y			:= nommu.o
 mmu-$(CONFIG_MMU)	:= fremap.o highmem.o madvise.o memory.o mincore.o \
 			   mlock.o mmap.o mprotect.o mremap.o msync.o rmap.o \
-			   shmem.o vmalloc.o
+			   vmalloc.o
 
 obj-y			:= bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \
 			   page_alloc.o page-writeback.o pdflush.o prio_tree.o \
@@ -16,3 +16,6 @@
 obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o
 obj-$(CONFIG_HUGETLBFS)	+= hugetlb.o
 obj-$(CONFIG_NUMA) 	+= mempolicy.o
+obj-$(CONFIG_SHMEM) += shmem.o
+obj-$(CONFIG_TINY_SHMEM) += tiny-shmem.o
+
Index: tiny/mm/tiny-shmem.c
===================================================================
--- tiny.orig/mm/tiny-shmem.c	2003-09-12 12:14:37.000000000 -0500
+++ tiny/mm/tiny-shmem.c	2004-07-19 13:43:35.000000000 -0500
@@ -0,0 +1,128 @@
+/*
+ * tiny-shmem.c: simple shmemfs and tmpfs using ramfs code
+ *
+ * Matt Mackall <mpm@selenic.com> January, 2004
+ * derived from mm/shmem.c and fs/ramfs/inode.c
+ *
+ * This is intended for small system where the benefits of the full
+ * shmem code (swap-backed and resource-limited) are outweighed by their
+ * complexity. On systems without swap and not using userspace /dev/shm,
+ * this code should be effectively equivalent, but much lighter weight.
+ */
+
+#include <linux/fs.h>
+#include <linux/init.h>
+#include <linux/devfs_fs_kernel.h>
+#include <linux/vfs.h>
+#include <linux/mount.h>
+#include <linux/file.h>
+#include <linux/mm.h>
+#include <linux/module.h>
+#include <linux/swap.h>
+
+extern struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev);
+extern struct super_block *ramfs_get_sb(struct file_system_type *fs_type,
+	 int flags, const char *dev_name, void *data);
+extern struct file_operations ramfs_file_operations;
+extern struct vm_operations_struct generic_file_vm_ops;
+
+static struct file_system_type tmpfs_fs_type = {
+	.name		= "tmpfs",
+	.get_sb		= ramfs_get_sb,
+	.kill_sb	= kill_litter_super,
+};
+
+static struct vfsmount *shm_mnt;
+
+static int __init init_tmpfs(void)
+{
+	register_filesystem(&tmpfs_fs_type);
+#ifdef CONFIG_TMPFS
+	devfs_mk_dir("shm");
+#endif
+	shm_mnt = kern_mount(&tmpfs_fs_type);
+	return 0;
+}
+module_init(init_tmpfs)
+
+/*
+ * shmem_file_setup - get an unlinked file living in tmpfs
+ *
+ * @name: name for dentry (to be seen in /proc/<pid>/maps
+ * @size: size to be set for the file
+ *
+ */
+struct file *shmem_file_setup(char *name, loff_t size, unsigned long flags)
+{
+	int error;
+	struct file *file;
+	struct inode *inode;
+	struct dentry *dentry, *root;
+	struct qstr this;
+
+	if (IS_ERR(shm_mnt))
+		return (void *)shm_mnt;
+
+	error = -ENOMEM;
+	this.name = name;
+	this.len = strlen(name);
+	this.hash = 0; /* will go */
+	root = shm_mnt->mnt_root;
+	dentry = d_alloc(root, &this);
+	if (!dentry)
+		goto put_memory;
+
+	error = -ENFILE;
+	file = get_empty_filp();
+	if (!file)
+		goto put_dentry;
+
+	error = -ENOSPC;
+	inode = ramfs_get_inode(root->d_sb, S_IFREG | S_IRWXUGO, 0);
+	if (!inode)
+		goto close_file;
+
+	d_instantiate(dentry, inode);
+	inode->i_size = size;
+	inode->i_nlink = 0;	/* It is unlinked */
+	file->f_vfsmnt = mntget(shm_mnt);
+	file->f_dentry = dentry;
+	file->f_op = &ramfs_file_operations;
+	file->f_mode = FMODE_WRITE | FMODE_READ;
+	return file;
+
+close_file:
+	put_filp(file);
+put_dentry:
+	dput(dentry);
+put_memory:
+	return ERR_PTR(error);
+}
+
+/*
+ * shmem_zero_setup - setup a shared anonymous mapping
+ *
+ * @vma: the vma to be mmapped is prepared by do_mmap_pgoff
+ */
+int shmem_zero_setup(struct vm_area_struct *vma)
+{
+	struct file *file;
+	loff_t size = vma->vm_end - vma->vm_start;
+
+	file = shmem_file_setup("dev/zero", size, vma->vm_flags);
+	if (IS_ERR(file))
+		return PTR_ERR(file);
+
+	if (vma->vm_file)
+		fput(vma->vm_file);
+	vma->vm_file = file;
+	vma->vm_ops = &generic_file_vm_ops;
+	return 0;
+}
+
+int shmem_unuse(swp_entry_t entry, struct page *page)
+{
+	return 0;
+}
+
+EXPORT_SYMBOL(shmem_file_setup);
Index: tiny/mm/filemap.c
===================================================================
--- tiny.orig/mm/filemap.c	2004-07-15 13:50:20.000000000 -0500
+++ tiny/mm/filemap.c	2004-07-19 13:43:35.000000000 -0500
@@ -1410,7 +1410,7 @@
 	return 0;
 }
 
-static struct vm_operations_struct generic_file_vm_ops = {
+struct vm_operations_struct generic_file_vm_ops = {
 	.nopage		= filemap_nopage,
 	.populate	= filemap_populate,
 };
Index: tiny/include/linux/mm.h
===================================================================
--- tiny.orig/include/linux/mm.h	2004-07-15 13:50:20.000000000 -0500
+++ tiny/include/linux/mm.h	2004-07-19 13:55:07.000000000 -0500
@@ -489,14 +489,23 @@
 
 extern void show_free_areas(void);
 
+#ifdef CONFIG_SHMEM
 struct page *shmem_nopage(struct vm_area_struct * vma,
 			unsigned long address, int *type);
 int shmem_set_policy(struct vm_area_struct *vma, struct mempolicy *new);
 struct mempolicy *shmem_get_policy(struct vm_area_struct *vma,
 					unsigned long addr);
-struct file *shmem_file_setup(char * name, loff_t size, unsigned long flags);
 void shmem_lock(struct file * file, int lock);
+#else
+#define shmem_nopage filemap_nopage
+#define shmem_lock(a, b) /* always in memory, no need to lock */
+#define shmem_set_policy(a, b) (0)
+#define shmem_get_policy(a, b) (NULL)
+#endif
+
 int shmem_zero_setup(struct vm_area_struct *);
+struct file *shmem_file_setup(char * name, loff_t size, unsigned long flags);
+
 
 /*
  * Parameter block passed down to zap_pte_range in exceptional cases.
Index: tiny/fs/ramfs/inode.c
===================================================================
--- tiny.orig/fs/ramfs/inode.c	2004-07-15 13:50:19.000000000 -0500
+++ tiny/fs/ramfs/inode.c	2004-07-19 13:43:35.000000000 -0500
@@ -39,7 +39,7 @@
 
 static struct super_operations ramfs_ops;
 static struct address_space_operations ramfs_aops;
-static struct file_operations ramfs_file_operations;
+extern struct file_operations ramfs_file_operations;
 static struct inode_operations ramfs_file_inode_operations;
 static struct inode_operations ramfs_dir_inode_operations;
 
@@ -48,7 +48,7 @@
 	.memory_backed	= 1,	/* Does not contribute to dirty memory */
 };
 
-static struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev)
+struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev)
 {
 	struct inode * inode = new_inode(sb);
 
@@ -146,7 +146,7 @@
 	.commit_write	= simple_commit_write
 };
 
-static struct file_operations ramfs_file_operations = {
+struct file_operations ramfs_file_operations = {
 	.read		= generic_file_read,
 	.write		= generic_file_write,
 	.mmap		= generic_file_mmap,
@@ -199,7 +199,7 @@
 	return 0;
 }
 
-static struct super_block *ramfs_get_sb(struct file_system_type *fs_type,
+struct super_block *ramfs_get_sb(struct file_system_type *fs_type,
 	int flags, const char *dev_name, void *data)
 {
 	return get_sb_nodev(fs_type, flags, data, ramfs_fill_super);
Index: tiny/init/Kconfig
===================================================================
--- tiny.orig/init/Kconfig	2004-07-19 13:43:35.000000000 -0500
+++ tiny/init/Kconfig	2004-07-19 13:43:35.000000000 -0500
@@ -461,8 +461,22 @@
 	help
 	  Disabling this moves operations on semaphores out of line.
 
+config SHMEM
+	default y
+	bool "Use full shmem filesystem" if EMBEDDED && MMU
+	help
+	  The shmem is an internal filesystem used to manage shared memory.
+	  It is backed by swap and manages resource limits. It is also exported
+	  to userspace as tmpfs if TMPFS is enabled. Disabling this
+	  option replaces shmem and tmpfs with the much simpler ramfs code,
+	  which may be appropriate on small systems without swap.
+
 endmenu		# General setup
 
+config TINY_SHMEM
+	default !SHMEM
+	bool
+
 config CRC32_CALC
 	default !CRC32_TABLES
 	bool

-- 
Mathematics is the supreme nostalgia of our time.

Re: [PATCH] Allow to disable shmem.o

[Andi Kleen]

On Sun, Aug 08, 2004 at 02:03:09PM +0100, Hugh Dickins wrote:
> > To make sure this doesn't happen by
> > accident I made the TMPFS configuration dependent on CONFIG_EMBEDDED.
> > This means a normal configuration enables TMPFS, which is ok 
> > since it is only minor additional overhead to shmem.o and also quite
> > useful for shared memory.
> 
> At the least you need to add a dependency on CONFIG_SYSVIPC.
> And you shouldn't disable standard functionality without
> saying so in the Kconfig help.

Hmm, right. Appended is a new patch that adds a select TMPFS to SYSVIPC.
i suspect the standard functionality is already covered by CONFIG_EMBEDDED.

> You're completely changing the meaning and configurability of
> CONFIG_TMPFS.  Perhaps it's not a useful config option any more
> (too often needed "y", too little saved by "n"), and yours is
> more useful.  But if you want to go this way, you must change
> its name and Documentation and #ifdefs too.

Perhaps, but calling it CONFIG_TMPFS is not too bad.

> I do agree that mm/shmem.o is larger (did someone at the back say
> "bloated"?) than we'd wish, that embedded in particular would like
> it smaller, and embedded might have no need to support SysV IPC
> and Posix SHM and shared anonymous and tmpfs.
> 
> But I prefer Matt's tiny tiny-shmem.c which does support all those,
> using ramfs instead, and says in Kconfig what it's doing.
> But perhaps you're wanting to avoid ramfs too?


Makes sense, adding this may be an additional useful step.
For the beginning I think my patch is fine though.

RAMFS cannot be turned off right now. I tried. 

> 
> A lot (but less than I once imagined) of the overhead in mm/shmem.o
> does come from its support for swap.  I've not been eager to pepper
> it with #ifdef CONFIG_SWAPs; but if there's a real demand for that
> I could give it a try (while abstracting away the #ifdefs as much
> as possible).

I personally don't want to turn off CONFIG_SWAP. I doubt my architecture
would even compile without that.

Andrew, please consider merging this version.

-Andi

-------------------------------------------------------

Allow to disable shmem.o by disabling CONFIG_TMPFS. This will 
disable MAP_ANONYMOUS|MAP_SHARED. To make sure this doesn't
happen by mistake make tmpfs dependent on CONFIG_EMBEDDED now.


diff -u linux-2.6.7-boot64/drivers/char/mem.c-SHMEM linux-2.6.7-boot64/drivers/char/mem.c
diff -u linux-2.6.7-boot64/include/linux/mm.h-SHMEM linux-2.6.7-boot64/include/linux/mm.h
--- linux-2.6.7-boot64/include/linux/mm.h-SHMEM	2004-06-16 12:23:40.000000000 +0200
+++ linux-2.6.7-boot64/include/linux/mm.h	2004-08-08 00:06:04.000000000 +0200
@@ -496,7 +496,15 @@
 					unsigned long addr);
 struct file *shmem_file_setup(char * name, loff_t size, unsigned long flags);
 void shmem_lock(struct file * file, int lock);
+
+#ifdef CONFIG_TMPFS
 int shmem_zero_setup(struct vm_area_struct *);
+#else
+static inline int shmem_zero_setup(struct vm_area_struct *vma)
+{
+	return -EINVAL;
+}
+#endif
 
 /*
  * Parameter block passed down to zap_pte_range in exceptional cases.
diff -u linux-2.6.7-boot64/include/linux/swap.h-SHMEM linux-2.6.7-boot64/include/linux/swap.h
--- linux-2.6.7-boot64/include/linux/swap.h-SHMEM	2004-06-16 12:23:41.000000000 +0200
+++ linux-2.6.7-boot64/include/linux/swap.h	2004-08-08 00:07:23.000000000 +0200
@@ -177,8 +177,15 @@
 extern int vm_swappiness;
 
 #ifdef CONFIG_MMU
+#ifdef CONFIG_TMPFS
 /* linux/mm/shmem.c */
 extern int shmem_unuse(swp_entry_t entry, struct page *page);
+#else
+static inline int shmem_unuse(swp_entry_t entry, struct page *page)
+{
+	return 0;
+}
+#endif
 #endif /* CONFIG_MMU */
 
 extern void swap_unplug_io_fn(struct backing_dev_info *, struct page *);
diff -u linux-2.6.7-boot64/fs/Kconfig-SHMEM linux-2.6.7-boot64/fs/Kconfig
--- linux-2.6.7-boot64/fs/Kconfig-SHMEM	2004-06-16 12:23:18.000000000 +0200
+++ linux-2.6.7-boot64/fs/Kconfig	2004-08-08 00:07:46.000000000 +0200
@@ -914,7 +914,8 @@
 	  extended attributes for file security labels, say N.
 
 config TMPFS
-	bool "Virtual memory file system support (former shm fs)"
+	bool "Virtual memory file system support (former shm fs)" if EMBEDDED
+	default y
 	help
 	  Tmpfs is a file system which keeps all files in virtual memory.
 
diff -u linux-2.6.7-boot64/mm/Makefile-SHMEM linux-2.6.7-boot64/mm/Makefile
--- linux-2.6.7-boot64/mm/Makefile-SHMEM	2004-06-16 12:23:42.000000000 +0200
+++ linux-2.6.7-boot64/mm/Makefile	2004-08-07 23:57:49.000000000 +0200
@@ -5,13 +5,14 @@
 mmu-y			:= nommu.o
 mmu-$(CONFIG_MMU)	:= fremap.o highmem.o madvise.o memory.o mincore.o \
 			   mlock.o mmap.o mprotect.o mremap.o msync.o rmap.o \
-			   shmem.o vmalloc.o
+			   vmalloc.o
 
 obj-y			:= bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \
 			   page_alloc.o page-writeback.o pdflush.o prio_tree.o \
 			   readahead.o slab.o swap.o truncate.o vmscan.o \
 			   $(mmu-y)
 
+obj-$(CONFIG_TMPFS)	+= shmem.o
 obj-$(CONFIG_SWAP)	+= page_io.o swap_state.o swapfile.o
 obj-$(CONFIG_HUGETLBFS)	+= hugetlb.o
 obj-$(CONFIG_NUMA) 	+= mempolicy.o
diff -u linux-2.6.7-boot64/mm/mmap.c-SHMEM linux-2.6.7-boot64/mm/mmap.c
diff -u linux-2.6.7-boot64/mm/swapfile.c-SHMEM linux-2.6.7-boot64/mm/swapfile.c
diff -u linux-2.6.7-boot64/init/Kconfig-SHMEM linux-2.6.7-boot64/init/Kconfig
--- linux-2.6.7-boot64/init/Kconfig-SHMEM	2004-06-16 12:23:42.000000000 +0200
+++ linux-2.6.7-boot64/init/Kconfig	2004-08-08 16:12:50.000000000 +0200
@@ -77,6 +77,7 @@
 
 config SYSVIPC
 	bool "System V IPC"
+	select TMPFS
 	---help---
 	  Inter Process Communication is a suite of library functions and
 	  system calls which let processes (running programs) synchronize and

Re: [PATCH] Allow to disable shmem.o

[Andi Kleen]

On Sun, Aug 08, 2004 at 09:07:05AM -0500, Matt Mackall wrote:
> +extern struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev);
> +extern struct super_block *ramfs_get_sb(struct file_system_type *fs_type,
> +	 int flags, const char *dev_name, void *data);
> +extern struct file_operations ramfs_file_operations;
> +extern struct vm_operations_struct generic_file_vm_ops;

This should be all in header files.

> + */
> +struct file *shmem_file_setup(char *name, loff_t size, unsigned long flags)
> +{
> +	int error;
> +	struct file *file;
> +	struct inode *inode;
> +	struct dentry *dentry, *root;
> +	struct qstr this;
> +
> +	if (IS_ERR(shm_mnt))
> +		return (void *)shm_mnt;

Why this strange cast? 

> +
> +	error = -ENOMEM;
> +	this.name = name;
> +	this.len = strlen(name);
> +	this.hash = 0; /* will go */
> +	root = shm_mnt->mnt_root;
> +	dentry = d_alloc(root, &this);
> +	if (!dentry)
> +		goto put_memory;
> +
> +	error = -ENFILE;
> +	file = get_empty_filp();
> +	if (!file)
> +		goto put_dentry;
> +
> +	error = -ENOSPC;
> +	inode = ramfs_get_inode(root->d_sb, S_IFREG | S_IRWXUGO, 0);

Hmm, won't this allow everybody else to open it in /proc/pid/fd/ ? 
(existing shmem.c seems to use it too, but it looks a bit bogus) 

-AndI

Re: [PATCH] Allow to disable shmem.o

[Hugh Dickins]

On Sun, 8 Aug 2004, Matt Mackall wrote:
> On Sun, Aug 08, 2004 at 02:03:09PM +0100, Hugh Dickins wrote:
> > 
> > But I prefer Matt's tiny tiny-shmem.c which does support all those,
> > using ramfs instead, and says in Kconfig what it's doing.
> > But perhaps you're wanting to avoid ramfs too?
> 
> Ramfs is hard to avoid as it's used internally for / at boot and so on.

Ah, right.

> My patch for comparison. Comments appreciated.

Looks pretty good, but would need rediffing against mainline or -mm
if heading that way: the init/Kconfig patch is peculiar to your -tiny.

The comment at the head of tiny-shmem.c:

> + * This is intended for small system where the benefits of the full
> + * shmem code (swap-backed and resource-limited) are outweighed by their
> + * complexity. On systems without swap and not using userspace /dev/shm,
> + * this code should be effectively equivalent, but much lighter weight.

Very fair statement.  But isn't it actually supporting /dev/shm fine?

And your shmem_file_setup hasn't quite kept up with the times:
shmat oopses in vma_link because shmem_file_setup is lacking a

	file->f_mapping = inode->i_mapping;

Plus Andi rightly advises externs in a header file.

Hugh

Re: [PATCH] Allow to disable shmem.o

[Hugh Dickins]

On 8 Aug 2004, Andi Kleen wrote:
> i suspect the standard functionality is already covered by CONFIG_EMBEDDED.

It would still be appropriate to say what functionality is lost
by configuring out, rather than leaving everyone to guess.

> > You're completely changing the meaning and configurability of
> > CONFIG_TMPFS.  Perhaps it's not a useful config option any more
> > (too often needed "y", too little saved by "n"), and yours is
> > more useful.  But if you want to go this way, you must change
> > its name and Documentation and #ifdefs too.
> 
> Perhaps, but calling it CONFIG_TMPFS is not too bad.

Okay, so I come along to clean up after you.  Yes, we could work it that
way.  But somehow I still prefer Matt's patch, which offers a lot more.

Hugh

Re: [PATCH] Allow to disable shmem.o

[Hugh Dickins]

On 8 Aug 2004, Andi Kleen wrote:
> On Sun, Aug 08, 2004 at 09:07:05AM -0500, Matt Mackall wrote:
> > +extern struct inode *ramfs_get_inode(struct super_block *sb, int mode, dev_t dev);
> > +extern struct super_block *ramfs_get_sb(struct file_system_type *fs_type,
> > +	 int flags, const char *dev_name, void *data);
> > +extern struct file_operations ramfs_file_operations;
> > +extern struct vm_operations_struct generic_file_vm_ops;
> 
> This should be all in header files.

Yes.

> > +	if (IS_ERR(shm_mnt))
> > +		return (void *)shm_mnt;
> 
> Why this strange cast? 

Matt copied from shmem.c, so blame me.  Strange cast because it's one
of those IS_ERR things, an -errno hiding inside a pointer, and it's
being propagated from an initialization error on shm_mnt to a runtime
error on any struct file for that mount.  I wrote (void *) because it
seemed to express the ambiguity better than pretending struct file *.

> > +	inode = ramfs_get_inode(root->d_sb, S_IFREG | S_IRWXUGO, 0);
> 
> Hmm, won't this allow everybody else to open it in /proc/pid/fd/ ? 
> (existing shmem.c seems to use it too, but it looks a bit bogus) 

But there's no fd associated with it?

Hugh

Re: [PATCH] Allow to disable shmem.o

[Andrew Morton]

Hugh Dickins <hugh@veritas.com> wrote:
>
> But somehow I still prefer Matt's patch, which offers a lot more.

Me too - it's sneaky.

I suspect it's more SMp scalable than shmem.c too?

Re: [PATCH] Allow to disable shmem.o

[Hugh Dickins]

On Sun, 8 Aug 2004, Andrew Morton wrote:
> Hugh Dickins <hugh@veritas.com> wrote:
> >
> > But somehow I still prefer Matt's patch, which offers a lot more.
> 
> Me too - it's sneaky.
> 
> I suspect it's more SMp scalable than shmem.c too?

I bet it is (unless it falls into generic filepage overheads
which shmem is skating past e.g. attempts to readahead).

Though I hope not notably more so by the time I've finished
reworking shmem_getpage (was on the right track yesterday,
but other issues have intervened today).

They should both come down to find_get_page (find_lock_page in
shmem.c's case, only an issue when contention on same page),
alloc_page, add_to_page_cache: contention on tree_lock.

Its main deficiency (aside from the lack of swap use, which in
some contexts is a plus) is its lack of resource limiting.

Hugh