Re: SCSI bug

From: James Bottomley <James.Bottomley@HansenPartnership.com>
To: Helge Deller <deller@gmx.de>, John David Anglin <dave.anglin@bell.net>
Cc: linux-parisc List <linux-parisc@vger.kernel.org>
Subject: Re: SCSI bug
Date: Sun, 21 Feb 2016 13:49:39 -0800	[thread overview]
Message-ID: <1456091379.2340.22.camel@HansenPartnership.com> (raw)
In-Reply-To: <56CA2982.1030607@gmx.de>

On Sun, 2016-02-21 at 22:17 +0100, Helge Deller wrote:
> On 21.02.2016 22:09, John David Anglin wrote:
> > On 2016-02-21, at 3:28 PM, James Bottomley wrote:
> > 
> > > That said, I still can't reproduce this, so you're going to have
> > > to
> > > help me find it.  Current theory is ll_merge_request_fn() it
> > > looks like
> > > there's scope for miscalculation in there.  Can you instrument
> > > this
> > > line
> > > 
> > > 	/* Merge is OK... */
> > > 	req->nr_phys_segments = total_phys_segments;
> > > 
> > > To add just before the return
> > > 
> > > if (req->nr_phys_segments != __blk_recalc_rq_segments(rq->q, rq
> > > ->bio,	false) 
> > > 	printk("MISMATCH IN MERGE: got %d, should get %d\n", 
> > > 		req->nr_phys_segments,
> > > 		__blk_recalc_rq_segments(rq->q, rq->bio, false));
> > 
> > This didn't trigger.  There were some typos:
> 
> It didn't trigger for me either.

OK, can you instrument the same thing in ll_new_hw_segment() after 

	req->nr_phys_segments += nr_phys_segs;

> > diff --git a/block/blk-merge.c b/block/blk-merge.c
> > index d9c3a75..e8969ef 100644
> > --- a/block/blk-merge.c
> > +++ b/block/blk-merge.c
> > @@ -545,6 +545,12 @@ static int ll_merge_requests_fn(struct
> > request_queue *q, struct request *req,
> >  
> >         /* Merge is OK... */
> >         req->nr_phys_segments = total_phys_segments;
> > +
> > +if (req->nr_phys_segments != __blk_recalc_rq_segments(req->q, req
> > ->bio,        false)) 
> > +       printk("MISMATCH IN MERGE: got %d, should get %d\n", 
> > +               req->nr_phys_segments,
> > +               __blk_recalc_rq_segments(req->q, req->bio, false));
> > +
> >         return 1;
> >  }
> 
> Interestingly it now triggered somewhere else...
> I enabled CONFIG_DEBUG_SG, which I had enabled the last few times as
> well, but it now happened
> for the first time:

No, that just means the sg table you initialised was too short: the
last element didn't get a sg_magic set; it's effectively the same
error, just showing differently.

James