Re: [net-next 1/2] ipv6: sr: add a per namespace sysctl to control seg6 flowlabel
From: Ahmed Abdelsalam Date: Mon, 23 Apr 2018 23:36:59 +0200 > This patch adds a per namespace sysctl, named 'seg6_flowlabel', to be used > by seg6_do_srh_encap() to control the behaviour of setting the flowlabel > value of outer IPv6. > > The currently support behaviours are as follows: > -1 set flowlabel to zero. > 0 copy flowlabel from Inner paceket in case of Inner IPv6 (0 for IPv4/L2) > 1 Compute the flowlabel using seg6_make_flowlabel() > > Signed-off-by: Ahmed Abdelsalam There really isn't a reason to make this a separate patch. Adding a sysctl that nothing refers to doesn't add much value. So please combine patches #1 and #2.
Re: [RFC] mm: kmemleak: replace __GFP_NOFAIL to GFP_NOWAIT in gfp_kmemleak_mask
On Tue, Apr 24, 2018 at 7:02 PM, Michal Hocko wrote:
> On Tue 24-04-18 12:48:50, Chunyu Hu wrote:
>>
>>
>> - Original Message -
>> > From: "Michal Hocko"
>> > To: "Chunyu Hu"
>> > Cc: "Dmitry Vyukov" , "Catalin Marinas"
>> > , "Chunyu Hu"
>> > , "LKML" , "Linux-MM"
>> >
>> > Sent: Tuesday, April 24, 2018 9:20:57 PM
>> > Subject: Re: [RFC] mm: kmemleak: replace __GFP_NOFAIL to GFP_NOWAIT in
>> > gfp_kmemleak_mask
>> >
>> > On Mon 23-04-18 12:17:32, Chunyu Hu wrote:
>> > [...]
>> > > So if there is a new flag, it would be the 25th bits.
>> >
>> > No new flags please. Can you simply store a simple bool into
>> > fail_page_alloc
>> > and have save/restore api for that?
>>
>> Hi Michal,
>>
>> I still don't get your point. The original NOFAIL added in kmemleak was
>> for skipping fault injection in page/slab allocation for kmemleak object,
>> since kmemleak will disable itself until next reboot, whenever it hit an
>> allocation failure, in that case, it will lose effect to check kmemleak
>> in errer path rose by fault injection. But NOFAULT's effect is more than
>> skipping fault injection, it's also for hard allocation. So a dedicated flag
>> for skipping fault injection in specified slab/page allocation was mentioned.
>
> I am not familiar with the kmemleak all that much, but fiddling with the
> gfp_mask is a wrong way to achieve kmemleak specific action. I might be
I would say this is more like slab fault injection-specific action. It
can be used in other debugging facilities. Slab fault injection is a
part of slab. Slab behavior is generally controlled with gfp_mask.
> easilly wrong but I do not see any code that would restore the original
> gfp_mask down the kmem_cache_alloc path.
>
>> d9570ee3bd1d ("kmemleak: allow to coexist with fault injection")
>>
>> Do you mean something like below, with the save/store api? But looks like
>> to make it possible to skip a specified allocation, not global disabling,
>> a bool is not enough, and a gfp_flag is also needed. Maybe I missed
>> something?
>
> Yes, this is essentially what I meant. It is still a global thing which
> is not all that great and if it matters then you can make it per
> task_struct. That really depends on the code flow here.
If we go this route, it definitely needs to be per task and also needs
to work with interrupts: switch on interrupts and not corrupt on
interrupts. A gfp flag is free of these problems.
Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process.
Andrey Grodzovsky writes: > On 04/24/2018 12:23 PM, Eric W. Biederman wrote: >> Andrey Grodzovsky writes: >> >>> Avoid calling wait_event_killable when you are possibly being called >>> from get_signal routine since in that case you end up in a deadlock >>> where you are alreay blocked in singla processing any trying to wait >>> on a new signal. >> I am curious what the call path that is problematic here. > > Here is the problematic call stack > > [<0>] drm_sched_entity_fini+0x10a/0x3a0 [gpu_sched] > [<0>] amdgpu_ctx_do_release+0x129/0x170 [amdgpu] > [<0>] amdgpu_ctx_mgr_fini+0xd5/0xe0 [amdgpu] > [<0>] amdgpu_driver_postclose_kms+0xcd/0x440 [amdgpu] > [<0>] drm_release+0x414/0x5b0 [drm] > [<0>] __fput+0x176/0x350 > [<0>] task_work_run+0xa1/0xc0 > [<0>] do_exit+0x48f/0x1280 > [<0>] do_group_exit+0x89/0x140 > [<0>] get_signal+0x375/0x8f0 > [<0>] do_signal+0x79/0xaa0 > [<0>] exit_to_usermode_loop+0x83/0xd0 > [<0>] do_syscall_64+0x244/0x270 > [<0>] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 > [<0>] 0x > > On exit from system call you process all the signals you received and > encounter a fatal signal which triggers process termination. > >> >> In general waiting seems wrong when the process has already been >> fatally killed as indicated by PF_SIGNALED. > > So indeed this patch avoids wait in this case. > >> >> Returning -ERESTARTSYS seems wrong as nothing should make it back even >> to the edge of userspace here. > > Can you clarify please - what should be returned here instead ? __fput does not have a return code. I don't see the return code of release being used anywhere. So any return code is going to be lost. So maybe something that talks to the drm/kernel layer but don't expect your system call to be restarted, which is what -ERESTARTSYS asks for. Hmm. When looking at the code that is merged versus whatever your patch is against it gets even clearer. The -ERESTARTSYS return code doesn't even get out of drm_sched_entity_fini. Caring at all about process state at that point is wrong, as except for being in ``process'' context where you can sleep nothing is connected to a process. Let me respectfully suggest that the wait_event_killable on that code path is wrong. Possibly you want a wait_event_timeout if you are very nice. But the code already has the logic necessary to handle what happens if it can't sleep. So I think the justification needs to be why you are trying to sleep there at all. The progress guarantee needs to come from the gpu layer or the AMD driver not from someone getting impatient and sending SIGKILL to a dead process. Eric >> >> Given that this is the only use of PF_SIGNALED outside of bsd process >> accounting I find this code very suspicious. >> >> It looks the code path that gets called during exit is buggy and needs >> to be sorted out. >> >> Eric >> >> >>> Signed-off-by: Andrey Grodzovsky >>> --- >>> drivers/gpu/drm/scheduler/gpu_scheduler.c | 5 +++-- >>> 1 file changed, 3 insertions(+), 2 deletions(-) >>> >>> diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> b/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> index 088ff2b..09fd258 100644 >>> --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c >>> @@ -227,9 +227,10 @@ void drm_sched_entity_do_release(struct >>> drm_gpu_scheduler *sched, >>> return; >>> /** >>> * The client will not queue more IBs during this fini, consume existing >>> -* queued IBs or discard them on SIGKILL >>> +* queued IBs or discard them when in death signal state since >>> +* wait_event_killable can't receive signals in that state. >>> */ >>> - if ((current->flags & PF_SIGNALED) && current->exit_code == SIGKILL) >>> + if (current->flags & PF_SIGNALED) >>> entity->fini_status = -ERESTARTSYS; >>> else >>> entity->fini_status = wait_event_killable(sched->job_scheduled,
Re: [PATCH v3 2/3] regulator: add support for SY8106A regulator
On Mon, Apr 23, 2018 at 10:46:56PM +0800, Icenowy Zheng wrote:
> --- /dev/null
> +++ b/drivers/regulator/sy8106a-regulator.c
> @@ -0,0 +1,176 @@
> +// SPDX-License-Identifier: GPL-2.0+
> +/*
> + * sy8106a-regulator.c - Regulator device driver for SY8106A
Just make the entire thing a C++ comment so it looks consistent and
joined up.
> +static int sy8106a_set_voltage_sel(struct regulator_dev *rdev, unsigned int
> sel)
> +{
> + /* We use our set_voltage_sel in order to avoid unnecessary I2C
> + * chatter, because the regulator_get_voltage_sel_regmap using
> + * apply_bit would perform 4 unnecessary transfers instead of one,
> + * increasing the chance of error.
> + */
> + return regmap_write(rdev->regmap, rdev->desc->vsel_reg,
> + sel | SY8106A_GO_BIT);
Why would it do these extra transfers? Is this just the fact that you
didn't set up a register cache (though the r/m/w cycle should only add
the read)? We could put some logic in the core regmap code to detect
that an _update_bits() call is going to write to the whole register,
though it'd be even easier to just let this register be cached.
Generally if we can usefully optimize things we should do it at the
framework level.
> + if (reg & SY8106A_GO_BIT)
> + return reg & rdev->desc->vsel_mask;
> + else
> + return (chip->fixed_voltage - rdev->desc->min_uV) /
> +rdev->desc->uV_step;
You could use the standard get_voltage_sel() if you provide a mapping
operation that set everything with _GO_BIT set to return the fixed
voltage. Though looking at this it seems that the fixed voltage will
always be one that could be set via the register anyway so I'm wondering
if the easiest thing here isn't to just have the driver turn off _GO_BIT
during probe() and not worry about the special case at runtime.
signature.asc
Description: PGP signature
Re: [PATCH v4 7/8] drm/i2c: tda998x: register as a drm bridge
On Tue, Apr 24, 2018 at 07:04:16PM +0300, Jyri Sarha wrote:
> On 24/04/18 13:14, Peter Rosin wrote:
> > On 2018-04-24 10:08, Russell King - ARM Linux wrote:
> >> On Tue, Apr 24, 2018 at 08:58:42AM +0200, Peter Rosin wrote:
> >>> On 2018-04-23 18:08, Russell King - ARM Linux wrote:
> On Mon, Apr 23, 2018 at 09:23:00AM +0200, Peter Rosin wrote:
> > static int tda998x_remove(struct i2c_client *client)
> > {
> > - component_del(&client->dev, &tda998x_ops);
> > + struct device *dev = &client->dev;
> > + struct tda998x_bridge *bridge = dev_get_drvdata(dev);
> > +
> > + drm_bridge_remove(&bridge->bridge);
> > + component_del(dev, &tda998x_ops);
> > +
>
> I'd like to ask a rather fundamental question about DRM bridge support,
> because I suspect that there's a major fsckup here.
>
> The above is the function that deals with the TDA998x device being
> unbound from the driver. With the component API, this results in the
> DRM device correctly being torn down, because one of the hardware
> devices has gone.
>
> With DRM bridge, the bridge is merely removed from the list of
> bridges:
>
> void drm_bridge_remove(struct drm_bridge *bridge)
> {
> mutex_lock(&bridge_lock);
> list_del_init(&bridge->list);
> mutex_unlock(&bridge_lock);
> }
> EXPORT_SYMBOL(drm_bridge_remove);
>
> and the memory backing the "struct tda998x_bridge" (which contains
> the struct drm_bridge) will be freed by the devm subsystem.
>
> However, there is no notification into the rest of the DRM subsystem
> that the device has gone away. Worse, the memory that is still in
> use by DRM has now been freed, so further use of the DRM device
> results in a use-after-free bug.
>
> This is really not good, and to me looks like a fundamental problem
> with the DRM bridge code. I see nothing in the DRM bridge code that
> deals with the lifetime of a "DRM bridge" or indeed the lifetime of
> the actual device itself.
>
> So, from what I can see, there seems to be a fundamental lifetime
> issue with the design of the DRM bridge code. This needs to be
> fixed.
> >>>
> >>> Oh crap. A gigantic can of worms...
> >>
> >> Yes, it's especially annoying for me, having put the effort in to
> >> the component helper to cover all these cases.
> >>
> >>> Would a patch (completely untested btw) along this line of thinking make
> >>> any difference whatsoever?
> >>
> >> It looks interesting - from what I can see of the device links code,
> >> it would have the effect of unbinding the DRM device just before
> >> TDA998x is unbound, so that's an improvement.
> >>
> >> However, from what I can see, the link vanishes at that point (as
> >> DL_FLAG_AUTOREMOVE is set), and re-binding the TDA998x device results
> >> in nothing further happening - the link will be recreated, but there
> >> appears to be nothing that triggers the "consumer" to rebind at that
> >> point. Maybe I've missed something?
> >
> > Right, auto-remove is a no-go. So, improving on the previous...
> >
> > (I think drm_panel might suffer from this issue too?)
>
> Yes it does and I took a shot at trying to fix it at the end of the
> previous merge window, but gave up as I run out of time. I re-spun the
> work now after reading this thread. I add you and Russell to cc.
Right, and these exact problems are what the component helper is
there to sort out, in a subsystem independent way.
What is the problem with the component helper that people seem to
be soo loathed to use it, instead preferring to come up with sub-
standard and broken alternatives?
--
RMK's Patch system: http://www.armlinux.org.uk/developer/patches/
FTTC broadband for 0.8mile line in suburbia: sync at 8.8Mbps down 630kbps up
According to speedtest.net: 8.21Mbps down 510kbps up
Re: vmalloc with GFP_NOFS
On Tue, 24 Apr 2018, Michal Hocko wrote:
> On Tue 24-04-18 12:46:55, Mikulas Patocka wrote:
> >
> >
> > On Tue, 24 Apr 2018, Michal Hocko wrote:
> >
> > > Hi,
> > > it seems that we still have few vmalloc users who perform GFP_NOFS
> > > allocation:
> > > drivers/mtd/ubi/io.c
> > > fs/ext4/xattr.c
> > > fs/gfs2/dir.c
> > > fs/gfs2/quota.c
> > > fs/nfs/blocklayout/extent_tree.c
> > > fs/ubifs/debug.c
> > > fs/ubifs/lprops.c
> > > fs/ubifs/lpt_commit.c
> > > fs/ubifs/orphan.c
> > >
> > > Unfortunatelly vmalloc doesn't suppoer GFP_NOFS semantinc properly
> > > because we do have hardocded GFP_KERNEL allocations deep inside the
> > > vmalloc layers. That means that if GFP_NOFS really protects from
> > > recursion into the fs deadlocks then the vmalloc call is broken.
> > >
> > > What to do about this? Well, there are two things. Firstly, it would be
> > > really great to double check whether the GFP_NOFS is really needed. I
> > > cannot judge that because I am not familiar with the code. It would be
> > > great if the respective maintainers (hopefully get_maintainer.sh pointed
> > > me to all relevant ones). If there is not reclaim recursion issue then
> > > simply use the standard vmalloc (aka GFP_KERNEL request).
> > >
> > > If the use is really valid then we have a way to do the vmalloc
> > > allocation properly. We have memalloc_nofs_{save,restore} scope api. How
> > > does that work? You simply call memalloc_nofs_save when the reclaim
> > > recursion critical section starts (e.g. when you take a lock which is
> > > then used in the reclaim path - e.g. shrinker) and memalloc_nofs_restore
> > > when the critical section ends. _All_ allocations within that scope
> > > will get GFP_NOFS semantic automagically. If you are not sure about the
> > > scope itself then the easiest workaround is to wrap the vmalloc itself
> > > with a big fat comment that this should be revisited.
> > >
> > > Does that sound like something that can be done in a reasonable time?
> > > I have tried to bring this up in the past but our speed is glacial and
> > > there are attempts to do hacks like checking for abusers inside the
> > > vmalloc which is just too ugly to live.
> > >
> > > Please do not hesitate to get back to me if something is not clear.
> > >
> > > Thanks!
> > > --
> > > Michal Hocko
> > > SUSE Labs
> >
> > I made a patch that adds memalloc_noio/fs_save around these calls a year
> > ago: http://lkml.iu.edu/hypermail/linux/kernel/1707.0/01376.html
>
> Yeah, and that is the wrong approach.
It is crude, but it fixes the deadlock possibility. Then, the maintainers
will have a lot of time to refactor the code and move these
memalloc_noio_save calls to the proper scope.
> Let's try to fix this properly
> this time. As the above outlines, the worst case we can end up mid-term
> would be to wrap vmalloc calls with the scope api with a TODO. But I am
> pretty sure the respective maintainers can come up with a better
> solution. I am definitely willing to help here.
> --
> Michal Hocko
> SUSE Labs
Mikulas
Re: [PATCH 7/9] Pmalloc Rare Write: modify selected pools
On 24/04/18 16:33, Igor Stoppa wrote:
On 24/04/18 15:50, Matthew Wilcox wrote:
On Mon, Apr 23, 2018 at 04:54:56PM +0400, Igor Stoppa wrote:
While the vanilla version of pmalloc provides support for permanently
transitioning between writable and read-only of a memory pool, this
patch seeks to support a separate class of data, which would still
benefit from write protection, most of the time, but it still needs to
be modifiable. Maybe very seldom, but still cannot be permanently marked
as read-only.
This seems like a horrible idea that basically makes this feature
useless.
I would say the right way to do this is to have:
struct modifiable_data {
struct immutable_data *d;
...
};
Then allocate a new pool, change d and destroy the old pool.
I'm not sure I understand.
A few cups of coffee later ...
This seems like a regression from my case.
My case (see the example with the initialized state) is:
static void *pointer_to_pmalloc_memory __ro_after_init;
then, during init:
pointer_to_pmalloc_memory = pmalloc(pool, size);
then init happens
*pointer_to_pmalloc_memory = some_value;
pmalloc_protect_pool(pool9;
and to change the value:
support_variable = some_other_value;
pmalloc_rare_write(pool, pointer_to_pmalloc_memory,
&support_variable, size)
But in this case the pmalloc allocation would be assigned to a writable
variable.
This seems like a regression to me: at this point who cares anymore
about the pmalloc memory?
Just rewrite the pointer to point to somewhere else that is writable and
has the desired (from the attacker) value.
It doesn't even require gadgets. pmalloc becomes useless.
Do I still need more coffee?
--
igor
Re: [PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG
On Tue 24-04-18 13:00:11, Mikulas Patocka wrote: > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > On Tue 24-04-18 11:50:30, Mikulas Patocka wrote: > > > > > > > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > > > > > On Mon 23-04-18 20:06:16, Mikulas Patocka wrote: > > > > [...] > > > > > @@ -404,6 +405,12 @@ void *kvmalloc_node(size_t size, gfp_t f > > > > >*/ > > > > > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); > > > > > > > > > > +#ifdef CONFIG_DEBUG_SG > > > > > + /* Catch bugs when the caller uses DMA API on the result of > > > > > kvmalloc. */ > > > > > + if (!(prandom_u32_max(2) & 1)) > > > > > + goto do_vmalloc; > > > > > +#endif > > > > > > > > I really do not think there is anything DEBUG_SG specific here. Why you > > > > simply do not follow should_failslab path or even reuse the function? > > > > > > CONFIG_DEBUG_SG is enabled by default in RHEL and Fedora debug kernel (if > > > you don't like CONFIG_DEBUG_SG, pick any other option that is enabled > > > there). > > > > Are you telling me that you are shaping a debugging functionality basing > > on what RHEL has enabled? And you call me evil. This is just rediculous. > > > > > Fail-injection framework is if off by default and it must be explicitly > > > enabled and configured by the user - and most users won't enable it. > > > > It can be enabled easily. And if you care enough for your debugging > > kernel then just make it enabled unconditionally. > > So, should we add a new option CONFIG_KVMALLOC_FALLBACK_DEFAULT? I'm not > quite sure if 3 lines of debugging code need an extra option, but if you > don't want to reuse any existing debug option, it may be possible. Adding > it to the RHEL debug kernel would be trivial. Wouldn't it be equally trivial to simply enable the fault injection? You would get additional failure paths testing as a bonus. -- Michal Hocko SUSE Labs
Re: [RFC] mm: kmemleak: replace __GFP_NOFAIL to GFP_NOWAIT in gfp_kmemleak_mask
On Tue 24-04-18 12:48:50, Chunyu Hu wrote:
>
>
> - Original Message -
> > From: "Michal Hocko"
> > To: "Chunyu Hu"
> > Cc: "Dmitry Vyukov" , "Catalin Marinas"
> > , "Chunyu Hu"
> > , "LKML" , "Linux-MM"
> >
> > Sent: Tuesday, April 24, 2018 9:20:57 PM
> > Subject: Re: [RFC] mm: kmemleak: replace __GFP_NOFAIL to GFP_NOWAIT in
> > gfp_kmemleak_mask
> >
> > On Mon 23-04-18 12:17:32, Chunyu Hu wrote:
> > [...]
> > > So if there is a new flag, it would be the 25th bits.
> >
> > No new flags please. Can you simply store a simple bool into fail_page_alloc
> > and have save/restore api for that?
>
> Hi Michal,
>
> I still don't get your point. The original NOFAIL added in kmemleak was
> for skipping fault injection in page/slab allocation for kmemleak object,
> since kmemleak will disable itself until next reboot, whenever it hit an
> allocation failure, in that case, it will lose effect to check kmemleak
> in errer path rose by fault injection. But NOFAULT's effect is more than
> skipping fault injection, it's also for hard allocation. So a dedicated flag
> for skipping fault injection in specified slab/page allocation was mentioned.
I am not familiar with the kmemleak all that much, but fiddling with the
gfp_mask is a wrong way to achieve kmemleak specific action. I might be
easilly wrong but I do not see any code that would restore the original
gfp_mask down the kmem_cache_alloc path.
> d9570ee3bd1d ("kmemleak: allow to coexist with fault injection")
>
> Do you mean something like below, with the save/store api? But looks like
> to make it possible to skip a specified allocation, not global disabling,
> a bool is not enough, and a gfp_flag is also needed. Maybe I missed something?
Yes, this is essentially what I meant. It is still a global thing which
is not all that great and if it matters then you can make it per
task_struct. That really depends on the code flow here.
--
Michal Hocko
SUSE Labs
Re: [PATCH] platform/x86: acer-wmi: add another KEY_POWER keycode
On Tue, 2018-04-24 at 19:15 +0300, Andy Shevchenko wrote: > On Mon, Apr 23, 2018 at 5:48 PM, Gianfranco Costamagna > wrote: > > Hello, > > > > > Can you give your corresponding tag as well? > > > > > > we are implementing the fix right now, please follow bug > > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1766054 > > OK, I postpone this patch until new version. Or I missed the point and it's working? Tell me what to do with it. For now I pushed to my review and testing queue, thanks! -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG
On Tue, 24 Apr 2018, Michal Hocko wrote: > On Tue 24-04-18 11:50:30, Mikulas Patocka wrote: > > > > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > > > On Mon 23-04-18 20:06:16, Mikulas Patocka wrote: > > > [...] > > > > @@ -404,6 +405,12 @@ void *kvmalloc_node(size_t size, gfp_t f > > > > */ > > > > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); > > > > > > > > +#ifdef CONFIG_DEBUG_SG > > > > + /* Catch bugs when the caller uses DMA API on the result of > > > > kvmalloc. */ > > > > + if (!(prandom_u32_max(2) & 1)) > > > > + goto do_vmalloc; > > > > +#endif > > > > > > I really do not think there is anything DEBUG_SG specific here. Why you > > > simply do not follow should_failslab path or even reuse the function? > > > > CONFIG_DEBUG_SG is enabled by default in RHEL and Fedora debug kernel (if > > you don't like CONFIG_DEBUG_SG, pick any other option that is enabled > > there). > > Are you telling me that you are shaping a debugging functionality basing > on what RHEL has enabled? And you call me evil. This is just rediculous. > > > Fail-injection framework is if off by default and it must be explicitly > > enabled and configured by the user - and most users won't enable it. > > It can be enabled easily. And if you care enough for your debugging > kernel then just make it enabled unconditionally. So, should we add a new option CONFIG_KVMALLOC_FALLBACK_DEFAULT? I'm not quite sure if 3 lines of debugging code need an extra option, but if you don't want to reuse any existing debug option, it may be possible. Adding it to the RHEL debug kernel would be trivial. Mikulas
Re: vmalloc with GFP_NOFS
On Tue 24-04-18 12:46:55, Mikulas Patocka wrote:
>
>
> On Tue, 24 Apr 2018, Michal Hocko wrote:
>
> > Hi,
> > it seems that we still have few vmalloc users who perform GFP_NOFS
> > allocation:
> > drivers/mtd/ubi/io.c
> > fs/ext4/xattr.c
> > fs/gfs2/dir.c
> > fs/gfs2/quota.c
> > fs/nfs/blocklayout/extent_tree.c
> > fs/ubifs/debug.c
> > fs/ubifs/lprops.c
> > fs/ubifs/lpt_commit.c
> > fs/ubifs/orphan.c
> >
> > Unfortunatelly vmalloc doesn't suppoer GFP_NOFS semantinc properly
> > because we do have hardocded GFP_KERNEL allocations deep inside the
> > vmalloc layers. That means that if GFP_NOFS really protects from
> > recursion into the fs deadlocks then the vmalloc call is broken.
> >
> > What to do about this? Well, there are two things. Firstly, it would be
> > really great to double check whether the GFP_NOFS is really needed. I
> > cannot judge that because I am not familiar with the code. It would be
> > great if the respective maintainers (hopefully get_maintainer.sh pointed
> > me to all relevant ones). If there is not reclaim recursion issue then
> > simply use the standard vmalloc (aka GFP_KERNEL request).
> >
> > If the use is really valid then we have a way to do the vmalloc
> > allocation properly. We have memalloc_nofs_{save,restore} scope api. How
> > does that work? You simply call memalloc_nofs_save when the reclaim
> > recursion critical section starts (e.g. when you take a lock which is
> > then used in the reclaim path - e.g. shrinker) and memalloc_nofs_restore
> > when the critical section ends. _All_ allocations within that scope
> > will get GFP_NOFS semantic automagically. If you are not sure about the
> > scope itself then the easiest workaround is to wrap the vmalloc itself
> > with a big fat comment that this should be revisited.
> >
> > Does that sound like something that can be done in a reasonable time?
> > I have tried to bring this up in the past but our speed is glacial and
> > there are attempts to do hacks like checking for abusers inside the
> > vmalloc which is just too ugly to live.
> >
> > Please do not hesitate to get back to me if something is not clear.
> >
> > Thanks!
> > --
> > Michal Hocko
> > SUSE Labs
>
> I made a patch that adds memalloc_noio/fs_save around these calls a year
> ago: http://lkml.iu.edu/hypermail/linux/kernel/1707.0/01376.html
Yeah, and that is the wrong approach. Let's try to fix this properly
this time. As the above outlines, the worst case we can end up mid-term
would be to wrap vmalloc calls with the scope api with a TODO. But I am
pretty sure the respective maintainers can come up with a better
solution. I am definitely willing to help here.
--
Michal Hocko
SUSE Labs
[PATCH 2/7] dt-bindings: add generic gnss binding
Describe generic properties for GNSS receivers.
Signed-off-by: Johan Hovold
---
.../devicetree/bindings/gnss/gnss.txt | 36 +++
MAINTAINERS | 1 +
2 files changed, 37 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gnss/gnss.txt
diff --git a/Documentation/devicetree/bindings/gnss/gnss.txt
b/Documentation/devicetree/bindings/gnss/gnss.txt
new file mode 100644
index ..bcdaca043eaa
--- /dev/null
+++ b/Documentation/devicetree/bindings/gnss/gnss.txt
@@ -0,0 +1,36 @@
+GNSS Receiver DT binding
+
+This documents the binding structure and common properties for GNSS receiver
+devices.
+
+A GNSS receiver node is a node named "gnss" and typically resides on a serial
+bus (e.g. UART, I2C or SPI).
+
+Please refer to the following documents for generic properties:
+
+ Documentation/devicetree/bindings/serial/slave-device.txt
+ Documentation/devicetree/bindings/spi/spi-bus.txt
+
+Required Properties:
+
+- compatible : A string reflecting the vendor and specific device the node
+ represents
+
+Optional Properties:
+- enable-gpios : GPIO used to power on (or off) the device
+- timepulse-gpios : Timepulse (e.g. 1PPS) GPIO
+
+Example:
+
+serial@1234 {
+ compatible = "ns16550a";
+
+ gnss {
+ compatible = "u-blox,neo-8";
+
+ vcc-supply = <&gnss_reg>;
+ timepulse-gpios = <&gpio0 16 GPIO_ACTIVE_HIGH>;
+
+ current-speed = <4800>;
+ };
+};
diff --git a/MAINTAINERS b/MAINTAINERS
index dc3df211c1a7..fa219e80a1f8 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5965,6 +5965,7 @@ F:include/uapi/linux/gigaset_dev.h
GNSS SUBSYSTEM
M: Johan Hovold
S: Maintained
+F: Documentation/devicetree/bindings/gnss/
F: drivers/gnss/
F: include/linux/gnss.h
--
2.17.0
Re: [PATCH] net: phy: TLK10X initial driver submission
On 04/19/2018 01:28 AM, Måns Andersson wrote:
> From: Mans Andersson
>
> Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys.
>
> In addition the TLK10X needs to be removed from DP83848 driver as the
> power back off support is added here for this device.
I would not think this is a compelling enough reason, you could very
well just adjust the dp83848.c driver just to account for these
properties that you are introducing. More comments below.
[snip]
> +#define TLK10X_INT_EN_MASK \
> + (TLK10X_MISR_ANC_INT_EN | \
> + TLK10X_MISR_DUP_INT_EN | \
> + TLK10X_MISR_SPD_INT_EN | \
> + TLK10X_MISR_LINK_INT_EN)
> +
> +struct tlk10x_private {
> + int pwrbo_level;
unsigned int
> +};
> +
> +static int tlk10x_read(struct phy_device *phydev, int reg)
> +{
> + if (reg & ~0x1f) {
> + /* Extended register */
> + phy_write(phydev, TLK10X_REGCR, 0x001F);
> + phy_write(phydev, TLK10X_ADDAR, reg);
> + phy_write(phydev, TLK10X_REGCR, 0x401F);
> + reg = TLK10X_ADDAR;
> + }
Humm, this looks a bit fragile, you would likely want to create separate
helper functions for these extended registers and make sure you handle
write failures as well. Also consider making use of the page helpers
from include/linux/phy.h.
> +
> + return phy_read(phydev, reg);
> +}
> +
> +static int tlk10x_write(struct phy_device *phydev, int reg, int val)
> +{
> + if (reg & ~0x1f) {
> + /* Extended register */
> + phy_write(phydev, TLK10X_REGCR, 0x001F);
> + phy_write(phydev, TLK10X_ADDAR, reg);
> + phy_write(phydev, TLK10X_REGCR, 0x401F);
> + reg = TLK10X_ADDAR;
> + }
Same here.
> +
> + return phy_write(phydev, reg, val);
> +}
> +
> +#ifdef CONFIG_OF_MDIO
> +static int tlk10x_of_init(struct phy_device *phydev)
> +{
> + struct tlk10x_private *tlk10x = phydev->priv;
> + struct device *dev = &phydev->mdio.dev;
> + struct device_node *of_node = dev->of_node;
> + int ret;
> +
> + if (!of_node)
> + return 0;
> +
> + ret = of_property_read_u32(of_node, "ti,power-back-off",
> +&tlk10x->pwrbo_level);
> + if (ret) {
> + dev_err(dev, "missing ti,power-back-off property");
> + tlk10x->pwrbo_level = 0;
This should not be necessary, that should be the default with a zero
initialized private data structure.
> + }
> +
> + return 0;
> +}
> +#else
> +static int tlk10x_of_init(struct phy_device *phydev)
> +{
> + return 0;
> +}
> +#endif /* CONFIG_OF_MDIO */
> +
> +static int tlk10x_config_init(struct phy_device *phydev)
> +{
> + int ret, reg;
> + struct tlk10x_private *tlk10x;
> +
> + ret = genphy_config_init(phydev);
> + if (ret < 0)
> + return ret;
> +
> + if (!phydev->priv) {
> + tlk10x = devm_kzalloc(&phydev->mdio.dev, sizeof(*tlk10x),
> + GFP_KERNEL);
> + if (!tlk10x)
> + return -ENOMEM;
> +
> + phydev->priv = tlk10x;
> + ret = tlk10x_of_init(phydev);
> + if (ret)
> + return ret;
> + } else {
> + tlk10x = (struct tlk10x_private *)phydev->priv;
> + }
You need to implement a probe() function that is responsible for
allocation private memory instead of doing this check.
> +
> + // Power back off
> + if (tlk10x->pwrbo_level < 0 || tlk10x->pwrbo_level > 3)
> + tlk10x->pwrbo_level = 0;
How can you have pwrb_level < 0 when you use of_read_property_u32()?
> + reg = tlk10x_read(phydev, TLK10X_PWRBOCR);
> + reg = ((reg & ~TLK10X_PWRBOCR_MASK)
> + | (tlk10x->pwrbo_level << 6));
One too many levels of parenthesis, the outer ones should not be necessary.
> + ret = tlk10x_write(phydev, TLK10X_PWRBOCR, reg);
> + if (ret < 0) {
> + dev_err(&phydev->mdio.dev,
> + "unable to set power back-off (err=%d)\n", ret);
> + return ret;
> + }
> + dev_info(&phydev->mdio.dev, "power back-off set to level %d\n",
> + tlk10x->pwrbo_level);
config_init() is called often, consider making this a debugging statement.
--
Florian
[PATCH 3/7] gnss: add generic serial driver
Add a generic serial GNSS driver (library) which provides a common
implementation for the gnss interface and power management (runtime and
system suspend). This allows GNSS drivers for specific chip to be
implemented by simply providing a set_power() callback to handle three
states: ACTIVE, STANDBY and OFF.
Signed-off-by: Johan Hovold
---
drivers/gnss/Kconfig | 7 +
drivers/gnss/Makefile | 3 +
drivers/gnss/serial.c | 288 ++
drivers/gnss/serial.h | 47 +++
4 files changed, 345 insertions(+)
create mode 100644 drivers/gnss/serial.c
create mode 100644 drivers/gnss/serial.h
diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
index 103fcc70992e..f8ee54f99a8d 100644
--- a/drivers/gnss/Kconfig
+++ b/drivers/gnss/Kconfig
@@ -9,3 +9,10 @@ menuconfig GNSS
To compile this driver as a module, choose M here: the module will
be called gnss.
+
+if GNSS
+
+config GNSS_SERIAL
+ tristate
+
+endif # GNSS
diff --git a/drivers/gnss/Makefile b/drivers/gnss/Makefile
index 1f7a7baab1d9..171aba71684d 100644
--- a/drivers/gnss/Makefile
+++ b/drivers/gnss/Makefile
@@ -5,3 +5,6 @@
obj-$(CONFIG_GNSS) += gnss.o
gnss-y := core.o
+
+obj-$(CONFIG_GNSS_SERIAL) += gnss-serial.o
+gnss-serial-y := serial.o
diff --git a/drivers/gnss/serial.c b/drivers/gnss/serial.c
new file mode 100644
index ..06a4b511f380
--- /dev/null
+++ b/drivers/gnss/serial.c
@@ -0,0 +1,288 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * Generic serial GNSS receiver driver
+ *
+ * Copyright (C) 2018 Johan Hovold
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "serial.h"
+
+static int gnss_serial_open(struct gnss_device *gdev)
+{
+ struct gnss_serial *gserial = gnss_get_drvdata(gdev);
+ struct serdev_device *serdev = gserial->serdev;
+ int ret;
+
+ ret = serdev_device_open(serdev);
+ if (ret)
+ return ret;
+
+ serdev_device_set_baudrate(serdev, gserial->speed);
+ serdev_device_set_flow_control(serdev, false);
+
+ ret = pm_runtime_get_sync(&serdev->dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(&serdev->dev);
+ goto err_close;
+ }
+
+ return 0;
+
+err_close:
+ serdev_device_close(serdev);
+
+ return ret;
+}
+
+static void gnss_serial_close(struct gnss_device *gdev)
+{
+ struct gnss_serial *gserial = gnss_get_drvdata(gdev);
+ struct serdev_device *serdev = gserial->serdev;
+
+ serdev_device_close(serdev);
+
+ pm_runtime_put(&serdev->dev);
+}
+
+static int gnss_serial_write_raw(struct gnss_device *gdev,
+ const unsigned char *buf, size_t count)
+{
+ struct gnss_serial *gserial = gnss_get_drvdata(gdev);
+ struct serdev_device *serdev = gserial->serdev;
+ int ret;
+
+ /* write is only buffered synchronously */
+ ret = serdev_device_write(serdev, buf, count, 0);
+ if (ret < 0)
+ return ret;
+
+ /* FIXME: determine if interrupted? */
+ serdev_device_wait_until_sent(serdev, 0);
+
+ return count;
+}
+
+static const struct gnss_operations gnss_serial_gnss_ops = {
+ .open = gnss_serial_open,
+ .close = gnss_serial_close,
+ .write_raw = gnss_serial_write_raw,
+};
+
+static int gnss_serial_receive_buf(struct serdev_device *serdev,
+ const unsigned char *buf, size_t count)
+{
+ struct gnss_serial *gserial = serdev_device_get_drvdata(serdev);
+ struct gnss_device *gdev = gserial->gdev;
+
+ return gnss_insert_raw(gdev, buf, count);
+}
+
+static const struct serdev_device_ops gnss_serial_serdev_ops = {
+ .receive_buf= gnss_serial_receive_buf,
+ .write_wakeup = serdev_device_write_wakeup,
+};
+
+static int gnss_serial_set_power(struct gnss_serial *gserial,
+ enum gnss_serial_pm_state state)
+{
+ if (!gserial->ops || !gserial->ops->set_power)
+ return 0;
+
+ return gserial->ops->set_power(gserial, state);
+}
+
+/*
+ * FIXME: need to provide subdriver defaults or separate dt parsing from
+ * allocation.
+ */
+static int gnss_serial_parse_dt(struct serdev_device *serdev)
+{
+ struct gnss_serial *gserial = serdev_device_get_drvdata(serdev);
+ struct device_node *node = serdev->dev.of_node;
+ u32 speed = 4800;
+
+ of_property_read_u32(node, "current-speed", &speed);
+
+ gserial->speed = speed;
+
+ return 0;
+}
+
+struct gnss_serial *
+gnss_serial_allocate(struct serdev_device *serdev, size_t data_size)
+{
+ struct gnss_serial *gserial;
+ struct gnss_device *gdev;
+ int ret;
+
+ gserial = kzalloc(sizeof(*gserial) + data_size, GFP_KERNEL);
+ if (!gserial)
+ return ERR_PTR(-ENOMEM);
+
+ gdev = gnss_allocate_device(&serdev->dev);
+ if (!gde
[PATCH 4/7] dt-bindings: gnss: add u-blox binding
Add binding for u-blox GNSS receivers.
Note that the u-blox product names encodes form factor (e.g. "neo"),
chipset (e.g. "8") and variant (e.g. "q"), but that only formfactor and
chipset is used for the compatible strings (for now).
Signed-off-by: Johan Hovold
---
.../devicetree/bindings/gnss/u-blox.txt | 31 +++
.../devicetree/bindings/vendor-prefixes.txt | 1 +
2 files changed, 32 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gnss/u-blox.txt
diff --git a/Documentation/devicetree/bindings/gnss/u-blox.txt
b/Documentation/devicetree/bindings/gnss/u-blox.txt
new file mode 100644
index ..bb54b83a177f
--- /dev/null
+++ b/Documentation/devicetree/bindings/gnss/u-blox.txt
@@ -0,0 +1,31 @@
+u-blox GNSS Receiver DT binding
+
+The u-blox GNSS receivers can use UART, DDC (I2C), SPI and USB interfaces.
+
+Please see Documentation/devicetree/bindings/gnss/gnss.txt for generic
+properties.
+
+Required Properties:
+
+- compatible : Must be one of
+
+ "u-blox,neo-8"
+ "u-blox,neo-m8"
+
+- vcc-supply : Main voltage regulator (VCC)
+
+Optional Properties:
+
+- timepulse-gpios : Timepulse (e.g. 1PPS) GPIO (TIMEPULSE)
+
+Example:
+
+serial@1234 {
+ compatible = "ns16550a";
+
+ gnss {
+ compatible = "u-blox,neo-8";
+
+ vcc-supply = <&gnss_reg>;
+ };
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index b5f978a4cac6..2128dfdf73f1 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -374,6 +374,7 @@ tronsmart Tronsmart
truly Truly Semiconductors Limited
tsdTheobroma Systems Design und Consulting GmbH
tyan Tyan Computer Corporation
+u-blox u-blox
ucrobotics uCRobotics
ubnt Ubiquiti Networks
udoo Udoo
--
2.17.0
[PATCH 5/7] gnss: add driver for u-blox receivers
Add driver for serial-connected u-blox GNSS receivers.
Note that the driver uses the generic GNSS serial implementation and
therefore essentially only manages power abstracted into three power
states: ACTIVE, STANDBY, and OFF.
For u-blox receivers with a single supply and no enable-gpios, this
simply means that the main supply is disabled in STANDBY and OFF.
Note that timepulse-support is not yet implemented.
Signed-off-by: Johan Hovold
---
drivers/gnss/Kconfig | 13 +
drivers/gnss/Makefile | 3 +
drivers/gnss/ubx.c| 133 ++
3 files changed, 149 insertions(+)
create mode 100644 drivers/gnss/ubx.c
diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
index f8ee54f99a8d..784b8c0367d9 100644
--- a/drivers/gnss/Kconfig
+++ b/drivers/gnss/Kconfig
@@ -15,4 +15,17 @@ if GNSS
config GNSS_SERIAL
tristate
+config GNSS_UBX_SERIAL
+ tristate "u-blox GNSS receiver support"
+ depends on SERIAL_DEV_BUS
+ select GNSS_SERIAL
+ ---help---
+ Say Y here if you have a u-blox GNSS receiver which uses a serial
+ interface.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gnss-ubx.
+
+ If unsure, say N.
+
endif # GNSS
diff --git a/drivers/gnss/Makefile b/drivers/gnss/Makefile
index 171aba71684d..d9295b20b7bc 100644
--- a/drivers/gnss/Makefile
+++ b/drivers/gnss/Makefile
@@ -8,3 +8,6 @@ gnss-y := core.o
obj-$(CONFIG_GNSS_SERIAL) += gnss-serial.o
gnss-serial-y := serial.o
+
+obj-$(CONFIG_GNSS_UBX_SERIAL) += gnss-ubx.o
+gnss-ubx-y := ubx.o
diff --git a/drivers/gnss/ubx.c b/drivers/gnss/ubx.c
new file mode 100644
index ..2f2f00202b5b
--- /dev/null
+++ b/drivers/gnss/ubx.c
@@ -0,0 +1,133 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * u-blox GNSS receiver driver
+ *
+ * Copyright (C) 2018 Johan Hovold
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#include "serial.h"
+
+struct ubx_data {
+ struct regulator *vcc;
+};
+
+static int ubx_set_active(struct gnss_serial *gserial)
+{
+ struct ubx_data *data = gnss_serial_get_drvdata(gserial);
+ int ret;
+
+ dev_dbg(&gserial->serdev->dev, "%s\n", __func__);
+
+ ret = regulator_enable(data->vcc);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int ubx_set_standby(struct gnss_serial *gserial)
+{
+ struct ubx_data *data = gnss_serial_get_drvdata(gserial);
+ int ret;
+
+ dev_dbg(&gserial->serdev->dev, "%s\n", __func__);
+
+ ret = regulator_disable(data->vcc);
+ if (ret)
+ return ret;
+
+ return 0;
+}
+
+static int
+ubx_set_power(struct gnss_serial *gserial, enum gnss_serial_pm_state state)
+{
+ switch (state) {
+ case GNSS_SERIAL_ACTIVE:
+ return ubx_set_active(gserial);
+ case GNSS_SERIAL_OFF:
+ case GNSS_SERIAL_STANDBY:
+ return ubx_set_standby(gserial);
+ }
+
+ return -EINVAL;
+}
+
+const struct gnss_serial_ops ubx_gserial_ops = {
+ .set_power = ubx_set_power,
+};
+
+static int ubx_probe(struct serdev_device *serdev)
+{
+ struct gnss_serial *gserial;
+ struct ubx_data *data;
+ int ret;
+
+ gserial = gnss_serial_allocate(serdev, sizeof(*data));
+ if (IS_ERR(gserial)) {
+ ret = PTR_ERR(gserial);
+ return ret;
+ }
+
+ gserial->ops = &ubx_gserial_ops;
+
+ data = gnss_serial_get_drvdata(gserial);
+
+ data->vcc = devm_regulator_get(&serdev->dev, "vcc");
+ if (IS_ERR(data->vcc)) {
+ ret = PTR_ERR(data->vcc);
+ goto err_free_gserial;
+ }
+
+ ret = gnss_serial_register(gserial);
+ if (ret)
+ goto err_free_gserial;
+
+ return 0;
+
+err_free_gserial:
+ gnss_serial_free(gserial);
+
+ return ret;
+}
+
+static void ubx_remove(struct serdev_device *serdev)
+{
+ struct gnss_serial *gserial = serdev_device_get_drvdata(serdev);
+
+ gnss_serial_deregister(gserial);
+ gnss_serial_free(gserial);
+};
+
+#ifdef CONFIG_OF
+static const struct of_device_id ubx_of_match[] = {
+ { .compatible = "u-blox,neo-8" },
+ { .compatible = "u-blox,neo-m8" },
+ {},
+};
+MODULE_DEVICE_TABLE(of, ubx_of_match);
+#endif
+
+static struct serdev_device_driver ubx_driver = {
+ .driver = {
+ .name = "gnss-ubx",
+ .of_match_table = of_match_ptr(ubx_of_match),
+ .pm = &gnss_serial_pm_ops,
+ },
+ .probe = ubx_probe,
+ .remove = ubx_remove,
+};
+module_serdev_device_driver(ubx_driver);
+
+MODULE_AUTHOR("Johan Hovold ");
+MODULE_DESCRIPTION("u-blox GNSS receiver driver");
+MODULE_LICENSE("GPL v2");
--
2.17.0
[PATCH 0/7] gnss: add new GNSS subsystem
This series adds a new subsystem for GNSS receivers (e.g. GPS receivers). While GNSS receivers are typically accessed using a UART interface they often also support other I/O interfaces such as I2C, SPI and USB, while yet other devices use iomem or even some form of remote-processor messaging (rpmsg). The new GNSS subsystem abstracts the underlying interface and provides a new "gnss" class type, which exposes a character-device interface (e.g. /dev/gnss0) to user space. This allows GNSS receivers to have a representation in the Linux device model, something which is important not least for power management purposes and which also allows for easy detection and (eventually) identification of GNSS devices. Note that the character-device interface provides raw access to whatever protocol the receiver is (currently) using, such as NMEA 0183, UBX or SiRF Binary. These protocols are expected to be continued to be handled by user space for the time being, even if some hybrid solutions are also conceivable (e.g. to have kernel drivers issue management commands). This will still allow for better platform integration by allowing GNSS devices and their resources (e.g. regulators and enable-gpios) to be described by firmware and managed by kernel drivers rather than platform-specific scripts and services. While the current interface is kept minimal, it could be extended using IOCTLs, sysfs or uevents as needs and proper abstraction levels are identified and determined (e.g. for device and feature identification). Another possible extension is to add generic 1PPS support. I decided to go with a custom character-device interface rather than pretend that these abstract GNSS devices are still TTY devices (e.g. /dev/ttyGNSS0). Obviously, modifying line settings or reading modem control signals does not make any sense for a device using, say, a USB (not USB-serial) or iomem interface. This also means, however, that user space would no longer be able to set the line speed to match a new port configuration that can be set using the various GNSS protocols when the underlying interface is indeed a UART; instead this would need to be taken care of by the driver. Also note that writes are always synchronous instead of requiring user space to call tcdrain() after every command. This all seems to work well-enough (e.g. with gpsd), but please let me know if I've overlooked something which would indeed require a TTY interface instead. As proof-of-concept I have implemented drivers for receivers based on two common GNSS chipsets (SiRFstar and u-blox), but due to lack of hardware these have so far only been tested using mockup devices and a USB-serial-based GPS device (using out-of-tree code). [ Let me know if you've got any evalutation kits to spare. ] Finally, note that documentation (including kerneldoc) remains to be written, but hopefully this will not hinder review given that the current interfaces are fairly self-describing. Johan Johan Hovold (7): gnss: add GNSS receiver subsystem dt-bindings: add generic gnss binding gnss: add generic serial driver dt-bindings: gnss: add u-blox binding gnss: add driver for u-blox receivers dt-bindings: gnss: add sirfstar binding gnss: add driver for sirfstar-based receivers .../devicetree/bindings/gnss/gnss.txt | 36 ++ .../devicetree/bindings/gnss/sirfstar.txt | 38 ++ .../devicetree/bindings/gnss/u-blox.txt | 31 ++ .../devicetree/bindings/vendor-prefixes.txt | 4 + MAINTAINERS | 7 + drivers/Kconfig | 2 + drivers/Makefile | 1 + drivers/gnss/Kconfig | 43 ++ drivers/gnss/Makefile | 16 + drivers/gnss/core.c | 385 drivers/gnss/serial.c | 288 drivers/gnss/serial.h | 47 ++ drivers/gnss/sirf.c | 415 ++ drivers/gnss/ubx.c| 133 ++ include/linux/gnss.h | 64 +++ 15 files changed, 1510 insertions(+) create mode 100644 Documentation/devicetree/bindings/gnss/gnss.txt create mode 100644 Documentation/devicetree/bindings/gnss/sirfstar.txt create mode 100644 Documentation/devicetree/bindings/gnss/u-blox.txt create mode 100644 drivers/gnss/Kconfig create mode 100644 drivers/gnss/Makefile create mode 100644 drivers/gnss/core.c create mode 100644 drivers/gnss/serial.c create mode 100644 drivers/gnss/serial.h create mode 100644 drivers/gnss/sirf.c create mode 100644 drivers/gnss/ubx.c create mode 100644 include/linux/gnss.h -- 2.17.0
Re: [tip:x86/urgent] x86/io: Define readq()/writeq() to use 64-bit type
On Sat, 2018-03-31 at 20:45 +0200, Ingo Molnar wrote: > * Andy Shevchenko wrote: > > > On Sat, Mar 31, 2018 at 3:06 PM, Andy Shevchenko > > wrote: > > > On Sat, Mar 31, 2018 at 1:22 PM, Ingo Molnar > > > wrote: > > > > * Ingo Molnar wrote: > > > > [tip:x86/urgent 14/14] > > > > drivers/infiniband/hw/hns/hns_roce_hw_v1.c:1690:22: sparse: > > > > incorrect type in argument 1 (different base types) > > > > > > > > Since you were on Cc: of that report I assumed you'd take care > > > > of it. > > > > > > Ah, yes. This one I fixed. > > > > https://git.kernel.org/pub/scm/linux/kernel/git/rdma/rdma.git/commit > > /?h=for-next&id=71591d1280e5ef02c2af2ffb9801d0c842973be9 > > Yeah, so this fix in the RDMA tree needs to go upstream first, before > we can apply > the changes to the interface. Could you resend at around v4.17-rc1 or > so? Btw, should I recend or you can still pick it up from linux-next or mailing list? I can resend tomorrow if needed. -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v3 02/12] KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
On 24 April 2018 at 17:46, Christoffer Dall wrote: > On Fri, Apr 13, 2018 at 10:20:48AM +0200, Eric Auger wrote: >> --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt >> +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt >> @@ -27,9 +27,32 @@ Groups: >>VCPU and all of the redistributor pages are contiguous. >>Only valid for KVM_DEV_TYPE_ARM_VGIC_V3. >>This address needs to be 64K aligned. >> + >> +KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION (rw, 64-bit) >> + The attr field of kvm_device_attr encodes 3 values: >> + bits: | 63 52 | 51 16 | 15 - 12 |11 - 0 >> + values: | count | base | flags | index >> + - index encodes the unique redistributor region index > > I'm not entirely sure I understand the purpose of the index field. > Isn't a redistributor region identified uniquely by its base address? You need a way to tell the difference beween: (1) redistributors for CPUs 0..63 at 0x4000, redistributors for 64..127 at 0x8000 (2) redistributors for CPUs 0..63 at 0x8000, redistributors for 64..127 at 0x4000 The index field tells you which order the redistributor regions go in. thanks -- PMM
[PATCH 6/7] dt-bindings: gnss: add sirfstar binding
Add binding for SiRFstar-based GNSS receivers.
Note that while four compatible-strings are initially added representing
devices which differ in which I/O interfaces they support, they
otherwise essentially share the same feature set.
Pin and supply names (and some recommended timings) vary slightly, but
the binding recommends using a common set of names.
Note that the wakeup gpio is not intended to be as a wakeup source, but
rather to detect the current power state of the device (active or
hibernate).
Signed-off-by: Johan Hovold
---
.../devicetree/bindings/gnss/sirfstar.txt | 38 +++
.../devicetree/bindings/vendor-prefixes.txt | 3 ++
2 files changed, 41 insertions(+)
create mode 100644 Documentation/devicetree/bindings/gnss/sirfstar.txt
diff --git a/Documentation/devicetree/bindings/gnss/sirfstar.txt
b/Documentation/devicetree/bindings/gnss/sirfstar.txt
new file mode 100644
index ..5e6a02aec49a
--- /dev/null
+++ b/Documentation/devicetree/bindings/gnss/sirfstar.txt
@@ -0,0 +1,38 @@
+SiRFstar-based GNSS Receiver DT binding
+
+SiRFstar chipsets are used in GNSS-receiver modules produced by several
+vendors and can use UART, SPI or I2C interfaces.
+
+Please see Documentation/devicetree/bindings/gnss/gnss.txt for generic
+properties.
+
+Required Properties:
+
+- compatible : Must be one of
+
+ "fastrax,uc430"
+ "linx,r4"
+ "wi2wi,w2sg0008i"
+ "wi2wi,w2sg0084i"
+
+- vcc-supply : Main voltage regulator (3V3_IN, VDD, VCC)
+
+Optional Properties:
+
+- enable-gpios : GPIO used to power on and off device (ON_OFF)
+- wakeup-gpios : GPIO used to determine device power state (WAKEUP, RFPWRUP)
+- timepulse-gpios : Timepulse (e.g 1PPS) GPIO (1PPS, TM)
+
+Example:
+
+serial@1234 {
+ compatible = "ns16550a";
+
+ gnss {
+ compatible = "wi2wi,w2sg0084i";
+
+ vcc-supply = <&gnss_reg>;
+ enable-gpios = <&gpio0 16 GPIO_ACTIVE_HIGH>;
+ wakeup-gpios = <&gpio0 17 GPIO_ACTIVE_HIGH>;
+ };
+};
diff --git a/Documentation/devicetree/bindings/vendor-prefixes.txt
b/Documentation/devicetree/bindings/vendor-prefixes.txt
index 2128dfdf73f1..ddd81c82082d 100644
--- a/Documentation/devicetree/bindings/vendor-prefixes.txt
+++ b/Documentation/devicetree/bindings/vendor-prefixes.txt
@@ -120,6 +120,7 @@ excito Excito
ezchip EZchip Semiconductor
fairphone Fairphone B.V.
faradayFaraday Technology Corporation
+fastraxFastrax Oy
fcsFairchild Semiconductor
fireflyFirefly
focaltech FocalTech Systems Co.,Ltd
@@ -197,6 +198,7 @@ licheepiLichee Pi
linaro Linaro Limited
linksysBelkin International, Inc. (Linksys)
linux Linux-specific binding
+linx Linx Technologies
lltc Linear Technology Corporation
lsiLSI Corp. (LSI Logic)
lwnLiebherr-Werk Nenzing GmbH
@@ -390,6 +392,7 @@ vivante Vivante Corporation
vocore VoCore Studio
voipac Voipac Technologies s.r.o.
votVision Optical Technology Co., Ltd.
+wi2wi Wi2Wi
wd Western Digital Corp.
wetek WeTek Electronics, limited.
wexler Wexler
--
2.17.0
[PATCH 7/7] gnss: add driver for sirfstar-based receivers
Add driver for serial-connected SiRFstar-based GNSS receivers.
These devices typically boot into hibernate mode from which they can be
woken using a pulse on the ON_OFF (enable) input pin. Once active, a
pulse on the same ON_OFF pin is used to put the device back into
hibernate mode. The current state can be determined by sampling the
WAKEUP output.
Hardware configurations where WAKEUP has been connected to ON_OFF (and
where an initial WAKEUP pulse during boot is sufficient to have the
device boot into active mode) are also supported. In this case, device
power is managed using the main-supply regulator only.
Note that configurations where WAKEUP is left not connected, so that the
device power state can only indirectly be determined using the I/O
interface, is currently not supported. It should be fairly
straight-forward to extend the current implementation with such support
however (and this this is the main reason for not using the generic
serial implementation for this driver).
Note that timepulse-support is left unimplemented.
Signed-off-by: Johan Hovold
---
drivers/gnss/Kconfig | 12 ++
drivers/gnss/Makefile | 3 +
drivers/gnss/sirf.c | 415 ++
3 files changed, 430 insertions(+)
create mode 100644 drivers/gnss/sirf.c
diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
index 784b8c0367d9..6abc88514512 100644
--- a/drivers/gnss/Kconfig
+++ b/drivers/gnss/Kconfig
@@ -15,6 +15,18 @@ if GNSS
config GNSS_SERIAL
tristate
+config GNSS_SIRF_SERIAL
+ tristate "SiRFstar GNSS receiver support"
+ depends on SERIAL_DEV_BUS
+ ---help---
+ Say Y here if you have a SiRFstar-based GNSS receiver which uses a
+ serial interface.
+
+ To compile this driver as a module, choose M here: the module will
+ be called gnss-sirf.
+
+ If unsure, say N.
+
config GNSS_UBX_SERIAL
tristate "u-blox GNSS receiver support"
depends on SERIAL_DEV_BUS
diff --git a/drivers/gnss/Makefile b/drivers/gnss/Makefile
index d9295b20b7bc..5cf0ebe0330a 100644
--- a/drivers/gnss/Makefile
+++ b/drivers/gnss/Makefile
@@ -9,5 +9,8 @@ gnss-y := core.o
obj-$(CONFIG_GNSS_SERIAL) += gnss-serial.o
gnss-serial-y := serial.o
+obj-$(CONFIG_GNSS_SIRF_SERIAL) += gnss-sirf.o
+gnss-sirf-y := sirf.o
+
obj-$(CONFIG_GNSS_UBX_SERIAL) += gnss-ubx.o
gnss-ubx-y := ubx.o
diff --git a/drivers/gnss/sirf.c b/drivers/gnss/sirf.c
new file mode 100644
index ..9d46da1530ca
--- /dev/null
+++ b/drivers/gnss/sirf.c
@@ -0,0 +1,415 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * SiRFstar GNSS receiver driver
+ *
+ * Copyright (C) 2018 Johan Hovold
+ */
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define SIRF_BOOT_DELAY500
+#define SIRF_ON_OFF_PULSE_TIME 100
+#define SIRF_ACTIVATE_TIMEOUT 200
+#define SIRF_HIBERNATE_TIMEOUT 200
+
+struct sirf_data {
+ struct gnss_device *gdev;
+ struct serdev_device *serdev;
+ speed_t speed;
+ struct regulator *vcc;
+ struct gpio_desc *on_off;
+ struct gpio_desc *wakeup;
+ int irq;
+ bool active;
+ wait_queue_head_t power_wait;
+};
+
+static int sirf_open(struct gnss_device *gdev)
+{
+ struct sirf_data *data = gnss_get_drvdata(gdev);
+ struct serdev_device *serdev = data->serdev;
+ int ret;
+
+ ret = serdev_device_open(serdev);
+ if (ret)
+ return ret;
+
+ serdev_device_set_baudrate(serdev, data->speed);
+ serdev_device_set_flow_control(serdev, false);
+
+ ret = pm_runtime_get_sync(&serdev->dev);
+ if (ret < 0) {
+ dev_err(&gdev->dev, "failed to runtime resume: %d\n", ret);
+ pm_runtime_put_noidle(&serdev->dev);
+ goto err_close;
+ }
+
+ return 0;
+
+err_close:
+ serdev_device_close(serdev);
+
+ return ret;
+}
+
+static void sirf_close(struct gnss_device *gdev)
+{
+ struct sirf_data *data = gnss_get_drvdata(gdev);
+ struct serdev_device *serdev = data->serdev;
+
+ serdev_device_close(serdev);
+
+ pm_runtime_put(&serdev->dev);
+}
+
+static int sirf_write_raw(struct gnss_device *gdev,
+ const unsigned char *buf, size_t count)
+{
+ struct sirf_data *data = gnss_get_drvdata(gdev);
+ struct serdev_device *serdev = data->serdev;
+ int ret;
+
+ /* write is only buffered synchronously */
+ ret = serdev_device_write(serdev, buf, count, 0);
+ if (ret < 0)
+ return ret;
+
+ /* FIXME: determine if interrupted? */
+ serdev_device_wait_until_sent(serdev, 0);
+
+ return count;
+}
+
+static const struct gnss_operations sirf_gnss_ops = {
+ .open = sirf_open,
+ .close = sirf_close,
+ .write_raw
Re: [PATCH 1/3] signals: Allow generation of SIGKILL to exiting task.
On 04/24/2018 12:42 PM, Eric W. Biederman wrote:
Andrey Grodzovsky writes:
Currently calling wait_event_killable as part of exiting process
will stall forever since SIGKILL generation is suppresed by PF_EXITING.
In our partilaur case AMDGPU driver wants to flush all GPU jobs in
flight before shutting down. But if some job hangs the pipe we still want to
be able to kill it and avoid a process in D state.
I should clarify. This absolutely can not be done.
PF_EXITING is set just before a task starts tearing down it's signal
handling.
So delivering any signal, or otherwise depending on signal handling
after PF_EXITING is set can not be done. That abstraction is gone.
I see, so you suggest it's the driver responsibility to avoid creating
such code path that ends up
calling wait_event_killable from exit call stack (PF_EXITING == 1) ?
Andrey
Eric
Signed-off-by: Andrey Grodzovsky
---
kernel/signal.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/kernel/signal.c b/kernel/signal.c
index c6e4c83..c49c706 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -886,10 +886,10 @@ static inline int wants_signal(int sig, struct
task_struct *p)
{
if (sigismember(&p->blocked, sig))
return 0;
- if (p->flags & PF_EXITING)
- return 0;
if (sig == SIGKILL)
return 1;
+ if (p->flags & PF_EXITING)
+ return 0;
if (task_is_stopped_or_traced(p))
return 0;
return task_curr(p) || !signal_pending(p);
[PATCH 1/7] gnss: add GNSS receiver subsystem
Add a new subsystem for GNSS (e.g. GPS) receivers.
While GNSS receivers are typically accessed using a UART interface they
often also support other I/O interfaces such as I2C, SPI and USB, while
yet other devices use iomem or even some form of remote-processor
messaging (rpmsg).
The new GNSS subsystem abstracts the underlying interface and provides a
new "gnss" class type, which exposes a character-device interface (e.g.
/dev/gnss0) to user space. This allows GNSS receivers to have a
representation in the Linux device model, something which is important
not least for power management purposes.
Note that the character-device interface provides raw access to whatever
protocol the receiver is (currently) using, such as NMEA 0183, UBX or
SiRF Binary. These protocols are expected to be continued to be handled
by user space for the time being, even if some hybrid solutions are also
conceivable (e.g. to have kernel drivers issue management commands).
This will still allow for better platform integration by allowing GNSS
devices and their resources (e.g. regulators and enable-gpios) to be
described by firmware and managed by kernel drivers rather than
platform-specific scripts and services.
While the current interface is kept minimal, it could be extended using
IOCTLs, sysfs or uevents as needs and proper abstraction levels are
identified and determined (e.g. for device and feature identification).
Signed-off-by: Johan Hovold
---
MAINTAINERS | 6 +
drivers/Kconfig | 2 +
drivers/Makefile | 1 +
drivers/gnss/Kconfig | 11 ++
drivers/gnss/Makefile | 7 +
drivers/gnss/core.c | 385 ++
include/linux/gnss.h | 64 +++
7 files changed, 476 insertions(+)
create mode 100644 drivers/gnss/Kconfig
create mode 100644 drivers/gnss/Makefile
create mode 100644 drivers/gnss/core.c
create mode 100644 include/linux/gnss.h
diff --git a/MAINTAINERS b/MAINTAINERS
index 0a1410d5a621..dc3df211c1a7 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -5962,6 +5962,12 @@ F: Documentation/isdn/README.gigaset
F: drivers/isdn/gigaset/
F: include/uapi/linux/gigaset_dev.h
+GNSS SUBSYSTEM
+M: Johan Hovold
+S: Maintained
+F: drivers/gnss/
+F: include/linux/gnss.h
+
GO7007 MPEG CODEC
M: Hans Verkuil
L: linux-me...@vger.kernel.org
diff --git a/drivers/Kconfig b/drivers/Kconfig
index 95b9ccc08165..ab4d43923c4d 100644
--- a/drivers/Kconfig
+++ b/drivers/Kconfig
@@ -9,6 +9,8 @@ source "drivers/bus/Kconfig"
source "drivers/connector/Kconfig"
+source "drivers/gnss/Kconfig"
+
source "drivers/mtd/Kconfig"
source "drivers/of/Kconfig"
diff --git a/drivers/Makefile b/drivers/Makefile
index 24cd47014657..cc9a7c5f7d2c 100644
--- a/drivers/Makefile
+++ b/drivers/Makefile
@@ -185,3 +185,4 @@ obj-$(CONFIG_TEE) += tee/
obj-$(CONFIG_MULTIPLEXER) += mux/
obj-$(CONFIG_UNISYS_VISORBUS) += visorbus/
obj-$(CONFIG_SIOX) += siox/
+obj-$(CONFIG_GNSS) += gnss/
diff --git a/drivers/gnss/Kconfig b/drivers/gnss/Kconfig
new file mode 100644
index ..103fcc70992e
--- /dev/null
+++ b/drivers/gnss/Kconfig
@@ -0,0 +1,11 @@
+#
+# GNSS receiver configuration
+#
+
+menuconfig GNSS
+ tristate "GNSS receiver support"
+ ---help---
+ Say Y here if you have a GNSS receiver (e.g. a GPS receiver).
+
+ To compile this driver as a module, choose M here: the module will
+ be called gnss.
diff --git a/drivers/gnss/Makefile b/drivers/gnss/Makefile
new file mode 100644
index ..1f7a7baab1d9
--- /dev/null
+++ b/drivers/gnss/Makefile
@@ -0,0 +1,7 @@
+# SPDX-License-Identifier: GPL-2.0
+#
+# Makefile for the GNSS subsystem.
+#
+
+obj-$(CONFIG_GNSS) += gnss.o
+gnss-y := core.o
diff --git a/drivers/gnss/core.c b/drivers/gnss/core.c
new file mode 100644
index ..9b88c7e72233
--- /dev/null
+++ b/drivers/gnss/core.c
@@ -0,0 +1,385 @@
+// SPDX-License-Identifier: GPL-2.0
+/*
+ * GNSS receiver core
+ *
+ * Copyright (C) 2018 Johan Hovold
+ */
+
+#define pr_fmt(fmt) KBUILD_MODNAME ": " fmt
+
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define GNSS_MINORS16
+
+static DEFINE_IDA(gnss_minors);
+static dev_t gnss_first;
+
+/* FIFO size must be a power of two */
+#define GNSS_READ_FIFO_SIZE4096
+#define GNSS_WRITE_BUF_SIZE1024
+
+#define to_gnss_device(d) container_of((d), struct gnss_device, dev)
+
+static int gnss_open(struct inode *inode, struct file *file)
+{
+ struct gnss_device *gdev;
+ int ret = 0;
+
+ gdev = container_of(inode->i_cdev, struct gnss_device, cdev);
+
+ get_device(&gdev->dev);
+
+ nonseekable_open(inode, file);
+ file->private_data = gdev;
+
+ down_write(&gdev->rwsem);
+ if (gdev->disconnected) {
+ ret = -ENODEV;
+ goto unlock;
+ }
+
+
Re: [PATCH v5 21/23] ASoC: qdsp6: dt-bindings: Add apq8096 machine bindings
Thanks for the review. On 24/04/18 17:25, Rob Herring wrote: On Wed, Apr 18, 2018 at 04:31:55PM +0100, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla Add devicetree bindings documentation file for Qualcomm apq8096 sound card. Signed-off-by: Srinivas Kandagatla --- .../devicetree/bindings/sound/qcom,apq8096.txt | 76 ++ 1 file changed, 76 insertions(+) create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8096.txt diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8096.txt b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt new file mode 100644 index ..37e23d926b95 --- /dev/null +++ b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt @@ -0,0 +1,76 @@ +* Qualcomm Technologies APQ8096 ASoC sound card driver + +This binding describes the APQ8096 sound card, which uses qdsp for audio. + +- compatible: + Usage: required + Value type: + Definition: must be "qcom,apq8096-sndcard" + +- qcom,audio-routing: + Usage: Optional + Value type: + Definition: A list of the connections between audio components. + Each entry is a pair of strings, the first being the + connection's sink, the second being the connection's + source. Valid names could be power supplies, MicBias + of codec and the jacks on the board: Please list out valid values here. I can list the values for the HDMI playback use-case, but the list would grow as we start adding wcd9335 codec support. + += dailinks +Each subnode of sndcard represents either a dailink, and subnodes of each +dailinks would be cpu/codec/platform dais. + +- link-name: Not a standard property, but I guess that sneaked in with the 8016 binding... Yes, I followed 8016 bindings. Am happy to prefix this with qcom if that makes more sense. + Usage: required + Value type: + Definition: User friendly name for dai link + += CPU, PLATFORM, CODEC dais subnodes +- cpu: + Usage: required + Value type: + Definition: cpu dai sub-node + +- codec: + Usage: Optional + Value type: + Definition: codec dai sub-node + +- platform: + Usage: Optional + Value type: + Definition: platform dai sub-node + +- sound-dai: + Usage: required + Value type: phandle with args. Yep. --srini
Re: [RFC] mm: kmemleak: replace __GFP_NOFAIL to GFP_NOWAIT in gfp_kmemleak_mask
- Original Message -
> From: "Michal Hocko"
> To: "Chunyu Hu"
> Cc: "Dmitry Vyukov" , "Catalin Marinas"
> , "Chunyu Hu"
> , "LKML" , "Linux-MM"
>
> Sent: Tuesday, April 24, 2018 9:20:57 PM
> Subject: Re: [RFC] mm: kmemleak: replace __GFP_NOFAIL to GFP_NOWAIT in
> gfp_kmemleak_mask
>
> On Mon 23-04-18 12:17:32, Chunyu Hu wrote:
> [...]
> > So if there is a new flag, it would be the 25th bits.
>
> No new flags please. Can you simply store a simple bool into fail_page_alloc
> and have save/restore api for that?
Hi Michal,
I still don't get your point. The original NOFAIL added in kmemleak was
for skipping fault injection in page/slab allocation for kmemleak object,
since kmemleak will disable itself until next reboot, whenever it hit an
allocation failure, in that case, it will lose effect to check kmemleak
in errer path rose by fault injection. But NOFAULT's effect is more than
skipping fault injection, it's also for hard allocation. So a dedicated flag
for skipping fault injection in specified slab/page allocation was mentioned.
d9570ee3bd1d ("kmemleak: allow to coexist with fault injection")
Do you mean something like below, with the save/store api? But looks like
to make it possible to skip a specified allocation, not global disabling,
a bool is not enough, and a gfp_flag is also needed. Maybe I missed something?
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 905db9d..ca6f609 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -3053,14 +3053,28 @@ struct page *rmqueue(struct zone *preferred_zone,
bool ignore_gfp_highmem;
bool ignore_gfp_reclaim;
+ bool ignore_kmemleak_fault;
u32 min_order;
} fail_page_alloc = {
.attr = FAULT_ATTR_INITIALIZER,
.ignore_gfp_reclaim = true,
.ignore_gfp_highmem = true,
+ .ignore_kmemleak_fault = true,
.min_order = 1,
};
+bool saved_fail_page_alloc_ignore;
+void fail_page_alloc_ignore_save(void)
+{
+ saved_fail_page_alloc_ignore = fail_page_alloc.ignore_kmemleak_fault;
+ fail_page_alloc.ignore_kmemleak_fault = true;
+}
+
+void fail_page_alloc_ignore_restore(void)
+{
+ fail_page_alloc.ignore_kmemleak_fault = saved_fail_page_alloc_ignore;
+}
+
static int __init setup_fail_page_alloc(char *str)
{
return setup_fault_attr(&fail_page_alloc.attr, str);
@@ -3075,6 +3089,9 @@ static bool should_fail_alloc_page(gfp_t gfp_mask,
unsigned int order)
return false;
if (fail_page_alloc.ignore_gfp_highmem && (gfp_mask & __GFP_HIGHMEM))
return false;
+ /* looks like here we still need a new GFP_KMEMLKEAK flag ... */
+ if (fail_page_alloc.ignore_kmemleak_fault && (gfp_mask & __GFP_KMEMLEAK))
+ return false;
if (fail_page_alloc.ignore_gfp_reclaim &&
(gfp_mask & __GFP_DIRECT_RECLAIM))
return false;
>
> --
> Michal Hocko
> SUSE Labs
>
--
Regards,
Chunyu Hu
Re: hrtimer (rdmavt RNR timer) was lost
On Tue, Apr 24, 2018 at 04:54:58PM +0200, Thomas Gleixner wrote:
> On Mon, 23 Apr 2018, Thomas Gleixner wrote:
> > On Mon, 23 Apr 2018, Wan, Kaike wrote:
> > > > Can you apply the following debug patch and enable the hrtimer_start
> > > > trace
> > > > point and send me the full trace or upload it somewhere?
> > >
> > > The original trace was about 29GB and I filtered it with
> > > "66dda1ea" (the offending base) to generate a 1.4GB file that I
> > > could open and investigate. I am not sure how I can send them to you. Do
> > > you have somewhere I can upload to?
> > >
> > > I can try your debug patch and again I am anticipating a big trace file.
> >
> > Well, you can find the spot where the fail happens and then extract the
> > full thing from 2s before that point to 1s after. That should be reasonably
> > small and good enough. Let me know when you have it and how big it is
> > (compressed) and we'll figure something out how to transport it.
>
> Thanks for the more complete data which actually let me decode the wreckage.
>
> So you have NO_HZ enabled and looking at the trace then this is related:
>
> -0 [003] dN.. 3598605307236: hrtimer_start:
> hrtimer=04346740 function=tick_sched_timer expires=71217100
> softexpires=71217100mode=ABS|PINNED base=66dda1ea
> next=708914d7
> -0 [003] dN.. 3598605307601: hrtimer_post_add:
> hrtimer=04346740 function=tick_sched_timer base=66dda1ea
> next=04346740
>
> -0 [002] d.h. 3598605313255: hrtimer_start:
> hrtimer=662d2dd8 function=rvt_rc_rnr_retry [rdmavt]
> expires=712172915662 softexpires=712172915662mode=REL base=66dda1ea
> next=04346740
> -0 [002] d.h. 3599538740786: hrtimer_post_add:
> hrtimer=662d2dd8 function=rvt_rc_rnr_retry [rdmavt]
> base=66dda1ea next=662d2dd8
>
> -0 [003] dn.. 3599538742242: hrtimer_pre_remove:
> hrtimer=04346740 function=tick_sched_timer base=66dda1ea
> next=662d2dd8
> -0 [003] dn.. 3599538743084: hrtimer_post_remove:
> hrtimer=04346740 function=tick_sched_timer base=66dda1ea
> next=662d2dd8
>
> -0 [003] dn.. 3599538743830: hrtimer_start:
> hrtimer=04346740 function=tick_sched_timer expires=71676700
> softexpires=71676700mode=ABS|PINNED base=66dda1ea
> next=662d2dd8
> -0 [003] dn.. 3599538744560: hrtimer_post_add:
> hrtimer=04346740 function=tick_sched_timer base=66dda1ea
> next=662d2dd8
>
> And staring at the NOHZ code I'm pretty sure, I know what happens.
>
> CPU 3 CPU 2
>
> idle
> start tick_sched_timer 71217100
> start rdmavt timer 712172915662
> lock(base)
> tick_nohz_stop_tick()
> tick = 71676700 timerqueue_add(tmr)
>
> hrtimer_set_expires(&ts->sched_timer, tick); < FAIL
>
> if (tmr->exp <
> queue->next->exp)
> hrtimer_start(sched_timer)queue->next = tmr;
> lock(base)
> unlock(base)
> timerqueue_remove()
> timerqueue_add()
>
>
> So ts->sched_timer is still queued and queue->next is pointing to it, but
> then expires is modified. So the other side sees the new expiry time and
> makes the rdmavt timer the new queue->next. All f*cked from there.
>
> The problem was introduced with:
>
> d4af6d933ccf ("nohz: Fix spurious warning when hrtimer and clockevent get
> out of sync")
>
> The changelog is not really helpful, but I can't see why the expiry time of
> ts->sched_timer should be updated before the timer is [re]started in case
> of HIGHRES + NOHZ. hrtimer_start() sets the expiry time, so that should be
> sufficient. Of course the tick needs to be stored in ts->sched_timer for
> the !HIGHRES + HOHZ case, but that's trivial enough to do. Patch against
> Linus tree below. It's easy to backport in case you want to run it against
> the kernel which made the observation simpler.
>
> I need to come up with integration of hrtimer_set_expires() into debug
> objects to catch this kind of horrors. Groan
Sorry for all that, that must have been hairy enough to debug :-(
I thought that I could fiddle with the sched_timer because nothing else
is supposed to touch it concurrently, but that forgot about the whole
queue locked under cpu_base. My fault.
The fix looks all good to me, thanks!
ACK.
Re: [PATCH v5 1/2] char: sparc64: Add privileged ADI driver
On 04/23/2018 11:52 AM, Greg KH wrote:
On Mon, Apr 23, 2018 at 11:33:31AM -0600, Tom Hromatka wrote:
SPARC M7 and newer processors utilize ADI to version and
protect memory. This driver is capable of reading/writing
ADI/MCD versions from privileged user space processes.
Addresses in the adi file are mapped linearly to physical
memory at a ratio of 1:adi_blksz. Thus, a read (or write)
of offset K in the file operates upon the ADI version at
physical address K * adi_blksz. The version information
is encoded as one version per byte. Intended consumers
are makedumpfile and crash.
What do you mean by "crash"? Should this tie into the pstore
infrastructure, or is this just a userspace thing? Just curious.
My apologies. I was referring to the crash utility:
https://github.com/crash-utility/crash
A future commit to store the ADI versions to the pstore would be
really cool. I am fearful the amount of ADI data could overwhelm
the pstore, though. The latest sparc machines support 4 TB of RAM
which could mean several GBs of ADI versions. But storing the ADI
versions pertaining to the failing code should be possible. I need
to do more research...
Minor code comments below now that the license stuff is correct, I
decided to read the code :)
:)
+#include
+#include
+#include
+#include
+#include
+#include
+#include
+
+#define MODULE_NAME"adi"
What's wrong with KBUILD_MODNAME? Just use that instead of MODULE_NAME
later on in the file.
Good catch. I'll do that in the next rev of this patch.
+#define MAX_BUF_SZ 4096
PAGE_SIZE? Just curious.
When a user requests a large read/write in makedumpfile or the crash
utility, these tools typically make requests in 4096-sized chunks.
I believe you are correct that these operations are based upon page
size, but I have not verified. I was hesitant to connect MAX_BUF_SZ to
PAGE_SIZE without this verification. I'll look into it more...
+
+static int adi_open(struct inode *inode, struct file *file)
+{
+ file->f_mode |= FMODE_UNSIGNED_OFFSET;
That's odd, why?
sparc64 currently supports 4 TB of RAM (and could support much more in the
future). Offsets into this ADI privileged driver are address / 64, but
that could change also in the future depending upon cache line sizes. I
was afraid that future sparc systems could have very large file offsets.
Overkill?
+ return 0;
+}
+
+static int read_mcd_tag(unsigned long addr)
+{
+ long err;
+ int ver;
+
+ __asm__ __volatile__(
+ "1:ldxa [%[addr]] %[asi], %[ver]\n"
+ " mov 0, %[err]\n"
+ "2:\n"
+ " .section .fixup,#alloc,#execinstr\n"
+ " .align 4\n"
+ "3:sethi %%hi(2b), %%g1\n"
+ " jmpl %%g1 + %%lo(2b), %%g0\n"
+ " mov %[invalid], %[err]\n"
+ " .previous\n"
+ " .section __ex_table, \"a\"\n"
+ " .align 4\n"
+ " .word 1b, 3b\n"
+ " .previous\n"
+ : [ver] "=r" (ver), [err] "=r" (err)
+ : [addr] "r" (addr), [invalid] "i" (EFAULT),
+ [asi] "i" (ASI_MCD_REAL)
+ : "memory", "g1"
+ );
+
+ if (err)
+ return -EFAULT;
+ else
+ return ver;
+}
+
+static ssize_t adi_read(struct file *file, char __user *buf,
+ size_t count, loff_t *offp)
+{
+ size_t ver_buf_sz, bytes_read = 0;
+ int ver_buf_idx = 0;
+ loff_t offset;
+ u8 *ver_buf;
+ ssize_t ret;
+
+ ver_buf_sz = min_t(size_t, count, MAX_BUF_SZ);
+ ver_buf = kmalloc(ver_buf_sz, GFP_KERNEL);
+ if (!ver_buf)
+ return -ENOMEM;
+
+ offset = (*offp) * adi_blksize();
+
+ while (bytes_read < count) {
+ ret = read_mcd_tag(offset);
+ if (ret < 0)
+ goto out;
+
+ ver_buf[ver_buf_idx] = (u8)ret;
Are you sure ret fits in 8 bits here?
Yes, I believe so. read_mcd_tag() will return a negative number
on an error - which is checked a couple lines above. Otherwise,
the read succeeded which means a valid ADI version was returned.
Valid ADI versions are 0 through 16.
+ ver_buf_idx++;
+ offset += adi_blksize();
+
+ if (ver_buf_idx >= ver_buf_sz) {
+ if (copy_to_user(buf + bytes_read, ver_buf,
+ver_buf_sz)) {
+ ret = -EFAULT;
+ goto out;
+ }
+
+ bytes_read += ver_buf_sz;
+ ver_buf_idx = 0;
+
+ ver_buf_sz = min(count - bytes_read,
+(size_t)MAX_BUF_SZ);
+ }
+ }
+
+ (*offp) += bytes_read;
+ ret = bytes_read;
+out:
+ kfree(ver_buf);
+ retu
Re: [PATCH v4 4/9] vsprintf: Consolidate handling of unknown pointer specifiers
On Wed, 2018-04-11 at 11:52 +0200, Petr Mladek wrote: > On Tue 2018-04-10 14:41:55, Andy Shevchenko wrote: > > On Mon, 2018-04-09 at 15:50 +0200, Petr Mladek wrote: > > > On Sat 2018-04-07 17:26:40, Andy Shevchenko wrote: > > I think about 1-7 and 9 that can go as is before your changes. > > And patch 8 postpone > > Done. All the 8 patches are in printk.git, branch > for-4.18-vsprintf-cleanup. Thanks! > I am sorry that we have missed 4.17. In theory, it still might be > possible to push them there. But there does not seem to be a real > reason to rush them. Agree. -- Andy Shevchenko Intel Finland Oy
Re: [PATCH v3 06/12] KVM: arm/arm64: Helper to register a new redistributor region
On Fri, Apr 13, 2018 at 10:20:52AM +0200, Eric Auger wrote:
> We introduce a new helper that creates and inserts a new redistributor
> region into the rdist region list. This helper both handles the case
> where the redistributor region size is known at registration time
> and the legacy case where it is not (eventually depending on the number
> of online vcpus). Depending on pfns, we perform all the possible checks
> that we can do:
>
> - end of memory crossing
> - incorrect alignment of the base address
> - collision with distributor region if already defined
> - collision with already registered rdist regions
> - check of the new index
>
> Rdist regions must be inserted by increasing order of indices. Indices
> must be contiguous.
>
> We also introduce vgic_v3_rdist_region_from_index() which will be used
> from the vgic kvm-device, later on.
>
> Signed-off-by: Eric Auger
> ---
> virt/kvm/arm/vgic/vgic-mmio-v3.c | 95
> +---
> virt/kvm/arm/vgic/vgic-v3.c | 29
> virt/kvm/arm/vgic/vgic.h | 14 ++
> 3 files changed, 122 insertions(+), 16 deletions(-)
>
> diff --git a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> index ce5c927..5273fb8 100644
> --- a/virt/kvm/arm/vgic/vgic-mmio-v3.c
> +++ b/virt/kvm/arm/vgic/vgic-mmio-v3.c
> @@ -680,14 +680,66 @@ static int vgic_register_all_redist_iodevs(struct kvm
> *kvm)
> return ret;
> }
>
> -int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
> +/**
> + * vgic_v3_insert_redist_region - Insert a new redistributor region
> + *
> + * Performs various checks before inserting the rdist region in the list.
> + * Those tests depend on whether the size of the rdist region is known
> + * (ie. count != 0). The list is sorted by rdist region index.
> + *
> + * @kvm: kvm handle
> + * @index: redist region index
> + * @base: base of the new rdist region
> + * @count: number of redistributors the region is made of (of 0 in the old
> style
> + * single region, whose size is induced from the number of vcpus)
> + *
> + * Return 0 on success, < 0 otherwise
> + */
> +static int vgic_v3_insert_redist_region(struct kvm *kvm, uint32_t index,
> + gpa_t base, uint32_t count)
> {
> - struct vgic_dist *vgic = &kvm->arch.vgic;
> + struct vgic_dist *d = &kvm->arch.vgic;
> struct vgic_redist_region *rdreg;
> + struct list_head *rd_regions = &d->rd_regions;
> + struct list_head *last = rd_regions->prev;
> +
nit: extra blank line?
> + gpa_t new_start, new_end;
> + size_t size = count * KVM_VGIC_V3_REDIST_SIZE;
> int ret;
>
> - /* vgic_check_ioaddr makes sure we don't do this twice */
> - if (!list_empty(&vgic->rd_regions))
> + /* single rdist region already set ?*/
> + if (!count && !list_empty(rd_regions))
> + return -EINVAL;
> +
> + /* cross the end of memory ? */
> + if (base + size < base)
> + return -EINVAL;
what is the size of memory? This seems to check for a gpa_t overflow,
but not againt the IPA space of the VM...
> +
> + if (list_empty(rd_regions)) {
> + if (index != 0)
> + return -EINVAL;
note, I think this can be simplified if we can rid of the index.
> + } else {
> + rdreg = list_entry(last, struct vgic_redist_region, list);
you can use list_last_entry here and get rid of the 'last' temporary
variable above.
> + if (index != rdreg->index + 1)
> + return -EINVAL;
> +
> + /* Cannot add an explicitly sized regions after legacy region */
> + if (!rdreg->count)
> + return -EINVAL;
> + }
> +
> + /*
> + * collision with already set dist region ?
> + * this assumes we know the size of the new rdist region (pfns != 0)
> + * otherwise we can only test this when all vcpus are registered
> + */
I don't really understand this commentary... :(
> + if (!count && !IS_VGIC_ADDR_UNDEF(d->vgic_dist_base) &&
> + (!(d->vgic_dist_base + KVM_VGIC_V3_DIST_SIZE <= base)) &&
> + (!(base + size <= d->vgic_dist_base)))
> + return -EINVAL;
Can't you call vgic_v3_check_base() here instead?
> +
> + /* collision with any other rdist region? */
> + if (vgic_v3_rdist_overlap(kvm, base, size))
> return -EINVAL;
>
> rdreg = kzalloc(sizeof(*rdreg), GFP_KERNEL);
> @@ -696,17 +748,32 @@ int vgic_v3_set_redist_base(struct kvm *kvm, u64 addr)
>
> rdreg->base = VGIC_ADDR_UNDEF;
>
> - ret = vgic_check_ioaddr(kvm, &rdreg->base, addr, SZ_64K);
> + ret = vgic_check_ioaddr(kvm, &rdreg->base, base, SZ_64K);
> if (ret)
> - goto out;
> + goto free;
>
> - rdreg->base = addr;
> - if (!vgic_v3_check_base(kvm)) {
> - ret = -EINVAL;
> - goto out;
> - }
> + rdreg->base = base;
> +
Re: vmalloc with GFP_NOFS
On Tue, 24 Apr 2018, Michal Hocko wrote:
> Hi,
> it seems that we still have few vmalloc users who perform GFP_NOFS
> allocation:
> drivers/mtd/ubi/io.c
> fs/ext4/xattr.c
> fs/gfs2/dir.c
> fs/gfs2/quota.c
> fs/nfs/blocklayout/extent_tree.c
> fs/ubifs/debug.c
> fs/ubifs/lprops.c
> fs/ubifs/lpt_commit.c
> fs/ubifs/orphan.c
>
> Unfortunatelly vmalloc doesn't suppoer GFP_NOFS semantinc properly
> because we do have hardocded GFP_KERNEL allocations deep inside the
> vmalloc layers. That means that if GFP_NOFS really protects from
> recursion into the fs deadlocks then the vmalloc call is broken.
>
> What to do about this? Well, there are two things. Firstly, it would be
> really great to double check whether the GFP_NOFS is really needed. I
> cannot judge that because I am not familiar with the code. It would be
> great if the respective maintainers (hopefully get_maintainer.sh pointed
> me to all relevant ones). If there is not reclaim recursion issue then
> simply use the standard vmalloc (aka GFP_KERNEL request).
>
> If the use is really valid then we have a way to do the vmalloc
> allocation properly. We have memalloc_nofs_{save,restore} scope api. How
> does that work? You simply call memalloc_nofs_save when the reclaim
> recursion critical section starts (e.g. when you take a lock which is
> then used in the reclaim path - e.g. shrinker) and memalloc_nofs_restore
> when the critical section ends. _All_ allocations within that scope
> will get GFP_NOFS semantic automagically. If you are not sure about the
> scope itself then the easiest workaround is to wrap the vmalloc itself
> with a big fat comment that this should be revisited.
>
> Does that sound like something that can be done in a reasonable time?
> I have tried to bring this up in the past but our speed is glacial and
> there are attempts to do hacks like checking for abusers inside the
> vmalloc which is just too ugly to live.
>
> Please do not hesitate to get back to me if something is not clear.
>
> Thanks!
> --
> Michal Hocko
> SUSE Labs
I made a patch that adds memalloc_noio/fs_save around these calls a year
ago: http://lkml.iu.edu/hypermail/linux/kernel/1707.0/01376.html
Mikulas
Re: [PATCH v3 02/12] KVM: arm/arm64: Document KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION
On Fri, Apr 13, 2018 at 10:20:48AM +0200, Eric Auger wrote: > We introduce a new KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attribute in > KVM_DEV_ARM_VGIC_GRP_ADDR group. It allows userspace to provide the > base address and size of a redistributor region > > Compared to KVM_VGIC_V3_ADDR_TYPE_REDIST, this new attribute allows > to declare several separate redistributor regions. > > So the whole redist space does not need to be contiguous anymore. > > Signed-off-by: Eric Auger > --- > Documentation/virtual/kvm/devices/arm-vgic-v3.txt | 25 > ++- > 1 file changed, 24 insertions(+), 1 deletion(-) > > diff --git a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > index 9293b45..cbc4328 100644 > --- a/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > +++ b/Documentation/virtual/kvm/devices/arm-vgic-v3.txt > @@ -27,9 +27,32 @@ Groups: >VCPU and all of the redistributor pages are contiguous. >Only valid for KVM_DEV_TYPE_ARM_VGIC_V3. >This address needs to be 64K aligned. > + > +KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION (rw, 64-bit) > + The attr field of kvm_device_attr encodes 3 values: > + bits: | 63 52 | 51 16 | 15 - 12 |11 - 0 > + values: | count | base | flags | index > + - index encodes the unique redistributor region index I'm not entirely sure I understand the purpose of the index field. Isn't a redistributor region identified uniquely by its base address? Otherwise this looks good. Thanks, -Christoffer > + - flags: reserved for future use, currently 0 > + - base field encodes bits [51:16] of the guest physical base address > +of the first redistributor in the region. > + - count encodes the number of redistributors in the region. Must be > +greater than 0. > + There are two 64K pages for each redistributor in the region and > + redistributors are laid out contiguously within the region. Regions > + are filled with redistributors in the index order. The sum of all > + region count fields must be greater than or equal to the number of > + VCPUs. Redistributor regions must be registered in the incremental > + index order, starting from index 0. > + Only valid for KVM_DEV_TYPE_ARM_VGIC_V3. > + > + It is invalid to mix calls with KVM_VGIC_V3_ADDR_TYPE_REDIST and > + KVM_VGIC_V3_ADDR_TYPE_REDIST_REGION attributes. > + >Errors: > -E2BIG: Address outside of addressable IPA range > --EINVAL: Incorrectly aligned address > +-EINVAL: Incorrectly aligned address, bad redistributor region > + count/index, mixed redistributor region attribute usage > -EEXIST: Address already configured > -ENXIO: The group or attribute is unknown/unsupported for this device > or hardware support is missing. > -- > 2.5.5 >
[PATCH v6] fs: dax: Adding new return type vm_fault_t
Use new return type vm_fault_t for fault handler. For
now, this is just documenting that the function returns
a VM_FAULT value rather than an errno. Once all instances
are converted, vm_fault_t will become a distinct type.
commit 1c8f422059ae ("mm: change return type to vm_fault_t")
There was an existing bug inside dax_load_hole()
if vm_insert_mixed had failed to allocate a page table,
we'd return VM_FAULT_NOPAGE instead of VM_FAULT_OOM.
With new vmf_insert_mixed() this issue is addressed.
vm_insert_mixed_mkwrite has inefficiency when it returns
an error value, driver has to convert it to vm_fault_t
type. With new vmf_insert_mixed_mkwrite() this limitation
will be addressed.
Signed-off-by: Souptick Joarder
Reviewed-by: Jan Kara
Reviewed-by: Matthew Wilcox
---
v2: vm_insert_mixed_mkwrite is replaced by new
vmf_insert_mixed_mkwrite
v3: Addressed Matthew's comment. One patch which
changes both at the same time. The history
should be bisectable so that it compiles and
works at every point.
v4: Updated the change log
v5: Updated the change log
v6: Added comment in source file
fs/dax.c| 78 +
include/linux/dax.h | 4 +--
include/linux/mm.h | 4 +--
mm/memory.c | 21 ---
4 files changed, 58 insertions(+), 49 deletions(-)
diff --git a/fs/dax.c b/fs/dax.c
index aaec72de..821986c 100644
--- a/fs/dax.c
+++ b/fs/dax.c
@@ -905,12 +905,12 @@ static int dax_iomap_pfn(struct iomap *iomap, loff_t pos,
size_t size,
* If this page is ever written to we will re-fault and change the mapping to
* point to real DAX storage instead.
*/
-static int dax_load_hole(struct address_space *mapping, void *entry,
+static vm_fault_t dax_load_hole(struct address_space *mapping, void *entry,
struct vm_fault *vmf)
{
struct inode *inode = mapping->host;
unsigned long vaddr = vmf->address;
- int ret = VM_FAULT_NOPAGE;
+ vm_fault_t ret = VM_FAULT_NOPAGE;
struct page *zero_page;
void *entry2;
pfn_t pfn;
@@ -929,7 +929,7 @@ static int dax_load_hole(struct address_space *mapping,
void *entry,
goto out;
}
- vm_insert_mixed(vmf->vma, vaddr, pfn);
+ ret = vmf_insert_mixed(vmf->vma, vaddr, pfn);
out:
trace_dax_load_hole(inode, vmf, ret);
return ret;
@@ -1112,7 +1112,7 @@ int __dax_zero_page_range(struct block_device *bdev,
}
EXPORT_SYMBOL_GPL(dax_iomap_rw);
-static int dax_fault_return(int error)
+static vm_fault_t dax_fault_return(int error)
{
if (error == 0)
return VM_FAULT_NOPAGE;
@@ -1132,7 +1132,7 @@ static bool dax_fault_is_synchronous(unsigned long flags,
&& (iomap->flags & IOMAP_F_DIRTY);
}
-static int dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
+static vm_fault_t dax_iomap_pte_fault(struct vm_fault *vmf, pfn_t *pfnp,
int *iomap_errp, const struct iomap_ops *ops)
{
struct vm_area_struct *vma = vmf->vma;
@@ -1145,18 +1145,18 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
pfn_t *pfnp,
int error, major = 0;
bool write = vmf->flags & FAULT_FLAG_WRITE;
bool sync;
- int vmf_ret = 0;
+ vm_fault_t ret = 0;
void *entry;
pfn_t pfn;
- trace_dax_pte_fault(inode, vmf, vmf_ret);
+ trace_dax_pte_fault(inode, vmf, ret);
/*
* Check whether offset isn't beyond end of file now. Caller is supposed
* to hold locks serializing us with truncate / punch hole so this is
* a reliable test.
*/
if (pos >= i_size_read(inode)) {
- vmf_ret = VM_FAULT_SIGBUS;
+ ret = VM_FAULT_SIGBUS;
goto out;
}
@@ -1165,7 +1165,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
pfn_t *pfnp,
entry = grab_mapping_entry(mapping, vmf->pgoff, 0);
if (IS_ERR(entry)) {
- vmf_ret = dax_fault_return(PTR_ERR(entry));
+ ret = dax_fault_return(PTR_ERR(entry));
goto out;
}
@@ -1176,7 +1176,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
pfn_t *pfnp,
* retried.
*/
if (pmd_trans_huge(*vmf->pmd) || pmd_devmap(*vmf->pmd)) {
- vmf_ret = VM_FAULT_NOPAGE;
+ ret = VM_FAULT_NOPAGE;
goto unlock_entry;
}
@@ -1189,7 +1189,7 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
pfn_t *pfnp,
if (iomap_errp)
*iomap_errp = error;
if (error) {
- vmf_ret = dax_fault_return(error);
+ ret = dax_fault_return(error);
goto unlock_entry;
}
if (WARN_ON_ONCE(iomap.offset + iomap.length < pos + PAGE_SIZE)) {
@@ -1219,9 +1219,9 @@ static int dax_iomap_pte_fault(struct vm_fault *vmf,
pfn_t *pfnp,
goto error_finish_ioma
Re: [PATCH 1/3] signals: Allow generation of SIGKILL to exiting task.
Andrey Grodzovsky writes:
> Currently calling wait_event_killable as part of exiting process
> will stall forever since SIGKILL generation is suppresed by PF_EXITING.
>
> In our partilaur case AMDGPU driver wants to flush all GPU jobs in
> flight before shutting down. But if some job hangs the pipe we still want to
> be able to kill it and avoid a process in D state.
I should clarify. This absolutely can not be done.
PF_EXITING is set just before a task starts tearing down it's signal
handling.
So delivering any signal, or otherwise depending on signal handling
after PF_EXITING is set can not be done. That abstraction is gone.
Eric
> Signed-off-by: Andrey Grodzovsky
> ---
> kernel/signal.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index c6e4c83..c49c706 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -886,10 +886,10 @@ static inline int wants_signal(int sig, struct
> task_struct *p)
> {
> if (sigismember(&p->blocked, sig))
> return 0;
> - if (p->flags & PF_EXITING)
> - return 0;
> if (sig == SIGKILL)
> return 1;
> + if (p->flags & PF_EXITING)
> + return 0;
> if (task_is_stopped_or_traced(p))
> return 0;
> return task_curr(p) || !signal_pending(p);
Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process.
On 04/24/2018 12:23 PM, Eric W. Biederman wrote: Andrey Grodzovsky writes: Avoid calling wait_event_killable when you are possibly being called from get_signal routine since in that case you end up in a deadlock where you are alreay blocked in singla processing any trying to wait on a new signal. I am curious what the call path that is problematic here. Here is the problematic call stack [<0>] drm_sched_entity_fini+0x10a/0x3a0 [gpu_sched] [<0>] amdgpu_ctx_do_release+0x129/0x170 [amdgpu] [<0>] amdgpu_ctx_mgr_fini+0xd5/0xe0 [amdgpu] [<0>] amdgpu_driver_postclose_kms+0xcd/0x440 [amdgpu] [<0>] drm_release+0x414/0x5b0 [drm] [<0>] __fput+0x176/0x350 [<0>] task_work_run+0xa1/0xc0 [<0>] do_exit+0x48f/0x1280 [<0>] do_group_exit+0x89/0x140 [<0>] get_signal+0x375/0x8f0 [<0>] do_signal+0x79/0xaa0 [<0>] exit_to_usermode_loop+0x83/0xd0 [<0>] do_syscall_64+0x244/0x270 [<0>] entry_SYSCALL_64_after_hwframe+0x3d/0xa2 [<0>] 0x On exit from system call you process all the signals you received and encounter a fatal signal which triggers process termination. In general waiting seems wrong when the process has already been fatally killed as indicated by PF_SIGNALED. So indeed this patch avoids wait in this case. Returning -ERESTARTSYS seems wrong as nothing should make it back even to the edge of userspace here. Can you clarify please - what should be returned here instead ? Andrey Given that this is the only use of PF_SIGNALED outside of bsd process accounting I find this code very suspicious. It looks the code path that gets called during exit is buggy and needs to be sorted out. Eric Signed-off-by: Andrey Grodzovsky --- drivers/gpu/drm/scheduler/gpu_scheduler.c | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c b/drivers/gpu/drm/scheduler/gpu_scheduler.c index 088ff2b..09fd258 100644 --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c @@ -227,9 +227,10 @@ void drm_sched_entity_do_release(struct drm_gpu_scheduler *sched, return; /** * The client will not queue more IBs during this fini, consume existing -* queued IBs or discard them on SIGKILL +* queued IBs or discard them when in death signal state since +* wait_event_killable can't receive signals in that state. */ - if ((current->flags & PF_SIGNALED) && current->exit_code == SIGKILL) + if (current->flags & PF_SIGNALED) entity->fini_status = -ERESTARTSYS; else entity->fini_status = wait_event_killable(sched->job_scheduled,
Re: [PATCH 4/6] drm/panel: simple: Add support for Banana Pi 7" S070WV20-CT16 panel
On Thu, Apr 19, 2018 at 05:32:23PM +0800, Chen-Yu Tsai wrote: > This panel is marketed as Banana Pi 7" LCD display. On the back is > a sticker denoting the model name S070WV20-CT16. > > This is a 7" 800x480 panel connected through a 24-bit RGB interface. > However the panel only does 262k colors. > > Signed-off-by: Chen-Yu Tsai > --- > .../display/panel/bananapi,s070wv20-ct16.txt | 7 ++ > drivers/gpu/drm/panel/panel-simple.c | 25 +++ > 2 files changed, 32 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt > > diff --git > a/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt > b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt > new file mode 100644 > index ..2ec35ce36e9a > --- /dev/null > +++ > b/Documentation/devicetree/bindings/display/panel/bananapi,s070wv20-ct16.txt > @@ -0,0 +1,7 @@ > +Banana Pi 7" (S070WV20-CT16) TFT LCD Panel > + > +Required properties: > +- compatible: should be "bananapi,s070wv20-ct16" > + > +This binding is compatible with the simple-panel binding, which is specified > +in simple-panel.txt in this directory. Does this panel have a power supply (or more than 1) or backlight? I can't know that by just refering to simple-panel.txt.
Re: [patch V3 00/10] rslib: Cleanup and VLA removal
On Sun, Apr 22, 2018 at 9:23 AM, Thomas Gleixner wrote: > Kees tried to get rid of the Variable Length Arrays in the Reed-Solomon > library by replacing them with fixed length arrays on stack. Though they > are rather large and Andrew did not fall in love with that solution. > > This series addresses that in a different way by splitting the rs control > structure up, so that each invocation of rs_init() returns a new instance > in which the decoder buffers are allocated. The polynom tables which build > the base for the RS codecs are still shareable between the instances to > avoid large allocations and initializations of the same data over and over. > > The usage sites have been audited and fixed up where necessary to > accomodate the decoder change which forbids parallel decoder invocation for > a particular rs control instance to prevent buffer corruption. > > While at it the patch set tidies up the code and converts the related files > over to use SPDX license identifiers. > > Changes since V2: > >Provide a gfp aware variant of init_rs and use it in the alloc callback >of the dm/verity_fec rs_pool mempool allocations > >Picked up Reviewed/Acked-by tags and fixed the subject line for the MTD >specific patch. Excellent, thanks! > Changes since V1: > >Simplify error path in the diskonchip code and use the proper >function to free the decoder. > > As this spawns multiple subsystems it should either go through Andrews tree > or Kees can route it with his other hardening stuff. I'll create a VLA sub-tree to my KSPP stuff and start collecting this and other VLA fixes that maintainers haven't taken yet. Once build-testing finishes, I'll push it out for -next. -Kees -- Kees Cook Pixel Security
Re: [PATCH 01/10] vfio: ccw: Moving state change out of IRQ context
On 04/24/2018 11:59 AM, Cornelia Huck wrote:
On Tue, 24 Apr 2018 10:40:56 +0200
Pierre Morel wrote:
On 24/04/2018 08:54, Dong Jia Shi wrote:
* Pierre Morel [2018-04-19 16:48:04 +0200]:
[...]
@@ -94,9 +83,15 @@ static void vfio_ccw_sch_io_todo(struct work_struct *work)
static void vfio_ccw_sch_irq(struct subchannel *sch)
{
struct vfio_ccw_private *private = dev_get_drvdata(&sch->dev);
+ struct irb *irb = this_cpu_ptr(&cio_irb);
inc_irq_stat(IRQIO_CIO);
- vfio_ccw_fsm_event(private, VFIO_CCW_EVENT_INTERRUPT);
+ memcpy(&private->irb, irb, sizeof(*irb));
+
+ WARN_ON(work_pending(&private->io_work));
Hmm, why do we need this?
The current design insure that we have not two concurrent SSCH requests.
How ever I want here to track spurious interrupt.
If we implement cancel, halt or clear requests, we also may trigger (AFAIU)
a second interrupts depending on races between instructions, controller
and device.
You won't get an interrupt for a successful cancel. If you do a
halt/clear, you will make the subchannel halt/clear pending in addition
to start pending and you'll only get one interrupt (if the I/O has
progressed far enough, you won't be able to issue a hsch). The
interesting case is:
- guest does a ssch, we do a ssch on the device
- the guest does a csch before it got the interrupt for the ssch
- before we do the csch on the device, the subchannel is already status
pending with completion of the ssch
- after we issue the csch, we get a second interrupt (for the csch)
I think we should present two interrupts to the guest in that case.
Races between issuing ssch/hsch/csch and the subchannel becoming status
pending happen on real hardware as well, we're just more likely to see
them with the vfio layer in between.
AFAIU this will be the problem of the person implementing the clear
and the halt for vfio-ccw. I.e. it's a non-problem right now.
(I'm currently trying to recall what we're doing with unsolicited
interrupts. These are fun wrt deferred cc 1; I'm not sure if there are
cases where we want to present a deferred cc to the guest.)
What are 'fun wrt deferred cc 1' again? The deferred cc I understand
but wrt does not click at all.
Also, doing a second ssch before we got final state for the first one
is perfectly valid. Linux just does not do it, so I'm not sure if we
should invest too much time there.
We do not need it strongly.
+ queue_work(vfio_ccw_work_q, &private->io_work);
+ if (private->completion)
+ complete(private->completion);
}
static int vfio_ccw_sch_probe(struct subchannel *sch)
[...]
Re: [PATCH 1/1] tools: power/acpi, revert to LD = gcc
On Tue, Apr 24, 2018 at 12:43 AM, Jiri Slaby wrote:
> Commit 7ed1c1901fe5 (tools: fix cross-compile var clobbering) removed
> setting of LD to $(CROSS_COMPILE)gcc. This broke build of acpica
> (acpidump) in power/acpi:
> ld: unrecognized option '-D_LINUX'
>
> The tools pass CFLAGS to the linker (incl. -D_LINUX), so revert this
> particular change and let LD be $(CC) again. Note that the old behaviour
> was a bit different, it used $(CROSS_COMPILE)gcc which was eliminated by
> the commit 7ed1c1901fe5. We use $(CC) for that reason.
>
> Signed-off-by: Jiri Slaby
> Cc: Martin Kelly
> Cc: "Rafael J. Wysocki"
> Cc: Len Brown
> Cc: Robert Moore
> Cc: Erik Schmauss
> Cc: linux-a...@vger.kernel.org
> Cc: de...@acpica.org
> ---
> tools/power/acpi/Makefile.config | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/tools/power/acpi/Makefile.config
> b/tools/power/acpi/Makefile.config
> index 2cccbba64418..f304be71c278 100644
> --- a/tools/power/acpi/Makefile.config
> +++ b/tools/power/acpi/Makefile.config
> @@ -56,6 +56,7 @@ INSTALL_SCRIPT = ${INSTALL_PROGRAM}
> # to compile vs uClibc, that can be done here as well.
> CROSS = #/usr/i386-linux-uclibc/usr/bin/i386-uclibc-
> CROSS_COMPILE ?= $(CROSS)
> +LD = $(CC)
> HOSTCC = gcc
>
> # check if compiler option is supported
> --
> 2.16.3
>
Reviewed-by: Martin Kelly
Thanks for noticing this and sorry I missed it!
Re: [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from ring hang.
On 04/24/2018 12:14 PM, Eric W. Biederman wrote:
Andrey Grodzovsky writes:
If the ring is hanging for some reason allow to recover the waiting
by sending fatal signal.
Originally-by: David Panariti
Signed-off-by: Andrey Grodzovsky
---
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 14 ++
1 file changed, 10 insertions(+), 4 deletions(-)
diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
index eb80edf..37a36af 100644
--- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
+++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
@@ -421,10 +421,16 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
unsigned ring_id)
if (other) {
signed long r;
- r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT);
- if (r < 0) {
- DRM_ERROR("Error (%ld) waiting for fence!\n", r);
- return r;
+
+ while (true) {
+ if ((r = dma_fence_wait_timeout(other, true,
+ MAX_SCHEDULE_TIMEOUT)) >= 0)
+ return 0;
+
+ if (fatal_signal_pending(current)) {
+ DRM_ERROR("Error (%ld) waiting for fence!\n",
r);
+ return r;
+ }
It looks like if you make this code say:
if (fatal_signal_pending(current) ||
(current->flags & PF_EXITING)) {
DRM_ERROR("Error (%ld) waiting for fence!\n",
r);
return r;
}
}
Than you would not need the horrible hack want_signal to deliver signals
to processes who have passed exit_signal() and don't expect to need
their signal handling mechanisms anymore.
Let me clarify, the change in want_signal wasn't addressing this code
but hang in
drm_sched_entity_do_release->wait_event_killable, when you try to
gracefully terminate by waiting
for all job completions on the GPU pipe you process is using.
Andrey
Eric
Re: [PATCH 2/8] dt-bindings: display: bridge: thc63lvd1024: Add lvds map property
On Thu, Apr 19, 2018 at 11:31:03AM +0200, Jacopo Mondi wrote: > The THC63LVD1024 LVDS to RGB bridge supports two different input mapping > modes, selectable by means of an external pin. > > Describe the LVDS mode map through a newly defined mandatory property in > device tree bindings. > > Signed-off-by: Jacopo Mondi > --- > .../devicetree/bindings/display/bridge/thine,thc63lvd1024.txt | 3 > +++ > 1 file changed, 3 insertions(+) +1 for what Laurent said. Reviewed-by: Rob Herring
Re: [PATCH] net: phy: allow scanning busses with missing phys
On 04/24/2018 09:09 AM, Alexandre Belloni wrote:
> Some MDIO busses will error out when trying to read a phy address with no
> phy present at that address. In that case, probing the bus will fail
> because __mdiobus_register() is scanning the bus for all possible phys
> addresses.
>
> In case MII_PHYSID1 returns -EIO or -ENODEV, consider there is no phy at
> this address and set the phy ID to 0x which is then properly
> handled in get_phy_device().
Humm, why not have your MDIO bus implementation do the scanning itself
in a reset() callback, which happens before probing the bus, and based
on the results, set phy_mask accordingly such that only PHYs present are
populated?
My only concern with your change is that we are having a special
treatment for EIO and ENODEV, so we must make sure MDIO bus drivers are
all conforming to that.
>
> Suggested-by: Andrew Lunn
> Signed-off-by: Alexandre Belloni
> ---
> drivers/net/phy/phy_device.c | 11 ++-
> 1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
> index ac23322a32e1..9e4ba8e80a18 100644
> --- a/drivers/net/phy/phy_device.c
> +++ b/drivers/net/phy/phy_device.c
> @@ -535,8 +535,17 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32
> *phy_id,
>
> /* Grab the bits from PHYIR1, and put them in the upper half */
> phy_reg = mdiobus_read(bus, addr, MII_PHYSID1);
> - if (phy_reg < 0)
> + if (phy_reg < 0) {
> + /* if there is no device, return without an error so scanning
> + * the bus works properly
> + */
> + if (phy_reg == -EIO || phy_reg == -ENODEV) {
> + *phy_id = 0x;
> + return 0;
> + }
> +
> return -EIO;
> + }
>
> *phy_id = (phy_reg & 0x) << 16;
>
>
--
Florian
Re: [PATCH] net: sh-eth: fix sh_eth_start_xmit()'s return type
Hello! On 04/24/2018 04:17 PM, Luc Van Oostenryck wrote: > The method ndo_start_xmit() is defined as returning an 'netdev_tx_t', > which is a typedef for an enum type, but the implementation in this > driver returns an 'int'. > > Fix this by returning 'netdev_tx_t' in this driver too. > > Signed-off-by: Luc Van Oostenryck Acked-by: Sergei Shtylyov [...] > diff --git a/drivers/net/ethernet/renesas/sh_eth.c > b/drivers/net/ethernet/renesas/sh_eth.c > index b6b90a631..0875a169f 100644 > --- a/drivers/net/ethernet/renesas/sh_eth.c > +++ b/drivers/net/ethernet/renesas/sh_eth.c > @@ -2454,7 +2454,7 @@ static void sh_eth_tx_timeout(struct net_device *ndev) > } > > /* Packet transmit function */ > -static int sh_eth_start_xmit(struct sk_buff *skb, struct net_device *ndev) > +static netdev_tx_t sh_eth_start_xmit(struct sk_buff *skb, struct net_device > *ndev) But aren't you violating 80-column limit? [...] MBR, Sergei
Re: [PATCH 2/3] ASoC: AMD: Move clk enable from hw_params/free to startup/shutdown
On Mon, Apr 23, 2018 at 9:03 PM Vijendar Mukunda
wrote:
> From: Akshu Agrawal
> hw_param can be called multiple times and thus we can have
> more clk enable. The clk may not get diabled due to refcounting.
> startup/shutdown ensures single clk enable/disable call.
> Signed-off-by: Akshu Agrawal
> Signed-off-by: Vijendar Mukunda
> ---
> sound/soc/amd/acp-da7219-max98357a.c | 54
> 1 file changed, 37 insertions(+), 17 deletions(-)
> diff --git a/sound/soc/amd/acp-da7219-max98357a.c
b/sound/soc/amd/acp-da7219-max98357a.c
> index b205c78..0f16f6d 100644
> --- a/sound/soc/amd/acp-da7219-max98357a.c
> +++ b/sound/soc/amd/acp-da7219-max98357a.c
> @@ -38,8 +38,7 @@
> #include "../codecs/da7219.h"
> #include "../codecs/da7219-aad.h"
> -#define CZ_PLAT_CLK 2400
> -#define MCLK_RATE 24576000
> +#define CZ_PLAT_CLK 2500
> #define DUAL_CHANNEL 2
> static struct snd_soc_jack cz_jack;
> @@ -62,7 +61,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime
*rtd)
> }
> ret = snd_soc_dai_set_pll(codec_dai, 0, DA7219_SYSCLK_PLL,
> - CZ_PLAT_CLK, MCLK_RATE);
> + CZ_PLAT_CLK, DA7219_PLL_FREQ_OUT_98304);
These are unrelated fixes that should be in their own patch.
> if (ret < 0) {
> dev_err(rtd->dev, "can't set codec pll: %d\n", ret);
> return ret;
> @@ -85,8 +84,7 @@ static int cz_da7219_init(struct snd_soc_pcm_runtime
*rtd)
> return 0;
> }
> -static int cz_da7219_hw_params(struct snd_pcm_substream *substream,
> -struct snd_pcm_hw_params *params)
> +static int da7219_clk_enable(struct snd_pcm_substream *substream)
> {
> int ret = 0;
> struct snd_soc_pcm_runtime *rtd = substream->private_data;
> @@ -100,11 +98,9 @@ static int cz_da7219_hw_params(struct
snd_pcm_substream *substream,
> return ret;
> }
> -static int cz_da7219_hw_free(struct snd_pcm_substream *substream)
> +static void da7219_clk_disable(void)
> {
> clk_disable_unprepare(da7219_dai_clk);
> -
> - return 0;
> }
> static const unsigned int channels[] = {
> @@ -127,7 +123,7 @@ static const struct snd_pcm_hw_constraint_list
constraints_channels = {
> .mask = 0,
> };
> -static int cz_fe_startup(struct snd_pcm_substream *substream)
> +static int cz_da7219_startup(struct snd_pcm_substream *substream)
> {
> struct snd_pcm_runtime *runtime = substream->runtime;
> @@ -141,23 +137,47 @@ static int cz_fe_startup(struct snd_pcm_substream
*substream)
> snd_pcm_hw_constraint_list(runtime, 0, SNDRV_PCM_HW_PARAM_RATE,
> &constraints_rates);
> - return 0;
> + return da7219_clk_enable(substream);
> +}
> +
> +static void cz_da7219_shutdown(struct snd_pcm_substream *substream)
> +{
> + da7219_clk_disable();
> +}
> +
> +static int cz_max_startup(struct snd_pcm_substream *substream)
> +{
> + return da7219_clk_enable(substream);
> +}
> +
> +static void cz_max_shutdown(struct snd_pcm_substream *substream)
> +{
> + da7219_clk_disable();
> +}
> +
> +static int cz_dmic_startup(struct snd_pcm_substream *substream)
> +{
> + return da7219_clk_enable(substream);
> +}
> +
> +static void cz_dmic_shutdown(struct snd_pcm_substream *substream)
> +{
> + da7219_clk_disable();
> }
This is ok, or you could combine the common cz_max_* & cz_dmic_*.
> static struct snd_soc_ops cz_da7219_cap_ops = {
I think these should all be "static const struct snd_soc_ops" (please fix
in a separate patch).
> - .hw_params = cz_da7219_hw_params,
> - .hw_free = cz_da7219_hw_free,
> - .startup = cz_fe_startup,
> + .startup = cz_da7219_startup,
> + .shutdown = cz_da7219_shutdown,
> };
> static struct snd_soc_ops cz_max_play_ops = {
> - .hw_params = cz_da7219_hw_params,
> - .hw_free = cz_da7219_hw_free,
> + .startup = cz_max_startup,
> + .shutdown = cz_max_shutdown,
> };
> static struct snd_soc_ops cz_dmic_cap_ops = {
> - .hw_params = cz_da7219_hw_params,
> - .hw_free = cz_da7219_hw_free,
> + .startup = cz_dmic_startup,
> + .shutdown = cz_dmic_shutdown,
> };
> static struct snd_soc_dai_link cz_dai_7219_98357[] = {
> --
> 2.7.4
Re: [PATCH] net: phy: TLK10X initial driver submission
On Thu, Apr 19, 2018 at 10:28:16AM +0200, Måns Andersson wrote:
> From: Mans Andersson
>
> Add suport for the TI TLK105 and TLK106 10/100Mbit ethernet phys.
>
> In addition the TLK10X needs to be removed from DP83848 driver as the
> power back off support is added here for this device.
>
> Datasheet:
> http://www.ti.com/lit/gpn/tlk106
> ---
> .../devicetree/bindings/net/ti,tlk10x.txt | 27 +++
Please split bindings to a separate patch.
> drivers/net/phy/Kconfig| 5 +
> drivers/net/phy/Makefile | 1 +
> drivers/net/phy/dp83848.c | 3 -
> drivers/net/phy/tlk10x.c | 209
> +
> 5 files changed, 242 insertions(+), 3 deletions(-)
> create mode 100644 Documentation/devicetree/bindings/net/ti,tlk10x.txt
> create mode 100644 drivers/net/phy/tlk10x.c
>
> diff --git a/Documentation/devicetree/bindings/net/ti,tlk10x.txt
> b/Documentation/devicetree/bindings/net/ti,tlk10x.txt
> new file mode 100644
> index 000..371d0d7
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/net/ti,tlk10x.txt
> @@ -0,0 +1,27 @@
> +* Texas Instruments - TLK105 / TLK106 ethernet PHYs
> +
> +Required properties:
> + - reg - The ID number for the phy, usually a small integer
Isn't this the MDIO bus address?
This should have a compatible string too.
> +
> +Optional properties:
> + - ti,power-back-off - Power Back Off Level
> + Please refer to data sheet chapter 8.6 and TI Application
> + Note SLLA3228
> + 0 - Normal Operation
> + 1 - Level 1 (up to 140m cable between TLK link partners)
> + 2 - Level 2 (up to 100m cable between TLK link partners)
> + 3 - Level 3 (up to 80m cable between TLK link partners)
> +
> +Default child nodes are standard Ethernet PHY device
> +nodes as described in Documentation/devicetree/bindings/net/phy.txt
> +
> +Example:
> +
> + ethernet-phy@0 {
> + reg = <0>;
> + ti,power-back-off = <2>;
> + };
> +
> +Datasheets and documentation can be found at:
> +http://www.ti.com/lit/gpn/tlk106
> +http://www.ti.com/lit/an/slla328/slla328.pdf
Move this to before the properties.
Rob
Re: KASAN: use-after-free Read in alloc_pid
Tetsuo Handa writes:
> On 2018/04/10 23:33, Tetsuo Handa wrote:
>> syzbot wrote:
>>> syzbot has found reproducer for the following crash on upstream commit
>>> c18bb396d3d261ebbb4efbc05129c5d354c541e4 (Tue Apr 10 00:04:10 2018 +)
>>> Merge git://git.kernel.org/pub/scm/linux/kernel/git/davem/net
>>> syzbot dashboard link:
>>> https://syzkaller.appspot.com/bug?extid=7a1cff37dbbef9e7ba4c
>>>
>> While we are waiting for
>>
>> rpc_pipefs: fix double-dput()
>> rpc_pipefs: deal with early sget() failures
>> kernfs: deal with early sget() failures
>> procfs: deal with early sget() failures
>> orangefs_kill_sb(): deal with allocation failures
>> nfsd_umount(): deal with early sget() failures
>> nfs: avoid double-free on early sget() failures
>> jffs2_kill_sb(): deal with failed allocations
>> hypfs_kill_super(): deal with failed allocations
>>
>> in
>> https://git.kernel.org/pub/scm/linux/kernel/git/viro/vfs.git/log/?h=for-linus
>> ,
>> I think the patch at
>>
>> WARNING in kill_block_super
>>
>> https://syzkaller.appspot.com/bug?id=588996a25a2587be2e3a54e8646728fb9cae44e7
>>
>> is better.
>>
>
> OK. The patch was sent to linux.git as commit 8e04944f0ea8b838.
>
> #syz fix: mm,vmscan: Allow preallocating memory for
> register_shrinker().
Sigh no fixes tag on the commit you sent to Linus, and no
cc'ing of stable.
Can you please update the stable folks that 9ee332d99e4d ("sget():
handle failures of register_shrinker()") is fixed with the commit you
just sent to Linus?
Thank you,
Eric
Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
On Tue, 24 Apr 2018, Michal Hocko wrote: > On Tue 24-04-18 11:30:40, Mikulas Patocka wrote: > > > > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > > > On Mon 23-04-18 20:25:15, Mikulas Patocka wrote: > > > > > > > Fixing __vmalloc code > > > > is easy and it doesn't require cooperation with maintainers. > > > > > > But it is a hack against the intention of the scope api. > > > > It is not! > > This discussion simply doesn't make much sense it seems. The scope API > is to document the scope of the reclaim recursion critical section. That > certainly is not a utility function like vmalloc. That 15-line __vmalloc bugfix doesn't prevent you (or any other kernel developer) from converting the code to the scope API. You make nonsensical excuses. Mikulas
Re: [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from ring hang.
"Panariti, David" writes:
> Andrey Grodzovsky writes:
>> Kind of dma_fence_wait_killable, except that we don't have such API
>> (maybe worth adding ?)
> Depends on how many places it would be called, or think it might be called.
> Can always factor on the 2nd time it's needed.
> Factoring, IMO, rarely hurts. The factored function can easily be visited
> using `M-.' ;->
>
> Also, if the wait could be very long, would a log message, something like
> "xxx has run for Y seconds." help?
> I personally hate hanging w/no info.
Ugh. This loop appears susceptible to loosing wake ups. There are
races between when a wake-up happens, when we clear the sleeping state,
and when we test the stat to see if we should stat awake. So yes
implementing a dma_fence_wait_killable that handles of all that
correctly sounds like an very good idea.
Eric
>> If the ring is hanging for some reason allow to recover the waiting by
>> sending fatal signal.
>>
>> Originally-by: David Panariti
>> Signed-off-by: Andrey Grodzovsky
>> ---
>> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 14 ++
>> 1 file changed, 10 insertions(+), 4 deletions(-)
>>
>> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> index eb80edf..37a36af 100644
>> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
>> @@ -421,10 +421,16 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
>> unsigned ring_id)
>>
>> if (other) {
>> signed long r;
>> - r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT);
>> - if (r < 0) {
>> - DRM_ERROR("Error (%ld) waiting for fence!\n", r);
>> - return r;
>> +
>> + while (true) {
>> + if ((r = dma_fence_wait_timeout(other, true,
>> + MAX_SCHEDULE_TIMEOUT)) >= 0)
>> + return 0;
>> +
>> + if (fatal_signal_pending(current)) {
>> + DRM_ERROR("Error (%ld) waiting for fence!\n",
>> r);
>> + return r;
>> + }
>> }
>> }
>>
>> --
>> 2.7.4
>>
Eric
[PATCH 2/3] powerpc/nohash: remove _PAGE_BUSY
_PAGE_BUSY is always 0, remove it Signed-off-by: Christophe Leroy --- arch/powerpc/include/asm/nohash/64/pgtable.h | 10 +++--- arch/powerpc/include/asm/nohash/pte-book3e.h | 5 - 2 files changed, 3 insertions(+), 12 deletions(-) diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h b/arch/powerpc/include/asm/nohash/64/pgtable.h index 251d74c9013e..e8de7cb4d3fb 100644 --- a/arch/powerpc/include/asm/nohash/64/pgtable.h +++ b/arch/powerpc/include/asm/nohash/64/pgtable.h @@ -186,14 +186,12 @@ static inline unsigned long pte_update(struct mm_struct *mm, __asm__ __volatile__( "1: ldarx %0,0,%3 # pte_update\n\ - andi. %1,%0,%6\n\ - bne-1b \n\ andc%1,%0,%4 \n\ - or %1,%1,%7\n\ + or %1,%1,%6\n\ stdcx. %1,0,%3 \n\ bne-1b" : "=&r" (old), "=&r" (tmp), "=m" (*ptep) - : "r" (ptep), "r" (clr), "m" (*ptep), "i" (_PAGE_BUSY), "r" (set) + : "r" (ptep), "r" (clr), "m" (*ptep), "r" (set) : "cc" ); #else unsigned long old = pte_val(*ptep); @@ -290,13 +288,11 @@ static inline void __ptep_set_access_flags(struct mm_struct *mm, __asm__ __volatile__( "1: ldarx %0,0,%4\n\ - andi. %1,%0,%6\n\ - bne-1b \n\ or %0,%3,%0\n\ stdcx. %0,0,%4\n\ bne-1b" :"=&r" (old), "=&r" (tmp), "=m" (*ptep) - :"r" (bits), "r" (ptep), "m" (*ptep), "i" (_PAGE_BUSY) + :"r" (bits), "r" (ptep), "m" (*ptep) :"cc"); #else unsigned long old = pte_val(*ptep); diff --git a/arch/powerpc/include/asm/nohash/pte-book3e.h b/arch/powerpc/include/asm/nohash/pte-book3e.h index 9ff51b4c0cac..12730b81cd98 100644 --- a/arch/powerpc/include/asm/nohash/pte-book3e.h +++ b/arch/powerpc/include/asm/nohash/pte-book3e.h @@ -57,13 +57,8 @@ #define _PAGE_USER (_PAGE_BAP_UR | _PAGE_BAP_SR) /* Can be read */ #define _PAGE_PRIVILEGED (_PAGE_BAP_SR) -#define _PAGE_BUSY 0 - #define _PAGE_SPECIAL _PAGE_SW0 -/* Flags to be preserved on PTE modifications */ -#define _PAGE_HPTEFLAGS_PAGE_BUSY - /* Base page size */ #ifdef CONFIG_PPC_64K_PAGES #define _PAGE_PSIZE_PAGE_PSIZE_64K -- 2.13.3
[PATCH 3/3] powerpc/nohash: use IS_ENABLED() to simplify __set_pte_at()
By using IS_ENABLED() we can simplify __set_pte_at() by removing
redundant *ptep = pte
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/nohash/pgtable.h | 23 ---
1 file changed, 8 insertions(+), 15 deletions(-)
diff --git a/arch/powerpc/include/asm/nohash/pgtable.h
b/arch/powerpc/include/asm/nohash/pgtable.h
index f2fe3cbe90af..077472640b35 100644
--- a/arch/powerpc/include/asm/nohash/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/pgtable.h
@@ -148,40 +148,33 @@ extern void set_pte_at(struct mm_struct *mm, unsigned
long addr, pte_t *ptep,
static inline void __set_pte_at(struct mm_struct *mm, unsigned long addr,
pte_t *ptep, pte_t pte, int percpu)
{
-#if defined(CONFIG_PPC32) && defined(CONFIG_PTE_64BIT)
/* Second case is 32-bit with 64-bit PTE. In this case, we
* can just store as long as we do the two halves in the right order
* with a barrier in between.
* In the percpu case, we also fallback to the simple update
*/
- if (percpu) {
- *ptep = pte;
+ if (IS_ENABLED(CONFIG_PPC32) && IS_ENABLED(CONFIG_PTE_64BIT) &&
!percpu) {
+ __asm__ __volatile__("\
+ stw%U0%X0 %2,%0\n\
+ eieio\n\
+ stw%U0%X0 %L2,%1"
+ : "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
+ : "r" (pte) : "memory");
return;
}
- __asm__ __volatile__("\
- stw%U0%X0 %2,%0\n\
- eieio\n\
- stw%U0%X0 %L2,%1"
- : "=m" (*ptep), "=m" (*((unsigned char *)ptep+4))
- : "r" (pte) : "memory");
-
-#else
/* Anything else just stores the PTE normally. That covers all 64-bit
* cases, and 32-bit non-hash with 32-bit PTEs.
*/
*ptep = pte;
-#ifdef CONFIG_PPC_BOOK3E_64
/*
* With hardware tablewalk, a sync is needed to ensure that
* subsequent accesses see the PTE we just wrote. Unlike userspace
* mappings, we can't tolerate spurious faults, so make sure
* the new PTE will be seen the first time.
*/
- if (is_kernel_addr(addr))
+ if (IS_ENABLED(CONFIG_PPC_BOOK3E_64) && is_kernel_addr(addr))
mb();
-#endif
-#endif
}
--
2.13.3
[PATCH 1/3] powerpc/nohash: remove hash related code from nohash headers.
When nohash and book3s header were split, some hash related stuff
remained in the nohash header. This patch removes them.
Signed-off-by: Christophe Leroy
---
arch/powerpc/include/asm/nohash/32/pgtable.h | 29 +++--
arch/powerpc/include/asm/nohash/64/pgtable.h | 16 ++--
arch/powerpc/include/asm/nohash/pgtable.h| 38 +++-
arch/powerpc/include/asm/nohash/pte-book3e.h | 1 -
4 files changed, 10 insertions(+), 74 deletions(-)
diff --git a/arch/powerpc/include/asm/nohash/32/pgtable.h
b/arch/powerpc/include/asm/nohash/32/pgtable.h
index a717b5c39b9c..b413abcd5a09 100644
--- a/arch/powerpc/include/asm/nohash/32/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/32/pgtable.h
@@ -129,7 +129,7 @@ extern int icache_44x_need_flush;
#ifndef __ASSEMBLY__
#define pte_clear(mm, addr, ptep) \
- do { pte_update(ptep, ~_PAGE_HASHPTE, 0); } while (0)
+ do { pte_update(ptep, ~0, 0); } while (0)
#define pmd_none(pmd) (!pmd_val(pmd))
#definepmd_bad(pmd)(pmd_val(pmd) & _PMD_BAD)
@@ -142,21 +142,6 @@ static inline void pmd_clear(pmd_t *pmdp)
/*
- * When flushing the tlb entry for a page, we also need to flush the hash
- * table entry. flush_hash_pages is assembler (for speed) in hashtable.S.
- */
-extern int flush_hash_pages(unsigned context, unsigned long va,
- unsigned long pmdval, int count);
-
-/* Add an HPTE to the hash table */
-extern void add_hash_page(unsigned context, unsigned long va,
- unsigned long pmdval);
-
-/* Flush an entry from the TLB/hash table */
-extern void flush_hash_entry(struct mm_struct *mm, pte_t *ptep,
-unsigned long address);
-
-/*
* PTE updates. This function is called whenever an existing
* valid PTE is updated. This does -not- include set_pte_at()
* which nowadays only sets a new PTE.
@@ -242,12 +227,6 @@ static inline int __ptep_test_and_clear_young(unsigned int
context, unsigned lon
{
unsigned long old;
old = pte_update(ptep, _PAGE_ACCESSED, 0);
-#if _PAGE_HASHPTE != 0
- if (old & _PAGE_HASHPTE) {
- unsigned long ptephys = __pa(ptep) & PAGE_MASK;
- flush_hash_pages(context, addr, ptephys, 1);
- }
-#endif
return (old & _PAGE_ACCESSED) != 0;
}
#define ptep_test_and_clear_young(__vma, __addr, __ptep) \
@@ -257,7 +236,7 @@ static inline int __ptep_test_and_clear_young(unsigned int
context, unsigned lon
static inline pte_t ptep_get_and_clear(struct mm_struct *mm, unsigned long
addr,
pte_t *ptep)
{
- return __pte(pte_update(ptep, ~_PAGE_HASHPTE, 0));
+ return __pte(pte_update(ptep, ~0, 0));
}
#define __HAVE_ARCH_PTEP_SET_WRPROTECT
@@ -285,7 +264,7 @@ static inline void __ptep_set_access_flags(struct mm_struct
*mm,
}
#define __HAVE_ARCH_PTE_SAME
-#define pte_same(A,B) (((pte_val(A) ^ pte_val(B)) & ~_PAGE_HASHPTE) == 0)
+#define pte_same(A,B) ((pte_val(A) ^ pte_val(B)) == 0)
/*
* Note that on Book E processors, the pmd contains the kernel virtual
@@ -326,7 +305,7 @@ static inline void __ptep_set_access_flags(struct mm_struct
*mm,
/*
* Encode and decode a swap entry.
* Note that the bits we use in a PTE for representing a swap entry
- * must not include the _PAGE_PRESENT bit or the _PAGE_HASHPTE bit (if used).
+ * must not include the _PAGE_PRESENT bit.
* -- paulus
*/
#define __swp_type(entry) ((entry).val & 0x1f)
diff --git a/arch/powerpc/include/asm/nohash/64/pgtable.h
b/arch/powerpc/include/asm/nohash/64/pgtable.h
index 5c5f75d005ad..251d74c9013e 100644
--- a/arch/powerpc/include/asm/nohash/64/pgtable.h
+++ b/arch/powerpc/include/asm/nohash/64/pgtable.h
@@ -173,8 +173,6 @@ static inline void pgd_set(pgd_t *pgdp, unsigned long val)
/* to find an entry in a kernel page-table-directory */
/* This now only contains the vmalloc pages */
#define pgd_offset_k(address) pgd_offset(&init_mm, address)
-extern void hpte_need_flush(struct mm_struct *mm, unsigned long addr,
- pte_t *ptep, unsigned long pte, int huge);
/* Atomic PTE updates */
static inline unsigned long pte_update(struct mm_struct *mm,
@@ -205,11 +203,6 @@ static inline unsigned long pte_update(struct mm_struct
*mm,
if (!huge)
assert_pte_locked(mm, addr);
-#ifdef CONFIG_PPC_BOOK3S_64
- if (old & _PAGE_HASHPTE)
- hpte_need_flush(mm, addr, ptep, old, huge);
-#endif
-
return old;
}
@@ -218,7 +211,7 @@ static inline int __ptep_test_and_clear_young(struct
mm_struct *mm,
{
unsigned long old;
- if ((pte_val(*ptep) & (_PAGE_ACCESSED | _PAGE_HASHPTE)) == 0)
+ if (pte_young(*ptep))
return 0;
old = pte_update(mm, addr, ptep, _PAGE_ACCESSED, 0, 0);
return (old & _PAGE_ACCESSED) != 0;
@@ -312,7 +305,7 @@ static inline void __ptep_set_access_flags(struct mm_struct
Re: [PATCH v2 2/2] tpm: reduce polling time to usecs for even finer granularity
On Tue, Apr 17, 2018 at 09:12:46AM -0400, Nayna Jain wrote: > The TPM burstcount and status commands are supposed to return very > quickly [1][2]. This patch further reduces the TPM poll sleep time to usecs > in get_burstcount() and wait_for_tpm_stat() by calling usleep_range() > directly. > > After this change, performance on a TPM 1.2 with an 8 byte burstcount for > 1000 extends improved from ~10.7 sec to ~7 sec. > > [1] From TCG Specification "TCG PC Client Specific TPM Interface > Specification (TIS), Family 1.2": > > "NOTE : It takes roughly 330 ns per byte transfer on LPC. 256 bytes would > take 84 us, which is a long time to stall the CPU. Chipsets may not be > designed to post this much data to LPC; therefore, the CPU itself is > stalled for much of this time. Sending 1 kB would take 350 μs. Therefore, > even if the TPM_STS_x.burstCount field is a high value, software SHOULD > be interruptible during this period." > > [2] From TCG Specification 2.0, "TCG PC Client Platform TPM Profile > (PTP) Specification": > > "It takes roughly 330 ns per byte transfer on LPC. 256 bytes would take > 84 us. Chipsets may not be designed to post this much data to LPC; > therefore, the CPU itself is stalled for much of this time. Sending 1 kB > would take 350 us. Therefore, even if the TPM_STS_x.burstCount field is a > high value, software should be interruptible during this period. For SPI, > assuming 20MHz clock and 64-byte transfers, it would take about 120 usec > to move 256B of data. Sending 1kB would take about 500 usec. If the > transactions are done using 4 bytes at a time, then it would take about > 1 msec. to transfer 1kB of data." > > Signed-off-by: Nayna Jain Great, thanks for finding those references. Kind of stuff that I will forget within months and have to revisit with git blame/log :-) Reviewed-by: Jarkko Sakkinen /Jarkko
Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
On Tue 24-04-18 10:12:42, Michal Hocko wrote: > On Tue 24-04-18 11:30:40, Mikulas Patocka wrote: > > > > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > > > On Mon 23-04-18 20:25:15, Mikulas Patocka wrote: > > > > > > > Fixing __vmalloc code > > > > is easy and it doesn't require cooperation with maintainers. > > > > > > But it is a hack against the intention of the scope api. > > > > It is not! > > This discussion simply doesn't make much sense it seems. The scope API > is to document the scope of the reclaim recursion critical section. That > certainly is not a utility function like vmalloc. http://lkml.kernel.org/r/20180424162712.gl17...@dhcp22.suse.cz let's see how it rolls this time. -- Michal Hocko SUSE Labs
Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core
On 4/24/2018 9:01 AM, Andy Shevchenko wrote: On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote: This commit adds driver implementation for PECI bus core into linux driver framework. All comments you got for patch 6 are applicable here. And perhaps in the rest of the series. The rule of thumb: when you get even single comment in a certain place, re-check _entire_ series for the same / similar patterns! Thanks for your advice. I'll keep that in mind. Jae
Re: [PATCH v3] kvmalloc: always use vmalloc if CONFIG_DEBUG_SG
On Tue 24-04-18 11:50:30, Mikulas Patocka wrote: > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > On Mon 23-04-18 20:06:16, Mikulas Patocka wrote: > > [...] > > > @@ -404,6 +405,12 @@ void *kvmalloc_node(size_t size, gfp_t f > > >*/ > > > WARN_ON_ONCE((flags & GFP_KERNEL) != GFP_KERNEL); > > > > > > +#ifdef CONFIG_DEBUG_SG > > > + /* Catch bugs when the caller uses DMA API on the result of kvmalloc. */ > > > + if (!(prandom_u32_max(2) & 1)) > > > + goto do_vmalloc; > > > +#endif > > > > I really do not think there is anything DEBUG_SG specific here. Why you > > simply do not follow should_failslab path or even reuse the function? > > CONFIG_DEBUG_SG is enabled by default in RHEL and Fedora debug kernel (if > you don't like CONFIG_DEBUG_SG, pick any other option that is enabled > there). Are you telling me that you are shaping a debugging functionality basing on what RHEL has enabled? And you call me evil. This is just rediculous. > Fail-injection framework is if off by default and it must be explicitly > enabled and configured by the user - and most users won't enable it. It can be enabled easily. And if you care enough for your debugging kernel then just make it enabled unconditionally. -- Michal Hocko SUSE Labs
Re: kernel BUG at fs/f2fs/node.c:LINE!
On 04/24, Dmitry Vyukov wrote: > On Tue, Apr 24, 2018 at 8:42 AM, syzbot > wrote: > > Hello, > > > > syzbot tried to test the proposed patch but build/boot failed: > > > > failed to create VM pool: failed to create GCE image: failed to get create > > image operation operation-1524552040215-56a926ecc71d9-3edfeb8b-8abca81c: > > googleapi: Error 403: Rate Limit Exceeded, rateLimitExceeded > > > > > > Tested on git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git/dev > > commit > > b549e322861b99f1862fb5f08dfb5e5753a38635 (Sat Apr 21 06:44:59 2018 +) > > f2fs: check cap_resource only for data blocks > > > > compiler: gcc (GCC) 8.0.1 20180413 (experimental) > > Patch: https://syzkaller.appspot.com/x/patch.diff?id=5565179303559168 > > Kernel config: > > https://syzkaller.appspot.com/x/.config?id=1808800213120130118 > > Hi Jaegeuk, > > Re the last failure (rate limiting), now should be fixed with: > https://github.com/google/syzkaller/commit/e2f4bf8f3818d49baf0f3789add75d5fd506ad8d > which adds some backoff-and-retry logic. > > Re the first one, do I understand it correctly that dev-test already > contains the fix that you want to test? > We did not foresee such case initially, but you are not the first one > to try this. I've added support for testing without a patch now: > https://github.com/google/syzkaller/commit/9366d03f001170479319878905031f63d4377c46 > and updated the docs: > https://github.com/google/syzkaller/blob/master/docs/syzbot.md#testing-patches > > So now this should work: > > #syz test: git://git.kernel.org/pub/scm/linux/kernel/git/jaegeuk/f2fs.git > dev-test Cool! Thank you so much.
Re: [PATCH v2 1/2] tpm: reduce poll sleep time in tpm_transmit()
On Tue, Apr 17, 2018 at 09:12:45AM -0400, Nayna Jain wrote: > The TPM polling code in tpm_transmit sleeps in each loop iteration for > 5 msecs. However, the TPM might return earlier, and thus waiting for > 5 msecs adds an unnecessary delay. This patch reduces the polling sleep > time in tpm_transmit() from 5 msecs to 1 msecs. I'm not sure what TPM returning earlier has to do with this. TPM probably never returns exactly in the spec defined timeout/duration. I just don't understand reasoning in this paragraph. > Additionally, this patch renames TPM_POLL_SLEEP and moves it to tpm.h as > an enum value. > > After this change, performance on a TPM 1.2 with an 8 byte burstcount > for 1000 extends improved from ~14 sec to ~10.7 sec. You cannot give absolute numbers without a context (platform, software). > Signed-off-by: Nayna Jain /Jarkko
Re: [PATCH 1/3] dt-bindings: iio: stm32-adc: add support for STM32MP1.
On Wed, Apr 18, 2018 at 05:37:52PM +0200, Fabrice Gasnier wrote: > Document support for STM32MP1 ADC. It's quite similar to STM32H7 ADC. > Introduce "st,stm32mp1-adc" compatible to handle variants of this > hardware such as vregready flag, interrupts, clock rate. > > Signed-off-by: Fabrice Gasnier > --- > Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt | 6 +- > 1 file changed, 5 insertions(+), 1 deletion(-) > > diff --git a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt > b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt > index e8bb824..9994384 100644 > --- a/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt > +++ b/Documentation/devicetree/bindings/iio/adc/st,stm32-adc.txt > @@ -24,8 +24,11 @@ Required properties: > - compatible: Should be one of: >"st,stm32f4-adc-core" >"st,stm32h7-adc-core" > + "st,stm32mp1-adc-core" > - reg: Offset and length of the ADC block register set. > -- interrupts: Must contain the interrupt for ADC block. > +- interrupts: One or more interrupts for ADC block. Some parts like stm32f4 > + and stm32h7 share a common ADC interrupt line. stm32mp1 has separate > + lines for each ADC within ADC block. How many interrupt lines is that? > - clocks: Core can use up to two clocks, depending on part used: >- "adc" clock: for the analog circuitry, common to all ADCs. > It's required on stm32f4. > @@ -53,6 +56,7 @@ Required properties: > - compatible: Should be one of: >"st,stm32f4-adc" >"st,stm32h7-adc" > + "st,stm32mp1-adc" > - reg: Offset of ADC instance in ADC block (e.g. may be 0x0, 0x100, 0x200). > - clocks: Input clock private to this ADC instance. It's required only on >stm32f4, that has per instance clock input for registers access. > -- > 1.9.1 >
vmalloc with GFP_NOFS
Hi,
it seems that we still have few vmalloc users who perform GFP_NOFS
allocation:
drivers/mtd/ubi/io.c
fs/ext4/xattr.c
fs/gfs2/dir.c
fs/gfs2/quota.c
fs/nfs/blocklayout/extent_tree.c
fs/ubifs/debug.c
fs/ubifs/lprops.c
fs/ubifs/lpt_commit.c
fs/ubifs/orphan.c
Unfortunatelly vmalloc doesn't suppoer GFP_NOFS semantinc properly
because we do have hardocded GFP_KERNEL allocations deep inside the
vmalloc layers. That means that if GFP_NOFS really protects from
recursion into the fs deadlocks then the vmalloc call is broken.
What to do about this? Well, there are two things. Firstly, it would be
really great to double check whether the GFP_NOFS is really needed. I
cannot judge that because I am not familiar with the code. It would be
great if the respective maintainers (hopefully get_maintainer.sh pointed
me to all relevant ones). If there is not reclaim recursion issue then
simply use the standard vmalloc (aka GFP_KERNEL request).
If the use is really valid then we have a way to do the vmalloc
allocation properly. We have memalloc_nofs_{save,restore} scope api. How
does that work? You simply call memalloc_nofs_save when the reclaim
recursion critical section starts (e.g. when you take a lock which is
then used in the reclaim path - e.g. shrinker) and memalloc_nofs_restore
when the critical section ends. _All_ allocations within that scope
will get GFP_NOFS semantic automagically. If you are not sure about the
scope itself then the easiest workaround is to wrap the vmalloc itself
with a big fat comment that this should be revisited.
Does that sound like something that can be done in a reasonable time?
I have tried to bring this up in the past but our speed is glacial and
there are attempts to do hacks like checking for abusers inside the
vmalloc which is just too ugly to live.
Please do not hesitate to get back to me if something is not clear.
Thanks!
--
Michal Hocko
SUSE Labs
Re: [PATCH v3 09/10] drivers/hwmon: Add PECI hwmon client drivers
Hi Andy,
Thanks a lot for your review. Please check my inline answers.
On 4/24/2018 8:56 AM, Andy Shevchenko wrote:
On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote:
drivers/hwmon/peci-cputemp.c | 783
++
drivers/hwmon/peci-dimmtemp.c | 432 +++
Does it make sense one driver per patch?
Yes, I'll separate it into two patches.
+#define CLIENT_CPU_ID_MASK0xf0ff0 /* Mask for Family / Model
info */
+struct cpu_gen_info {
+ u32 type;
+ u32 cpu_id;
+ u32 core_max;
+};
+static const struct cpu_gen_info cpu_gen_info_table[] = {
+ { .type = CPU_GEN_HSX,
+ .cpu_id = 0x306f0, /* Family code: 6, Model number: 63
(0x3f) */
+ .core_max = CORE_MAX_ON_HSX },
+ { .type = CPU_GEN_BRX,
+ .cpu_id = 0x406f0, /* Family code: 6, Model number: 79
(0x4f) */
+ .core_max = CORE_MAX_ON_BDX },
+ { .type = CPU_GEN_SKX,
+ .cpu_id = 0x50650, /* Family code: 6, Model number: 85
(0x55) */
+ .core_max = CORE_MAX_ON_SKX },
+};
Are we talking about x86 CPU IDs here?
If so, why x86 corresponding headers, including intel-family.h are not
used?
Yes, that would make more sense. I'll include the intel-family.h and
will use these defines instead:
INTEL_FAM6_HASWELL_X
INTEL_FAM6_BROADWELL_X
INTEL_FAM6_SKYLAKE_X
Thanks,
Jae
Re: [PATCH v4 1/3] dell-led: Change dell-led.h to dell-common.h
On Fri, 20 Apr 2018 11:44:30 +0200,
Kai-Heng Feng wrote:
>
> This header will be used for more than just led. Change it to a more
> generic name.
>
> Cc: Mario Limonciello
> Signed-off-by: Kai-Heng Feng
BTW, wouldn't it be better postpone the rename?
Renaming a file in a cross-tree patchset makes harder to apply in
general.
thanks,
Takashi
> ---
> v4: Change the commit message to clarify there's no more runtime pm
> warning.
> Also skip the check for thunderbolt attached devices.
>
> v3: Simplify dell_switchable_gfx_is_enabled() by returning bool instead
> of error code.
> Use DMI_DEV_TYPE_OEM_STRING to match Dell System.
>
> v2: Mario suggested to squash the HDA part into the same series.
>
> drivers/platform/x86/dell-laptop.c | 2 +-
> include/linux/{dell-led.h => dell-common.h} | 4 ++--
> sound/pci/hda/dell_wmi_helper.c | 2 +-
> 3 files changed, 4 insertions(+), 4 deletions(-)
> rename include/linux/{dell-led.h => dell-common.h} (61%)
>
> diff --git a/drivers/platform/x86/dell-laptop.c
> b/drivers/platform/x86/dell-laptop.c
> index c52c6723374b..8ba820e6c3d0 100644
> --- a/drivers/platform/x86/dell-laptop.c
> +++ b/drivers/platform/x86/dell-laptop.c
> @@ -29,7 +29,7 @@
> #include
> #include
> #include
> -#include
> +#include
> #include
> #include
> #include "dell-rbtn.h"
> diff --git a/include/linux/dell-led.h b/include/linux/dell-common.h
> similarity index 61%
> rename from include/linux/dell-led.h
> rename to include/linux/dell-common.h
> index 92521471517f..37e4b614dd74 100644
> --- a/include/linux/dell-led.h
> +++ b/include/linux/dell-common.h
> @@ -1,6 +1,6 @@
> /* SPDX-License-Identifier: GPL-2.0 */
> -#ifndef __DELL_LED_H__
> -#define __DELL_LED_H__
> +#ifndef __DELL_COMMON_H__
> +#define __DELL_COMMON_H__
>
> int dell_micmute_led_set(int on);
>
> diff --git a/sound/pci/hda/dell_wmi_helper.c b/sound/pci/hda/dell_wmi_helper.c
> index 1b48a8c19d28..56050cc3c0ee 100644
> --- a/sound/pci/hda/dell_wmi_helper.c
> +++ b/sound/pci/hda/dell_wmi_helper.c
> @@ -4,7 +4,7 @@
> */
>
> #if IS_ENABLED(CONFIG_DELL_LAPTOP)
> -#include
> +#include
>
> enum {
> MICMUTE_LED_ON,
> --
> 2.17.0
>
>
Re: Regression 4.17-rc1: SSD doesnʼt properly resume causing system hang (NULL pointer dereference)
On Tue, 2018-04-24 at 18:14 +0200, Paul Menzel wrote: > Since Linux 4.17-rc1, resume from ACPI on the Lenovo X60 fails because > of a NULL pointer dereference. As the drive doesn’t come back, messages > can be seen on the display, but the system cannot be controlled anymore, > and has to be forcefully shut down. Are you using scsi-mq or the legacy SCSI core? If you are not sure how to answer this question, please share the kernel config and kernel command line parameters. Thanks, Bart.
Re: [PATCH v5 21/23] ASoC: qdsp6: dt-bindings: Add apq8096 machine bindings
On Wed, Apr 18, 2018 at 04:31:55PM +0100, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla
>
> Add devicetree bindings documentation file for Qualcomm apq8096 sound card.
>
> Signed-off-by: Srinivas Kandagatla
> ---
> .../devicetree/bindings/sound/qcom,apq8096.txt | 76
> ++
> 1 file changed, 76 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,apq8096.txt
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
> b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
> new file mode 100644
> index ..37e23d926b95
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,apq8096.txt
> @@ -0,0 +1,76 @@
> +* Qualcomm Technologies APQ8096 ASoC sound card driver
> +
> +This binding describes the APQ8096 sound card, which uses qdsp for audio.
> +
> +- compatible:
> + Usage: required
> + Value type:
> + Definition: must be "qcom,apq8096-sndcard"
> +
> +- qcom,audio-routing:
> + Usage: Optional
> + Value type:
> + Definition: A list of the connections between audio components.
> + Each entry is a pair of strings, the first being the
> + connection's sink, the second being the connection's
> + source. Valid names could be power supplies, MicBias
> + of codec and the jacks on the board:
Please list out valid values here.
> +
> += dailinks
> +Each subnode of sndcard represents either a dailink, and subnodes of each
> +dailinks would be cpu/codec/platform dais.
> +
> +- link-name:
Not a standard property, but I guess that sneaked in with the 8016
binding...
> + Usage: required
> + Value type:
> + Definition: User friendly name for dai link
> +
> += CPU, PLATFORM, CODEC dais subnodes
> +- cpu:
> + Usage: required
> + Value type:
> + Definition: cpu dai sub-node
> +
> +- codec:
> + Usage: Optional
> + Value type:
> + Definition: codec dai sub-node
> +
> +- platform:
> + Usage: Optional
> + Value type:
> + Definition: platform dai sub-node
> +
> +- sound-dai:
> + Usage: required
> + Value type:
phandle with args.
> + Definition: dai phandle/s and port of CPU/CODEC/PLATFORM node.
> +
> +Example:
> +
> +audio {
> + compatible = "qcom,apq8096-sndcard";
> + qcom,model = "DB820c";
> +
> + mm1-dai-link {
> + link-name = "MultiMedia1";
> + cpu {
> + sound-dai = <&q6asmdai MSM_FRONTEND_DAI_MULTIMEDIA1>;
> + };
> + };
> +
> + hdmi-dai-link {
> + link-name = "HDMI Playback";
> + cpu {
> + sound-dai = <&q6afe HDMI_RX>;
> + };
> +
> + platform {
> + sound-dai = <&q6adm>;
> + };
> +
> + codec {
> + sound-dai = <&hdmi 0>;
> + };
> + };
> +};
> --
> 2.16.2
>
Re: [PATCH 2/3] drm/scheduler: Don't call wait_event_killable for signaled process.
Andrey Grodzovsky writes: > Avoid calling wait_event_killable when you are possibly being called > from get_signal routine since in that case you end up in a deadlock > where you are alreay blocked in singla processing any trying to wait > on a new signal. I am curious what the call path that is problematic here. In general waiting seems wrong when the process has already been fatally killed as indicated by PF_SIGNALED. Returning -ERESTARTSYS seems wrong as nothing should make it back even to the edge of userspace here. Given that this is the only use of PF_SIGNALED outside of bsd process accounting I find this code very suspicious. It looks the code path that gets called during exit is buggy and needs to be sorted out. Eric > Signed-off-by: Andrey Grodzovsky > --- > drivers/gpu/drm/scheduler/gpu_scheduler.c | 5 +++-- > 1 file changed, 3 insertions(+), 2 deletions(-) > > diff --git a/drivers/gpu/drm/scheduler/gpu_scheduler.c > b/drivers/gpu/drm/scheduler/gpu_scheduler.c > index 088ff2b..09fd258 100644 > --- a/drivers/gpu/drm/scheduler/gpu_scheduler.c > +++ b/drivers/gpu/drm/scheduler/gpu_scheduler.c > @@ -227,9 +227,10 @@ void drm_sched_entity_do_release(struct > drm_gpu_scheduler *sched, > return; > /** >* The client will not queue more IBs during this fini, consume existing > - * queued IBs or discard them on SIGKILL > + * queued IBs or discard them when in death signal state since > + * wait_event_killable can't receive signals in that state. > */ > - if ((current->flags & PF_SIGNALED) && current->exit_code == SIGKILL) > + if (current->flags & PF_SIGNALED) > entity->fini_status = -ERESTARTSYS; > else > entity->fini_status = wait_event_killable(sched->job_scheduled,
Re: [RESEND PATCH] staging: bcm2835-audio: Disconnect and free vchi_instance on module_exit()
On Tue, Apr 24, 2018 at 10:44 AM, Kirill Marinushkin wrote: > In the current implementation, vchi_instance is inited during the first > call of bcm2835_audio_open_connection(), and is never freed. It causes a > memory leak when the module `snd_bcm2835` is removed. > Signed-off-by: Kirill Marinushkin > Cc: Eric Anholt > Cc: Stefan Wahren > Cc: Greg Kroah-Hartman > Cc: Florian Fainelli > Cc: Ray Jui > Cc: Scott Branden > Cc: Andy Shevchenko > Cc: bcm-kernel-feedback-l...@broadcom.com > Cc: linux-rpi-ker...@lists.infradead.org > Cc: linux-arm-ker...@lists.infradead.org > Cc: de...@driverdev.osuosl.org > Cc: linux-kernel@vger.kernel.org AFAIR I gave you a tag and you again missed it. Before sending anything just check twice if all prerequisites are fulfilled. And yes, kbuild bot is right. You need to return known value. -- With Best Regards, Andy Shevchenko
Re: [PATCH v2 3/5] ALSA: xen-front: Implement Xen event channel handling
On 04/24/2018 06:02 PM, Takashi Iwai wrote:
On Tue, 24 Apr 2018 16:58:43 +0200,
Oleksandr Andrushchenko wrote:
On 04/24/2018 05:35 PM, Takashi Iwai wrote:
On Tue, 24 Apr 2018 16:29:15 +0200,
Oleksandr Andrushchenko wrote:
On 04/24/2018 05:20 PM, Takashi Iwai wrote:
On Mon, 16 Apr 2018 08:24:51 +0200,
Oleksandr Andrushchenko wrote:
+static irqreturn_t evtchnl_interrupt_req(int irq, void *dev_id)
+{
+ struct xen_snd_front_evtchnl *channel = dev_id;
+ struct xen_snd_front_info *front_info = channel->front_info;
+ struct xensnd_resp *resp;
+ RING_IDX i, rp;
+ unsigned long flags;
+
+ if (unlikely(channel->state != EVTCHNL_STATE_CONNECTED))
+ return IRQ_HANDLED;
+
+ spin_lock_irqsave(&front_info->io_lock, flags);
+
+again:
+ rp = channel->u.req.ring.sring->rsp_prod;
+ /* ensure we see queued responses up to rp */
+ rmb();
+
+ for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
I'm not familiar with Xen stuff in general, but through a quick
glance, this kind of code worries me a bit.
If channel->u.req.ring.rsp_cons has a bogus number, this may lead to a
very long loop, no? Better to have a sanity check of the ring buffer
size.
In this loop I have:
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
and the RING_GET_RESPONSE macro is designed in the way that
it wraps around when *i* in the question gets bigger than
the ring size:
#define RING_GET_REQUEST(_r, _idx) \
(&((_r)->sring->ring[((_idx) & (RING_SIZE(_r) - 1))].req))
So, even if the counter has a bogus number it will not last long
Hm, this prevents from accessing outside the ring buffer, but does it
change the loop behavior?
no, it doesn't
Suppose channel->u.req.ring_rsp_cons = 1, and rp = 0, the loop below
would still consume the whole 32bit counts, no?
for (i = channel->u.req.ring.rsp_cons; i != rp; i++) {
resp = RING_GET_RESPONSE(&channel->u.req.ring, i);
...
}
You are right here and the comment is totally valid.
I'll put an additional check like here [1] and here [2]
Will this address your comment?
Yep, this kind of sanity checks should work.
Great, will implement the checks this way then
thanks,
Takashi
Thank you,
Oleksandr
Takashi
Thank you,
Oleksandr
[1]
https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1127
[2]
https://elixir.bootlin.com/linux/v4.17-rc2/source/drivers/block/xen-blkback/blkback.c#L1135
Re: [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from ring hang.
> Kind of dma_fence_wait_killable, except that we don't have such API
> (maybe worth adding ?)
Depends on how many places it would be called, or think it might be called.
Can always factor on the 2nd time it's needed.
Factoring, IMO, rarely hurts. The factored function can easily be visited
using `M-.' ;->
Also, if the wait could be very long, would a log message, something like "xxx
has run for Y seconds." help?
I personally hate hanging w/no info.
regards,
davep
From: Grodzovsky, Andrey
Sent: Tuesday, April 24, 2018 11:58:19 AM
To: Panariti, David; linux-kernel@vger.kernel.org; amd-...@lists.freedesktop.org
Cc: Deucher, Alexander; Koenig, Christian; o...@redhat.com;
a...@linux-foundation.org; ebied...@xmission.com
Subject: Re: [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from
ring hang.
On 04/24/2018 11:52 AM, Panariti, David wrote:
> Hi,
>
> It looks like there can be an infinite loop if neither of the if()'s become
> true.
> Is that an impossible condition?
That intended, we want to wait until either the fence signals or fatal
signal received, we don't want to terminate the wait if fence is not
signaled even when interrupted by non fatal signal.
Kind of dma_fence_wait_killable, except that we don't have such API
(maybe worth adding ?)
Andrey
>
> -Original Message-
> From: Andrey Grodzovsky
> Sent: Tuesday, April 24, 2018 11:31 AM
> To: linux-kernel@vger.kernel.org; amd-...@lists.freedesktop.org
> Cc: Deucher, Alexander ; Koenig, Christian
> ; Panariti, David ;
> o...@redhat.com; a...@linux-foundation.org; ebied...@xmission.com;
> Grodzovsky, Andrey
> Subject: [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from
> ring hang.
>
> If the ring is hanging for some reason allow to recover the waiting by
> sending fatal signal.
>
> Originally-by: David Panariti
> Signed-off-by: Andrey Grodzovsky
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index eb80edf..37a36af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -421,10 +421,16 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
> unsigned ring_id)
>
> if (other) {
> signed long r;
> - r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT);
> - if (r < 0) {
> - DRM_ERROR("Error (%ld) waiting for fence!\n", r);
> - return r;
> +
> + while (true) {
> + if ((r = dma_fence_wait_timeout(other, true,
> + MAX_SCHEDULE_TIMEOUT)) >= 0)
> + return 0;
> +
> + if (fatal_signal_pending(current)) {
> + DRM_ERROR("Error (%ld) waiting for fence!\n",
> r);
> + return r;
> + }
> }
> }
>
> --
> 2.7.4
>
Re: [PATCH v5 06/23] ASoC: qdsp6: dt-bindings: Add q6asm dt bindings
Thanks for the review comments,
On 24/04/18 17:17, Rob Herring wrote:
On Wed, Apr 18, 2018 at 04:31:40PM +0100, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla
This patch add DT bindings for ASM (Audio Stream Manager) DSP module.
Signed-off-by: Srinivas Kandagatla
Reviewed-and-tested-by: Rohit kumar
---
.../devicetree/bindings/sound/qcom,q6asm.txt | 33 ++
include/dt-bindings/sound/qcom,q6asm.h | 22 +++
2 files changed, 55 insertions(+)
create mode 100644 Documentation/devicetree/bindings/sound/qcom,q6asm.txt
create mode 100644 include/dt-bindings/sound/qcom,q6asm.h
diff --git a/Documentation/devicetree/bindings/sound/qcom,q6asm.txt
b/Documentation/devicetree/bindings/sound/qcom,q6asm.txt
new file mode 100644
index ..d034a50a202a
--- /dev/null
+++ b/Documentation/devicetree/bindings/sound/qcom,q6asm.txt
@@ -0,0 +1,33 @@
+Qualcomm Audio Stream Manager (Q6ASM) binding
+
+Q6ASM is one of the APR audio service on Q6DSP.
+Please refer to qcom,apr.txt for details of the coommon apr service bindings
+used by the apr service device.
Need to be clear this is a child of APR.
Yep, I will add a line to state this explicitly.
+
+- but must contain the following property:
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: must be "qcom,asm-v.".
+ Or "qcom,asm" where the version number can be queried
+ from DSP.
+ example "qcom,asm-v2.0"
+
+= ASM DAIs (Digial Audio Interface)
+"dais" subnode of the ASM node represents dai specific configuration
+
+- #sound-dai-cells
+ Usage: required
+ Value type:
+ Definition: Must be 1
+
+= EXAMPLE
+
+q6asm {
Unit address needed.
Agree, Will fix all such instances.
+ compatible = "qcom,q6asm";
Doesn't match the doc.
Opps!! will fix all such instances with afe and asm too.
[PATCH] of: overlay: Stop leaking resources on overlay removal
Only the overlay notifier callbacks have a chance to potentially get hold of references to those two resources, but they do not store them. So it is safe to stop the intentional leaking. See also https://lkml.org/lkml/2018/4/23/1063 and following. Signed-off-by: Jan Kiszka --- Ideally, we sort out any remaining worries during the 4.17-rc cycle. drivers/of/overlay.c | 13 ++--- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c index b35fe88f1851..3553f1f57a62 100644 --- a/drivers/of/overlay.c +++ b/drivers/of/overlay.c @@ -671,17 +671,8 @@ static void free_overlay_changeset(struct overlay_changeset *ovcs) of_node_put(ovcs->fragments[i].overlay); } kfree(ovcs->fragments); - - /* -* TODO -* -* would like to: kfree(ovcs->overlay_tree); -* but can not since drivers may have pointers into this data -* -* would like to: kfree(ovcs->fdt); -* but can not since drivers may have pointers into this data -*/ - + kfree(ovcs->overlay_tree); + kfree(ovcs->fdt); kfree(ovcs); } -- 2.13.6
[PATCH] crypto: remove Speck
This NSA-designed cipher was rejected for inclusion in international
standards by ISO/IEC. Before anyone actually starts using it by
accident, let's just not ship it at all.
Signed-off-by: Jason A. Donenfeld
---
arch/arm/crypto/Kconfig |6 -
arch/arm/crypto/Makefile|2 -
arch/arm/crypto/speck-neon-core.S | 432
arch/arm/crypto/speck-neon-glue.c | 288 --
arch/arm64/crypto/Kconfig |6 -
arch/arm64/crypto/Makefile |3 -
arch/arm64/crypto/speck-neon-core.S | 352 ---
arch/arm64/crypto/speck-neon-glue.c | 282 -
arch/s390/defconfig |1 -
crypto/Kconfig | 14 -
crypto/Makefile |1 -
crypto/speck.c | 307 --
crypto/testmgr.c| 36 -
crypto/testmgr.h| 1486 ---
include/crypto/speck.h | 62 --
15 files changed, 3278 deletions(-)
delete mode 100644 arch/arm/crypto/speck-neon-core.S
delete mode 100644 arch/arm/crypto/speck-neon-glue.c
delete mode 100644 arch/arm64/crypto/speck-neon-core.S
delete mode 100644 arch/arm64/crypto/speck-neon-glue.c
delete mode 100644 crypto/speck.c
delete mode 100644 include/crypto/speck.h
diff --git a/arch/arm/crypto/Kconfig b/arch/arm/crypto/Kconfig
index 925d1364727a..b8e69fe282b8 100644
--- a/arch/arm/crypto/Kconfig
+++ b/arch/arm/crypto/Kconfig
@@ -121,10 +121,4 @@ config CRYPTO_CHACHA20_NEON
select CRYPTO_BLKCIPHER
select CRYPTO_CHACHA20
-config CRYPTO_SPECK_NEON
- tristate "NEON accelerated Speck cipher algorithms"
- depends on KERNEL_MODE_NEON
- select CRYPTO_BLKCIPHER
- select CRYPTO_SPECK
-
endif
diff --git a/arch/arm/crypto/Makefile b/arch/arm/crypto/Makefile
index 8de542c48ade..bd5bceef0605 100644
--- a/arch/arm/crypto/Makefile
+++ b/arch/arm/crypto/Makefile
@@ -10,7 +10,6 @@ obj-$(CONFIG_CRYPTO_SHA1_ARM_NEON) += sha1-arm-neon.o
obj-$(CONFIG_CRYPTO_SHA256_ARM) += sha256-arm.o
obj-$(CONFIG_CRYPTO_SHA512_ARM) += sha512-arm.o
obj-$(CONFIG_CRYPTO_CHACHA20_NEON) += chacha20-neon.o
-obj-$(CONFIG_CRYPTO_SPECK_NEON) += speck-neon.o
ce-obj-$(CONFIG_CRYPTO_AES_ARM_CE) += aes-arm-ce.o
ce-obj-$(CONFIG_CRYPTO_SHA1_ARM_CE) += sha1-arm-ce.o
@@ -54,7 +53,6 @@ ghash-arm-ce-y:= ghash-ce-core.o ghash-ce-glue.o
crct10dif-arm-ce-y := crct10dif-ce-core.o crct10dif-ce-glue.o
crc32-arm-ce-y:= crc32-ce-core.o crc32-ce-glue.o
chacha20-neon-y := chacha20-neon-core.o chacha20-neon-glue.o
-speck-neon-y := speck-neon-core.o speck-neon-glue.o
ifdef REGENERATE_ARM_CRYPTO
quiet_cmd_perl = PERL$@
diff --git a/arch/arm/crypto/speck-neon-core.S
b/arch/arm/crypto/speck-neon-core.S
deleted file mode 100644
index 3c1e203e53b9..
--- a/arch/arm/crypto/speck-neon-core.S
+++ /dev/null
@@ -1,432 +0,0 @@
-// SPDX-License-Identifier: GPL-2.0
-/*
- * NEON-accelerated implementation of Speck128-XTS and Speck64-XTS
- *
- * Copyright (c) 2018 Google, Inc
- *
- * Author: Eric Biggers
- */
-
-#include
-
- .text
- .fpuneon
-
- // arguments
- ROUND_KEYS .reqr0 // const {u64,u32} *round_keys
- NROUNDS .reqr1 // int nrounds
- DST .reqr2 // void *dst
- SRC .reqr3 // const void *src
- NBYTES .reqr4 // unsigned int nbytes
- TWEAK .reqr5 // void *tweak
-
- // registers which hold the data being encrypted/decrypted
- X0 .reqq0
- X0_L.reqd0
- X0_H.reqd1
- Y0 .reqq1
- Y0_H.reqd3
- X1 .reqq2
- X1_L.reqd4
- X1_H.reqd5
- Y1 .reqq3
- Y1_H.reqd7
- X2 .reqq4
- X2_L.reqd8
- X2_H.reqd9
- Y2 .reqq5
- Y2_H.reqd11
- X3 .reqq6
- X3_L.reqd12
- X3_H.reqd13
- Y3 .reqq7
- Y3_H.reqd15
-
- // the round key, duplicated in all lanes
- ROUND_KEY .reqq8
- ROUND_KEY_L .reqd16
- ROUND_KEY_H .reqd17
-
- // index vector for vtbl-based 8-bit rotates
- ROTATE_TABLE.reqd18
-
- // multiplication table for updating XTS tweaks
- GF128MUL_TABLE .reqd19
- GF64MUL_TABLE .reqd19
-
- // current XTS tweak value(s)
- TWEAKV .reqq10
- TWEAKV_L.reqd20
- TWEAKV_H.reqd21
-
- TMP0.reqq12
- TMP0_L .reqd24
- TMP0_H .reqd25
- TMP1.reqq13
- TMP2.reqq14
-
[PATCH v2] sc16is7xx: Check for an error when the clock is enabled.
When the clock is enabled, check if there is an error. Otherwise
clk_get_rate() can be called without enabled clock.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 0814e8d5da2b ("sc16is7xx: enable the clock")
Signed-off-by: Stefan Potyra
---
drivers/tty/serial/sc16is7xx.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/tty/serial/sc16is7xx.c b/drivers/tty/serial/sc16is7xx.c
index 65792a3539d0..243c96025053 100644
--- a/drivers/tty/serial/sc16is7xx.c
+++ b/drivers/tty/serial/sc16is7xx.c
@@ -1168,7 +1168,10 @@ static int sc16is7xx_probe(struct device *dev,
else
return PTR_ERR(s->clk);
} else {
- clk_prepare_enable(s->clk);
+ ret = clk_prepare_enable(s->clk);
+ if (ret)
+ return ret;
+
freq = clk_get_rate(s->clk);
}
--
2.17.0
Re: [net-next regression] kselftest failure in fib_nl_newrule()
On Tue, Apr 24, 2018 at 2:46 AM, Anders Roxell wrote:
> Hi,
>
> fib-onlink-tests.sh (from kselftest) found a regression between
> next-20180424 [1] (worked with tag next-20180423 [2])
>
> here is tree commits that look suspicious specially this patch (sha:
> f9d4b0c1e969)
> rewrites fib_nl_newrule().his patch (sha: f9d4b0c1e969) rewrites
> fib_nl_newrule().
>
> b16fb418b1bf ("net: fib_rules: add extack support")
> f9d4b0c1e969 ("fib_rules: move common handling of newrule delrule msgs
> into fib_nl2rule")
> 8a14e46f1402 ("net/ipv6: Fix missing rcu dereferences on from")
>
> Cheers,
> Anders
> [1] https://lkft.validation.linaro.org/scheduler/job/195181#L3447
> [2] https://lkft.validation.linaro.org/scheduler/job/193410#L3438
Thanks for the report.
It should be fixed by my last commit:
9c20b9372fba "net: fib_rules: fix l3mdev netlink attr processing"
Just ran them again and they pass.
Re: [PATCH v5 06/23] ASoC: qdsp6: dt-bindings: Add q6asm dt bindings
On Wed, Apr 18, 2018 at 04:31:40PM +0100, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla
>
> This patch add DT bindings for ASM (Audio Stream Manager) DSP module.
>
> Signed-off-by: Srinivas Kandagatla
> Reviewed-and-tested-by: Rohit kumar
> ---
> .../devicetree/bindings/sound/qcom,q6asm.txt | 33
> ++
> include/dt-bindings/sound/qcom,q6asm.h | 22 +++
> 2 files changed, 55 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,q6asm.txt
> create mode 100644 include/dt-bindings/sound/qcom,q6asm.h
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6asm.txt
> b/Documentation/devicetree/bindings/sound/qcom,q6asm.txt
> new file mode 100644
> index ..d034a50a202a
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,q6asm.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Audio Stream Manager (Q6ASM) binding
> +
> +Q6ASM is one of the APR audio service on Q6DSP.
> +Please refer to qcom,apr.txt for details of the coommon apr service bindings
> +used by the apr service device.
Need to be clear this is a child of APR.
> +
> +- but must contain the following property:
> +
> +- compatible:
> + Usage: required
> + Value type:
> + Definition: must be "qcom,asm-v.".
> + Or "qcom,asm" where the version number can be queried
> + from DSP.
> + example "qcom,asm-v2.0"
> +
> += ASM DAIs (Digial Audio Interface)
> +"dais" subnode of the ASM node represents dai specific configuration
> +
> +- #sound-dai-cells
> + Usage: required
> + Value type:
> + Definition: Must be 1
> +
> += EXAMPLE
> +
> +q6asm {
Unit address needed.
> + compatible = "qcom,q6asm";
Doesn't match the doc.
> + reg = ;
> + q6asmdai: dais {
> + #sound-dai-cells = <1>;
> + };
> +};
> diff --git a/include/dt-bindings/sound/qcom,q6asm.h
> b/include/dt-bindings/sound/qcom,q6asm.h
> new file mode 100644
> index ..2e11b15d930d
> --- /dev/null
> +++ b/include/dt-bindings/sound/qcom,q6asm.h
> @@ -0,0 +1,22 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#ifndef __DT_BINDINGS_Q6_ASM_H__
> +#define __DT_BINDINGS_Q6_ASM_H__
> +
> +#define MSM_FRONTEND_DAI_MULTIMEDIA10
> +#define MSM_FRONTEND_DAI_MULTIMEDIA21
> +#define MSM_FRONTEND_DAI_MULTIMEDIA32
> +#define MSM_FRONTEND_DAI_MULTIMEDIA43
> +#define MSM_FRONTEND_DAI_MULTIMEDIA54
> +#define MSM_FRONTEND_DAI_MULTIMEDIA65
> +#define MSM_FRONTEND_DAI_MULTIMEDIA76
> +#define MSM_FRONTEND_DAI_MULTIMEDIA87
> +#define MSM_FRONTEND_DAI_MULTIMEDIA98
> +#define MSM_FRONTEND_DAI_MULTIMEDIA10 9
> +#define MSM_FRONTEND_DAI_MULTIMEDIA11 10
> +#define MSM_FRONTEND_DAI_MULTIMEDIA12 11
> +#define MSM_FRONTEND_DAI_MULTIMEDIA13 12
> +#define MSM_FRONTEND_DAI_MULTIMEDIA14 13
> +#define MSM_FRONTEND_DAI_MULTIMEDIA15 14
> +#define MSM_FRONTEND_DAI_MULTIMEDIA16 15
> +
> +#endif /* __DT_BINDINGS_Q6_ASM_H__ */
> --
> 2.16.2
>
[PATCH v2] spi/bcm63xx-hspi: Enable the clock before calling clk_get_rate().
Enable the clock prior to calling clk_get_rate(), because clk_get_rate()
should only be called if the clock is enabled.
Found by Linux Driver Verification project (linuxtesting.org).
Fixes: 142168eba9dc (spi: "bcm63xx-hsspi: add bcm63xx HSSPI driver")
Signed-off-by: Stefan Potyra
---
drivers/spi/spi-bcm63xx-hsspi.c | 8
1 file changed, 4 insertions(+), 4 deletions(-)
diff --git a/drivers/spi/spi-bcm63xx-hsspi.c b/drivers/spi/spi-bcm63xx-hsspi.c
index cbcba614b253..46cd9b683d22 100644
--- a/drivers/spi/spi-bcm63xx-hsspi.c
+++ b/drivers/spi/spi-bcm63xx-hsspi.c
@@ -352,6 +352,10 @@ static int bcm63xx_hsspi_probe(struct platform_device
*pdev)
if (IS_ERR(clk))
return PTR_ERR(clk);
+ ret = clk_prepare_enable(clk);
+ if (ret)
+ return ret;
+
rate = clk_get_rate(clk);
if (!rate) {
struct clk *pll_clk = devm_clk_get(dev, "pll");
@@ -364,10 +368,6 @@ static int bcm63xx_hsspi_probe(struct platform_device
*pdev)
return -EINVAL;
}
- ret = clk_prepare_enable(clk);
- if (ret)
- return ret;
-
master = spi_alloc_master(&pdev->dev, sizeof(*bs));
if (!master) {
ret = -ENOMEM;
--
2.17.0
Re: [PATCH 3/3] drm/amdgpu: Switch to interrupted wait to recover from ring hang.
Andrey Grodzovsky writes:
> If the ring is hanging for some reason allow to recover the waiting
> by sending fatal signal.
>
> Originally-by: David Panariti
> Signed-off-by: Andrey Grodzovsky
> ---
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> index eb80edf..37a36af 100644
> --- a/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> +++ b/drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c
> @@ -421,10 +421,16 @@ int amdgpu_ctx_wait_prev_fence(struct amdgpu_ctx *ctx,
> unsigned ring_id)
>
> if (other) {
> signed long r;
> - r = dma_fence_wait_timeout(other, false, MAX_SCHEDULE_TIMEOUT);
> - if (r < 0) {
> - DRM_ERROR("Error (%ld) waiting for fence!\n", r);
> - return r;
> +
> + while (true) {
> + if ((r = dma_fence_wait_timeout(other, true,
> + MAX_SCHEDULE_TIMEOUT)) >= 0)
> + return 0;
> +
> + if (fatal_signal_pending(current)) {
> + DRM_ERROR("Error (%ld) waiting for fence!\n",
> r);
> + return r;
> + }
It looks like if you make this code say:
if (fatal_signal_pending(current) ||
(current->flags & PF_EXITING)) {
DRM_ERROR("Error (%ld) waiting for fence!\n",
r);
return r;
> }
> }
Than you would not need the horrible hack want_signal to deliver signals
to processes who have passed exit_signal() and don't expect to need
their signal handling mechanisms anymore.
Eric
Re: [PATCH] platform/x86: acer-wmi: add another KEY_POWER keycode
On Mon, Apr 23, 2018 at 5:48 PM, Gianfranco Costamagna wrote: > Hello, > >>Can you give your corresponding tag as well? > > > we are implementing the fix right now, please follow bug > https://bugs.launchpad.net/ubuntu/+source/linux/+bug/1766054 OK, I postpone this patch until new version. -- With Best Regards, Andy Shevchenko
Re: [PATCH v5 05/23] ASoC: qdsp6: dt-bindings: Add q6adm dt bindings
On Wed, Apr 18, 2018 at 04:31:39PM +0100, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla
>
> This patch add DT bindings for ADM (Audio Device Manager) DSP module.
> This module implements mixer controls to setup the connections between
> AFE ports and ASM streams.
>
> Signed-off-by: Srinivas Kandagatla
> Reviewed-and-tested-by: Rohit kumar
> ---
> .../devicetree/bindings/sound/qcom,q6adm.txt | 33
> ++
> 1 file changed, 33 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,q6adm.txt
Similar comments in this one.
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6adm.txt
> b/Documentation/devicetree/bindings/sound/qcom,q6adm.txt
> new file mode 100644
> index ..f1e2df8d2ea8
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,q6adm.txt
> @@ -0,0 +1,33 @@
> +Qualcomm Audio Device Manager (Q6ADM) binding
> +
> +Q6ADM is one of the APR audio service on Q6DSP.
> +Please refer to qcom,apr.txt for details of the coommon apr service bindings
> +used by the apr service device.
> +
> +- but must contain the following property:
> +
> +- compatible:
> + Usage: required
> + Value type:
> + Definition: must be "qcom,adm-v.".
> +Or "qcom,adm" where the version number can be queried
> +from DSP.
> +example "qcom,adm-v2.0"
> +
> +
> += ADM routing
> +"routing" subnode of the ADM node represents adm routing specific
> configuration
> +
> +- #sound-dai-cells
> + Usage: required
> + Value type:
> + Definition: Must be 0
> +
> += EXAMPLE
> +q6adm {
a6adm@?
Needs to be the number, not a define.
> + compatible = "qcom,q6adm";
> + reg = ;
> + q6routing: routing {
> + #sound-dai-cells = <0>;
> + };
> +};
> --
> 2.16.2
>
Re: [PATCH v5 04/23] ASoC: qdsp6: dt-bindings: Add q6afe dt bindings
On Wed, Apr 18, 2018 at 04:31:38PM +0100, srinivas.kandaga...@linaro.org wrote:
> From: Srinivas Kandagatla
>
> This patch add DT bindings for AFE (Audio Frontend) DSP module.
>
> Signed-off-by: Srinivas Kandagatla
> Reviewed-and-tested-by: Rohit kumar
> ---
> .../devicetree/bindings/sound/qcom,q6afe.txt | 88
> ++
> include/dt-bindings/sound/qcom,q6afe.h | 31
> 2 files changed, 119 insertions(+)
> create mode 100644 Documentation/devicetree/bindings/sound/qcom,q6afe.txt
> create mode 100644 include/dt-bindings/sound/qcom,q6afe.h
>
> diff --git a/Documentation/devicetree/bindings/sound/qcom,q6afe.txt
> b/Documentation/devicetree/bindings/sound/qcom,q6afe.txt
> new file mode 100644
> index ..3925726a3319
> --- /dev/null
> +++ b/Documentation/devicetree/bindings/sound/qcom,q6afe.txt
> @@ -0,0 +1,88 @@
> +Qualcomm Audio Front End (Q6AFE) binding
> +
> +AFE is one of the APR audio service on Q6DSP
> +Please refer to qcom,apr.txt for details of the common apr service bindings
> +used by all apr services.
> +
> +- but must contain the following property:
> +
> +- compatible:
> + Usage: required
> + Value type:
> + Definition: must be "qcom,afe-v."
> + Or "qcom,afe" where the version number can be queried
> + from DSP.
> + example "qcom,afe-v2.0"
Doesn't match the APR example.
> +
> += AFE DAIs (Digial Audio Interface)
> +"dais" subnode of the AFE node represents dai specific configuration
> +
> +- #sound-dai-cells
> + Usage: required
> + Value type:
array?
> + Definition: Must be 1
> +
> +- reg
> + Usage: required
> + Value type:
array or single value?
> + Definition: Must be dai id
> +
> +- qcom,sd-lines
> + Usage: required for mi2s interface
> + Value type:
> + Definition: Must be list of serial data lines used by this dai.
> + should be one or more of the 1-4 sd lines.
> +
> += EXAMPLE
> +
> +q6afe {
> + compatible = "qcom,q6afe";
> + reg = ;
> +
> + dais {
> + #sound-dai-cells = <1>;
> + hdmi@1 {
> + reg = <1>;
> + };
> +
> + prim-mi2s-rx@16 {
> + reg = <16>;
> + qcom,sd-lines = <1 3>;
> + };
> +
> + prim-mi2s-tx@17 {
> + reg = <17>;
> + qcom,sd-lines = <2>;
> + };
> +
> + sec-mi2s-rx@18 {
> + reg = <18>;
> + qcom,sd-lines = <1 4>;
> + };
> +
> + sec-mi2s-tx@19 {
> + reg = <19>;
> + qcom,sd-lines = <2>;
> + };
> +
> + tert-mi2s-rx@20 {
> + reg = <20>;
> + qcom,sd-lines = <2 4>;
> + };
> +
> + tert-mi2s-tx@21 {
> + reg = <21>;
> + qcom,sd-lines = <1>;
> + };
> +
> + quat-mi2s-rx@22 {
> + reg = <22>;
> + qcom,sd-lines = <1>;
> + };
> +
> + quat-mi2s-tx@23 {
> + reg = <23>;
> + qcom,sd-lines = <2>;
> + };
> + };
> +};
> diff --git a/include/dt-bindings/sound/qcom,q6afe.h
> b/include/dt-bindings/sound/qcom,q6afe.h
> new file mode 100644
> index ..3c7868394889
> --- /dev/null
> +++ b/include/dt-bindings/sound/qcom,q6afe.h
> @@ -0,0 +1,31 @@
> +// SPDX-License-Identifier: GPL-2.0
> +#ifndef __DT_BINDINGS_Q6_AFE_H__
> +#define __DT_BINDINGS_Q6_AFE_H__
> +
> +/* Audio Front End (AFE) virtual ports IDs */
> +#define HDMI_RX 1
> +#define SLIMBUS_0_RX2
> +#define SLIMBUS_0_TX3
> +#define SLIMBUS_1_RX4
> +#define SLIMBUS_1_TX5
> +#define SLIMBUS_2_RX6
> +#define SLIMBUS_2_TX7
> +#define SLIMBUS_3_RX8
> +#define SLIMBUS_3_TX9
> +#define SLIMBUS_4_RX10
> +#define SLIMBUS_4_TX11
> +#define SLIMBUS_5_RX12
> +#define SLIMBUS_5_TX13
> +#define SLIMBUS_6_RX14
> +#define SLIMBUS_6_TX15
> +#define PRIMARY_MI2S_RX 16
> +#define PRIMARY_MI2S_TX 17
> +#define SECONDARY_MI2S_RX18
> +#define SECONDARY_MI2S_TX19
> +#define TERTIARY_MI2S_RX 20
> +#define TERTIARY_MI2S_TX 21
> +#define QUATERNARY_MI2S_RX 22
> +#define QUATERNARY_MI2S_TX 23
> +
> +#endif /* __DT_BINDINGS_Q6_AFE_H__ */
> +
> --
> 2.16.2
>
Re: [PATCH] kvmalloc: always use vmalloc if CONFIG_DEBUG_VM
On Tue 24-04-18 11:30:40, Mikulas Patocka wrote: > > > On Tue, 24 Apr 2018, Michal Hocko wrote: > > > On Mon 23-04-18 20:25:15, Mikulas Patocka wrote: > > > > > Fixing __vmalloc code > > > is easy and it doesn't require cooperation with maintainers. > > > > But it is a hack against the intention of the scope api. > > It is not! This discussion simply doesn't make much sense it seems. The scope API is to document the scope of the reclaim recursion critical section. That certainly is not a utility function like vmalloc. -- Michal Hocko SUSE Labs
Re: [RFC work-in-progress 5/7] !WIP ARM: davinci: convert da850 to using the davinci clocksource driver
2018-04-24 10:10 GMT+02:00 Sekhar Nori :
> On Tuesday 24 April 2018 12:08 AM, Bartosz Golaszewski wrote:
>> From: Bartosz Golaszewski
>>
>> Signed-off-by: Bartosz Golaszewski
>> ---
>> arch/arm/mach-davinci/da850.c | 15 +++
>> 1 file changed, 15 insertions(+)
>>
>> diff --git a/arch/arm/mach-davinci/da850.c b/arch/arm/mach-davinci/da850.c
>> index 3f27d46ca43f..712964383bc2 100644
>> --- a/arch/arm/mach-davinci/da850.c
>> +++ b/arch/arm/mach-davinci/da850.c
>> @@ -757,13 +757,28 @@ void __init da850_init(void)
>> WARN(!da8xx_syscfg1_base, "Unable to map syscfg1 module");
>> }
>>
>> +static struct platform_device da850_timer_device = {
>> + .name = "timer-davinci",
>> + .id = -1,
>> +};
>> +
>> void __init da850_init_time(void)
>> {
>> struct clk *clk;
>> + int ret;
>>
>> clk = clk_register_fixed_rate(NULL, "ref_clk", NULL, 0,
>> DA850_REF_FREQ);
>>
>> davinci_timer_init(clk);
>> +
>> + ret = clk_register_clkdev(clk, NULL, "timer-davinci");
>> + if (ret) {
>> + pr_err("%s: error registering ref_clk: %d", __func__, ret);
>> + return;
>> + }
>
> This part had to be added because you don't have clocks converted to
> early platform devices in your branch, right?
>
> Thanks,
> Sekhar
Yes. Also: I've stumbled upon some problems with the current
implementation of early platform drivers making simple conversion of
psc and pll to early devices impossible. I've mostly fixed them and
hope to be able to send something today or early tomorrow.
Bart
[PATCH] dma-debug: Check scatterlist segments
Drivers/subsystems creating scatterlists for DMA should be taking care
to respect the scatter-gather limitations of the appropriate device, as
described by dma_parms. A DMA API implementation cannot feasibly split
a scatterlist into *more* entries than originally passed, so it is not
well defined what they should do when given a segment larger than the
limit they are also required to respect.
Conversely, devices which are less limited than the rather conservative
defaults, or indeed have no limitations at all (e.g. GPUs with their own
internal MMU), should be encouraged to set appropriate dma_parms, as
they may get more efficient DMA mapping performance out of it.
Signed-off-by: Robin Murphy
---
lib/dma-debug.c | 26 ++
1 file changed, 26 insertions(+)
diff --git a/lib/dma-debug.c b/lib/dma-debug.c
index 7f5cdc1e6b29..9f158941004d 100644
--- a/lib/dma-debug.c
+++ b/lib/dma-debug.c
@@ -1293,6 +1293,30 @@ static void check_sync(struct device *dev,
put_hash_bucket(bucket, &flags);
}
+static void check_sg_segment(struct device *dev, struct scatterlist *sg)
+{
+ unsigned int max_seg = dma_get_max_seg_size(dev);
+ dma_addr_t start, end, boundary = dma_get_seg_boundary(dev);
+
+ /*
+* Either the driver forgot to set dma_parms appropriately, or
+* whoever generated the list forgot to check them.
+*/
+ if (sg->length > max_seg)
+ err_printk(dev, NULL, "DMA-API: mapping sg segment longer than
device claims to support [len=%u] [max=%u]\n",
+ sg->length, max_seg);
+ /*
+* In some cases this could potentially be the DMA API
+* implementation's fault, but it would usually imply that
+* the scatterlist was built inappropriately to begin with.
+*/
+ start = sg_dma_address(sg);
+ end = start + sg_dma_len(sg) - 1;
+ if ((start ^ end) & ~boundary)
+ err_printk(dev, NULL, "DMA-API: mapping sg segment across
boundary [start=0x%016llx] [end=0x%016llx] [boundary=0x%016llx]\n",
+ start, end, boundary);
+}
+
void debug_dma_map_page(struct device *dev, struct page *page, size_t offset,
size_t size, int direction, dma_addr_t dma_addr,
bool map_single)
@@ -1423,6 +1447,8 @@ void debug_dma_map_sg(struct device *dev, struct
scatterlist *sg,
check_for_illegal_area(dev, sg_virt(s), sg_dma_len(s));
}
+ check_sg_segment(dev, s);
+
add_dma_entry(entry);
}
}
--
2.17.0.dirty
Re: [PATCH 1/3] signals: Allow generation of SIGKILL to exiting task.
Andrey Grodzovsky writes:
> Currently calling wait_event_killable as part of exiting process
> will stall forever since SIGKILL generation is suppresed by PF_EXITING.
>
> In our partilaur case AMDGPU driver wants to flush all GPU jobs in
> flight before shutting down. But if some job hangs the pipe we still want to
> be able to kill it and avoid a process in D state.
This makes me profoundly uncomfotable. You are changing the linux
semantics of what it means for a process to be exiting. Functionally
this may require all kinds of changes to when we allow processes to stop
processing signals.
So without a really good thought out explanation that takes into account
all of the issues involved in process exiting and posix conformance.
Nacked-by: "Eric W. Biederman"
Eric
> Signed-off-by: Andrey Grodzovsky
> ---
> kernel/signal.c | 4 ++--
> 1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/kernel/signal.c b/kernel/signal.c
> index c6e4c83..c49c706 100644
> --- a/kernel/signal.c
> +++ b/kernel/signal.c
> @@ -886,10 +886,10 @@ static inline int wants_signal(int sig, struct
> task_struct *p)
> {
> if (sigismember(&p->blocked, sig))
> return 0;
> - if (p->flags & PF_EXITING)
> - return 0;
> if (sig == SIGKILL)
> return 1;
> + if (p->flags & PF_EXITING)
> + return 0;
> if (task_is_stopped_or_traced(p))
> return 0;
> return task_curr(p) || !signal_pending(p);
Re: [PATCH v5 12/44] clk: davinci: Add platform information for TI DA850 PSC
On 04/24/2018 03:28 AM, Sekhar Nori wrote: On Monday 23 April 2018 08:29 PM, David Lechner wrote: On 04/06/2018 11:46 AM, Stephen Boyd wrote: Quoting Sekhar Nori (2018-04-06 02:37:03) Can you please check that and confirm there is no issue with genpd and using CLK_OF_DECLARE() to initialize clocks? Unless you report an issue back, or Mike and Stephen have ideas about how to handle the dependency between PSC/PLL derived timer clock initialization and and timer_probe(), I think we need to move back to using CLK_OF_DECLARE(). In such a case, please use the hybrid approach where the clks required for the clockevent and/or clocksource are registered in the early CLK_OF_DECLARE path but the rest of the clks get registered with a proper platform device and driver. There are examples of this approach on other platforms already. I looked at this a bit last week, but I didn't come up with any approach that I was happy with. It seems like it would be nice to just register the absolute minimum clocks needed. On DA8XX, that would just be the PLL0 AUXCLK. On most of the other SoCs, it would be the PLL AUXCLK plus one LPSC clock. The AUXCLKs are easy because they are just a simple gate from the oscillator. The LPSC clocks are a bit more tricky because they have a complex sequence for turning on. Furthermore, on DM646X, we need the whole PLL up to SYSCLK3 plus one LPSC clock, so things get a bit messy there. Things might change in the context of work being done here by Bartosz for converting clocks to early platform devices. But, keeping that development aside for a moment: I think this means the PLLs and PSCs need to be CLK_OF_DECLARE(). What we can have as platform devices are clocks that are not in the path to get timer clock working (like CFGCHIP clocks). CLK_OF_DECLARE() only matters on DA850. None of the other SoCs use device tree. And on DA850, the timer0 clock is just the PLL0 AUXCLK, so PSCs can still be platform devices there. And in fact, one of the PSC clocks on DA850 depends on async3, which is a CFGCHIP clock, so if we made the PSCs CLK_OF_DECLARE(), then we also have to make at least the async3 CFGCHIP clock CLK_OF_DECLARE(). But, like I said, I think that can be avoided by leaving the PSC clocks as a platform device on DA850 (and DA830). For everything else, we need the legacy board file equivalent of CLK_OF_DECLARE(), which is basically what I had here in v5 of the series. So, if we want this to keep moving before the if/when of Bartosz's early platform device stuff, I'm thinking that I will make the PLL and PSC drivers loadable with or without a platform device and let each SoC pick which one it should use.
Re: [PATCH v5 1/2] media: ov2680: dt: Add bindings for OV2680
Hi Fabio, On Tue 24 Apr 2018 at 15:53, Fabio Estevam wrote: Hi Rui, On Thu, Apr 19, 2018 at 8:00 AM, Rui Miguel Silva wrote: Add device tree binding documentation for the OV2680 camera sensor. Reviewed-by: Rob Herring CC: devicet...@vger.kernel.org Signed-off-by: Rui Miguel Silva --- .../devicetree/bindings/media/i2c/ov2680.txt | 40 +++ 1 file changed, 40 insertions(+) create mode 100644 Documentation/devicetree/bindings/media/i2c/ov2680.txt diff --git a/Documentation/devicetree/bindings/media/i2c/ov2680.txt b/Documentation/devicetree/bindings/media/i2c/ov2680.txt new file mode 100644 index ..0e29f1a113c0 --- /dev/null +++ b/Documentation/devicetree/bindings/media/i2c/ov2680.txt @@ -0,0 +1,40 @@ +* Omnivision OV2680 MIPI CSI-2 sensor + +Required Properties: +- compatible: should be "ovti,ov2680". +- clocks: reference to the xvclk input clock. +- clock-names: should be "xvclk". You missed to pass the camera power supplies as required properties: Urgh, yes, you are right, I will add this. --- Cheers, Rui DOVDD-supply AVDD-supply DVDD-supply
Re: [PATCH] kmemleak: Report if we need to tune KMEMLEAK_EARLY_LOG_SIZE
On 2018-04-24 17:55, Catalin Marinas wrote:
> On Tue, Apr 24, 2018 at 05:51:15PM +0200, Jan Kiszka wrote:
>> ...rather than just mysteriously disabling it.
>>
>> Signed-off-by: Jan Kiszka
>> ---
>> mm/kmemleak.c | 1 +
>> 1 file changed, 1 insertion(+)
>>
>> diff --git a/mm/kmemleak.c b/mm/kmemleak.c
>> index 9a085d525bbc..156c0c69cc5c 100644
>> --- a/mm/kmemleak.c
>> +++ b/mm/kmemleak.c
>> @@ -863,6 +863,7 @@ static void __init log_early(int op_type, const void
>> *ptr, size_t size,
>>
>> if (crt_early_log >= ARRAY_SIZE(early_log)) {
>> crt_early_log++;
>> +pr_warn("Too many early logs\n");
>
> That's already printed, though later where we have an idea of how big the
> early
> log needs to be:
>
> if (crt_early_log > ARRAY_SIZE(early_log))
> pr_warn("Early log buffer exceeded (%d), please increase
> DEBUG_KMEMLEAK_EARLY_LOG_SIZE\n",
> crt_early_log);
>
Well, that's good, but where you read "detector disabled", there is no
hint on that. I missed that because subsystems tend to not do any
further actions after telling they are off.
BTW, my system (virtual ARM64 target) required 26035 entries, which is a
few orders of magnitude above the default and pretty close the the
limit. What's causing this? Other debug options?
Jan
--
Siemens AG, Corporate Technology, CT RDA IOT SES-DE
Corporate Competence Center Embedded Linux
Re: [PATCH v7 2/5] of: change overlay apply input data from unflattened to FDT
On Tue, Apr 24, 2018 at 12:29 AM, Jan Kiszka wrote:
> On 2018-04-24 00:38, Frank Rowand wrote:
>> Hi Jan,
>>
>> + Alan Tull for fpga perspective
>>
>> On 04/22/18 03:30, Jan Kiszka wrote:
>>> On 2018-04-11 07:42, Jan Kiszka wrote:
On 2018-04-05 23:12, Rob Herring wrote:
> On Thu, Apr 5, 2018 at 2:28 PM, Frank Rowand
> wrote:
>> On 04/05/18 12:13, Jan Kiszka wrote:
>>> On 2018-04-05 20:59, Frank Rowand wrote:
Hi Jan,
On 04/04/18 15:35, Jan Kiszka wrote:
> Hi Frank,
>
> On 2018-03-04 01:17, frowand.l...@gmail.com wrote:
>> From: Frank Rowand
>>
>> Move duplicating and unflattening of an overlay flattened devicetree
>> (FDT) into the overlay application code. To accomplish this,
>> of_overlay_apply() is replaced by of_overlay_fdt_apply().
>>
>> The copy of the FDT (aka "duplicate FDT") now belongs to devicetree
>> code, which is thus responsible for freeing the duplicate FDT. The
>> caller of of_overlay_fdt_apply() remains responsible for freeing the
>> original FDT.
>>
>> The unflattened devicetree now belongs to devicetree code, which is
>> thus responsible for freeing the unflattened devicetree.
>>
>> These ownership changes prevent early freeing of the duplicated FDT
>> or the unflattened devicetree, which could result in use after free
>> errors.
>>
>> of_overlay_fdt_apply() is a private function for the anticipated
>> overlay loader.
>
> We are using of_fdt_unflatten_tree + of_overlay_apply in the
> (out-of-tree) Jailhouse loader driver in order to register a virtual
> device during hypervisor activation with Linux. The DT overlay is
> created from a a template but modified prior to application to account
> for runtime-specific parameters. See [1] for the current
> implementation.
>
> I'm now wondering how to model that scenario best with the new API.
> Given that the loader lost ownership of the unflattened tree but the
> modification API exist only for the that DT state, I'm not yet seeing
> a
> clear solution. Should we apply the template in disabled form (status
> =
> "disabled"), modify it, and then activate it while it is already
> applied?
Thank you for the pointer to the driver - that makes it much easier to
understand the use case and consider solutions.
If you can make the changes directly on the FDT instead of on the
expanded devicetree, then you could move to the new API.
>>>
>>> Are there some examples/references on how to edit FDTs in-place in the
>>> kernel? I'd like to avoid writing the n-th FDT parser/generator.
>>
>> I don't know of any existing in-kernel edits of the FDT (but they might
>> exist). The functions to access an FDT are in libfdt, which is in
>> scripts/dtc/libfdt/.
>
> Let's please not go down that route of doing FDT modifications. There
> is little reason to other than for early boot changes. And it is much
> easier to work on unflattened trees.
I just briefly looked into libfdt, and it would have meant building it
into the module as there are no library functions exported by the kernel
either. Another reason to drop that.
What's apparently working now is the pattern I initially suggested:
Register template with status = "disabled" as overlay, then prepare and
apply changeset that contains all needed modifications and sets the
status to "ok". I might be leaking additional resources, but to find
that out, I will now finally have to resolve clean unbinding of the
generic PCI host controller [1] first.
>>>
>>> static void free_overlay_changeset(struct overlay_changeset *ovcs)
>>> {
>>> [...]
>>> /*
>>> * TODO
>>> *
>>> * would like to: kfree(ovcs->overlay_tree);
>>> * but can not since drivers may have pointers into this data
>>> *
>>> * would like to: kfree(ovcs->fdt);
>>> * but can not since drivers may have pointers into this data
>>> */
>>>
>>> kfree(ovcs);
>>> }
>>>
>>> What's this? I have kmemleak now jumping at me over this. Who is suppose
>>> to plug these leaks? The caller of of_overlay_fdt_apply has no pointers
>>> to those objects. I would say that's a regression of the new API.
>>
>> The problem already existed but it was hidden. We have never been able to
>> kfree() these object because we do not know if there are any pointers into
>> these objects. The new API makes the problem visible to kmemleak.
>
> My old code didn't have the problem because there was no one steeling
> pointers to my overlay, and I was able to safely release all the
> resources that I or the core on my behalf al
[PATCH] net: phy: allow scanning busses with missing phys
Some MDIO busses will error out when trying to read a phy address with no
phy present at that address. In that case, probing the bus will fail
because __mdiobus_register() is scanning the bus for all possible phys
addresses.
In case MII_PHYSID1 returns -EIO or -ENODEV, consider there is no phy at
this address and set the phy ID to 0x which is then properly
handled in get_phy_device().
Suggested-by: Andrew Lunn
Signed-off-by: Alexandre Belloni
---
drivers/net/phy/phy_device.c | 11 ++-
1 file changed, 10 insertions(+), 1 deletion(-)
diff --git a/drivers/net/phy/phy_device.c b/drivers/net/phy/phy_device.c
index ac23322a32e1..9e4ba8e80a18 100644
--- a/drivers/net/phy/phy_device.c
+++ b/drivers/net/phy/phy_device.c
@@ -535,8 +535,17 @@ static int get_phy_id(struct mii_bus *bus, int addr, u32
*phy_id,
/* Grab the bits from PHYIR1, and put them in the upper half */
phy_reg = mdiobus_read(bus, addr, MII_PHYSID1);
- if (phy_reg < 0)
+ if (phy_reg < 0) {
+ /* if there is no device, return without an error so scanning
+* the bus works properly
+*/
+ if (phy_reg == -EIO || phy_reg == -ENODEV) {
+ *phy_id = 0x;
+ return 0;
+ }
+
return -EIO;
+ }
*phy_id = (phy_reg & 0x) << 16;
--
2.17.0
Re: [PATCH] Makefile: Fix typo s/HOST_LOADLIBES/HOST_LOADLIBS
2018-04-25 0:59 GMT+09:00 Sedat Dilek : >> Masahiro Yamada hat am 24. April 2018 um >> 17:51 geschrieben: >> >> >> 2018-04-24 17:44 GMT+09:00 Sedat Dilek : >> > Signed-off-by: Sedat Dilek >> > --- >> > Makefile | 4 ++-- >> > 1 file changed, 2 insertions(+), 2 deletions(-) >> > >> > diff --git a/Makefile b/Makefile >> > index 83b6c541565a..e1869dc08288 100644 >> > --- a/Makefile >> > +++ b/Makefile >> > @@ -366,7 +366,7 @@ HOSTCFLAGS := -Wall -Wmissing-prototypes >> > -Wstrict-prototypes -O2 \ >> > -fomit-frame-pointer -std=gnu89 $(HOST_LFS_CFLAGS) >> > HOSTCXXFLAGS := -O2 $(HOST_LFS_CFLAGS) >> > HOSTLDFLAGS := $(HOST_LFS_LDFLAGS) >> > -HOST_LOADLIBES := $(HOST_LFS_LIBS) >> > +HOST_LOADLIBS := $(HOST_LFS_LIBS) >> > >> > # Make variables (CC, etc...) >> > AS = $(CROSS_COMPILE)as >> > @@ -433,7 +433,7 @@ LDFLAGS := >> > GCC_PLUGINS_CFLAGS := >> > >> > export ARCH SRCARCH CONFIG_SHELL HOSTCC HOSTCFLAGS CROSS_COMPILE AS LD CC >> > -export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBES >> > +export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBS >> > export MAKE LEX YACC AWK GENKSYMS INSTALLKERNEL PERL PYTHON PYTHON2 >> > PYTHON3 UTS_MACHINE >> > export HOSTCXX HOSTCXXFLAGS LDFLAGS_MODULE CHECK CHECKFLAGS >> > >> >> Why is this a typo? >> >> >> Our Makefiles use HOST_LOADLIBES consistently. >> > > OK. Do you happen to know why it is LOADLIB*E*S? I do not know. Historical convention. LOADLIBES and/or LDLIBS is conventionally used. You will see them by running 'make --print-data-base' > - Sedat - > >> >> $ git grep HOST_LOADLIB >> Makefile:HOST_LOADLIBES := $(HOST_LFS_LIBS) >> Makefile:export CPP AR NM STRIP OBJCOPY OBJDUMP HOSTLDFLAGS HOST_LOADLIBES >> scripts/Makefile.host: $(HOST_LOADLIBES) $(HOSTLOADLIBES_$(@F)) >> scripts/Makefile.host:$(HOST_LOADLIBES) >> $(HOSTLOADLIBES_$(@F)) >> scripts/Makefile.host:$(HOST_LOADLIBES) >> $(HOSTLOADLIBES_$(@F)) >> scripts/Makefile.host:$(HOST_LOADLIBES) >> $(HOSTLOADLIBES_$(@F)) >> scripts/Makefile.host:$(HOST_LOADLIBES) >> $(HOSTLOADLIBES_$(@F)) >> >> >> >> >> -- >> Best Regards >> Masahiro Yamada > > -- > Mit freundlichen Grüssen, > Sedat Dilek > > Telefon: +49 2166 9901-153 > E-Mail: sedat.di...@credativ.de > Internet: https://www.credativ.de/ > > GPG-Fingerprint: EA6D E17D D269 AC7E 101D C910 476F 2B3B 0AF7 F86B > > credativ GmbH, Trompeterallee 108, 41189 Mönchengladbach > Handelsregister: Amtsgericht Mönchengladbach, HR-Nummer: HRB 12080, > UID-Nummer: DE204566209 > Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer > > ** > Jetzt neu: > Elephant Shed - PostgreSQL Appliance > PostgreSQL und alles was dazugehört > > Von Backup über Monitoring bis Reporting: > https://elephant-shed.io/ > ** > -- > To unsubscribe from this list: send the line "unsubscribe linux-kbuild" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- Best Regards Masahiro Yamada
Re: [PATCH 37/61] perf: simplify getting .drvdata
On Thu, Apr 19, 2018 at 04:06:07PM +0200, Wolfram Sang wrote: > We should get drvdata from struct device directly. Going via > platform_device is an unneeded step back and forth. > > Signed-off-by: Wolfram Sang > --- > > Build tested only. buildbot is happy. Please apply individually. Thanks, Wolfram. I'll pick this up via the arm perf tree. Will
Re: [PATCH v3 2/5] efi: Add embedded peripheral firmware support
On Tue, 2018-04-24 at 17:09 +0200, Hans de Goede wrote:
> Hi,
>
> On 23-04-18 23:11, Luis R. Rodriguez wrote:
> > Hans, please see use of READING_FIRMWARE_PREALLOC_BUFFER, we'll need a new
> > ID
> > and security for this type of request so IMA can reject it if the policy is
> > configured for it.
>
> Hmm, interesting, actually it seems like the whole existence
> of READING_FIRMWARE_PREALLOC_BUFFER is a mistake, the IMA
> framework really does not care if we are loading the firmware
> into memory allocated by the firmware-loader code, or into
> memory allocated by the device-driver requesting the firmware.
>
> As such the current IMA code (from v4.17-rc2) actually does
> not handle READING_FIRMWARE_PREALLOC_BUFFER at all,
Right, it doesn't yet address READING_FIRMWARE_PREALLOC_BUFFER, but
should.
Depending on whether the device requesting the firmware has access to
the DMA memory, before the signature verification, will determine how
IMA-appraisal addresses READING_FIRMWARE_PREALLOC_BUFFER.
Mimi
> here
> are bits of code from: security/integrity/ima/ima_main.c:
>
> static int read_idmap[READING_MAX_ID] = {
> [READING_FIRMWARE] = FIRMWARE_CHECK,
> [READING_MODULE] = MODULE_CHECK,
> [READING_KEXEC_IMAGE] = KEXEC_KERNEL_CHECK,
> [READING_KEXEC_INITRAMFS] = KEXEC_INITRAMFS_CHECK,
> [READING_POLICY] = POLICY_CHECK
> };
>
> int ima_post_read_file(struct file *file, void *buf, loff_t size,
> ...
> if (!file && read_id == READING_FIRMWARE) {
> if ((ima_appraise & IMA_APPRAISE_FIRMWARE) &&
> (ima_appraise & IMA_APPRAISE_ENFORCE))
> return -EACCES; /* INTEGRITY_UNKNOWN */
> return 0;
> }
>
> Which show that the IMA code is not handling
> READING_FIRMWARE_PREALLOC_BUFFER as it should (I believe it
> should handle it the same as READING_FIRMWARE).
>
> Now we could fix that, but the only user of
> READING_FIRMWARE_PREALLOC_BUFFER is the code which originally
> introduced it:
>
> https://patchwork.kernel.org/patch/9162011/
>
> So I believe it might be better to instead replace it
> with just READING_FIRMWARE and find another way to tell
> kernel_read_file() that there is a pre-allocated buffer,
> perhaps the easiest way there is that *buf must be
> NULL when the caller wants kernel_read_file() to
> vmalloc the mem. This would of course require auditing
> all callers that the buf which the pass in is initialized
> to NULL.
>
> Either way adding a third READING_FIRMWARE_FOO to the
> kernel_read_file_id enum seems like a bad idea, from
> the IMA pov firmware is firmware.
>
> What this whole exercise has shown me though is that
> I need to call security_kernel_post_read_file() when
> loading EFI embedded firmware. I will add a call to
> security_kernel_post_read_file() for v4 of the patch-set.
>
> > Please Cc Kees in future patches.
>
> Will do.
>
> Regards,
>
> Hans
>
Re: [PATCH] rave-sp: Remove VLA
On Mon, Apr 23, 2018 at 10:43 PM, Lee Jones wrote:
> On Mon, 23 Apr 2018, Kyle Spiers wrote:
>
>> As part of the effort to remove VLAs from the kernel[1], this creates
>> constants for the checksum lengths of CCITT and 8B2C and changes
>> crc_calculated to be the maximum size of a checksum.
>>
>> https://lkml.org/lkml/2018/3/7/621
>>
>> Signed-off-by: Kyle Spiers
>> ---
>> drivers/mfd/rave-sp.c | 11 +--
>> 1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/drivers/mfd/rave-sp.c b/drivers/mfd/rave-sp.c
>> index 5c858e784a89..99fa482419f9 100644
>> --- a/drivers/mfd/rave-sp.c
>> +++ b/drivers/mfd/rave-sp.c
>> @@ -45,7 +45,9 @@
>> #define RAVE_SP_DLE 0x10
>>
>> #define RAVE_SP_MAX_DATA_SIZE64
>> -#define RAVE_SP_CHECKSUM_SIZE2 /* Worst case scenario on
>> RDU2 */
>> +#define RAVE_SP_CHECKSUM_8B2C1
>> +#define RAVE_SP_CHECKSUM_CCITT 2
>> +#define RAVE_SP_CHECKSUM_SIZERAVE_SP_CHECKSUM_CCITT
>> /*
>> * We don't store STX, ETX and unescaped bytes, so Rx is only
>> * DATA + CSUM
>> @@ -415,7 +417,12 @@ static void rave_sp_receive_frame(struct rave_sp *sp,
>> const size_t payload_length = length - checksum_length;
>> const u8 *crc_reported = &data[payload_length];
>> struct device *dev = &sp->serdev->dev;
>> - u8 crc_calculated[checksum_length];
>> + u8 crc_calculated[RAVE_SP_CHECKSUM_SIZE];
>> +
>> + if (unlikely(length > sizeof(crc_calculated))) {
>
> Forgive me if I have this wrong (it's still very early here), but this
> doesn't leave any room for the payload?
>
> <-- length -->
> <- payload length ->
> [CK][CK][D][A][T][A] .. [64]
>
> It is my hope that length would always be larger than the size of the
> checksum, or else there would never be any data?
>
> Should this not be:
>
> if (unlikely(length > RAVE_SP_MAX_DATA_SIZE))
Oh, whoops, no, this should be:
+ if (unlikely(checksum_lengh > sizeof(crc_calculated))) {
(To validate the VLA max size.)
-Kees
--
Kees Cook
Pixel Security
Re: simplify procfs code for seq_file instances
On Tue, Apr 24, 2018 at 08:19:16AM -0700, Andrew Morton wrote: > > > I want to ask if it is time to start using poorman function overloading > > > with _b_c_e(). There are millions of allocation functions for example, > > > all slightly difference, and people will add more. Seeing /proc interfaces > > > doubled like this is painful. > > > > Function overloading is totally unacceptable. > > > > And I very much disagree with a tradeoff that keeps 5000 lines of > > code vs a few new helpers. > > OK, the curiosity and suspense are killing me. What the heck is > "function overloading with _b_c_e()"? The way I understood Alexey was to use have a proc_create macro that can take different ops types. Although the short cut for __builtin_types_compatible_p would be _b_t_c or similar, so maybe I misunderstood him.
Re: [PATCH] powerpc/signal32: Use fault_in_pages_readable() to prefault user context
Le 24/04/2018 à 16:50, Mathieu Malaterre a écrit :
On Tue, Apr 24, 2018 at 4:45 PM, Mathieu Malaterre wrote:
On Tue, Apr 24, 2018 at 4:17 PM, Christophe Leroy
wrote:
Use fault_in_pages_readable() to prefault user context
instead of open coding
Signed-off-by: Christophe Leroy
---
arch/powerpc/kernel/signal_32.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 492f03451877..cfacb2726152 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#ifdef CONFIG_PPC64
#include
@@ -1045,7 +1046,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
struct ucontext __user *new_ctx,
int ctx_size, int r6, int r7, int r8, struct pt_regs
*regs)
{
- unsigned char tmp __maybe_unused;
int ctx_has_vsx_region = 0;
#ifdef CONFIG_PPC64
@@ -1109,9 +1109,8 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
}
if (new_ctx == NULL)
return 0;
- if (!access_ok(VERIFY_READ, new_ctx, ctx_size)
- || __get_user(tmp, (u8 __user *) new_ctx)
- || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1))
+ if (!access_ok(VERIFY_READ, new_ctx, ctx_size) ||
+ fault_in_pages_readable((u8 __user *)new_ctx, ctx_size))
return -EFAULT;
/*
@@ -1231,7 +1230,6 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
{
struct sig_dbg_op op;
int i;
- unsigned char tmp __maybe_unused;
unsigned long new_msr = regs->msr;
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
unsigned long new_dbcr0 = current->thread.debug.dbcr0;
@@ -1287,9 +1285,8 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
current->thread.debug.dbcr0 = new_dbcr0;
#endif
- if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx))
- || __get_user(tmp, (u8 __user *) ctx)
- || __get_user(tmp, (u8 __user *) (ctx + 1) - 1))
+ if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx)) ||
+ fault_in_pages_readable((u8 __user *)ctx, 1))
I believe you meant:
fault_in_pages_readable((u8 __user *)new_ctx, ctx_size)
Since (u8 __user *) (ctx + 1) - 1 really is (u8 __user *) new_ctx + ctx_size - 1
Without the copy/paste errors:
[...]
fault_in_pages_readable((u8 __user *)ctx, sizeof(*ctx))
Since (u8 __user *) (ctx + 1) - 1 really is (u8 __user *) ctx + sizeof(*ctx) - 1
[...]
Oops you're right thanks.
v2 submitted
Christophe
return -EFAULT;
/*
--
2.13.3
Re: [PATCH v4 7/8] drm/i2c: tda998x: register as a drm bridge
On 24/04/18 13:14, Peter Rosin wrote:
> On 2018-04-24 10:08, Russell King - ARM Linux wrote:
>> On Tue, Apr 24, 2018 at 08:58:42AM +0200, Peter Rosin wrote:
>>> On 2018-04-23 18:08, Russell King - ARM Linux wrote:
On Mon, Apr 23, 2018 at 09:23:00AM +0200, Peter Rosin wrote:
> static int tda998x_remove(struct i2c_client *client)
> {
> - component_del(&client->dev, &tda998x_ops);
> + struct device *dev = &client->dev;
> + struct tda998x_bridge *bridge = dev_get_drvdata(dev);
> +
> + drm_bridge_remove(&bridge->bridge);
> + component_del(dev, &tda998x_ops);
> +
I'd like to ask a rather fundamental question about DRM bridge support,
because I suspect that there's a major fsckup here.
The above is the function that deals with the TDA998x device being
unbound from the driver. With the component API, this results in the
DRM device correctly being torn down, because one of the hardware
devices has gone.
With DRM bridge, the bridge is merely removed from the list of
bridges:
void drm_bridge_remove(struct drm_bridge *bridge)
{
mutex_lock(&bridge_lock);
list_del_init(&bridge->list);
mutex_unlock(&bridge_lock);
}
EXPORT_SYMBOL(drm_bridge_remove);
and the memory backing the "struct tda998x_bridge" (which contains
the struct drm_bridge) will be freed by the devm subsystem.
However, there is no notification into the rest of the DRM subsystem
that the device has gone away. Worse, the memory that is still in
use by DRM has now been freed, so further use of the DRM device
results in a use-after-free bug.
This is really not good, and to me looks like a fundamental problem
with the DRM bridge code. I see nothing in the DRM bridge code that
deals with the lifetime of a "DRM bridge" or indeed the lifetime of
the actual device itself.
So, from what I can see, there seems to be a fundamental lifetime
issue with the design of the DRM bridge code. This needs to be
fixed.
>>>
>>> Oh crap. A gigantic can of worms...
>>
>> Yes, it's especially annoying for me, having put the effort in to
>> the component helper to cover all these cases.
>>
>>> Would a patch (completely untested btw) along this line of thinking make
>>> any difference whatsoever?
>>
>> It looks interesting - from what I can see of the device links code,
>> it would have the effect of unbinding the DRM device just before
>> TDA998x is unbound, so that's an improvement.
>>
>> However, from what I can see, the link vanishes at that point (as
>> DL_FLAG_AUTOREMOVE is set), and re-binding the TDA998x device results
>> in nothing further happening - the link will be recreated, but there
>> appears to be nothing that triggers the "consumer" to rebind at that
>> point. Maybe I've missed something?
>
> Right, auto-remove is a no-go. So, improving on the previous...
>
> (I think drm_panel might suffer from this issue too?)
Yes it does and I took a shot at trying to fix it at the end of the
previous merge window, but gave up as I run out of time. I re-spun the
work now after reading this thread. I add you and Russell to cc.
As far as I understand the re-binding problem can be solved with this patch:
https://lists.freedesktop.org/archives/dri-devel/2018-February/166907.html
The device_links are such a new concept that there is at least hope this
change won't have too unwanted side effects.
> > Cheers,
> Peter
>
> diff --git a/drivers/gpu/drm/drm_bridge.c b/drivers/gpu/drm/drm_bridge.c
> index 1638bfe9627c..b1365cfee445 100644
> --- a/drivers/gpu/drm/drm_bridge.c
> +++ b/drivers/gpu/drm/drm_bridge.c
> @@ -121,12 +121,17 @@ int drm_bridge_attach(struct drm_encoder *encoder,
> struct drm_bridge *bridge,
> if (bridge->dev)
> return -EBUSY;
>
> + bridge->link = device_link_add(encoder->dev->dev, bridge->owner, 0);
> + if (!bridge->link)
> + return -EINVAL;
> +
At least this piece code should prepare for the bridge owner and the
master drm device being the same, I which case the link should not be
needed. This happens at least when a panel is attached using
drm_panel_bridge_add().
Best reagards,
Jyri
> bridge->dev = encoder->dev;
> bridge->encoder = encoder;
>
> if (bridge->funcs->attach) {
> ret = bridge->funcs->attach(bridge);
> if (ret < 0) {
> + device_link_del(bridge->link);
> bridge->dev = NULL;
> bridge->encoder = NULL;
> return ret;
> @@ -153,6 +158,8 @@ void drm_bridge_detach(struct drm_bridge *bridge)
> if (bridge->funcs->detach)
> bridge->funcs->detach(bridge);
>
> + device_link_del(bridge->link);
> +
> bridge->dev = NULL;
> }
>
> diff --git a/drivers/gpu/drm/i2c/tda998x_drv.c
> b/drivers/gpu/drm/i2c/tda998x_d
Re: [PATCH v5 01/23] soc: qcom dt-bindings: Add APR bus bindings
Thanks for the review comments,
On 24/04/18 16:52, Rob Herring wrote:
On Wed, Apr 18, 2018 at 04:31:35PM +0100, srinivas.kandaga...@linaro.org wrote:
From: Srinivas Kandagatla
This patch add dt bindings for Qualcomm APR (Asynchronous Packet Router)
bus driver. This bus is used for communicating with DSP which provides
audio and various other services to cpu.
Signed-off-by: Srinivas Kandagatla
---
.../devicetree/bindings/soc/qcom/qcom,apr.txt | 85 ++
This should probably go under bindings/sound/ as it is at least mostly
sound related.
There are other non audio dsp/modem services that will be using apr.
include/dt-bindings/soc/qcom,apr.h | 27 +++
2 files changed, 112 insertions(+)
create mode 100644 Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
create mode 100644 include/dt-bindings/soc/qcom,apr.h
diff --git a/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
new file mode 100644
index ..85cc0433fb00
--- /dev/null
+++ b/Documentation/devicetree/bindings/soc/qcom/qcom,apr.txt
@@ -0,0 +1,85 @@
+Qualcomm APR (Asynchronous Packet Router) binding
+
+This binding describes the Qualcomm APR. APR is a IPC protocol for
+communication between Application processor and QDSP. APR is mainly
+used for audio/voice services on the QDSP.
+
+- compatible:
+ Usage: required
+ Value type:
+ Definition: must be "qcom,apr-v", example "qcom,apr-v2"
+
+- qcom,dest-domain-id
+ Usage: required
+ Value type:
If this is an array, what do the indexes of the array correspond to?
This should be
Value type:
these indexes point to the remote processor ID.
+ Definition: Destination processor ID.
+ Possible values are :
+ 1 - APR simulator
+ 2 - PC
+ 3 - MODEM
+ 4 - ADSP
+ 5 - APPS
+ 6 - MODEM2
+ 7 - APPS2
+
+= APR SERVICES
+Each subnode of the APR node represents service tied to this apr. The name
+of the nodes are not important. The properties of these nodes are defined
+by the individual bindings for the specific service
+- All APR services MUST contain the following property:
+
+- reg
+ Usage: required
+ Value type:
Not really an array, right?
No, again this is Value type:
+ Definition: APR Service ID
+ Possible values are :
+ 3 - DSP Core Service
+ 4 - Audio Front End Service.
+ 5 - Voice Stream Manager Service.
+ 6 - Voice processing manager.
+ 7 - Audio Stream Manager Service.
+ 8 - Audio Device Manager Service.
+ 9 - Multimode voice manager.
+ 10 - Core voice stream.
+ 11 - Core voice processor.
+ 12 - Ultrasound stream manager.
+ 13 - Listen stream manager.
+
+= EXAMPLE
+The following example represents a QDSP based sound card on a MSM8996 device
+which uses apr as communication between Apps and QDSP.
+
+ apr {
+ compatible = "qcom,apr-v2";
+ qcom,smd-channels = "apr_audio_svc";
Drop this?
Sure
--srini
[PATCH v2] powerpc/signal32: Use fault_in_pages_readable() to prefault user context
Use fault_in_pages_readable() to prefault user context
instead of open coding
Signed-off-by: Christophe Leroy
---
v2: using sizeof(*ctx) as size of ctx instead of 1
arch/powerpc/kernel/signal_32.c | 13 +
1 file changed, 5 insertions(+), 8 deletions(-)
diff --git a/arch/powerpc/kernel/signal_32.c b/arch/powerpc/kernel/signal_32.c
index 492f03451877..4a9e4d6d555b 100644
--- a/arch/powerpc/kernel/signal_32.c
+++ b/arch/powerpc/kernel/signal_32.c
@@ -25,6 +25,7 @@
#include
#include
#include
+#include
#include
#ifdef CONFIG_PPC64
#include
@@ -1045,7 +1046,6 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
struct ucontext __user *new_ctx,
int ctx_size, int r6, int r7, int r8, struct pt_regs *regs)
{
- unsigned char tmp __maybe_unused;
int ctx_has_vsx_region = 0;
#ifdef CONFIG_PPC64
@@ -1109,9 +1109,8 @@ long sys_swapcontext(struct ucontext __user *old_ctx,
}
if (new_ctx == NULL)
return 0;
- if (!access_ok(VERIFY_READ, new_ctx, ctx_size)
- || __get_user(tmp, (u8 __user *) new_ctx)
- || __get_user(tmp, (u8 __user *) new_ctx + ctx_size - 1))
+ if (!access_ok(VERIFY_READ, new_ctx, ctx_size) ||
+ fault_in_pages_readable((u8 __user *)new_ctx, ctx_size))
return -EFAULT;
/*
@@ -1231,7 +1230,6 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
{
struct sig_dbg_op op;
int i;
- unsigned char tmp __maybe_unused;
unsigned long new_msr = regs->msr;
#ifdef CONFIG_PPC_ADV_DEBUG_REGS
unsigned long new_dbcr0 = current->thread.debug.dbcr0;
@@ -1287,9 +1285,8 @@ int sys_debug_setcontext(struct ucontext __user *ctx,
current->thread.debug.dbcr0 = new_dbcr0;
#endif
- if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx))
- || __get_user(tmp, (u8 __user *) ctx)
- || __get_user(tmp, (u8 __user *) (ctx + 1) - 1))
+ if (!access_ok(VERIFY_READ, ctx, sizeof(*ctx)) ||
+ fault_in_pages_readable((u8 __user *)ctx, sizeof(*ctx)))
return -EFAULT;
/*
--
2.13.3
Re: [PATCH] tpm: tpm_crb: relinquish locality on error path.
On Fri, Apr 20, 2018 at 01:19:12PM +, Winkler, Tomas wrote:
> > > On Tue, 2018-04-10 at 09:00 +, Winkler, Tomas wrote:
> > > > >
> > > > > On Sat, 2018-04-07 at 19:12 +0300, Tomas Winkler wrote:
> > > > > > In crb_map_io() function, __crb_request_locality() is called
> > > > > > prior to crb_cmd_ready(), but if one of the consecutive function
> > > > > > fails the flow bails out instead of trying to relinquish locality.
> > > > > > This patch adds goto jump to __crb_relinquish_locality() on the
> > > > > > error path.
> > > > > >
> > > > > > Fixes: 888d867df441 (tpm: cmd_ready command can be issued only
> > > > > > after granting
> > > > > > locality)
> > > > > > Signed-off-by: Tomas Winkler
> > > > > > ---
> > > > > > drivers/char/tpm/tpm_crb.c | 10 +++---
> > > > > > 1 file changed, 7 insertions(+), 3 deletions(-)
> > > > > >
> > > > > > diff --git a/drivers/char/tpm/tpm_crb.c
> > > > > > b/drivers/char/tpm/tpm_crb.c index 7f78482cd157..34fbc6cb097b
> > > > > > 100644
> > > > > > --- a/drivers/char/tpm/tpm_crb.c
> > > > > > +++ b/drivers/char/tpm/tpm_crb.c
> > > > > > @@ -511,8 +511,10 @@ static int crb_map_io(struct acpi_device
> > > > > > *device, struct crb_priv *priv,
> > > > > >
> > > > > > priv->regs_t = crb_map_res(dev, priv, &io_res, buf-
> > > > > > >control_address,
> > > > > >sizeof(struct crb_regs_tail));
> > > > > > - if (IS_ERR(priv->regs_t))
> > > > > > - return PTR_ERR(priv->regs_t);
> > > > > > + if (IS_ERR(priv->regs_t)) {
> > > > > > + ret = PTR_ERR(priv->regs_t);
> > > > > > + goto out_relinquish_locality;
> > > > > > + }
> > > > > >
> > > > > > /*
> > > > > > * PTT HW bug w/a: wake up the device to access @@ -520,7
> > > > > > +522,7
> > > > >
> > > > > @@
> > > > > > static int crb_map_io(struct acpi_device *device, struct
> > > > > > crb_priv *priv,
> > > > > > */
> > > > > > ret = crb_cmd_ready(dev, priv);
> > > > > > if (ret)
> > > > > > - return ret;
> > > > > > + goto out_relinquish_locality;
> > > > > >
> > > > > > pa_high = ioread32(&priv->regs_t->ctrl_cmd_pa_high);
> > > > > > pa_low = ioread32(&priv->regs_t->ctrl_cmd_pa_low);
> > > > > > @@ -565,6 +567,8 @@ static int crb_map_io(struct acpi_device
> > > > > > *device, struct crb_priv *priv,
> > > > > >
> > > > > > crb_go_idle(dev, priv);
> > > > > >
> > > > > > +out_relinquish_locality:
> > > > > > +
> > > > > > __crb_relinquish_locality(dev, priv, 0);
> > > > > >
> > > > > > return ret;
> > > > >
> > > > > Thanks, please just call it before returning in the error path.
> > > >
> > > > Can you please elaborate why, isn't the centralized exiting of
> > > > functions preferred kernel coding style?
> > > > https://www.kernel.org/doc/html/v4.11/process/coding-style.html#cent
> > > > ra
> > > > lized-ex
> > > > iting-of-functions
> > >
> > > You exit only from one location (not multiple) and not from a nested
> > > context. Here you just add more complexity by doing this.
> >
> > Where is the complexity ? I see it as a standard way of undoing on exit.
> > Tomas
>
> Jarkko, can you please respond.
> Thanks
> Tomas
I was away for Mon-Wed last week and did not work on TPM for Thu-Fri.
My earlier comment was incorrect as there are two locations to exit (not
sure how I managed to overlook the patch that way).
Thus, I have only two very minor requets:
* Remove the extra newline (the last line addition in the patch).
* Use just label named out as we have only one exception handler.
I'll move on to testing, and if it it passes, I can do those updates
myself.
/Jarkko
Re: [PATCH 58/61] video: fbdev: omap2: omapfb: displays: simplify getting .drvdata
On Thursday, April 19, 2018 04:06:28 PM Wolfram Sang wrote: > We should get drvdata from struct device directly. Going via > platform_device is an unneeded step back and forth. > > Signed-off-by: Wolfram Sang Patch queued for 4.18, thanks. Best regards, -- Bartlomiej Zolnierkiewicz Samsung R&D Institute Poland Samsung Electronics
Re: [PATCH v3 03/10] drivers/peci: Add support for PECI bus driver core
On Tue, 2018-04-10 at 11:32 -0700, Jae Hyun Yoo wrote: > This commit adds driver implementation for PECI bus core into linux > driver framework. > All comments you got for patch 6 are applicable here. And perhaps in the rest of the series. The rule of thumb: when you get even single comment in a certain place, re-check _entire_ series for the same / similar patterns! -- Andy Shevchenko Intel Finland Oy
Re: [RFC v4 3/4] irqflags: Avoid unnecessary calls to trace_ if you can
On Tue, Apr 24, 2018 at 8:56 AM, Paul E. McKenney wrote: > On Mon, Apr 23, 2018 at 05:22:44PM -0400, Steven Rostedt wrote: >> On Mon, 23 Apr 2018 13:12:21 -0400 (EDT) >> Mathieu Desnoyers wrote: >> >> >> > I'm inclined to explicitly declare the tracepoints with their given >> > synchronization method. Tracepoint probe callback functions for currently >> > existing tracepoints expect to have preemption disabled when invoked. >> > This assumption will not be true anymore for srcu-tracepoints. >> >> Actually, why not have a flag attached to the tracepoint_func that >> states if it expects preemption to be enabled or not? If a >> trace_##event##_srcu() is called, then simply disable preemption before >> calling the callbacks for it. That way if a callback is fine for use >> with srcu, then it would require calling >> >> register_trace_##event##_may_sleep(); >> >> Then if someone uses this on a tracepoint where preemption is disabled, >> we simply do not call it. > > One more stupid question... If we are having to trace so much stuff > in the idle loop, are we perhaps grossly overstating the extent of that > "idle" loop? For being called "idle", this code seems quite busy! ;-) The performance hit I am observing is when running a heavy workload, like hackbench or something like that. That's what I am trying to correct. By the way is there any limitation on using SRCU too early during boot? I backported Mathieu's srcu tracepoint patches but the kernel hangs pretty early in the boot. I register lockdep probes in start_kernel. I am hoping that's not why. I could also have just screwed up the backporting... may be for my testing, I will just replace the rcu API with the srcu instead of all of Mathieu's new TRACE_EVENT macros for SRCU, since all I am trying to do right now is measure the performance of my patches with SRCU. thanks, - Joel
