Re: SMP barriers missing?

I've checked this in.
On Fri, 2006-01-06 at 16:06 +0530, Mithlesh Thukral wrote:
> hi ,
> 
> I have modified the kernel/kgdb.c by:
> 
> 1. converting the procindebug to an array of atomic_t type.
> 2. removing barrier() call while wait for the slave processors to enter
> or leave KGDB.
> 
> I have attached the interdiff between the current core-lite.patch and my
> new core-lite.patch.
> 
> Regards,
> Mithlesh Thukral
> 
> On Fri, 2006-01-06 at 14:45 +0530, Amit Kale wrote:
> > On Friday 06 Jan 2006 1:32 pm, Jim Blandy wrote:
> > > On 1/5/06, Amit Kale <amitkale@xxxxxxxxxxxxxx> wrote:
> > > > Given that KGDB isn't exactly a high contention region and x86 
> > > > processors
> > > > are intelligent enough to do cache snoops, I am not too surprised that
> > > > no-one has seen any problems.
> > >
> > > Right.  The i386 makes everything pretty simple.
> > >
> > > > We should convert procindebug to an array of atomic_t and then use
> > > > atomic_* macros to update and access them. atomic_set on i386, x86_64 
> > > > and
> > > > ia64 architectures doesn't invalidate cache, though it does on other
> > > > architectures like arm.
> > >
> > > Is that enough?  If we change kgdb_handle_exception to say:
> > >
> > >   /* spin_lock code is good enough as a barrier so we don't
> > >    * need one here */
> > >   atomic_set (&procindebug[smp_processor_id()]);
> > >
> > >   /* Wait a reasonable time for the other CPUs to be notified and
> > >    * be waiting for us.  Very early on this could be imperfect
> > >    * as num_online_cpus() could be 0.*/
> > >   for (i = 0; i < ROUNDUP_WAIT; i++) {
> > >           int cpu, num = 0;
> > >           for (cpu = 0; cpu < NR_CPUS; cpu++) {
> > >                         if (atomic_read (&procindebug[cpu]))
> > >                           num++;
> > >           }
> > >           if (num >= num_online_cpus()) {
> > >                   all_cpus_synced = 1;
> > >                   break;
> > >           }
> > >   }
> > >
> > > that's still no good: atomic_read and atomic_set don't have any
> > > barrier semantics, if I'm reading atomic_ops.txt correctly.  You need
> > > an smp_mb () before the inner 'for' loop, unless there's some atomic_
> > > op I haven't noticed.
> > 
> > atomic_read, atomic_set and atomic_inc are defined in a way that ensures 
> > that 
> > the value being read, written or updated is always consistent and correct 
> > across all processors. If these macros don't include a memory barrier on 
> > some 
> > platform, that would be because that platform doesn't need it.
> > 
> > >
> > > Also, in kgdb_wait, the loop needs to be rearranged as follows, to see
> > > the main CPU's store to procindebug:
> > >
> > >         for (;;) {
> > >                 barrier ();
> > >                 if (procindebug[atomic_read(&debugger_active) - 1])
> > >                         break;
> > >
> > >           int i = 10;     /* an arbitrary number */
> > >           while (--i)
> > >                   cpu_relax();
> > >   }
> > >
> > > But at this point, you're not actually using any atomic_ ops that have
> > > barrier semantics, so there's no point in making procindebug atomic
> > > anyway.  You can leave it as a plain old int.
> > 
> > There is no need to modify this loop also, when we go to atomic_t 
> > procindebug, 
> > the barrier() statement can be removed.
> > 
> > If you are using a "atomic_t var;" declaration to update and share the 
> > variable "var" across all CPUs, there is no need to add any extra code 
> > lines. 
> > atomic_* macros will take care of consistency and atomicity across all 
> > CPUs. 
> > atomic_* code is appropriate for each architecture.
> > 
> > -Amit
> > 
> > 
> > -------------------------------------------------------
> > This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
> > for problems?  Stop!  Download the new AJAX search engine that makes
> > searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
> > http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click
> > _______________________________________________
> > Kgdb-bugreport mailing list
> > Kgdb-bugreport@xxxxxxxxxxxxxxxxxxxxx
> > https://lists.sourceforge.net/lists/listinfo/kgdb-bugreport
> > 
> > 
-- 
...Miline
LinSysSoft Technologies
Ask me about KGDB Pro
http://www.linsyssoft.com/Kgdbpro.html



-------------------------------------------------------
This SF.net email is sponsored by: Splunk Inc. Do you grep through log files
for problems?  Stop!  Download the new AJAX search engine that makes
searching your log files as easy as surfing the  web.  DOWNLOAD SPLUNK!
http://ads.osdn.com/?ad_id=7637&alloc_id=16865&op=click