2.4.2 fs/inode.c

2.4.2 fs/inode.c

[Jan Harkes]

I found some code that seems wrong and didn't even match it's comment.
Patch is against 2.4.2, but should go cleanly against 2.4.3-pre6 as well.

Jan


--- linux/fs/inode.c.orig	Thu Mar 22 13:20:55 2001
+++ linux/fs/inode.c	Thu Mar 22 13:21:32 2001
@@ -133,7 +133,7 @@
 
 	if (sb) {
 		/* Don't do this for I_DIRTY_PAGES - that doesn't actually dirty the inode itself */
-		if (flags & (I_DIRTY | I_DIRTY_SYNC)) {
+		if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
 			if (sb->s_op && sb->s_op->dirty_inode)
 				sb->s_op->dirty_inode(inode);
 		}

Re: 2.4.2 fs/inode.c

[Stephen C. Tweedie]

Hi,

On Thu, Mar 22, 2001 at 01:42:15PM -0500, Jan Harkes wrote:
> 
> I found some code that seems wrong and didn't even match it's comment.
> Patch is against 2.4.2, but should go cleanly against 2.4.3-pre6 as well.
 
Patch looks fine to me.  Have you tested it?  If this goes wrong,
things break badly...

>  		/* Don't do this for I_DIRTY_PAGES - that doesn't actually dirty the inode itself */
> -		if (flags & (I_DIRTY | I_DIRTY_SYNC)) {
> +		if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {

--Stephen

Re: 2.4.2 fs/inode.c

[Jan Harkes]

On Thu, Mar 22, 2001 at 07:04:52PM +0000, Stephen C. Tweedie wrote:
> Hi,
> 
> On Thu, Mar 22, 2001 at 01:42:15PM -0500, Jan Harkes wrote:
> > 
> > I found some code that seems wrong and didn't even match it's comment.
> > Patch is against 2.4.2, but should go cleanly against 2.4.3-pre6 as well.
>  
> Patch looks fine to me.  Have you tested it?  If this goes wrong,
> things break badly...

I've been running it for about a night and a morning now, nothing bad
has happened, my ext2 filesystem shows up clean when forcing a fsck.

If things actually break badly, it is a very serious bug in the
underlying FS. The FS should not 'happen to work' just because the VFS
inadvertedly marked unmodified inodes as being dirty.

Jan

Re: 2.4.2 fs/inode.c

[Chris Mason]

On Thursday, March 22, 2001 07:04:52 PM +0000 "Stephen C. Tweedie"
<sct@redhat.com> wrote:

> Hi,
> 
> On Thu, Mar 22, 2001 at 01:42:15PM -0500, Jan Harkes wrote:
>> 
>> I found some code that seems wrong and didn't even match it's comment.
>> Patch is against 2.4.2, but should go cleanly against 2.4.3-pre6 as well.
>  
> Patch looks fine to me.  Have you tested it?  If this goes wrong,
> things break badly...

This should only affect reiserfs, and it should be a good thing.  I'll do
some tests, thanks for spotting.

> 
>>  		/* Don't do this for I_DIRTY_PAGES - that doesn't actually dirty the
>>  		inode itself */ -		if (flags & (I_DIRTY | I_DIRTY_SYNC)) {
>> +		if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {

-chris

Re: 2.4.2 fs/inode.c

[Chris Mason]

On Thursday, March 22, 2001 01:42:15 PM -0500 Jan Harkes
<jaharkes@cs.cmu.edu> wrote:

> 
> I found some code that seems wrong and didn't even match it's comment.
> Patch is against 2.4.2, but should go cleanly against 2.4.3-pre6 as well.
> 

Ok, this looks correct, makes reiserfs faster, and survived under load.  The
idea was to only call dirty_inode if sync_one might decide the inode needs
flushing to disk.  So, the check in __mark_inode_dirty should look the same
as the check in sync_one.

> --- linux/fs/inode.c.orig	Thu Mar 22 13:20:55 2001
> +++ linux/fs/inode.c	Thu Mar 22 13:21:32 2001
> @@ -133,7 +133,7 @@
>  
> 	 if (sb) {
> 		 /* Don't do this for I_DIRTY_PAGES - that doesn't actually dirty the
> 		 inode itself */ 
> -		if (flags & (I_DIRTY | I_DIRTY_SYNC)) {
> +		if (flags & (I_DIRTY_SYNC | I_DIRTY_DATASYNC)) {
> 			 if (sb->s_op && sb->s_op->dirty_inode)
> 				 sb->s_op->dirty_inode(inode);
> 		 }
> -

-chris