[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Zhaolei]

lttctl need debugfs mounted,
This patch make lttctl mount debugfs automatically when it is not mounted.

Little cleanup also done(combile 2 getdebugfsmntdir() into one).

Applies on top of ltt-control-0.63-03012009.

Signed-off-by: Zhao lei <zhaolei at cn.fujitsu.com>
---
 liblttctl/liblttctl.c |   54 +++++++++++++++++++++++++++++---------------------
 liblttctl/lttctl.h    |    3 ++
 lttctl/lttctl.c       |   40 +++++++------------------------------
 3 files changed, 43 insertions(+), 54 deletions(-)
diff -Nur ltt-control-0.63-03012009.org/liblttctl/liblttctl.c ltt-control-0.63-03012009.new/liblttctl/liblttctl.c
--- ltt-control-0.63-03012009.org/liblttctl/liblttctl.c	2009-01-07 09:52:05.000000000 +0800
+++ ltt-control-0.63-03012009.new/liblttctl/liblttctl.c	2009-01-07 14:48:18.000000000 +0800
@@ -39,27 +39,7 @@
 
 static int initdebugfsmntdir(void)
 {
-	char mnt_dir[PATH_MAX];
-	char mnt_type[PATH_MAX];
-
-	FILE *fp = fopen("/proc/mounts", "r");
-	if (!fp) {
-		fprintf(stderr, "%s: Can't open /proc/mounts\n", __func__);
-		return 1;
-	}
-
-	while (1) {
-		if (fscanf(fp, "%*s %s %s %*s %*s %*s", mnt_dir, mnt_type)
-			<= 0) {
-			fprintf(stderr, "%s: debugfs mountpoint not found\n",
-				__func__);
-			return 1;
-		}
-		if (!strcmp(mnt_type, "debugfs")) {
-			strcpy(debugfsmntdir, mnt_dir);
-			return 0;
-		}
-	}
+	return getdebugfsmntdir(debugfsmntdir);
 }
 
 int lttctl_init(void)
@@ -70,7 +50,7 @@
 
 	ret = initdebugfsmntdir();
 	if (ret) {
-		fprintf(stderr, "Debugfs mount point not found\n");
+		fprintf(stderr, "Get debugfs mount point failed\n");
 		return 1;
 	}
 
@@ -664,3 +644,33 @@
 arg_error:
 	return ret;
 }
+
+int getdebugfsmntdir(char *mntdir)
+{
+	char mnt_dir[PATH_MAX];
+	char mnt_type[PATH_MAX];
+	int trymount_done = 0;
+
+	FILE *fp = fopen("/proc/mounts", "r");
+	if (!fp)
+		return -EINVAL;
+
+find_again:
+	while (1) {
+		if (fscanf(fp, "%*s %s %s %*s %*s %*s", mnt_dir, mnt_type) <= 0)
+			break;
+
+		if (!strcmp(mnt_type, "debugfs")) {
+			strcpy(mntdir, mnt_dir);
+			return 0;
+		}
+	}
+
+	if (!trymount_done) {
+		mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL);
+		trymount_done = 1;
+		goto find_again;
+	}
+
+	return -ENOENT;
+}
diff -Nur ltt-control-0.63-03012009.org/liblttctl/lttctl.h ltt-control-0.63-03012009.new/liblttctl/lttctl.h
--- ltt-control-0.63-03012009.org/liblttctl/lttctl.h	2009-01-07 09:52:05.000000000 +0800
+++ ltt-control-0.63-03012009.new/liblttctl/lttctl.h	2009-01-07 14:48:18.000000000 +0800
@@ -36,4 +36,7 @@
 int lttctl_set_channel_subbuf_size(const char *name, const char *channel,
 		unsigned subbuf_size);
 
+/* Helper functions */
+int getdebugfsmntdir(char *mntdir);
+
 #endif /*_LIBLTT_H */
diff -Nur ltt-control-0.63-03012009.org/lttctl/lttctl.c ltt-control-0.63-03012009.new/lttctl/lttctl.c
--- ltt-control-0.63-03012009.org/lttctl/lttctl.c	2009-01-07 09:52:05.000000000 +0800
+++ ltt-control-0.63-03012009.new/lttctl/lttctl.c	2009-01-07 14:48:18.000000000 +0800
@@ -142,26 +142,6 @@
 	printf("\n");
 }
 
