Home » Mailing list archives » tux3 » 2008 » December » 7

Re: [Tux3] [PATCH] rename and rmdir

Previous thread: [Tux3] Hello world! by Daniel Phillips on Saturday, August 23, 2008 - 4:53 pm. (4 messages)

Next thread: [Tux3] Time fields in Tux3 by Daniel Phillips on Monday, August 25, 2008 - 2:51 pm. (20 messages)
[view original message]
From: Michael Pattrick
Subject: [Tux3] [PATCH] rename and rmdir
Date: Saturday, December 6, 2008 - 8:53 pm

Inspired by Pranith, I thought I would try to add to the directory
functions in namei.c. Attached is a patch for rename and rmdir, I cant
really vouch for the 'correctness' of it, but I tested it out for a
while in UML and it seems to work correctly.

This is really my first patch for low level file system operations, so
I won't be offended if you point out any flaws.

Cheers,
Michael Pattrick

[ message continues ]
[view original message]
From: Daniel Phillips
Subject: [Tux3] Inode attributes
Date: Sunday, August 24, 2008 - 5:27 am

Inode attributes are central to Tux3.  Not only do they carry the usual
data needed to implement Posix semantics, they are a key part of the
Tux3 versioning scheme.

Tux3 departs from the usual idea of fixed size inodes on disk, and
instead represents an inode as a linear list of attributes packed
together end to end, as many as are needed to represent the inode.
This representation is forced by Tux3's versioning scheme: all
versions of all attributes of the inode are represented contiguously
in one region of the inode table tree.  This is necessary in order to
for version inheritance to work efficiently (notwithstanding plans to 
break this version blob up into independent subtrees later in Tux3's
life to scale up to very large numbers of versions).

It is possible for an inode to grow quite large if some attribute, say
the file size, changes in every version and a large number of versions
are retained.  The intial Tux3 code will support 512 versions (10 bits
allocated for version label = 1024 versions, but need to allow for half
of these as ghosts, see the versioned pointer post).  If a file grows
continuously its size will be different in every version, so we might
well have to store 512 different size attributes for some files.  So
size attributes better be small.  (Hmm, why don't we delta encode them
then, the versioning algorithm inherently identifies the historical
chain anyway.  Good idea, let's make a note to do that later.)

Small in this case translates into 16 bytes per attribute.  We need
60 bits for the file size (1 exabyte with byte granularity) and also
need to store the mtime every time it changes, so we put it into the
same attribute.  For every kind of inode attribute, we need to store
the version, 10 bits, and a 4 bit tag to identify the kind of attribute
when we scan through the list of attributes for the inode.  48 bits
worth of mtime should be enough, which all adds up to a couple bits
short of 16 bytes.

A digression: up till now I have been playing ...
[ message continues ]
[view original message]
From: Michael Pattrick
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Saturday, December 6, 2008 - 9:35 pm

The previous patch didn't include my changes to tux3.h, here is a correction

Cheers,
Michael Pattrick

On Sat, Dec 6, 2008 at 10:53 PM, Michael Pattrick

[ message continues ]
[view original message]
From: Daniel Phillips
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Saturday, December 6, 2008 - 11:06 pm

Hi Michael,

Welcome, and congratulations for getting something working at all in a
low level filesystem on your first try!

Well, tux3_rename and tux3_rmdir look plausible, but tux_add_link is a
cut and paste of tux_create_entry.  You should be able to write it as a
call to the original tux_create_entry.

Regards,

Daniel

_______________________________________________
Tux3 mailing list
Tux3@tux3.org
http://mailman.tux3.org/cgi-bin/mailman/listinfo/tux3

[ message continues ]
[view original message]
From: Michael Pattrick
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Sunday, December 7, 2008 - 11:51 am

I see what you mean. Attached is another patch with less code duplication.

Cheers,
Michael Pattrick

[ message continues ]
[view original message]
From: Daniel Phillips
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Sunday, December 7, 2008 - 4:49 pm

Hi Pattrick,

It is much easier to see what is going on now!

First, trivial things.  The convention for source style in Tux3 is
"lindent":

   cat kernel/namei.c | /src/2.6.26.5.tux3/scripts/Lindent >namei.c.lindented

Then cut just your function back into the file.  Specifically:

   - Spaces before and after binary operators like " = " and keywords
     like "else".

   - No space between a function name and parameter list

Real issues.  This should only be done if the destination is a
directory:

+		err = -ENOTEMPTY;
+		if (!tux_dir_is_empty (new_inode))
+			goto out;

The inc_link_counts and dec_link_counts have to balance for both old
and new inode (which may be the same - the rename may be either to an
existing name or a new name).  Ext2 adds the new entry before it
deletes the old one, in case there is an error in the add.  Your
code will leave the inode with no link in that case, probably not a
good idea.

To tell the truth, I do not know what the link incs and decs in the
Ext2 rename are supposed to do,  Maybe somebody can clear this up?

Regards,

Daniel

_______________________________________________
Tux3 mailing list
Tux3@tux3.org
http://mailman.tux3.org/cgi-bin/mailman/listinfo/tux3

[ message continues ]
[view original message]
From: Daniel Phillips
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Monday, December 8, 2008 - 3:35 am

I lindented it and checked it in, complete with warts :-)

You are cordially invited to help fix it.  Fire up your irc client and
drop in on irc.oftc.net, #tux3 if you like.

Regards,

Daniel

_______________________________________________
Tux3 mailing list
Tux3@tux3.org
http://mailman.tux3.org/cgi-bin/mailman/listinfo/tux3

[ message continues ]
[view original message]
From: Pranith Kumar
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Monday, December 8, 2008 - 6:44 am

Hello,

Sorry to hijack this but I cleaned it up for rmdir.

Regards,
Pranith.

--- user/kernel/namei.c.orig	2008-12-08 18:26:26.186324000 +0530
+++ user/kernel/namei.c	2008-12-08 18:55:13.633735000 +0530
@@ -31,6 +31,19 @@ static int tux_add_dirent(struct inode *
 	return 0;
 }

+static int tux_del_dirent(struct inode *dir, struct dentry *dentry)
+{
+	struct buffer_head *buffer;
+	tux_dirent *entry;
+	int err = -ENOENT;
+
+	entry = tux_find_entry(dir, dentry->d_name.name, dentry->d_name.len,
+			       &buffer);
+	if (entry)
+		err = tux_delete_entry(buffer, entry);
+	return err;
+}
+
 static int tux3_create(struct inode *dir, struct dentry *dentry, int mode,
 		       struct nameidata *nd)
 {
@@ -98,19 +111,11 @@ static int tux3_symlink(struct inode *di
 static int tux3_unlink(struct inode *dir, struct dentry *dentry)
 {
 	struct inode *inode = dentry->d_inode;
-	struct buffer_head *buffer;
-	tux_dirent *entry;
-	int err = -ENOENT;
+	int err = tux_del_dirent(dir, dentry);

-	entry = tux_find_entry(dir, dentry->d_name.name, dentry->d_name.len,
-			       &buffer);
-	if (entry) {
-		err = tux_delete_entry(buffer, entry);
-		if (!err) {
-			inode->i_ctime = dir->i_ctime;
-			inode_dec_link_count(inode);
-			err = 0;
-		}
+	if (!err) {
+		inode->i_ctime = dir->i_ctime;
+		inode_dec_link_count(inode);
 	}
 	return err;
 }
@@ -173,13 +178,13 @@ static int tux3_rmdir(struct inode *dir,
 {
 	struct inode *inode = dentry->d_inode;
 	int err = -ENOTEMPTY;
-	struct buffer_head *buffer;
-	tux_dirent *de;
+
 	if (tux_dir_is_empty(inode)) {
-		de = tux_find_entry(dir, dentry->d_name.name, dentry->d_name.len, &buffer);
-		err = tux_delete_entry(buffer, de);
+
+		err = tux_del_dirent(dir, dentry);

 		if (!err) {
+			inode->i_ctime = dir->i_ctime;
 			inode->i_size = 0;
 			inode_dec_link_count(inode);
 			inode_dec_link_count(dir);

_______________________________________________
Tux3 mailing ...
[ message continues ]
[view original message]
From: Daniel Phillips
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Monday, December 8, 2008 - 2:58 pm

Nicely done!

Note: the 80 column rule for lindent was relaxed some time ago, so it
is ok to let lines go a little over if that improves readability.  I
followed up with a minor edit.

I converted tux_find_entry to use the ERR_PTR interface as an example
of how to do this.  We should use ERR_PTR more widely, particularly
in the blockget/blockread interface.  While doing this I spotted an
unhandled blockread error in tux_find_entry.

Regards,

Daniel

_______________________________________________
Tux3 mailing list
Tux3@tux3.org
http://mailman.tux3.org/cgi-bin/mailman/listinfo/tux3

[ message continues ]
[view original message]
From: Daniel Phillips
Subject: Re: [Tux3] [PATCH] rename and rmdir
Date: Monday, December 8, 2008 - 3:04 pm

...and here is the patch:

   http://hg.tux3.org/tux3?cs=6d49f259c4ac

Regards,

Daniel

_______________________________________________
Tux3 mailing list
Tux3@tux3.org
http://mailman.tux3.org/cgi-bin/mailman/listinfo/tux3

[ message continues ]
Previous thread: [Tux3] Hello world! by Daniel Phillips on Saturday, August 23, 2008 - 4:53 pm. (4 messages)

Next thread: [Tux3] Time fields in Tux3 by Daniel Phillips on Monday, August 25, 2008 - 2:51 pm. (20 messages)