From mboxrd@z Thu Jan 1 00:00:00 1970 From: Manish Kumar Singh Subject: RE: [PATCH] bonding:avoid repeated display of same link status change Date: Tue, 20 Nov 2018 02:41:52 -0800 (PST) Message-ID: References: <20181031105729.7442-1-mk.singh@oracle.com> <20181102.233138.738200505012734856.davem@davemloft.net> <20181104194121.GA29914@unicorn.suse.cz> Mime-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Transfer-Encoding: quoted-printable Cc: netdev@vger.kernel.org, eric.dumazet@gmail.com, j.vosburgh@gmail.com, vfalico@gmail.com, andy@greyhouse.net, linux-kernel@vger.kernel.org To: Michal Kubecek , David Miller Return-path: In-Reply-To: <20181104194121.GA29914@unicorn.suse.cz> Sender: linux-kernel-owner@vger.kernel.org List-Id: netdev.vger.kernel.org > -----Original Message----- > From: Michal Kubecek [mailto:mkubecek@suse.cz] > Sent: 05 =E0=A4=A8=E0=A4=B5=E0=A4=AE=E0=A5=8D=E0=A4=AC=E0=A4=B0 2018 01:1= 1 > To: David Miller > Cc: Manish Kumar Singh; netdev@vger.kernel.org; eric.dumazet@gmail.com; > j.vosburgh@gmail.com; vfalico@gmail.com; andy@greyhouse.net; linux- > kernel@vger.kernel.org > Subject: Re: [PATCH] bonding:avoid repeated display of same link status > change >=20 > On Fri, Nov 02, 2018 at 11:31:38PM -0700, David Miller wrote: > > From: mk.singh@oracle.com > > Date: Wed, 31 Oct 2018 16:27:28 +0530 > > > > > -=09=09=09if (slave->delay) { > > > +=09=09=09if (slave->delay && > > > +=09=09=09 !atomic64_read(&bond->rtnl_needed)) { > > ... > > > +=09=09=09 !atomic64_read(&bond->rtnl_needed)) { > > ... > > > +=09=09=09atomic64_set(&bond->rtnl_needed, 1); > > ... > > > +=09=09atomic64_set(&bond->rtnl_needed, 0); > > ... > > > @@ -229,6 +229,7 @@ struct bonding { > > > =09struct=09 dentry *debug_dir; > > > #endif /* CONFIG_DEBUG_FS */ > > > =09struct rtnl_link_stats64 bond_stats; > > > +=09atomic64_t rtnl_needed; > > > > There is nothing "atomic" about a value that is only set and read. > > > > And using a full 64-bit value for something taking on only '0' and > > '1' is unnecessary as well. >=20 > Part of the misunderstanding is caused by the fact that this is actually > a v4 but not marked as such: >=20 > v1: https://patchwork.ozlabs.org/patch/955789/ > v2: https://patchwork.ozlabs.org/patch/970421/ > v3: https://patchwork.ozlabs.org/patch/988241/ >=20 > When commenting v3, I didn't know about the v2 discussion where Eric > Dumazet NACKed the patch because of potential conflict issues: >=20 > https://patchwork.ozlabs.org/patch/970421/#1992397 > https://patchwork.ozlabs.org/patch/988241/#2017317 >=20 > On the other hand, there is no need for atomic64_t. Simple atomic_t > (with explaining comment) would suffice. On architectures allowing > atomic read/write for 32-bit integers, there would be no performance > penalty. On architectures not allowing it, atomic_read() and > atomic_set() are implemented to be safe. Sorry for late response, I was off to work for couple of weeks. v3: https://patchwork.ozlabs.org/patch/988241/ was sent with atomic_t and = after seeing your comment, I sent it with atomic64_t. Please let me know if v3 was fine ?=20 Thanks, Manish >=20 > Michal Kubecek