On Friday 02/19 at 07:14 -0600, Corey Minyard wrote: > On 02/19/2016 12:41 AM, Calvin Owens wrote: > >Hello, > > > >I've got a few boxes that are leaking memory in handle_new_recv_msgs() > >in ipmi_msghandler. AFAICS this is intentional, there's even an explicit > >counter that tracks the number of times smi_msg is leaked. > > Are you 100% sure about this?
I'm absolutely certain this is where I'm leaking: I threw in a printk() and saw exact correlation between the number of times I saw that and the number of kmalloc-1024 objects leaked. But... > There's no intentional leak, a negative return from this function > means the message was used for another purpose and thus shouldn't be > freed. There's only one situation where this happens and you should > never hit it in normal operation. This is actually extremely helpful: we have some horrible non-upstream code behind this that I thought I had ruled out being at fault here, but this sounds like it might be. In any case, I won't waste your time any more until I can reproduce it on upstream (which is unfortunately impossible on this particular HW I see the leak on, otherwise I would have done that in the first place...). Thanks very much for the prompt response, I really appreciate it. Calvin > >I'm guessing there was a reason for doing this, but there wasn't any > >discussion about it on LKML when the patch was accepted. Can you clarify > >why something like the below patch won't work? I tried it on one of my > >leaky boxes and nothing obviously horrible happened. > > Well, that's because nothing probably happened, and it probably had no > effect on the leak. A better comment on this code is probably in > order. But that patch is incorrect. > > I doubt the leak is here. If you are having a leak, is it possible to > characterize it better? Are you handling commands from IPMB? Are you > handling LAN commands here? > > -corey > > > > >Thanks, > >Calvin > > > >----8<---- > >From: Calvin Owens <[email protected]> > >Subject: [PATCH] ipmi_msghandler: Don't leak memory on errors > > > >Signed-off-by: Calvin Owens <[email protected]> > >--- > > drivers/char/ipmi/ipmi_msghandler.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > >diff --git a/drivers/char/ipmi/ipmi_msghandler.c > >b/drivers/char/ipmi/ipmi_msghandler.c > >index 94fb407..ed82ffa 100644 > >--- a/drivers/char/ipmi/ipmi_msghandler.c > >+++ b/drivers/char/ipmi/ipmi_msghandler.c > >@@ -3834,10 +3834,7 @@ static void handle_new_recv_msgs(ipmi_smi_t intf) > > break; > > } else { > > list_del(&smi_msg->link); > >- if (rv == 0) > >- /* Message handled */ > >- ipmi_free_smi_msg(smi_msg); > >- /* If rv < 0, fatal error, del but don't free. */ > >+ ipmi_free_smi_msg(smi_msg); > > } > > } > > if (!run_to_completion) >
