Messages in this thread | | | Date | Fri, 20 Jan 2012 15:33:25 +0530 | From | Rabin Vincent <> | Subject | Re: [PATCHv2] backing-dev: fix wakeup timer races with bdi_unregister() |
| |
On Fri, Jan 20, 2012 at 03:15:32PM +0900, Namjae Jeon wrote: > 2012/1/20 Rabin Vincent <rabin@rab.in>: > > On Fri, Jan 20, 2012 at 05:16, Namjae Jeon <linkinjeon@gmail.com> wrote: > >>> bdi_debug_unregister(bdi); > >>> - device_unregister(bdi->dev); > >>> + > >>> + spin_lock_bh(&bdi->wb_lock); > >>> bdi->dev = NULL; > >>> + spin_unlock_bh(&bdi->wb_lock); > >> Hi. > >> Would you explain me why you add spinlock in here ? > > > > wakeup_timer_fn() does the following, where the > > trace_writeback_wake_forker_thread() also accesses bdi->dev. > > It does this under the wb_lock: > > > > } else if (bdi->dev) { > > /* > > * When bdi tasks are inactive for long time, they are killed. > > * In this case we have to wake-up the forker thread which > > * should create and run the bdi thread. > > */ > > trace_writeback_wake_forker_thread(bdi); > > > > If we don't have the lock above, the bdi->dev could potentially be > > cleared after the check but before the tracepoint is hit, leading to a > > NULL pointer dereference. > Is there no possibility trace_writeback_wake_forker_thread is called > after spin_unlock of bdi->de= null ?
wakeup_timer_fn() holds the wb_lock across the check for bdi->dev != NULL and the call to trace_writeback_wake_forker_thread(), so no. -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
| |