Re: [PATCH -rt] scheduling while atomic in fs/file.c

[Daniel Walker]

On Mon, 2006-05-15 at 03:04 -0400, Steven Rostedt wrote:
> On Mon, 15 May 2006, Steven Rostedt wrote:
> 
> > On Sun, 14 May 2006, Daniel Walker wrote:
> >
> > > On Sun, 2006-05-14 at 12:44 -0400, Steven Rostedt wrote:
> > > > On Sun, 14 May 2006, Daniel Walker wrote:
> > > >
> > > > > Quite the smp_processor_id() wanrings. I don't see any SMP
> > > > > concerns here . It just adds to a percpu list, so it shouldn't
> > > > > matter if it switches after sampling fdtable_defer_list .
> > > >
> > > > I'm not so sure that there isn't SMP concerns here. I have to catch a
> > > > train in a few minutes, otherwise I would look deeper into this. But this
> > > > might be a candidate to turn fdtable_defer_list into a per_cpu_locked.
> > >
> > > I reviewed it again, and it looks like these percpu structures have a
> > > spinlock to protect the list from being emptied by a work queue while
> > > things are being added to the list . The lock appears to be used
> > > properly .  The work queue frees struct fdtable pointers added to the
> > > list , the only place these structures are added is in the block I've
> > > modified .
> > >
> > > I think making this a locked percpu would just be overkill ..
> > >
> >
> > It seems that the timer is percpu. So it has a timer for each cpu.  If you
> > switch CPUs after the put, the modtimer might put the fddef->timer onto
> > another CPU, and thus have more than one going off on the same CPU.
> >
> 
> Just to clarify, although fdtable_defer_list_init(int cpu) creates a timer
> for each CPU but sets them to the same CPU.  The mod_timer in the changed
> function is what is used to spread the timers out.

The timer is able able to migrate CPU's , also the work queue will
easily switch cpu's . That was true before .

> Although your patch wont actually break anything, since it is unlikely
> that the timer would be moved, and if it was, it would probably be put
> back again. The design is just not clean.  It's best to keep the timer
> where it is.

Why would it be a problem if the timer moved ? Or the work queue? both
are protected under a spinlock which is consistently held . 

Daniel

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