Messages in this thread | | | Date | Mon, 10 Dec 2007 12:41:23 +0100 | From | Jens Axboe <> | Subject | Re: [RFC] [PATCH] A clean aEvgeniy pproach to writeout throttling |
| |
On Mon, Dec 10 2007, Daniel Phillips wrote: > On Monday 10 December 2007 02:47, Jens Axboe wrote: > > Ehm, this patch is so broken it's not even funny - did you even > > compile? You would have noticed the warning on request_queue_t, > > surely. The big problem is the last hunk here though, how would that > > work on stacked devices? Clue: ->bi_bdev is not const, it can change > > after a call to ->make_request_fn(). > > Such paranoia. Yes, the patch was compiled. Yes, the warning was > slipped through. No, it is not substantive, and in fact was removed > from another branch of our tree already. > > Ignoring the rhetoric, apparently you missed the line: > > + if (q && q->metric && !bio->bi_queue) { > > The prevents any reference ti bi_bdev after the intial call to > generic_make_request. Thanks to Evgeniy for pointing out the need for > this measure on the last go round.
Which saves the initial target, for ease of accounting at end io time - that's not the point. What happens when ->make_request_fn() changes bio->bi_bdev and returns 1, causing another iteration of the __generic_make_request() loop? 'q' is no longer the valid target, bdev_get_queue(bio->bi_bdev) is.
> "So broken" is a gross exaggeration. Substantive comments welcome.
Or you could try and make an effort to understand the comment instead of just glancing over it.
-- Jens Axboe
| |