[PATCH 05/10] AXFS: axfs_profiling.c

[PATCH 05/10] AXFS: axfs_profiling.c

[Jared Hulbert]

Profiling is a fault instrumentation and /proc formating system.
This is used to get an accurate picture of what the pages are actually used.
Using this info the image can be optimized for XIP

Signed-off-by: Jared Hulbert <jaredeh@gmail.com>
---
diff --git a/fs/axfs/axfs_profiling.c b/fs/axfs/axfs_profiling.c
new file mode 100644
index 0000000..30d881c
--- /dev/null
+++ b/fs/axfs/axfs_profiling.c
@@ -0,0 +1,594 @@
+/*
+ * Advanced XIP File System for Linux - AXFS
+ *   Readonly, compressed, and XIP filesystem for Linux systems big and small
+ *
+ * Copyright(c) 2008 Numonyx
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms and conditions of the GNU General Public License,
+ * version 2, as published by the Free Software Foundation.
+ *
+ * Authors:
+ *  Eric Anderson
+ *  Jared Hulbert <jaredeh@gmail.com>
+ *  Sujaya Srinivasan
+ *  Justin Treon
+ *
+ * More info and current contacts at http://axfs.sourceforge.net
+ *
+ * axfs_profiling.c -
+ *   Tracks pages of files that enter the page cache.  Outputs through a proc
+ *   file which generates a comma separated data file with path, page offset,
+ *   count of times entered page cache.
+ */
+
+#include <linux/axfs.h>
+#ifdef CONFIG_AXFS_PROFILING
+#include <linux/module.h>
+#include <linux/vmalloc.h>
+#include <linux/proc_fs.h>
+
+#define AXFS_PROC_DIR_NAME "axfs"
+
+struct axfs_profiling_manager {
+	struct axfs_profiling_data *profiling_data;
+	struct axfs_super *sbi;
+	u32 *dir_structure;
+	u32 size;
+};
+
+#define MAX_STRING_LEN 1024
+
+/* Handles for our Directory and File */
+static struct proc_dir_entry *axfs_proc_dir;
+static u32 proc_name_inc;
+
+/******************************************************************************
+ *
+ * axfs_init_profile_dir_structure
+ *
+ * Description:
+ *   Creates the structures for tracking the page usage data and creates the
+ *   proc file that will be used to get the data.
+ *
+ * Parameters:
+ *    (IN) manager - pointer to the profile manager for the filing system
+ *
+ *    (IN) num_inodes - number of files in the system
+ *
+ * Returns:
+ *    0
+ *
+ *****************************************************************************/
+static int axfs_init_profile_dir_structure(struct axfs_profiling_manager
+					   *manager, u32 num_inodes)
+{
+
+	struct axfs_super *sbi = (struct axfs_super *)manager->sbi;
+	u32 child_index = 0, i, j;
+	u32 *dir_structure = manager->dir_structure;
+
+	/* loop through each inode in the image and find all
+	   of the directories and mark their children */
+	for (i = 0; i < num_inodes; i++) {
+		/* determine if the entry is a directory */
+		if (!S_ISDIR(AXFS_GET_MODE(sbi, i)))
+			continue;
+
+		/* get the index number for this directory */
+		child_index = AXFS_GET_INODE_ARRAY_INDEX(sbi, i);
+
+		/* get the offset to its children */
+		for (j = 0; j < AXFS_GET_INODE_NUM_ENTRIES(sbi, i); j++) {
+			if (dir_structure[child_index + j] != 0) {
+				printk(KERN_ERR
+				       "axfs: ERROR inode was already set old "
+				       "%lu new %lu\n", (unsigned long)
+				       dir_structure[child_index + j],
+				       (unsigned long)i);
+			}
+			dir_structure[child_index + j] = i;
+		}
+	}
+
+	return 0;
+}
+
+/******************************************************************************
+ *
+ * axfs_get_directory_path
+ *
+ * Description:
+ *   Determines the directory path of every file for printing the spreadsheet.
+ *
+ * Parameters:
+ *    (IN) manager - Pointer to axfs profile manager
+ *
+ *    (OUT) buffer - Pointer to the printable directory path for each file
+ *
+ *    (IN) inode_number - Inode number of file to look up
+ *
+ * Returns:
+ *    Size of the path to the file
+ *
+ *
+ **************************************************************************/
+static int axfs_get_directory_path(struct axfs_profiling_manager *manager,
+				   char *buffer, u32 inode_number)
+{
+	u32 path_depth = 0;
+	u32 path_size = 0;
+	u32 string_len = 0;
+	u32 index = inode_number;
+	u32 dir_number;
+	u8 **path_array = NULL;
+	struct axfs_super *sbi = (struct axfs_super *)manager->sbi;
+	int i;
+
+	/* determine how deep the directory path is and how big the name
+	   string will be walk back until the root directory index is found
+	   (index 0 is root) */
+	while (manager->dir_structure[index] != 0) {
+		path_depth++;
+		/* set the index to the index of the parent directory */
+		index = manager->dir_structure[index];
+	}
+
+	if (path_depth != 0) {
+		/* create an array that will hold a pointer for each of the
+		   directories names */
+		path_array = vmalloc(path_depth * sizeof(*path_array));
+		if (path_array == NULL) {
+			printk(KERN_DEBUG
+			       "axfs: directory_path vmalloc failed.\n");
+			goto out;
+		}
+	}
+
+	index = manager->dir_structure[inode_number];
+	for (i = path_depth; i > 0; i--) {
+		/* get the array_index for the directory corresponding to
+		   index */
+		dir_number = AXFS_GET_INODE_ARRAY_INDEX(sbi, index);
+
+		/* store a pointer to the name in the array */
+		path_array[(i - 1)] = (u8 *) AXFS_GET_INODE_NAME(sbi, index);
+
+		index = manager->dir_structure[index];
+	}
+
+	/* now print out the directory structure from the begining */
+	string_len = sprintf(buffer, "./");
+	path_size += string_len;
+	for (i = 0; i < path_depth; i++) {
+		buffer = buffer + string_len;
+		string_len = sprintf(buffer, "%s/", (char *)path_array[i]);
+		path_size += string_len;
+	}
+
+	if (path_array != NULL)
+		vfree(path_array);
+
+out:
+	return path_size;
+
+}
+
+static ssize_t axfs_procfile_read(char *buffer,
+				  char **buffer_location,
+				  off_t offset, int buffer_length, int *eof,
+				  void *data)
+{
+	struct axfs_profiling_manager *man;
+	struct axfs_profiling_data *profile;
+	struct axfs_super *sbi;
+	u64 array_index;
+	u64 loop_size, inode_page_offset, node_offset, inode_number;
+	u64 print_len = 0;
+	unsigned long addr;
+	int len = 0;
+	int i;
+	char *buff, *name = NULL;
+
+	man = (struct axfs_profiling_manager *)data;
+	sbi = man->sbi;
+
+	loop_size = man->size / sizeof(*profile);
+
+	/* If all data has been returned set EOF */
+	if (offset >= loop_size) {
+		*eof = 1;
+		return 0;
+	}
+
+	buff = buffer;
+	/* print as much as the buffer can take */
+	for (i = offset; i < loop_size; i++) {
+
+		if ((print_len + MAX_STRING_LEN) > buffer_length)
+			break;
+		/* get the first profile data structure */
+		profile = &(man->profiling_data[i]);
+
+		if (profile->count == 0)
+			continue;
+
+		inode_number = profile->inode_number;
+
+		/* file names can be duplicated so we must print out the path */
+		len = axfs_get_directory_path(man, buff, inode_number);
+
+		print_len += len;
+		buff += len;
+
+		/* get a pointer to the inode name */
+		array_index = AXFS_GET_INODE_ARRAY_INDEX(sbi, inode_number);
+		name = (char *)AXFS_GET_INODE_NAME(sbi, inode_number);
+
+		/* need to convert the page number in the node area to
+		   the page number within the file */
+		node_offset = i;
+		/* gives the offset of the node in the node list area
+		   then substract that from the */
+		inode_page_offset = node_offset - array_index;
+
+		/* set everything up to print out */
+		addr = (unsigned long)(inode_page_offset * PAGE_SIZE);
+		len = sprintf(buff, "%s,%lu,%lu\n", name, addr, profile->count);
+
+		print_len += len;
+		buff += len;
+	}
+
+	/* return the number of items printed.
+	   This will be added to offset and passed back to us */
+	*buffer_location = (char *)(i - offset);
+
+	return print_len;
+}
+
+static ssize_t axfs_procfile_write(struct file *file,
+				   const char *buffer, unsigned long count,
+				   void *data)
+{
+	struct axfs_profiling_manager *man_ptr =
+	    (struct axfs_profiling_manager *)data;
+
+	if ((count >= 2) && (0 == memcmp(buffer, "on", 2))) {
+		man_ptr->sbi->profiling_on = TRUE;
+	} else if ((count >= 3) && (0 == memcmp(buffer, "off", 3))) {
+		man_ptr->sbi->profiling_on = FALSE;
+	} else if ((count >= 5) && (0 == memcmp(buffer, "clear", 5))) {
+		memset(man_ptr->profiling_data, 0, man_ptr->size);
+	} else {
+		printk(KERN_INFO
+		       "axfs: Unknown command.  Supported options are:\n");
+		printk(KERN_INFO "\t\"on\"\tTurn on profiling\n");
+		printk(KERN_INFO "\t\"off\"\tTurn off profiling\n");
+		printk(KERN_INFO "\t\"clear\"\tClear profiling buffer\n");
+	}
+
+	return count;
+}
+
+static int axfs_create_proc_directory(void)
+{
+	if (axfs_proc_dir == NULL) {
+		axfs_proc_dir = proc_mkdir(AXFS_PROC_DIR_NAME, NULL);
+		if (!axfs_proc_dir) {
+			printk(KERN_WARNING
+			       "axfs: Failed to create directory\n");
+			return FALSE;
+		}
+	}
+	return TRUE;
+}
+
+static void axfs_delete_proc_directory(void)
+{
+	/* Determine if there are any directory elements
+	   and remove if all of the proc files are removed. */
+	if (axfs_proc_dir != NULL) {
+		if (axfs_proc_dir->subdir == NULL) {
+			remove_proc_entry(AXFS_PROC_DIR_NAME, NULL);
+			axfs_proc_dir = NULL;
+		}
+	}
+}
+
+/******************************************************************************
+ *
+ * axfs_delete_proc_file
+ *
+ * Description:
+ *   Will search through the proc directory for the correct proc file,
+ *   then delete it
+ *
+ * Parameters:
+ *    (IN) sbi- axfs superblock pointer to determine which proc file to remove
+ *
+ * Returns:
+ *    The profiling manager pointer for the proc file.
+ *
+ *****************************************************************************/
+static struct axfs_profiling_manager *axfs_delete_proc_file(struct axfs_super
+							    *sbi)
+{
+	struct proc_dir_entry *current_proc_file;
+	struct axfs_profiling_manager *manager;
+	void *rv = NULL;
+
+	if (!axfs_proc_dir)
+		return NULL;
+
+	/* Walk through the proc file entries to find the matching sbi */
+	current_proc_file = axfs_proc_dir->subdir;
+
+	while (current_proc_file != NULL) {
+		manager = current_proc_file->data;
+		if (manager == NULL) {
+			printk(KERN_WARNING
+			       "axfs: Error removing proc file private "
+			       "data was NULL.\n");
+			rv = NULL;
+			break;
+		}
+		if (manager->sbi == sbi) {
+			/* we found the match */
+			remove_proc_entry(current_proc_file->name,
+					  axfs_proc_dir);
+			rv = (void *)manager;
+			break;
+		}
+		current_proc_file = axfs_proc_dir->next;
+	}
+	return (struct axfs_profiling_manager *)rv;
+}
+
+/******************************************************************************
+ *
+ * axfs_register_profiling_proc
+ *
+ * Description:
+ *   Will register the instance of the proc file for a given volume.
+ *
+ * Parameters:
+ *    (IN) manager - Pointer to the profiling manager for the axfs volume
+ *
+ * Returns:
+ *    0 or error number
+ *
+ *****************************************************************************/
+static int axfs_register_profiling_proc(struct axfs_profiling_manager *manager)
+{
+	int rv = 0;
+	struct proc_dir_entry *proc_file;
+	char file_name[20];
+
+	if (!axfs_create_proc_directory()) {
+		rv = -ENOMEM;
+		goto out;
+	}
+
+	sprintf(file_name, "volume%d", proc_name_inc);
+	proc_file = create_proc_entry(file_name, (mode_t) 0644, axfs_proc_dir);
+	if (proc_file == NULL) {
+		remove_proc_entry(file_name, axfs_proc_dir);
+		axfs_delete_proc_directory();
+		rv = -ENOMEM;
+		goto out;
+	}
+
+	proc_name_inc++;
+	proc_file->read_proc = axfs_procfile_read;
+	proc_file->write_proc = axfs_procfile_write;
+	proc_file->owner = THIS_MODULE;
+	proc_file->mode = S_IRUSR | S_IWUSR | S_IRGRP | S_IROTH;
+	proc_file->uid = 0;
+	proc_file->gid = 0;
+	proc_file->data = manager;
+
+	printk(KERN_DEBUG "axfs: Proc entry created\n");
+
+out:
+	return rv;
+}
+
+/******************************************************************************
+ *
+ * axfs_unregister_profiling_proc
+ *
+ * Description:
+ *   Will unregister the instance of the proc file for the volume that was
+ *   mounted.  If this is the last volume mounted then the proc directory
+ *   will also be removed.
+ *
+ * Parameters:
+ *    (IN) sbi- axfs superblock pointer to determine which proc file to remove
+ *
+ * Returns:
+ *    The profiling manager pointer for the proc file.
+ *
+ *****************************************************************************/
+static struct axfs_profiling_manager *axfs_unregister_profiling_proc(struct
+								     axfs_super
+								     *sbi)
+{
+	struct axfs_profiling_manager *manager;
+	manager = axfs_delete_proc_file(sbi);
+	axfs_delete_proc_directory();
+	return manager;
+}
+
+/******************************************************************************
+ *
+ * axfs_init_profiling
+ *
+ * Description:
+ *   Creates the structures for tracking the page usage data and creates the
+ *   proc file that will be used to get the data.
+ *
+ * Parameters:
+ *    (IN) sbi- axfs superblock pointer
+ *
+ * Returns:
+ *    TRUE or FALSE
+ *
+ *****************************************************************************/
+int axfs_init_profiling(struct axfs_super *sbi)
+{
+
+	u32 num_nodes, num_inodes;
+	struct axfs_profiling_manager *manager = NULL;
+	struct axfs_profiling_data *profile_data = NULL;
+	int err = -ENOMEM;
+
+	/* determine the max number of pages in the FS */
+	num_nodes = sbi->blocks;
+	if (!num_nodes)
+		return 0;
+
+	manager = vmalloc(sizeof(*manager));
+	if (!manager)
+		goto out;
+
+	profile_data = vmalloc(num_nodes * sizeof(*profile_data));
+	if (!profile_data)
+		goto out;
+
+	memset(profile_data, 0, num_nodes * sizeof(*profile_data));
+
+	/* determine the max number of inodes in the FS */
+	num_inodes = sbi->files;
+
+	manager->dir_structure = vmalloc(num_inodes * sizeof(u32 *));
+	if (!manager->dir_structure)
+		goto out;
+
+	memset(manager->dir_structure, 0, (num_inodes * sizeof(u32 *)));
+
+	manager->profiling_data = profile_data;
+	manager->size = num_nodes * sizeof(*profile_data);
+	manager->sbi = sbi;
+	sbi->profiling_on = TRUE;	/* Turn on profiling by default */
+	sbi->profile_data_ptr = profile_data;
+
+	err = axfs_init_profile_dir_structure(manager, num_inodes);
+	if (err)
+		goto out;
+
+	err = axfs_register_profiling_proc(manager);
+	if (err)
+		goto out;
+
+	return 0;
+
+out:
+	if (manager->dir_structure)
+		vfree(manager->dir_structure);
+	if (profile_data)
+		vfree(profile_data);
+	if (manager)
+		vfree(manager);
+	return err;
+}
+
+/******************************************************************************
+ *
+ * axfs_shutdown_profiling
+ *
+ * Description:
+ *   Remove the proc file for this volume and release the memory in the
+ *   profiling manager
+ *
+ * Parameters:
+ *    (IN) sbi- axfs superblock pointer
+ *
+ * Returns:
+ *    TRUE or FALSE
+ *
+ *****************************************************************************/
+int axfs_shutdown_profiling(struct axfs_super *sbi)
+{
+	struct axfs_profiling_manager *manager;
+	/* remove the proc file for this volume and release the memory in the
+	   profiling manager */
+
+	if (!sbi)
+		return TRUE;
+
+	if (!sbi->profile_data_ptr)
+		return TRUE;
+
+	manager = axfs_unregister_profiling_proc(sbi);
+
+	if (manager == NULL)
+		return FALSE;
+
+	if (manager->dir_structure)
+		vfree(manager->dir_structure);
+	if (manager->profiling_data)
+		vfree(manager->profiling_data);
+	if (manager)
+		vfree(manager);
+	return TRUE;
+}
+
+/******************************************************************************
+ *
+ * axfs_profiling_add
+ *
+ * Description:
+ *    Log when a node is paged into memory by incrementing the count in the
+ *    array profile data structure.
+ *
+ * Parameters:
+ *    (IN) sbi- axfs superblock pointer
+ *
+ *    (IN) array_index - The offset into the nodes table of file (node number)
+ *
+ *    (IN) axfs_inode_number - Inode of the node to determine file name later
+ *
+ * Returns:
+ *    none
+ *
+ *****************************************************************************/
+void axfs_profiling_add(struct axfs_super *sbi, unsigned long array_index,
+			unsigned int axfs_inode_number)
+{
+	unsigned long addr;
+	struct axfs_profiling_data *profile_data;
+
+	if (sbi->profiling_on != TRUE)
+		return;
+
+	addr = (unsigned long)sbi->profile_data_ptr;
+	addr += array_index * sizeof(*profile_data);
+
+	profile_data = (struct axfs_profiling_data *)addr;
+
+	/* Record the inode number to determine the file name later. */
+	profile_data->inode_number = axfs_inode_number;
+
+	/* Increment the number of times the node has been paged in */
+	profile_data->count++;
+}
+
+#else
+
+int axfs_init_profiling(struct axfs_super *sbi)
+{
+	return 0;
+}
+
+int axfs_shutdown_profiling(struct axfs_super *sbi)
+{
+	return 0;
+}
+
+void axfs_profiling_add(struct axfs_super *sbi, unsigned long array_index,
+			unsigned int axfs_inode_number)
+{
+}
+
+#endif /* CONFIG_AXFS_PROFILING */

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Carsten Otte]

