UBIFS assert failed in ubifs_dirty_inode

I am trying use an UBIFS root filesystem on my PowerPC MPC8544 but I am seeing some intermitent problems with "UBIFS assert failed in ubifs_dirty_inode" errors.

On the first boot after I program the NAND with a fresh UBI image, everything seems to work ok.

After that, on subsequent powercycles or reboots, I sometimes see a boot with the following error:

[    5.984232] UBIFS assert failed in ubifs_dirty_inode at 377 (pid 1011)

If that message occurs, in less than 1 or 2 minutes, the system is unresponsive and I start to see infinite messages of:

[   55.072791] UBIFS assert failed in ubifs_release_budget at 566 (pid 974)
[   55.079502] Call Trace:
[   55.081960] [df187ce0] [c00070f8] show_stack+0x3c/0x17c (unreliable)
[   55.088334] [df187d20] [c0196a44] ubifs_release_budget+0x3e0/0x5d0
[   55.094533] [df187d30] [c01752d4] release_existing_page_budget+0x30/0x40
[   55.101241] [df187d60] [c01765ac] do_writepage+0xd0/0x1e4
[   55.106652] [df187da0] [c00786e0] __writepage+0x24/0x80
[   55.111884] [df187db0] [c0078b14] write_cache_pages+0x184/0x310
[   55.117817] [df187e50] [c00c5cb0] writeback_single_inode+0xac/0x288

[   55.124092] [df187e80] [c00c6608] writeback_inodes_wb+0x2d4/0x458
[   55.130193] [df187ed0] [c00c68b8] wb_writeback+0x12c/0x200
[   55.135686] [df187f40] [c00c6cb0] wb_do_writeback+0x224/0x244
[   55.141439] [df187f80] [c00c6d38] bdi_writeback_task+0x68/0xa8
[   55.147282] [df187fa0] [c0086430] bdi_start_fn+0x80/0x104
[   55.152691] [df187fc0] [c0050cc4] kthread+0x78/0x7c
[   55.157581] [df187ff0] [c000fac8] kernel_thread+0x4c/0x68
[   55.163362] UBIFS assert failed in ubifs_budget_space at 463 (pid 1139)

... Goes on forever, or at least the target becomes unresponsive and I need to power cycle.

It occurs in both the DENX 2.6.31 and the DENX 2.6.33-rc4 (which was merged the infradead kernel on December 10th).

I have the latest and greatest mtd-utils which I needed to fix my ubiformat problem.

It occurs no matter whether I ubiformat an ubi image on the target or whether I install the rootfs via untaring to a ubifs mount point.

Here are the boot messages of one where the ubifs fails:

Mounting local filesystems: [ OK ]

[    5.783968] UBIFS assert failed in ubifs_dirty_inode at 377 (pid 

1011)

The most interesting stack dump for the first error is not there because it was not printed to the serial console. Please, take a look here:

http://www.linux-mtd.infradead.org/doc/ubifs.html#L_how_send_bugreport

At the item number 4 you'll find some solutions. The 'ignore_loglevel' kernel option should be suitable for you. Please, add it and send me the stack dump.

I am trying use an UBIFS root filesystem on my PowerPC MPC8544 but I am seeing some intermitent problems with "UBIFS assert failed in ubifs_dirty_inode" errors.

On the first boot after I program the NAND with a fresh UBI image, everything seems to work ok.

After that, on subsequent powercycles or reboots, I sometimes see a boot with the following error:

[    5.984232] UBIFS assert failed in ubifs_dirty_inode at 377 (pid 1011)

If that message occurs, in less than 1 or 2 minutes, the system is unresponsive and I start to see infinite messages of:

[   55.072791] UBIFS assert failed in ubifs_release_budget at 566 (pid 974)
[   55.079502] Call Trace:
[   55.081960] [df187ce0] [c00070f8] show_stack+0x3c/0x17c (unreliable)
[   55.088334] [df187d20] [c0196a44] ubifs_release_budget+0x3e0/0x5d0
[   55.094533] [df187d30] [c01752d4] release_existing_page_budget+0x30/0x40
[   55.101241] [df187d60] [c01765ac] do_writepage+0xd0/0x1e4
[   55.106652] [df187da0] [c00786e0] __writepage+0x24/0x80
[   55.111884] [df187db0] [c0078b14] write_cache_pages+0x184/0x310
[   55.117817] [df187e50] [c00c5cb0] writeback_single_inode+0xac/0x288

[   55.124092] [df187e80] [c00c6608] writeback_inodes_wb+0x2d4/0x458
[   55.130193] [df187ed0] [c00c68b8] wb_writeback+0x12c/0x200
[   55.135686] [df187f40] [c00c6cb0] wb_do_writeback+0x224/0x244
[   55.141439] [df187f80] [c00c6d38] bdi_writeback_task+0x68/0xa8
[   55.147282] [df187fa0] [c0086430] bdi_start_fn+0x80/0x104
[   55.152691] [df187fc0] [c0050cc4] kthread+0x78/0x7c
[   55.157581] [df187ff0] [c000fac8] kernel_thread+0x4c/0x68
[   55.163362] UBIFS assert failed in ubifs_budget_space at 463 (pid 1139)

Quoted 11 lines · Jeff Angielski

I am trying use an UBIFS root filesystem on my PowerPC MPC8544 but I am seeing some intermitent problems with "UBIFS assert failed in ubifs_dirty_inode" errors.

On the first boot after I program the NAND with a fresh UBI image, everything seems to work ok.

After that, on subsequent powercycles or reboots, I sometimes see a boot with the following error:

[    5.984232] UBIFS assert failed in ubifs_dirty_inode at 377 (pid 1011)

Interesting.

The stack trace for this assertion is:

[   42.724193] [df121e60] [c00070f8] show_stack+0x3c/0x17c (unreliable)
[   42.730566] [df121ea0] [c017e754] ubifs_dirty_inode+0xe0/0xe4
[   42.736325] [df121eb0] [c00c6fbc] __mark_inode_dirty+0x3c/0x16c
[   42.742260] [df121ec0] [c01f9034] random_write+0x8c/0xa4
[   42.747584] [df121ef0] [c00a4d2c] vfs_write+0xb4/0x184
[   42.752730] [df121f10] [c00a53e8] sys_write+0x4c/0x90
[   42.757795] [df121f40] [c000fd48] ret_from_syscall+0x0/0x3c

Which leads us to

~/git/linux-2.6/drivers/char/random.c, where we can find this code:

        inode->i_mtime = current_fs_time(inode->i_sb);
        mark_inode_dirty(inode);
        return (ssize_t)count;

which is the reason for the assertion and for the further budgeting screw-up.

The thing is that UBIFS has to allocate budget every-time a clean inode is made dirty. But the 'random' driver bypasses UBIFS budget allocation, and instead, changes mtime directly and marks the inode as dirty directly.

The driver should instead call the ->setattr() method of the inode, which should do the right thing. IOW, something like this is needed:

drivers/char/random.c +1 -0

index 8258982..f911781 100644
@@ -1108,6 +1108,7 @@ static ssize_t random_write(struct file *file,
 const char __user *buffer,
 {
        size_t ret;
        struct inode *inode = file->f_path.dentry->d_inode;
+       struct iattr;
 
        ret = write_pool(&blocking_pool, buffer, count);

if (ret)

drivers/char/random.c +5 -2

@@ -1116,8 +1117,11 @@ static ssize_t random_write(struct file *file,
 const char __user *buffer,
        if (ret)
                return ret;
 
-       inode->i_mtime = current_fs_time(inode->i_sb);
-       mark_inode_dirty(inode);
+       iattr->i_mtime = current_fs_time(inode->i_sb);
+       iattr->ia_valid = ATTR_ATIME;
+       ret = inode_setattr(inode, &attr);
+       if (ret)
+               return ret;
        return (ssize_t)count;
 }

Note - I did not even compile-test this. Would you try that please :-)

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

diff -r 29db0c391ce8 drivers/char/random.c

drivers/char/random.c +0 -6

@@ -1051,12 +1051,6 @@
 				/* like a named pipe */
 	}
 
-	/*
-	 * If we gave the user some bytes, update the access time.
-	 */
-	if (count)
-		file_accessed(file);
-
 	return (count ? count : retval);
 }
 

Quoted 6 lines · Matt Mackall

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

Quoted 8 lines

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

Quoted 32 lines · Matt Mackall

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

diff -r 29db0c391ce8 drivers/char/random.c

drivers/char/random.c +0 -6

@@ -1051,12 +1051,6 @@
 				/* like a named pipe */
 	}
 
-	/*
-	 * If we gave the user some bytes, update the access time.
-	 */
-	if (count)
-		file_accessed(file);
-
 	return (count ? count : retval);
 }
 

drivers/char/random.c line 1114

Quoted 7 lines

drivers/char/random.c +0 -2

@@ -1116,8 +1110,6 @@
 	if (ret)
 		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
 	return (ssize_t)count;
 }

It may brake other FSes expectations, theoretically, as well.

Anyway, I'm perfectly fine if this is removed.

Jeff, could you please try Matt's patch and report back if you still have issues or not. If no, you can use this as a temporary work-around until a proper fix hits upstream or ubifs-2.6.git.

Quoted 43 lines · Artem Bityutskiy

Quoted 32 lines · Matt Mackall

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

diff -r 29db0c391ce8 drivers/char/random.c

drivers/char/random.c +0 -6

