public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
From: Martin Dalecki <dalecki@evision-ventures.com>
To: unlisted-recipients:; (no To-header on input)@localhost.localdomain
Cc: Linux Kernel Mailing List <linux-kernel@vger.kernel.org>
Subject: Deep look into VFS
Date: Wed, 05 Dec 2001 14:11:25 +0100	[thread overview]
Message-ID: <3C0E1CFD.1E2265FB@evision-ventures.com> (raw)
In-Reply-To: <E16BJvR-0002uc-00@the-village.bc.nu> <E16BK7Y-0000Rk-00@starship.berlin>

Yerstoday I had  a look into the virtual VFS. Out of this
the following question araises for me.

Inside fs/inode.c we have a generic clear_inode(). All fine
all well at one palce the usage of this function goes as follows:
(the function in question is iput() from the same file)

	if (op && op->delete_inode) {
		void (*delete)(struct inode *) = op->delete_inode;
		if (!is_bad_inode(inode))
			DQUOT_INIT(inode);
		/* s_op->delete_inode internally recalls clear_inode() */
		delete(inode);
	} else
		clear_inode(inode);

Well my tought was, that it would be nice to avoid
the explicit callback to inode from driver code in the middle for
nowhere, which would allow us to change the above code sequence into
the much cleaner:

	if (op && op->delete_inode) {
		void (*delete)(struct inode *) = op->delete_inode;
		if (!is_bad_inode(inode))
			DQUOT_INIT(inode);
		delete(inode);
	} 
	clear_inode(inode);

Therefore I have looked at all the places, where clear_inode
is actually called inside the FS implementation code.

shmmem() told me that the above change would be entierly fine with it.

We have however the following in ext2/ialloc.c:


/*
 * NOTE! When we get the inode, we're the only people
 * that have access to it, and as such there are no
 * race conditions we have to worry about. The inode
 * is not on the hash-lists, and it cannot be reached
 * through the filesystem because the directory entry
 * has been deleted earlier.
 *
 * HOWEVER: we must make sure that we get no aliases,
 * which means that we have to call "clear_inode()"
 * _before_ we mark the inode not in use in the inode
 * bitmaps. Otherwise a newly created file might use
 * the same inode number (not actually the same pointer
 * though), and then we'd have two inodes sharing the
 * same inode number and space on the harddisk.
 */
void ext2_free_inode (struct inode * inode)
{
...
	
	lock_super (sb);
...
	/* Do this BEFORE marking the inode not in use or returning an error */
	clear_inode (inode);

...
	unlock_super (sb);
}

Unless I'm compleatly misguided the lock on the superblock
should entierly prevent the race described inside the header comment
and we should be able to delete clear_inode from this function.

Question is: Can someone with more knowlendge of the intimidate 
inner workings of the VFS tell me whatever my suspiction is
right or not?

Thanks in advance...

PS. Deleting clear_inode() would help to simplify the
delete_inode parameters quite a significant bit, as
well as deleting the tail union in struct inode - that's the goal.

  parent reply	other threads:[~2001-12-05 13:21 UTC|newest]

Thread overview: 53+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2001-12-03 18:12 Linux/Pro -- clusters Donald Becker
2001-12-04  1:55 ` Davide Libenzi
2001-12-04  2:09   ` Donald Becker
2001-12-04  2:23     ` Davide Libenzi
2001-12-04  2:34       ` Alexander Viro
2001-12-04  9:10     ` Alan Cox
2001-12-04  9:30       ` Thomas Langås
2001-12-04  9:45         ` Alan Cox
2001-12-04 11:34           ` Thomas Langås
2001-12-05 21:57         ` Linus Torvalds
2001-12-05 23:05           ` Andre Hedrick
2001-12-06  4:31             ` Daniel Phillips
2001-12-05 23:49           ` Alan Cox
2001-12-05 23:48             ` Andre Hedrick
2001-12-06 16:58             ` Linus Torvalds
2001-12-06 18:02               ` Alan Cox
2001-12-06 18:07                 ` Linus Torvalds
2001-12-06 18:12                   ` Kai Henningsen
2001-12-06 20:46                     ` Linus Torvalds
2001-12-06 22:40                     ` Alan Cox
2001-12-06 18:33                   ` Alan Cox
2001-12-06 18:55                     ` Linus Torvalds
2001-12-06 19:19                       ` Alan Cox
2001-12-06 20:37                         ` Linus Torvalds
2001-12-06 22:35                           ` Alan Cox
2001-12-06 22:34                             ` Linus Torvalds
2001-12-06 22:58                               ` Alexander Viro
2001-12-07 10:14                     ` Martin Dalecki
2001-12-07 10:37                       ` Alan Cox
2001-12-07 10:56                         ` Martin Dalecki
2001-12-07 12:08                           ` Alan Cox
2001-12-07 20:51                             ` On re-working the major/minor system Erik Andersen
2001-12-07 21:21                               ` H. Peter Anvin
2001-12-07 21:55                                 ` Erik Andersen
2001-12-07 22:04                                   ` H. Peter Anvin
2001-12-07 23:07                                     ` Erik Andersen
2001-12-07 23:12                                       ` H. Peter Anvin
2001-12-08 11:42                                         ` Alan Cox
2001-12-08 20:37                                           ` H. Peter Anvin
2001-12-09 12:06                                   ` Kai Henningsen
2001-12-09 21:57                                     ` H. Peter Anvin
2001-12-11 20:45                                       ` Kai Henningsen
2001-12-06 18:38               ` Linux/Pro -- clusters Doug Ledford
2001-12-04 14:37     ` Daniel Phillips
2001-12-04 15:19       ` Jeff Garzik
2001-12-04 17:16         ` Daniel Phillips
2001-12-04 17:20           ` Jeff Garzik
2001-12-04 18:04           ` Alan Cox
2001-12-04 18:16             ` Daniel Phillips
2001-12-04 20:20               ` Andrew Morton
2001-12-05 13:11               ` Martin Dalecki [this message]
2001-12-05 15:19                 ` Deep look into VFS Alexander Viro
2001-12-05 15:30                   ` Martin Dalecki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=3C0E1CFD.1E2265FB@evision-ventures.com \
    --to=dalecki@evision-ventures.com \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox