[Patch] Remove debugfs entries on bad path

[Patch] Remove debugfs entries on bad path

[Stefan Raspl]

debugfs directory entries for devices are not removed on bad path.
If so, subsequent attempts to start blktrace on the respective
devices will not succeed due to existing directories.

Signed-off-by: Stefan Raspl <stefan.raspl@linux.vnet.ibm.com>

---
 kernel/trace/blktrace.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -262,6 +262,7 @@ static void blk_trace_free(struct blk_tr
 {
 	debugfs_remove(bt->msg_file);
 	debugfs_remove(bt->dropped_file);
+	debugfs_remove(bt->dir);
 	relay_close(bt->rchan);
 	free_percpu(bt->sequence);
 	free_percpu(bt->msg_data);

[Patch] Remove debugfs entries on bad path

[Stefan Raspl]

debugfs directory entries for devices are not removed on bad path.
If so, subsequent attempts to start blktrace on the respective
devices will not succeed due to existing directories.

Signed-off-by: Stefan Raspl <stefan.raspl@linux.vnet.ibm.com>

---
 kernel/trace/blktrace.c |    1 +
 1 file changed, 1 insertion(+)

--- a/kernel/trace/blktrace.c
+++ b/kernel/trace/blktrace.c
@@ -262,6 +262,7 @@ static void blk_trace_free(struct blk_tr
 {
 	debugfs_remove(bt->msg_file);
 	debugfs_remove(bt->dropped_file);
+	debugfs_remove(bt->dir);
 	relay_close(bt->rchan);
 	free_percpu(bt->sequence);
 	free_percpu(bt->msg_data);

Re: [Patch] Remove debugfs entries on bad path

[Li Zefan]

A better title:
	[Patch] blktrace: remove debugfs entries on bad path

Stefan Raspl wrote:
> debugfs directory entries for devices are not removed on bad path.

Can you be more elaborate on how to reproduce this issue?

> If so, subsequent attempts to start blktrace on the respective
> devices will not succeed due to existing directories.
> 
> Signed-off-by: Stefan Raspl <stefan.raspl@linux.vnet.ibm.com>
> 
> ---
>  kernel/trace/blktrace.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -262,6 +262,7 @@ static void blk_trace_free(struct blk_tr
>  {
>  	debugfs_remove(bt->msg_file);
>  	debugfs_remove(bt->dropped_file);
> +	debugfs_remove(bt->dir);
>  	relay_close(bt->rchan);
>  	free_percpu(bt->sequence);
>  	free_percpu(bt->msg_data);
> 

Re: [Patch] Remove debugfs entries on bad path

[Li Zefan]

A better title:
	[Patch] blktrace: remove debugfs entries on bad path

Stefan Raspl wrote:
> debugfs directory entries for devices are not removed on bad path.

Can you be more elaborate on how to reproduce this issue?

> If so, subsequent attempts to start blktrace on the respective
> devices will not succeed due to existing directories.
> 
> Signed-off-by: Stefan Raspl <stefan.raspl@linux.vnet.ibm.com>
> 
> ---
>  kernel/trace/blktrace.c |    1 +
>  1 file changed, 1 insertion(+)
> 
> --- a/kernel/trace/blktrace.c
> +++ b/kernel/trace/blktrace.c
> @@ -262,6 +262,7 @@ static void blk_trace_free(struct blk_tr
>  {
>  	debugfs_remove(bt->msg_file);
>  	debugfs_remove(bt->dropped_file);
> +	debugfs_remove(bt->dir);
>  	relay_close(bt->rchan);
>  	free_percpu(bt->sequence);
>  	free_percpu(bt->msg_data);
> 

Re: [Patch] blktrace: remove debugfs entries on bad path

[Stefan Raspl]

Li Zefan wrote:
> A better title:
> 	[Patch] blktrace: remove debugfs entries on bad path
> 
> Stefan Raspl wrote:
>> debugfs directory entries for devices are not removed on bad path.
> 
> Can you be more elaborate on how to reproduce this issue?

It can happen for most bad pathes within do_blk_trace_setup(). One
way to trigger is to set the Vmalloc space via the respective kernel
parameter to a value that is so small that it will not suffice for
the 2MB per device and cpu that is required. For instance, set
vmalloc\x10M and start blktrace on a system with 2 cpus on 7 devices.
Depending on how much Vmalloc space is still free, the final 2 or 3
devices will fail with a message like this:

	BLKTRACESETUP(2) /dev/sdu failed: 5/Input/output error

Best regards,
Stefan Raspl

Re: [Patch] blktrace: remove debugfs entries on bad path

[Stefan Raspl]

Li Zefan wrote:
> A better title:
> 	[Patch] blktrace: remove debugfs entries on bad path
> 
> Stefan Raspl wrote:
>> debugfs directory entries for devices are not removed on bad path.
> 
> Can you be more elaborate on how to reproduce this issue?

It can happen for most bad pathes within do_blk_trace_setup(). One
way to trigger is to set the Vmalloc space via the respective kernel
parameter to a value that is so small that it will not suffice for
the 2MB per device and cpu that is required. For instance, set
vmalloc=10M and start blktrace on a system with 2 cpus on 7 devices.
Depending on how much Vmalloc space is still free, the final 2 or 3
devices will fail with a message like this:

	BLKTRACESETUP(2) /dev/sdu failed: 5/Input/output error

Best regards,
Stefan Raspl

Re: [Patch] blktrace: remove debugfs entries on bad path

[Li Zefan]

Stefan Raspl wrote:
> Li Zefan wrote:
>> A better title:
>> 	[Patch] blktrace: remove debugfs entries on bad path
>>
>> Stefan Raspl wrote:
>>> debugfs directory entries for devices are not removed on bad path.
>> Can you be more elaborate on how to reproduce this issue?
> 
> It can happen for most bad pathes within do_blk_trace_setup(). One

Now I see what you meant by "bad path". :)

I'd say "some failure pathes", but not "most".

> way to trigger is to set the Vmalloc space via the respective kernel
> parameter to a value that is so small that it will not suffice for
> the 2MB per device and cpu that is required. For instance, set
> vmalloc\x10M and start blktrace on a system with 2 cpus on 7 devices.
> Depending on how much Vmalloc space is still free, the final 2 or 3
> devices will fail with a message like this:
> 
> 	BLKTRACESETUP(2) /dev/sdu failed: 5/Input/output error
> 

Yes, this can happen under extreme memory stress.

Thanks for the fix:
	Acked-by: Li Zefan <lizf@cn.fujitsu.com>

Could you resend the patch to Ingo, with a better title and a better
changelog (but IMHO we don't need to explain how to set up a memory
stress to trigger this issue) and my ack tag? Now blktrace patches
go to Ingo.

Re: [Patch] blktrace: remove debugfs entries on bad path

[Li Zefan]

Stefan Raspl wrote:
> Li Zefan wrote:
>> A better title:
>> 	[Patch] blktrace: remove debugfs entries on bad path
>>
>> Stefan Raspl wrote:
>>> debugfs directory entries for devices are not removed on bad path.
>> Can you be more elaborate on how to reproduce this issue?
> 
> It can happen for most bad pathes within do_blk_trace_setup(). One

Now I see what you meant by "bad path". :)

I'd say "some failure pathes", but not "most".

> way to trigger is to set the Vmalloc space via the respective kernel
> parameter to a value that is so small that it will not suffice for
> the 2MB per device and cpu that is required. For instance, set
> vmalloc=10M and start blktrace on a system with 2 cpus on 7 devices.
> Depending on how much Vmalloc space is still free, the final 2 or 3
> devices will fail with a message like this:
> 
> 	BLKTRACESETUP(2) /dev/sdu failed: 5/Input/output error
> 

Yes, this can happen under extreme memory stress.

Thanks for the fix:
	Acked-by: Li Zefan <lizf@cn.fujitsu.com>

Could you resend the patch to Ingo, with a better title and a better
changelog (but IMHO we don't need to explain how to set up a memory
stress to trigger this issue) and my ack tag? Now blktrace patches
go to Ingo.