All of lore.kernel.org
 help / color / mirror / Atom feed
* [Buildroot] [PATCH 1/2] wget: fix ssl detection in static libs configuration
From: Arnout Vandecappelle @ 2016-11-08 21:45 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <1478630202-1630-1-git-send-email-rahul.bedarkar@imgtec.com>



On 08-11-16 19:36, Rahul Bedarkar wrote:
> When building wget with openssl in static libs configuration, wget
> build system fails detect openssl because it doesn't specify LD flags
                    to

> for private libs used by openssl. This specifically happens when we
> pass --with-libssl-prefix to configure which tries to find ssl using
> custom flags. If we don't specify --with-libssl-prefix, it relies on
> pkg-config files to detect ssl and it's LD flags which helps with static
> linking.

 Interesting point to note: the --with-libssl-prefix dates from a time when it
didn't have pkg-config support yet. It should have been removed when pkg-config
support was added, with the bump to 1.16.1.

> 
> This commit removes --with-libssl-prefix conf opts. Since this case is
> similar to gnutls, we remove same conf opts for gnutls as well.
> 
> wget can be built with either gnutls or openssl crypto libraries, so
> separate optional support for both is not required. This commit also
> does minor optimization by checking for either gnutls or openssl while
> at it.

 That really should have been a separate patch, but OK.


Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> 
> Fixes:
>   http://autobuild.buildroot.net/results/c6a/c6abdff37b86471cf8b0ceffeff5472042923de0/
> 
> Signed-off-by: Rahul Bedarkar <rahul.bedarkar@imgtec.com>
> ---
>  package/wget/wget.mk | 19 +++++--------------
>  1 file changed, 5 insertions(+), 14 deletions(-)
> 
> diff --git a/package/wget/wget.mk b/package/wget/wget.mk
> index 9cda76b..c9efc03 100644
> --- a/package/wget/wget.mk
> +++ b/package/wget/wget.mk
> @@ -17,26 +17,17 @@ WGET_DEPENDENCIES += busybox
>  endif
>  
>  ifeq ($(BR2_PACKAGE_GNUTLS),y)
> -WGET_CONF_OPTS += \
> -	--with-ssl=gnutls \
> -	--with-libgnutls-prefix=$(STAGING_DIR)
> +WGET_CONF_OPTS += --with-ssl=gnutls
>  WGET_DEPENDENCIES += gnutls
> -endif
> -
> -ifeq ($(BR2_PACKAGE_OPENSSL),y)
> -WGET_CONF_OPTS += --with-ssl=openssl --with-libssl-prefix=$(STAGING_DIR)
> +else ifeq ($(BR2_PACKAGE_OPENSSL),y)
> +WGET_CONF_OPTS += --with-ssl=openssl
>  WGET_DEPENDENCIES += openssl
> +else
> +WGET_CONF_OPTS += --without-ssl
>  endif
>  
>  ifeq ($(BR2_PACKAGE_UTIL_LINUX_LIBUUID),y)
>  WGET_DEPENDENCIES += util-linux
>  endif
>  
> -# --with-ssl is default
> -ifneq ($(BR2_PACKAGE_GNUTLS),y)
> -ifneq ($(BR2_PACKAGE_OPENSSL),y)
> -WGET_CONF_OPTS += --without-ssl
> -endif
> -endif
> -
>  $(eval $(autotools-package))
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply

* Re: 25G RDMA networking thoughts???
From: Sage Weil @ 2016-11-08 21:45 UTC (permalink / raw)
  To: LIU, Fei; +Cc: Haomai Wang, ceph-devel
In-Reply-To: <16962d43-7f03-4e92-9eee-b3f4ab80408c.james.liu@alibaba-inc.com>

[-- Attachment #1: Type: TEXT/PLAIN, Size: 1788 bytes --]

On Wed, 9 Nov 2016, LIU, Fei wrote:
> Hi Sage,
>    Yes, Totally understood.  25G RDMA network for Ceph cluster is built for
> interal test. Xio messenger and Async messenger(But it can only support
> infiniband,right) are going to be two options. We are carefully evaluate
> both of these two options. But the most important goal in the end is to see
> how bluestore works with rdma to bring down the total latency for workload
> like OLTP.  
> 
> Hi Haomai,
>    Would you mind let us know when the async messenger is going to support
> ethernet if not support yet?

The default async backend is PosixStack which is all TCP-based.  (And 
async is now the default messenger in kraken.)

sage

> 
>    Regards,
>    James
>       ------------------------------------------------------------------
> From:Sage Weil <sweil@redhat.com>
> Time:2016 Nov 8 (Tue) 13:19
> To:James <james.liu@alibaba-inc.com>
> Subject:Re: 25G RDMA networking thoughts???
> 
> [adding ceph-devel]
> 
> On Wed, 9 Nov 2016, LIU, Fei wrote:
> > Hi Sage,
> >    I was wondering do you have any thoughts of 25G RDMA networking
> > construction besides of xio-messenger/async?  Is there any guidance to bu
> ild
> > 25G RDMA netowrk to better control the whole Ceph cluster latency?
> 
> The only RDMA options right now are XioMessenger and AsyncMessenger's new 
> RDMA backend. Both are experimental, but we'd be very interested in 
> hearing about your experience.
> 
> I wouldn't assume that latency is network-related, though.  More often 
> than not we're finding it's the OSD backend or the OSD request internals 
> (e.g., request scheduling or peering) that's the culprit...
> 
> sage
> 
> 
> 
> 

^ permalink raw reply

* [PATCH] MIPS: fix duplicate define
From: Sudip Mukherjee @ 2016-11-08 21:43 UTC (permalink / raw)
  To: Ralf Baechle, Paul Burton; +Cc: linux-kernel, linux-mips, Sudip Mukherjee

The mips build of ip27_defconfig is failing with the error:
In file included from ../arch/mips/include/asm/mach-ip27/spaces.h:29:0,
                 from ../arch/mips/include/asm/page.h:12,
                 from ../arch/mips/vdso/vdso.h:26,
                 from ../arch/mips/vdso/gettimeofday.c:11:
../arch/mips/include/asm/mach-generic/spaces.h:28:0:
	error: "CAC_BASE" redefined [-Werror]
 #define CAC_BASE  _AC(0x80000000, UL)
 
In file included from ../arch/mips/include/asm/page.h:12:0,
                 from ../arch/mips/vdso/vdso.h:26,
                 from ../arch/mips/vdso/gettimeofday.c:11:
../arch/mips/include/asm/mach-ip27/spaces.h:22:0:
	note: this is the location of the previous definition
 #define CAC_BASE  0xa800000000000000

Add a condition to check if CAC_BASE is already defined, and define it
only if it is not yet defined.

Fixes: 3ffc17d8768b ("MIPS: Adjust MIPS64 CAC_BASE to reflect Config.K0")
Signed-off-by: Sudip Mukherjee <sudip.mukherjee@codethink.co.uk>
---

Build log is at:
https://travis-ci.org/sudipm-mukherjee/parport/jobs/174134289

 arch/mips/include/asm/mach-generic/spaces.h | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/arch/mips/include/asm/mach-generic/spaces.h b/arch/mips/include/asm/mach-generic/spaces.h
index 952b0fd..61b75da 100644
--- a/arch/mips/include/asm/mach-generic/spaces.h
+++ b/arch/mips/include/asm/mach-generic/spaces.h
@@ -23,10 +23,12 @@
 
 #ifdef CONFIG_32BIT
 #ifdef CONFIG_KVM_GUEST
+#ifndef CAC_BASE
 #define CAC_BASE		_AC(0x40000000, UL)
 #else
 #define CAC_BASE		_AC(0x80000000, UL)
 #endif
+#endif
 #ifndef IO_BASE
 #define IO_BASE			_AC(0xa0000000, UL)
 #endif
-- 
1.9.1

^ permalink raw reply related

* Re: [Qemu-devel] [PATCH v11 13/22] vfio: Introduce common function to add capabilities
From: Alex Williamson @ 2016-11-08 21:42 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: Alexey Kardashevskiy, pbonzini, kraxel, cjia, qemu-devel, kvm,
	kevin.tian, jike.song, bjsdjshi, linux-kernel
In-Reply-To: <62fbdd2a-6e5b-6a2f-6312-6c2f35bb577c@nvidia.com>

On Wed, 9 Nov 2016 02:16:17 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 11/8/2016 12:59 PM, Alexey Kardashevskiy wrote:
> > On 05/11/16 08:10, Kirti Wankhede wrote:  
> >> Vendor driver using mediated device framework should use
> >> vfio_info_add_capability() to add capabilities.
> >> Introduced this function to reduce code duplication in vendor drivers.
> >>
> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> >> Signed-off-by: Neo Jia <cjia@nvidia.com>
> >> Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
> >> ---
> >>  drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  include/linux/vfio.h |  3 +++
> >>  2 files changed, 62 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> >> index 4ed1a6a247c6..9a03be0942a1 100644
> >> --- a/drivers/vfio/vfio.c
> >> +++ b/drivers/vfio/vfio.c
> >> @@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
> >>  	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
> >>  		tmp->next += offset;
> >>  }
> >> -EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
> >> +EXPORT_SYMBOL(vfio_info_cap_shift);  
> > 
> > 
> > Why this change?
> > 
> >   
> 
> We want this symbol to be available to all drivers.

IOW, from proprietary drivers.  It makes me uncomfortable how many
non-GPL symbols we're adding (or converting) in this effort, but I'm
trying to look objectively at every export as to whether a non-GPL
caller of the function is legitimately separate from in-kernel code.
For instance are they making use of data structures intrinsic to GPL'd
code.  In this case we're converting a symbol that's just manipulating
a data buffer to add an offset to each element in a chain.  The entries
are documented in a uapi header.  Kirti asked me about this one, and I
couldn't find any basis to raise an objection.  If you spot any reason
that any of the export symbols in these series really should be GPL,
please raise the issue.

