On Thu, Aug 22, 2024 at 6:36 AM <takakura@xxxxxxxxxxxxx> wrote: > > From: Ryo Takakura <takakura@xxxxxxxxxxxxx> > > While adding counters in do_add_counters(), we call > xt_write_recseq_begin/end to indicate that counters are being updated. > Updates are being tracked so that the counters retrieved by get_counters() > will reflect concurrent updates. > > However, there is no need to track the updates done by do_add_counters() as > both do_add_counters() and get_counters() acquire per ipv4,ipv6,arp mutex > beforehand which prevents concurrent update and retrieval between the two. > > Moreover, as the xt_write_recseq_begin/end is shared among ipv4,ipv6,arp, > do_add_counters() called by one of ipv4,ipv6,arp can falsely delay the > synchronization of concurrent get_counters() or xt_replace_table() called > by any other than the one calling do_add_counters(). > > So remove xt_write_recseq_begin/end from do_add_counters() for ipv4,ipv6,arp. Completely wrong patch. There is no way we can update pairs of 64bit counters without any synchronization. This is per cpu sequence, the 'shared among ipv4,ipv6,arp' part is moot. We could use cmpxchg128 on 64bit arches, but I suspect there will be no improvement.
