* Re: [PATCH net-next] tcp: provide SYN headers for passive connections
From: Michael Kerrisk (man-pages) @ 2015-05-04 6:47 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric B Munson, Tom Herbert, David S. Miller, Linux API, netdev
In-Reply-To: <1430714086.3711.165.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
Eric,
On 4 May 2015 at 06:34, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> From: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
>
> This patch allows a server application to get the TCP SYN headers for
> its passive connections. This is useful if the server is doing
> fingerprinting of clients based on SYN packet contents.
>
> Two socket options are added: TCP_SAVE_SYN and TCP_SAVED_SYN.
>
> The first is used on a socket to enable saving the SYN headers
> for child connections. This can be set before or after the listen()
> call.
>
> The latter is used to retrieve the SYN headers for passive connections,
> if the parent listener has enabled TCP_SAVE_SYN.
>
> TCP_SAVED_SYN is read once, it frees the saved SYN headers.
>
> The data returned in TCP_SAVED_SYN are network (IPv4/IPv6) and TCP
> headers.
This description is a little thin, so I'm unclear on one or two
points. TCP_SAVE_SYN is clearly applied to the listening socket. But
what about TCP_SAVED_SYN? Is that applied to the connected socket
returned by accept()?
The highly similar naming of these two seems unfortunate. At the very
least, it makes for easy confusion in conversations about the two
options. It would be better to have names that were more distinct.
Perhaps the latter could be TCP_CONN_SYN or TCP_CONN_SAVED_SYN, for
example?
Thanks,
Michael
> Original patch was written by Tom Herbert, I changed it to not hold
> a full skb (and associated dst and conntracking reference).
>
> We have used such patch for about 3 years at Google.
>
> Signed-off-by: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
> ---
> include/linux/tcp.h | 8 ++++++++
> include/net/request_sock.h | 4 +++-
> include/uapi/linux/tcp.h | 2 ++
> net/ipv4/tcp.c | 35 +++++++++++++++++++++++++++++++++++
> net/ipv4/tcp_input.c | 18 ++++++++++++++++++
> net/ipv4/tcp_ipv4.c | 1 +
> net/ipv4/tcp_minisocks.c | 3 +++
> 7 files changed, 70 insertions(+), 1 deletion(-)
>
> diff --git a/include/linux/tcp.h b/include/linux/tcp.h
> index 3b2911502a8c..e6fb5df22db1 100644
> --- a/include/linux/tcp.h
> +++ b/include/linux/tcp.h
> @@ -199,6 +199,7 @@ struct tcp_sock {
> syn_fastopen:1, /* SYN includes Fast Open option */
> syn_fastopen_exp:1,/* SYN includes Fast Open exp. option */
> syn_data_acked:1,/* data in SYN is acked by SYN-ACK */
> + save_syn:1, /* Save headers of SYN packet */
> is_cwnd_limited:1;/* forward progress limited by snd_cwnd? */
> u32 tlp_high_seq; /* snd_nxt at the time of TLP retransmit. */
>
> @@ -326,6 +327,7 @@ struct tcp_sock {
> * socket. Used to retransmit SYNACKs etc.
> */
> struct request_sock *fastopen_rsk;
> + u32 *saved_syn;
> };
>
> enum tsq_flags {
> @@ -393,4 +395,10 @@ static inline int fastopen_init_queue(struct sock *sk, int backlog)
> return 0;
> }
>
> +static inline void tcp_saved_syn_free(struct tcp_sock *tp)
> +{
> + kfree(tp->saved_syn);
> + tp->saved_syn = NULL;
> +}
> +
> #endif /* _LINUX_TCP_H */
> diff --git a/include/net/request_sock.h b/include/net/request_sock.h
> index 9f4265ce8892..87935cad2f7b 100644
> --- a/include/net/request_sock.h
> +++ b/include/net/request_sock.h
> @@ -64,6 +64,7 @@ struct request_sock {
> struct timer_list rsk_timer;
> const struct request_sock_ops *rsk_ops;
> struct sock *sk;
> + u32 *saved_syn;
> u32 secid;
> u32 peer_secid;
> };
> @@ -77,7 +78,7 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener)
> req->rsk_ops = ops;
> sock_hold(sk_listener);
> req->rsk_listener = sk_listener;
> -
> + req->saved_syn = NULL;
> /* Following is temporary. It is coupled with debugging
> * helpers in reqsk_put() & reqsk_free()
> */
> @@ -104,6 +105,7 @@ static inline void reqsk_free(struct request_sock *req)
> req->rsk_ops->destructor(req);
> if (req->rsk_listener)
> sock_put(req->rsk_listener);
> + kfree(req->saved_syn);
> kmem_cache_free(req->rsk_ops->slab, req);
> }
>
> diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
> index faa72f4fa547..51ebedba577f 100644
> --- a/include/uapi/linux/tcp.h
> +++ b/include/uapi/linux/tcp.h
> @@ -113,6 +113,8 @@ enum {
> #define TCP_TIMESTAMP 24
> #define TCP_NOTSENT_LOWAT 25 /* limit number of unsent bytes in write queue */
> #define TCP_CC_INFO 26 /* Get Congestion Control (optional) info */
> +#define TCP_SAVE_SYN 27 /* Record SYN headers for new connections */
> +#define TCP_SAVED_SYN 28 /* Get SYN headers recorded for connection */
>
> struct tcp_repair_opt {
> __u32 opt_code;
> diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
> index 46efa03d2b11..ecccfdc50d76 100644
> --- a/net/ipv4/tcp.c
> +++ b/net/ipv4/tcp.c
> @@ -2482,6 +2482,13 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
> icsk->icsk_syn_retries = val;
> break;
>
> + case TCP_SAVE_SYN:
> + if (val < 0 || val > 1)
> + err = -EINVAL;
> + else
> + tp->save_syn = val;
> + break;
> +
> case TCP_LINGER2:
> if (val < 0)
> tp->linger2 = -1;
> @@ -2818,6 +2825,34 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
> case TCP_NOTSENT_LOWAT:
> val = tp->notsent_lowat;
> break;
> + case TCP_SAVE_SYN:
> + val = tp->save_syn;
> + break;
> + case TCP_SAVED_SYN: {
> + if (get_user(len, optlen))
> + return -EFAULT;
> +
> + lock_sock(sk);
> + if (tp->saved_syn) {
> + len = min_t(unsigned int, tp->saved_syn[0], len);
> + if (put_user(len, optlen)) {
> + release_sock(sk);
> + return -EFAULT;
> + }
> + if (copy_to_user(optval, tp->saved_syn + 1, len)) {
> + release_sock(sk);
> + return -EFAULT;
> + }
> + tcp_saved_syn_free(tp);
> + release_sock(sk);
> + } else {
> + release_sock(sk);
> + len = 0;
> + if (put_user(len, optlen))
> + return -EFAULT;
> + }
> + return 0;
> + }
> default:
> return -ENOPROTOOPT;
> }
> diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
> index 09bdc4abfcbb..df2ca615cd0c 100644
> --- a/net/ipv4/tcp_input.c
> +++ b/net/ipv4/tcp_input.c
> @@ -6060,6 +6060,23 @@ static bool tcp_syn_flood_action(struct sock *sk,
> return want_cookie;
> }
>
> +static void tcp_reqsk_record_syn(const struct sock *sk,
> + struct request_sock *req,
> + const struct sk_buff *skb)
> +{
> + if (tcp_sk(sk)->save_syn) {
> + u32 len = skb_network_header_len(skb) + tcp_hdrlen(skb);
> + u32 *copy;
> +
> + copy = kmalloc(len + sizeof(u32), GFP_ATOMIC);
> + if (copy) {
> + copy[0] = len;
> + memcpy(©[1], skb_network_header(skb), len);
> + req->saved_syn = copy;
> + }
> + }
> +}
> +
> int tcp_conn_request(struct request_sock_ops *rsk_ops,
> const struct tcp_request_sock_ops *af_ops,
> struct sock *sk, struct sk_buff *skb)
> @@ -6192,6 +6209,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
> tcp_rsk(req)->tfo_listener = false;
> af_ops->queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
> }
> + tcp_reqsk_record_syn(sk, req, skb);
>
> return 0;
>
> diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
> index fc1c658ec6c1..91cb4768a860 100644
> --- a/net/ipv4/tcp_ipv4.c
> +++ b/net/ipv4/tcp_ipv4.c
> @@ -1802,6 +1802,7 @@ void tcp_v4_destroy_sock(struct sock *sk)
>
> /* If socket is aborted during connect operation */
> tcp_free_fastopen_req(tp);
> + tcp_saved_syn_free(tp);
>
> sk_sockets_allocated_dec(sk);
> sock_release_memcg(sk);
> diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
> index e5d7649136fc..ebe2ab2596ed 100644
> --- a/net/ipv4/tcp_minisocks.c
> +++ b/net/ipv4/tcp_minisocks.c
> @@ -536,6 +536,9 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
> newtp->fastopen_rsk = NULL;
> newtp->syn_data_acked = 0;
>
> + newtp->saved_syn = req->saved_syn;
> + req->saved_syn = NULL;
> +
> TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_PASSIVEOPENS);
> }
> return newsk;
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-api" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
--
Michael Kerrisk
Linux man-pages maintainer; http://www.kernel.org/doc/man-pages/
Linux/UNIX System Programming Training: http://man7.org/training/
^ permalink raw reply
* [PATCH net-next] tcp: provide SYN headers for passive connections
From: Eric Dumazet @ 2015-05-04 4:34 UTC (permalink / raw)
To: Eric B Munson
Cc: Tom Herbert, David S. Miller, linux-api-u79uwXL29TY76Z2rM5mHXA,
netdev
In-Reply-To: <1430512894.3711.140.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
From: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
This patch allows a server application to get the TCP SYN headers for
its passive connections. This is useful if the server is doing
fingerprinting of clients based on SYN packet contents.
Two socket options are added: TCP_SAVE_SYN and TCP_SAVED_SYN.
The first is used on a socket to enable saving the SYN headers
for child connections. This can be set before or after the listen()
call.
The latter is used to retrieve the SYN headers for passive connections,
if the parent listener has enabled TCP_SAVE_SYN.
TCP_SAVED_SYN is read once, it frees the saved SYN headers.
The data returned in TCP_SAVED_SYN are network (IPv4/IPv6) and TCP
headers.
Original patch was written by Tom Herbert, I changed it to not hold
a full skb (and associated dst and conntracking reference).
We have used such patch for about 3 years at Google.
Signed-off-by: Eric Dumazet <edumazet-hpIqsD4AKlfQT0dZR+AlfA@public.gmane.org>
---
include/linux/tcp.h | 8 ++++++++
include/net/request_sock.h | 4 +++-
include/uapi/linux/tcp.h | 2 ++
net/ipv4/tcp.c | 35 +++++++++++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 18 ++++++++++++++++++
net/ipv4/tcp_ipv4.c | 1 +
net/ipv4/tcp_minisocks.c | 3 +++
7 files changed, 70 insertions(+), 1 deletion(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 3b2911502a8c..e6fb5df22db1 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -199,6 +199,7 @@ struct tcp_sock {
syn_fastopen:1, /* SYN includes Fast Open option */
syn_fastopen_exp:1,/* SYN includes Fast Open exp. option */
syn_data_acked:1,/* data in SYN is acked by SYN-ACK */
+ save_syn:1, /* Save headers of SYN packet */
is_cwnd_limited:1;/* forward progress limited by snd_cwnd? */
u32 tlp_high_seq; /* snd_nxt at the time of TLP retransmit. */
@@ -326,6 +327,7 @@ struct tcp_sock {
* socket. Used to retransmit SYNACKs etc.
*/
struct request_sock *fastopen_rsk;
+ u32 *saved_syn;
};
enum tsq_flags {
@@ -393,4 +395,10 @@ static inline int fastopen_init_queue(struct sock *sk, int backlog)
return 0;
}
+static inline void tcp_saved_syn_free(struct tcp_sock *tp)
+{
+ kfree(tp->saved_syn);
+ tp->saved_syn = NULL;
+}
+
#endif /* _LINUX_TCP_H */
diff --git a/include/net/request_sock.h b/include/net/request_sock.h
index 9f4265ce8892..87935cad2f7b 100644
--- a/include/net/request_sock.h
+++ b/include/net/request_sock.h
@@ -64,6 +64,7 @@ struct request_sock {
struct timer_list rsk_timer;
const struct request_sock_ops *rsk_ops;
struct sock *sk;
+ u32 *saved_syn;
u32 secid;
u32 peer_secid;
};
@@ -77,7 +78,7 @@ reqsk_alloc(const struct request_sock_ops *ops, struct sock *sk_listener)
req->rsk_ops = ops;
sock_hold(sk_listener);
req->rsk_listener = sk_listener;
-
+ req->saved_syn = NULL;
/* Following is temporary. It is coupled with debugging
* helpers in reqsk_put() & reqsk_free()
*/
@@ -104,6 +105,7 @@ static inline void reqsk_free(struct request_sock *req)
req->rsk_ops->destructor(req);
if (req->rsk_listener)
sock_put(req->rsk_listener);
+ kfree(req->saved_syn);
kmem_cache_free(req->rsk_ops->slab, req);
}
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index faa72f4fa547..51ebedba577f 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -113,6 +113,8 @@ enum {
#define TCP_TIMESTAMP 24
#define TCP_NOTSENT_LOWAT 25 /* limit number of unsent bytes in write queue */
#define TCP_CC_INFO 26 /* Get Congestion Control (optional) info */
+#define TCP_SAVE_SYN 27 /* Record SYN headers for new connections */
+#define TCP_SAVED_SYN 28 /* Get SYN headers recorded for connection */
struct tcp_repair_opt {
__u32 opt_code;
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 46efa03d2b11..ecccfdc50d76 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2482,6 +2482,13 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
icsk->icsk_syn_retries = val;
break;
+ case TCP_SAVE_SYN:
+ if (val < 0 || val > 1)
+ err = -EINVAL;
+ else
+ tp->save_syn = val;
+ break;
+
case TCP_LINGER2:
if (val < 0)
tp->linger2 = -1;
@@ -2818,6 +2825,34 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
case TCP_NOTSENT_LOWAT:
val = tp->notsent_lowat;
break;
+ case TCP_SAVE_SYN:
+ val = tp->save_syn;
+ break;
+ case TCP_SAVED_SYN: {
+ if (get_user(len, optlen))
+ return -EFAULT;
+
+ lock_sock(sk);
+ if (tp->saved_syn) {
+ len = min_t(unsigned int, tp->saved_syn[0], len);
+ if (put_user(len, optlen)) {
+ release_sock(sk);
+ return -EFAULT;
+ }
+ if (copy_to_user(optval, tp->saved_syn + 1, len)) {
+ release_sock(sk);
+ return -EFAULT;
+ }
+ tcp_saved_syn_free(tp);
+ release_sock(sk);
+ } else {
+ release_sock(sk);
+ len = 0;
+ if (put_user(len, optlen))
+ return -EFAULT;
+ }
+ return 0;
+ }
default:
return -ENOPROTOOPT;
}
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 09bdc4abfcbb..df2ca615cd0c 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6060,6 +6060,23 @@ static bool tcp_syn_flood_action(struct sock *sk,
return want_cookie;
}
+static void tcp_reqsk_record_syn(const struct sock *sk,
+ struct request_sock *req,
+ const struct sk_buff *skb)
+{
+ if (tcp_sk(sk)->save_syn) {
+ u32 len = skb_network_header_len(skb) + tcp_hdrlen(skb);
+ u32 *copy;
+
+ copy = kmalloc(len + sizeof(u32), GFP_ATOMIC);
+ if (copy) {
+ copy[0] = len;
+ memcpy(©[1], skb_network_header(skb), len);
+ req->saved_syn = copy;
+ }
+ }
+}
+
int tcp_conn_request(struct request_sock_ops *rsk_ops,
const struct tcp_request_sock_ops *af_ops,
struct sock *sk, struct sk_buff *skb)
@@ -6192,6 +6209,7 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
tcp_rsk(req)->tfo_listener = false;
af_ops->queue_hash_add(sk, req, TCP_TIMEOUT_INIT);
}
+ tcp_reqsk_record_syn(sk, req, skb);
return 0;
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index fc1c658ec6c1..91cb4768a860 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -1802,6 +1802,7 @@ void tcp_v4_destroy_sock(struct sock *sk)
/* If socket is aborted during connect operation */
tcp_free_fastopen_req(tp);
+ tcp_saved_syn_free(tp);
sk_sockets_allocated_dec(sk);
sock_release_memcg(sk);
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index e5d7649136fc..ebe2ab2596ed 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -536,6 +536,9 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
newtp->fastopen_rsk = NULL;
newtp->syn_data_acked = 0;
+ newtp->saved_syn = req->saved_syn;
+ req->saved_syn = NULL;
+
TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_PASSIVEOPENS);
}
return newsk;
^ permalink raw reply related
* Re: [PATCH v3 3/3] proc: add kpageidle file
From: Minchan Kim @ 2015-05-04 3:17 UTC (permalink / raw)
To: Vladimir Davydov
Cc: Andrew Morton, Johannes Weiner, Michal Hocko, Greg Thelen,
Michel Lespinasse, David Rientjes, Pavel Emelyanov,
Cyrill Gorcunov, Jonathan Corbet, linux-api, linux-doc, linux-mm,
cgroups, linux-kernel
In-Reply-To: <20150430145055.GB17640@esperanza>
On Thu, Apr 30, 2015 at 05:50:55PM +0300, Vladimir Davydov wrote:
> On Thu, Apr 30, 2015 at 05:25:31PM +0900, Minchan Kim wrote:
> > On Wed, Apr 29, 2015 at 12:12:48PM +0300, Vladimir Davydov wrote:
> > > On Wed, Apr 29, 2015 at 01:35:36PM +0900, Minchan Kim wrote:
> > > > On Tue, Apr 28, 2015 at 03:24:42PM +0300, Vladimir Davydov wrote:
> > > > > +#ifdef CONFIG_IDLE_PAGE_TRACKING
> > > > > +static struct page *kpageidle_get_page(unsigned long pfn)
> > > > > +{
> > > > > + struct page *page;
> > > > > +
> > > > > + if (!pfn_valid(pfn))
> > > > > + return NULL;
> > > > > + page = pfn_to_page(pfn);
> > > > > + /*
> > > > > + * We are only interested in user memory pages, i.e. pages that are
> > > > > + * allocated and on an LRU list.
> > > > > + */
> > > > > + if (!page || page_count(page) == 0 || !PageLRU(page))
> > > > > + return NULL;
> > > > > + if (!get_page_unless_zero(page))
> > > > > + return NULL;
> > > > > + if (unlikely(!PageLRU(page))) {
> > > >
> > > > What lock protect the check PageLRU?
> > > > If it is racing ClearPageLRU, what happens?
> > >
> > > If we hold a reference to a page and see that it's on an LRU list, it
> > > will surely remain a user memory page at least until we release the
> > > reference to it, so it must be safe to play with idle/young flags. If we
> >
> > The problem is that you pass the page in rmap reverse logic(ie, page_referenced)
> > once you judge it's LRU page so if it is false-positive, what happens?
> > A question is SetPageLRU, PageLRU, ClearPageLRU keeps memory ordering?
> > IOW, all of fields from struct page rmap can acccess should be set up completely
> > before LRU checking. Otherwise, something will be broken.
>
> So, basically you are concerned about the case when we encounter a
> freshly allocated page, which has PG_lru bit set and it's going to
> become anonymous, but it is still in the process of rmap initialization,
> i.e. its ->mapping or ->mapcount may still be uninitialized, right?
>
> AFAICS, page_referenced should handle such pages fine. Look, it only
> needs ->index, ->mapping, and ->mapcount.
>
> If ->mapping is unset, than it is NULL and rmap_walk_anon_lock ->
> page_lock_anon_vma_read will return NULL so that rmap_walk will be a
> no-op.
>
> If ->index is not initialized, than at worst we will go to
> anon_vma_interval_tree_foreach over a wrong interval, in which case we
> will see that the page is actually not mapped in page_referenced_one ->
> page_check_address and again do nothing.
>
> If ->mapcount is not initialized it is -1, and page_lock_anon_vma_read
> will return NULL, just as it does in case ->mapping = NULL.
>
> For file pages, we always take PG_locked before checking ->mapping, so
> it must be valid.
>
> Thanks,
> Vladimir
do_anonymous_page
page_add_new_anon_rmap
atomic_set(&page->_mapcount, 0);
__page_set_anon_rmap
anon_vma = (void *) anon_vma + PAGE_MAPPING_ANON;
page->mapping = (struct address_space *) anon_vma;
page->index = linear_page_index(vma, address);
lru_cache_add
__pagevec_lru_add_fn
SetPageLRU(page);
During the procedure, there is no lock to prevent race. Then, at least,
we need a write memory barrier to guarantee other fields set up before
SetPageLRU. (Of course, PageLRU should have read-memory barrier to work
well) But I can't find any barrier, either.
IOW, any fields you said could be out of order store without any lock or
memory barrier. You might argue atomic op is a barrier on x86 but it
doesn't guarantee other arches work like that so we need explict momory
barrier or lock.
Let's have a theoretical example.
CPU 0 CPU 1
do_anonymous_page
__page_set_anon_rmap
/* out of order happened so SetPageLRU is done ahead */
SetPageLRU(page)
/* Compilr changed store operation like below */
page->mapping = (struct address_space *) anon_vma;
/* Big stall happens */
/* idletacking judged it as LRU page so pass the page
in page_reference */
page_refernced
page_rmapping return true because
page->mapping has some vaule but not complete
so it calls rmap_walk_file.
it's okay to pass non-completed anon page in rmap_walk_file?
page->mapping = (struct address_space *)
((void *)page_mapping + PAGE_MAPPING_ANON);
It's too theoretical so it might be hard to happen in real practice.
My point is there is nothing to prevent explict race.
Even if there is no problem with other lock, it's fragile.
Do I miss something?
I think general way to handle PageLRU are ahead isolation or zone->lru_lock.
--
Kind regards,
Minchan Kim
--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org. For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>
^ permalink raw reply
* Re: [RFC PATCH 3/3] iio: derive the mounting matrix from ACPI _PLD objects
From: Sathyanarayanan Kuppuswamy @ 2015-05-04 1:11 UTC (permalink / raw)
To: Octavian Purdila
Cc: Jonathan Cameron, Lars-Peter Clausen, Peter Meerwald,
Robert Moore, Rafael J Wysocki, lenb-DgEjT+Ai2ygdnm+yROfE0A,
linux-api-u79uwXL29TY76Z2rM5mHXA, lkml,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-acpi-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
Srinivas Pandruvada
In-Reply-To: <CAE1zot+fusrvow+s2+CmoctcSD=c2BxoSyd2b=7MBv-pg68A+Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Octavian,
On 04/27/2015 07:23 PM, Octavian Purdila wrote:
> On Tue, Apr 28, 2015 at 12:57 AM, sathyanarayanan kuppuswamy
> <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>> Hi
>>
>> On 04/27/2015 08:54 AM, Octavian Purdila wrote:
>>> On Mon, Apr 27, 2015 at 6:42 PM, Kuppuswamy Sathyanarayanan
>>> <sathyanarayanan.kuppuswamy-VuQAYsv1563Yd54FQh9/CA@public.gmane.org> wrote:
>>>> Since Acpi framework already exports this info to user space, Why not do
>>>> this derivation in user space code ? Why do we need new ABI, if the same
>>>> can be derived from existing one.
>>>>
>>> The ABI was added in the previous patch so that we can present the
>>> sensor orientation information to userspace even in the case of device
>>> tree.
>> If the main reason for implementing a new ABI is to support DT platforms,
>> Why not implement a version of _PLD for device tree ? Don't you think it
>> would be much better than adding a new ABI to export redundant information ?
>>
> IMO the mounting matrix is more consistent with the IIO ABIs. Although
> I have no issue with repicating _PLD for device tree if people agree
> that it is better.
Since your main issue is, device tree lacking ABI to specify location
information, you should consider fixing it there. Let's wait for others
comment on this.
If you think mounting matrix provides more information than what is
supported
by _PLD, then we should consider implementing another ABI. AFAIK, that
is not
the case here.
Adding adding a new ABI to represent the information that can be derived
from existing ABI does not seem to be useful.
>
>> Also the location information of the device is not just specific to iio
>> drivers. You should consider that we would have similar requirements for
>> devices implemented as input or platform drivers.
> The upstream standard for those sensors where the orientation matters
> (accelerometer, gyro, compass) is IIO.
>
> Granted, there are other device types for which the orientation
> information may be useful (e.g. camera). However the actual
> interpretation and action to be taken is different for each subsystem
> (e.g. in the camera case do the correction via V4L2_CID_HFLIP /
> V4L2_CID_VFLIP) so I think it is better to expose it at the subsystem
> level in a way consistent with the subsystem's ABIs.
I agree that location information is used differently at different
sub systems. But my question is why we need a new ABI ?
Why not handle it in user space ?
--
--
Sathyanarayanan KN
Android Kernel Developer
^ permalink raw reply
* Re: [PATCH] iio: doc: fix typo
From: Jonathan Cameron @ 2015-05-03 19:14 UTC (permalink / raw)
To: Martin Kepplinger, irina.tirdea-ral2JQCrhuEAvxtiuMwx3w
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1430404582-13115-1-git-send-email-martink-1KBjaw7Xf1+zQB+pC5nmwQ@public.gmane.org>
On 30/04/15 15:36, Martin Kepplinger wrote:
> Since we have deviceX, we don't need accelX. This has no users as of now, so
> correcting this is no problem.
>
> Signed-off-by: Martin Kepplinger <martink-1KBjaw7Xf1+zQB+pC5nmwQ@public.gmane.org>
The X is certainly misleading. The interface could have a number there,
(if we have a device with multiple parallel accelerometers - unusual, but
not unheard of) but it would be the channel index, not the device index
and hence should be Y not X.
Or as we don't have any of these anyway, just drop the X and leave it as
an un-indexed accelerometer channel.
Jonathan
> ---
>
> That's really just a question now. If I'm wrong, sorry for the noise.
>
> Documentation/ABI/testing/sysfs-bus-iio | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 3befcb1..efd1334 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1119,7 +1119,7 @@ Description:
> This attribute is used to read the amount of quadrature error
> present in the device at a given time.
>
> -What: /sys/.../iio:deviceX/in_accelX_power_mode
> +What: /sys/.../iio:deviceX/in_accel_power_mode
> KernelVersion: 3.11
> Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Description:
>
^ permalink raw reply
* Re: [PATCH] of: unittest: overlay: Keep track of created overlays
From: Pantelis Antoniou @ 2015-05-03 17:56 UTC (permalink / raw)
To: Rob Herring
Cc: Grant Likely, Andrew Morton, Matt Porter, Koen Kooi,
Guenter Roeck, devicetree-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CAL_JsqJ7GxMMiu7U7u6bgDLut0Eme6TOowZZW9A3WXrjFuAexA-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Hi Rob,
> On Apr 29, 2015, at 18:55 , Rob Herring <robherring2-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
>
> On Fri, Apr 24, 2015 at 4:42 AM, Pantelis Antoniou
> <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org> wrote:
>> During the course of the overlay selftests some of them remain
>> applied. While this does not pose a real problem, make sure you track
>> them and destroy them at the end of the test.
>>
>> Signed-off-by: Pantelis Antoniou <pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w@public.gmane.org>
>
> This is already in for 4.1. Is this the same as the prior version.
>
It is the same as the earlier one.
> Rob
>
Regards
— Pantelis
^ permalink raw reply
* Re: [RFC PATCH 2/3] iio: allow better control for flushing the hardware fifo
From: Octavian Purdila @ 2015-05-03 6:11 UTC (permalink / raw)
To: Lars-Peter Clausen
Cc: Jonathan Cameron, Hartmut Knaack, Peter Meerwald,
linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, lkml,
Adriana Reus, linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55450C9E.4060409-Qo5EllUWu/uELgA04lAiVw@public.gmane.org>
On Sat, May 2, 2015 at 8:42 PM, Lars-Peter Clausen <lars-Qo5EllUWu/uELgA04lAiVw@public.gmane.org> wrote:
> On 04/29/2015 01:18 PM, Octavian Purdila wrote:
>>
>> Some applications need to be able to flush [1] the hardware fifo of
>> the device and to receive events of when that happened [2] so that it
>> can ignore stale data.
>>
>> This patch adds a new event (IIO_EV_TYPE_HWFIFO_FLUSHED) that should
>> be sent to userspace when a flush has been completed. The application
>> will be able to identify which are the samples to ignore based on the
>> timestamp of the event.
>>
>> To allow applications to accurately generate a hardware fifo flush on
>> demand, this patch also adds a new sysfs entry that triggers a
>> hardware fifo flush when written to.
>>
>> [1]
>> https://source.android.com/devices/sensors/hal-interface.html#flush_sensor
>> [2]
>> https://source.android.com/devices/sensors/hal-interface.html#metadata_flush_complete_events
>
>
> Since there is no asynchronous queue for commands to be executed in IIO
> adding a asynchronous completion event doesn't make too much sense. This is
> something that needs to be handled at the HAL level.
>
> The HAL needs to have a queue of commands that need to be executed where new
> events can be added asynchronously, then has a loop which goes through the
> commands in the queue and executes them, and once executed generated the
> appropriate completion event.
>
Hi Lars,
Thanks for the review.
We can't do this at the HAL level because the needed information is
only available at the HAL level. At the HAL level each received sample
from the driver is converted to an event. When doing a flush the HAL
must add a special event (flush complete) after the last sample in the
hardware fifo. But the HAL does not know how many samples are in the
hardware fifo, how many are in the device buffer, etc.
>
> I really wish that document would specify what is actually meant by flush.
> Copy the FIFO content to a software buffer or discard the FIFO content.
>
It does say: "... and flushes the FIFO; those events are delivered as
usual (i.e.: as if the maximum reporting latency had expired) ..."
>
>>
>> Signed-off-by: Octavian Purdila <octavian.purdila-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
>> ---
>> Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
>> include/linux/iio/sysfs.h | 3 +++
>> include/uapi/linux/iio/types.h | 1 +
>> 3 files changed, 15 insertions(+)
>>
>> diff --git a/Documentation/ABI/testing/sysfs-bus-iio
>> b/Documentation/ABI/testing/sysfs-bus-iio
>> index 866b4ec..bb4d8de 100644
>> --- a/Documentation/ABI/testing/sysfs-bus-iio
>> +++ b/Documentation/ABI/testing/sysfs-bus-iio
>> @@ -1375,3 +1375,14 @@ Description:
>> The emissivity ratio of the surface in the field of view
>> of the
>> contactless temperature sensor. Emissivity varies from 0
>> to 1,
>> with 1 being the emissivity of a black body.
>> +
>> +What: /sys/bus/iio/devices/iio:deviceX/buffer/hwfifo_flush
>> +KernelVersion: 4.2
>> +Contact: linux-iio-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> +Description:
>> + Write only entry that accepts a single strictly positive
>> integer
>> + specifying the number of samples to flush from the
>> hardware fifo
>> + to the device buffer. When the flush is completed an
>> + IIO_EV_TYPE_HWFIFO_FLUSHED event is generated. The event
>> has the
>> + timestamp equal with the timestamp of last sample that was
>> + flushed from the hardware fifo.
>
>
> I'd prefer this to be handled through the normal read() API rather than
> having a side channel for it. Big question is how though. We could specify
> that reading in O_NONBLOCK mode will always read data if it is available and
> not only if it is above the watermark threshold.
Do you mean to try and flush when the available data in the device
buffer is less then the requested size? That should work and hopefully
the ABI change does not matter since the hwfifo stuff has not been
released yet.
I prefer the explicit flush though. I think it is better to have the
ABIs clearly visible instead of being buried in the details.
^ permalink raw reply
* Re: [RFC PATCH 2/3] iio: allow better control for flushing the hardware fifo
From: Lars-Peter Clausen @ 2015-05-02 17:42 UTC (permalink / raw)
To: Octavian Purdila, jic23
Cc: knaack.h, pmeerw, linux-iio, linux-kernel, adriana.reus,
linux-api
In-Reply-To: <1430306340-5026-3-git-send-email-octavian.purdila@intel.com>
On 04/29/2015 01:18 PM, Octavian Purdila wrote:
> Some applications need to be able to flush [1] the hardware fifo of
> the device and to receive events of when that happened [2] so that it
> can ignore stale data.
>
> This patch adds a new event (IIO_EV_TYPE_HWFIFO_FLUSHED) that should
> be sent to userspace when a flush has been completed. The application
> will be able to identify which are the samples to ignore based on the
> timestamp of the event.
>
> To allow applications to accurately generate a hardware fifo flush on
> demand, this patch also adds a new sysfs entry that triggers a
> hardware fifo flush when written to.
>
> [1] https://source.android.com/devices/sensors/hal-interface.html#flush_sensor
> [2] https://source.android.com/devices/sensors/hal-interface.html#metadata_flush_complete_events
Since there is no asynchronous queue for commands to be executed in IIO
adding a asynchronous completion event doesn't make too much sense. This is
something that needs to be handled at the HAL level.
The HAL needs to have a queue of commands that need to be executed where new
events can be added asynchronously, then has a loop which goes through the
commands in the queue and executes them, and once executed generated the
appropriate completion event.
I really wish that document would specify what is actually meant by flush.
Copy the FIFO content to a software buffer or discard the FIFO content.
>
> Signed-off-by: Octavian Purdila <octavian.purdila@intel.com>
> ---
> Documentation/ABI/testing/sysfs-bus-iio | 11 +++++++++++
> include/linux/iio/sysfs.h | 3 +++
> include/uapi/linux/iio/types.h | 1 +
> 3 files changed, 15 insertions(+)
>
> diff --git a/Documentation/ABI/testing/sysfs-bus-iio b/Documentation/ABI/testing/sysfs-bus-iio
> index 866b4ec..bb4d8de 100644
> --- a/Documentation/ABI/testing/sysfs-bus-iio
> +++ b/Documentation/ABI/testing/sysfs-bus-iio
> @@ -1375,3 +1375,14 @@ Description:
> The emissivity ratio of the surface in the field of view of the
> contactless temperature sensor. Emissivity varies from 0 to 1,
> with 1 being the emissivity of a black body.
> +
> +What: /sys/bus/iio/devices/iio:deviceX/buffer/hwfifo_flush
> +KernelVersion: 4.2
> +Contact: linux-iio@vger.kernel.org
> +Description:
> + Write only entry that accepts a single strictly positive integer
> + specifying the number of samples to flush from the hardware fifo
> + to the device buffer. When the flush is completed an
> + IIO_EV_TYPE_HWFIFO_FLUSHED event is generated. The event has the
> + timestamp equal with the timestamp of last sample that was
> + flushed from the hardware fifo.
I'd prefer this to be handled through the normal read() API rather than
having a side channel for it. Big question is how though. We could specify
that reading in O_NONBLOCK mode will always read data if it is available and
not only if it is above the watermark threshold.
^ permalink raw reply
* Re: [PATCH v7 05/15] dt-bindings: Document the STM32 reset bindings
From: Daniel Thompson @ 2015-05-02 10:01 UTC (permalink / raw)
To: Maxime Coquelin
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Daniel Lezcano,
Joe Perches, Vladimir Zapolskiy, Jonathan Corbet, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
In-Reply-To: <CALszF6Bw900E9MtpaVmvkFxsrOODRkNg+RxRXz5V3WghHtsxbw@mail.gmail.com>
On 02/05/15 08:55, Maxime Coquelin wrote:
> 2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
>> On 30/04/15 17:20, Maxime Coquelin wrote:
>>>
>>> This adds documentation of device tree bindings for the
>>> STM32 reset controller.
>>>
>>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>>> Acked-by: Rob Herring <robh@kernel.org>
>>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>>> ---
>>> .../devicetree/bindings/reset/st,stm32-rcc.txt | 107
>>> +++++++++++++++++++++
>>> 1 file changed, 107 insertions(+)
>>> create mode 100644
>>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>>
>>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>> new file mode 100644
>>> index 0000000..c1b0f8d
>>> --- /dev/null
>>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>> @@ -0,0 +1,107 @@
>>> +STMicroelectronics STM32 Peripheral Reset Controller
>>> +====================================================
>>> +
>>> +The RCC IP is both a reset and a clock controller. This documentation
>>> only
>>> +documents the reset part.
>>> +
>>> +Please also refer to reset.txt in this directory for common reset
>>> +controller binding usage.
>>> +
>>> +Required properties:
>>> +- compatible: Should be "st,stm32-rcc"
>>> +- reg: should be register base and length as documented in the
>>> + datasheet
>>> +- #reset-cells: 1, see below
>>> +
>>> +example:
>>> +
>>> +rcc: reset@40023800 {
>>> + #reset-cells = <1>;
>>> + compatible = "st,stm32-rcc";
>>
>>
>> Do you intend the clock driver to use the same compatible string (given it
>> is the same bit of hardware).
>>
>> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me that
>> the register layout of the PLLs and dividers can be the same on the f7 parts
>> (and later).
>
> I agree we need a compatible dedicate to f4 series for clocks, and
> maybe even one for f429 (to be checked).
> For the reset part, we don't have this need.
>
> So either we use only "st,stm32f4" as you suggest, or we can have both
> in device tree:
>
> rcc: reset@40023800 {
> #reset-cells = <1>;
> compatible = "st,stm32f4-rcc", "st,stm32-rcc";
> reg = <0x40023800 0x400>;
> };
>
> What do you think?
Having both makes sense. The reset driver probably doesn't care about
differences between F4 and F7 (I know very little about F7 but I can't
think of any obvious h/ware evolution that would confuse the current
reset driver).
>>> + reg = <0x40023800 0x400>;
>>> +};
>>> +
>>> +Specifying softreset control of devices
>>> +=======================================
>>> +
>>> +Device nodes should specify the reset channel required in their "resets"
>>> +property, containing a phandle to the reset device node and an index
>>> specifying
>>> +which channel to use.
>>> +The index is the bit number within the RCC registers bank, starting from
>>> RCC
>>> +base address.
>>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>>> +Where bit_offset is the bit offset within the register.
>>> +For example, for CRC reset:
>>> + crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12
>>> = 140
>>> +
>>> +example:
>>> +
>>> + timer2 {
>>> + resets = <&rcc 256>;
>>> + };
>>> +
>>> +List of valid indices for STM32F429:
>>> + - gpioa: 128
>>> + - gpiob: 129
>>> ...
>>> <snip>
>>> ...
>>> + - sai1: 310
>>> + - ltdc: 314
>>
>>
>> These numbers are stable for all STM32F4 family parts. Should this table go
>> into a dt-bindings header file?
>>
>
> This has already been discussed with Philipp and Arnd in earlier
> versions of this series [0].
> I initially created a header file, and we decided going this way finally.
Thanks for the link. I had overlooked that (I only really started paying
attention at v5; I should probably have looked further back before
commenting).
However...
Arnd's concerns about mergability of headers can also be met by using
h/ware values in the header file can't there. To be honest my comment
was pretty heavily influenced after having read a recent patch from Rob
Herring ( https://lkml.org/lkml/2015/5/1/14 ) which does exactly this.
The main reason I got interested in having a header is that the reset
bits and the clock gate bits are encoded using the same bit patterns so
I wondering it we could express that only once.
I guess it doesn't matter that much, especially given there is only one
.dtsi file, and we can add a header later and remain binary compatible.
However if the same number set does end up repeated in different .dtsi
files I think that would motivate adding a header for F4 family.
Daniel.
^ permalink raw reply
* Re: [PATCH v7 05/15] dt-bindings: Document the STM32 reset bindings
From: Maxime Coquelin @ 2015-05-02 7:55 UTC (permalink / raw)
To: Daniel Thompson
Cc: Uwe Kleine-König, Andreas Färber, Geert Uytterhoeven,
Rob Herring, Philipp Zabel, Linus Walleij, Arnd Bergmann,
Stefan Agner, Peter Meerwald, Paul Bolle, Peter Hurley,
Andy Shevchenko, Chanwoo Choi, Russell King, Daniel Lezcano,
Joe Perches, Vladimir Zapolskiy, Jonathan Corbet, Pawel Moll,
Mark Rutland, Ian Campbell, Kumar Gala
In-Reply-To: <55433467.2010603@linaro.org>
2015-05-01 10:08 GMT+02:00 Daniel Thompson <daniel.thompson@linaro.org>:
> On 30/04/15 17:20, Maxime Coquelin wrote:
>>
>> This adds documentation of device tree bindings for the
>> STM32 reset controller.
>>
>> Tested-by: Chanwoo Choi <cw00.choi@samsung.com>
>> Acked-by: Philipp Zabel <p.zabel@pengutronix.de>
>> Acked-by: Rob Herring <robh@kernel.org>
>> Signed-off-by: Maxime Coquelin <mcoquelin.stm32@gmail.com>
>> ---
>> .../devicetree/bindings/reset/st,stm32-rcc.txt | 107
>> +++++++++++++++++++++
>> 1 file changed, 107 insertions(+)
>> create mode 100644
>> Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>>
>> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>> b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>> new file mode 100644
>> index 0000000..c1b0f8d
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>> @@ -0,0 +1,107 @@
>> +STMicroelectronics STM32 Peripheral Reset Controller
>> +====================================================
>> +
>> +The RCC IP is both a reset and a clock controller. This documentation
>> only
>> +documents the reset part.
>> +
>> +Please also refer to reset.txt in this directory for common reset
>> +controller binding usage.
>> +
>> +Required properties:
>> +- compatible: Should be "st,stm32-rcc"
>> +- reg: should be register base and length as documented in the
>> + datasheet
>> +- #reset-cells: 1, see below
>> +
>> +example:
>> +
>> +rcc: reset@40023800 {
>> + #reset-cells = <1>;
>> + compatible = "st,stm32-rcc";
>
>
> Do you intend the clock driver to use the same compatible string (given it
> is the same bit of hardware).
>
> If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me that
> the register layout of the PLLs and dividers can be the same on the f7 parts
> (and later).
I agree we need a compatible dedicate to f4 series for clocks, and
maybe even one for f429 (to be checked).
For the reset part, we don't have this need.
So either we use only "st,stm32f4" as you suggest, or we can have both
in device tree:
rcc: reset@40023800 {
#reset-cells = <1>;
compatible = "st,stm32f4-rcc", "st,stm32-rcc";
reg = <0x40023800 0x400>;
};
What do you think?
>
>
>> + reg = <0x40023800 0x400>;
>> +};
>> +
>> +Specifying softreset control of devices
>> +=======================================
>> +
>> +Device nodes should specify the reset channel required in their "resets"
>> +property, containing a phandle to the reset device node and an index
>> specifying
>> +which channel to use.
>> +The index is the bit number within the RCC registers bank, starting from
>> RCC
>> +base address.
>> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
>> +Where bit_offset is the bit offset within the register.
>> +For example, for CRC reset:
>> + crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12
>> = 140
>> +
>> +example:
>> +
>> + timer2 {
>> + resets = <&rcc 256>;
>> + };
>> +
>> +List of valid indices for STM32F429:
>> + - gpioa: 128
>> + - gpiob: 129
>> + - gpioc: 130
>> + - gpiod: 131
>> + - gpioe: 132
>> + - gpiof: 133
>> + - gpiog: 134
>> + - gpioh: 135
>> + - gpioi: 136
>> + - gpioj: 137
>> + - gpiok: 138
>> + - crc: 140
>> + - dma1: 149
>> + - dma2: 150
>> + - dma2d: 151
>> + - ethmac: 153
>> + - otghs: 157
>> + - dcmi: 160
>> + - cryp: 164
>> + - hash: 165
>> + - rng: 166
>> + - otgfs: 167
>> + - fmc: 192
>> + - tim2: 256
>> + - tim3: 257
>> + - tim4: 258
>> + - tim5: 259
>> + - tim6: 260
>> + - tim7: 261
>> + - tim12: 262
>> + - tim13: 263
>> + - tim14: 264
>> + - wwdg: 267
>> + - spi2: 270
>> + - spi3: 271
>> + - uart2: 273
>> + - uart3: 274
>> + - uart4: 275
>> + - uart5: 276
>> + - i2c1: 277
>> + - i2c2: 278
>> + - i2c3: 279
>> + - can1: 281
>> + - can2: 282
>> + - pwr: 284
>> + - dac: 285
>> + - uart7: 286
>> + - uart8: 287
>> + - tim1: 288
>> + - tim8: 289
>> + - usart1: 292
>> + - usart6: 293
>> + - adc: 296
>> + - sdio: 299
>> + - spi1: 300
>> + - spi4: 301
>> + - syscfg: 302
>> + - tim9: 304
>> + - tim10: 305
>> + - tim11: 306
>> + - spi5: 308
>> + - spi6: 309
>> + - sai1: 310
>> + - ltdc: 314
>
>
> These numbers are stable for all STM32F4 family parts. Should this table go
> into a dt-bindings header file?
>
This has already been discussed with Philipp and Arnd in earlier
versions of this series [0].
I initially created a header file, and we decided going this way finally.
Regards,
Maxime
[0]: https://lkml.org/lkml/2015/3/10/692
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric Dumazet @ 2015-05-01 20:41 UTC (permalink / raw)
To: Eric B Munson
Cc: Tom Herbert, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
Linux Kernel Network Developers, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150501202908.GC6113-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
On Fri, 2015-05-01 at 16:29 -0400, Eric B Munson wrote:
>
>
> As long as your implementation provides the IP and TCP headers, I would
> be happy with that. I am also happy to rework my implementation to
> extract and cache information when the request structure is built. If
> you all have an implementation that you want to post, I will add my ack
> if it meets our needs as well.
Yes, I believe it will be easier we provide our implementation instead
of reviewing yours ;)
For example you had :
+ case TCP_SAVED_SYN:
+ if (!((1 << sk->sk_state) & TCPF_LISTEN))
+ err = -EINVAL;
+ tp->saved_syn = !!(val);
+ break;
But if you return an error, tp->saved_syn should be left unchanged.
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric B Munson @ 2015-05-01 20:29 UTC (permalink / raw)
To: Eric Dumazet
Cc: Tom Herbert, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
Linux Kernel Network Developers, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1430511800.3711.138.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 3793 bytes --]
On Fri, 01 May 2015, Eric Dumazet wrote:
> On Fri, 2015-05-01 at 16:14 -0400, Eric B Munson wrote:
> > On Fri, 01 May 2015, Tom Herbert wrote:
> >
> > > On Fri, May 1, 2015 at 11:42 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > > On Fri, 2015-05-01 at 13:43 -0400, Eric B Munson wrote:
> > > >> In order to enable policy decisions in userspace, the data contained in
> > > >> the SYN packet would be useful for tracking or identifying connections.
> > > >> Only parts of this data are available to userspace after the hand shake
> > > >> is completed. This patch exposes a new setsockopt() option that will,
> > > >> when used with a listening socket, ask the kernel to cache the skb
> > > >> holding the SYN packet for retrieval later. The SYN skbs will not be
> > > >> saved while the kernel is in syn cookie mode.
> > > >>
> > > >> The same option will ask the kernel for the packet headers when used
> > > >> with getsockopt() with the socket returned from accept(). The cached
> > > >> packet will only be available for the first getsockopt() call, the skb
> > > >> is consumed after the requested data is copied to userspace. Subsequent
> > > >> calls will return -ENOENT. Because of this behavior, getsockopt() will
> > > >> return -E2BIG if the caller supplied a buffer that is too small to hold
> > > >> the skb header.
> > > >>
> > > >> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> > > >> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> > > >> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> > > >> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> > > >> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> > > >> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > >> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > > >> ---
> > > >
> > > > We have a similar patch here at Google, but we do not hold one skb and
> > > > dst per saved syn. That can be ~4KB for some drivers.
> > > >
> > > > Only a kmalloc() with the needed part (headers), usually less than 128
> > > > bytes. We store the length in first byte of this allocation.
> > > >
> > > > This has a huge difference if you want to have ~4 million request socks.
> > > >
> > > +1 on kmalloc solution. I posted a similar patch a couple of years ago
> > > https://patchwork.ozlabs.org/patch/146034/. There was pushback on
> > > memory usage and this having to narrow of a use case.
> > >
> > > Tom
> > >
> >
> > I cached the skb largely to take advantage of the built in reference
> > counting and avoid having to manage allocating memory and ownership of
> > said memory. For V2, how about I keep the skb reference in the request
> > structure and kmalloc() a buffer, to be owned by the tcp sock structure,
> > when the new tcp socket is created? This would also simplify the
> > getsockopt() so that the data was available to all callers until the
> > socket is closed.
>
> Please do not keep a reference on skb. This has a too big cost.
>
> Have you read that we plan to have up to 4 or 10 million request socks ?
>
> skb also holds a dst.
>
> We can upstream our implementation (based on Tom prior patch), we have
> been using it more than 2 years with success.
>
>
As long as your implementation provides the IP and TCP headers, I would
be happy with that. I am also happy to rework my implementation to
extract and cache information when the request structure is built. If
you all have an implementation that you want to post, I will add my ack
if it meets our needs as well.
Eric
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Andy Lutomirski @ 2015-05-01 20:28 UTC (permalink / raw)
To: Eric B Munson
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Network Development,
Linux API, linux-kernel@vger.kernel.org
In-Reply-To: <20150501200136.GA6113@akamai.com>
On Fri, May 1, 2015 at 1:01 PM, Eric B Munson <emunson@akamai.com> wrote:
> On Fri, 01 May 2015, Andy Lutomirski wrote:
>
>> On Fri, May 1, 2015 at 10:43 AM, Eric B Munson <emunson@akamai.com> wrote:
>> > In order to enable policy decisions in userspace, the data contained in
>> > the SYN packet would be useful for tracking or identifying connections.
>> > Only parts of this data are available to userspace after the hand shake
>> > is completed. This patch exposes a new setsockopt() option that will,
>> > when used with a listening socket, ask the kernel to cache the skb
>> > holding the SYN packet for retrieval later. The SYN skbs will not be
>> > saved while the kernel is in syn cookie mode.
>> >
>> > The same option will ask the kernel for the packet headers when used
>> > with getsockopt() with the socket returned from accept(). The cached
>> > packet will only be available for the first getsockopt() call, the skb
>> > is consumed after the requested data is copied to userspace. Subsequent
>> > calls will return -ENOENT. Because of this behavior, getsockopt() will
>> > return -E2BIG if the caller supplied a buffer that is too small to hold
>> > the skb header.
>>
>> What's the purpose and what headers are you returning?
>
> Currently the ethernet, IP, and TCP headers are being returned. The IP
> and TCP headers will be used by userspace to make decisions on how to
> handle incoming connections. The ethernet headers are being returned
> for completeness, I would be fine not including them in what is copied
> if that is a concern, however the team requesting this change here
> requires the IP and TCP headers.
>
>>
>> There was a bit of a mixup with tx timestamps where the set of headers
>> returned was possibly excessive and incompletely thought out the first
>> time around.
>
> With this in mind, we could drop copying the ethernet headers and simply
> hold onto the IP and TCP headers.
That's probably better. If nothing else, you avoid breaking userspace
when you're not using Ethernet.
--Andy
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric Dumazet @ 2015-05-01 20:23 UTC (permalink / raw)
To: Eric B Munson
Cc: Tom Herbert, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
Linux Kernel Network Developers, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150501201417.GB6113-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
On Fri, 2015-05-01 at 16:14 -0400, Eric B Munson wrote:
> On Fri, 01 May 2015, Tom Herbert wrote:
>
> > On Fri, May 1, 2015 at 11:42 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > > On Fri, 2015-05-01 at 13:43 -0400, Eric B Munson wrote:
> > >> In order to enable policy decisions in userspace, the data contained in
> > >> the SYN packet would be useful for tracking or identifying connections.
> > >> Only parts of this data are available to userspace after the hand shake
> > >> is completed. This patch exposes a new setsockopt() option that will,
> > >> when used with a listening socket, ask the kernel to cache the skb
> > >> holding the SYN packet for retrieval later. The SYN skbs will not be
> > >> saved while the kernel is in syn cookie mode.
> > >>
> > >> The same option will ask the kernel for the packet headers when used
> > >> with getsockopt() with the socket returned from accept(). The cached
> > >> packet will only be available for the first getsockopt() call, the skb
> > >> is consumed after the requested data is copied to userspace. Subsequent
> > >> calls will return -ENOENT. Because of this behavior, getsockopt() will
> > >> return -E2BIG if the caller supplied a buffer that is too small to hold
> > >> the skb header.
> > >>
> > >> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> > >> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> > >> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> > >> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> > >> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> > >> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > >> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> > >> ---
> > >
> > > We have a similar patch here at Google, but we do not hold one skb and
> > > dst per saved syn. That can be ~4KB for some drivers.
> > >
> > > Only a kmalloc() with the needed part (headers), usually less than 128
> > > bytes. We store the length in first byte of this allocation.
> > >
> > > This has a huge difference if you want to have ~4 million request socks.
> > >
> > +1 on kmalloc solution. I posted a similar patch a couple of years ago
> > https://patchwork.ozlabs.org/patch/146034/. There was pushback on
> > memory usage and this having to narrow of a use case.
> >
> > Tom
> >
>
> I cached the skb largely to take advantage of the built in reference
> counting and avoid having to manage allocating memory and ownership of
> said memory. For V2, how about I keep the skb reference in the request
> structure and kmalloc() a buffer, to be owned by the tcp sock structure,
> when the new tcp socket is created? This would also simplify the
> getsockopt() so that the data was available to all callers until the
> socket is closed.
Please do not keep a reference on skb. This has a too big cost.
Have you read that we plan to have up to 4 or 10 million request socks ?
skb also holds a dst.
We can upstream our implementation (based on Tom prior patch), we have
been using it more than 2 years with success.
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric B Munson @ 2015-05-01 20:14 UTC (permalink / raw)
To: Tom Herbert
Cc: Eric Dumazet, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
Linux Kernel Network Developers, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <CALx6S34ftz_wDoPwcJg_cMQu4QtnBJF-=d+gF5ieTA=d=r31-Q-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 2846 bytes --]
On Fri, 01 May 2015, Tom Herbert wrote:
> On Fri, May 1, 2015 at 11:42 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> > On Fri, 2015-05-01 at 13:43 -0400, Eric B Munson wrote:
> >> In order to enable policy decisions in userspace, the data contained in
> >> the SYN packet would be useful for tracking or identifying connections.
> >> Only parts of this data are available to userspace after the hand shake
> >> is completed. This patch exposes a new setsockopt() option that will,
> >> when used with a listening socket, ask the kernel to cache the skb
> >> holding the SYN packet for retrieval later. The SYN skbs will not be
> >> saved while the kernel is in syn cookie mode.
> >>
> >> The same option will ask the kernel for the packet headers when used
> >> with getsockopt() with the socket returned from accept(). The cached
> >> packet will only be available for the first getsockopt() call, the skb
> >> is consumed after the requested data is copied to userspace. Subsequent
> >> calls will return -ENOENT. Because of this behavior, getsockopt() will
> >> return -E2BIG if the caller supplied a buffer that is too small to hold
> >> the skb header.
> >>
> >> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> >> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> >> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> >> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> >> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> >> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> >> ---
> >
> > We have a similar patch here at Google, but we do not hold one skb and
> > dst per saved syn. That can be ~4KB for some drivers.
> >
> > Only a kmalloc() with the needed part (headers), usually less than 128
> > bytes. We store the length in first byte of this allocation.
> >
> > This has a huge difference if you want to have ~4 million request socks.
> >
> +1 on kmalloc solution. I posted a similar patch a couple of years ago
> https://patchwork.ozlabs.org/patch/146034/. There was pushback on
> memory usage and this having to narrow of a use case.
>
> Tom
>
I cached the skb largely to take advantage of the built in reference
counting and avoid having to manage allocating memory and ownership of
said memory. For V2, how about I keep the skb reference in the request
structure and kmalloc() a buffer, to be owned by the tcp sock structure,
when the new tcp socket is created? This would also simplify the
getsockopt() so that the data was available to all callers until the
socket is closed.
Eric
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric B Munson @ 2015-05-01 20:01 UTC (permalink / raw)
To: Andy Lutomirski
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Network Development,
Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <CALCETrWi6h3DRu6Z8jJ_-MiWqRRyKZHntpJFNON=GpAjMDYXmQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1892 bytes --]
On Fri, 01 May 2015, Andy Lutomirski wrote:
> On Fri, May 1, 2015 at 10:43 AM, Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org> wrote:
> > In order to enable policy decisions in userspace, the data contained in
> > the SYN packet would be useful for tracking or identifying connections.
> > Only parts of this data are available to userspace after the hand shake
> > is completed. This patch exposes a new setsockopt() option that will,
> > when used with a listening socket, ask the kernel to cache the skb
> > holding the SYN packet for retrieval later. The SYN skbs will not be
> > saved while the kernel is in syn cookie mode.
> >
> > The same option will ask the kernel for the packet headers when used
> > with getsockopt() with the socket returned from accept(). The cached
> > packet will only be available for the first getsockopt() call, the skb
> > is consumed after the requested data is copied to userspace. Subsequent
> > calls will return -ENOENT. Because of this behavior, getsockopt() will
> > return -E2BIG if the caller supplied a buffer that is too small to hold
> > the skb header.
>
> What's the purpose and what headers are you returning?
Currently the ethernet, IP, and TCP headers are being returned. The IP
and TCP headers will be used by userspace to make decisions on how to
handle incoming connections. The ethernet headers are being returned
for completeness, I would be fine not including them in what is copied
if that is a concern, however the team requesting this change here
requires the IP and TCP headers.
>
> There was a bit of a mixup with tx timestamps where the set of headers
> returned was possibly excessive and incompletely thought out the first
> time around.
With this in mind, we could drop copying the ethernet headers and simply
hold onto the IP and TCP headers.
>
> --Andy
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Tom Herbert @ 2015-05-01 19:55 UTC (permalink / raw)
To: Eric Dumazet
Cc: Eric B Munson, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
Linux Kernel Network Developers, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1430505777.3711.135.camel-XN9IlZ5yJG9HTL0Zs8A6p/gx64E7kk8eUsxypvmhUTTZJqsBc5GL+g@public.gmane.org>
On Fri, May 1, 2015 at 11:42 AM, Eric Dumazet <eric.dumazet-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org> wrote:
> On Fri, 2015-05-01 at 13:43 -0400, Eric B Munson wrote:
>> In order to enable policy decisions in userspace, the data contained in
>> the SYN packet would be useful for tracking or identifying connections.
>> Only parts of this data are available to userspace after the hand shake
>> is completed. This patch exposes a new setsockopt() option that will,
>> when used with a listening socket, ask the kernel to cache the skb
>> holding the SYN packet for retrieval later. The SYN skbs will not be
>> saved while the kernel is in syn cookie mode.
>>
>> The same option will ask the kernel for the packet headers when used
>> with getsockopt() with the socket returned from accept(). The cached
>> packet will only be available for the first getsockopt() call, the skb
>> is consumed after the requested data is copied to userspace. Subsequent
>> calls will return -ENOENT. Because of this behavior, getsockopt() will
>> return -E2BIG if the caller supplied a buffer that is too small to hold
>> the skb header.
>>
>> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
>> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
>> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
>> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
>> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
>> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
>> ---
>
> We have a similar patch here at Google, but we do not hold one skb and
> dst per saved syn. That can be ~4KB for some drivers.
>
> Only a kmalloc() with the needed part (headers), usually less than 128
> bytes. We store the length in first byte of this allocation.
>
> This has a huge difference if you want to have ~4 million request socks.
>
+1 on kmalloc solution. I posted a similar patch a couple of years ago
https://patchwork.ozlabs.org/patch/146034/. There was pushback on
memory usage and this having to narrow of a use case.
Tom
>
>
>
> --
> To unsubscribe from this list: send the line "unsubscribe netdev" in
> the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Andy Lutomirski @ 2015-05-01 19:27 UTC (permalink / raw)
To: Eric B Munson
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, Network Development,
Linux API, linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <1430502237-5619-1-git-send-email-emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
On Fri, May 1, 2015 at 10:43 AM, Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org> wrote:
> In order to enable policy decisions in userspace, the data contained in
> the SYN packet would be useful for tracking or identifying connections.
> Only parts of this data are available to userspace after the hand shake
> is completed. This patch exposes a new setsockopt() option that will,
> when used with a listening socket, ask the kernel to cache the skb
> holding the SYN packet for retrieval later. The SYN skbs will not be
> saved while the kernel is in syn cookie mode.
>
> The same option will ask the kernel for the packet headers when used
> with getsockopt() with the socket returned from accept(). The cached
> packet will only be available for the first getsockopt() call, the skb
> is consumed after the requested data is copied to userspace. Subsequent
> calls will return -ENOENT. Because of this behavior, getsockopt() will
> return -E2BIG if the caller supplied a buffer that is too small to hold
> the skb header.
What's the purpose and what headers are you returning?
There was a bit of a mixup with tx timestamps where the set of headers
returned was possibly excessive and incompletely thought out the first
time around.
--Andy
^ permalink raw reply
* Re: [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric Dumazet @ 2015-05-01 18:42 UTC (permalink / raw)
To: Eric B Munson
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1430502237-5619-1-git-send-email-emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
On Fri, 2015-05-01 at 13:43 -0400, Eric B Munson wrote:
> In order to enable policy decisions in userspace, the data contained in
> the SYN packet would be useful for tracking or identifying connections.
> Only parts of this data are available to userspace after the hand shake
> is completed. This patch exposes a new setsockopt() option that will,
> when used with a listening socket, ask the kernel to cache the skb
> holding the SYN packet for retrieval later. The SYN skbs will not be
> saved while the kernel is in syn cookie mode.
>
> The same option will ask the kernel for the packet headers when used
> with getsockopt() with the socket returned from accept(). The cached
> packet will only be available for the first getsockopt() call, the skb
> is consumed after the requested data is copied to userspace. Subsequent
> calls will return -ENOENT. Because of this behavior, getsockopt() will
> return -E2BIG if the caller supplied a buffer that is too small to hold
> the skb header.
>
> Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
> Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
> Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
> Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
> Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
> Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
> ---
We have a similar patch here at Google, but we do not hold one skb and
dst per saved syn. That can be ~4KB for some drivers.
Only a kmalloc() with the needed part (headers), usually less than 128
bytes. We store the length in first byte of this allocation.
This has a huge difference if you want to have ~4 million request socks.
^ permalink raw reply
* Re: [PATCH v2 01/11] coresight-etm4x: Adding CoreSight ETM4x driver
From: Paul Bolle @ 2015-05-01 18:28 UTC (permalink / raw)
To: Mathieu Poirier
Cc: Greg KH,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kaixu Xia,
Chunyan Zhang
In-Reply-To: <CANLsYkywp0o8YtjkFwfvEiAje-w2jCWbs_82SiD29xdirA_7vQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
Mathieu Poirier schreef op vr 01-05-2015 om 08:39 [-0600]:
> On 30 April 2015 at 15:29, Paul Bolle <pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org> wrote:
> > On Wed, 2015-04-29 at 11:16 -0600, Mathieu Poirier wrote:
> >> +#include <linux/module.h>
> >
> > Is this include needed?
>
> It is needed for "module_param_named()".
I guess that it should suffice to include just
include/linux/moduleparam.h. (Note that [...]/module.h includes
[...]/moduleparam.h.)
> >> +module_amba_driver(etm4x_driver);
> >
> > In a message I sent a short while ago, I suggested that for built-in
> > only code this is equivalent to calling
> > amba_driver_register(&etm4x_driver);
> >
> > from within a function marked with some sort of *initcall(). Please
> > double check.
>
> Built-in as a module or not using "module_amba_driver()" deals with
> redundant code.
This is becoming yet another pet peeve: idempotent macros prefixed with
module_. driver_ would be a better prefix. But, in this case,
driver_amba_driver() is silly, amba_driver is used already, and
register_amba_driver() looks like amba_driver_register() way too much.
I fear that this pet peeve is going to hang around my place for quite
some time.
Thanks,
Paul Bolle
^ permalink raw reply
* [PATCH] Allow TCP connections to cache SYN packet for userspace inspection
From: Eric B Munson @ 2015-05-01 17:43 UTC (permalink / raw)
To: David S. Miller
Cc: Eric B Munson, Alexey Kuznetsov, James Morris, Hideaki YOSHIFUJI,
Patrick McHardy, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In order to enable policy decisions in userspace, the data contained in
the SYN packet would be useful for tracking or identifying connections.
Only parts of this data are available to userspace after the hand shake
is completed. This patch exposes a new setsockopt() option that will,
when used with a listening socket, ask the kernel to cache the skb
holding the SYN packet for retrieval later. The SYN skbs will not be
saved while the kernel is in syn cookie mode.
The same option will ask the kernel for the packet headers when used
with getsockopt() with the socket returned from accept(). The cached
packet will only be available for the first getsockopt() call, the skb
is consumed after the requested data is copied to userspace. Subsequent
calls will return -ENOENT. Because of this behavior, getsockopt() will
return -E2BIG if the caller supplied a buffer that is too small to hold
the skb header.
Signed-off-by: Eric B Munson <emunson-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
Cc: Alexey Kuznetsov <kuznet-v/Mj1YrvjDBInbfyfbPRSQ@public.gmane.org>
Cc: James Morris <jmorris-gx6/JNMH7DfYtjvyW6yDsg@public.gmane.org>
Cc: Hideaki YOSHIFUJI <yoshfuji-VfPWfsRibaP+Ru+s062T9g@public.gmane.org>
Cc: Patrick McHardy <kaber-dcUjhNyLwpNeoWH0uzbU5w@public.gmane.org>
Cc: netdev-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
---
include/linux/tcp.h | 4 +++-
include/net/inet_sock.h | 1 +
include/uapi/linux/tcp.h | 1 +
net/ipv4/inet_connection_sock.c | 33 +++++++++++++++++++--------------
net/ipv4/tcp.c | 41 +++++++++++++++++++++++++++++++++++++++++
net/ipv4/tcp_input.c | 4 ++++
net/ipv4/tcp_ipv4.c | 1 +
net/ipv4/tcp_minisocks.c | 1 +
net/ipv6/tcp_ipv6.c | 1 +
9 files changed, 72 insertions(+), 15 deletions(-)
diff --git a/include/linux/tcp.h b/include/linux/tcp.h
index 0caa3a2..2c39d07 100644
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -191,7 +191,8 @@ struct tcp_sock {
syn_fastopen:1, /* SYN includes Fast Open option */
syn_fastopen_exp:1,/* SYN includes Fast Open exp. option */
syn_data_acked:1,/* data in SYN is acked by SYN-ACK */
- is_cwnd_limited:1;/* forward progress limited by snd_cwnd? */
+ is_cwnd_limited:1,/* forward progress limited by snd_cwnd? */
+ saved_syn:1;/* keep a copy of the syn packet */
u32 tlp_high_seq; /* snd_nxt at the time of TLP retransmit. */
/* RTT measurement */
@@ -318,6 +319,7 @@ struct tcp_sock {
* socket. Used to retransmit SYNACKs etc.
*/
struct request_sock *fastopen_rsk;
+ struct sk_buff *syn_skb;
};
enum tsq_flags {
diff --git a/include/net/inet_sock.h b/include/net/inet_sock.h
index b6c3737..cc0c18b 100644
--- a/include/net/inet_sock.h
+++ b/include/net/inet_sock.h
@@ -98,6 +98,7 @@ struct inet_request_sock {
struct ip_options_rcu *opt;
struct sk_buff *pktopts;
};
+ struct sk_buff *syn_skb;
};
static inline struct inet_request_sock *inet_rsk(const struct request_sock *sk)
diff --git a/include/uapi/linux/tcp.h b/include/uapi/linux/tcp.h
index 3b97183..5d32550 100644
--- a/include/uapi/linux/tcp.h
+++ b/include/uapi/linux/tcp.h
@@ -112,6 +112,7 @@ enum {
#define TCP_FASTOPEN 23 /* Enable FastOpen on listeners */
#define TCP_TIMESTAMP 24
#define TCP_NOTSENT_LOWAT 25 /* limit number of unsent bytes in write queue */
+#define TCP_SAVED_SYN 26 /* cache SYN packets for retrieval by userspace */
struct tcp_repair_opt {
__u32 opt_code;
diff --git a/net/ipv4/inet_connection_sock.c b/net/ipv4/inet_connection_sock.c
index 8976ca4..2abcd50 100644
--- a/net/ipv4/inet_connection_sock.c
+++ b/net/ipv4/inet_connection_sock.c
@@ -325,21 +325,26 @@ struct sock *inet_csk_accept(struct sock *sk, int flags, int *err)
newsk = req->sk;
sk_acceptq_removed(sk);
- if (sk->sk_protocol == IPPROTO_TCP &&
- tcp_rsk(req)->tfo_listener &&
- queue->fastopenq) {
- spin_lock_bh(&queue->fastopenq->lock);
- if (tcp_rsk(req)->tfo_listener) {
- /* We are still waiting for the final ACK from 3WHS
- * so can't free req now. Instead, we set req->sk to
- * NULL to signify that the child socket is taken
- * so reqsk_fastopen_remove() will free the req
- * when 3WHS finishes (or is aborted).
- */
- req->sk = NULL;
- req = NULL;
+ if (sk->sk_protocol == IPPROTO_TCP) {
+ tcp_sk(newsk)->saved_syn = tcp_sk(sk)->saved_syn;
+ if (inet_rsk(req)->syn_skb)
+ tcp_sk(newsk)->syn_skb = skb_get(inet_rsk(req)->syn_skb);
+
+ if (tcp_rsk(req)->tfo_listener && queue->fastopenq) {
+ spin_lock_bh(&queue->fastopenq->lock);
+ if (tcp_rsk(req)->tfo_listener) {
+ /* We are still waiting for the final ACK from
+ * 3WHS so can't free req now. Instead, we set
+ * req->sk to NULL to signify that the child
+ * socket is taken so reqsk_fastopen_remove()
+ * will free the req when 3WHS finishes (or is
+ * aborted).
+ */
+ req->sk = NULL;
+ req = NULL;
+ }
+ spin_unlock_bh(&queue->fastopenq->lock);
}
- spin_unlock_bh(&queue->fastopenq->lock);
}
out:
release_sock(sk);
diff --git a/net/ipv4/tcp.c b/net/ipv4/tcp.c
index 8c5cd9e..dcfc0b7 100644
--- a/net/ipv4/tcp.c
+++ b/net/ipv4/tcp.c
@@ -2227,6 +2227,8 @@ EXPORT_SYMBOL(tcp_disconnect);
void tcp_sock_destruct(struct sock *sk)
{
+ consume_skb(tcp_sk(sk)->syn_skb);
+
inet_sock_destruct(sk);
kfree(inet_csk(sk)->icsk_accept_queue.fastopenq);
@@ -2558,6 +2560,11 @@ static int do_tcp_setsockopt(struct sock *sk, int level,
tp->notsent_lowat = val;
sk->sk_write_space(sk);
break;
+ case TCP_SAVED_SYN:
+ if (!((1 << sk->sk_state) & TCPF_LISTEN))
+ err = -EINVAL;
+ tp->saved_syn = !!(val);
+ break;
default:
err = -ENOPROTOOPT;
break;
@@ -2738,6 +2745,40 @@ static int do_tcp_getsockopt(struct sock *sk, int level,
val = !icsk->icsk_ack.pingpong;
break;
+ case TCP_SAVED_SYN: {
+ struct sk_buff *syn = xchg(&tp->syn_skb, NULL);
+ int bufsz;
+ int ret = -EFAULT;
+
+ if (get_user(len, optlen))
+ goto reset;
+
+ ret = -EINVAL;
+ if ((1 << sk->sk_state) & TCPF_LISTEN)
+ goto reset;
+ if (!tp->saved_syn)
+ goto reset;
+ ret = -ENOENT;
+ if (!syn)
+ goto reset;
+ bufsz = (unsigned long)skb_tail_pointer(syn) - (unsigned long)eth_hdr(syn);
+ ret = -E2BIG;
+ if (len < bufsz)
+ goto reset;
+
+ ret = -EFAULT;
+ if (put_user(bufsz, optlen))
+ goto reset;
+ if (copy_to_user(optval, eth_hdr(syn), bufsz))
+ goto reset;
+ consume_skb(syn);
+
+ return 0;
+reset:
+ tp->syn_skb = syn;
+ return ret;
+ }
+
case TCP_CONGESTION:
if (get_user(len, optlen))
return -EFAULT;
diff --git a/net/ipv4/tcp_input.c b/net/ipv4/tcp_input.c
index 3a4d9b34..b5a61d2 100644
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -6005,6 +6005,7 @@ struct request_sock *inet_reqsk_alloc(const struct request_sock_ops *ops,
kmemcheck_annotate_bitfield(ireq, flags);
ireq->opt = NULL;
+ ireq->syn_skb = NULL;
atomic64_set(&ireq->ir_cookie, 0);
ireq->ireq_state = TCP_NEW_SYN_RECV;
write_pnet(&ireq->ireq_net, sock_net(sk_listener));
@@ -6163,6 +6164,9 @@ int tcp_conn_request(struct request_sock_ops *rsk_ops,
inet_rsk(req)->ecn_ok = 0;
}
+ if (!want_cookie && tp->saved_syn)
+ inet_rsk(req)->syn_skb = skb_get(skb);
+
tcp_rsk(req)->snt_isn = isn;
tcp_openreq_init_rwin(req, sk, dst);
fastopen = !want_cookie &&
diff --git a/net/ipv4/tcp_ipv4.c b/net/ipv4/tcp_ipv4.c
index fc1c658..c63661d 100644
--- a/net/ipv4/tcp_ipv4.c
+++ b/net/ipv4/tcp_ipv4.c
@@ -853,6 +853,7 @@ static int tcp_v4_send_synack(struct sock *sk, struct dst_entry *dst,
*/
static void tcp_v4_reqsk_destructor(struct request_sock *req)
{
+ consume_skb(inet_rsk(req)->syn_skb);
kfree(inet_rsk(req)->opt);
}
diff --git a/net/ipv4/tcp_minisocks.c b/net/ipv4/tcp_minisocks.c
index e5d7649..b3ffa73 100644
--- a/net/ipv4/tcp_minisocks.c
+++ b/net/ipv4/tcp_minisocks.c
@@ -535,6 +535,7 @@ struct sock *tcp_create_openreq_child(struct sock *sk, struct request_sock *req,
tcp_ecn_openreq_child(newtp, req);
newtp->fastopen_rsk = NULL;
newtp->syn_data_acked = 0;
+ newtp->syn_skb = NULL;
TCP_INC_STATS_BH(sock_net(sk), TCP_MIB_PASSIVEOPENS);
}
diff --git a/net/ipv6/tcp_ipv6.c b/net/ipv6/tcp_ipv6.c
index b6575d6..400ea2e 100644
--- a/net/ipv6/tcp_ipv6.c
+++ b/net/ipv6/tcp_ipv6.c
@@ -475,6 +475,7 @@ done:
static void tcp_v6_reqsk_destructor(struct request_sock *req)
{
+ consume_skb(inet_rsk(req)->syn_skb);
kfree_skb(inet_rsk(req)->pktopts);
}
--
1.9.1
^ permalink raw reply related
* Re: [PATCH v2 01/11] coresight-etm4x: Adding CoreSight ETM4x driver
From: Mathieu Poirier @ 2015-05-01 14:39 UTC (permalink / raw)
To: Paul Bolle
Cc: Greg KH,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Kaixu Xia,
Chunyan Zhang
In-Reply-To: <1430429397.2187.34.camel@x220>
On 30 April 2015 at 15:29, Paul Bolle <pebolle-IWqWACnzNjzz+pZb47iToQ@public.gmane.org> wrote:
> On Wed, 2015-04-29 at 11:16 -0600, Mathieu Poirier wrote:
>> --- a/drivers/hwtracing/coresight/Kconfig
>> +++ b/drivers/hwtracing/coresight/Kconfig
>
>> +config CORESIGHT_SOURCE_ETM4X
>> + bool "CoreSight Embedded Trace Macrocell 4.x driver"
>> + depends on ARM64
>> + select CORESIGHT_LINKS_AND_SINKS
>> + help
>> + This driver provides support for the ETM4.x tracer module, tracing the
>> + instructions that a processor is executing. This is primarily useful
>> + for instruction level tracing. Depending on the implemented version
>> + data tracing may also be available.
>
> (Please add an empty line here.)
>
>> endif
>
>> --- a/drivers/hwtracing/coresight/Makefile
>> +++ b/drivers/hwtracing/coresight/Makefile
>
>> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
>
> CORESIGHT_SOURCE_ETM4X is a bool symbol, so coresight-etm4x.o can never
> be part of a module, right?
>
> (If I'm wrong, we're done here.)
>
>> --- /dev/null
>> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
>
>> +#include <linux/module.h>
>
> Is this include needed?
It is needed for "module_param_named()".
>
>> +static struct amba_driver etm4x_driver = {
>> + .drv = {
>> + .name = "coresight-etm4x",
>> + .owner = THIS_MODULE,
>
> For built-in code this is basically equivalent to NULL (see
> include/linux/export.h).
>
>> + },
>> + .probe = etm4_probe,
>> + .remove = etm4_remove,
>> + .id_table = etm4_ids,
>> +};
>> +
>> +module_amba_driver(etm4x_driver);
>
> In a message I sent a short while ago, I suggested that for built-in
> only code this is equivalent to calling
> amba_driver_register(&etm4x_driver);
>
> from within a function marked with some sort of *initcall(). Please
> double check.
Built-in as a module or not using "module_amba_driver()" deals with
redundant code.
>
>> +MODULE_LICENSE("GPL v2");
>> +MODULE_DESCRIPTION("CoreSight Embedded Trace Macrocell v4 driver");
>
> These macros will be effectively be preprocessed away for built-in only
> code.
I agree.
>
> (There is also a reference to module_param_named(). I don't know by
> heart how that works for built-in only code, sorry.)
>
>
> Paul Bolle
>
^ permalink raw reply
* Re: [PATCH v7 05/15] dt-bindings: Document the STM32 reset bindings
From: Daniel Thompson @ 2015-05-01 8:08 UTC (permalink / raw)
To: Maxime Coquelin, u.kleine-koenig-bIcnvbaLZ9MEGnE8C9+IrQ,
afaerber-l3A5Bk7waGM, geert-Td1EMuHUCqxL1ZNQvxDV9g, Rob Herring,
Philipp Zabel, Linus Walleij, Arnd Bergmann, stefan-XLVq0VzYD2Y,
pmeerw-jW+XmwGofnusTnJN9+BGXg, pebolle-IWqWACnzNjzz+pZb47iToQ,
peter-WaGBZJeGNqdsbIuE7sb01tBPR1lH4CV8,
andy.shevchenko-Re5JQEeQqe8AvxtiuMwx3w,
cw00.choi-Sze3O3UU22JBDgjK7y7TUQ, Russell King, Daniel Lezcano,
joe-6d6DIl74uiNBDgjK7y7TUQ, Vladimir Zapolskiy
Cc: Jonathan Corbet, Pawel Moll, Mark Rutland, Ian Campbell,
Kumar Gala, Thomas Gleixner, Greg Kroah-Hartman, Jiri Slaby,
Andrew Morton, David S. Miller, Mauro Carvalho Chehab,
Antti Palosaari, Tejun Heo, Will Deacon, Nikolay Borisov,
Rusty Russell, Kees Cook, Michal Marek,
linux-doc-u79uwXL29TY76Z2rM5mHXA,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-gpio-u79uwXL29TY76Z2rM5mHXA,
linux-serial-u79uwXL29TY76Z2rM5mHXA,
linux-arch-u79uwXL29TZNg+MwTxZMZA
In-Reply-To: <1430410844-16062-6-git-send-email-mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
On 30/04/15 17:20, Maxime Coquelin wrote:
> This adds documentation of device tree bindings for the
> STM32 reset controller.
>
> Tested-by: Chanwoo Choi <cw00.choi-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
> Acked-by: Philipp Zabel <p.zabel-bIcnvbaLZ9MEGnE8C9+IrQ@public.gmane.org>
> Acked-by: Rob Herring <robh-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org>
> Signed-off-by: Maxime Coquelin <mcoquelin.stm32-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
> ---
> .../devicetree/bindings/reset/st,stm32-rcc.txt | 107 +++++++++++++++++++++
> 1 file changed, 107 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
>
> diff --git a/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
> new file mode 100644
> index 0000000..c1b0f8d
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/reset/st,stm32-rcc.txt
> @@ -0,0 +1,107 @@
> +STMicroelectronics STM32 Peripheral Reset Controller
> +====================================================
> +
> +The RCC IP is both a reset and a clock controller. This documentation only
> +documents the reset part.
> +
> +Please also refer to reset.txt in this directory for common reset
> +controller binding usage.
> +
> +Required properties:
> +- compatible: Should be "st,stm32-rcc"
> +- reg: should be register base and length as documented in the
> + datasheet
> +- #reset-cells: 1, see below
> +
> +example:
> +
> +rcc: reset@40023800 {
> + #reset-cells = <1>;
> + compatible = "st,stm32-rcc";
Do you intend the clock driver to use the same compatible string (given
it is the same bit of hardware).
If so, is it better to use st,stm32f4-rcc here? It seems unlikey to me
that the register layout of the PLLs and dividers can be the same on the
f7 parts (and later).
> + reg = <0x40023800 0x400>;
> +};
> +
> +Specifying softreset control of devices
> +=======================================
> +
> +Device nodes should specify the reset channel required in their "resets"
> +property, containing a phandle to the reset device node and an index specifying
> +which channel to use.
> +The index is the bit number within the RCC registers bank, starting from RCC
> +base address.
> +It is calculated as: index = register_offset / 4 * 32 + bit_offset.
> +Where bit_offset is the bit offset within the register.
> +For example, for CRC reset:
> + crc = AHB1RSTR_offset / 4 * 32 + CRCRST_bit_offset = 0x10 / 4 * 32 + 12 = 140
> +
> +example:
> +
> + timer2 {
> + resets = <&rcc 256>;
> + };
> +
> +List of valid indices for STM32F429:
> + - gpioa: 128
> + - gpiob: 129
> + - gpioc: 130
> + - gpiod: 131
> + - gpioe: 132
> + - gpiof: 133
> + - gpiog: 134
> + - gpioh: 135
> + - gpioi: 136
> + - gpioj: 137
> + - gpiok: 138
> + - crc: 140
> + - dma1: 149
> + - dma2: 150
> + - dma2d: 151
> + - ethmac: 153
> + - otghs: 157
> + - dcmi: 160
> + - cryp: 164
> + - hash: 165
> + - rng: 166
> + - otgfs: 167
> + - fmc: 192
> + - tim2: 256
> + - tim3: 257
> + - tim4: 258
> + - tim5: 259
> + - tim6: 260
> + - tim7: 261
> + - tim12: 262
> + - tim13: 263
> + - tim14: 264
> + - wwdg: 267
> + - spi2: 270
> + - spi3: 271
> + - uart2: 273
> + - uart3: 274
> + - uart4: 275
> + - uart5: 276
> + - i2c1: 277
> + - i2c2: 278
> + - i2c3: 279
> + - can1: 281
> + - can2: 282
> + - pwr: 284
> + - dac: 285
> + - uart7: 286
> + - uart8: 287
> + - tim1: 288
> + - tim8: 289
> + - usart1: 292
> + - usart6: 293
> + - adc: 296
> + - sdio: 299
> + - spi1: 300
> + - spi4: 301
> + - syscfg: 302
> + - tim9: 304
> + - tim10: 305
> + - tim11: 306
> + - spi5: 308
> + - spi6: 309
> + - sai1: 310
> + - ltdc: 314
These numbers are stable for all STM32F4 family parts. Should this table
go into a dt-bindings header file?
^ permalink raw reply
* Re: [PATCH v2 01/11] coresight-etm4x: Adding CoreSight ETM4x driver
From: Paul Bolle @ 2015-04-30 21:29 UTC (permalink / raw)
To: Mathieu Poirier
Cc: gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
kaixu.xia-QSEj5FYQhm4dnm+yROfE0A,
zhang.chunyan-QSEj5FYQhm4dnm+yROfE0A
In-Reply-To: <1430327795-10710-2-git-send-email-mathieu.poirier-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
On Wed, 2015-04-29 at 11:16 -0600, Mathieu Poirier wrote:
> --- a/drivers/hwtracing/coresight/Kconfig
> +++ b/drivers/hwtracing/coresight/Kconfig
> +config CORESIGHT_SOURCE_ETM4X
> + bool "CoreSight Embedded Trace Macrocell 4.x driver"
> + depends on ARM64
> + select CORESIGHT_LINKS_AND_SINKS
> + help
> + This driver provides support for the ETM4.x tracer module, tracing the
> + instructions that a processor is executing. This is primarily useful
> + for instruction level tracing. Depending on the implemented version
> + data tracing may also be available.
(Please add an empty line here.)
> endif
> --- a/drivers/hwtracing/coresight/Makefile
> +++ b/drivers/hwtracing/coresight/Makefile
> +obj-$(CONFIG_CORESIGHT_SOURCE_ETM4X) += coresight-etm4x.o
CORESIGHT_SOURCE_ETM4X is a bool symbol, so coresight-etm4x.o can never
be part of a module, right?
(If I'm wrong, we're done here.)
> --- /dev/null
> +++ b/drivers/hwtracing/coresight/coresight-etm4x.c
> +#include <linux/module.h>
Is this include needed?
> +static struct amba_driver etm4x_driver = {
> + .drv = {
> + .name = "coresight-etm4x",
> + .owner = THIS_MODULE,
For built-in code this is basically equivalent to NULL (see
include/linux/export.h).
> + },
> + .probe = etm4_probe,
> + .remove = etm4_remove,
> + .id_table = etm4_ids,
> +};
> +
> +module_amba_driver(etm4x_driver);
In a message I sent a short while ago, I suggested that for built-in
only code this is equivalent to calling
amba_driver_register(&etm4x_driver);
from within a function marked with some sort of *initcall(). Please
double check.
> +MODULE_LICENSE("GPL v2");
> +MODULE_DESCRIPTION("CoreSight Embedded Trace Macrocell v4 driver");
These macros will be effectively be preprocessed away for built-in only
code.
(There is also a reference to module_param_named(). I don't know by
heart how that works for built-in only code, sorry.)
Paul Bolle
^ permalink raw reply
* Re: Regression: Requiring CAP_SYS_ADMIN for /proc/<pid>/pagemap causes application-level breakage
From: Mark Williamson @ 2015-04-30 18:45 UTC (permalink / raw)
To: Kirill A. Shutemov
Cc: Konstantin Khlebnikov, Linus Torvalds, Mark Seaborn, kernel list,
Kirill A. Shutemov, Pavel Emelyanov, Andrew Morton,
Andy Lutomirski, Linux API, Finn Grimwood, Daniel James
In-Reply-To: <20150430132230.GE15874-nhfs4B5ZimeFUdmeq17FyvUpdFzICT1y@public.gmane.org>
>> Something like this (see patch in attachment)
>
> THP is not covered.
>
> Any comments on kcmp() idea?
It seems like a modified kcmp() would also be a valid approach but, as
you noted, probably speed-limited for our purposes. As you say, there
is the option of a vector-oriented equivalent. It seems like a
generally nice facility to have available in the kernel but my
suspicion is that it wouldn't be optimal for us...
My thinking is that using soft-dirty might give us the best
performance on platforms where it's available. We're only using
fork() as a cunning/hacky way of tracking what pages change;
soft-dirty would allow us to avoid that too, whereas using kcmp()
would still require the forking overhead.
Does that make sense, or have I missed something?
Thanks,
Mark
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox