linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] mm: unify shmem and tiny-shmem
@ 2008-09-30 23:49 Matt Mackall
  2008-10-01 15:00 ` David Howells
                   ` (2 more replies)
  0 siblings, 3 replies; 7+ messages in thread
From: Matt Mackall @ 2008-09-30 23:49 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Nick Piggin, David Howells, Linux Kernel Mailing List, Andrew Morton

(This applies on top of Nick's second tiny-shmem patch, which hasn't
made it to mainline yet(!). But as this deletes tiny-shmem.c, you can
probably ignore the rejects.)

tiny-shmem shares most of its 130 lines of code with shmem and tends
to break when particular bits of shmem get modified. Unifying saves
code and makes keeping these two in sync much easier.

before:
  14367	    392	     24	  14783	   39bf	mm/shmem.o
    396      72       8     476	    1dc	mm/tiny-shmem.o

after:
  14367	    392	     24	  14783	   39bf	mm/shmem.o
    412	     72       8     492	    1ec	mm/shmem.o tiny

Signed-off-by: Matt Mackall <mpm@selenic.com>

diff -r 04ceba0d1126 -r 4d75a71b1e9d init/Kconfig
--- a/init/Kconfig	Tue Sep 30 15:36:27 2008 -0500
+++ b/init/Kconfig	Tue Sep 30 18:44:12 2008 -0500
@@ -805,10 +805,6 @@
 	boolean
 	select PLIST
 
-config TINY_SHMEM
-	default !SHMEM
-	bool
-
 config BASE_SMALL
 	int
 	default 0 if BASE_FULL
diff -r 04ceba0d1126 -r 4d75a71b1e9d mm/Makefile
--- a/mm/Makefile	Tue Sep 30 15:36:27 2008 -0500
+++ b/mm/Makefile	Tue Sep 30 18:44:12 2008 -0500
@@ -9,7 +9,7 @@
 
 obj-y			:= bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \
 			   maccess.o page_alloc.o page-writeback.o pdflush.o \
-			   readahead.o swap.o truncate.o vmscan.o \
+			   readahead.o swap.o truncate.o vmscan.o shmem.o \
 			   prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \
 			   page_isolation.o mm_init.o $(mmu-y)
 
@@ -21,9 +21,7 @@
 obj-$(CONFIG_NUMA) 	+= mempolicy.o
 obj-$(CONFIG_SPARSEMEM)	+= sparse.o
 obj-$(CONFIG_SPARSEMEM_VMEMMAP) += sparse-vmemmap.o
-obj-$(CONFIG_SHMEM) += shmem.o
 obj-$(CONFIG_TMPFS_POSIX_ACL) += shmem_acl.o
-obj-$(CONFIG_TINY_SHMEM) += tiny-shmem.o
 obj-$(CONFIG_SLOB) += slob.o
 obj-$(CONFIG_MMU_NOTIFIER) += mmu_notifier.o
 obj-$(CONFIG_SLAB) += slab.o
diff -r 04ceba0d1126 -r 4d75a71b1e9d mm/shmem.c
--- a/mm/shmem.c	Tue Sep 30 15:36:27 2008 -0500
+++ b/mm/shmem.c	Tue Sep 30 18:44:12 2008 -0500
@@ -14,31 +14,39 @@
  * Copyright (c) 2004, Luke Kenneth Casson Leighton <lkcl@lkcl.net>
  * Copyright (c) 2004 Red Hat, Inc., James Morris <jmorris@redhat.com>
  *
+ * tiny-shmem:
+ * Copyright (c) 2004, 2008 Matt Mackall <mpm@selenic.com>
+ *
  * This file is released under the GPL.
  */
 
+#include <linux/fs.h>
+#include <linux/init.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>
+
+static struct vfsmount *shm_mnt;
+
+#ifdef CONFIG_SHMEM
 /*
  * This virtual memory filesystem is heavily based on the ramfs. It
  * extends ramfs by the ability to use swap and honor resource limits
  * which makes it a completely usable filesystem.
  */
 
-#include <linux/module.h>
-#include <linux/init.h>
-#include <linux/fs.h>
 #include <linux/xattr.h>
 #include <linux/exportfs.h>
 #include <linux/generic_acl.h>
