[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <51A7B009.3030709@linux.intel.com>
Date: Thu, 30 May 2013 23:01:13 +0300
From: Eliezer Tamir <eliezer.tamir@...ux.intel.com>
To: Amir Vadai <amirv@...lanox.com>
CC: David Miller <davem@...emloft.net>, linux-kernel@...r.kernel.org,
netdev@...r.kernel.org,
Jesse Brandeburg <jesse.brandeburg@...el.com>,
Don Skidmore <donald.c.skidmore@...el.com>,
e1000-devel@...ts.sourceforge.net,
Willem de Bruijn <willemb@...gle.com>,
Eric Dumazet <erdnetdev@...il.com>,
Ben Hutchings <bhutchings@...arflare.com>,
Andi Kleen <andi@...stfloor.org>, HPA <hpa@...or.com>,
Eilon Greenstien <eilong@...adcom.com>,
Or Gerlitz <or.gerlitz@...il.com>,
Alex Rosenbaum <alexr@...lanox.com>,
Eliezer Tamir <eliezer@...ir.org.il>
Subject: Re: [PATCH v7 net-next 2/5] net: implement support for low latency
socket polling
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@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists