linux-kernel - Re: [PATCH -rt] rt-slab: fix cpu inconsistency case

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: <Pine.LNX.4.58.0803211635160.3220@gandalf.stny.rr.com>
Date:	Fri, 21 Mar 2008 16:37:35 -0400 (EDT)
From:	Steven Rostedt <rostedt@...dmis.org>
To:	Hiroshi Shimamoto <h-shimamoto@...jp.nec.com>
cc:	Ingo Molnar <mingo@...e.hu>, Thomas Gleixner <tglx@...utronix.de>,
	linux-kernel@...r.kernel.org, linux-rt-users@...r.kernel.org
Subject: Re: [PATCH -rt] rt-slab: fix cpu inconsistency case



On Thu, 20 Mar 2008, Hiroshi Shimamoto wrote:

>
> mm/slab.c:cache_alloc_refill()
> 	if (unlikely(!ac->avail)) {
> 		int x;
> 		x = cache_grow(cachep, flags | GFP_THISNODE, cpu_to_node(*this_cpu), NULL, this_cpu);
>
> 		/* cache_grow can reenable interrupts, then ac could change. */
> 		ac = cpu_cache_get(cachep, *this_cpu);
>
> The comment says, "ac could change", but it never if *this_cpu is same.
>
> In cache_alloc_refill(), cpu_cache_get() should called with the valid cpu id
> after cache_grow(). Because on !PREEMPT_RT, the array_cache is protected by
> disabling irqs, so array_cache of other cpu shouldn't be accessed.

Ah, sorry I missed the !PREEMPT_RT part.

>
> >
> >>  # define slab_irq_disable_rt(flags)	do { (void)(flags); } while (0)
> >>  # define slab_irq_enable_rt(flags)	do { (void)(flags); } while (0)
> >>  # define slab_spin_lock_irq(lock, cpu) \
> >> @@ -160,8 +160,8 @@ DEFINE_PER_CPU_LOCKED(int, slab_irq_locks) = { 0, };
> >>  	do { slab_irq_enable(cpu); (void) (flags); } while (0)
> >>  # define slab_irq_disable_rt(cpu)	slab_irq_disable(cpu)
> >>  # define slab_irq_enable_rt(cpu)	slab_irq_enable(cpu)
> >> -# define slab_irq_disable_nort()	do { } while (0)
> >> -# define slab_irq_enable_nort()		do { } while (0)
> >> +# define slab_irq_disable_nort(cpu)	do { } while (0)
> >> +# define slab_irq_enable_nort(cpu)	do { } while (0)
> >
> > And these are the PREEMPT_RT version. So basically, this patch is a nop
> > for PREEMPT_RT. I doubt it will solve your bug ;-)
>
> yes, it's nop. I'm sorry, I didn't show .config.
> My kernel config is !PREEMPT_RT, it's same as the deadlock report.
> I guess it's a problem only !PREEMPT_RT.

Hmm, I'll have to look deeper into this on Monday (I'm off today -
Friday).

Is this a bug in mainline? The !PREEMPT_RT case should be as close to
mainline as possible, with no actual changes in object code. If this is
not the case, then we need to fix that.

I'll try to remember to take a look at this on Monday.

Thanks,

-- Steve

--
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/