Message ID | 1564132635-57634-1-git-send-email-zhangshaokun@hisilicon.com |
---|---|
State | Changes Requested |
Delegated to: | David Miller |
Headers | show |
Series | Revert "net: get rid of an signed integer overflow in ip_idents_reserve()" | expand |
On 7/26/19 11:17 AM, Shaokun Zhang wrote: > From: Yang Guo <guoyang2@huawei.com> > > There is an significant performance regression with the following > commit-id <adb03115f459> > ("net: get rid of an signed integer overflow in ip_idents_reserve()"). > > So, you jump around and took ownership of this issue, while some of us are already working on it ? Have you first checked that current UBSAN versions will not complain anymore ? A revert adding back the original issue would be silly, performance of benchmarks is nice but secondary.
Hi Eric, On 2019/7/26 17:58, Eric Dumazet wrote: > > > On 7/26/19 11:17 AM, Shaokun Zhang wrote: >> From: Yang Guo <guoyang2@huawei.com> >> >> There is an significant performance regression with the following >> commit-id <adb03115f459> >> ("net: get rid of an signed integer overflow in ip_idents_reserve()"). >> >> > > So, you jump around and took ownership of this issue, while some of us > are already working on it ? > Any update about this issue? Thanks, Shaokun > Have you first checked that current UBSAN versions will not complain anymore ? > > A revert adding back the original issue would be silly, performance of > benchmarks is nice but secondary. > > >
diff --git a/net/ipv4/route.c b/net/ipv4/route.c index 517300d..dff457b 100644 --- a/net/ipv4/route.c +++ b/net/ipv4/route.c @@ -489,18 +489,12 @@ u32 ip_idents_reserve(u32 hash, int segs) atomic_t *p_id = ip_idents + hash % IP_IDENTS_SZ; u32 old = READ_ONCE(*p_tstamp); u32 now = (u32)jiffies; - u32 new, delta = 0; + u32 delta = 0; if (old != now && cmpxchg(p_tstamp, old, now) == old) delta = prandom_u32_max(now - old); - /* Do not use atomic_add_return() as it makes UBSAN unhappy */ - do { - old = (u32)atomic_read(p_id); - new = old + delta + segs; - } while (atomic_cmpxchg(p_id, old, new) != old); - - return new - segs; + return atomic_add_return(segs + delta, p_id) - segs; } EXPORT_SYMBOL(ip_idents_reserve);