[sysfs] why ->delete_inode() instead of ->clear_inode()?

[sysfs] why ->delete_inode() instead of ->clear_inode()?

[Al Viro]

	Is there any problem with simply doing

void sysfs_clear_inode(struct inode *inode)
{
	struct sysfs_dirent *sd  = inode->i_private;
	sysfs_put(sd);
}

instead of (mis)using ->delete_inode()?  Note that clear_inode() is
not going to be called on any inodes with positive i_count, that
you are forcibly evicting them from icache on the final iput() anyway
and that clear_inode() is never called twice on the same struct
inode()...

Al, very unenthusiastic about ->delete_inode() instances in general - that
sucker gets abused in the strangest ways by a lot of filesystems ;-/

Re: [sysfs] why ->delete_inode() instead of ->clear_inode()?

[Eric W. Biederman]

Al Viro <viro@ZenIV.linux.org.uk> writes:

> 	Is there any problem with simply doing
>
> void sysfs_clear_inode(struct inode *inode)
> {
> 	struct sysfs_dirent *sd  = inode->i_private;
> 	sysfs_put(sd);
> }

I can't see any problems with a quick review.

Given that drop_inode calls generic_delete_inode I don't see that
changing that will make any difference in the case of sysfs.

I can't remember why I did it that way but I can say nothing terribly subtle
is going on.  We need a reference to the sysfs_dirent from the inode and when
the inode goes away we need to clear the reference.

>From the look of things proc_delete_inode can also be changed to a clear_inode
method if that is preferred.

Eric

Re: [sysfs] why ->delete_inode() instead of ->clear_inode()?

[Al Viro]

On Sat, May 29, 2010 at 12:06:34AM -0700, Eric W. Biederman wrote:

> >From the look of things proc_delete_inode can also be changed to a clear_inode
> method if that is preferred.

Already that way in the local tree, along with quite a few other filesystems...

Re: [sysfs] why ->delete_inode() instead of ->clear_inode()?

[Eric W. Biederman]

Al Viro <viro@ZenIV.linux.org.uk> writes:

> On Sat, May 29, 2010 at 12:06:34AM -0700, Eric W. Biederman wrote:
>
>> >From the look of things proc_delete_inode can also be changed to a clear_inode
>> method if that is preferred.
>
> Already that way in the local tree, along with quite a few other filesystems...

It has been my goal, to get sysfs looking roughly like other filesystems
in the kernel.  I'm glad to see that sysfs is finally looking enough
like other filesystems it can participate in global cleanups.

Eric