Skip to contentSkip to conversation

[2.4] Reiserfs changeset 2 out of 4, please apply.

BK Hans Reiser 3 participants 5 messages May 7, 2002

lore
Share & tools
lore
#0 lore raw

Hello!

You can get this changeset from bk://thebsh.namesys.com/bk/reiser3-linux-2.4

This changeset are cleaning up reiserfscode, removes stale comments, and rewrites some "borrowed" functions so that all of the code in reiserfs subdir should now only belong to NAMESYS.

Diffstat:

 fs/reiserfs/bitmap.c        |    5 
 fs/reiserfs/dir.c           |    4 
 fs/reiserfs/inode.c         |   37 ----
 fs/reiserfs/namei.c         |  184 ++++++------------------
 fs/reiserfs/super.c         |  326 ++++++++++++++++++++++++++------------------
 include/linux/reiserfs_fs.h |    4 
 6 files changed, 250 insertions(+), 310 deletions(-)

Plain test patch:
# This is a BitKeeper generated patch for the following project:
# Project Name: Linux kernel tree
# This patch format is intended for GNU patch command version 2.5 or higher. # This patch includes the following deltas:
# ChangeSet 1.383.2.38 -> 1.383.2.39

#	   fs/reiserfs/dir.c	1.11    -> 1.12   
#	 fs/reiserfs/namei.c	1.19    -> 1.20   
#	 fs/reiserfs/inode.c	1.32    -> 1.33   

# include/linux/reiserfs_fs.h 1.16 -> 1.17
# fs/reiserfs/super.c 1.19 -> 1.20
# fs/reiserfs/bitmap.c 1.12 -> 1.13
#
# The following is the BitKeeper ChangeSet Log
# --------------------------------------------
# 02/04/30 [email protected] 1.383.2.39
# reiserfs_fs.h, super.c, namei.c, inode.c, dir.c, bitmap.c:

#   Here is a patch which remakes in original manner last bits in reiserfs
#   code which looked like created by copy-n-paste-ing from other sources.
#   Most of them are removed comments, some are adding intermidiate
#   variables. The only one interesting change is new
#   reiserfs_parse_options.

# --------------------------------------------
#

fs/reiserfs/bitmap.c +1 -4
diff -Nru a/fs/reiserfs/bitmap.c b/fs/reiserfs/bitmap.c
@@ -663,10 +663,7 @@
   return ret;
 }
 
-//
-// a portion of this function, was derived from minix or ext2's
-// analog. You should be able to tell which portion by looking at the
-// ext2 code and comparing. 
+
 static void __discard_prealloc (struct reiserfs_transaction_handle * th,
 				struct inode * inode)
 {
fs/reiserfs/dir.c +2 -2
diff -Nru a/fs/reiserfs/dir.c b/fs/reiserfs/dir.c
@@ -106,7 +106,7 @@
 		if (!d_name[d_reclen - 1])
 		    d_reclen = strlen (d_name);
 	
-		if (d_reclen > REISERFS_MAX_NAME_LEN(inode->i_sb->s_blocksize)){
+		if (d_reclen > REISERFS_MAX_NAME(inode->i_sb->s_blocksize)){
 		    /* too big to send back to VFS */
 		    continue ;
 		}
@@ -177,7 +177,7 @@
 
 
  end:
-    // FIXME: ext2_readdir does not reset f_pos
+
     filp->f_pos = next_pos;
     pathrelse (&path_to_entry);
     reiserfs_check_path(&path_to_entry) ;
fs/reiserfs/inode.c +4 -33
diff -Nru a/fs/reiserfs/inode.c b/fs/reiserfs/inode.c
@@ -19,10 +19,7 @@
 
 static int reiserfs_get_block (struct inode * inode, long block,
 			       struct buffer_head * bh_result, int create);
-//
-// initially this function was derived from minix or ext2's analog and
-// evolved as the prototype did
-//
+
 void reiserfs_delete_inode (struct inode * inode)
 {
     int jbegin_count = JOURNAL_PER_BALANCE_CNT * 2; 
@@ -111,8 +108,7 @@
 }
 
 //
-// FIXME: we might cache recently accessed indirect item (or at least
-// first 15 pointers just like ext2 does
+// FIXME: we might cache recently accessed indirect item
 
 // Ugh.  Not too eager for that....
 //  I cut the code until such time as I see a convincing argument (benchmark).
@@ -543,12 +539,7 @@
 #endif
     return reiserfs_new_unf_blocknrs (th, allocated_block_nr, tag);
 }
-//
-// initially this function was derived from ext2's analog and evolved
-// as the prototype did.  You'll need to look at the ext2 version to
-// determine which parts are derivative, if any, understanding that
-// there are only so many ways to code to a given interface.
-//
+
 static int reiserfs_get_block (struct inode * inode, long block,
 			       struct buffer_head * bh_result, int create)
 {
@@ -1142,11 +1133,6 @@
 }
 
 
-//
-// initially this function was derived from minix or ext2's analog and
-// evolved as the prototype did
-//
-
 /* looks for stat data in the tree, and fills up the fields of in-core
    inode stat data fields */
 void reiserfs_read_inode2 (struct inode * inode, void *p)
@@ -1368,10 +1354,6 @@
 }
 
 