Jared Hulbert wrote:
> Profiling is a fault instrumentation and /proc formating system.
> This is used to get an accurate picture of what the pages are actually used.
> Using this info the image can be optimized for XIP
Exporting profiling data for a file system in another file system 
(/proc) seems not very straigtforward to me. I think it is worth 
considering to export this information via the same mount point.

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[David Woodhouse]

On Thu, 2008-08-21 at 10:44 +0200, Carsten Otte wrote:
> Jared Hulbert wrote:
> > Profiling is a fault instrumentation and /proc formating system.
> > This is used to get an accurate picture of what the pages are actually used.
> > Using this info the image can be optimized for XIP

> Exporting profiling data for a file system in another file system 
> (/proc) seems not very straigtforward to me. I think it is worth 
> considering to export this information via the same mount point.

I would have said sysfs, rather than 'the same mount point'.

-- 
David Woodhouse                            Open Source Technology Centre
David.Woodhouse@intel.com                              Intel Corporation

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Carsten Otte]

David Woodhouse wrote:
> On Thu, 2008-08-21 at 10:44 +0200, Carsten Otte wrote:
>> Jared Hulbert wrote:
>>> Profiling is a fault instrumentation and /proc formating system.
>>> This is used to get an accurate picture of what the pages are actually used.
>>> Using this info the image can be optimized for XIP
> 
>> Exporting profiling data for a file system in another file system 
>> (/proc) seems not very straigtforward to me. I think it is worth 
>> considering to export this information via the same mount point.
> 
> I would have said sysfs, rather than 'the same mount point'.
Well, filesystems are usually not represented in the device model. 
It'd be possible to add a system device for it, but that does'nt feel 
like the right solution to me.

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Arnd Bergmann]

