Messages in this thread | | | Date | Mon, 11 Apr 2011 09:42:51 +0200 | From | Jens Axboe <> | Subject | Re: block: ioc->refcount accessed twice in put_io_context()? |
| |
On 2011-04-11 03:54, Shaohua Li wrote: > 2011/4/10 Paul Bolle <pebolle@tiscali.nl>: >> 0) Looking for clues to solve a problem I ran into, I noticed something >> odd in block/blk-ioc.c:put_io_context(). It seems it accesses the atomic >> variable ioc->refcount twice in a way which suggests things might race. >> >> 1) Code is more exact than words, so this (entirely untested) patch to >> solve this possible race might describe better what this is all about: >> >> @@ -33,12 +33,16 @@ static void cfq_dtor(struct io_context *ioc) >> */ >> int put_io_context(struct io_context *ioc) >> { >> + int new; >> + >> if (ioc == NULL) >> return 1; >> >> - BUG_ON(atomic_long_read(&ioc->refcount) == 0); >> + new = atomic_long_dec_return(&ioc->refcount); >> + >> + BUG_ON(new < 0); >> >> - if (atomic_long_dec_and_test(&ioc->refcount)) { >> + if (new == 0) { >> rcu_read_lock(); >> cfq_dtor(ioc); >> rcu_read_unlock(); >> > so you hit this line? > BUG_ON(atomic_long_read(&ioc->refcount) == 0); > this suggests something else is already wrong, you should fix that.
Indeed, there is nothing wrong with having the BUG_ON() there first and doing the decrement later. If the BUG_ON() is hit, then it's not a race conditon - it's a plain bug in the code.
-- Jens Axboe
| |