Messages in this thread | | | Date | Wed, 17 Aug 2011 09:27:46 +0200 | From | Tejun Heo <> | Subject | Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked |
| |
Hello,
On Tue, Aug 16, 2011 at 10:50:22PM +0100, Matt Fleming wrote: > On Tue, 2011-08-16 at 21:51 +0200, Tejun Heo wrote: > > I agree with the patchset but given that daemonize() isn't all that > > popular and you already posted most (or was it all?) conversions, > > wouldn't it be better to do this in a single patchset? ie. Convert > > all daemonize() users, kill daemonize(), and drop the hack from > > allow_signal(). > > But because daemonize() is exported by the kernel should it go through > the Documentation/feature-removal-schedule.txt procedure? And if so, can > the allow_signal() patch still go in before daemonize() is removed?
IMHO, not really. APIs get modified and dropped all the time and only small fraction goes through feature-removal-schedule. For APIs which are widely used and/or difficult to migrate from, it sure makes sense to do the staged removal but in this case it's an interface which is quite unpopular and with relatively easy workaround (just use kthread).
The worst thing we can do regarding API change is silently changing semantics while not changing the interface. For this patchset I don't think it would matter all that much but is going that route. ie. allow_signal() behavior is proposed to be changed because in-kernel daemonize() users don't depend on it while leaving daemonize() alone. This is much worse than simply removing daemonize() with sufficient explanation in the commit message. Out-of-kernel user which depended on the combination working would now be left with code which compiles fine but behaves differently, which sucks big time.
These changes _are_ related and interdependent, and routing these small changes through different trees often end up delaying things unnecessarily. One subsystem maintainer forgets to apply a patch or send pull request and it can get easily drawn out half a year and people forget what the original change was about after a while often leading to half done conversions. So, let's please collect all the related patches into one series, drop all in-kernel daemonize() users, kill daemonize() and then change allow_signal() behavior.
Thanks.
-- tejun
| |