On Thursday 21 August 2008, David Woodhouse wrote:
> On Thu, 2008-08-21 at 10:44 +0200, Carsten Otte wrote:
> >
> > Exporting profiling data for a file system in another file system 
> > (/proc) seems not very straigtforward to me. I think it is worth 
> > considering to export this information via the same mount point.
> 
> I would have said sysfs, rather than 'the same mount point'.
> 

Let me throw in debugfs as my preferred option. sysfs is for stable
interfaces, while profiling generally fits into the debugging category.

	Arnd <><

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Jared Hulbert]

On Thu, Aug 21, 2008 at 4:39 AM, Arnd Bergmann <arnd@arndb.de> wrote:
> On Thursday 21 August 2008, David Woodhouse wrote:
>> On Thu, 2008-08-21 at 10:44 +0200, Carsten Otte wrote:
>> >
>> > Exporting profiling data for a file system in another file system
>> > (/proc) seems not very straigtforward to me. I think it is worth
>> > considering to export this information via the same mount point.
>>
>> I would have said sysfs, rather than 'the same mount point'.
>>
>
> Let me throw in debugfs as my preferred option. sysfs is for stable
> interfaces, while profiling generally fits into the debugging category.

Three responses, three suggestions....

1) same mount point -
I don't see how this works without an ioctl.  I can't just make up
files in my mounted filesystem.   You expect the mounted version to
match input to the mkfs.  I'd not be happy with an ioctl.  You can
just read it.

