svn commit: r277213 - in head: share/man/man9 sys/kern sys/ofed/include/linux sys/sys

[Hans Petter Selasky]

On 01/20/15 22:11, Gleb Smirnoff wrote:
> On Tue, Jan 20, 2015 at 09:51:26AM +0200, Konstantin Belousov wrote:
> K> > Like stated in the manual page, callout_reset_curcpu/on() does not work
> K> > with MPSAFE callouts any more!
> K> I.e. you 'fixed' some undeterminate bugs in callout migration by not
> K> doing migration at all anymore.
> K>
> K> >
> K> > You need to use callout_init_{mtx,rm,rw} and remove the custom locking
> K> > inside the callback in the TCP stack to get it working like before!
> K>
> K> No, you need to do this, if you think that whole callout KPI must be
> K> rototiled.  It is up to the person who modifies the KPI, to ensure that
> K> existing code is not broken.
> K>
> K> As I understand, currently we are back to the one-cpu callouts.
> K> Do other people consider this situation acceptable ?
>
> I think this isn't acceptable. The commit to a complex subsystem
> lacked a review from persons involved in the system before. The
> commit to subsystem broke consumers of the subsystem and this
> was even done not accidentially, but due to Hans not caring about
> it.
>
> As for me this is enough to request a backout, and let the change
> back in only after proper review.
>

Hi Gleb,

Backing out my callout API patch means we will for sure re-introduce an 
unknown callout spinlock hang, as noted to me by several people. What do 
you think about that?

Maybe "Jason Wolfe" CC'ed can add to 10-stable w/o my patches:

int
callout_reset_sbt_on(struct callout *c, sbintime_t sbt, sbintime_t 
precision,
     void (*ftn)(void *), void *arg, int cpu, int flags)
{
         sbintime_t to_sbt, pr;
         struct callout_cpu *cc;
         int cancelled, direct;

+       cpu = timeout_cpu;   /* XXX test code XXX */

         cancelled = 0;

And see if he observes a callout spinlock hang or not on his test setup. 
The patch above should force all callouts to the same thread basically. 
Then we could maybe see if single threading the callouts has anything to 
do with solving the spinlock hang.

The "rewritten" callout API still has all the features and capabilities 
the old one had, when used as described in "man 9 callout".

At the present moment I'm not technically convinced a backout is correct.

Gleb: I think we would see far better results with high speed internet 
links using TCP if we could extend the LRO (large receive offload) code 
to accumulate more than 64KBytes worth of data per call to the TCP stack 
instead of complaining about some callouts ending up on the same thread! 
Actually I have a patch for that.

--HPS