Re: Thread flags modified without set_thread_flag() (non atomically)

[Mathieu Desnoyers]

Andrew Morton wrote:
> On Thu, 1 Mar 2007 10:34:51 +0100 Haavard Skinnemoen <hskinnemoen@xxxxxxxxx> wrote:
>
>   
> > > On Mon, 26 Feb 2007 12:10:37 -0800 Mathieu Desnoyers <compudj@xxxxxxxxxx> wrote:
> > >       
> > > > avr32/kernel/ptrace.c:          ti->flags |= _TIF_BREAKPOINT;
> > > >         
> > > No, I don't immediately see anything in the flush_old_exec() code path
> > > which tells us that nobody else can look up this thread_info (or be holding
> > > a ref to it) in this context.
> > >
> > >
> > >       
> > > > avr32/kernel/ptrace.c:                  ti->flags |= TIF_SINGLE_STEP;
> > > >         
> > > heh.  Haarvard, you got a bug.
> > >       
> > Heh, yeah. That would indeed explain some strange gdb behaviour. It
> > will only trigger when single-stepping into an exception or interrupt
> > handler so thanks for pointing it out; I would have had a hard time
> > figuring it out on my own...
> >     
>
> yup, tricky.
>
> If there's a lesson here, it is "don't provide #defines in the header for
> both versions".
>
>   

Hrm, but the bitmask version is useful (and correctly used) whenever
the flag is read and tested.

The proper way to do this would be to change every use the _TIF_* flag 
in a flag comparison
for a call to test_ti_thread_flag(). I wonder if gcc optimizes multiple 
constant test_bit() applying
on the same variable linked by logical and/or so it becomes a single 
read and a small set of comparisons.

> The block code does a similar thing:
>
> #define REQ_RW          (1 << __REQ_RW)
> #define REQ_FAILFAST    (1 << __REQ_FAILFAST)
> #define REQ_SORTED      (1 << __REQ_SORTED)
> #define REQ_SOFTBARRIER (1 << __REQ_SOFTBARRIER)
>
> and I've caught Jens using the wrong identifier at least twice in the past.
>
> It's better I think to just provide #defines for the bit offsets and
> open-code the shifting if needed.  Like PG_foo and BH_Foo.
>
>   
> > I don't think either of those need to be atomic though, since both of
> > them happen in monitor mode with interrupts disabled.
> >     
>
> That's true until you implement SMP ;)
>   

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/