[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 "/* ... */"

 
+#ifdef CONFIG_PREFETCH_CORE
+void prefetch_page_release_hook(struct page *page);
+#endif
+

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;
-
+	
+#ifdef CONFIG_PREFETCH_CORE
+	prefetch_page_release_hook(page);
+#endif

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 {
+	START_TRACING,
+	STOP_TRACING,
+	CONTINUE_TRACING
+};

*_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