> >include/scsi/libfc.h: struct fcoe_dev_stats *dev_stats[NR_CPUS]; > >NR_CPUS arrays are strongly discouraged because they lead to waste on >kernels compiled with large NR_CPUS, ideally use per CPU data or the >per cpu allocator or at least a dynamic allocation with >num_possible_cpus() >In general every use of NR_CPUS is dubious. e.g. you have some loops >over that, >these should be all over possible cpus. fcoe_percpu[] should be also >per cpu data, not an array. > >It would probably simplify the code a lot if you just went to global >per cpu counters instead of per cpu device counters. I don't know >if that is feasible or not. > >static int __init fcoe_init(void) > >There are CPU hotplug races between the notifier and the >for_each_online_cpu() >loop (probably not a serious problem, such races are common all over >the >code base, just wanted to mention it) > Hi, Andi, Thanks for the comments, I have submitted a patch to open-fcoe.org to convert the foce_percpu[] to be kernel percpu var as you suggested. We also have a dev_stats[NR_CPUS] as a member of the struct fc_lport. It stores an array of pointers to struct fcoe_dev_stats, which keeps track of all the statistics of fcoe traffic. The fc_lport is dynamically allocated upon creating fcoe instance from a given network interface name. The dev_stats are allocated initially in a for_each_online_cpu)() loop, and after that, corresponding dev_stats is allocated/freed upon CPU_ONLINE/CPU_DEAD event callback fcoe_cpu_callback(), do you think this is also a problem in using NR_CPUS? Thanks. yi -- To unsubscribe from this list: send the line "unsubscribe linux-scsi" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