-#include <linux/mm.h>
 #include <linux/mman.h>
-#include <linux/file.h>
-#include <linux/swap.h>
 #include <linux/pagemap.h>
 #include <linux/string.h>
 #include <linux/slab.h>
 #include <linux/backing-dev.h>
 #include <linux/shmem_fs.h>
-#include <linux/mount.h>
 #include <linux/writeback.h>
 #include <linux/vfs.h>
 #include <linux/blkdev.h>
@@ -2483,7 +2491,6 @@
 	.get_sb		= shmem_get_sb,
 	.kill_sb	= kill_litter_super,
 };
-static struct vfsmount *shm_mnt;
 
 static int __init init_tmpfs(void)
 {
@@ -2522,7 +2529,62 @@
 	shm_mnt = ERR_PTR(error);
 	return error;
 }
-module_init(init_tmpfs)
+
+#else /* !CONFIG_SHMEM */
+
+/*
+ * tiny-shmem: simple shmemfs and tmpfs using ramfs code
+ *
+ * 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 this code should be
+ * effectively equivalent, but much lighter weight.
+ */
+
+#include <linux/ramfs.h>
+
+static struct file_system_type tmpfs_fs_type = {
+	.name		= "tmpfs",
+	.get_sb		= ramfs_get_sb,
+	.kill_sb	= kill_litter_super,
+};
+
+static int __init init_tmpfs(void)
+{
+	BUG_ON(register_filesystem(&tmpfs_fs_type) != 0);
+
+	shm_mnt = kern_mount(&tmpfs_fs_type);
+	BUG_ON(IS_ERR(shm_mnt));
+
+	return 0;
+}
+
+int shmem_unuse(swp_entry_t entry, struct page *page)
+{
+	return 0;
+}
+
+#ifndef CONFIG_MMU
+unsigned long shmem_get_unmapped_area(struct file *file,
+				      unsigned long addr,
+				      unsigned long len,
+				      unsigned long pgoff,
+				      unsigned long flags)
+{
+	return ramfs_nommu_get_unmapped_area(file, addr, len, pgoff, flags);
+}
+#endif
+
+#define shmem_file_operations ramfs_file_operations
+#define shmem_unacct_size(a, b)
+#define shmem_vm_ops generic_file_vm_ops
+#define shmem_get_inode ramfs_get_inode
+#define SHMEM_MAX_BYTES LONG_MAX
+#define shmem_acct_size(a, b) 0
+
+#endif /* CONFIG_SHMEM */
+
+/* common code */
 
 /**
  * shmem_file_setup - get an unlinked file living in tmpfs
@@ -2566,12 +2628,20 @@
 	if (!inode)
 		goto close_file;
 
+#ifdef CONFIG_SHMEM
 	SHMEM_I(inode)->flags = flags & VM_ACCOUNT;
+#endif
 	d_instantiate(dentry, inode);
 	inode->i_size = size;
 	inode->i_nlink = 0;	/* It is unlinked */
 	init_file(file, shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
-			&shmem_file_operations);
+		  &shmem_file_operations);
+
+#ifndef CONFIG_MMU
+	error = ramfs_nommu_expand_for_mapping(inode, size);
+	if (error)
+		goto close_file;
+#endif
 	return file;
 
 close_file:
@@ -2602,3 +2672,5 @@
 	vma->vm_ops = &shmem_vm_ops;
 	return 0;
 }
+
+module_init(init_tmpfs)
diff -r 04ceba0d1126 -r 4d75a71b1e9d mm/tiny-shmem.c
--- a/mm/tiny-shmem.c	Tue Sep 30 15:36:27 2008 -0500
+++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
@@ -1,133 +0,0 @@
-/*
- * 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 this code should be
- * effectively equivalent, but much lighter weight.
- */
-
-#include <linux/fs.h>
-#include <linux/init.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>
-#include <linux/ramfs.h>
-
-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)
-{
-	BUG_ON(register_filesystem(&tmpfs_fs_type) != 0);
-
-	shm_mnt = kern_mount(&tmpfs_fs_type);
-	BUG_ON(IS_ERR(shm_mnt));
-
-	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
- * @flags: vm_flags
- */
-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 */
-        init_file(file, shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
-                        &ramfs_file_operations);
-
-#ifndef CONFIG_MMU
-	error = ramfs_nommu_expand_for_mapping(inode, size);
-	if (error)
-		goto close_file;
-#endif
-	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;
-}
-
-#ifndef CONFIG_MMU
-unsigned long shmem_get_unmapped_area(struct file *file,
-				      unsigned long addr,
-				      unsigned long len,
-				      unsigned long pgoff,
-				      unsigned long flags)
-{
-	return ramfs_nommu_get_unmapped_area(file, addr, len, pgoff, flags);
-}
-#endif

