Skip to content

Commit

Permalink
dio: remove bogus refcounting BUG_ON
Browse files Browse the repository at this point in the history
Badari Pulavarty reported a case of this BUG_ON is triggering during
testing.  It's completely bogus and should be removed.

It's trying to notice if we left references to the dio hanging around in
the sync case.  They should have been dropped as IO completed while this
path was in dio_await_completion().  This condition will also be
checked, via some twisty logic, by the BUG_ON(ret != -EIOCBQUEUED) a few
lines lower.  So to start this BUG_ON() is redundant.

More fatally, it's dereferencing dio-> after having dropped its
reference.  It's only safe to dereference the dio after releasing the
lock if the final reference was just dropped.  Another CPU might free
the dio in bio completion and reuse the memory after this path drops the
dio lock but before the BUG_ON() is evaluated.

This patch passed aio+dio regression unit tests and aio-stress on ext3.

Signed-off-by: Zach Brown <[email protected]>
Cc: Badari Pulavarty <[email protected]>
Cc: Andrew Morton <[email protected]>
Signed-off-by: Linus Torvalds <[email protected]>
  • Loading branch information
Zach Brown authored and Linus Torvalds committed Jul 4, 2007
1 parent 5dcccd8 commit fcb82f8
Showing 1 changed file with 1 addition and 1 deletion.
2 changes: 1 addition & 1 deletion fs/direct-io.c
Original file line number Diff line number Diff line change
Expand Up @@ -1106,7 +1106,7 @@ direct_io_worker(int rw, struct kiocb *iocb, struct inode *inode,
spin_lock_irqsave(&dio->bio_lock, flags);
ret2 = --dio->refcount;
spin_unlock_irqrestore(&dio->bio_lock, flags);
BUG_ON(!dio->is_async && ret2 != 0);

if (ret2 == 0) {
ret = dio_complete(dio, offset, ret);
kfree(dio);
Expand Down

0 comments on commit fcb82f8

Please sign in to comment.