[PATCH] ctdb: calculate queue input buffer size correctly

Martin Schwenke martin at meltin.net
Thu Aug 9 06:35:02 UTC 2018


On Thu, 26 Jul 2018 13:21:05 +0200, Swen Schillig via samba-technical
<samba-technical at lists.samba.org> wrote:

> Please review and push if happy.
> 
> Thanks in advance for your support.

This looks like a good optimisation and it is now quite easy to
understand.

Can we please make this 2 patches for the price of 1, separating out
the switch to use MIN() to reduce num_ready when necessary?  That's a
logic change that I think is unrelated to the optimisation.  I agree
that the use of MIN() makes the logic clearer, so I think we should do
it as a patch pre- or post- the optimisation.

ioctl_list(2) says that FIONREAD expects an *int, so I don't think the
type of num_ready should change to a uint32_t.  I'm guessing there's a
platform out there where this might cause a problem.  I've checked a
few places in the Samba code and the same int that is set by FIONREAD
seems to be passed to read(2), as we're seeing in the current code.

With those 2 changes:

Reviewed-by: Martin Schwenke <martin at meltin.net>

I'm wondering is whether there's a way of avoiding the duplication of
logic where the packet size is read from the buffer. One possibility is
to rename the .extend element to .pkt_size and set it instead of a
local in queue_io_read().  However, this seems messy and would probably
clutter the patch for the optimisation.  So, I think we should go with
duplication right now to get the optimisation in and think about it
later.  :-)

peace & happiness,
martin



More information about the samba-technical mailing list