-- 
Mathematics is the supreme nostalgia of our time.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: unify shmem and tiny-shmem
  2008-09-30 23:49 [PATCH] mm: unify shmem and tiny-shmem Matt Mackall
@ 2008-10-01 15:00 ` David Howells
  2008-10-01 18:39 ` Hugh Dickins
  2008-10-02 22:23 ` David Howells
  2 siblings, 0 replies; 7+ messages in thread
From: David Howells @ 2008-10-01 15:00 UTC (permalink / raw)
  To: Matt Mackall
  Cc: dhowells, Hugh Dickins, Nick Piggin, Linux Kernel Mailing List,
	Andrew Morton

Matt Mackall <mpm@selenic.com> wrote:

> (This applies on top of Nick's second tiny-shmem patch, which hasn't
> made it to mainline yet(!). But as this deletes tiny-shmem.c, you can
> probably ignore the rejects.)
> 
> tiny-shmem shares most of its 130 lines of code with shmem and tends
> to break when particular bits of shmem get modified. Unifying saves
> code and makes keeping these two in sync much easier.
> 
> before:
>   14367	    392	     24	  14783	   39bf	mm/shmem.o
>     396      72       8     476	    1dc	mm/tiny-shmem.o
> 
> after:
>   14367	    392	     24	  14783	   39bf	mm/shmem.o
>     412	     72       8     492	    1ec	mm/shmem.o tiny
> 
> Signed-off-by: Matt Mackall <mpm@selenic.com>

Works with my test program:

	http://people.redhat.com/~dhowells/doshm.c

Compile and run:

	doshm sysv

warthog>size mm/tiny-shmem.o
   text    data     bss     dec     hex filename
    788      36       4     828     33c mm/tiny-shmem.o
warthog>size mm/shmem.o
   text    data     bss     dec     hex filename
    832      36       4     872     368 mm/shmem.o


Acked-by: David Howells <dhowells@redhat.com>

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: unify shmem and tiny-shmem
  2008-09-30 23:49 [PATCH] mm: unify shmem and tiny-shmem Matt Mackall
  2008-10-01 15:00 ` David Howells
@ 2008-10-01 18:39 ` Hugh Dickins
  2008-10-02  5:38   ` Nick Piggin
  2008-10-02 18:57   ` Matt Mackall
  2008-10-02 22:23 ` David Howells
  2 siblings, 2 replies; 7+ messages in thread
From: Hugh Dickins @ 2008-10-01 18:39 UTC (permalink / raw)
  To: Matt Mackall
  Cc: Nick Piggin, David Howells, Linux Kernel Mailing List, Andrew Morton

On Tue, 30 Sep 2008, Matt Mackall wrote:

> (This applies on top of Nick's second tiny-shmem patch, which hasn't
> made it to mainline yet(!).

Gosh, thanks for mentioning that, you're right, it hasn't.  I saw
git pull updating mm/tiny-shmem.c a few days ago, and took it for
granted that that was the #ifndef CONFIG_MMU fixed version; but it
was the earlier one without that fix, despite us all knowing that
fix was needed.  I'd have pushed it myself for -rc8 if I'd realized
it was still missing: Nick, please push it to Linus a.s.a.p. (with 
Acks from Matt and David and me) - or ask me to do so (I'm not sure
when Andrew is back up and running).

> But as this deletes tiny-shmem.c, you can
> probably ignore the rejects.)
> 
> tiny-shmem shares most of its 130 lines of code with shmem and tends
> to break when particular bits of shmem get modified. Unifying saves
> code and makes keeping these two in sync much easier.
> 
> before:
>   14367	    392	     24	  14783	   39bf	mm/shmem.o
>     396      72       8     476	    1dc	mm/tiny-shmem.o
> 
> after:
>   14367	    392	     24	  14783	   39bf	mm/shmem.o
>     412	     72       8     492	    1ec	mm/shmem.o tiny
> 
> Signed-off-by: Matt Mackall <mpm@selenic.com>

That's much nicer than I'd been imagining, thanks Matt: I've no qualms
about the few #ifdefs you're adding to mm/shmem.c, it does seem an
outright improvement that you're bringing them together.

A few minor points.

> 
> diff -r 04ceba0d1126 -r 4d75a71b1e9d init/Kconfig
> --- a/init/Kconfig	Tue Sep 30 15:36:27 2008 -0500
> +++ b/init/Kconfig	Tue Sep 30 18:44:12 2008 -0500
> @@ -805,10 +805,6 @@
>  	boolean
>  	select PLIST
>  
> -config TINY_SHMEM
> -	default !SHMEM
> -	bool
> -

(It's slightly confusing that !CONFIG_SHMEM now builds mm/shmem.c;
but never mind, it's much more important to propagate oldconfigs aright.)

>  config BASE_SMALL
>  	int
>  	default 0 if BASE_FULL
> diff -r 04ceba0d1126 -r 4d75a71b1e9d mm/Makefile
> --- a/mm/Makefile	Tue Sep 30 15:36:27 2008 -0500
> +++ b/mm/Makefile	Tue Sep 30 18:44:12 2008 -0500
> @@ -9,7 +9,7 @@
>  
>  obj-y			:= bootmem.o filemap.o mempool.o oom_kill.o fadvise.o \
>  			   maccess.o page_alloc.o page-writeback.o pdflush.o \
> -			   readahead.o swap.o truncate.o vmscan.o \
> +			   readahead.o swap.o truncate.o vmscan.o shmem.o \
>  			   prio_tree.o util.o mmzone.o vmstat.o backing-dev.o \
>  			   page_isolation.o mm_init.o $(mmu-y)
>  
> @@ -21,9 +21,7 @@
>  obj-$(CONFIG_NUMA) 	+= mempolicy.o
>  obj-$(CONFIG_SPARSEMEM)	+= sparse.o
>  obj-$(CONFIG_SPARSEMEM_VMEMMAP) += sparse-vmemmap.o
> -obj-$(CONFIG_SHMEM) += shmem.o
>  obj-$(CONFIG_TMPFS_POSIX_ACL) += shmem_acl.o
> -obj-$(CONFIG_TINY_SHMEM) += tiny-shmem.o
>  obj-$(CONFIG_SLOB) += slob.o
>  obj-$(CONFIG_MMU_NOTIFIER) += mmu_notifier.o
>  obj-$(CONFIG_SLAB) += slab.o
> diff -r 04ceba0d1126 -r 4d75a71b1e9d mm/shmem.c
> --- a/mm/shmem.c	Tue Sep 30 15:36:27 2008 -0500
> +++ b/mm/shmem.c	Tue Sep 30 18:44:12 2008 -0500
> @@ -14,31 +14,39 @@
>   * Copyright (c) 2004, Luke Kenneth Casson Leighton <lkcl@lkcl.net>
>   * Copyright (c) 2004 Red Hat, Inc., James Morris <jmorris@redhat.com>
>   *
> + * tiny-shmem:
> + * Copyright (c) 2004, 2008 Matt Mackall <mpm@selenic.com>
> + *
>   * This file is released under the GPL.
>   */
>  
> +#include <linux/fs.h>
> +#include <linux/init.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>
> +
> +static struct vfsmount *shm_mnt;
> +
> +#ifdef CONFIG_SHMEM
>  /*
>   * This virtual memory filesystem is heavily based on the ramfs. It
>   * extends ramfs by the ability to use swap and honor resource limits
>   * which makes it a completely usable filesystem.
>   */
>  
> -#include <linux/module.h>
> -#include <linux/init.h>
> -#include <linux/fs.h>
>  #include <linux/xattr.h>
>  #include <linux/exportfs.h>
>  #include <linux/generic_acl.h>
> -#include <linux/mm.h>
>  #include <linux/mman.h>
> -#include <linux/file.h>
> -#include <linux/swap.h>
>  #include <linux/pagemap.h>
>  #include <linux/string.h>
>  #include <linux/slab.h>
>  #include <linux/backing-dev.h>
>  #include <linux/shmem_fs.h>
> -#include <linux/mount.h>
>  #include <linux/writeback.h>
>  #include <linux/vfs.h>
>  #include <linux/blkdev.h>
> @@ -2483,7 +2491,6 @@
>  	.get_sb		= shmem_get_sb,
>  	.kill_sb	= kill_litter_super,
>  };
> -static struct vfsmount *shm_mnt;
>  
>  static int __init init_tmpfs(void)
>  {
> @@ -2522,7 +2529,62 @@
>  	shm_mnt = ERR_PTR(error);
>  	return error;
>  }
> -module_init(init_tmpfs)
> +
> +#else /* !CONFIG_SHMEM */
> +
> +/*
> + * tiny-shmem: simple shmemfs and tmpfs using ramfs code
> + *
> + * 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 this code should be
> + * effectively equivalent, but much lighter weight.
> + */
> +
> +#include <linux/ramfs.h>
> +
> +static struct file_system_type tmpfs_fs_type = {
> +	.name		= "tmpfs",
> +	.get_sb		= ramfs_get_sb,
> +	.kill_sb	= kill_litter_super,
> +};
> +
> +static int __init init_tmpfs(void)
> +{
> +	BUG_ON(register_filesystem(&tmpfs_fs_type) != 0);
> +
> +	shm_mnt = kern_mount(&tmpfs_fs_type);
> +	BUG_ON(IS_ERR(shm_mnt));
> +
> +	return 0;
> +}
> +
> +int shmem_unuse(swp_entry_t entry, struct page *page)
> +{
> +	return 0;
> +}
> +
> +#ifndef CONFIG_MMU
> +unsigned long shmem_get_unmapped_area(struct file *file,
> +				      unsigned long addr,
> +				      unsigned long len,
> +				      unsigned long pgoff,
> +				      unsigned long flags)
> +{
> +	return ramfs_nommu_get_unmapped_area(file, addr, len, pgoff, flags);
> +}
> +#endif