2) sysfs -
I agree with Carsten, I don't see how this fits in the sysfs hierarchy.

3) debugfs -
I don't know diddly about this.

So why not /proc?

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Arnd Bergmann]

On Thursday 21 August 2008, Jared Hulbert wrote:

> 1) same mount point -
> I don't see how this works without an ioctl.  I can't just make up
> files in my mounted filesystem.   You expect the mounted version to
> match input to the mkfs.  I'd not be happy with an ioctl.  You can
> just read it.

I think what Carsten was suggesting is that you create extra files
in the file system that behave like your current procfs files.
This limits the choice for names inside of the file system, and
therefor I think should not be done.
 
> 2) sysfs -
> I agree with Carsten, I don't see how this fits in the sysfs hierarchy.

You can create attributes below the device you have mounted.
Technically possible, the main issue I see with this is having
to maintain ABI compatibility.

> 3) debugfs -
> I don't know diddly about this.
> 
> So why not /proc?

/proc has the same ABI restrictions as sysfs. We more or less stopped
allowing new files in /proc some 5 years ago for this reason. I didn't
even read beyond the word /proc to know that what you do here is wrong.
debugfs is normally easier to use than procfs as well, you just
define some file_operations with read/write callbacks and call
debugfs_create_file with the path name below /sys/kernel/debug.

If I may give yet another suggestion:

4) no profiling at all
The profiling code has certainly been useful to you during development,
and you should keep that code around for your own work on it,
but maybe you should not push that upstream, because regular users
are not going to need it.

	Arnd <><

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Jared Hulbert]

> /proc has the same ABI restrictions as sysfs. We more or less stopped
> allowing new files in /proc some 5 years ago for this reason. I didn't
> even read beyond the word /proc to know that what you do here is wrong.
> debugfs is normally easier to use than procfs as well, you just
> define some file_operations with read/write callbacks and call
> debugfs_create_file with the path name below /sys/kernel/debug.

no /proc.
thanks for the explanation.

So /sys/kernel/debug/axfs/volume0 would work?

> 4) no profiling at all
> The profiling code has certainly been useful to you during development,
> and you should keep that code around for your own work on it,
> but maybe you should not push that upstream, because regular users
> are not going to need it.

Nope.  Profiling is absolutely fundamental to how AXFS works.  Read
the [PATCH 00/10] thread again.

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Arnd Bergmann]

On Thursday 21 August 2008, Jared Hulbert wrote:
> So /sys/kernel/debug/axfs/volume0 would work?
> 
> > 4) no profiling at all
> > The profiling code has certainly been useful to you during development,
> > and you should keep that code around for your own work on it,
> > but maybe you should not push that upstream, because regular users
> > are not going to need it.
> 
> Nope.  Profiling is absolutely fundamental to how AXFS works.  Read
> the [PATCH 00/10] thread again.

