2 / 3 _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
Applied, except for one chunk which added a barrier before update of req_cons. This was not mentioned in the changeset comment and I do not believe there is any race that needs fixing (certainly not between memcpy of request data and read of req_cons in make_response()). I applied the rest of the patch as-is. K. On 2/11/06 06:00, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:> > 2 / 3 > _______________________________________________ > Xen-devel mailing list > Xen-devel@lists.xensource.com > http://lists.xensource.com/xen-devel_______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Tue, Nov 07, 2006 at 11:16:13AM +0000, Keir Fraser wrote:> Applied, except for one chunk which added a barrier before update of > req_cons. This was not mentioned in the changeset comment and I do not > believe there is any race that needs fixing (certainly not between memcpy of > request data and read of req_cons in make_response()). I applied the rest of > the patch as-is.Thanks you for applying. Hmm.. since you don''t believe any race, then please revert this mb patch except NULL check chunk. I certainly observed that tapdisk failed to get new request on IA64. If it is really caused by the race, I will sent a patch again. (or I''ll sent another patch which fixes another issues) BTW, why is wmb() in write_rsp_to_ring() of tools/blktap/drivers/tapdisk.c necessary? RING_PUSH_RESPONSES() of kick_responses() issues wmb() so that wmb() in write_rsp_to_ring() isn''t needed, I think. thanks. -- yamahata _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 7/11/06 12:19, "Isaku Yamahata" <yamahata@valinux.co.jp> wrote:> Hmm.. since you don''t believe any race, then please revert > this mb patch except NULL check chunk. > I certainly observed that tapdisk failed to get new request on IA64. > If it is really caused by the race, I will sent a patch again. > (or I''ll sent another patch which fixes another issues)I agreed the other barriers were needed, just not that one. If it were required, we''d have the same race in blkback.c.> BTW, > why is wmb() in write_rsp_to_ring() of tools/blktap/drivers/tapdisk.c > necessary? > RING_PUSH_RESPONSES() of kick_responses() issues wmb() so that > wmb() in write_rsp_to_ring() isn''t needed, I think.I agree. I''ll remove it. -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel