Re: [GIT PULL] Revert of the IO stat fix

[Maxim Levitsky <maximlevitsky@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 1760 bytes --]

On Sun, 2010-10-24 at 22:42 +0200, Jens Axboe wrote:
> On 2010-10-24 22:35, Linus Torvalds wrote:
> > On Sun, Oct 24, 2010 at 1:09 PM, Jens Axboe <jaxboe@fusionio.com> wrote:
> >>
> >> The fix for cross-partition merges screwing up disk stats turns out
> >> to be problematic on various levels. Lets revert this one so we have
> >> time to come up with a proper solution for this.
> > 
> > Hmm.. I think the reverted patch looks like it really is the right
> > thing to do, so I hate reverting it this early. What were the problems
> > with it?
> > 
> > Btw, one thing that seems to be missing in the original commit (which
> > is not necessarily the reason for the trouble, of course), is that
> > elv_rq_merge_ok() seems to not check the partition. As far as I can
> > tell, we should have a
> > 
> >     if (req->part != bio->bi_bdev->bd_part)
> >        return 0;
> > 
> > there, no? And you should _not_ set rq->part in "drive_stat_acct()",
> > you should set it from bio->bi_bdev->bd_part when you create the
> > request.
> > 
> > (And if it is NULL, just don't do partition accounting at all)
> > 
> > Hmm? What am I missing? What were the bugs?
> 
> The patch itself is sound, the problems are around the area of it not
> really liking non-elevator devices with the elv_quiesce_start/end()
> parts. I had the below patch for that, but then I could not decide
> whether we were fully safe on queue free after talking to Vivek about
> it.
> 
> So that shows up as an oops on removal of mspro for instance, and loop
> throws a fit as well. Coverage of the bug is too large just to let side
> idle for a few days.
Btw I had that bug reproduced 100% for all removeable devices I have.
(xD, ms, usb).


Attached logs for each.

Best regards,
	Maxim Levitsky

[-- Attachment #2: logs.tar.bz2 --]
[-- Type: application/x-bzip-compressed-tar, Size: 38268 bytes --]