This ifndef is the ugliest part of it: and looking into it, guess
what, shmem_get_unmapped_area() hasn't been used since 2.6.20 -
that's right, isn't it, David?

So please make a separate preliminary patch to remove it from
mm/tiny-shmem.c and include/linux/mm.h, so that little ugliness
never gets here - or if you think that's the wrong way around,
leave it to me to remove from mm/shmem.c later on.

> +
> +#define shmem_file_operations ramfs_file_operations
> +#define shmem_unacct_size(a, b)
> +#define shmem_vm_ops generic_file_vm_ops
> +#define shmem_get_inode ramfs_get_inode
> +#define SHMEM_MAX_BYTES LONG_MAX
> +#define shmem_acct_size(a, b) 0

I'd prefer to rearrange that:

#define shmem_vm_ops		generic_file_vm_ops
#define shmem_file_operations	ramfs_file_operations
#define shmem_get_inode		ramfs_get_inode
#define shmem_acct_size(a, b)	0
#define shmem_unacct_size(a, b)	do { } while (0)
#define SHMEM_MAX_BYTES		LLONG_MAX

Your LONG_MAX is too limiting, since it's on a loff_t:
CONFIG_SHMEM limits files according to the range of its swap
tables, but ramfs doesn't require that extra limitation, and
LONG_MAX could be too small on 32-bit.  And hopefully gcc will
optimize away the > LLONG_MAX test entirely.

> +
> +#endif /* CONFIG_SHMEM */
> +
> +/* common code */
>  
>  /**
>   * shmem_file_setup - get an unlinked file living in tmpfs
> @@ -2566,12 +2628,20 @@
>  	if (!inode)
>  		goto close_file;
>  
> +#ifdef CONFIG_SHMEM
>  	SHMEM_I(inode)->flags = flags & VM_ACCOUNT;
> +#endif
>  	d_instantiate(dentry, inode);
>  	inode->i_size = size;
>  	inode->i_nlink = 0;	/* It is unlinked */
>  	init_file(file, shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
> -			&shmem_file_operations);
> +		  &shmem_file_operations);

