Re: [Nbd] [PATCH] Only send one reply on oversize writes
- To: Alex Bligh <alex@...872...>
- Cc: nbd-general@lists.sourceforge.net
- Subject: Re: [Nbd] [PATCH] Only send one reply on oversize writes
- From: Wouter Verhelst <w@...112...>
- Date: Sat, 28 May 2011 12:12:14 +0200
- Message-id: <20110528101214.GA9316@...510...>
- In-reply-to: <33FC6E538A76BA35B1D798B4@...874...>
- References: <1306560268-19484-1-git-send-email-alex@...872...> <20110528085523.GC32221@...510...> <33FC6E538A76BA35B1D798B4@...874...>
On Sat, May 28, 2011 at 10:45:20AM +0100, Alex Bligh wrote:
> Wouter,
>
> --On 28 May 2011 10:55:23 +0200 Wouter Verhelst <w@...112...> wrote:
>
> > The oversize does check oversized writes, but it doesn't do more than
> > 'check whether the server returns something', in the way of the
> > throughput test. It would be a good idea to check the handle, indeed.
> >
> > The tests are pretty basic. They're better than nothing, but I am very
> > much aware that their coverage is... minimal. Your 'integrity' test has
> > just ramped up the coverage by a gazillion, which is great. Thanks for
> > that.
>
> Even the integrity test does a poor job testing for replies. I
> keep the inflight requests, then free them when the reply comes in.
> I do stomp over them, but essentially (just like the kernel) a
> duplicate (or a bad handle) will access memory that has been freed
> (or not allocated) because (just like the kernel) I am using the
> pointer to the request as the handle. For that reason, I check
> the magic in the /tester's/ copy of the request block too, and
> stomp over it before freeing, which hopefully should catch things
> before a SEGV, but isn't guaranteed to.
I don't think that's a big problem. If the test segfaults, it will cause
a FAIL and thus will fulfill its purpose. Of course, in that case,
figuring out why it failed would be harder than when the test were to
issue a proper error message, but it's always better than nothing.
> To do it properly, I either
> need to do a hack into the malloc library ("does this pointer
> point to a block of size X") or maintain a red/black tree or
> similar of outstanding requests - queues won't do it as tens of
> thousands of packets can be inflight at once.
What about a hash table instead? There's a GHashTable in libglib-2.0,
which we're already using. That would also allow reusing the original
handle number, which would make it slightly easier to match against
what's in the transaction log.
> This seemed too much like hard work.
:-)
--
The volume of a pizza of thickness a and radius z can be described by
the following formula:
pi zz a
Reply to: