linux-kernel - Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Fri, 15 Mar 2019 20:26:30 -0700
From:   Greg KH <gregkh@...uxfoundation.org>
To:     "Enrico Weigelt, metux IT consult" <lkml@...ux.net>
Cc:     "Enrico Weigelt, metux IT consult" <info@...ux.net>,
        linux-kernel@...r.kernel.org, eric@...olt.net,
        stefan.wahren@...e.com, f.fainelli@...il.com, rjui@...adcom.com,
        sbranden@...adcom.com, bcm-kernel-feedback-list@...adcom.com,
        andriy.shevchenko@...ux.intel.com, vz@...ia.com,
        matthias.bgg@...il.com, yamada.masahiro@...ionext.com,
        tklauser@...tanz.ch, richard.genoud@...il.com,
        macro@...ux-mips.org, u.kleine-koenig@...gutronix.de,
        kernel@...gutronix.de, slemieux.tyco@...il.com,
        andy.gross@...aro.org, david.brown@...aro.org, shawnguo@...nel.org,
        s.hauer@...gutronix.de, festevam@...il.com, linux-imx@....com,
        baohua@...nel.org, jacmet@...site.dk, linux-serial@...r.kernel.org,
        linux-arm-msm@...r.kernel.org, linuxppc-dev@...ts.ozlabs.org
Subject: Re: [PATCH v2 10/45] drivers: tty: serial: zs: use devm_* functions

On Fri, Mar 15, 2019 at 08:12:47PM +0100, Enrico Weigelt, metux IT consult wrote:
> On 15.03.19 15:26, Greg KH wrote:
> > Yes, there are lots of drivers for devices that are never unloaded or
> > removed from the system.  The fact that no one has reported any problems
> > with them means that they are never used in situations like this.
> 
> So, never touch a running system ?

No, it's just that those systems do not allow those devices to be
removed because they are probably not on a removable bus.

> > No, you need to have a good reason why it needs to be changed, when you
> > can not verify that this actually resolves a problem.  As this patch
> > shows, you just replaced one api call with another, so nothing changed
> > at all, except you actually took up more memory and logic to do the same
> > thing :(
> 
> Okay, I was on a wrong track here - I had the silly idea that it would
> make things easier if we'd do it the same way everywhere.

"Consistent" is good, and valid, but touching old drivers that have few
users is always risky, and you need a solid reason to do so.

> >> IMHO, we should have a closer look at those and check whether that's
> >> really okay (just adding request/release blindly could cause trouble)
> > 
> > I agree, please feel free to audit these to verify they are all correct.
> > But that's not what you did here, so that's not a valid justification
> > for these patches to be accepted.
> 
> Understood. Assuming I've found some of these cases, shall I use devm
> oder just add the missing release ?

If it actually makes the code "simpler" or "more obvious", sure, that's
fine.  But churn for churns sake is not ok.

> >> By the way: do you have some public branch where you're collecting
> >> accepted patches, which I could base mine on ? (tty.git/tty-next ?)
> > 
> > Yes, that is the tree and branch, but remember that none of my trees can
> > open up until after -rc1 is out.
> 
> So, within a merge window, you put everything else on hold ?

I put the review of new patch submissions on hold, yes.  Almost all
maintainers do that as we can not add new patches to our trees at that
point in time.

And I do have other things I do during that period so it's not like I'm
just sitting around doing nothing :)

thanks,

greg k-h