@@ -1051,12 +1051,6 @@
 				/* like a named pipe */
 	}
 
-	/*
-	 * If we gave the user some bytes, update the access time.
-	 */
-	if (count)
-		file_accessed(file);
-
 	return (count ? count : retval);
 }
 

drivers/char/random.c line 1114

Quoted 7 lines

drivers/char/random.c +0 -2

@@ -1116,8 +1110,6 @@
 	if (ret)
 		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
 	return (ssize_t)count;
 }

It may brake other FSes expectations, theoretically, as well.

Quoted 5 lines

Anyway, I'm perfectly fine if this is removed.

Jeff, could you please try Matt's patch and report back if you still have issues or not. If no, you can use this as a temporary work-around until a proper fix hits upstream or ubifs-2.6.git.

Matt's patch did not compile as written. I tried to implement what I think he was trying to do and created this patch (it seems to match the guts of what inode_setattr() was looking for):

drivers/char/random.c +1 -0

index 8258982..70f16c7 100644
@@ -1108,6 +1108,7 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  {
  	size_t ret;
  	struct inode *inode = file->f_path.dentry->d_inode;
+	struct iattr attr;
 
  	ret = write_pool(&blocking_pool, buffer, count);

if (ret)

drivers/char/random.c +6 -2

@@ -1116,8 +1117,12 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  	if (ret)
  		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
+	attr.ia_mtime = current_fs_time(inode->i_sb);
+	attr.ia_valid = ATTR_MTIME;
+	ret = inode_setattr(inode, &attr);
+	if (ret)
+		return ret;
+
  	return (ssize_t)count;
  }

However, this patch does not fix the problem. I still see the same errors. Matt, is this what you were trying to do?

Quoted 80 lines · Jeff Angielski

Quoted 42 lines · Artem Bityutskiy

Quoted 32 lines · Matt Mackall

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

diff -r 29db0c391ce8 drivers/char/random.c

drivers/char/random.c +0 -6

@@ -1051,12 +1051,6 @@
 				/* like a named pipe */
 	}
 
-	/*
-	 * If we gave the user some bytes, update the access time.
-	 */
-	if (count)
-		file_accessed(file);
-
 	return (count ? count : retval);
 }
 

drivers/char/random.c line 1114

Quoted 7 lines

drivers/char/random.c +0 -2

@@ -1116,8 +1110,6 @@
 	if (ret)
 		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
 	return (ssize_t)count;
 }

It may brake other FSes expectations, theoretically, as well.

Quoted 5 lines

Anyway, I'm perfectly fine if this is removed.

Jeff, could you please try Matt's patch and report back if you still have issues or not. If no, you can use this as a temporary work-around until a proper fix hits upstream or ubifs-2.6.git.

Matt's patch did not compile as written. I tried to implement what I think he was trying to do and created this patch (it seems to match the guts of what inode_setattr() was looking for):

drivers/char/random.c +1 -0

index 8258982..70f16c7 100644
@@ -1108,6 +1108,7 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  {
  	size_t ret;
  	struct inode *inode = file->f_path.dentry->d_inode;
+	struct iattr attr;
 
  	ret = write_pool(&blocking_pool, buffer, count);

if (ret)

drivers/char/random.c +6 -2

@@ -1116,8 +1117,12 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  	if (ret)
  		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
+	attr.ia_mtime = current_fs_time(inode->i_sb);
+	attr.ia_valid = ATTR_MTIME;
+	ret = inode_setattr(inode, &attr);
+	if (ret)
+		return ret;
+
  	return (ssize_t)count;
  }

Quoted 1 line

However, this patch does not fix the problem. I still see the same errors. Matt, is this what you were trying to do?

That doesn't look anything like my patch? And mine was test compiled.

Quoted 83 lines · Matt Mackall

Quoted 82 lines · Jeff Angielski

Quoted 42 lines · Artem Bityutskiy

Quoted 32 lines · Matt Mackall

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

diff -r 29db0c391ce8 drivers/char/random.c

drivers/char/random.c +0 -6

@@ -1051,12 +1051,6 @@
 				/* like a named pipe */
 	}
 
-	/*
-	 * If we gave the user some bytes, update the access time.
-	 */
-	if (count)
-		file_accessed(file);
-
 	return (count ? count : retval);
 }
 

drivers/char/random.c line 1114

Quoted 7 lines

drivers/char/random.c +0 -2

@@ -1116,8 +1110,6 @@
 	if (ret)
 		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
 	return (ssize_t)count;
 }

It may brake other FSes expectations, theoretically, as well.

Quoted 5 lines

Anyway, I'm perfectly fine if this is removed.

Jeff, could you please try Matt's patch and report back if you still have issues or not. If no, you can use this as a temporary work-around until a proper fix hits upstream or ubifs-2.6.git.

Matt's patch did not compile as written. I tried to implement what I think he was trying to do and created this patch (it seems to match the guts of what inode_setattr() was looking for):

drivers/char/random.c +1 -0

index 8258982..70f16c7 100644
@@ -1108,6 +1108,7 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  {
  	size_t ret;
  	struct inode *inode = file->f_path.dentry->d_inode;
+	struct iattr attr;
 
  	ret = write_pool(&blocking_pool, buffer, count);

if (ret)

drivers/char/random.c +6 -2

@@ -1116,8 +1117,12 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  	if (ret)
  		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
+	attr.ia_mtime = current_fs_time(inode->i_sb);
+	attr.ia_valid = ATTR_MTIME;
+	ret = inode_setattr(inode, &attr);
+	if (ret)
+		return ret;
+
  	return (ssize_t)count;
  }

Quoted 4 lines

Quoted 2 lines

However, this patch does not fix the problem. I still see the same errors. Matt, is this what you were trying to do?

That doesn't look anything like my patch? And mine was test compiled.

Ahh, you would be right. I mixed up authors. My bad. ;)

Matt's patch that removes the offending code works fine.

Artem's patch that tries to fix the offending code (and does not compile as posted) does not work.

Quoted 89 lines · Jeff Angielski

Quoted 87 lines · Matt Mackall

Quoted 82 lines · Jeff Angielski

Quoted 42 lines · Artem Bityutskiy

Quoted 32 lines · Matt Mackall

Hmm. I'd just as soon drop it entirely. Here's a patch. Herbert, you want to send this through your crypto tree?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

diff -r 29db0c391ce8 drivers/char/random.c

drivers/char/random.c +0 -6

@@ -1051,12 +1051,6 @@
 				/* like a named pipe */
 	}
 
-	/*
-	 * If we gave the user some bytes, update the access time.
-	 */
-	if (count)
-		file_accessed(file);
-
 	return (count ? count : retval);
 }
 

drivers/char/random.c line 1114

Quoted 7 lines

drivers/char/random.c +0 -2

@@ -1116,8 +1110,6 @@
 	if (ret)
 		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
 	return (ssize_t)count;
 }

It may brake other FSes expectations, theoretically, as well.

Quoted 5 lines

Anyway, I'm perfectly fine if this is removed.

Jeff, could you please try Matt's patch and report back if you still have issues or not. If no, you can use this as a temporary work-around until a proper fix hits upstream or ubifs-2.6.git.

Matt's patch did not compile as written. I tried to implement what I think he was trying to do and created this patch (it seems to match the guts of what inode_setattr() was looking for):

drivers/char/random.c +1 -0

index 8258982..70f16c7 100644
@@ -1108,6 +1108,7 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  {
  	size_t ret;
  	struct inode *inode = file->f_path.dentry->d_inode;
+	struct iattr attr;
 
  	ret = write_pool(&blocking_pool, buffer, count);

if (ret)

drivers/char/random.c +6 -2

@@ -1116,8 +1117,12 @@ static ssize_t random_write(struct file *file, 
 const char __user *buffer,
  	if (ret)
  		return ret;
 
-	inode->i_mtime = current_fs_time(inode->i_sb);
-	mark_inode_dirty(inode);
+	attr.ia_mtime = current_fs_time(inode->i_sb);
+	attr.ia_valid = ATTR_MTIME;
+	ret = inode_setattr(inode, &attr);
+	if (ret)
+		return ret;
+
  	return (ssize_t)count;
  }

Quoted 2 lines

However, this patch does not fix the problem. I still see the same errors. Matt, is this what you were trying to do?

That doesn't look anything like my patch? And mine was test compiled.

Quoted 6 lines

Ahh, you would be right. I mixed up authors. My bad. ;)

Matt's patch that removes the offending code works fine.

Artem's patch that tries to fix the offending code (and does not compile as posted) does not work.

Thanks for testing. So, who would bring Matt's patch upstream then, hmm?

random: drop weird m_time/a_time manipulation

No other driver does anything remotely like this that I know of except for the tty drivers, and I can't see any reason for random/urandom to do it. In fact, it's a (trivial, harmless) timing information leak. And obviously, it generates power- and flash-cycle wasting I/O, especially if combined with something like hwrngd. Also, it breaks ubifs's expectations.

Signed-off-by Matt Mackall <[email protected]>

Thanks for testing. So, who would bring Matt's patch upstream then, hmm?

Quoted 2 lines · Artem Bityutskiy

Thanks for testing. So, who would bring Matt's patch upstream then, hmm?

I've got it in crypto-2.6. Thanks!

I've just cloned:

git://git.kernel.org/pub/scm/linux/kernel/git/herbert/crypto-2.6

and do not see it there.

Matt's patch that removes the offending code works fine.