Mailing List Archive

[PATCH 2/3] blktap: mb fixes
2 / 3
Re: [PATCH 2/3] blktap: mb fixes [ In reply to ]
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
Re: [PATCH 2/3] blktap: mb fixes [ In reply to ]
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
Re: [PATCH 2/3] blktap: mb fixes [ In reply to ]
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