There is race issue when concurrently splice_read main trace_pipe and per_cpu trace_pipes which will result in data read out being different from what actually writen. As suggested by Steven: > I believe we should add a ref count to trace_pipe and the per_cpu > trace_pipes, where if they are opened, nothing else can read it. > > Opening trace_pipe locks all per_cpu ref counts, if any of them are > open, then the trace_pipe open will fail (and releases any ref counts > it had taken). > > Opening a per_cpu trace_pipe will up the ref count for just that > CPU buffer. This will allow multiple tasks to read different per_cpu > trace_pipe files, but will prevent the main trace_pipe file from > being opened. But because we only need to know whether per_cpu trace_pipe is open or not, using a cpumask instead of using ref count may be easier. After this patch, users will find that: - Main trace_pipe can be opened by only one user, and if it is opened, all per_cpu trace_pipes cannot be opened; - Per_cpu trace_pipes can be opened by multiple users, but each per_cpu trace_pipe can only be opened by one user. And if one of them is opened, main trace_pipe cannot be opened. Link: https://lore.kernel.org/linux-trace-kernel/20230818022645.1948314-1-zhengyejian1@huawei.com
Showing the changes to kernel (2 of 2 files). Show all
Unified · Split
| @@ -6718,10 +6718,36 @@ tracing_max_lat_write(struct file *filp, const char __user *ubuf, · 6717 unmodified lines | |||
| 6718 | 6718 | ||
| 6719 | #endif | 6719 | #endif |
| 6720 | 6720 | ||
| 6721 | static int open_pipe_on_cpu(struct trace_array *tr, int cpu) | ||
| 6722 | { | ||
| 6723 | if (cpu == RING_BUFFER_ALL_CPUS) { | ||
| 6724 | if (cpumask_empty(tr->pipe_cpumask)) { | ||
| 6725 | cpumask_setall(tr->pipe_cpumask); | ||
| 6726 | return 0; | ||
| 6727 | } | ||
| 6728 | } else if (!cpumask_test_cpu(cpu, tr->pipe_cpumask)) { | ||
| 6729 | cpumask_set_cpu(cpu, tr->pipe_cpumask); | ||
| 6730 | return 0; | ||
| 6731 | } | ||
| 6732 | return -EBUSY; | ||
| 6733 | } | ||
| 6734 | |||
| 6735 | static void close_pipe_on_cpu(struct trace_array *tr, int cpu) | ||
| 6736 | { | ||
| 6737 | if (cpu == RING_BUFFER_ALL_CPUS) { | ||
| 6738 | WARN_ON(!cpumask_full(tr->pipe_cpumask)); | ||
| 6739 | cpumask_clear(tr->pipe_cpumask); | ||
| 6740 | } else { | ||
| 6741 | WARN_ON(!cpumask_test_cpu(cpu, tr->pipe_cpumask)); | ||
| 6742 | cpumask_clear_cpu(cpu, tr->pipe_cpumask); | ||
| 6743 | } | ||
| 6744 | } | ||
| 6745 | |||
| 6721 | static int tracing_open_pipe(struct inode *inode, struct file *filp) | 6746 | static int tracing_open_pipe(struct inode *inode, struct file *filp) |
| 6722 | { | 6747 | { |
| 6723 | struct trace_array *tr = inode->i_private; | 6748 | struct trace_array *tr = inode->i_private; |
| 6724 | struct trace_iterator *iter; | 6749 | struct trace_iterator *iter; |
| 6750 | int cpu; | ||
| 6725 | int ret; | 6751 | int ret; |
| 6726 | 6752 | ||
| 6727 | ret = tracing_check_open_get_tr(tr); | 6753 | ret = tracing_check_open_get_tr(tr); |
| @@ -6729,13 +6755,16 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp) · 1 unmodified line | |||
| 6729 | return ret; | 6755 | return ret; |
| 6730 | 6756 | ||
| 6731 | mutex_lock(&trace_types_lock); | 6757 | mutex_lock(&trace_types_lock); |
| 6758 | cpu = tracing_get_cpu(inode); | ||
| 6759 | ret = open_pipe_on_cpu(tr, cpu); | ||
| 6760 | if (ret) | ||
| 6761 | goto fail_pipe_on_cpu; | ||
| 6732 | 6762 | ||
| 6733 | /* create a buffer to store the information to pass to userspace */ | 6763 | /* create a buffer to store the information to pass to userspace */ |
| 6734 | iter = kzalloc(sizeof(*iter), GFP_KERNEL); | 6764 | iter = kzalloc(sizeof(*iter), GFP_KERNEL); |
| 6735 | if (!iter) { | 6765 | if (!iter) { |
| 6736 | ret = -ENOMEM; | 6766 | ret = -ENOMEM; |
| 6737 | __trace_array_put(tr); | 6767 | goto fail_alloc_iter; |
| 6738 | goto out; | ||
| 6739 | } | 6768 | } |
| 6740 | 6769 | ||
| 6741 | trace_seq_init(&iter->seq); | 6770 | trace_seq_init(&iter->seq); |
| @@ -6758,7 +6787,7 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp) · 16 unmodified lines | |||
| 6758 | 6787 | ||
| 6759 | iter->tr = tr; | 6788 | iter->tr = tr; |
| 6760 | iter->array_buffer = &tr->array_buffer; | 6789 | iter->array_buffer = &tr->array_buffer; |
| 6761 | iter->cpu_file = tracing_get_cpu(inode); | 6790 | iter->cpu_file = cpu; |
| 6762 | mutex_init(&iter->mutex); | 6791 | mutex_init(&iter->mutex); |
| 6763 | filp->private_data = iter; | 6792 | filp->private_data = iter; |
| 6764 | 6793 | ||
| @@ -6768,12 +6797,15 @@ static int tracing_open_pipe(struct inode *inode, struct file *filp) · 3 unmodified lines | |||
| 6768 | nonseekable_open(inode, filp); | 6797 | nonseekable_open(inode, filp); |
| 6769 | 6798 | ||
| 6770 | tr->trace_ref++; | 6799 | tr->trace_ref++; |
| 6771 | out: | 6800 | |
| 6772 | mutex_unlock(&trace_types_lock); | 6801 | mutex_unlock(&trace_types_lock); |
| 6773 | return ret; | 6802 | return ret; |
| 6774 | 6803 | ||
| 6775 | fail: | 6804 | fail: |
| 6776 | kfree(iter); | 6805 | kfree(iter); |
| 6806 | fail_alloc_iter: | ||
| 6807 | close_pipe_on_cpu(tr, cpu); | ||
| 6808 | fail_pipe_on_cpu: | ||
| 6777 | __trace_array_put(tr); | 6809 | __trace_array_put(tr); |
| 6778 | mutex_unlock(&trace_types_lock); | 6810 | mutex_unlock(&trace_types_lock); |
| 6779 | return ret; | 6811 | return ret; |
| @@ -6790,7 +6822,7 @@ static int tracing_release_pipe(struct inode *inode, struct file *file) · 10 unmodified lines | |||
| 6790 | 6822 | ||
| 6791 | if (iter->trace->pipe_close) | 6823 | if (iter->trace->pipe_close) |
| 6792 | iter->trace->pipe_close(iter); | 6824 | iter->trace->pipe_close(iter); |
| 6793 | 6825 | close_pipe_on_cpu(tr, iter->cpu_file); | |
| 6794 | mutex_unlock(&trace_types_lock); | 6826 | mutex_unlock(&trace_types_lock); |
| 6795 | 6827 | ||
| 6796 | free_cpumask_var(iter->started); | 6828 | free_cpumask_var(iter->started); |
| @@ -9454,6 +9486,9 @@ static struct trace_array *trace_array_create(const char *name) · 2657 unmodified lines | |||
| 9454 | if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL)) | 9486 | if (!alloc_cpumask_var(&tr->tracing_cpumask, GFP_KERNEL)) |
| 9455 | goto out_free_tr; | 9487 | goto out_free_tr; |
| 9456 | 9488 | ||
| 9489 | if (!alloc_cpumask_var(&tr->pipe_cpumask, GFP_KERNEL)) | ||
| 9490 | goto out_free_tr; | ||
| 9491 | |||
| 9457 | tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS; | 9492 | tr->trace_flags = global_trace.trace_flags & ~ZEROED_TRACE_FLAGS; |
| 9458 | 9493 | ||
| 9459 | cpumask_copy(tr->tracing_cpumask, cpu_all_mask); | 9494 | cpumask_copy(tr->tracing_cpumask, cpu_all_mask); |
| @@ -9495,6 +9530,7 @@ static struct trace_array *trace_array_create(const char *name) · 35 unmodified lines | |||
| 9495 | out_free_tr: | 9530 | out_free_tr: |
| 9496 | ftrace_free_ftrace_ops(tr); | 9531 | ftrace_free_ftrace_ops(tr); |
| 9497 | free_trace_buffers(tr); | 9532 | free_trace_buffers(tr); |
| 9533 | free_cpumask_var(tr->pipe_cpumask); | ||
| 9498 | free_cpumask_var(tr->tracing_cpumask); | 9534 | free_cpumask_var(tr->tracing_cpumask); |
| 9499 | kfree(tr->name); | 9535 | kfree(tr->name); |
| 9500 | kfree(tr); | 9536 | kfree(tr); |
| @@ -9597,6 +9633,7 @@ static int __remove_instance(struct trace_array *tr) · 96 unmodified lines | |||
| 9597 | } | 9633 | } |
| 9598 | kfree(tr->topts); | 9634 | kfree(tr->topts); |
| 9599 | 9635 | ||
| 9636 | free_cpumask_var(tr->pipe_cpumask); | ||
| 9600 | free_cpumask_var(tr->tracing_cpumask); | 9637 | free_cpumask_var(tr->tracing_cpumask); |
| 9601 | kfree(tr->name); | 9638 | kfree(tr->name); |
| 9602 | kfree(tr); | 9639 | kfree(tr); |
| @@ -10394,12 +10431,14 @@ __init static int tracer_alloc_buffers(void) · 791 unmodified lines | |||
| 10394 | if (trace_create_savedcmd() < 0) | 10431 | if (trace_create_savedcmd() < 0) |
| 10395 | goto out_free_temp_buffer; | 10432 | goto out_free_temp_buffer; |
| 10396 | 10433 | ||
| 10434 | if (!alloc_cpumask_var(&global_trace.pipe_cpumask, GFP_KERNEL)) | ||
| 10435 | goto out_free_savedcmd; | ||
| 10436 | |||
| 10397 | /* TODO: make the number of buffers hot pluggable with CPUS */ | 10437 | /* TODO: make the number of buffers hot pluggable with CPUS */ |
| 10398 | if (allocate_trace_buffers(&global_trace, ring_buf_size) < 0) { | 10438 | if (allocate_trace_buffers(&global_trace, ring_buf_size) < 0) { |
| 10399 | MEM_FAIL(1, "tracer: failed to allocate ring buffer!\n"); | 10439 | MEM_FAIL(1, "tracer: failed to allocate ring buffer!\n"); |
| 10400 | goto out_free_savedcmd; | 10440 | goto out_free_pipe_cpumask; |
| 10401 | } | 10441 | } |
| 10402 | |||
| 10403 | if (global_trace.buffer_disabled) | 10442 | if (global_trace.buffer_disabled) |
| 10404 | tracing_off(); | 10443 | tracing_off(); |
| 10405 | 10444 | ||
| @@ -10452,6 +10491,8 @@ __init static int tracer_alloc_buffers(void) · 46 unmodified lines | |||
| 10452 | 10491 | ||
| 10453 | return 0; | 10492 | return 0; |
| 10454 | 10493 | ||
| 10494 | out_free_pipe_cpumask: | ||
| 10495 | free_cpumask_var(global_trace.pipe_cpumask); | ||
| 10455 | out_free_savedcmd: | 10496 | out_free_savedcmd: |
| 10456 | free_saved_cmdlines_buffer(savedcmd); | 10497 | free_saved_cmdlines_buffer(savedcmd); |
| 10457 | out_free_temp_buffer: | 10498 | out_free_temp_buffer: |
| @@ -377,6 +377,8 @@ struct trace_array { · 376 unmodified lines | |||
| 377 | struct list_head events; | 377 | struct list_head events; |
| 378 | struct trace_event_file *trace_marker_file; | 378 | struct trace_event_file *trace_marker_file; |
| 379 | cpumask_var_t tracing_cpumask; /* only trace on set CPUs */ | 379 | cpumask_var_t tracing_cpumask; /* only trace on set CPUs */ |
| 380 | /* one per_cpu trace_pipe can be opened by only one user */ | ||
| 381 | cpumask_var_t pipe_cpumask; | ||
| 380 | int ref; | 382 | int ref; |
| 381 | int trace_ref; | 383 | int trace_ref; |
| 382 | #ifdef CONFIG_FUNCTION_TRACER | 384 | #ifdef CONFIG_FUNCTION_TRACER |