Re: [RFC PATCH v2 02/20] x86: Set the write-protect cache mode for full PAT support

2016-08-24 Thread Borislav Petkov
On Mon, Aug 22, 2016 at 05:35:50PM -0500, Tom Lendacky wrote:
> For processors that support PAT, set the write-protect cache mode
> (_PAGE_CACHE_MODE_WP) entry to the actual write-protect value (x05).
> 
> Signed-off-by: Tom Lendacky 
> ---
>  arch/x86/mm/pat.c |4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

Acked-by: Borislav Petkov 

-- 
Regards/Gruss,
Boris.

ECO tip #101: Trim your mails when you reply.
--
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] docs: split up the driver book

2016-08-24 Thread Jonathan Corbet
On Tue, 23 Aug 2016 11:30:16 -0300
Mauro Carvalho Chehab  wrote:

> On the output text, you'll see two places with "@:c:func:threadfn()".
> 
> The problem here is that threadfn() is a function argument. While this
> used to work with DocBooks, now with Sphinx this is not handled well.
> 
> I got some other similar cases on media. There, I opted to just remove
> the () on some places, or to replace it by \(\) to avoid kernel-doc
> to do the wrong thing. 

I have a different idea: why not just add another regexp to the
kernel-doc house of cards? :)  The following seems to make these issues
go away pretty nicely, and didn't cause any change at all to the
media/gpu output...

Stacking up ordering-dependent regexps is not a path to long-term joy; at
some point, we will likely want a smarter parser for kerneldoc comments.
But this seems to improve things for the moment.

jon

>From 5dccd4fb9f3c0b6468f38efab8c1d6232d3e701b Mon Sep 17 00:00:00 2001
From: Jonathan Corbet 
Date: Wed, 24 Aug 2016 16:31:15 -0600
Subject: [PATCH] docs: Special-case function-pointer parameters in kernel-doc

Add yet another regex to kernel-doc to trap @param() references separately
and not produce corrupt RST markup.

Signed-off-by: Jonathan Corbet 
---
 scripts/kernel-doc | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/scripts/kernel-doc b/scripts/kernel-doc
index 4f2e9049e8fa..c681e8f0ecc2 100755
--- a/scripts/kernel-doc
+++ b/scripts/kernel-doc
@@ -212,6 +212,7 @@ my $anon_struct_union = 0;
 my $type_constant = '\%([-_\w]+)';
 my $type_func = '(\w+)\(\)';
 my $type_param = '\@(\w+)';
+my $type_fp_param = '\@(\w+)\(\)';  # Special RST handling for func ptr params
 my $type_struct = '\&((struct\s*)*[_\w]+)';
 my $type_struct_xml = '\\((struct\s*)*[_\w]+)';
 my $type_env = '(\$\w+)';
