From: Vivek Goyal <vgoyal@redhat.com>
To: Sergey Senozhatsky <sergey.senozhatsky@gmail.com>
Cc: Jens Axboe <jaxboe@fusionio.com>,
Philipp Reisner <philipp.reisner@linbit.com>,
Andrew Morton <akpm@linux-foundation.org>,
Lars Ellenberg <lars.ellenberg@linbit.com>,
"Stephen M. Cameron" <scameron@beardog.cce.hp.com>,
linux-kernel@vger.kernel.org
Subject: Re: [PATCH] loop: queue_lock NULL pointer derefence in blk_throtl_exit (v2)
Date: Thu, 20 Jan 2011 10:43:27 -0500 [thread overview]
Message-ID: <20110120154327.GD18875@redhat.com> (raw)
In-Reply-To: <20110120151738.GA4665@swordfish.minsk.epam.com>
On Thu, Jan 20, 2011 at 05:18:22PM +0200, Sergey Senozhatsky wrote:
> 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.
ok, actually cfq_exit_queue() will be called only if elevator has been
initilialzed and if elevator has been initialialized that means
q->queue_lock also has been initilalized so that should not be a problem.
I am not very particular about this thing. So I will leave it to you.
Thanks
Vivek
next prev parent reply other threads:[~2011-01-20 15:45 UTC|newest]
Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top
2011-01-14 19:25 [PATCH] loop: queue_lock NULL pointer derefence in blk_throtl_exit Sergey Senozhatsky
2011-01-20 0:32 ` Jens Axboe
2011-01-20 11:32 ` Vivek Goyal
2011-01-20 12:58 ` [PATCH] loop: queue_lock NULL pointer derefence in blk_throtl_exit (v2) Sergey Senozhatsky
2011-01-20 14:35 ` Vivek Goyal
2011-01-20 15:18 ` Sergey Senozhatsky
2011-01-20 15:43 ` Vivek Goyal [this message]
2011-01-21 12:55 ` [PATCH] loop: queue_lock NULL pointer derefence in blk_throtl_exit (v3) Sergey Senozhatsky
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=20110120154327.GD18875@redhat.com \
--to=vgoyal@redhat.com \
--cc=akpm@linux-foundation.org \
--cc=jaxboe@fusionio.com \
--cc=lars.ellenberg@linbit.com \
--cc=linux-kernel@vger.kernel.org \
--cc=philipp.reisner@linbit.com \
--cc=scameron@beardog.cce.hp.com \
--cc=sergey.senozhatsky@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.