Hi, I am using linux kernel 2.4.2 and I have 1 GB ram.
I tried to boot the system with a ramdisk size of
600MB. It was ok when I did "mke2fs" on it, but when I
mounted it, it failed (Magic number mismatch). I tried
this several times and found that all ramdisk sizes
larger than 513MB could cause trouble. Could anyone
please kindly give me some hints? I would like to have
a larger ramdisk (around 800MB).
(note: I tried ramfs but it seems to have memory
leakage when files are deleted and created frequently;
tmpfs is ok, but the pages may be swapped, which is
not desirable in my case...)
Thank you very much!
Regards,
Roy
__________________________________________________
Do You Yahoo!?
Buy the perfect holiday gifts at Yahoo! Shopping.
http://shopping.yahoo.com
wrt the ramfs leak (the referenced patch below worked for me),
is the ramfs usage limits patch + this fix going into
the official 2.4 soon as it was in the ac series for ages?
thanks,
Padraig.
-------- Original Message --------
Subject: Re: ramfs leak
Date: Mon, 12 Nov 2001 11:47:41 +0900
From: Tachino Nobuhiro <[email protected]>
To: [email protected] (W Christopher Martin)
CC: [email protected], [email protected] (Padraig Brady)
Hello,
At Fri, 9 Nov 2001 15:40:43 -0500 (EST),
W Christopher Martin wrote:
>
> Padraig Brady writes:
> > When I remove files from a ramfs the space is not reclaimed?
> > What am I doing wrong? Details below.
>
> Nothing. We've noticed the same thing. It's a bug and was
> first reported back in July, but no one has provided a fix yet.
> I've had a brief look at the source code, but nothing obvious
> pops out at me.
I think you should use tmpfs instead of ramfs, but if you really want to
use ramfs,
the patch below may fix the problem.
diff -Nur linux-2.4.13-ac7.org/fs/ramfs/inode.c
linux-2.4.13-ac7/fs/ramfs/inode.c
--- linux-2.4.13-ac7.org/fs/ramfs/inode.c Mon Nov 12 11:00:47 2001
+++ linux-2.4.13-ac7/fs/ramfs/inode.c Mon Nov 12 11:26:40 2001
@@ -182,12 +182,9 @@
{
struct ramfs_sb_info *rsb = RAMFS_SB(inode->i_sb);
-
if (! Page_Uptodate(page))
-
return;
-
lock_rsb(rsb);
-
-
ClearPageDirty(page);
+
if (Page_Uptodate(page))
+
ClearPageDirty(page);
rsb->free_pages++;
inode->i_blocks -= IBLOCKS_PER_PAGE;
Roy S.C. Ho wrote:
> Hi, I am using linux kernel 2.4.2 and I have 1 GB ram.
> I tried to boot the system with a ramdisk size of
> 600MB. It was ok when I did "mke2fs" on it, but when I
> mounted it, it failed (Magic number mismatch). I tried
> this several times and found that all ramdisk sizes
> larger than 513MB could cause trouble. Could anyone
> please kindly give me some hints? I would like to have
> a larger ramdisk (around 800MB).
>
> (note: I tried ramfs but it seems to have memory
> leakage when files are deleted and created frequently;
> tmpfs is ok, but the pages may be swapped, which is
> not desirable in my case...)
does the patch attached fix your problem with ramfs?
> Thank you very much!
>
> Regards,
> Roy
> wrt the ramfs leak (the referenced patch below worked for me),
> is the ramfs usage limits patch + this fix going into
> the official 2.4 soon as it was in the ac series for ages?
The -ac ramfs changes need the mm operations changes. Someone has to go
merge that with Andrea-vm then you can get ramfs fixed and accounting sorted
out in shmfs
At Tue, 4 Dec 2001 20:14:16 +0000 (GMT),
Alan Cox wrote:
>
> > wrt the ramfs leak (the referenced patch below worked for me),
> > is the ramfs usage limits patch + this fix going into
> > the official 2.4 soon as it was in the ac series for ages?
>
> The -ac ramfs changes need the mm operations changes. Someone has to go
> merge that with Andrea-vm then you can get ramfs fixed and accounting sorted
> out in shmfs
How about the patch below? This is almost the same as the ramfs in 2.4.13-ac7
but shmfs style accounting merged. It works for me, but I definitely believe
expert's code review is required.
diff -Nur linux-2.4.17-pre2.org/fs/ramfs/inode.c linux-2.4.17-pre2/fs/ramfs/inode.c
--- linux-2.4.17-pre2.org/fs/ramfs/inode.c Mon Dec 3 10:46:08 2001
+++ linux-2.4.17-pre2/fs/ramfs/inode.c Wed Dec 5 15:34:03 2001
@@ -29,22 +29,187 @@
#include <linux/init.h>
#include <linux/string.h>
#include <linux/locks.h>
+#include <linux/highmem.h>
+#include <linux/slab.h>
#include <asm/uaccess.h>
+#include <linux/spinlock.h>
+
+#if PAGE_CACHE_SIZE % 1024
+#error Oh no, PAGE_CACHE_SIZE is not divisible by 1k! I cannot cope.
+#endif
+
+#define IBLOCKS_PER_PAGE (PAGE_CACHE_SIZE / 512)
+#define K_PER_PAGE (PAGE_CACHE_SIZE / 1024)
/* some random number */
#define RAMFS_MAGIC 0x858458f6
+static struct inode_operations ramfs_inode_operations;
+static struct inode_operations ramfs_symlink_inode_operations;
static struct super_operations ramfs_ops;
static struct address_space_operations ramfs_aops;
static struct file_operations ramfs_dir_operations;
static struct file_operations ramfs_file_operations;
static struct inode_operations ramfs_dir_inode_operations;
+/*
+ * ramfs super-block data in memory
+ */
+struct ramfs_sb_info {
+ /* Prevent races accessing the used block
+ * counts. Conceptually, this could probably be a semaphore,
+ * but the only thing we do while holding the lock is
+ * arithmetic, so there's no point */
+ spinlock_t ramfs_lock;
+
+ /* It is important that at least the free counts below be
+ signed. free_XXX may become negative if a limit is changed
+ downwards (by a remount) below the current usage. */
+
+ /* maximum number of pages in a file */
+ long max_file_pages;
+
+ /* max total number of data pages */
+ long max_pages;
+ /* free_pages = max_pages - total number of pages currently in use */
+ long free_pages;
+
+ /* max number of inodes */
+ long max_inodes;
+ /* free_inodes = max_inodes - total number of inodes currently in use */
+ long free_inodes;
+
+ /* max number of dentries */
+ long max_dentries;
+ /* free_dentries = max_dentries - total number of dentries in use */
+ long free_dentries;
+};
+
+#define RAMFS_SB(sb) ((struct ramfs_sb_info *)((sb)->u.generic_sbp))
+
+/*
+ * Resource limit helper functions
+ */
+
+static inline void lock_rsb(struct ramfs_sb_info *rsb)
+{
+ spin_lock(&(rsb->ramfs_lock));
+}
+
+static inline void unlock_rsb(struct ramfs_sb_info *rsb)
+{
+ spin_unlock(&(rsb->ramfs_lock));
+}
+
+/* Decrements the free inode count and returns true, or returns false
+ * if there are no free inodes */
+static int ramfs_alloc_inode(struct super_block *sb)
+{
+ struct ramfs_sb_info *rsb = RAMFS_SB(sb);
+ int ret = 1;
+
+ lock_rsb(rsb);
+ if (!rsb->max_inodes || rsb->free_inodes > 0)
+ rsb->free_inodes--;
+ else
+ ret = 0;
+ unlock_rsb(rsb);
+
+ return ret;
+}
+
+/* Increments the free inode count */
+static void ramfs_dealloc_inode(struct super_block *sb)
+{
+ struct ramfs_sb_info *rsb = RAMFS_SB(sb);
+
+ lock_rsb(rsb);
+ rsb->free_inodes++;
+ unlock_rsb(rsb);
+}
+
+/* Decrements the free dentry count and returns true, or returns false
+ * if there are no free dentries */
+static int ramfs_alloc_dentry(struct super_block *sb)
+{
+ struct ramfs_sb_info *rsb = RAMFS_SB(sb);
+ int ret = 1;
+
+ lock_rsb(rsb);
+ if (!rsb->max_dentries || rsb->free_dentries > 0)
+ rsb->free_dentries--;
+ else
+ ret = 0;
+ unlock_rsb(rsb);
+
+ return ret;
+}
+
+/* Increments the free dentry count */
+static void ramfs_dealloc_dentry(struct super_block *sb)
+{
+ struct ramfs_sb_info *rsb = RAMFS_SB(sb);
+
+ lock_rsb(rsb);
+ rsb->free_dentries++;
+ unlock_rsb(rsb);
+}
+
+/* If the given page can be added to the give inode for ramfs, return
+ * true and update the filesystem's free page count and the inode's
+ * i_blocks field. Always returns true if the file is already used by
+ * ramfs (ie. PageDirty(page) is true) */
+int ramfs_alloc_page(struct inode *inode, struct page *page)
+{
+ struct ramfs_sb_info *rsb = RAMFS_SB(inode->i_sb);
+ int ret = 1;
+
+ lock_rsb(rsb);
+
+ if ( (rsb->free_pages > 0) &&
+ ( !rsb->max_file_pages ||
+ (inode->i_data.nrpages <= rsb->max_file_pages) ) ) {
+ inode->i_blocks += IBLOCKS_PER_PAGE;
+ rsb->free_pages--;
+ } else
+ ret = 0;
+
+ unlock_rsb(rsb);
+
+ return ret;
+}
+
+static void ramfs_recalc_inode(struct inode *inode)
+{
+ struct ramfs_sb_info *rsb = RAMFS_SB(inode->i_sb);
+ unsigned long freed;
+
+ freed = inode->i_blocks/IBLOCKS_PER_PAGE - inode->i_mapping->nrpages;
+ if (freed) {
+ inode->i_blocks -= freed * IBLOCKS_PER_PAGE;
+ lock_rsb(rsb);
+ rsb->free_pages += freed;
+ unlock_rsb(rsb);
+ }
+}
+
static int ramfs_statfs(struct super_block *sb, struct statfs *buf)
{
+ struct ramfs_sb_info *rsb = RAMFS_SB(sb);
+
+ lock_rsb(rsb);
+ buf->f_blocks = rsb->max_pages;
+ buf->f_files = rsb->max_inodes;
+
+ buf->f_bfree = rsb->free_pages;
+ buf->f_bavail = buf->f_bfree;
+ buf->f_ffree = rsb->free_inodes;
+ unlock_rsb(rsb);
+
buf->f_type = RAMFS_MAGIC;
buf->f_bsize = PAGE_CACHE_SIZE;
+// buf->f_spare[0] = rsb->free_dentries; /* debug */
buf->f_namelen = 255;
return 0;
}
@@ -77,8 +242,12 @@
static int ramfs_prepare_write(struct file *file, struct page *page, unsigned offset, unsigned to)
{
+ struct inode *inode = (struct inode *)page->mapping->host;
void *addr = kmap(page);
if (!Page_Uptodate(page)) {
+ if (!ramfs_alloc_page(inode, page))
+ return -ENOSPC;
+
memset(addr, 0, PAGE_CACHE_SIZE);
flush_dcache_page(page);
SetPageUptodate(page);
@@ -98,9 +267,20 @@
return 0;
}
+static void ramfs_truncate(struct inode *inode)
+{
+ inode->i_ctime = inode->i_mtime = CURRENT_TIME;
+ ramfs_recalc_inode(inode);
+}
+
struct inode *ramfs_get_inode(struct super_block *sb, int mode, int dev)
{
- struct inode * inode = new_inode(sb);
+ struct inode * inode;
+
+ if (! ramfs_alloc_inode(sb))
+ return NULL;
+
+ inode = new_inode(sb);
if (inode) {
inode->i_mode = mode;
@@ -116,6 +296,7 @@
init_special_inode(inode, mode, dev);
break;
case S_IFREG:
+ inode->i_op = &ramfs_inode_operations;
inode->i_fop = &ramfs_file_operations;
break;
case S_IFDIR:
@@ -123,26 +304,38 @@
inode->i_fop = &ramfs_dir_operations;
break;
case S_IFLNK:
- inode->i_op = &page_symlink_inode_operations;
+ inode->i_op = &ramfs_symlink_inode_operations;
break;
}
- }
+ } else
+ ramfs_dealloc_inode(sb);
+
return inode;
}
/*
- * File creation. Allocate an inode, and we're done..
+ * File creation. Allocate an inode, update free inode and dentry counts
+ * and we're done..
*/
static int ramfs_mknod(struct inode *dir, struct dentry *dentry, int mode, int dev)
{
- struct inode * inode = ramfs_get_inode(dir->i_sb, mode, dev);
+ struct super_block *sb = dir->i_sb;
+ struct inode * inode;
int error = -ENOSPC;
+ if (! ramfs_alloc_dentry(sb))
+ return error;
+
+ inode = ramfs_get_inode(dir->i_sb, mode, dev);
+
if (inode) {
d_instantiate(dentry, inode);
dget(dentry); /* Extra count - pin the dentry in core */
error = 0;
+ } else {
+ ramfs_dealloc_dentry(sb);
}
+
return error;
}
@@ -161,11 +354,15 @@
*/
static int ramfs_link(struct dentry *old_dentry, struct inode * dir, struct dentry * dentry)
{
+ struct super_block *sb = dir->i_sb;
struct inode *inode = old_dentry->d_inode;
if (S_ISDIR(inode->i_mode))
return -EPERM;
+ if (! ramfs_alloc_dentry(sb))
+ return -ENOSPC;
+
inode->i_nlink++;
atomic_inc(&inode->i_count); /* New dentry reference */
dget(dentry); /* Extra pinning count for the created dentry */
@@ -212,6 +409,7 @@
*/
static int ramfs_unlink(struct inode * dir, struct dentry *dentry)
{
+ struct super_block *sb = dir->i_sb;
int retval = -ENOTEMPTY;
if (ramfs_empty(dentry)) {
@@ -219,6 +417,9 @@
inode->i_nlink--;
dput(dentry); /* Undo the count from "create" - this does all the work */
+
+ ramfs_dealloc_dentry(sb);
+
retval = 0;
}
return retval;
@@ -234,6 +435,8 @@
*/
static int ramfs_rename(struct inode * old_dir, struct dentry *old_dentry, struct inode * new_dir,struct dentry *new_dentry)
{
+ struct super_block *sb = new_dir->i_sb;
+
int error = -ENOTEMPTY;
if (ramfs_empty(new_dentry)) {
@@ -241,6 +444,7 @@
if (inode) {
inode->i_nlink--;
dput(new_dentry);
+ ramfs_dealloc_dentry(sb);
}
error = 0;
}
@@ -260,11 +464,185 @@
return error;
}
+static void ramfs_delete_inode(struct inode *inode)
+{
+ ramfs_truncate(inode);
+ ramfs_dealloc_inode(inode->i_sb);
+ clear_inode(inode);
+}
+
+static void ramfs_put_super(struct super_block *sb)
+{
+ kfree(sb->u.generic_sbp);
+}
+
+struct ramfs_params {
+ long pages;
+ long filepages;
+ long inodes;
+ long dentries;
+};
+
+static int parse_options(char * options, struct ramfs_params *p)
+{
+ char save = 0, *savep = NULL, *optname, *value;
+
+ p->pages = -1;
+ p->filepages = -1;
+ p->inodes = -1;
+ p->dentries = -1;
+
+ for (optname = strtok(options,","); optname;
+ optname = strtok(NULL,",")) {
+ if ((value = strchr(optname,'=')) != NULL) {
+ save = *value;
+ savep = value;
+ *value++ = 0;
+ }
+
+ if (!strcmp(optname, "maxfilesize") && value) {
+ p->filepages = simple_strtoul(value, &value, 0)
+ / K_PER_PAGE;
+ if (*value)
+ return -EINVAL;
+ } else if (!strcmp(optname, "maxsize") && value) {
+ p->pages = simple_strtoul(value, &value, 0)
+ / K_PER_PAGE;
+ if (*value)
+ return -EINVAL;
+ } else if (!strcmp(optname, "maxinodes") && value) {
+ p->inodes = simple_strtoul(value, &value, 0);
+ if (*value)
+ return -EINVAL;
+ } else if (!strcmp(optname, "maxdentries") && value) {
+ p->dentries = simple_strtoul(value, &value, 0);
+ if (*value)
+ return -EINVAL;
+ }
+
+ if (optname != options)
+ *(optname-1) = ',';
+ if (value)
+ *savep = save;
+/* if (ret == 0) */
+/* break; */
+ }
+
+ return 0;
+}
+
+static void init_limits(struct ramfs_sb_info *rsb, struct ramfs_params *p)
+{
+ struct sysinfo si;
+
+ si_meminfo(&si);
+
+ /* By default we set the limits to be:
+ - Allow this ramfs to take up to half of all available RAM
+ - No limit on filesize (except no file may be bigger that
+ the total max size, obviously)
+ - dentries limited to one per 4k of data space
+ - No limit to the number of inodes (except that there
+ are never more inodes than dentries).
+ */
+ rsb->max_pages = (si.totalram / 2);
+
+ if (p->pages >= 0)
+ rsb->max_pages = p->pages;
+
+ rsb->max_file_pages = 0;
+ if (p->filepages >= 0)
+ rsb->max_file_pages = p->filepages;
+
+ rsb->max_dentries = rsb->max_pages * K_PER_PAGE / 4;
+ if (p->dentries >= 0)
+ rsb->max_dentries = p->dentries;
+
+ rsb->max_inodes = 0;
+ if (p->inodes >= 0)
+ rsb->max_inodes = p->inodes;
+
+ rsb->free_pages = rsb->max_pages;
+ rsb->free_inodes = rsb->max_inodes;
+ rsb->free_dentries = rsb->max_dentries;
+
+ return;
+}
+
+/* reset_limits is called during a remount to change the usage limits.
+
+ This will suceed, even if the new limits are lower than current
+ usage. This is the intended behaviour - new allocations will fail
+ until usage falls below the new limit */
+static void reset_limits(struct ramfs_sb_info *rsb, struct ramfs_params *p)
+{
+ lock_rsb(rsb);
+
+ if (p->pages >= 0) {
+ int used_pages = rsb->max_pages - rsb->free_pages;
+
+ rsb->max_pages = p->pages;
+ rsb->free_pages = rsb->max_pages - used_pages;
+ }
+
+ if (p->filepages >= 0) {
+ rsb->max_file_pages = p->filepages;
+ }
+
+
+ if (p->dentries >= 0) {
+ int used_dentries = rsb->max_dentries - rsb->free_dentries;
+
+ rsb->max_dentries = p->dentries;
+ rsb->free_dentries = rsb->max_dentries - used_dentries;
+ }
+
+ if (p->inodes >= 0) {
+ int used_inodes = rsb->max_inodes - rsb->free_inodes;
+
+ rsb->max_inodes = p->inodes;
+ rsb->free_inodes = rsb->max_inodes - used_inodes;
+ }
+
+ unlock_rsb(rsb);
+}
+
+static int ramfs_remount(struct super_block * sb, int * flags, char * data)
+{
+ struct ramfs_params params;
+ struct ramfs_sb_info * rsb = RAMFS_SB(sb);
+
+ if (parse_options((char *)data, ¶ms) != 0)
+ return -EINVAL;
+
+ reset_limits(rsb, ¶ms);
+
+ printk(KERN_DEBUG "ramfs: remounted with options: %s\n",
+ data ? (char *)data : "<defaults>" );
+ printk(KERN_DEBUG "ramfs: max_pages=%ld max_file_pages=%ld \
+max_inodes=%ld max_dentries=%ld\n",
+ rsb->max_pages, rsb->max_file_pages,
+ rsb->max_inodes, rsb->max_dentries);
+
+ return 0;
+}
+
static int ramfs_sync_file(struct file * file, struct dentry *dentry, int datasync)
{
return 0;
}
+
+static struct inode_operations ramfs_inode_operations = {
+ truncate: ramfs_truncate,
+};
+
+static struct inode_operations ramfs_symlink_inode_operations = {
+ truncate: ramfs_truncate,
+ readlink: page_readlink,
+ follow_link: page_follow_link,
+};
+
static struct address_space_operations ramfs_aops = {
readpage: ramfs_readpage,
writepage: fail_writepage,
@@ -273,6 +651,7 @@
};
static struct file_operations ramfs_file_operations = {
+ llseek: generic_file_llseek,
read: generic_file_read,
write: generic_file_write,
mmap: generic_file_mmap,
@@ -300,17 +679,36 @@
static struct super_operations ramfs_ops = {
statfs: ramfs_statfs,
put_inode: force_delete,
+ delete_inode: ramfs_delete_inode,
+ put_super: ramfs_put_super,
+ remount_fs: ramfs_remount,
};
static struct super_block *ramfs_read_super(struct super_block * sb, void * data, int silent)
{
struct inode * inode;
struct dentry * root;
+ struct ramfs_sb_info * rsb;
+ struct ramfs_params params;
sb->s_blocksize = PAGE_CACHE_SIZE;
sb->s_blocksize_bits = PAGE_CACHE_SHIFT;
sb->s_magic = RAMFS_MAGIC;
sb->s_op = &ramfs_ops;
+ sb->s_maxbytes = MAX_NON_LFS; /* Why? */
+
+ rsb = kmalloc(sizeof(struct ramfs_sb_info), GFP_KERNEL);
+ RAMFS_SB(sb) = rsb;
+ if (!rsb)
+ return NULL;
+
+ spin_lock_init(&rsb->ramfs_lock);
+
+ if (parse_options((char *)data, ¶ms) != 0)
+ return NULL;
+
+ init_limits(rsb, ¶ms);
+
inode = ramfs_get_inode(sb, S_IFDIR | 0755, 0);
if (!inode)
return NULL;
@@ -321,6 +719,13 @@
return NULL;
}
sb->s_root = root;
+
+ printk(KERN_DEBUG "ramfs: mounted with options: %s\n",
+ data ? (char *)data : "<defaults>" );
+ printk(KERN_DEBUG "ramfs: max_pages=%ld max_file_pages=%ld \
+max_inodes=%ld max_dentries=%ld\n",
+ rsb->max_pages, rsb->max_file_pages,
+ rsb->max_inodes, rsb->max_dentries);
return sb;
}
Hi Alan,
On Tue, 4 Dec 2001, Alan Cox wrote:
>> wrt the ramfs leak (the referenced patch below worked for me),
>> is the ramfs usage limits patch + this fix going into
>> the official 2.4 soon as it was in the ac series for ages?
>
> The -ac ramfs changes need the mm operations changes. Someone has to
> go merge that with Andrea-vm then you can get ramfs fixed and
> accounting sorted out in shmfs
To be pendantic here: Accounting is right in tmpfs (^=shmfs).
What's missing is the global statistic of how many in memory pages in
the page cache are actually tmpfs pages.
Greetings
Christoph
Hi Tachino,
On Wed, 05 Dec 2001, Tachino Nobuhiro wrote:
> + if (!strcmp(optname, "maxfilesize") && value) {
> + p->filepages = simple_strtoul(value, &value, 0)
> + / K_PER_PAGE;
> + if (*value)
> + return -EINVAL;
> + } else if (!strcmp(optname, "maxsize") && value) {
> + p->pages = simple_strtoul(value, &value, 0)
> + / K_PER_PAGE;
> + if (*value)
> + return -EINVAL;
> + } else if (!strcmp(optname, "maxinodes") && value) {
> + p->inodes = simple_strtoul(value, &value, 0);
> + if (*value)
> + return -EINVAL;
> + } else if (!strcmp(optname, "maxdentries") && value) {
> + p->dentries = simple_strtoul(value, &value, 0);
> + if (*value)
> + return -EINVAL;
> + }
Please! If you do the limit checking for ramfs adapt the same options
like shmem.c i.e. size,nr_inodes,nr_blocks,mode(+uid+gid). Don't
invent yet another mount option set. Also give them the same
semantics. Best would be to use shmem_parse_options.
Further thought: Wouldn't it be better to add a no_swap mount option
to shmem and try to merge the two? There is a lot of code duplication
between mm/shmem.c and fs/ramfs/inode.c.
Greetings
Christoph
At 05 Dec 2001 09:23:03 +0100,
Christoph Rohland wrote:
>
> Hi Tachino,
>
> On Wed, 05 Dec 2001, Tachino Nobuhiro wrote:
> > + if (!strcmp(optname, "maxfilesize") && value) {
> > + p->filepages = simple_strtoul(value, &value, 0)
> > + / K_PER_PAGE;
> > + if (*value)
> > + return -EINVAL;
> > + } else if (!strcmp(optname, "maxsize") && value) {
> > + p->pages = simple_strtoul(value, &value, 0)
> > + / K_PER_PAGE;
> > + if (*value)
> > + return -EINVAL;
> > + } else if (!strcmp(optname, "maxinodes") && value) {
> > + p->inodes = simple_strtoul(value, &value, 0);
> > + if (*value)
> > + return -EINVAL;
> > + } else if (!strcmp(optname, "maxdentries") && value) {
> > + p->dentries = simple_strtoul(value, &value, 0);
> > + if (*value)
> > + return -EINVAL;
> > + }
>
> Please! If you do the limit checking for ramfs adapt the same options
> like shmem.c i.e. size,nr_inodes,nr_blocks,mode(+uid+gid). Don't
> invent yet another mount option set. Also give them the same
> semantics. Best would be to use shmem_parse_options.
These options are not my invention. Ramfs in 2.4.13-ac7 already has them.
But I agree the original options are not easy to understand, so if compatibility
does not matter, I am glad to change them.
> Further thought: Wouldn't it be better to add a no_swap mount option
> to shmem and try to merge the two? There is a lot of code duplication
> between mm/shmem.c and fs/ramfs/inode.c.
>
I thought that too. but I don't know it should be done in stable kernel series.
Hi Padraig,
Thanks for the patch. I applied it to fs/ramfs/inode.c
in 2.4.13-ac7 and compile it as a module in 2.4.2
(removepage was changed back to truncatepage, of
course), but it seems that memory leak still occured
(I have to stick to 2.4.2 as one of my production
systems uses it...) Any idea?
By the way, is it possible to use ramdisk with the
size larger than 600MB? /var/log/messages repeatedly
reported the following when I tried to mount a
ext2-formatted ramdisk of 600MB:
Dec 5 17:22:27 roy-home kernel: set_blocksize: dev
01:00 buffer_dirty 655360 size 1024
....
....
Dec 5 17:22:27 roy-home kernel: EXT2-fs: Magic
mismatch, very weird !
Many thanks,
Roy
--- Padraig Brady <[email protected]> wrote:
> Roy S.C. Ho wrote:
>
> > Hi, I am using linux kernel 2.4.2 and I have 1 GB
> ram.
> > I tried to boot the system with a ramdisk size of
> > 600MB. It was ok when I did "mke2fs" on it, but
> when I
> > mounted it, it failed (Magic number mismatch). I
> tried
> > this several times and found that all ramdisk
> sizes
> > larger than 513MB could cause trouble. Could
> anyone
> > please kindly give me some hints? I would like to
> have
> > a larger ramdisk (around 800MB).
> >
> > (note: I tried ramfs but it seems to have memory
> > leakage when files are deleted and created
> frequently;
> > tmpfs is ok, but the pages may be swapped, which
> is
> > not desirable in my case...)
>
>
> does the patch attached fix your problem with ramfs?
>
>
__________________________________________________
Do You Yahoo!?
Buy the perfect holiday gifts at Yahoo! Shopping.
http://shopping.yahoo.com
Hi Tachino,
On Wed, 05 Dec 2001, Tachino Nobuhiro wrote:
>> Please! If you do the limit checking for ramfs adapt the same
>> options like shmem.c
>> i.e. size,nr_inodes,nr_blocks,mode(+uid+gid). Don't invent yet
>> another mount option set. Also give them the same semantics. Best
>> would be to use shmem_parse_options.
>
> These options are not my invention. Ramfs in 2.4.13-ac7 already has
> them. But I agree the original options are not easy to understand,
> so if compatibility does not matter, I am glad to change them.
But this was never integrated into the stock kernel.
>> Further thought: Wouldn't it be better to add a no_swap mount
>> option to shmem and try to merge the two? There is a lot of code
>> duplication between mm/shmem.c and fs/ramfs/inode.c.
>
> I thought that too. but I don't know it should be done in stable
> kernel series.
But we have 2.5 now. And that's where this should be done.
Greetings
Christoph
On Wed, Dec 05, 2001 at 09:23:03AM +0100, Christoph Rohland wrote:
> Hi Tachino,
>
> On Wed, 05 Dec 2001, Tachino Nobuhiro wrote:
> > + if (!strcmp(optname, "maxfilesize") && value) {
> > + p->filepages = simple_strtoul(value, &value, 0)
> > + / K_PER_PAGE;
> > + if (*value)
> > + return -EINVAL;
> > + } else if (!strcmp(optname, "maxsize") && value) {
> > + p->pages = simple_strtoul(value, &value, 0)
> > + / K_PER_PAGE;
> > + if (*value)
> > + return -EINVAL;
> > + } else if (!strcmp(optname, "maxinodes") && value) {
> > + p->inodes = simple_strtoul(value, &value, 0);
> > + if (*value)
> > + return -EINVAL;
> > + } else if (!strcmp(optname, "maxdentries") && value) {
> > + p->dentries = simple_strtoul(value, &value, 0);
> > + if (*value)
> > + return -EINVAL;
> > + }
>
> Please! If you do the limit checking for ramfs adapt the same options
> like shmem.c i.e. size,nr_inodes,nr_blocks,mode(+uid+gid). Don't
> invent yet another mount option set. Also give them the same
> semantics. Best would be to use shmem_parse_options.
The options are different because the ramfs limits patch predates
shmfs.
> Further thought: Wouldn't it be better to add a no_swap mount option
> to shmem and try to merge the two? There is a lot of code duplication
> between mm/shmem.c and fs/ramfs/inode.c.
Possibly. In fact the patch to fs/ramfs/inode.c will be insufficient
- the limits patch also requires a change to struct
address_space_operations in fs.h, and also a change in mm/pagemap.c.
shmfs applies the limits in a different way which doesn't need this, I
haven't looked at it enough to see how it's done - by the time shmfs
came around I'd moved on from the ramfs stuff.
On the other hand one of the nice things about ramfs is it's
simplicity and ramfs with limits is quite a bit less complex than
shmfs. Of course, ramfs without limits is even simpler which is, I
believe, why Linus didn't merge the patch in the first place.
--
David Gibson | For every complex problem there is a
[email protected] | solution which is simple, neat and
| wrong. -- H.L. Mencken
http://www.ozlabs.org/people/dgibson
> simplicity and ramfs with limits is quite a bit less complex than
> shmfs. Of course, ramfs without limits is even simpler which is, I
> believe, why Linus didn't merge the patch in the first place.
Ramfs without limits is useless. It doesn't provide any guidance to an fs
implementor about error handling. Its _too_ simple.
Alan
Hi David,
On Thu, 6 Dec 2001, David Gibson wrote:
> The options are different because the ramfs limits patch predates
> shmfs.
But tmpfs made it earlier into the kernel and if we want to merge the
ramfs patch we should unify the options.
>> Further thought: Wouldn't it be better to add a no_swap mount
>> option to shmem and try to merge the two? There is a lot of code
>> duplication between mm/shmem.c and fs/ramfs/inode.c.
>
> Possibly. In fact the patch to fs/ramfs/inode.c will be
> insufficient - the limits patch also requires a change to struct
> address_space_operations in fs.h, and also a change in mm/pagemap.c.
> shmfs applies the limits in a different way which doesn't need this, I
> haven't looked at it enough to see how it's done - by the time shmfs
> came around I'd moved on from the ramfs stuff.
I thought the patch in question does it without the removepage
operation.
> On the other hand one of the nice things about ramfs is it's
> simplicity and ramfs with limits is quite a bit less complex than
> shmfs.
But the core of shmem is always compiled. And the rest is as simple as
ramfs...
Greetings
Christoph
On Sat, Dec 08, 2001 at 10:53:48AM +0100, Christoph Rohland wrote:
> Hi David,
>
> On Thu, 6 Dec 2001, David Gibson wrote:
> > The options are different because the ramfs limits patch predates
> > shmfs.
>
> But tmpfs made it earlier into the kernel and if we want to merge the
> ramfs patch we should unify the options.
I know, just giving you the background.
> >> Further thought: Wouldn't it be better to add a no_swap mount
> >> option to shmem and try to merge the two? There is a lot of code
> >> duplication between mm/shmem.c and fs/ramfs/inode.c.
> >
> > Possibly. In fact the patch to fs/ramfs/inode.c will be
> > insufficient - the limits patch also requires a change to struct
> > address_space_operations in fs.h, and also a change in mm/pagemap.c.
> > shmfs applies the limits in a different way which doesn't need this, I
> > haven't looked at it enough to see how it's done - by the time shmfs
> > came around I'd moved on from the ramfs stuff.
>
> I thought the patch in question does it without the removepage
> operation.
Oh, so it does, I wonder who did that. Yes, I thought of doing it the
way its done there but rejected it on the grounds of ugliness - plus I
wasn't sure of some details of the VFS which meant I wasn't sure if it
would always work correctly.
> > On the other hand one of the nice things about ramfs is it's
> > simplicity and ramfs with limits is quite a bit less complex than
> > shmfs.
>
> But the core of shmem is always compiled. And the rest is as simple as
> ramfs...
The point of keeping ramfs simple isn't to reduce the kernel image
size, it's to keep it useful as an example of how to make a trivial
filesystem.
--
David Gibson | For every complex problem there is a
[email protected] | solution which is simple, neat and
| wrong. -- H.L. Mencken
http://www.ozlabs.org/people/dgibson
On Sun, Dec 16, 2001 at 04:34:01PM +0100, Christoph Rohland wrote:
> Hi David,
>
> On Fri, 14 Dec 2001, David Gibson wrote:
> >> But the core of shmem is always compiled. And the rest is as simple
> >> as ramfs...
> >
> > The point of keeping ramfs simple isn't to reduce the kernel image
> > size, it's to keep it useful as an example of how to make a trivial
> > filesystem.
>
> >From this point of view I prefer the oversimplified version in the
> stock kernel. We should probably correct the comment about missing
> features like limits and timestamps and tag it as experimental. But
> IMHO this version explains the underlying concept best.
>
>
> If we want a useable ramfs implementation we should try to keep the
> image size down and try to make it as similar to tmpfs as
> possible. This would keep the administrators overhead low.
>
> I append two cummulative patches against 2.4.17-rc1 (only slightly
> tested):
>
> 1) Add removepage to the addresspace_operations to simplify
> shmem.c. This is taken from your ramfs limits patch.
> 2) Add support for a ramfs2 filesystem type to shmem.c. The only
> feature missing compared to ramfs + limits are loopback devices on
> top of ramfs files. It does not add memory need compared to
> ramfs. Have a look how small this is.
I don't think there's a lot of point playing with this in 2.4.x. In
2.5 it depends a bit on what changes to the VFS happen. I recall that
near the end of the 2.3 cycle Al Viro proposed a change to add
essentially a removepage operation (he called it detachpage IIRC) - he
was using it to eliminate a bunch of the "if (page->buffers)" tests.
I don't know it that's still on the cards - it so it could certainly
simplify both ramfs and shmfs.
--
David Gibson | For every complex problem there is a
[email protected] | solution which is simple, neat and
| wrong. -- H.L. Mencken
http://www.ozlabs.org/people/dgibson
Hi David,
On Mon, 17 Dec 2001, David Gibson wrote:
> I don't think there's a lot of point playing with this in 2.4.x. In
> 2.5 it depends a bit on what changes to the VFS happen.
I know and it was not meant for inclusion into 2.4. But 2.5 is not the
nicest target to test ideas right now ;-)
Greetings
Christoph