* 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: [RFC v3 1/4] fs: Add generic file system event notifications
From: Dave Chinner @ 2015-06-19 0:03 UTC (permalink / raw)
To: Beata Michalska
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: <55828064.5040301@samsung.com>
On Thu, Jun 18, 2015 at 10:25:08AM +0200, Beata Michalska wrote:
> 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:
....
> > 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.
....
> > 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.
>
> 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.
I was hoping that you'd think a little more about my suggestion and
work out how to do background threshold event detection generically.
I kind of left it as "an exercise for the reader" because it seems
obvious to me.
Hint: ->statfs allows you to get the total, free and used space
from filesystems in a generic manner.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
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: Greg Kroah-Hartman @ 2015-06-19 4:27 UTC (permalink / raw)
To: Sergei Zviagintsev
Cc: Daniel Mack, David Herrmann, Djalal Harouni, Shuah Khan,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <1434558808-16129-1-git-send-email-sergei-E844GTqJAzo@public.gmane.org>
On Wed, Jun 17, 2015 at 07:33:23PM +0300, 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(-)
All now applied, thanks.
greg k-h
^ permalink raw reply
* Re: [PATCH v5 06/11] nvmem: Add bindings for simple nvmem framework
From: maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w @ 2015-06-19 10:36 UTC (permalink / raw)
To: Srinivas Kandagatla
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
Greg Kroah-Hartman, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Rob Herring, Mark Brown,
Kumar Gala, mporter-OWPKS81ov/FWk0Htik3J/w, Maxime Ripard,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Stefan Agner
In-Reply-To: <1432226652-8947-1-git-send-email-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
Hello Srinivas,
On 15-05-21 17:44:12, Srinivas Kandagatla wrote:
> This patch adds bindings for simple nvmem framework which allows nvmem
> consumers to talk to nvmem providers to get access to nvmem cell data.
>
> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
> [Maxime Ripard: intial version of eeprom framework]
> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
> ---
> Documentation/devicetree/bindings/nvmem/nvmem.txt | 84 +++++++++++++++++++++++
> 1 file changed, 84 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
>
> 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.
> +
> +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.
> +
> +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.
> +
> +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.
> +
> +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;
> +
> + };
> + ...
> + };
We have a need of exposing information like SoC ID, revision and such
which is what this nvmem framework proposes to be suitable for. Till
now I was trying a different approach for the same [1].
The On Chip One Time Programmable block on the Vybrid can be handled
nicely with this nvmem framework however I had a few queries with
regards to the framework after trying it on a Vybrid VF61 SoC.
1. From what I understand, getting the information in hex is the only
way possible as of now? Would it be possible to expose the information
as strings from different paths under /sys/class/nvmem/*/nvmem/?
For example, if I have a sub node as below
ocotp: ocotp@400a5000 {
compatible = "fsl,vf610-ocotp";
reg = <0x400a5000 0x1000>;
ocotp_cfg0: cfg0@410 {
reg = <0x410 0x1>;
};
ocotp_cfg1: cfg1@420 {
reg = <0x420 0x1>;
};
};
The values from the above register locations represented by the two
sub nodes above can perhaps be exposed as strings? Even if they are
not exposed as strings, making them available in separate paths, is
that something which can be done? So depending on the sub node as
above, /sys/class/nvmem/ocotp0/nvmem/cfg0/ would give values from
the registers specified.
Basically the output of /sys/class/nvmem/*/nvmem being restricted
to only the subnodes specified is what I was hoping to get along
with separate paths.
2. What if the required information is scattered across different memory
regions? In my case, the SoC ID is available from one OCOTP peripheral
block, the revision is in a separate ROM area at the start of the memory
map and some CPU information I am interested in is in another memory
region. I am not sure what would be the right way to approach it with
the nvmem framework.
[1] https://lkml.org/lkml/2015/6/5/189
Thanks & Regards,
Sanchayan Maity.
<snip>
^ permalink raw reply
* Re: [PATCH v5 06/11] nvmem: Add bindings for simple nvmem framework
From: Srinivas Kandagatla @ 2015-06-19 10:59 UTC (permalink / raw)
To: maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w
Cc: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
devicetree-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
Greg Kroah-Hartman, s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ, linux-kernel-u79uwXL29TY76Z2rM5mHXA,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w, Rob Herring, Mark Brown,
Kumar Gala, mporter-OWPKS81ov/FWk0Htik3J/w, Maxime Ripard,
linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, Stefan Agner
In-Reply-To: <20150619103647.GA3913@Sanchayan-Arch>
On 19/06/15 11:36, maitysanchayan-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org wrote:
> Hello Srinivas,
>
> On 15-05-21 17:44:12, Srinivas Kandagatla wrote:
>> This patch adds bindings for simple nvmem framework which allows nvmem
>> consumers to talk to nvmem providers to get access to nvmem cell data.
>>
>> Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
>> [Maxime Ripard: intial version of eeprom framework]
>> Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
>> ---
>> Documentation/devicetree/bindings/nvmem/nvmem.txt | 84 +++++++++++++++++++++++
>> 1 file changed, 84 insertions(+)
>> create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
>>
>> 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.
>> +
>> +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.
>> +
>> +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.
>> +
>> +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.
>> +
>> +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;
>> +
>> + };
>> + ...
>> + };
>
> We have a need of exposing information like SoC ID, revision and such
> which is what this nvmem framework proposes to be suitable for. Till
> now I was trying a different approach for the same [1].
>
> The On Chip One Time Programmable block on the Vybrid can be handled
> nicely with this nvmem framework however I had a few queries with
> regards to the framework after trying it on a Vybrid VF61 SoC.
>
> 1. From what I understand, getting the information in hex is the only
> way possible as of now? Would it be possible to expose the information
nvmem file in the /sys/ is just a binary file. hexdump is one of the
ways to dump the data, the user can read the binary file and interpret
the data in the way he wants it.
> as strings from different paths under /sys/class/nvmem/*/nvmem/?
>
> For example, if I have a sub node as below
>
> ocotp: ocotp@400a5000 {
> compatible = "fsl,vf610-ocotp";
> reg = <0x400a5000 0x1000>;
>
> ocotp_cfg0: cfg0@410 {
> reg = <0x410 0x1>;
> };
>
> ocotp_cfg1: cfg1@420 {
> reg = <0x420 0x1>;
> };
> };
>
> The values from the above register locations represented by the two
> sub nodes above can perhaps be exposed as strings? Even if they are
> not exposed as strings, making them available in separate paths, is
> that something which can be done? So depending on the sub node as
> above, /sys/class/nvmem/ocotp0/nvmem/cfg0/ would give values from
> the registers specified.
I was thinking of add similar support to show cells in
/sys/class/nvmem/*/cells/cfg0 in future once the framework is merged.
For now I want to keep it simple. This would be binary content again.
you can dump it using strings any program which wants to interpret it
differently.
>
> Basically the output of /sys/class/nvmem/*/nvmem being restricted
> to only the subnodes specified is what I was hoping to get along
> with separate paths.
>
> 2. What if the required information is scattered across different memory
> regions? In my case, the SoC ID is available from one OCOTP peripheral
> block, the revision is in a separate ROM area at the start of the memory
> map and some CPU information I am interested in is in another memory
> region. I am not sure what would be the right way to approach it with
> the nvmem framework.
I think you would have two providers in that case one is ocotp and other
is ROM.
DT would look something like this:
/* Provider */
ocotp {
...
soc-id {
reg = ...;
};
};
rom {
...
soc-reg {
reg = ...;
};
};
/* consumer */
consumer {
nvmem-cell = <&soc_id>, <&soc_rev>;
nvmem-cell-names = "soc-id", "soc-revision";
};
--srini
>
> [1] https://lkml.org/lkml/2015/6/5/189
>
> Thanks & Regards,
> Sanchayan Maity.
>
> <snip>
>
^ permalink raw reply
* [PATCH v6 01/12] KVM: add comments for kvm_debug_exit_arch struct
From: Alex Bennée @ 2015-06-19 12:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: Jonathan Corbet, Gleb Natapov, jan.kiszka,
open list:DOCUMENTATION, open list, open list:ABI/API, dahi,
r65777, bp
In-Reply-To: <1434716630-18260-1-git-send-email-alex.bennee@linaro.org>
Bring into line with the comments for the other structures and their
KVM_EXIT_* cases. Also update api.txt to reflect use in kvm_run
documentation.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
Reviewed-by: David Hildenbrand <dahi@linux.vnet.ibm.com>
Reviewed-by: Andrew Jones <drjones@redhat.com>
Acked-by: Christoffer Dall <christoffer.dall@linaro.org>
---
v2
- add comments for other exit types
v3
- s/commentary/comments/
- add rb tags
- update api.txt kvm_run to include KVM_EXIT_DEBUG desc
v4
- sp fixes
- add a-b
---
Documentation/virtual/kvm/api.txt | 4 +++-
include/uapi/linux/kvm.h | 3 +++
2 files changed, 6 insertions(+), 1 deletion(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 9fa2bf8..c34c32d 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -3070,11 +3070,13 @@ data_offset describes where the data is located (KVM_EXIT_IO_OUT) or
where kvm expects application code to place the data for the next
KVM_RUN invocation (KVM_EXIT_IO_IN). Data format is a packed array.
+ /* KVM_EXIT_DEBUG */
struct {
struct kvm_debug_exit_arch arch;
} debug;
-Unused.
+If the exit_reason is KVM_EXIT_DEBUG, then a vcpu is processing a debug event
+for which architecture specific information is returned.
/* KVM_EXIT_MMIO */
struct {
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 4b60056..70ac641 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -237,6 +237,7 @@ struct kvm_run {
__u32 count;
__u64 data_offset; /* relative to kvm_run start */
} io;
+ /* KVM_EXIT_DEBUG */
struct {
struct kvm_debug_exit_arch arch;
} debug;
@@ -285,6 +286,7 @@ struct kvm_run {
__u32 data;
__u8 is_write;
} dcr;
+ /* KVM_EXIT_INTERNAL_ERROR */
struct {
__u32 suberror;
/* Available with KVM_CAP_INTERNAL_ERROR_DATA: */
@@ -295,6 +297,7 @@ struct kvm_run {
struct {
__u64 gprs[32];
} osi;
+ /* KVM_EXIT_PAPR_HCALL */
struct {
__u64 nr;
__u64 ret;
--
2.4.3
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related
* [PATCH v6 10/12] KVM: arm64: guest debug, HW assisted debug support
From: Alex Bennée @ 2015-06-19 12:23 UTC (permalink / raw)
To: kvm, linux-arm-kernel, kvmarm, christoffer.dall, marc.zyngier,
peter.maydell, agraf, drjones, pbonzini, zhichao.huang
Cc: Lorenzo Pieralisi, Russell King, Jonathan Corbet, Gleb Natapov,
jan.kiszka, open list:DOCUMENTATION, Will Deacon, open list,
open list:ABI/API, dahi, Peter Zijlstra, Catalin Marinas, r65777,
bp, Ingo Molnar
In-Reply-To: <1434716630-18260-1-git-send-email-alex.bennee@linaro.org>
This adds support for userspace to control the HW debug registers for
guest debug. In the debug ioctl we copy the IMPDEF defined number of
registers into a new register set called host_debug_state. There is now
a new vcpu parameter called debug_ptr which selects which register set
is to copied into the real registers when world switch occurs.
I've moved some helper functions into the hw_breakpoint.h header for
re-use.
As with single step we need to tweak the guest registers to enable the
exceptions so we need to save and restore those bits.
Two new capabilities have been added to the KVM_EXTENSION ioctl to allow
userspace to query the number of hardware break and watch points
available on the host hardware.
Signed-off-by: Alex Bennée <alex.bennee@linaro.org>
---
v2
- switched to C setup
- replace host debug registers directly into context
- minor tweak to api docs
- setup right register for debug
- add FAR_EL2 to debug exit structure
- add support for trapping debug register access
v3
- remove stray trace statement
- fix spacing around operators (various)
- clean-up usage of trap_debug
- introduce debug_ptr, replace excessive memcpy stuff
- don't use memcpy in ioctl, just assign
- update cap ioctl documentation
- reword a number comments
- rename host_debug_state->external_debug_state
v4
- use the new u32/u64 split debug_ptr approach
- fix some wording/comments
v5
- don't set MDSCR_EL1.KDE (not needed)
v6
- update wording given change in commentary
- KVM_GUESTDBG_USE_HW_BP->KVM_GUESTDBG_USE_HW
---
Documentation/virtual/kvm/api.txt | 7 ++++++-
arch/arm/kvm/arm.c | 7 +++++++
arch/arm64/include/asm/hw_breakpoint.h | 12 +++++++++++
arch/arm64/include/asm/kvm_host.h | 6 +++++-
arch/arm64/kernel/hw_breakpoint.c | 12 -----------
arch/arm64/kvm/debug.c | 37 +++++++++++++++++++++++++++++-----
arch/arm64/kvm/handle_exit.c | 6 ++++++
arch/arm64/kvm/reset.c | 12 +++++++++++
include/uapi/linux/kvm.h | 2 ++
9 files changed, 82 insertions(+), 19 deletions(-)
diff --git a/Documentation/virtual/kvm/api.txt b/Documentation/virtual/kvm/api.txt
index 33c8143..ada57df 100644
--- a/Documentation/virtual/kvm/api.txt
+++ b/Documentation/virtual/kvm/api.txt
@@ -2668,7 +2668,7 @@ The top 16 bits of the control field are architecture specific control
flags which can include the following:
- KVM_GUESTDBG_USE_SW_BP: using software breakpoints [x86, arm64]
- - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390]
+ - KVM_GUESTDBG_USE_HW_BP: using hardware breakpoints [x86, s390, arm64]
- KVM_GUESTDBG_INJECT_DB: inject DB type exception [x86]
- KVM_GUESTDBG_INJECT_BP: inject BP type exception [x86]
- KVM_GUESTDBG_EXIT_PENDING: trigger an immediate guest exit [s390]
@@ -2683,6 +2683,11 @@ updated to the correct (supplied) values.
The second part of the structure is architecture specific and
typically contains a set of debug registers.
+For arm64 the number of debug registers is implementation defined and
+can be determined by querying the KVM_CAP_GUEST_DEBUG_HW_BPS and
+KVM_CAP_GUEST_DEBUG_HW_WPS capabilities which return a positive number
+indicating the number of supported registers.
+
When debug events exit the main run loop with the reason
KVM_EXIT_DEBUG with the kvm_debug_exit_arch part of the kvm_run
structure containing architecture specific debug information.
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index 0d17c7b..60c4045 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -307,6 +307,7 @@ void kvm_arch_vcpu_put(struct kvm_vcpu *vcpu)
#define KVM_GUESTDBG_VALID_MASK (KVM_GUESTDBG_ENABLE | \
KVM_GUESTDBG_USE_SW_BP | \
+ KVM_GUESTDBG_USE_HW | \
KVM_GUESTDBG_SINGLESTEP)
/**
@@ -327,6 +328,12 @@ int kvm_arch_vcpu_ioctl_set_guest_debug(struct kvm_vcpu *vcpu,
if (dbg->control & KVM_GUESTDBG_ENABLE) {
vcpu->guest_debug = dbg->control;
+
+ /* Hardware assisted Break and Watch points */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW) {
+ vcpu->arch.external_debug_state = dbg->arch;
+ }
+
} else {
/* If not enabled clear all flags */
vcpu->guest_debug = 0;
diff --git a/arch/arm64/include/asm/hw_breakpoint.h b/arch/arm64/include/asm/hw_breakpoint.h
index 52b484b..c450552 100644
--- a/arch/arm64/include/asm/hw_breakpoint.h
+++ b/arch/arm64/include/asm/hw_breakpoint.h
@@ -130,6 +130,18 @@ static inline void ptrace_hw_copy_thread(struct task_struct *task)
}
#endif
+/* Determine number of BRP registers available. */
+static inline int get_num_brps(void)
+{
+ return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
+}
+
+/* Determine number of WRP registers available. */
+static inline int get_num_wrps(void)
+{
+ return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
+}
+
extern struct pmu perf_ops_bp;
#endif /* __KERNEL__ */
diff --git a/arch/arm64/include/asm/kvm_host.h b/arch/arm64/include/asm/kvm_host.h
index 9697daf..0a3ee7b 100644
--- a/arch/arm64/include/asm/kvm_host.h
+++ b/arch/arm64/include/asm/kvm_host.h
@@ -116,13 +116,17 @@ struct kvm_vcpu_arch {
* debugging the guest from the host and to maintain separate host and
* guest state during world switches. vcpu_debug_state are the debug
* registers of the vcpu as the guest sees them. host_debug_state are
- * the host registers which are saved and restored during world switches.
+ * the host registers which are saved and restored during
+ * world switches. external_debug_state contains the debug
+ * values we want to debugging the guest. This is set via the
+ * KVM_SET_GUEST_DEBUG ioctl.
*
* debug_ptr points to the set of debug registers that should be loaded
* onto the hardware when running the guest.
*/
struct kvm_guest_debug_arch *debug_ptr;
struct kvm_guest_debug_arch vcpu_debug_state;
+ struct kvm_guest_debug_arch external_debug_state;
/* Pointer to host CPU context */
kvm_cpu_context_t *host_cpu_context;
diff --git a/arch/arm64/kernel/hw_breakpoint.c b/arch/arm64/kernel/hw_breakpoint.c
index e7d934d..3a41bbf 100644
--- a/arch/arm64/kernel/hw_breakpoint.c
+++ b/arch/arm64/kernel/hw_breakpoint.c
@@ -49,18 +49,6 @@ static DEFINE_PER_CPU(int, stepping_kernel_bp);
static int core_num_brps;
static int core_num_wrps;
-/* Determine number of BRP registers available. */
-static int get_num_brps(void)
-{
- return ((read_cpuid(ID_AA64DFR0_EL1) >> 12) & 0xf) + 1;
-}
-
-/* Determine number of WRP registers available. */
-static int get_num_wrps(void)
-{
- return ((read_cpuid(ID_AA64DFR0_EL1) >> 20) & 0xf) + 1;
-}
-
int hw_breakpoint_slots(int type)
{
/*
diff --git a/arch/arm64/kvm/debug.c b/arch/arm64/kvm/debug.c
index d439eb8..b287bbc 100644
--- a/arch/arm64/kvm/debug.c
+++ b/arch/arm64/kvm/debug.c
@@ -96,10 +96,6 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
MDCR_EL2_TDRA |
MDCR_EL2_TDOSA);
- /* Trap on access to debug registers? */
- if (trap_debug)
- vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
-
/* Is Guest debugging in effect? */
if (vcpu->guest_debug) {
/* Route all software debug exceptions to EL2 */
@@ -134,11 +130,42 @@ void kvm_arm_setup_debug(struct kvm_vcpu *vcpu)
} else {
vcpu_sys_reg(vcpu, MDSCR_EL1) &= ~DBG_MDSCR_SS;
}
+
+ /*
+ * HW Breakpoints and watchpoints
+ *
+ * We simply switch the debug_ptr to point to our new
+ * external_debug_state which has been populated by the
+ * debug ioctl. The existing KVM_ARM64_DEBUG_DIRTY
+ * mechanism ensures the registers are updated on the
+ * world switch.
+ */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW) {
+ /* Enable breakpoints/watchpoints */
+ vcpu_sys_reg(vcpu, MDSCR_EL1) |= DBG_MDSCR_MDE;
+
+ vcpu->arch.debug_ptr = &vcpu->arch.external_debug_state;
+ vcpu->arch.debug_flags |= KVM_ARM64_DEBUG_DIRTY;
+ trap_debug = true;
+ }
}
+
+ /* Trap debug register access */
+ if (trap_debug)
+ vcpu->arch.mdcr_el2 |= MDCR_EL2_TDA;
}
void kvm_arm_clear_debug(struct kvm_vcpu *vcpu)
{
- if (vcpu->guest_debug)
+ if (vcpu->guest_debug) {
restore_guest_debug_regs(vcpu);
+
+ /*
+ * If we were using HW debug we need to restore the
+ * debug_ptr to the guest debug state.
+ */
+ if (vcpu->guest_debug & KVM_GUESTDBG_USE_HW)
+ vcpu->arch.debug_ptr = &vcpu->arch.vcpu_debug_state;
+
+ }
}
diff --git a/arch/arm64/kvm/handle_exit.c b/arch/arm64/kvm/handle_exit.c
index e9de13e..68a0759 100644
--- a/arch/arm64/kvm/handle_exit.c
+++ b/arch/arm64/kvm/handle_exit.c
@@ -103,7 +103,11 @@ static int kvm_handle_guest_debug(struct kvm_vcpu *vcpu, struct kvm_run *run)
run->debug.arch.hsr = hsr;
switch (hsr >> ESR_ELx_EC_SHIFT) {
+ case ESR_ELx_EC_WATCHPT_LOW:
+ run->debug.arch.far = vcpu->arch.fault.far_el2;
+ /* fall through */
case ESR_ELx_EC_SOFTSTP_LOW:
+ case ESR_ELx_EC_BREAKPT_LOW:
case ESR_ELx_EC_BKPT32:
case ESR_ELx_EC_BRK64:
break;
@@ -132,6 +136,8 @@ static exit_handle_fn arm_exit_handlers[] = {
[ESR_ELx_EC_IABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_DABT_LOW] = kvm_handle_guest_abort,
[ESR_ELx_EC_SOFTSTP_LOW]= kvm_handle_guest_debug,
+ [ESR_ELx_EC_WATCHPT_LOW]= kvm_handle_guest_debug,
+ [ESR_ELx_EC_BREAKPT_LOW]= kvm_handle_guest_debug,
[ESR_ELx_EC_BKPT32] = kvm_handle_guest_debug,
[ESR_ELx_EC_BRK64] = kvm_handle_guest_debug,
};
diff --git a/arch/arm64/kvm/reset.c b/arch/arm64/kvm/reset.c
index 0b43265..21d5a62 100644
--- a/arch/arm64/kvm/reset.c
+++ b/arch/arm64/kvm/reset.c
@@ -56,6 +56,12 @@ static bool cpu_has_32bit_el1(void)
return !!(pfr0 & 0x20);
}
+/**
+ * kvm_arch_dev_ioctl_check_extension
+ *
+ * We currently assume that the number of HW registers is uniform
+ * across all CPUs (see cpuinfo_sanity_check).
+ */
int kvm_arch_dev_ioctl_check_extension(long ext)
{
int r;
@@ -64,6 +70,12 @@ int kvm_arch_dev_ioctl_check_extension(long ext)
case KVM_CAP_ARM_EL1_32BIT:
r = cpu_has_32bit_el1();
break;
+ case KVM_CAP_GUEST_DEBUG_HW_BPS:
+ r = get_num_brps();
+ break;
+ case KVM_CAP_GUEST_DEBUG_HW_WPS:
+ r = get_num_wrps();
+ break;
default:
r = 0;
}
diff --git a/include/uapi/linux/kvm.h b/include/uapi/linux/kvm.h
index 70ac641..f020dd0 100644
--- a/include/uapi/linux/kvm.h
+++ b/include/uapi/linux/kvm.h
@@ -817,6 +817,8 @@ struct kvm_ppc_smmu_info {
#define KVM_CAP_S390_INJECT_IRQ 113
#define KVM_CAP_S390_IRQ_STATE 114
#define KVM_CAP_PPC_HWRNG 115
+#define KVM_CAP_GUEST_DEBUG_HW_BPS 116
+#define KVM_CAP_GUEST_DEBUG_HW_WPS 117
#ifdef KVM_CAP_IRQ_ROUTING
--
2.4.3
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
^ permalink raw reply related
* Re: [RESEND PATCH V2 1/3] Add mmap flag to request pages are locked after page fault
From: Michal Hocko @ 2015-06-19 14:57 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: <20150618203048.GB2329@akamai.com>
On Thu 18-06-15 16:30:48, Eric B Munson wrote:
> On Thu, 18 Jun 2015, Michal Hocko wrote:
[...]
> > 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.
MAP_FAULTPOPULATE would be usable for other cases as well. E.g. fault
around is all or nothing feature. Either all mappings (which support
this) fault around or none. There is no way to tell the kernel that
this particular mapping shouldn't fault around. I haven't seen such a
request yet but we have seen requests to have a way to opt out from
a global policy in the past (e.g. per-process opt out from THP). So
I can imagine somebody will come with a request to opt out from any
speculative operations on the mapped area in the future.
> 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.
I am not sure I understand your point here. Could you be more specific
how would you check for that and what for?
>From my understanding MAP_LOCKONFAULT is essentially
MAP_FAULTPOPULATE|MAP_LOCKED with a quite obvious semantic (unlike
single MAP_LOCKED unfortunately). I would love to also have
MAP_LOCKED|MAP_POPULATE (aka full mlock semantic) but I am really
skeptical considering how my previous attempt to make MAP_POPULATE
reasonable went.
> 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.
mlock with flags sounds like a good step but I am not sure it will make
sense in the future. POSIX has screwed that and I am not sure how many
applications would use it. This ship has sailed long time ago.
> That way we give users the
> ability to request VM_LOCKONFAULT for memory allocated using something
> other than mmap.
mmap(MAP_FAULTPOPULATE); mlock() would have the same semantic even
without changing mlock syscall.
> > > 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
> > > ---
[...]
--
Michal Hocko
SUSE Labs
^ 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-19 16:43 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: <20150619145708.GG4913@dhcp22.suse.cz>
[-- Attachment #1: Type: text/plain, Size: 4808 bytes --]
On Fri, 19 Jun 2015, Michal Hocko wrote:
> On Thu 18-06-15 16:30:48, Eric B Munson wrote:
> > On Thu, 18 Jun 2015, Michal Hocko wrote:
> [...]
> > > 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.
>
> MAP_FAULTPOPULATE would be usable for other cases as well. E.g. fault
> around is all or nothing feature. Either all mappings (which support
> this) fault around or none. There is no way to tell the kernel that
> this particular mapping shouldn't fault around. I haven't seen such a
> request yet but we have seen requests to have a way to opt out from
> a global policy in the past (e.g. per-process opt out from THP). So
> I can imagine somebody will come with a request to opt out from any
> speculative operations on the mapped area in the future.
>
> > 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.
>
> I am not sure I understand your point here. Could you be more specific
> how would you check for that and what for?
My thought on detecting was that someone might want to know if they had
a VMA that was VM_LOCKED but had not been made present becuase of a
failure in mmap. We don't have a way today, but adding VM_LOCKONFAULT
is at least explicit about what is happening which would make detecting
the VM_LOCKED but not present state easier. This assumes that
MAP_FAULTPOPULATE does not translate to a VMA flag, but it sounds like
it would have to.
>
> From my understanding MAP_LOCKONFAULT is essentially
> MAP_FAULTPOPULATE|MAP_LOCKED with a quite obvious semantic (unlike
> single MAP_LOCKED unfortunately). I would love to also have
> MAP_LOCKED|MAP_POPULATE (aka full mlock semantic) but I am really
> skeptical considering how my previous attempt to make MAP_POPULATE
> reasonable went.
Are you objecting to the addition of the VMA flag VM_LOCKONFAULT, or the
new MAP_LOCKONFAULT flag (or both)? If you prefer that MAP_LOCKED |
MAP_FAULTPOPULATE means that VM_LOCKONFAULT is set, I am fine with that
instead of introducing MAP_LOCKONFAULT. I went with the new flag
because to date, we have a one to one mapping of MAP_* to VM_* flags.
>
> > 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.
>
> mlock with flags sounds like a good step but I am not sure it will make
> sense in the future. POSIX has screwed that and I am not sure how many
> applications would use it. This ship has sailed long time ago.
I don't know either, but the code is the question, right? I know that
we have at least one team that wants it here.
>
> > That way we give users the
> > ability to request VM_LOCKONFAULT for memory allocated using something
> > other than mmap.
>
> mmap(MAP_FAULTPOPULATE); mlock() would have the same semantic even
> without changing mlock syscall.
That is true as long as MAP_FAULTPOPULATE set a flag in the VMA(s). It
doesn't cover the actual case I was asking about, which is how do I get
lock on fault on malloc'd memory?
>
> > > > 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
> > > > ---
> [...]
> --
> Michal Hocko
> SUSE Labs
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC v3 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-06-19 17:28 UTC (permalink / raw)
To: Dave Chinner
Cc: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-fsdevel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, greg-U8xfFu+wG4EAvxtiuMwx3w,
jack-AlSwsSmVLrQ, tytso-3s7WtUTddSA,
adilger.kernel-m1MBpc4rdrD3fQ9qLvQP4Q,
hughd-hpIqsD4AKlfQT0dZR+AlfA, lczerner-H+wXaHxf7aLQT0dZR+AlfA,
hch-wEGCiKHe2LqWVfeAwA7xHQ, linux-ext4-u79uwXL29TY76Z2rM5mHXA,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
kyungmin.park-Sze3O3UU22JBDgjK7y7TUQ,
kmpark-wEGCiKHe2LqWVfeAwA7xHQ
In-Reply-To: <20150619000341.GM10224@dastard>
On 06/19/2015 02:03 AM, Dave Chinner wrote:
> On Thu, Jun 18, 2015 at 10:25:08AM +0200, Beata Michalska wrote:
>> 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-Sze3O3UU22JBDgjK7y7TUQ@public.gmane.org>
>>>
>>> This has massive scalability problems:
> ....
>>> 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.
> ....
>>> 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.
>>
>> 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.
>
> I was hoping that you'd think a little more about my suggestion and
> work out how to do background threshold event detection generically.
> I kind of left it as "an exercise for the reader" because it seems
> obvious to me.
>
> Hint: ->statfs allows you to get the total, free and used space
> from filesystems in a generic manner.
>
> Cheers,
>
> Dave.
>
I haven't given up on that, so yes, I'm still working on a more suitable
generic solution.
Background detection is one of the options, though it needs some more thoughts.
Giving up the sync approach means less accuracy for the threshold notifications,
but I guess this could be fine-tuned to get an acceptable level. Another bump:
how this tuning is supposed to be done (additional config option maybe)?
The interface would have to keep it somehow sane - but what would 'sane' mean
in this case (?) Also, I'm not sure whether single approach would server here
well for all the potentially supported file systems so this would have to be
properly adjusted (taking the threshold levels into consideration as well).
And still,it would require some form of synchronization with tracked fs so that
this 'detection' is not being unnecessarily performed (i.e. while fs remains frozen).
There is also an idea of using an interface resembling the stackable fs:
a transparent file system layered on top of the tracked one
(solely for the tracking purposes). This would simplify handling the trace
object's lifetime - no more list of registered traces.
It would also give a way of tracking (to some extent) the changes in the amount
of available space, which combined with tweaked background check could give
a solution with less performance overhead than the original one.
I'll try this one and see how it goes.
Thank You for your feedback so far - I really appreciate it.
Best Regards
Beata
^ permalink raw reply
* [PATCH] selftests: add quicktest support
From: Shuah Khan @ 2015-06-19 19:55 UTC (permalink / raw)
To: linux-api, linux-kernel; +Cc: Shuah Khan
Add quicktest support to enable users to choose to run
tests that complete in a short time. Choosing this option
excludes tests that take longer time complete e.g: timers.
User can specify quicktest option from kernel top level or
selftests directory.
Kernel top level directory:
make quicktest=1 kselftest
tools/testing/selftests directory:
make quicktest=1 run_tests
Signed-off-by: Shuah Khan <shuahkh@osg.samsung.com>
---
tools/testing/selftests/Makefile | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/tools/testing/selftests/Makefile b/tools/testing/selftests/Makefile
index 7474b7d9..24ae9e8 100644
--- a/tools/testing/selftests/Makefile
+++ b/tools/testing/selftests/Makefile
@@ -16,11 +16,15 @@ TARGETS += ptrace
TARGETS += seccomp
TARGETS += size
TARGETS += sysctl
+ifneq (1, $(quicktest))
TARGETS += timers
+endif
TARGETS += user
TARGETS += vm
TARGETS += x86
#Please keep the TARGETS list alphabetically sorted
+# Run "make quicktest=1 run_tests" or
+# "make quicktest=1 kselftest from top level Makefile
TARGETS_HOTPLUG = cpu-hotplug
TARGETS_HOTPLUG += memory-hotplug
--
2.1.4
^ permalink raw reply related
* Re: [RFC v3 1/4] fs: Add generic file system event notifications
From: Dave Chinner @ 2015-06-19 23:21 UTC (permalink / raw)
To: Beata Michalska
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: <5584512B.5020301@samsung.com>
On Fri, Jun 19, 2015 at 07:28:11PM +0200, Beata Michalska wrote:
> On 06/19/2015 02:03 AM, Dave Chinner wrote:
> > On Thu, Jun 18, 2015 at 10:25:08AM +0200, Beata Michalska wrote:
> >> 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:
> > ....
> >>> 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.
> > ....
> >>> 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.
> >>
> >> 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.
> >
> > I was hoping that you'd think a little more about my suggestion and
> > work out how to do background threshold event detection generically.
> > I kind of left it as "an exercise for the reader" because it seems
> > obvious to me.
> >
> > Hint: ->statfs allows you to get the total, free and used space
> > from filesystems in a generic manner.
> >
> > Cheers,
> >
> > Dave.
> >
>
> I haven't given up on that, so yes, I'm still working on a more suitable
> generic solution.
> Background detection is one of the options, though it needs some more thoughts.
> Giving up the sync approach means less accuracy for the threshold notifications,
> but I guess this could be fine-tuned to get an acceptable level.
Accuracy really doesn't matter for threshold notifications - by the
time the event is delivered to userspace it can already be wrong.
> Another bump:
> how this tuning is supposed to be done (additional config option maybe)?
Why would you need to tune it at all? You can't *stop* the operation
that is triggering the threshold, so a few seconds delay on delivery
isn't going to make any difference to anyone....
You're overthinking this massively. All this needs is a work item
per superblock, and when the thresholds are turned on it queues a
self-repeating delayed work that calls ->statfs, checks against the
configured threshold, issues an event if necessary, and then queues
itself again to run next period. When the threshold is turned off,
the work is cancelled.
Another option: a kernel thread that runs periodically and just
calls iterate_supers() with a function that checks the sb for
threshold events, and if configured runs ->statfs and does the work,
otherwise skips the sb. That avoids all the lifetime issues with
using workqueues, you don't need a struct work, etc.
> There is also an idea of using an interface resembling the stackable fs:
No. Just .... No.
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
--
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
* [PATCH] Doc:ABI/testing: Fix typo in sysfs-bus-fcoe
From: Masanari Iida @ 2015-06-20 2:17 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA, vasu.dev-ral2JQCrhuEAvxtiuMwx3w,
corbet-T1hC0tSOHrs, linux-doc-u79uwXL29TY76Z2rM5mHXA,
fcoe-devel-s9riP+hp16TNLxjTenLetw
Cc: Masanari Iida
This patch fix some spelling typo in sysfs-bus-fcoe
Signed-off-by: Masanari Iida <standby24x7-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
Documentation/ABI/testing/sysfs-bus-fcoe | 12 ++++++------
1 file changed, 6 insertions(+), 6 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-bus-fcoe b/Documentation/ABI/testing/sysfs-bus-fcoe
index 21640ea..657df13 100644
--- a/Documentation/ABI/testing/sysfs-bus-fcoe
+++ b/Documentation/ABI/testing/sysfs-bus-fcoe
@@ -32,7 +32,7 @@ Description: 'FCoE Controller' instances on the fcoe bus.
Attributes:
- fcf_dev_loss_tmo: Device loss timeout peroid (see below). Changing
+ fcf_dev_loss_tmo: Device loss timeout period (see below). Changing
this value will change the dev_loss_tmo for all
FCFs discovered by this controller.
@@ -61,7 +61,7 @@ Attributes:
lesb/err_block: Link Error Status Block (LESB) block error count.
lesb/fcs_error: Link Error Status Block (LESB) Fibre Channel
- Serivces error count.
+ Services error count.
Notes: ctlr_X (global increment starting at 0)
@@ -85,7 +85,7 @@ Attributes:
fabric.
selected: 1 indicates that the switch has been selected for use;
- 0 indicates that the swich will not be used.
+ 0 indicates that the switch will not be used.
fc_map: The Fibre Channel MAP
@@ -93,7 +93,7 @@ Attributes:
mac: The FCF's MAC address
- fka_peroid: The FIP Keep-Alive peroid
+ fka_period: The FIP Keep-Alive period
fabric_state: The internal kernel state
"Unknown" - Initialization value
@@ -101,9 +101,9 @@ Attributes:
"Connected" - Host is connected to the FCF
"Deleted" - FCF is being removed from the system
- dev_loss_tmo: The device loss timeout peroid for this FCF.
+ dev_loss_tmo: The device loss timeout period for this FCF.
-Notes: A device loss infrastructre similar to the FC Transport's
+Notes: A device loss infrastructure similar to the FC Transport's
is present in fcoe_sysfs. It is nice to have so that a
link flapping adapter doesn't continually advance the count
used to identify the discovered FCF. FCFs will exist in a
--
2.4.4.408.g16da57c
^ permalink raw reply related
* Re: [PATCH v7 00/16] libnvdimm: non-volatile memory devices
From: Stephen Rothwell @ 2015-06-20 12:40 UTC (permalink / raw)
To: Dan Williams
Cc: linux-nvdimm-hn68Rpc1hR1g9hUCZPvPmw, Boaz Harrosh,
rafael-DgEjT+Ai2ygdnm+yROfE0A, neilb-l3A5Bk7waGM, Lv Zheng,
H. Peter Anvin, mingo-DgEjT+Ai2ygdnm+yROfE0A, Rafael J. Wysocki,
Robert Moore, hch-jcswGhMUV9g, linux-acpi-u79uwXL29TY76Z2rM5mHXA,
jmoyer-H+wXaHxf7aLQT0dZR+AlfA, Nicholas Moulin, Matthew Wilcox,
Ross Zwisler, Jens Axboe, Borislav Petkov, Thomas Gleixner,
axboe-tSWWG44O7X1aa/9Udqfwiw,
gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
linux-kernel-u79uwXL29TY76Z2rM5mHXA, Andy Lutomirski,
linux-api-u79uwXL29TY76Z2rM5mHXA,
akpm-de/tnXTf+JLsfHDXvbKv3WD2FQJk+8+b, Linus Torvalds
In-Reply-To: <20150617215027.11597.61414.stgit-p8uTFz9XbKj2zm6wflaqv1nYeNYlB/vhral2JQCrhuEAvxtiuMwx3w@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 1227 bytes --]
Hi Dan,
On Wed, 17 Jun 2015 19:13:19 -0400 Dan Williams <dan.j.williams-ral2JQCrhuEAvxtiuMwx3w@public.gmane.org> wrote:
>
> A new sub-system in support of non-volatile memory storage devices.
>
> Stephen, please add libnvdimm-for-next to -next:
>
> git://git.kernel.org/pub/scm/linux/kernel/git/djbw/nvdimm libnvdimm-for-next
Thanks for adding your subsystem tree as a participant of linux-next. As
you may know, this is not a judgment of your code. The purpose of
linux-next is for integration testing and to lower the impact of
conflicts between subsystems in the next merge window.
You will need to ensure that the patches/commits in your tree/series have
been:
* submitted under GPL v2 (or later) and include the Contributor's
Signed-off-by,
* posted to the relevant mailing list,
* reviewed by you (or another maintainer of your subsystem tree),
* successfully unit tested, and
* destined for the current or next Linux merge window.
Basically, this should be just what you would send to Linus (or ask him
to fetch). It is allowed to be rebased if you deem it necessary.
--
Cheers,
Stephen Rothwell
sfr-3FnU+UHB4dNDw9hX6IcOSA@public.gmane.org
[-- Attachment #2: OpenPGP 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: Peter Nørlund @ 2015-06-20 18:42 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev-u79uwXL29TY76Z2rM5mHXA, David S. Miller, Alexey Kuznetsov,
James Morris, Hideaki YOSHIFUJI, Patrick McHardy,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <55831F0D.3040703-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org>
On Thu, 18 Jun 2015 12:42:05 -0700
Alexander Duyck <alexander.h.duyck-H+wXaHxf7aLQT0dZR+AlfA@public.gmane.org> wrote:
> 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.
>
Good point. I'll skip the renaming.
> > 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.
>
Right.
> > +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?
>
Yes. Although I think it is rare for the weight to be that large, the
API supports it, so I'd better make sure nothing weird happens. Or is
it too hypothetical? Today, if one were to add two paths, each with a
weight above 2^30, the total weight would overflow and become negative.
fib_select_multipath would then think the route is dead and always
select the first path.
> > + 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?
>
Yes - and -1 is used to ensure that dead paths are never selected.
> 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.
>
It is true that fib_nhs is an int, but nh_sel is an unsigned char. So
although you could technically have more than 256 paths, fib_result
would have to be modified to actually support more paths.
> 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.
>
You are right, it is superfluous. If all paths are dead, we never
actually use the factor for anything.
> 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.
Got it.
>
> > @@ -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.
The old algorithm tried to be kinda random by using jiffies, but since
jiffies has a relatively low resolution (at best 1kHz), at lot of
packets in a row would get the same jiffies value. The code did try to
adhere to the weight ratios and as a consequence it really worked like
round robin (except at every tick when the jiffies variable would
increment and offset the round robin slightly).
Considering that, I thought I'd avoid any false pretenses and just do
round robin. But since the new algorithm relies on thresholds and not
modulo, the most significant bits are the ones that need to flip at
every packet. So by reversing the bit order, I get exactly that. At a
slow packet rate this is somewhat costly as the bitrev8 lookup table
must be fetched from L3 cache or even memory, but as the packet rate
increase, the likelihood of it being in L2 or L1 cache increases and
cost go down.
Regarding moving from u8 to larger integers, I chose 8-bit resolution
since hashing L3 and L4 headers with XOR into a single byte actually
produced fairly good results with 8 bits, while switching to 16 bits
was terrible. Considering the purpose of the round robin counter here,
the primary advantage of switching to higher resolution would be to
better support a total weight above 255.
^ permalink raw reply
* Re: [PATCH net-next 2/3] ipv4: L3 and L4 hash-based multipath routing
From: Peter Nørlund @ 2015-06-20 18:46 UTC (permalink / raw)
To: Alexander Duyck
Cc: netdev, David S. Miller, Alexey Kuznetsov, James Morris,
Hideaki YOSHIFUJI, Patrick McHardy, linux-api
In-Reply-To: <55834BA6.7050405@redhat.com>
On Thu, 18 Jun 2015 15:52:22 -0700
Alexander Duyck <alexander.h.duyck@redhat.com> wrote:
>
>
> 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.
>
When I begun to work on the multipath code, I took a look at what the
hardware routers did, and found that you could typically choose between
XOR, CRC16, and CRC32. CRC32 produces excellent hashes but is slow
(unless you take advantage of SSE 4.2), while XOR is fast but less
optimal. When I tested the XOR hash on the routers of ordbogen.com (my
work place), I was actually surprised to see that the XOR hash was
sufficient as long as I stuck with 8 bits. But you are probably right.
While HTTP-traffic will likely balance well enough with XOR, other kind
of traffic might not. I didn't know the Jenkins hash function, so I
haven't tested it, but by the looks of it, I too think it is a good
candidate. It also adds the possibility of extending the key space to
16 bits or more.
> > +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.
>
Just like the hash algorithms, I looked at the hardware routers for
inspiration, and they typically do like this. It is a known problem,
and it can just as well happen with TCP, so I don't think adding a
special case for UDP is the way to go. But what about looking at the
don't-fragment-bit? If the DF-bit is set on a packet, it is usually set
on all packets in that flow, so one can assume that that flow will
never fragment. If it is clear, you can't be sure and should probably
avoid L4 hashing altogether. The interesting question is how many flows
will be excluded from L4 hashing with this approach.
> > + 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.
>
Definably not a bad idea. It would remove unnecessary overhead when
running per-packet multipath.
> > #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: Michal Hocko @ 2015-06-22 12:38 UTC (permalink / raw)
To: Eric B Munson
Cc: Andrew Morton, linux-alpha-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-parisc-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
sparclinux-u79uwXL29TY76Z2rM5mHXA,
linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150619164333.GD2329-JqFfY2XvxFXQT0dZR+AlfA@public.gmane.org>
On Fri 19-06-15 12:43:33, Eric B Munson wrote:
> On Fri, 19 Jun 2015, Michal Hocko wrote:
>
> > On Thu 18-06-15 16:30:48, Eric B Munson wrote:
> > > On Thu, 18 Jun 2015, Michal Hocko wrote:
> > [...]
> > > > 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.
> >
> > MAP_FAULTPOPULATE would be usable for other cases as well. E.g. fault
> > around is all or nothing feature. Either all mappings (which support
> > this) fault around or none. There is no way to tell the kernel that
> > this particular mapping shouldn't fault around. I haven't seen such a
> > request yet but we have seen requests to have a way to opt out from
> > a global policy in the past (e.g. per-process opt out from THP). So
> > I can imagine somebody will come with a request to opt out from any
> > speculative operations on the mapped area in the future.
> >
> > > 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.
> >
> > I am not sure I understand your point here. Could you be more specific
> > how would you check for that and what for?
>
> My thought on detecting was that someone might want to know if they had
> a VMA that was VM_LOCKED but had not been made present becuase of a
> failure in mmap. We don't have a way today, but adding VM_LOCKONFAULT
> is at least explicit about what is happening which would make detecting
> the VM_LOCKED but not present state easier.
One could use /proc/<pid>/pagemap to query the residency.
> This assumes that
> MAP_FAULTPOPULATE does not translate to a VMA flag, but it sounds like
> it would have to.
Yes, it would have to have a VM flag for the vma.
> > From my understanding MAP_LOCKONFAULT is essentially
> > MAP_FAULTPOPULATE|MAP_LOCKED with a quite obvious semantic (unlike
> > single MAP_LOCKED unfortunately). I would love to also have
> > MAP_LOCKED|MAP_POPULATE (aka full mlock semantic) but I am really
> > skeptical considering how my previous attempt to make MAP_POPULATE
> > reasonable went.
>
> Are you objecting to the addition of the VMA flag VM_LOCKONFAULT, or the
> new MAP_LOCKONFAULT flag (or both)?
I thought the MAP_FAULTPOPULATE (or any other better name) would
directly translate into VM_FAULTPOPULATE and wouldn't be tight to the
locked semantic. We already have VM_LOCKED for that. The direct effect
of the flag would be to prevent from population other than the direct
page fault - including any speculative actions like fault around or
read-ahead.
> If you prefer that MAP_LOCKED |
> MAP_FAULTPOPULATE means that VM_LOCKONFAULT is set, I am fine with that
> instead of introducing MAP_LOCKONFAULT. I went with the new flag
> because to date, we have a one to one mapping of MAP_* to VM_* flags.
>
> >
> > > 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.
> >
> > mlock with flags sounds like a good step but I am not sure it will make
> > sense in the future. POSIX has screwed that and I am not sure how many
> > applications would use it. This ship has sailed long time ago.
>
> I don't know either, but the code is the question, right? I know that
> we have at least one team that wants it here.
>
> >
> > > That way we give users the
> > > ability to request VM_LOCKONFAULT for memory allocated using something
> > > other than mmap.
> >
> > mmap(MAP_FAULTPOPULATE); mlock() would have the same semantic even
> > without changing mlock syscall.
>
> That is true as long as MAP_FAULTPOPULATE set a flag in the VMA(s). It
> doesn't cover the actual case I was asking about, which is how do I get
> lock on fault on malloc'd memory?
OK I see your point now. We would indeed need a flag argument for mlock.
--
Michal Hocko
SUSE Labs
^ 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-22 14:18 UTC (permalink / raw)
To: Michal Hocko
Cc: Andrew Morton, linux-alpha-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
linux-mips-6z/3iImG2C8G8FEW9MqTrA,
linux-parisc-u79uwXL29TY76Z2rM5mHXA,
linuxppc-dev-uLR06cmDAlY/bJ5BZ2RsiQ,
sparclinux-u79uwXL29TY76Z2rM5mHXA,
linux-xtensa-PjhNF2WwrV/0Sa2dR60CXw,
linux-mm-Bw31MaZKKs3YtjvyW6yDsg,
linux-arch-u79uwXL29TY76Z2rM5mHXA,
linux-api-u79uwXL29TY76Z2rM5mHXA
In-Reply-To: <20150622123826.GF4430-2MMpYkNvuYDjFM9bn6wA6Q@public.gmane.org>
[-- Attachment #1: Type: text/plain, Size: 5698 bytes --]
On Mon, 22 Jun 2015, Michal Hocko wrote:
> On Fri 19-06-15 12:43:33, Eric B Munson wrote:
> > On Fri, 19 Jun 2015, Michal Hocko wrote:
> >
> > > On Thu 18-06-15 16:30:48, Eric B Munson wrote:
> > > > On Thu, 18 Jun 2015, Michal Hocko wrote:
> > > [...]
> > > > > 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.
> > >
> > > MAP_FAULTPOPULATE would be usable for other cases as well. E.g. fault
> > > around is all or nothing feature. Either all mappings (which support
> > > this) fault around or none. There is no way to tell the kernel that
> > > this particular mapping shouldn't fault around. I haven't seen such a
> > > request yet but we have seen requests to have a way to opt out from
> > > a global policy in the past (e.g. per-process opt out from THP). So
> > > I can imagine somebody will come with a request to opt out from any
> > > speculative operations on the mapped area in the future.
> > >
> > > > 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.
> > >
> > > I am not sure I understand your point here. Could you be more specific
> > > how would you check for that and what for?
> >
> > My thought on detecting was that someone might want to know if they had
> > a VMA that was VM_LOCKED but had not been made present becuase of a
> > failure in mmap. We don't have a way today, but adding VM_LOCKONFAULT
> > is at least explicit about what is happening which would make detecting
> > the VM_LOCKED but not present state easier.
>
> One could use /proc/<pid>/pagemap to query the residency.
>
> > This assumes that
> > MAP_FAULTPOPULATE does not translate to a VMA flag, but it sounds like
> > it would have to.
>
> Yes, it would have to have a VM flag for the vma.
>
> > > From my understanding MAP_LOCKONFAULT is essentially
> > > MAP_FAULTPOPULATE|MAP_LOCKED with a quite obvious semantic (unlike
> > > single MAP_LOCKED unfortunately). I would love to also have
> > > MAP_LOCKED|MAP_POPULATE (aka full mlock semantic) but I am really
> > > skeptical considering how my previous attempt to make MAP_POPULATE
> > > reasonable went.
> >
> > Are you objecting to the addition of the VMA flag VM_LOCKONFAULT, or the
> > new MAP_LOCKONFAULT flag (or both)?
>
> I thought the MAP_FAULTPOPULATE (or any other better name) would
> directly translate into VM_FAULTPOPULATE and wouldn't be tight to the
> locked semantic. We already have VM_LOCKED for that. The direct effect
> of the flag would be to prevent from population other than the direct
> page fault - including any speculative actions like fault around or
> read-ahead.
I like the ability to control other speculative population, but I am not
sure about overloading it with the VM_LOCKONFAULT case. Here is my
concern. If we are using VM_FAULTPOPULATE | VM_LOCKED to denote
LOCKONFAULT, how can we tell the difference between someone that wants
to avoid read-ahead and wants to use mlock()? This might lead to some
interesting states with mlock() and munlock() that take flags. For
instance, using VM_LOCKONFAULT mlock(MLOCK_ONFAULT) followed by
munlock(MLOCK_LOCKED) leaves the VMAs in the same state with
VM_LOCKONFAULT set. If we use VM_FAULTPOPULATE, the same pair of calls
would clear VM_LOCKED, but leave VM_FAULTPOPULATE. It may not matter in
the end, but I am concerned about the subtleties here.
>
> > If you prefer that MAP_LOCKED |
> > MAP_FAULTPOPULATE means that VM_LOCKONFAULT is set, I am fine with that
> > instead of introducing MAP_LOCKONFAULT. I went with the new flag
> > because to date, we have a one to one mapping of MAP_* to VM_* flags.
> >
> > >
> > > > 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.
> > >
> > > mlock with flags sounds like a good step but I am not sure it will make
> > > sense in the future. POSIX has screwed that and I am not sure how many
> > > applications would use it. This ship has sailed long time ago.
> >
> > I don't know either, but the code is the question, right? I know that
> > we have at least one team that wants it here.
> >
> > >
> > > > That way we give users the
> > > > ability to request VM_LOCKONFAULT for memory allocated using something
> > > > other than mmap.
> > >
> > > mmap(MAP_FAULTPOPULATE); mlock() would have the same semantic even
> > > without changing mlock syscall.
> >
> > That is true as long as MAP_FAULTPOPULATE set a flag in the VMA(s). It
> > doesn't cover the actual case I was asking about, which is how do I get
> > lock on fault on malloc'd memory?
>
> OK I see your point now. We would indeed need a flag argument for mlock.
> --
> Michal Hocko
> SUSE Labs
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [RFC v3 1/4] fs: Add generic file system event notifications
From: Beata Michalska @ 2015-06-22 15:46 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: <20150619232117.GN10224@dastard>
On 06/20/2015 01:21 AM, Dave Chinner wrote:
> On Fri, Jun 19, 2015 at 07:28:11PM +0200, Beata Michalska wrote:
>> On 06/19/2015 02:03 AM, Dave Chinner wrote:
>>> On Thu, Jun 18, 2015 at 10:25:08AM +0200, Beata Michalska wrote:
>>>> 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:
>>> ....
>>>>> 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.
>>> ....
>>>>> 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.
>>>>
>>>> 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.
>>>
>>> I was hoping that you'd think a little more about my suggestion and
>>> work out how to do background threshold event detection generically.
>>> I kind of left it as "an exercise for the reader" because it seems
>>> obvious to me.
>>>
>>> Hint: ->statfs allows you to get the total, free and used space
>>> from filesystems in a generic manner.
>>>
>>> Cheers,
>>>
>>> Dave.
>>>
>>
>> I haven't given up on that, so yes, I'm still working on a more suitable
>> generic solution.
>> Background detection is one of the options, though it needs some more thoughts.
>> Giving up the sync approach means less accuracy for the threshold notifications,
>> but I guess this could be fine-tuned to get an acceptable level.
>
> Accuracy really doesn't matter for threshold notifications - by the
> time the event is delivered to userspace it can already be wrong.
>
>> Another bump:
>> how this tuning is supposed to be done (additional config option maybe)?
>
> Why would you need to tune it at all? You can't *stop* the operation
> that is triggering the threshold, so a few seconds delay on delivery
> isn't going to make any difference to anyone....
>
> You're overthinking this massively. All this needs is a work item
> per superblock, and when the thresholds are turned on it queues a
> self-repeating delayed work that calls ->statfs, checks against the
> configured threshold, issues an event if necessary, and then queues
> itself again to run next period. When the threshold is turned off,
> the work is cancelled.
>
> Another option: a kernel thread that runs periodically and just
> calls iterate_supers() with a function that checks the sb for
> threshold events, and if configured runs ->statfs and does the work,
> otherwise skips the sb. That avoids all the lifetime issues with
> using workqueues, you don't need a struct work, etc.
>
>> There is also an idea of using an interface resembling the stackable fs:
>
> No. Just .... No.
>
> Cheers,
>
> Dave.
>
Alright, I'll make appropriate changes to move the threshold
verification into the background and see how it works.
Thanks,
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] Doc:ABI/testing: Fix typo in sysfs-bus-fcoe
From: Jonathan Corbet @ 2015-06-22 16:35 UTC (permalink / raw)
To: Masanari Iida; +Cc: linux-kernel, linux-api, vasu.dev, linux-doc, fcoe-devel
In-Reply-To: <1434766640-23799-1-git-send-email-standby24x7@gmail.com>
On Sat, 20 Jun 2015 11:17:20 +0900
Masanari Iida <standby24x7@gmail.com> wrote:
> This patch fix some spelling typo in sysfs-bus-fcoe
Applied to the docs tree, thanks.
jon
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
^ permalink raw reply
* [PATCH v7 0/3] Enable PPI sysfs interface for TPM 2.0
From: Jarkko Sakkinen @ 2015-06-22 17:24 UTC (permalink / raw)
To: tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: peterhuewe-Mmb7MZpHnFY, gregkh-hQyY1W1yCW8ekmWlsbkhG0B+6BGkLq7r,
jgunthorpe-ePGOBjL8dl3ta4EC/59zMFaTQe2KTcn/, Jarkko Sakkinen,
Guenter Roeck, open list:ABI/API, NeilBrown, Tejun Heo
v7:
* Fixed compile error when CONFIG_SYSFS is not enabled.
v6:
* Updated documentation.
v5:
* Removed dangling export of kernfs_remove_by_name_ns() from the sysfs
patch.
v4:
* Use sysfs_remove_link()
v3:
* Fixed to_tpm_chip() macro.
* Split into two patches.
* Renamed sysfs_link_group_to_kobj to sysfs_link_entry_to_kobj
* Only create the "backwards compatibility" symlink for TPM 1.x devices.
Jarkko Sakkinen (3):
sysfs: added sysfs_link_entry_to_kobj()
tpm: move the PPI attributes to character device directory.
tpm: update PPI documentation to address the location change.
Documentation/ABI/testing/sysfs-driver-ppi | 19 +++++++++-----
drivers/char/tpm/tpm-chip.c | 24 +++++++++++------
drivers/char/tpm/tpm.h | 17 +++++-------
drivers/char/tpm/tpm_ppi.c | 34 ++++++++----------------
fs/sysfs/group.c | 42 ++++++++++++++++++++++++++++++
include/linux/sysfs.h | 9 +++++++
6 files changed, 96 insertions(+), 49 deletions(-)
--
2.1.4
^ permalink raw reply
* [PATCH v7 3/3] tpm: update PPI documentation to address the location change.
From: Jarkko Sakkinen @ 2015-06-22 17:24 UTC (permalink / raw)
To: tpmdd-devel, linux-kernel
Cc: peterhuewe, gregkh, jgunthorpe, Jarkko Sakkinen,
open list:ABI/API
In-Reply-To: <1434993894-5911-1-git-send-email-jarkko.sakkinen@linux.intel.com>
Updated Documentation/ABI/testing/sysfs-driver-ppi in order to explain
where PPI attributes are located and how backwards compatiblity is
addressed.
Signed-off-by: Jarkko Sakkinen <jarkko.sakkinen@linux.intel.com>
---
Documentation/ABI/testing/sysfs-driver-ppi | 19 ++++++++++++-------
1 file changed, 12 insertions(+), 7 deletions(-)
diff --git a/Documentation/ABI/testing/sysfs-driver-ppi b/Documentation/ABI/testing/sysfs-driver-ppi
index 7d1435b..9921ef2 100644
--- a/Documentation/ABI/testing/sysfs-driver-ppi
+++ b/Documentation/ABI/testing/sysfs-driver-ppi
@@ -1,4 +1,4 @@
-What: /sys/devices/pnp0/<bus-num>/ppi/
+What: /sys/class/tpm/tpmX/ppi/
Date: August 2012
Kernel Version: 3.6
Contact: xiaoyan.zhang@intel.com
@@ -8,9 +8,14 @@ Description:
folder makes sense. The folder path can be got by command
'find /sys/ -name 'pcrs''. For the detail information of PPI,
please refer to the PPI specification from
+
http://www.trustedcomputinggroup.org/
-What: /sys/devices/pnp0/<bus-num>/ppi/version
+ In Linux 4.2 ppi was moved to the character device directory.
+ A symlink from tpmX/device/ppi to tpmX/ppi to provide backwards
+ compatibility.
+
+What: /sys/class/tpm/tpmX/ppi/version
Date: August 2012
Contact: xiaoyan.zhang@intel.com
Description:
@@ -18,7 +23,7 @@ Description:
platform.
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/request
+What: /sys/class/tpm/tpmX/ppi/request
Date: August 2012
Contact: xiaoyan.zhang@intel.com
Description:
@@ -28,7 +33,7 @@ Description:
integer value range from 1 to 160, and 0 means no request.
This file can be read and written.
-What: /sys/devices/pnp0/00:<bus-num>/ppi/response
+What: /sys/class/tpm/tpmX/ppi/response
Date: August 2012
Contact: xiaoyan.zhang@intel.com
Description:
@@ -37,7 +42,7 @@ Description:
: <response description>".
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/transition_action
+What: /sys/class/tpm/tpmX/ppi/transition_action
Date: August 2012
Contact: xiaoyan.zhang@intel.com
Description:
@@ -47,7 +52,7 @@ Description:
description>".
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/tcg_operations
+What: /sys/class/tpm/tpmX/ppi/tcg_operations
Date: August 2012
Contact: xiaoyan.zhang@intel.com
Description:
@@ -58,7 +63,7 @@ Description:
This attribute is only supported by PPI version 1.2+.
This file is readonly.
-What: /sys/devices/pnp0/<bus-num>/ppi/vs_operations
+What: /sys/class/tpm/tpmX/ppi/vs_operations
Date: August 2012
Contact: xiaoyan.zhang@intel.com
Description:
--
2.1.4
^ permalink raw reply related
* [PATCH v2] ipc: Modify message queue accounting to reflect both total user data and auxiliary kernel data
From: Marcus Gelderie @ 2015-06-22 22:25 UTC (permalink / raw)
To: linux-kernel-u79uwXL29TY76Z2rM5mHXA
Cc: mtk.manpages-Re5JQEeQqe8AvxtiuMwx3w,
dhowells-H+wXaHxf7aLQT0dZR+AlfA,
viro-RmSDqhL/yNMiFSDQTTA3OLVCufUGDwFn,
dledford-H+wXaHxf7aLQT0dZR+AlfA, John Duffy, Arto Bendiken,
linux-api-u79uwXL29TY76Z2rM5mHXA, Davidlohr Bueso,
redmnic-Re5JQEeQqe8AvxtiuMwx3w
A while back, the message queue implementation in the kernel was
improved to use btrees to speed up retrieval of messages (commit
d6629859b36). The patch introducing the improved kernel handling of
message queues (using btrees) has, as a by-product, changed the
meaning of the QSIZE field in the pseudo-file created for the queue.
Before, this field reflected the size of the user-data in the queue.
Since, it also takes kernel data structures into account. For
example, if 13 bytes of user data are in the queue, on my machine the
file reports a size of 61 bytes.
There was some discussion on this topic before (for example
https://lkml.org/lkml/2014/10/1/115). Commenting on the lkml, Michael
Kerrisk gave the following background (https://lkml.org/lkml/2015/6/16/74):
The pseudofiles in the mqueue filesystem (usually mounted at
/dev/mqueue) expose fields with metadata describing a message
queue. One of these fields, QSIZE, as originally implemented,
showed the total number of bytes of user data in all messages in
the message queue, and this feature was documented from the
beginning in the mq_overview(7) page. In 3.5, some other (useful)
work happened to break the user-space API in a couple of places,
including the value exposed via QSIZE, which now includes a measure
of kernel overhead bytes for the queue, a figure that renders QSIZE
useless for its original purpose, since there's no way to deduce
the number of overhead bytes consumed by the implementation.
(The other user-space breakage was subsequently fixed.)
Reporting the size of the message queue in kernel has its merits, but
doing so in the QSIZE field of the pseudo file corresponding to the
queue is a breaking change, as mentioned above. This patch therefore
returns the QSIZE field to its original meaning. At the same time,
it introduces a new field QKERSIZE that reflects the size of the queue
in kernel (user data + kernel data).
It should be noted that the resource limit RLIMIT_MSGQUEUE is counted
against the worst-case size of the queue (in both the old and the new
implementation) and is therefore not affected by this change, nor by
the previous one changing the way of accounting.
Signed-off-by: Marcus Gelderie <redmnic-Re5JQEeQqe8AvxtiuMwx3w@public.gmane.org>
---
ipc/mqueue.c | 20 ++++++++++++++------
1 file changed, 14 insertions(+), 6 deletions(-)
diff --git a/ipc/mqueue.c b/ipc/mqueue.c
index 3aaea7f..7d4c464 100644
--- a/ipc/mqueue.c
+++ b/ipc/mqueue.c
@@ -41,7 +41,7 @@
#define MQUEUE_MAGIC 0x19800202
#define DIRENT_SIZE 20
-#define FILENT_SIZE 80
+#define FILENT_SIZE 90
#define SEND 0
#define RECV 1
@@ -82,8 +82,12 @@ struct mqueue_inode_info {
/* for tasks waiting for free space and messages, respectively */
struct ext_wait_queue e_wait_q[2];
- unsigned long qsize; /* size of queue in memory (sum of all msgs) */
-};
+ /* size of queue in memory (sum of all msgs plus kernel
+ * data structures) */
+ unsigned long qsize;
+
+ /* size of user data in the queue (sum of all msgs) */
+ unsigned long q_usersize; };
static const struct inode_operations mqueue_dir_inode_operations;
static const struct file_operations mqueue_file_operations;
@@ -151,6 +155,7 @@ static int msg_insert(struct msg_msg *msg, struct mqueue_inode_info *info)
insert_msg:
info->attr.mq_curmsgs++;
info->qsize += msg->m_ts;
+ info->q_usersize += msg->m_ts;
list_add_tail(&msg->m_list, &leaf->msg_list);
return 0;
}
@@ -210,6 +215,7 @@ try_again:
}
info->attr.mq_curmsgs--;
info->qsize -= msg->m_ts;
+ info->q_usersize -= msg->m_ts;
return msg;
}
@@ -246,6 +252,7 @@ static struct inode *mqueue_get_inode(struct super_block *sb,
info->notify_owner = NULL;
info->notify_user_ns = NULL;
info->qsize = 0;
+ info->q_usersize = 0;
info->user = NULL; /* set when all is ok */
info->msg_tree = RB_ROOT;
info->node_cache = NULL;
@@ -491,13 +498,14 @@ static ssize_t mqueue_read_file(struct file *filp, char __user *u_data,
spin_lock(&info->lock);
snprintf(buffer, sizeof(buffer),
- "QSIZE:%-10lu NOTIFY:%-5d SIGNO:%-5d NOTIFY_PID:%-6d\n",
- info->qsize,
+ "QSIZE:%-10lu NOTIFY:%-5d SIGNO:%-5d NOTIFY_PID:%-6d QKERSIZE:%-10lu\n",
+ info->q_usersize,
info->notify_owner ? info->notify.sigev_notify : 0,
(info->notify_owner &&
info->notify.sigev_notify == SIGEV_SIGNAL) ?
info->notify.sigev_signo : 0,
- pid_vnr(info->notify_owner));
+ pid_vnr(info->notify_owner),
+ info->qsize);
spin_unlock(&info->lock);
buffer[sizeof(buffer)-1] = '\0';
--
2.4.4
^ permalink raw reply related
* [PATCH v6 0/9] Add simple NVMEM Framework via regmap.
From: Srinivas Kandagatla @ 2015-06-22 23:07 UTC (permalink / raw)
To: linux-arm-kernel, Greg Kroah-Hartman
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown, s.hauer,
linux-api, linux-kernel, devicetree, linux-arm-msm, arnd, sboyd,
pantelis.antoniou, mporter, stefan.wahren, wxt,
Srinivas Kandagatla
Thankyou all for providing inputs and comments on previous versions of this
patchset. Here is the v6 of the patchset addressing all the issues raised as
part of previous versions review.
This patchset adds a new simple NVMEM framework to kernel, and it is tested
with various drivers like "QCOM thermal sensors", "QCOM cpr driver",
"begal bone cape manager" and few more on the way.
Greg KH,
Is it possible to take these patches via your tree Or do you think it
should go via ARM-SOC tree?
Up until now, NVMEM drivers like eeprom were stored in drivers/misc, where they
all had to duplicate pretty much the same code to register a sysfs file, allow
in-kernel users to access the content of the devices they were driving, etc.
This was also a problem as far as other in-kernel users were involved, since
the solutions used were pretty much different from on driver to another, there
was a rather big abstraction leak.
Introduction of this framework aims at solving this. It also introduces DT
representation for consumer devices to go get the data they require (MAC
Addresses, SoC/Revision ID, part numbers, and so on) from the NVMEMs.
After learning few things about QCOM qfprom and other eeprom/efuses, which
has packed fields at bit level. Which makes it important to add support to
such memories. This version adds support to this type of non volatile
memories by adding support to bit level nvmem-cells.
Having regmap interface to this framework would give much better
abstraction for nvmems on different buses.
patch 1-4 Introduces the NVMEM framework.
Patch 5-6 Adds Qualcomm specific qfprom driver.
Patch 7 migrates an existing driver to nvmem framework.
Patch 8 adds entry in MAINTAINERS.
Its also possible to migrate other nvmem drivers to this framework.
Providers APIs:
nvmem_register/unregister();
Consumers APIs:
Cell based apis for both DT/Non-DT:
nvmem_cell_get()/nvmem_cell_put();
devm_nvmem_cell_get()/devm_nvmem_cell_put();
of_nvmem_cell_get()
nvmem_cell_read()/nvmem_cell_write();
Raw byte access apis for both DT/non-DT.
nvmem_device_get()/nvmem_device_put()
devm_nvmem_device_get()/nvmem_device_put()
of_nvmem_device_get()
nvmem_device_read()/nvmem_device_write();
nvmem_device_cell_read()/nvmem_device_cell_write();
Device Tree:
/* 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;
};
...
};
userspace interface: binary file in /sys/class/nvmem/*/nvmem
ex:
hexdump /sys/class/nvmem/qfprom0/nvmem
0000000 0000 0000 0000 0000 0000 0000 0000 0000
*
00000a0 db10 2240 0000 e000 0c00 0c00 0000 0c00
0000000 0000 0000 0000 0000 0000 0000 0000 0000
...
*
0001000
Changes since v5(https://lkml.org/lkml/2015/5/21/643)
* skipped regmap patches which are already merged by Mark.
* Fixed lot of style related comments by Stephen.
* Fixed sunxi driver.
* added devm_* variants requested by Stephen.
* added of_* variants requested by Pantelis Antoniou
* added read_only options for non-dt drivers.
* added basic how-to doc.
Changes since v4(https://lkml.org/lkml/2015/3/30/725)
* rename eeprom to nvmem suggested by Matt Porter
* added bit level support to nvmem cells, if not framework is
not usable for qcom platforms.
* removed ternary operator shortcut suggested by Mark B.
* unified consumer/provider apis for both DT and non-DT.
* added name support for cell.
* added bit level bindings.
* added read-only support suggested by Matt Porter and others.
* added new nvmem_device based consumer apis.
Changes since v3(https://lkml.org/lkml/2015/3/24/1066)
* simplified logic in bin_attr_eeprom_read/write spotted by Mark Brown.
* moved from using regmap_bulk_read/write to regmap_raw_read/write
spotted by Mark Brown
* Fixed return error codes for the dummy wrappers spotted by Sascha Hauer
* Fixed various error code checks in core spotted by Sascha Hauer.
* Simplified consumer bindings suggested by Sascha Hauer.
* Added eeprom-mmio helper functions.
Changes since v2(https://lkml.org/lkml/2015/3/13/168)
* Fixed error handling in eeprom_register spotted by Mark Brown
* Added new regmap_get_max_register() and regmap_get_reg_stride().
* Fixed module build errors reported by kbuild robot.
* recycle the ids when eeprom provider is released.
Changes since v1(https://lkml.org/lkml/2015/3/5/153)
* Fix various Licencing issues spotted by Paul Bolle and Mark Brown
* Allow eeprom core to build as module spotted by Paul Bolle.
* Fix various kconfig issues spotted by Paul Bolle.
* remove unessary atomic varible spotted by Mark Brown.
* Few cleanups and common up some of the code in core.
* Add qfprom bindings.
Changes since RFC(https://lkml.org/lkml/2015/2/19/307)
* Fix documentation and error checks in read/write spotted by Andrew Lunn
* Kconfig fix suggested by Stephen Boyd.
* Add module owner suggested by Stephen Boyd and others.
* Fix unsafe handling of eeprom in unregister spotted by Russell and Mark Brown.
* seperate bindings patch as suggested by Rob.
* Add MAINTAINERS as suggested by Rob.
* Added support to allow reading eeprom for things like serial number which
* canbe scatters across.
* Added eeprom data using reg property suggested by Sascha and Stephen.
* Added non-DT support.
* Move kerneldoc to the src files spotted by Mark Brown.
* Remove local list and do eeprom lookup by using class_find_device()
Thanks,
srini
Maxime Ripard (1):
nvmem: sunxi: Move the SID driver to the nvmem framework
Srinivas Kandagatla (8):
nvmem: Add a simple NVMEM framework for nvmem providers
nvmem: Add a simple NVMEM framework for consumers
nvmem: Add nvmem_device based consumer apis.
nvmem: Add bindings for simple nvmem framework
Documentation: nvmem: add nvmem api level and how-to doc
nvmem: qfprom: Add Qualcomm QFPROM support.
nvmem: qfprom: Add bindings for qfprom
nvmem: Add to MAINTAINERS for nvmem framework
Documentation/ABI/testing/sysfs-driver-sunxi-sid | 22 -
.../bindings/misc/allwinner,sunxi-sid.txt | 17 -
.../bindings/nvmem/allwinner,sunxi-sid.txt | 21 +
Documentation/devicetree/bindings/nvmem/nvmem.txt | 85 ++
Documentation/devicetree/bindings/nvmem/qfprom.txt | 35 +
Documentation/nvmem/nvmem.txt | 151 +++
MAINTAINERS | 9 +
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/misc/eeprom/Kconfig | 13 -
drivers/misc/eeprom/Makefile | 1 -
drivers/misc/eeprom/sunxi_sid.c | 156 ---
drivers/nvmem/Kconfig | 36 +
drivers/nvmem/Makefile | 12 +
drivers/nvmem/core.c | 1069 ++++++++++++++++++++
drivers/nvmem/qfprom.c | 89 ++
drivers/nvmem/sunxi-sid.c | 160 +++
include/linux/nvmem-consumer.h | 154 +++
include/linux/nvmem-provider.h | 48 +
19 files changed, 1872 insertions(+), 209 deletions(-)
delete mode 100644 Documentation/ABI/testing/sysfs-driver-sunxi-sid
delete mode 100644 Documentation/devicetree/bindings/misc/allwinner,sunxi-sid.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/allwinner,sunxi-sid.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/nvmem.txt
create mode 100644 Documentation/devicetree/bindings/nvmem/qfprom.txt
create mode 100644 Documentation/nvmem/nvmem.txt
delete mode 100644 drivers/misc/eeprom/sunxi_sid.c
create mode 100644 drivers/nvmem/Kconfig
create mode 100644 drivers/nvmem/Makefile
create mode 100644 drivers/nvmem/core.c
create mode 100644 drivers/nvmem/qfprom.c
create mode 100644 drivers/nvmem/sunxi-sid.c
create mode 100644 include/linux/nvmem-consumer.h
create mode 100644 include/linux/nvmem-provider.h
--
1.9.1
^ permalink raw reply
* [PATCH v6 1/9] nvmem: Add a simple NVMEM framework for nvmem providers
From: Srinivas Kandagatla @ 2015-06-22 23:08 UTC (permalink / raw)
To: linux-arm-kernel-IAPFreCvJWM7uuMidbF8XUB+6BGkLq7r,
Greg Kroah-Hartman
Cc: Maxime Ripard, Rob Herring, Kumar Gala, Mark Brown,
s.hauer-bIcnvbaLZ9MEGnE8C9+IrQ, linux-api-u79uwXL29TY76Z2rM5mHXA,
linux-kernel-u79uwXL29TY76Z2rM5mHXA,
devicetree-u79uwXL29TY76Z2rM5mHXA,
linux-arm-msm-u79uwXL29TY76Z2rM5mHXA, arnd-r2nGTMty4D4,
sboyd-sgV2jX0FEOL9JmXXK+q4OQ,
pantelis.antoniou-OWPKS81ov/FWk0Htik3J/w,
mporter-OWPKS81ov/FWk0Htik3J/w, stefan.wahren-eS4NqCHxEME,
wxt-TNX95d0MmH7DzftRWevZcw, Srinivas Kandagatla
In-Reply-To: <1435014459-26138-1-git-send-email-srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
This patch adds just providers part of the framework just to enable easy
review.
Up until now, NVMEM drivers like eeprom were stored in drivers/misc,
where they all had to duplicate pretty much the same code to register
a sysfs file, allow in-kernel users to access the content of the devices
they were driving, etc.
This was also a problem as far as other in-kernel users were involved,
since the solutions used were pretty much different from on driver to
another, there was a rather big abstraction leak.
This introduction of this framework aims at solving this. It also
introduces DT representation for consumer devices to go get the data
they require (MAC Addresses, SoC/Revision ID, part numbers, and so on)
from the nvmems.
Having regmap interface to this framework would give much better
abstraction for nvmems on different buses.
Signed-off-by: Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org>
[Maxime Ripard: intial version of eeprom framework]
Signed-off-by: Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org>
---
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/nvmem/Kconfig | 10 ++
drivers/nvmem/Makefile | 6 +
drivers/nvmem/core.c | 398 +++++++++++++++++++++++++++++++++++++++++
include/linux/nvmem-provider.h | 54 ++++++
6 files changed, 471 insertions(+)
create mode 100644 drivers/nvmem/Kconfig
create mode 100644 drivers/nvmem/Makefile
create mode 100644 drivers/nvmem/core.c
create mode 100644 include/linux/nvmem-provider.h
diff --git a/drivers/Kconfig b/drivers/Kconfig
index c0cc96b..69d7305 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -182,4 +182,6 @@ source "drivers/thunderbolt/Kconfig"
source "drivers/android/Kconfig"
+source "drivers/nvmem/Kconfig"
+
endmenu
diff --git a/drivers/Makefile b/drivers/Makefile
index 46d2554..f86b897 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -165,3 +165,4 @@ obj-$(CONFIG_RAS) += ras/
obj-$(CONFIG_THUNDERBOLT) += thunderbolt/
obj-$(CONFIG_CORESIGHT) += hwtracing/coresight/
obj-$(CONFIG_ANDROID) += android/
+obj-$(CONFIG_NVMEM) += nvmem/
diff --git a/drivers/nvmem/Kconfig b/drivers/nvmem/Kconfig
new file mode 100644
index 0000000..f157b6d
--- /dev/null
+++ b/drivers/nvmem/Kconfig
@@ -0,0 +1,10 @@
+menuconfig NVMEM
+ tristate "NVMEM Support"
+ select REGMAP
+ help
+ Support for NVMEM devices.
+
+ This framework is designed to provide a generic interface to NVMEM
+ from both the Linux Kernel and the userspace.
+
+ If unsure, say no.
diff --git a/drivers/nvmem/Makefile b/drivers/nvmem/Makefile
new file mode 100644
index 0000000..6df2c69
--- /dev/null
+++ b/drivers/nvmem/Makefile
@@ -0,0 +1,6 @@
+#
+# Makefile for nvmem drivers.
+#
+
+obj-$(CONFIG_NVMEM) += nvmem_core.o
+nvmem_core-y := core.o
diff --git a/drivers/nvmem/core.c b/drivers/nvmem/core.c
new file mode 100644
index 0000000..17a826d
--- /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>
+
+struct nvmem_device {
+ const char *name;
+ struct regmap *regmap;
+ struct module *owner;
+ struct device dev;
+ int stride;
+ int word_size;
+ int ncells;
+ int id;
+ int users;
+ size_t size;
+ bool read_only;
+};
+
+struct nvmem_cell {
+ const char *name;
+ int offset;
+ int bytes;
+ int bit_offset;
+ int nbits;
+ struct nvmem_device *nvmem;
+ struct list_head node;
+};
+
+static DEFINE_MUTEX(nvmem_mutex);
+static DEFINE_IDA(nvmem_ida);
+
+static LIST_HEAD(nvmem_cells);
+static DEFINE_MUTEX(nvmem_cells_mutex);
+
+#define to_nvmem_device(d) container_of(d, struct nvmem_device, dev)
+
+static ssize_t bin_attr_nvmem_read(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+ int rc;
+
+ /* Stop the user from reading */
+ if (pos > nvmem->size)
+ return 0;
+
+ if (pos + count > nvmem->size)
+ count = nvmem->size - pos;
+
+ count = count/nvmem->word_size * nvmem->word_size;
+
+ rc = regmap_raw_read(nvmem->regmap, pos, buf, count);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return count;
+}
+
+static ssize_t bin_attr_nvmem_write(struct file *filp, struct kobject *kobj,
+ struct bin_attribute *attr,
+ char *buf, loff_t pos, size_t count)
+{
+ struct device *dev = container_of(kobj, struct device, kobj);
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+ int rc;
+
+ /* Stop the user from writing */
+ if (pos > nvmem->size)
+ return 0;
+
+ if (pos + count > nvmem->size)
+ count = nvmem->size - pos;
+
+ count = count/nvmem->word_size * nvmem->word_size;
+
+ rc = regmap_raw_write(nvmem->regmap, pos, buf, count);
+
+ if (IS_ERR_VALUE(rc))
+ return rc;
+
+ return count;
+}
+
+/* default read/write permissions */
+static struct bin_attribute bin_attr_nvmem = {
+ .attr = {
+ .name = "nvmem",
+ .mode = S_IWUSR | S_IRUGO,
+ },
+ .read = bin_attr_nvmem_read,
+ .write = bin_attr_nvmem_write,
+};
+
+static struct bin_attribute *nvmem_bin_attributes[] = {
+ &bin_attr_nvmem,
+ NULL,
+};
+
+static const struct attribute_group nvmem_bin_group = {
+ .bin_attrs = nvmem_bin_attributes,
+};
+
+static const struct attribute_group *nvmem_dev_groups[] = {
+ &nvmem_bin_group,
+ NULL,
+};
+
+/* read only permission */
+static struct bin_attribute bin_attr_ro_nvmem = {
+ .attr = {
+ .name = "nvmem",
+ .mode = S_IRUGO,
+ },
+ .read = bin_attr_nvmem_read,
+};
+
+static struct bin_attribute *nvmem_bin_ro_attributes[] = {
+ &bin_attr_ro_nvmem,
+ NULL,
+};
+static const struct attribute_group nvmem_bin_ro_group = {
+ .bin_attrs = nvmem_bin_ro_attributes,
+};
+
+static void nvmem_release(struct device *dev)
+{
+ struct nvmem_device *nvmem = to_nvmem_device(dev);
+
+ ida_simple_remove(&nvmem_ida, nvmem->id);
+ kfree(nvmem);
+}
+
+static struct class nvmem_class = {
+ .name = "nvmem",
+ .dev_groups = nvmem_dev_groups,
+ .dev_release = nvmem_release,
+};
+
+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)
+{
+ 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)
+ 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;
+ 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 void nvmem_cell_add(struct nvmem_cell *cell)
+{
+ mutex_lock(&nvmem_cells_mutex);
+ list_add_tail(&cell->node, &nvmem_cells);
+ mutex_unlock(&nvmem_cells_mutex);
+}
+
+static int nvmem_cell_info_to_nvmem_cell(struct nvmem_device *nvmem,
+ struct nvmem_cell_info *info,
+ struct nvmem_cell *cell)
+{
+ cell->nvmem = nvmem;
+ cell->offset = info->offset;
+ cell->bytes = info->bytes;
+ cell->name = info->name;
+
+ cell->bit_offset = info->bit_offset;
+ cell->nbits = info->nbits;
+
+ if (cell->nbits)
+ cell->bytes = DIV_ROUND_UP(cell->nbits + cell->bit_offset,
+ BITS_PER_BYTE);
+
+ if (!IS_ALIGNED(cell->offset, nvmem->stride)) {
+ dev_err(&nvmem->dev,
+ "cell %s unaligned to nvmem stride %d\n",
+ cell->name, nvmem->stride);
+ return -EINVAL;
+ }
+
+ return 0;
+}
+
+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);
+ if (!cells)
+ return -ENOMEM;
+
+ for (i = 0; i < cfg->ncells; i++) {
+ cells[i] = kzalloc(sizeof(**cells), GFP_KERNEL);
+ if (!cells[i]) {
+ rval = -ENOMEM;
+ goto err;
+ }
+
+ rval = nvmem_cell_info_to_nvmem_cell(nvmem, &info[i], cells[i]);
+ if (IS_ERR_VALUE(rval)) {
+ kfree(cells[i]);
+ goto err;
+ }
+
+ nvmem_cell_add(cells[i]);
+ }
+
+ nvmem->ncells = cfg->ncells;
+ /* remove tmp array */
+ kfree(cells);
+
+ return 0;
+err:
+ while (--i)
+ nvmem_cell_drop(cells[i]);
+
+ return rval;
+}
+
+/**
+ * nvmem_register() - Register a nvmem device for given nvmem_config.
+ * Also creates an binary entry in /sys/class/nvmem/dev-name/nvmem
+ *
+ * @config: nvmem device configuration with which nvmem device is created.
+ *
+ * The return value will be an ERR_PTR() on error or a valid pointer
+ * to nvmem_device.
+ */
+
+struct nvmem_device *nvmem_register(struct nvmem_config *config)
+{
+ struct nvmem_device *nvmem;
+ struct regmap *rm;
+ int rval;
+
+ if (!config->dev)
+ return ERR_PTR(-EINVAL);
+
+ rm = dev_get_regmap(config->dev, NULL);
+ if (!rm) {
+ dev_err(config->dev, "Regmap not found\n");
+ return ERR_PTR(-EINVAL);
+ }
+
+ nvmem = kzalloc(sizeof(*nvmem), GFP_KERNEL);
+ if (!nvmem)
+ return ERR_PTR(-ENOMEM);
+
+ nvmem->id = ida_simple_get(&nvmem_ida, 0, 0, GFP_KERNEL);
+ if (nvmem->id < 0) {
+ kfree(nvmem);
+ return ERR_PTR(nvmem->id);
+ }
+
+ nvmem->regmap = rm;
+ nvmem->owner = config->owner;
+ nvmem->stride = regmap_get_reg_stride(rm);
+ nvmem->word_size = regmap_get_val_bytes(rm);
+ nvmem->size = regmap_get_max_register(rm) + nvmem->stride;
+ nvmem->dev.class = &nvmem_class;
+ nvmem->dev.parent = config->dev;
+ nvmem->dev.of_node = config->dev->of_node;
+ dev_set_name(&nvmem->dev, "%s%d",
+ config->name ? : "nvmem", config->id);
+
+ nvmem->read_only = nvmem->dev.of_node ?
+ of_property_read_bool(nvmem->dev.of_node,
+ "read-only") :
+ config->read_only;
+
+ 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);
+
+ if (config->cells)
+ nvmem_add_cells(nvmem, config);
+
+ return nvmem;
+}
+EXPORT_SYMBOL_GPL(nvmem_register);
+
+/**
+ * nvmem_unregister() - Unregister previously registered nvmem device
+ *
+ * @nvmem: Pointer to previously registered nvmem device.
+ *
+ * The return value will be an non zero on error or a zero on success.
+ */
+int nvmem_unregister(struct nvmem_device *nvmem)
+{
+ mutex_lock(&nvmem_mutex);
+ if (nvmem->users) {
+ mutex_unlock(&nvmem_mutex);
+ return -EBUSY;
+ }
+ mutex_unlock(&nvmem_mutex);
+
+ nvmem_device_remove_all_cells(nvmem);
+ device_del(&nvmem->dev);
+
+ return 0;
+}
+EXPORT_SYMBOL_GPL(nvmem_unregister);
+
+static int __init nvmem_init(void)
+{
+ return class_register(&nvmem_class);
+}
+
+static void __exit nvmem_exit(void)
+{
+ class_unregister(&nvmem_class);
+}
+
+subsys_initcall(nvmem_init);
+module_exit(nvmem_exit);
+
+MODULE_AUTHOR("Srinivas Kandagatla <srinivas.kandagatla-QSEj5FYQhm4dnm+yROfE0A@public.gmane.org");
+MODULE_AUTHOR("Maxime Ripard <maxime.ripard-wi1+55ScJUtKEb57/3fJTNBPR1lH4CV8@public.gmane.org");
+MODULE_DESCRIPTION("nvmem Driver Core");
+MODULE_LICENSE("GPL v2");
diff --git a/include/linux/nvmem-provider.h b/include/linux/nvmem-provider.h
new file mode 100644
index 0000000..f589d3b
--- /dev/null
+++ b/include/linux/nvmem-provider.h
@@ -0,0 +1,54 @@
+/*
+ * nvmem framework provider.
+ *
+ * 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 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_PROVIDER_H
+#define _LINUX_NVMEM_PROVIDER_H
+
+struct nvmem_device;
+
+struct nvmem_cell_info {
+ const char *name;
+ int offset;
+ int bytes;
+ int bit_offset;
+ int nbits;
+};
+
+struct nvmem_config {
+ struct device *dev;
+ const char *name;
+ int id;
+ struct module *owner;
+ struct nvmem_cell_info *cells;
+ int ncells;
+ bool read_only;
+};
+
+#if IS_ENABLED(CONFIG_NVMEM)
+
+struct nvmem_device *nvmem_register(struct nvmem_config *cfg);
+int nvmem_unregister(struct nvmem_device *nvmem);
+
+#else
+
+static inline struct nvmem_device *nvmem_register(struct nvmem_config *cfg)
+{
+ return ERR_PTR(-ENOSYS);
+}
+
+static inline int nvmem_unregister(struct nvmem_device *nvmem)
+{
+ return -ENOSYS;
+}
+
+#endif /* CONFIG_NVMEM */
+
+#endif /* ifndef _LINUX_NVMEM_PROVIDER_H */
--
1.9.1
^ permalink raw reply related
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