Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

binder: fix redefinition of seq_file attributes

The patchset in [1] exported some definitions to binder_internal.h in
order to make the debugfs entries such as 'stats' and 'transaction_log'
available in a binderfs instance. However, the DEFINE_SHOW_ATTRIBUTE
macro expands into a static function/variable pair, which in turn get
redefined each time a source file includes this internal header.

This problem was made evident after a report from the kernel test robot
<lkp@intel.com> where several W=1 build warnings are seen in downstream
kernels. See the following example:

include/../drivers/android/binder_internal.h:111:23: warning: 'binder_stats_fops' defined but not used [-Wunused-const-variable=]
111 | DEFINE_SHOW_ATTRIBUTE(binder_stats);
| ^~~~~~~~~~~~
include/linux/seq_file.h:174:37: note: in definition of macro 'DEFINE_SHOW_ATTRIBUTE'
174 | static const struct file_operations __name ## _fops = { \
| ^~~~~~

This patch fixes the above issues by moving back the definitions into
binder.c and instead creates an array of the debugfs entries which is
more convenient to share with binderfs and iterate through.

[1] https://lore.kernel.org/all/20190903161655.107408-1-hridya@google.com/

Fixes: 0e13e452dafc ("binder: Add stats, state and transactions files")
Fixes: 03e2e07e3814 ("binder: Make transaction_log available in binderfs")
Reported-by: kernel test robot <lkp@intel.com>
Signed-off-by: Carlos Llamas <cmllamas@google.com>
Link: https://lore.kernel.org/r/20220701182041.2134313-1-cmllamas@google.com
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Carlos Llamas and committed by
Greg Kroah-Hartman
b7e241bb ffff4913

+100 -107
+79 -35
drivers/android/binder.c
··· 197 197 atomic_inc(&binder_stats.obj_created[type]); 198 198 } 199 199 200 - struct binder_transaction_log binder_transaction_log; 201 - struct binder_transaction_log binder_transaction_log_failed; 200 + struct binder_transaction_log_entry { 201 + int debug_id; 202 + int debug_id_done; 203 + int call_type; 204 + int from_proc; 205 + int from_thread; 206 + int target_handle; 207 + int to_proc; 208 + int to_thread; 209 + int to_node; 210 + int data_size; 211 + int offsets_size; 212 + int return_error_line; 213 + uint32_t return_error; 214 + uint32_t return_error_param; 215 + char context_name[BINDERFS_MAX_NAME + 1]; 216 + }; 217 + 218 + struct binder_transaction_log { 219 + atomic_t cur; 220 + bool full; 221 + struct binder_transaction_log_entry entry[32]; 222 + }; 223 + 224 + static struct binder_transaction_log binder_transaction_log; 225 + static struct binder_transaction_log binder_transaction_log_failed; 202 226 203 227 static struct binder_transaction_log_entry *binder_transaction_log_add( 204 228 struct binder_transaction_log *log) ··· 6300 6276 print_binder_stats(m, " ", &proc->stats); 6301 6277 } 6302 6278 6303 - 6304 - int binder_state_show(struct seq_file *m, void *unused) 6279 + static int state_show(struct seq_file *m, void *unused) 6305 6280 { 6306 6281 struct binder_proc *proc; 6307 6282 struct binder_node *node; ··· 6339 6316 return 0; 6340 6317 } 6341 6318 6342 - int binder_stats_show(struct seq_file *m, void *unused) 6319 + static int stats_show(struct seq_file *m, void *unused) 6343 6320 { 6344 6321 struct binder_proc *proc; 6345 6322 ··· 6355 6332 return 0; 6356 6333 } 6357 6334 6358 - int binder_transactions_show(struct seq_file *m, void *unused) 6335 + static int transactions_show(struct seq_file *m, void *unused) 6359 6336 { 6360 6337 struct binder_proc *proc; 6361 6338 ··· 6411 6388 "\n" : " (incomplete)\n"); 6412 6389 } 6413 6390 6414 - int binder_transaction_log_show(struct seq_file *m, void *unused) 6391 + static int transaction_log_show(struct seq_file *m, void *unused) 6415 6392 { 6416 6393 struct binder_transaction_log *log = m->private; 6417 6394 unsigned int log_cur = atomic_read(&log->cur); ··· 6441 6418 .open = binder_open, 6442 6419 .flush = binder_flush, 6443 6420 .release = binder_release, 6421 + }; 6422 + 6423 + DEFINE_SHOW_ATTRIBUTE(state); 6424 + DEFINE_SHOW_ATTRIBUTE(stats); 6425 + DEFINE_SHOW_ATTRIBUTE(transactions); 6426 + DEFINE_SHOW_ATTRIBUTE(transaction_log); 6427 + 6428 + const struct binder_debugfs_entry binder_debugfs_entries[] = { 6429 + { 6430 + .name = "state", 6431 + .mode = 0444, 6432 + .fops = &state_fops, 6433 + .data = NULL, 6434 + }, 6435 + { 6436 + .name = "stats", 6437 + .mode = 0444, 6438 + .fops = &stats_fops, 6439 + .data = NULL, 6440 + }, 6441 + { 6442 + .name = "transactions", 6443 + .mode = 0444, 6444 + .fops = &transactions_fops, 6445 + .data = NULL, 6446 + }, 6447 + { 6448 + .name = "transaction_log", 6449 + .mode = 0444, 6450 + .fops = &transaction_log_fops, 6451 + .data = &binder_transaction_log, 6452 + }, 6453 + { 6454 + .name = "failed_transaction_log", 6455 + .mode = 0444, 6456 + .fops = &transaction_log_fops, 6457 + .data = &binder_transaction_log_failed, 6458 + }, 6459 + {} /* terminator */ 6444 6460 }; 6445 6461 6446 6462 static int __init init_binder_device(const char *name) ··· 6527 6465 atomic_set(&binder_transaction_log_failed.cur, ~0U); 6528 6466 6529 6467 binder_debugfs_dir_entry_root = debugfs_create_dir("binder", NULL); 6530 - if (binder_debugfs_dir_entry_root) 6468 + if (binder_debugfs_dir_entry_root) { 6469 + const struct binder_debugfs_entry *db_entry; 6470 + 6471 + binder_for_each_debugfs_entry(db_entry) 6472 + debugfs_create_file(db_entry->name, 6473 + db_entry->mode, 6474 + binder_debugfs_dir_entry_root, 6475 + db_entry->data, 6476 + db_entry->fops); 6477 + 6531 6478 binder_debugfs_dir_entry_proc = debugfs_create_dir("proc", 6532 6479 binder_debugfs_dir_entry_root); 6533 - 6534 - if (binder_debugfs_dir_entry_root) { 6535 - debugfs_create_file("state", 6536 - 0444, 6537 - binder_debugfs_dir_entry_root, 6538 - NULL, 6539 - &binder_state_fops); 6540 - debugfs_create_file("stats", 6541 - 0444, 6542 - binder_debugfs_dir_entry_root, 6543 - NULL, 6544 - &binder_stats_fops); 6545 - debugfs_create_file("transactions", 6546 - 0444, 6547 - binder_debugfs_dir_entry_root, 6548 - NULL, 6549 - &binder_transactions_fops); 6550 - debugfs_create_file("transaction_log", 6551 - 0444, 6552 - binder_debugfs_dir_entry_root, 6553 - &binder_transaction_log, 6554 - &binder_transaction_log_fops); 6555 - debugfs_create_file("failed_transaction_log", 6556 - 0444, 6557 - binder_debugfs_dir_entry_root, 6558 - &binder_transaction_log_failed, 6559 - &binder_transaction_log_fops); 6560 6480 } 6561 6481 6562 6482 if (!IS_ENABLED(CONFIG_ANDROID_BINDERFS) &&
+11 -35
drivers/android/binder_internal.h
··· 107 107 } 108 108 #endif 109 109 110 - int binder_stats_show(struct seq_file *m, void *unused); 111 - DEFINE_SHOW_ATTRIBUTE(binder_stats); 112 - 113 - int binder_state_show(struct seq_file *m, void *unused); 114 - DEFINE_SHOW_ATTRIBUTE(binder_state); 115 - 116 - int binder_transactions_show(struct seq_file *m, void *unused); 117 - DEFINE_SHOW_ATTRIBUTE(binder_transactions); 118 - 119 - int binder_transaction_log_show(struct seq_file *m, void *unused); 120 - DEFINE_SHOW_ATTRIBUTE(binder_transaction_log); 121 - 122 - struct binder_transaction_log_entry { 123 - int debug_id; 124 - int debug_id_done; 125 - int call_type; 126 - int from_proc; 127 - int from_thread; 128 - int target_handle; 129 - int to_proc; 130 - int to_thread; 131 - int to_node; 132 - int data_size; 133 - int offsets_size; 134 - int return_error_line; 135 - uint32_t return_error; 136 - uint32_t return_error_param; 137 - char context_name[BINDERFS_MAX_NAME + 1]; 110 + struct binder_debugfs_entry { 111 + const char *name; 112 + umode_t mode; 113 + const struct file_operations *fops; 114 + void *data; 138 115 }; 139 116 140 - struct binder_transaction_log { 141 - atomic_t cur; 142 - bool full; 143 - struct binder_transaction_log_entry entry[32]; 144 - }; 117 + extern const struct binder_debugfs_entry binder_debugfs_entries[]; 118 + 119 + #define binder_for_each_debugfs_entry(entry) \ 120 + for ((entry) = binder_debugfs_entries; \ 121 + (entry)->name; \ 122 + (entry)++) 145 123 146 124 enum binder_stat_types { 147 125 BINDER_STAT_PROC, ··· 558 580 }; 559 581 }; 560 582 561 - extern struct binder_transaction_log binder_transaction_log; 562 - extern struct binder_transaction_log binder_transaction_log_failed; 563 583 #endif /* _LINUX_BINDER_INTERNAL_H */
+10 -37
drivers/android/binderfs.c
··· 629 629 static int init_binder_logs(struct super_block *sb) 630 630 { 631 631 struct dentry *binder_logs_root_dir, *dentry, *proc_log_dir; 632 + const struct binder_debugfs_entry *db_entry; 632 633 struct binderfs_info *info; 633 634 int ret = 0; 634 635 ··· 640 639 goto out; 641 640 } 642 641 643 - dentry = binderfs_create_file(binder_logs_root_dir, "stats", 644 - &binder_stats_fops, NULL); 645 - if (IS_ERR(dentry)) { 646 - ret = PTR_ERR(dentry); 647 - goto out; 648 - } 649 - 650 - dentry = binderfs_create_file(binder_logs_root_dir, "state", 651 - &binder_state_fops, NULL); 652 - if (IS_ERR(dentry)) { 653 - ret = PTR_ERR(dentry); 654 - goto out; 655 - } 656 - 657 - dentry = binderfs_create_file(binder_logs_root_dir, "transactions", 658 - &binder_transactions_fops, NULL); 659 - if (IS_ERR(dentry)) { 660 - ret = PTR_ERR(dentry); 661 - goto out; 662 - } 663 - 664 - dentry = binderfs_create_file(binder_logs_root_dir, 665 - "transaction_log", 666 - &binder_transaction_log_fops, 667 - &binder_transaction_log); 668 - if (IS_ERR(dentry)) { 669 - ret = PTR_ERR(dentry); 670 - goto out; 671 - } 672 - 673 - dentry = binderfs_create_file(binder_logs_root_dir, 674 - "failed_transaction_log", 675 - &binder_transaction_log_fops, 676 - &binder_transaction_log_failed); 677 - if (IS_ERR(dentry)) { 678 - ret = PTR_ERR(dentry); 679 - goto out; 642 + binder_for_each_debugfs_entry(db_entry) { 643 + dentry = binderfs_create_file(binder_logs_root_dir, 644 + db_entry->name, 645 + db_entry->fops, 646 + db_entry->data); 647 + if (IS_ERR(dentry)) { 648 + ret = PTR_ERR(dentry); 649 + goto out; 650 + } 680 651 } 681 652 682 653 proc_log_dir = binderfs_create_dir(binder_logs_root_dir, "proc");