Ok, understood it now. So it actually is a stable interface into
the file system, which means that debugfs might not be the best
solution.

I need to think about this some more. So far none of the options
are perfect. What I can think of so far includes:

1. An ioctl on the mount point of the fs
2. An ioctl that you need to call on each file
3. sysfs files is /sys/fs/axfs/
4. A new virtual file system to be mounted to /sys/fs/axfs/
5. A file below the device in sysfs, e.g. /sys/block/mtdblk0/axfs-profile

I've also taken a look at the format of the profiling data file.
I'm not sure that it is ideal if you want to be able to share
identical data blocks between files. Do you currently do that
in your mkfs? The fs format certainly allows it. I would expect
that if you checksum each page, you can find duplicates on a page
basis and save some space this way. However, it can make profiling
harder if you count based on blocks but report the data based on
the file name. Not sure what a better solution would look like.

	Arnd <><

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Carsten Otte]

Jared Hulbert wrote:
>> The profiling code has certainly been useful to you during development,
>> and you should keep that code around for your own work on it,
>> but maybe you should not push that upstream, because regular users
>> are not going to need it.
> 
> Nope.  Profiling is absolutely fundamental to how AXFS works.  Read
> the [PATCH 00/10] thread again.
I agree, the profiling part is the sweet spot. Profiling should be in, 
probably even selectable at runtime as opposed to a config-switch. 
This way we could have it in enterprise distros that would'nt build 
another kernel image for this.

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Geert Uytterhoeven]

[-- Attachment #1: Type: TEXT/PLAIN, Size: 980 bytes --]

On Thu, 21 Aug 2008, Arnd Bergmann wrote:
> On Thursday 21 August 2008, Jared Hulbert wrote:
> 4) no profiling at all
> The profiling code has certainly been useful to you during development,
> and you should keep that code around for your own work on it,
> but maybe you should not push that upstream, because regular users
> are not going to need it.

The profiling is needed to feedback into mkfs.axfs, to decide which pages to
make XIP (in NOR) or not (in NAND). So yes, normal users need it when creating
file systems for a mixed NOR/NAND FLASH system.

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Techsoft Centre Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone:    +32 (0)2 700 8453
Fax:      +32 (0)2 700 8622
E-mail:   Geert.Uytterhoeven@sonycom.com
Internet: http://www.sony-europe.com/

A division of Sony Europe (Belgium) N.V.
VAT BE 0413.825.160 · RPR Brussels
Fortis · BIC GEBABEBB · IBAN BE41293037680010

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Arnd Bergmann]

On Thursday 21 August 2008, Jared Hulbert wrote:
> 1) same mount point -
> I don't see how this works without an ioctl.  I can't just make up
> files in my mounted filesystem.   You expect the mounted version to
> match input to the mkfs.  I'd not be happy with an ioctl.  You can
> just read it.
> 
> 2) sysfs -
> I agree with Carsten, I don't see how this fits in the sysfs hierarchy.
> 
> 3) debugfs -
> I don't know diddly about this.