-static int getdebugfsmntdir(char *mntdir)
-{
-	char mnt_dir[PATH_MAX];
-	char mnt_type[PATH_MAX];
-
-	FILE *fp = fopen("/proc/mounts", "r");
-	if (!fp)
-		return -EINVAL;
-
-	while (1) {
-		if (fscanf(fp, "%*s %s %s %*s %*s %*s", mnt_dir, mnt_type) <= 0)
-			return -ENOENT;
-
-		if (!strcmp(mnt_type, "debugfs")) {
-			strcpy(mntdir, mnt_dir);
-			return 0;
-		}
-	}
-}
-
 /*
  * Separate option name to 3 fields
  * Ex:
@@ -559,19 +539,15 @@
 			if (getdebugfsmntdir(channel_root_default) == 0) {
 				strcat(channel_root_default, "/ltt");
 				opt_channel_root = channel_root_default;
+			} else {
+				fprintf(stderr,
+					"Channel_root is necessary for -w"
+					" option, but neither --channel_root"
+					" option\n"
+					"specified, nor debugfs's mount dir"
+					" found, mount debugfs also failed\n");
+				return -EINVAL;
 			}
-		/* Todo:
-		 * if (!opt_channel_root)
-		 *	if (auto_mount_debugfs_dir(channel_root_default) == 0)
-		 *		opt_channel_root = debugfs_dir_mnt_point;
-		 */
-		if (!opt_channel_root) {
-			fprintf(stderr,
-				"Channel_root is necessary for -w option,"
-				" but neither --channel_root option\n"
-				"specified, nor debugfs's mount dir found.\n");
-			return -EINVAL;
-		}
 
 		if (opt_dump_threads == 0)
 			opt_dump_threads = 1;

[ltt-dev] [PATCH 2/2] Make lttctl load ltt-trace-control module automatically

[Zhaolei]

lttctl need to use ltt-trace-control module.
This patch make lttctl try to load ltt-trace-control module automatically when
ltt-trace-control is compiled as a module and not loaded.

Applies on ltt-control-0.63-03012009 with previous patch of:
[PATCH 1/2] Make lttctl mount debugfs automatically

Signed-off-by: Zhao lei <zhaolei at cn.fujitsu.com>
---
 liblttctl.c |   51 +++++++++++++++++++++++++++++++++++++++------------
 1 file changed, 39 insertions(+), 12 deletions(-)
diff -Nur ltt-control-0.63-03012009.org/liblttctl/liblttctl.c ltt-control-0.63-03012009.new/liblttctl/liblttctl.c
--- ltt-control-0.63-03012009.org/liblttctl/liblttctl.c	2009-01-07 15:54:45.000000000 +0800
+++ ltt-control-0.63-03012009.new/liblttctl/liblttctl.c	2009-01-07 15:54:49.000000000 +0800
@@ -42,11 +42,44 @@
 	return getdebugfsmntdir(debugfsmntdir);
 }
 
+/*
+ * This function must called posterior to initdebugfsmntdir(),
+ * because it need to use debugfsmntdir[] which is inited in initdebugfsmntdir()
+ */
+static int initmodule(void)
+{
+	char controldirname[PATH_MAX];
+	DIR *dir;
+	int tryload_done = 0;
+
+	sprintf(controldirname, "%s/ltt/control/", debugfsmntdir);
+
+check_again:
+	/*
+	 * Check ltt control's debugfs dir
+	 *
+	 * We don't check is ltt-trace-control module exist, because it maybe
+	 * compiled into kernel.
+	 */
+	dir = opendir(controldirname);
+	if (dir) {
+		closedir(dir);
+		return 0;
+	}
+
+	if (!tryload_done) {
+		system("modprobe ltt-trace-control");
+		tryload_done = 1;
+		goto check_again;
+	}
+
+	return -ENOENT;
+}
+
 int lttctl_init(void)
 {
 	int ret;
-	DIR *dir;
-	char controldirname[PATH_MAX];
+
 
 	ret = initdebugfsmntdir();
 	if (ret) {
@@ -54,18 +87,12 @@
 		return 1;
 	}
 
-	/* check ltt control's debugfs dir */
-	sprintf(controldirname, "%s/ltt/control/", debugfsmntdir);
-
-	dir = opendir(controldirname);
-	if (!dir) {
-		fprintf(stderr, "ltt-trace-control's debugfs dir not found\n");
-		closedir(dir);
-		return -errno;
+	ret = initmodule();
+	if (ret) {
+		fprintf(stderr, "Control module seems not work\n");
+		return 1;
 	}
 
-	closedir(dir);
-
 	return 0;
 }
 

[ltt-dev] [PATCH 2/2] Make lttctl load ltt-trace-control module automatically

[Mathieu Desnoyers]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> lttctl need to use ltt-trace-control module.
> This patch make lttctl try to load ltt-trace-control module automatically when
> ltt-trace-control is compiled as a module and not loaded.
> 
> Applies on ltt-control-0.63-03012009 with previous patch of:
> [PATCH 1/2] Make lttctl mount debugfs automatically
> 

Applied to ltt-control 0.64, thanks !

Mathieu

> Signed-off-by: Zhao lei <zhaolei at cn.fujitsu.com>
> ---
>  liblttctl.c |   51 +++++++++++++++++++++++++++++++++++++++------------
>  1 file changed, 39 insertions(+), 12 deletions(-)
> diff -Nur ltt-control-0.63-03012009.org/liblttctl/liblttctl.c ltt-control-0.63-03012009.new/liblttctl/liblttctl.c
> --- ltt-control-0.63-03012009.org/liblttctl/liblttctl.c	2009-01-07 15:54:45.000000000 +0800
> +++ ltt-control-0.63-03012009.new/liblttctl/liblttctl.c	2009-01-07 15:54:49.000000000 +0800
> @@ -42,11 +42,44 @@
>  	return getdebugfsmntdir(debugfsmntdir);
>  }
>  
> +/*
> + * This function must called posterior to initdebugfsmntdir(),
> + * because it need to use debugfsmntdir[] which is inited in initdebugfsmntdir()
> + */
> +static int initmodule(void)
> +{
> +	char controldirname[PATH_MAX];
> +	DIR *dir;
> +	int tryload_done = 0;
> +
> +	sprintf(controldirname, "%s/ltt/control/", debugfsmntdir);
> +
> +check_again:
> +	/*
> +	 * Check ltt control's debugfs dir
> +	 *
> +	 * We don't check is ltt-trace-control module exist, because it maybe
> +	 * compiled into kernel.
> +	 */
> +	dir = opendir(controldirname);
> +	if (dir) {
> +		closedir(dir);
> +		return 0;
> +	}
> +
> +	if (!tryload_done) {
> +		system("modprobe ltt-trace-control");
> +		tryload_done = 1;
> +		goto check_again;
> +	}
> +
> +	return -ENOENT;
> +}
> +
>  int lttctl_init(void)
>  {
>  	int ret;
> -	DIR *dir;
> -	char controldirname[PATH_MAX];
> +
>  
>  	ret = initdebugfsmntdir();
>  	if (ret) {
> @@ -54,18 +87,12 @@
>  		return 1;
>  	}
>  
> -	/* check ltt control's debugfs dir */
> -	sprintf(controldirname, "%s/ltt/control/", debugfsmntdir);
> -
> -	dir = opendir(controldirname);
> -	if (!dir) {
> -		fprintf(stderr, "ltt-trace-control's debugfs dir not found\n");
> -		closedir(dir);
> -		return -errno;
> +	ret = initmodule();
> +	if (ret) {
> +		fprintf(stderr, "Control module seems not work\n");
> +		return 1;
>  	}
>  
> -	closedir(dir);
> -
>  	return 0;
>  }
>  
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Pierre-Marc Fournier]

Zhaolei wrote:
> lttctl need debugfs mounted,
> This patch make lttctl mount debugfs automatically when it is not mounted.
> 

Isn't having a utility such as lttctl mount filesystems against standard
unix semantics? Can you give an example of another standard command that
uses a filesystem and that tries to mount it if it isn't already
mounted? My guess is we should warn about the filesystem not being
mounted, but not try to call mount() directly.

> Little cleanup also done(combile 2 getdebugfsmntdir() into one).
> 
> Applies on top of ltt-control-0.63-03012009.
> 

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Zhaolei]

* From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> Zhaolei wrote:
>> lttctl need debugfs mounted,
>> This patch make lttctl mount debugfs automatically when it is not mounted.
>> 
> 
> Isn't having a utility such as lttctl mount filesystems against standard
> unix semantics? Can you give an example of another standard command that
> uses a filesystem and that tries to mount it if it isn't already
> mounted? 
Hello, Pierre-Marc,

I can give a an example.
It is SystemTap.

Regards
Zhaolei

> My guess is we should warn about the filesystem not being
> mounted, but not try to call mount() directly.
> 
>> Little cleanup also done(combile 2 getdebugfsmntdir() into one).
>> 
>> Applies on top of ltt-control-0.63-03012009.
>> 
> 
>

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Mathieu Desnoyers]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> > Zhaolei wrote:
> >> lttctl need debugfs mounted,
> >> This patch make lttctl mount debugfs automatically when it is not mounted.
> >> 
> > 
> > Isn't having a utility such as lttctl mount filesystems against standard
> > unix semantics? Can you give an example of another standard command that
> > uses a filesystem and that tries to mount it if it isn't already
> > mounted? 
> Hello, Pierre-Marc,
> 
> I can give a an example.
> It is SystemTap.
> 

No offence, but can you find an example which is not explicitly
described as "bad taste" example by many people in the Linux kernel
community ?

Mathieu

> Regards
> Zhaolei
> 
> > My guess is we should warn about the filesystem not being
> > mounted, but not try to call mount() directly.
> > 
> >> Little cleanup also done(combile 2 getdebugfsmntdir() into one).
> >> 
> >> Applies on top of ltt-control-0.63-03012009.
> >> 
> > 
> >
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Zhaolei]

* From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
>* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
>> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
>> > Zhaolei wrote:
>> >> lttctl need debugfs mounted,
>> >> This patch make lttctl mount debugfs automatically when it is not mounted.
>> >> 
>> > 
>> > Isn't having a utility such as lttctl mount filesystems against standard
>> > unix semantics? Can you give an example of another standard command that
>> > uses a filesystem and that tries to mount it if it isn't already
>> > mounted? 
>> Hello, Pierre-Marc,
>> 
>> I can give a an example.
>> It is SystemTap.
>> 
> 
> No offence, but can you find an example which is not explicitly
> described as "bad taste" example by many people in the Linux kernel
> community ?
Hello, Mathieu

I can't find another example of mounting debugfs automatically.
Maybe it is because debugfs is not widely used as others such as sysfs and
procfs.

sysfs and procfs are mounted in boot, and user-mode tools need not check and
mount then again. But debugfs is not too useful for normal user who just use
linux to edit text and browse internet, so mount debugfs in boot period is not
a good idea.
So, IMHO, debugfs's usage is not same as sysfs and procfs, we need not mount
sysfs and procfs in user-mode program, but maybe it is different for debugfs.

On the other hand, I also understand your opinion, mount a fs in program looks
strange(but can make user easy to use...), so you can feel free to ignore this
patch.
Same with [PATCH 2/2] Make lttctl load ltt-trace-control module automatically.

Regards
Zhaolei

> 
> Mathieu
> 
>> Regards
>> Zhaolei
>> 
>> > My guess is we should warn about the filesystem not being
>> > mounted, but not try to call mount() directly.
>> > 
>> >> Little cleanup also done(combile 2 getdebugfsmntdir() into one).
>> >> 
>> >> Applies on top of ltt-control-0.63-03012009.
>> >> 
>> > 
>> >
>> _______________________________________________
>> ltt-dev mailing list
>> ltt-dev at lists.casi.polymtl.ca
>> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
>> 
> 
> -- 
> Mathieu Desnoyers
> OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> 
>

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Mathieu Desnoyers]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> >> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> >> > Zhaolei wrote:
> >> >> lttctl need debugfs mounted,
> >> >> This patch make lttctl mount debugfs automatically when it is not mounted.
> >> >> 
> >> > 
> >> > Isn't having a utility such as lttctl mount filesystems against standard
> >> > unix semantics? Can you give an example of another standard command that
> >> > uses a filesystem and that tries to mount it if it isn't already
> >> > mounted? 
> >> Hello, Pierre-Marc,
> >> 
> >> I can give a an example.
> >> It is SystemTap.
> >> 
> > 
> > No offence, but can you find an example which is not explicitly
> > described as "bad taste" example by many people in the Linux kernel
> > community ?
> Hello, Mathieu
> 
> I can't find another example of mounting debugfs automatically.
> Maybe it is because debugfs is not widely used as others such as sysfs and
> procfs.
> 
> sysfs and procfs are mounted in boot, and user-mode tools need not check and
> mount then again. But debugfs is not too useful for normal user who just use
> linux to edit text and browse internet, so mount debugfs in boot period is not
> a good idea.
> So, IMHO, debugfs's usage is not same as sysfs and procfs, we need not mount
> sysfs and procfs in user-mode program, but maybe it is different for debugfs.
> 
> On the other hand, I also understand your opinion, mount a fs in program looks
> strange(but can make user easy to use...), so you can feel free to ignore this
> patch.
> Same with [PATCH 2/2] Make lttctl load ltt-trace-control module automatically.
> 

I wonder what's Greg KH (and others) opinion about this ?

Mathieu


> Regards
> Zhaolei
> 
> > 
> > Mathieu
> > 
> >> Regards
> >> Zhaolei
> >> 
> >> > My guess is we should warn about the filesystem not being
> >> > mounted, but not try to call mount() directly.
> >> > 
> >> >> Little cleanup also done(combile 2 getdebugfsmntdir() into one).
> >> >> 
> >> >> Applies on top of ltt-control-0.63-03012009.
> >> >> 
> >> > 
> >> >
> >> _______________________________________________
> >> ltt-dev mailing list
> >> ltt-dev at lists.casi.polymtl.ca
> >> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> >> 
> > 
> > -- 
> > Mathieu Desnoyers
> > OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68
> > 
> >
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Greg KH]

On Thu, Jan 08, 2009 at 10:30:22PM -0500, Mathieu Desnoyers wrote:
> * Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> > >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > >> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> > >> > Zhaolei wrote:
> > >> >> lttctl need debugfs mounted,
> > >> >> This patch make lttctl mount debugfs automatically when it is not mounted.
> > >> >> 
> > >> > 
> > >> > Isn't having a utility such as lttctl mount filesystems against standard
> > >> > unix semantics? Can you give an example of another standard command that
> > >> > uses a filesystem and that tries to mount it if it isn't already
> > >> > mounted? 
> > >> Hello, Pierre-Marc,
> > >> 
> > >> I can give a an example.
> > >> It is SystemTap.
> > >> 
> > > 
> > > No offence, but can you find an example which is not explicitly
> > > described as "bad taste" example by many people in the Linux kernel
> > > community ?
> > Hello, Mathieu
> > 
> > I can't find another example of mounting debugfs automatically.

Have you looked?  openSUSE mounts it automatically, and I think Fedora
also does so.

thanks,

greg k-h

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Josh Boyer]

On Tue, Jan 13, 2009 at 03:54:19PM -0800, Greg KH wrote:
>On Thu, Jan 08, 2009 at 10:30:22PM -0500, Mathieu Desnoyers wrote:
>> * Zhaolei (zhaolei at cn.fujitsu.com) wrote:
>> > * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
>> > >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
>> > >> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
>> > >> > Zhaolei wrote:
>> > >> >> lttctl need debugfs mounted,
>> > >> >> This patch make lttctl mount debugfs automatically when it is not mounted.
>> > >> >> 
>> > >> > 
>> > >> > Isn't having a utility such as lttctl mount filesystems against standard
>> > >> > unix semantics? Can you give an example of another standard command that
>> > >> > uses a filesystem and that tries to mount it if it isn't already
>> > >> > mounted? 
>> > >> Hello, Pierre-Marc,
>> > >> 
>> > >> I can give a an example.
>> > >> It is SystemTap.
>> > >> 
>> > > 
>> > > No offence, but can you find an example which is not explicitly
>> > > described as "bad taste" example by many people in the Linux kernel
>> > > community ?
>> > Hello, Mathieu
>> > 
>> > I can't find another example of mounting debugfs automatically.
>
>Have you looked?  openSUSE mounts it automatically, and I think Fedora
>also does so.

Fedora doesn't.  At least not with the last 3 stable releases.

josh

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Mathieu Desnoyers]

* Greg KH (gregkh at suse.de) wrote:
> On Thu, Jan 08, 2009 at 10:30:22PM -0500, Mathieu Desnoyers wrote:
> > * Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > > * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> > > >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > > >> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> > > >> > Zhaolei wrote:
> > > >> >> lttctl need debugfs mounted,
> > > >> >> This patch make lttctl mount debugfs automatically when it is not mounted.
> > > >> >> 
> > > >> > 
> > > >> > Isn't having a utility such as lttctl mount filesystems against standard
> > > >> > unix semantics? Can you give an example of another standard command that
> > > >> > uses a filesystem and that tries to mount it if it isn't already
> > > >> > mounted? 
> > > >> Hello, Pierre-Marc,
> > > >> 
> > > >> I can give a an example.
> > > >> It is SystemTap.
> > > >> 
> > > > 
> > > > No offence, but can you find an example which is not explicitly
> > > > described as "bad taste" example by many people in the Linux kernel
> > > > community ?
> > > Hello, Mathieu
> > > 
> > > I can't find another example of mounting debugfs automatically.
> 
> Have you looked?  openSUSE mounts it automatically, and I think Fedora
> also does so.
> 

It makes sense for a whole distribution to mount debugfs automatically,
but is it OK for a single userspace application to have this kind of
behavior ?

Mathieu

> thanks,
> 
> greg k-h
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Greg KH]

On Wed, Jan 14, 2009 at 12:16:02AM -0500, Mathieu Desnoyers wrote:
> * Greg KH (gregkh at suse.de) wrote:
> > On Thu, Jan 08, 2009 at 10:30:22PM -0500, Mathieu Desnoyers wrote:
> > > * Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > > > * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> > > > >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > > > >> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> > > > >> > Zhaolei wrote:
> > > > >> >> lttctl need debugfs mounted,
> > > > >> >> This patch make lttctl mount debugfs automatically when it is not mounted.
> > > > >> >> 
> > > > >> > 
> > > > >> > Isn't having a utility such as lttctl mount filesystems against standard
> > > > >> > unix semantics? Can you give an example of another standard command that
> > > > >> > uses a filesystem and that tries to mount it if it isn't already
> > > > >> > mounted? 
> > > > >> Hello, Pierre-Marc,
> > > > >> 
> > > > >> I can give a an example.
> > > > >> It is SystemTap.
> > > > >> 
> > > > > 
> > > > > No offence, but can you find an example which is not explicitly
> > > > > described as "bad taste" example by many people in the Linux kernel
> > > > > community ?
> > > > Hello, Mathieu
> > > > 
> > > > I can't find another example of mounting debugfs automatically.
> > 
> > Have you looked?  openSUSE mounts it automatically, and I think Fedora
> > also does so.
> > 
> 
> It makes sense for a whole distribution to mount debugfs automatically,
> but is it OK for a single userspace application to have this kind of
> behavior ?

If it's a "debug" application, why not?

thanks,

greg k-h

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Mathieu Desnoyers]

* Greg KH (gregkh at suse.de) wrote:
> On Wed, Jan 14, 2009 at 12:16:02AM -0500, Mathieu Desnoyers wrote:
> > * Greg KH (gregkh at suse.de) wrote:
> > > On Thu, Jan 08, 2009 at 10:30:22PM -0500, Mathieu Desnoyers wrote:
> > > > * Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > > > > * From: "Mathieu Desnoyers" <compudj@krystal.dyndns.org>
> > > > > >* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> > > > > >> * From: "Pierre-Marc Fournier" <pierre-marc.fournier@polymtl.ca>
> > > > > >> > Zhaolei wrote:
> > > > > >> >> lttctl need debugfs mounted,
> > > > > >> >> This patch make lttctl mount debugfs automatically when it is not mounted.
> > > > > >> >> 
> > > > > >> > 
> > > > > >> > Isn't having a utility such as lttctl mount filesystems against standard
> > > > > >> > unix semantics? Can you give an example of another standard command that
> > > > > >> > uses a filesystem and that tries to mount it if it isn't already
> > > > > >> > mounted? 
> > > > > >> Hello, Pierre-Marc,
> > > > > >> 
> > > > > >> I can give a an example.
> > > > > >> It is SystemTap.
> > > > > >> 
> > > > > > 
> > > > > > No offence, but can you find an example which is not explicitly
> > > > > > described as "bad taste" example by many people in the Linux kernel
> > > > > > community ?
> > > > > Hello, Mathieu
> > > > > 
> > > > > I can't find another example of mounting debugfs automatically.
> > > 
> > > Have you looked?  openSUSE mounts it automatically, and I think Fedora
> > > also does so.
> > > 
> > 
> > It makes sense for a whole distribution to mount debugfs automatically,
> > but is it OK for a single userspace application to have this kind of
> > behavior ?
> 
> If it's a "debug" application, why not?
> 

Yep, I'll merge his patch in ltt-control then. Thanks for the feedback.

Mathieu

> thanks,
> 
> greg k-h
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

[ltt-dev] [PATCH 1/2] Make lttctl mount debugfs automatically

[Mathieu Desnoyers]

* Zhaolei (zhaolei at cn.fujitsu.com) wrote:
> lttctl need debugfs mounted,
> This patch make lttctl mount debugfs automatically when it is not mounted.
> 
> Little cleanup also done(combile 2 getdebugfsmntdir() into one).
> 
> Applies on top of ltt-control-0.63-03012009.
> 

Applied to ltt-control 0.64, thanks !

Mathieu

> Signed-off-by: Zhao lei <zhaolei at cn.fujitsu.com>
> ---
>  liblttctl/liblttctl.c |   54 +++++++++++++++++++++++++++++---------------------
>  liblttctl/lttctl.h    |    3 ++
>  lttctl/lttctl.c       |   40 +++++++------------------------------
>  3 files changed, 43 insertions(+), 54 deletions(-)
> diff -Nur ltt-control-0.63-03012009.org/liblttctl/liblttctl.c ltt-control-0.63-03012009.new/liblttctl/liblttctl.c
> --- ltt-control-0.63-03012009.org/liblttctl/liblttctl.c	2009-01-07 09:52:05.000000000 +0800
> +++ ltt-control-0.63-03012009.new/liblttctl/liblttctl.c	2009-01-07 14:48:18.000000000 +0800
> @@ -39,27 +39,7 @@
>  
>  static int initdebugfsmntdir(void)
>  {
> -	char mnt_dir[PATH_MAX];
> -	char mnt_type[PATH_MAX];
> -
> -	FILE *fp = fopen("/proc/mounts", "r");
> -	if (!fp) {
> -		fprintf(stderr, "%s: Can't open /proc/mounts\n", __func__);
> -		return 1;
> -	}
> -
> -	while (1) {
> -		if (fscanf(fp, "%*s %s %s %*s %*s %*s", mnt_dir, mnt_type)
> -			<= 0) {
> -			fprintf(stderr, "%s: debugfs mountpoint not found\n",
> -				__func__);
> -			return 1;
> -		}
> -		if (!strcmp(mnt_type, "debugfs")) {
> -			strcpy(debugfsmntdir, mnt_dir);
> -			return 0;
> -		}
> -	}
> +	return getdebugfsmntdir(debugfsmntdir);
>  }
>  
>  int lttctl_init(void)
> @@ -70,7 +50,7 @@
>  
>  	ret = initdebugfsmntdir();
>  	if (ret) {
> -		fprintf(stderr, "Debugfs mount point not found\n");
> +		fprintf(stderr, "Get debugfs mount point failed\n");
>  		return 1;
>  	}
>  
> @@ -664,3 +644,33 @@
>  arg_error:
>  	return ret;
>  }
> +
> +int getdebugfsmntdir(char *mntdir)
> +{
> +	char mnt_dir[PATH_MAX];
> +	char mnt_type[PATH_MAX];
> +	int trymount_done = 0;
> +
> +	FILE *fp = fopen("/proc/mounts", "r");
> +	if (!fp)
> +		return -EINVAL;
> +
> +find_again:
> +	while (1) {
> +		if (fscanf(fp, "%*s %s %s %*s %*s %*s", mnt_dir, mnt_type) <= 0)
> +			break;
> +
> +		if (!strcmp(mnt_type, "debugfs")) {
> +			strcpy(mntdir, mnt_dir);
> +			return 0;
> +		}
> +	}
> +
> +	if (!trymount_done) {
> +		mount("debugfs", "/sys/kernel/debug/", "debugfs", 0, NULL);
> +		trymount_done = 1;
> +		goto find_again;
> +	}
> +
> +	return -ENOENT;
> +}
> diff -Nur ltt-control-0.63-03012009.org/liblttctl/lttctl.h ltt-control-0.63-03012009.new/liblttctl/lttctl.h
> --- ltt-control-0.63-03012009.org/liblttctl/lttctl.h	2009-01-07 09:52:05.000000000 +0800
> +++ ltt-control-0.63-03012009.new/liblttctl/lttctl.h	2009-01-07 14:48:18.000000000 +0800
> @@ -36,4 +36,7 @@
>  int lttctl_set_channel_subbuf_size(const char *name, const char *channel,
>  		unsigned subbuf_size);
>  
> +/* Helper functions */
> +int getdebugfsmntdir(char *mntdir);
> +
>  #endif /*_LIBLTT_H */
> diff -Nur ltt-control-0.63-03012009.org/lttctl/lttctl.c ltt-control-0.63-03012009.new/lttctl/lttctl.c
> --- ltt-control-0.63-03012009.org/lttctl/lttctl.c	2009-01-07 09:52:05.000000000 +0800
> +++ ltt-control-0.63-03012009.new/lttctl/lttctl.c	2009-01-07 14:48:18.000000000 +0800
> @@ -142,26 +142,6 @@
>  	printf("\n");
>  }
>  
> -static int getdebugfsmntdir(char *mntdir)
> -{
> -	char mnt_dir[PATH_MAX];
> -	char mnt_type[PATH_MAX];
> -
> -	FILE *fp = fopen("/proc/mounts", "r");
> -	if (!fp)
> -		return -EINVAL;
> -
> -	while (1) {
> -		if (fscanf(fp, "%*s %s %s %*s %*s %*s", mnt_dir, mnt_type) <= 0)
> -			return -ENOENT;
> -
> -		if (!strcmp(mnt_type, "debugfs")) {
> -			strcpy(mntdir, mnt_dir);
> -			return 0;
> -		}
> -	}
> -}
> -
>  /*
>   * Separate option name to 3 fields
>   * Ex:
> @@ -559,19 +539,15 @@
>  			if (getdebugfsmntdir(channel_root_default) == 0) {
>  				strcat(channel_root_default, "/ltt");
>  				opt_channel_root = channel_root_default;
> +			} else {
> +				fprintf(stderr,
> +					"Channel_root is necessary for -w"
> +					" option, but neither --channel_root"
> +					" option\n"
> +					"specified, nor debugfs's mount dir"
> +					" found, mount debugfs also failed\n");
> +				return -EINVAL;
>  			}
> -		/* Todo:
> -		 * if (!opt_channel_root)
> -		 *	if (auto_mount_debugfs_dir(channel_root_default) == 0)
> -		 *		opt_channel_root = debugfs_dir_mnt_point;
> -		 */
> -		if (!opt_channel_root) {
> -			fprintf(stderr,
> -				"Channel_root is necessary for -w option,"
> -				" but neither --channel_root option\n"
> -				"specified, nor debugfs's mount dir found.\n");
> -			return -EINVAL;
> -		}
>  
>  		if (opt_dump_threads == 0)
>  			opt_dump_threads = 1;
> 
> 
> _______________________________________________
> ltt-dev mailing list
> ltt-dev at lists.casi.polymtl.ca
> http://lists.casi.polymtl.ca/cgi-bin/mailman/listinfo/ltt-dev
> 

-- 
Mathieu Desnoyers
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68