@@ -292,6 +293,7 @@ my @highlights_rst = (
# Note: need to escape () to avoid func matching later
[$type_member_func, "\\:c\\:type\\:`\$1\$2() 
<\$1>`"],
[$type_member, "\\:c\\:type\\:`\$1\$2 <\$1>`"],
+  [$type_fp_param, "**\$1()**"],
[$type_func, "\\:c\\:func\\:`\$1()`"],
[$type_struct_full, "\\:c\\:type\\:`\$1 \$2 <\$2>`"],
[$type_enum_full, "\\:c\\:type\\:`\$1 \$2 <\$2>`"],
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] docs: split up the driver book

2016-08-24 Thread Mauro Carvalho Chehab
Em Wed, 24 Aug 2016 16:46:22 -0600
Jonathan Corbet  escreveu:

> On Tue, 23 Aug 2016 11:30:16 -0300
> Mauro Carvalho Chehab  wrote:
> 
> > On the output text, you'll see two places with "@:c:func:threadfn()".
> > 
> > The problem here is that threadfn() is a function argument. While this
> > used to work with DocBooks, now with Sphinx this is not handled well.
> > 
> > I got some other similar cases on media. There, I opted to just remove
> > the () on some places, or to replace it by \(\) to avoid kernel-doc
> > to do the wrong thing.   
> 
> I have a different idea: why not just add another regexp to the
> kernel-doc house of cards? :) 

Yeah, we can do that, but still we need to check if the references
are being properly solved (e. g. if the parser is doing the right
thing).

Ideally, the regex should also check for the next symbols, as
things like: @foo->bar should be translated to:
:c:type:`foo`\ ->bar

As Sphinx doesn't handle well non-space chars[1] before :c: or after the
grave accent ("`").

[1] Actually, it seems to work fine with a few symbols, like commas
and dots.

> The following seems to make these issues
> go away pretty nicely, and didn't cause any change at all to the
> media/gpu output...
> 
> Stacking up ordering-dependent regexps is not a path to long-term joy; at
> some point, we will likely want a smarter parser for kerneldoc comments.
> But this seems to improve things for the moment.

Good!

> 
> jon
> 
> From 5dccd4fb9f3c0b6468f38efab8c1d6232d3e701b Mon Sep 17 00:00:00 2001
> From: Jonathan Corbet 
> Date: Wed, 24 Aug 2016 16:31:15 -0600
> Subject: [PATCH] docs: Special-case function-pointer parameters in kernel-doc
> 
> Add yet another regex to kernel-doc to trap @param() references separately
> and not produce corrupt RST markup.
> 
> Signed-off-by: Jonathan Corbet 
> ---
>  scripts/kernel-doc | 2 ++
>  1 file changed, 2 insertions(+)
> 
> diff --git a/scripts/kernel-doc b/scripts/kernel-doc
> index 4f2e9049e8fa..c681e8f0ecc2 100755
> --- a/scripts/kernel-doc
> +++ b/scripts/kernel-doc
> @@ -212,6 +212,7 @@ my $anon_struct_union = 0;
>  my $type_constant = '\%([-_\w]+)';
>  my $type_func = '(\w+)\(\)';
>  my $type_param = '\@(\w+)';
> +my $type_fp_param = '\@(\w+)\(\)';  # Special RST handling for func ptr 
> params
>  my $type_struct = '\&((struct\s*)*[_\w]+)';
>  my $type_struct_xml = '\\((struct\s*)*[_\w]+)';
>  my $type_env = '(\$\w+)';
> @@ -292,6 +293,7 @@ my @highlights_rst = (
> # Note: need to escape () to avoid func matching later
> [$type_member_func, "\\:c\\:type\\:`\$1\$2() 
> <\$1>`"],
> [$type_member, "\\:c\\:type\\:`\$1\$2 <\$1>`"],
> +[$type_fp_param, "**\$1()**"],

Hmm... shoudn't it be a reference to the struct (or to the struct member),
instead of bold?

> [$type_func, "\\:c\\:func\\:`\$1()`"],
> [$type_struct_full, "\\:c\\:type\\:`\$1 \$2 <\$2>`"],
> [$type_enum_full, "\\:c\\:type\\:`\$1 \$2 <\$2>`"],



Thanks,
Mauro
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Rafał Miłecki
On 24 August 2016 at 23:04, Greg KH  wrote:
> On Wed, Aug 24, 2016 at 11:29:51AM +0200, Rafał Miłecki wrote:
>> On 24 August 2016 at 11:22, Greg KH  wrote:
>> > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
>> >> +static ssize_t ports_show(struct device *dev, struct device_attribute 
>> >> *attr,
>> >> +   char *buf)
>> >> +{
>> >> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> >> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data;
>> >> + struct usbport_trig_port *port;
>> >> + ssize_t ret = 0;
>> >> + int len;
>> >> +
>> >> + list_for_each_entry(port, _data->ports, list) {
>> >> + len = sprintf(buf + ret, "%s\n", port->name);
>> >> + if (len >= 0)
>> >> + ret += len;
>> >> + }
>> >> +
>> >> + return ret;
>> >> +}
>> >
>> > sysfs is "one value per file", here you are listing a bunch of things in
>> > one sysfs file.  Please don't do that.
>>
>> OK. What do you think about creating "ports" subdirectory and creating
>> file-per-port in it? Then I'd need to bring back something like
>> "new_port" and "remove_port". Does it sound OK?
>
> Maybe, I don't know.  Why is "USB" somehow unique here?  Why isn't this
> the same for PCI slots/ports?  pccard?  sdcard?  thunderbolt?

Good question. I would like to extend this USB port trigger in the
future by reacting to USB activity. This involves playing with URBs
and I believe that at that point it'd be getting too much USB specific
to /rule them all/.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/5] Networking cgroup controller

2016-08-24 Thread महेश बंडेवार
On Tue, Aug 23, 2016 at 1:49 AM, Parav Pandit  wrote:
> Hi Anoop,
>
> Regardless of usecase, I think this functionality is best handled as
> LSM functionality instead of cgroup.
>
I'm not so sure about that. Cgroup APIs are useful and this is just an
extension to it.


> Tasks which are proposed in this patch are related to access control checks.
> LSM already has required hooks for socket operations such as bind(),
> listen() as few small examples.
>
> Refer to security_socket_listen() which invokes LSM specific hooks.
> This is invoked in source/net/socket.c as part of listen() system call.
> LSM hook callback can check whether a given a process can listen to
> requested UDP port or not.
>
This has administrative overhead that is not addressed. The underlying
cgroup infrastructure takes care of it in this (current)
implementation.

> Parav
>
>
[...]
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv11 3/3] rdmacg: Added documentation for rdmacg

2016-08-24 Thread Rami Rosen
Hi,

> +Whenever RDMA resource charing occurs, owner rdma cgroup is returned to
Should be: charging instead of charing

> +(b) Query resource limit:
> +cat /sys/fs/cgroup/rdma/2/rdma.max
> +#Output:
> +mlx4_0 uctx=max pd=max ah=2 mr=100 mw=max cq=max srq=max qp=10 flow=max
> +ocrdma1 uctx=1 pd=5 ah=1 mr=10 cq=10 srq=max qp=20 flow=max flow=max

Is this really so: double"flow=max" at the end of the ocrdma1 line?
(flow=max flow=max)

> +5-4. RDMA
> +
> +The "rdma" controller regulates the distribution and accounting of
> +of RDMA resources.
"of of" should be only a single "of"


> + mlx4_1 uctx=1 ah=0 pd=1 cq=4 qp=4 mr=100 srq=0 flow=10
> + ocrdma1 uctx=2 pd=2 ah=2 mr=20 mw=max cq=1 srq=1 qp=10 flow=10

Seems to be inconsistency here: in the first line you have qp=4
*before* srq=0, but in the second line you have qp=10 *after* srq=1.

Keep on the good work!

Regards,
Rami Rosen
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv11 3/3] rdmacg: Added documentation for rdmacg

2016-08-24 Thread Tejun Heo
On Mon, Aug 22, 2016 at 06:03:51PM +0530, Parav Pandit wrote:
> +  rdma.max
> + A readwrite file that exists for all the cgroups except root that

Can you please add that it's a nested-keyed file?

...
> +  rdma.current
> + A read-only file that describes current resource usage.

Ditto.

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Greg KH
On Wed, Aug 24, 2016 at 11:26:27AM +0200, Rafał Miłecki wrote:
> On 24 August 2016 at 11:21, Greg KH  wrote:
> > On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> >> From: Rafał Miłecki 
> >>
> >> This commit adds a new trigger responsible for turning on LED when USB
> >> device gets connected to the specified USB port. This can can useful for
> >> various home routers that have USB port(s) and a proper LED telling user
> >> a device is connected.
> >>
> >> The trigger gets its documentation file but basically it just requires
> >> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> >>
> >> During work on this trigger there was a plan to add DT bindings for it,
> >> but there wasn't an agreement on the format yet. This can be worked on
> >> later, a sysfs interface is needed anyway for platforms not using DT.
> >>
> >> Signed-off-by: Rafał Miłecki 
> >> ---
> >> V2: Trying to add DT support, idea postponed as it will take more time
> >> to discuss the bindings.
> >> V3: Fix typos in commit and Documentation (thanks Jacek!)
> >> Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> >> Check if there is USB device connected after adding new USB port
> >> Fix memory leak or two
> >>
> >> Felipe: I'd like to ask for your Ack before having this patch pushed.
> >> ---
> >>  Documentation/leds/ledtrig-usbport.txt |  49 +++
> >>  drivers/leds/trigger/Kconfig   |   8 ++
> >>  drivers/leds/trigger/Makefile  |   1 +
> >>  drivers/leds/trigger/ledtrig-usbport.c | 253 
> >> +
> >>  4 files changed, 311 insertions(+)
> >>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
> >>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c
> >
> > You are adding sysfs files without adding a Documentation/ABI/ entry,
> > please never do that.
> 
> This is the way all LED triggers are documented, I just blindly
> assumed it's OK. Could you be a bit more helpful and help me to
> understand further steps? Should I drop
> Documentation/leds/ledtrig-usbport.txt and add proper entry in
> Documentation/ABI/testing? Or should I keep both files? What about
> current sysfs of other triggers? Should they be moved/documented in
> ABI?

In the end, all should be moved into Documentation/ABI/  But for you,
might as well start documenting them in there for any new ones so you
don't have to just move them later.

thanks,

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCHv11 0/3] rdmacg: IB/core: rdma controller support

2016-08-24 Thread Tejun Heo
(cc'ing Christoph)

On Mon, Aug 22, 2016 at 06:03:48PM +0530, Parav Pandit wrote:
> rdmacg: IB/core: rdma controller support
> 
> Patch is generated and tested against below Doug's linux-rdma
> git tree.
> 
> URL: git://git.kernel.org/pub/scm/linux/kernel/git/dledford/rdma.git
> Branch: master
> 
> Patchset is also compiled and tested against below Tejun's cgroup tree
> using cgroup v2 mode.
> URL: git://git.kernel.org/pub/scm/linux/kernel/git/tj/cgroup.git
> Branch: master
> 
> Overview:
> Currently user space applications can easily take away all the rdma
> device specific resources such as AH, CQ, QP, MR etc. Due to which other
> applications in other cgroup or kernel space ULPs may not even get chance
> to allocate any rdma resources. This results into service unavailibility.
> 
> RDMA cgroup addresses this issue by allowing resource accounting,
> limit enforcement on per cgroup, per rdma device basis.
> 
> RDMA uverbs layer will enforce limits on well defined RDMA verb
> resources without any HCA vendor device driver involvement.
> 
> RDMA uverbs layer will not do limit enforcement of HCA hw vendor
> specific resources. Instead rdma cgroup provides set of APIs
> through which vendor specific drivers can do resource accounting
> by making use of rdma cgroup.
> 
> Resource limit enforcement is hierarchical.
> 
> When process is migrated with active RDMA resources, rdma cgroup
> continues to uncharge original cgroup for allocated resource. New resource
> is charged to current process's cgroup, which means if the process is
> migrated with active resources, for new resources it will be charged to
> new cgroup and old resources will be correctly uncharged from old cgroup.
> 
> Changes from v10:
>   * (To address comments from Tejun, Christoph)
>1. Removed unused rpool_list_lock from rdma_cgroup structure.
>2. Moved rdma resource definition to rdma cgroup instead of IB stack
>3. Added prefix rdmacg to static instances
>4. Simplified locking with single mutex for all operations
>5. Following approach of atomically allocating object and
>   charging resource in hirerchy
>6. Code simplification due to single lock
>7. Using for_each_set_bit API for bit operation
>8. Renamed list heads as Objects instead of _head
>9. Renamed list entries as _node instead of _list.
>   10. Made usage_num to 64 bit to avoid overflow and to avoid 
>   additional code to track non zero number of usage counts.
>   * (To address comments from Doug)
>1. Added copyright and GPLv2 license

Looks good to me.  I just have a nit in the documentation.  Christoph,
what do you think?

Thanks.

-- 
tejun
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Rafał Miłecki
On 24 August 2016 at 20:48, Bjørn Mork  wrote:
> Rafał Miłecki  writes:
>
>> The last big missing thing is Documentation update (this is why I'm
>> sending RFC). Greg pointed out we should have some entries in
>> Documentation/ABI, but it seems none of triggers have it.
>
> There's a lot missing, but there is at least one exception:
> The "inverted" attribute of the  gpio and backlight triggers is
> documented as part of Documentation/ABI/testing/sysfs-class-led
>
>> Any idea why is that?
>
> Manual enforcement fails from time to time? The requirement was less
> strict in the early days of sysfs? Does it matter?
>
>> Do we need to change it? Or is it required for new code only?
>
> The lack of ABI docs is a bug. Don't add new code with known bugs. Old
> code should be fixed, but there is no immediate *need* to fix it.

Don't get me wrong. I care about code quality and documentation. I
mean to follow kernel rules. It's just some things may be not clear to
ppl and it would be nice to get any explanation/help. That said thanks
a lot of your e-mail, I'll try to expand pointed Documentation.

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe

2016-08-24 Thread Luis R. Rodriguez
On Wed, Aug 24, 2016 at 08:55:55AM +0200, Daniel Vetter wrote:
> On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez  wrote:
> > Thou shalt not make firmware calls early on init or probe.

<-- snip -->

> > There are 4 offenders at this time:
> >
> > mcgrof@ergon ~/linux-next (git::20160609)$ export 
> > COCCI=scripts/coccinelle/api/request_firmware.cocci
> > mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report
> >
> > drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its 
> > init routine on line 321.
> > drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on 
> > its probe routine on line 136.
> > drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its 
> > probe routine on line 796.
> > drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on 
> > its probe routine on line 1246.
> 
> Plus all gpu drivers which need firmware. And yes we must load them at
> probe

Do you have an upstream driver in mind that does this ? Is it on device
drier module probe or a DRM subsystem specific probe call of some sort ?

> because people are generally pissed when they boot their machine
> and the screen goes black. On top of that a lot of people want their
> gpu drivers to be built-in, but can't ship the firmware blobs in the
> kernel image because gpl. Yep, there's a bit a contradiction there ...

Can they use initramfs for this ?

Also just curious -- as with other subsystems, is it possible to load
a generic driver first, say vesa, and then a more enhanced one later ?
I am not saying this is ideal or am I suggesting this, I'd just like
to know the feasibility of this.

> I think what would work is loading the different subsystems of the
> driver in parallel (we already do that largely)

Init level stuff is actually pretty synchronous, and in fact both
init and probe are called serially. There are a few subsystems which
have been doing things a bit differently, but these are exceptions.

When you say we already do this largely, can you describe a bit more
precisely what you mean ?

>, and then if one
> firmware blob isn't there yet simply stall that async worker until it
> shows up.

Is this an existing framework or do you mean if we add something
generic to do this async loading of subsystems ?

> But the answers I've gotten thus far from request_firmware()
> folks (well at least Greg) is don't do that ...

Well in this patch set I'm adding myself as a MAINTAINER and I've
been extending the firmware API recently to help with a few new
features I want, I've been wanting to hear more feedback from
other's needs and I had actually not gotten much -- except
only recently with the usermode helper and reasons why some
folks thought they could not use direct firmware loading from
the fs. I'm keen to hear or more use cases and needs specially if
they have to do with improving boot time and asynchronous boot.

> Is that problem still somewhere on the radar? 

Not mine.

> Atm there's various
> wait_for_rootfs hacks out there floating in vendor/product trees.

This one I've heard about recently, and I suggested two possible
solutions, with a preference to a simply notification of when
rootfs is available from userspace.

> "Avoid at all costs" sounds like upstream prefers to not care about
> android/cros in those case (yes I know most arm socs don't need
> firmware, which would make it a problem fo just be a subset of all
> devices).

In my days of dealing with Android I learned most folks did not frankly
care too much about upstream-first model. That means things were reactive.
That's a business mind set and that's fine. However for upstream we want
what is best and to discuss. I haven't seen proposals so, so long as
we just hear of "hacks" that some folks do in vendor/product trees,
what can we do ?

In so far as async probe is concerned -- that is now upstream.

http://www.do-not-panic.com/2015/12/linux-asynchronous-probe.html

In so far as modules are concerned -- this should work without issue now, and
if there is an issue its very likely a bug in the subsystem.  As I noted in the
post, built-in support requires more love. A simple option for you to test this
is to test the two debug patches at the end there and boot. Alternatively inits
can just peg the async request for all modules. Should be an easy change, just
hadn't had a change to do it yet. Maybe its time.

I'm also trying to make more async functionality possible early in boot with
dependencies annotated somehow, and have a bit of work to help with this (refer
to recent linker tables patches) already which may even be possible to used to
facelift our old kernel init levels -- but as I've studied this I've also
observed others working on very similar problems, nothing is quite taking a
large picture of this and trying to generalize this. Its why I proposed it as a
topic for KS.

So .. I agree, let's avoid the hacks. Patches welcomed.

  Luis
--
To 

Re: [PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Bjørn Mork
Rafał Miłecki  writes:

> The last big missing thing is Documentation update (this is why I'm
> sending RFC). Greg pointed out we should have some entries in
> Documentation/ABI, but it seems none of triggers have it.

There's a lot missing, but there is at least one exception:
The "inverted" attribute of the  gpio and backlight triggers is
documented as part of Documentation/ABI/testing/sysfs-class-led

> Any idea why is that?

Manual enforcement fails from time to time? The requirement was less
strict in the early days of sysfs? Does it matter?

> Do we need to change it? Or is it required for new code only?

The lack of ABI docs is a bug. Don't add new code with known bugs. Old
code should be fixed, but there is no immediate *need* to fix it.


Bjørn
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC PATCH-tip v4 10/10] locking/rwsem: Add a boot parameter to reader spinning threshold

2016-08-24 Thread Waiman Long

On 08/24/2016 12:00 AM, Davidlohr Bueso wrote:

On Thu, 18 Aug 2016, Waiman Long wrote:


The default reader spining threshold is current set to 4096. However,
the right reader spinning threshold may vary from one system to
another and among the different architectures. This patch adds a new
kernel boot parameter to modify the threshold value. This enables
better tailoring to the needs of different systems as well as for
testing purposes.


It also means that those systems could very easily be incorrectly 
tailored.

(and worse case scenario: reboot, is obviously a terrible way to get rid
of any issues). I very much disagree with exposing this sort of core 
stuff,

it should work well for everyone out of the box, not relying on users to
properly configure this.

Thanks,
Davidlohr


I also have some concern about exposing this kernel parameter as it will 
be hard to tune. That is why I put it at the end to gauge the opinion of 
others. I will leave this out when I send out the next version.


Cheers,
Longman
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[PATCH RFC V3.5] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Rafał Miłecki
From: Rafał Miłecki 

This commit adds a new trigger responsible for turning on LED when USB
device gets connected to the specified USB port. This can can useful for
various home routers that have USB port(s) and a proper LED telling user
a device is connected.

The trigger gets its documentation file but basically it just requires
specifying USB port in a Linux format (e.g. echo 1-1 > new_port).

During work on this trigger there was a plan to add DT bindings for it,
but there wasn't an agreement on the format yet. This can be worked on
later, a sysfs interface is needed anyway for platforms not using DT.

Signed-off-by: Rafał Miłecki 
---
V2: Trying to add DT support, idea postponed as it will take more time
to discuss the bindings.
V3: Fix typos in commit and Documentation (thanks Jacek!)
Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
Check if there is USB device connected after adding new USB port
Fix memory leak or two
V3.5: Fix e-mail address (thanks Matthias)
  Simplify conditions in usbport_trig_notify (thx Matthias)
  Make "ports" a subdirectory with file per port, to match one value
  per file sysfs rule (thanks Greg)
  As "ports" couldn't be used for adding and removing ports anymore,
  there are now "new_port" and "remove_port". Having them makes this
  API also common with e.g. pci and usb buses.

The last big missing thing is Documentation update (this is why I'm
sending RFC). Greg pointed out we should have some entries in
Documentation/ABI, but it seems none of triggers have it. Any idea why
is that? Do we need to change it? Or is it required for new code only?
If so, should I care about Documentation/leds/ledtrig-usbport.txt at
all in this patch?

For now I didn't update Documentation/leds/ledtrig-usbport.txt with the
new new_port and remove_port API, until I get a clue how to proceed.
---
 Documentation/leds/ledtrig-usbport.txt |  49 ++
 drivers/leds/trigger/Kconfig   |   8 +
 drivers/leds/trigger/Makefile  |   1 +
 drivers/leds/trigger/ledtrig-usbport.c | 309 +
 4 files changed, 367 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-usbport.txt
 create mode 100644 drivers/leds/trigger/ledtrig-usbport.c

diff --git a/Documentation/leds/ledtrig-usbport.txt 
b/Documentation/leds/ledtrig-usbport.txt
new file mode 100644
index 000..fa42227
--- /dev/null
+++ b/Documentation/leds/ledtrig-usbport.txt
@@ -0,0 +1,49 @@
+USB port LED trigger
+
+
+This LED trigger can be used for signalling to the user a presence of USB 
device
+in a given port. It simply turns on LED when device appears and turns it off
+when it disappears.
+
+It requires specifying a list of USB ports that should be observed. Used format
+matches Linux kernel format and consists of a root hub number and a hub port
+separated by a dash (e.g. 3-1).
+
+It is also possible to handle devices with internal hubs (that are always
+connected to the root hub). User can simply specify internal hub ports then
+(e.g. 1-1.1, 1-1.2, etc.).
+
+Please note that this trigger allows assigning multiple USB ports to a single
+LED. This can be useful in two cases:
+
+1) Device with single USB LED and few physical ports
+
+In such a case LED will be turned on as long as there is at least one connected
+USB device.
+
+2) Device with a physical port handled by few controllers
+
+Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 physical
+port may be handled by ohci-platform, ehci-platform and xhci-hcd. If there is
+only one LED user will most likely want to assign ports from all 3 hubs.
+
+
+This trigger can be activated from user space on led class devices as shown
+below:
+
+  echo usbport > trigger
+
+This adds the following sysfs attributes to the LED:
+
+  ports - Reading it lists all USB ports assigned to the trigger. Writing USB
+ port number to it will make this driver start observing it. It's also
+ possible to remove USB port from observable list by writing it with a
+ "-" prefix.
+
+Example use-case:
+
+  echo usbport > trigger
+  echo 4-1 > ports
+  echo 2-1 > ports
+  echo -4-1 > ports
+  cat ports
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 3f9ddb9..bdd6fd2 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC
  a different trigger.
  If unsure, say Y.
 
+config LEDS_TRIGGER_USBPORT
+   tristate "USB port LED trigger"
+   depends on LEDS_TRIGGERS && USB
+   help
+ This allows LEDs to be controlled by USB events. Enabling this option
+ allows specifying list of USB ports that should turn on LED when some
+ USB device gets connected.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index a72c43c..56e1741 100644
--- 

Re: [PATCH] doc-rst:sphinx-extensions: add metadata parallel-safe

2016-08-24 Thread Mauro Carvalho Chehab
Em Wed, 24 Aug 2016 15:35:24 +0200
Markus Heiser  escreveu:

> From: Markus Heiser 
> 
> The setup() function of a Sphinx-extension can return a dictionary. This
> is treated by Sphinx as metadata of the extension [1].
> 
> With metadata "parallel_read_safe = True" a extension is marked as
> save for "parallel reading of source". This is needed if you want
> build in parallel with N processes. E.g.:
> 
>   make SPHINXOPTS=-j4 htmldocs
> 
> will no longer log warnings like:
> 
>   WARNING: the foobar extension does not declare if it is safe for
>   parallel reading, assuming it isn't - please ask the extension author
>   to check and make it explicit.
> 
> Add metadata to extensions:
> 
> * kernel-doc
> * flat-table
> * kernel-include
> 
> [1] http://www.sphinx-doc.org/en/stable/extdev/#extension-metadata
> 
> Signed-off-by: Markus Heiser 

Tested here on my desktop:

$ make cleandocs; time make SPHINXOPTS="-q" DOCBOOKS="" SPHINXDIRS=media 
SPHINX_CONF="conf.py" htmldocs
  SPHINX  htmldocs --> file:///devel/v4l/patchwork/Documentation/output/media;
make[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make 
rule.
  PARSE   include/uapi/linux/dvb/audio.h
  PARSE   include/uapi/linux/dvb/ca.h
  PARSE   include/uapi/linux/dvb/dmx.h
  PARSE   include/uapi/linux/dvb/frontend.h
  PARSE   include/uapi/linux/dvb/net.h
  PARSE   include/uapi/linux/dvb/video.h
  PARSE   include/uapi/linux/videodev2.h
  PARSE   include/uapi/linux/media.h
  PARSE   include/linux/cec.h
  PARSE   include/uapi/linux/lirc.h
load additional sphinx-config: /devel/v4l/patchwork/Documentation/media/conf.py
/devel/v4l/patchwork/Documentation/media/uapi/v4l/extended-controls.rst:2116: 
WARNING: Inline literal start-string without end-string.
/devel/v4l/patchwork/Documentation/media/uapi/v4l/extended-controls.rst:2116: 
WARNING: Inline literal start-string without end-string.
  SKIPDocBook htmldocs target (DOCBOOKS="" specified).

real0m55.837s
user0m54.620s
sys 0m1.333s

$ make cleandocs; time make SPHINXOPTS="-q -j8" DOCBOOKS="" SPHINXDIRS=media 
SPHINX_CONF="conf.py" htmldocs
  SPHINX  htmldocs --> file:///devel/v4l/patchwork/Documentation/output/media;
make[2]: warning: jobserver unavailable: using -j1.  Add '+' to parent make 
rule.
  PARSE   include/uapi/linux/dvb/audio.h
  PARSE   include/uapi/linux/dvb/ca.h
  PARSE   include/uapi/linux/dvb/dmx.h
  PARSE   include/uapi/linux/dvb/frontend.h
  PARSE   include/uapi/linux/dvb/net.h
  PARSE   include/uapi/linux/dvb/video.h
  PARSE   include/uapi/linux/videodev2.h
  PARSE   include/uapi/linux/media.h
  PARSE   include/linux/cec.h
  PARSE   include/uapi/linux/lirc.h
load additional sphinx-config: /devel/v4l/patchwork/Documentation/media/conf.py
/devel/v4l/patchwork/Documentation/media/uapi/v4l/extended-controls.rst:2116: 
WARNING: Inline literal start-string without end-string.
/devel/v4l/patchwork/Documentation/media/uapi/v4l/extended-controls.rst:2116: 
WARNING: Inline literal start-string without end-string.
  SKIPDocBook htmldocs target (DOCBOOKS="" specified).

real0m22.340s
user1m21.041s
sys 0m3.624s

Time reduced from 55 to 22 seconds. Sounds good enough!

Tested-by: Mauro Carvalho Chehab 

Thanks!
Mauro

PS: on my server with 16 dual-thread Xeon CPU, the gain with a
bigger value for -j was not impressive. Got about the same time as
with -j8 or -j32 there.

> ---
>  Documentation/sphinx/kernel-doc.py | 8 
>  Documentation/sphinx/kernel_include.py | 7 +++
>  Documentation/sphinx/rstFlatTable.py   | 6 ++
>  3 files changed, 21 insertions(+)
>  mode change 100644 => 100755 Documentation/sphinx/rstFlatTable.py
> 
> diff --git a/Documentation/sphinx/kernel-doc.py 
> b/Documentation/sphinx/kernel-doc.py
> index f6920c0..d15e07f 100644
> --- a/Documentation/sphinx/kernel-doc.py
> +++ b/Documentation/sphinx/kernel-doc.py
> @@ -39,6 +39,8 @@ from docutils.parsers.rst import directives
>  from sphinx.util.compat import Directive
>  from sphinx.ext.autodoc import AutodocReporter
>  
> +__version__  = '1.0'
> +
>  class KernelDocDirective(Directive):
>  """Extract kernel-doc comments from the specified file"""
>  required_argument = 1
> @@ -139,3 +141,9 @@ def setup(app):
>  app.add_config_value('kerneldoc_verbosity', 1, 'env')
>  
>  app.add_directive('kernel-doc', KernelDocDirective)
> +
> +return dict(
> +version = __version__,
> +parallel_read_safe = True,
> +parallel_write_safe = True
> +)
> diff --git a/Documentation/sphinx/kernel_include.py 
> b/Documentation/sphinx/kernel_include.py
> index db57382..f523aa6 100755
> --- a/Documentation/sphinx/kernel_include.py
> +++ b/Documentation/sphinx/kernel_include.py
> @@ -39,11 +39,18 @@ from docutils.parsers.rst import directives
>  from docutils.parsers.rst.directives.body import CodeBlock, NumberLines
>  from 

[PATCH] [media] extended-controls.rst: fix a build warning

2016-08-24 Thread Mauro Carvalho Chehab
/devel/v4l/patchwork/Documentation/media/uapi/v4l/extended-controls.rst:2116: 
WARNING: Inline literal start-string without end-string.

Signed-off-by: Mauro Carvalho Chehab 
---
 Documentation/media/uapi/v4l/extended-controls.rst | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/media/uapi/v4l/extended-controls.rst 
b/Documentation/media/uapi/v4l/extended-controls.rst
index 9c6aff3e97c1..1f1518e4859d 100644
--- a/Documentation/media/uapi/v4l/extended-controls.rst
+++ b/Documentation/media/uapi/v4l/extended-controls.rst
@@ -2112,8 +2112,8 @@ enum v4l2_mpeg_video_h264_sei_fp_arrangement_type -
 
 .. _v4l2-mpeg-video-h264-fmo-map-type:
 
-``V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE`` 
-(enum)
+``V4L2_CID_MPEG_VIDEO_H264_FMO_MAP_TYPE``
+   (enum)
 
 enum v4l2_mpeg_video_h264_fmo_map_type -
 When using FMO, the map type divides the image in different scan
-- 
2.7.4

--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Matthias Brugger



On 24/08/16 13:02, Rafał Miłecki wrote:

On 24 August 2016 at 12:49, Matthias Brugger  wrote:

On 24/08/16 00:03, Rafał Miłecki wrote:


[...]


+static int usbport_trig_notify(struct notifier_block *nb, unsigned long
action,
+  void *data)
+{
+   struct usbport_trig_data *usbport_data =
+   container_of(nb, struct usbport_trig_data, nb);
+   struct led_classdev *led_cdev = usbport_data->led_cdev;
+
+   switch (action) {
+   case USB_DEVICE_ADD:
+   if (usbport_trig_usb_dev_observed(usbport_data, data)) {



Maybe we should switch this and fist see if the usbport is observed before
evaluating the action. Also cast data to "struct usb_device *" to make that
clear.


I'm aware there is one duplicated line of code, I did to first
evaluate very quick test (checking unsigned long value), then iterate
over ports & keep only 1 switch block. I could move
usbport_trig_usb_dev_observed call up, but it would be executed for
other actions as well (currently just USB_BUS_ADD and USB_BUS_REMOVE).



Ok. I'm a USB noop but from my understanding the notifier is only called 
when a device or a hub gets added/removed. So this shouldn't happen that 
much. Therefor it has no impact if we check if the usb device is in the 
observer list for all actions.





+   if (usbport_data->count++ == 0)



I'm a bit puzzled. I think:
if (++usbport_data->count > 0)
makes this more consistent with the remove case.


Your condition would be always true (as we don't use negative
numbers). The point is to enable LED only if it was disabled before.
So I need to increase counter unconditionally but enable LED only if
initial value (before increasing it) was 0.



Got it. My personal opinion is, that adding one line for 
incrementing/decrementing the counter would help to make this 
crystal-clear to everyone (at least to me :)


Cheers,
Matthias




+module_init(usbport_trig_init);
+module_exit(usbport_trig_exit);
+
+MODULE_AUTHOR("Rafał Miłecki ");



Nit: ra...@milecki.pl


Oops, thanks!


--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Matthias Brugger



On 24/08/16 00:03, Rafał Miłecki wrote:

From: Rafał Miłecki 

This commit adds a new trigger responsible for turning on LED when USB
device gets connected to the specified USB port. This can can useful for
various home routers that have USB port(s) and a proper LED telling user
a device is connected.

The trigger gets its documentation file but basically it just requires
specifying USB port in a Linux format (e.g. echo 1-1 > new_port).

During work on this trigger there was a plan to add DT bindings for it,
but there wasn't an agreement on the format yet. This can be worked on
later, a sysfs interface is needed anyway for platforms not using DT.

Signed-off-by: Rafał Miłecki 
---
V2: Trying to add DT support, idea postponed as it will take more time
to discuss the bindings.
V3: Fix typos in commit and Documentation (thanks Jacek!)
Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
Check if there is USB device connected after adding new USB port
Fix memory leak or two

Felipe: I'd like to ask for your Ack before having this patch pushed.
---
 Documentation/leds/ledtrig-usbport.txt |  49 +++
 drivers/leds/trigger/Kconfig   |   8 ++
 drivers/leds/trigger/Makefile  |   1 +
 drivers/leds/trigger/ledtrig-usbport.c | 253 +
 4 files changed, 311 insertions(+)
 create mode 100644 Documentation/leds/ledtrig-usbport.txt
 create mode 100644 drivers/leds/trigger/ledtrig-usbport.c

diff --git a/Documentation/leds/ledtrig-usbport.txt 
b/Documentation/leds/ledtrig-usbport.txt
new file mode 100644
index 000..fa42227
--- /dev/null
+++ b/Documentation/leds/ledtrig-usbport.txt
@@ -0,0 +1,49 @@
+USB port LED trigger
+
+
+This LED trigger can be used for signalling to the user a presence of USB 
device
+in a given port. It simply turns on LED when device appears and turns it off
+when it disappears.
+
+It requires specifying a list of USB ports that should be observed. Used format
+matches Linux kernel format and consists of a root hub number and a hub port
+separated by a dash (e.g. 3-1).
+
+It is also possible to handle devices with internal hubs (that are always
+connected to the root hub). User can simply specify internal hub ports then
+(e.g. 1-1.1, 1-1.2, etc.).
+
+Please note that this trigger allows assigning multiple USB ports to a single
+LED. This can be useful in two cases:
+
+1) Device with single USB LED and few physical ports
+
+In such a case LED will be turned on as long as there is at least one connected
+USB device.
+
+2) Device with a physical port handled by few controllers
+
+Some devices have e.g. one controller per PHY standard. E.g. USB 3.0 physical
+port may be handled by ohci-platform, ehci-platform and xhci-hcd. If there is
+only one LED user will most likely want to assign ports from all 3 hubs.
+
+
+This trigger can be activated from user space on led class devices as shown
+below:
+
+  echo usbport > trigger
+
+This adds the following sysfs attributes to the LED:
+
+  ports - Reading it lists all USB ports assigned to the trigger. Writing USB
+ port number to it will make this driver start observing it. It's also
+ possible to remove USB port from observable list by writing it with a
+ "-" prefix.
+
+Example use-case:
+
+  echo usbport > trigger
+  echo 4-1 > ports
+  echo 2-1 > ports
+  echo -4-1 > ports
+  cat ports
diff --git a/drivers/leds/trigger/Kconfig b/drivers/leds/trigger/Kconfig
index 3f9ddb9..bdd6fd2 100644
--- a/drivers/leds/trigger/Kconfig
+++ b/drivers/leds/trigger/Kconfig
@@ -126,4 +126,12 @@ config LEDS_TRIGGER_PANIC
  a different trigger.
  If unsure, say Y.

+config LEDS_TRIGGER_USBPORT
+   tristate "USB port LED trigger"
+   depends on LEDS_TRIGGERS && USB
+   help
+ This allows LEDs to be controlled by USB events. Enabling this option
+ allows specifying list of USB ports that should turn on LED when some
+ USB device gets connected.
+
 endif # LEDS_TRIGGERS
diff --git a/drivers/leds/trigger/Makefile b/drivers/leds/trigger/Makefile
index a72c43c..56e1741 100644
--- a/drivers/leds/trigger/Makefile
+++ b/drivers/leds/trigger/Makefile
@@ -10,3 +10,4 @@ obj-$(CONFIG_LEDS_TRIGGER_DEFAULT_ON) += ledtrig-default-on.o
 obj-$(CONFIG_LEDS_TRIGGER_TRANSIENT)   += ledtrig-transient.o
 obj-$(CONFIG_LEDS_TRIGGER_CAMERA)  += ledtrig-camera.o
 obj-$(CONFIG_LEDS_TRIGGER_PANIC)   += ledtrig-panic.o
+obj-$(CONFIG_LEDS_TRIGGER_USBPORT) += ledtrig-usbport.o
diff --git a/drivers/leds/trigger/ledtrig-usbport.c 
b/drivers/leds/trigger/ledtrig-usbport.c
new file mode 100644
index 000..7f5237c
--- /dev/null
+++ b/drivers/leds/trigger/ledtrig-usbport.c
@@ -0,0 +1,253 @@
+/*
+ * USB port LED trigger
+ *
+ * Copyright (C) 2016 Rafał Miłecki 
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the 

Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Rafał Miłecki
On 24 August 2016 at 11:22, Greg KH  wrote:
> On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
>> +static ssize_t ports_show(struct device *dev, struct device_attribute *attr,
>> +   char *buf)
>> +{
>> + struct led_classdev *led_cdev = dev_get_drvdata(dev);
>> + struct usbport_trig_data *usbport_data = led_cdev->trigger_data;
>> + struct usbport_trig_port *port;
>> + ssize_t ret = 0;
>> + int len;
>> +
>> + list_for_each_entry(port, _data->ports, list) {
>> + len = sprintf(buf + ret, "%s\n", port->name);
>> + if (len >= 0)
>> + ret += len;
>> + }
>> +
>> + return ret;
>> +}
>
> sysfs is "one value per file", here you are listing a bunch of things in
> one sysfs file.  Please don't do that.

OK. What do you think about creating "ports" subdirectory and creating
file-per-port in it? Then I'd need to bring back something like
"new_port" and "remove_port". Does it sound OK?

-- 
Rafał
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH V3] leds: trigger: Introduce an USB port trigger

2016-08-24 Thread Greg KH
On Wed, Aug 24, 2016 at 12:03:29AM +0200, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> This commit adds a new trigger responsible for turning on LED when USB
> device gets connected to the specified USB port. This can can useful for
> various home routers that have USB port(s) and a proper LED telling user
> a device is connected.
> 
> The trigger gets its documentation file but basically it just requires
> specifying USB port in a Linux format (e.g. echo 1-1 > new_port).
> 
> During work on this trigger there was a plan to add DT bindings for it,
> but there wasn't an agreement on the format yet. This can be worked on
> later, a sysfs interface is needed anyway for platforms not using DT.
> 
> Signed-off-by: Rafał Miłecki 
> ---
> V2: Trying to add DT support, idea postponed as it will take more time
> to discuss the bindings.
> V3: Fix typos in commit and Documentation (thanks Jacek!)
> Use "ports" sysfs file for adding and removing USB ports (thx Jacek)
> Check if there is USB device connected after adding new USB port
> Fix memory leak or two
> 
> Felipe: I'd like to ask for your Ack before having this patch pushed.
> ---
>  Documentation/leds/ledtrig-usbport.txt |  49 +++
>  drivers/leds/trigger/Kconfig   |   8 ++
>  drivers/leds/trigger/Makefile  |   1 +
>  drivers/leds/trigger/ledtrig-usbport.c | 253 
> +
>  4 files changed, 311 insertions(+)
>  create mode 100644 Documentation/leds/ledtrig-usbport.txt
>  create mode 100644 drivers/leds/trigger/ledtrig-usbport.c

You are adding sysfs files without adding a Documentation/ABI/ entry,
please never do that.

NAK.

greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH v2 2/5] firmware: annotate thou shalt not request fw on init or probe

2016-08-24 Thread Daniel Vetter
On Fri, Jun 17, 2016 at 12:54 AM, Luis R. Rodriguez  wrote:
> Thou shalt not make firmware calls early on init or probe.
>
> systemd already ripped support out for the usermode helper
> a while ago, there are still users that require the usermode helper,
> however systemd's use of the usermode helper exacerbated a long
> lasting issue of the fact that we have many drivers that load
> firmware on init or probe. Independent of the usermode helper
> loading firmware on init or probe is a bad idea for a few reasons.
>
> When the firmware is read directly from the filesystem by the kernel,
> if the driver is built-in to the kernel the firmware may not yet be
> available, for such uses one could use initramfs and stuff the firmware
> inside, or one also use CONFIG_EXTRA_FIRMWARE; however not all distributions
> use this, as such generally one cannot count on this. There is another
> corner cases to consider, since we are accessing the firmware directly folks
> cannot expect new found firmware on a filesystem after we switch off from
> an initramfs with pivot_root().
>
> Supporting such situations is possible today but fixing it for good is
> really complex due to the large amount of variablity in the boot up
> process.
>
> Instead just document the expectations properly and add a grammar rule to
> enable folks to check / validate / police if drivers are using the request
> firmware API early on init or probe.
>
> The SmPL rule used to check for the probe routine is loose and is
> currently defined through a regexp, that can easily be extended to any
> other known bus probe routine names. It also uses the new Python
> iteration support which allows us to backtrack from a request_firmware*()
> call back to a possible probe or init, iteratively. Without iteration
> we would only be able to get reports for callers who directly use the
> request_firmware*() API on the initial probe or init routine.
>
> There are 4 offenders at this time:
>
> mcgrof@ergon ~/linux-next (git::20160609)$ export 
> COCCI=scripts/coccinelle/api/request_firmware.cocci
> mcgrof@ergon ~/linux-next (git::20160609)$ make coccicheck MODE=report
>
> drivers/fmc/fmc-fakedev.c: ERROR: driver call request firmware call on its 
> init routine on line 321.
> drivers/fmc/fmc-write-eeprom.c: ERROR: driver call request firmware call on 
> its probe routine on line 136.
> drivers/tty/serial/rp2.c: ERROR: driver call request firmware call on its 
> probe routine on line 796.
> drivers/tty/serial/ucc_uart.c: ERROR: driver call request firmware call on 
> its probe routine on line 1246.

Plus all gpu drivers which need firmware. And yes we must load them at
probe because people are generally pissed when they boot their machine
and the screen goes black. On top of that a lot of people want their
gpu drivers to be built-in, but can't ship the firmware blobs in the
kernel image because gpl. Yep, there's a bit a contradiction there ...

I think what would work is loading the different subsystems of the
driver in parallel (we already do that largely), and then if one
firmware blob isn't there yet simply stall that async worker until it
shows up. But the answers I've gotten thus far from request_firmware()
folks (well at least Greg) is don't do that ...

Is that problem still somewhere on the radar? Atm there's various
wait_for_rootfs hacks out there floating in vendor/product trees.
"Avoid at all costs" sounds like upstream prefers to not care about
android/cros in those case (yes I know most arm socs don't need
firmware, which would make it a problem fo just be a subset of all
devices).
-Daniel

> I checked and verified all these are valid reports. This list also matches
> the same list as in 20150805, so we have fortunately not gotten worse.
> Let's keep it that way and also fix these drivers.
>
> v2: changes from v1 [0]:
>
> o This now supports iteration, this extends our coverage on the report
>
> o Update documentation and commit log to accept the fate of not being
>   able to remove the usermode helper.
>
> [0] 
> https://lkml.kernel.org/r/1440811107-861-1-git-send-email-mcg...@do-not-panic.com
>
> Cc: Alessandro Rubini 
> Cc: Kevin Cernekee 
> Cc: Daniel Vetter 
> Cc: Mimi Zohar 
> Cc: David Woodhouse 
> Cc: Kees Cook 
> Cc: Dmitry Torokhov 
> Cc: Ming Lei 
> Cc: Jonathan Corbet 
> Cc: Julia Lawall 
> Cc: Gilles Muller 
> Cc: Nicolas Palix 
> Cc: Thierry Martinez 
> Cc: Michal Marek 
> Cc: co...@systeme.lip6.fr
> Cc: Alessandro Rubini 
> Cc: Kevin Cernekee 
> Cc: Greg Kroah-Hartman 
> Cc: Jiri Slaby 
> Cc: linux-ser...@vger.kernel.org
> Cc: