Skip to contentSkip to conversation

statistics: no include hell for users

RFC PATCH In review Martin Peschke 2 participants 3 messages Jun 6, 2007 – Jun 7, 2007

Subsystems (2)
Library code s390
lore
Share & tools
lore

Martin Peschke

author
#0 lore raw

RFC PATCH 1/4 statistics: no include hell for users

Cleaning up inclusion of header files. Don't want to create an include hell for users.

Maintain void pointers instead of dentry pointers for debugfs files. We never poke inside those dentries anyway. We just keep them in order to pass them back as a kind of handle to debugfs on cleanup.

Getting rid of seq_file as parameter to the label-function. Now we just pass a buffer to the label-function.

Signed-off-by Martin Peschke <[email protected]>

---

 include/linux/statistic.h |   11 ++++-------
 lib/statistic.c           |   23 +++++++++++++++++++----
 2 files changed, 23 insertions(+), 11 deletions(-)
include/linux/statistic.h +4 -7
Index: linux/include/linux/statistic.h
===================================================================
@@ -23,9 +23,7 @@
 #ifndef STATISTIC_H
 #define STATISTIC_H
 
-#include <linux/fs.h>
 #include <linux/types.h>
-#include <linux/percpu.h>
 
 /**
  * struct statistic_info - description of a class of statistics

@@ -124,17 +122,16 @@ struct statistic {
 struct statistic_interface {
 /* private: */
 	struct list_head	 list;
-	struct dentry		*debugfs_dir;
-	struct dentry		*data_file;
-	struct dentry		*def_file;
+	void			*debugfs_dir;
+	void			*data_file;
+	void			*def_file;
 /* public: */
 	struct statistic	*stat;
 	struct statistic_info	*info;
 	int			 number;
 	int			(*pull)(struct statistic_interface *interface);
 	void			(*label)(struct statistic_interface *interface,
-					 int i, void *key,
-					 struct seq_file *seq);
+					 int i, void *key, char *buf, int size);
 	void			*private;
 };
lib/statistic.c +19 -4
Index: linux/lib/statistic.c
===================================================================
@@ -483,6 +483,23 @@ static void statistic_parse_line(struct 
 
 /* sequential files comprising user interface */
 
+#define STATISTIC_LABEL_SIZE	128
+static void statistic_label(struct statistic_interface *interface, int i,
+			    void *key, struct seq_file *seq)
+{
+	struct statistic_info *info = &interface->info[i];
+	char *buf;
+
+	if (!(info->flags & STATISTIC_FLAGS_LABEL))
+		return;
+	buf = kzalloc(STATISTIC_LABEL_SIZE, GFP_ATOMIC);
+	if (!buf)
+		return;
+	interface->label(interface, i, key, buf, STATISTIC_LABEL_SIZE - 1);
+	seq_printf(seq, "%s", buf);
+	kfree(buf);
+}
+
 struct statistic_seq_private {
 	struct statistic_interface *interface;
 	int i;

@@ -995,8 +1012,7 @@ static void _statistic_data_histogram(st
 				      int i)
 {
 	seq_printf(seq, "%s %s%Ld %Lu ", info->name, prefix, bound, hits);
-	if (info->flags & STATISTIC_FLAGS_LABEL)
-		interface->label(interface, i, &bound, seq);
+	statistic_label(interface, i, &bound, seq);
 	seq_printf(seq, "\n");
 }
 

@@ -1218,8 +1234,7 @@ static void statistic_data_sparse(struct
 			seq_printf(seq, "0x%Lx ",
 				   *(signed long long *)entry->key);
 		seq_printf(seq, "%Lu ", (unsigned long long)entry->hits);
-		if (info->flags & STATISTIC_FLAGS_LABEL)
-			interface->label(interface, i, entry->key, seq);
+		statistic_label(interface, i, entry->key, seq);
 		seq_printf(seq, "\n");
 	}
 }
#1 lore raw

include/linux/statistic.h struct statistic { · line 127

Quoted 10 lines · Martin Peschke

struct statistic_interface {
/* private: */

        struct list_head         list;
include/linux/statistic.h +3 -3
-       struct dentry           *debugfs_dir;
-       struct dentry           *data_file;
-       struct dentry           *def_file;
+       void                    *debugfs_dir;
+       void                    *data_file;
+       void                    *def_file;

If you don't actually dereference the pointer, you should just be able to declare:

	struct dentry;

and be done with it, right? You don't _need_ the includes to have just pointers.

-- Dave

author
#2 lore raw
Quoted 11 lines · Dave Hansen

include/linux/statistic.h struct statistic { · line 127

Quoted 9 lines · Martin Peschke

struct statistic_interface {
/* private: */

        struct list_head         list;
include/linux/statistic.h +3 -3
-       struct dentry           *debugfs_dir;
-       struct dentry           *data_file;
-       struct dentry           *def_file;
+       void                    *debugfs_dir;
+       void                    *data_file;
+       void                    *def_file;
Quoted 7 lines

If you don't actually dereference the pointer, you should just be able to declare:

	struct dentry;

and be done with it, right? You don't _need_ the includes to have just pointers.

Ah, looks like an established trick in kernel include files. I guess I can revert the other, seq_file related change then as well. Thank you. Will change my local copy.

Martin