Hi,
here are the UBIFS fixes we have scheduled for 2.6.28. We are
planning to send them to Linus a little bit later. I send them
for your review.
The "UBIFS: allow for gaps when dirtying the LPT" patch solves
the problem reported by Deepak Saxena.
The "UBIFS: remove printk" fixes a little bit annoying message
from UBIFS which comes every time we re-mount the FS.
The "UBIFS: do not print scary memory allocation warnings",
"UBIFS: do not allocate too much", and
"UBIFS: pre-allocate bulk-read buffer"
fix UBIFS memory allocation problems when bulk-read is enabled.
Adrian Hunter (1):
UBIFS: allow for gaps when dirtying the LPT
Artem Bityutskiy (6):
UBIFS: remove printk
MAINTAINERS: change UBI/UBIFS git tree URLs
UBIFS: fix compilation warnings
UBIFS: do not print scary memory allocation warnings
UBIFS: do not allocate too much
UBIFS: pre-allocate bulk-read buffer
Harvey Harrison (1):
UBIFS: endian handling fixes and annotations
MAINTAINERS | 4 +-
fs/ubifs/commit.c | 4 +-
fs/ubifs/debug.c | 66 +++++++++++++++++++++--------------
fs/ubifs/dir.c | 5 ++-
fs/ubifs/file.c | 91 ++++++++++++++++++++++++++++++++++---------------
fs/ubifs/journal.c | 8 +++--
fs/ubifs/key.h | 4 +-
fs/ubifs/lpt_commit.c | 2 -
fs/ubifs/orphan.c | 28 +++++++++------
fs/ubifs/recovery.c | 17 +++++----
fs/ubifs/replay.c | 2 +-
fs/ubifs/sb.c | 9 +++--
fs/ubifs/super.c | 70 +++++++++++++++++++++++++++++++------
fs/ubifs/tnc.c | 12 +++++--
fs/ubifs/ubifs.h | 12 +++++--
15 files changed, 223 insertions(+), 111 deletions(-)
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)
From: Harvey Harrison <[email protected]>
Noticed by sparse:
fs/ubifs/file.c:75:2: warning: restricted __le64 degrades to integer
fs/ubifs/file.c:629:4: warning: restricted __le64 degrades to integer
fs/ubifs/dir.c:431:3: warning: restricted __le64 degrades to integer
This should be checked to ensure the ubifs_assert is working as
intended, I've done the suggested annotation in this patch.
fs/ubifs/sb.c:298:6: warning: incorrect type in assignment (different base types)
fs/ubifs/sb.c:298:6: expected int [signed] [assigned] tmp
fs/ubifs/sb.c:298:6: got restricted __le64 [usertype] <noident>
fs/ubifs/sb.c:299:19: warning: incorrect type in assignment (different base types)
fs/ubifs/sb.c:299:19: expected restricted __le64 [usertype] atime_sec
fs/ubifs/sb.c:299:19: got int [signed] [assigned] tmp
fs/ubifs/sb.c:300:19: warning: incorrect type in assignment (different base types)
fs/ubifs/sb.c:300:19: expected restricted __le64 [usertype] ctime_sec
fs/ubifs/sb.c:300:19: got int [signed] [assigned] tmp
fs/ubifs/sb.c:301:19: warning: incorrect type in assignment (different base types)
fs/ubifs/sb.c:301:19: expected restricted __le64 [usertype] mtime_sec
fs/ubifs/sb.c:301:19: got int [signed] [assigned] tmp
This looks like a bugfix as your tmp was a u32 so there was truncation in
the atime, mtime, ctime value, probably not intentional, add a tmp_le64
and use it here.
fs/ubifs/key.h:348:9: warning: cast to restricted __le32
fs/ubifs/key.h:348:9: warning: cast to restricted __le32
fs/ubifs/key.h:419:9: warning: cast to restricted __le32
Read from the annotated union member instead.
fs/ubifs/recovery.c:175:13: warning: incorrect type in assignment (different base types)
fs/ubifs/recovery.c:175:13: expected unsigned int [unsigned] [usertype] save_flags
fs/ubifs/recovery.c:175:13: got restricted __le32 [usertype] flags
fs/ubifs/recovery.c:186:13: warning: incorrect type in assignment (different base types)
fs/ubifs/recovery.c:186:13: expected restricted __le32 [usertype] flags
fs/ubifs/recovery.c:186:13: got unsigned int [unsigned] [usertype] save_flags
Do byteshifting at compile time of the flag value. Annotate the saved_flags
as le32.
fs/ubifs/debug.c:368:10: warning: cast to restricted __le32
fs/ubifs/debug.c:368:10: warning: cast from restricted __le64
Should be checked if the truncation was intentional, I've changed the
printk to print the full width.
Signed-off-by: Harvey Harrison <[email protected]>
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/ubifs/debug.c | 4 ++--
fs/ubifs/dir.c | 3 ++-
fs/ubifs/file.c | 4 ++--
fs/ubifs/key.h | 4 ++--
fs/ubifs/recovery.c | 4 ++--
fs/ubifs/sb.c | 9 +++++----
6 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index 7186400..f9deccb 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -364,8 +364,8 @@ void dbg_dump_node(const struct ubifs_info *c, const void *node)
le32_to_cpu(mst->ihead_lnum));
printk(KERN_DEBUG "\tihead_offs %u\n",
le32_to_cpu(mst->ihead_offs));
- printk(KERN_DEBUG "\tindex_size %u\n",
- le32_to_cpu(mst->index_size));
+ printk(KERN_DEBUG "\tindex_size %llu\n",
+ (unsigned long long)le64_to_cpu(mst->index_size));
printk(KERN_DEBUG "\tlpt_lnum %u\n",
le32_to_cpu(mst->lpt_lnum));
printk(KERN_DEBUG "\tlpt_offs %u\n",
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 526c01e..37a9e60 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -428,7 +428,8 @@ static int ubifs_readdir(struct file *file, void *dirent, filldir_t filldir)
dbg_gen("feed '%s', ino %llu, new f_pos %#x",
dent->name, (unsigned long long)le64_to_cpu(dent->inum),
key_hash_flash(c, &dent->key));
- ubifs_assert(dent->ch.sqnum > ubifs_inode(dir)->creat_sqnum);
+ ubifs_assert(le64_to_cpu(dent->ch.sqnum) >
+ ubifs_inode(dir)->creat_sqnum);
nm.len = le16_to_cpu(dent->nlen);
over = filldir(dirent, dent->name, nm.len, file->f_pos,
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 51cf511..9124eee 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -72,7 +72,7 @@ static int read_block(struct inode *inode, void *addr, unsigned int block,
return err;
}
- ubifs_assert(dn->ch.sqnum > ubifs_inode(inode)->creat_sqnum);
+ ubifs_assert(le64_to_cpu(dn->ch.sqnum) > ubifs_inode(inode)->creat_sqnum);
len = le32_to_cpu(dn->size);
if (len <= 0 || len > UBIFS_BLOCK_SIZE)
@@ -626,7 +626,7 @@ static int populate_page(struct ubifs_info *c, struct page *page,
dn = bu->buf + (bu->zbranch[nn].offs - offs);
- ubifs_assert(dn->ch.sqnum >
+ ubifs_assert(le64_to_cpu(dn->ch.sqnum) >
ubifs_inode(inode)->creat_sqnum);
len = le32_to_cpu(dn->size);
diff --git a/fs/ubifs/key.h b/fs/ubifs/key.h
index 9ee6508..3f1f16b 100644
--- a/fs/ubifs/key.h
+++ b/fs/ubifs/key.h
@@ -345,7 +345,7 @@ static inline int key_type_flash(const struct ubifs_info *c, const void *k)
{
const union ubifs_key *key = k;
- return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
+ return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
}
/**
@@ -416,7 +416,7 @@ static inline unsigned int key_block_flash(const struct ubifs_info *c,
{
const union ubifs_key *key = k;
- return le32_to_cpu(key->u32[1]) & UBIFS_S_KEY_BLOCK_MASK;
+ return le32_to_cpu(key->j32[1]) & UBIFS_S_KEY_BLOCK_MASK;
}
/**
diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c
index 77d26c1..bed9742 100644
--- a/fs/ubifs/recovery.c
+++ b/fs/ubifs/recovery.c
@@ -168,12 +168,12 @@ static int write_rcvrd_mst_node(struct ubifs_info *c,
struct ubifs_mst_node *mst)
{
int err = 0, lnum = UBIFS_MST_LNUM, sz = c->mst_node_alsz;
- uint32_t save_flags;
+ __le32 save_flags;
dbg_rcvry("recovery");
save_flags = mst->flags;
- mst->flags = cpu_to_le32(le32_to_cpu(mst->flags) | UBIFS_MST_RCVRY);
+ mst->flags |= cpu_to_le32(UBIFS_MST_RCVRY);
ubifs_prepare_node(c, mst, UBIFS_MST_NODE_SZ, 1);
err = ubi_leb_change(c->ubi, lnum, mst, sz, UBI_SHORTTERM);
diff --git a/fs/ubifs/sb.c b/fs/ubifs/sb.c
index 2bf753b..0f39235 100644
--- a/fs/ubifs/sb.c
+++ b/fs/ubifs/sb.c
@@ -81,6 +81,7 @@ static int create_default_filesystem(struct ubifs_info *c)
int lpt_lebs, lpt_first, orph_lebs, big_lpt, ino_waste, sup_flags = 0;
int min_leb_cnt = UBIFS_MIN_LEB_CNT;
uint64_t tmp64, main_bytes;
+ __le64 tmp_le64;
/* Some functions called from here depend on the @c->key_len filed */
c->key_len = UBIFS_SK_LEN;
@@ -295,10 +296,10 @@ static int create_default_filesystem(struct ubifs_info *c)
ino->ch.node_type = UBIFS_INO_NODE;
ino->creat_sqnum = cpu_to_le64(++c->max_sqnum);
ino->nlink = cpu_to_le32(2);
- tmp = cpu_to_le64(CURRENT_TIME_SEC.tv_sec);
- ino->atime_sec = tmp;
- ino->ctime_sec = tmp;
- ino->mtime_sec = tmp;
+ tmp_le64 = cpu_to_le64(CURRENT_TIME_SEC.tv_sec);
+ ino->atime_sec = tmp_le64;
+ ino->ctime_sec = tmp_le64;
+ ino->mtime_sec = tmp_le64;
ino->atime_nsec = 0;
ino->ctime_nsec = 0;
ino->mtime_nsec = 0;
--
1.5.4.3
From: Artem Bityutskiy <[email protected]>
Remove the "UBIFS background thread ubifs_bgd0_0 started" message.
We kill the background thread when we switch to R/O mode, and
start it again whan we switch to R/W mode. OLPC is doing this
many times during boot, and we see this message many times as
well, which is irritating. So just kill the message.
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/ubifs/commit.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/fs/ubifs/commit.c b/fs/ubifs/commit.c
index 0a6aa2c..b49884c 100644
--- a/fs/ubifs/commit.c
+++ b/fs/ubifs/commit.c
@@ -234,8 +234,8 @@ int ubifs_bg_thread(void *info)
int err;
struct ubifs_info *c = info;
- ubifs_msg("background thread \"%s\" started, PID %d",
- c->bgt_name, current->pid);
+ dbg_msg("background thread \"%s\" started, PID %d",
+ c->bgt_name, current->pid);
set_freezable();
while (1) {
--
1.5.4.3
From: Artem Bityutskiy <[email protected]>
Signed-off-by: Artem Bityutskiy <[email protected]>
---
MAINTAINERS | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/MAINTAINERS b/MAINTAINERS
index d643e86..7e897ee 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -4192,7 +4192,7 @@ M: [email protected]
P: Adrian Hunter
M: [email protected]
L: [email protected]
-T: git git://git.infradead.org/~dedekind/ubifs-2.6.git
+T: git git://git.infradead.org/ubifs-2.6.git
W: http://www.linux-mtd.infradead.org/doc/ubifs.html
S: Maintained
@@ -4246,7 +4246,7 @@ P: Artem Bityutskiy
M: [email protected]
W: http://www.linux-mtd.infradead.org/
L: [email protected]
-T: git git://git.infradead.org/~dedekind/ubi-2.6.git
+T: git git://git.infradead.org/ubi-2.6.git
S: Maintained
USB ACM DRIVER
--
1.5.4.3
From: Artem Bityutskiy <[email protected]>
Bulk-read allocates a lot of memory with 'kmalloc()', and when it
is/gets fragmented 'kmalloc()' fails with a scarry warning. But
because bulk-read is just an optimization, UBIFS keeps working fine.
Supress the warning by passing __GFP_NOWARN option to 'kmalloc()'.
This patch also introduces a macro for the magic 128KiB constant.
This is just neater.
Note, this is not really fixes the problem we had, but just hides
the warnings. The further patches fix the problem.
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/ubifs/file.c | 4 ++--
fs/ubifs/super.c | 17 ++++++++++++-----
fs/ubifs/ubifs.h | 2 +-
3 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 9124eee..8be827c 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -705,12 +705,12 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct page *page1)
int err, page_idx, page_cnt, ret = 0, n = 0;
loff_t isize;
- bu = kmalloc(sizeof(struct bu_info), GFP_NOFS);
+ bu = kmalloc(sizeof(struct bu_info), GFP_NOFS | __GFP_NOWARN);
if (!bu)
return 0;
bu->buf_len = c->bulk_read_buf_size;
- bu->buf = kmalloc(bu->buf_len, GFP_NOFS);
+ bu->buf = kmalloc(bu->buf_len, GFP_NOFS | __GFP_NOWARN);
if (!bu->buf)
goto out_free;
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 8780efb..ea493e6 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -36,6 +36,12 @@
#include <linux/mount.h>
#include "ubifs.h"
+/*
+ * Maximum amount of memory we may 'kmalloc()' without worrying that we are
+ * allocating too much.
+ */
+#define UBIFS_KMALLOC_OK (128*1024)
+
/* Slab cache for UBIFS inodes */
struct kmem_cache *ubifs_inode_slab;
@@ -561,17 +567,18 @@ static int init_constants_early(struct ubifs_info *c)
* calculations when reporting free space.
*/
c->leb_overhead = c->leb_size % UBIFS_MAX_DATA_NODE_SZ;
+
/* Buffer size for bulk-reads */
c->bulk_read_buf_size = UBIFS_MAX_BULK_READ * UBIFS_MAX_DATA_NODE_SZ;
if (c->bulk_read_buf_size > c->leb_size)
c->bulk_read_buf_size = c->leb_size;
- if (c->bulk_read_buf_size > 128 * 1024) {
- /* Check if we can kmalloc more than 128KiB */
- void *try = kmalloc(c->bulk_read_buf_size, GFP_KERNEL);
-
+ if (c->bulk_read_buf_size > UBIFS_KMALLOC_OK) {
+ /* Check if we can kmalloc that much */
+ void *try = kmalloc(c->bulk_read_buf_size,
+ GFP_KERNEL | __GFP_NOWARN);
kfree(try);
if (!try)
- c->bulk_read_buf_size = 128 * 1024;
+ c->bulk_read_buf_size = UBIFS_KMALLOC_OK;
}
return 0;
}
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index a7bd32f..06ba51e 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -753,7 +753,7 @@ struct ubifs_znode {
};
/**
- * struct bu_info - bulk-read information
+ * struct bu_info - bulk-read information.
* @key: first data node key
* @zbranch: zbranches of data nodes to bulk read
* @buf: buffer to read into
--
1.5.4.3
From: Artem Bityutskiy <[email protected]>
Bulk-read allocates 128KiB or more using kmalloc. The allocation
starts failing often when the memory gets fragmented. UBIFS still
works fine in this case, because it falls-back to standard
(non-optimized) read method, though. This patch teaches bulk-read
to allocate exactly the amount of memory it needs, instead of
allocating 128KiB every time.
This patch is also a preparation to the further fix where we'll
have a pre-allocated bulk-read buffer as well. For example, now
the @bu object is prepared in 'ubifs_bulk_read()', so we could
path either pre-allocated or allocated information to
'ubifs_do_bulk_read()' later. Or teaching 'ubifs_do_bulk_read()'
not to allocate 'bu->buf' if it is already there.
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/ubifs/file.c | 70 +++++++++++++++++++++++++++++++++++------------------
fs/ubifs/super.c | 12 ++++----
fs/ubifs/tnc.c | 7 ++++-
fs/ubifs/ubifs.h | 4 +-
4 files changed, 60 insertions(+), 33 deletions(-)
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 8be827c..0c5c27d 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -691,32 +691,22 @@ out_err:
/**
* ubifs_do_bulk_read - do bulk-read.
* @c: UBIFS file-system description object
- * @page1: first page
+ * @bu: bulk-read information
+ * @page1: first page to read
*
* This function returns %1 if the bulk-read is done, otherwise %0 is returned.
*/
-static int ubifs_do_bulk_read(struct ubifs_info *c, struct page *page1)
+static int ubifs_do_bulk_read(struct ubifs_info *c, struct bu_info *bu,
+ struct page *page1)
{
pgoff_t offset = page1->index, end_index;
struct address_space *mapping = page1->mapping;
struct inode *inode = mapping->host;
struct ubifs_inode *ui = ubifs_inode(inode);
- struct bu_info *bu;
int err, page_idx, page_cnt, ret = 0, n = 0;
+ int allocate = bu->buf ? 0 : 1;
loff_t isize;
- bu = kmalloc(sizeof(struct bu_info), GFP_NOFS | __GFP_NOWARN);
- if (!bu)
- return 0;
-
- bu->buf_len = c->bulk_read_buf_size;
- bu->buf = kmalloc(bu->buf_len, GFP_NOFS | __GFP_NOWARN);
- if (!bu->buf)
- goto out_free;
-
- data_key_init(c, &bu->key, inode->i_ino,
- offset << UBIFS_BLOCKS_PER_PAGE_SHIFT);
-
err = ubifs_tnc_get_bu_keys(c, bu);
if (err)
goto out_warn;
@@ -735,12 +725,25 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct page *page1)
* together. If all the pages were like this, bulk-read would
* reduce performance, so we turn it off for a while.
*/
- ui->read_in_a_row = 0;
- ui->bulk_read = 0;
- goto out_free;
+ goto out_bu_off;
}
if (bu->cnt) {
+ if (allocate) {
+ /*
+ * Allocate bulk-read buffer depending on how many data
+ * nodes we are going to read.
+ */
+ bu->buf_len = bu->zbranch[bu->cnt - 1].offs +
+ bu->zbranch[bu->cnt - 1].len -
+ bu->zbranch[0].offs;
+ ubifs_assert(bu->buf_len > 0);
+ ubifs_assert(bu->buf_len <= c->leb_size);
+ bu->buf = kmalloc(bu->buf_len, GFP_NOFS | __GFP_NOWARN);
+ if (!bu->buf)
+ goto out_bu_off;
+ }
+
err = ubifs_tnc_bulk_read(c, bu);
if (err)
goto out_warn;
@@ -779,13 +782,17 @@ static int ubifs_do_bulk_read(struct ubifs_info *c, struct page *page1)
ui->last_page_read = offset + page_idx - 1;
out_free:
- kfree(bu->buf);
- kfree(bu);
+ if (allocate)
+ kfree(bu->buf);
return ret;
out_warn:
ubifs_warn("ignoring error %d and skipping bulk-read", err);
goto out_free;
+
+out_bu_off:
+ ui->read_in_a_row = ui->bulk_read = 0;
+ goto out_free;
}
/**
@@ -803,18 +810,20 @@ static int ubifs_bulk_read(struct page *page)
struct ubifs_info *c = inode->i_sb->s_fs_info;
struct ubifs_inode *ui = ubifs_inode(inode);
pgoff_t index = page->index, last_page_read = ui->last_page_read;
- int ret = 0;
+ struct bu_info *bu;
+ int err = 0;
ui->last_page_read = index;
-
if (!c->bulk_read)
return 0;
+
/*
* Bulk-read is protected by ui_mutex, but it is an optimization, so
* don't bother if we cannot lock the mutex.
*/
if (!mutex_trylock(&ui->ui_mutex))
return 0;
+
if (index != last_page_read + 1) {
/* Turn off bulk-read if we stop reading sequentially */
ui->read_in_a_row = 1;
@@ -822,6 +831,7 @@ static int ubifs_bulk_read(struct page *page)
ui->bulk_read = 0;
goto out_unlock;
}
+
if (!ui->bulk_read) {
ui->read_in_a_row += 1;
if (ui->read_in_a_row < 3)
@@ -829,10 +839,22 @@ static int ubifs_bulk_read(struct page *page)
/* Three reads in a row, so switch on bulk-read */
ui->bulk_read = 1;
}
- ret = ubifs_do_bulk_read(c, page);
+
+ bu = kmalloc(sizeof(struct bu_info), GFP_NOFS | __GFP_NOWARN);
+ if (!bu)
+ return 0;
+
+ bu->buf = NULL;
+ bu->buf_len = c->max_bu_buf_len;
+ data_key_init(c, &bu->key, inode->i_ino,
+ page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT);
+
+ err = ubifs_do_bulk_read(c, bu, page);
+ kfree(bu);
+
out_unlock:
mutex_unlock(&ui->ui_mutex);
- return ret;
+ return err;
}
static int ubifs_readpage(struct file *file, struct page *page)
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index ea493e6..1d51156 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -569,16 +569,16 @@ static int init_constants_early(struct ubifs_info *c)
c->leb_overhead = c->leb_size % UBIFS_MAX_DATA_NODE_SZ;
/* Buffer size for bulk-reads */
- c->bulk_read_buf_size = UBIFS_MAX_BULK_READ * UBIFS_MAX_DATA_NODE_SZ;
- if (c->bulk_read_buf_size > c->leb_size)
- c->bulk_read_buf_size = c->leb_size;
- if (c->bulk_read_buf_size > UBIFS_KMALLOC_OK) {
+ c->max_bu_buf_len = UBIFS_MAX_BULK_READ * UBIFS_MAX_DATA_NODE_SZ;
+ if (c->max_bu_buf_len > c->leb_size)
+ c->max_bu_buf_len = c->leb_size;
+ if (c->max_bu_buf_len > UBIFS_KMALLOC_OK) {
/* Check if we can kmalloc that much */
- void *try = kmalloc(c->bulk_read_buf_size,
+ void *try = kmalloc(c->max_bu_buf_len,
GFP_KERNEL | __GFP_NOWARN);
kfree(try);
if (!try)
- c->bulk_read_buf_size = UBIFS_KMALLOC_OK;
+ c->max_bu_buf_len = UBIFS_KMALLOC_OK;
}
return 0;
}
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index 99e9a74..6eef534 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -1501,7 +1501,12 @@ out:
* @bu: bulk-read parameters and results
*
* Lookup consecutive data node keys for the same inode that reside
- * consecutively in the same LEB.
+ * consecutively in the same LEB. This function returns zero in case of success
+ * and a negative error code in case of failure.
+ *
+ * Note, if the bulk-read buffer length (@bu->buf_len) is known, this function
+ * makes sure bulk-read nodes fit the buffer. Otherwise, this function prepares
+ * maxumum possible amount of nodes for bulk-read.
*/
int ubifs_tnc_get_bu_keys(struct ubifs_info *c, struct bu_info *bu)
{
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 06ba51e..870b5c4 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -969,7 +969,7 @@ struct ubifs_mount_opts {
* @mst_node: master node
* @mst_offs: offset of valid master node
* @mst_mutex: protects the master node area, @mst_node, and @mst_offs
- * @bulk_read_buf_size: buffer size for bulk-reads
+ * @max_bu_buf_len: maximum bulk-read buffer length
*
* @log_lebs: number of logical eraseblocks in the log
* @log_bytes: log size in bytes
@@ -1217,7 +1217,7 @@ struct ubifs_info {
struct ubifs_mst_node *mst_node;
int mst_offs;
struct mutex mst_mutex;
- int bulk_read_buf_size;
+ int max_bu_buf_len;
int log_lebs;
long long log_bytes;
--
1.5.4.3
From: Artem Bityutskiy <[email protected]>
To avoid memory allocation failure during bulk-read, pre-allocate
a bulk-read buffer, so that if there is only one bulk-reader at
a time, it would just use the pre-allocated buffer and would not
do any memory allocation. However, if there are more than 1 bulk-
reader, then only one reader would use the pre-allocated buffer,
while the other reader would allocate the buffer for itself.
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/ubifs/file.c | 31 ++++++++++++++++++++--------
fs/ubifs/super.c | 57 +++++++++++++++++++++++++++++++++++++++++++++--------
fs/ubifs/ubifs.h | 6 +++++
3 files changed, 76 insertions(+), 18 deletions(-)
diff --git a/fs/ubifs/file.c b/fs/ubifs/file.c
index 0c5c27d..2624411 100644
--- a/fs/ubifs/file.c
+++ b/fs/ubifs/file.c
@@ -811,15 +811,15 @@ static int ubifs_bulk_read(struct page *page)
struct ubifs_inode *ui = ubifs_inode(inode);
pgoff_t index = page->index, last_page_read = ui->last_page_read;
struct bu_info *bu;
- int err = 0;
+ int err = 0, allocated = 0;
ui->last_page_read = index;
if (!c->bulk_read)
return 0;
/*
- * Bulk-read is protected by ui_mutex, but it is an optimization, so
- * don't bother if we cannot lock the mutex.
+ * Bulk-read is protected by @ui->ui_mutex, but it is an optimization,
+ * so don't bother if we cannot lock the mutex.
*/
if (!mutex_trylock(&ui->ui_mutex))
return 0;
@@ -840,17 +840,30 @@ static int ubifs_bulk_read(struct page *page)
ui->bulk_read = 1;
}
- bu = kmalloc(sizeof(struct bu_info), GFP_NOFS | __GFP_NOWARN);
- if (!bu)
- return 0;
+ /*
+ * If possible, try to use pre-allocated bulk-read information, which
+ * is protected by @c->bu_mutex.
+ */
+ if (mutex_trylock(&c->bu_mutex))
+ bu = &c->bu;
+ else {
+ bu = kmalloc(sizeof(struct bu_info), GFP_NOFS | __GFP_NOWARN);
+ if (!bu)
+ goto out_unlock;
+
+ bu->buf = NULL;
+ allocated = 1;
+ }
- bu->buf = NULL;
bu->buf_len = c->max_bu_buf_len;
data_key_init(c, &bu->key, inode->i_ino,
page->index << UBIFS_BLOCKS_PER_PAGE_SHIFT);
-
err = ubifs_do_bulk_read(c, bu, page);
- kfree(bu);
+
+ if (!allocated)
+ mutex_unlock(&c->bu_mutex);
+ else
+ kfree(bu);
out_unlock:
mutex_unlock(&ui->ui_mutex);
diff --git a/fs/ubifs/super.c b/fs/ubifs/super.c
index 1d51156..d80b2ae 100644
--- a/fs/ubifs/super.c
+++ b/fs/ubifs/super.c
@@ -572,14 +572,6 @@ static int init_constants_early(struct ubifs_info *c)
c->max_bu_buf_len = UBIFS_MAX_BULK_READ * UBIFS_MAX_DATA_NODE_SZ;
if (c->max_bu_buf_len > c->leb_size)
c->max_bu_buf_len = c->leb_size;
- if (c->max_bu_buf_len > UBIFS_KMALLOC_OK) {
- /* Check if we can kmalloc that much */
- void *try = kmalloc(c->max_bu_buf_len,
- GFP_KERNEL | __GFP_NOWARN);
- kfree(try);
- if (!try)
- c->max_bu_buf_len = UBIFS_KMALLOC_OK;
- }
return 0;
}
@@ -999,6 +991,34 @@ static void destroy_journal(struct ubifs_info *c)
}
/**
+ * bu_init - initialize bulk-read information.
+ * @c: UBIFS file-system description object
+ */
+static void bu_init(struct ubifs_info *c)
+{
+ ubifs_assert(c->bulk_read == 1);
+
+ if (c->bu.buf)
+ return; /* Already initialized */
+
+again:
+ c->bu.buf = kmalloc(c->max_bu_buf_len, GFP_KERNEL | __GFP_NOWARN);
+ if (!c->bu.buf) {
+ if (c->max_bu_buf_len > UBIFS_KMALLOC_OK) {
+ c->max_bu_buf_len = UBIFS_KMALLOC_OK;
+ goto again;
+ }
+
+ /* Just disable bulk-read */
+ ubifs_warn("Cannot allocate %d bytes of memory for bulk-read, "
+ "disabling it", c->max_bu_buf_len);
+ c->mount_opts.bulk_read = 1;
+ c->bulk_read = 0;
+ return;
+ }
+}
+
+/**
* mount_ubifs - mount UBIFS file-system.
* @c: UBIFS file-system description object
*
@@ -1066,6 +1086,13 @@ static int mount_ubifs(struct ubifs_info *c)
goto out_free;
}
+ if (c->bulk_read == 1)
+ bu_init(c);
+
+ /*
+ * We have to check all CRCs, even for data nodes, when we mount the FS
+ * (specifically, when we are replaying).
+ */
c->always_chk_crc = 1;
err = ubifs_read_superblock(c);
@@ -1296,6 +1323,7 @@ out_cbuf:
out_dereg:
dbg_failure_mode_deregistration(c);
out_free:
+ kfree(c->bu.buf);
vfree(c->ileb_buf);
vfree(c->sbuf);
kfree(c->bottom_up_buf);
@@ -1332,10 +1360,11 @@ static void ubifs_umount(struct ubifs_info *c)
kfree(c->cbuf);
kfree(c->rcvrd_mst_node);
kfree(c->mst_node);
+ kfree(c->bu.buf);
+ vfree(c->ileb_buf);
vfree(c->sbuf);
kfree(c->bottom_up_buf);
UBIFS_DBG(vfree(c->dbg_buf));
- vfree(c->ileb_buf);
dbg_failure_mode_deregistration(c);
}
@@ -1633,6 +1662,7 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
ubifs_err("invalid or unknown remount parameter");
return err;
}
+
if ((sb->s_flags & MS_RDONLY) && !(*flags & MS_RDONLY)) {
err = ubifs_remount_rw(c);
if (err)
@@ -1640,6 +1670,14 @@ static int ubifs_remount_fs(struct super_block *sb, int *flags, char *data)
} else if (!(sb->s_flags & MS_RDONLY) && (*flags & MS_RDONLY))
ubifs_remount_ro(c);
+ if (c->bulk_read == 1)
+ bu_init(c);
+ else {
+ dbg_gen("disable bulk-read");
+ kfree(c->bu.buf);
+ c->bu.buf = NULL;
+ }
+
return 0;
}
@@ -1730,6 +1768,7 @@ static int ubifs_fill_super(struct super_block *sb, void *data, int silent)
mutex_init(&c->log_mutex);
mutex_init(&c->mst_mutex);
mutex_init(&c->umount_mutex);
+ mutex_init(&c->bu_mutex);
init_waitqueue_head(&c->cmt_wq);
c->buds = RB_ROOT;
c->old_idx = RB_ROOT;
diff --git a/fs/ubifs/ubifs.h b/fs/ubifs/ubifs.h
index 870b5c4..46b1725 100644
--- a/fs/ubifs/ubifs.h
+++ b/fs/ubifs/ubifs.h
@@ -969,7 +969,10 @@ struct ubifs_mount_opts {
* @mst_node: master node
* @mst_offs: offset of valid master node
* @mst_mutex: protects the master node area, @mst_node, and @mst_offs
+ *
* @max_bu_buf_len: maximum bulk-read buffer length
+ * @bu_mutex: protects the pre-allocated bulk-read buffer and @c->bu
+ * @bu: pre-allocated bulk-read information
*
* @log_lebs: number of logical eraseblocks in the log
* @log_bytes: log size in bytes
@@ -1217,7 +1220,10 @@ struct ubifs_info {
struct ubifs_mst_node *mst_node;
int mst_offs;
struct mutex mst_mutex;
+
int max_bu_buf_len;
+ struct mutex bu_mutex;
+ struct bu_info bu;
int log_lebs;
long long log_bytes;
--
1.5.4.3
From: Adrian Hunter <[email protected]>
The LPT may have gaps in it because initially empty LEBs
are not added by mkfs.ubifs - because it does not know how
many there are. Then UBIFS allocates empty LEBs in the
reverse order that they are discovered i.e. they are
added to, and removed from, the front of a list. That
creates a gap in the middle of the LPT.
The function dirtying the LPT tree (for the purpose of
small model garbage collection) assumed that a gap could
only occur at the very end of the LPT and stopped dirtying
prematurely, which in turn resulted in the LPT running
out of space - something that is designed to be impossible.
Signed-off-by: Adrian Hunter <[email protected]>
---
fs/ubifs/lpt_commit.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/fs/ubifs/lpt_commit.c b/fs/ubifs/lpt_commit.c
index eed5a00..a41434b 100644
--- a/fs/ubifs/lpt_commit.c
+++ b/fs/ubifs/lpt_commit.c
@@ -571,8 +571,6 @@ static struct ubifs_pnode *next_pnode(struct ubifs_info *c,
/* We assume here that LEB zero is never an LPT LEB */
if (nnode->nbranch[iip].lnum)
return ubifs_get_pnode(c, nnode, iip);
- else
- return NULL;
}
/* Go up while can't go right */
--
1.5.4.3
From: Artem Bityutskiy <[email protected]>
We print 'ino_t' type using '%lu' printk() placeholder, but this
results in many warnings when compiling for Alpha platform. Fix
this by adding (unsingned long) casts.
Fixes these warnings:
fs/ubifs/journal.c:693: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/journal.c:1131: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/dir.c:163: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/tnc.c:2680: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/tnc.c:2700: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/replay.c:1066: warning: format '%lu' expects type 'long unsigned int', but argument 7 has type 'ino_t'
fs/ubifs/orphan.c:108: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:135: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:142: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:154: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:159: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:451: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:539: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:612: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:843: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/orphan.c:856: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/recovery.c:1438: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/recovery.c:1443: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/recovery.c:1475: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/recovery.c:1495: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:105: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:105: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:110: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:110: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:114: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:114: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:118: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:118: warning: format '%lu' expects type 'long unsigned int', but argument 3 has type 'ino_t'
fs/ubifs/debug.c:1591: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1671: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1674: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/debug.c:1680: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1699: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/debug.c:1788: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/debug.c:1821: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/debug.c:1833: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/debug.c:1924: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1932: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1938: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1945: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1953: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1960: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1967: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1973: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1988: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
fs/ubifs/debug.c:1991: warning: format '%lu' expects type 'long unsigned int', but argument 5 has type 'ino_t'
fs/ubifs/debug.c:2009: warning: format '%lu' expects type 'long unsigned int', but argument 2 has type 'ino_t'
Reported-by: Randy Dunlap <[email protected]>
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/ubifs/debug.c | 62 ++++++++++++++++++++++++++++++--------------------
fs/ubifs/dir.c | 2 +-
fs/ubifs/journal.c | 8 ++++--
fs/ubifs/orphan.c | 28 +++++++++++++----------
fs/ubifs/recovery.c | 13 +++++-----
fs/ubifs/replay.c | 2 +-
fs/ubifs/tnc.c | 5 ++-
7 files changed, 70 insertions(+), 50 deletions(-)
diff --git a/fs/ubifs/debug.c b/fs/ubifs/debug.c
index f9deccb..510ffa0 100644
--- a/fs/ubifs/debug.c
+++ b/fs/ubifs/debug.c
@@ -101,21 +101,24 @@ static void sprintf_key(const struct ubifs_info *c, const union ubifs_key *key,
if (c->key_fmt == UBIFS_SIMPLE_KEY_FMT) {
switch (type) {
case UBIFS_INO_KEY:
- sprintf(p, "(%lu, %s)", key_inum(c, key),
+ sprintf(p, "(%lu, %s)", (unsigned long)key_inum(c, key),
get_key_type(type));
break;
case UBIFS_DENT_KEY:
case UBIFS_XENT_KEY:
- sprintf(p, "(%lu, %s, %#08x)", key_inum(c, key),
+ sprintf(p, "(%lu, %s, %#08x)",
+ (unsigned long)key_inum(c, key),
get_key_type(type), key_hash(c, key));
break;
case UBIFS_DATA_KEY:
- sprintf(p, "(%lu, %s, %u)", key_inum(c, key),
+ sprintf(p, "(%lu, %s, %u)",
+ (unsigned long)key_inum(c, key),
get_key_type(type), key_block(c, key));
break;
case UBIFS_TRUN_KEY:
sprintf(p, "(%lu, %s)",
- key_inum(c, key), get_key_type(type));
+ (unsigned long)key_inum(c, key),
+ get_key_type(type));
break;
default:
sprintf(p, "(bad key type: %#08x, %#08x)",
@@ -1589,7 +1592,7 @@ static struct fsck_inode *add_inode(struct ubifs_info *c,
if (inum > c->highest_inum) {
ubifs_err("too high inode number, max. is %lu",
- c->highest_inum);
+ (unsigned long)c->highest_inum);
return ERR_PTR(-EINVAL);
}
@@ -1668,16 +1671,18 @@ static struct fsck_inode *read_add_inode(struct ubifs_info *c,
ino_key_init(c, &key, inum);
err = ubifs_lookup_level0(c, &key, &znode, &n);
if (!err) {
- ubifs_err("inode %lu not found in index", inum);
+ ubifs_err("inode %lu not found in index", (unsigned long)inum);
return ERR_PTR(-ENOENT);
} else if (err < 0) {
- ubifs_err("error %d while looking up inode %lu", err, inum);
+ ubifs_err("error %d while looking up inode %lu",
+ err, (unsigned long)inum);
return ERR_PTR(err);
}
zbr = &znode->zbranch[n];
if (zbr->len < UBIFS_INO_NODE_SZ) {
- ubifs_err("bad node %lu node length %d", inum, zbr->len);
+ ubifs_err("bad node %lu node length %d",
+ (unsigned long)inum, zbr->len);
return ERR_PTR(-EINVAL);
}
@@ -1697,7 +1702,7 @@ static struct fsck_inode *read_add_inode(struct ubifs_info *c,
kfree(ino);
if (IS_ERR(fscki)) {
ubifs_err("error %ld while adding inode %lu node",
- PTR_ERR(fscki), inum);
+ PTR_ERR(fscki), (unsigned long)inum);
return fscki;
}
@@ -1786,7 +1791,8 @@ static int check_leaf(struct ubifs_info *c, struct ubifs_zbranch *zbr,
if (IS_ERR(fscki)) {
err = PTR_ERR(fscki);
ubifs_err("error %d while processing data node and "
- "trying to find inode node %lu", err, inum);
+ "trying to find inode node %lu",
+ err, (unsigned long)inum);
goto out_dump;
}
@@ -1819,7 +1825,8 @@ static int check_leaf(struct ubifs_info *c, struct ubifs_zbranch *zbr,
if (IS_ERR(fscki)) {
err = PTR_ERR(fscki);
ubifs_err("error %d while processing entry node and "
- "trying to find inode node %lu", err, inum);
+ "trying to find inode node %lu",
+ err, (unsigned long)inum);
goto out_dump;
}
@@ -1832,7 +1839,7 @@ static int check_leaf(struct ubifs_info *c, struct ubifs_zbranch *zbr,
err = PTR_ERR(fscki);
ubifs_err("error %d while processing entry node and "
"trying to find parent inode node %lu",
- err, inum);
+ err, (unsigned long)inum);
goto out_dump;
}
@@ -1923,7 +1930,8 @@ static int check_inodes(struct ubifs_info *c, struct fsck_data *fsckd)
fscki->references != 1) {
ubifs_err("directory inode %lu has %d "
"direntries which refer it, but "
- "should be 1", fscki->inum,
+ "should be 1",
+ (unsigned long)fscki->inum,
fscki->references);
goto out_dump;
}
@@ -1931,27 +1939,29 @@ static int check_inodes(struct ubifs_info *c, struct fsck_data *fsckd)
fscki->references != 0) {
ubifs_err("root inode %lu has non-zero (%d) "
"direntries which refer it",
- fscki->inum, fscki->references);
+ (unsigned long)fscki->inum,
+ fscki->references);
goto out_dump;
}
if (fscki->calc_sz != fscki->size) {
ubifs_err("directory inode %lu size is %lld, "
"but calculated size is %lld",
- fscki->inum, fscki->size,
- fscki->calc_sz);
+ (unsigned long)fscki->inum,
+ fscki->size, fscki->calc_sz);
goto out_dump;
}
if (fscki->calc_cnt != fscki->nlink) {
ubifs_err("directory inode %lu nlink is %d, "
"but calculated nlink is %d",
- fscki->inum, fscki->nlink,
- fscki->calc_cnt);
+ (unsigned long)fscki->inum,
+ fscki->nlink, fscki->calc_cnt);
goto out_dump;
}
} else {
if (fscki->references != fscki->nlink) {
ubifs_err("inode %lu nlink is %d, but "
- "calculated nlink is %d", fscki->inum,
+ "calculated nlink is %d",
+ (unsigned long)fscki->inum,
fscki->nlink, fscki->references);
goto out_dump;
}
@@ -1959,20 +1969,21 @@ static int check_inodes(struct ubifs_info *c, struct fsck_data *fsckd)
if (fscki->xattr_sz != fscki->calc_xsz) {
ubifs_err("inode %lu has xattr size %u, but "
"calculated size is %lld",
- fscki->inum, fscki->xattr_sz,
+ (unsigned long)fscki->inum, fscki->xattr_sz,
fscki->calc_xsz);
goto out_dump;
}
if (fscki->xattr_cnt != fscki->calc_xcnt) {
ubifs_err("inode %lu has %u xattrs, but "
- "calculated count is %lld", fscki->inum,
+ "calculated count is %lld",
+ (unsigned long)fscki->inum,
fscki->xattr_cnt, fscki->calc_xcnt);
goto out_dump;
}
if (fscki->xattr_nms != fscki->calc_xnms) {
ubifs_err("inode %lu has xattr names' size %u, but "
"calculated names' size is %lld",
- fscki->inum, fscki->xattr_nms,
+ (unsigned long)fscki->inum, fscki->xattr_nms,
fscki->calc_xnms);
goto out_dump;
}
@@ -1985,11 +1996,12 @@ out_dump:
ino_key_init(c, &key, fscki->inum);
err = ubifs_lookup_level0(c, &key, &znode, &n);
if (!err) {
- ubifs_err("inode %lu not found in index", fscki->inum);
+ ubifs_err("inode %lu not found in index",
+ (unsigned long)fscki->inum);
return -ENOENT;
} else if (err < 0) {
ubifs_err("error %d while looking up inode %lu",
- err, fscki->inum);
+ err, (unsigned long)fscki->inum);
return err;
}
@@ -2007,7 +2019,7 @@ out_dump:
}
ubifs_msg("dump of the inode %lu sitting in LEB %d:%d",
- fscki->inum, zbr->lnum, zbr->offs);
+ (unsigned long)fscki->inum, zbr->lnum, zbr->offs);
dbg_dump_node(c, ino);
kfree(ino);
return -EINVAL;
diff --git a/fs/ubifs/dir.c b/fs/ubifs/dir.c
index 37a9e60..0422c98 100644
--- a/fs/ubifs/dir.c
+++ b/fs/ubifs/dir.c
@@ -161,7 +161,7 @@ struct inode *ubifs_new_inode(struct ubifs_info *c, const struct inode *dir,
return ERR_PTR(-EINVAL);
}
ubifs_warn("running out of inode numbers (current %lu, max %d)",
- c->highest_inum, INUM_WATERMARK);
+ (unsigned long)c->highest_inum, INUM_WATERMARK);
}
inode->i_ino = ++c->highest_inum;
diff --git a/fs/ubifs/journal.c b/fs/ubifs/journal.c
index 22993f8..f91b745 100644
--- a/fs/ubifs/journal.c
+++ b/fs/ubifs/journal.c
@@ -690,8 +690,9 @@ int ubifs_jnl_write_data(struct ubifs_info *c, const struct inode *inode,
int dlen = UBIFS_DATA_NODE_SZ + UBIFS_BLOCK_SIZE * WORST_COMPR_FACTOR;
struct ubifs_inode *ui = ubifs_inode(inode);
- dbg_jnl("ino %lu, blk %u, len %d, key %s", key_inum(c, key),
- key_block(c, key), len, DBGKEY(key));
+ dbg_jnl("ino %lu, blk %u, len %d, key %s",
+ (unsigned long)key_inum(c, key), key_block(c, key), len,
+ DBGKEY(key));
ubifs_assert(len <= UBIFS_BLOCK_SIZE);
data = kmalloc(dlen, GFP_NOFS);
@@ -1128,7 +1129,8 @@ int ubifs_jnl_truncate(struct ubifs_info *c, const struct inode *inode,
ino_t inum = inode->i_ino;
unsigned int blk;
- dbg_jnl("ino %lu, size %lld -> %lld", inum, old_size, new_size);
+ dbg_jnl("ino %lu, size %lld -> %lld",
+ (unsigned long)inum, old_size, new_size);
ubifs_assert(!ui->data_len);
ubifs_assert(S_ISREG(inode->i_mode));
ubifs_assert(mutex_is_locked(&ui->ui_mutex));
diff --git a/fs/ubifs/orphan.c b/fs/ubifs/orphan.c
index 02d3462..9bd5a43 100644
--- a/fs/ubifs/orphan.c
+++ b/fs/ubifs/orphan.c
@@ -105,7 +105,7 @@ int ubifs_add_orphan(struct ubifs_info *c, ino_t inum)
list_add_tail(&orphan->list, &c->orph_list);
list_add_tail(&orphan->new_list, &c->orph_new);
spin_unlock(&c->orphan_lock);
- dbg_gen("ino %lu", inum);
+ dbg_gen("ino %lu", (unsigned long)inum);
return 0;
}
@@ -132,14 +132,16 @@ void ubifs_delete_orphan(struct ubifs_info *c, ino_t inum)
else {
if (o->dnext) {
spin_unlock(&c->orphan_lock);
- dbg_gen("deleted twice ino %lu", inum);
+ dbg_gen("deleted twice ino %lu",
+ (unsigned long)inum);
return;
}
if (o->cnext) {
o->dnext = c->orph_dnext;
c->orph_dnext = o;
spin_unlock(&c->orphan_lock);
- dbg_gen("delete later ino %lu", inum);
+ dbg_gen("delete later ino %lu",
+ (unsigned long)inum);
return;
}
rb_erase(p, &c->orph_tree);
@@ -151,12 +153,12 @@ void ubifs_delete_orphan(struct ubifs_info *c, ino_t inum)
}
spin_unlock(&c->orphan_lock);
kfree(o);
- dbg_gen("inum %lu", inum);
+ dbg_gen("inum %lu", (unsigned long)inum);
return;
}
}
spin_unlock(&c->orphan_lock);
- dbg_err("missing orphan ino %lu", inum);
+ dbg_err("missing orphan ino %lu", (unsigned long)inum);
dbg_dump_stack();
}
@@ -448,7 +450,7 @@ static void erase_deleted(struct ubifs_info *c)
rb_erase(&orphan->rb, &c->orph_tree);
list_del(&orphan->list);
c->tot_orphans -= 1;
- dbg_gen("deleting orphan ino %lu", orphan->inum);
+ dbg_gen("deleting orphan ino %lu", (unsigned long)orphan->inum);
kfree(orphan);
}
c->orph_dnext = NULL;
@@ -536,8 +538,8 @@ static int insert_dead_orphan(struct ubifs_info *c, ino_t inum)
list_add_tail(&orphan->list, &c->orph_list);
orphan->dnext = c->orph_dnext;
c->orph_dnext = orphan;
- dbg_mnt("ino %lu, new %d, tot %d",
- inum, c->new_orphans, c->tot_orphans);
+ dbg_mnt("ino %lu, new %d, tot %d", (unsigned long)inum,
+ c->new_orphans, c->tot_orphans);
return 0;
}
@@ -609,7 +611,8 @@ static int do_kill_orphans(struct ubifs_info *c, struct ubifs_scan_leb *sleb,
n = (le32_to_cpu(orph->ch.len) - UBIFS_ORPH_NODE_SZ) >> 3;
for (i = 0; i < n; i++) {
inum = le64_to_cpu(orph->inos[i]);
- dbg_rcvry("deleting orphaned inode %lu", inum);
+ dbg_rcvry("deleting orphaned inode %lu",
+ (unsigned long)inum);
err = ubifs_tnc_remove_ino(c, inum);
if (err)
return err;
@@ -840,8 +843,8 @@ static int dbg_orphan_check(struct ubifs_info *c, struct ubifs_zbranch *zbr,
if (inum != ci->last_ino) {
/* Lowest node type is the inode node, so it comes first */
if (key_type(c, &zbr->key) != UBIFS_INO_KEY)
- ubifs_err("found orphan node ino %lu, type %d", inum,
- key_type(c, &zbr->key));
+ ubifs_err("found orphan node ino %lu, type %d",
+ (unsigned long)inum, key_type(c, &zbr->key));
ci->last_ino = inum;
ci->tot_inos += 1;
err = ubifs_tnc_read_node(c, zbr, ci->node);
@@ -853,7 +856,8 @@ static int dbg_orphan_check(struct ubifs_info *c, struct ubifs_zbranch *zbr,
/* Must be recorded as an orphan */
if (!dbg_find_check_orphan(&ci->root, inum) &&
!dbg_find_orphan(c, inum)) {
- ubifs_err("missing orphan, ino %lu", inum);
+ ubifs_err("missing orphan, ino %lu",
+ (unsigned long)inum);
ci->missing += 1;
}
}
diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c
index bed9742..90acac6 100644
--- a/fs/ubifs/recovery.c
+++ b/fs/ubifs/recovery.c
@@ -1435,13 +1435,13 @@ static int fix_size_in_place(struct ubifs_info *c, struct size_entry *e)
err = ubi_leb_change(c->ubi, lnum, c->sbuf, len, UBI_UNKNOWN);
if (err)
goto out;
- dbg_rcvry("inode %lu at %d:%d size %lld -> %lld ", e->inum, lnum, offs,
- i_size, e->d_size);
+ dbg_rcvry("inode %lu at %d:%d size %lld -> %lld ",
+ (unsigned long)e->inum, lnum, offs, i_size, e->d_size);
return 0;
out:
ubifs_warn("inode %lu failed to fix size %lld -> %lld error %d",
- e->inum, e->i_size, e->d_size, err);
+ (unsigned long)e->inum, e->i_size, e->d_size, err);
return err;
}
@@ -1472,7 +1472,8 @@ int ubifs_recover_size(struct ubifs_info *c)
return err;
if (err == -ENOENT) {
/* Remove data nodes that have no inode */
- dbg_rcvry("removing ino %lu", e->inum);
+ dbg_rcvry("removing ino %lu",
+ (unsigned long)e->inum);
err = ubifs_tnc_remove_ino(c, e->inum);
if (err)
return err;
@@ -1493,8 +1494,8 @@ int ubifs_recover_size(struct ubifs_info *c)
return PTR_ERR(inode);
if (inode->i_size < e->d_size) {
dbg_rcvry("ino %lu size %lld -> %lld",
- e->inum, e->d_size,
- inode->i_size);
+ (unsigned long)e->inum,
+ e->d_size, inode->i_size);
inode->i_size = e->d_size;
ubifs_inode(inode)->ui_size = e->d_size;
e->inode = inode;
diff --git a/fs/ubifs/replay.c b/fs/ubifs/replay.c
index 7399692..21f7d04 100644
--- a/fs/ubifs/replay.c
+++ b/fs/ubifs/replay.c
@@ -1065,7 +1065,7 @@ int ubifs_replay_journal(struct ubifs_info *c)
ubifs_assert(c->bud_bytes <= c->max_bud_bytes || c->need_recovery);
dbg_mnt("finished, log head LEB %d:%d, max_sqnum %llu, "
"highest_inum %lu", c->lhead_lnum, c->lhead_offs, c->max_sqnum,
- c->highest_inum);
+ (unsigned long)c->highest_inum);
out:
destroy_replay_tree(c);
destroy_bud_list(c);
diff --git a/fs/ubifs/tnc.c b/fs/ubifs/tnc.c
index d27fd91..99e9a74 100644
--- a/fs/ubifs/tnc.c
+++ b/fs/ubifs/tnc.c
@@ -2677,7 +2677,7 @@ int ubifs_tnc_remove_ino(struct ubifs_info *c, ino_t inum)
struct ubifs_dent_node *xent, *pxent = NULL;
struct qstr nm = { .name = NULL };
- dbg_tnc("ino %lu", inum);
+ dbg_tnc("ino %lu", (unsigned long)inum);
/*
* Walk all extended attribute entries and remove them together with
@@ -2697,7 +2697,8 @@ int ubifs_tnc_remove_ino(struct ubifs_info *c, ino_t inum)
}
xattr_inum = le64_to_cpu(xent->inum);
- dbg_tnc("xent '%s', ino %lu", xent->name, xattr_inum);
+ dbg_tnc("xent '%s', ino %lu", xent->name,
+ (unsigned long)xattr_inum);
nm.name = xent->name;
nm.len = le16_to_cpu(xent->nlen);
--
1.5.4.3
* Artem Bityutskiy | 2008-11-21 19:19:26 [+0200]:
>From: Artem Bityutskiy <[email protected]>
>
>We print 'ino_t' type using '%lu' printk() placeholder, but this
>results in many warnings when compiling for Alpha platform. Fix
>this by adding (unsingned long) casts.
>
>Fixes these warnings:
>
>fs/ubifs/journal.c:693: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
Wouldn't %z help? A cast sounds wrong.
Sebastian
* Artem Bityutskiy | 2008-11-21 19:19:24 [+0200]:
>index 9ee6508..3f1f16b 100644
>--- a/fs/ubifs/key.h
>+++ b/fs/ubifs/key.h
>@@ -345,7 +345,7 @@ static inline int key_type_flash(const struct ubifs_info *c, const void *k)
> {
> const union ubifs_key *key = k;
>
>- return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
>+ return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
If you would change such references to something like
|return le32_to_cpup(&key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
then on powerpc
text data bss dec hex filename
155384 1284 24 156692 26414 ubifs-b4.ko
155372 1284 24 156680 26408 ubifs-after.ko
because now it is possible to load the value as LE from memory instead
of loading it BE and swapping it afterwads.
> }
>
> /**
>@@ -416,7 +416,7 @@ static inline unsigned int key_block_flash(const struct ubifs_info *c,
> {
> const union ubifs_key *key = k;
>
>- return le32_to_cpu(key->u32[1]) & UBIFS_S_KEY_BLOCK_MASK;
>+ return le32_to_cpu(key->j32[1]) & UBIFS_S_KEY_BLOCK_MASK;
> }
This and the previous change look like a bugfix for something that
should trigger during recovery or something? Shouldn't I fail in
ubifs_validate_entry() during recovery?
> /**
>diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c
>index 77d26c1..bed9742 100644
>--- a/fs/ubifs/recovery.c
>+++ b/fs/ubifs/recovery.c
>@@ -168,12 +168,12 @@ static int write_rcvrd_mst_node(struct ubifs_info *c,
> struct ubifs_mst_node *mst)
> {
> int err = 0, lnum = UBIFS_MST_LNUM, sz = c->mst_node_alsz;
>- uint32_t save_flags;
>+ __le32 save_flags;
>
> dbg_rcvry("recovery");
>
> save_flags = mst->flags;
>- mst->flags = cpu_to_le32(le32_to_cpu(mst->flags) | UBIFS_MST_RCVRY);
>+ mst->flags |= cpu_to_le32(UBIFS_MST_RCVRY);
another micro optimisation would be to use __constant_cpu_to_le32()
Sebastian
On Sat, 2008-11-22 at 20:27 +0100, Sebastian Andrzej Siewior wrote:
> * Artem Bityutskiy | 2008-11-21 19:19:24 [+0200]:
>
> > save_flags = mst->flags;
> >- mst->flags = cpu_to_le32(le32_to_cpu(mst->flags) | UBIFS_MST_RCVRY);
> >+ mst->flags |= cpu_to_le32(UBIFS_MST_RCVRY);
>
> another micro optimisation would be to use __constant_cpu_to_le32()
>
Nope, no difference, cpu_to_le32 does constant-detection these days and
so there ends up being no difference.
Harvey
* Harvey Harrison | 2008-11-22 19:21:29 [-0800]:
>Nope, no difference, cpu_to_le32 does constant-detection these days and
>so there ends up being no difference.
Indeed. __builtin_constant_p() does the trick. So I guess we could get
rid of __constant_cpu_to_XX32() and its
| grep -RE __constant_cpu_to_[bl]e32 . | wc -l
| 413
users.
>
>Harvey
>
Sebastian
Sebastian Andrzej Siewior wrote:
> >- return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> >+ return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
>
> If you would change such references to something like
> |return le32_to_cpup(&key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> then on powerpc
>
> text data bss dec hex filename
> 155384 1284 24 156692 26414 ubifs-b4.ko
> 155372 1284 24 156680 26408 ubifs-after.ko
>
> because now it is possible to load the value as LE from memory instead
> of loading it BE and swapping it afterwads.
If le32_to_cpu used __builtin_bswap32 (a GCC builtin), wouldn't
GCC do this trivial optimisation itself?
-- Jamie
Sebastian Andrzej Siewior wrote:
> * Artem Bityutskiy | 2008-11-21 19:19:26 [+0200]:
>
>> From: Artem Bityutskiy <[email protected]>
>>
>> We print 'ino_t' type using '%lu' printk() placeholder, but this
>> results in many warnings when compiling for Alpha platform. Fix
>> this by adding (unsingned long) casts.
>>
>> Fixes these warnings:
>>
>> fs/ubifs/journal.c:693: warning: format '%lu' expects type 'long unsigned int', but argument 4 has type 'ino_t'
>
> Wouldn't %z help? A cast sounds wrong.
Doesn't help:
fs/ubifs/journal.c:699: warning: format ‘%zu’ expects type ‘size_t’, but argument 4 has type ‘ino_t’
Sebastian Andrzej Siewior wrote:
> * Artem Bityutskiy | 2008-11-21 19:19:24 [+0200]:
>
>> index 9ee6508..3f1f16b 100644
>> --- a/fs/ubifs/key.h
>> +++ b/fs/ubifs/key.h
>> @@ -345,7 +345,7 @@ static inline int key_type_flash(const struct ubifs_info *c, const void *k)
>> {
>> const union ubifs_key *key = k;
>>
>> - return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
>> + return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
>
> If you would change such references to something like
> |return le32_to_cpup(&key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> then on powerpc
>
> text data bss dec hex filename
> 155384 1284 24 156692 26414 ubifs-b4.ko
> 155372 1284 24 156680 26408 ubifs-after.ko
>
> because now it is possible to load the value as LE from memory instead
> of loading it BE and swapping it afterwads.
Wouldn't that be true for every le32_to_cpu of an lvalue? Shame you can't
do:
is_lvalue(x) ? le32_to_cpup(&(x)) : le32_to_cpu(x)
>> }
>>
>> /**
>> @@ -416,7 +416,7 @@ static inline unsigned int key_block_flash(const struct ubifs_info *c,
>> {
>> const union ubifs_key *key = k;
>>
>> - return le32_to_cpu(key->u32[1]) & UBIFS_S_KEY_BLOCK_MASK;
>> + return le32_to_cpu(key->j32[1]) & UBIFS_S_KEY_BLOCK_MASK;
>> }
>
> This and the previous change look like a bugfix for something that
> should trigger during recovery or something? Shouldn't I fail in
> ubifs_validate_entry() during recovery?
This is just about casting. key->u32[1] and key->j32[1] are the same object.
There is no "real" bug in these two cases - just compilation warnings.
>> /**
>> diff --git a/fs/ubifs/recovery.c b/fs/ubifs/recovery.c
>> index 77d26c1..bed9742 100644
>> --- a/fs/ubifs/recovery.c
>> +++ b/fs/ubifs/recovery.c
>> @@ -168,12 +168,12 @@ static int write_rcvrd_mst_node(struct ubifs_info *c,
>> struct ubifs_mst_node *mst)
>> {
>> int err = 0, lnum = UBIFS_MST_LNUM, sz = c->mst_node_alsz;
>> - uint32_t save_flags;
>> + __le32 save_flags;
>>
>> dbg_rcvry("recovery");
>>
>> save_flags = mst->flags;
>> - mst->flags = cpu_to_le32(le32_to_cpu(mst->flags) | UBIFS_MST_RCVRY);
>> + mst->flags |= cpu_to_le32(UBIFS_MST_RCVRY);
>
> another micro optimisation would be to use __constant_cpu_to_le32()
As per Harvey's reply.
On Mon, 2008-11-24 at 16:19 +0200, Adrian Hunter wrote:
> Sebastian Andrzej Siewior wrote:
> > * Artem Bityutskiy | 2008-11-21 19:19:24 [+0200]:
> >
> >> index 9ee6508..3f1f16b 100644
> >> --- a/fs/ubifs/key.h
> >> +++ b/fs/ubifs/key.h
> >> @@ -345,7 +345,7 @@ static inline int key_type_flash(const struct ubifs_info *c, const void *k)
> >> {
> >> const union ubifs_key *key = k;
> >>
> >> - return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> >> + return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> >
> > If you would change such references to something like
> > |return le32_to_cpup(&key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> > then on powerpc
> >
> > text data bss dec hex filename
> > 155384 1284 24 156692 26414 ubifs-b4.ko
> > 155372 1284 24 156680 26408 ubifs-after.ko
> >
> > because now it is possible to load the value as LE from memory instead
> > of loading it BE and swapping it afterwads.
>
> Wouldn't that be true for every le32_to_cpu of an lvalue? Shame you can't
> do:
>
> is_lvalue(x) ? le32_to_cpup(&(x)) : le32_to_cpu(x)
>
No, you wouldn't want to do the above if the lvalue was on the stack as most
of the time the extra code to setup a pointer to a stack variable ends up
being more expensive than just using cpu_to_le32.
Cheers,
Harvey
* Adrian Hunter | 2008-11-24 12:03:41 [+0200]:
> Doesn't help:
>
> fs/ubifs/journal.c:699: warning: format ???%zu??? expects type
> ???size_t???, but argument 4 has type ???ino_t???
Indeed. BUT:
Is there actually a reason why Alpha is the only arch having
__kernel_ino_t defined as unsigned int instead of unsigned long ?
(except s390 in 32bit mode).
I just checked and I haven't seen anything that would point out that
__kernel_ino_t / ino_t is part of user space API.
What I've found instead is for instance that ext2 relies that ino_t is a
long:
|struct inode *ext2_iget (struct super_block *sb, unsigned long ino)
|{
....
| raw_inode = ext2_get_inode(inode->i_sb, ino, &bh);
and the prototype is:
|static struct ext2_inode *ext2_get_inode(struct super_block *sb, ino_t ino,
| struct buffer_head **p)
So we lose the upper 32bit on Alpha. Unless the whole system is
self-contained and the ext2_iget() user never passes something > ino_t.
Any comments?
Sebastian
On Sat, 2008-11-22 at 20:27 +0100, Sebastian Andrzej Siewior wrote:
> >index 9ee6508..3f1f16b 100644
> >--- a/fs/ubifs/key.h
> >+++ b/fs/ubifs/key.h
> >@@ -345,7 +345,7 @@ static inline int key_type_flash(const struct ubifs_info *c, const void *k)
> > {
> > const union ubifs_key *key = k;
> >
> >- return le32_to_cpu(key->u32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> >+ return le32_to_cpu(key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
>
> If you would change such references to something like
> |return le32_to_cpup(&key->j32[1]) >> UBIFS_S_KEY_BLOCK_BITS;
> then on powerpc
>
> text data bss dec hex filename
> 155384 1284 24 156692 26414 ubifs-b4.ko
> 155372 1284 24 156680 26408 ubifs-after.ko
>
> because now it is possible to load the value as LE from memory instead
> of loading it BE and swapping it afterwads.
Well, I think stuff like this should be done by either GCC or by a PPC-specific
'le32_to_cpu()' implementation.
--
Best regards,
Artem Bityutskiy (Битюцкий Артём)