Ok, so now yet another suggestion, which may sound a little strange:

oprofilefs

I believe you can use the oprofile infrastructure to record data
about file accesses, even independent of the file system you
are looking at.

It's probably a lot of work to get it right, but I would be worth it.

	Arnd <><

______________________________________________________
Linux MTD discussion mailing list
http://lists.infradead.org/mailman/listinfo/linux-mtd/

Re: [PATCH 05/10] AXFS: axfs_profiling.c

[Arnd Bergmann]

On Friday 22 August 2008, you wrote:
> You mean to take this off list?

No, i replied to your mail that was sent just to me.
Putting everyone back on now

> > In 3, you create files with sysfs_create_file, and are fairly limited
> > with how you can use it. A structured file like you have in procfs
> > would not be allowed. File names are fixed, directory names can
> > be used to identify the mounted file systems. You can create symlinks
> > between your directory and other things in sysfs.
> 
> What do you mean a structured file wouldn't be allowed?  What's in them then?

sysfs files are meant to have just a single value. Some have a list of
values of the same type, but a file that needs a nontrivial parser
(even sscanf) is not allowed in sysfs, by convention.
There is also the technical limitation of the size to a single page,
which makes it hard to write variable size data.

> > In 4, you write a whole file system like debugfs (it's not as hard
> > as it sounds) and are free to do anything in there, but you can't
> > easily symlink to sysfs.
> 
> Argh.  No it might not be too bad to do to do, but it sounds like a
> maintenance hassle.  Sounds like the best option though.
> 
> Why did we decide debugfs is a bad fit?

It's basically the same as debugfs -- actually I once started a patch
to make it a single function call to instantiate a debugfs-like
file system, but I never finished that patch.

debugfs is a bad idea here because it is not meant for stable interfaces
but rather ad-hoc stuff. In a distribution kernel, debugfs is supposed
to be empty.

> > So where does a page show up in the profile if you have two identical
> > files and both are mapped?
> 
> In which ever file was actually read.  The kernel driver doesn't
> really know pages are redundant.

ok.

> > Will the kernel map them to the same page
> > but count the files separately, or will it show the same count for both?
> 
> I count faults on pages in mmap() so I don't really care whether a
> page is mapped twice or just once.  I'll count it every time you fault
> it even if it's the same physical page.  It's the image builders job
> to figure out if there are redundant pages.

ok, makes sense.

I think there is still another option, which would be to generalize
the profiling interface so it can work with arbitrary file systems.
I'm sure that other people can benefit from that as well, e.g. for
optimizing boot times on disks. For such a general interface,
a per-file ioctl would fit best, and then file systems can implement
it if they want, or it can be moved into VFS.

	Arnd <><