From cf5eba53346fbfdf1b80e05ca3fd7fe2ec841077 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Sun, 14 May 2006 04:06:24 +0100 Subject: [JFFS2] Reduce excessive node count for syslog files. We currently get fairly poor behaviour with files which get many short writes, such as system logs. This is because we end up with many tiny data nodes, and the rbtree gets massive. None of these nodes are actually obsolete, so they are counted as 'clean' space. Eraseblocks can be entirely full of these nodes (which are REF_NORMAL instead of REF_PRISTINE), and still they count entirely towards 'used_size' and the eraseblocks can sit on the clean_list for a long time without being picked for GC. One way to alleviate this in the long term is to account REF_NORMAL space separately from REF_PRISTINE space, rather than counting them both towards used_size. Then these eraseblocks can be picked for GC and the offending nodes will be garbage collected. The short-term fix, though -- which probably makes sense even if we do eventually implement the above -- is to merge these nodes as they're written. When we write the last byte in a page, write the _whole_ page. This obsoletes the earlier nodes in the page _immediately_ and we don't even need to wait for the garbage collection to do it. Original implementation from Ferenc Havasi Signed-off-by: David Woodhouse --- fs/jffs2/file.c | 20 ++++++++++++++------ 1 file changed, 14 insertions(+), 6 deletions(-) (limited to 'fs/jffs2') diff --git a/fs/jffs2/file.c b/fs/jffs2/file.c index 9f4171213e5..3349db0a786 100644 --- a/fs/jffs2/file.c +++ b/fs/jffs2/file.c @@ -215,12 +215,20 @@ static int jffs2_commit_write (struct file *filp, struct page *pg, D1(printk(KERN_DEBUG "jffs2_commit_write(): ino #%lu, page at 0x%lx, range %d-%d, flags %lx\n", inode->i_ino, pg->index << PAGE_CACHE_SHIFT, start, end, pg->flags)); - if (!start && end == PAGE_CACHE_SIZE) { - /* We need to avoid deadlock with page_cache_read() in - jffs2_garbage_collect_pass(). So we have to mark the - page up to date, to prevent page_cache_read() from - trying to re-lock it. */ - SetPageUptodate(pg); + if (end == PAGE_CACHE_SIZE) { + if (!start) { + /* We need to avoid deadlock with page_cache_read() in + jffs2_garbage_collect_pass(). So we have to mark the + page up to date, to prevent page_cache_read() from + trying to re-lock it. */ + SetPageUptodate(pg); + } else { + /* When writing out the end of a page, write out the + _whole_ page. This helps to reduce the number of + nodes in files which have many short writes, like + syslog files. */ + start = aligned_start = 0; + } } ri = jffs2_alloc_raw_inode(); -- cgit v1.2.3-70-g09d2 From 3e68fbb59b3d4e6b47b65e9928b5929e02179759 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Mon, 15 May 2006 00:49:43 +0100 Subject: [JFFS2] Don't pack on-medium structures, because GCC emits crappy code If we use __attribute__((packed)), GCC will _also_ assume that the structures aren't sensibly aligned, and it'll emit code to cope with that instead of straight word load/save. This can be _very_ suboptimal on architectures like ARM. Ideally, we want an attribute which just tells GCC not to do any padding, without the alignment side-effects. In the absense of that, we'll just drop the 'packed' attribute and hope that everything stays as it was (which to be fair is fairly much what we expect). And add some paranoia checks in the initialisation code, which should be optimised away completely in the normal case. Signed-off-by: David Woodhouse --- fs/jffs2/super.c | 14 +++++++++++++- include/linux/jffs2.h | 14 +++++++------- 2 files changed, 20 insertions(+), 8 deletions(-) (limited to 'fs/jffs2') diff --git a/fs/jffs2/super.c b/fs/jffs2/super.c index ffd8e84b22c..5f73de58692 100644 --- a/fs/jffs2/super.c +++ b/fs/jffs2/super.c @@ -320,6 +320,18 @@ static int __init init_jffs2_fs(void) { int ret; + /* Paranoia checks for on-medium structures. If we ask GCC + to pack them with __attribute__((packed)) then it _also_ + assumes that they're not aligned -- so it emits crappy + code on some architectures. Ideally we want an attribute + which means just 'no padding', without the alignment + thing. But GCC doesn't have that -- we have to just + hope the structs are the right sizes, instead. */ + BUG_ON(sizeof(struct jffs2_unknown_node) != 12); + BUG_ON(sizeof(struct jffs2_raw_dirent) != 40); + BUG_ON(sizeof(struct jffs2_raw_inode) != 68); + BUG_ON(sizeof(struct jffs2_raw_summary) != 32); + printk(KERN_INFO "JFFS2 version 2.2." #ifdef CONFIG_JFFS2_FS_WRITEBUFFER " (NAND)" @@ -327,7 +339,7 @@ static int __init init_jffs2_fs(void) #ifdef CONFIG_JFFS2_SUMMARY " (SUMMARY) " #endif - " (C) 2001-2003 Red Hat, Inc.\n"); + " (C) 2001-2006 Red Hat, Inc.\n"); jffs2_inode_cachep = kmem_cache_create("jffs2_i", sizeof(struct jffs2_inode_info), diff --git a/include/linux/jffs2.h b/include/linux/jffs2.h index cf792bb3c72..228ad72f7dd 100644 --- a/include/linux/jffs2.h +++ b/include/linux/jffs2.h @@ -82,15 +82,15 @@ typedef struct { uint32_t v32; -} __attribute__((packed)) jint32_t; +} jint32_t; typedef struct { uint32_t m; -} __attribute__((packed)) jmode_t; +} jmode_t; typedef struct { uint16_t v16; -} __attribute__((packed)) jint16_t; +} jint16_t; struct jffs2_unknown_node { @@ -99,7 +99,7 @@ struct jffs2_unknown_node jint16_t nodetype; jint32_t totlen; /* So we can skip over nodes we don't grok */ jint32_t hdr_crc; -} __attribute__((packed)); +}; struct jffs2_raw_dirent { @@ -117,7 +117,7 @@ struct jffs2_raw_dirent jint32_t node_crc; jint32_t name_crc; uint8_t name[0]; -} __attribute__((packed)); +}; /* The JFFS2 raw inode structure: Used for storage on physical media. */ /* The uid, gid, atime, mtime and ctime members could be longer, but @@ -149,7 +149,7 @@ struct jffs2_raw_inode jint32_t data_crc; /* CRC for the (compressed) data. */ jint32_t node_crc; /* CRC for the raw inode (excluding data) */ uint8_t data[0]; -} __attribute__((packed)); +}; struct jffs2_raw_summary { @@ -163,7 +163,7 @@ struct jffs2_raw_summary jint32_t sum_crc; /* summary information crc */ jint32_t node_crc; /* node crc */ jint32_t sum[0]; /* inode summary info */ -} __attribute__((packed)); +}; union jffs2_node_union { -- cgit v1.2.3-70-g09d2 From 184f565210c6c8a852c53ffc070f9add61e0f331 Mon Sep 17 00:00:00 2001 From: Andrew Morton Date: Mon, 15 May 2006 13:45:58 +0100 Subject: [JFFS2] Fix printk format in some error messages. fs/jffs2/nodelist.c: In function `check_node_data': fs/jffs2/nodelist.c:441: warning: unsigned int format, different type arg (arg 4) fs/jffs2/nodelist.c:464: warning: int format, different type arg (arg 5) Modified from Andrew's original fix because while his terminal may indeed only have eighty columns, mine only has _TWENTYFOUR_ lines. So the cosmetic fluff is perfectly OK out past column 80 where it was -- the casual reader doesn't _care_ about anything more than the fact that it goes 'if (foo) JFFS2_WARNING...', and there's no point wasting a whole line to display the tail end of the printk which nobody actually cares about. Signed-off-by: Andrew Morton Signed-off-by: David Woodhouse --- fs/jffs2/nodelist.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'fs/jffs2') diff --git a/fs/jffs2/nodelist.c b/fs/jffs2/nodelist.c index d4d0c41490c..7d71593d7fd 100644 --- a/fs/jffs2/nodelist.c +++ b/fs/jffs2/nodelist.c @@ -438,7 +438,7 @@ static int check_node_data(struct jffs2_sb_info *c, struct jffs2_tmp_dnode_info if (c->mtd->point) { err = c->mtd->point(c->mtd, ofs, len, &retlen, &buffer); if (!err && retlen < tn->csize) { - JFFS2_WARNING("MTD point returned len too short: %u instead of %u.\n", retlen, tn->csize); + JFFS2_WARNING("MTD point returned len too short: %zu instead of %u.\n", retlen, tn->csize); c->mtd->unpoint(c->mtd, buffer, ofs, len); } else if (err) JFFS2_WARNING("MTD point failed: error code %d.\n", err); @@ -461,7 +461,7 @@ static int check_node_data(struct jffs2_sb_info *c, struct jffs2_tmp_dnode_info } if (retlen != len) { - JFFS2_ERROR("short read at %#08x: %d instead of %d.\n", ofs, retlen, len); + JFFS2_ERROR("short read at %#08x: %zd instead of %d.\n", ofs, retlen, len); err = -EIO; goto free_out; } -- cgit v1.2.3-70-g09d2 From 5b5ffbc1e6d62d89747f3f59c09b2e488a7d7fce Mon Sep 17 00:00:00 2001 From: Florin Malita Date: Mon, 15 May 2006 23:42:31 +0100 Subject: [PATCH] jffs2: memory leak in jffs2_scan_medium() If jffs2_scan_eraseblock() fails and the exit path is taken, 's' is not being deallocated. Reported by Coverity, CID: 1258. Signed-off-by: Florin Malita Signed-off-by: David Woodhouse --- fs/jffs2/scan.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'fs/jffs2') diff --git a/fs/jffs2/scan.c b/fs/jffs2/scan.c index cf55b221fc2..eca0996def6 100644 --- a/fs/jffs2/scan.c +++ b/fs/jffs2/scan.c @@ -222,9 +222,6 @@ int jffs2_scan_medium(struct jffs2_sb_info *c) } } - if (jffs2_sum_active() && s) - kfree(s); - /* Nextblock dirty is always seen as wasted, because we cannot recycle it now */ if (c->nextblock && (c->nextblock->dirty_size)) { c->nextblock->wasted_size += c->nextblock->dirty_size; @@ -266,6 +263,9 @@ int jffs2_scan_medium(struct jffs2_sb_info *c) else c->mtd->unpoint(c->mtd, flashbuf, 0, c->mtd->size); #endif + if (s) + kfree(s); + return ret; } -- cgit v1.2.3-70-g09d2 From 7d2beb135986477f53da77303356bd04329f8d0e Mon Sep 17 00:00:00 2001 From: David Brownell Date: Tue, 16 May 2006 16:08:10 +0100 Subject: [JFFS2] Fix section mismatch warnings in JFFS2. Mark certain functions with __init and __exit appropriately. Signed-off-by: David Brownell Signed-off-by: David Woodhouse --- fs/jffs2/compr.c | 4 ++-- fs/jffs2/compr_zlib.c | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) (limited to 'fs/jffs2') diff --git a/fs/jffs2/compr.c b/fs/jffs2/compr.c index e7944e665b9..5f45e01d71e 100644 --- a/fs/jffs2/compr.c +++ b/fs/jffs2/compr.c @@ -412,7 +412,7 @@ void jffs2_free_comprbuf(unsigned char *comprbuf, unsigned char *orig) kfree(comprbuf); } -int jffs2_compressors_init(void) +int __init jffs2_compressors_init(void) { /* Registering compressors */ #ifdef CONFIG_JFFS2_ZLIB @@ -440,7 +440,7 @@ int jffs2_compressors_init(void) return 0; } -int jffs2_compressors_exit(void) +int __exit jffs2_compressors_exit(void) { /* Unregistering compressors */ #ifdef CONFIG_JFFS2_RUBIN diff --git a/fs/jffs2/compr_zlib.c b/fs/jffs2/compr_zlib.c index 5c63e0cdcf4..d43cbac4fb9 100644 --- a/fs/jffs2/compr_zlib.c +++ b/fs/jffs2/compr_zlib.c @@ -60,7 +60,7 @@ static int __init alloc_workspaces(void) return 0; } -static void free_workspaces(void) +static void __exit free_workspaces(void) { vfree(def_strm.workspace); vfree(inf_strm.workspace); @@ -216,7 +216,7 @@ int __init jffs2_zlib_init(void) return ret; } -void jffs2_zlib_exit(void) +void __exit jffs2_zlib_exit(void) { jffs2_unregister_compressor(&jffs2_zlib_comp); free_workspaces(); -- cgit v1.2.3-70-g09d2 From c41ff6e5f38b02ff927d0d510e28dc1392bb4690 Mon Sep 17 00:00:00 2001 From: David Woodhouse Date: Tue, 16 May 2006 17:05:33 +0100 Subject: [JFFS2] Fix printk format in jffs2_sum_write_data() error message. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit fs/jffs2/summary.c: In function ‘jffs2_sum_write_data’: fs/jffs2/summary.c:658: warning: format ‘%zd’ expects type ‘signed size_t’, but argument 4 has type ‘uint32_t’ Signed-off-by: David Woodhouse --- fs/jffs2/summary.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'fs/jffs2') diff --git a/fs/jffs2/summary.c b/fs/jffs2/summary.c index 7b0ed77a4c3..48293c197f1 100644 --- a/fs/jffs2/summary.c +++ b/fs/jffs2/summary.c @@ -655,7 +655,7 @@ static int jffs2_sum_write_data(struct jffs2_sb_info *c, struct jffs2_eraseblock if (ret || (retlen != infosize)) { - JFFS2_WARNING("Write of %d bytes at 0x%08x failed. returned %d, retlen %zu\n", + JFFS2_WARNING("Write of %u bytes at 0x%08x failed. returned %d, retlen %zd\n", infosize, jeb->offset + c->sector_size - jeb->free_size, ret, retlen); c->summary->sum_size = JFFS2_SUMMARY_NOSUM_SIZE; -- cgit v1.2.3-70-g09d2