diff mbox

ext3: sreadahead hooks

Message ID 20081014101735.04107779@infradead.org
State Not Applicable, archived
Headers show

Commit Message

Arjan van de Ven Oct. 14, 2008, 2:17 p.m. UTC 
From 3d7a0ca0ee8a755251251bd9ddca0866c25acdc2 Mon Sep 17 00:00:00 2001
From: Arjan van de Ven <arjan@linux.intel.com>
Date: Tue, 14 Oct 2008 10:12:08 -0400
Subject: [PATCH] ext3: sreadahead hooks

The sreadahead program, used to make the OS boot faster, needs to know in
the approximate order in files are used during the boot process. This patch
adds the ext3 hook for this functionality, basically it stores "jiffies"
into the inode at allocation time, and exposes it via an EXT3 ioctl (yes I
know but ioctl seems fitting for this).

Signed-off-by: Arjan van de Ven <arjan@linux.intel.com>
---
 fs/ext3/ioctl.c           |    3 +++
 fs/ext3/super.c           |    1 +
 include/linux/ext3_fs.h   |    1 +
 include/linux/ext3_fs_i.h |    2 ++
 4 files changed, 7 insertions(+), 0 deletions(-)

Comments

Christoph Hellwig Oct. 14, 2008, 3:13 p.m. UTC | #1 
On Tue, Oct 14, 2008 at 10:17:35AM -0400, Arjan van de Ven wrote:
> 
> >From 3d7a0ca0ee8a755251251bd9ddca0866c25acdc2 Mon Sep 17 00:00:00 2001
> From: Arjan van de Ven <arjan@linux.intel.com>
> Date: Tue, 14 Oct 2008 10:12:08 -0400
> Subject: [PATCH] ext3: sreadahead hooks
> 
> The sreadahead program, used to make the OS boot faster, needs to know in
> the approximate order in files are used during the boot process. This patch
> adds the ext3 hook for this functionality, basically it stores "jiffies"
> into the inode at allocation time, and exposes it via an EXT3 ioctl (yes I
> know but ioctl seems fitting for this).

Even if it's an ioctl there's absolutely no point in making this
fileystem specific.  Also the name is rather dumb and non-descriptive.

--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Oct. 14, 2008, 3:51 p.m. UTC | #2 
Christoph Hellwig wrote:
> On Tue, Oct 14, 2008 at 10:17:35AM -0400, Arjan van de Ven wrote:
>> >From 3d7a0ca0ee8a755251251bd9ddca0866c25acdc2 Mon Sep 17 00:00:00 2001
>> From: Arjan van de Ven <arjan@linux.intel.com>
>> Date: Tue, 14 Oct 2008 10:12:08 -0400
>> Subject: [PATCH] ext3: sreadahead hooks
>>
>> The sreadahead program, used to make the OS boot faster, needs to know in
>> the approximate order in files are used during the boot process. This patch
>> adds the ext3 hook for this functionality, basically it stores "jiffies"
>> into the inode at allocation time, and exposes it via an EXT3 ioctl (yes I
>> know but ioctl seems fitting for this).
> 
> Even if it's an ioctl there's absolutely no point in making this
> fileystem specific.  Also the name is rather dumb and non-descriptive.

I have to agree, both the ioctl name and the new field are not very
descriptive - created_when sounds an awful lot like ctime but it's not.

and INODE_JIFFIES really doesn't mean anything at all w/o extra context.
 But I'm trying to think of some nice names.  :)

