[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20161220095135.GA13910@kroah.com>
Date: Tue, 20 Dec 2016 10:51:35 +0100
From: Greg KH <gregkh@...uxfoundation.org>
To: "Reshetova, Elena" <elena.reshetova@...el.com>
Cc: Peter Zijlstra <peterz@...radead.org>,
Liljestrand Hans <ishkamiel@...il.com>,
"kernel-hardening@...ts.openwall.com" <kernel-hardening@...ts.openwall.com>,
Kees Cook <keescook@...omium.org>,
"will.deacon@....com" <will.deacon@....com>,
Boqun Feng <boqun.feng@...il.com>,
David Windsor <dwindsor@...il.com>, "aik@...abs.ru" <aik@...abs.ru>,
"david@...son.dropbear.id.au" <david@...son.dropbear.id.au>
Subject: Re: Conversion from atomic_t to refcount_t: summary of issues
On Tue, Dec 20, 2016 at 09:40:29AM +0000, Reshetova, Elena wrote:
>
> > On Tue, Dec 20, 2016 at 09:13:58AM +0000, Reshetova, Elena wrote:
> > > > On Mon, Dec 19, 2016 at 07:55:15AM +0000, Reshetova, Elena wrote:
> > > > > Well, again, you are right in theory, but in practice for example for struct
> > > > sched_group { atomic_t ref; ... }:
> > > > >
> > > > > http://lxr.free-electrons.com/source/kernel/sched/core.c#L6178
> > > > >
> > > > > To me this is a refcounter that needs the protection.
> > > >
> > > > Only if you have more than UINT_MAX CPUs or something like that.
> > > >
> > > > And if you really really want to use refcount_t there, you could +1 the
> > > > scheme and it'd work again.
> > >
> > > Well, yes, probably, but there are many cases like this in practice,
> > > so we would need to have a good plan how to get it all submitted and
> > > tested properly. The current patch set is already bigger than what we
> > > had before and it is only growing.
> >
> > kernel programming is hard :)
> >
> > Don't get frustrated, it's going to be a lot of work, just break it up
> > into chunks and go at it...
> >
> > > Hans will provide more info later today based on his testing, which
> > > shows many places in kernel core where we DO actually have increment
> > > on zero happening in practice and whole kernel doesn't even boot with
> > > the strictest approach (refusing to inc on zero). And we are only able
> > > to test for x86....
> > >
> > > Given the massive amount of changes, it would be good to merge this at
> > > least in couple of stages:
> > >
> > > 1) first soft version of refcount_t API which at least allows
> > > increment on zero and all atomic_t used as refcounter occurrences that
> > > don't require reference counter scheme change (+1 or other)
> >
> > Why not merge the "correct" implementation? Don't submit something that
> > doesn't work well. Then fix up the instances that are broken when you
> > convert them to this new api.
>
> It is not that the implementation is incorrect, it is just less
> radical change in logical behavior. The main issue is going to be
> testing.
Again, kernel programming is hard :)
> It is hard to make sure we don't break things up, so that's why
> usually a softer approach is to do such big changes in parts. We can
> test on x86 and do at least compilation for arm, but what about the
> rest? It is a logical change which is bigger than we had before and
> consequences might be severe if we miss smth.
You add the correct implementation of refcount_t, and then push the
individual conversions through the various subsystem maintainers who
will review and test the code for correctness. Just like any other api
change we do. Why is this somehow "different"?
> > > 2) patch set that fixes all problematic places (potentially with code rewrite)
> > > 3) patch that removes possibility of inc on zero from refcount_t
> >
> > That implies that 3) would not happen for another year or so, not good.
> > Do it right the first time.
>
> I didn't have that timetable in mind, I would say couple of months the most.
3 months is just one kernel development cycle, this is going to take
longer than that, but optimism is nice to have :)
thanks,
greg k-h
Powered by blists - more mailing lists
Confused about mailing lists and their use?
Read about mailing lists on Wikipedia
and check out these
guidelines on proper formatting of your messages.
