inode->i_op->rename semantics .

inode->i_op->rename semantics .

[Nir Tzachar]

hello all.

im having problem with the mentioned semantics.

can someone please clarify what am i supposed to do in the rename
function??

this is what i thought ( im dealing only with regular files, no
directories. ) :

if we'll ignore all the directory structure menagement, it comes to this->

        struct inode *new_inode = new_dentry->inode;
       	if (new_inode){
		new_inode->i_nlink--;
       		mark_inode_dirty(new_inode);
       		dput(new_dentry);
	}
	else { /* do nothing... */ }

is this correct? well, i thought it was, but the new_inode is not deleted
by the vfs as i supposed .

i think if someone could clarify where in the vfs code the new_inode and
the old_inode get switched ( if at all, or am i supposed to do it? ) it
will solve my misunderstanding.

anyway, thank u all.



========================================================================
nir.

Re: inode->i_op->rename semantics .

[viro]

On Tue, May 13, 2003 at 12:29:41PM +0300, Nir Tzachar wrote:

> can someone please clarify what am i supposed to do in the rename
> function??

Er...  rename the damn thing?
 
> if we'll ignore all the directory structure menagement, it comes to this->
> 
>         struct inode *new_inode = new_dentry->inode;
>        	if (new_inode){
> 		new_inode->i_nlink--;
>        		mark_inode_dirty(new_inode);
>        		dput(new_dentry);
			????????????????
> 	}
> 	else { /* do nothing... */ }
> 
> is this correct? well, i thought it was, but the new_inode is not deleted
> by the vfs as i supposed .

dput() is wrong there.
 
> i think if someone could clarify where in the vfs code the new_inode and
> the old_inode get switched ( if at all, or am i supposed to do it? ) it
> will solve my misunderstanding.

d_move() called from fs/namei.c

Re: inode->i_op->rename semantics .

[Nir Tzachar]

> > can someone please clarify what am i supposed to do in the rename
> > function??
>
> Er...rename the damn thing?

duh....

but be a bit serious: all the "renaming" __stuff__ is done at the vfs layer.
so what the specific f/s is expected to implement?



========================================================================
nir.

Re: inode->i_op->rename semantics .

[Nikita Danilov]

Nir Tzachar writes:
 > > > can someone please clarify what am i supposed to do in the rename
 > > > function??
 > >
 > > Er...rename the damn thing?
 > 
 > duh....
 > 
 > but be a bit serious: all the "renaming" __stuff__ is done at the vfs layer.
 > so what the specific f/s is expected to implement?
 > 

Update persistent data. Take a look at fs/ext2/namei.c:ext2_rename()

 > 
 > 
 > ========================================================================
 > nir.
 > 

Nikita.

Re: inode->i_op->rename semantics .

[Charles Manning]

On Wednesday 14 May 2003 03:23, Nir Tzachar wrote:
> > > can someone please clarify what am i supposed to do in the rename
> > > function??
> >
> > Er...rename the damn thing?
>
> duh....
>
> but be a bit serious: all the "renaming" __stuff__ is done at the vfs
> layer. so what the specific f/s is expected to implement?

You should be able to get a good idea by looking at what ext2 does. But shop 
around, I found that for the fs I wrote it often helped to look at a few 
different fs to find something that more closely fitted the way my fs works.
Therefore look also at JFFS2.

-- CHarles

Re: inode->i_op->rename semantics .

[Nir Tzachar]

On Wed, 14 May 2003, Charles Manning wrote:

> > but be a bit serious: all the "renaming" __stuff__ is done at the vfs
> > layer. so what the specific f/s is expected to implement?
>
> You should be able to get a good idea by looking at what ext2 does. But shop
> around, I found that for the fs I wrote it often helped to look at a few
> different fs to find something that more closely fitted the way my fs works.
> Therefore look also at JFFS2.

well, i looked at ext2 b4 sending the mail. what i got from it is the
following concept:

static int
srfs_rename(struct inode *old_dir, struct dentry *old_dentry,
            struct inode *new_dir, struct dentry *new_dentry)
{

