On 9/27/13 2:56 PM, Jeff Moyer wrote: > Jeff Mahoney <[email protected]> writes: > >> On 9/27/13 2:43 PM, Jeff Moyer wrote: >>> Jeff Mahoney <[email protected]> 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
signature.asc
Description: OpenPGP digital signature
