* [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE
@ 2024-03-19 22:49 Zijie Zhao
2024-03-20 2:41 ` Jason Xing
2024-03-20 18:03 ` Julian Anastasov
0 siblings, 2 replies; 3+ messages in thread
From: Zijie Zhao @ 2024-03-19 22:49 UTC (permalink / raw
To: horms, ja, davem, dsahern, edumazet, kuba, pabeni
Cc: netdev, lvs-devel, chenyuan0y
Dear IPVS maintainers,
We encountered an unusual usage of sysctl parameter while analyzing
kernel source code.
In include/net/ip_vs.h, line 1062 - 1070:
```
static inline int sysctl_sync_threshold(struct netns_ipvs *ipvs)
{
return ipvs->sysctl_sync_threshold[0];
}
static inline int sysctl_sync_period(struct netns_ipvs *ipvs)
{
return READ_ONCE(ipvs->sysctl_sync_threshold[1]);
}
```
Here, sysctl_sync_threshold[1] is accessed behind `READ_ONCE`, but
sysctl_sync_threshold[0] is not. Should sysctl_sync_threshold[0] also be
guarded by `READ_ONCE`?
Please kindly let us know if we missed any key information and this is
actually intended. We appreciate your information and time! Thanks!
Links to the code:
https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1064
https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1069
Best,
Zijie
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE
2024-03-19 22:49 [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE Zijie Zhao
@ 2024-03-20 2:41 ` Jason Xing
2024-03-20 18:03 ` Julian Anastasov
1 sibling, 0 replies; 3+ messages in thread
From: Jason Xing @ 2024-03-20 2:41 UTC (permalink / raw
To: Zijie Zhao
Cc: horms, ja, davem, dsahern, edumazet, kuba, pabeni, netdev,
lvs-devel, chenyuan0y
On Wed, Mar 20, 2024 at 6:49 AM Zijie Zhao <zzjas98@gmail.com> wrote:
>
> Dear IPVS maintainers,
>
> We encountered an unusual usage of sysctl parameter while analyzing
> kernel source code.
>
>
> In include/net/ip_vs.h, line 1062 - 1070:
>
> ```
> static inline int sysctl_sync_threshold(struct netns_ipvs *ipvs)
> {
> return ipvs->sysctl_sync_threshold[0];
> }
>
> static inline int sysctl_sync_period(struct netns_ipvs *ipvs)
> {
> return READ_ONCE(ipvs->sysctl_sync_threshold[1]);
> }
> ```
>
> Here, sysctl_sync_threshold[1] is accessed behind `READ_ONCE`, but
> sysctl_sync_threshold[0] is not. Should sysctl_sync_threshold[0] also be
> guarded by `READ_ONCE`?
I'm not so sure and clear about the detailed history.
AFAIK, readers accessing this sysctl knob (sysctl_sync_threshold)
should be protected by READ_ONCE() because it can be changed
concurrently. Probably the commit 749c42b620a95 missed this point many
years ago and then commit 6aa7de059173a followed that.
Thanks,
Jason
>
> Please kindly let us know if we missed any key information and this is
> actually intended. We appreciate your information and time! Thanks!
>
>
> Links to the code:
> https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1064
> https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1069
>
> Best,
> Zijie
>
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE
2024-03-19 22:49 [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE Zijie Zhao
2024-03-20 2:41 ` Jason Xing
@ 2024-03-20 18:03 ` Julian Anastasov
1 sibling, 0 replies; 3+ messages in thread
From: Julian Anastasov @ 2024-03-20 18:03 UTC (permalink / raw
To: Zijie Zhao
Cc: horms, davem, dsahern, edumazet, kuba, pabeni, netdev, lvs-devel,
chenyuan0y
Hello,
On Tue, 19 Mar 2024, Zijie Zhao wrote:
> Dear IPVS maintainers,
>
> We encountered an unusual usage of sysctl parameter while analyzing kernel
> source code.
>
>
> In include/net/ip_vs.h, line 1062 - 1070:
>
> ```
> static inline int sysctl_sync_threshold(struct netns_ipvs *ipvs)
> {
> return ipvs->sysctl_sync_threshold[0];
> }
>
> static inline int sysctl_sync_period(struct netns_ipvs *ipvs)
> {
> return READ_ONCE(ipvs->sysctl_sync_threshold[1]);
> }
> ```
>
> Here, sysctl_sync_threshold[1] is accessed behind `READ_ONCE`, but
> sysctl_sync_threshold[0] is not. Should sysctl_sync_threshold[0] also be
> guarded by `READ_ONCE`?
>
> Please kindly let us know if we missed any key information and this is
> actually intended. We appreciate your information and time! Thanks!
Difference comes from commit 749c42b620a9 where we protect us
from division by zero by using ACCESS_ONCE at that time. The idea was
to read the value only once. Nowadays, READ_ONCE also has the role to
avoid load tearing, so, yes, all sysctl vars should be accessed with
READ_ONCE but this is a low priority goal for now.
> Links to the code:
> https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1064
> https://elixir.bootlin.com/linux/v6.8.1/source/include/net/ip_vs.h#L1069
>
> Best,
> Zijie
Regards
--
Julian Anastasov <ja@ssi.bg>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2024-03-20 18:04 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-03-19 22:49 [net] Question about ipvs->sysctl_sync_threshold and READ_ONCE Zijie Zhao
2024-03-20 2:41 ` Jason Xing
2024-03-20 18:03 ` Julian Anastasov
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).