Hello, Alex.
I'm confused (as usual).
On Thu, May 26, 2011 at 12:00:36PM +0100, Alex Bligh wrote:
> blkdev_issue_flush appears to be a NOOP for request-based srivers, because
Ummm... request based drivers have __make_request as its
->make_request_fn() which is set during queue allocation.
> it returns -ENXIO if no make_request_function is present. AIUI only bio
> based drivers have a make_request_function. The preceding comment suggests
> this is an attempt to avoid making a request to bio based drivers before
> their queue is set up.
If you look at the original commit which added the condition,
commit f10d9f617a65905c556c3b37c9b9646ae7d04ed7
Author: Dave Chinner <dchinner@xxxxxxxxxx>
Date: Tue Jul 13 17:50:50 2010 +1000
blkdev: check for valid request queue before issuing flush
Issuing a blkdev_issue_flush() on an unconfigured loop device causes a panic as
q->make_request_fn is not configured. This can occur when trying to mount the
unconfigured loop device as an XFS filesystem. There are no guards that catch
the bio before the request function is called because we don't add a payload to
the bio. Instead, manually check this case as soon as we have a pointer to the
queue to flush.
Signed-off-by: Dave Chinner <dchinner@xxxxxxxxxx>
Signed-off-by: Jens Axboe <jaxboe@xxxxxxxxxxxx>
It was added to work around uniitialized queue from loop device. I
think this is the wrong place to check for this tho. We should have
made loop to always initialize queue and use placeholder
make_request_fn for inactive devices.
> This should not exclude request based drivers from
> getting any flushes. Instead therefore check for EITHER
> make_request_function OR request_function and queue flags. The check for
> queue flags may be unnecessary as the block layer may simply not send the
> flush on.
Have you tested the code actually works as you expect it to? Before
and after?
Thanks.
--
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
