| Message ID | 20240613082821.849-2-jack@suse.cz (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | nfs: Improve throughput for random buffered writes | expand |
Reviewed-by: Sagi Grimberg <sagi@grimberg.me>
On Thu, 2024-06-13 at 10:28 +0200, Jan Kara wrote: > Atomic types should better be initialized with atomic_long_set() > instead > of relying on zeroing done by kzalloc(). Clean this up. > > Signed-off-by: Jan Kara <jack@suse.cz> > --- > fs/nfs/client.c | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c > index de77848ae654..3b252dceebf5 100644 > --- a/fs/nfs/client.c > +++ b/fs/nfs/client.c > @@ -994,6 +994,8 @@ struct nfs_server *nfs_alloc_server(void) > > server->change_attr_type = NFS4_CHANGE_TYPE_IS_UNDEFINED; > > + atomic_long_set(&server->writeback, 0); > + > ida_init(&server->openowner_id); > ida_init(&server->lockowner_id); > pnfs_init_server(server); I'm guilty of doing this, well, all over the place. Is there any plausible scenario where another task could see this value set to non- zero after kzalloc()? One would hope not... -- Jeff Layton <jlayton@kernel.org>
On Thu 13-06-24 15:56:36, Jeff Layton wrote: > On Thu, 2024-06-13 at 10:28 +0200, Jan Kara wrote: > > Atomic types should better be initialized with atomic_long_set() > > instead > > of relying on zeroing done by kzalloc(). Clean this up. > > > > Signed-off-by: Jan Kara <jack@suse.cz> > > --- > > fs/nfs/client.c | 2 ++ > > 1 file changed, 2 insertions(+) > > > > diff --git a/fs/nfs/client.c b/fs/nfs/client.c > > index de77848ae654..3b252dceebf5 100644 > > --- a/fs/nfs/client.c > > +++ b/fs/nfs/client.c > > @@ -994,6 +994,8 @@ struct nfs_server *nfs_alloc_server(void) > > > > server->change_attr_type = NFS4_CHANGE_TYPE_IS_UNDEFINED; > > > > + atomic_long_set(&server->writeback, 0); > > + > > ida_init(&server->openowner_id); > > ida_init(&server->lockowner_id); > > pnfs_init_server(server); > > I'm guilty of doing this, well, all over the place. Is there any > plausible scenario where another task could see this value set to non- > zero after kzalloc()? One would hope not... No, it is not a practical problem these days. It is more a theoretical correctness thing that atomic_t should be initialized through atomic_set() and not memset() because in theory you don't know how some weird architecture decides to implement it. Honza
diff --git a/fs/nfs/client.c b/fs/nfs/client.c index de77848ae654..3b252dceebf5 100644 --- a/fs/nfs/client.c +++ b/fs/nfs/client.c @@ -994,6 +994,8 @@ struct nfs_server *nfs_alloc_server(void) server->change_attr_type = NFS4_CHANGE_TYPE_IS_UNDEFINED; + atomic_long_set(&server->writeback, 0); + ida_init(&server->openowner_id); ida_init(&server->lockowner_id); pnfs_init_server(server);
Atomic types should better be initialized with atomic_long_set() instead of relying on zeroing done by kzalloc(). Clean this up. Signed-off-by: Jan Kara <jack@suse.cz> --- fs/nfs/client.c | 2 ++ 1 file changed, 2 insertions(+)