From: Jeff Layton <jlayton@kernel.org>
Sent: Tuesday, February 28, 2023 8:24 AM

On Mon, 2023-02-27 at 14:48 +0000, Andrew Klaassen wrote:

quoted

+struct rpc_timeout *xprt_alloc_timeout(const struct rpc_timeout
*timeo,
+                                    const struct rpc_timeout
*default_timeo)
+{
+     struct rpc_timeout *timeout;
+
+     timeout = kzalloc(sizeof(*timeout), GFP_KERNEL);
+     if (!timeout)
+             return ERR_PTR(-ENOMEM);
+     if (timeo)
+             memcpy(timeout, timeo, sizeof(struct rpc_timeout));
+     else
+             memcpy(timeout, default_timeo, sizeof(struct
rpc_timeout));

I don't think you need an allocation here. struct rpc_timeout is quite
small and it only contains a bunch of integers. I think it'd be better
to just embed this in struct rpc_xprt instead.

I missed this in my initial reply; apologies.  What do you mean by "embed" in this case?

FWIW, every time I tried assigning xprt->timeout without an allocation the timeout values would be correct just after the assignment in xs_setup_tcp, but by the time the code got to xs_tcp_set_socket_timeouts the timeout would be filled with random values.  I'm sure this reflects my limitations as not-a-C-programmer, but no matter which way I tried it I couldn't stop that from happening until I allocated memory.

Thanks.

Andrew Klaassen