* Re: [PATCH net-next 2/3] ipv4: L3 and L4 hash-based multipath routing
From: Alexander Duyck @ 2015-06-18 22:52 UTC (permalink / raw)
To: Peter Nørlund, netdev
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-api
In-Reply-To: <1434571686-5149-3-git-send-email-pch@ordbogen.com>
On 06/17/2015 01:08 PM, Peter Nørlund wrote:
> This patch adds L3 and L4 hash-based multipath routing, selectable on a
> per-route basis with the reintroduced RTA_MP_ALGO attribute. The default is
> now RT_MP_ALG_L3_HASH.
>
> Signed-off-by: Peter Nørlund <pch@ordbogen.com>
> ---
> include/net/ip_fib.h | 4 ++-
> include/net/route.h | 5 ++--
> include/uapi/linux/rtnetlink.h | 14 ++++++++++-
> net/ipv4/fib_frontend.c | 4 +++
> net/ipv4/fib_semantics.c | 34 ++++++++++++++++++++++---
> net/ipv4/icmp.c | 4 +--
> net/ipv4/route.c | 56 +++++++++++++++++++++++++++++++++++-------
> net/ipv4/xfrm4_policy.c | 2 +-
> 8 files changed, 103 insertions(+), 20 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 4be4f25..250d98e 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -37,6 +37,7 @@ struct fib_config {
> u32 fc_flags;
> u32 fc_priority;
> __be32 fc_prefsrc;
> + int fc_mp_alg;
> struct nlattr *fc_mx;
> struct rtnexthop *fc_mp;
> int fc_mx_len;
> @@ -116,6 +117,7 @@ struct fib_info {
> int fib_nhs;
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> int fib_mp_weight;
> + int fib_mp_alg;
> #endif
> struct rcu_head rcu;
> struct fib_nh fib_nh[0];
> @@ -308,7 +310,7 @@ int ip_fib_check_default(__be32 gw, struct net_device *dev);
> int fib_sync_down_dev(struct net_device *dev, int force);
> int fib_sync_down_addr(struct net *net, __be32 local);
> int fib_sync_up(struct net_device *dev);
> -void fib_select_multipath(struct fib_result *res);
> +void fib_select_multipath(struct fib_result *res, const struct flowi4 *flow);
>
> /* Exported by fib_trie.c */
> void fib_trie_init(void);
> diff --git a/include/net/route.h b/include/net/route.h
> index fe22d03..1fc7deb 100644
> --- a/include/net/route.h
> +++ b/include/net/route.h
> @@ -110,7 +110,8 @@ struct in_device;
> int ip_rt_init(void);
> void rt_cache_flush(struct net *net);
> void rt_flush_dev(struct net_device *dev);
> -struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp);
> +struct rtable *__ip_route_output_key(struct net *, struct flowi4 *flp,
> + const struct flowi4 *mp_flow);
> struct rtable *ip_route_output_flow(struct net *, struct flowi4 *flp,
> struct sock *sk);
> struct dst_entry *ipv4_blackhole_route(struct net *net,
> @@ -267,7 +268,7 @@ static inline struct rtable *ip_route_connect(struct flowi4 *fl4,
> sport, dport, sk);
>
> if (!dst || !src) {
> - rt = __ip_route_output_key(net, fl4);
> + rt = __ip_route_output_key(net, fl4, NULL);
> if (IS_ERR(rt))
> return rt;
> ip_rt_put(rt);
> diff --git a/include/uapi/linux/rtnetlink.h b/include/uapi/linux/rtnetlink.h
> index 17fb02f..dff4a72 100644
> --- a/include/uapi/linux/rtnetlink.h
> +++ b/include/uapi/linux/rtnetlink.h
> @@ -271,6 +271,18 @@ enum rt_scope_t {
> #define RTM_F_EQUALIZE 0x400 /* Multipath equalizer: NI */
> #define RTM_F_PREFIX 0x800 /* Prefix addresses */
>
> +/* Multipath algorithms */
> +
> +enum rt_mp_alg_t {
> + RT_MP_ALG_L3_HASH, /* Was IP_MP_ALG_NONE */
> + RT_MP_ALG_PER_PACKET, /* Was IP_MP_ALG_RR */
> + RT_MP_ALG_DRR, /* not used */
> + RT_MP_ALG_RANDOM, /* not used */
> + RT_MP_ALG_WRANDOM, /* not used */
> + RT_MP_ALG_L4_HASH,
> + __RT_MP_ALG_MAX
> +};
> +
> /* Reserved table identifiers */
>
> enum rt_class_t {
> @@ -301,7 +313,7 @@ enum rtattr_type_t {
> RTA_FLOW,
> RTA_CACHEINFO,
> RTA_SESSION, /* no longer used */
> - RTA_MP_ALGO, /* no longer used */
> + RTA_MP_ALGO,
> RTA_TABLE,
> RTA_MARK,
> RTA_MFC_STATS,
> diff --git a/net/ipv4/fib_frontend.c b/net/ipv4/fib_frontend.c
> index 872494e..376e8c1 100644
> --- a/net/ipv4/fib_frontend.c
> +++ b/net/ipv4/fib_frontend.c
> @@ -590,6 +590,7 @@ const struct nla_policy rtm_ipv4_policy[RTA_MAX + 1] = {
> [RTA_PREFSRC] = { .type = NLA_U32 },
> [RTA_METRICS] = { .type = NLA_NESTED },
> [RTA_MULTIPATH] = { .len = sizeof(struct rtnexthop) },
> + [RTA_MP_ALGO] = { .type = NLA_U32 },
> [RTA_FLOW] = { .type = NLA_U32 },
> };
>
> @@ -650,6 +651,9 @@ static int rtm_to_fib_config(struct net *net, struct sk_buff *skb,
> cfg->fc_mp = nla_data(attr);
> cfg->fc_mp_len = nla_len(attr);
> break;
> + case RTA_MP_ALGO:
> + cfg->fc_mp_alg = nla_get_u32(attr);
> + break;
> case RTA_FLOW:
> cfg->fc_flow = nla_get_u32(attr);
> break;
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 8c8df80..da06e88 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -257,6 +257,11 @@ static inline int nh_comp(const struct fib_info *fi, const struct fib_info *ofi)
> {
> const struct fib_nh *onh = ofi->fib_nh;
>
> +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> + if (fi->fib_mp_alg != ofi->fib_mp_alg)
> + return -1;
> +#endif
> +
> for_nexthops(fi) {
> if (nh->nh_oif != onh->nh_oif ||
> nh->nh_gw != onh->nh_gw ||
> @@ -896,6 +901,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
>
> if (cfg->fc_mp) {
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> + fi->fib_mp_alg = cfg->fc_mp_alg;
> err = fib_get_nhs(fi, cfg->fc_mp, cfg->fc_mp_len, cfg);
> if (err != 0)
> goto failure;
> @@ -1085,6 +1091,10 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
> struct rtnexthop *rtnh;
> struct nlattr *mp;
>
> + if (fi->fib_mp_alg &&
> + nla_put_u32(skb, RTA_MP_ALGO, fi->fib_mp_alg))
> + goto nla_put_failure;
> +
> mp = nla_nest_start(skb, RTA_MULTIPATH);
> if (!mp)
> goto nla_put_failure;
> @@ -1312,15 +1322,31 @@ int fib_sync_up(struct net_device *dev)
> }
>
> /*
> - * The algorithm is suboptimal, but it provides really
> - * fair weighted route distribution.
> + * Compute multipath hash based on 3- or 5-tuple
> */
> -void fib_select_multipath(struct fib_result *res)
> +static int fib_multipath_hash(const struct fib_result *res,
> + const struct flowi4 *flow)
> +{
> + u32 hash = flow->saddr ^ flow->daddr;
> +
> + if (res->fi->fib_mp_alg == RT_MP_ALG_L4_HASH && flow->flowi4_proto != 0)
> + hash ^= flow->flowi4_proto ^ flow->fl4_sport ^ flow->fl4_dport;
> +
> + hash ^= hash >> 16;
> + hash ^= hash >> 8;
> + return hash & 0xFF;
> +}
> +
This hash is still far from optimal. Really I think you should look at
something such as jhash_3words or the like for mixing up the addresses.
Right now just XORing the values together like you are will end up
with a fairly high collision rate since for example in the case of two
endpoints on the same subnet you would lose the subnet as a result of
XORing the source and destination addresses. Also you would lose the
port data in the case of a protocol using something such as UDP where
the source and destination ports might be the same value.
> +void fib_select_multipath(struct fib_result *res, const struct flowi4 *flow)
> {
> struct fib_info *fi = res->fi;
> u8 w;
>
> - w = bitrev8(this_cpu_inc_return(fib_mp_rr_counter));
> + if (res->fi->fib_mp_alg == RT_MP_ALG_PER_PACKET) {
> + w = bitrev8(this_cpu_inc_return(fib_mp_rr_counter));
> + } else {
> + w = fib_multipath_hash(res, flow);
> + }
>
> for_nexthops(fi) {
> if (w >= atomic_read(&nh->nh_mp_upper_bound))
> diff --git a/net/ipv4/icmp.c b/net/ipv4/icmp.c
> index f5203fb..3abcfea 100644
> --- a/net/ipv4/icmp.c
> +++ b/net/ipv4/icmp.c
> @@ -459,7 +459,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
> fl4->fl4_icmp_type = type;
> fl4->fl4_icmp_code = code;
> security_skb_classify_flow(skb_in, flowi4_to_flowi(fl4));
> - rt = __ip_route_output_key(net, fl4);
> + rt = __ip_route_output_key(net, fl4, NULL);
> if (IS_ERR(rt))
> return rt;
>
> @@ -481,7 +481,7 @@ static struct rtable *icmp_route_lookup(struct net *net,
> goto relookup_failed;
>
> if (inet_addr_type(net, fl4_dec.saddr) == RTN_LOCAL) {
> - rt2 = __ip_route_output_key(net, &fl4_dec);
> + rt2 = __ip_route_output_key(net, &fl4_dec, NULL);
> if (IS_ERR(rt2))
> err = PTR_ERR(rt2);
> } else {
> diff --git a/net/ipv4/route.c b/net/ipv4/route.c
> index f605598..a1ec62c 100644
> --- a/net/ipv4/route.c
> +++ b/net/ipv4/route.c
> @@ -1006,7 +1006,7 @@ void ipv4_update_pmtu(struct sk_buff *skb, struct net *net, u32 mtu,
>
> __build_flow_key(&fl4, NULL, iph, oif,
> RT_TOS(iph->tos), protocol, mark, flow_flags);
> - rt = __ip_route_output_key(net, &fl4);
> + rt = __ip_route_output_key(net, &fl4, NULL);
> if (!IS_ERR(rt)) {
> __ip_rt_update_pmtu(rt, &fl4, mtu);
> ip_rt_put(rt);
> @@ -1025,7 +1025,7 @@ static void __ipv4_sk_update_pmtu(struct sk_buff *skb, struct sock *sk, u32 mtu)
> if (!fl4.flowi4_mark)
> fl4.flowi4_mark = IP4_REPLY_MARK(sock_net(sk), skb->mark);
>
> - rt = __ip_route_output_key(sock_net(sk), &fl4);
> + rt = __ip_route_output_key(sock_net(sk), &fl4, NULL);
> if (!IS_ERR(rt)) {
> __ip_rt_update_pmtu(rt, &fl4, mtu);
> ip_rt_put(rt);
> @@ -1094,7 +1094,7 @@ void ipv4_redirect(struct sk_buff *skb, struct net *net,
>
> __build_flow_key(&fl4, NULL, iph, oif,
> RT_TOS(iph->tos), protocol, mark, flow_flags);
> - rt = __ip_route_output_key(net, &fl4);
> + rt = __ip_route_output_key(net, &fl4, NULL);
> if (!IS_ERR(rt)) {
> __ip_do_redirect(rt, skb, &fl4, false);
> ip_rt_put(rt);
> @@ -1109,7 +1109,7 @@ void ipv4_sk_redirect(struct sk_buff *skb, struct sock *sk)
> struct rtable *rt;
>
> __build_flow_key(&fl4, sk, iph, 0, 0, 0, 0, 0);
> - rt = __ip_route_output_key(sock_net(sk), &fl4);
> + rt = __ip_route_output_key(sock_net(sk), &fl4, NULL);
> if (!IS_ERR(rt)) {
> __ip_do_redirect(rt, skb, &fl4, false);
> ip_rt_put(rt);
> @@ -1631,6 +1631,39 @@ out:
> return err;
> }
>
> +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> +/* Fill flow key data based on packet for use in multipath routing. */
> +static void ip_multipath_flow(const struct sk_buff *skb, struct flowi4 *flow)
> +{
> + const struct iphdr *iph;
> +
> + iph = ip_hdr(skb);
> +
> + flow->saddr = iph->saddr;
> + flow->daddr = iph->daddr;
> + flow->flowi4_proto = iph->protocol;
> + flow->fl4_sport = 0;
> + flow->fl4_dport = 0;
> +
> + if (unlikely(ip_is_fragment(iph)))
> + return;
> +
I'm not sure if checking for fragmentation is enough. For example if
this system is routing and received a flow of UDP packets, some
fragmented some not then it might end up mixing them over 2 separate
next hops since some will include L4 header data and some won't.
As such you may want to have the option to exclude UDP from the
protocols listed below.
> + if (iph->protocol == IPPROTO_TCP ||
> + iph->protocol == IPPROTO_UDP ||
> + iph->protocol == IPPROTO_SCTP) {
> + __be16 _ports;
> + const __be16 *ports;
> +
> + ports = skb_header_pointer(skb, iph->ihl * 4, sizeof(_ports),
> + &_ports);
> + if (ports) {
> + flow->fl4_sport = ports[0];
> + flow->fl4_dport = ports[1];
> + }
> + }
> +}
> +#endif /* CONFIG_IP_ROUTE_MULTIPATH */
> +
> static int ip_mkroute_input(struct sk_buff *skb,
> struct fib_result *res,
> const struct flowi4 *fl4,
> @@ -1638,8 +1671,12 @@ static int ip_mkroute_input(struct sk_buff *skb,
> __be32 daddr, __be32 saddr, u32 tos)
> {
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> - if (res->fi && res->fi->fib_nhs > 1)
> - fib_select_multipath(res);
> + if (res->fi && res->fi->fib_nhs > 1) {
> + struct flowi4 mp_flow;
> +
> + ip_multipath_flow(skb, &mp_flow);
> + fib_select_multipath(res, &mp_flow);
> + }
What is the point in populating the mp_flow if you don't know if it is
going to be used? You are populating it in ip_multipath_flow, and then
you might completely ignore it in fib_select_multipath.
Maybe instead of passing the mp_flow you could instead look at passing a
function pointer that would alter the flow for the multipath case instead.
> #endif
>
> /* create a routing cache entry */
> @@ -2012,7 +2049,8 @@ add:
> * Major route resolver routine.
> */
>
> -struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
> +struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4,
> + const struct flowi4 *mp_flow)
> {
> struct net_device *dev_out = NULL;
> __u8 tos = RT_FL_TOS(fl4);
> @@ -2170,7 +2208,7 @@ struct rtable *__ip_route_output_key(struct net *net, struct flowi4 *fl4)
>
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> if (res.fi->fib_nhs > 1 && fl4->flowi4_oif == 0)
> - fib_select_multipath(&res);
> + fib_select_multipath(&res, (mp_flow ? mp_flow : fl4));
> else
> #endif
> if (!res.prefixlen &&
> @@ -2273,7 +2311,7 @@ struct dst_entry *ipv4_blackhole_route(struct net *net, struct dst_entry *dst_or
> struct rtable *ip_route_output_flow(struct net *net, struct flowi4 *flp4,
> struct sock *sk)
> {
> - struct rtable *rt = __ip_route_output_key(net, flp4);
> + struct rtable *rt = __ip_route_output_key(net, flp4, NULL);
>
> if (IS_ERR(rt))
> return rt;
> diff --git a/net/ipv4/xfrm4_policy.c b/net/ipv4/xfrm4_policy.c
> index bff6974..7eae158 100644
> --- a/net/ipv4/xfrm4_policy.c
> +++ b/net/ipv4/xfrm4_policy.c
> @@ -31,7 +31,7 @@ static struct dst_entry *__xfrm4_dst_lookup(struct net *net, struct flowi4 *fl4,
> if (saddr)
> fl4->saddr = saddr->a4;
>
> - rt = __ip_route_output_key(net, fl4);
> + rt = __ip_route_output_key(net, fl4, NULL);
> if (!IS_ERR(rt))
> return &rt->dst;
>
>
^ permalink raw reply
* Re: [RESEND PATCH V2 1/3] Add mmap flag to request pages are locked after page fault
From: Eric B Munson @ 2015-06-18 20:30 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-alpha, linux-kernel, linux-mips,
linux-parisc, linuxppc-dev, sparclinux, linux-xtensa, linux-mm,
linux-arch, linux-api
In-Reply-To: <20150618152907.GG5858@dhcp22.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 11761 bytes --]
On Thu, 18 Jun 2015, Michal Hocko wrote:
> [Sorry for the late reply - I meant to answer in the previous threads
> but something always preempted me from that]
>
> On Wed 10-06-15 09:26:48, Eric B Munson wrote:
> > The cost of faulting in all memory to be locked can be very high when
> > working with large mappings. If only portions of the mapping will be
> > used this can incur a high penalty for locking.
> >
> > For the example of a large file, this is the usage pattern for a large
> > statical language model (probably applies to other statical or graphical
> > models as well). For the security example, any application transacting
> > in data that cannot be swapped out (credit card data, medical records,
> > etc).
>
> Such a use case makes some sense to me but I am not sure the way you
> implement it is the right one. This is another mlock related flag for
> mmap with a different semantic. You do not want to prefault but e.g. is
> the readahead or fault around acceptable? I do not see anything in your
> patch to handle those...
We haven't bumped into readahead or fault around causing performance
problems for us. If they cause problems for users when LOCKONFAULT is
in use then we can address them.
>
> Wouldn't it be much more reasonable and straightforward to have
> MAP_FAULTPOPULATE as a counterpart for MAP_POPULATE which would
> explicitly disallow any form of pre-faulting? It would be usable for
> other usecases than with MAP_LOCKED combination.
I don't see a clear case for it being more reasonable, it is one
possible way to solve the problem. But I think it leaves us in an even
more akward state WRT VMA flags. As you noted in your fix for the
mmap() man page, one can get into a state where a VMA is VM_LOCKED, but
not present. Having VM_LOCKONFAULT states that this was intentional, if
we go to using MAP_FAULTPOPULATE instead of MAP_LOCKONFAULT, we no
longer set VM_LOCKONFAULT (unless we want to start mapping it to the
presence of two MAP_ flags). This can make detecting the MAP_LOCKED +
populate failure state harder.
If this is the preferred path for mmap(), I am fine with that. However,
I would like to see the new system calls that Andrew mentioned (and that
I am testing patches for) go in as well. That way we give users the
ability to request VM_LOCKONFAULT for memory allocated using something
other than mmap.
>
> > This patch introduces the ability to request that pages are not
> > pre-faulted, but are placed on the unevictable LRU when they are finally
> > faulted in.
> >
> > To keep accounting checks out of the page fault path, users are billed
> > for the entire mapping lock as if MAP_LOCKED was used.
> >
> > Signed-off-by: Eric B Munson <emunson@akamai.com>
> > Cc: Michal Hocko <mhocko@suse.cz>
> > Cc: linux-alpha@vger.kernel.org
> > Cc: linux-kernel@vger.kernel.org
> > Cc: linux-mips@linux-mips.org
> > Cc: linux-parisc@vger.kernel.org
> > Cc: linuxppc-dev@lists.ozlabs.org
> > Cc: sparclinux@vger.kernel.org
> > Cc: linux-xtensa@linux-xtensa.org
> > Cc: linux-mm@kvack.org
> > Cc: linux-arch@vger.kernel.org
> > Cc: linux-api@vger.kernel.org
> > ---
> > arch/alpha/include/uapi/asm/mman.h | 1 +
> > arch/mips/include/uapi/asm/mman.h | 1 +
> > arch/parisc/include/uapi/asm/mman.h | 1 +
> > arch/powerpc/include/uapi/asm/mman.h | 1 +
> > arch/sparc/include/uapi/asm/mman.h | 1 +
> > arch/tile/include/uapi/asm/mman.h | 1 +
> > arch/xtensa/include/uapi/asm/mman.h | 1 +
> > include/linux/mm.h | 1 +
> > include/linux/mman.h | 3 ++-
> > include/uapi/asm-generic/mman.h | 1 +
> > mm/mmap.c | 4 ++--
> > mm/swap.c | 3 ++-
> > 12 files changed, 15 insertions(+), 4 deletions(-)
> >
> > diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
> > index 0086b47..15e96e1 100644
> > --- a/arch/alpha/include/uapi/asm/mman.h
> > +++ b/arch/alpha/include/uapi/asm/mman.h
> > @@ -30,6 +30,7 @@
> > #define MAP_NONBLOCK 0x40000 /* do not block on IO */
> > #define MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x100000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x200000 /* Lock pages after they are faulted in, do not prefault */
> >
> > #define MS_ASYNC 1 /* sync memory asynchronously */
> > #define MS_SYNC 2 /* synchronous memory sync */
> > diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
> > index cfcb876..47846a5 100644
> > --- a/arch/mips/include/uapi/asm/mman.h
> > +++ b/arch/mips/include/uapi/asm/mman.h
> > @@ -48,6 +48,7 @@
> > #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> > #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x100000 /* Lock pages after they are faulted in, do not prefault */
> >
> > /*
> > * Flags for msync
> > diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
> > index 294d251..1514cd7 100644
> > --- a/arch/parisc/include/uapi/asm/mman.h
> > +++ b/arch/parisc/include/uapi/asm/mman.h
> > @@ -24,6 +24,7 @@
> > #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> > #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x100000 /* Lock pages after they are faulted in, do not prefault */
> >
> > #define MS_SYNC 1 /* synchronous memory sync */
> > #define MS_ASYNC 2 /* sync memory asynchronously */
> > diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
> > index 6ea26df..fce74fe 100644
> > --- a/arch/powerpc/include/uapi/asm/mman.h
> > +++ b/arch/powerpc/include/uapi/asm/mman.h
> > @@ -27,5 +27,6 @@
> > #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> > #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x80000 /* Lock pages after they are faulted in, do not prefault */
> >
> > #endif /* _UAPI_ASM_POWERPC_MMAN_H */
> > diff --git a/arch/sparc/include/uapi/asm/mman.h b/arch/sparc/include/uapi/asm/mman.h
> > index 0b14df3..12425d8 100644
> > --- a/arch/sparc/include/uapi/asm/mman.h
> > +++ b/arch/sparc/include/uapi/asm/mman.h
> > @@ -22,6 +22,7 @@
> > #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> > #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x80000 /* Lock pages after they are faulted in, do not prefault */
> >
> >
> > #endif /* _UAPI__SPARC_MMAN_H__ */
> > diff --git a/arch/tile/include/uapi/asm/mman.h b/arch/tile/include/uapi/asm/mman.h
> > index 81b8fc3..ec04eaf 100644
> > --- a/arch/tile/include/uapi/asm/mman.h
> > +++ b/arch/tile/include/uapi/asm/mman.h
> > @@ -29,6 +29,7 @@
> > #define MAP_DENYWRITE 0x0800 /* ETXTBSY */
> > #define MAP_EXECUTABLE 0x1000 /* mark it as an executable */
> > #define MAP_HUGETLB 0x4000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x8000 /* Lock pages after they are faulted in, do not prefault */
> >
> >
> > /*
> > diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
> > index 201aec0..42d43cc 100644
> > --- a/arch/xtensa/include/uapi/asm/mman.h
> > +++ b/arch/xtensa/include/uapi/asm/mman.h
> > @@ -55,6 +55,7 @@
> > #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> > #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x100000 /* Lock pages after they are faulted in, do not prefault */
> > #ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
> > # define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be
> > * uninitialized */
> > diff --git a/include/linux/mm.h b/include/linux/mm.h
> > index 0755b9f..3e31457 100644
> > --- a/include/linux/mm.h
> > +++ b/include/linux/mm.h
> > @@ -126,6 +126,7 @@ extern unsigned int kobjsize(const void *objp);
> > #define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
> > #define VM_DENYWRITE 0x00000800 /* ETXTBSY on write attempts.. */
> >
> > +#define VM_LOCKONFAULT 0x00001000 /* Lock the pages covered when they are faulted in */
> > #define VM_LOCKED 0x00002000
> > #define VM_IO 0x00004000 /* Memory mapped I/O or similar */
> >
> > diff --git a/include/linux/mman.h b/include/linux/mman.h
> > index 16373c8..437264b 100644
> > --- a/include/linux/mman.h
> > +++ b/include/linux/mman.h
> > @@ -86,7 +86,8 @@ calc_vm_flag_bits(unsigned long flags)
> > {
> > return _calc_vm_trans(flags, MAP_GROWSDOWN, VM_GROWSDOWN ) |
> > _calc_vm_trans(flags, MAP_DENYWRITE, VM_DENYWRITE ) |
> > - _calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED );
> > + _calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED ) |
> > + _calc_vm_trans(flags, MAP_LOCKONFAULT,VM_LOCKONFAULT);
> > }
> >
> > unsigned long vm_commit_limit(void);
> > diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
> > index e9fe6fd..fc4e586 100644
> > --- a/include/uapi/asm-generic/mman.h
> > +++ b/include/uapi/asm-generic/mman.h
> > @@ -12,6 +12,7 @@
> > #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> > #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> > #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> > +#define MAP_LOCKONFAULT 0x80000 /* Lock pages after they are faulted in, do not prefault */
> >
> > /* Bits [26:31] are reserved, see mman-common.h for MAP_HUGETLB usage */
> >
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index bb50cac..ba1a6bf 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -1233,7 +1233,7 @@ static inline int mlock_future_check(struct mm_struct *mm,
> > unsigned long locked, lock_limit;
> >
> > /* mlock MCL_FUTURE? */
> > - if (flags & VM_LOCKED) {
> > + if (flags & (VM_LOCKED | VM_LOCKONFAULT)) {
> > locked = len >> PAGE_SHIFT;
> > locked += mm->locked_vm;
> > lock_limit = rlimit(RLIMIT_MEMLOCK);
> > @@ -1301,7 +1301,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
> > vm_flags = calc_vm_prot_bits(prot) | calc_vm_flag_bits(flags) |
> > mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
> >
> > - if (flags & MAP_LOCKED)
> > + if (flags & (MAP_LOCKED | MAP_LOCKONFAULT))
> > if (!can_do_mlock())
> > return -EPERM;
> >
> > diff --git a/mm/swap.c b/mm/swap.c
> > index a7251a8..07c905e 100644
> > --- a/mm/swap.c
> > +++ b/mm/swap.c
> > @@ -711,7 +711,8 @@ void lru_cache_add_active_or_unevictable(struct page *page,
> > {
> > VM_BUG_ON_PAGE(PageLRU(page), page);
> >
> > - if (likely((vma->vm_flags & (VM_LOCKED | VM_SPECIAL)) != VM_LOCKED)) {
> > + if (likely((vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT)) == 0) ||
> > + (vma->vm_flags & VM_SPECIAL)) {
> > SetPageActive(page);
> > lru_cache_add(page);
> > return;
> > --
> > 1.9.1
> >
>
> --
> Michal Hocko
> SUSE Labs
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH net-next 1/3] ipv4: Lock-less per-packet multipath
From: Alexander Duyck @ 2015-06-18 19:42 UTC (permalink / raw)
To: Peter Nørlund, netdev-u79uwXL29TY76Z2rM5mHXA
Cc: David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434571686-5149-2-git-send-email-pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>
On 06/17/2015 01:08 PM, Peter Nørlund wrote:
> The current multipath attempted to be quasi random, but in most cases it
> behaved just like a round robin balancing. This patch refactors the
> algorithm to be exactly that and in doing so, avoids the spin lock.
>
> The new design paves the way for hash-based multipath, replacing the
> modulo with thresholds, minimizing disruption in case of failing paths or
> route replacements.
>
> Signed-off-by: Peter Nørlund <pch-chEQUL3jiZBWk0Htik3J/w@public.gmane.org>
> ---
> include/net/ip_fib.h | 6 +--
> net/ipv4/Kconfig | 1 +
> net/ipv4/fib_semantics.c | 116 ++++++++++++++++++++++++++---------------------
> 3 files changed, 68 insertions(+), 55 deletions(-)
>
> diff --git a/include/net/ip_fib.h b/include/net/ip_fib.h
> index 54271ed..4be4f25 100644
> --- a/include/net/ip_fib.h
> +++ b/include/net/ip_fib.h
> @@ -76,8 +76,8 @@ struct fib_nh {
> unsigned int nh_flags;
> unsigned char nh_scope;
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> - int nh_weight;
> - int nh_power;
> + int nh_mp_weight;
> + atomic_t nh_mp_upper_bound;
> #endif
> #ifdef CONFIG_IP_ROUTE_CLASSID
> __u32 nh_tclassid;
> @@ -115,7 +115,7 @@ struct fib_info {
> #define fib_advmss fib_metrics[RTAX_ADVMSS-1]
> int fib_nhs;
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> - int fib_power;
> + int fib_mp_weight;
> #endif
> struct rcu_head rcu;
> struct fib_nh fib_nh[0];
I could do without some of this renaming. For example you could
probably not bother with adding the _mp piece to the name. That way we
don't have to track all the nh_weight -> nh_mp_weight changes. Also
you could probably just use the name fib_weight since not including the
_mp was already the convention for the multipath portions of the
structure anyway.
This isn't really improving readability at all so I would say don't
bother renaming it.
> diff --git a/net/ipv4/Kconfig b/net/ipv4/Kconfig
> index d83071d..cb91f67 100644
> --- a/net/ipv4/Kconfig
> +++ b/net/ipv4/Kconfig
> @@ -81,6 +81,7 @@ config IP_MULTIPLE_TABLES
> config IP_ROUTE_MULTIPATH
> bool "IP: equal cost multipath"
> depends on IP_ADVANCED_ROUTER
> + select BITREVERSE
> help
> Normally, the routing tables specify a single action to be taken in
> a deterministic manner for a given packet. If you say Y here
> diff --git a/net/ipv4/fib_semantics.c b/net/ipv4/fib_semantics.c
> index 28ec3c1..8c8df80 100644
> --- a/net/ipv4/fib_semantics.c
> +++ b/net/ipv4/fib_semantics.c
> @@ -15,6 +15,7 @@
>
> #include <asm/uaccess.h>
> #include <linux/bitops.h>
> +#include <linux/bitrev.h>
> #include <linux/types.h>
> #include <linux/kernel.h>
> #include <linux/jiffies.h>
> @@ -57,7 +58,7 @@ static struct hlist_head fib_info_devhash[DEVINDEX_HASHSIZE];
>
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
>
> -static DEFINE_SPINLOCK(fib_multipath_lock);
> +static DEFINE_PER_CPU(u8, fib_mp_rr_counter);
>
> #define for_nexthops(fi) { \
> int nhsel; const struct fib_nh *nh; \
> @@ -261,7 +262,7 @@ static inline int nh_comp(const struct fib_info *fi, const struct fib_info *ofi)
> nh->nh_gw != onh->nh_gw ||
> nh->nh_scope != onh->nh_scope ||
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> - nh->nh_weight != onh->nh_weight ||
> + nh->nh_mp_weight != onh->nh_mp_weight ||
> #endif
> #ifdef CONFIG_IP_ROUTE_CLASSID
> nh->nh_tclassid != onh->nh_tclassid ||
> @@ -449,6 +450,43 @@ static int fib_count_nexthops(struct rtnexthop *rtnh, int remaining)
> return remaining > 0 ? 0 : nhs;
> }
>
This is a good example. If we don't do the rename we don't have to
review changes like the one above which just add extra overhead to the
patch.
> +static void fib_rebalance(struct fib_info *fi)
> +{
> + int factor;
> + int total;
> + int w;
> +
> + if (fi->fib_nhs < 2)
> + return;
> +
> + total = 0;
> + for_nexthops(fi) {
> + if (!(nh->nh_flags & RTNH_F_DEAD))
> + total += nh->nh_mp_weight;
> + } endfor_nexthops(fi);
> +
> + if (likely(total != 0)) {
> + factor = DIV_ROUND_UP(total, 8388608);
> + total /= factor;
> + } else {
> + factor = 1;
> + }
> +
So where does the 8388608 value come from? Is it just here to help
restrict the upper_bound to a u8 value?
> + w = 0;
> + change_nexthops(fi) {
> + int upper_bound;
> +
> + if (nexthop_nh->nh_flags & RTNH_F_DEAD) {
> + upper_bound = -1;
> + } else {
> + w += nexthop_nh->nh_mp_weight / factor;
> + upper_bound = DIV_ROUND_CLOSEST(256 * w, total);
> + }
This is doing some confusing stuff. I assume the whole point is to get
the value to convert the upper_bound into a u8 value based on the weight
where you end with 256 as the last value, do I have that right?
I'm not really sure that is the best way to go from a code standpoint
since fib_nhs is an int which technically means we could support more
than 256 next hops for a given route.
Also I assume that it is safe to divide by total because the assumption
is that if one or more routes are not dead then total has a non-zero
value, do I have that right? If so what is the point of setting the
factor to 1 above, unless you are just hacking around a divide by 0 error.
Maybe what you should do above is simply return if total == 0 since it
doesn't look like you can safely update anything otherwise.
> +
> + atomic_set(&nexthop_nh->nh_mp_upper_bound, upper_bound);
> + } endfor_nexthops(fi);
> +}
> +
> static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
> int remaining, struct fib_config *cfg)
> {
> @@ -461,7 +499,7 @@ static int fib_get_nhs(struct fib_info *fi, struct rtnexthop *rtnh,
> nexthop_nh->nh_flags =
> (cfg->fc_flags & ~0xFF) | rtnh->rtnh_flags;
> nexthop_nh->nh_oif = rtnh->rtnh_ifindex;
> - nexthop_nh->nh_weight = rtnh->rtnh_hops + 1;
> + nexthop_nh->nh_mp_weight = rtnh->rtnh_hops + 1;
>
> attrlen = rtnh_attrlen(rtnh);
> if (attrlen > 0) {
> @@ -884,7 +922,7 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
> fi->fib_net->ipv4.fib_num_tclassid_users++;
> #endif
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> - nh->nh_weight = 1;
> + nh->nh_mp_weight = 1;
> #endif
> }
>
More unnecessary rename noise.
> @@ -936,8 +974,15 @@ struct fib_info *fib_create_info(struct fib_config *cfg)
>
> change_nexthops(fi) {
> fib_info_update_nh_saddr(net, nexthop_nh);
> +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> + fi->fib_mp_weight += nexthop_nh->nh_mp_weight;
> +#endif
> } endfor_nexthops(fi)
>
> +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> + fib_rebalance(fi);
> +#endif
> +
> link_it:
> ofi = fib_find_info(fi);
> if (ofi) {
> @@ -1050,7 +1095,7 @@ int fib_dump_info(struct sk_buff *skb, u32 portid, u32 seq, int event,
> goto nla_put_failure;
>
> rtnh->rtnh_flags = nh->nh_flags & 0xFF;
> - rtnh->rtnh_hops = nh->nh_weight - 1;
> + rtnh->rtnh_hops = nh->nh_mp_weight - 1;
> rtnh->rtnh_ifindex = nh->nh_oif;
>
> if (nh->nh_gw &&
> @@ -1130,12 +1175,6 @@ int fib_sync_down_dev(struct net_device *dev, int force)
> else if (nexthop_nh->nh_dev == dev &&
> nexthop_nh->nh_scope != scope) {
> nexthop_nh->nh_flags |= RTNH_F_DEAD;
> -#ifdef CONFIG_IP_ROUTE_MULTIPATH
> - spin_lock_bh(&fib_multipath_lock);
> - fi->fib_power -= nexthop_nh->nh_power;
> - nexthop_nh->nh_power = 0;
> - spin_unlock_bh(&fib_multipath_lock);
> -#endif
> dead++;
> }
> #ifdef CONFIG_IP_ROUTE_MULTIPATH
> @@ -1149,6 +1188,10 @@ int fib_sync_down_dev(struct net_device *dev, int force)
> fi->fib_flags |= RTNH_F_DEAD;
> ret++;
> }
> +
> +#ifdef CONFIG_IP_ROUTE_MULTIPATH
> + fib_rebalance(fi);
> +#endif
> }
>
> return ret;
> @@ -1254,16 +1297,15 @@ int fib_sync_up(struct net_device *dev)
> !__in_dev_get_rtnl(dev))
> continue;
> alive++;
> - spin_lock_bh(&fib_multipath_lock);
> - nexthop_nh->nh_power = 0;
> nexthop_nh->nh_flags &= ~RTNH_F_DEAD;
> - spin_unlock_bh(&fib_multipath_lock);
> } endfor_nexthops(fi)
>
> if (alive > 0) {
> fi->fib_flags &= ~RTNH_F_DEAD;
> ret++;
> }
> +
> + fib_rebalance(fi);
> }
>
> return ret;
> @@ -1276,49 +1318,19 @@ int fib_sync_up(struct net_device *dev)
> void fib_select_multipath(struct fib_result *res)
> {
> struct fib_info *fi = res->fi;
> - int w;
> -
> - spin_lock_bh(&fib_multipath_lock);
> - if (fi->fib_power <= 0) {
> - int power = 0;
> - change_nexthops(fi) {
> - if (!(nexthop_nh->nh_flags & RTNH_F_DEAD)) {
> - power += nexthop_nh->nh_weight;
> - nexthop_nh->nh_power = nexthop_nh->nh_weight;
> - }
> - } endfor_nexthops(fi);
> - fi->fib_power = power;
> - if (power <= 0) {
> - spin_unlock_bh(&fib_multipath_lock);
> - /* Race condition: route has just become dead. */
> - res->nh_sel = 0;
> - return;
> - }
> - }
> + u8 w;
>
> + w = bitrev8(this_cpu_inc_return(fib_mp_rr_counter));
>
> - /* w should be random number [0..fi->fib_power-1],
> - * it is pretty bad approximation.
> - */
> -
> - w = jiffies % fi->fib_power;
> + for_nexthops(fi) {
> + if (w >= atomic_read(&nh->nh_mp_upper_bound))
> + continue;
>
> - change_nexthops(fi) {
> - if (!(nexthop_nh->nh_flags & RTNH_F_DEAD) &&
> - nexthop_nh->nh_power) {
> - w -= nexthop_nh->nh_power;
> - if (w <= 0) {
> - nexthop_nh->nh_power--;
> - fi->fib_power--;
> - res->nh_sel = nhsel;
> - spin_unlock_bh(&fib_multipath_lock);
> - return;
> - }
> - }
> + res->nh_sel = nhsel;
> + return;
> } endfor_nexthops(fi);
>
> - /* Race condition: route has just become dead. */
> + /* Race condition: route has just become dead */
> res->nh_sel = 0;
> - spin_unlock_bh(&fib_multipath_lock);
> }
> #endif
>
Instead of doing bitrev8 why not just hash jiffies since that is
essentially what you are doing with this counter anyway. You could
probably even look at converting from a u8 to something like a u32 which
would give you a more pseudo random distribution.
^ permalink raw reply
* Re: UAPI headers including non-UAPI headers by accident?
From: Andy Lutomirski @ 2015-06-18 18:57 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Michael Kerrisk-manpages, Andy Lutomirski,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
David Woodhouse
In-Reply-To: <9141492.JqTlmk9dHk@wuerfel>
On Thu, Jun 18, 2015 at 12:58 AM, Arnd Bergmann <arnd-r2nGTMty4D4@public.gmane.org> wrote:
> On Thursday 18 June 2015 09:52:36 Michael Kerrisk wrote:
>> [CC += David]
>>
>> On 2 June 2015 at 18:36, Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
>> > include/uapi/linux/signal.h starts with:
>> >
>> > #ifndef _UAPI_LINUX_SIGNAL_H
>> > #define _UAPI_LINUX_SIGNAL_H
>> >
>> > #include <asm/signal.h>
>> > #include <asm/siginfo.h>
>> >
>> > This causes it to include <asm/signal.h>, which is not the same thing
>> > as <uapi/asm/signal.h>. Changing that will break userspace use of
>> > this header, though, as the uapi/ won't get removed.
>> >
>> > What's the correct fix? This is causing trouble with a UML build for me.
>>
>> Perhaps David has some insight, since he architected the original UAPI split.
>
> The uapi headers are installed without the uapi prefix. This means
> that inside of the kernel, we get
>
>
> linux/signal.h
> -> uapi/linux/signal.h
> -> asm/signal.h
> -> uapi/asm/signal.h
>
> while in the installed headers we just get
>
> linux/signal.h
> -> asm/signal.h
>
> This all looks right to me: user space only sees the exported portions
> under the traditional names, while the kernel sees both the kernel-side
> and user-side definitions from the same path.
>
It seems counterintuitive and error-prone to me that including
<uapi/linux/signal.h> would pull in non-UAPI asm/signal.h declarations
in the kernel but not when used from userspace. It would make it very
easy to break the header such that it's only broken in a userspace
context.
--Andy
--
Andy Lutomirski
AMA Capital Management, LLC
^ permalink raw reply
* Re: [PATCH] selftests: add seccomp suite
From: Kees Cook @ 2015-06-18 18:00 UTC (permalink / raw)
To: Michael Ellerman
Cc: LKML, Daniel Borkmann, Shuah Khan, Andy Lutomirski, Will Drewry,
Andrew Morton, Greg KH, Mauro Carvalho Chehab, David S. Miller,
Arnd Bergmann, Joe Perches, Jingoo Han, Linux API
In-Reply-To: <1434609685.25157.5.camel-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org>
On Wed, Jun 17, 2015 at 11:41 PM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
> On Wed, 2015-06-17 at 11:12 -0700, Kees Cook wrote:
>> On Wed, Jun 17, 2015 at 12:31 AM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
>> > On Tue, 2015-06-16 at 10:54 -0700, Kees Cook wrote:
>> >> This imports the existing seccomp test suite into the kernel's selftests
>> >> tree. It contains extensive testing of seccomp features and corner cases.
>> >> There remain additional tests to move into the kernel tree, but they have
>> >> not yet been ported to all the architectures seccomp supports:
>> >> https://github.com/redpig/seccomp/tree/master/tests
>> >>
>> >> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
>> >> ---
>> >> MAINTAINERS | 1 +
>> >> tools/testing/selftests/Makefile | 1 +
>> >> tools/testing/selftests/seccomp/.gitignore | 1 +
>> >> tools/testing/selftests/seccomp/Makefile | 10 +
>> >> tools/testing/selftests/seccomp/seccomp_bpf.c | 2109 ++++++++++++++++++++++++
>> >> tools/testing/selftests/seccomp/test_harness.h | 537 ++++++
>> >
>> >
>> > Thanks very much for adding this, it would have been very helpful recently when
>> > I was trying to get seccomp filter working on powerpc :)
>> >
>> > I get one failure in TRACE_syscall.syscall_dropped:
>> >
>> > seccomp_bpf.c:1394:TRACE_syscall.syscall_dropped:Expected 1 (1) == syscall(207) (18446744073709551615)
>> >
>> >
>> > So it looks like we're returning -1 instead of 1.
>> >
>> > That's probably a bug in our handling of the return value, or maybe an
>> > inconsistency across the arches. I'll try and find time to dig into it.
>>
>> Ah-ha! Excellent. Did you add an implementation for change_syscall()
>> in seccomp_bpf.c? I don't have a powerpc method in there. I would have
>> expected both TRACE_syscall.syscall_redirected and .syscall_dropped to
>> fail without that.
>
> Yeah I did add a change_syscall() implementation, patch below.
Great!
>> If you did, maybe something isn't right with regs.SYSCALL_RET ? That's
>> where the return value being tested on a skipped syscall is stored.
>
> Yeah I saw that too, and I think you're probably right that's where the problem
> is. It doesn't seem to matter what I put in SYSCALL_RET I always get -1, so I
> think there's a bug in my kernel code.
>
> Will try and work it out tonight.
>
> cheers
>
>
> diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
> index c5abe7fd7590..1bced19c54fb 100644
> --- a/tools/testing/selftests/seccomp/seccomp_bpf.c
> +++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
> @@ -14,6 +14,7 @@
> #include <linux/filter.h>
> #include <sys/prctl.h>
> #include <sys/ptrace.h>
> +#include <sys/types.h>
> #include <sys/user.h>
> #include <linux/prctl.h>
> #include <linux/ptrace.h>
> @@ -1199,6 +1200,10 @@ TEST_F(TRACE_poke, getpid_runs_normally)
> # define ARCH_REGS struct user_pt_regs
> # define SYSCALL_NUM regs[8]
> # define SYSCALL_RET regs[0]
> +#elif defined(__powerpc__)
> +# define ARCH_REGS struct pt_regs
> +# define SYSCALL_NUM gpr[0]
> +# define SYSCALL_RET gpr[3]
> #else
> # error "Do not know how to find your architecture's registers and syscalls"
> #endif
> @@ -1246,6 +1251,10 @@ void change_syscall(struct __test_metadata *_metadata,
> EXPECT_EQ(0, ret);
> }
>
> +#elif defined(__powerpc__)
> + {
> + regs.SYSCALL_NUM = syscall;
> + }
This can be collapsed into the first #if test with the other
architectures, but otherwise looks great.
> #else
> ASSERT_EQ(1, 0) {
> TH_LOG("How is the syscall changed on this architecture?");
> @@ -1396,6 +1405,8 @@ TEST_F(TRACE_syscall, syscall_dropped)
> # define __NR_seccomp 383
> # elif defined(__aarch64__)
> # define __NR_seccomp 277
> +# elif defined(__powerpc__)
> +# define __NR_seccomp 358
> # else
> # warning "seccomp syscall number unknown for this architecture"
> # define __NR_seccomp 0xffff
>
>
>
Thanks!
-Kees
--
Kees Cook
Chrome OS Security
^ permalink raw reply
* Re: [RFC v2 23/24] m68k/mac: Fix PRAM accessors
From: Andreas Schwab @ 2015-06-18 16:52 UTC (permalink / raw)
To: Finn Thain
Cc: Geert Uytterhoeven, linux-kernel@vger.kernel.org, Linux/m68k,
linuxppc-dev@lists.ozlabs.org, linux-api@vger.kernel.org
In-Reply-To: <alpine.LNX.2.00.1506152213340.12762-i19888lE8tflDoPlx7XIcw@public.gmane.org>
Finn Thain <fthain-PnMVE5gNl/W9dxLkCovUHLpzq4S04n8Q@public.gmane.org> writes:
> When I have reliable documentation I always define macros. So I agree that
> "command" bytes like 0x34 and 0x3800 should have names but what are the
> correct names? Are we constructing an opcode containing RTC register file
> addresses or are we issuing read/write accesses to chip registers?
Any name will do as long as the magic constant is not duplicated. It is
more important to document the relation between two uses than to have a
correct name (which can trivially be updated later).
Andreas.
--
Andreas Schwab, schwab-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5
"And now for something completely different."
^ permalink raw reply
* Re: [RESEND PATCH V2 1/3] Add mmap flag to request pages are locked after page fault
From: Michal Hocko @ 2015-06-18 15:29 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, linux-alpha, linux-kernel, linux-mips,
linux-parisc, linuxppc-dev, sparclinux, linux-xtensa, linux-mm,
linux-arch, linux-api
In-Reply-To: <1433942810-7852-2-git-send-email-emunson@akamai.com>
[Sorry for the late reply - I meant to answer in the previous threads
but something always preempted me from that]
On Wed 10-06-15 09:26:48, Eric B Munson wrote:
> The cost of faulting in all memory to be locked can be very high when
> working with large mappings. If only portions of the mapping will be
> used this can incur a high penalty for locking.
>
> For the example of a large file, this is the usage pattern for a large
> statical language model (probably applies to other statical or graphical
> models as well). For the security example, any application transacting
> in data that cannot be swapped out (credit card data, medical records,
> etc).
Such a use case makes some sense to me but I am not sure the way you
implement it is the right one. This is another mlock related flag for
mmap with a different semantic. You do not want to prefault but e.g. is
the readahead or fault around acceptable? I do not see anything in your
patch to handle those...
Wouldn't it be much more reasonable and straightforward to have
MAP_FAULTPOPULATE as a counterpart for MAP_POPULATE which would
explicitly disallow any form of pre-faulting? It would be usable for
other usecases than with MAP_LOCKED combination.
> This patch introduces the ability to request that pages are not
> pre-faulted, but are placed on the unevictable LRU when they are finally
> faulted in.
>
> To keep accounting checks out of the page fault path, users are billed
> for the entire mapping lock as if MAP_LOCKED was used.
>
> Signed-off-by: Eric B Munson <emunson@akamai.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: linux-alpha@vger.kernel.org
> Cc: linux-kernel@vger.kernel.org
> Cc: linux-mips@linux-mips.org
> Cc: linux-parisc@vger.kernel.org
> Cc: linuxppc-dev@lists.ozlabs.org
> Cc: sparclinux@vger.kernel.org
> Cc: linux-xtensa@linux-xtensa.org
> Cc: linux-mm@kvack.org
> Cc: linux-arch@vger.kernel.org
> Cc: linux-api@vger.kernel.org
> ---
> arch/alpha/include/uapi/asm/mman.h | 1 +
> arch/mips/include/uapi/asm/mman.h | 1 +
> arch/parisc/include/uapi/asm/mman.h | 1 +
> arch/powerpc/include/uapi/asm/mman.h | 1 +
> arch/sparc/include/uapi/asm/mman.h | 1 +
> arch/tile/include/uapi/asm/mman.h | 1 +
> arch/xtensa/include/uapi/asm/mman.h | 1 +
> include/linux/mm.h | 1 +
> include/linux/mman.h | 3 ++-
> include/uapi/asm-generic/mman.h | 1 +
> mm/mmap.c | 4 ++--
> mm/swap.c | 3 ++-
> 12 files changed, 15 insertions(+), 4 deletions(-)
>
> diff --git a/arch/alpha/include/uapi/asm/mman.h b/arch/alpha/include/uapi/asm/mman.h
> index 0086b47..15e96e1 100644
> --- a/arch/alpha/include/uapi/asm/mman.h
> +++ b/arch/alpha/include/uapi/asm/mman.h
> @@ -30,6 +30,7 @@
> #define MAP_NONBLOCK 0x40000 /* do not block on IO */
> #define MAP_STACK 0x80000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x100000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x200000 /* Lock pages after they are faulted in, do not prefault */
>
> #define MS_ASYNC 1 /* sync memory asynchronously */
> #define MS_SYNC 2 /* synchronous memory sync */
> diff --git a/arch/mips/include/uapi/asm/mman.h b/arch/mips/include/uapi/asm/mman.h
> index cfcb876..47846a5 100644
> --- a/arch/mips/include/uapi/asm/mman.h
> +++ b/arch/mips/include/uapi/asm/mman.h
> @@ -48,6 +48,7 @@
> #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x100000 /* Lock pages after they are faulted in, do not prefault */
>
> /*
> * Flags for msync
> diff --git a/arch/parisc/include/uapi/asm/mman.h b/arch/parisc/include/uapi/asm/mman.h
> index 294d251..1514cd7 100644
> --- a/arch/parisc/include/uapi/asm/mman.h
> +++ b/arch/parisc/include/uapi/asm/mman.h
> @@ -24,6 +24,7 @@
> #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x100000 /* Lock pages after they are faulted in, do not prefault */
>
> #define MS_SYNC 1 /* synchronous memory sync */
> #define MS_ASYNC 2 /* sync memory asynchronously */
> diff --git a/arch/powerpc/include/uapi/asm/mman.h b/arch/powerpc/include/uapi/asm/mman.h
> index 6ea26df..fce74fe 100644
> --- a/arch/powerpc/include/uapi/asm/mman.h
> +++ b/arch/powerpc/include/uapi/asm/mman.h
> @@ -27,5 +27,6 @@
> #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x80000 /* Lock pages after they are faulted in, do not prefault */
>
> #endif /* _UAPI_ASM_POWERPC_MMAN_H */
> diff --git a/arch/sparc/include/uapi/asm/mman.h b/arch/sparc/include/uapi/asm/mman.h
> index 0b14df3..12425d8 100644
> --- a/arch/sparc/include/uapi/asm/mman.h
> +++ b/arch/sparc/include/uapi/asm/mman.h
> @@ -22,6 +22,7 @@
> #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x80000 /* Lock pages after they are faulted in, do not prefault */
>
>
> #endif /* _UAPI__SPARC_MMAN_H__ */
> diff --git a/arch/tile/include/uapi/asm/mman.h b/arch/tile/include/uapi/asm/mman.h
> index 81b8fc3..ec04eaf 100644
> --- a/arch/tile/include/uapi/asm/mman.h
> +++ b/arch/tile/include/uapi/asm/mman.h
> @@ -29,6 +29,7 @@
> #define MAP_DENYWRITE 0x0800 /* ETXTBSY */
> #define MAP_EXECUTABLE 0x1000 /* mark it as an executable */
> #define MAP_HUGETLB 0x4000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x8000 /* Lock pages after they are faulted in, do not prefault */
>
>
> /*
> diff --git a/arch/xtensa/include/uapi/asm/mman.h b/arch/xtensa/include/uapi/asm/mman.h
> index 201aec0..42d43cc 100644
> --- a/arch/xtensa/include/uapi/asm/mman.h
> +++ b/arch/xtensa/include/uapi/asm/mman.h
> @@ -55,6 +55,7 @@
> #define MAP_NONBLOCK 0x20000 /* do not block on IO */
> #define MAP_STACK 0x40000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x80000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x100000 /* Lock pages after they are faulted in, do not prefault */
> #ifdef CONFIG_MMAP_ALLOW_UNINITIALIZED
> # define MAP_UNINITIALIZED 0x4000000 /* For anonymous mmap, memory could be
> * uninitialized */
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 0755b9f..3e31457 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -126,6 +126,7 @@ extern unsigned int kobjsize(const void *objp);
> #define VM_PFNMAP 0x00000400 /* Page-ranges managed without "struct page", just pure PFN */
> #define VM_DENYWRITE 0x00000800 /* ETXTBSY on write attempts.. */
>
> +#define VM_LOCKONFAULT 0x00001000 /* Lock the pages covered when they are faulted in */
> #define VM_LOCKED 0x00002000
> #define VM_IO 0x00004000 /* Memory mapped I/O or similar */
>
> diff --git a/include/linux/mman.h b/include/linux/mman.h
> index 16373c8..437264b 100644
> --- a/include/linux/mman.h
> +++ b/include/linux/mman.h
> @@ -86,7 +86,8 @@ calc_vm_flag_bits(unsigned long flags)
> {
> return _calc_vm_trans(flags, MAP_GROWSDOWN, VM_GROWSDOWN ) |
> _calc_vm_trans(flags, MAP_DENYWRITE, VM_DENYWRITE ) |
> - _calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED );
> + _calc_vm_trans(flags, MAP_LOCKED, VM_LOCKED ) |
> + _calc_vm_trans(flags, MAP_LOCKONFAULT,VM_LOCKONFAULT);
> }
>
> unsigned long vm_commit_limit(void);
> diff --git a/include/uapi/asm-generic/mman.h b/include/uapi/asm-generic/mman.h
> index e9fe6fd..fc4e586 100644
> --- a/include/uapi/asm-generic/mman.h
> +++ b/include/uapi/asm-generic/mman.h
> @@ -12,6 +12,7 @@
> #define MAP_NONBLOCK 0x10000 /* do not block on IO */
> #define MAP_STACK 0x20000 /* give out an address that is best suited for process/thread stacks */
> #define MAP_HUGETLB 0x40000 /* create a huge page mapping */
> +#define MAP_LOCKONFAULT 0x80000 /* Lock pages after they are faulted in, do not prefault */
>
> /* Bits [26:31] are reserved, see mman-common.h for MAP_HUGETLB usage */
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index bb50cac..ba1a6bf 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1233,7 +1233,7 @@ static inline int mlock_future_check(struct mm_struct *mm,
> unsigned long locked, lock_limit;
>
> /* mlock MCL_FUTURE? */
> - if (flags & VM_LOCKED) {
> + if (flags & (VM_LOCKED | VM_LOCKONFAULT)) {
> locked = len >> PAGE_SHIFT;
> locked += mm->locked_vm;
> lock_limit = rlimit(RLIMIT_MEMLOCK);
> @@ -1301,7 +1301,7 @@ unsigned long do_mmap_pgoff(struct file *file, unsigned long addr,
> vm_flags = calc_vm_prot_bits(prot) | calc_vm_flag_bits(flags) |
> mm->def_flags | VM_MAYREAD | VM_MAYWRITE | VM_MAYEXEC;
>
> - if (flags & MAP_LOCKED)
> + if (flags & (MAP_LOCKED | MAP_LOCKONFAULT))
> if (!can_do_mlock())
> return -EPERM;
>
> diff --git a/mm/swap.c b/mm/swap.c
> index a7251a8..07c905e 100644
> --- a/mm/swap.c
> +++ b/mm/swap.c
> @@ -711,7 +711,8 @@ void lru_cache_add_active_or_unevictable(struct page *page,
> {
> VM_BUG_ON_PAGE(PageLRU(page), page);
>
> - if (likely((vma->vm_flags & (VM_LOCKED | VM_SPECIAL)) != VM_LOCKED)) {
> + if (likely((vma->vm_flags & (VM_LOCKED | VM_LOCKONFAULT)) == 0) ||
> + (vma->vm_flags & VM_SPECIAL)) {
> SetPageActive(page);
> lru_cache_add(page);
> return;
> --
> 1.9.1
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply
* Re: [RFC v3 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-06-18 14:50 UTC (permalink / raw)
To: Kinglong Mee
Cc: linux-kernel, linux-fsdevel, linux-api, greg, jack, tytso,
adilger.kernel, hughd, lczerner, hch, linux-ext4, linux-mm,
kyungmin.park, kmpark
In-Reply-To: <5582A8C1.3000002@gmail.com>
Hi,
On 06/18/2015 01:17 PM, Kinglong Mee wrote:
> On 6/16/2015 9:09 PM, Beata Michalska wrote:
>> Introduce configurable generic interface for file
>> system-wide event notifications, to provide file
>> systems with a common way of reporting any potential
>> issues as they emerge.
> ... snip ...
>> +
>> +Sample request could look like the following:
>> +
>> + echo /sample/mount/point G T 710000 500000 > /sys/fs/events/config
>> +
>> +Multiple request might be specified provided they are separated with semicolon.
>> +
>> +The configuration itself might be modified at any time. One can add/remove
>> +particular event types for given fielsystem, modify the threshold levels,
>> +and remove single or all entries from the 'config' file.
>> +
>> + - Adding new event type:
>> +
>> + $ echo MOUNT EVENT_TYPE > /sys/fs/events/config
>> +
>> +(Note that is is enough to provide the event type to be enabled without
>
> Should be "Note that it is ... " here ?
Right
>
>> +the already set ones.)
>> +
>> + - Removing event type:
>> +
>> + $ echo '!MOUNT EVENT_TYPE' > /sys/fs/events/config
>> +
>> + - Updating threshold limits:
>> +
>> + $ echo MOUNT T L1 L2 > /sys/fs/events/config
>> +
>> + - Removing single entry:
>> +
>> + $ echo '!MOUNT' > /sys/fs/events/config
>> +
>> + - Removing all entries:
>> +
>> + $ echo > /sys/fs/events/config
>> +
>> +Reading the file will list all registered entries with their current set-up
>> +along with some additional info like the filesystem type and the backing device
>> +name if available.
>> +
>> +Final, though a very important note on the configuration: when and if the
>> +actual events are being triggered falls way beyond the scope of the generic
>> +filesystem events interface. It is up to a particular filesystem
>> +implementation which events are to be supported - if any at all. So if
>> +given filesystem does not support the event notifications, an attempt to
>> +enable those through 'config' file will fail.
>> +
>> +
>> +3. The generic netlink interface support:
>> +=========================================
>> +
>> +Whenever an event notification is triggered (by given filesystem) the current
>> +configuration is being validated to decide whether a userpsace notification
>> +should be launched. If there has been no request (in a mean of 'config' file
>> +entry) for given event, one will be silently disregarded. If, on the other
>> +hand, someone is 'watching' given filesystem for specific events, a generic
>> +netlink message will be sent. A dedicated multicast group has been provided
>> +solely for this purpose so in order to receive such notifications, one should
>> +subscribe to this new multicast group. As for now only the init network
>> +namespace is being supported.
>> +
>> +3.1 Message format
>> +
>> +The FS_NL_C_EVENT shall be stored within the generic netlink message header
>> +as the command field. The message payload will provide more detailed info:
>> +the backing device major and minor numbers, the event code and the id of
>> +the process which action led to the event occurrence. In case of threshold
>> +notifications, the current number of available blocks will be included
>> +in the payload as well.
>> +
>> +
>> + 0 1 2 3
>> + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
>> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>> + | NETLINK MESSAGE HEADER |
>> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>> + | GENERIC NETLINK MESSAGE HEADER |
>> + | (with FS_NL_C_EVENT as genlmsghdr cdm field) |
>
> cmd, not cdm.
ditto
>
>> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>> + | Optional user specific message header |
>> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>> + | GENERIC MESSAGE PAYLOAD: |
>> + +---------------------------------------------------------------+
>> + | FS_NL_A_EVENT_ID (NLA_U32) |
>> + +---------------------------------------------------------------+
>> + | FS_NL_A_DEV_MAJOR (NLA_U32) |
>> + +---------------------------------------------------------------+
>> + | FS_NL_A_DEV_MINOR (NLA_U32) |
>> + +---------------------------------------------------------------+
>> + | FS_NL_A_CAUSED_ID (NLA_U32) |
>
> Should be NLA_U64 ? The following uses as,
>
> + if (nla_put_u64(skb, FS_NL_A_CAUSED_ID, pid_vnr(task_pid(current))))
> + return -EINVAL;
>
Yes, or nla_put_u32 - either way my bad
> Also, I'd like FS_NL_A_CAUSED_PID than FS_NL_A_CAUSED_ID.
Alright
>
>> + +---------------------------------------------------------------+
>> + | FS_NL_A_DATA (NLA_U64) |
>> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
>> +
>> +
>> +The above figure is based on:
>> + http://www.linuxfoundation.org/collaborate/workgroups/networking/generic_netlink_howto#Message_Format
>> +
>> +
> ... snip...
>> + seq_putc(m, ' ');
>> + if (sb->s_op->show_devname) {
>> + sb->s_op->show_devname(m, en->mnt_path.mnt->mnt_root);
>> + } else {
>> + seq_escape(m, r_mnt->mnt_devname ? r_mnt->mnt_devname : "none",
>> + " \t\n\\");
>> + }
>> + seq_puts(m, " (");
>> +
>> + nmask = en->notify;
>> + for (match = fs_etypes; match->pattern; ++match) {
>> + if (match->token & nmask) {
>> + seq_puts(m, match->pattern);
>
> Print here is better.
>
> if (match->pattern & FS_EVENT_THRESH)
> seq_printf(m, " %llu %llu", en->th.lrange, en->th.urange);
>
>> + nmask &= ~match->token;
>> + if (nmask)
>> + seq_putc(m, ',');
>> + }
>> + }
>> + seq_printf(m, " %llu %llu", en->th.lrange, en->th.urange);
>
> Don't print the lrange/urange (always be zero) when without FS_EVENT_THRESH.
>
ditto
>> + seq_puts(m, ")\n");
>> + return 0;
>> +}
>> +
>> +static const struct seq_operations fs_trace_seq_ops = {
>> + .start = fs_trace_seq_start,
>> + .next = fs_trace_seq_next,
>> + .stop = fs_trace_seq_stop,
>> + .show = fs_trace_seq_show,
>> +};
>> +
>> +static int fs_trace_open(struct inode *inode, struct file *file)
>> +{
>> + return seq_open(file, &fs_trace_seq_ops);
>> +}
>> +
>> +static const struct file_operations fs_trace_fops = {
>> + .owner = THIS_MODULE,
>> + .open = fs_trace_open,
>> + .write = fs_trace_write,
>> + .read = seq_read,
>> + .llseek = seq_lseek,
>> + .release = seq_release,
>> +};
>> +
>> +static int fs_trace_init(void)
>> +{
>> + fs_trace_cachep = KMEM_CACHE(fs_trace_entry, 0);
>> + if (!fs_trace_cachep)
>> + return -EINVAL;
>> + init_waitqueue_head(&trace_wq);
>> + return 0;
>> +}
>> +
>> +/* VFS support */
>> +static int fs_trace_fill_super(struct super_block *sb, void *data, int silen)
>> +{
>> + int ret;
>> + static struct tree_descr desc[] = {
>> + [2] = {
>> + .name = "config",
>> + .ops = &fs_trace_fops,
>> + .mode = S_IWUSR | S_IRUGO,
>> + },
>> + {""},
>> + };
>> +
>> + ret = simple_fill_super(sb, 0x7246332, desc);
>> + return !ret ? fs_trace_init() : ret;
>> +}
>> +
>> +static struct dentry *fs_trace_do_mount(struct file_system_type *fs_type,
>> + int ntype, const char *dev_name, void *data)
>> +{
>> + return mount_single(fs_type, ntype, data, fs_trace_fill_super);
>> +}
>> +
>> +static void fs_trace_kill_super(struct super_block *sb)
>> +{
>> + /*
>> + * The rcu_barrier here will/should make sure all call_rcu
>> + * callbacks are completed - still there might be some active
>> + * trace objects in use which can make calling the
>> + * kmem_cache_destroy unsafe. So we wait until all traces
>> + * are finally released.
>> + */
>> + fs_remove_all_traces();
>> + rcu_barrier();
>> + wait_event(trace_wq, !atomic_read(&stray_traces));
>> +
>> + kmem_cache_destroy(fs_trace_cachep);
>> + kill_litter_super(sb);
>> +}
>> +
>> +static struct kset *fs_trace_kset;
>> +
>> +static struct file_system_type fs_trace_fstype = {
>> + .name = "fstrace",
>> + .mount = fs_trace_do_mount,
>> + .kill_sb = fs_trace_kill_super,
>> +};
>> +
>> +static void __init fs_trace_vfs_init(void)
>> +{
>> + fs_trace_kset = kset_create_and_add("events", NULL, fs_kobj);
>> +
>> + if (!fs_trace_kset)
>> + return;
>> +
>> + if (!register_filesystem(&fs_trace_fstype)) {
>> + if (!fs_event_netlink_register())
>> + return;
>> + unregister_filesystem(&fs_trace_fstype);
>> + }
>> + kset_unregister(fs_trace_kset);
>> +}
>> +
>> +static int __init fs_trace_evens_init(void)
>> +{
>> + fs_trace_vfs_init();
>> + return 0;
>> +};
>> +module_init(fs_trace_evens_init);
>> +
>> diff --git a/fs/events/fs_event.h b/fs/events/fs_event.h
>> new file mode 100644
>> index 0000000..23f24c8
>> --- /dev/null
>> +++ b/fs/events/fs_event.h
>> @@ -0,0 +1,22 @@
>> +/*
>> + * Copyright(c) 2015 Samsung Electronics. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2.
>> + *
>> + * The full GNU General Public License is included in this distribution in the
>> + * file called COPYING.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + */
>> +
>> +#ifndef __GENERIC_FS_EVENTS_H
>> +#define __GENERIC_FS_EVENTS_H
>> +
>> +int fs_event_netlink_register(void);
>> +void fs_event_netlink_unregister(void);
>> +
>> +#endif /* __GENERIC_FS_EVENTS_H */
>> diff --git a/fs/events/fs_event_netlink.c b/fs/events/fs_event_netlink.c
>> new file mode 100644
>> index 0000000..0c97eb7
>> --- /dev/null
>> +++ b/fs/events/fs_event_netlink.c
>> @@ -0,0 +1,104 @@
>> +/*
>> + * Copyright(c) 2015 Samsung Electronics. All rights reserved.
>> + *
>> + * This program is free software; you can redistribute it and/or modify it
>> + * under the terms of the GNU General Public License version 2.
>> + *
>> + * The full GNU General Public License is included in this distribution in the
>> + * file called COPYING.
>> + *
>> + * This program is distributed in the hope that it will be useful, but WITHOUT
>> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
>> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
>> + * more details.
>> + */
>> +#include <linux/fs.h>
>> +#include <linux/init.h>
>> +#include <linux/kernel.h>
>> +#include <linux/sched.h>
>> +#include <linux/slab.h>
>> +#include <net/netlink.h>
>> +#include <net/genetlink.h>
>> +#include "fs_event.h"
>> +
>> +static const struct genl_multicast_group fs_event_mcgroups[] = {
>> + { .name = FS_EVENTS_MCAST_GRP_NAME, },
>> +};
>> +
>> +static struct genl_family fs_event_family = {
>> + .id = GENL_ID_GENERATE,
>> + .name = FS_EVENTS_FAMILY_NAME,
>> + .version = 1,
>> + .maxattr = FS_NL_A_MAX,
>> + .mcgrps = fs_event_mcgroups,
>> + .n_mcgrps = ARRAY_SIZE(fs_event_mcgroups),
>> +};
>> +
>> +int fs_netlink_send_event(size_t size, unsigned int event_id,
>> + int (*compose_msg)(struct sk_buff *skb, void *data),
>> + void *cbdata)
>> +{
>> + static atomic_t seq;
>> + struct sk_buff *skb;
>> + void *msg_head;
>> + int ret = 0;
>> +
>> + if (!size || !compose_msg)
>> + return -EINVAL;
>> +
>> + /* Skip if there are no listeners */
>> + if (!genl_has_listeners(&fs_event_family, &init_net, 0))
>> + return 0;
>> +
>> + if (event_id != FS_EVENT_NONE)
>> + size += nla_total_size(sizeof(u32));
>> + size += nla_total_size(sizeof(u64));
>
> What is this for ?
>
This should actually get removed :)
> thanks
> Kinglong Mee
>
Thank You,
Best Regards
Beata
--
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: [PATCH v5 08/11] nvmem: qfprom: Add Qualcomm QFPROM support.
From: Srinivas Kandagatla @ 2015-06-18 13:22 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, Greg Kroah-Hartman,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w
In-Reply-To: <5580AA9B.7040001-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 17/06/15 00:00, Stephen Boyd wrote:
> On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
>> index f157b6d..e665e23 100644
>> --- a/drivers/nvmem/Kconfig
>> +++ b/drivers/nvmem/Kconfig
>> @@ -8,3 +8,18 @@ menuconfig NVMEM
>> from both the Linux Kernel and the userspace.
>>
>> If unsure, say no.
>> +
>> +if NVMEM
>> +
>> +config QCOM_QFPROM
>> + tristate "QCOM QFPROM Support"
>> + depends on ARCH_QCOM
>
> || COMPILE_TEST?
>
Yes, makes sense. I will add it.
>> + select REGMAP_MMIO
>> + help
>> + Say y here to enable QFPROM support. The QFPROM provides access
>> + functions for QFPROM data to rest of the drivers via nvmem interface.
>> +
>> + This driver can also be built as a module. If so, the module
>> + will be called nvmem-qfprom.
>> +
>> +endif
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index f694cfc..caea611 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -5,3 +5,7 @@
>> obj-$(CONFIG_NVMEM) += nvmem_core.o
>> nvmem_core-y := core.o
>> nvmem_core-y += nvmem-mmio.o
>> +
>> +# Devices
>> +obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
>> +nvmem_qfprom-y := qfprom.o
>
> Why not just
>
> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
>
> ?
I will recheck on this.
>
>> diff --git a/drivers/nvmem/qfprom.c b/drivers/nvmem/qfprom.c
>> new file mode 100644
>> index 0000000..5ea84bb
>> --- /dev/null
>> +++ b/drivers/nvmem/qfprom.c
>> @@ -0,0 +1,51 @@
>> +/*
>> + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include "nvmem-mmio.h"
>> +
>> +static struct regmap_config qfprom_regmap_config = {
>
> const?
Will fix all the comments.
>
>> + .reg_bits = 32,
>> + .val_bits = 8,
>> + .reg_stride = 1,
>> +};
>> +
>> +static struct nvmem_config econfig = {
>
> const?
>
>> + .name = "qfprom",
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static struct nvmem_mmio_data qfprom_data = {
>
> const?
>
>> + .nvmem_config = &econfig,
>> + .regmap_config = &qfprom_regmap_config,
>> +};
>>
>
^ permalink raw reply
* Re: [PATCH v5 10/11] nvmem: sunxi: Move the SID driver to the nvmem framework
From: Srinivas Kandagatla @ 2015-06-18 13:09 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown, s.hauer,
Greg Kroah-Hartman, linux-api, linux-kernel, devicetree,
linux-arm-msm, arnd, pantelis.antoniou, mporter
In-Reply-To: <5580AB72.6010909@codeaurora.org>
On 17/06/15 00:04, Stephen Boyd wrote:
> On 05/21/2015 09:45 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
>> index caea611..cc46791 100644
>> --- a/drivers/nvmem/Makefile
>> +++ b/drivers/nvmem/Makefile
>> @@ -9,3 +9,5 @@ nvmem_core-y += nvmem-mmio.o
>> # Devices
>> obj-$(CONFIG_QCOM_QFPROM) += nvmem_qfprom.o
>> nvmem_qfprom-y := qfprom.o
>> +obj-$(CONFIG_NVMEM_SUNXI_SID) += nvmem-sunxi-sid.o
>> +nvmem-sunxi-sid-y := sunxi-sid.o
>
> Oh I see, so the module has nvmem- in the name. Isn't there some way to
> add a rule to do that for all provider drivers?
>
I will give it a try, and also fix the other comments on this patch.
>> diff --git a/drivers/nvmem/sunxi-sid.c b/drivers/nvmem/sunxi-sid.c
>> new file mode 100644
>> index 0000000..5bfce35
>> --- /dev/null
>> +++ b/drivers/nvmem/sunxi-sid.c
>> @@ -0,0 +1,64 @@
>> +/*
>> + * Allwinner sunXi SoCs Security ID support.
>> + *
>> + * Copyright (c) 2013 Oliver Schinagl <oliver@schinagl.nl>
>> + * Copyright (C) 2014 Maxime Ripard <maxime.ripard@free-electrons.com>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License as published by
>> + * the Free Software Foundation; either version 2 of the License, or
>> + * (at your option) any later version.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + *
>> + */
>> +
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include "nvmem-mmio.h"
>> +
>> +static bool sunxi_sid_writeable_reg(struct device *dev, unsigned int reg)
>> +{
>> + return false;
>> +}
>> +
>> +static struct nvmem_config econfig = {
>
> const?
>
>> + .name = "sunix-sid",
>> + .owner = THIS_MODULE,
>> +};
>> +
>> +static struct regmap_config sunxi_sid_regmap_config = {
>
> const?
>
>> + .reg_bits = 32,
>> + .val_bits = 32,
>> + .reg_stride = 4,
>> + .writeable_reg = sunxi_sid_writeable_reg,
>> +};
>> +
>> +static struct nvmem_mmio_data sunxi_data = {
>
> const?
>
>> + .nvmem_config = &econfig,
>> + .regmap_config = &sunxi_sid_regmap_config,
>> +};
>> +
>
>
^ permalink raw reply
* Re: [PATCH v5 07/11] nvmem: Add simple nvmem-mmio consumer helper functions.
From: Srinivas Kandagatla @ 2015-06-18 13:08 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, Greg Kroah-Hartman,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w
In-Reply-To: <5580AA05.90709-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 16/06/15 23:58, Stephen Boyd wrote:
> On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/nvmem-mmio.c b/drivers/nvmem/nvmem-mmio.c
>> new file mode 100644
>> index 0000000..0d8131f
>> --- /dev/null
>> +++ b/drivers/nvmem/nvmem-mmio.c
>> @@ -0,0 +1,69 @@
>> +/*
>> + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/module.h>
>> +#include <linux/err.h>
>> +#include <linux/of.h>
>> +#include <linux/of_device.h>
>
> #include <linux/platform_device.h>
> #include <linux/nvmem_provider.h>
> #include <linux/regmap.h>
>
These are included in nvmem-mmio.h, however for clarity sake I could add
them here too.
>> +#include "nvmem-mmio.h"
>> +
>> +int nvmem_mmio_remove(struct platform_device *pdev)
>> +{
>> + struct nvmem_device *nvmem = platform_get_drvdata(pdev);
>> +
>> + return nvmem_unregister(nvmem);
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_mmio_remove);
>> +
>> +int nvmem_mmio_probe(struct platform_device *pdev)
>> +{
>> +
> [...]
>> +
>> + platform_set_drvdata(pdev, nvmem);
>
> It may be better to return the nvmem device instead so that the one
> drvdata member is usable by the calling driver.
Makes sense.. I will give it a try.
>
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_mmio_probe);
>
> Kernel-doc on these exported functions?
Yep, thanks for spotting, I will fix it.
>
>> diff --git a/drivers/nvmem/nvmem-mmio.h b/drivers/nvmem/nvmem-mmio.h
>> new file mode 100644
>> index 0000000..a2ad4e5
>> --- /dev/null
>> +++ b/drivers/nvmem/nvmem-mmio.h
>> @@ -0,0 +1,41 @@
>> +/*
>> + * MMIO based nvmem providers.
>> + *
>> + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef _LINUX_NVMEM_MMIO_H
>> +#define _LINUX_NVMEM_MMIO_H
>> +
>> +#include <linux/platform_device.h>
>
> Forward declare struct platform_device instead.
>
>> +#include <linux/nvmem-provider.h>
>
> Forward declare nvmem_config instead.
>
>> +#include <linux/regmap.h>
>
> Forward declare regmap_config instead.
>
>> +
>> +struct nvmem_mmio_data {
>> + struct regmap_config *regmap_config;
>> + struct nvmem_config *nvmem_config;
>> +};
>> +
>> +#if IS_ENABLED(CONFIG_NVMEM)
>> +
>> +int nvmem_mmio_probe(struct platform_device *pdev);
>> +int nvmem_mmio_remove(struct platform_device *pdev);
>
^ permalink raw reply
* Re: [PATCH v5 06/11] nvmem: Add bindings for simple nvmem framework
From: Srinivas Kandagatla @ 2015-06-18 13:01 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, Greg Kroah-Hartman,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w
In-Reply-To: <5580A900.9070902-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 16/06/15 23:53, Stephen Boyd wrote:
> On 05/21/2015 09:44 AM, Srinivas Kandagatla wrote:
>> diff --git a/Documentation/devicetree/bindings/nvmem/nvmem.txt b/Documentation/devicetree/bindings/nvmem/nvmem.txt
>> new file mode 100644
>> index 0000000..ecea654
>> --- /dev/null
>> +++ b/Documentation/devicetree/bindings/nvmem/nvmem.txt
>> @@ -0,0 +1,84 @@
>> += NVMEM Data Device Tree Bindings =
>> +
>> +This binding is intended to represent the location of hardware
>> +configuration data stored in NVMEMs.
>
> It would be worthwhile spelling out what NVMEM stands for.
>
>> +
>> +On a significant proportion of boards, the manufacturer has stored
>> +some data on NVMEM, for the OS to be able to retrieve these information
>> +and act upon it. Obviously, the OS has to know about where to retrieve
>> +these data from, and where they are stored on the storage device.
>> +
>> +This document is here to document this.
>> +
>> += Data providers =
>> +Contains bindings specific to provider drivers and data cells as children
>> +to this node.
>
> children of this node?
>
Yep, will fix the text
>> +
>> +Optional properties:
>> + read-only: Mark the provider as read only.
>> +
>> += Data cells =
>> +These are the child nodes of the provider which contain data cell
>> +information like offset and size in nvmem provider.
>> +
>> +Required properties:
>> +reg: specifies the offset in byte within that storage device, start bit
>> + in the byte and the length in bits of the data we care about.
>> + There could be more then one offset-length pairs in this property.
>
> s/then/than/
Yep.
>
>> +
>> +Optional properties:
>> +
>> +bit-offset: specifies the offset in bit within the address range specified
>> + by reg property. Can take values from 0-7.
>> +nbits: specifies number of bits this cell occupies starting from bit-offset.
>> +
>
> Hopefully the consumer knows the endianness of the data stored.
As we read byte-byte, does it matter, as long as consumer gets them in
the same order as its stored.
>
>> +For example:
>> +
>> + /* Provider */
>> + qfprom: qfprom@00700000 {
>> + ...
>> +
>> + /* Data cells */
>> + tsens_calibration: calib@404 {
>> + reg = <0x404 0x10>;
>> + };
>> +
>> + tsens_calibration_bckp: calib_bckp@504 {
>> + reg = <0x504 0x11>;
>> + bit-offset = 6;
>> + nbits = 128;
>> + };
>> +
>> + pvs_version: pvs-version@6 {
>> + reg = <0x6 0x2>
>> + bit-offset = 7;
>> + nbits = 2;
>> + };
>> +
>> + speed_bin: speed-bin@c{
>> + reg = <0xc 0x1>;
>> + bit-offset = 2;
>> + nbits = 3;
>> +
>> + };
>> + ...
>> + };
>> +
>> += Data consumers =
>> +Are device nodes which consume nvmem data cells/providers.
>> +
>> +Required-properties:
>> +nvmem-cell: list of phandle to the nvmem data cells.
>> +nvmem-cell-names: names for the each nvmem-cell specified
>> +
>> +Optional-properties:
>> +nvmem : list of phandles to nvmem providers.
>> +nvmem-names: names for the each nvmem provider.
>
> Is nvmem-names required if nvmem is used?
Yes, will fix it.
>
^ permalink raw reply
* Re: [PATCH v5 05/11] nvmem: Add nvmem_device based consumer apis.
From: Srinivas Kandagatla @ 2015-06-18 12:57 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, Greg Kroah-Hartman,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w
In-Reply-To: <5580A7EA.2090909-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
On 16/06/15 23:49, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> index 8a4b358..68ee8d1 100644
>> --- a/drivers/nvmem/core.c
>> +++ b/drivers/nvmem/core.c
>> @@ -436,6 +436,51 @@ static int __nvmem_device_put(struct nvmem_device *nvmem)
>> return 0;
>> }
>>
>> +static int nvmem_match(struct device *dev, const void *data)
>> +{
>> + return !strcmp(dev_name(dev), (const char *)data);
>
> Unnecessary cast.
Sure, will fix it.
>
>> +}
>> +
>> +static struct nvmem_device *nvmem_find(const char *name)
>> +{
>> + struct device *d;
>> +
>> + d = class_find_device(&nvmem_class, NULL, (void *)name, nvmem_match);
>
> Unnecessary cast
Will fix it.
>
>> +
>> + return d ? to_nvmem_device(d) : NULL;
>> +}
>
> [...]
>> +
>> +void nvmem_device_put(struct nvmem_device *nvmem)
>> +{
>> + __nvmem_device_put(nvmem);
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_put);
>>
>> +int nvmem_device_cell_read(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_cell_read);
>> +
>> +int nvmem_device_cell_write(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_cell_write);
>> +
>> +int nvmem_device_read(struct nvmem_device *nvmem,
>> + unsigned int offset,
>> + size_t bytes, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_read);
>> +
>> +int nvmem_device_write(struct nvmem_device *nvmem,
>> + unsigned int offset,
>> + size_t bytes, void *buf)
>> +{
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_device_write);
>
> Can you please add kernel-doc on these exported APIs?
>
Thanks for spotting, I will add them.
>> +
>> static int nvmem_init(void)
>> {
>> return class_register(&nvmem_class);
>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>> index c3fa8c7..66c67ba 100644
>> --- a/include/linux/nvmem-consumer.h
>> +++ b/include/linux/nvmem-consumer.h
>> @@ -23,6 +32,18 @@ void nvmem_cell_put(struct nvmem_cell *cell);
>> void *nvmem_cell_read(struct nvmem_cell *cell, ssize_t *len);
>> int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len);
>>
>> +/* direct nvmem device read/write interface */
>> +struct nvmem_device *nvmem_device_get(struct device *dev, const char *name);
>> +void nvmem_device_put(struct nvmem_device *nvmem);
>> +int nvmem_device_read(struct nvmem_device *nvmem, unsigned int offset,
>> + size_t bytes, void *buf);
>> +int nvmem_device_write(struct nvmem_device *nvmem, unsigned int offset,
>> + size_t bytes, void *buf);
>> +int nvmem_device_cell_read(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf);
>> +int nvmem_device_cell_write(struct nvmem_device *nvmem,
>> + struct nvmem_cell_info *info, void *buf);
>> +
>
> Can we also have devm_nvmem_*_get() APIs please?
Sure, I will spin it in next version.
>
>> diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
>> index 4908b37..7a982cd 100644
>> --- a/include/linux/nvmem-provider.h
>> +++ b/include/linux/nvmem-provider.h
>> @@ -12,15 +12,9 @@
>> #ifndef _LINUX_NVMEM_PROVIDER_H
>> #define _LINUX_NVMEM_PROVIDER_H
>>
>> -struct nvmem_device;
>> +#include <linux/nvmem-consumer.h>
>>
>> -struct nvmem_cell_info {
>> - const char *name;
>> - int offset;
>> - int bytes;
>> - int bit_offset;
>> - int nbits;
>> -};
>> +struct nvmem_device;
>
> Should this diff be part of an earlier patch?
Possibly, something wrong with this diff, I will fix it.
>
^ permalink raw reply
* Re: [PATCH v5 04/11] nvmem: Add a simple NVMEM framework for consumers
From: Srinivas Kandagatla @ 2015-06-18 12:56 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel
Cc: devicetree, arnd, Greg Kroah-Hartman, s.hauer, linux-kernel,
pantelis.antoniou, Rob Herring, Mark Brown, Kumar Gala, mporter,
Maxime Ripard, linux-api, linux-arm-msm
In-Reply-To: <5580A345.30204@codeaurora.org>
On 16/06/15 23:29, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>> @@ -379,6 +380,351 @@ int nvmem_unregister(struct nvmem_device *nvmem)
> [...]
>> +
>> + return nvmem;
>> +}
>> +
>> +static int __nvmem_device_put(struct nvmem_device *nvmem)
>
> Why does this return int? It's not used anywhere.
>
I can remove it.
>> +{
>> + module_put(nvmem->owner);
>> + mutex_lock(&nvmem_mutex);
>> + nvmem->users--;
>> + mutex_unlock(&nvmem_mutex);
>> +
>> + return 0;
>> +}
>> +
>> +static struct nvmem_cell *nvmem_cell_get_from_list(const char *cell_id)
>> +{
>> + struct nvmem_cell *cell = NULL;
>> + struct nvmem_device *nvmem;
>> +
>> + nvmem = __nvmem_device_get(NULL, &cell, cell_id);
>> + if (IS_ERR(nvmem))
>> + return (struct nvmem_cell *)nvmem;
>
> ERR_CAST?
Will fix it.
>
>> +
>> +
>> + return cell;
>> +
>> +}
>> +
>> +static struct nvmem_cell *of_nvmem_cell_get(struct device_node *np,
>> + const char *name)
>> +{
>> + struct device_node *cell_np, *nvmem_np;
>> + struct nvmem_cell *cell;
>> + struct nvmem_device *nvmem;
>> + const __be32 *addr;
>> + int rval, len, index;
>> +
>> + index = of_property_match_string(np, "nvmem-cell-names", name);
>> +
>> + cell_np = of_parse_phandle(np, "nvmem-cell", index);
>> + if (!cell_np)
>> + return ERR_PTR(-EINVAL);
>> +
>> + nvmem_np = of_get_next_parent(cell_np);
>> + if (!nvmem_np)
>> + return ERR_PTR(-EINVAL);
>> +
>> + nvmem = __nvmem_device_get(nvmem_np, NULL, NULL);
>> + if (IS_ERR(nvmem))
>> + return (struct nvmem_cell *)nvmem;
>
> ERR_CAST?
Will fix it.
>
>> +
>> + addr = of_get_property(cell_np, "reg", &len);
>> + if (!addr || (len < 2 * sizeof(int))) {
>> + dev_err(&nvmem->dev, "of_i2c: invalid reg on %s\n",
>
> huh? of_i2c?
>
:-)
Will fix it.
>
>> +
>> + /* if it's not end on byte boundary */
>> + if ((nbits + bit_offset) % BITS_PER_BYTE) {
>> + /* setup the last byte with msb bits from nvmem */
>> + rc = regmap_raw_read(nvmem->regmap,
>> + cell->offset + cell->bytes - 1, &v, 1);
>> + *p |= GENMASK(7, (nbits + bit_offset) % BITS_PER_BYTE) & v;
>> +
>> + }
>> +
>> + return buf;
>> +}
>> +
>> +/**
>> + * nvmem_cell_write(): Write to a given nvmem cell
>
> This isn't kernel doc notation. It should be like
>
> nvmem_cell_write - Write to a given nvmem cell
>
Will fix as suggested by Sascha as
foobar() - short function description of foobar
Looks correct according to Documentation/kernel-doc-nano-HOWTO.txt
>> + *
>> + * @cell: nvmem cell to be written.
>> + * @buf: Buffer to be written.
>> + * @len: length of buffer to be written to nvmem cell.
>> + *
>> + * The return value will be an length of bytes written or non zero on failure.
>> + */
>> +int nvmem_cell_write(struct nvmem_cell *cell, void *buf, ssize_t len)
> [..]
>> +
>> static int nvmem_init(void)
>> {
>> return class_register(&nvmem_class);
>> diff --git a/include/linux/nvmem-consumer.h b/include/linux/nvmem-consumer.h
>> new file mode 100644
>> index 0000000..c3fa8c7
>> --- /dev/null
>> +++ b/include/linux/nvmem-consumer.h
>> @@ -0,0 +1,49 @@
>> +/*
>> + * nvmem framework consumer.
>> + *
>> + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla@linaro.org>
>> + * Copyright (C) 2013 Maxime Ripard <maxime.ripard@free-electrons.com>
>> + *
>> + * This file is licensed under the terms of the GNU General Public
>> + * License version 2. This program is licensed "as is" without any
>> + * warranty of any kind, whether express or implied.
>> + */
>> +
>> +#ifndef _LINUX_NVMEM_CONSUMER_H
>> +#define _LINUX_NVMEM_CONSUMER_H
>> +
>> +/* consumer cookie */
>> +struct nvmem_cell;
>> +
>> +#if IS_ENABLED(CONFIG_NVMEM)
>> +
>> +/* Cell based interface */
>> +struct nvmem_cell *nvmem_cell_get(struct device *dev, const char *name);
>
> We should probably forward declare struct device in this file too.
Yep I will do it.
>
^ permalink raw reply
* Re: [PATCH v5 03/11] nvmem: Add a simple NVMEM framework for nvmem providers
From: Srinivas Kandagatla @ 2015-06-18 12:46 UTC (permalink / raw)
To: Stephen Boyd, linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, Greg Kroah-Hartman,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w
In-Reply-To: <5580A678.4080304-sgV2jX0FEOL9JmXXK+q4OQ@public.gmane.org>
Many thanks for review.
On 16/06/15 23:43, Stephen Boyd wrote:
> On 05/21/2015 09:43 AM, Srinivas Kandagatla wrote:
>>
>> diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
>> new file mode 100644
>> index 0000000..6c2f0b1
>> --- /dev/null
>> +++ b/drivers/nvmem/core.c
>> @@ -0,0 +1,398 @@
>> +/*
>> + * nvmem framework core.
>> + *
>> + * Copyright (C) 2015 Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> + * Copyright (C) 2013 Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 and
>> + * only version 2 as published by the Free Software Foundation.
>> + *
>> + * This program is distributed in the hope that it will be useful,
>> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
>> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
>> + * GNU General Public License for more details.
>> + */
>> +
>> +#include <linux/device.h>
>> +#include <linux/nvmem-provider.h>
>> +#include <linux/export.h>
>> +#include <linux/fs.h>
>> +#include <linux/idr.h>
>> +#include <linux/init.h>
>> +#include <linux/regmap.h>
>> +#include <linux/module.h>
>> +#include <linux/of.h>
>> +#include <linux/slab.h>
>> +#include <linux/uaccess.h>
>
> Is this include used?
>
Yep, not required.
>> +
>> +static int of_nvmem_match(struct device *dev, const void *nvmem_np)
>> +{
>> + return dev->of_node == nvmem_np;
>> +}
>> +
>> +static struct nvmem_device *of_nvmem_find(struct device_node *nvmem_np)
>
> const?
Sure,
>
>> +{
>> + struct device *d;
>> +
>> + if (!nvmem_np)
>> + return NULL;
>> +
>> + d = class_find_device(&nvmem_class, NULL, nvmem_np, of_nvmem_match);
>> +
>> + return d ? to_nvmem_device(d) : NULL;
>> +}
>> +
>> +static struct nvmem_cell *nvmem_find_cell(const char *cell_id)
>> +{
>> + struct nvmem_cell *p;
>> +
>> + list_for_each_entry(p, &nvmem_cells, node) {
>
> Unnecessary braces.
Yep, Will remove it.
>
>> + if (p && !strcmp(p->name, cell_id))
>> + return p;
>> + }
>> +
>> + return NULL;
>> +}
>> +
>> +static void nvmem_cell_drop(struct nvmem_cell *cell)
>> +{
>> + mutex_lock(&nvmem_cells_mutex);
>> + list_del(&cell->node);
>> + mutex_unlock(&nvmem_cells_mutex);
>> + kfree(cell);
>> +}
>> +
>> +static void nvmem_device_remove_all_cells(struct nvmem_device *nvmem)
>> +{
>> + struct nvmem_cell *cell = NULL;
>
> Unnecessary initialization
Yes.
>
>> + struct list_head *p, *n;
>> +
>> + list_for_each_safe(p, n, &nvmem_cells) {
>> + cell = list_entry(p, struct nvmem_cell, node);
>> + if (cell->nvmem == nvmem)
>> + nvmem_cell_drop(cell);
>> + }
> [..]
>> +
>> +static int nvmem_add_cells(struct nvmem_device *nvmem,
>> + struct nvmem_config *cfg)
>> +{
>> + struct nvmem_cell **cells;
>> + struct nvmem_cell_info *info = cfg->cells;
>> + int i, rval;
>> +
>> + cells = kzalloc(sizeof(*cells) * cfg->ncells, GFP_KERNEL);
>
> kcalloc
This is temporary array, I did this on intention, to make it easy to
kfree cells which are found invalid at runtime.
>
>> + if (!cells)
>> + return -ENOMEM;
>> +
>> + for (i = 0; i < cfg->ncells; i++) {
>> + cells[i] = kzalloc(sizeof(struct nvmem_cell), GFP_KERNEL);
>
> sizeof(**cells) ?
Yep.
>
>> +
>> +/**
>> + * nvmem_register(): Register a nvmem device for given nvmem.
>> + * Also creates an binary entry in /sys/class/nvmem/dev-name/nvmem
>> + *
>> + * @nvmem: nvmem device that needs to be created
>
> You mean @config?
>
Yes, I will fix it.
>> + *
>> + * The return value will be an ERR_PTR() on error or a valid pointer
>> + nvmem->dev.of_node = config->dev->of_node;
>> + dev_set_name(&nvmem->dev, "%s%d",
>> + config->name ? : "nvmem", config->id);
>
> It may be better to always name it nvmem%d so that we don't allow the
> possibility of conflicts.
We can do that, but I wanted to make the sysfs and dev entries more
readable than just nvmem0, nvmem1...
>
>> +
>> + nvmem->read_only = of_property_read_bool(nvmem->dev.of_node,
>> + "read-only");
>
> What if we're not using DT? How would we specify read_only?
>
Thanks for spotting, you are correct, I need to add read_only flag to
nvmem_config too.
>> +
>> + device_initialize(&nvmem->dev);
>> +
>> + dev_dbg(&nvmem->dev, "Registering nvmem device %s\n",
>> + dev_name(&nvmem->dev));
>> +
>> + rval = device_add(&nvmem->dev);
>> + if (rval) {
>> + ida_simple_remove(&nvmem_ida, nvmem->id);
>> + kfree(nvmem);
>> + return ERR_PTR(rval);
>> + }
>> +
>> + /* update sysfs attributes */
>> + if (nvmem->read_only)
>> + sysfs_update_group(&nvmem->dev.kobj, &nvmem_bin_ro_group);
>
> It would be nice if this could be done before the device was registered.
> Perhaps have two device_types, one for read-only and one for read/write?
The attributes are actually coming from the class object, so we have no
choice to update the attributes before the device is registered.
May it would be more safe to have default as read-only and modify it to
read/write based on read-only flag?
>
>> +
>> + */
>> +int nvmem_unregister(struct nvmem_device *nvmem)
>> +{
>> + mutex_lock(&nvmem_mutex);
>> + if (nvmem->users) {
>> + mutex_unlock(&nvmem_mutex);
>> + return -EBUSY;
>
> Hmm... that doesn't seem nice. Typically when something is unregistered
> we have to pull the rug out from underneath the users and start
> returning errors to them. The provider needs to be free to unregister
> because it's been forcibly removed. So really this function should
> return void.
>
The consumer api is get/put style, so consumers who already have
references to the provider, removing provider forcefully might lead to
dangling pointer. Having said that I can give a try and see how it looks.
>> + }
>> + mutex_unlock(&nvmem_mutex);
>> +
>> + nvmem_device_remove_all_cells(nvmem);
>> + device_del(&nvmem->dev);
>> +
>> + return 0;
>> +}
>> +EXPORT_SYMBOL_GPL(nvmem_unregister);
>> +
>> +static int nvmem_init(void)
>
> __init? And __exit on nvmem_exit?
yep, will do that.
>
>> +{
>> + return class_register(&nvmem_class);
>
> I thought class was on the way out? Aren't we supposed to use bus types
> for new stuff?
Do you remember any conversation on the list about this? I could not
find it on web.
on the other hand, nvmem is not really a bus, making it a bus type
sounds incorrect to me.
--srini
>
--
To unsubscribe from this list: send the line "unsubscribe devicetree" 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: [RFC v3 1/4] fs: Add generic file system event notifications
From: Kinglong Mee @ 2015-06-18 11:17 UTC (permalink / raw)
To: Beata Michalska, linux-kernel, linux-fsdevel, linux-api
Cc: greg, jack, tytso, adilger.kernel, hughd, lczerner, hch,
linux-ext4, linux-mm, kyungmin.park, kmpark, kinglongmee
In-Reply-To: <1434460173-18427-2-git-send-email-b.michalska@samsung.com>
On 6/16/2015 9:09 PM, Beata Michalska wrote:
> Introduce configurable generic interface for file
> system-wide event notifications, to provide file
> systems with a common way of reporting any potential
> issues as they emerge.
... snip ...
> +
> +Sample request could look like the following:
> +
> + echo /sample/mount/point G T 710000 500000 > /sys/fs/events/config
> +
> +Multiple request might be specified provided they are separated with semicolon.
> +
> +The configuration itself might be modified at any time. One can add/remove
> +particular event types for given fielsystem, modify the threshold levels,
> +and remove single or all entries from the 'config' file.
> +
> + - Adding new event type:
> +
> + $ echo MOUNT EVENT_TYPE > /sys/fs/events/config
> +
> +(Note that is is enough to provide the event type to be enabled without
Should be "Note that it is ... " here ?
> +the already set ones.)
> +
> + - Removing event type:
> +
> + $ echo '!MOUNT EVENT_TYPE' > /sys/fs/events/config
> +
> + - Updating threshold limits:
> +
> + $ echo MOUNT T L1 L2 > /sys/fs/events/config
> +
> + - Removing single entry:
> +
> + $ echo '!MOUNT' > /sys/fs/events/config
> +
> + - Removing all entries:
> +
> + $ echo > /sys/fs/events/config
> +
> +Reading the file will list all registered entries with their current set-up
> +along with some additional info like the filesystem type and the backing device
> +name if available.
> +
> +Final, though a very important note on the configuration: when and if the
> +actual events are being triggered falls way beyond the scope of the generic
> +filesystem events interface. It is up to a particular filesystem
> +implementation which events are to be supported - if any at all. So if
> +given filesystem does not support the event notifications, an attempt to
> +enable those through 'config' file will fail.
> +
> +
> +3. The generic netlink interface support:
> +=========================================
> +
> +Whenever an event notification is triggered (by given filesystem) the current
> +configuration is being validated to decide whether a userpsace notification
> +should be launched. If there has been no request (in a mean of 'config' file
> +entry) for given event, one will be silently disregarded. If, on the other
> +hand, someone is 'watching' given filesystem for specific events, a generic
> +netlink message will be sent. A dedicated multicast group has been provided
> +solely for this purpose so in order to receive such notifications, one should
> +subscribe to this new multicast group. As for now only the init network
> +namespace is being supported.
> +
> +3.1 Message format
> +
> +The FS_NL_C_EVENT shall be stored within the generic netlink message header
> +as the command field. The message payload will provide more detailed info:
> +the backing device major and minor numbers, the event code and the id of
> +the process which action led to the event occurrence. In case of threshold
> +notifications, the current number of available blocks will be included
> +in the payload as well.
> +
> +
> + 0 1 2 3
> + 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + | NETLINK MESSAGE HEADER |
> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + | GENERIC NETLINK MESSAGE HEADER |
> + | (with FS_NL_C_EVENT as genlmsghdr cdm field) |
cmd, not cdm.
> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + | Optional user specific message header |
> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> + | GENERIC MESSAGE PAYLOAD: |
> + +---------------------------------------------------------------+
> + | FS_NL_A_EVENT_ID (NLA_U32) |
> + +---------------------------------------------------------------+
> + | FS_NL_A_DEV_MAJOR (NLA_U32) |
> + +---------------------------------------------------------------+
> + | FS_NL_A_DEV_MINOR (NLA_U32) |
> + +---------------------------------------------------------------+
> + | FS_NL_A_CAUSED_ID (NLA_U32) |
Should be NLA_U64 ? The following uses as,
+ if (nla_put_u64(skb, FS_NL_A_CAUSED_ID, pid_vnr(task_pid(current))))
+ return -EINVAL;
Also, I'd like FS_NL_A_CAUSED_PID than FS_NL_A_CAUSED_ID.
> + +---------------------------------------------------------------+
> + | FS_NL_A_DATA (NLA_U64) |
> + +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
> +
> +
> +The above figure is based on:
> + http://www.linuxfoundation.org/collaborate/workgroups/networking/generic_netlink_howto#Message_Format
> +
> +
... snip...
> + seq_putc(m, ' ');
> + if (sb->s_op->show_devname) {
> + sb->s_op->show_devname(m, en->mnt_path.mnt->mnt_root);
> + } else {
> + seq_escape(m, r_mnt->mnt_devname ? r_mnt->mnt_devname : "none",
> + " \t\n\\");
> + }
> + seq_puts(m, " (");
> +
> + nmask = en->notify;
> + for (match = fs_etypes; match->pattern; ++match) {
> + if (match->token & nmask) {
> + seq_puts(m, match->pattern);
Print here is better.
if (match->pattern & FS_EVENT_THRESH)
seq_printf(m, " %llu %llu", en->th.lrange, en->th.urange);
> + nmask &= ~match->token;
> + if (nmask)
> + seq_putc(m, ',');
> + }
> + }
> + seq_printf(m, " %llu %llu", en->th.lrange, en->th.urange);
Don't print the lrange/urange (always be zero) when without FS_EVENT_THRESH.
> + seq_puts(m, ")\n");
> + return 0;
> +}
> +
> +static const struct seq_operations fs_trace_seq_ops = {
> + .start = fs_trace_seq_start,
> + .next = fs_trace_seq_next,
> + .stop = fs_trace_seq_stop,
> + .show = fs_trace_seq_show,
> +};
> +
> +static int fs_trace_open(struct inode *inode, struct file *file)
> +{
> + return seq_open(file, &fs_trace_seq_ops);
> +}
> +
> +static const struct file_operations fs_trace_fops = {
> + .owner = THIS_MODULE,
> + .open = fs_trace_open,
> + .write = fs_trace_write,
> + .read = seq_read,
> + .llseek = seq_lseek,
> + .release = seq_release,
> +};
> +
> +static int fs_trace_init(void)
> +{
> + fs_trace_cachep = KMEM_CACHE(fs_trace_entry, 0);
> + if (!fs_trace_cachep)
> + return -EINVAL;
> + init_waitqueue_head(&trace_wq);
> + return 0;
> +}
> +
> +/* VFS support */
> +static int fs_trace_fill_super(struct super_block *sb, void *data, int silen)
> +{
> + int ret;
> + static struct tree_descr desc[] = {
> + [2] = {
> + .name = "config",
> + .ops = &fs_trace_fops,
> + .mode = S_IWUSR | S_IRUGO,
> + },
> + {""},
> + };
> +
> + ret = simple_fill_super(sb, 0x7246332, desc);
> + return !ret ? fs_trace_init() : ret;
> +}
> +
> +static struct dentry *fs_trace_do_mount(struct file_system_type *fs_type,
> + int ntype, const char *dev_name, void *data)
> +{
> + return mount_single(fs_type, ntype, data, fs_trace_fill_super);
> +}
> +
> +static void fs_trace_kill_super(struct super_block *sb)
> +{
> + /*
> + * The rcu_barrier here will/should make sure all call_rcu
> + * callbacks are completed - still there might be some active
> + * trace objects in use which can make calling the
> + * kmem_cache_destroy unsafe. So we wait until all traces
> + * are finally released.
> + */
> + fs_remove_all_traces();
> + rcu_barrier();
> + wait_event(trace_wq, !atomic_read(&stray_traces));
> +
> + kmem_cache_destroy(fs_trace_cachep);
> + kill_litter_super(sb);
> +}
> +
> +static struct kset *fs_trace_kset;
> +
> +static struct file_system_type fs_trace_fstype = {
> + .name = "fstrace",
> + .mount = fs_trace_do_mount,
> + .kill_sb = fs_trace_kill_super,
> +};
> +
> +static void __init fs_trace_vfs_init(void)
> +{
> + fs_trace_kset = kset_create_and_add("events", NULL, fs_kobj);
> +
> + if (!fs_trace_kset)
> + return;
> +
> + if (!register_filesystem(&fs_trace_fstype)) {
> + if (!fs_event_netlink_register())
> + return;
> + unregister_filesystem(&fs_trace_fstype);
> + }
> + kset_unregister(fs_trace_kset);
> +}
> +
> +static int __init fs_trace_evens_init(void)
> +{
> + fs_trace_vfs_init();
> + return 0;
> +};
> +module_init(fs_trace_evens_init);
> +
> diff --git a/fs/events/fs_event.h b/fs/events/fs_event.h
> new file mode 100644
> index 0000000..23f24c8
> --- /dev/null
> +++ b/fs/events/fs_event.h
> @@ -0,0 +1,22 @@
> +/*
> + * Copyright(c) 2015 Samsung Electronics. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2.
> + *
> + * The full GNU General Public License is included in this distribution in the
> + * file called COPYING.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +
> +#ifndef __GENERIC_FS_EVENTS_H
> +#define __GENERIC_FS_EVENTS_H
> +
> +int fs_event_netlink_register(void);
> +void fs_event_netlink_unregister(void);
> +
> +#endif /* __GENERIC_FS_EVENTS_H */
> diff --git a/fs/events/fs_event_netlink.c b/fs/events/fs_event_netlink.c
> new file mode 100644
> index 0000000..0c97eb7
> --- /dev/null
> +++ b/fs/events/fs_event_netlink.c
> @@ -0,0 +1,104 @@
> +/*
> + * Copyright(c) 2015 Samsung Electronics. All rights reserved.
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License version 2.
> + *
> + * The full GNU General Public License is included in this distribution in the
> + * file called COPYING.
> + *
> + * This program is distributed in the hope that it will be useful, but WITHOUT
> + * ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or
> + * FITNESS FOR A PARTICULAR PURPOSE. See the GNU General Public License for
> + * more details.
> + */
> +#include <linux/fs.h>
> +#include <linux/init.h>
> +#include <linux/kernel.h>
> +#include <linux/sched.h>
> +#include <linux/slab.h>
> +#include <net/netlink.h>
> +#include <net/genetlink.h>
> +#include "fs_event.h"
> +
> +static const struct genl_multicast_group fs_event_mcgroups[] = {
> + { .name = FS_EVENTS_MCAST_GRP_NAME, },
> +};
> +
> +static struct genl_family fs_event_family = {
> + .id = GENL_ID_GENERATE,
> + .name = FS_EVENTS_FAMILY_NAME,
> + .version = 1,
> + .maxattr = FS_NL_A_MAX,
> + .mcgrps = fs_event_mcgroups,
> + .n_mcgrps = ARRAY_SIZE(fs_event_mcgroups),
> +};
> +
> +int fs_netlink_send_event(size_t size, unsigned int event_id,
> + int (*compose_msg)(struct sk_buff *skb, void *data),
> + void *cbdata)
> +{
> + static atomic_t seq;
> + struct sk_buff *skb;
> + void *msg_head;
> + int ret = 0;
> +
> + if (!size || !compose_msg)
> + return -EINVAL;
> +
> + /* Skip if there are no listeners */
> + if (!genl_has_listeners(&fs_event_family, &init_net, 0))
> + return 0;
> +
> + if (event_id != FS_EVENT_NONE)
> + size += nla_total_size(sizeof(u32));
> + size += nla_total_size(sizeof(u64));
What is this for ?
thanks
Kinglong Mee
--
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: [PATCH v2 0/5] selftests/kdbus: small fixes
From: David Herrmann @ 2015-06-18 10:05 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Shuah Khan, Sergei Zviagintsev, Daniel Mack, David Herrmann,
Djalal Harouni, linux-kernel, Linux API
In-Reply-To: <20150617191130.GB3186-U8xfFu+wG4EAvxtiuMwx3w@public.gmane.org>
Hi
On Wed, Jun 17, 2015 at 9:11 PM, Greg Kroah-Hartman
<gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r@public.gmane.org> wrote:
> On Wed, Jun 17, 2015 at 11:07:46AM -0600, Shuah Khan wrote:
>> On 06/17/2015 10:33 AM, Sergei Zviagintsev wrote:
>> > Set of different fixes over selftests/kdbus.
>> >
>> > These were first sent as independent patches, and in v2 they just
>> > properly threaded.
>> >
>> > Links to previous patches:
>> >
>> > http://lkml.kernel.org/g/1434550697-24785-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org
>> > http://lkml.kernel.org/g/1434550697-24785-2-git-send-email-sergei-E844GTqJAzo@public.gmane.org
>> > http://lkml.kernel.org/g/1434550697-24785-3-git-send-email-sergei-E844GTqJAzo@public.gmane.org
>> > http://lkml.kernel.org/g/1434550697-24785-4-git-send-email-sergei-E844GTqJAzo@public.gmane.org
>> > http://lkml.kernel.org/g/1434550697-24785-5-git-send-email-sergei-E844GTqJAzo@public.gmane.org
>> >
>> > Sergei Zviagintsev (5):
>> > selftests/kdbus: handle cap_get_proc() error properly
>> > selftests/kdbus: drop useless assignment
>> > selftests/kdbus: remove useless initializations from
>> > kdbus_clone_userns_test()
>> > selftests/kdbus: drop duplicated code from __kdbus_msg_send()
>> > selftests/kdbus: fix error paths in __kdbus_msg_send()
>> >
>> > tools/testing/selftests/kdbus/kdbus-util.c | 46 ++++++++++++------------
>> > tools/testing/selftests/kdbus/test-metadata-ns.c | 9 ++---
>> > 2 files changed, 27 insertions(+), 28 deletions(-)
>> >
>>
>> Nice work! Thanks for these fixes.
>> Greg! Are you going to apply these into your kdbus tree?
>
> Yes, I will take them.
The 5 selftest changes are:
Reviewed-by: David Herrmann <dh.herrmann-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
Thanks
David
^ permalink raw reply
* Re: [PATCH] HID: roccat: Removing all modules except Kone
From: Jiri Kosina @ 2015-06-18 9:42 UTC (permalink / raw)
To: Stefan Achatz
Cc: linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-input-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1429202372.2351.6.camel-/ZsuMndpQpv+LLWKTYdlgUWBcfzl8p5a/AviwAImx+Y@public.gmane.org>
On Thu, 16 Apr 2015, Stefan Achatz wrote:
> This patch removes all roccat hid modules except the one for the first Kone.
>
> Package roccat-tools is the only known user of these modules. Starting with
> roccat-tools-2.2.0, released 7 months ago, I'm not using these modules
> anymore. All functionality has been moved to userspace using hidraw.
7 months means 2 or 3 major kernel releases, and no depreciation process
has happened.
Could you please work on more user-friendly depreciation plan for these
drivers?
Thanks,
--
Jiri Kosina
SUSE Labs
^ permalink raw reply
* Re: [RFC v3 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-06-18 8:25 UTC (permalink / raw)
To: Dave Chinner
Cc: linux-kernel, linux-fsdevel, linux-api, greg, jack, tytso,
adilger.kernel, hughd, lczerner, hch, linux-ext4, linux-mm,
kyungmin.park, kmpark
In-Reply-To: <20150617230605.GK10224@dastard>
Hi,
On 06/18/2015 01:06 AM, Dave Chinner wrote:
> On Tue, Jun 16, 2015 at 03:09:30PM +0200, Beata Michalska wrote:
>> Introduce configurable generic interface for file
>> system-wide event notifications, to provide file
>> systems with a common way of reporting any potential
>> issues as they emerge.
>>
>> The notifications are to be issued through generic
>> netlink interface by newly introduced multicast group.
>>
>> Threshold notifications have been included, allowing
>> triggering an event whenever the amount of free space drops
>> below a certain level - or levels to be more precise as two
>> of them are being supported: the lower and the upper range.
>> The notifications work both ways: once the threshold level
>> has been reached, an event shall be generated whenever
>> the number of available blocks goes up again re-activating
>> the threshold.
>>
>> The interface has been exposed through a vfs. Once mounted,
>> it serves as an entry point for the set-up where one can
>> register for particular file system events.
>>
>> Signed-off-by: Beata Michalska <b.michalska@samsung.com>
>
> This has massive scalability problems:
>
>> + 4.3 Threshold notifications:
>> +
>> + #include <linux/fs_event.h>
>> + void fs_event_alloc_space(struct super_block *sb, u64 ncount);
>> + void fs_event_free_space(struct super_block *sb, u64 ncount);
>> +
>> + Each filesystme supporting the threshold notifications should call
>> + fs_event_alloc_space/fs_event_free_space respectively whenever the
>> + amount of available blocks changes.
>> + - sb: the filesystem's super block
>> + - ncount: number of blocks being acquired/released
>
> ... here.
>
>> + Note that to properly handle the threshold notifications the fs events
>> + interface needs to be kept up to date by the filesystems. Each should
>> + register fs_trace_operations to enable querying the current number of
>> + available blocks.
>
> Have you noticed that the filesystems have percpu counters for
> tracking global space usage? There's good reason for that - taking a
> spinlock in such a hot accounting path causes severe contention.
>
>> +static void fs_event_send(struct fs_trace_entry *en, unsigned int event_id)
>> +{
>> + size_t size = nla_total_size(sizeof(u32)) * 2 +
>> + nla_total_size(sizeof(u64));
>> +
>> + fs_netlink_send_event(size, event_id, create_common_msg, en);
>> +}
>> +
>> +static void fs_event_send_thresh(struct fs_trace_entry *en,
>> + unsigned int event_id)
>> +{
>> + size_t size = nla_total_size(sizeof(u32)) * 2 +
>> + nla_total_size(sizeof(u64)) * 2;
>> +
>> + fs_netlink_send_event(size, event_id, create_thresh_msg, en);
>> +}
>> +
>> +void fs_event_notify(struct super_block *sb, unsigned int event_id)
>> +{
>> + struct fs_trace_entry *en;
>> +
>> + en = fs_trace_entry_get_rcu(sb);
>> + if (!en)
>> + return;
>> +
>> + spin_lock(&en->lock);
>> + if (atomic_read(&en->active) && (en->notify & FS_EVENT_GENERIC))
>> + fs_event_send(en, event_id);
>> + spin_unlock(&en->lock);
>> + fs_trace_entry_put(en);
>> +}
>> +EXPORT_SYMBOL(fs_event_notify);
>> +
>> +void fs_event_alloc_space(struct super_block *sb, u64 ncount)
>> +{
>> + struct fs_trace_entry *en;
>> + s64 count;
>> +
>> + en = fs_trace_entry_get_rcu(sb);
>> + if (!en)
>> + return;
>
> Adds an atomic write to get the trace entry,
>
>> + spin_lock(&en->lock);
>
> a spin lock to lock the entry,
>
>
>> + if (!atomic_read(&en->active) || !(en->notify & FS_EVENT_THRESH))
>> + goto leave;
>> + /*
>> + * we shouldn't drop below 0 here,
>> + * unless there is a sync issue somewhere (?)
>> + */
>> + count = en->th.avail_space - ncount;
>> + en->th.avail_space = count < 0 ? 0 : count;
>> +
>> + if (en->th.avail_space > en->th.lrange)
>> + /* Not 'even' close - leave */
>> + goto leave;
>> +
>> + if (en->th.avail_space > en->th.urange) {
>> + /* Close enough - the lower range has been reached */
>> + if (!(en->th.state & THRESH_LR_BEYOND)) {
>> + /* Send notification */
>> + fs_event_send_thresh(en, FS_THR_LRBELOW);
>> + en->th.state &= ~THRESH_LR_BELOW;
>> + en->th.state |= THRESH_LR_BEYOND;
>> + }
>> + goto leave;
>
> Then puts the entire netlink send path inside this spinlock, which
> includes memory allocation and all sorts of non-filesystem code
> paths. And it may be inside critical filesystem locks as well....
>
> Apart from the serialisation problem of the locking, adding
> memory allocation and the network send path to filesystem code
> that is effectively considered "innermost" filesystem code is going
> to have all sorts of problems for various filesystems. In the XFS
> case, we simply cannot execute this sort of function in the places
> where we update global space accounting.
>
> As it is, I think the basic concept of separate tracking of free
> space if fundamentally flawed. What I think needs to be done is that
> filesystems need access to the thresholds for events, and then the
> filesystems call fs_event_send_thresh() themselves from appropriate
> contexts (ie. without compromising locking, scalability, memory
> allocation recursion constraints, etc).
>
> e.g. instead of tracking every change in free space, a filesystem
> might execute this once every few seconds from a workqueue:
>
> event = fs_event_need_space_warning(sb, <fs_free_space>)
> if (event)
> fs_event_send_thresh(sb, event);
>
> User still gets warnings about space usage, but there's no runtime
> overhead or problems with lock/memory allocation contexts, etc.
>
> Cheers,
>
> Dave.
>
Having fs to keep a firm hand on thresholds limits would indeed be
far more sane approach though that would require each fs to
add support for that and handle most of it on their own. Avoiding
this was the main rationale behind this rfc.
If fs people agree to that, I'll be more than willing to drop this
in favour of the per-fs tracking solution.
Personally, I hope they will.
Best Regards
Beata
^ permalink raw reply
* Re: [PATCH v2 net-next 0/3] bpf: share helpers between tracing and networking
From: Daniel Borkmann @ 2015-06-18 8:10 UTC (permalink / raw)
To: Alexei Starovoitov, David Miller
Cc: luto-kltTT9wpgjJwATOyAt5JVQ, mingo-DgEjT+Ai2ygdnm+yROfE0A,
rostedt-nx8X9YLhiw1AfugRpC6u6w, wangnan0-hv44wF8Li93QT0dZR+AlfA,
lizefan-hv44wF8Li93QT0dZR+AlfA,
daniel.wagner-98C5kh4wR6ohFhg+JK9F0w,
linux-api-u79uwXL29TY76Z2rM5mHXA, netdev-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <558058A0.7030201-uqk4Ao+rVK5Wk0Htik3J/w@public.gmane.org>
On 06/16/2015 07:10 PM, Alexei Starovoitov wrote:
...
> Ideally we would allow a blend of tracing and networking programs,
> then the best solution would be one or two stable tracepoints in
> networking stack where skb is visible and receiving/transmitting task
> is also visible, then skb->len and task->pid together would give nice
> foundation for accurate stats.
I think combining both seems interesting anyway, we need to find
a way to make this gluing of both worlds easy to use, though. It's
certainly interesting for stats/diagnostics, but one wouldn't be
able to use the current/future skb eBPF helpers from {cls,act}_bpf
in that context.
^ permalink raw reply
* Re: UAPI headers including non-UAPI headers by accident?
From: Arnd Bergmann @ 2015-06-18 7:58 UTC (permalink / raw)
To: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w
Cc: Andy Lutomirski,
linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
David Woodhouse
In-Reply-To: <CAKgNAkjLmBUayYjVOr6Stcbmcn=ENb3Uyt0dkFQCmn-Lxdn0xw-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Thursday 18 June 2015 09:52:36 Michael Kerrisk wrote:
> [CC += David]
>
> On 2 June 2015 at 18:36, Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> > include/uapi/linux/signal.h starts with:
> >
> > #ifndef _UAPI_LINUX_SIGNAL_H
> > #define _UAPI_LINUX_SIGNAL_H
> >
> > #include <asm/signal.h>
> > #include <asm/siginfo.h>
> >
> > This causes it to include <asm/signal.h>, which is not the same thing
> > as <uapi/asm/signal.h>. Changing that will break userspace use of
> > this header, though, as the uapi/ won't get removed.
> >
> > What's the correct fix? This is causing trouble with a UML build for me.
>
> Perhaps David has some insight, since he architected the original UAPI split.
The uapi headers are installed without the uapi prefix. This means
that inside of the kernel, we get
linux/signal.h
-> uapi/linux/signal.h
-> asm/signal.h
-> uapi/asm/signal.h
while in the installed headers we just get
linux/signal.h
-> asm/signal.h
This all looks right to me: user space only sees the exported portions
under the traditional names, while the kernel sees both the kernel-side
and user-side definitions from the same path.
Arnd
^ permalink raw reply
* Re: UAPI headers including non-UAPI headers by accident?
From: Michael Kerrisk (man-pages) @ 2015-06-18 7:52 UTC (permalink / raw)
To: Andy Lutomirski
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux API,
David Woodhouse
In-Reply-To: <CALCETrVaWJVDFydhXv320iCuFhTMi9txx+MpzsA_Q7yeZxMDSQ-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
[CC += David]
On 2 June 2015 at 18:36, Andy Lutomirski <luto-DgEjT+Ai2ygdnm+yROfE0A@public.gmane.org> wrote:
> include/uapi/linux/signal.h starts with:
>
> #ifndef _UAPI_LINUX_SIGNAL_H
> #define _UAPI_LINUX_SIGNAL_H
>
> #include <asm/signal.h>
> #include <asm/siginfo.h>
>
> This causes it to include <asm/signal.h>, which is not the same thing
> as <uapi/asm/signal.h>. Changing that will break userspace use of
> this header, though, as the uapi/ won't get removed.
>
> What's the correct fix? This is causing trouble with a UML build for me.
Perhaps David has some insight, since he architected the original UAPI split.
Cheers,
Michael
^ permalink raw reply
* Re: [RFC v2 23/24] m68k/mac: Fix PRAM accessors
From: Geert Uytterhoeven @ 2015-06-18 6:59 UTC (permalink / raw)
To: Finn Thain
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA@public.gmane.org, Linux/m68k,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ@public.gmane.org,
linux-api-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
In-Reply-To: <alpine.LNX.2.00.1506181417260.30499-i19888lE8tflDoPlx7XIcw@public.gmane.org>
Hi Finn,
On Thu, Jun 18, 2015 at 6:49 AM, Finn Thain <fthain-PnMVE5gNl/W9dxLkCovUHLpzq4S04n8Q@public.gmane.org> wrote:
> On Tue, 16 Jun 2015, in which I wrote:
>> On Mon, 15 Jun 2015, Geert Uytterhoeven wrote:
>> > More magic values...
>>
>> [...] The only useful RTC documentation I've ever come across is this:
>> http://mac.linux-m68k.org/devel/plushw.php
>
> This document appears to be Inside Macintosh vol. III ch. 2. It describes
> the early RTC chip that lacks two-byte operations and XPRAM, and pre-dates
> all Mac hardware supported in mainline Linux. But it does offer some
> useful data, though not enough to answer all of your criticisms (as I
> said).
I understand.
>> [...] I think they should be applied across the entire file, and in a
>> different patch. Inconsistent use of such macros would be undesirable
>> IMHO.
>
> So, unless you have other ideas, I will revise this patch and insert an
> earlier patch to address existing code, and codify what little reliable
> chip data we have.
Thanks!
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert-Td1EMuHUCqxL1ZNQvxDV9g@public.gmane.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: [PATCH] selftests: add seccomp suite
From: Michael Ellerman @ 2015-06-18 6:41 UTC (permalink / raw)
To: Kees Cook
Cc: LKML, Daniel Borkmann, Shuah Khan, Andy Lutomirski, Will Drewry,
Andrew Morton, Greg KH, Mauro Carvalho Chehab, David S. Miller,
Arnd Bergmann, Joe Perches, Jingoo Han, Linux API
In-Reply-To: <CAGXu5jK3xuCqJoE9W+b_5yH+TffDDaL5tDFyZhKRx_K-Qqqk2g-JsoAwUIsXosN+BqQ9rBEUg@public.gmane.org>
On Wed, 2015-06-17 at 11:12 -0700, Kees Cook wrote:
> On Wed, Jun 17, 2015 at 12:31 AM, Michael Ellerman <mpe-Gsx/Oe8HsFggBc27wqDAHg@public.gmane.org> wrote:
> > On Tue, 2015-06-16 at 10:54 -0700, Kees Cook wrote:
> >> This imports the existing seccomp test suite into the kernel's selftests
> >> tree. It contains extensive testing of seccomp features and corner cases.
> >> There remain additional tests to move into the kernel tree, but they have
> >> not yet been ported to all the architectures seccomp supports:
> >> https://github.com/redpig/seccomp/tree/master/tests
> >>
> >> Signed-off-by: Kees Cook <keescook-F7+t8E8rja9g9hUCZPvPmw@public.gmane.org>
> >> ---
> >> MAINTAINERS | 1 +
> >> tools/testing/selftests/Makefile | 1 +
> >> tools/testing/selftests/seccomp/.gitignore | 1 +
> >> tools/testing/selftests/seccomp/Makefile | 10 +
> >> tools/testing/selftests/seccomp/seccomp_bpf.c | 2109 ++++++++++++++++++++++++
> >> tools/testing/selftests/seccomp/test_harness.h | 537 ++++++
> >
> >
> > Thanks very much for adding this, it would have been very helpful recently when
> > I was trying to get seccomp filter working on powerpc :)
> >
> > I get one failure in TRACE_syscall.syscall_dropped:
> >
> > seccomp_bpf.c:1394:TRACE_syscall.syscall_dropped:Expected 1 (1) == syscall(207) (18446744073709551615)
> >
> >
> > So it looks like we're returning -1 instead of 1.
> >
> > That's probably a bug in our handling of the return value, or maybe an
> > inconsistency across the arches. I'll try and find time to dig into it.
>
> Ah-ha! Excellent. Did you add an implementation for change_syscall()
> in seccomp_bpf.c? I don't have a powerpc method in there. I would have
> expected both TRACE_syscall.syscall_redirected and .syscall_dropped to
> fail without that.
Yeah I did add a change_syscall() implementation, patch below.
> If you did, maybe something isn't right with regs.SYSCALL_RET ? That's
> where the return value being tested on a skipped syscall is stored.
Yeah I saw that too, and I think you're probably right that's where the problem
is. It doesn't seem to matter what I put in SYSCALL_RET I always get -1, so I
think there's a bug in my kernel code.
Will try and work it out tonight.
cheers
diff --git a/tools/testing/selftests/seccomp/seccomp_bpf.c b/tools/testing/selftests/seccomp/seccomp_bpf.c
index c5abe7fd7590..1bced19c54fb 100644
--- a/tools/testing/selftests/seccomp/seccomp_bpf.c
+++ b/tools/testing/selftests/seccomp/seccomp_bpf.c
@@ -14,6 +14,7 @@
#include <linux/filter.h>
#include <sys/prctl.h>
#include <sys/ptrace.h>
+#include <sys/types.h>
#include <sys/user.h>
#include <linux/prctl.h>
#include <linux/ptrace.h>
@@ -1199,6 +1200,10 @@ TEST_F(TRACE_poke, getpid_runs_normally)
# define ARCH_REGS struct user_pt_regs
# define SYSCALL_NUM regs[8]
# define SYSCALL_RET regs[0]
+#elif defined(__powerpc__)
+# define ARCH_REGS struct pt_regs
+# define SYSCALL_NUM gpr[0]
+# define SYSCALL_RET gpr[3]
#else
# error "Do not know how to find your architecture's registers and syscalls"
#endif
@@ -1246,6 +1251,10 @@ void change_syscall(struct __test_metadata *_metadata,
EXPECT_EQ(0, ret);
}
+#elif defined(__powerpc__)
+ {
+ regs.SYSCALL_NUM = syscall;
+ }
#else
ASSERT_EQ(1, 0) {
TH_LOG("How is the syscall changed on this architecture?");
@@ -1396,6 +1405,8 @@ TEST_F(TRACE_syscall, syscall_dropped)
# define __NR_seccomp 383
# elif defined(__aarch64__)
# define __NR_seccomp 277
+# elif defined(__powerpc__)
+# define __NR_seccomp 358
# else
# warning "seccomp syscall number unknown for this architecture"
# define __NR_seccomp 0xffff
^ permalink raw reply related
* Re: [RFC v2 23/24] m68k/mac: Fix PRAM accessors
From: Finn Thain @ 2015-06-18 4:49 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: linux-kernel@vger.kernel.org, Linux/m68k,
linuxppc-dev@lists.ozlabs.org, linux-api@vger.kernel.org
In-Reply-To: <alpine.LNX.2.00.1506152213340.12762@nippy.intranet>
Hi Geert,
Further to my previous email,
On Tue, 16 Jun 2015, in which I wrote:
>
> On Mon, 15 Jun 2015, Geert Uytterhoeven wrote:
>
> >
> > More magic values...
>
> [...] The only useful RTC documentation I've ever come across is this:
> http://mac.linux-m68k.org/devel/plushw.php
This document appears to be Inside Macintosh vol. III ch. 2. It describes
the early RTC chip that lacks two-byte operations and XPRAM, and pre-dates
all Mac hardware supported in mainline Linux. But it does offer some
useful data, though not enough to answer all of your criticisms (as I
said).
> [...] I think they should be applied across the entire file, and in a
> different patch. Inconsistent use of such macros would be undesirable
> IMHO.
So, unless you have other ideas, I will revise this patch and insert an
earlier patch to address existing code, and codify what little reliable
chip data we have.
--
^ 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