Hey! you didn't need to change that ;)

> +
> +#ifndef CONFIG_MMU
> +	error = ramfs_nommu_expand_for_mapping(inode, size);
> +	if (error)
> +		goto close_file;
> +#endif
>  	return file;
>  
>  close_file:
> @@ -2602,3 +2672,5 @@
>  	vma->vm_ops = &shmem_vm_ops;
>  	return 0;
>  }
> +
> +module_init(init_tmpfs)
> diff -r 04ceba0d1126 -r 4d75a71b1e9d mm/tiny-shmem.c
> --- a/mm/tiny-shmem.c	Tue Sep 30 15:36:27 2008 -0500
> +++ /dev/null	Thu Jan 01 00:00:00 1970 +0000
> @@ -1,133 +0,0 @@
> -/*
> - * 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 this code should be
> - * effectively equivalent, but much lighter weight.
> - */
> -
> -#include <linux/fs.h>
> -#include <linux/init.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>
> -#include <linux/ramfs.h>
> -
> -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)
> -{
> -	BUG_ON(register_filesystem(&tmpfs_fs_type) != 0);
> -
> -	shm_mnt = kern_mount(&tmpfs_fs_type);
> -	BUG_ON(IS_ERR(shm_mnt));
> -
> -	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
> - * @flags: vm_flags
> - */
> -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 */
> -        init_file(file, shm_mnt, dentry, FMODE_WRITE | FMODE_READ,
> -                        &ramfs_file_operations);
> -
> -#ifndef CONFIG_MMU
> -	error = ramfs_nommu_expand_for_mapping(inode, size);
> -	if (error)
> -		goto close_file;
> -#endif
> -	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;
> -}
> -
> -#ifndef CONFIG_MMU
> -unsigned long shmem_get_unmapped_area(struct file *file,
> -				      unsigned long addr,
> -				      unsigned long len,
> -				      unsigned long pgoff,
> -				      unsigned long flags)
> -{
> -	return ramfs_nommu_get_unmapped_area(file, addr, len, pgoff, flags);
> -}
> -#endif

On a different but related subject:
do you think we need to retain the CONFIG_TMPFS option?  It's rather
odd these days, since everybody gets ramfs, and you give them tmpfs
via ramfs without CONFIG_SHMEM.  If anybody wants to cut out the
TMPFS code overhead these days, wouldn't they be using !CONFIG_SHMEM?

Hugh

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: unify shmem and tiny-shmem
  2008-10-01 18:39 ` Hugh Dickins
@ 2008-10-02  5:38   ` Nick Piggin
  2008-10-02 18:57   ` Matt Mackall
  1 sibling, 0 replies; 7+ messages in thread
From: Nick Piggin @ 2008-10-02  5:38 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Matt Mackall, Nick Piggin, David Howells,
	Linux Kernel Mailing List, Andrew Morton

On Thursday 02 October 2008 04:39, Hugh Dickins wrote:
> On Tue, 30 Sep 2008, Matt Mackall wrote:
> > (This applies on top of Nick's second tiny-shmem patch, which hasn't
> > made it to mainline yet(!).
>
> Gosh, thanks for mentioning that, you're right, it hasn't.  I saw
> git pull updating mm/tiny-shmem.c a few days ago, and took it for
> granted that that was the #ifndef CONFIG_MMU fixed version; but it
> was the earlier one without that fix, despite us all knowing that
> fix was needed.  I'd have pushed it myself for -rc8 if I'd realized
> it was still missing: Nick, please push it to Linus a.s.a.p. (with
> Acks from Matt and David and me) - or ask me to do so (I'm not sure
> when Andrew is back up and running).

Linus must have missed it I guess, because I think I did address it to
him.

If you or Matt or Andrew push it up, it might get taken seriously ;)

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: unify shmem and tiny-shmem
  2008-10-01 18:39 ` Hugh Dickins
  2008-10-02  5:38   ` Nick Piggin
@ 2008-10-02 18:57   ` Matt Mackall
  1 sibling, 0 replies; 7+ messages in thread
From: Matt Mackall @ 2008-10-02 18:57 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: Nick Piggin, David Howells, Linux Kernel Mailing List, Andrew Morton


On Wed, 2008-10-01 at 19:39 +0100, Hugh Dickins wrote:
> On a different but related subject:
> do you think we need to retain the CONFIG_TMPFS option?  It's rather
> odd these days, since everybody gets ramfs, and you give them tmpfs
> via ramfs without CONFIG_SHMEM.  If anybody wants to cut out the
> TMPFS code overhead these days, wouldn't they be using !CONFIG_SHMEM?

I agree, it's pretty hard to see a situation where you'd want full
swap-backed shm and not full swap-backed tmpfs. I'll spin up a patch to
follow on my unification.

-- 
Mathematics is the supreme nostalgia of our time.


^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: unify shmem and tiny-shmem
  2008-09-30 23:49 [PATCH] mm: unify shmem and tiny-shmem Matt Mackall
  2008-10-01 15:00 ` David Howells
  2008-10-01 18:39 ` Hugh Dickins
@ 2008-10-02 22:23 ` David Howells
  2008-10-02 22:44   ` Matt Mackall
  2 siblings, 1 reply; 7+ messages in thread
From: David Howells @ 2008-10-02 22:23 UTC (permalink / raw)
  To: Hugh Dickins
  Cc: dhowells, Matt Mackall, Nick Piggin, Linux Kernel Mailing List,
	Andrew Morton

Hugh Dickins <hugh@veritas.com> wrote:

> That's much nicer than I'd been imagining, thanks Matt: I've no qualms
> about the few #ifdefs you're adding to mm/shmem.c, it does seem an
> outright improvement that you're bringing them together.

It's not an outright improvement.  shmem.o is still larger than tiny-shmem.o,
albeit not a lot; however, some embedded people really do count the bytes.

> This ifndef is the ugliest part of it: and looking into it, guess
> what, shmem_get_unmapped_area() hasn't been used since 2.6.20 -
> that's right, isn't it, David?

Ummm...  It seems to be.  I think it used to be used by SYSV SHM.  Certainly
deleting that function causes no link problems.

David

^ permalink raw reply	[flat|nested] 7+ messages in thread

* Re: [PATCH] mm: unify shmem and tiny-shmem
  2008-10-02 22:23 ` David Howells
@ 2008-10-02 22:44   ` Matt Mackall
  0 siblings, 0 replies; 7+ messages in thread
From: Matt Mackall @ 2008-10-02 22:44 UTC (permalink / raw)
  To: David Howells
  Cc: Hugh Dickins, Nick Piggin, Linux Kernel Mailing List, Andrew Morton


On Thu, 2008-10-02 at 23:23 +0100, David Howells wrote:
> Hugh Dickins <hugh@veritas.com> wrote:
> 
> > That's much nicer than I'd been imagining, thanks Matt: I've no qualms
> > about the few #ifdefs you're adding to mm/shmem.c, it does seem an
> > outright improvement that you're bringing them together.
> 
> It's not an outright improvement.  shmem.o is still larger than tiny-shmem.o,
> albeit not a lot; however, some embedded people really do count the bytes.

Here the difference comes down to 16 bytes for the "if (size < 0)"
check/branch, which is a worthwhile check.

> > This ifndef is the ugliest part of it: and looking into it, guess
> > what, shmem_get_unmapped_area() hasn't been used since 2.6.20 -
> > that's right, isn't it, David?
> 
> Ummm...  It seems to be.  I think it used to be used by SYSV SHM.  Certainly
> deleting that function causes no link problems.

I've got a patch queued to drop it, will post shortly.

-- 
Mathematics is the supreme nostalgia of our time.


^ permalink raw reply	[flat|nested] 7+ messages in thread

end of thread, other threads:[~2008-10-02 22:47 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2008-09-30 23:49 [PATCH] mm: unify shmem and tiny-shmem Matt Mackall
2008-10-01 15:00 ` David Howells
2008-10-01 18:39 ` Hugh Dickins
2008-10-02  5:38   ` Nick Piggin
2008-10-02 18:57   ` Matt Mackall
2008-10-02 22:23 ` David Howells
2008-10-02 22:44   ` Matt Mackall

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).