[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Re: [Nbd] [PATCH] Only send one reply on oversize writes



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: