Re: [PATCH] blktrace: fix race with open trace files and directoryremoval

[Jeff Mahoney]

On 9/27/13 2:56 PM, Jeff Moyer wrote:
> Jeff Mahoney <jeffm@xxxxxxxx> writes:
> 
>> On 9/27/13 2:43 PM, Jeff Moyer wrote:
>>> Jeff Mahoney <jeffm@xxxxxxxx> writes:
>>>
>>>> There's a bug in the blktrace client where it will stop and tear down
>>>> all of the tracing instances for devices it's opened whether it
>>>> successfully completed the setup or not.
>>>>
>>>> By starting multiple blktrace processes on the same device, it's possible
>>>> to permanently disable blktrace on that device. The cause is that when
>>>> the first blktrace process to exit tears down the directory structure,
>>>> the trace files are still held open. Debugfs removes the dentries for the
>>>> open files just fine but the relay implementation doesn't remove the
>>>> dentries until all of the references to the file are dropped. This means
>>>> that if there are open files when debugfs_remove is called for the device
>>>> directory, the directory is not empty and can't be removed. Since the
>>>> shutdown of the blktrace structure xchg's the structure out, there's no
>>>> way to clean up the directory and any new blktrace processes will fail
>>>> to start because it can't create the directory.
>>>>
>>>> This patch adds a kref to blk_trace so that we can release it after the
>>>> initial reference as well as all of the references accumulated by the
>>>> relay files are dropped.
>>>
>>> Can't we just do proper unwinding of errors in the do_blktrace_setup
>>> function?  In other words, don't just blindly call blk_trace_free, but
>>> instead just undo anything we've done.
>>
>> No. It's not the setup that's causing the problem. It's one process
>> holding the trace files open while another process calls BLKTRACETEARDOWN.
> 
> Ah, right.  So, in that case I'd rather restrict the ioctl to just the
> process that setup the trace.  Jens, Tejun, any opinions?

We'd also need to check to see if the task that started the trace is
still around.

-Jeff

-- 
Jeff Mahoney
SUSE Labs

Attachment: signature.asc
Description: OpenPGP digital signature