* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Boris Brezillon @ 2016-02-04 14:01 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160204110203.GL4455@sirena.org.uk>
Hi Mark, Thierry,
On Thu, 4 Feb 2016 11:02:03 +0000
Mark Brown <broonie@kernel.org> wrote:
> On Wed, Feb 03, 2016 at 11:04:20AM -0800, Doug Anderson wrote:
>
> > Sure. ...but you agree that somehow you need a new API call for this,
> > right? Somehow the PWM regulator needs to be able to say that it
> > wants the hardware state, not the initial state as specified in the
> > device tree.
>
> Wouldn't the most direct way to do that be to just not specify anything
> in the DT? If there *is* something in DT but we ignore it that's a bit
> weird.
Just adding some inputs on this specific aspect. The reason we have to
specify a period (and, to a lesser extent, the polarity) in the DT or
PWM lookup table is because what most PWM users want is to specify a
dutycycle relatively to a predefined period value.
If we decide to remove those information from the DT, then you'll need
a way to define it somewhere else, and then the is question is 'where?'.
Users that really want to control their period (this could the case for
the clk-pwm driver) could completely ignore DT/lookup-table information
and set the period and absolute dutycycle directly.
Now, from what I seen, what most PWM users want to do is:
pwm_set_rel_duty_scale(pwm, rel_value, scale);
or
rel_duty = pwm_get_rel_duty_cycle(pwm, scale);
where scale depends on the precision you need for your use case (most
of the time it's expressed in percent).
So, how about providing this kind of API (this is what I proposed in
one of my previous email)?
This would not only solve our problem (say you have a period at
boot-time that differs from the one you'll set when first applying a
new relative duty cycle, then the resulting relative value would still
be correct), but it would also remove a lot of boiler plate code from
PWM users code (if you take a look at pwm-regulator, pwm-leds, pwm-fan
and probably others, you'll see that they are all doing this conversion
manually).
Now, the last blocking point is, what if the PWM driver does not
implement HW-readout. In this case, the pwm-regulator will probably
expose a 0V output (IIRC, dutycycle is set to 0 by default) when it's
actually providing something else. But is this really important? I
mean, if the user really wants to have a reliable information, then he
will implement initial-state retrieval in its PWM controller driver.
Alternatively, we could put a flag specifying whether the PWM chip
supports initial state retrieval.
Am I missing something?
Best Regards,
Boris
--
Boris Brezillon, Free Electrons
Embedded Linux and Kernel engineering
http://free-electrons.com
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Mark Brown @ 2016-02-04 11:02 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=U_x1cxx6RrHuVgVV8GbOwm9DXXJcyiArbbQySUOaxrvQ@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 442 bytes --]
On Wed, Feb 03, 2016 at 11:04:20AM -0800, Doug Anderson wrote:
> Sure. ...but you agree that somehow you need a new API call for this,
> right? Somehow the PWM regulator needs to be able to say that it
> wants the hardware state, not the initial state as specified in the
> device tree.
Wouldn't the most direct way to do that be to just not specify anything
in the DT? If there *is* something in DT but we ignore it that's a bit
weird.
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]
^ permalink raw reply
* Re: fb_write missing in skeleton
From: Geert Uytterhoeven @ 2016-02-04 9:34 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAJ2oMhJb_GJYAmd8Ey=4uZGOo69oKk6BhuR2=AUcACGi=0irDw@mail.gmail.com>
Hi Ran,
On Thu, Feb 4, 2016 at 10:22 AM, Ran Shalit <ranshalit@gmail.com> wrote:
> On Thu, Feb 4, 2016 at 10:16 AM, Geert Uytterhoeven
> <geert@linux-m68k.org> wrote:
>> On Wed, Feb 3, 2016 at 9:54 PM, Ran Shalit <ranshalit@gmail.com> wrote:
>>> Is there any reason why fb_write is missing in skeletonfb.c ?
>>> Isn't it required ?
>>
>> No, it's not required, cfr. the comments in include/linux/fb.h:
>>
>> /* For framebuffers with strange non linear layouts or that do not
>> * work with normal memory mapped access
>> */
>> ssize_t (*fb_read)(struct fb_info *info, char __user *buf,
>> size_t count, loff_t *ppos);
>> ssize_t (*fb_write)(struct fb_info *info, const char __user *buf,
>> size_t count, loff_t *ppos);
>>
>> If not implemented, drivers/video/fbdev/core/fbmem.c:fb_write() will use
>> normal memory mapped access.
> But When we do:
> > cp mybitmap > /dev/fb0
> Which method is called (isn't fb_write is called) ?
drivers/video/fbdev/core/fbmem.c:fb_write()
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* Re: fb_write missing in skeleton
From: Ran Shalit @ 2016-02-04 9:22 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAJ2oMhJb_GJYAmd8Ey=4uZGOo69oKk6BhuR2=AUcACGi=0irDw@mail.gmail.com>
On Thu, Feb 4, 2016 at 10:16 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Ran,
>
> On Wed, Feb 3, 2016 at 9:54 PM, Ran Shalit <ranshalit@gmail.com> wrote:
>> Is there any reason why fb_write is missing in skeletonfb.c ?
>> Isn't it required ?
>
> No, it's not required, cfr. the comments in include/linux/fb.h:
>
> /* For framebuffers with strange non linear layouts or that do not
> * work with normal memory mapped access
> */
> ssize_t (*fb_read)(struct fb_info *info, char __user *buf,
> size_t count, loff_t *ppos);
> ssize_t (*fb_write)(struct fb_info *info, const char __user *buf,
> size_t count, loff_t *ppos);
>
> If not implemented, drivers/video/fbdev/core/fbmem.c:fb_write() will use
> normal memory mapped access.
>
> Gr{oetje,eeting}s,
>
> Geert
>
> --
> Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
>
> In personal conversations with technical people, I call myself a hacker. But
> when I'm talking to journalists I just say "programmer" or something like that.
> -- Linus Torvalds
Thanks Geert !
But When we do:
> cp mybitmap > /dev/fb0
Which method is called (isn't fb_write is called) ?
Is there a readme/tutorial which list the relation between userspace
operations (read/write/ioctl etc) to framebuffer drivers callbacks ?
I couldn't find any.
Best Regards,
Ran
^ permalink raw reply
* Re: fb_write missing in skeleton
From: Geert Uytterhoeven @ 2016-02-04 8:16 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <CAJ2oMhJb_GJYAmd8Ey=4uZGOo69oKk6BhuR2=AUcACGi=0irDw@mail.gmail.com>
Hi Ran,
On Wed, Feb 3, 2016 at 9:54 PM, Ran Shalit <ranshalit@gmail.com> wrote:
> Is there any reason why fb_write is missing in skeletonfb.c ?
> Isn't it required ?
No, it's not required, cfr. the comments in include/linux/fb.h:
/* For framebuffers with strange non linear layouts or that do not
* work with normal memory mapped access
*/
ssize_t (*fb_read)(struct fb_info *info, char __user *buf,
size_t count, loff_t *ppos);
ssize_t (*fb_write)(struct fb_info *info, const char __user *buf,
size_t count, loff_t *ppos);
If not implemented, drivers/video/fbdev/core/fbmem.c:fb_write() will use
normal memory mapped access.
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply
* fb_write missing in skeleton
From: Ran Shalit @ 2016-02-03 20:54 UTC (permalink / raw)
To: linux-fbdev
Hello,
Is there any reason why fb_write is missing in skeletonfb.c ?
Isn't it required ?
Thank you,
Ran
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Doug Anderson @ 2016-02-03 19:04 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160203145337.GD9650@ulmo>
Thierry
On Wed, Feb 3, 2016 at 6:53 AM, Thierry Reding <thierry.reding@gmail.com> wrote:
>> A) The software state here is the period and flags (AKA "inverted),
>> right? It does seem possible that you could apply the period and
>> flags while keeping the calculated bootup duty cycle percentage
>> (presuming that the PWM was actually enabled at probe time and there
>> was a bootup duty cycle at all). That would basically say that
>> whenever you set the period of a PWM then the duty cycle of the PWM
>> should remain the same percentage. That actually seems quite sane
>> IMHO. It seems much saner than trying to keep the duty cycle "ns"
>> when the period changes or resetting the PWM to some default when the
>> period changes.
>
> That really depends on the use-case. If you're interested in the output
> power of the PWM then, yes, this is sane. But it might not be the right
> answer in other cases.
Ah, I see. You're envisioning a device where active time in "ns" is
more important than the percentage of active time. Perhaps an LED
where it's more important to have it on for no more than .1 seconds
(so we don't drive it too long and burn it out?). If we slowed down
the duty cycle and adjusted the period to match, we could get into a
bad state.
>> B) Alternatively, I'd also say that setting a period without a duty
>> cycle doesn't make a lot of sense. ...so you could just apply the
>> period at the same time that you apply the duty cycle the first time.
>> Presumably you'd want to "lie" to the callers of the PWM subsystem and
>> tell them that you already changed the period even though the change
>> won't really take effect until they actually set the duty cycle. If
>> anyone cared to find out the true hardware period we could add a new
>> pwm_get_hw_period(). ...or since the only reason you'd want to know
>> the hardware period would be if you're trying to read the current duty
>> cycle percentage, you could instead add "pwm_get_hw_state()" and have
>> that return both the hardware period ns and duty cycle ns (which is
>> the most accurate way to return the "percentage" without using fix or
>> floating point math).
>
> But then you get into a situation where behaviour is dependent on the
> PWM driver, whereas this is really very specific to one specific use-
> case.
This is because only some drivers would be able to read the hardware
state? I'm not sure how we can get away from that. In all proposals
we've talked about (including what you propose below, right?) the PWM
regulator will need a PWM driver that can read hardware state. Only
PWM drivers that have been upgraded to support reading hardware state
can use the PWM regulator (or at least only those drivers will be able
to use the PWM regulator glitch-free).
When we add a new feature then it's expected that only updated drivers
will support that feature.
We need to make sure that we don't regress / negatively change the
behavior for anyone running non-updated drivers. ...and we should
strive to require as few changes to drivers as possible. ...but if
the best we can do requires changes to the PWM driver API then we will
certainly have differences depending on the PWM driver.
> In the end the PWM API is as low-level as it is because it needs to be
> flexible enough to cope with other use-cases. In the general case the
> simple truth is that it doesn't make sense to set a period without the
> duty cycle and vice versa. That's why pwm_config() takes both as input
> parameters. The atomic API is going to take that one step further in
> that you need to specify the complete state of the PWM when applying.
I guess this is still showing the strangeness of the device tree
specifying a period without a duty cycle. You just said it doesn't
make sense, but that's exactly what the device tree has.
I'd imagine that the only reason that the device tree specifies just
the period is that it's intended to be there only for clients that
don't care about the specific duty cycle in terms of seconds but
_only_ care about the duty cycle in terms of percentage. Anyone who
cared about the duty cycle in terms of seconds would presumably also
care about specifying the period (in terms of seconds) in the same
place they specify the duty cycle.
>> I think this is like my suggestion B), right? AKA the PWM regulator
>> would be the sole caller of pwm_get_hw_state() and it would use this
>> to figure out the existing duty cycle percentage. Then it would
>> translate that into "ns" and would set the duty cycle. Upon the first
>> set of the duty cycle both the period and duty cycle would be applied
>> at the same time.
>
> Yes and no. I think the PWM regulator would need to get the current
> hardware state and derive the output power from duty cycle and period.
> It would then need to rescale to whatever new period it wants to use
> to ensure the same output power is used.
Right. We all agree that somehow this needs to happen, it's just a
question of the implementation.
>> > That doesn't really get us closer, though. There is still the issue of
>> > the user having to deal with two states: the current hardware state and
>> > the software state as configured in DT or board files.
>>
>> I think the only users that need to deal with this are one that need a
>> seamless transition from bootup settings. Adding a new API call to
>> support a new feature like this doesn't seem insane, and anyone who
>> doesn't want this new feature can just never call the new API.
>>
>> The only thing that would "change" from the point of view of old
>> drivers is that the PWM period wouldn't change at bootup until the
>> duty cycle was set. IMHO this is probably a bug fix. AKA, for a PWM
>> backlight, imagine:
>>
>> 1. Firmware sets period to 20000 ns, duty cycle to 8000 ns (40%)
>> 2. Linux boots up and sets period to 10000 ns. Brightness of
>> backlight instantly goes to 80%.
>> 3. Eventually something decides to set the backlight duty cycle and it
>> goes to the proper rate.
>>
>> Skipping #2 seems like the right move. ...or did I misunderstand how
>> something works?
>
> I'm not aware of any code in the PWM subsystem that would do this
> automatically. If you don't call any of the pwm_*() functions the
> hardware state should not be modified. The responsibility is with
> the user drivers.
Ah, OK. I must have gotten confused.
Oh, I see. So pwm_get_period() is actually "lying" about the hardware
today! So what you're saying is that at boot time we grab the period
out of the device tree but we _don't_ apply it to the hardware, right?
I was thinking it would get applied right away...
That means that if you call pwm_get_period() right away at boot time
you're not getting the current period of the hardware but the period
that was specified in the device tree.
So all we need is a new API call that lets you read the hardware
values and make sure that the PWM regulator calls that before anyone
calls pwm_config(). That's roughly B) above.
> That is, it is up to the regulator or backlight driver to apply any new
> configuration to a PWM channel on boot, or leave it as is. For
> backlight it's probably fine to simply apply some default, since having
> the brightness change on boot isn't going to be terribly irritating.
>
> With the atomic API it would be possible to avoid even this and have the
> backlight driver simply read out the current hardware state and apply an
> equivalent brightness even if the period changed.
>
> For the regulator case you'd need to read out the current state and then
> recompute the values that will yield the same output power given data
> specified in DT. I think that much is already implemented in Boris'
> series, and it's really only the details that are being debated.
>
> The problematic issue is still that we might have a disparity between
> the current hardware state and the state initially specified by DT. In
> the general case the DT will specify the period and polarity of the PWM
> signal and leave it up to the user driver to determine what a correct
> duty cycle would be. With the atomic API we'll essentially have two
> states: the current (hardware) state and the "initial" state, which is
> what an OS will see as the state to apply. The problem now is that once
> you have applied the initial state with a duty cycle you've determined,
> there is no longer a need to keep it around. But there's also no way to
> know when this is the case. So the controversial part about all this is
> when to start using the current state rather that the initial state.
>
> The most straightforward way to solve this would be to apply the initial
> configuration on driver probe. That is, when the pwm-regulator driver
> gets probed it would retrieve the current and initial states, then
> adjust the current state such that it matches the initial state but with
> a duty cycle that yields the same output power as the current state, and
> finally apply the new state. After that, every regulator_set_voltage()
> call could simply operate on the current state and adjust the duty cycle
> exclusively.
>
> Does that sound reasonable?
Sure. ...but you agree that somehow you need a new API call for this,
right? Somehow the PWM regulator needs to be able to say that it
wants the hardware state, not the initial state as specified in the
device tree.
-Doug
^ permalink raw reply
* Re: [PATCH v3 00/12] pwm: add support for atomic update
From: Thierry Reding @ 2016-02-03 14:53 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <CAD=FV=Xvxeqo3imUmjukC5iYk3EZkrJ04G7C1B-w4xyr-r6-2Q@mail.gmail.com>
[-- Attachment #1: Type: text/plain, Size: 13874 bytes --]
On Mon, Jan 25, 2016 at 10:51:20AM -0800, Doug Anderson wrote:
> Hi,
>
> On Mon, Jan 25, 2016 at 9:08 AM, Thierry Reding
> <thierry.reding@gmail.com> wrote:
> > I really don't understand this design decision. I presume that the PWM
> > controlling this system-critical logic is driven by the SoC? So if the
> > regulator is system-critical, doesn't that make it a chicken and egg
> > problem? How can the SoC turn the PWM on if it doesn't have power? But
> > perhaps I'm completely misunderstanding what you're saying. Perhaps if
> > somebody could summarize how exactly this works, it would help better
> > understand the requirements or what's the correct thing to do.
>
> Sure, here's how the dang thing works, as I understand it.
>
> First, an overview of PWM regulator in general (maybe you know this,
> but to get us on the same page). There's an external regulator on the
> system. Looking on at least one board I see a TLV62565 specifically.
>
> From the docs of TLV62565, I see it describe the situation as the chip
> being able to provide an adjustable output voltage configurable via an
> external resistor divider. In simplified terms words you can adjust
> the output voltage of the regulator by tweaking the inputs to one of
> its pins. I'm just a software guy so I can't explain all the details
> of it, but the net-net of the situation is is that you can hook this
> configuration pin up to the output of a PWM (with a bunch of well
> balanced resistors and capacitors) and then you can set the voltage
> based on the output of the PWM.
>
>
> OK, so what happens at bootup? At bootup most of the pins of the
> rk3288 (including the PWM) are configured as inputs with a pull. The
> particular pin hooked up to this PWM has a pulldown. Remember that
> we've got this nicely balanced set of resistors and capacitors hooked
> up to the output of our PWM pin? So what happens when we have this
> pin configured as an input? As I understand it / remember it:
>
> * input w/ no pull: equivalent to 50% duty cycle on the PWM
> * input w/ pull down: equivalent to slightly higher voltage than 50%
> duty cycle on the PWM
> * input w/ pull up: equivalent to slightly lower voltage than 50% duty
> cycle on the PWM
>
> On our particular board that means that the rail comes up with roughly
> 1.1V. If you drive the PWM at 100% (or set the pin to output high)
> you get .86V and if you drive the PWM at 0% (or set the pin to output
> low) you get 1.36V.
>
> Now, 1.1V is plenty of voltage to boot the system. In fact most of
> the logic within the SoC can run as low as 0.95V I think. ...but 0.86
> V is not enough to run the logic parts of the system (even at their
> default bootup frequencies) 1.1V is _definitely_ not enough to run the
> SDRAM memory controller at full speed.
>
>
> So the bootloader wants to run the system fast so it can boot fast.
> It increases the CPU rails (as is typical for a bootloader) and moves
> the ARM CPU to 1.8GHz (from the relatively slow boot frequency) and
> also raises the logic rail to 1.2V (or I think 1.15 V on systems w/
> different memory configs) and inits the SDRAM controller to run at
> full speed. Then it boots Linux.
>
> Note: apparently in U-Boot they actually boot system slower (this was
> at least true 1.5 years ago with some reference U-Boot Rockchip
> provided). If I understand correctly they _didn't_ init the SDRAM
> controller as full speed in the bootloader and just left the logic
> rail at its bootup default. If everyone had done that then our job
> would be "easier" because we wouldn't need to read in the voltage
> provided by the bootloader (by reading the PWM and cros-referencing
> with our table), though even in that case we'd have to be very careful
> not to glitch the line (since .86 V is too low). Of course all of
> those systems are stuck running at a very slow memory speed until
> Linux gets DDR Frequency support for Rockchip whereas systems with our
> bootloader not only boot faster but also get to use the full memory
> speed even without any Linux DDRFreq drivers.
>
>
> In any case: I think I've demonstrated how a critical system rail can
> be using a PWM regulator and how glitching that PWM regulator at boot
> time can be catastrophic. Possibly it's not critical to be able to
> "read" the voltage that that bootloader left things configured at
> (it's mostly nice for debugging purposes), but it's definitely
> important to make sure we don't set it to some default and important
> to never glitch it. Said another way, presumably a DDR Freq driver
> would be able to switch the memory controller frequency sanely by
> reading the memory controller frequency and using that to figure out
> whether it needed to up the logic rail before or after the DDR Freq
> change.
Thanks for going into so much detail, this helps a lot in understanding
the actual problem we need to solve.
> > The problem that we've encountered is that since the PWM parameters are
> > specified in DT (or board files), there is the possibility of the PWM
> > hardware state and the board parameters disagreeing. To resolve such
> > situations there must be a point in time where both hardware state and
> > software state must be synchronized. Now the most straightforward way to
> > do that would be to simply apply the software state and be done with it.
> > However the software state initially lacks the duty cycle because it is
> > a parameter that usually depends on the use-case (for backlight for
> > instance it controls the brightness, for regulators it controls the
> > output voltage, ...).
>
> Excuse me for not knowing all details that have been talked about before, but...
>
> A) The software state here is the period and flags (AKA "inverted),
> right? It does seem possible that you could apply the period and
> flags while keeping the calculated bootup duty cycle percentage
> (presuming that the PWM was actually enabled at probe time and there
> was a bootup duty cycle at all). That would basically say that
> whenever you set the period of a PWM then the duty cycle of the PWM
> should remain the same percentage. That actually seems quite sane
> IMHO. It seems much saner than trying to keep the duty cycle "ns"
> when the period changes or resetting the PWM to some default when the
> period changes.
That really depends on the use-case. If you're interested in the output
power of the PWM then, yes, this is sane. But it might not be the right
answer in other cases.
> B) Alternatively, I'd also say that setting a period without a duty
> cycle doesn't make a lot of sense. ...so you could just apply the
> period at the same time that you apply the duty cycle the first time.
> Presumably you'd want to "lie" to the callers of the PWM subsystem and
> tell them that you already changed the period even though the change
> won't really take effect until they actually set the duty cycle. If
> anyone cared to find out the true hardware period we could add a new
> pwm_get_hw_period(). ...or since the only reason you'd want to know
> the hardware period would be if you're trying to read the current duty
> cycle percentage, you could instead add "pwm_get_hw_state()" and have
> that return both the hardware period ns and duty cycle ns (which is
> the most accurate way to return the "percentage" without using fix or
> floating point math).
But then you get into a situation where behaviour is dependent on the
PWM driver, whereas this is really very specific to one specific use-
case.
In the end the PWM API is as low-level as it is because it needs to be
flexible enough to cope with other use-cases. In the general case the
simple truth is that it doesn't make sense to set a period without the
duty cycle and vice versa. That's why pwm_config() takes both as input
parameters. The atomic API is going to take that one step further in
that you need to specify the complete state of the PWM when applying.
> Both of the above options seems like it could be sensible. The 2nd
> seems cleaner because it doesn't require you to recalculate /
> approximate the old duty cycle using a new period, but it's slightly
> uglier because it no longer returns the true hardware state from
> pwm_get_period().
>
>
> > Applying the software state as-is also means that there's no reason at
> > all to read out the hardware state in the first place, because it will
> > simply be discarded.
>
> Pretty sure we can't discard the hardware duty cycle at bootup, as per above.
>
>
> > An alternative would be to discard the software state and trust the
> > hardware to be configured correctly. That's somewhat risky because we
> > don't know if the hardware is properly configured. Or Linux might have
> > different requirements from the firmware and hence needs to configure
> > the PWM differently.
>
> Doesn't seem like a good idea either.
>
>
> > Neither of the above are very attractive options. The best I've been
> > able to come up with so far is to completely remove this decision from
> > the PWM subsystem and let users handle this. That is, a PWM regulator
> > driver would have to have all the knowledge about how to configure the
> > PWM for its needs. So upon probe, the PWM regulator driver would inspect
> > the current state of the PWM and adjust if necessary, then apply again.
> > Ideally of course it wouldn't have to do anything because the hardware
> > PWM state would match the software configuration. The idea here is that
> > the PWM regulator driver knows exactly what duty cycle to configure to
> > obtain the desired output voltage.
>
> I think this is like my suggestion B), right? AKA the PWM regulator
> would be the sole caller of pwm_get_hw_state() and it would use this
> to figure out the existing duty cycle percentage. Then it would
> translate that into "ns" and would set the duty cycle. Upon the first
> set of the duty cycle both the period and duty cycle would be applied
> at the same time.
Yes and no. I think the PWM regulator would need to get the current
hardware state and derive the output power from duty cycle and period.
It would then need to rescale to whatever new period it wants to use
to ensure the same output power is used.
> > That doesn't really get us closer, though. There is still the issue of
> > the user having to deal with two states: the current hardware state and
> > the software state as configured in DT or board files.
>
> I think the only users that need to deal with this are one that need a
> seamless transition from bootup settings. Adding a new API call to
> support a new feature like this doesn't seem insane, and anyone who
> doesn't want this new feature can just never call the new API.
>
> The only thing that would "change" from the point of view of old
> drivers is that the PWM period wouldn't change at bootup until the
> duty cycle was set. IMHO this is probably a bug fix. AKA, for a PWM
> backlight, imagine:
>
> 1. Firmware sets period to 20000 ns, duty cycle to 8000 ns (40%)
> 2. Linux boots up and sets period to 10000 ns. Brightness of
> backlight instantly goes to 80%.
> 3. Eventually something decides to set the backlight duty cycle and it
> goes to the proper rate.
>
> Skipping #2 seems like the right move. ...or did I misunderstand how
> something works?
I'm not aware of any code in the PWM subsystem that would do this
automatically. If you don't call any of the pwm_*() functions the
hardware state should not be modified. The responsibility is with
the user drivers.
That is, it is up to the regulator or backlight driver to apply any new
configuration to a PWM channel on boot, or leave it as is. For
backlight it's probably fine to simply apply some default, since having
the brightness change on boot isn't going to be terribly irritating.
With the atomic API it would be possible to avoid even this and have the
backlight driver simply read out the current hardware state and apply an
equivalent brightness even if the period changed.
For the regulator case you'd need to read out the current state and then
recompute the values that will yield the same output power given data
specified in DT. I think that much is already implemented in Boris'
series, and it's really only the details that are being debated.
The problematic issue is still that we might have a disparity between
the current hardware state and the state initially specified by DT. In
the general case the DT will specify the period and polarity of the PWM
signal and leave it up to the user driver to determine what a correct
duty cycle would be. With the atomic API we'll essentially have two
states: the current (hardware) state and the "initial" state, which is
what an OS will see as the state to apply. The problem now is that once
you have applied the initial state with a duty cycle you've determined,
there is no longer a need to keep it around. But there's also no way to
know when this is the case. So the controversial part about all this is
when to start using the current state rather that the initial state.
The most straightforward way to solve this would be to apply the initial
configuration on driver probe. That is, when the pwm-regulator driver
gets probed it would retrieve the current and initial states, then
adjust the current state such that it matches the initial state but with
a duty cycle that yields the same output power as the current state, and
finally apply the new state. After that, every regulator_set_voltage()
call could simply operate on the current state and adjust the duty cycle
exclusively.
Does that sound reasonable?
Thierry
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply
* Re: [PATCH] video: ARM CLCD: runtime check for Versatile
From: Arnd Bergmann @ 2016-02-03 9:05 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <20160203004154.GA10826@n2100.arm.linux.org.uk>
On Wednesday 03 February 2016 00:41:54 Russell King - ARM Linux wrote:
> On Tue, Feb 02, 2016 at 10:10:49PM +0100, Arnd Bergmann wrote:
> > On Tuesday 02 February 2016 21:47:39 Linus Walleij wrote:
> > > } else {
> > > -#ifdef CONFIG_ARCH_VERSATILE
> > > - fb->off_ienb = CLCD_PL111_IENB;
> > > - fb->off_cntl = CLCD_PL111_CNTL;
> > > -#else
> > > - fb->off_ienb = CLCD_PL110_IENB;
> > > - fb->off_cntl = CLCD_PL110_CNTL;
> > > -#endif
> > > + if (of_machine_is_compatible("arm,versatile-ab") ||
> > > + of_machine_is_compatible("arm,versatile-pb")) {
> > > + fb->off_ienb = CLCD_PL111_IENB;
> > > + fb->off_cntl = CLCD_PL111_CNTL;
> > > + } else {
> > > + fb->off_ienb = CLCD_PL110_IENB;
> > > + fb->off_cntl = CLCD_PL110_CNTL;
> > > + }
> > > }
> > >
> >
> > Could that be done based on the AMBA device ID instead?
>
> Unfortunately not. It's a mistake made on one Versatile board
> which reverses the registers. There's nothing to distinguish it
> in the primecell itself.
Ok, I see. It would be nice to have this knowledge locally in
the driver, e.g. through a special compatible string for the
variant on the versatile, or a bool property, but this way seems
good enough as we can be reasonably sure that nobody else made the
same mistake.
Arnd
^ permalink raw reply
* Re: [PATCH] video: ARM CLCD: runtime check for Versatile
From: Russell King - ARM Linux @ 2016-02-03 0:41 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <25978729.oSoMp4ZgxA@wuerfel>
On Tue, Feb 02, 2016 at 10:10:49PM +0100, Arnd Bergmann wrote:
> On Tuesday 02 February 2016 21:47:39 Linus Walleij wrote:
> > } else {
> > -#ifdef CONFIG_ARCH_VERSATILE
> > - fb->off_ienb = CLCD_PL111_IENB;
> > - fb->off_cntl = CLCD_PL111_CNTL;
> > -#else
> > - fb->off_ienb = CLCD_PL110_IENB;
> > - fb->off_cntl = CLCD_PL110_CNTL;
> > -#endif
> > + if (of_machine_is_compatible("arm,versatile-ab") ||
> > + of_machine_is_compatible("arm,versatile-pb")) {
> > + fb->off_ienb = CLCD_PL111_IENB;
> > + fb->off_cntl = CLCD_PL111_CNTL;
> > + } else {
> > + fb->off_ienb = CLCD_PL110_IENB;
> > + fb->off_cntl = CLCD_PL110_CNTL;
> > + }
> > }
> >
>
> Could that be done based on the AMBA device ID instead?
Unfortunately not. It's a mistake made on one Versatile board
which reverses the registers. There's nothing to distinguish it
in the primecell itself.
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply
* Re: [PATCH] video: ARM CLCD: runtime check for Versatile
From: Arnd Bergmann @ 2016-02-02 21:10 UTC (permalink / raw)
To: linux-arm-kernel
In-Reply-To: <1454446059-30526-1-git-send-email-linus.walleij@linaro.org>
On Tuesday 02 February 2016 21:47:39 Linus Walleij wrote:
> } else {
> -#ifdef CONFIG_ARCH_VERSATILE
> - fb->off_ienb = CLCD_PL111_IENB;
> - fb->off_cntl = CLCD_PL111_CNTL;
> -#else
> - fb->off_ienb = CLCD_PL110_IENB;
> - fb->off_cntl = CLCD_PL110_CNTL;
> -#endif
> + if (of_machine_is_compatible("arm,versatile-ab") ||
> + of_machine_is_compatible("arm,versatile-pb")) {
> + fb->off_ienb = CLCD_PL111_IENB;
> + fb->off_cntl = CLCD_PL111_CNTL;
> + } else {
> + fb->off_ienb = CLCD_PL110_IENB;
> + fb->off_cntl = CLCD_PL110_CNTL;
> + }
> }
>
Could that be done based on the AMBA device ID instead?
Arnd
^ permalink raw reply
* [PATCH] video: ARM CLCD: runtime check for Versatile
From: Linus Walleij @ 2016-02-02 20:47 UTC (permalink / raw)
To: linux-arm-kernel
The current compile-time check for inversed IENB/CNTL does not
work in multiplatform boots: as soon as versatile is included
in the build, the IENB/CNTL is switched and breaks graphics.
Convert this to a runtime switch.
Cc: stable@vger.kernel.org
Cc: Rob Herring <robh@kernel.org>
Cc: Russell King <linux@arm.linux.org.uk>
Fixes: a29da136de34 ("ARM: versatile: convert to multi-platform")
Signed-off-by: Linus Walleij <linus.walleij@linaro.org>
---
drivers/video/fbdev/amba-clcd.c | 15 ++++++++-------
1 file changed, 8 insertions(+), 7 deletions(-)
diff --git a/drivers/video/fbdev/amba-clcd.c b/drivers/video/fbdev/amba-clcd.c
index 9362424c2340..f9ef06d0cd48 100644
--- a/drivers/video/fbdev/amba-clcd.c
+++ b/drivers/video/fbdev/amba-clcd.c
@@ -440,13 +440,14 @@ static int clcdfb_register(struct clcd_fb *fb)
fb->off_ienb = CLCD_PL111_IENB;
fb->off_cntl = CLCD_PL111_CNTL;
} else {
-#ifdef CONFIG_ARCH_VERSATILE
- fb->off_ienb = CLCD_PL111_IENB;
- fb->off_cntl = CLCD_PL111_CNTL;
-#else
- fb->off_ienb = CLCD_PL110_IENB;
- fb->off_cntl = CLCD_PL110_CNTL;
-#endif
+ if (of_machine_is_compatible("arm,versatile-ab") ||
+ of_machine_is_compatible("arm,versatile-pb")) {
+ fb->off_ienb = CLCD_PL111_IENB;
+ fb->off_cntl = CLCD_PL111_CNTL;
+ } else {
+ fb->off_ienb = CLCD_PL110_IENB;
+ fb->off_cntl = CLCD_PL110_CNTL;
+ }
}
fb->clk = clk_get(&fb->dev->dev, NULL);
--
2.4.3
^ permalink raw reply related
* RE: [PATCH] fbdev: ssd1307fb: Fix activating the charge pump on ssd1306
From: Kai Ruhnau @ 2016-02-02 8:43 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1454339039-4976-1-git-send-email-kai.ruhnau@target-sg.com>
Hi,
I just saw that the same thing was already fixed by Julian in November, sorry for doubling this.
Best regards
Kai
-----Original Message-----
From: Kai Ruhnau
Sent: Montag, 1. Februar 2016 16:04
To: plagnioj@jcrosoft.com; tomi.valkeinen@ti.com
Cc: niederp@physik.uni-kl.de; linux-fbdev@vger.kernel.org; Kai Ruhnau <kai.ruhnau@target-sg.com>
Subject: [PATCH] fbdev: ssd1307fb: Fix activating the charge pump on ssd1306
The charge pump is activated by sending 0x14 where the higher bit is
always 1 and the lower bit actually toggles it.
Signed-off-by: Kai Ruhnau <kai.ruhnau@target-sg.com>
---
drivers/video/fbdev/ssd1307fb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index fa34808..20d249f 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -285,7 +285,7 @@ static void ssd1307fb_deferred_io(struct fb_info *info,
static int ssd1307fb_init(struct ssd1307fb_par *par)
{
int ret;
- u32 precharge, dclk, com_invdir, compins;
+ u32 precharge, dclk, com_invdir, compins, chargepump;
if (par->device_info->need_pwm) {
par->pwm = pwm_get(&par->client->dev, NULL);
@@ -388,8 +388,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
if (ret < 0)
return ret;
- ret = ssd1307fb_write_cmd(par->client,
- (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
+ chargepump = 0x10 | (par->device_info->need_chargepump & 0x1) << 2;
+ ret = ssd1307fb_write_cmd(par->client, chargepump);
if (ret < 0)
return ret;
--
2.4.3
--
Kai Ruhnau
Software Manager
T:+49 202 769302 19
Target Systemelektronik GmbH & Co. KG
Heinz-Fangman-Straße 4
42287 Wuppertal
Amtsgericht Wuppertal HRA 23898
Persönlich haftende Gesellschafterin
Target Systemelektronik Beteiligungs GmbH
Heinz-Fangman-Straße 4, 42287 Wuppertal
Amtsgericht Wuppertal HRB 25346
Geschäftsführer: Jürgen Stein
--
--
Kai Ruhnau
Software Manager
T:+49 202 769302 19
Target Systemelektronik GmbH & Co. KG
Heinz-Fangman-Straße 4
42287 Wuppertal
Amtsgericht Wuppertal HRA 23898
Persönlich haftende Gesellschafterin
Target Systemelektronik Beteiligungs GmbH
Heinz-Fangman-Straße 4, 42287 Wuppertal
Amtsgericht Wuppertal HRB 25346
Geschäftsführer: Jürgen Stein
--
^ permalink raw reply related
* [PATCH] fbdev: ssd1307fb: Fix activating the charge pump on ssd1306
From: Kai Ruhnau @ 2016-02-01 15:03 UTC (permalink / raw)
To: linux-fbdev
The charge pump is activated by sending 0x14 where the higher bit is
always 1 and the lower bit actually toggles it.
Signed-off-by: Kai Ruhnau <kai.ruhnau@target-sg.com>
---
drivers/video/fbdev/ssd1307fb.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/drivers/video/fbdev/ssd1307fb.c b/drivers/video/fbdev/ssd1307fb.c
index fa34808..20d249f 100644
--- a/drivers/video/fbdev/ssd1307fb.c
+++ b/drivers/video/fbdev/ssd1307fb.c
@@ -285,7 +285,7 @@ static void ssd1307fb_deferred_io(struct fb_info *info,
static int ssd1307fb_init(struct ssd1307fb_par *par)
{
int ret;
- u32 precharge, dclk, com_invdir, compins;
+ u32 precharge, dclk, com_invdir, compins, chargepump;
if (par->device_info->need_pwm) {
par->pwm = pwm_get(&par->client->dev, NULL);
@@ -388,8 +388,8 @@ static int ssd1307fb_init(struct ssd1307fb_par *par)
if (ret < 0)
return ret;
- ret = ssd1307fb_write_cmd(par->client,
- (par->device_info->need_chargepump & 0x1 << 2) & 0x14);
+ chargepump = 0x10 | (par->device_info->need_chargepump & 0x1) << 2;
+ ret = ssd1307fb_write_cmd(par->client, chargepump);
if (ret < 0)
return ret;
--
2.4.3
--
Kai Ruhnau
Software Manager
T:+49 202 769302 19
Target Systemelektronik GmbH & Co. KG
Heinz-Fangman-Straße 4
42287 Wuppertal
Amtsgericht Wuppertal HRA 23898
Persönlich haftende Gesellschafterin
Target Systemelektronik Beteiligungs GmbH
Heinz-Fangman-Straße 4, 42287 Wuppertal
Amtsgericht Wuppertal HRB 25346
Geschäftsführer: Jürgen Stein
--
^ permalink raw reply related
* Re: [PATCH v2 0/3] fbdev: omap2: panel-dpi: support reset-gpios and vcc regulator
From: Uwe Kleine-König @ 2016-02-01 10:07 UTC (permalink / raw)
To: linux-fbdev
In-Reply-To: <1450610002-10531-1-git-send-email-u.kleine-koenig@pengutronix.de>
Hello Tomi,
On Sun, Dec 20, 2015 at 12:13:19PM +0100, Uwe Kleine-König wrote:
> Hello,
>
> these patches are what is remaining from my previous series sent
> starting with
> Message-Id: 1449753107-11410-1-git-send-email-uwe@kleine-koenig.org .
>
> I split out the changes to
> Documentation/devicetree/bindings/video/panel-dpi.txt in a (single)
> separate patch. Also I changed the behaviour of the reset gpio to never
> assert it because there are too many different needs.
>
> Have fun
> Uwe
>
> Uwe Kleine-König (3):
> devicetree/bindings: add reset-gpios and vcc-supply for panel-dpi
> fbdev: omap2: panel-dpi: make (limited) use of a reset gpio
> fbdev: omap2: panel-dpi: implement support for a vcc regulator
you didn't comment these patches, do you have them still on your radar?
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König |
Industrial Linux Solutions | http://www.pengutronix.de/ |
^ permalink raw reply
* [patch] video: fbdev: metronomefb: two harmless off by one bugs
From: Dan Carpenter @ 2016-01-30 14:44 UTC (permalink / raw)
To: Jean-Christophe Plagniol-Villard, Jaya Kumar
Cc: Tomi Valkeinen, linux-fbdev, linux-kernel, kernel-janitors
par->metromem_cmd->args[] is an array of 31 elements of size u16. Here
we have initialized the first "i" elements and want to set the rest to
zero.
The issue here is that ARRAY_SIZE(par->metromem_cmd->args) is 31 and not
32 as in the original code. It means that we set ->csum to zero, but
that is harmless because we immediately set it to the correct value on
the next line.
Still, the buffer overflow upsets static checkers so let's correct the
math.
Signed-off-by: Dan Carpenter <dan.carpenter@oracle.com>
diff --git a/drivers/video/fbdev/metronomefb.c b/drivers/video/fbdev/metronomefb.c
index ad04a01..abb6bbf 100644
--- a/drivers/video/fbdev/metronomefb.c
+++ b/drivers/video/fbdev/metronomefb.c
@@ -354,7 +354,8 @@ static int metronome_powerup_cmd(struct metronomefb_par *par)
}
/* the rest are 0 */
- memset((u8 *) (par->metromem_cmd->args + i), 0, (32-i)*2);
+ memset(&par->metromem_cmd->args[i], 0,
+ (ARRAY_SIZE(par->metromem_cmd->args) - i) * 2);
par->metromem_cmd->csum = cs;
@@ -376,7 +377,8 @@ static int metronome_config_cmd(struct metronomefb_par *par)
memcpy(par->metromem_cmd->args, epd_frame_table[par->dt].config,
sizeof(epd_frame_table[par->dt].config));
/* the rest are 0 */
- memset((u8 *) (par->metromem_cmd->args + 4), 0, (32-4)*2);
+ memset(&par->metromem_cmd->args[4], 0,
+ (ARRAY_SIZE(par->metromem_cmd->args) - 4) * 2);
par->metromem_cmd->csum = 0xCC10;
par->metromem_cmd->csum += calc_img_cksum(par->metromem_cmd->args, 4);
^ permalink raw reply related
* [PATCH tip v7 7/7] rcu: use simple wait queues where possible in rcutree
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
From: Paul Gortmaker <paul.gortmaker@windriver.com>
As of commit dae6e64d2bcfd4b06304ab864c7e3a4f6b5fedf4 ("rcu: Introduce
proper blocking to no-CBs kthreads GP waits") the RCU subsystem started
making use of wait queues.
Here we convert all additions of RCU wait queues to use simple wait queues,
since they don't need the extra overhead of the full wait queue features.
Originally this was done for RT kernels[1], since we would get things like...
BUG: sleeping function called from invalid context at kernel/rtmutex.c:659
in_atomic(): 1, irqs_disabled(): 1, pid: 8, name: rcu_preempt
Pid: 8, comm: rcu_preempt Not tainted
Call Trace:
[<ffffffff8106c8d0>] __might_sleep+0xd0/0xf0
[<ffffffff817d77b4>] rt_spin_lock+0x24/0x50
[<ffffffff8106fcf6>] __wake_up+0x36/0x70
[<ffffffff810c4542>] rcu_gp_kthread+0x4d2/0x680
[<ffffffff8105f910>] ? __init_waitqueue_head+0x50/0x50
[<ffffffff810c4070>] ? rcu_gp_fqs+0x80/0x80
[<ffffffff8105eabb>] kthread+0xdb/0xe0
[<ffffffff8106b912>] ? finish_task_switch+0x52/0x100
[<ffffffff817e0754>] kernel_thread_helper+0x4/0x10
[<ffffffff8105e9e0>] ? __init_kthread_worker+0x60/0x60
[<ffffffff817e0750>] ? gs_change+0xb/0xb
...and hence simple wait queues were deployed on RT out of necessity
(as simple wait uses a raw lock), but mainline might as well take
advantage of the more streamline support as well.
[1] This is a carry forward of work from v3.10-rt; the original conversion
was by Thomas on an earlier -rt version, and Sebastian extended it to
additional post-3.10 added RCU waiters; here I've added a commit log and
unified the RCU changes into one, and uprev'd it to match mainline RCU.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Paul E. McKenney <paulmck@linux.vnet.ibm.com>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
kernel/rcu/tree.c | 22 +++++++++++-----------
kernel/rcu/tree.h | 13 +++++++------
kernel/rcu/tree_plugin.h | 26 +++++++++++++-------------
3 files changed, 31 insertions(+), 30 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index baf6d09..c3bfbaa 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1610,7 +1610,7 @@ static void rcu_gp_kthread_wake(struct rcu_state *rsp)
!READ_ONCE(rsp->gp_flags) ||
!rsp->gp_kthread)
return;
- wake_up(&rsp->gp_wq);
+ swake_up(&rsp->gp_wq);
}
/*
@@ -1990,7 +1990,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
int nocb = 0;
struct rcu_data *rdp;
struct rcu_node *rnp = rcu_get_root(rsp);
- wait_queue_head_t *sq;
+ struct swait_queue_head *sq;
WRITE_ONCE(rsp->gp_activity, jiffies);
raw_spin_lock_irq(&rnp->lock);
@@ -2078,7 +2078,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("reqwait"));
rsp->gp_state = RCU_GP_WAIT_GPS;
- wait_event_interruptible(rsp->gp_wq,
+ swait_event_interruptible(rsp->gp_wq,
READ_ONCE(rsp->gp_flags) &
RCU_GP_FLAG_INIT);
rsp->gp_state = RCU_GP_DONE_GPS;
@@ -2108,7 +2108,7 @@ static int __noreturn rcu_gp_kthread(void *arg)
READ_ONCE(rsp->gpnum),
TPS("fqswait"));
rsp->gp_state = RCU_GP_WAIT_FQS;
- ret = wait_event_interruptible_timeout(rsp->gp_wq,
+ ret = swait_event_interruptible_timeout(rsp->gp_wq,
rcu_gp_fqs_check_wake(rsp, &gf), j);
rsp->gp_state = RCU_GP_DOING_FQS;
/* Locking provides needed memory barriers. */
@@ -2232,7 +2232,7 @@ static void rcu_report_qs_rsp(struct rcu_state *rsp, unsigned long flags)
WARN_ON_ONCE(!rcu_gp_in_progress(rsp));
WRITE_ONCE(rsp->gp_flags, READ_ONCE(rsp->gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore(&rcu_get_root(rsp)->lock, flags);
- rcu_gp_kthread_wake(rsp);
+ swake_up(&rsp->gp_wq); /* Memory barrier implied by swake_up() path. */
}
/*
@@ -2893,7 +2893,7 @@ static void force_quiescent_state(struct rcu_state *rsp)
}
WRITE_ONCE(rsp->gp_flags, READ_ONCE(rsp->gp_flags) | RCU_GP_FLAG_FQS);
raw_spin_unlock_irqrestore(&rnp_old->lock, flags);
- rcu_gp_kthread_wake(rsp);
+ swake_up(&rsp->gp_wq); /* Memory barrier implied by swake_up() path. */
}
/*
@@ -3526,7 +3526,7 @@ static void __rcu_report_exp_rnp(struct rcu_state *rsp, struct rcu_node *rnp,
raw_spin_unlock_irqrestore(&rnp->lock, flags);
if (wake) {
smp_mb(); /* EGP done before wake_up(). */
- wake_up(&rsp->expedited_wq);
+ swake_up(&rsp->expedited_wq);
}
break;
}
@@ -3783,7 +3783,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
jiffies_start = jiffies;
for (;;) {
- ret = wait_event_interruptible_timeout(
+ ret = swait_event_timeout(
rsp->expedited_wq,
sync_rcu_preempt_exp_done(rnp_root),
jiffies_stall);
@@ -3791,7 +3791,7 @@ static void synchronize_sched_expedited_wait(struct rcu_state *rsp)
return;
if (ret < 0) {
/* Hit a signal, disable CPU stall warnings. */
- wait_event(rsp->expedited_wq,
+ swait_event(rsp->expedited_wq,
sync_rcu_preempt_exp_done(rnp_root));
return;
}
@@ -4457,8 +4457,8 @@ static void __init rcu_init_one(struct rcu_state *rsp,
}
}
- init_waitqueue_head(&rsp->gp_wq);
- init_waitqueue_head(&rsp->expedited_wq);
+ init_swait_queue_head(&rsp->gp_wq);
+ init_swait_queue_head(&rsp->expedited_wq);
rnp = rsp->level[rcu_num_lvls - 1];
for_each_possible_cpu(i) {
while (i > rnp->grphi)
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index aa47e2c..8a69b6d 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -27,6 +27,7 @@
#include <linux/threads.h>
#include <linux/cpumask.h>
#include <linux/seqlock.h>
+#include <linux/swait.h>
#include <linux/stop_machine.h>
/*
@@ -241,7 +242,7 @@ struct rcu_node {
/* Refused to boost: not sure why, though. */
/* This can happen due to race conditions. */
#ifdef CONFIG_RCU_NOCB_CPU
- wait_queue_head_t nocb_gp_wq[2];
+ struct swait_queue_head nocb_gp_wq[2];
/* Place for rcu_nocb_kthread() to wait GP. */
#endif /* #ifdef CONFIG_RCU_NOCB_CPU */
int need_future_gp[2];
@@ -393,7 +394,7 @@ struct rcu_data {
atomic_long_t nocb_q_count_lazy; /* invocation (all stages). */
struct rcu_head *nocb_follower_head; /* CBs ready to invoke. */
struct rcu_head **nocb_follower_tail;
- wait_queue_head_t nocb_wq; /* For nocb kthreads to sleep on. */
+ struct swait_queue_head nocb_wq; /* For nocb kthreads to sleep on. */
struct task_struct *nocb_kthread;
int nocb_defer_wakeup; /* Defer wakeup of nocb_kthread. */
@@ -472,7 +473,7 @@ struct rcu_state {
unsigned long gpnum; /* Current gp number. */
unsigned long completed; /* # of last completed gp. */
struct task_struct *gp_kthread; /* Task for grace periods. */
- wait_queue_head_t gp_wq; /* Where GP task waits. */
+ struct swait_queue_head gp_wq; /* Where GP task waits. */
short gp_flags; /* Commands for GP task. */
short gp_state; /* GP kthread sleep state. */
@@ -504,7 +505,7 @@ struct rcu_state {
atomic_long_t expedited_workdone3; /* # done by others #3. */
atomic_long_t expedited_normal; /* # fallbacks to normal. */
atomic_t expedited_need_qs; /* # CPUs left to check in. */
- wait_queue_head_t expedited_wq; /* Wait for check-ins. */
+ struct swait_queue_head expedited_wq; /* Wait for check-ins. */
int ncpus_snap; /* # CPUs seen last time. */
unsigned long jiffies_force_qs; /* Time at which to invoke */
@@ -607,8 +608,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
static void increment_cpu_stall_ticks(void);
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
-static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp);
-static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq);
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp);
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq);
static void rcu_init_one_nocb(struct rcu_node *rnp);
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
bool lazy, unsigned long flags);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 99b1ce6..3891984 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1822,9 +1822,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
* grace period.
*/
-static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
- wake_up_all(sq);
+ swake_up_all(sq);
}
/*
@@ -1840,15 +1840,15 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
}
-static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
{
return &rnp->nocb_gp_wq[rnp->completed & 0x1];
}
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
- init_waitqueue_head(&rnp->nocb_gp_wq[0]);
- init_waitqueue_head(&rnp->nocb_gp_wq[1]);
+ init_swait_queue_head(&rnp->nocb_gp_wq[0]);
+ init_swait_queue_head(&rnp->nocb_gp_wq[1]);
}
#ifndef CONFIG_RCU_NOCB_CPU_ALL
@@ -1873,7 +1873,7 @@ static void wake_nocb_leader(struct rcu_data *rdp, bool force)
if (READ_ONCE(rdp_leader->nocb_leader_sleep) || force) {
/* Prior smp_mb__after_atomic() orders against prior enqueue. */
WRITE_ONCE(rdp_leader->nocb_leader_sleep, false);
- wake_up(&rdp_leader->nocb_wq);
+ swake_up(&rdp_leader->nocb_wq);
}
}
@@ -2086,7 +2086,7 @@ static void rcu_nocb_wait_gp(struct rcu_data *rdp)
*/
trace_rcu_future_gp(rnp, rdp, c, TPS("StartWait"));
for (;;) {
- wait_event_interruptible(
+ swait_event_interruptible(
rnp->nocb_gp_wq[c & 0x1],
(d = ULONG_CMP_GE(READ_ONCE(rnp->completed), c)));
if (likely(d))
@@ -2114,7 +2114,7 @@ wait_again:
/* Wait for callbacks to appear. */
if (!rcu_nocb_poll) {
trace_rcu_nocb_wake(my_rdp->rsp->name, my_rdp->cpu, "Sleep");
- wait_event_interruptible(my_rdp->nocb_wq,
+ swait_event_interruptible(my_rdp->nocb_wq,
!READ_ONCE(my_rdp->nocb_leader_sleep));
/* Memory barrier handled by smp_mb() calls below and repoll. */
} else if (firsttime) {
@@ -2189,7 +2189,7 @@ wait_again:
* List was empty, wake up the follower.
* Memory barriers supplied by atomic_long_add().
*/
- wake_up(&rdp->nocb_wq);
+ swake_up(&rdp->nocb_wq);
}
}
@@ -2210,7 +2210,7 @@ static void nocb_follower_wait(struct rcu_data *rdp)
if (!rcu_nocb_poll) {
trace_rcu_nocb_wake(rdp->rsp->name, rdp->cpu,
"FollowerSleep");
- wait_event_interruptible(rdp->nocb_wq,
+ swait_event_interruptible(rdp->nocb_wq,
READ_ONCE(rdp->nocb_follower_head));
} else if (firsttime) {
/* Don't drown trace log with "Poll"! */
@@ -2369,7 +2369,7 @@ void __init rcu_init_nohz(void)
static void __init rcu_boot_init_nocb_percpu_data(struct rcu_data *rdp)
{
rdp->nocb_tail = &rdp->nocb_head;
- init_waitqueue_head(&rdp->nocb_wq);
+ init_swait_queue_head(&rdp->nocb_wq);
rdp->nocb_follower_tail = &rdp->nocb_follower_head;
}
@@ -2519,7 +2519,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
return false;
}
-static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
+static void rcu_nocb_gp_cleanup(struct swait_queue_head *sq)
{
}
@@ -2527,7 +2527,7 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
{
}
-static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+static struct swait_queue_head *rcu_nocb_gp_get(struct rcu_node *rnp)
{
return NULL;
}
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 6/7] rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
rcu_nocb_gp_cleanup() is called while holding rnp->lock. Currently,
this is okay because the wake_up_all() in rcu_nocb_gp_cleanup() will
not enable the IRQs. lockdep is happy.
By switching over using swait this is not true anymore. swake_up_all()
enables the IRQs while processing the waiters. __do_softirq() can now
run and will eventually call rcu_process_callbacks() which wants to
grap nrp->lock.
Let's move the rcu_nocb_gp_cleanup() call outside the lock before we
switch over to swait.
If we would hold the rnp->lock and use swait, lockdep reports
following:
================ [ INFO: inconsistent lock state ]
4.2.0-rc5-00025-g9a73ba0 #136 Not tainted
---------------------------------
inconsistent {IN-SOFTIRQ-W} -> {SOFTIRQ-ON-W} usage.
rcu_preempt/8 [HC0[0]:SC0[0]:HE1:SE1] takes:
(rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
{IN-SOFTIRQ-W} state was registered at:
[<ffffffff81109b9f>] __lock_acquire+0xd5f/0x21e0
[<ffffffff8110be0f>] lock_acquire+0xdf/0x2b0
[<ffffffff81841cc9>] _raw_spin_lock_irqsave+0x59/0xa0
[<ffffffff81136991>] rcu_process_callbacks+0x141/0x3c0
[<ffffffff810b1a9d>] __do_softirq+0x14d/0x670
[<ffffffff810b2214>] irq_exit+0x104/0x110
[<ffffffff81844e96>] smp_apic_timer_interrupt+0x46/0x60
[<ffffffff81842e70>] apic_timer_interrupt+0x70/0x80
[<ffffffff810dba66>] rq_attach_root+0xa6/0x100
[<ffffffff810dbc2d>] cpu_attach_domain+0x16d/0x650
[<ffffffff810e4b42>] build_sched_domains+0x942/0xb00
[<ffffffff821777c2>] sched_init_smp+0x509/0x5c1
[<ffffffff821551e3>] kernel_init_freeable+0x172/0x28f
[<ffffffff8182cdce>] kernel_init+0xe/0xe0
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
irq event stamp: 76
hardirqs last enabled at (75): [<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
hardirqs last disabled at (76): [<ffffffff8184116f>] _raw_spin_lock_irq+0x1f/0x90
softirqs last enabled at (0): [<ffffffff810a8df2>] copy_process.part.26+0x602/0x1cf0
softirqs last disabled at (0): [< (null)>] (null)
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(rcu_node_1);
<Interrupt>
lock(rcu_node_1);
*** DEADLOCK ***
1 lock held by rcu_preempt/8:
#0: (rcu_node_1){+.?...}, at: [<ffffffff811387c7>] rcu_gp_kthread+0xb97/0xeb0
stack backtrace:
CPU: 0 PID: 8 Comm: rcu_preempt Not tainted 4.2.0-rc5-00025-g9a73ba0 #136
Hardware name: Dell Inc. PowerEdge R820/066N7P, BIOS 2.0.20 01/16/2014
0000000000000000 000000006d7e67d8 ffff881fb081fbd8 ffffffff818379e0
0000000000000000 ffff881fb0812a00 ffff881fb081fc38 ffffffff8110813b
0000000000000000 0000000000000001 ffff881f00000001 ffffffff8102fa4f
Call Trace:
[<ffffffff818379e0>] dump_stack+0x4f/0x7b
[<ffffffff8110813b>] print_usage_bug+0x1db/0x1e0
[<ffffffff8102fa4f>] ? save_stack_trace+0x2f/0x50
[<ffffffff811087ad>] mark_lock+0x66d/0x6e0
[<ffffffff81107790>] ? check_usage_forwards+0x150/0x150
[<ffffffff81108898>] mark_held_locks+0x78/0xa0
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff81108a28>] trace_hardirqs_on_caller+0x168/0x220
[<ffffffff81108aed>] trace_hardirqs_on+0xd/0x10
[<ffffffff81841330>] _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810fd1c7>] swake_up_all+0xb7/0xe0
[<ffffffff811386e1>] rcu_gp_kthread+0xab1/0xeb0
[<ffffffff811089bf>] ? trace_hardirqs_on_caller+0xff/0x220
[<ffffffff81841341>] ? _raw_spin_unlock_irq+0x41/0x60
[<ffffffff81137c30>] ? rcu_barrier+0x20/0x20
[<ffffffff810d2014>] kthread+0x104/0x120
[<ffffffff81841330>] ? _raw_spin_unlock_irq+0x30/0x60
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
[<ffffffff8184231f>] ret_from_fork+0x3f/0x70
[<ffffffff810d1f10>] ? kthread_create_on_node+0x260/0x260
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: "Paul E. McKenney" <paulmck@linux.vnet.ibm.com>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
kernel/rcu/tree.c | 4 +++-
kernel/rcu/tree.h | 3 ++-
kernel/rcu/tree_plugin.h | 16 +++++++++++++---
3 files changed, 18 insertions(+), 5 deletions(-)
diff --git a/kernel/rcu/tree.c b/kernel/rcu/tree.c
index f07343b..baf6d09 100644
--- a/kernel/rcu/tree.c
+++ b/kernel/rcu/tree.c
@@ -1590,7 +1590,6 @@ static int rcu_future_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
int needmore;
struct rcu_data *rdp = this_cpu_ptr(rsp->rda);
- rcu_nocb_gp_cleanup(rsp, rnp);
rnp->need_future_gp[c & 0x1] = 0;
needmore = rnp->need_future_gp[(c + 1) & 0x1];
trace_rcu_future_gp(rnp, rdp, c,
@@ -1991,6 +1990,7 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
int nocb = 0;
struct rcu_data *rdp;
struct rcu_node *rnp = rcu_get_root(rsp);
+ wait_queue_head_t *sq;
WRITE_ONCE(rsp->gp_activity, jiffies);
raw_spin_lock_irq(&rnp->lock);
@@ -2029,7 +2029,9 @@ static void rcu_gp_cleanup(struct rcu_state *rsp)
needgp = __note_gp_changes(rsp, rnp, rdp) || needgp;
/* smp_mb() provided by prior unlock-lock pair. */
nocb += rcu_future_gp_cleanup(rsp, rnp);
+ sq = rcu_nocb_gp_get(rnp);
raw_spin_unlock_irq(&rnp->lock);
+ rcu_nocb_gp_cleanup(sq);
cond_resched_rcu_qs();
WRITE_ONCE(rsp->gp_activity, jiffies);
rcu_gp_slow(rsp, gp_cleanup_delay);
diff --git a/kernel/rcu/tree.h b/kernel/rcu/tree.h
index 9fb4e23..aa47e2c 100644
--- a/kernel/rcu/tree.h
+++ b/kernel/rcu/tree.h
@@ -607,7 +607,8 @@ static void zero_cpu_stall_ticks(struct rcu_data *rdp);
static void increment_cpu_stall_ticks(void);
static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu);
static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq);
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp);
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp);
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq);
static void rcu_init_one_nocb(struct rcu_node *rnp);
static bool __call_rcu_nocb(struct rcu_data *rdp, struct rcu_head *rhp,
bool lazy, unsigned long flags);
diff --git a/kernel/rcu/tree_plugin.h b/kernel/rcu/tree_plugin.h
index 630c197..99b1ce6 100644
--- a/kernel/rcu/tree_plugin.h
+++ b/kernel/rcu/tree_plugin.h
@@ -1822,9 +1822,9 @@ early_param("rcu_nocb_poll", parse_rcu_nocb_poll);
* Wake up any no-CBs CPUs' kthreads that were waiting on the just-ended
* grace period.
*/
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
{
- wake_up_all(&rnp->nocb_gp_wq[rnp->completed & 0x1]);
+ wake_up_all(sq);
}
/*
@@ -1840,6 +1840,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
rnp->need_future_gp[(rnp->completed + 1) & 0x1] += nrq;
}
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return &rnp->nocb_gp_wq[rnp->completed & 0x1];
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
init_waitqueue_head(&rnp->nocb_gp_wq[0]);
@@ -2514,7 +2519,7 @@ static bool rcu_nocb_cpu_needs_barrier(struct rcu_state *rsp, int cpu)
return false;
}
-static void rcu_nocb_gp_cleanup(struct rcu_state *rsp, struct rcu_node *rnp)
+static void rcu_nocb_gp_cleanup(wait_queue_head_t *sq)
{
}
@@ -2522,6 +2527,11 @@ static void rcu_nocb_gp_set(struct rcu_node *rnp, int nrq)
{
}
+static wait_queue_head_t *rcu_nocb_gp_get(struct rcu_node *rnp)
+{
+ return NULL;
+}
+
static void rcu_init_one_nocb(struct rcu_node *rnp)
{
}
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 5/7] KVM: use simple waitqueue for vcpu->wq
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
From: Marcelo Tosatti <mtosatti@redhat.com>
The problem:
On -rt, an emulated LAPIC timer instances has the following path:
1) hard interrupt
2) ksoftirqd is scheduled
3) ksoftirqd wakes up vcpu thread
4) vcpu thread is scheduled
This extra context switch introduces unnecessary latency in the
LAPIC path for a KVM guest.
The solution:
Allow waking up vcpu thread from hardirq context,
thus avoiding the need for ksoftirqd to be scheduled.
Normal waitqueues make use of spinlocks, which on -RT
are sleepable locks. Therefore, waking up a waitqueue
waiter involves locking a sleeping lock, which
is not allowed from hard interrupt context.
cyclictest command line:
This patch reduces the average latency in my tests from 14us to 11us.
Daniel writes:
Paolo asked for numbers from kvm-unit-tests/tscdeadline_latency
benchmark on mainline. The test was run 1000 times on
tip/sched/core 4.4.0-rc8-01134-g0905f04:
./x86-run x86/tscdeadline_latency.flat -cpu host
with idle=poll.
The test seems not to deliver really stable numbers though most of
them are smaller. Paolo write:
"Anything above ~10000 cycles means that the host went to C1 or
lower---the number means more or less nothing in that case.
The mean shows an improvement indeed."
Before:
min max mean std
count 1000.000000 1000.000000 1000.000000 1000.000000
mean 5162.596000 2019270.084000 5824.491541 20681.645558
std 75.431231 622607.723969 89.575700 6492.272062
min 4466.000000 23928.000000 5537.926500 585.864966
25% 5163.000000 1613252.750000 5790.132275 16683.745433
50% 5175.000000 2281919.000000 5834.654000 23151.990026
75% 5190.000000 2382865.750000 5861.412950 24148.206168
max 5228.000000 4175158.000000 6254.827300 46481.048691
After
min max mean std
count 1000.000000 1000.00000 1000.000000 1000.000000
mean 5143.511000 2076886.10300 5813.312474 21207.357565
std 77.668322 610413.09583 86.541500 6331.915127
min 4427.000000 25103.00000 5529.756600 559.187707
25% 5148.000000 1691272.75000 5784.889825 17473.518244
50% 5160.000000 2308328.50000 5832.025000 23464.837068
75% 5172.000000 2393037.75000 5853.177675 24223.969976
max 5222.000000 3922458.00000 6186.720500 42520.379830
[Patch was originaly based on the swait implementation found in the -rt
tree. Daniel ported it to mainline's version and gathered the
benchmark numbers for tscdeadline_latency test.]
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Marcelo Tosatti <mtosatti@redhat.com>
Cc: Paolo Bonzini <pbonzini@redhat.com>
---
arch/arm/kvm/arm.c | 8 ++++----
arch/arm/kvm/psci.c | 4 ++--
arch/mips/kvm/mips.c | 8 ++++----
arch/powerpc/include/asm/kvm_host.h | 4 ++--
arch/powerpc/kvm/book3s_hv.c | 23 +++++++++++------------
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kvm/interrupt.c | 4 ++--
arch/x86/kvm/lapic.c | 6 +++---
include/linux/kvm_host.h | 5 +++--
virt/kvm/async_pf.c | 4 ++--
virt/kvm/kvm_main.c | 17 ++++++++---------
11 files changed, 42 insertions(+), 43 deletions(-)
diff --git a/arch/arm/kvm/arm.c b/arch/arm/kvm/arm.c
index e06fd29..f7e3235 100644
--- a/arch/arm/kvm/arm.c
+++ b/arch/arm/kvm/arm.c
@@ -498,18 +498,18 @@ static void kvm_arm_resume_guest(struct kvm *kvm)
struct kvm_vcpu *vcpu;
kvm_for_each_vcpu(i, vcpu, kvm) {
- wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
+ struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
vcpu->arch.pause = false;
- wake_up_interruptible(wq);
+ swake_up(wq);
}
}
static void vcpu_sleep(struct kvm_vcpu *vcpu)
{
- wait_queue_head_t *wq = kvm_arch_vcpu_wq(vcpu);
+ struct swait_queue_head *wq = kvm_arch_vcpu_wq(vcpu);
- wait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
+ swait_event_interruptible(*wq, ((!vcpu->arch.power_off) &&
(!vcpu->arch.pause)));
}
diff --git a/arch/arm/kvm/psci.c b/arch/arm/kvm/psci.c
index a9b3b90..c2b1315 100644
--- a/arch/arm/kvm/psci.c
+++ b/arch/arm/kvm/psci.c
@@ -70,7 +70,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
{
struct kvm *kvm = source_vcpu->kvm;
struct kvm_vcpu *vcpu = NULL;
- wait_queue_head_t *wq;
+ struct swait_queue_head *wq;
unsigned long cpu_id;
unsigned long context_id;
phys_addr_t target_pc;
@@ -119,7 +119,7 @@ static unsigned long kvm_psci_vcpu_on(struct kvm_vcpu *source_vcpu)
smp_mb(); /* Make sure the above is visible */
wq = kvm_arch_vcpu_wq(vcpu);
- wake_up_interruptible(wq);
+ swake_up(wq);
return PSCI_RET_SUCCESS;
}
diff --git a/arch/mips/kvm/mips.c b/arch/mips/kvm/mips.c
index b9b803f..28e9acb 100644
--- a/arch/mips/kvm/mips.c
+++ b/arch/mips/kvm/mips.c
@@ -445,8 +445,8 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu,
dvcpu->arch.wait = 0;
- if (waitqueue_active(&dvcpu->wq))
- wake_up_interruptible(&dvcpu->wq);
+ if (swait_active(&dvcpu->wq))
+ swake_up(&dvcpu->wq);
return 0;
}
@@ -1174,8 +1174,8 @@ static void kvm_mips_comparecount_func(unsigned long data)
kvm_mips_callbacks->queue_timer_int(vcpu);
vcpu->arch.wait = 0;
- if (waitqueue_active(&vcpu->wq))
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq))
+ swake_up(&vcpu->wq);
}
/* low level hrtimer wake routine */
diff --git a/arch/powerpc/include/asm/kvm_host.h b/arch/powerpc/include/asm/kvm_host.h
index cfa758c..f8673ff 100644
--- a/arch/powerpc/include/asm/kvm_host.h
+++ b/arch/powerpc/include/asm/kvm_host.h
@@ -286,7 +286,7 @@ struct kvmppc_vcore {
struct list_head runnable_threads;
struct list_head preempt_list;
spinlock_t lock;
- wait_queue_head_t wq;
+ struct swait_queue_head wq;
spinlock_t stoltb_lock; /* protects stolen_tb and preempt_tb */
u64 stolen_tb;
u64 preempt_tb;
@@ -626,7 +626,7 @@ struct kvm_vcpu_arch {
u8 prodded;
u32 last_inst;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
struct kvmppc_vcore *vcore;
int ret;
int trap;
diff --git a/arch/powerpc/kvm/book3s_hv.c b/arch/powerpc/kvm/book3s_hv.c
index a7352b5..df34a64 100644
--- a/arch/powerpc/kvm/book3s_hv.c
+++ b/arch/powerpc/kvm/book3s_hv.c
@@ -114,11 +114,11 @@ static bool kvmppc_ipi_thread(int cpu)
static void kvmppc_fast_vcpu_kick_hv(struct kvm_vcpu *vcpu)
{
int cpu;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
wqp = kvm_arch_vcpu_wq(vcpu);
- if (waitqueue_active(wqp)) {
- wake_up_interruptible(wqp);
+ if (swait_active(wqp)) {
+ swake_up(wqp);
++vcpu->stat.halt_wakeup;
}
@@ -707,8 +707,8 @@ int kvmppc_pseries_do_hcall(struct kvm_vcpu *vcpu)
tvcpu->arch.prodded = 1;
smp_mb();
if (vcpu->arch.ceded) {
- if (waitqueue_active(&vcpu->wq)) {
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq)) {
+ swake_up(&vcpu->wq);
vcpu->stat.halt_wakeup++;
}
}
@@ -1447,7 +1447,7 @@ static struct kvmppc_vcore *kvmppc_vcore_create(struct kvm *kvm, int core)
INIT_LIST_HEAD(&vcore->runnable_threads);
spin_lock_init(&vcore->lock);
spin_lock_init(&vcore->stoltb_lock);
- init_waitqueue_head(&vcore->wq);
+ init_swait_queue_head(&vcore->wq);
vcore->preempt_tb = TB_NIL;
vcore->lpcr = kvm->arch.lpcr;
vcore->first_vcpuid = core * threads_per_subcore;
@@ -2519,10 +2519,9 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
{
struct kvm_vcpu *vcpu;
int do_sleep = 1;
+ DECLARE_SWAITQUEUE(wait);
- DEFINE_WAIT(wait);
-
- prepare_to_wait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
+ prepare_to_swait(&vc->wq, &wait, TASK_INTERRUPTIBLE);
/*
* Check one last time for pending exceptions and ceded state after
@@ -2536,7 +2535,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
}
if (!do_sleep) {
- finish_wait(&vc->wq, &wait);
+ finish_swait(&vc->wq, &wait);
return;
}
@@ -2544,7 +2543,7 @@ static void kvmppc_vcore_blocked(struct kvmppc_vcore *vc)
trace_kvmppc_vcore_blocked(vc, 0);
spin_unlock(&vc->lock);
schedule();
- finish_wait(&vc->wq, &wait);
+ finish_swait(&vc->wq, &wait);
spin_lock(&vc->lock);
vc->vcore_state = VCORE_INACTIVE;
trace_kvmppc_vcore_blocked(vc, 1);
@@ -2600,7 +2599,7 @@ static int kvmppc_run_vcpu(struct kvm_run *kvm_run, struct kvm_vcpu *vcpu)
kvmppc_start_thread(vcpu, vc);
trace_kvm_guest_enter(vcpu);
} else if (vc->vcore_state = VCORE_SLEEPING) {
- wake_up(&vc->wq);
+ swake_up(&vc->wq);
}
}
diff --git a/arch/s390/include/asm/kvm_host.h b/arch/s390/include/asm/kvm_host.h
index efaac2c..c66831f 100644
--- a/arch/s390/include/asm/kvm_host.h
+++ b/arch/s390/include/asm/kvm_host.h
@@ -427,7 +427,7 @@ struct kvm_s390_irq_payload {
struct kvm_s390_local_interrupt {
spinlock_t lock;
struct kvm_s390_float_interrupt *float_int;
- wait_queue_head_t *wq;
+ struct swait_queue_head *wq;
atomic_t *cpuflags;
DECLARE_BITMAP(sigp_emerg_pending, KVM_MAX_VCPUS);
struct kvm_s390_irq_payload irq;
diff --git a/arch/s390/kvm/interrupt.c b/arch/s390/kvm/interrupt.c
index 6a75352..cc862c4 100644
--- a/arch/s390/kvm/interrupt.c
+++ b/arch/s390/kvm/interrupt.c
@@ -868,13 +868,13 @@ no_timer:
void kvm_s390_vcpu_wakeup(struct kvm_vcpu *vcpu)
{
- if (waitqueue_active(&vcpu->wq)) {
+ if (swait_active(&vcpu->wq)) {
/*
* The vcpu gave up the cpu voluntarily, mark it as a good
* yield-candidate.
*/
vcpu->preempted = true;
- wake_up_interruptible(&vcpu->wq);
+ swake_up(&vcpu->wq);
vcpu->stat.halt_wakeup++;
}
}
diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
index 4d30b86..34dc14f 100644
--- a/arch/x86/kvm/lapic.c
+++ b/arch/x86/kvm/lapic.c
@@ -1195,7 +1195,7 @@ static void apic_update_lvtt(struct kvm_lapic *apic)
static void apic_timer_expired(struct kvm_lapic *apic)
{
struct kvm_vcpu *vcpu = apic->vcpu;
- wait_queue_head_t *q = &vcpu->wq;
+ struct swait_queue_head *q = &vcpu->wq;
struct kvm_timer *ktimer = &apic->lapic_timer;
if (atomic_read(&apic->lapic_timer.pending))
@@ -1204,8 +1204,8 @@ static void apic_timer_expired(struct kvm_lapic *apic)
atomic_inc(&apic->lapic_timer.pending);
kvm_set_pending_timer(vcpu);
- if (waitqueue_active(q))
- wake_up_interruptible(q);
+ if (swait_active(q))
+ swake_up(q);
if (apic_lvtt_tscdeadline(apic))
ktimer->expired_tscdeadline = ktimer->tscdeadline;
diff --git a/include/linux/kvm_host.h b/include/linux/kvm_host.h
index c923350..c690acc 100644
--- a/include/linux/kvm_host.h
+++ b/include/linux/kvm_host.h
@@ -25,6 +25,7 @@
#include <linux/irqflags.h>
#include <linux/context_tracking.h>
#include <linux/irqbypass.h>
+#include <linux/swait.h>
#include <asm/signal.h>
#include <linux/kvm.h>
@@ -243,7 +244,7 @@ struct kvm_vcpu {
int fpu_active;
int guest_fpu_loaded, guest_xcr0_loaded;
unsigned char fpu_counter;
- wait_queue_head_t wq;
+ struct swait_queue_head wq;
struct pid *pid;
int sigset_active;
sigset_t sigset;
@@ -794,7 +795,7 @@ static inline bool kvm_arch_has_assigned_device(struct kvm *kvm)
}
#endif
-static inline wait_queue_head_t *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
+static inline struct swait_queue_head *kvm_arch_vcpu_wq(struct kvm_vcpu *vcpu)
{
#ifdef __KVM_HAVE_ARCH_WQP
return vcpu->arch.wqp;
diff --git a/virt/kvm/async_pf.c b/virt/kvm/async_pf.c
index 77d42be..cd8477c 100644
--- a/virt/kvm/async_pf.c
+++ b/virt/kvm/async_pf.c
@@ -98,8 +98,8 @@ static void async_pf_execute(struct work_struct *work)
* This memory barrier pairs with prepare_to_wait's set_current_state()
*/
smp_mb();
- if (waitqueue_active(&vcpu->wq))
- wake_up_interruptible(&vcpu->wq);
+ if (swait_active(&vcpu->wq))
+ swake_up(&vcpu->wq);
mmput(mm);
kvm_put_kvm(vcpu->kvm);
diff --git a/virt/kvm/kvm_main.c b/virt/kvm/kvm_main.c
index 484079e..43fad2e 100644
--- a/virt/kvm/kvm_main.c
+++ b/virt/kvm/kvm_main.c
@@ -226,8 +226,7 @@ int kvm_vcpu_init(struct kvm_vcpu *vcpu, struct kvm *kvm, unsigned id)
vcpu->kvm = kvm;
vcpu->vcpu_id = id;
vcpu->pid = NULL;
- vcpu->halt_poll_ns = 0;
- init_waitqueue_head(&vcpu->wq);
+ init_swait_queue_head(&vcpu->wq);
kvm_async_pf_vcpu_init(vcpu);
vcpu->pre_pcpu = -1;
@@ -1999,7 +1998,7 @@ static int kvm_vcpu_check_block(struct kvm_vcpu *vcpu)
void kvm_vcpu_block(struct kvm_vcpu *vcpu)
{
ktime_t start, cur;
- DEFINE_WAIT(wait);
+ DECLARE_SWAITQUEUE(wait);
bool waited = false;
u64 block_ns;
@@ -2024,7 +2023,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
kvm_arch_vcpu_blocking(vcpu);
for (;;) {
- prepare_to_wait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
+ prepare_to_swait(&vcpu->wq, &wait, TASK_INTERRUPTIBLE);
if (kvm_vcpu_check_block(vcpu) < 0)
break;
@@ -2033,7 +2032,7 @@ void kvm_vcpu_block(struct kvm_vcpu *vcpu)
schedule();
}
- finish_wait(&vcpu->wq, &wait);
+ finish_swait(&vcpu->wq, &wait);
cur = ktime_get();
kvm_arch_vcpu_unblocking(vcpu);
@@ -2065,11 +2064,11 @@ void kvm_vcpu_kick(struct kvm_vcpu *vcpu)
{
int me;
int cpu = vcpu->cpu;
- wait_queue_head_t *wqp;
+ struct swait_queue_head *wqp;
wqp = kvm_arch_vcpu_wq(vcpu);
- if (waitqueue_active(wqp)) {
- wake_up_interruptible(wqp);
+ if (swait_active(wqp)) {
+ swake_up(wqp);
++vcpu->stat.halt_wakeup;
}
@@ -2170,7 +2169,7 @@ void kvm_vcpu_on_spin(struct kvm_vcpu *me)
continue;
if (vcpu = me)
continue;
- if (waitqueue_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
+ if (swait_active(&vcpu->wq) && !kvm_arch_vcpu_runnable(vcpu))
continue;
if (!kvm_vcpu_eligible_for_directed_yield(vcpu))
continue;
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 4/7] kbuild: Add option to turn incompatible pointer check into error
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
With the introduction of the simple wait API we have two very
similar APIs in the kernel. For example wake_up() and swake_up()
is only one character away. Although the compiler will warn
happily the wrong usage it keeps on going an even links the kernel.
Thomas and Peter would rather like to see early missuses reported
as error early on.
In a first attempt we tried to wrap all swait and wait calls
into a macro which has an compile time type assertion. The result
was pretty ugly and wasn't able to catch all wrong usages.
woken_wake_function(), autoremove_wake_function() and wake_bit_function()
are assigned as function pointers. Wrapping them with a macro around is
not possible. Prefixing them with '_' was also not a real option
because there some users in the kernel which do use them as well.
All in all this attempt looked to intrusive and too ugly.
An alternative is to turn the pointer type check into an error which
catches wrong type uses. Obviously not only the swait/wait ones. That
isn't a bad thing either.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
---
Makefile | 3 +++
1 file changed, 3 insertions(+)
diff --git a/Makefile b/Makefile
index 9d94ade..adfb6a08 100644
--- a/Makefile
+++ b/Makefile
@@ -767,6 +767,9 @@ KBUILD_CFLAGS += $(call cc-option,-Werror=strict-prototypes)
# Prohibit date/time macros, which would make the build non-deterministic
KBUILD_CFLAGS += $(call cc-option,-WerrorÚte-time)
+# enforce correct pointer usage
+KBUILD_CFLAGS += $(call cc-option,-Werror=incompatible-pointer-types)
+
# use the deterministic mode of AR if available
KBUILD_ARFLAGS := $(call ar-option,D)
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 3/7] wait.[ch]: Introduce the simple waitqueue (swait) implementation
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
From: "Peter Zijlstra (Intel)" <peterz@infradead.org>
The existing wait queue support has support for custom wake up call
backs, wake flags, wake key (passed to call back) and exclusive
flags that allow wakers to be tagged as exclusive, for limiting
the number of wakers.
In a lot of cases, none of these features are used, and hence we
can benefit from a slimmed down version that lowers memory overhead
and reduces runtime overhead.
The concept originated from -rt, where waitqueues are a constant
source of trouble, as we can't convert the head lock to a raw
spinlock due to fancy and long lasting callbacks.
With the removal of custom callbacks, we can use a raw lock for
queue list manipulations, hence allowing the simple wait support
to be used in -rt.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Acked-by: Peter Zijlstra (Intel) <peterz@infradead.org>
Originally-by: Thomas Gleixner <tglx@linutronix.de>
Cc: Paul Gortmaker <paul.gortmaker@windriver.com>
[Patch is from PeterZ which is based on Thomas version.
Commit message is written by Paul G.
Daniel:
- And some compile issues fixed.
- Added non-lazy implementation of swake_up_locked as suggested by Boqun Feng.]
---
include/linux/swait.h | 172 ++++++++++++++++++++++++++++++++++++++++++++++++++
kernel/sched/Makefile | 2 +-
kernel/sched/swait.c | 123 ++++++++++++++++++++++++++++++++++++
3 files changed, 296 insertions(+), 1 deletion(-)
create mode 100644 include/linux/swait.h
create mode 100644 kernel/sched/swait.c
diff --git a/include/linux/swait.h b/include/linux/swait.h
new file mode 100644
index 0000000..c1f9c62
--- /dev/null
+++ b/include/linux/swait.h
@@ -0,0 +1,172 @@
+#ifndef _LINUX_SWAIT_H
+#define _LINUX_SWAIT_H
+
+#include <linux/list.h>
+#include <linux/stddef.h>
+#include <linux/spinlock.h>
+#include <asm/current.h>
+
+/*
+ * Simple wait queues
+ *
+ * While these are very similar to the other/complex wait queues (wait.h) the
+ * most important difference is that the simple waitqueue allows for
+ * deterministic behaviour -- IOW it has strictly bounded IRQ and lock hold
+ * times.
+ *
+ * In order to make this so, we had to drop a fair number of features of the
+ * other waitqueue code; notably:
+ *
+ * - mixing INTERRUPTIBLE and UNINTERRUPTIBLE sleeps on the same waitqueue;
+ * all wakeups are TASK_NORMAL in order to avoid O(n) lookups for the right
+ * sleeper state.
+ *
+ * - the exclusive mode; because this requires preserving the list order
+ * and this is hard.
+ *
+ * - custom wake functions; because you cannot give any guarantees about
+ * random code.
+ *
+ * As a side effect of this; the data structures are slimmer.
+ *
+ * One would recommend using this wait queue where possible.
+ */
+
+struct task_struct;
+
+struct swait_queue_head {
+ raw_spinlock_t lock;
+ struct list_head task_list;
+};
+
+struct swait_queue {
+ struct task_struct *task;
+ struct list_head task_list;
+};
+
+#define __SWAITQUEUE_INITIALIZER(name) { \
+ .task = current, \
+ .task_list = LIST_HEAD_INIT((name).task_list), \
+}
+
+#define DECLARE_SWAITQUEUE(name) \
+ struct swait_queue name = __SWAITQUEUE_INITIALIZER(name)
+
+#define __SWAIT_QUEUE_HEAD_INITIALIZER(name) { \
+ .lock = __RAW_SPIN_LOCK_UNLOCKED(name.lock), \
+ .task_list = LIST_HEAD_INIT((name).task_list), \
+}
+
+#define DECLARE_SWAIT_QUEUE_HEAD(name) \
+ struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INITIALIZER(name)
+
+extern void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+ struct lock_class_key *key);
+
+#define init_swait_queue_head(q) \
+ do { \
+ static struct lock_class_key __key; \
+ __init_swait_queue_head((q), #q, &__key); \
+ } while (0)
+
+#ifdef CONFIG_LOCKDEP
+# define __SWAIT_QUEUE_HEAD_INIT_ONSTACK(name) \
+ ({ init_swait_queue_head(&name); name; })
+# define DECLARE_SWAIT_QUEUE_HEAD_ONSTACK(name) \
+ struct swait_queue_head name = __SWAIT_QUEUE_HEAD_INIT_ONSTACK(name)
+#else
+# define DECLARE_SWAIT_QUEUE_HEAD_ONSTACK(name) \
+ DECLARE_SWAIT_QUEUE_HEAD(name)
+#endif
+
+static inline int swait_active(struct swait_queue_head *q)
+{
+ return !list_empty(&q->task_list);
+}
+
+extern void swake_up(struct swait_queue_head *q);
+extern void swake_up_all(struct swait_queue_head *q);
+extern void swake_up_locked(struct swait_queue_head *q);
+
+extern void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state);
+extern long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state);
+
+extern void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+extern void finish_swait(struct swait_queue_head *q, struct swait_queue *wait);
+
+/* as per ___wait_event() but for swait, therefore "exclusive = 0" */
+#define ___swait_event(wq, condition, state, ret, cmd) \
+({ \
+ struct swait_queue __wait; \
+ long __ret = ret; \
+ \
+ INIT_LIST_HEAD(&__wait.task_list); \
+ for (;;) { \
+ long __int = prepare_to_swait_event(&wq, &__wait, state);\
+ \
+ if (condition) \
+ break; \
+ \
+ if (___wait_is_interruptible(state) && __int) { \
+ __ret = __int; \
+ break; \
+ } \
+ \
+ cmd; \
+ } \
+ finish_swait(&wq, &__wait); \
+ __ret; \
+})
+
+#define __swait_event(wq, condition) \
+ (void)___swait_event(wq, condition, TASK_UNINTERRUPTIBLE, 0, \
+ schedule())
+
+#define swait_event(wq, condition) \
+do { \
+ if (condition) \
+ break; \
+ __swait_event(wq, condition); \
+} while (0)
+
+#define __swait_event_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_UNINTERRUPTIBLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_timeout(wq, condition, timeout); \
+ __ret; \
+})
+
+#define __swait_event_interruptible(wq, condition) \
+ ___swait_event(wq, condition, TASK_INTERRUPTIBLE, 0, \
+ schedule())
+
+#define swait_event_interruptible(wq, condition) \
+({ \
+ int __ret = 0; \
+ if (!(condition)) \
+ __ret = __swait_event_interruptible(wq, condition); \
+ __ret; \
+})
+
+#define __swait_event_interruptible_timeout(wq, condition, timeout) \
+ ___swait_event(wq, ___wait_cond_timeout(condition), \
+ TASK_INTERRUPTIBLE, timeout, \
+ __ret = schedule_timeout(__ret))
+
+#define swait_event_interruptible_timeout(wq, condition, timeout) \
+({ \
+ long __ret = timeout; \
+ if (!___wait_cond_timeout(condition)) \
+ __ret = __swait_event_interruptible_timeout(wq, \
+ condition, timeout); \
+ __ret; \
+})
+
+#endif /* _LINUX_SWAIT_H */
diff --git a/kernel/sched/Makefile b/kernel/sched/Makefile
index 6768797..7d4cba2 100644
--- a/kernel/sched/Makefile
+++ b/kernel/sched/Makefile
@@ -13,7 +13,7 @@ endif
obj-y += core.o loadavg.o clock.o cputime.o
obj-y += idle_task.o fair.o rt.o deadline.o stop_task.o
-obj-y += wait.o completion.o idle.o
+obj-y += wait.o swait.o completion.o idle.o
obj-$(CONFIG_SMP) += cpupri.o cpudeadline.o
obj-$(CONFIG_SCHED_AUTOGROUP) += auto_group.o
obj-$(CONFIG_SCHEDSTATS) += stats.o
diff --git a/kernel/sched/swait.c b/kernel/sched/swait.c
new file mode 100644
index 0000000..82f0dff
--- /dev/null
+++ b/kernel/sched/swait.c
@@ -0,0 +1,123 @@
+#include <linux/sched.h>
+#include <linux/swait.h>
+
+void __init_swait_queue_head(struct swait_queue_head *q, const char *name,
+ struct lock_class_key *key)
+{
+ raw_spin_lock_init(&q->lock);
+ lockdep_set_class_and_name(&q->lock, key, name);
+ INIT_LIST_HEAD(&q->task_list);
+}
+EXPORT_SYMBOL(__init_swait_queue_head);
+
+/*
+ * The thing about the wake_up_state() return value; I think we can ignore it.
+ *
+ * If for some reason it would return 0, that means the previously waiting
+ * task is already running, so it will observe condition true (or has already).
+ */
+void swake_up_locked(struct swait_queue_head *q)
+{
+ struct swait_queue *curr;
+
+ if (list_empty(&q->task_list))
+ return;
+
+ curr = list_first_entry(&q->task_list, typeof(*curr), task_list);
+ wake_up_process(curr->task);
+ list_del_init(&curr->task_list);
+}
+EXPORT_SYMBOL(swake_up_locked);
+
+void swake_up(struct swait_queue_head *q)
+{
+ unsigned long flags;
+
+ if (!swait_active(q))
+ return;
+
+ raw_spin_lock_irqsave(&q->lock, flags);
+ swake_up_locked(q);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+}
+EXPORT_SYMBOL(swake_up);
+
+/*
+ * Does not allow usage from IRQ disabled, since we must be able to
+ * release IRQs to guarantee bounded hold time.
+ */
+void swake_up_all(struct swait_queue_head *q)
+{
+ struct swait_queue *curr;
+ LIST_HEAD(tmp);
+
+ if (!swait_active(q))
+ return;
+
+ raw_spin_lock_irq(&q->lock);
+ list_splice_init(&q->task_list, &tmp);
+ while (!list_empty(&tmp)) {
+ curr = list_first_entry(&tmp, typeof(*curr), task_list);
+
+ wake_up_state(curr->task, TASK_NORMAL);
+ list_del_init(&curr->task_list);
+
+ if (list_empty(&tmp))
+ break;
+
+ raw_spin_unlock_irq(&q->lock);
+ raw_spin_lock_irq(&q->lock);
+ }
+ raw_spin_unlock_irq(&q->lock);
+}
+EXPORT_SYMBOL(swake_up_all);
+
+void __prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ wait->task = current;
+ if (list_empty(&wait->task_list))
+ list_add(&wait->task_list, &q->task_list);
+}
+
+void prepare_to_swait(struct swait_queue_head *q, struct swait_queue *wait, int state)
+{
+ unsigned long flags;
+
+ raw_spin_lock_irqsave(&q->lock, flags);
+ __prepare_to_swait(q, wait);
+ set_current_state(state);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+}
+EXPORT_SYMBOL(prepare_to_swait);
+
+long prepare_to_swait_event(struct swait_queue_head *q, struct swait_queue *wait, int state)
+{
+ if (signal_pending_state(state, current))
+ return -ERESTARTSYS;
+
+ prepare_to_swait(q, wait, state);
+
+ return 0;
+}
+EXPORT_SYMBOL(prepare_to_swait_event);
+
+void __finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ __set_current_state(TASK_RUNNING);
+ if (!list_empty(&wait->task_list))
+ list_del_init(&wait->task_list);
+}
+
+void finish_swait(struct swait_queue_head *q, struct swait_queue *wait)
+{
+ unsigned long flags;
+
+ __set_current_state(TASK_RUNNING);
+
+ if (!list_empty_careful(&wait->task_list)) {
+ raw_spin_lock_irqsave(&q->lock, flags);
+ list_del_init(&wait->task_list);
+ raw_spin_unlock_irqrestore(&q->lock, flags);
+ }
+}
+EXPORT_SYMBOL(finish_swait);
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 2/7] MIPS: Differentiate between 32 and 64 bit ELF header
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
Depending on the configuration either the 32 or 64 bit version of
elf_check_arch() is defined. parse_crash_elf32_headers() does
some basic verification of the ELF header via elf_check_arch().
parse_crash_elf64_headers() does it via vmcore_elf64_check_arch()
which expands to the same elf_check_check().
In file included from include/linux/elf.h:4:0,
from fs/proc/vmcore.c:13:
fs/proc/vmcore.c: In function 'parse_crash_elf64_headers':
>> arch/mips/include/asm/elf.h:228:23: error: initialization from incompatible pointer type [-Werror=incompatible-pointer-types]
struct elfhdr *__h = (hdr); \
^
include/linux/crash_dump.h:41:37: note: in expansion of macro 'elf_check_arch'
#define vmcore_elf64_check_arch(x) (elf_check_arch(x) || vmcore_elf_check_arch_cross(x))
^
fs/proc/vmcore.c:1015:4: note: in expansion of macro 'vmcore_elf64_check_arch'
!vmcore_elf64_check_arch(&ehdr) ||
^
Since the MIPS ELF header for 32 bit and 64 bit differ we need
to check accordingly.
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
arch/mips/include/asm/elf.h | 68 ++++++++++++++++++++++++---------------------
1 file changed, 37 insertions(+), 31 deletions(-)
diff --git a/arch/mips/include/asm/elf.h b/arch/mips/include/asm/elf.h
index b01a6ff..e311d60 100644
--- a/arch/mips/include/asm/elf.h
+++ b/arch/mips/include/asm/elf.h
@@ -205,27 +205,10 @@ struct mips_elf_abiflags_v0 {
#define MIPS_ABI_FP_64 6 /* -mips32r2 -mfp64 */
#define MIPS_ABI_FP_64A 7 /* -mips32r2 -mfp64 -mno-odd-spreg */
-#ifdef CONFIG_32BIT
-
-/*
- * In order to be sure that we don't attempt to execute an O32 binary which
- * requires 64 bit FP (FR=1) on a system which does not support it we refuse
- * to execute any binary which has bits specified by the following macro set
- * in its ELF header flags.
- */
-#ifdef CONFIG_MIPS_O32_FP64_SUPPORT
-# define __MIPS_O32_FP64_MUST_BE_ZERO 0
-#else
-# define __MIPS_O32_FP64_MUST_BE_ZERO EF_MIPS_FP64
-#endif
-
-/*
- * This is used to ensure we don't load something for the wrong architecture.
- */
-#define elf_check_arch(hdr) \
+#define elf_check_arch_32(hdr) \
({ \
int __res = 1; \
- struct elfhdr *__h = (hdr); \
+ Elf32_Ehdr *__h = (hdr); \
\
if (__h->e_machine != EM_MIPS) \
__res = 0; \
@@ -242,6 +225,40 @@ struct mips_elf_abiflags_v0 {
__res; \
})
+#define elf_check_arch_64(hdr) \
+({ \
+ int __res = 1; \
+ Elf64_Ehdr *__h = (hdr); \
+ \
+ if (__h->e_machine != EM_MIPS) \
+ __res = 0; \
+ if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
+ __res = 0; \
+ \
+ __res; \
+})
+
+#define vmcore_elf64_check_arch(x) (elf_check_arch_64(x) || vmcore_elf_check_arch_cross(x))
+
+#ifdef CONFIG_32BIT
+
+/*
+ * In order to be sure that we don't attempt to execute an O32 binary which
+ * requires 64 bit FP (FR=1) on a system which does not support it we refuse
+ * to execute any binary which has bits specified by the following macro set
+ * in its ELF header flags.
+ */
+#ifdef CONFIG_MIPS_O32_FP64_SUPPORT
+# define __MIPS_O32_FP64_MUST_BE_ZERO 0
+#else
+# define __MIPS_O32_FP64_MUST_BE_ZERO EF_MIPS_FP64
+#endif
+
+/*
+ * This is used to ensure we don't load something for the wrong architecture.
+ */
+#define elf_check_arch(x) elf_check_arch_32(x)
+
/*
* These are used to set parameters in the core dumps.
*/
@@ -253,18 +270,7 @@ struct mips_elf_abiflags_v0 {
/*
* This is used to ensure we don't load something for the wrong architecture.
*/
-#define elf_check_arch(hdr) \
-({ \
- int __res = 1; \
- struct elfhdr *__h = (hdr); \
- \
- if (__h->e_machine != EM_MIPS) \
- __res = 0; \
- if (__h->e_ident[EI_CLASS] != ELFCLASS64) \
- __res = 0; \
- \
- __res; \
-})
+#define elf_check_arch(x) elf_check_arch_64(x)
/*
* These are used to set parameters in the core dumps.
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 1/7] video: Use bool instead int pointer for get_opt_bool() argument
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
In-Reply-To: <1454076208-28354-1-git-send-email-daniel.wagner@bmw-carit.de>
As the function name already indicates that get_opt_bool() parses
for a bool. It is not a surprise that compiler is complaining
about it when -Werror=incompatible-pointer-types is used:
drivers/video/fbdev/intelfb/intelfbdrv.c: In function ‘intelfb_setup’:
drivers/video/fbdev/intelfb/intelfbdrv.c:353:39: error: passing argument 3 of ‘get_opt_bool’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if (get_opt_bool(this_opt, "accel", &accel))
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index bbec737..bf20744 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -302,7 +302,7 @@ static __inline__ int get_opt_int(const char *this_opt, const char *name,
}
static __inline__ int get_opt_bool(const char *this_opt, const char *name,
- int *ret)
+ bool *ret)
{
if (!ret)
return 0;
--
2.5.0
^ permalink raw reply related
* [PATCH tip v7 0/7] Simple wait queue support
From: Daniel Wagner @ 2016-01-29 14:03 UTC (permalink / raw)
To: linux-kernel, linux-rt-users
Cc: linux-fbdev, linux-mips, Marcelo Tosatti, Paolo Bonzini,
Paul E. McKenney, Paul Gortmaker, Peter Zijlstra (Intel),
Thomas Gleixner, Steven Rostedt, Boqun Feng, Maik Broemme,
Ralf Baechle, Daniel Wagner
Hi,
As it turns out I missed a few incompatible-pointer-types errors.
Furthermore I even missed to update one calling side on wait in the ARM KVM
code base. Luckily the kbuild test robot found them all.
These patches are against
tip/sched/core 0905f04eb21fc1c2e690bed5d0418a061d56c225
also available as git tree:
git://git.kernel.org/pub/scm/linux/kernel/git/wagi/linux.git tip-swait
cheers,
daniel
changes since v6:
- fixed a couple of incompatible-pointer-types errors
- fixed a missing KVM ARM wait -> swait update
changes since v5:
- unconditionally add -Werror=incompatible-pointer-types
- updated KVM statistics in commit message
- rebased on tip/sched/core
- added ack-by PeterZ
changes since v4:
- replaced patch #2 which tried to force to compiler to
exit with an error by using compile time assertion type
check macros. Instead use -Werror=incompatible-pointer-types
to tell the compiler to barf loudly.
- fixed wrong API usage in patch 4 as reported by Boqun.
changes since v3
- rebased it on tip/sched/core (KVM bits have changed slightly)
- added compile time type check assertion
- added non lazy version of swake_up_locked()
changes since v2
- rebased again on tip/master. The patches apply
cleanly on v4.3-rc6 too.
- fixed up mips
- reordered patches to avoid lockdep warning when doing bissect.
- remove unnecessary initialization of rsp->rda in rcu_init_one().
changes since v1 (PATCH v0)
- rebased and fixed some typos found by cross building
for S390, ARM and powerpc. For some unknown reason didn't catch
them last time.
- dropped completion patches because it is not clear yet
how to handle complete_all() calls hard-irq/atomic contexts
and swake_up_all.
changes since v0 (RFC v0)
- promoted the series to PATCH state instead of RFC
- fixed a few fallouts with build all and some cross compilers
such ARM, PowerPC, S390.
- Added the simple waitqueue transformation for KVM from -rt
including some numbers requested by Paolo.
- Added a commit message to PeterZ's patch. Hope he likes it.
[I got the numbering wrong in v1, so instead 'PATCH v1' you find it
as 'PATCH v0' series]
v6: https://lkml.org/lkml/2016/1/28/462
v5: https://lkml.org/lkml/2015/11/30/318
v4: https://lwn.net/Articles/665655/
v3: https://lwn.net/Articles/661415/
v2: https://lwn.net/Articles/660628/
v1: https://lwn.net/Articles/656942/
v0: https://lwn.net/Articles/653586/
Daniel Wagner (4):
video: Use bool instead int pointer for get_opt_bool() argument
MIPS: Differentiate between 32 and 64 bit ELF header
kbuild: Add option to turn incompatible pointer check into error
rcu: Do not call rcu_nocb_gp_cleanup() while holding rnp->lock
Marcelo Tosatti (1):
KVM: use simple waitqueue for vcpu->wq
Paul Gortmaker (1):
rcu: use simple wait queues where possible in rcutree
Peter Zijlstra (Intel) (1):
wait.[ch]: Introduce the simple waitqueue (swait) implementation
Makefile | 3 +
arch/arm/kvm/arm.c | 8 +-
arch/arm/kvm/psci.c | 4 +-
arch/mips/include/asm/elf.h | 68 ++++++------
arch/mips/kvm/mips.c | 8 +-
arch/powerpc/include/asm/kvm_host.h | 4 +-
arch/powerpc/kvm/book3s_hv.c | 23 ++---
arch/s390/include/asm/kvm_host.h | 2 +-
arch/s390/kvm/interrupt.c | 4 +-
arch/x86/kvm/lapic.c | 6 +-
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
include/linux/kvm_host.h | 5 +-
include/linux/swait.h | 172 +++++++++++++++++++++++++++++++
kernel/rcu/tree.c | 24 +++--
kernel/rcu/tree.h | 12 ++-
kernel/rcu/tree_plugin.h | 32 ++++--
kernel/sched/Makefile | 2 +-
kernel/sched/swait.c | 123 ++++++++++++++++++++++
virt/kvm/async_pf.c | 4 +-
virt/kvm/kvm_main.c | 17 ++-
20 files changed, 420 insertions(+), 103 deletions(-)
create mode 100644 include/linux/swait.h
create mode 100644 kernel/sched/swait.c
--
2.5.0
^ permalink raw reply
* [PATCH] video: Use bool instead int pointer for get_opt_bool() argument
From: Daniel Wagner @ 2016-01-29 13:23 UTC (permalink / raw)
To: linux-kernel, linux-fbdev; +Cc: Maik Broemme, Daniel Wagner
In-Reply-To: <1453992270-4688-1-git-send-email-daniel.wagner@bmw-carit.de>
As the function name already indicates that get_opt_bool() parses
for a bool. It is not a surprise that compiler is complaining
about it when -Werror=incompatible-pointer-types is used:
drivers/video/fbdev/intelfb/intelfbdrv.c: In function ‘intelfb_setup’:
drivers/video/fbdev/intelfb/intelfbdrv.c:353:39: error: passing argument 3 of ‘get_opt_bool’ from incompatible pointer type [-Werror=incompatible-pointer-types]
if (get_opt_bool(this_opt, "accel", &accel))
Signed-off-by: Daniel Wagner <daniel.wagner@bmw-carit.de>
Reported-by: Fengguang Wu <fengguang.wu@intel.com>
---
Hi,
In the 'simple wait queue support' series is a patch
which turns on -Werror=incompatible-pointer-types which will
result in a compile error for intelfb.
https://lkml.org/lkml/2016/1/28/462
Even if that patch wont make it, this one makes sense (at least
for me :))
I'll prepend this patch to the next version of the series in order
to see if I got rid of all incompatible pointer types errors caught
by the kbuild test robot.
cheers,
daniel
drivers/video/fbdev/intelfb/intelfbdrv.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/video/fbdev/intelfb/intelfbdrv.c b/drivers/video/fbdev/intelfb/intelfbdrv.c
index bbec737..bf20744 100644
--- a/drivers/video/fbdev/intelfb/intelfbdrv.c
+++ b/drivers/video/fbdev/intelfb/intelfbdrv.c
@@ -302,7 +302,7 @@ static __inline__ int get_opt_int(const char *this_opt, const char *name,
}
static __inline__ int get_opt_bool(const char *this_opt, const char *name,
- int *ret)
+ bool *ret)
{
if (!ret)
return 0;
--
2.5.0
^ permalink raw reply related
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox