On Wed, May 3, 2017 at 2:37 PM, Gao Feng <[email protected]> wrote: >> From: Xin Long [mailto:[email protected]] >> Sent: Wednesday, May 3, 2017 1:38 PM >> On Wed, May 3, 2017 at 10:07 AM, Gao Feng <[email protected]> >> wrote: >> >> From: [email protected] >> >> [mailto:[email protected]] >> >> On Behalf Of Xin Long >> >> Sent: Wednesday, May 3, 2017 12:59 AM On Tue, May 2, 2017 at 7:03 PM, >> >> Gao Feng <[email protected]> wrote: >> >> >> From: Xin Long [mailto:[email protected]] >> >> >> Sent: Tuesday, May 2, 2017 3:56 PM On Sat, Apr 29, 2017 at 11:51 >> >> >> AM, <[email protected]> wrote: >> >> >> > From: Gao Feng <[email protected]> > [...] >> > The fix you mentioned change the original logic. >> > The dev->vstats is freed in advance in the ndo_uninit, not destructor. >> > It may break the backward. >> Sorry, I didn't get your "backward" >> I can't see there will be any problem caused by it. >> can you say this patch also break the 'backward' ? >> https://patchwork.ozlabs.org/patch/748964/ >> >> It's really weird to do dev->reg_state check in ndo_unint ndo_unint is >> supposed >> to free the memory alloced in ndo_init. >> > > I am not sure if it would break the backward, so I said it MAY break. > I assumed there may be someone would access the dev->vstats after ndo_uninit, > because current veth driver free the mem in the destructor. > I selected this approach because I don't want to bring new bugs during fix > bug. > > If you're sure it is safe to free dev->vstats in ndo_uninit, I would like to > update it. yes, stats are accessed in .ndo_start_xmit waited by synchronize_net() and .ndo_get_stats64 protected by rtnl_lock().
> > BTW there are too many drivers which have possible memleak. > You could find the list by > https://www.mail-archive.com/[email protected]/msg166629.html. ah, cool. I'm not sure about other dev's stuff, have to check them for sure later. > > Some drivers allocate the resources in ndo_init, free some in ndo_uninit and > free left in destructor. > I think there are some reasons. > We could not move all free in the ndo_uninit from destructor. What's your > opinion? > > Best Regards > Feng > > >
