[PATCH 1/2] Implementation of prefetch core functionality (tracing and prefetching)

Ben Collins ben.collins at ubuntu.com
Fri Aug 10 19:26:14 UTC 2007

+///Checks if @line is exactly the same as @param_name, not exceeding param_name length for safety.
+int param_match(char *line, char *param_name);

I'm hoping this code is aimed at getting included upstream. As such,
please change the "// ..." style comments to "/* ... */"

+void prefetch_page_release_hook(struct page *page);

Should probably be in prefetch_core.h and include that instead of a
hardcoded declaration.

@@ -115,7 +119,11 @@ generic_file_direct_IO(int rw, struct ki
 void __remove_from_page_cache(struct page *page)
 	struct address_space *mapping = page->mapping;
+	prefetch_page_release_hook(page);

Likewise, the header should declare this as a nop macro, e.g.
do{}while(0) when PREFETCH_CORE is not defined. Keeps the code clean.

+enum {
+	DEFAULT_TRACE_SIZE_KB = 256 //default trace size is 256 KB

Single unnamed enum, probably should just be a define.

+enum tracing_command_t {

*_t should be for typedefs, not enum names. Just remove the _t

+///Structure holding all information needed for tracing
+struct prefetch_trace_t {

+struct prefetch_trace_t prefetch_trace = {

Same for these.

+	Set if walk_pages() decided that it is the start of tracing
+	and bits should be cleared, not recorded.
+	Using it is protected by inode_lock.
+	If lock breaking is enabled, this variable makes sure that
+	second caller of walk_pages(START_TRACING) will not
+	race with first caller and will not start recording changes.
+static int clearing_in_progress = 0;

No need to initialize globals to zero.

There's a lot of other formatting and style issues. Lots of places where
you actually use typedefs for structs, when you really shouldn't be.

Please read Documentation/CodingStyle and fix things up appropriately.

Ubuntu   : http://www.ubuntu.com/
Linux1394: http://wiki.linux1394.org/

More information about the kernel-team mailing list