Re: [PATCH v7 net-next 2/5] net: implement support for low latencysocket polling

[Eliezer Tamir]

On 30/05/2013 18:07, Amir Vadai wrote:
> On 30/05/2013 14:41, Eliezer Tamir wrote:
> > diff --git a/fs/select.c b/fs/select.c
> > @@ -486,6 +488,8 @@ int do_select(int n, fd_set_bits *fds, struct timespec *end_time)
> >   			break;
> >   		}
> >
> > +		if (can_poll_ll(ll_time))
> > +			continue;
> I don't see here discrimination between sockets that you want to poll
> and other sockets.
> So it means that select will busy poll every type of file descriptor
> instead of going to sleep.
> Should have a condition with something like sk_valid_ll()

As I said earlier, select and poll are far from complete.
We are working on this.
Right now when you turn this on, all sockets get the same treatment.
sk_valid_ll() can't work here, because we don't have a single socket to 
work with.

At the moment, I think the right way to solve this might be to add
new flags to poll.h So that select/poll can signal to sock_poll
that it wants to busy-wait and sock_poll can tell select/poll that
it has found a socket that can support it.

Remember that the information is dynamic so we can't know in advance
which sockets can busy-poll.

If anyone else has a suggestion I would really like to hear it.

> > +static inline void sk_mark_ll(struct sock *sk, struct sk_buff *skb)
> > +{
> > +}
> > +
> > +static inline bool can_poll_ll(unsigned long end_time)
> should use here cycles_t too.

yes

> > +{
> > +	return false;
> > +}
> > +
> > +#endif /* CONFIG_NET_LL_RX_POLL */
> > +#endif /* _LINUX_NET_LL_POLL_H */
>
>
> Also, something general about this patch. I think you should split out
> to separate patches all the users of the feature, like you did for TCP.
> I would suggest small patches for UDP, select and poll.

Splitting out the users of the features as you call it will only move 
out about 30 lines of patch, I think it will make reviewing harder since 
you will not be able to see the whole picture in one patch.
But if anyone else thinks this is a good idea, I will do it.

-Eliezer
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/