| 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
| ||
|
Date: Fri, 06 May 2011 16:28:39 -0700 From: john stultz <johnstul@...ibm.com> To: Eric Dumazet <eric.dumazet@...il.com> Cc: Andi Kleen <andi@...stfloor.org>, Thomas Gleixner <tglx@...utronix.de>, lkml <linux-kernel@...r.kernel.org>, Paul Mackerras <paulus@...ba.org>, "Paul E. McKenney" <paulmck@...ux.vnet.ibm.com>, Anton Blanchard <anton@...ba.org>, Ingo Molnar <mingo@...e.hu> Subject: Re: [RFC] time: xtime_lock is held too long On Sat, 2011-05-07 at 01:00 +0200, Eric Dumazet wrote: > Le vendredi 06 mai 2011 à 15:46 -0700, john stultz a écrit : > > On Sat, 2011-05-07 at 00:30 +0200, Eric Dumazet wrote: > > > I can see many cpus entering tick_do_update_jiffies64() and all are > > > calling write_seqlock(&xtime_lock); > > > > > > Only first one can perform the work, but all others are waiting on the > > > spinlock, get it, change seqcount, and realize they have nothing to > > > do... > > > > Huh. So who is calling tick_do_update_jiffies64 in your case? I know the > > sched_tick_timer and tick_nohz_handler checks to make sure > > tick_do_timer_cpu == cpu to avoid exactly the thundering heard problem > > on the jiffies update. > > > > There's other spots that call tick_do_update_jiffies64, but I thought > > those were more rare. So there may be something else wrong going on > > here. > > > > That I can answer : [snip] > (I added do_timestamp1/do_timestamp2) after/before write_seqlock()/write_sequnlock() > > <idle>-0 [003] 920.355377: do_timestamp1 <-tick_do_update_jiffies64 > <idle>-0 [006] 920.355377: tick_do_update_jiffies64 <-tick_sched_timer > <idle>-0 [003] 920.355378: do_timestamp2 <-tick_do_update_jiffies64 > <idle>-0 [000] 920.355657: tick_do_update_jiffies64 <-tick_check_idle > <idle>-0 [000] 920.355660: tick_do_update_jiffies64 <-tick_nohz_restart_sched_tick Thomas, any clues why this would be getting hammered? > > > /* Reevalute with xtime_lock held */ > > > - write_seqlock(&xtime_lock); > > > + spin_lock(&xtime_lock.lock); > > > > Oof.. No, this is too ugly and really just abuses the seqlock structure. > > > > That was a hack/POC, of course, but we can cleanup seqlock.h to provide > clean thing. A seqlock_t should include a seqcount_t and a spinlock_t. Maybe I'm misunderstanding, but you seem to be trying to create some sort of a layered lock from the seqlock ? I don't quite understand why your proposing this instead of actually splitting the lock out? > > If you really want to untangle what xtime_lock protects, you need to > > introduce a new lock (I suggest in the timekeeper structure) to protect > > the timekeeping data. > > > > Then we can refine xtime_lock to also just protect the jiffies/tick > > management bits as well if needed. > > For the moment I am trying to understand the code. Its quite complex and > uses a monolithic seqlock, defeating seqlock power. Defeating seqlock power? My thoughts are that seqlocks are nice lightweight reader/writer locks that avoid writer starvation. You seem to be trying to redefine or extend them to be something else which is more subtle. I agree, the code is complex! I'm just not sure adding more complicated/subtle locking mechanisms is a good solution. Instead I'd suggest simply splitting up the locks (by using new locks) to reduce the amount of data that is being protected by a single lock. But again, maybe I'm misunderstanding you? thanks -john -- 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/
Powered by blists - more mailing lists