After the IPMI disconnect problem, the memory kept rising and we tried
to unload the driver to free the memory. However, only part of the
free memory is recovered after the driver is uninstalled. Using
ebpf to hook free functions, we find that neither ipmi_user nor
ipmi_smi_msg is free, only ipmi_recv_msg is free.
We find that the deliver_smi_err_response call in clean_smi_msgs does
the destroy processing on each message from the xmit_msg queue without
checking the return value and free ipmi_smi_msg.
deliver_smi_err_response is called only at this location. Adding the
free handling has no effect.
To verify, try using ebpf to trace the free function.
$ bpftrace -e 'kretprobe:ipmi_alloc_recv_msg {printf("alloc rcv
%p\n",retval);} kprobe:free_recv_msg {printf("free recv %p\n",
arg0)} kretprobe:ipmi_alloc_smi_msg {printf("alloc smi %p\n",
retval);} kprobe:free_smi_msg {printf("free smi %p\n",arg0)}'
Signed-off-by: Zhang Yuchen <[email protected]>
---
drivers/char/ipmi/ipmi_msghandler.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
index c8a3b208f923..7a7534046b5b 100644
--- a/drivers/char/ipmi/ipmi_msghandler.c
+++ b/drivers/char/ipmi/ipmi_msghandler.c
@@ -3710,12 +3710,15 @@ static void deliver_smi_err_response(struct ipmi_smi *intf,
struct ipmi_smi_msg *msg,
unsigned char err)
{
+ int rv;
msg->rsp[0] = msg->data[0] | 4;
msg->rsp[1] = msg->data[1];
msg->rsp[2] = err;
msg->rsp_size = 3;
/* It's an error, so it will never requeue, no need to check return. */
- handle_one_recv_msg(intf, msg);
+ rv = handle_one_recv_msg(intf, msg);
+ if (rv == 0)
+ ipmi_free_smi_msg(msg);
}
static void cleanup_smi_msgs(struct ipmi_smi *intf)
--
2.30.2
On Fri, Oct 07, 2022 at 05:26:17PM +0800, Zhang Yuchen wrote:
> After the IPMI disconnect problem, the memory kept rising and we tried
> to unload the driver to free the memory. However, only part of the
> free memory is recovered after the driver is uninstalled. Using
> ebpf to hook free functions, we find that neither ipmi_user nor
> ipmi_smi_msg is free, only ipmi_recv_msg is free.
>
> We find that the deliver_smi_err_response call in clean_smi_msgs does
> the destroy processing on each message from the xmit_msg queue without
> checking the return value and free ipmi_smi_msg.
>
> deliver_smi_err_response is called only at this location. Adding the
> free handling has no effect.
>
> To verify, try using ebpf to trace the free function.
>
> $ bpftrace -e 'kretprobe:ipmi_alloc_recv_msg {printf("alloc rcv
> %p\n",retval);} kprobe:free_recv_msg {printf("free recv %p\n",
> arg0)} kretprobe:ipmi_alloc_smi_msg {printf("alloc smi %p\n",
> retval);} kprobe:free_smi_msg {printf("free smi %p\n",arg0)}'
>
> Signed-off-by: Zhang Yuchen <[email protected]>
> ---
> drivers/char/ipmi/ipmi_msghandler.c | 5 ++++-
> 1 file changed, 4 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/char/ipmi/ipmi_msghandler.c b/drivers/char/ipmi/ipmi_msghandler.c
> index c8a3b208f923..7a7534046b5b 100644
> --- a/drivers/char/ipmi/ipmi_msghandler.c
> +++ b/drivers/char/ipmi/ipmi_msghandler.c
> @@ -3710,12 +3710,15 @@ static void deliver_smi_err_response(struct ipmi_smi *intf,
> struct ipmi_smi_msg *msg,
> unsigned char err)
> {
> + int rv;
> msg->rsp[0] = msg->data[0] | 4;
> msg->rsp[1] = msg->data[1];
> msg->rsp[2] = err;
> msg->rsp_size = 3;
> /* It's an error, so it will never requeue, no need to check return. */
The above comment is wrong, but yes, this is correct. I'll queue this
and remove the comment.
Thanks,
-corey
> - handle_one_recv_msg(intf, msg);
> + rv = handle_one_recv_msg(intf, msg);
> + if (rv == 0)
> + ipmi_free_smi_msg(msg);
> }
>
> static void cleanup_smi_msgs(struct ipmi_smi *intf)
> --
> 2.30.2
>