> >>  
> >> +static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
> >> +{
> >> +	struct vfio_info_cap_header *header;
> >> +	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
> >> +	size_t size;
> >> +
> >> +	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
> >> +	header = vfio_info_cap_add(caps, size,
> >> +				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
> >> +	if (IS_ERR(header))
> >> +		return PTR_ERR(header);
> >> +
> >> +	sparse_cap = container_of(header,
> >> +			struct vfio_region_info_cap_sparse_mmap, header);
> >> +	sparse_cap->nr_areas = sparse->nr_areas;
> >> +	memcpy(sparse_cap->areas, sparse->areas,
> >> +	       sparse->nr_areas * sizeof(*sparse->areas));
> >> +	return 0;
> >> +}
> >> +
> >> +static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
> >> +{
> >> +	struct vfio_info_cap_header *header;
> >> +	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
> >> +
> >> +	header = vfio_info_cap_add(caps, sizeof(*cap),
> >> +				   VFIO_REGION_INFO_CAP_TYPE, 1);
> >> +	if (IS_ERR(header))
> >> +		return PTR_ERR(header);
> >> +
> >> +	type_cap = container_of(header, struct vfio_region_info_cap_type,
> >> +				header);
> >> +	type_cap->type = cap->type;
> >> +	type_cap->subtype = cap->subtype;
> >> +	return 0;
> >> +}
> >> +
> >> +int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
> >> +			     void *cap_type)
> >> +{
> >> +	int ret = -EINVAL;
> >> +
> >> +	if (!cap_type)
> >> +		return 0;
> >> +
> >> +	switch (cap_type_id) {
> >> +	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
> >> +		ret = sparse_mmap_cap(caps, cap_type);
> >> +		break;
> >> +
> >> +	case VFIO_REGION_INFO_CAP_TYPE:
> >> +		ret = region_type_cap(caps, cap_type);
> >> +		break;
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL(vfio_info_add_capability);
> >>  
> >>  /*
> >>   * Pin a set of guest PFNs and return their associated host PFNs for local
> >> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> >> index dcda8fccefab..cf90393a11e2 100644
> >> --- a/include/linux/vfio.h
> >> +++ b/include/linux/vfio.h
> >> @@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
> >>  		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
> >>  extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
> >>  
> >> +extern int vfio_info_add_capability(struct vfio_info_cap *caps,
> >> +				    int cap_type_id, void *cap_type);
> >> +  
> > 
> > 
> > It would make it easier to review and bisect if 14/22 was squashed into
> > this one.   
> 
> This was split based on Alex's suggestion on earlier version of this
> patchset.

Yeah, generally squashing patches together is the opposite of what we
want for review and bisect.  In this case the symbol exports should
avoid any defined-but-unused warnings.  Thanks,

Alex

^ permalink raw reply

* Re: [PATCH v11 13/22] vfio: Introduce common function to add capabilities
From: Alex Williamson @ 2016-11-08 21:42 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: Alexey Kardashevskiy, pbonzini, kraxel, cjia, qemu-devel, kvm,
	kevin.tian, jike.song, bjsdjshi, linux-kernel
In-Reply-To: <62fbdd2a-6e5b-6a2f-6312-6c2f35bb577c@nvidia.com>

On Wed, 9 Nov 2016 02:16:17 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 11/8/2016 12:59 PM, Alexey Kardashevskiy wrote:
> > On 05/11/16 08:10, Kirti Wankhede wrote:  
> >> Vendor driver using mediated device framework should use
> >> vfio_info_add_capability() to add capabilities.
> >> Introduced this function to reduce code duplication in vendor drivers.
> >>
> >> Signed-off-by: Kirti Wankhede <kwankhede@nvidia.com>
> >> Signed-off-by: Neo Jia <cjia@nvidia.com>
> >> Change-Id: I6fca329fa2291f37a2c859d0bc97574d9e2ce1a6
> >> ---
> >>  drivers/vfio/vfio.c  | 60 +++++++++++++++++++++++++++++++++++++++++++++++++++-
> >>  include/linux/vfio.h |  3 +++
> >>  2 files changed, 62 insertions(+), 1 deletion(-)
> >>
> >> diff --git a/drivers/vfio/vfio.c b/drivers/vfio/vfio.c
> >> index 4ed1a6a247c6..9a03be0942a1 100644
> >> --- a/drivers/vfio/vfio.c
> >> +++ b/drivers/vfio/vfio.c
> >> @@ -1797,8 +1797,66 @@ void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset)
> >>  	for (tmp = caps->buf; tmp->next; tmp = (void *)tmp + tmp->next - offset)
> >>  		tmp->next += offset;
> >>  }
> >> -EXPORT_SYMBOL_GPL(vfio_info_cap_shift);
> >> +EXPORT_SYMBOL(vfio_info_cap_shift);  
> > 
> > 
> > Why this change?
> > 
> >   
> 
> We want this symbol to be available to all drivers.

IOW, from proprietary drivers.  It makes me uncomfortable how many
non-GPL symbols we're adding (or converting) in this effort, but I'm
trying to look objectively at every export as to whether a non-GPL
caller of the function is legitimately separate from in-kernel code.
For instance are they making use of data structures intrinsic to GPL'd
code.  In this case we're converting a symbol that's just manipulating
a data buffer to add an offset to each element in a chain.  The entries
are documented in a uapi header.  Kirti asked me about this one, and I
couldn't find any basis to raise an objection.  If you spot any reason
that any of the export symbols in these series really should be GPL,
please raise the issue.

> >>  
> >> +static int sparse_mmap_cap(struct vfio_info_cap *caps, void *cap_type)
> >> +{
> >> +	struct vfio_info_cap_header *header;
> >> +	struct vfio_region_info_cap_sparse_mmap *sparse_cap, *sparse = cap_type;
> >> +	size_t size;
> >> +
> >> +	size = sizeof(*sparse) + sparse->nr_areas *  sizeof(*sparse->areas);
> >> +	header = vfio_info_cap_add(caps, size,
> >> +				   VFIO_REGION_INFO_CAP_SPARSE_MMAP, 1);
> >> +	if (IS_ERR(header))
> >> +		return PTR_ERR(header);
> >> +
> >> +	sparse_cap = container_of(header,
> >> +			struct vfio_region_info_cap_sparse_mmap, header);
> >> +	sparse_cap->nr_areas = sparse->nr_areas;
> >> +	memcpy(sparse_cap->areas, sparse->areas,
> >> +	       sparse->nr_areas * sizeof(*sparse->areas));
> >> +	return 0;
> >> +}
> >> +
> >> +static int region_type_cap(struct vfio_info_cap *caps, void *cap_type)
> >> +{
> >> +	struct vfio_info_cap_header *header;
> >> +	struct vfio_region_info_cap_type *type_cap, *cap = cap_type;
> >> +
> >> +	header = vfio_info_cap_add(caps, sizeof(*cap),
> >> +				   VFIO_REGION_INFO_CAP_TYPE, 1);
> >> +	if (IS_ERR(header))
> >> +		return PTR_ERR(header);
> >> +
> >> +	type_cap = container_of(header, struct vfio_region_info_cap_type,
> >> +				header);
> >> +	type_cap->type = cap->type;
> >> +	type_cap->subtype = cap->subtype;
> >> +	return 0;
> >> +}
> >> +
> >> +int vfio_info_add_capability(struct vfio_info_cap *caps, int cap_type_id,
> >> +			     void *cap_type)
> >> +{
> >> +	int ret = -EINVAL;
> >> +
> >> +	if (!cap_type)
> >> +		return 0;
> >> +
> >> +	switch (cap_type_id) {
> >> +	case VFIO_REGION_INFO_CAP_SPARSE_MMAP:
> >> +		ret = sparse_mmap_cap(caps, cap_type);
> >> +		break;
> >> +
> >> +	case VFIO_REGION_INFO_CAP_TYPE:
> >> +		ret = region_type_cap(caps, cap_type);
> >> +		break;
> >> +	}
> >> +
> >> +	return ret;
> >> +}
> >> +EXPORT_SYMBOL(vfio_info_add_capability);
> >>  
> >>  /*
> >>   * Pin a set of guest PFNs and return their associated host PFNs for local
> >> diff --git a/include/linux/vfio.h b/include/linux/vfio.h
> >> index dcda8fccefab..cf90393a11e2 100644
> >> --- a/include/linux/vfio.h
> >> +++ b/include/linux/vfio.h
> >> @@ -113,6 +113,9 @@ extern struct vfio_info_cap_header *vfio_info_cap_add(
> >>  		struct vfio_info_cap *caps, size_t size, u16 id, u16 version);
> >>  extern void vfio_info_cap_shift(struct vfio_info_cap *caps, size_t offset);
> >>  
> >> +extern int vfio_info_add_capability(struct vfio_info_cap *caps,
> >> +				    int cap_type_id, void *cap_type);
> >> +  
> > 
> > 
> > It would make it easier to review and bisect if 14/22 was squashed into
> > this one.   
> 
> This was split based on Alex's suggestion on earlier version of this
> patchset.

Yeah, generally squashing patches together is the opposite of what we
want for review and bisect.  In this case the symbol exports should
avoid any defined-but-unused warnings.  Thanks,

Alex

^ permalink raw reply

* Re: svc_xprt_put is no longer BH-safe
From: J. Bruce Fields @ 2016-11-08 21:42 UTC (permalink / raw)
  To: Chuck Lever; +Cc: Linux NFS Mailing List
In-Reply-To: <8AD6B9DB-E777-46E4-BB7E-774F5FC8B2B6@oracle.com>

On Tue, Nov 08, 2016 at 04:05:54PM -0500, Chuck Lever wrote:
> 
> > On Nov 8, 2016, at 3:20 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> > 
> > On Tue, Nov 08, 2016 at 03:13:13PM -0500, Chuck Lever wrote:
> >> 
> >>> On Nov 8, 2016, at 3:03 PM, J. Bruce Fields <bfields@fieldses.org> wrote:
> >>> 
> >>> On Sat, Oct 29, 2016 at 12:21:03PM -0400, Chuck Lever wrote:
> >>>> Hi Bruce-
> >>> 
> >>> Sorry for the slow response!
> >>> 
> >>> ...
> >>>> In commit 39a9beab5acb83176e8b9a4f0778749a09341f1f ('rpc: share one xps between
> >>>> all backchannels') you added:
> >>>> 
> >>>> diff --git a/net/sunrpc/svc_xprt.c b/net/sunrpc/svc_xprt.c
> >>>> index f5572e3..4f01f63 100644
> >>>> --- a/net/sunrpc/svc_xprt.c
> >>>> +++ b/net/sunrpc/svc_xprt.c
> >>>> @@ -136,6 +136,8 @@ static void svc_xprt_free(struct kref *kref)
> >>>>       /* See comment on corresponding get in xs_setup_bc_tcp(): */
> >>>>       if (xprt->xpt_bc_xprt)
> >>>>               xprt_put(xprt->xpt_bc_xprt);
> >>>> +       if (xprt->xpt_bc_xps)
> >>>> +               xprt_switch_put(xprt->xpt_bc_xps);
> >>>>       xprt->xpt_ops->xpo_free(xprt);
> >>>>       module_put(owner);
> >>>> }
> >>>> 
> >>>> svc_xprt_free() is invoked by svc_xprt_put(). svc_xprt_put() is called
> >>>> from svc_rdma in soft IRQ context (eg. svc_rdma_wc_receive).
> >>> 
> >>> Is that necessary?  I wonder why the svcrdma code seems to be taking so
> >>> many of its own references on svc_xprts.
> >> 
> >> The idea is to keep the xprt around while Work Requests (I/O) are running,
> >> so that the xprt is guaranteed to be there during work completions. The
> >> completion handlers (where svc_xprt_put is often invoked) run in soft IRQ
> >> context.
> >> 
> >> It's simple to change completions to use a Work Queue instead, but testing
> >> so far shows that will result in a performance loss. I'm still studying it.
> >> 
> >> Is there another way to keep the xprt's ref count boosted while I/O is
> >> going on?
> > 
> > Why do you need the svc_xprt in the completion?
> 
> 1. The svc_xprt contains the svcrdma_xprt, which contains the Send Queue
> accounting mechanism. SQ accounting has to be updated for each completion:
> the completion indicates that the SQ entry used by this Work Request is
> now available, and that other callers waiting for enough SQEs can go ahead.
> 
> 2. When handling a Receive completion, the incoming RPC message is enqueued
> on the svc_xprt via svc_xprt_enqueue, unless RDMA Reads are needed.
> 
> 3. When handling a Read completion, the incoming RPC message is enqueued on
> the svc_xprt via svc_xprt_enqueue.
> 
> 
> > Can the xpo_detach method wait for any pending completions?
> 
> So the completions would call wake_up on a waitqueue, or use a kernel
> completion? That sounds more expensive than managing an atomic reference
> count.
> 
> I suppose some other reference count could be used to trigger a work item
> that would invoke xpo_detach.
> 
> But based on your comments, then, svc_xprt_put() was never intended to be
> BH-safe.

I'm not sure what was intended.  It doesn't look to me like any other
callers require it.

--b.

> 
> 
> > --b.
> > 
> >> 
> >> 
> >>>> However, xprt_switch_put() takes a spin lock (xps_lock) which is locked
> >>>> everywhere without disabling BHs.
> >>>> 
> >>>> It looks to me like 39a9beab5acb makes svc_xprt_put() no longer BH-safe?
> >>>> Not sure if svc_xprt_put() was intended to be BH-safe beforehand.
> >>>> 
> >>>> Maybe xprt_switch_put() could be invoked in ->xpo_free, but that seems
> >>>> like a temporary solution.
> >>> 
> >>> Since xpo_free is also called from svc_xprt_put that doesn't sound like
> >>> it would change anything.  Or do we not trunk over RDMA for some reason?
> >> 
> >> It's quite desirable to trunk NFS/RDMA on multiple connections, and it
> >> should work just like it does for TCP, but so far it's not been tested.
> 
> 
> --
> Chuck Lever
> 
> 

^ permalink raw reply

* Re: [PATCH] checkpatch: Fix warnings when --no-tree is used
From: Jeff Kirsher @ 2016-11-08 21:40 UTC (permalink / raw)
  To: apw, joe; +Cc: linux-kernel
In-Reply-To: <1477081625-47881-1-git-send-email-jeffrey.t.kirsher@intel.com>

[-- Attachment #1: Type: text/plain, Size: 1063 bytes --]

On Fri, 2016-10-21 at 13:27 -0700, Jeff Kirsher wrote:
> When users run checkpatch.pl with --no-tree option, $root is not
> defined, which causes an ugly warning that it is not initialized at
> line 764.  The test for whether or not a file is maintained should
> only be run when a kernel tree is defined (i.e. not when --no-tree
> option is being used)
> 
> Signed-off-by: Jeff Kirsher <jeffrey.t.kirsher@intel.com>
> ---
>  scripts/checkpatch.pl | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)

Ping? Joe? Andy?  Where are we at with this patch?

> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl
> index a8368d1..7a54978 100755
> --- a/scripts/checkpatch.pl
> +++ b/scripts/checkpatch.pl
> @@ -2352,7 +2352,7 @@ sub process {
>  			}
>  		}
>  
> -		if ($found_file) {
> +		if ($found_file && $tree) {
>  			if (is_maintained_obsolete($realfile)) {
>  				WARN("OBSOLETE",
>  				     "$realfile is marked as 'obsolete'
> in the MAINTAINERS hierarchy.  No unnecessary modifications please.\n");

[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 819 bytes --]

^ permalink raw reply

* [Buildroot] [PATCH 1/1] php-fpm: remove config comments to fix startup
From: Arnout Vandecappelle @ 2016-11-08 21:40 UTC (permalink / raw)
  To: buildroot
In-Reply-To: <1478634582-15683-1-git-send-email-fhunleth@troodon-software.com>



On 08-11-16 20:49, Frank Hunleth wrote:
> The comments in the Buildroot-provided php-fpm.conf would produce the
> following error when starting php-fpm:
> 
> ERROR: [/etc/php-fpm.conf:2] value is NULL for a ZEND_INI_PARSER_ENTRY
> 
> Removing the comments fixes the problem.
> 
> Signed-off-by: Frank Hunleth <fhunleth@troodon-software.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Regards,
 Arnout

> ---
>  package/php/php-fpm.conf | 3 ---
>  1 file changed, 3 deletions(-)
> 
> diff --git a/package/php/php-fpm.conf b/package/php/php-fpm.conf
> index b75a58f..88014b8 100644
> --- a/package/php/php-fpm.conf
> +++ b/package/php/php-fpm.conf
> @@ -1,9 +1,6 @@
>  [www]
> -# Only start children when there are requests to be processed
>  pm = ondemand
> -# Terminate them again after there haven't been any for 2 minutes
>  pm.process_idle_timeout = 120s
> -# Maximum number of children processing PHP requests concurrently
>  pm.max_children = 5
>  
>  listen = /var/run/php-fpm.sock
> 

-- 
Arnout Vandecappelle                          arnout at mind be
Senior Embedded Software Architect            +32-16-286500
Essensium/Mind                                http://www.mind.be
G.Geenslaan 9, 3001 Leuven, Belgium           BE 872 984 063 RPR Leuven
LinkedIn profile: http://www.linkedin.com/in/arnoutvandecappelle
GPG fingerprint:  7493 020B C7E3 8618 8DEC 222C 82EB F404 F9AC 0DDF

^ permalink raw reply

* Re: [PATCH 0/7] drm: add atomic state logging and debugfs
From: Sean Paul @ 2016-11-08 21:39 UTC (permalink / raw)
  To: Rob Clark; +Cc: dri-devel
In-Reply-To: <1478358492-30738-1-git-send-email-robdclark@gmail.com>

On Sat, Nov 5, 2016 at 11:08 AM, Rob Clark <robdclark@gmail.com> wrote:
> I realized that I had not re-sent this after updating from review
> comments, and adding kerneldoc.
>
> The drm/msm bits I can include in my msm-next pull-req for 4.10.  Just
> including them here to show example usage.
>
> There will be a minor conflict to resolve around drm_get_format_name(),
> depending on what the final solution there is.  But that should be
> trivial.  If needed I can rebase after that lands.  But would be nice
> if this ended up in drm-next for 4.10 so that I can land the drm/msm
> bits (and some later patches that use drm_printer to dump SMP state
> in debugfs and on error irqs)
>
> Rob Clark (7):
>   drm: helper macros to print composite types
>   drm: add helper for printing to log or seq_file
>   drm: add helpers to go from plane state to drm_rect
>   drm/atomic: add new drm_debug bit to dump atomic state
>   drm/atomic: add debugfs file to dump out atomic state
>   drm/msm/mdp5: add atomic_print_state support
>   drm/msm: module param to dump state on error irq
>

Applied the series to drm-misc, thanks

Sean



>  Documentation/gpu/drm-internals.rst       |  17 ++++
>  drivers/gpu/drm/Makefile                  |   2 +-
>  drivers/gpu/drm/drm_atomic.c              | 156 ++++++++++++++++++++++++++++++
>  drivers/gpu/drm/drm_debugfs.c             |   9 ++
>  drivers/gpu/drm/drm_modes.c               |   8 +-
>  drivers/gpu/drm/drm_plane_helper.c        |  11 +--
>  drivers/gpu/drm/drm_print.c               |  54 +++++++++++
>  drivers/gpu/drm/drm_rect.c                |  11 +--
>  drivers/gpu/drm/i915/intel_display.c      |  10 +-
>  drivers/gpu/drm/i915/intel_sprite.c       |  11 +--
>  drivers/gpu/drm/msm/mdp/mdp4/mdp4_irq.c   |  10 ++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_irq.c   |  11 +++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_kms.h   |  12 +++
>  drivers/gpu/drm/msm/mdp/mdp5/mdp5_plane.c |  18 +++-
>  drivers/gpu/drm/msm/msm_drv.c             |   4 +
>  include/drm/drmP.h                        |  22 +++++
>  include/drm/drm_atomic.h                  |   7 ++
>  include/drm/drm_connector.h               |  13 +++
>  include/drm/drm_crtc.h                    |  13 +++
>  include/drm/drm_plane.h                   |  36 +++++++
>  include/drm/drm_print.h                   | 117 ++++++++++++++++++++++
>  21 files changed, 508 insertions(+), 44 deletions(-)
>  create mode 100644 drivers/gpu/drm/drm_print.c
>  create mode 100644 include/drm/drm_print.h
>
> --
> 2.7.4
>
_______________________________________________
dri-devel mailing list
dri-devel@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/dri-devel

^ permalink raw reply

* RE: GPF in sidtab_context_to_sid
From: Roberts, William C @ 2016-11-08 21:39 UTC (permalink / raw)
  To: Paul Moore; +Cc: selinux@tycho.nsa.gov
In-Reply-To: <CAHC9VhS5Fg1qegytKsG5Tz6MvO=pLxn3qmzDsvkcVhC5jx7d3g@mail.gmail.com>



> -----Original Message-----
> From: Paul Moore [mailto:paul@paul-moore.com]
> Sent: Tuesday, November 8, 2016 12:57 PM
> To: Roberts, William C <william.c.roberts@intel.com>
> Cc: selinux@tycho.nsa.gov
> Subject: Re: GPF in sidtab_context_to_sid
> 
> On Tue, Nov 8, 2016 at 1:26 PM, Roberts, William C <william.c.roberts@intel.com>
> wrote:
> > I found a very similar oops online:
> >
> > http://oops.kernel.org/oops/general-protection-fault-in-sidtab_context
> > _to_sid/
> >
> > Has anyone encountered this bug?
> >
> > I had something reported to me very similar where the faulting
> > instruction
> > was:
> >
> > 0xffffffff8133c81e <+174>:   mov    0x14(%r12),%eax
> >
> > Addr2line on vmlinux produced:
> >
> > $ addr2line -f -e ./vmlinux ffffffff8133c81e context_cmp
> > kernel/cht/security/selinux/ss/context.h:152
> 
> I'm guessing you don't have a reproducer?

Supposedly, I am digging that slowly out of the reporters. If I can use it to
reproduce, I'll let you know.

> 
> It looks like both these kernels are older (3.x), have you seen this on anything
> recent?

No.

> 
> --
> paul moore
> www.paul-moore.com

^ permalink raw reply

* Re: [PATCH v2] fs/nfsd/nfs4callback: Remove deprecated create_singlethread_workqueue
From: J. Bruce Fields @ 2016-11-08 21:39 UTC (permalink / raw)
  To: Bhaktipriya Shridhar; +Cc: Jeff Layton, Tejun Heo, linux-nfs, linux-kernel
In-Reply-To: <20160830205348.GA31915@Karyakshetra>

Apologies, just cleaning out old mail and finding some I should have
responded to long ago:

On Wed, Aug 31, 2016 at 02:23:48AM +0530, Bhaktipriya Shridhar wrote:
> The workqueue "callback_wq" queues a single work item &cb->cb_work per
> nfsd4_callback instance and thus, it doesn't require execution ordering.

What's "execution ordering"?

We definitely do depend on the fact that at most one of these is running
at a time.

--b.

> Hence, alloc_workqueue has been used to replace the
> deprecated create_singlethread_workqueue instance.

> 
> The WQ_MEM_RECLAIM flag has not been set since this is an in-kernel nfs
> server and isn't involved in memory reclaim operations on the local
> host.
> 
> Since there are fixed number of work items, explicit concurrency
> limit is unnecessary here.
> 
> Signed-off-by: Bhaktipriya Shridhar <bhaktipriya96@gmail.com>
> ---
>  Changes in v2:
> 	- No change. Made this a separate patch (categorised based on
> 	  directories).
> 
>  fs/nfsd/nfs4callback.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/fs/nfsd/nfs4callback.c b/fs/nfsd/nfs4callback.c
> index 7389cb1..a6611c6 100644
> --- a/fs/nfsd/nfs4callback.c
> +++ b/fs/nfsd/nfs4callback.c
> @@ -1021,7 +1021,7 @@ static const struct rpc_call_ops nfsd4_cb_ops = {
> 
>  int nfsd4_create_callback_queue(void)
>  {
> -	callback_wq = create_singlethread_workqueue("nfsd4_callbacks");
> +	callback_wq = alloc_workqueue("nfsd4_callbacks", 0, 0);
>  	if (!callback_wq)
>  		return -ENOMEM;
>  	return 0;
> --
> 2.1.4

^ permalink raw reply

* Re: [v4.9-rc4] dvb-usb/cinergyT2 NULL pointer dereference
From: Mauro Carvalho Chehab @ 2016-11-08 21:38 UTC (permalink / raw)
  To: Benjamin Larsson
  Cc: Linus Torvalds, Jörg Otte, Patrick Boettcher,
	Mauro Carvalho Chehab, Linux Kernel Mailing List,
	Linux Media Mailing List
In-Reply-To: <354bc87c-79a1-bb37-6225-988c8fa429a5@southpole.se>

Em Tue, 8 Nov 2016 22:15:24 +0100
Benjamin Larsson <benjamin@southpole.se> escreveu:

> On 11/08/2016 09:22 PM, Mauro Carvalho Chehab wrote:
> > Em Tue, 8 Nov 2016 10:42:03 -0800
> > Linus Torvalds <torvalds@linux-foundation.org> escreveu:
> >
> >> On Sun, Nov 6, 2016 at 7:40 AM, Jörg Otte <jrg.otte@gmail.com> wrote:
> >>> Since v4.9-rc4 I get following crash in dvb-usb-cinergyT2 module.
> >>
> >> Looks like it's commit 5ef8ed0e5608f ("[media] cinergyT2-core: don't
> >> do DMA on stack"), which movced the DMA data array from the stack to
> >> the "private" pointer. In the process it also added serialization in
> >> the form of "data_mutex", but and now it oopses on that mutex because
> >> the private pointer is NULL.
> >>
> >> It looks like the "->private" pointer is allocated in dvb_usb_adapter_init()
> >>
> >> cinergyt2_usb_probe ->
> >>   dvb_usb_device_init ->
> >>     dvb_usb_init() ->
> >>       dvb_usb_adapter_init()
> >>
> >> but the dvb_usb_init() function calls dvb_usb_device_power_ctrl()
> >> (which calls the "power_ctrl" function, which is
> >> cinergyt2_power_ctrl() for that drive) *before* it initializes the
> >> private field.
> >>
> >> Mauro, Patrick, could dvb_usb_adapter_init() be called earlier, perhaps?
> >
> > Calling it earlier won't work, as we need to load the firmware before
> > sending the power control commands on some devices.
> >
> > Probably the best here is to pass an extra optional function parameter
> > that will initialize the mutex before calling any functions.
> >
> > Btw, if it broke here, the DMA fixes will likely break on other drivers.
> > So, after Jörg tests this patch, I'll work on a patch series addressing
> > this issue on the other drivers I touched.
> >
> > Regards,
> > Mauro
> 
> Just for reference I got the following call trace a week ago. I looks 
> like this confirms that other drivers are affected also.

Yeah, I avoided serializing the logic that detects if the firmware is
loaded, but forgot that the power control had the same issue. The
newer dvb usb drivers use the dvb-usb-v2, so I didn't touch this
code for a while.

I'll need to review all touched drivers to be sure that they'll all
do the right thing. The good news is that it will likely simplify
the drivers' logic a little bit.

Thanks,
Mauro

^ permalink raw reply

* Re: [PATCH v4 8/8] iio: envelope-detector: ADC driver based on a DAC and a comparator
From: Peter Rosin @ 2016-11-08 17:03 UTC (permalink / raw)
  To: Thomas Gleixner
  Cc: linux-kernel, Jonathan Cameron, Hartmut Knaack,
	Lars-Peter Clausen, Peter Meerwald-Stadler, Rob Herring,
	Mark Rutland, Daniel Baluta, Slawomir Stepien, linux-iio,
	devicetree
In-Reply-To: <alpine.DEB.2.20.1611081630450.3501@nanos>

On 2016-11-08 16:59, Thomas Gleixner wrote:
> On Tue, 8 Nov 2016, Peter Rosin wrote:
>> +/*
>> + * The envelope_detector_comp_latch function works together with the compare
>> + * interrupt service routine below (envelope_detector_comp_isr) as a latch
>> + * (one-bit memory) for if the interrupt has triggered since last calling
>> + * this function.
>> + * The ..._comp_isr function disables the interrupt so that the cpu does not
>> + * need to service a possible interrupt flood from the comparator when no-one
>> + * cares anyway, and this ..._comp_latch function reenables them again if
>> + * needed.
>> + */
>> +static int envelope_detector_comp_latch(struct envelope *env)
>> +{
>> +	int comp;
>> +
>> +	spin_lock_irq(&env->comp_lock);
>> +	comp = env->comp;
>> +	env->comp = 0;
>> +	spin_unlock_irq(&env->comp_lock);
>> +
>> +	if (!comp)
>> +		return 0;
>> +
>> +	/*
>> +	 * The irq was disabled, and is reenabled just now.
>> +	 * But there might have been a pending irq that
>> +	 * happened while the irq was disabled that fires
>> +	 * just as the irq is reenabled. That is not what
>> +	 * is desired.
>> +	 */
>> +	enable_irq(env->comp_irq);
>> +
>> +	/* So, synchronize this possibly pending irq... */
>> +	synchronize_irq(env->comp_irq);
>> +
>> +	/* ...and redo the whole dance. */
>> +	spin_lock_irq(&env->comp_lock);
>> +	comp = env->comp;
>> +	env->comp = 0;
>> +	spin_unlock_irq(&env->comp_lock);
>> +
>> +	if (comp)
>> +		enable_irq(env->comp_irq);
> 
> So you need that whole dance including the delayed work because you cannot
> call iio_write_channel_raw() from hard interrupt context, right?

It's not the "cannot call from hard irq context" that made me do that, it's...

> So you might just register a threaded interrupt handler, which should make
> this whole thing way simpler.
> 
>      devm_request_threaded_irq(dev, irq, NULL, your_isr, IRQF_ONESHOT, ...);
> 
> The core will mask the interrupt line until the threaded handler is
> finished. The threaded handler is invoked with preemption enabled, so you
> can sleep there as long as you want. So you can do everything in your
> handler and the above dance is just not required.

...that I couldn't work out how to reenable a oneshot irq once it had fired,
short of freeing the irq and requesting it again. That seemed entirely
bogus, the driver shouldn't risk losing a resource like that so I don't know
what I didn't see? Or maybe it was that I had a hard time resolving the race
between the irq and the timeout in a nice way. I honestly don't remember
why exactly I abandoned oneshot irqs, but this enable/sync/enable dance
was much nicer than what I came up with for the oneshot irq solution I
originally worked on.

Or maybe I had problems with the possibly pending irq also when using a
oneshot irq, but didn't realize it? That was something I discovered quite
late in the process, some time after moving away from oneshot irqs. Are
pending irqs cleared when requesting (or reenabling, however that is done)
a oneshot irq?

Anyway, I do not want the interrupt to be serviced when no one is interested,
since I'm afraid that nasty input might generate a flood of interrupts that
might disturb other things that the cpu is doing. Which means that I need
to enable/disable the interrupt as needed.

However, what *I* thought Jonathan wanted input on was the part where the
interrupt edge/level is flipped when requesting "inverted" signals in
envelope_store_invert(). That could perhaps be seen as unorthodox and in
need of more eyes?

Cheers,
Peter

^ permalink raw reply

* Re: [alsa-devel] [PATCH] ASoC: fsl: fix fsl_spdif.c build errors
From: Kees Cook @ 2016-11-08 21:38 UTC (permalink / raw)
  To: Randy Dunlap
  Cc: Fabio Estevam, Timur Tabi, Nicolin Chen, Xiubo Li, Fabio Estevam,
	Liam Girdwood, Mark Brown, Geliang Tang,
	moderated for non-subscribers, LKML
In-Reply-To: <42b77fd8-4f54-2638-9110-f2fe0d52f629@infradead.org>

On Sun, Nov 6, 2016 at 1:40 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
> On 11/06/16 11:43, Fabio Estevam wrote:
>> Hi Randy,
>>
>> On Sat, Nov 5, 2016 at 9:16 PM, Randy Dunlap <rdunlap@infradead.org> wrote:
>>> From: Randy Dunlap <rdunlap@infradead.org>
>>>
>>> Fix build errors in sound/soc/fsl/fsl_spdif.c by selecting BITREVERSE.
>>> Fixes these build errors:
>>
>> Care to explain why the error is happening?
>
> The driver uses bitreverse functions but does not select BITREVERSE
> in its kconfig description, so the bitrev functions are not built
> into the kernel.  In the kbuild robot supplied config file,
> FSL_SPDIF=y and BITREVERSE=m, which causes the build error.
> By having this driver select BITREVERSE, CONFIG_BITREVERSE=y so there
> is no build error.
>
>>> sound/built-in.o: In function `spdif_write_channel_status':
>>> fsl_spdif.c:(.text+0xbe39d): undefined reference to `byte_rev_table'
>>> fsl_spdif.c:(.text+0xbe3a8): undefined reference to `byte_rev_table'
>>> fsl_spdif.c:(.text+0xbe3be): undefined reference to `byte_rev_table'
>>> fsl_spdif.c:(.text+0xbe3d8): undefined reference to `byte_rev_table'
>>>
>>> Signed-off-by: Randy Dunlap <rdunlap@infradead.org>
>>> Reported-by: kbuild test robot <fengguang.wu@intel.com>
>>> Applies-to: all 3.x, all 4.x
>>
>> Not sure if this last line is correct. The kbuild report says this
>> error starts to happen with:
>> commit: 8cfc8ddc99df9509a46043b14af81f5c6a223eab pstore: add lzo/lz4
>> compression support
>
> That commit makes no sense to me regarding this build error.
>
> This driver uses bitrev functions even way back in 3.x so this
> patch could be applied to many stable kernel versions.

Yeah, I scratched my head on this too, but noted that the 0-day report
was from a 5 month old build, so I kind of assumed it was an 0-day
glitch.

-Kees

>
>> on a x86_64 randconfig.
>>
>> Also, why only SND_SOC_FSL_SPDIF needs to have BITREVERSE selected?
>
> It's the only source file in sound/soc/fsl/ that uses bitreverse functions.
>
>
> --
> ~Randy



-- 
Kees Cook
Nexus Security

^ permalink raw reply

* Re: [PATCH 0/3] gitk: memory consumption improvements
From: Junio C Hamano @ 2016-11-08 21:37 UTC (permalink / raw)
  To: Markus Hitter; +Cc: git@vger.kernel.org, Paul Mackerras
In-Reply-To: <de7cd593-0c10-4e93-1681-7e123504f5d5@jump-ing.de>

Markus Hitter <mah@jump-ing.de> writes:

> List, Paul,
>
> after searching for a while on why Gitk sometimes consumes
> exorbitant amounts of memory I found a pair of minor issues and
> also a big one: the text widget comes with an unlimited undo
> manager, which is turned on be default. Considering that each line
> is inserted seperately, this piles up a huuuge undo stack ... for
> a read-only text widget. Simply turning off this undo manager
> saves about 95% of memory when viewing large commits (with tens of
> thousands of diff lines).

You made me laugh crazy hard while in a waiting room in a clinic
yesterday with the cover letter; people around gave me a strange
look but I couldn't help.

This is a single liner with the largest gain in the history of this
project.  Very well spotted.

Are all semi-modern Tcl/Tk in service have this -undo thing so that
we can pass unconditionally to the text widget like the patch does?

If we had to do this conditionally, that robs the fun of "just
adding 8 bytes to the source to reduce 1+GB memory consumption", but
even if we had to go conditional, this is a great find.

Well done.

^ permalink raw reply

* [PATCH] proc: optimize render_sigset_t()
From: Andrei Vagin @ 2016-11-08 21:37 UTC (permalink / raw)
  To: linux-kernel; +Cc: Andrei Vagin, Andrew Morton, Alexey Dobriyan

render_sigset_t() requires about 30% of time to generate
/proc/pid/status.

- 74.44% sys_read
 - 74.40% vfs_read
    - 74.01% __vfs_read
       - 73.36% seq_read
          - 72.97% proc_single_show
             - 72.26% proc_pid_status
                + 29.79% render_sigset_t
                + 11.47% task_mem
                + 5.60% render_cap_t
                + 4.95% seq_printf
                + 4.28% cpuset_task_status_allowed

seq_printf is called for each symbol of a signal mask. This patch
collect a whole mask in a buffer and prints it for one call of
seq_puts().

  - 65.02% proc_single_show
     - 63.75% proc_pid_status
        + 15.73% task_mem
        + 7.42% render_sigset_t
        + 7.39% render_cap_t
        + 6.46% cpuset_task_status_allowed

/proc/pid/status is generated 25% faster with this optimization.

Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Alexey Dobriyan <adobriyan@gmail.com>
Signed-off-by: Andrei Vagin <avagin@openvz.org>
---
 fs/proc/array.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/fs/proc/array.c b/fs/proc/array.c
index 81818ad..0190c3e 100644
--- a/fs/proc/array.c
+++ b/fs/proc/array.c
@@ -232,11 +232,13 @@ static inline void task_state(struct seq_file *m, struct pid_namespace *ns,
 void render_sigset_t(struct seq_file *m, const char *header,
 				sigset_t *set)
 {
-	int i;
+	char buf[_NSIG / 4 + 2];
+	int i, j;
 
 	seq_puts(m, header);
 
 	i = _NSIG;
+	j = 0;
 	do {
 		int x = 0;
 
@@ -245,10 +247,13 @@ void render_sigset_t(struct seq_file *m, const char *header,
 		if (sigismember(set, i+2)) x |= 2;
 		if (sigismember(set, i+3)) x |= 4;
 		if (sigismember(set, i+4)) x |= 8;
-		seq_printf(m, "%x", x);
+		buf[j++] = hex_asc[x];
 	} while (i >= 4);
 
-	seq_putc(m, '\n');
+	buf[j++] = '\n';
+	buf[j++] = 0;
+
+	seq_puts(m, buf);
 }
 
 static void collect_sigign_sigcatch(struct task_struct *p, sigset_t *ign,
-- 
2.7.4

^ permalink raw reply related

* Re: [RFC PATCH net-next] net: ethtool: add support for forward error correction modes
From: Vidya Sagar Ravipati @ 2016-11-08 21:33 UTC (permalink / raw)
  To: Gal Pressman
  Cc: David Miller, John W. Linville, Saeed Mahameed, Gal Pressman,
	netdev, Oded Wertheim, Ariel Almog, Yuval Mintz, Tzahi Oved,
	Roee Shapiro, Aviad Raveh, Dustin Byford, Roopa Prabhu
In-Reply-To: <25b7fce6-49f9-bd54-5c12-c72298d107c1@gmail.com>

On Thu, Nov 3, 2016 at 6:24 AM, Gal Pressman <galp.dev@gmail.com> wrote:
>
>
> On 25/10/2016 05:50, Vidya Sagar Ravipati wrote:
>> SET FEC option:
>> root@tor: ethtool --set-fec  swp1 encoding [off | RS | BaseR | auto] autoneg [off | on]
>>
>> Encoding: Types of encoding
>> Off    :  Turning off any encoding
>> RS     :  enforcing RS-FEC encoding on supported speeds
>> BaseR  :  enforcing Base R encoding on supported speeds
>> Auto   :  Default FEC settings  for  divers , and would represent
>
> divers? :)
Drivers :)
>
>>           asking the hardware to essentially go into a best effort mode.
>>
>> Here are a few examples of what we would expect if encoding=auto:
>> - if autoneg is on, we are  expecting FEC to be negotiated as on or off
>>   as long as protocol supports it
>> - if the hardware is capable of detecting the FEC encoding on it's
>>       receiver it will reconfigure its encoder to match
>> - in absence of the above, the configuration would be set to IEEE
>>   defaults.
>
> Not sure I follow, why do we need an autoneg option if encoding type can be set to auto?
Auto is one of the FEC configuration modes  which indicates  the
drivers to set the IEEE defaults based on speed/duplex combination of
the port. i.e. RS FEC mode will be set in case of 100G/full

Auto negotiation is the configuration for the link  to negotiate the
FEC capabilities and different FEC modes with other endpoint  using
encoded bits D44:47 in base link code word.

^ permalink raw reply

* [iproute PATCH 1/2] ipaddress: Simplify vf_info parsing
From: Phil Sutter @ 2016-11-08 21:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20161108212912.27258-1-phil@nwl.cc>

Commit 7b8179c780a1a ("iproute2: Add new command to ip link to
enable/disable VF spoof check") tried to add support for
IFLA_VF_SPOOFCHK in a backwards-compatible manner, but aparently overdid
it: parse_rtattr_nested() handles missing attributes perfectly fine in
that it will leave the relevant field unassigned so calling code can
just compare against NULL. There is no need to layback from the previous
(IFLA_VF_TX_RATE) attribute to the next to check if IFLA_VF_SPOOFCHK is
present or not. To the contrary, it establishes a potentially incorrect
assumption of these two attributes directly following each other which
may not be the case (although up to now, kernel aligns them this way).

This patch cleans up the code to adhere to the common way of checking
for attribute existence. It has been tested to return correct results
regardless of whether the kernel exports IFLA_VF_SPOOFCHK or not.

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ipaddress.c | 44 ++++++++++----------------------------------
 1 file changed, 10 insertions(+), 34 deletions(-)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index 7f05258f43453..df0f1b9c94c58 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -322,10 +322,7 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
 {
 	struct ifla_vf_mac *vf_mac;
 	struct ifla_vf_tx_rate *vf_tx_rate;
-	struct ifla_vf_spoofchk *vf_spoofchk;
-	struct ifla_vf_link_state *vf_linkstate;
 	struct rtattr *vf[IFLA_VF_MAX + 1] = {};
-	struct rtattr *tmp;
 
 	SPRINT_BUF(b1);
 
@@ -339,31 +336,6 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
 	vf_mac = RTA_DATA(vf[IFLA_VF_MAC]);
 	vf_tx_rate = RTA_DATA(vf[IFLA_VF_TX_RATE]);
 
-	/* Check if the spoof checking vf info type is supported by
-	 * this kernel.
-	 */
-	tmp = (struct rtattr *)((char *)vf[IFLA_VF_TX_RATE] +
-				vf[IFLA_VF_TX_RATE]->rta_len);
-
-	if (tmp->rta_type != IFLA_VF_SPOOFCHK)
-		vf_spoofchk = NULL;
-	else
-		vf_spoofchk = RTA_DATA(vf[IFLA_VF_SPOOFCHK]);
-
-	if (vf_spoofchk) {
-		/* Check if the link state vf info type is supported by
-		 * this kernel.
-		 */
-		tmp = (struct rtattr *)((char *)vf[IFLA_VF_SPOOFCHK] +
-					vf[IFLA_VF_SPOOFCHK]->rta_len);
-
-		if (tmp->rta_type != IFLA_VF_LINK_STATE)
-			vf_linkstate = NULL;
-		else
-			vf_linkstate = RTA_DATA(vf[IFLA_VF_LINK_STATE]);
-	} else
-		vf_linkstate = NULL;
-
 	fprintf(fp, "%s    vf %d MAC %s", _SL_, vf_mac->vf,
 		ll_addr_n2a((unsigned char *)&vf_mac->mac,
 			    ETH_ALEN, 0, b1, sizeof(b1)));
@@ -407,14 +379,18 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
 		if (vf_rate->min_tx_rate)
 			fprintf(fp, ", min_tx_rate %dMbps", vf_rate->min_tx_rate);
 	}
+	if (vf[IFLA_VF_SPOOFCHK]) {
+		struct ifla_vf_spoofchk *vf_spoofchk =
+			RTA_DATA(vf[IFLA_VF_SPOOFCHK]);
 
-	if (vf_spoofchk && vf_spoofchk->setting != -1) {
-		if (vf_spoofchk->setting)
-			fprintf(fp, ", spoof checking on");
-		else
-			fprintf(fp, ", spoof checking off");
+		if (vf_spoofchk->setting != -1)
+			fprintf(fp, ", spoof checking %s",
+			        vf_spoofchk->setting ? "on" : "off");
 	}
-	if (vf_linkstate) {
+	if (vf[IFLA_VF_LINK_STATE]) {
+		struct ifla_vf_link_state *vf_linkstate =
+			RTA_DATA(vf[IFLA_VF_LINK_STATE]);
+
 		if (vf_linkstate->link_state == IFLA_VF_LINK_STATE_AUTO)
 			fprintf(fp, ", link-state auto");
 		else if (vf_linkstate->link_state == IFLA_VF_LINK_STATE_ENABLE)
-- 
2.10.0

^ permalink raw reply related

* [iproute PATCH 2/2] ipaddress: Print IFLA_VF_QUERY_RSS_EN setting
From: Phil Sutter @ 2016-11-08 21:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev
In-Reply-To: <20161108212912.27258-1-phil@nwl.cc>

Signed-off-by: Phil Sutter <phil@nwl.cc>
---
 ip/ipaddress.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/ip/ipaddress.c b/ip/ipaddress.c
index df0f1b9c94c58..c9f769fb748e4 100644
--- a/ip/ipaddress.c
+++ b/ip/ipaddress.c
@@ -405,6 +405,14 @@ static void print_vfinfo(FILE *fp, struct rtattr *vfinfo)
 			fprintf(fp, ", trust %s",
 			        vf_trust->setting ? "on" : "off");
 	}
+	if (vf[IFLA_VF_RSS_QUERY_EN]) {
+		struct ifla_vf_rss_query_en *rss_query =
+			RTA_DATA(vf[IFLA_VF_RSS_QUERY_EN]);
+
+		if (rss_query->setting != -1)
+			fprintf(fp, ", query_rss %s",
+			        rss_query->setting ? "on" : "off");
+	}
 	if (vf[IFLA_VF_STATS] && show_stats)
 		print_vf_stats64(fp, vf[IFLA_VF_STATS]);
 }
-- 
2.10.0

^ permalink raw reply related

* [iproute PATCH 0/2] Resend: Simplify and enhance vf_info parsing
From: Phil Sutter @ 2016-11-08 21:29 UTC (permalink / raw)
  To: Stephen Hemminger; +Cc: netdev

This patch series got lost in a discussion about whether the code the
first patch removes is necessary or not - static analysis as well as my
tests showed it is not. Therefore resending this with updated
description of patch 1 to contain the discussion's gist.

Phil Sutter (2):
  ipaddress: Simplify vf_info parsing
  ipaddress: Print IFLA_VF_QUERY_RSS_EN setting

 ip/ipaddress.c | 52 ++++++++++++++++++----------------------------------
 1 file changed, 18 insertions(+), 34 deletions(-)

-- 
2.10.0

^ permalink raw reply

* Re: [Qemu-devel] [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP
From: Alex Williamson @ 2016-11-08 21:28 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: pbonzini, kraxel, cjia, qemu-devel, kvm, kevin.tian, jike.song,
	bjsdjshi, linux-kernel
In-Reply-To: <675f0d46-aa2b-7404-701e-d9ce17b64549@nvidia.com>

On Wed, 9 Nov 2016 01:29:19 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 11/8/2016 11:16 PM, Alex Williamson wrote:
> > On Tue, 8 Nov 2016 21:56:29 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >   
> >> On 11/8/2016 5:15 AM, Alex Williamson wrote:  
> >>> On Sat, 5 Nov 2016 02:40:45 +0530
> >>> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >>>     
> >> ...  
> >>>>  
> >>>> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)    
> >>>
> >>> Is the expectation here that this is a generic notifier for all
> >>> vfio->mdev signaling?  That should probably be made clear in the mdev
> >>> API to avoid vendor drivers assuming their notifier callback only
> >>> occurs for unmaps, even if that's currently the case.
> >>>     
> >>
> >> Ok. Adding comment about notifier callback in mdev_device which is part
> >> of next patch.
> >>
> >> ...
> >>  
> >>>>  	mutex_lock(&iommu->lock);
> >>>>  
> >>>> -	if (!iommu->external_domain) {
> >>>> +	/* Fail if notifier list is empty */
> >>>> +	if ((!iommu->external_domain) || (!iommu->notifier.head)) {
> >>>>  		ret = -EINVAL;
> >>>>  		goto pin_done;
> >>>>  	}
> >>>> @@ -867,6 +870,11 @@ unlock:
> >>>>  	/* Report how much was unmapped */
> >>>>  	unmap->size = unmapped;
> >>>>  
> >>>> +	if (unmapped && iommu->external_domain)
> >>>> +		blocking_notifier_call_chain(&iommu->notifier,
> >>>> +					     VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> >>>> +					     unmap);    
> >>>
> >>> This is after the fact, there's already a gap here where pages are
> >>> unpinned and the mdev device is still running.    
> >>
> >> Oh, there is a bug here, now unpin_pages() take user_pfn as argument and
> >> find vfio_dma. If its not found, it doesn't unpin pages. We have to call
> >> this notifier before vfio_remove_dma(). But if we call this before
> >> vfio_remove_dma() there will be deadlock since iommu->lock is already
> >> held here and vfio_iommu_type1_unpin_pages() will also try to hold
> >> iommu->lock.
> >> If we want to call blocking_notifier_call_chain() before
> >> vfio_remove_dma(), sequence should be:
> >>
> >> unmapped += dma->size;
> >> mutex_unlock(&iommu->lock);
> >> if (iommu->external_domain)) {
> >> 	struct vfio_iommu_type1_dma_unmap nb_unmap;
> >>
> >> 	nb_unmap.iova = dma->iova;
> >> 	nb_unmap.size = dma->size;
> >> 	blocking_notifier_call_chain(&iommu->notifier,
> >>         	                     VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> >>                 	             &nb_unmap);
> >> }
> >> mutex_lock(&iommu->lock);
> >> vfio_remove_dma(iommu, dma);  
> > 
> > It seems like it would be worthwhile to have the rb-tree rooted in the
> > vfio-dma, then we only need to call the notifier if there are pages
> > pinned within that vfio-dma (ie. the rb-tree is not empty).  We can
> > then release the lock call the notifier, re-acquire the lock, and
> > BUG_ON if the rb-tree still is not empty.  We might get duplicate pfns
> > between separate vfio_dma structs, but as I mentioned in other replies,
> > that seems like an exception that we don't need to optimize for.
> >   
> 
> If we don't optimize for the case where iova from different vfio_dma are
> mapped to same pfn and we would not consider this case for page
> accounting then:

Just to clarify, the current code (not handling mdevs) will pin and do
page accounting per iova, regardless of whether the iova translates to a
unique pfn.  As long as we do no worse than that, I'm ok.

> - have rb tree of pinned iova, where key would be iova, in each vfio_dma
> structure.
> - iova tracking structure would have iova and ref_count only.
> - page accounting would only count number of iova's in rb_tree, case
> where different iova could map to same pfn would not be considered in
> this implementation for now.
> - vfio_unpin_pages() would have user_pfn and pfn as input, we would
> validate that iova exist in rb tree and trust vendor driver that
> corresponding pfn is correct, there is no validation of pfn. If want
> validate pfn, call GUP, verify pfn and call put_pfn().
> - In .release() or .detach_group() path, if there are entries in this rb
> tree, call GUP again using that iova, get pfn and then call
> put_pfn(pfn) for ref_count+1 times. This is because we are not keeping
> pfn in our tracking logic.

Wait a sec, if we detach a group from the container and it's not the
last group in the container (which would trigger a release), we can't
assume anything about which vfio_dma entries were associated with that
device.  The vendor driver, through the release of the device(s) within
that group, needs to unpin.  In a container release, we need to send a
notifier to the vendor driver(s) to cause an unpin.  This is the only
mechanism we have to ensure that vendor drivers are not leaking
references.  If during the release, after the notifier, if any
vfio_pfns remain, we need to BUG_ON, just like we need to do for any
other DMA_UNMAP.

Also, I'll say it again, I also don't like this API of passing around
potentially giant arrays, and especially the API of relying on the
vendor driver to tell us an arbitrary pfn to unpin.  If we make the
assumption that vendor drivers do not pin lots and lots of memory,
perhaps we could use a struct vfio_pfn as:

struct vfio_pfn {
        struct rb_node          node;
	dma_addr_t		iova; /* key */
        unsigned long           pfn;
        atomic_t                ref_count;
};

This puts us at 44-bytes per pfn, which isn't great, but I think it
puts us in a better position with the API that we could make use of a
page-table or sparse array in the future that would eliminate the
rb_node and make the iova implicit in the location of the data
structure.  That would leave only the pfn and ref_count, which could
potentially be combined into a single 8-byte field if we had per
vfio_dma (or higher) locking to avoid the atomic_t (and we're happy that
the reference count is always less than PAGE_SIZE, ie. we could fail
pinning if we get to that point).

That would allow for the unpin call to not provide the pfn, so could we
then look at whether we need the batching provided by the iova array at
all?  I don't have a feel for the size of memory that gets pinned by
the vendor driver, the frequency of pinning, or whether usage of
hugepages for the guest is likely to translate into contiguous memory
requests through this API.  What's your feeling?  Thanks,

Alex

^ permalink raw reply

* Re: [PATCH v11 11/22] vfio iommu: Add blocking notifier to notify DMA_UNMAP
From: Alex Williamson @ 2016-11-08 21:28 UTC (permalink / raw)
  To: Kirti Wankhede
  Cc: pbonzini, kraxel, cjia, qemu-devel, kvm, kevin.tian, jike.song,
	bjsdjshi, linux-kernel
In-Reply-To: <675f0d46-aa2b-7404-701e-d9ce17b64549@nvidia.com>

On Wed, 9 Nov 2016 01:29:19 +0530
Kirti Wankhede <kwankhede@nvidia.com> wrote:

> On 11/8/2016 11:16 PM, Alex Williamson wrote:
> > On Tue, 8 Nov 2016 21:56:29 +0530
> > Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >   
> >> On 11/8/2016 5:15 AM, Alex Williamson wrote:  
> >>> On Sat, 5 Nov 2016 02:40:45 +0530
> >>> Kirti Wankhede <kwankhede@nvidia.com> wrote:
> >>>     
> >> ...  
> >>>>  
> >>>> +int vfio_register_notifier(struct device *dev, struct notifier_block *nb)    
> >>>
> >>> Is the expectation here that this is a generic notifier for all
> >>> vfio->mdev signaling?  That should probably be made clear in the mdev
> >>> API to avoid vendor drivers assuming their notifier callback only
> >>> occurs for unmaps, even if that's currently the case.
> >>>     
> >>
> >> Ok. Adding comment about notifier callback in mdev_device which is part
> >> of next patch.
> >>
> >> ...
> >>  
> >>>>  	mutex_lock(&iommu->lock);
> >>>>  
> >>>> -	if (!iommu->external_domain) {
> >>>> +	/* Fail if notifier list is empty */
> >>>> +	if ((!iommu->external_domain) || (!iommu->notifier.head)) {
> >>>>  		ret = -EINVAL;
> >>>>  		goto pin_done;
> >>>>  	}
> >>>> @@ -867,6 +870,11 @@ unlock:
> >>>>  	/* Report how much was unmapped */
> >>>>  	unmap->size = unmapped;
> >>>>  
> >>>> +	if (unmapped && iommu->external_domain)
> >>>> +		blocking_notifier_call_chain(&iommu->notifier,
> >>>> +					     VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> >>>> +					     unmap);    
> >>>
> >>> This is after the fact, there's already a gap here where pages are
> >>> unpinned and the mdev device is still running.    
> >>
> >> Oh, there is a bug here, now unpin_pages() take user_pfn as argument and
> >> find vfio_dma. If its not found, it doesn't unpin pages. We have to call
> >> this notifier before vfio_remove_dma(). But if we call this before
> >> vfio_remove_dma() there will be deadlock since iommu->lock is already
> >> held here and vfio_iommu_type1_unpin_pages() will also try to hold
> >> iommu->lock.
> >> If we want to call blocking_notifier_call_chain() before
> >> vfio_remove_dma(), sequence should be:
> >>
> >> unmapped += dma->size;
> >> mutex_unlock(&iommu->lock);
> >> if (iommu->external_domain)) {
> >> 	struct vfio_iommu_type1_dma_unmap nb_unmap;
> >>
> >> 	nb_unmap.iova = dma->iova;
> >> 	nb_unmap.size = dma->size;
> >> 	blocking_notifier_call_chain(&iommu->notifier,
> >>         	                     VFIO_IOMMU_NOTIFY_DMA_UNMAP,
> >>                 	             &nb_unmap);
> >> }
> >> mutex_lock(&iommu->lock);
> >> vfio_remove_dma(iommu, dma);  
> > 
> > It seems like it would be worthwhile to have the rb-tree rooted in the
> > vfio-dma, then we only need to call the notifier if there are pages
> > pinned within that vfio-dma (ie. the rb-tree is not empty).  We can
> > then release the lock call the notifier, re-acquire the lock, and
> > BUG_ON if the rb-tree still is not empty.  We might get duplicate pfns
> > between separate vfio_dma structs, but as I mentioned in other replies,
> > that seems like an exception that we don't need to optimize for.
> >   
> 
> If we don't optimize for the case where iova from different vfio_dma are
> mapped to same pfn and we would not consider this case for page
> accounting then:

Just to clarify, the current code (not handling mdevs) will pin and do
page accounting per iova, regardless of whether the iova translates to a
unique pfn.  As long as we do no worse than that, I'm ok.

> - have rb tree of pinned iova, where key would be iova, in each vfio_dma
> structure.
> - iova tracking structure would have iova and ref_count only.
> - page accounting would only count number of iova's in rb_tree, case
> where different iova could map to same pfn would not be considered in
> this implementation for now.
> - vfio_unpin_pages() would have user_pfn and pfn as input, we would
> validate that iova exist in rb tree and trust vendor driver that
> corresponding pfn is correct, there is no validation of pfn. If want
> validate pfn, call GUP, verify pfn and call put_pfn().
> - In .release() or .detach_group() path, if there are entries in this rb
> tree, call GUP again using that iova, get pfn and then call
> put_pfn(pfn) for ref_count+1 times. This is because we are not keeping
> pfn in our tracking logic.

Wait a sec, if we detach a group from the container and it's not the
last group in the container (which would trigger a release), we can't
assume anything about which vfio_dma entries were associated with that
device.  The vendor driver, through the release of the device(s) within
that group, needs to unpin.  In a container release, we need to send a
notifier to the vendor driver(s) to cause an unpin.  This is the only
mechanism we have to ensure that vendor drivers are not leaking
references.  If during the release, after the notifier, if any
vfio_pfns remain, we need to BUG_ON, just like we need to do for any
other DMA_UNMAP.

Also, I'll say it again, I also don't like this API of passing around
potentially giant arrays, and especially the API of relying on the
vendor driver to tell us an arbitrary pfn to unpin.  If we make the
assumption that vendor drivers do not pin lots and lots of memory,
perhaps we could use a struct vfio_pfn as:

struct vfio_pfn {
        struct rb_node          node;
	dma_addr_t		iova; /* key */
        unsigned long           pfn;
        atomic_t                ref_count;
};

This puts us at 44-bytes per pfn, which isn't great, but I think it
puts us in a better position with the API that we could make use of a
page-table or sparse array in the future that would eliminate the
rb_node and make the iova implicit in the location of the data
structure.  That would leave only the pfn and ref_count, which could
potentially be combined into a single 8-byte field if we had per
vfio_dma (or higher) locking to avoid the atomic_t (and we're happy that
the reference count is always less than PAGE_SIZE, ie. we could fail
pinning if we get to that point).

That would allow for the unpin call to not provide the pfn, so could we
then look at whether we need the batching provided by the iova array at
all?  I don't have a feel for the size of memory that gets pinned by
the vendor driver, the frequency of pinning, or whether usage of
hugepages for the guest is likely to translate into contiguous memory
requests through this API.  What's your feeling?  Thanks,

Alex

^ permalink raw reply

* Re: [alsa-devel] [PATCH 2/9] ALSA: ac97: add an ac97 bus
From: Robert Jarzmik @ 2016-11-08 21:18 UTC (permalink / raw)
  To: Lars-Peter Clausen
  Cc: Dmitry Torokhov, Lee Jones, Sebastian Reichel, Jaroslav Kysela,
	Takashi Iwai, Daniel Mack, Haojian Zhuang, Liam Girdwood,
	Mark Brown, alsa-devel, linux-pm, patches, linux-kernel,
	linux-input, linux-arm-kernel
In-Reply-To: <d57cdfba-da22-652d-3d75-6d1b754cf1d5@metafoo.de>

Lars-Peter Clausen <lars@metafoo.de> writes:

> On 10/26/2016 09:41 PM, Robert Jarzmik wrote:
>> +#define to_ac97_device(d) container_of(d, struct ac97_codec_device, dev)
>> +#define to_ac97_driver(d) container_of(d, struct ac97_codec_driver, driver)
>
> In my opinion these should be inline functions rather than macros as that
> generates much more legible compiler errors e.g. in case there is a type
> mismatch.
Sure, why not.

>> +struct ac97_codec_driver {
>> +	struct device_driver	driver;
>> +	int			(*probe)(struct ac97_codec_device *);
>> +	int			(*remove)(struct ac97_codec_device *);
>> +	int			(*suspend)(struct ac97_codec_device *);
>> +	int			(*resume)(struct ac97_codec_device *);
>> +	void			(*shutdown)(struct ac97_codec_device *);
>
> The suspend, resume and shutdown callbacks are never used. Which is good,
> since all new frameworks should use dev_pm_ops. Just drop the from the struct.
Ok.

>> +/**
>> + * struct ac97_controller - The AC97 controller of the AC-Link
>> + * @ops:		the AC97 operations.
>> + * @controllers:	linked list of all existing controllers.
>> + * @dev:		the device providing the AC97 controller.
>> + * @slots_available:	the mask of accessible/scanable codecs.
>> + * @codecs:		the 4 possible AC97 codecs (NULL if none found).
>> + * @codecs_pdata:	platform_data for each codec (NULL if no pdata).
>> + *
>> + * This structure is internal to AC97 bus, and should not be used by the
>> + * controllers themselves, excepting for using @dev.
>> + */
>> +struct ac97_controller {
>> +	const struct ac97_controller_ops *ops;
>> +	struct list_head controllers;
>> +	struct device *dev;
>
> I'd make the controller itself a struct dev, rather than just having the
> pointer to the parent. This is more idiomatic and matches what other
> subsystems do. It has several advantages, you get proper refcounting of your
> controller structure, the controller gets its own sysfs directory where the
> CODECs appear as children, you don't need the manual sysfs attribute
> creation and removal in ac97_controler_{un,}register().

If you mean having "struct device dev" instead of "struct device *dev", it has
also a drawback : all the legacy platforms have already a probing method, be
that platform data, device-tree or something else.

I'm a bit converned about the conversion toll. Maybe I've not understood your
point fully, so please feel free to explain, with an actual example even better.

>> +	void (*wait)(struct ac97_controller *adrv, int slot);
>> +	void (*init)(struct ac97_controller *adrv, int slot);
>
> Neither wait nor init are ever used.
This is because I've not begun to porting sound/pci/ac97_codec.c into
sound/ac97.

>> +	if ((codec_num < 0) || (codec_num >= AC97_BUS_MAX_CODECS))
>> +		return ERR_PTR(-ERANGE);
>
> I'd make this EINVAL.
Ok.

>> +	adev = container_of(dev, struct ac97_codec_device, dev);
>
> to_ac97_device()
Sure.

>> +	codec_name = kasprintf(GFP_KERNEL, "%s:%d", dev_name(ac97_ctrl->dev),
>> +			       idx);
>> +	codec->dev.init_name = codec_name;
>
> init_name is only for statically allocated devices. Use dev_set_name(dev,
> ...). No need for kasprintf() either as dev_set_name() takes a format string.
I'll try again, I seem to remember having tried that and it failed, but I can't
remember why.

> For this you need to split device_register into device_initialize() and
> device_add(). But usually that is what you want anyway.
Let me try again.

>> +	ret = sysfs_create_link(&codec->dev.kobj, &ac97_ctrl->dev->kobj,
>> +				"ac97_controller");
>
> Since the CODEC is a child of the controller this should not be necessary as
> this just points one directory up. It's like `ln -s .. parent`
This creates :
/sys/bus/ac97/devices/pxa2xx-ac97\:0/ac97_controller

Of course as you pointed out, it's a 'ln -s .. parent' like link, but it seems
to me very unfriendly to have :
 - /sys/bus/ac97/devices/pxa2xx-ac97\:0/../ac97_operations
 - while /sys/bus/ac97/devices/ac97_operations doesn't exist (obviously)

Mmm, I don't know for this one, my mind is not set, it's a bit hard to tell if
it's only an unecessary link bringing "comfort", or something usefull.

>
>> +	if (ret)
>> +		goto err_unregister_device;
>> +
>> +	return 0;
>> +err_unregister_device:
>> +	put_device(&codec->dev);
>> +err_free_codec:
>> +	kfree(codec);
>
> Since the struct is reference counted, the freeing is done in the release
> callback and this leads to a double free.
A yes in the "goto err_unregister_device" case right, while it's necessary in
the "goto err_free_codec" case ... I need to rework that a bit.

>> +int snd_ac97_codec_driver_register(struct ac97_codec_driver *drv)
>> +{
>> +	int ret;
>> +
>> +	drv->driver.bus = &ac97_bus_type;
>> +
>> +	ret = driver_register(&drv->driver);
>> +	if (!ret)
>> +		ac97_rescan_all_controllers();
>
> Rescanning the bus when a new codec driver is registered should not be
> neccessary. The bus is scanned once when the controller is registered, this
> creates the device. The device driver core will take care of binding the
> device to the driver, if the driver is registered after thed evice.
That's because you suppose the initial scanning found all the ac97 codecs.
But that's an incomplete vision as a codec might be powered off at that time and
not seen by the first scanning, while the new scanning will discover it.

>> +static int ac97_ctrl_codecs_unregister(struct ac97_controller *ac97_ctrl)
>> +{
>> +	int i;
>> +
>> +	for (i = 0; i < AC97_BUS_MAX_CODECS; i++)
>> +		if (ac97_ctrl->codecs[i])
>> +			put_device(&ac97_ctrl->codecs[i]->dev);
>
> This should be device_unregister() to match the device_register() in
> ac97_codec_add().
Right.

>> +
>> +	return 0;
>> +}
>> +
>> +static ssize_t cold_reset_store(struct device *dev,
>> +				struct device_attribute *attr, const char *buf,
>> +				size_t len)
>> +{
>> +	struct ac97_controller *ac97_ctrl = ac97_ctrl_find(dev);
>> +
>> +	if (!dev)
>> +		return -ENODEV;
>
> dev is never NULL here.
Ok.

> And for the ac97_ctrl there is a race condition. It could be unregistered and
> freed after ac97_ctrl_find() returned sucessfully, but before ac97_ctrl->ops
> is used.
A good catch, the ac97_controllers_mutex is missing.

> Same here.
Indeed.

>> +
>> +	ac97_ctrl->ops->warm_reset(ac97_ctrl);
>> +	return len;
>> +}
>> +static DEVICE_ATTR_WO(warm_reset);
>> +
>> +static struct attribute *ac97_controller_device_attrs[] = {
>> +	&dev_attr_cold_reset.attr,
>> +	&dev_attr_warm_reset.attr,
>> +	NULL
>> +};
>
> This adds new userspace ABI that is not documented at the moment.
Very true. And as all userspace interface, it needs to be discussed. If nobody
complains, I'll add the documentation for next patch round.
>
>> +int snd_ac97_controller_register(const struct ac97_controller_ops *ops,
>> +				 struct device *dev,
>> +				 unsigned short slots_available,
>> +				 void **codecs_pdata)
>
> In my opinion this should return a handle to a ac97 controller which can
> then be passed to snd_ac97_controller_unregister(). This is in my opinion
> the better approach rather than looking up the controller by parent device.
This is another "legacy drivers" issue.

The legacy driver (the one probed by platform_data or devicetree) doesn't
necessarily have a private structure to store this ac97_controller pointer. This
enables an "easier" porting of existing drivers.

>> +/**
>> + * snd_ac97_controller_unregister - unregister an ac97 controller
>> + * @dev: the device previously provided to ac97_controller_register()
>> + *
>> + * Returns 0 on success, negative upon error
>
> Unregister must not be able to fail. Hotunplug is one of the core concepts
> of the device driver model and there is really nothing that can be done to
> prevent a device from disappearing, so there is no sensible way of handling
> the error (and your pxa driver modifications simply ignore it as well).
>
> This also means the framework needs to cope with the case where the
> controller is removed and the CODEC devices are still present. All future
> operations should return -ENODEV in that case.
Ah ... that will require a serious modification, and I see your point, so I'll
prepare this for next patchset.
>> +static struct bus_type ac97_bus_type = {
>> +	.name		= "ac97",
>> +	.dev_attrs	= ac97_dev_attrs,
>
> dev_attrs is deprecated in favor of dev_groups (See commit 880ffb5c6).
Ok.

>> index 000000000000..a835f03744bf
>> --- /dev/null
>> +++ b/sound/ac97/codec.c
>> @@ -0,0 +1,15 @@
>> +/*
>> + *  Copyright (C) 2016 Robert Jarzmik <robert.jarzmik@free.fr>
>> + *
>> + * This program is free software; you can redistribute it and/or modify
>> + * it under the terms of the GNU General Public License version 2 as
>> + * published by the Free Software Foundation.
>> + */
>> +
>> +#include <sound/ac97_codec.h>
>> +#include <sound/ac97/codec.h>
>> +#include <sound/ac97/controller.h>
>> +#include <linux/device.h>
>> +#include <linux/slab.h>
>> +#include <sound/soc.h>	/* For compat_ac97_* */
>> +
>
> I'm not sure I understand what this file does.
Ah yes, I'll remove it.
It's the future conversion of sound/pci/ac97_codec.c, but it's ... empty so far.

Thanks very much for the very detailed review.

-- 
Robert

^ permalink raw reply

* Re: [PATCH 1/7] genirq/affinity: Introduce struct irq_affinity
From: Bjorn Helgaas @ 2016-11-08 21:25 UTC (permalink / raw)
  To: Christoph Hellwig; +Cc: tglx, axboe, linux-block, linux-pci, linux-kernel
In-Reply-To: <1478544462-9549-2-git-send-email-hch@lst.de>

On Mon, Nov 07, 2016 at 10:47:36AM -0800, Christoph Hellwig wrote:
> From: Christogh Hellwig <hch@lst.de>
> 
> Some drivers (various network and RDMA adapter for example) have a MSI-X
> vector layout where most of the vectors are used for I/O queues and should
> have CPU affinity assigned to them, but some (usually 1 but sometimes more)
> at the beginning or end are used for low-performance admin or configuration
> work and should not have any explicit affinity assigned to them.
> 
> This adds a new irq_affinity structure, which will be passed through a
> variant of pci_irq_alloc_vectors that allows to specify these
> requirements (and is extensible to any future quirks in that area) so that
> the core IRQ affinity algorithm can take this quirks into account.
> 
> Signed-off-by: Christogh Hellwig <hch@lst.de>

s/Christogh/Christoph/ (also above)

What tree would you prefer?  I vote for the IRQ tree since that seems
to be where the interesting parts are, and I think I acked all the PCI
bits.

> + * struct irq_affinity - Description for auto irq affinity assignements
> + * @pre_vectors:	Reserved vectors at the beginning of the MSIX
> + *			vector space
> + * @post_vectors:	Reserved vectors at the end of the MSIX
> + *			vector space

Maybe include something more informative than just "reserved", e.g.,
"Don't apply affinity to @pre_vectors at beginning of MSI-X vector
space" or "Vectors at beginning of MSI-X vector space that are exempt
from affinity"?

> + */
> +struct irq_affinity {
> +	int	pre_vectors;
> +	int	post_vectors;
> +};
> +
>  #if defined(CONFIG_SMP)
>  
>  extern cpumask_var_t irq_default_affinity;
> -- 
> 2.1.4
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-pci" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH v4] clk: renesas: cpg-mssr: add R8A7745 support
From: Sergei Shtylyov @ 2016-11-08 21:25 UTC (permalink / raw)
  To: mturquette, linux-clk, sboyd, robh+dt, mark.rutland, devicetree
  Cc: linux-renesas-soc, horms+renesas
In-Reply-To: <3539878.XFtRJDB9iq@wasted.cogentembedded.com>

Add RZ/G1E (R8A7745) Clock  Pulse Generator / Module Standby and Software
Reset support, using the CPG/MSSR driver core and the common R-Car Gen2
(and RZ/G) code.

Based on the proof-of-concept R8A7791 CPG/MSSR patch by Geert Uytterhoeven
<geert+renesas@glider.be>.

Signed-off-by: Sergei Shtylyov <sergei.shtylyov@cogentembedded.com>
Reviewed-by: Geert Uytterhoeven <geert+renesas@glider.be>

---
This patch  is against the 'clk-next' branch of CLK group's 'linux.git' repo
plus the R8A7743 clock driver patch. It depends on the common R-Car gen2 (and
RZ/G) support just posted.

Changes in version 4:
- changed the Z2 clock's divisor to 1;
- passed  the PLL0 divisor to rcar_gen2_cpg_init();
- renamed the ACP clock to CPEX;
- removed the thermal module clock.

Changes in version 3:
- removed the FDP1-1 module clock;
- added Geert's tag.

Changes in version 2:
- changed the Z2 clock's divisor to 3.

 Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt |    5 
 drivers/clk/renesas/Kconfig                                  |    1 
 drivers/clk/renesas/Makefile                                 |    1 
 drivers/clk/renesas/r8a7745-cpg-mssr.c                       |  259 +++++++++++
 drivers/clk/renesas/renesas-cpg-mssr.c                       |    6 
 drivers/clk/renesas/renesas-cpg-mssr.h                       |    1 
 6 files changed, 271 insertions(+), 2 deletions(-)

Index: linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
===================================================================
--- linux.orig/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
+++ linux/Documentation/devicetree/bindings/clock/renesas,cpg-mssr.txt
@@ -14,6 +14,7 @@ They provide the following functionaliti
 Required Properties:
   - compatible: Must be one of:
       - "renesas,r8a7743-cpg-mssr" for the r8a7743 SoC (RZ/G1M)
+      - "renesas,r8a7745-cpg-mssr" for the r8a7745 SoC (RZ/G1E)
       - "renesas,r8a7795-cpg-mssr" for the r8a7795 SoC (R-Car H3)
       - "renesas,r8a7796-cpg-mssr" for the r8a7796 SoC (R-Car M3-W)
 
@@ -23,8 +24,8 @@ Required Properties:
   - clocks: References to external parent clocks, one entry for each entry in
     clock-names
   - clock-names: List of external parent clock names. Valid names are:
-      - "extal" (r8a7743, r8a7795, r8a7796)
-      - "usb_extal" (r8a7743)
+      - "extal" (r8a7743, r8a7745, r8a7795, r8a7796)
+      - "usb_extal" (r8a7743, r8a7745)
       - "extalr" (r8a7795, r8a7796)
 
   - #clock-cells: Must be 2
Index: linux/drivers/clk/renesas/Kconfig
===================================================================
--- linux.orig/drivers/clk/renesas/Kconfig
+++ linux/drivers/clk/renesas/Kconfig
@@ -1,6 +1,7 @@
 config CLK_RENESAS_CPG_MSSR
 	bool
 	default y if ARCH_R8A7743
+	default y if ARCH_R8A7745
 	default y if ARCH_R8A7795
 	default y if ARCH_R8A7796
 
Index: linux/drivers/clk/renesas/Makefile
===================================================================
--- linux.orig/drivers/clk/renesas/Makefile
+++ linux/drivers/clk/renesas/Makefile
@@ -3,6 +3,7 @@ obj-$(CONFIG_ARCH_R7S72100)		+= clk-rz.o
 obj-$(CONFIG_ARCH_R8A73A4)		+= clk-r8a73a4.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7740)		+= clk-r8a7740.o clk-div6.o
 obj-$(CONFIG_ARCH_R8A7743)		+= r8a7743-cpg-mssr.o rcar-gen2-cpg.o
+obj-$(CONFIG_ARCH_R8A7745)		+= r8a7745-cpg-mssr.o rcar-gen2-cpg.o
 obj-$(CONFIG_ARCH_R8A7778)		+= clk-r8a7778.o
 obj-$(CONFIG_ARCH_R8A7779)		+= clk-r8a7779.o
 obj-$(CONFIG_ARCH_R8A7790)		+= clk-rcar-gen2.o clk-div6.o
Index: linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
===================================================================
--- /dev/null
+++ linux/drivers/clk/renesas/r8a7745-cpg-mssr.c
@@ -0,0 +1,259 @@
+/*
+ * r8a7745 Clock Pulse Generator / Module Standby and Software Reset
+ *
+ * Copyright (C) 2016 Cogent Embedded Inc.
+ *
+ * This program is free software; you can redistribute it and/or modify it
+ * under the terms of the GNU General Public License version 2 as published
+ * by the Free Software Foundation; of the License.
+ */
+
+#include <linux/device.h>
+#include <linux/init.h>
+#include <linux/kernel.h>
+#include <linux/soc/renesas/rcar-rst.h>
+
+#include <dt-bindings/clock/r8a7745-cpg-mssr.h>
+
+#include "renesas-cpg-mssr.h"
+#include "rcar-gen2-cpg.h"
+
+enum clk_ids {
+	/* Core Clock Outputs exported to DT */
+	LAST_DT_CORE_CLK = R8A7745_CLK_OSC,
+
+	/* External Input Clocks */
+	CLK_EXTAL,
+	CLK_USB_EXTAL,
+
+	/* Internal Core Clocks */
+	CLK_MAIN,
+	CLK_PLL0,
+	CLK_PLL1,
+	CLK_PLL3,
+	CLK_PLL1_DIV2,
+
+	/* Module Clocks */
+	MOD_CLK_BASE
+};
+
+static const struct cpg_core_clk r8a7745_core_clks[] __initconst = {
+	/* External Clock Inputs */
+	DEF_INPUT("extal",	CLK_EXTAL),
+	DEF_INPUT("usb_extal",	CLK_USB_EXTAL),
+
+	/* Internal Core Clocks */
+	DEF_BASE(".main",	CLK_MAIN, CLK_TYPE_GEN2_MAIN, CLK_EXTAL),
+	DEF_BASE(".pll0",	CLK_PLL0, CLK_TYPE_GEN2_PLL0, CLK_MAIN),
+	DEF_BASE(".pll1",	CLK_PLL1, CLK_TYPE_GEN2_PLL1, CLK_MAIN),
+	DEF_BASE(".pll3",	CLK_PLL3, CLK_TYPE_GEN2_PLL3, CLK_MAIN),
+
+	DEF_FIXED(".pll1_div2", CLK_PLL1_DIV2, CLK_PLL1, 2, 1),
+
+	/* Core Clock Outputs */
+	DEF_BASE("lb",	 R8A7745_CLK_LB,   CLK_TYPE_GEN2_LB,   CLK_PLL1),
+	DEF_BASE("sdh",  R8A7745_CLK_SDH,  CLK_TYPE_GEN2_SDH,  CLK_PLL1),
+	DEF_BASE("sd0",  R8A7745_CLK_SD0,  CLK_TYPE_GEN2_SD0,  CLK_PLL1),
+	DEF_BASE("qspi", R8A7745_CLK_QSPI, CLK_TYPE_GEN2_QSPI, CLK_PLL1_DIV2),
+	DEF_BASE("rcan", R8A7745_CLK_RCAN, CLK_TYPE_GEN2_RCAN, CLK_USB_EXTAL),
+
+	DEF_FIXED("z2",     R8A7745_CLK_Z2,    CLK_PLL0,	  1, 1),
+	DEF_FIXED("zg",     R8A7745_CLK_ZG,    CLK_PLL1,          6, 1),
+	DEF_FIXED("zx",     R8A7745_CLK_ZX,    CLK_PLL1,          3, 1),
+	DEF_FIXED("zs",     R8A7745_CLK_ZS,    CLK_PLL1,          6, 1),
+	DEF_FIXED("hp",     R8A7745_CLK_HP,    CLK_PLL1,         12, 1),
+	DEF_FIXED("b",      R8A7745_CLK_B,     CLK_PLL1,         12, 1),
+	DEF_FIXED("p",      R8A7745_CLK_P,     CLK_PLL1,         24, 1),
+	DEF_FIXED("cl",     R8A7745_CLK_CL,    CLK_PLL1,         48, 1),
+	DEF_FIXED("cp",     R8A7745_CLK_CP,    CLK_PLL1,         48, 1),
+	DEF_FIXED("m2",     R8A7745_CLK_M2,    CLK_PLL1,          8, 1),
+	DEF_FIXED("zb3",    R8A7745_CLK_ZB3,   CLK_PLL3,          4, 1),
+	DEF_FIXED("zb3d2",  R8A7745_CLK_ZB3D2, CLK_PLL3,          8, 1),
+	DEF_FIXED("ddr",    R8A7745_CLK_DDR,   CLK_PLL3,          8, 1),
+	DEF_FIXED("mp",     R8A7745_CLK_MP,    CLK_PLL1_DIV2,    15, 1),
+	DEF_FIXED("cpex",   R8A7745_CLK_CPEX,  CLK_EXTAL,         2, 1),
+	DEF_FIXED("r",      R8A7745_CLK_R,     CLK_PLL1,      49152, 1),
+	DEF_FIXED("osc",    R8A7745_CLK_OSC,   CLK_PLL1,      12288, 1),
+
+	DEF_DIV6P1("sd2",   R8A7745_CLK_SD2,   CLK_PLL1_DIV2, 0x078),
+	DEF_DIV6P1("sd3",   R8A7745_CLK_SD3,   CLK_PLL1_DIV2, 0x26c),
+	DEF_DIV6P1("mmc0",  R8A7745_CLK_MMC0,  CLK_PLL1_DIV2, 0x240),
+};
+
+static const struct mssr_mod_clk r8a7745_mod_clks[] __initconst = {
+	DEF_MOD("msiof0",	    0,	R8A7745_CLK_MP),
+	DEF_MOD("vcp0",		  101,	R8A7745_CLK_ZS),
+	DEF_MOD("vpc0",		  103,	R8A7745_CLK_ZS),
+	DEF_MOD("tmu1",		  111,	R8A7745_CLK_P),
+	DEF_MOD("3dg",		  112,	R8A7745_CLK_ZG),
+	DEF_MOD("2d-dmac",	  115,	R8A7745_CLK_ZS),
+	DEF_MOD("fdp1-0",	  119,	R8A7745_CLK_ZS),
+	DEF_MOD("tmu3",		  121,	R8A7745_CLK_P),
+	DEF_MOD("tmu2",		  122,	R8A7745_CLK_P),
+	DEF_MOD("cmt0",		  124,	R8A7745_CLK_R),
+	DEF_MOD("tmu0",		  125,	R8A7745_CLK_CP),
+	DEF_MOD("vsp1du0",	  128,	R8A7745_CLK_ZS),
+	DEF_MOD("vsp1-sy",	  131,	R8A7745_CLK_ZS),
+	DEF_MOD("scifa2",	  202,	R8A7745_CLK_MP),
+	DEF_MOD("scifa1",	  203,	R8A7745_CLK_MP),
+	DEF_MOD("scifa0",	  204,	R8A7745_CLK_MP),
+	DEF_MOD("msiof2",	  205,	R8A7745_CLK_MP),
+	DEF_MOD("scifb0",	  206,	R8A7745_CLK_MP),
+	DEF_MOD("scifb1",	  207,	R8A7745_CLK_MP),
+	DEF_MOD("msiof1",	  208,	R8A7745_CLK_MP),
+	DEF_MOD("scifb2",	  216,	R8A7745_CLK_MP),
+	DEF_MOD("sys-dmac1",	  218,	R8A7745_CLK_ZS),
+	DEF_MOD("sys-dmac0",	  219,	R8A7745_CLK_ZS),
+	DEF_MOD("tpu0",		  304,	R8A7745_CLK_CP),
+	DEF_MOD("sdhi3",	  311,	R8A7745_CLK_SD3),
+	DEF_MOD("sdhi2",	  312,	R8A7745_CLK_SD2),
+	DEF_MOD("sdhi0",	  314,	R8A7745_CLK_SD0),
+	DEF_MOD("mmcif0",	  315,	R8A7745_CLK_MMC0),
+	DEF_MOD("iic0",		  318,	R8A7745_CLK_HP),
+	DEF_MOD("iic1",		  323,	R8A7745_CLK_HP),
+	DEF_MOD("cmt1",		  329,	R8A7745_CLK_R),
+	DEF_MOD("usbhs-dmac0",	  330,	R8A7745_CLK_HP),
+	DEF_MOD("usbhs-dmac1",	  331,	R8A7745_CLK_HP),
+	DEF_MOD("irqc",		  407,	R8A7745_CLK_CP),
+	DEF_MOD("intc-sys",	  408,	R8A7745_CLK_ZS),
+	DEF_MOD("audio-dmac0",	  502,	R8A7745_CLK_HP),
+	DEF_MOD("pwm",		  523,	R8A7745_CLK_P),
+	DEF_MOD("usb-ehci",	  703,	R8A7745_CLK_MP),
+	DEF_MOD("usbhs",	  704,	R8A7745_CLK_HP),
+	DEF_MOD("hscif2",	  713,	R8A7745_CLK_ZS),
+	DEF_MOD("scif5",	  714,	R8A7745_CLK_P),
+	DEF_MOD("scif4",	  715,	R8A7745_CLK_P),
+	DEF_MOD("hscif1",	  716,	R8A7745_CLK_ZS),
+	DEF_MOD("hscif0",	  717,	R8A7745_CLK_ZS),
+	DEF_MOD("scif3",	  718,	R8A7745_CLK_P),
+	DEF_MOD("scif2",	  719,	R8A7745_CLK_P),
+	DEF_MOD("scif1",	  720,	R8A7745_CLK_P),
+	DEF_MOD("scif0",	  721,	R8A7745_CLK_P),
+	DEF_MOD("du0",		  724,	R8A7745_CLK_ZX),
+	DEF_MOD("ipmmu-sgx",	  800,	R8A7745_CLK_ZX),
+	DEF_MOD("vin1",		  810,	R8A7745_CLK_ZG),
+	DEF_MOD("vin0",		  811,	R8A7745_CLK_ZG),
+	DEF_MOD("etheravb",	  812,	R8A7745_CLK_HP),
+	DEF_MOD("ether",	  813,	R8A7745_CLK_P),
+	DEF_MOD("gpio6",	  905,	R8A7745_CLK_CP),
+	DEF_MOD("gpio5",	  907,	R8A7745_CLK_CP),
+	DEF_MOD("gpio4",	  908,	R8A7745_CLK_CP),
+	DEF_MOD("gpio3",	  909,	R8A7745_CLK_CP),
+	DEF_MOD("gpio2",	  910,	R8A7745_CLK_CP),
+	DEF_MOD("gpio1",	  911,	R8A7745_CLK_CP),
+	DEF_MOD("gpio0",	  912,	R8A7745_CLK_CP),
+	DEF_MOD("can1",		  915,	R8A7745_CLK_P),
+	DEF_MOD("can0",		  916,	R8A7745_CLK_P),
+	DEF_MOD("qspi_mod",	  917,	R8A7745_CLK_QSPI),
+	DEF_MOD("i2c5",		  925,	R8A7745_CLK_HP),
+	DEF_MOD("i2c4",		  927,	R8A7745_CLK_HP),
+	DEF_MOD("i2c3",		  928,	R8A7745_CLK_HP),
+	DEF_MOD("i2c2",		  929,	R8A7745_CLK_HP),
+	DEF_MOD("i2c1",		  930,	R8A7745_CLK_HP),
+	DEF_MOD("i2c0",		  931,	R8A7745_CLK_HP),
+	DEF_MOD("ssi-all",	 1005,	R8A7745_CLK_P),
+	DEF_MOD("ssi9",		 1006,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi8",		 1007,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi7",		 1008,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi6",		 1009,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi5",		 1010,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi4",		 1011,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi3",		 1012,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi2",		 1013,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi1",		 1014,	MOD_CLK_ID(1005)),
+	DEF_MOD("ssi0",		 1015,	MOD_CLK_ID(1005)),
+	DEF_MOD("scu-all",	 1017,	R8A7745_CLK_P),
+	DEF_MOD("scu-dvc1",	 1018,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-dvc0",	 1019,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-ctu1-mix1", 1020,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-ctu0-mix0", 1021,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src9",	 1022,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src8",	 1023,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src7",	 1024,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src6",	 1025,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src5",	 1026,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src4",	 1027,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src3",	 1028,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src2",	 1029,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src1",	 1030,	MOD_CLK_ID(1017)),
+	DEF_MOD("scu-src0",	 1031,	MOD_CLK_ID(1017)),
+	DEF_MOD("scifa3",	 1106,	R8A7745_CLK_MP),
+	DEF_MOD("scifa4",	 1107,	R8A7745_CLK_MP),
+	DEF_MOD("scifa5",	 1108,	R8A7745_CLK_MP),
+};
+
+static const unsigned int r8a7745_crit_mod_clks[] __initconst = {
+	MOD_CLK_ID(408),	/* INTC-SYS (GIC) */
+};
+
+/*
+ * CPG Clock Data
+ */
+
+/*
+ *    MD	EXTAL		PLL0	PLL1	PLL3
+ * 14 13 19	(MHz)		*1	*2
+ *---------------------------------------------------
+ * 0  0  0	15		x200/3	x208/2	x106
+ * 0  0  1	15		x200/3	x208/2	x88
+ * 0  1  0	20		x150/3	x156/2	x80
+ * 0  1  1	20		x150/3	x156/2	x66
+ * 1  0  0	26 / 2		x230/3	x240/2	x122
+ * 1  0  1	26 / 2		x230/3	x240/2	x102
+ * 1  1  0	30 / 2		x200/3	x208/2	x106
+ * 1  1  1	30 / 2		x200/3	x208/2	x88
+ *
+ * *1 :	Table 7.5b indicates VCO output (PLL0 = VCO/3)
+ * *2 :	Table 7.5b indicates VCO output (PLL1 = VCO/2)
+ */
+#define CPG_PLL_CONFIG_INDEX(md)	((((md) & BIT(14)) >> 12) | \
+					 (((md) & BIT(13)) >> 12) | \
+					 (((md) & BIT(19)) >> 19))
+
+static const struct rcar_gen2_cpg_pll_config cpg_pll_configs[8] __initconst = {
+	/* EXTAL div	PLL1 mult	PLL3 mult	PLL0 mult */
+	{ 1,		208,		106,		200	},
+	{ 1,		208,		88,		200	},
+	{ 1,		156,		80,		150	},
+	{ 1,		156,		66,		150	},
+	{ 2,		240,		122,		230	},
+	{ 2,		240,		102,		230	},
+	{ 2,		208,		106,		200	},
+	{ 2,		208,		88,		200	},
+};
+
+static int __init r8a7745_cpg_mssr_init(struct device *dev)
+{
+	const struct rcar_gen2_cpg_pll_config *cpg_pll_config;
+	u32 cpg_mode;
+	int error;
+
+	error = rcar_rst_read_mode_pins(&cpg_mode);
+	if (error)
+		return error;
+
+	cpg_pll_config = &cpg_pll_configs[CPG_PLL_CONFIG_INDEX(cpg_mode)];
+
+	return rcar_gen2_cpg_init(cpg_pll_config, 3);
+}
+
+const struct cpg_mssr_info r8a7745_cpg_mssr_info __initconst = {
+	/* Core Clocks */
+	.core_clks = r8a7745_core_clks,
+	.num_core_clks = ARRAY_SIZE(r8a7745_core_clks),
+	.last_dt_core_clk = LAST_DT_CORE_CLK,
+	.num_total_core_clks = MOD_CLK_BASE,
+
+	/* Module Clocks */
+	.mod_clks = r8a7745_mod_clks,
+	.num_mod_clks = ARRAY_SIZE(r8a7745_mod_clks),
+	.num_hw_mod_clks = 12 * 32,
+
+	/* Critical Module Clocks */
+	.crit_mod_clks = r8a7745_crit_mod_clks,
+	.num_crit_mod_clks = ARRAY_SIZE(r8a7745_crit_mod_clks),
+
+	/* Callbacks */
+	.init = r8a7745_cpg_mssr_init,
+	.cpg_clk_register = rcar_gen2_cpg_clk_register,
+};
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.c
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.c
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.c
@@ -509,6 +509,12 @@ static const struct of_device_id cpg_mss
 		.data = &r8a7743_cpg_mssr_info,
 	},
 #endif
+#ifdef CONFIG_ARCH_R8A7745
+	{
+		.compatible = "renesas,r8a7745-cpg-mssr",
+		.data = &r8a7745_cpg_mssr_info,
+	},
+#endif
 #ifdef CONFIG_ARCH_R8A7795
 	{
 		.compatible = "renesas,r8a7795-cpg-mssr",
Index: linux/drivers/clk/renesas/renesas-cpg-mssr.h
===================================================================
--- linux.orig/drivers/clk/renesas/renesas-cpg-mssr.h
+++ linux/drivers/clk/renesas/renesas-cpg-mssr.h
@@ -131,6 +131,7 @@ struct cpg_mssr_info {
 };
 
 extern const struct cpg_mssr_info r8a7743_cpg_mssr_info;
+extern const struct cpg_mssr_info r8a7745_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a7795_cpg_mssr_info;
 extern const struct cpg_mssr_info r8a7796_cpg_mssr_info;
 #endif

^ permalink raw reply


This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.