Re: [PATCH] loop: queue_lock NULL pointer derefence inblk_throtl_exit (v2)

From: Sergey Senozhatsky
Date: Thu Jan 20 2011 - 10:27:20 EST


On (01/20/11 09:35), Vivek Goyal wrote:
> Hi Sergey,
>

Hi Vivek,

> Can we expand a little bit on comment that why do we need to have
> q->queue_lock initialized here now. Basically in the past nobody tried
> to take q->queue_lock in blk_cleanup_queue() path hence things just
> worked. Now blk throttling code is new and it takes q->queue_lock hence we
> run into issues. This could be true for some other future code too.
>
> Secondly currently blk throttle code seems to be the only user dependent
> on this lock initialization. So it might make sense to move this code
> closer to the actual call and blk_release_queue() might be even better
> place to do it atleast for now.
>

I'm afraid it's not safe to move NULL-check-and-fix out from blk_cleanup_queue,
since we're performing elevator_exit(q->elevator) call, which may depend on
q->queue_lock. For example, (./cfq-iosched.c: .elevator_exit_fn = cfq_exit_queue)
cfq_exit_queue uses q->queue_lock.


Sergey

Attachment: pgp00000.pgp
Description: PGP signature