Re: [Qemu-devel] [PATCH v4 3/8] block: add request tracking

[Marcelo Tosatti <mtosatti@redhat.com>]

On Mon, Dec 05, 2011 at 04:20:55PM +0000, Stefan Hajnoczi wrote:
> On Mon, Dec 5, 2011 at 4:09 PM, Marcelo Tosatti <mtosatti@redhat.com> wrote:
> > On Wed, Nov 23, 2011 at 11:47:53AM +0000, Stefan Hajnoczi wrote:
> > On earlier discussion, you replied to me:
> >
> > "
> >>>      req = tracked_request_add(bs, sector_num, nb_sectors, false);
> >>
> >> The tracked request should include cluster round info?
> >
> > When checking A and B for overlap, only one of them needs to be
> > rounded in order for overlap detection to be correct.  Therefore we
> > can store the original request [start, length) in tracked_requests and
> > only round the new request.
> > "
> >
> > The problem AFAICS is this:
> >
> > - Store a non-cluster-aligned request in the tracked request list.
> > - Wait on that non-cluster-aligned request
> >  (wait_for_overlapping_requests).
> > - Submit cluster-aligned request for COR request.
> >
> > So, the tracked request list does not properly reflect the in-flight
> > COR requests. Which can result in:
> >
> > 1) guest reads sector 10.
> > 2) <sector_num=10,nb_sectors=2> added to tracked request list.
> > 3) COR code submits read for <sector_num=10,nb_sectors=2+cluster_align>
> 
> It will also round down to sector_num=0 when cluster size is 128
> sectors (e.g. qcow2 and qed).
> 
> > 4) unrelated guest operation writes to sector 13, nb_sectors=1. That is
> > allowed to proceed without waiting because tracked request list does not
> > reflect what COR in-flight requests.
> 
> The tracked request list has <sector_num=10, nb_sectors=2> and the
> candidate write request is <sector_num=13, nb_sectors=1>.
> 
> In wait_for_overlapping_requests() we round the candidate request to
> <sector_num=0, nb_sectors=cluster_size>.  This rounded request does
> overlap <sector_num=10, sectors=2> so it will need to wait.
> 
> Therefore CoR and write will not execute at the same time.
> 
> Does this make more sense?
> 
> Stefan

Ah yes same mistake on my part, again. Thanks.