Re: [PATCH RESEND v1 01/16] vfs: introduce some data structures
On Thu, Jan 10, 2013 at 8:48 AM, David Sterba wrote:
> On Thu, Dec 20, 2012 at 10:43:20PM +0800, zwu.ker...@gmail.com wrote:
>> --- /dev/null
>> +++ b/fs/hot_tracking.c
>> @@ -0,0 +1,109 @@
>> +/*
>> + * fs/hot_tracking.c
>
> From what I've undrestood the file name written here is not wanted, so
> please drop it (and from .h too)
Done.
>
>> + *
>> + * Copyright (C) 2012 IBM Corp. All rights reserved.
>> + * Written by Zhi Yong Wu
>> + *
>> + * This program is free software; you can redistribute it and/or
>> + * modify it under the terms of the GNU General Public
>> + * License v2 as published by the Free Software Foundation.
>
> A short description of the hot tracking feature or pointer to the
> Documentation/ file would be nice here.
ok, Done
>
>> + */
>> +
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include
>> +#include "hot_tracking.h"
>> +
>> +/* kmem_cache pointers for slab caches */
>
> This comment seems useless to me, I does not help understanding the code, just
> says the same what reads in C. There are more such redundant comments in the
> series, but I'm not going point to all of them right now.
Removed.
>
>> +static struct kmem_cache *hot_inode_item_cachep __read_mostly;
>> +static struct kmem_cache *hot_range_item_cachep __read_mostly;
>> +
>
>> --- /dev/null
>> +++ b/include/linux/hot_tracking.h
>> +/* The common info for both following structures */
>> +struct hot_comm_item {
>> + struct rb_node rb_node; /* rbtree index */
>> + struct hot_freq_data hot_freq_data; /* frequency data */
>> + spinlock_t lock; /* protects object data */
>> + struct kref refs; /* prevents kfree */
>> +};
>> +
>> +/* An item representing an inode and its access frequency */
>> +struct hot_inode_item {
>> + struct hot_comm_item hot_inode; /* node in hot_inode_tree */
>> + struct hot_rb_tree hot_range_tree; /* tree of ranges */
>> + spinlock_t lock; /* protect range tree */
>> + struct hot_rb_tree *hot_inode_tree;
>> + u64 i_ino; /* inode number from inode */
>> +};
>
> Please align the comments to something like this (or drop them if they seem
> redundant):
Done
>
> /* The common info for both following structures */
> struct hot_comm_item {
> struct rb_node rb_node; /* rbtree index */
> struct hot_freq_data hot_freq_data; /* frequency data */
> spinlock_t lock; /* protects object data */
> struct kref refs;/* prevents kfree */
> struct list_head n_list; /* list node index */
> };
>
> /* An item representing an inode and its access frequency */
> struct hot_inode_item {
> struct hot_comm_item hot_inode; /* node in hot_inode_tree */
> struct hot_rb_tree hot_range_tree; /* tree of ranges */
> spinlock_t lock; /* protect range tree */
> struct hot_rb_tree *hot_inode_tree;
> u64 i_ino; /* inode number from inode */
> };
>
>> +extern void __init hot_cache_init(void);
>
> this belongs to the private include fs/hot_tracking.h (because this is called
> only once by vfs init and not by filesystems), there's
> hot_track_init(superblock) for that purpose introduced later.
Done, Move it to fs/hot_tracking.h
>
>
> david
--
Regards,
Zhi Yong Wu
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND v1 01/16] vfs: introduce some data structures
On Thu, Dec 20, 2012 at 10:43:20PM +0800, zwu.ker...@gmail.com wrote:
> --- /dev/null
> +++ b/fs/hot_tracking.c
> @@ -0,0 +1,109 @@
> +/*
> + * fs/hot_tracking.c
>From what I've undrestood the file name written here is not wanted, so
please drop it (and from .h too)
> + *
> + * Copyright (C) 2012 IBM Corp. All rights reserved.
> + * Written by Zhi Yong Wu
> + *
> + * This program is free software; you can redistribute it and/or
> + * modify it under the terms of the GNU General Public
> + * License v2 as published by the Free Software Foundation.
A short description of the hot tracking feature or pointer to the
Documentation/ file would be nice here.
> + */
> +
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include
> +#include "hot_tracking.h"
> +
> +/* kmem_cache pointers for slab caches */
This comment seems useless to me, I does not help understanding the code, just
says the same what reads in C. There are more such redundant comments in the
series, but I'm not going point to all of them right now.
> +static struct kmem_cache *hot_inode_item_cachep __read_mostly;
> +static struct kmem_cache *hot_range_item_cachep __read_mostly;
> +
> --- /dev/null
> +++ b/include/linux/hot_tracking.h
> +/* The common info for both following structures */
> +struct hot_comm_item {
> + struct rb_node rb_node; /* rbtree index */
> + struct hot_freq_data hot_freq_data; /* frequency data */
> + spinlock_t lock; /* protects object data */
> + struct kref refs; /* prevents kfree */
> +};
> +
> +/* An item representing an inode and its access frequency */
> +struct hot_inode_item {
> + struct hot_comm_item hot_inode; /* node in hot_inode_tree */
> + struct hot_rb_tree hot_range_tree; /* tree of ranges */
> + spinlock_t lock; /* protect range tree */
> + struct hot_rb_tree *hot_inode_tree;
> + u64 i_ino; /* inode number from inode */
> +};
Please align the comments to something like this (or drop them if they seem
redundant):
/* The common info for both following structures */
struct hot_comm_item {
struct rb_node rb_node; /* rbtree index */
struct hot_freq_data hot_freq_data; /* frequency data */
spinlock_t lock; /* protects object data */
struct kref refs;/* prevents kfree */
struct list_head n_list; /* list node index */
};
/* An item representing an inode and its access frequency */
struct hot_inode_item {
struct hot_comm_item hot_inode; /* node in hot_inode_tree */
struct hot_rb_tree hot_range_tree; /* tree of ranges */
spinlock_t lock; /* protect range tree */
struct hot_rb_tree *hot_inode_tree;
u64 i_ino; /* inode number from inode */
};
> +extern void __init hot_cache_init(void);
this belongs to the private include fs/hot_tracking.h (because this is called
only once by vfs init and not by filesystems), there's
hot_track_init(superblock) for that purpose introduced later.
david
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND v1 01/16] vfs: introduce some data structures
On Thu, Dec 20, 2012 at 10:43:20PM +0800, zwu.ker...@gmail.com wrote:
--- /dev/null
+++ b/fs/hot_tracking.c
@@ -0,0 +1,109 @@
+/*
+ * fs/hot_tracking.c
From what I've undrestood the file name written here is not wanted, so
please drop it (and from .h too)
+ *
+ * Copyright (C) 2012 IBM Corp. All rights reserved.
+ * Written by Zhi Yong Wu wu...@linux.vnet.ibm.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
A short description of the hot tracking feature or pointer to the
Documentation/ file would be nice here.
+ */
+
+#include linux/list.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/spinlock.h
+#include linux/hardirq.h
+#include linux/fs.h
+#include linux/blkdev.h
+#include linux/types.h
+#include linux/limits.h
+#include hot_tracking.h
+
+/* kmem_cache pointers for slab caches */
This comment seems useless to me, I does not help understanding the code, just
says the same what reads in C. There are more such redundant comments in the
series, but I'm not going point to all of them right now.
+static struct kmem_cache *hot_inode_item_cachep __read_mostly;
+static struct kmem_cache *hot_range_item_cachep __read_mostly;
+
--- /dev/null
+++ b/include/linux/hot_tracking.h
+/* The common info for both following structures */
+struct hot_comm_item {
+ struct rb_node rb_node; /* rbtree index */
+ struct hot_freq_data hot_freq_data; /* frequency data */
+ spinlock_t lock; /* protects object data */
+ struct kref refs; /* prevents kfree */
+};
+
+/* An item representing an inode and its access frequency */
+struct hot_inode_item {
+ struct hot_comm_item hot_inode; /* node in hot_inode_tree */
+ struct hot_rb_tree hot_range_tree; /* tree of ranges */
+ spinlock_t lock; /* protect range tree */
+ struct hot_rb_tree *hot_inode_tree;
+ u64 i_ino; /* inode number from inode */
+};
Please align the comments to something like this (or drop them if they seem
redundant):
/* The common info for both following structures */
struct hot_comm_item {
struct rb_node rb_node; /* rbtree index */
struct hot_freq_data hot_freq_data; /* frequency data */
spinlock_t lock; /* protects object data */
struct kref refs;/* prevents kfree */
struct list_head n_list; /* list node index */
};
/* An item representing an inode and its access frequency */
struct hot_inode_item {
struct hot_comm_item hot_inode; /* node in hot_inode_tree */
struct hot_rb_tree hot_range_tree; /* tree of ranges */
spinlock_t lock; /* protect range tree */
struct hot_rb_tree *hot_inode_tree;
u64 i_ino; /* inode number from inode */
};
+extern void __init hot_cache_init(void);
this belongs to the private include fs/hot_tracking.h (because this is called
only once by vfs init and not by filesystems), there's
hot_track_init(superblock) for that purpose introduced later.
david
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH RESEND v1 01/16] vfs: introduce some data structures
On Thu, Jan 10, 2013 at 8:48 AM, David Sterba dste...@suse.cz wrote:
On Thu, Dec 20, 2012 at 10:43:20PM +0800, zwu.ker...@gmail.com wrote:
--- /dev/null
+++ b/fs/hot_tracking.c
@@ -0,0 +1,109 @@
+/*
+ * fs/hot_tracking.c
From what I've undrestood the file name written here is not wanted, so
please drop it (and from .h too)
Done.
+ *
+ * Copyright (C) 2012 IBM Corp. All rights reserved.
+ * Written by Zhi Yong Wu wu...@linux.vnet.ibm.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
A short description of the hot tracking feature or pointer to the
Documentation/ file would be nice here.
ok, Done
+ */
+
+#include linux/list.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/spinlock.h
+#include linux/hardirq.h
+#include linux/fs.h
+#include linux/blkdev.h
+#include linux/types.h
+#include linux/limits.h
+#include hot_tracking.h
+
+/* kmem_cache pointers for slab caches */
This comment seems useless to me, I does not help understanding the code, just
says the same what reads in C. There are more such redundant comments in the
series, but I'm not going point to all of them right now.
Removed.
+static struct kmem_cache *hot_inode_item_cachep __read_mostly;
+static struct kmem_cache *hot_range_item_cachep __read_mostly;
+
--- /dev/null
+++ b/include/linux/hot_tracking.h
+/* The common info for both following structures */
+struct hot_comm_item {
+ struct rb_node rb_node; /* rbtree index */
+ struct hot_freq_data hot_freq_data; /* frequency data */
+ spinlock_t lock; /* protects object data */
+ struct kref refs; /* prevents kfree */
+};
+
+/* An item representing an inode and its access frequency */
+struct hot_inode_item {
+ struct hot_comm_item hot_inode; /* node in hot_inode_tree */
+ struct hot_rb_tree hot_range_tree; /* tree of ranges */
+ spinlock_t lock; /* protect range tree */
+ struct hot_rb_tree *hot_inode_tree;
+ u64 i_ino; /* inode number from inode */
+};
Please align the comments to something like this (or drop them if they seem
redundant):
Done
/* The common info for both following structures */
struct hot_comm_item {
struct rb_node rb_node; /* rbtree index */
struct hot_freq_data hot_freq_data; /* frequency data */
spinlock_t lock; /* protects object data */
struct kref refs;/* prevents kfree */
struct list_head n_list; /* list node index */
};
/* An item representing an inode and its access frequency */
struct hot_inode_item {
struct hot_comm_item hot_inode; /* node in hot_inode_tree */
struct hot_rb_tree hot_range_tree; /* tree of ranges */
spinlock_t lock; /* protect range tree */
struct hot_rb_tree *hot_inode_tree;
u64 i_ino; /* inode number from inode */
};
+extern void __init hot_cache_init(void);
this belongs to the private include fs/hot_tracking.h (because this is called
only once by vfs init and not by filesystems), there's
hot_track_init(superblock) for that purpose introduced later.
Done, Move it to fs/hot_tracking.h
david
--
Regards,
Zhi Yong Wu
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
[PATCH RESEND v1 01/16] vfs: introduce some data structures
From: Zhi Yong Wu
One root structure hot_info is defined, is hooked
up in super_block, and will be used to hold radix tree
root, hash list root and some other information, etc.
Adds hot_inode_tree struct to keep track of
frequently accessed files, and be keyed by {inode, offset}.
Trees contain hot_inode_items representing those files
and ranges.
Having these trees means that vfs can quickly determine the
temperature of some data by doing some calculations on the
hot_freq_data struct that hangs off of the tree item.
Define two items hot_inode_item and hot_range_item,
one of them represents one tracked file
to keep track of its access frequency and the tree of
ranges in this file, while the latter represents
a file range of one inode.
Each of the two structures contains a hot_freq_data
struct with its frequency of access metrics (number of
{reads, writes}, last {read,write} time, frequency of
{reads,writes}).
Also, each hot_inode_item contains one hot_range_tree
struct which is keyed by {inode, offset, length}
and used to keep track of all the ranges in this file.
Signed-off-by: Zhi Yong Wu
---
fs/Makefile |2 +-
fs/dcache.c |2 +
fs/hot_tracking.c| 109 ++
fs/hot_tracking.h| 22
include/linux/hot_tracking.h | 78 ++
5 files changed, 212 insertions(+), 1 deletions(-)
create mode 100644 fs/hot_tracking.c
create mode 100644 fs/hot_tracking.h
create mode 100644 include/linux/hot_tracking.h
diff --git a/fs/Makefile b/fs/Makefile
index 1d7af79..f966dea 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \
attr.o bad_inode.o file.o filesystems.o namespace.o \
seq_file.o xattr.o libfs.o fs-writeback.o \
pnode.o drop_caches.o splice.o sync.o utimes.o \
- stack.o fs_struct.o statfs.o
+ stack.o fs_struct.o statfs.o hot_tracking.o
ifeq ($(CONFIG_BLOCK),y)
obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
diff --git a/fs/dcache.c b/fs/dcache.c
index 3a463d0..7d5be16 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -37,6 +37,7 @@
#include
#include
#include
+#include
#include "internal.h"
#include "mount.h"
@@ -3172,4 +3173,5 @@ void __init vfs_caches_init(unsigned long mempages)
mnt_init();
bdev_cache_init();
chrdev_init();
+ hot_cache_init();
}
diff --git a/fs/hot_tracking.c b/fs/hot_tracking.c
new file mode 100644
index 000..ef7ff09
--- /dev/null
+++ b/fs/hot_tracking.c
@@ -0,0 +1,109 @@
+/*
+ * fs/hot_tracking.c
+ *
+ * Copyright (C) 2012 IBM Corp. All rights reserved.
+ * Written by Zhi Yong Wu
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include "hot_tracking.h"
+
+/* kmem_cache pointers for slab caches */
+static struct kmem_cache *hot_inode_item_cachep __read_mostly;
+static struct kmem_cache *hot_range_item_cachep __read_mostly;
+
+/*
+ * Initialize the inode tree. Should be called for each new inode
+ * access or other user of the hot_inode interface.
+ */
+static void hot_inode_tree_init(struct hot_info *root)
+{
+ root->hot_inode_tree.map = RB_ROOT;
+ spin_lock_init(>lock);
+}
+
+/*
+ * Initialize the hot range tree. Should be called for each new inode
+ * access or other user of the hot_range interface.
+ */
+void hot_range_tree_init(struct hot_inode_item *he)
+{
+ he->hot_range_tree.map = RB_ROOT;
+ spin_lock_init(>lock);
+}
+
+/*
+ * Initialize a new hot_range_item structure. The new structure is
+ * returned with a reference count of one and needs to be
+ * freed using free_range_item()
+ */
+static void hot_range_item_init(struct hot_range_item *hr, loff_t start,
+ struct hot_inode_item *he)
+{
+ hr->start = start;
+ hr->len = RANGE_SIZE;
+ hr->hot_inode = he;
+ kref_init(>hot_range.refs);
+ spin_lock_init(>hot_range.lock);
+ hr->hot_range.hot_freq_data.avg_delta_reads = (u64) -1;
+ hr->hot_range.hot_freq_data.avg_delta_writes = (u64) -1;
+ hr->hot_range.hot_freq_data.flags = FREQ_DATA_TYPE_RANGE;
+}
+
+/*
+ * Initialize a new hot_inode_item structure. The new structure is
+ * returned with a reference count of one and needs to be
+ * freed using hot_free_inode_item()
+ */
+static void hot_inode_item_init(struct hot_inode_item *he,
+ u64 ino,
+ struct hot_rb_tree *hot_inode_tree)
+{
+ he->i_ino = ino;
+ he->hot_inode_tree = hot_inode_tree;
+ kref_init(>hot_inode.refs);
+
[PATCH RESEND v1 01/16] vfs: introduce some data structures
From: Zhi Yong Wu wu...@linux.vnet.ibm.com
One root structure hot_info is defined, is hooked
up in super_block, and will be used to hold radix tree
root, hash list root and some other information, etc.
Adds hot_inode_tree struct to keep track of
frequently accessed files, and be keyed by {inode, offset}.
Trees contain hot_inode_items representing those files
and ranges.
Having these trees means that vfs can quickly determine the
temperature of some data by doing some calculations on the
hot_freq_data struct that hangs off of the tree item.
Define two items hot_inode_item and hot_range_item,
one of them represents one tracked file
to keep track of its access frequency and the tree of
ranges in this file, while the latter represents
a file range of one inode.
Each of the two structures contains a hot_freq_data
struct with its frequency of access metrics (number of
{reads, writes}, last {read,write} time, frequency of
{reads,writes}).
Also, each hot_inode_item contains one hot_range_tree
struct which is keyed by {inode, offset, length}
and used to keep track of all the ranges in this file.
Signed-off-by: Zhi Yong Wu wu...@linux.vnet.ibm.com
---
fs/Makefile |2 +-
fs/dcache.c |2 +
fs/hot_tracking.c| 109 ++
fs/hot_tracking.h| 22
include/linux/hot_tracking.h | 78 ++
5 files changed, 212 insertions(+), 1 deletions(-)
create mode 100644 fs/hot_tracking.c
create mode 100644 fs/hot_tracking.h
create mode 100644 include/linux/hot_tracking.h
diff --git a/fs/Makefile b/fs/Makefile
index 1d7af79..f966dea 100644
--- a/fs/Makefile
+++ b/fs/Makefile
@@ -11,7 +11,7 @@ obj-y := open.o read_write.o file_table.o super.o \
attr.o bad_inode.o file.o filesystems.o namespace.o \
seq_file.o xattr.o libfs.o fs-writeback.o \
pnode.o drop_caches.o splice.o sync.o utimes.o \
- stack.o fs_struct.o statfs.o
+ stack.o fs_struct.o statfs.o hot_tracking.o
ifeq ($(CONFIG_BLOCK),y)
obj-y += buffer.o bio.o block_dev.o direct-io.o mpage.o ioprio.o
diff --git a/fs/dcache.c b/fs/dcache.c
index 3a463d0..7d5be16 100644
--- a/fs/dcache.c
+++ b/fs/dcache.c
@@ -37,6 +37,7 @@
#include linux/rculist_bl.h
#include linux/prefetch.h
#include linux/ratelimit.h
+#include linux/hot_tracking.h
#include internal.h
#include mount.h
@@ -3172,4 +3173,5 @@ void __init vfs_caches_init(unsigned long mempages)
mnt_init();
bdev_cache_init();
chrdev_init();
+ hot_cache_init();
}
diff --git a/fs/hot_tracking.c b/fs/hot_tracking.c
new file mode 100644
index 000..ef7ff09
--- /dev/null
+++ b/fs/hot_tracking.c
@@ -0,0 +1,109 @@
+/*
+ * fs/hot_tracking.c
+ *
+ * Copyright (C) 2012 IBM Corp. All rights reserved.
+ * Written by Zhi Yong Wu wu...@linux.vnet.ibm.com
+ *
+ * This program is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU General Public
+ * License v2 as published by the Free Software Foundation.
+ */
+
+#include linux/list.h
+#include linux/err.h
+#include linux/slab.h
+#include linux/module.h
+#include linux/spinlock.h
+#include linux/hardirq.h
+#include linux/fs.h
+#include linux/blkdev.h
+#include linux/types.h
+#include linux/limits.h
+#include hot_tracking.h
+
+/* kmem_cache pointers for slab caches */
+static struct kmem_cache *hot_inode_item_cachep __read_mostly;
+static struct kmem_cache *hot_range_item_cachep __read_mostly;
+
+/*
+ * Initialize the inode tree. Should be called for each new inode
+ * access or other user of the hot_inode interface.
+ */
+static void hot_inode_tree_init(struct hot_info *root)
+{
+ root-hot_inode_tree.map = RB_ROOT;
+ spin_lock_init(root-lock);
+}
+
+/*
+ * Initialize the hot range tree. Should be called for each new inode
+ * access or other user of the hot_range interface.
+ */
+void hot_range_tree_init(struct hot_inode_item *he)
+{
+ he-hot_range_tree.map = RB_ROOT;
+ spin_lock_init(he-lock);
+}
+
+/*
+ * Initialize a new hot_range_item structure. The new structure is
+ * returned with a reference count of one and needs to be
+ * freed using free_range_item()
+ */
+static void hot_range_item_init(struct hot_range_item *hr, loff_t start,
+ struct hot_inode_item *he)
+{
+ hr-start = start;
+ hr-len = RANGE_SIZE;
+ hr-hot_inode = he;
+ kref_init(hr-hot_range.refs);
+ spin_lock_init(hr-hot_range.lock);
+ hr-hot_range.hot_freq_data.avg_delta_reads = (u64) -1;
+ hr-hot_range.hot_freq_data.avg_delta_writes = (u64) -1;
+ hr-hot_range.hot_freq_data.flags = FREQ_DATA_TYPE_RANGE;
+}
+
+/*
+ * Initialize a new hot_inode_item structure. The new structure is
+ * returned with a reference count of one and needs to be
+ * freed using hot_free_inode_item()
+ */
+static void hot_inode_item_init(struct