	if (S_ISDIR(old_dentry->d_inode->i_mode)
		return -EPERM;
	(psaudo_code:) connect old inode to new dentry in f/s directory structure.
		       remove old dentry from f/s directory structure.
	if (new_dentry->d_inode) {
		new_dentry->d_inode->i_nlink--;
		mark_inode_dirty(new_dentry->d_inode);
	}
	mark_inode_dirty(old_dentry->d_inode);

	return 0;
}


this code works as far as renaming the object, but the new_inode does not
get destroyed... i get the new_dentry to hold old_inode, and new_node (if
existed) will remain on disk...
i cant tell what the problem is. hope someone can help...

thanks.


========================================================================
nir.

Re: inode->i_op->rename semantics .

[viro]

On Wed, May 14, 2003 at 08:54:00AM +0300, Nir Tzachar wrote:
> well, i looked at ext2 b4 sending the mail. what i got from it is the
> following concept:
 
> this code works as far as renaming the object, but the new_inode does not
> get destroyed... i get the new_dentry to hold old_inode, and new_node (if
> existed) will remain on disk...
> i cant tell what the problem is. hope someone can help...

Leave association between dentries and inodes alone; VFS will call
d_move() afterwards and it will do the right thing.

All you need to do in ->rename() is
	a) modify on-disk structures
	b) update ->i_nlink for inodes that need it.
That's it.  You don't need to change ->d_inode for any dentries.

Re: inode->i_op->rename semantics .

[Nir Tzachar]

first, thanks for your help ;)

now, i still haver some problems:

> Leave association between dentries and inodes alone; VFS will call
> d_move() afterwards and it will do the right thing.
im not messing with dentry<->inode associations .
anyway, i tried to read the d_move() code, but some explanations will be 
appreciated. 

> All you need to do in ->rename() is
> 	a) modify on-disk structures
im doing this.

> 	b) update ->i_nlink for inodes that need it.
which inodes need it? wont the vfs update ->i_nlink for them?
if i need to update ->i_nlink, should i also iput/iget them?? or will the 
vfs do this also?? (im asking, because i didnt see this kind of code 
anywhere in the vfs code. )


-- 
========================================================================
nir.

Re: inode->i_op->rename semantics .

[Charles Manning]

On Thursday 15 May 2003 19:46, Nir Tzachar wrote:
> first, thanks for your help ;)
>
> now, i still haver some problems:
> > Leave association between dentries and inodes alone; VFS will call
> > d_move() afterwards and it will do the right thing.
>
> im not messing with dentry<->inode associations .
> anyway, i tried to read the d_move() code, but some explanations will be
> appreciated.
>
> > All you need to do in ->rename() is
> > 	a) modify on-disk structures
>
> im doing this.
>
> > 	b) update ->i_nlink for inodes that need it.
>
> which inodes need it? wont the vfs update ->i_nlink for them?
> if i need to update ->i_nlink, should i also iput/iget them?? or will the
> vfs do this also?? (im asking, because i didnt see this kind of code
> anywhere in the vfs code. )

Nir, don't lose the faith yet. I had a lot of problems in this area too. I 
also found that the VFS interface here could have been a lot cleaner in some 
areas. Got me quite depressed. I'm still no guru so no doubt some experts 
could point to holes in this. Anyway, this code is working and shipping in 
many products.

The inodes refered to here are those where the rename target was already 
existing.

Here's the rename from YAFFS. You will see that it attempts to unlink the 
rename target. If this is true (ie. there was a target) then the nlink is 
adjusted.

-- CHarles

static int yaffs_rename(struct inode * old_dir, struct dentry *old_dentry, 
struct inode * new_dir,struct dentry *new_dentry)
{
	yaffs_Device *dev;
	int retVal = 0;
	int removed = 0;
	yaffs_Object *target;

	dev = yaffs_InodeToObject(old_dir)->myDev;

	yaffs_GrossLock(dev);

	// Check if the target is an existing directory that is not empty.
	target =    
yaffs_FindObjectByName(yaffs_InodeToObject(new_dir),new_dentry->d_name.name);

	if(target &&
	   target->variantType == YAFFS_OBJECT_TYPE_DIRECTORY &&
	   !list_empty(&target->variant.directoryVariant.children))
	{
		retVal = 0;
	}
	else
	{

		// Unlink the target if it exists
		removed = 
yaffs_Unlink(yaffs_InodeToObject(new_dir),new_dentry->d_name.name);


		retVal =  
yaffs_RenameObject(yaffs_InodeToObject(old_dir),old_dentry->d_name.name,
	yaffs_InodeToObject(new_dir),new_dentry->d_name.name);

	}
	yaffs_GrossUnlock(dev);

	if(retVal )
	{
		if(removed )
		{
			new_dentry->d_inode->i_nlink--;
			mark_inode_dirty(new_dentry->d_inode);
		}

		return 0;
	}
	else
	{
		return -ENOTEMPTY;
	}
}

-- CHarles

Re: inode->i_op->rename semantics .

[viro]

On Thu, May 15, 2003 at 10:46:42AM +0300, Nir Tzachar wrote:
> first, thanks for your help ;)
> 
> now, i still haver some problems:
> 
> > Leave association between dentries and inodes alone; VFS will call
> > d_move() afterwards and it will do the right thing.
> im not messing with dentry<->inode associations .
> anyway, i tried to read the d_move() code, but some explanations will be 
> appreciated. 
> 
> > All you need to do in ->rename() is
> > 	a) modify on-disk structures
> im doing this.
> 
> > 	b) update ->i_nlink for inodes that need it.
> which inodes need it? wont the vfs update ->i_nlink for them?

No.  ->i_nlink is the number of (on-disk) directory entries that point to
your (on-disk) inodes.  VFS has no business touching it - it belongs to
filesystem data structures and is responsibility of filesystem code.

In other words, ->i_nlink changes should reflect the changes of directory
tree:
	if target exists, we need to decrement its ->i_nlink since target
directory entry is not pointing to it anymore.
	if we are moving a directory, old parent should have ->i_nlink
decremented and new one - incremented, since '..' in the child now points
to new parent.
	if target was an empty directory, we should also decrement ->i_nlink
on target and target's parent since '.' and '..' in target are goners now.

> if i need to update ->i_nlink, should i also iput/iget them?? or will the 
> vfs do this also?? (im asking, because i didnt see this kind of code 
> anywhere in the vfs code. )

Updates of ->i_nlink have nothing to do with iput/iget.
 
	All right, let's take a look at the way it works.  Suppose we
rename foo/a to bar/a, both exist and are regular files.

	We have two dentries (for foo/a and bar/a resp.) and two inodes.
->rename() will update data structures on disk and decrement ->i_nlink
for target.  Then d_move() is called.  Note that at this point dentries
still reflect the *old* state - they had not been moved/renamed/etc. yet.
d_move() does it.  It will
	* unhash dentry of bar/a
	* exchange the names and parents between dentries of foo/a and bar/a
Now we have the dentry of target unhashed (still with the same inode) and
dentry of source successfully renamed.
Now we drop our references to both dentries.  Since target had been unhashed,
it will be freed as soon as the last reference is dropped.  Freeing it will
drop the reference to its inode (i.e. inode that used to be bar/a).
_If_ there was no other links to that inode, iput() will see that it's time
to delete it and call filesystem ->delete_inode().

	Note that we can't do anything about that sequence of operations.
Indeed, if somebody had bar/a opened, we have to keep it alive until it's
finally closed.  It is detached from bar, no lookups will find it, but we
can't free inode yet.  IOW, ->rename() itself shouldn't free any inodes -
it's left to VFS code.  Normally it happens when rename(2) does dput()
on dentries it had dealt with, but it can happen later if somebody is
also holding them.

	It might be easier if you had shown your code, actually.  Are you
sure that you do not leak references to dentries somewhere?  That would
explain the things - inode removal is triggered by releasing the last
reference to dentry and if that never happens...

Re: inode->i_op->rename semantics .

[Nir Tzachar]

hi AV.

thank you for ur help, it solved my problems,
and thanks for clarifying some of the vfs code....
( i rather wondered what d_move actually does. )

say, isnt it in order to publish some kind of a guide (more thorough than
the one in Documentation and anything i could find on the web + books... )
to the vfs? even explaining execution paths will do....
i think this kind of document, coming from the maintainer of
the vfs himself, will save us all much headaches......

thanks again for ur help ;)

========================================================================
nir.

Re: inode->i_op->rename semantics .

[David Chow]

Nir Tzachar wrote:

>hi AV.
>
>thank you for ur help, it solved my problems,
>and thanks for clarifying some of the vfs code....
>( i rather wondered what d_move actually does. )
>
>say, isnt it in order to publish some kind of a guide (more thorough than
>the one in Documentation and anything i could find on the web + books... )
>
Yes, I do have some notes on all i_ops, f_ops, a_ops for my own use 
(some still in my head). May be this should be further extend to form a 
book. Since the vfs.txt in the kernel doc written by Richard is pretty 
out-of-date and didn't have any examples. Especially for semantics from 
the Linux VFS. I thought of doing this for 2.4 but struggling for 2.5 as 
it worked a bit different in some cases.

>to the vfs? even explaining execution paths will do....
>i think this kind of document, coming from the maintainer of
>the vfs himself, will save us all much headaches......
>
>  
>
Al, I think you've been answering too many questions that I think it can 
be solved with an update-to-date simple doc. Do you have any plan on 
documenting the VFS api properly from i_ops, f_ops, a_ops and some 
detail explanation of those calls? Maybe something about 2.4 is quite 
good enough for a lot of people. If you have something, please do tell 
me so that I can collect them.

regards,
David Chow