On Fri, Nov 26, 2021 at 03:23:18PM +0100, Martin Wilck wrote: > On Wed, 2021-11-24 at 19:08 -0600, Benjamin Marzinski wrote: > > On Thu, Nov 18, 2021 at 11:58:22PM +0100, mwilck@xxxxxxxx wrote: > > > From: Martin Wilck <mwilck@xxxxxxxx> > > > > > > The previous patches added the state machine and the timeout > > > handling, > > > but there was no wakeup mechanism for the uxlsnr for cases where > > > client connections were waiting for the vecs lock. > > > > > > This patch uses the previously introduced wakeup mechanism of > > > struct mutex_lock for this purpose. Processes which unlock the > > > "global" vecs lock send an event in an eventfd which the uxlsnr > > > loop is polling for. > > > > > > As we are now woken up for servicing client handlers that don't > > > wait for input but for the lock, we need to set up the pollfds > > > differently, and iterate over all clients when handling events, > > > not only over the ones that are receiving. The hangup handling > > > is changed, too. We have to look at every client, even if one has > > > hung up. Note that I don't take client_lock for the loop in > > > uxsock_listen(), it's not necessary and will be removed elsewhere > > > in a follow-up patch. > > > > > > With this in place, the lock need not be taken in execute_handler() > > > any more. The uxlsnr only ever calls trylock() on the vecs lock, > > > avoiding any waiting for other threads to finish. > > > > > > Signed-off-by: Martin Wilck <mwilck@xxxxxxxx> > > > --- > > > multipathd/uxlsnr.c | 200 ++++++++++++++++++++++++++++------------ > > > ---- > > > 1 file changed, 129 insertions(+), 71 deletions(-) > > > > > > diff --git a/multipathd/uxlsnr.c b/multipathd/uxlsnr.c > > > index 87134d5..bf9780d 100644 > > > --- a/multipathd/uxlsnr.c > > > +++ b/multipathd/uxlsnr.c > > > > Nitpick: This would look clearer to me if, instead of a switch > > statement, it was just > > > > if (c->state != CLT_RECV) > > continue; > > > > polls[i].events = POLLIN; > > polls[i].fd = c->fd; > > ... > > That's true if you look at this patch in isolation. The reason I use a > switch statement is that with patch 32, we get another case to treat > here (CLT_SEND). At that point, the switch is at least on par wrt > clarity, IMO. > > No? Yep. I retrack my objection Reviewed-by: Benjamin Marzinski <bmarzins@xxxxxxxxxx> > > Martin -- dm-devel mailing list dm-devel@xxxxxxxxxx https://listman.redhat.com/mailman/listinfo/dm-devel
