In several places after setting u->len we return directly on error. Apart from the partial write case, it seems all of these should be resetting u->len to 0 as happens in the success case? regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 2/3/07 16:06, "John Levon" <levon@movementarian.org> wrote:> In several places after setting u->len we return directly on error. > Apart from the partial write case, it seems all of these should be > resetting u->len to 0 as happens in the success case?Indeed. Now fixed (along with a couple of other bugs in that function). -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, Mar 02, 2007 at 04:26:22PM +0000, Keir Fraser wrote:> On 2/3/07 16:06, "John Levon" <levon@movementarian.org> wrote: > > > In several places after setting u->len we return directly on error. > > Apart from the partial write case, it seems all of these should be > > resetting u->len to 0 as happens in the success case? > > Indeed. Now fixed (along with a couple of other bugs in that function).Also, the BUG_ON in xenbusdrv_queue_reply() looks odd to me (in fact the whole function does). Are we really guaranteed that a reply message will be less than a page size? And it doesn''t seem right to just stomp on the buffer contents if we wrap right past the consumer value. We''ve seen both the BUG_ON (well, the ASSERT) and "xenstore-ls /" hang. Looking into exactly why now... regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On 2/3/07 16:35, "John Levon" <levon@movementarian.org> wrote:> Also, the BUG_ON in xenbusdrv_queue_reply() looks odd to me (in fact the > whole function does). Are we really guaranteed that a reply message will > be less than a page size? And it doesn''t seem right to just stomp on the > buffer contents if we wrap right past the consumer value.Yes, it at least makes sense to discard the data rather than BUG. Actually we should dynamically allocate up to some rather larger threshold than 4kB. Will you fix this? -- Keir _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel
On Fri, Mar 02, 2007 at 04:46:50PM +0000, Keir Fraser wrote:> > Also, the BUG_ON in xenbusdrv_queue_reply() looks odd to me (in fact the > > whole function does). Are we really guaranteed that a reply message will > > be less than a page size? And it doesn''t seem right to just stomp on the > > buffer contents if we wrap right past the consumer value. > > Yes, it at least makes sense to discard the data rather than BUG. Actually > we should dynamically allocate up to some rather larger threshold than 4kB. > Will you fix this?I doubt I''ll have time to fix /both/ kernels, sorry... regards john _______________________________________________ Xen-devel mailing list Xen-devel@lists.xensource.com http://lists.xensource.com/xen-devel