What about making a new struct inode field and doing this update in
new_inode(), and making it a generic ioctl.  Are we ready to go that far?

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Eric Sandeen Oct. 14, 2008, 9:49 p.m. UTC | #3 
Eric Sandeen wrote:
> Christoph Hellwig wrote:
>> On Tue, Oct 14, 2008 at 10:17:35AM -0400, Arjan van de Ven wrote:
>>> >From 3d7a0ca0ee8a755251251bd9ddca0866c25acdc2 Mon Sep 17 00:00:00 2001
>>> From: Arjan van de Ven <arjan@linux.intel.com>
>>> Date: Tue, 14 Oct 2008 10:12:08 -0400
>>> Subject: [PATCH] ext3: sreadahead hooks
>>>
>>> The sreadahead program, used to make the OS boot faster, needs to know in
>>> the approximate order in files are used during the boot process. This patch
>>> adds the ext3 hook for this functionality, basically it stores "jiffies"
>>> into the inode at allocation time, and exposes it via an EXT3 ioctl (yes I
>>> know but ioctl seems fitting for this).
>> Even if it's an ioctl there's absolutely no point in making this
>> fileystem specific.  Also the name is rather dumb and non-descriptive.
> 
> I have to agree, both the ioctl name and the new field are not very
> descriptive - created_when sounds an awful lot like ctime but it's not.
> 
> and INODE_JIFFIES really doesn't mean anything at all w/o extra context.
>  But I'm trying to think of some nice names.  :)
> 
> What about making a new struct inode field and doing this update in
> new_inode(), and making it a generic ioctl.  Are we ready to go that far?

Or, as I thought about/mentioned to hch, and I guess he and Arjan
already discussed... :)  why not just use tracing infrastructure to get
this info, rather than adding new members to every inode on the system?

-Eric
--
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Karel Zak Oct. 19, 2008, 9:42 p.m. UTC | #4 
On Tue, Oct 14, 2008 at 04:49:51PM -0500, Eric Sandeen wrote:
> Eric Sandeen wrote:
> > Christoph Hellwig wrote:
> >> On Tue, Oct 14, 2008 at 10:17:35AM -0400, Arjan van de Ven wrote:
> >>> >From 3d7a0ca0ee8a755251251bd9ddca0866c25acdc2 Mon Sep 17 00:00:00 2001
> >>> From: Arjan van de Ven <arjan@linux.intel.com>
> >>> Date: Tue, 14 Oct 2008 10:12:08 -0400
> >>> Subject: [PATCH] ext3: sreadahead hooks
> >>>
> >>> The sreadahead program, used to make the OS boot faster, needs to know in
> >>> the approximate order in files are used during the boot process. This patch
> >>> adds the ext3 hook for this functionality, basically it stores "jiffies"
> >>> into the inode at allocation time, and exposes it via an EXT3 ioctl (yes I
> >>> know but ioctl seems fitting for this).
> >> Even if it's an ioctl there's absolutely no point in making this
> >> fileystem specific.  Also the name is rather dumb and non-descriptive.
> > 
> > I have to agree, both the ioctl name and the new field are not very
> > descriptive - created_when sounds an awful lot like ctime but it's not.
> > 
> > and INODE_JIFFIES really doesn't mean anything at all w/o extra context.
> >  But I'm trying to think of some nice names.  :)
> > 
> > What about making a new struct inode field and doing this update in
> > new_inode(), and making it a generic ioctl.  Are we ready to go that far?
> 
> Or, as I thought about/mentioned to hch, and I guess he and Arjan
> already discussed... :)  why not just use tracing infrastructure to get

 What do you mean by "tracing infrastructure"? Audit?

 I had the same question (especially when I'm an author of the
 readahead-collector that uses the audit subsystem for this task :-).
 The problem is that audit stuff is difficult to use (you have to
 disable auditd, and it's not simple to integrate it to the
 distribution).

 Arjan's solution is extremely simple and useful without invasive
 changes in distributions.

> this info, rather than adding new members to every inode on the system?

 #ifdef CONFIG_INODE_JIFFIES

 ('n' for servers which don't need to boot in 5 secs ;-)

    Karel
Arjan van de Ven Oct. 20, 2008, 3:51 a.m. UTC | #5 
On Sun, 19 Oct 2008 23:42:05 +0200
Karel Zak <kzak@redhat.com> wrote:

> On Tue, Oct 14, 2008 at 04:49:51PM -0500, Eric Sandeen wrote:
> > Eric Sandeen wrote:
> > > Christoph Hellwig wrote:
> > >> On Tue, Oct 14, 2008 at 10:17:35AM -0400, Arjan van de Ven wrote:
> > >>> >From 3d7a0ca0ee8a755251251bd9ddca0866c25acdc2 Mon Sep 17
> > >>> >00:00:00 2001
> > >>> From: Arjan van de Ven <arjan@linux.intel.com>
> > >>> Date: Tue, 14 Oct 2008 10:12:08 -0400
> > >>> Subject: [PATCH] ext3: sreadahead hooks
> > >>>
> > >>> The sreadahead program, used to make the OS boot faster, needs
> > >>> to know in the approximate order in files are used during the
> > >>> boot process. This patch adds the ext3 hook for this
> > >>> functionality, basically it stores "jiffies" into the inode at
> > >>> allocation time, and exposes it via an EXT3 ioctl (yes I know
> > >>> but ioctl seems fitting for this).
> > >> Even if it's an ioctl there's absolutely no point in making this
> > >> fileystem specific.  Also the name is rather dumb and
> > >> non-descriptive.
> > > 
> > > I have to agree, both the ioctl name and the new field are not
> > > very descriptive - created_when sounds an awful lot like ctime
> > > but it's not.
> > > 
> > > and INODE_JIFFIES really doesn't mean anything at all w/o extra
> > > context. But I'm trying to think of some nice names.  :)
> > > 
> > > What about making a new struct inode field and doing this update
> > > in new_inode(), and making it a generic ioctl.  Are we ready to
> > > go that far?
> > 
> > Or, as I thought about/mentioned to hch, and I guess he and Arjan
> > already discussed... :)  why not just use tracing infrastructure to
> > get
> 
>  What do you mean by "tracing infrastructure"? Audit?

ftrace

I'll be looking into making that one work soon; it'll be... interesting
and more complex than this patch, but if that's what it takes...
diff mbox

Patch

diff --git a/fs/ext3/ioctl.c b/fs/ext3/ioctl.c
index 0d0c701..b196f71 100644
--- a/fs/ext3/ioctl.c
+++ b/fs/ext3/ioctl.c
@@ -286,6 +286,9 @@  group_add_out:
 		mnt_drop_write(filp->f_path.mnt);
 		return err;
 	}
+	case EXT3_IOC_INODE_JIFFIES: {
+		return ei->created_when;
+	}
 
 
 	default:
diff --git a/fs/ext3/super.c b/fs/ext3/super.c
index f38a5af..c2c83dd 100644
--- a/fs/ext3/super.c
+++ b/fs/ext3/super.c
@@ -456,6 +456,7 @@  static struct inode *ext3_alloc_inode(struct super_block *sb)
 #endif
 	ei->i_block_alloc_info = NULL;
 	ei->vfs_inode.i_version = 1;
+	ei->created_when = jiffies;
 	return &ei->vfs_inode;
 }
 
diff --git a/include/linux/ext3_fs.h b/include/linux/ext3_fs.h
index 8120fa1..d1ba6b8 100644
--- a/include/linux/ext3_fs.h
+++ b/include/linux/ext3_fs.h
@@ -225,6 +225,7 @@  struct ext3_new_group_data {
 #endif
 #define EXT3_IOC_GETRSVSZ		_IOR('f', 5, long)
 #define EXT3_IOC_SETRSVSZ		_IOW('f', 6, long)
+#define EXT3_IOC_INODE_JIFFIES		_IOR('f', 19, long)
 
 /*
  * ioctl commands in 32 bit emulation
diff --git a/include/linux/ext3_fs_i.h b/include/linux/ext3_fs_i.h
index 7894dd0..87d512a 100644
--- a/include/linux/ext3_fs_i.h
+++ b/include/linux/ext3_fs_i.h
@@ -141,6 +141,8 @@  struct ext3_inode_info {
 	 * by other means, so we have truncate_mutex.
 	 */
 	struct mutex truncate_mutex;
+	unsigned long		created_when; /* jiffies of creation time */	
+
 	struct inode vfs_inode;
 };