-//
-// initially this function was derived from minix or ext2's analog and
-// evolved as the prototype did
-//
 /* looks for stat data, then copies fields to it, marks the buffer
    containing stat data as dirty */
 /* reiserfs inodes are never really dirty, since the dirty inode call
@@ -2042,18 +2024,13 @@
     return error ;
 }
 
-//
-// this is exactly what 2.3.99-pre9's ext2_readpage is
-//
+
 static int reiserfs_readpage (struct file *f, struct page * page)
 {
     return block_read_full_page (page, reiserfs_get_block);
 }
 
 
-//
-// modified from ext2_writepage is
-//
 static int reiserfs_writepage (struct page * page)
 {
     struct inode *inode = page->mapping->host ;
@@ -2062,9 +2039,6 @@
 }
 
 
-//
-// from ext2_prepare_write, but modified
-//
 int reiserfs_prepare_write(struct file *f, struct page *page, 
 			   unsigned from, unsigned to) {
     struct inode *inode = page->mapping->host ;
@@ -2074,9 +2048,6 @@
 }
 
 
-//
-// this is exactly what 2.3.99-pre9's ext2_bmap is
-//
 static int reiserfs_aop_bmap(struct address_space *as, long block) {
   return generic_block_bmap(as, block, reiserfs_bmap) ;
 }
fs/reiserfs/namei.c +49 -135

426 lines not on this page · raw message

fs/reiserfs/super.c +193 -133

423 lines not on this page · raw message

include/linux/reiserfs_fs.h +1 -3
diff -Nru a/include/linux/reiserfs_fs.h b/include/linux/reiserfs_fs.h
@@ -953,9 +953,7 @@
 #define B_I_E_NAME(bh,ih,entry_num) ((char *)(bh->b_data + ih_location(ih) + deh_location(B_I_DEH(bh,ih)+(entry_num))))
 
 // two entries per block (at least)
-//#define REISERFS_MAX_NAME_LEN(block_size) 
-//((block_size - BLKH_SIZE - IH_SIZE - DEH_SIZE * 2) / 2)
-#define REISERFS_MAX_NAME_LEN(block_size) 255
+#define REISERFS_MAX_NAME(block_size) 255
 
 
 /* this structure is used for operations on directory entries. It is
#1 lore raw
Quoted 7 lines · Hans Reiser

Hello!

You can get this changeset from bk://thebsh.namesys.com/bk/reiser3-linux-2.4

This changeset are cleaning up reiserfscode, removes stale comments, and rewrites some "borrowed" functions so that all of the code in reiserfs subdir should now only belong to NAMESYS.

It is the end of a release cycle on a stable kernel with huge changes to the IDE layer, and we have at least one unconfirmed report of problems with reiserfs+IDE after a crash.

This is not the right time to send in cleanups like this, especially when they bits as useless as the stuff below. #1, #2 and #4 look like valid fixes. #3 should probably be mixed with the iput deadlock fix like Oleg did in 2.5, and should wait until after 2.4.19.

-chris

@@ -957,6 +875,7 @@

     int windex ;
     struct reiserfs_transaction_handle th ;
     int jbegin_count = JOURNAL_PER_BALANCE_CNT * 3; 

+ time_t ctime;

     if (S_ISDIR(inode->i_mode))
@@ -984,7 +903,8 @@
     }
 
     inode->i_nlink++;
-    inode->i_ctime = CURRENT_TIME;
+    ctime = CURRENT_TIME;
+    inode->i_ctime = ctime;
     reiserfs_update_sd (&th, inode);
 
     atomic_inc(&inode->i_count) ;

@@ -1037,14 +957,6 @@
}

#2 lore raw
In reply to Hans Reiser

Hello!

Quoted 7 lines · Chris Mason
Quoted 4 lines

You can get this changeset from bk://thebsh.namesys.com/bk/reiser3-linux-2.4 This changeset are cleaning up reiserfscode, removes stale comments, and rewrites some "borrowed" functions so that all of the code in reiserfs subdir should now only belong to NAMESYS.

It is the end of a release cycle on a stable kernel with huge changes to the IDE layer, and we have at least one unconfirmed report of problems with reiserfs+IDE after a crash.

That's true.

Quoted 4 lines

This is not the right time to send in cleanups like this, especially when they bits as useless as the stuff below. #1, #2 and #4 look like valid fixes. #3 should probably be mixed with the iput deadlock fix like Oleg did in 2.5, and should wait until after 2.4.19.

#2 and $4 are cleanups, #1 and #3 are bugfixes.
And iput deadlock fix is too big of a change for 2.4.19, so it is not included. Let's see how will it behave in 2.5 first.
And cleanups are harmless ones, so there is no risk of getting these in.

In short, these changes are not "huge", and mostly non-intrusive.

Bye,
Oleg

#3 lore raw
In reply to Hans Reiser
Quoted 28 lines · Oleg Drokin

Hello!

Quoted 11 lines · Chris Mason
Quoted 6 lines

You can get this changeset from bk://thebsh.namesys.com/bk/reiser3-linux-2.4 This changeset are cleaning up reiserfscode, removes stale comments, and rewrites some "borrowed" functions so that all of the code in reiserfs subdir should now only belong to NAMESYS.

It is the end of a release cycle on a stable kernel with huge changes to the IDE layer, and we have at least one unconfirmed report of problems with reiserfs+IDE after a crash.

That's true.

Quoted 6 lines

This is not the right time to send in cleanups like this, especially when they bits as useless as the stuff below. #1, #2 and #4 look like valid fixes. #3 should probably be mixed with the iput deadlock fix like Oleg did in 2.5, and should wait until after 2.4.19.

Quoted 7 lines

#2 and $4 are cleanups, #1 and #3 are bugfixes.
And iput deadlock fix is too big of a change for 2.4.19, so it is not included. Let's see how will it behave in 2.5 first.
And cleanups are harmless ones, so there is no risk of getting these in.

In short, these changes are not "huge", and mostly non-intrusive.

Chris, I had much the same reaction initially, and then I looked at the details, and either it fixes a real bug in a simple manner, or it is a comment change, etc. It looks bigger than it is was what I finally concluded. Perhaps there is some detail in which I am wrong, but since it was all tested together I didn't feel like picking out just a few lines of change to leave out (since that would actually increase the risk of introducing a bug).

Hans

#4 lore raw
Quoted 9 lines · Hans Reiser
Quoted 2 lines

In short, these changes are not "huge", and mostly non-intrusive.

Chris, I had much the same reaction initially, and then I looked at the details, and either it fixes a real bug in a simple manner, or it is a comment change, etc. It looks bigger than it is was what I finally concluded. Perhaps there is some detail in which I am wrong, but since it was all tested together I didn't feel like picking out just a few lines of change to leave out (since that would actually increase the risk of introducing a bug).

I think it is very important to only include critical fixes at this stage in the release cycle, especially in a kernel where we are actively looking for a possible bug.

I also think that patch (#3) should be integrated in 2.4.20preX, along with the iput deadlock fix like Oleg did for 2.5. It will clean the code with fewer silly changes, and let us avoid rediffing the iput deadlock fix against the new stuff.

-chris