Messages in this thread | | | Date | Fri, 06 Feb 2015 17:59:55 +0530 | From | Raghavendra K T <> | Subject | Re: sched: memory corruption on completing completions |
| |
On 02/06/2015 04:07 AM, Davidlohr Bueso wrote: > On Thu, 2015-02-05 at 13:34 -0800, Linus Torvalds wrote: >> On Thu, Feb 5, 2015 at 1:02 PM, Sasha Levin <sasha.levin@oracle.com> wrote: >>> >>> Interestingly enough, according to that article this behaviour seems to be >>> "by design": >> >> Oh, it's definitely by design, it's just that the design looked at >> spinlocks without the admittedly very subtle issue of lifetime vs >> unlocking. >> >> Spinlocks (and completions) are special - for other locks we have >> basically allowed lifetimes to be separate from the lock state, and if >> you have a data structure with a mutex in it, you'll have to have some >> separate lifetime rule outside of the lock itself. But spinlocks and >> completions have their locking state tied into their lifetime. > > For spinlocks I find this very much a virtue. Tight lifetimes allow the > overall locking logic to be *simple* - keeping people from being "smart" > and bloating up spinlocks. Similarly, I hate how the paravirt > alternative blends in with regular (sane) bare metal code. What was > preventing this instead?? > > #ifdef CONFIG_PARAVIRT_SPINLOCKS > static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) > { > if (!static_key_false(¶virt_ticketlocks_enabled)) > return; > > add_smp(&lock->tickets.head, TICKET_LOCK_INC); > /* Do slowpath tail stuff... */ > } > #else > static __always_inline void arch_spin_unlock(arch_spinlock_t *lock) > { > __add(&lock->tickets.head, TICKET_LOCK_INC, UNLOCK_LOCK_PREFIX); > } > #endif > > I just don't see the point to all this TICKET_SLOWPATH_FLAG: > > #ifdef CONFIG_PARAVIRT_SPINLOCKS > #define __TICKET_LOCK_INC 2 > #define TICKET_SLOWPATH_FLAG ((__ticket_t)1) > #else > #define __TICKET_LOCK_INC 1 > #define TICKET_SLOWPATH_FLAG ((__ticket_t)0) > #endif > > when it is only for paravirt -- and the word slowpath implies the > general steps as part of the generic algorithm. Lets keep code for > simple locks simple. >
Good point, I will send this as a separate cleanup once I test the patch I have to correct the current problem.
| |