linux-kernel - Re: [PATCH 1/1] kthreads: allow_signal: don't play with ->blocked

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]

Message-ID: <20110817072746.GI4254@htj.dyndns.org>
Date:	Wed, 17 Aug 2011 09:27:46 +0200
From:	Tejun Heo <tj@...nel.org>
To:	Matt Fleming <matt@...sole-pimps.org>
Cc:	Oleg Nesterov <oleg@...hat.com>,
	Andrew Morton <akpm@...ux-foundation.org>,
	linux-kernel@...r.kernel.org
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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/