Re: [PATCH] cpufreq: cached_resolved_idx can not be negative
On Thu, Jul 30, 2020 at 9:38 AM Viresh Kumar wrote:
>
> It is not possible for cached_resolved_idx to be invalid here as the
> cpufreq core always sets index to a positive value.
>
> Change its type to unsigned int and fix qcom usage a bit.
Shouldn't you fix up idx in cpufreq_driver_resolve_freq() to be
unsigned int too?
> Signed-off-by: Viresh Kumar
> ---
> drivers/cpufreq/qcom-cpufreq-hw.c | 5 +
> include/linux/cpufreq.h | 2 +-
> 2 files changed, 2 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/cpufreq/qcom-cpufreq-hw.c
> b/drivers/cpufreq/qcom-cpufreq-hw.c
> index 0a04b6f03b9a..8c0842bd6c5a 100644
> --- a/drivers/cpufreq/qcom-cpufreq-hw.c
> +++ b/drivers/cpufreq/qcom-cpufreq-hw.c
> @@ -66,13 +66,10 @@ static unsigned int qcom_cpufreq_hw_fast_switch(struct
> cpufreq_policy *policy,
> unsigned int target_freq)
> {
> void __iomem *perf_state_reg = policy->driver_data;
> - int index;
> + unsigned int index;
> unsigned long freq;
>
> index = policy->cached_resolved_idx;
> - if (index < 0)
> - return 0;
> -
> writel_relaxed(index, perf_state_reg);
>
> freq = policy->freq_table[index].frequency;
> diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h
> index e62b022cb07e..58687a5bf9c8 100644
> --- a/include/linux/cpufreq.h
> +++ b/include/linux/cpufreq.h
> @@ -127,7 +127,7 @@ struct cpufreq_policy {
>
> /* Cached frequency lookup from cpufreq_driver_resolve_freq. */
> unsigned int cached_target_freq;
> - int cached_resolved_idx;
> + unsigned int cached_resolved_idx;
>
> /* Synchronization for frequency transitions */
> booltransition_ongoing; /* Tracks transition
> status */
> --
> 2.14.1
>
Re: [PATCH v3 2/3] Input: snvs_pwrkey - enable snvs clock as needed
On Wed, Jul 29, 2020 at 09:33:23AM +0200, Marco Felsch wrote:
> Hi,
>
> On 20-07-23 10:43, Horia Geantă wrote:
> > From: André Draszik
> >
> > At the moment, enabling this driver without the SNVS RTC driver
> > being active will hang the kernel as soon as the power button
> > is pressed.
> >
> > The reason is that in that case the SNVS isn't enabled, and
> > any attempt to read the SNVS registers will simply hang forever.
> >
> > Ensure the clock is enabled (during the interrupt handler) to
> > make this driver work.
> >
> > Also see commit 7f8993995410 ("drivers/rtc/rtc-snvs: add clock support")
> > and commit edb190cb1734
> > ("rtc: snvs: make sure clock is enabled for interrupt handle")
> > for similar updates to the snvs rtc driver.
> >
> > Signed-off-by: André Draszik
> > Reviewed-by: Horia Geantă
> > Signed-off-by: Horia Geantă
> > ---
> > drivers/input/keyboard/snvs_pwrkey.c | 28 +++-
> > 1 file changed, 27 insertions(+), 1 deletion(-)
> >
> > diff --git a/drivers/input/keyboard/snvs_pwrkey.c
> > b/drivers/input/keyboard/snvs_pwrkey.c
> > index 2f5e3ab5ed63..382d2ae82c9b 100644
> > --- a/drivers/input/keyboard/snvs_pwrkey.c
> > +++ b/drivers/input/keyboard/snvs_pwrkey.c
> > @@ -16,6 +16,7 @@
> > #include
> > #include
> > #include
> > +#include
> > #include
> > #include
> >
> > @@ -38,6 +39,7 @@ struct pwrkey_drv_data {
> > int wakeup;
> > struct timer_list check_timer;
> > struct input_dev *input;
> > + struct clk *clk;
> > u8 minor_rev;
> > };
> >
> > @@ -47,7 +49,10 @@ static void imx_imx_snvs_check_for_events(struct
> > timer_list *t)
> > struct input_dev *input = pdata->input;
> > u32 state;
> >
> > + clk_enable(pdata->clk);
> > regmap_read(pdata->snvs, SNVS_HPSR_REG, );
> > + clk_disable(pdata->clk);
> > +
> > state = state & SNVS_HPSR_BTN ? 1 : 0;
> >
> > /* only report new event if status changed */
> > @@ -74,11 +79,13 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq,
> > void *dev_id)
> >
> > pm_wakeup_event(input->dev.parent, 0);
> >
> > + clk_enable(pdata->clk);
> > +
> > regmap_read(pdata->snvs, SNVS_LPSR_REG, _status);
> > if (lp_status & SNVS_LPSR_SPO) {
> > if (pdata->minor_rev == 0) {
> > /*
> > -* The first generation i.MX6 SoCs only sends an
> > +* The first generation i.MX[6|7] SoCs only send an
> > * interrupt on button release. To mimic power-key
> > * usage, we'll prepend a press event.
> > */
> > @@ -96,6 +103,8 @@ static irqreturn_t imx_snvs_pwrkey_interrupt(int irq,
> > void *dev_id)
> > /* clear SPO status */
> > regmap_write(pdata->snvs, SNVS_LPSR_REG, SNVS_LPSR_SPO);
> >
> > + clk_disable(pdata->clk);
>
> I'm not so happy about clk_enable/disable() during the interrupt
> routine since those routines should be handled fast. Since we assume
> that the clock is always oni
We do? I thought that the fact that clock might be off is the root of
the problem.
> I would rather call clk_prepare_enable()
> during probe() and keep the clock on.
clk_enable() must not sleep and is supposed to be lightweight. This
interrupt I believe is supposed to be infrequent, so having it here
should be fine and might save us some power.
>
> > +
> > return IRQ_HANDLED;
> > }
> >
> > @@ -140,6 +149,23 @@ static int imx_snvs_pwrkey_probe(struct
> > platform_device *pdev)
> > if (pdata->irq < 0)
> > return -EINVAL;
> >
> > + pdata->clk = devm_clk_get_optional(>dev, "snvs-pwrkey");
> > + if (IS_ERR(pdata->clk))
> > + return PTR_ERR(pdata->clk);
> > +
> > + error = clk_prepare(pdata->clk);
> > + if (error) {
> > + dev_err(>dev, "failed to prepare the snvs clock\n");
> > + return error;
> > + }
> > + error = devm_add_action_or_reset(>dev,
> > + (void(*)(void *))clk_unprepare,
> > + pdata->clk);
>
> I'm not a fan about those casts. However, the intentation should be
> fixed.
Yes, can we please create a proper function for this (or maybe we will
finally get devm clk API?).
Thanks.
--
Dmitry
Re: [PATCH v3 3/3] Input: snvs_pwrkey - only IRQ_HANDLED for our own events
Hi Marco, On Wed, Jul 29, 2020 at 09:55:13AM +0200, Marco Felsch wrote: > Hi, > > On 20-07-23 10:43, Horia Geantă wrote: > > From: André Draszik > > > > The snvs_pwrkey shares the SNVS LPSR status register with the snvs_rtc. > > > > This driver here should only return IRQ_HANDLED if the status register > > indicates that the event we're handling in the irq handler was genuinely > > intended for this driver. Otheriwse the interrupt subsystem will > > assume the interrupt was handled successfully even though it wasn't > > at all. > > After checking the RM and the imx6qdl.dtsi I'm not very sure that this > is right since the snvs-powerkey has a seperate irq-line. So we can be > sure that this irq is for us. If this is the case we don't need to check > the SNVS_LPSR_REG instead we only need to clear it. Wouldn't we want to know if for some reason we get spurious interrupts? Thanks. -- Dmitry
[PATCH] Input: elan_i2c - Add ic type 0x15.
The update firmware flow of ic type 0x15 is same with ic type 0x14.
Signed-off-by: Jingle Wu
---
drivers/input/mouse/elan_i2c_core.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/input/mouse/elan_i2c_core.c
b/drivers/input/mouse/elan_i2c_core.c
index 8a0f224da423..c599e21a8478 100644
--- a/drivers/input/mouse/elan_i2c_core.c
+++ b/drivers/input/mouse/elan_i2c_core.c
@@ -138,6 +138,7 @@ static int elan_get_fwinfo(u16 ic_type, u8 iap_version, u16
*validpage_count,
*validpage_count = 2048;
break;
case 0x14:
+ case 0x15:
*validpage_count = 1024;
break;
default:
@@ -151,7 +152,7 @@ static int elan_get_fwinfo(u16 ic_type, u8 iap_version, u16
*validpage_count,
*signature_address =
(*validpage_count * ETP_FW_PAGE_SIZE) - ETP_FW_SIGNATURE_SIZE;
- if (ic_type == 0x14 && iap_version >= 2) {
+ if ((ic_type == 0x14 || ic_type == 0x15) && iap_version >= 2) {
*validpage_count /= 8;
*page_size = ETP_FW_PAGE_SIZE_512;
} else if (ic_type >= 0x0D && iap_version >= 1) {
--
2.17.1
Re: [RESEND PATCH v5 00/11] ppc64: enable kdump support for kexec_file_load syscall
On 28/07/20 8:02 am, piliu wrote: On 07/27/2020 03:36 AM, Hari Bathini wrote: Sorry! There was a gateway issue on my system while posting v5, due to which some patches did not make it through. Resending... This patch series enables kdump support for kexec_file_load system call (kexec -s -p) on PPC64. The changes are inspired from kexec-tools code but heavily modified for kernel consumption. The first patch adds a weak arch_kexec_locate_mem_hole() function to override locate memory hole logic suiting arch needs. There are some special regions in ppc64 which should be avoided while loading buffer & there are multiple callers to kexec_add_buffer making it complicated to maintain range sanity and using generic lookup at the same time. The second patch marks ppc64 specific code within arch/powerpc/kexec and arch/powerpc/purgatory to make the subsequent code changes easy to understand. The next patch adds helper function to setup different memory ranges needed for loading kdump kernel, booting into it and exporting the crashing kernel's elfcore. The fourth patch overrides arch_kexec_locate_mem_hole() function to locate memory hole for kdump segments by accounting for the special memory regions, referred to as excluded memory ranges, and sets kbuf->mem when a suitable memory region is found. The fifth patch moves walk_drmem_lmbs() out of .init section with a few changes to reuse it for setting up kdump kernel's usable memory ranges. The next patch uses walk_drmem_lmbs() to look up the LMBs and set linux,drconf-usable-memory & linux,usable-memory properties in order to restrict kdump kernel's memory usage. The seventh patch updates purgatory to setup r8 & r9 with opal base and opal entry addresses respectively to aid kernels built with CONFIG_PPC_EARLY_DEBUG_OPAL enabled. The next patch setups up backup region as a kexec segment while loading kdump kernel and teaches purgatory to copy data from source to destination. Patch 09 builds the elfcore header for the running kernel & passes the info to kdump kernel via "elfcorehdr=" parameter to export as /proc/vmcore file. The next patch sets up the memory reserve map for the kexec kernel and also claims kdump support for kdump as all the necessary changes are added. The last patch fixes a lookup issue for `kexec -l -s` case when memory is reserved for crashkernel. Tested the changes successfully on P8, P9 lpars, couple of OpenPOWER boxes, one with secureboot enabled, KVM guest and a simulator. v4 -> v5: * Dropped patches 07/12 & 08/12 and updated purgatory to do everything in assembly. Hello Pingfan, Sorry, I missed out on responding to this. I guess you achieve this by carefully selecting instruction to avoid relocation issue, right? Yes. No far branching or reference to data from elsewhere. Thanks Hari
Re: bpfilter logging write errors in dmesg
Ho Rodrigo,
please try this patch:
diff --git a/net/bpfilter/bpfilter_kern.c b/net/bpfilter/bpfilter_kern.c
index 1905e01c3aa9a7..4494ea6056cdb8 100644
--- a/net/bpfilter/bpfilter_kern.c
+++ b/net/bpfilter/bpfilter_kern.c
@@ -39,7 +39,7 @@ static int __bpfilter_process_sockopt(struct sock *sk, int
optname,
{
struct mbox_request req;
struct mbox_reply reply;
- loff_t pos;
+ loff_t pos = 0;
ssize_t n;
int ret = -EFAULT;
Re: [PATCH v17 18/21] mm/lru: introduce the relock_page_lruvec function
在 2020/7/30 上午1:52, Alexander Duyck 写道:
>> + rcu_read_lock();
>> + locked = mem_cgroup_page_lruvec(page, pgdat) == locked_lruvec;
>> + rcu_read_unlock();
>> +
>> + if (locked)
>> + return locked_lruvec;
>> +
>> + if (locked_lruvec)
>> + unlock_page_lruvec_irqrestore(locked_lruvec, *flags);
>> +
>> + return lock_page_lruvec_irqsave(page, flags);
>> +}
>> +
> So looking these over they seem to be pretty inefficient for what they
> do. Basically in worst case (locked_lruvec == NULL) you end up calling
> mem_cgoup_page_lruvec and all the rcu_read_lock/unlock a couple times
> for a single page. It might make more sense to structure this like:
> if (locked_lruvec) {
Uh, we still need to check this page's lruvec, that needs a rcu_read_lock.
to save a mem_cgroup_page_lruvec call, we have to open lock_page_lruvec
as your mentained before.
> if (lruvec_holds_page_lru_lock(page, locked_lruvec))
> return locked_lruvec;
>
> unlock_page_lruvec_irqrestore(locked_lruvec, *flags);
> }
> return lock_page_lruvec_irqsave(page, flags);
>
> The other piece that has me scratching my head is that I wonder if we
> couldn't do this without needing the rcu_read_lock. For example, what
> if we were to compare the page mem_cgroup pointer to the memcg back
> pointer stored in the mem_cgroup_per_node? It seems like ordering
> things this way would significantly reduce the overhead due to the
> pointer chasing to see if the page is in the locked lruvec or not.
>
If page->mem_cgroup always be charged. the following could be better.
+/* Don't lock again iff page's lruvec locked */
+static inline struct lruvec *relock_page_lruvec_irqsave(struct page *page,
+ struct lruvec *locked_lruvec, unsigned long *flags)
+{
+ struct lruvec *lruvec;
+
+ if (mem_cgroup_disabled())
+ return locked_lruvec;
+
+ /* user page always be charged */
+ VM_BUG_ON_PAGE(!page->mem_cgroup, page);
+
+ rcu_read_lock();
+ if (likely(lruvec_memcg(locked_lruvec) == page->mem_cgroup)) {
+ rcu_read_unlock();
+ return locked_lruvec;
+ }
+
+ if (locked_lruvec)
+ unlock_page_lruvec_irqrestore(locked_lruvec, *flags);
+
+ lruvec = mem_cgroup_page_lruvec(page, page_pgdat(page));
+ spin_lock_irqsave(>lru_lock, *flags);
+ rcu_read_unlock();
+ lruvec_memcg_debug(lruvec, page);
+
+ return lruvec;
+}
+
The user page is always be charged since readahead page is charged now.
and looks we also can apply this patch. I will test it to see if there is
other exception.
commit 826128346e50f6c60c513e166998466b593becad
Author: Alex Shi
Date: Thu Jul 30 13:58:38 2020 +0800
mm/memcg: remove useless check on page->mem_cgroup
Since readahead page will be charged on memcg too. We don't need to
check this exception now.
Signed-off-by: Alex Shi
diff --git a/mm/memcontrol.c b/mm/memcontrol.c
index af96217f2ec5..0c7f6bed199b 100644
--- a/mm/memcontrol.c
+++ b/mm/memcontrol.c
@@ -1336,12 +1336,6 @@ struct lruvec *mem_cgroup_page_lruvec(struct page *page,
struct pglist_data *pgd
VM_BUG_ON_PAGE(PageTail(page), page);
memcg = READ_ONCE(page->mem_cgroup);
- /*
-* Swapcache readahead pages are added to the LRU - and
-* possibly migrated - before they are charged.
-*/
- if (!memcg)
- memcg = root_mem_cgroup;
mz = mem_cgroup_page_nodeinfo(memcg, page);
lruvec = >lruvec;
@@ -6962,10 +6956,7 @@ void mem_cgroup_migrate(struct page *oldpage, struct
page *newpage)
if (newpage->mem_cgroup)
return;
- /* Swapcache readahead pages can get replaced before being charged */
memcg = oldpage->mem_cgroup;
- if (!memcg)
- return;
/* Force-charge the new page. The old one will be freed soon */
nr_pages = thp_nr_pages(newpage);
@@ -7160,10 +7151,6 @@ void mem_cgroup_swapout(struct page *page, swp_entry_t
entry)
memcg = page->mem_cgroup;
- /* Readahead page, never charged */
- if (!memcg)
- return;
-
/*
* In case the memcg owning these pages has been offlined and doesn't
* have an ID allocated to it anymore, charge the closest online
[PATCH 4/8] modules: unexport __module_text_address
__module_text_address is only used by built-in code. Signed-off-by: Christoph Hellwig --- kernel/module.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/module.c b/kernel/module.c index feeaa9629eb179..d241866f9d4a2b 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -4508,7 +4508,6 @@ struct module *__module_text_address(unsigned long addr) } return mod; } -EXPORT_SYMBOL_GPL(__module_text_address); /* Don't grab lock, we're oopsing. */ void print_modules(void) -- 2.27.0
[PATCH 5/8] modules: unexport __module_address
__module_address is only used by built-in code. Signed-off-by: Christoph Hellwig --- kernel/module.c | 1 - 1 file changed, 1 deletion(-) diff --git a/kernel/module.c b/kernel/module.c index d241866f9d4a2b..54e853c7212f72 100644 --- a/kernel/module.c +++ b/kernel/module.c @@ -4469,7 +4469,6 @@ struct module *__module_address(unsigned long addr) } return mod; } -EXPORT_SYMBOL_GPL(__module_address); /* * is_module_text_address - is this address inside module code? -- 2.27.0
inherit TAINT_PROPRIETARY_MODULE v2
Hi Jessica, we've had a bug in our resolution of _GPL modules since day one, that is a module can claim to be GPL licensed and use _GPL exports, while it also depends on symbols from non-GPL modules. This is used as a circumvention of the _GPL exports by using a small shim module using the _GPL exports and the other functionality. A recent example can be found here: https://lore.kernel.org/netdev/6376ca34-bc6f-45de-9ffd-7e32664c7...@fb.com/T/#md514322fdfa212afe9f1d3eb4e5f7eaefece36eb Changes since v1: - standardize on one spelling of "license" - fix a commit message type Diffstat: include/linux/module.h | 26 +++--- kernel/module.c| 46 +- 2 files changed, 32 insertions(+), 40 deletions(-)
[PATCH 2/8] modules: mark find_symbol static
find_symbol is only used in module.c.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 11 ---
kernel/module.c| 3 +--
2 files changed, 1 insertion(+), 13 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index f1fdbeef2153a8..90bdc362be3681 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -590,17 +590,6 @@ struct symsearch {
bool unused;
};
-/*
- * Search for an exported symbol by name.
- *
- * Must be called with module_mutex held or preemption disabled.
- */
-const struct kernel_symbol *find_symbol(const char *name,
- struct module **owner,
- const s32 **crc,
- bool gplok,
- bool warn);
-
/*
* Walk the exported symbol table
*
diff --git a/kernel/module.c b/kernel/module.c
index 17d64dae756c80..84da96a6d8241c 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -585,7 +585,7 @@ static bool find_exported_symbol_in_section(const struct
symsearch *syms,
/* Find an exported symbol and return it, along with, (optional) crc and
* (optional) module which owns it. Needs preempt disabled or module_mutex. */
-const struct kernel_symbol *find_symbol(const char *name,
+static const struct kernel_symbol *find_symbol(const char *name,
struct module **owner,
const s32 **crc,
bool gplok,
@@ -608,7 +608,6 @@ const struct kernel_symbol *find_symbol(const char *name,
pr_debug("Failed to find symbol %s\n", name);
return NULL;
}
-EXPORT_SYMBOL_GPL(find_symbol);
/*
* Search for module by name: must hold module_mutex (or preempt disabled
--
2.27.0
Re: [PATCH] cpufreq: cached_resolved_idx can not be negative
On 30-07-20, 11:29, Amit Kucheria wrote:
> On Thu, Jul 30, 2020 at 9:38 AM Viresh Kumar wrote:
> >
> > It is not possible for cached_resolved_idx to be invalid here as the
> > cpufreq core always sets index to a positive value.
> >
> > Change its type to unsigned int and fix qcom usage a bit.
>
> Shouldn't you fix up idx in cpufreq_driver_resolve_freq() to be
> unsigned int too?
Yes, merged this into the patch.
diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
index 0128de3603df..053d72e52a31 100644
--- a/drivers/cpufreq/cpufreq.c
+++ b/drivers/cpufreq/cpufreq.c
@@ -538,7 +538,7 @@ unsigned int cpufreq_driver_resolve_freq(struct
cpufreq_policy *policy,
policy->cached_target_freq = target_freq;
if (cpufreq_driver->target_index) {
- int idx;
+ unsigned int idx;
idx = cpufreq_frequency_table_target(policy, target_freq,
CPUFREQ_RELATION_L);
--
viresh
[PATCH 3/8] modules: mark each_symbol_section static
each_symbol_section is only used inside of module.c.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 9 -
kernel/module.c| 3 +--
2 files changed, 1 insertion(+), 11 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 90bdc362be3681..b79219eed83c56 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -590,15 +590,6 @@ struct symsearch {
bool unused;
};
-/*
- * Walk the exported symbol table
- *
- * Must be called with module_mutex held or preemption disabled.
- */
-bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
- struct module *owner,
- void *data), void *data);
-
/* Returns 0 and fills in value, defined and namebuf, or -ERANGE if
symnum out of range. */
int module_get_kallsym(unsigned int symnum, unsigned long *value, char *type,
diff --git a/kernel/module.c b/kernel/module.c
index 84da96a6d8241c..feeaa9629eb179 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -422,7 +422,7 @@ static bool each_symbol_in_section(const struct symsearch
*arr,
}
/* Returns true as soon as fn returns true, otherwise false. */
-bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
+static bool each_symbol_section(bool (*fn)(const struct symsearch *arr,
struct module *owner,
void *data),
void *data)
@@ -484,7 +484,6 @@ bool each_symbol_section(bool (*fn)(const struct symsearch
*arr,
}
return false;
}
-EXPORT_SYMBOL_GPL(each_symbol_section);
struct find_symbol_arg {
/* Input */
--
2.27.0
[PATCH 6/8] modules: rename the licence field in struct symsearch to license
Use the same spelling variant as the rest of the file.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 2 +-
kernel/module.c| 4 ++--
2 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index b79219eed83c56..be04ba2f881daa 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -586,7 +586,7 @@ struct symsearch {
NOT_GPL_ONLY,
GPL_ONLY,
WILL_BE_GPL_ONLY,
- } licence;
+ } license;
bool unused;
};
diff --git a/kernel/module.c b/kernel/module.c
index 54e853c7212f72..a0df783a8a6e5b 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -504,9 +504,9 @@ static bool check_exported_symbol(const struct symsearch
*syms,
struct find_symbol_arg *fsa = data;
if (!fsa->gplok) {
- if (syms->licence == GPL_ONLY)
+ if (syms->license == GPL_ONLY)
return false;
- if (syms->licence == WILL_BE_GPL_ONLY && fsa->warn) {
+ if (syms->license == WILL_BE_GPL_ONLY && fsa->warn) {
pr_warn("Symbol %s is being used by a non-GPL module, "
"which will not be allowed in the future\n",
fsa->name);
--
2.27.0
[PATCH 8/8] modules: inherit TAINT_PROPRIETARY_MODULE
If a TAINT_PROPRIETARY_MODULE exports symbol, inherit the taint flag
for all modules importing these symbols, and don't allow loading
symbols from TAINT_PROPRIETARY_MODULE modules if the module previously
imported gplonly symbols. Add a anti-circumvention devices so people
don't accidentally get themselves into trouble this way.
Comment from Greg:
Ah, the proven-to-be-illegal "GPL Condom" defense :)
Signed-off-by: Christoph Hellwig
Reviewed-by: Greg Kroah-Hartman
---
include/linux/module.h | 1 +
kernel/module.c| 12
2 files changed, 13 insertions(+)
diff --git a/include/linux/module.h b/include/linux/module.h
index 30b0f5fcdb3c37..e30ed5fa33a738 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -389,6 +389,7 @@ struct module {
unsigned int num_gpl_syms;
const struct kernel_symbol *gpl_syms;
const s32 *gpl_crcs;
+ bool using_gplonly_symbols;
#ifdef CONFIG_UNUSED_SYMBOLS
/* unused exported symbols. */
diff --git a/kernel/module.c b/kernel/module.c
index afb2bfdd5134b3..04f993863ae417 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -1456,6 +1456,18 @@ static const struct kernel_symbol *resolve_symbol(struct
module *mod,
if (!sym)
goto unlock;
+ if (license == GPL_ONLY)
+ mod->using_gplonly_symbols = true;
+
+ if (owner && test_bit(TAINT_PROPRIETARY_MODULE, >taints)) {
+ if (mod->using_gplonly_symbols) {
+ sym = NULL;
+ goto getname;
+ }
+ add_taint_module(mod, TAINT_PROPRIETARY_MODULE,
+LOCKDEP_NOW_UNRELIABLE);
+ }
+
if (!check_version(info, name, mod, crc)) {
sym = ERR_PTR(-EINVAL);
goto getname;
--
2.27.0
[PATCH 7/8] modules: return licensing information from find_symbol
Report the GPLONLY status through a new argument.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 2 +-
kernel/module.c| 16 +++-
2 files changed, 12 insertions(+), 6 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index be04ba2f881daa..30b0f5fcdb3c37 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -582,7 +582,7 @@ struct module *find_module(const char *name);
struct symsearch {
const struct kernel_symbol *start, *stop;
const s32 *crcs;
- enum {
+ enum mod_license {
NOT_GPL_ONLY,
GPL_ONLY,
WILL_BE_GPL_ONLY,
diff --git a/kernel/module.c b/kernel/module.c
index a0df783a8a6e5b..afb2bfdd5134b3 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -495,6 +495,7 @@ struct find_symbol_arg {
struct module *owner;
const s32 *crc;
const struct kernel_symbol *sym;
+ enum mod_license license;
};
static bool check_exported_symbol(const struct symsearch *syms,
@@ -528,6 +529,7 @@ static bool check_exported_symbol(const struct symsearch
*syms,
fsa->owner = owner;
fsa->crc = symversion(syms->crcs, symnum);
fsa->sym = >start[symnum];
+ fsa->license = syms->license;
return true;
}
@@ -587,6 +589,7 @@ static bool find_exported_symbol_in_section(const struct
symsearch *syms,
static const struct kernel_symbol *find_symbol(const char *name,
struct module **owner,
const s32 **crc,
+ enum mod_license *license,
bool gplok,
bool warn)
{
@@ -601,6 +604,8 @@ static const struct kernel_symbol *find_symbol(const char
*name,
*owner = fsa.owner;
if (crc)
*crc = fsa.crc;
+ if (license)
+ *license = fsa.license;
return fsa.sym;
}
@@ -1074,7 +1079,7 @@ void __symbol_put(const char *symbol)
struct module *owner;
preempt_disable();
- if (!find_symbol(symbol, , NULL, true, false))
+ if (!find_symbol(symbol, , NULL, NULL, true, false))
BUG();
module_put(owner);
preempt_enable();
@@ -1352,7 +1357,7 @@ static inline int check_modstruct_version(const struct
load_info *info,
* locking is necessary -- use preempt_disable() to placate lockdep.
*/
preempt_disable();
- if (!find_symbol("module_layout", NULL, , true, false)) {
+ if (!find_symbol("module_layout", NULL, , NULL, true, false)) {
preempt_enable();
BUG();
}
@@ -1436,6 +1441,7 @@ static const struct kernel_symbol *resolve_symbol(struct
module *mod,
struct module *owner;
const struct kernel_symbol *sym;
const s32 *crc;
+ enum mod_license license;
int err;
/*
@@ -1445,7 +1451,7 @@ static const struct kernel_symbol *resolve_symbol(struct
module *mod,
*/
sched_annotate_sleep();
mutex_lock(_mutex);
- sym = find_symbol(name, , ,
+ sym = find_symbol(name, , , ,
!(mod->taints & (1 << TAINT_PROPRIETARY_MODULE)),
true);
if (!sym)
goto unlock;
@@ -2216,7 +,7 @@ void *__symbol_get(const char *symbol)
const struct kernel_symbol *sym;
preempt_disable();
- sym = find_symbol(symbol, , NULL, true, true);
+ sym = find_symbol(symbol, , NULL, NULL, true, true);
if (sym && strong_try_module_get(owner))
sym = NULL;
preempt_enable();
@@ -2252,7 +2258,7 @@ static int verify_exported_symbols(struct module *mod)
for (i = 0; i < ARRAY_SIZE(arr); i++) {
for (s = arr[i].sym; s < arr[i].sym + arr[i].num; s++) {
if (find_symbol(kernel_symbol_name(s), , NULL,
- true, false)) {
+ NULL, true, false)) {
pr_err("%s: exports duplicate symbol %s"
" (owned by %s)\n",
mod->name, kernel_symbol_name(s),
--
2.27.0
[PATCH 1/8] modules: mark ref_module static
ref_module isn't used anywhere outside of module.c.
Signed-off-by: Christoph Hellwig
---
include/linux/module.h | 1 -
kernel/module.c| 6 ++
2 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/include/linux/module.h b/include/linux/module.h
index 2e6670860d275f..f1fdbeef2153a8 100644
--- a/include/linux/module.h
+++ b/include/linux/module.h
@@ -657,7 +657,6 @@ static inline void __module_get(struct module *module)
#define symbol_put_addr(p) do { } while (0)
#endif /* CONFIG_MODULE_UNLOAD */
-int ref_module(struct module *a, struct module *b);
/* This is a #define so the string doesn't get put in every .o file */
#define module_name(mod) \
diff --git a/kernel/module.c b/kernel/module.c
index aa183c9ac0a256..17d64dae756c80 100644
--- a/kernel/module.c
+++ b/kernel/module.c
@@ -869,7 +869,7 @@ static int add_module_usage(struct module *a, struct module
*b)
}
/* Module a uses b: caller needs module_mutex() */
-int ref_module(struct module *a, struct module *b)
+static int ref_module(struct module *a, struct module *b)
{
int err;
@@ -888,7 +888,6 @@ int ref_module(struct module *a, struct module *b)
}
return 0;
}
-EXPORT_SYMBOL_GPL(ref_module);
/* Clear the unload stuff of the module. */
static void module_unload_free(struct module *mod)
@@ -1169,11 +1168,10 @@ static inline void module_unload_free(struct module
*mod)
{
}
-int ref_module(struct module *a, struct module *b)
+static int ref_module(struct module *a, struct module *b)
{
return strong_try_module_get(b);
}
-EXPORT_SYMBOL_GPL(ref_module);
static inline int module_unload_init(struct module *mod)
{
--
2.27.0
[PATCH] uacce: fix some coding styles
1. delete some redundant code.
2. modify the module author information.
Signed-off-by: Kai Ye
---
Changes in V2:
deleted extra NULL pointer check in uacce_fops.
drivers/misc/uacce/uacce.c | 13 +
1 file changed, 1 insertion(+), 12 deletions(-)
diff --git a/drivers/misc/uacce/uacce.c b/drivers/misc/uacce/uacce.c
index 107028e..1d09707 100644
--- a/drivers/misc/uacce/uacce.c
+++ b/drivers/misc/uacce/uacce.c
@@ -239,17 +239,6 @@ static int uacce_fops_mmap(struct file *filep, struct
vm_area_struct *vma)
switch (type) {
case UACCE_QFRT_MMIO:
- if (!uacce->ops->mmap) {
- ret = -EINVAL;
- goto out_with_lock;
- }
-
- ret = uacce->ops->mmap(q, vma, qfr);
- if (ret)
- goto out_with_lock;
-
- break;
-
case UACCE_QFRT_DUS:
if (!uacce->ops->mmap) {
ret = -EINVAL;
@@ -541,5 +530,5 @@ subsys_initcall(uacce_init);
module_exit(uacce_exit);
MODULE_LICENSE("GPL");
-MODULE_AUTHOR("Hisilicon Tech. Co., Ltd.");
+MODULE_AUTHOR("HiSilicon Tech. Co., Ltd.");
MODULE_DESCRIPTION("Accelerator interface for Userland applications");
--
2.8.1
[PATCH net-next] liquidio: Replace vmalloc with kmalloc in octeon_register_dispatch_fn()
The size of struct octeon_dispatch is too small, it is better to use
kmalloc instead of vmalloc.
Suggested-by: Joe Perches
Signed-off-by: Wang Hai
---
drivers/net/ethernet/cavium/liquidio/octeon_device.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index 934115d18488..ac32facaa427 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -1056,7 +1056,7 @@ void octeon_delete_dispatch_list(struct octeon_device
*oct)
list_for_each_safe(temp, tmp2, ) {
list_del(temp);
- vfree(temp);
+ kfree(temp);
}
}
@@ -1152,13 +1152,10 @@ octeon_register_dispatch_fn(struct octeon_device *oct,
dev_dbg(>pci_dev->dev,
"Adding opcode to dispatch list linked list\n");
- dispatch = (struct octeon_dispatch *)
- vmalloc(sizeof(struct octeon_dispatch));
- if (!dispatch) {
- dev_err(>pci_dev->dev,
- "No memory to add dispatch function\n");
+ dispatch = kmalloc(sizeof(*dispatch), GFP_KERNEL);
+ if (!dispatch)
return 1;
- }
+
dispatch->opcode = combined_opcode;
dispatch->dispatch_fn = fn;
dispatch->arg = fn_arg;
--
2.17.1
Re: linux-next: build failure after merge of the origin tree
On Thu, Jul 30, 2020 at 05:22:50AM +0200, Willy Tarreau wrote:
> On Wed, Jul 29, 2020 at 08:17:48PM -0700, Kees Cook wrote:
> > And just another heads-up, the patch[1] (which was never sent to a public
> > list) also breaks arm64 (circular header needs?):
> (...)
>
> Definitely, we've just got a report about this, I'll have a look once
> I'm at the office. I'd like to check that we don't obviously break
> another arch by just removing percpu. If at least shuffling them around
> is sufficient that'd be nice. Otherwise we'll likely need a separate
> header (which is not a bad thing for the long term).
So Linus proposed a clean solution which might be harder to backport
but looks better for 5.8. However the attached one addresses the issue
for me on arm64 and still works on x86_64, arm, mips. I think we should
go with this one first then apply Linus' one on top of it to be long
term proof, and backport only the first one. Linus ?
Willy
>From 18fba9e2dfb16605a722e01f95d9e2d020efaa42 Mon Sep 17 00:00:00 2001
From: Willy Tarreau
Date: Thu, 30 Jul 2020 07:59:24 +0200
Subject: random: fix circular include dependency on arm64 after addition of
percpu.h
MIME-Version: 1.0
Content-Type: text/plain; charset=latin1
Content-Transfer-Encoding: 8bit
Daniel Díaz and Kees Cook independently reported that commit f227e3ec3b5c
("random32: update the net random state on interrupt and activity") broke
arm64 due to a circular dependency on include files since the addition of
percpu.h in random.h.
The correct fix would definitely be to move all the prandom32 stuff out
of random.h but for backporting, a smaller solution is preferred. This
one replaces linux/percpu.h with asm/percpu.h, and this fixes the problem
on x86_64, arm64, arm, and mips. Note that moving percpu.h around didn't
change anything and that removing it entirely broke differently. When
backporting, such options might still be considered if this patch fails
to help.
Reported-by: Daniel Díaz
Reported-by: Kees Cook
Fixes: f227e3ec3b5c
Cc: Stephen Rothwell
Cc: Linus Torvalds
Signed-off-by: Willy Tarreau
---
include/linux/random.h | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/include/linux/random.h b/include/linux/random.h
index f310897f051d..9ab7443bd91b 100644
--- a/include/linux/random.h
+++ b/include/linux/random.h
@@ -11,7 +11,7 @@
#include
#include
#include
-#include
+#include
#include
--
2.20.1
[PATCH 1/1] x86/tsr: Fix tsc frequency enumeration failure on lightning mountain SoC
Frequency descriptor of Lightning Mountain SoC doesn't have all the
frequency entries so resulting in the below failure causing kernel hang.
[0.00] Error MSR_FSB_FREQ index 15 is unknown
[0.00] tsc: Fast TSC calibration failed
So, add all the frequency entries in the Lightning Mountain SoC frequency
descriptor.
Fixes: 0cc5359d8fd45 ("x86/cpu: Update init data for new Airmont CPU model")
Fixes: 812c2d7506fd ("x86/tsc_msr: Use named struct initializers")
Signed-off-by: Dilip Kota
---
arch/x86/kernel/tsc_msr.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/arch/x86/kernel/tsc_msr.c b/arch/x86/kernel/tsc_msr.c
index 4fec6f3a1858b2..c255e10e914aa5 100644
--- a/arch/x86/kernel/tsc_msr.c
+++ b/arch/x86/kernel/tsc_msr.c
@@ -136,7 +136,8 @@ static const struct freq_desc freq_desc_ann = {
/* 24 MHz crystal? : 24 * 13 / 4 = 78 MHz */
static const struct freq_desc freq_desc_lgm = {
.use_msr_plat = true,
- .freqs = { 78000, 78000, 78000, 78000, 78000, 78000, 78000, 78000 },
+ .freqs = { 78000, 78000, 78000, 78000, 78000, 78000, 78000, 78000,
78000,
+ 78000, 78000, 78000, 78000, 78000, 78000, 78000 },
.mask = 0x0f,
};
--
2.11.0
Re: [PATCH v18 3/3] Input: new da7280 haptic driver
[Adding vsprintf maintainers to Cc:]
Hello,
On Wed, Jul 29, 2020 at 10:06:53PM -0700, Dmitry Torokhov wrote:
> On Wed, Jul 29, 2020 at 09:21:45AM +0200, Uwe Kleine-König wrote:
> > Hello,
> >
> > On Tue, Jul 28, 2020 at 11:36:38PM -0700, Dmitry Torokhov wrote:
> > > > v9:
> > > > - Removed the header file and put the definitions into the c
> > > > file.
> > > > - Updated the pwm code and error logs with %pE
> > >
> > > I believe the %pE is to format an escaped buffer, you probably want to
> > > %pe (lowercase) to print errors. I am also not quite sure if we want to
> > > use it in cases when we have non-pointer error, or we should stick with
> > > %d as most of the kernel does.
> >
> > compared with %d %pe is easier to understand as it emits "-ETIMEOUT"
> > instead of "-110". And yes, %pE is wrong.
>
> While I can see that symbolic name instead of a numeric constant might
> be appealing, I do not believe that we want fragments like this with
> endless conversions between integer and pointer errors:
>
> if (haptics->const_op_mode == DA7280_PWM_MODE) {
> haptics->pwm_dev = devm_pwm_get(dev, NULL);
> if (IS_ERR(haptics->pwm_dev)) {
> error = PTR_ERR(haptics->pwm_dev);
> if (error != -EPROBE_DEFER)
> dev_err(dev, "unable to request PWM: %pE\n",
> ERR_PTR(error));
> return error;
> }
>
> Maybe we should introduce something like '%de' for the integer error
> case?
I suggested that some time ago with limited success, see
https://lore.kernel.org/lkml/20200129115516.zsvxu56e6h7gh...@pathway.suse.cz/
.
> In the meantime I would prefer using %d when we have integer
> error. We should not see these error messages anyway ;)
I don't agree. Error messages are supposed to be helpful and I prefer
some casting between error pointer and error int over emitting bare
numbers to the kernel log. (And additionally the uglyness might help to
convince the vsprintf guys that %de might be a good idea after all :-)
Best regards
Uwe
--
Pengutronix e.K. | Uwe Kleine-König|
Industrial Linux Solutions | https://www.pengutronix.de/ |
signature.asc
Description: PGP signature
Re: [PATCH net-next] liquidio: Remove unneeded cast from memory allocation
在 2020/7/28 23:54, Joe Perches 写道:
On Tue, 2020-07-28 at 21:38 +0800, wanghai (M) wrote:
Thanks for your explanation. I got it.
Can it be modified like this?
[]
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -1152,11 +1152,8 @@ octeon_register_dispatch_fn(struct octeon_device
*oct,
dev_dbg(>pci_dev->dev,
"Adding opcode to dispatch list linked list\n");
- dispatch = (struct octeon_dispatch *)
- vmalloc(sizeof(struct octeon_dispatch));
+ dispatch = kmalloc(sizeof(struct octeon_dispatch),
GFP_KERNEL);
if (!dispatch) {
- dev_err(>pci_dev->dev,
- "No memory to add dispatch function\n");
return 1;
}
dispatch->opcode = combined_opcode;
Yes, but the free also needs to be changed.
I think it's:
---
drivers/net/ethernet/cavium/liquidio/octeon_device.c | 11 ---
1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
index 934115d18488..4ee4cb946e1d 100644
--- a/drivers/net/ethernet/cavium/liquidio/octeon_device.c
+++ b/drivers/net/ethernet/cavium/liquidio/octeon_device.c
@@ -1056,7 +1056,7 @@ void octeon_delete_dispatch_list(struct octeon_device
*oct)
list_for_each_safe(temp, tmp2, ) {
list_del(temp);
- vfree(temp);
+ kfree(temp);
}
}
@@ -1152,13 +1152,10 @@ octeon_register_dispatch_fn(struct octeon_device *oct,
dev_dbg(>pci_dev->dev,
"Adding opcode to dispatch list linked list\n");
- dispatch = (struct octeon_dispatch *)
- vmalloc(sizeof(struct octeon_dispatch));
- if (!dispatch) {
- dev_err(>pci_dev->dev,
- "No memory to add dispatch function\n");
+ dispatch = kmalloc(sizeof(struct octeon_dispatch), GFP_KERNEL);
+ if (!dispatch)
return 1;
- }
+
dispatch->opcode = combined_opcode;
dispatch->dispatch_fn = fn;
dispatch->arg = fn_arg;
.
Thanks for your suggestion. I just sent another patch for this.
"[PATCH net-next] liquidio: Replace vmalloc with kmalloc in
octeon_register_dispatch_fn()"
Re: [PATCH v4 2/4] input: gpio-vibra: Allow to use vcc-supply alone to control the vibrator
Hi Ondrej,
On Tue, Jul 14, 2020 at 12:23:01PM +0200, Ondrej Jirman wrote:
> Make enable-gpio optional to allow using this driver with boards that
> have vibrator connected to a power supply without intermediate gpio
> based enable circuitry.
>
> Also avoid a case where neither regulator nor enable gpio is specified,
> and bail out in probe in such a case.
>
> Signed-off-by: Ondrej Jirman
> ---
> drivers/input/misc/gpio-vibra.c | 14 ++
> 1 file changed, 10 insertions(+), 4 deletions(-)
>
> diff --git a/drivers/input/misc/gpio-vibra.c b/drivers/input/misc/gpio-vibra.c
> index f79f75595dd7..b3bb7e61ed1d 100644
> --- a/drivers/input/misc/gpio-vibra.c
> +++ b/drivers/input/misc/gpio-vibra.c
> @@ -39,7 +39,7 @@ static int gpio_vibrator_start(struct gpio_vibrator
> *vibrator)
> struct device *pdev = vibrator->input->dev.parent;
> int err;
>
> - if (!vibrator->vcc_on) {
> + if (vibrator->vcc && !vibrator->vcc_on) {
> err = regulator_enable(vibrator->vcc);
> if (err) {
> dev_err(pdev, "failed to enable regulator: %d\n", err);
> @@ -57,7 +57,7 @@ static void gpio_vibrator_stop(struct gpio_vibrator
> *vibrator)
> {
> gpiod_set_value_cansleep(vibrator->gpio, 0);
>
> - if (vibrator->vcc_on) {
> + if (vibrator->vcc && vibrator->vcc_on) {
> regulator_disable(vibrator->vcc);
> vibrator->vcc_on = false;
> }
> @@ -112,7 +112,7 @@ static int gpio_vibrator_probe(struct platform_device
> *pdev)
> if (!vibrator->input)
> return -ENOMEM;
>
> - vibrator->vcc = devm_regulator_get(>dev, "vcc");
> + vibrator->vcc = devm_regulator_get_optional(>dev, "vcc");
I know it is very surprising, but regulator_get_optional does not return
NULL when regulator is not present, but rather ERR_PTR(-ENODEV). You
need to replace it with NULL in the branch below, or change conditions
to !IS_ERR(virbrator->vcc) (and still handle -ENODEV in the branch
below).
> err = PTR_ERR_OR_ZERO(vibrator->vcc);
> if (err) {
> if (err != -EPROBE_DEFER)
> @@ -121,7 +121,8 @@ static int gpio_vibrator_probe(struct platform_device
> *pdev)
> return err;
> }
>
> - vibrator->gpio = devm_gpiod_get(>dev, "enable", GPIOD_OUT_LOW);
> + vibrator->gpio = devm_gpiod_get_optional(>dev, "enable",
> + GPIOD_OUT_LOW);
> err = PTR_ERR_OR_ZERO(vibrator->gpio);
> if (err) {
> if (err != -EPROBE_DEFER)
> @@ -130,6 +131,11 @@ static int gpio_vibrator_probe(struct platform_device
> *pdev)
> return err;
> }
>
> + if (!vibrator->vcc && !vibrator->gpio) {
> + dev_err(>dev, "Neither gpio nor regulator provided\n");
> + return -EINVAL;
> + }
> +
> INIT_WORK(>play_work, gpio_vibrator_play_work);
>
> vibrator->input->name = "gpio-vibrator";
> --
> 2.27.0
>
Thanks.
--
Dmitry
RE: [PATCH] remoteproc: virtio: support sharing vdev buffer
Hi Mathieu,
> Subject: Re: [PATCH] remoteproc: virtio: support sharing vdev buffer
>
> Hi Peng,
>
> On Wed, Jul 22, 2020 at 09:15:43PM +0800, Peng Fan wrote:
> > Support sharing vdev buffer between multiple vdevs by using name
> > "vdevbuffer".
> >
> > Reviewed-by: Richard Zhu
> > Signed-off-by: Peng Fan
> > ---
> > drivers/remoteproc/remoteproc_virtio.c | 2 ++
> > 1 file changed, 2 insertions(+)
> >
> > diff --git a/drivers/remoteproc/remoteproc_virtio.c
> > b/drivers/remoteproc/remoteproc_virtio.c
> > index dfd3808c34fd..5d78ebea111e 100644
> > --- a/drivers/remoteproc/remoteproc_virtio.c
> > +++ b/drivers/remoteproc/remoteproc_virtio.c
> > @@ -343,6 +343,8 @@ int rproc_add_virtio_dev(struct rproc_vdev *rvdev,
> > int id)
> >
> > /* Try to find dedicated vdev buffer carveout */
> > mem = rproc_find_carveout_by_name(rproc, "vdev%dbuffer",
> > rvdev->index);
> > + if (!mem)
> > + mem = rproc_find_carveout_by_name(rproc, "vdevbuffer");
>
> We already have a way to share buffers [1], do you think it would work for
> you?
> I would rather proceed that way to avoid introducing a 3rd way to deal with
> vdev buffers.
That should work for me. I just need to move the vdevbuffer to the 1st one
inside the memory-region.
BTW: Do you have time to give a review for
https://patchwork.kernel.org/patch/11688751/
Thanks,
Peng.
>
> Thanks,
> Mathieu
>
> [1].
> https://eur01.safelinks.protection.outlook.com/?url=https%3A%2F%2Felixir.b
> ootlin.com%2Flinux%2Fv5.8-rc4%2Fsource%2Fdrivers%2Fremoteproc%2Frem
> oteproc_virtio.c%23L389data=02%7C01%7Cpeng.fan%40nxp.com%7C
> 760ae238005c49fe20db08d833e193ca%7C686ea1d3bc2b4c6fa92cd99c5c30
> 1635%7C0%7C0%7C637316391223184319sdata=UTN8YVaEynomrwFx
> 7aTCJ7fp0fjfIS8fscfGG67naKk%3Dreserved=0
>
> > if (mem) {
> > phys_addr_t pa;
> >
> > --
> > 2.16.4
> >
Re: [PATCH] exfat: retain 'VolumeFlags' properly
Ping.. On 2020/07/15 19:06, Tetsuhiro Kohada wrote: It looks complicated. It would be better to simply set/clear VOLUME DIRTY bit. I think exfat_set_vol_flags() gets a little complicated, because it needs the followings (with bit operation) a) Set/Clear VOLUME_DIRTY. b) Set MEDIA_FAILUR. How about splitting these into separate functions as below? exfat_set_volume_dirty() exfat_set_vol_flags(sb, sbi->vol_flag | VOLUME_DIRTY); exfat_clear_volume_dirty() exfat_set_vol_flags(sb, sbi->vol_flag & ~VOLUME_DIRTY); exfat_set_media_failure() exfat_set_vol_flags(sb, sbi->vol_flag | MEDIA_FAILURE); The implementation is essentially the same for exfat_set_vol_flags(), but I think the intention of the operation will be easier to understand. BR --- Tetsuhiro Kohada
Re: [PATCH 2/2] thermal: cpufreq_cooling: Reuse effective_cpu_util()
On 17-07-20, 11:46, Vincent Guittot wrote: > On Thu, 16 Jul 2020 at 16:24, Lukasz Luba wrote: > > On 7/16/20 12:56 PM, Peter Zijlstra wrote: > > > Currently cpufreq_cooling appears to estimate the CPU energy usage by > > > calculating the percentage of idle time using the per-cpu cpustat stuff, > > > which is pretty horrific. > > > > Even worse, it then *samples* the *current* CPU frequency at that > > particular point in time and assumes that when the CPU wasn't idle > > during that period - it had *this* frequency... > > So there is 2 problems in the power calculation of cpufreq cooling device : > - How to get an accurate utilization level of the cpu which is what > this patch is trying to fix because using idle time is just wrong > whereas scheduler utilization is frequency invariant Since this patch is targeted only towards fixing this particular problem, should I change something in the patch to make it acceptable ? > - How to get power estimate from this utilization level. And as you > pointed out, using the current freq which is not accurate. This should be tackled separately I believe. -- viresh
Re: add file system helpers that take kernel pointers for the init code v4
On Wed, Jul 29, 2020 at 08:51:17PM +0100, Al Viro wrote: > On Tue, Jul 28, 2020 at 06:33:53PM +0200, Christoph Hellwig wrote: > > Hi Al and Linus, > > > > currently a lot of the file system calls in the early in code (and the > > devtmpfs kthread) rely on the implicit set_fs(KERNEL_DS) during boot. > > This is one of the few last remaining places we need to deal with to kill > > off set_fs entirely, so this series adds new helpers that take kernel > > pointers. These helpers are in init/ and marked __init and thus will > > be discarded after bootup. A few also need to be duplicated in devtmpfs, > > though unfortunately. > > > > The series sits on top of my previous > > > > "decruft the early init / initrd / initramfs code v2" > > Could you fold the fixes in the parent branch to avoid the bisect hazards? > As it is, you have e.g. "initd: pass a non-f_pos offset to > kernel_read/kernel_write" > that ought to go into "initrd: switch initrd loading to struct file based > APIs"... I'm not a huge fan of rebasing after it has been out for a long time and with pending other patches on top of it. But at your request I've now folded the fixes and force pushed it.
Re: [PATCH 1/2] reset-controller: ti: adjust the reset assert and deassert interface
On Wed, 2020-07-29 at 16:02 +0800, Philipp Zabel wrote:
> Hi Crystal, Matthias,
>
> On Wed, 2020-07-29 at 09:48 +0200, Matthias Brugger wrote:
> >
> > On 29/07/2020 09:39, Crystal Guo wrote:
> > > Add ti_syscon_reset() to integrate assert and deassert together,
> > > and change return value of the reset assert and deassert interface
> > > from regmap_update_bits to regmap_write_bits.
> > >
> > > when clear bit is already 1, regmap_update_bits can not write 1 to it
> > > again.
> > > Some IC has the feature that, when set bit is 1, the clear bit change
> > > to 1 together. It will truly clear bit to 0 by write 1 to the clear bit
> > >
> > > Signed-off-by: Crystal Guo
> > > ---
> > > drivers/reset/reset-ti-syscon.c | 13 +++--
> > > 1 file changed, 11 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/reset/reset-ti-syscon.c
> > > b/drivers/reset/reset-ti-syscon.c
> > > index a2635c2..5a8ec8f 100644
> > > --- a/drivers/reset/reset-ti-syscon.c
> > > +++ b/drivers/reset/reset-ti-syscon.c
> > > @@ -89,7 +89,7 @@ static int ti_syscon_reset_assert(struct
> > > reset_controller_dev *rcdev,
> > > mask = BIT(control->assert_bit);
> > > value = (control->flags & ASSERT_SET) ? mask : 0x0;
> > >
> > > - return regmap_update_bits(data->regmap, control->assert_offset, mask,
> > > value);
> > > + return regmap_write_bits(data->regmap, control->assert_offset, mask,
> > > value);
> >
> > Nack, this will break the driver for the other devices.
>
> I don't think this will break the driver for existing hardware.
> regmap_write_bits() is the same as regmap_update_bits(), it just forces
> the write in case the read already happens to return the correct value.
> Of course it would be good to check that this actually works.
Yes, regmap_write_bits() is the same as regmap_update_bits(), it would
not affect existed users. Or should I use a property to separate
regmap_write_bits() and regmap_update_bits() ?
>
> > The kernel has to work not just for your SoC but for all devices of all
> > architectures. You can't just hack something up, that will work on your
> > specific
> > SoC.
> >
> > Regards,
> > Matthias
> >
This TI driver was intend to be a generic reset controller
(https://lore.kernel.org/patchwork/cover/683585/), so this patch may
not just work on a specific SoC.
Thanks,
Crystal
> > > }
> > >
> > > /**
> > > @@ -120,7 +120,7 @@ static int ti_syscon_reset_deassert(struct
> > > reset_controller_dev *rcdev,
> > > mask = BIT(control->deassert_bit);
> > > value = (control->flags & DEASSERT_SET) ? mask : 0x0;
> > >
> > > - return regmap_update_bits(data->regmap, control->deassert_offset, mask,
> > > value);
> > > + return regmap_write_bits(data->regmap, control->deassert_offset, mask,
> > > value);
> > > }
> > >
> > > /**
> > > @@ -158,10 +158,19 @@ static int ti_syscon_reset_status(struct
> > > reset_controller_dev *rcdev,
> > > !(control->flags & STATUS_SET);
> > > }
> > >
> > > +static int ti_syscon_reset(struct reset_controller_dev *rcdev,
> > > +unsigned long id)
> > > +{
> > > + ti_syscon_reset_assert(rcdev, id);
> > > +
> > > + return ti_syscon_reset_deassert(rcdev, id);
> > > +}
> > > +
>
> I'm unsure about this one, though. This is an incompatible change. At
> the very least this would have to be optional depending on compatible.
>
> regards
> Philipp
I will add a property to make this change be optional, thanks for you
advice.
Thanks,
Crystal
Re: [PATCH] Input: elan_i2c - Add ic type 0x15.
On Thu, Jul 30, 2020 at 02:05:26AM -0400, Jingle Wu wrote: > The update firmware flow of ic type 0x15 is same with ic type 0x14. > > Signed-off-by: Jingle Wu Applied, thank you. -- Dmitry
Re: [PATCH] cpufreq: cached_resolved_idx can not be negative
On Thu, Jul 30, 2020 at 11:40 AM Viresh Kumar wrote:
>
> On 30-07-20, 11:29, Amit Kucheria wrote:
> > On Thu, Jul 30, 2020 at 9:38 AM Viresh Kumar
> > wrote:
> > >
> > > It is not possible for cached_resolved_idx to be invalid here as the
> > > cpufreq core always sets index to a positive value.
> > >
> > > Change its type to unsigned int and fix qcom usage a bit.
> >
> > Shouldn't you fix up idx in cpufreq_driver_resolve_freq() to be
> > unsigned int too?
>
> Yes, merged this into the patch.
Looking at this more closely, I found another call site for
cpufreq_frequency_table_target() in cpufreq.c that needs the index to
be unsigned int.
But then cpufreq_frequency_table_target() returns -EINVAL, so we
should be able to handle int values.
I think you will need to fix the unconditional assignment of
policy->cached_resolved_idx = idx
in cpufreq_driver_resolve_freq(). It doesn't check for -EINVAL, so the
qcom driver is write in checking for a negative value.
>
> diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c
> index 0128de3603df..053d72e52a31 100644
> --- a/drivers/cpufreq/cpufreq.c
> +++ b/drivers/cpufreq/cpufreq.c
> @@ -538,7 +538,7 @@ unsigned int cpufreq_driver_resolve_freq(struct
> cpufreq_policy *policy,
> policy->cached_target_freq = target_freq;
>
> if (cpufreq_driver->target_index) {
> - int idx;
> + unsigned int idx;
>
> idx = cpufreq_frequency_table_target(policy, target_freq,
> CPUFREQ_RELATION_L);
>
> --
> viresh
Re: [PATCH v18 3/3] Input: new da7280 haptic driver
On Thu, Jul 30, 2020 at 08:16:31AM +0200, Uwe Kleine-König wrote:
> [Adding vsprintf maintainers to Cc:]
>
> Hello,
>
> On Wed, Jul 29, 2020 at 10:06:53PM -0700, Dmitry Torokhov wrote:
> > On Wed, Jul 29, 2020 at 09:21:45AM +0200, Uwe Kleine-König wrote:
> > > Hello,
> > >
> > > On Tue, Jul 28, 2020 at 11:36:38PM -0700, Dmitry Torokhov wrote:
> > > > > v9:
> > > > > - Removed the header file and put the definitions into the c
> > > > > file.
> > > > > - Updated the pwm code and error logs with %pE
> > > >
> > > > I believe the %pE is to format an escaped buffer, you probably want to
> > > > %pe (lowercase) to print errors. I am also not quite sure if we want to
> > > > use it in cases when we have non-pointer error, or we should stick with
> > > > %d as most of the kernel does.
> > >
> > > compared with %d %pe is easier to understand as it emits "-ETIMEOUT"
> > > instead of "-110". And yes, %pE is wrong.
> >
> > While I can see that symbolic name instead of a numeric constant might
> > be appealing, I do not believe that we want fragments like this with
> > endless conversions between integer and pointer errors:
> >
> > if (haptics->const_op_mode == DA7280_PWM_MODE) {
> > haptics->pwm_dev = devm_pwm_get(dev, NULL);
> > if (IS_ERR(haptics->pwm_dev)) {
> > error = PTR_ERR(haptics->pwm_dev);
> > if (error != -EPROBE_DEFER)
> > dev_err(dev, "unable to request PWM: %pE\n",
> > ERR_PTR(error));
> > return error;
> > }
> >
> > Maybe we should introduce something like '%de' for the integer error
> > case?
>
> I suggested that some time ago with limited success, see
> https://lore.kernel.org/lkml/20200129115516.zsvxu56e6h7gh...@pathway.suse.cz/
> .
>
> > In the meantime I would prefer using %d when we have integer
> > error. We should not see these error messages anyway ;)
>
> I don't agree. Error messages are supposed to be helpful and I prefer
> some casting between error pointer and error int over emitting bare
> numbers to the kernel log. (And additionally the uglyness might help to
> convince the vsprintf guys that %de might be a good idea after all :-)
Sorry, but I do not think that I want to introduce this pointless
casting just to sway printk folks. Either we have proper way of
formatting error codes, or we do not and we stick with what was working
since forever.
Thanks.
--
Dmitry
[PATCH] s390/test_unwind: fix possible memleak in test_unwind()
test_unwind() misses to call kfree(bt) in an error path.
Add the missed function call to fix it.
Fixes: 0610154650f1 ("s390/test_unwind: print verbose unwinding results")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
arch/s390/lib/test_unwind.c | 1 +
1 file changed, 1 insertion(+)
diff --git a/arch/s390/lib/test_unwind.c b/arch/s390/lib/test_unwind.c
index 32b7a30b2485..b0b12b46bc57 100644
--- a/arch/s390/lib/test_unwind.c
+++ b/arch/s390/lib/test_unwind.c
@@ -63,6 +63,7 @@ static noinline int test_unwind(struct task_struct *task,
struct pt_regs *regs,
break;
if (state.reliable && !addr) {
pr_err("unwind state reliable but addr is 0\n");
+ kfree(bt);
return -EINVAL;
}
sprint_symbol(sym, addr);
--
2.17.1
Re: [PATCH] cpufreq: cached_resolved_idx can not be negative
On 30-07-20, 12:02, Amit Kucheria wrote: > Looking at this more closely, I found another call site for > cpufreq_frequency_table_target() in cpufreq.c that needs the index to > be unsigned int. > > But then cpufreq_frequency_table_target() returns -EINVAL, so we It returns -EINVAL only in the case where the relation is not valid, which will never happen. Maybe that should be marked with WARN or BUG and we should drop return value of -EINVAL. Rafael ? > should be able to handle int values. And so no. > I think you will need to fix the unconditional assignment of > policy->cached_resolved_idx = idx > in cpufreq_driver_resolve_freq(). It doesn't check for -EINVAL, so the > qcom driver is write in checking for a negative value. Right, I don't want it to have that check for the reason stated above. The point is I don't want code that verifies cached-idx at all, it is useless. -- viresh
[PATCH -next] bnxt_en: Remove superfluous memset()
Fixes coccicheck warning: ./drivers/net/ethernet/broadcom/bnxt/bnxt.c:3730:19-37: WARNING: dma_alloc_coherent use in stats -> hw_stats already zeroes out memory, so memset is not needed dma_alloc_coherent use in status already zeroes out memory, so memset is not needed Reported-by: Hulk Robot Signed-off-by: Li Heng --- drivers/net/ethernet/broadcom/bnxt/bnxt.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/drivers/net/ethernet/broadcom/bnxt/bnxt.c b/drivers/net/ethernet/broadcom/bnxt/bnxt.c index 2622d3c..31fb5a2 100644 --- a/drivers/net/ethernet/broadcom/bnxt/bnxt.c +++ b/drivers/net/ethernet/broadcom/bnxt/bnxt.c @@ -3732,8 +3732,6 @@ static int bnxt_alloc_stats_mem(struct bnxt *bp, struct bnxt_stats_mem *stats, if (!stats->hw_stats) return -ENOMEM; - memset(stats->hw_stats, 0, stats->len); - stats->sw_stats = kzalloc(stats->len, GFP_KERNEL); if (!stats->sw_stats) goto stats_mem_err; -- 2.7.4
{standard input}:20594: Error: found '(', expected: ')'
Hi Paul,
FYI, the error/warning still remains.
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: d3590ebf6f91350192737dd1d1b219c05277f067
commit: 21e3134b3ec09e722cbcda69788f206adc8db1f4 MIPS: barrier: Clean up rmb()
& wmb() definitions
date: 10 months ago
config: mips-randconfig-r021-20200729 (attached as .config)
compiler: mips-linux-gcc (GCC) 9.3.0
reproduce (this is a W=1 build):
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 21e3134b3ec09e722cbcda69788f206adc8db1f4
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day COMPILER=gcc-9.3.0 make.cross
ARCH=mips
If you fix the issue, kindly add following tag as appropriate
Reported-by: kernel test robot
All errors (new ones prefixed by >>):
drivers/misc/habanalabs/device.c: In function 'hpriv_release':
drivers/misc/habanalabs/device.c:45:17: warning: variable 'ctx' set but not
used [-Wunused-but-set-variable]
45 | struct hl_ctx *ctx;
| ^~~
{standard input}: Assembler messages:
>> {standard input}:20594: Error: found '(', expected: ')'
>> {standard input}:20594: Error: found '(', expected: ')'
>> {standard input}:20594: Error: non-constant expression in ".if" statement
>> {standard input}:20594: Error: junk at end of line, first unrecognized
>> character is `('
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
Re: [PATCH v3] usb: typec: tcpm: Migrate workqueue to RT priority for processing events
On Wed, Jul 29, 2020 at 07:24:57PM -0700, Badhri Jagan Sridharan wrote: > "tReceiverResponse 15 ms Section 6.6.2 > The receiver of a Message requiring a response Shall respond > within tReceiverResponse in order to ensure that the > sender’s SenderResponseTimer does not expire." > > When the cpu complex is busy running other lower priority > work items, TCPM's work queue sometimes does not get scheduled > on time to meet the above requirement from the spec. > Moving to kthread_work apis to run with real time priority. > Just lower than the default threaded irq priority, > MAX_USER_RT_PRIO/2 + 1. (Higher number implies lower priority). > > Further, as observed in 1ff688209e2e, moving to hrtimers to > overcome scheduling latency while scheduling the delayed work. > > TCPM has three work streams: > 1. tcpm_state_machine > 2. vdm_state_machine > 3. event_work > > tcpm_state_machine and vdm_state_machine both schedule work in > future i.e. delayed. Hence each of them have a corresponding > hrtimer, tcpm_state_machine_timer & vdm_state_machine_timer. > > When work is queued right away kthread_queue_work is used. > Else, the relevant timer is programmed and made to queue > the kthread_work upon timer expiry. > > kthread_create_worker only creates one kthread worker thread, > hence single threadedness of workqueue is retained. > > Signed-off-by: Badhri Jagan Sridharan > Reviewed-by: Guenter Roeck > Reviewed-by: Heikki Krogerus > --- > Changes since v1:(Guenter's suggestions) > - Remove redundant call to hrtimer_cancel while calling > hrtimer_start. > > Changes since v2:(Greg KH's suggestions) > - Rebase usb-next TOT. > 633198cd2945b7 (HEAD -> usb-next-1) usb: typec: tcpm: Migrate workqueue to > RT priority for processing events > fa56dd9152ef95 (origin/usb-next) Merge tag 'usb-serial-5.9-rc1' of > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-next > 25252919a1050e xhci: dbgtty: Make some functions static > b0e02550346e67 xhci: dbc: Make function xhci_dbc_ring_alloc() static > ca6377900974c3 Revert "usb: dwc2: override PHY input signals with usb role > switch support" > 09df709cb5aeb2 Revert "usb: dwc2: don't use ID/Vbus detection if > usb-role-switch on STM32MP15 SoCs" > 17a82716587e9d USB: iowarrior: fix up report size handling for some devices > e98ba8cc3f8a89 Merge tag 'usb-for-v5.9' of > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next > c97793089b11f7 Merge 5.8-rc7 into usb-next > 92ed301919932f (tag: v5.8-rc7, origin/usb-linus, origin/main) Linux 5.8-rc7 > Hm, still does not apply. I think it has something to do with other patches that landed before yours, can you rebase again? thanks, greg k-h
[PATCH v1 0/3] net: ethernet: use generic power management
Linux Kernel Mentee: Remove Legacy Power Management. The purpose of this patch series is to upgrade power management in net ethernet drivers. This has been done by upgrading .suspend() and .resume() callbacks. The upgrade makes sure that the involvement of PCI Core does not change the order of operations executed in a driver. Thus, does not change its behavior. In general, drivers with legacy PM, .suspend() and .resume() make use of PCI helper functions like pci_enable/disable_device_mem(), pci_set_power_state(), pci_save/restore_state(), pci_enable/disable_device(), etc. to complete their job. The conversion requires the removal of those function calls, change the callbacks' definition accordingly and make use of dev_pm_ops structure. All patches are compile-tested only. Test tools: - Compiler: gcc (GCC) 10.1.0 - allmodconfig build: make -j$(nproc) W=1 all Vaibhav Gupta (3): sc92031: use generic power management sis900: use generic power management tlan: use generic power management drivers/net/ethernet/silan/sc92031.c | 26 --- drivers/net/ethernet/sis/sis900.c| 23 +++-- drivers/net/ethernet/ti/tlan.c | 31 ++-- 3 files changed, 22 insertions(+), 58 deletions(-) -- 2.27.0
[PATCH v1 1/3] sc92031: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/net/ethernet/silan/sc92031.c | 26 +-
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/silan/sc92031.c
b/drivers/net/ethernet/silan/sc92031.c
index cb043eb1bdc1..f94078f8ebe5 100644
--- a/drivers/net/ethernet/silan/sc92031.c
+++ b/drivers/net/ethernet/silan/sc92031.c
@@ -1499,15 +1499,13 @@ static void sc92031_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-static int sc92031_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused sc92031_suspend(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct sc92031_priv *priv = netdev_priv(dev);
- pci_save_state(pdev);
-
if (!netif_running(dev))
- goto out;
+ return 0;
netif_device_detach(dev);
@@ -1521,22 +1519,16 @@ static int sc92031_suspend(struct pci_dev *pdev,
pm_message_t state)
spin_unlock_bh(>lock);
-out:
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
return 0;
}
-static int sc92031_resume(struct pci_dev *pdev)
+static int __maybe_unused sc92031_resume(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct sc92031_priv *priv = netdev_priv(dev);
- pci_restore_state(pdev);
- pci_set_power_state(pdev, PCI_D0);
-
if (!netif_running(dev))
- goto out;
+ return 0;
/* Interrupts already disabled by sc92031_suspend */
spin_lock_bh(>lock);
@@ -1553,7 +1545,6 @@ static int sc92031_resume(struct pci_dev *pdev)
else
netif_tx_disable(dev);
-out:
return 0;
}
@@ -1565,13 +1556,14 @@ static const struct pci_device_id
sc92031_pci_device_id_table[] = {
};
MODULE_DEVICE_TABLE(pci, sc92031_pci_device_id_table);
+static SIMPLE_DEV_PM_OPS(sc92031_pm_ops, sc92031_suspend, sc92031_resume);
+
static struct pci_driver sc92031_pci_driver = {
.name = SC92031_NAME,
.id_table = sc92031_pci_device_id_table,
.probe = sc92031_probe,
.remove = sc92031_remove,
- .suspend= sc92031_suspend,
- .resume = sc92031_resume,
+ .driver.pm = _pm_ops,
};
module_pci_driver(sc92031_pci_driver);
--
2.27.0
[PATCH v1 3/3] tlan: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/net/ethernet/ti/tlan.c | 31 ++-
1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
index 857709828058..c799945a39ef 100644
--- a/drivers/net/ethernet/ti/tlan.c
+++ b/drivers/net/ethernet/ti/tlan.c
@@ -345,33 +345,21 @@ static void tlan_stop(struct net_device *dev)
}
}
-#ifdef CONFIG_PM
-
-static int tlan_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused tlan_suspend(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
if (netif_running(dev))
tlan_stop(dev);
netif_device_detach(dev);
- pci_save_state(pdev);
- pci_disable_device(pdev);
- pci_wake_from_d3(pdev, false);
- pci_set_power_state(pdev, PCI_D3hot);
return 0;
}
-static int tlan_resume(struct pci_dev *pdev)
+static int __maybe_unused tlan_resume(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
- int rc = pci_enable_device(pdev);
-
- if (rc)
- return rc;
- pci_restore_state(pdev);
- pci_enable_wake(pdev, PCI_D0, 0);
+ struct net_device *dev = dev_get_drvdata(dev_d);
netif_device_attach(dev);
if (netif_running(dev))
@@ -380,21 +368,14 @@ static int tlan_resume(struct pci_dev *pdev)
return 0;
}
-#else /* CONFIG_PM */
-
-#define tlan_suspend NULL
-#define tlan_resumeNULL
-
-#endif /* CONFIG_PM */
-
+static SIMPLE_DEV_PM_OPS(tlan_pm_ops, tlan_suspend, tlan_resume);
static struct pci_driver tlan_driver = {
.name = "tlan",
.id_table = tlan_pci_tbl,
.probe = tlan_init_one,
.remove = tlan_remove_one,
- .suspend= tlan_suspend,
- .resume = tlan_resume,
+ .driver.pm = _pm_ops,
};
static int __init tlan_probe(void)
--
2.27.0
[PATCH v1 2/3] sis900: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/net/ethernet/sis/sis900.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/sis/sis900.c
b/drivers/net/ethernet/sis/sis900.c
index 81ed7589e33c..2af2c9816dfc 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -2493,11 +2493,9 @@ static void sis900_remove(struct pci_dev *pci_dev)
pci_release_regions(pci_dev);
}
-#ifdef CONFIG_PM
-
-static int sis900_suspend(struct pci_dev *pci_dev, pm_message_t state)
+static int __maybe_unused sis900_suspend(struct device *dev)
{
- struct net_device *net_dev = pci_get_drvdata(pci_dev);
+ struct net_device *net_dev = dev_get_drvdata(dev);
struct sis900_private *sis_priv = netdev_priv(net_dev);
void __iomem *ioaddr = sis_priv->ioaddr;
@@ -2510,22 +2508,17 @@ static int sis900_suspend(struct pci_dev *pci_dev,
pm_message_t state)
/* Stop the chip's Tx and Rx Status Machine */
sw32(cr, RxDIS | TxDIS | sr32(cr));
- pci_set_power_state(pci_dev, PCI_D3hot);
- pci_save_state(pci_dev);
-
return 0;
}
-static int sis900_resume(struct pci_dev *pci_dev)
+static int __maybe_unused sis900_resume(struct device *dev)
{
- struct net_device *net_dev = pci_get_drvdata(pci_dev);
+ struct net_device *net_dev = dev_get_drvdata(dev);
struct sis900_private *sis_priv = netdev_priv(net_dev);
void __iomem *ioaddr = sis_priv->ioaddr;
if(!netif_running(net_dev))
return 0;
- pci_restore_state(pci_dev);
- pci_set_power_state(pci_dev, PCI_D0);
sis900_init_rxfilter(net_dev);
@@ -2549,17 +2542,15 @@ static int sis900_resume(struct pci_dev *pci_dev)
return 0;
}
-#endif /* CONFIG_PM */
+
+static SIMPLE_DEV_PM_OPS(sis900_pm_ops, sis900_suspend, sis900_resume);
static struct pci_driver sis900_pci_driver = {
.name = SIS900_MODULE_NAME,
.id_table = sis900_pci_tbl,
.probe = sis900_probe,
.remove = sis900_remove,
-#ifdef CONFIG_PM
- .suspend= sis900_suspend,
- .resume = sis900_resume,
-#endif /* CONFIG_PM */
+ .driver.pm = _pm_ops,
};
static int __init sis900_init_module(void)
--
2.27.0
Re: [PATCH] vgacon: fix out of bounds write to the scrollback buffer
Hi, OTOH, you should have CCed all the (public) lists.
On 30. 07. 20, 4:50, 张云海 wrote:
> Zhang Xiao points out that the check should use > instead of >=,
> otherwise the last line will be skip.
> I agree with that, so I modify the patch.
> Could you please verify that it is still correct and sufficient?
IMO, yes, correct -- I was thinking about this yesterday too. Just an
example: hypothetically, if we had:
size_row = 1
tail = 29
size = 30
data[29] would be the last accessible member. Writing to data + tail (as
"29 + 1 > 30" doesn't hold, so the modified check would pass), i.e.
data[29] is still OK. So yes, > is OK, >= would waste space and would be
actually incorrect.
> BTW, Zhang Xiao also points out that the check after the memcpy can be
> remove.
> I also think that was right, but vgacon_scrollback_cur->tail may keep
> the value vgacon_scrollback_cur->size in some case. That is not a
> problem in vgacon_scrollback_update because of the check before the
> memcpy. However, that may break some other code which assumes that
> vgacon_scrollback_cur->tail won't be vgacon_scrollback_cur->size. I do
> not know if there are such code, and if it is the code actually should
> check it too. But I still not remove the check in the patch to make sure
> it won't breaks other code.
As I wrote about this yesterday:
===
I am also not sure the test I was pointing out on the top of this
message would be of any use after the change. But maybe leave the code
rest in peace.
===
I would let it as is in this particular code. Especially because
vgacon_scrolldelta takes ->tail into consideration and I was too lazy to
study the code there. But if you are willing to study the code there and
confirm the check is superfluous, feel free to remove it. Perhaps in a
separate patch. I was actually testing with the check removed and didn't
hit any issue (which means, in fact, exactly nothing).
> From ad143ede24ff4e61292cc9c96000100aacd97259 Mon Sep 17 00:00:00 2001
> From: Yunhai Zhang
> Date: Tue, 28 Jul 2020 09:58:03 +0800
> Subject: [PATCH] Fix for missing check in vgacon scrollback handling
>
> vgacon_scrollback_update() always left enbough room in the scrollback
"leaves enough"
> buffer for the next call, but if the console size changed that room
> might not actually be enough, and so we need to re-check.
Also, could you add reasoning why you are adding the check to the loop
and not outside (for instance, use your reasoning with numbers or CSI M
as an example).
Could you add a sample output here, something like I had:
===
This leads to random crashes or KASAN reports like:
BUG: KASAN: slab-out-of-bounds in vgacon_scroll+0x57a/0x8ed
===
It's then easier to google for when this happens to someone who runs
non-patched kernels.
> This fixes CVE-2020-14331.
>
> Reported-and-debugged-by: 张云海
> Reported-and-debugged-by: Yang Yingliang
> Reported-by: Kyungtae Kim
> Fixes: 15bdab959c9b ([PATCH] vgacon: Add support for soft scrollback)
> Cc: Linus Torvalds
> Cc: Greg KH
> Cc: Solar Designer
> Cc: "Srivatsa S. Bhat"
> Cc: Anthony Liguori
> Cc: Yang Yingliang
> Cc: Bartlomiej Zolnierkiewicz
Oh, and we should:
Cc: sta...@vger.kernel.org
> Signed-off-by: Yunhai Zhang
> ---
> drivers/video/console/vgacon.c | 4
> 1 file changed, 4 insertions(+)
>
> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
> index 998b0de1812f..37b5711cd958 100644
> --- a/drivers/video/console/vgacon.c
> +++ b/drivers/video/console/vgacon.c
> @@ -251,6 +251,10 @@ static void vgacon_scrollback_update(struct vc_data *c,
> int t, int count)
> p = (void *) (c->vc_origin + t * c->vc_size_row);
>
> while (count--) {
> + if ((vgacon_scrollback_cur->tail + c->vc_size_row) >
> + vgacon_scrollback_cur->size)
> + vgacon_scrollback_cur->tail = 0;
> +
> scr_memcpyw(vgacon_scrollback_cur->data +
> vgacon_scrollback_cur->tail,
> p, c->vc_size_row);
thanks,
--
js
suse labs
[PATCH net] qtnfmac: Missing platform_device_unregister() on error in qtnf_core_mac_alloc()
Add the missing platform_device_unregister() before return from
qtnf_core_mac_alloc() in the error handling case.
Fixes: 616f5701f4ab ("qtnfmac: assign each wiphy to its own virtual platform
device")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
drivers/net/wireless/quantenna/qtnfmac/core.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/wireless/quantenna/qtnfmac/core.c
b/drivers/net/wireless/quantenna/qtnfmac/core.c
index eea777f8acea..6aafff9d4231 100644
--- a/drivers/net/wireless/quantenna/qtnfmac/core.c
+++ b/drivers/net/wireless/quantenna/qtnfmac/core.c
@@ -446,8 +446,11 @@ static struct qtnf_wmac *qtnf_core_mac_alloc(struct
qtnf_bus *bus,
}
wiphy = qtnf_wiphy_allocate(bus, pdev);
- if (!wiphy)
+ if (!wiphy) {
+ if (pdev)
+ platform_device_unregister(pdev);
return ERR_PTR(-ENOMEM);
+ }
mac = wiphy_priv(wiphy);
--
2.17.1
[PATCH 2/2] reset: imx7: add the cm4 reset for i.MX8MQ
From: Peng Fan
Add the cm4 reset used by the remoteproc driver
Signed-off-by: Peng Fan
---
drivers/reset/reset-imx7.c | 4
1 file changed, 4 insertions(+)
diff --git a/drivers/reset/reset-imx7.c b/drivers/reset/reset-imx7.c
index d170fe663210..87b6e2d46fb6 100644
--- a/drivers/reset/reset-imx7.c
+++ b/drivers/reset/reset-imx7.c
@@ -178,6 +178,9 @@ static const struct imx7_src_signal
imx8mq_src_signals[IMX8MQ_RESET_NUM] = {
[IMX8MQ_RESET_A53_SOC_DBG_RESET]= { SRC_A53RCR0, BIT(20) },
[IMX8MQ_RESET_A53_L2RESET] = { SRC_A53RCR0, BIT(21) },
[IMX8MQ_RESET_SW_NON_SCLR_M4C_RST] = { SRC_M4RCR, BIT(0) },
+ [IMX8MQ_RESET_SW_M4C_RST] = { SRC_M4RCR, BIT(1) },
+ [IMX8MQ_RESET_SW_M4P_RST] = { SRC_M4RCR, BIT(2) },
+ [IMX8MQ_RESET_M4_ENABLE]= { SRC_M4RCR, BIT(3) },
[IMX8MQ_RESET_OTG1_PHY_RESET] = { SRC_USBOPHY1_RCR, BIT(0) },
[IMX8MQ_RESET_OTG2_PHY_RESET] = { SRC_USBOPHY2_RCR, BIT(0) },
[IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N]= { SRC_MIPIPHY_RCR, BIT(1) },
@@ -238,6 +241,7 @@ static int imx8mq_reset_set(struct reset_controller_dev
*rcdev,
case IMX8MQ_RESET_MIPI_DSI_DPI_RESET_N: /* fallthrough */
case IMX8MQ_RESET_MIPI_DSI_RESET_N: /* fallthrough */
case IMX8MQ_RESET_MIPI_DSI_RESET_BYTE_N:/* fallthrough */
+ case IMX8MQ_RESET_M4_ENABLE:
value = assert ? 0 : bit;
break;
}
--
2.16.4
[PATCH 1/2] dt-bindings: reset: imx8mq: add m4 reset
From: Peng Fan Add the m4 reset used by the remoteproc driver Signed-off-by: Peng Fan --- include/dt-bindings/reset/imx8mq-reset.h | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/include/dt-bindings/reset/imx8mq-reset.h b/include/dt-bindings/reset/imx8mq-reset.h index a5b570737582..705870693ec2 100644 --- a/include/dt-bindings/reset/imx8mq-reset.h +++ b/include/dt-bindings/reset/imx8mq-reset.h @@ -58,7 +58,10 @@ #define IMX8MQ_RESET_DDRC2_PRST47 /* i.MX8MM/i.MX8MN does NOT support */ #define IMX8MQ_RESET_DDRC2_CORE_RESET 48 /* i.MX8MM/i.MX8MN does NOT support */ #define IMX8MQ_RESET_DDRC2_PHY_RESET 49 /* i.MX8MM/i.MX8MN does NOT support */ +#define IMX8MQ_RESET_SW_M4C_RST50 +#define IMX8MQ_RESET_SW_M4P_RST51 +#define IMX8MQ_RESET_M4_ENABLE 52 -#define IMX8MQ_RESET_NUM 50 +#define IMX8MQ_RESET_NUM 53 #endif -- 2.16.4
Re: [PATCH 2/3] driver core: Use rwsem for kill_device() serialization
On Wed, Jul 08, 2020 at 03:27:02PM +0200, Lukas Wunner wrote:
> kill_device() is currently serialized with driver probing by way of the
> device_lock(). We're about to serialize it with device_add() as well
> to prevent addition of children below a device which is going away.
Why? Who does this? Shouldn't the bus that is trying to do this know
this is happening?
> However the parent's device_lock() cannot be taken by device_add()
> lest deadlocks occur: Addition of the parent may result in addition
> of children (as is the case with SPI controllers) and device_add()
> already takes the device_lock through the call to bus_probe_device() ->
> device_initial_probe() -> __device_attach().
>
> Avoid such deadlocks by introducing an rw_semaphore whose specific
> purpose is to serialize kill_device() with other parts of the kernel.
Ugh, another lock, really? :(
> Use an rw_semaphore instead of a mutex because the latter would preclude
> concurrent driver probing of multiple children below the same parent.
> The semaphore is acquired for writing when declaring a device dead and
> otherwise only acquired for reading. It is private to the driver core,
> obviating the need to acquire a lock when calling kill_device(), as is
> currently done in nvdimm's nd_device_unregister() and device_del().
>
> An alternative approach would be to convert the device_lock() itself to
> an rw_semaphore (instead of a mutex).
>
> Signed-off-by: Lukas Wunner
> Cc: Dan Williams
> Cc: Geert Uytterhoeven
> Cc: Pantelis Antoniou
> Cc: Alexander Duyck
> ---
> drivers/base/base.h | 2 ++
> drivers/base/core.c | 33 +++--
> drivers/base/dd.c| 8
> drivers/nvdimm/bus.c | 8 +---
> 4 files changed, 30 insertions(+), 21 deletions(-)
>
> diff --git a/drivers/base/base.h b/drivers/base/base.h
> index 95c22c0f90360..7e71a1d262ef6 100644
> --- a/drivers/base/base.h
> +++ b/drivers/base/base.h
> @@ -79,6 +79,7 @@ struct driver_private {
> * @async_driver - pointer to device driver awaiting probe via async_probe
> * @device - pointer back to the struct device that this structure is
> * associated with.
> + * @dead_sem - semaphore taken when declaring the device @dead.
> * @dead - This device is currently either in the process of or has been
> * removed from the system. Any asynchronous events scheduled for this
> * device should exit without taking any action.
> @@ -94,6 +95,7 @@ struct device_private {
> struct list_head deferred_probe;
> struct device_driver *async_driver;
> struct device *device;
> + struct rw_semaphore dead_sem;
> u8 dead:1;
> };
> #define to_device_private_parent(obj)\
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 67d39a90b45c7..057da42b1a660 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2526,6 +2526,7 @@ static int device_private_init(struct device *dev)
> klist_init(>p->klist_children, klist_children_get,
> klist_children_put);
> INIT_LIST_HEAD(>p->deferred_probe);
> + init_rwsem(>p->dead_sem);
> return 0;
> }
>
> @@ -2780,21 +2781,27 @@ void put_device(struct device *dev)
> }
> EXPORT_SYMBOL_GPL(put_device);
>
> +/**
> + * kill_device - declare device dead
> + * @dev: device in question
> + *
> + * Declare @dev dead to prevent it from binding to a driver.
> + * Return true if it was killed or false if it was already dead.
> + */
> bool kill_device(struct device *dev)
> {
> - /*
> - * Require the device lock and set the "dead" flag to guarantee that
> - * the update behavior is consistent with the other bitfields near
> - * it and that we cannot have an asynchronous probe routine trying
> - * to run while we are tearing out the bus/class/sysfs from
> - * underneath the device.
> - */
> - lockdep_assert_held(>mutex);
> + bool killed;
>
> - if (dev->p->dead)
> - return false;
> - dev->p->dead = true;
> - return true;
> + down_write(>p->dead_sem);
> + if (dev->p->dead) {
> + killed = false;
> + } else {
> + dev->p->dead = true;
> + killed = true;
> + }
> + up_write(>p->dead_sem);
> +
> + return killed;
> }
> EXPORT_SYMBOL_GPL(kill_device);
meta-comment, this should have been device_kill() :(
>
> @@ -2817,9 +2824,7 @@ void device_del(struct device *dev)
> struct kobject *glue_dir = NULL;
> struct class_interface *class_intf;
>
> - device_lock(dev);
> kill_device(dev);
> - device_unlock(dev);
>
> if (dev->fwnode && dev->fwnode->dev == dev)
> dev->fwnode->dev = NULL;
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 31c668651e824..9353d811cce83 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -817,6 +817,7 @@ static void __device_attach_async_helper(void *_dev,
> async_cookie_t cookie)
> };
>
>
RE: [PATCH v2] exfat: integrates dir-entry getting and validation
> Add validation for num, bh and type on getting dir-entry.
> ('file' and 'stream-ext' dir-entries are pre-validated to ensure success)
> Renamed
> exfat_get_dentry_cached() to exfat_get_validated_dentry() due to a change in
> functionality.
>
> Integrate type-validation with simplified.
> This will also recognize a dir-entry set that contains 'benign secondary'
> dir-entries.
>
> And, rename TYPE_EXTEND to TYPE_NAME.
>
> Suggested-by: Sungjong Seo
> Signed-off-by: Tetsuhiro Kohada
> ---
> Changes in v2
> - Change verification order
> - Verification loop start with index 2
>
> fs/exfat/dir.c | 144 ++--
> fs/exfat/exfat_fs.h | 15 +++--
> fs/exfat/file.c | 4 +-
> fs/exfat/inode.c| 6 +-
> fs/exfat/namei.c| 4 +-
> 5 files changed, 73 insertions(+), 100 deletions(-)
>
> diff --git a/fs/exfat/dir.c b/fs/exfat/dir.c index 573659bfbc55..09b85746e760
> 100644
> --- a/fs/exfat/dir.c
> +++ b/fs/exfat/dir.c
> @@ -33,6 +33,7 @@ static void exfat_get_uniname_from_ext_entry(struct
> super_block *sb, {
> int i;
> struct exfat_entry_set_cache *es;
> + struct exfat_dentry *ep;
>
> es = exfat_get_dentry_set(sb, p_dir, entry, ES_ALL_ENTRIES);
> if (!es)
> @@ -44,13 +45,9 @@ static void exfat_get_uniname_from_ext_entry(struct
> super_block *sb,
>* Third entry : first file-name entry
>* So, the index of first file-name dentry should start from 2.
>*/
> - for (i = 2; i < es->num_entries; i++) {
> - struct exfat_dentry *ep = exfat_get_dentry_cached(es, i);
> -
> - /* end of name entry */
> - if (exfat_get_entry_type(ep) != TYPE_EXTEND)
> - break;
>
> + i = 2;
> + while ((ep = exfat_get_validated_dentry(es, i++, TYPE_NAME))) {
As Sungjong said, I think that TYPE_NAME seems right to be validated in
exfat_get_dentry_set().
> exfat_extract_uni_name(ep, uniname);
> uniname += EXFAT_FILE_NAME_LEN;
> }
> @@ -372,7 +369,7 @@ unsigned int exfat_get_entry_type(struct exfat_dentry *ep)
> if (ep->type == EXFAT_STREAM)
> return TYPE_STREAM;
> if (ep->type == EXFAT_NAME)
> - return TYPE_EXTEND;
> + return TYPE_NAME;
> if (ep->type == EXFAT_ACL)
> return TYPE_ACL;
> return TYPE_CRITICAL_SEC;
> @@ -388,7 +385,7 @@ static void exfat_set_entry_type(struct exfat_dentry *ep,
> unsigned int type)
> ep->type &= EXFAT_DELETE;
> } else if (type == TYPE_STREAM) {
> ep->type = EXFAT_STREAM;
> - } else if (type == TYPE_EXTEND) {
> + } else if (type == TYPE_NAME) {
> ep->type = EXFAT_NAME;
> } else if (type == TYPE_BITMAP) {
> ep->type = EXFAT_BITMAP;
> @@ -421,7 +418,7 @@ static void exfat_init_name_entry(struct exfat_dentry
> *ep, {
> int i;
>
> - exfat_set_entry_type(ep, TYPE_EXTEND);
> + exfat_set_entry_type(ep, TYPE_NAME);
> ep->dentry.name.flags = 0x0;
>
> for (i = 0; i < EXFAT_FILE_NAME_LEN; i++) { @@ -594,12 +591,12 @@ void
> exfat_update_dir_chksum_with_entry_set(struct exfat_entry_set_cache *es)
> struct exfat_dentry *ep;
>
> for (i = 0; i < es->num_entries; i++) {
> - ep = exfat_get_dentry_cached(es, i);
> + ep = exfat_get_validated_dentry(es, i, TYPE_ALL);
> chksum = exfat_calc_chksum16(ep, DENTRY_SIZE, chksum,
>chksum_type);
> chksum_type = CS_DEFAULT;
> }
> - ep = exfat_get_dentry_cached(es, 0);
> + ep = exfat_get_validated_dentry(es, 0, TYPE_FILE);
> ep->dentry.file.checksum = cpu_to_le16(chksum);
> es->modified = true;
> }
> @@ -741,92 +738,66 @@ struct exfat_dentry *exfat_get_dentry(struct
> super_block *sb,
> return (struct exfat_dentry *)((*bh)->b_data + off); }
>
> -enum exfat_validate_dentry_mode {
> - ES_MODE_STARTED,
> - ES_MODE_GET_FILE_ENTRY,
> - ES_MODE_GET_STRM_ENTRY,
> - ES_MODE_GET_NAME_ENTRY,
> - ES_MODE_GET_CRITICAL_SEC_ENTRY,
> -};
> -
> -static bool exfat_validate_entry(unsigned int type,
> - enum exfat_validate_dentry_mode *mode)
> -{
> - if (type == TYPE_UNUSED || type == TYPE_DELETED)
> - return false;
> -
> - switch (*mode) {
> - case ES_MODE_STARTED:
> - if (type != TYPE_FILE && type != TYPE_DIR)
> - return false;
> - *mode = ES_MODE_GET_FILE_ENTRY;
> - return true;
> - case ES_MODE_GET_FILE_ENTRY:
> - if (type != TYPE_STREAM)
> - return false;
> - *mode = ES_MODE_GET_STRM_ENTRY;
> - return true;
> - case ES_MODE_GET_STRM_ENTRY:
> - if (type != TYPE_EXTEND)
> - return false;
> -
Re: [PATCH 0/3] Fix races on device removal
On Wed, Jul 08, 2020 at 03:27:00PM +0200, Lukas Wunner wrote: > Prevent dynamic SPI device addition below a controller which is > being removed. To do so, set the controller's "dead" flag using > kill_device() (patch [3/3]). Why is the SPI bus allowing this to happen? Don't you have a per-bus lock that can serialize all of this? thanks, greg k-h
[PATCH v1 3/3] tlan: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/net/ethernet/ti/tlan.c | 31 ++-
1 file changed, 6 insertions(+), 25 deletions(-)
diff --git a/drivers/net/ethernet/ti/tlan.c b/drivers/net/ethernet/ti/tlan.c
index 857709828058..c799945a39ef 100644
--- a/drivers/net/ethernet/ti/tlan.c
+++ b/drivers/net/ethernet/ti/tlan.c
@@ -345,33 +345,21 @@ static void tlan_stop(struct net_device *dev)
}
}
-#ifdef CONFIG_PM
-
-static int tlan_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused tlan_suspend(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
if (netif_running(dev))
tlan_stop(dev);
netif_device_detach(dev);
- pci_save_state(pdev);
- pci_disable_device(pdev);
- pci_wake_from_d3(pdev, false);
- pci_set_power_state(pdev, PCI_D3hot);
return 0;
}
-static int tlan_resume(struct pci_dev *pdev)
+static int __maybe_unused tlan_resume(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
- int rc = pci_enable_device(pdev);
-
- if (rc)
- return rc;
- pci_restore_state(pdev);
- pci_enable_wake(pdev, PCI_D0, 0);
+ struct net_device *dev = dev_get_drvdata(dev_d);
netif_device_attach(dev);
if (netif_running(dev))
@@ -380,21 +368,14 @@ static int tlan_resume(struct pci_dev *pdev)
return 0;
}
-#else /* CONFIG_PM */
-
-#define tlan_suspend NULL
-#define tlan_resumeNULL
-
-#endif /* CONFIG_PM */
-
+static SIMPLE_DEV_PM_OPS(tlan_pm_ops, tlan_suspend, tlan_resume);
static struct pci_driver tlan_driver = {
.name = "tlan",
.id_table = tlan_pci_tbl,
.probe = tlan_init_one,
.remove = tlan_remove_one,
- .suspend= tlan_suspend,
- .resume = tlan_resume,
+ .driver.pm = _pm_ops,
};
static int __init tlan_probe(void)
--
2.27.0
[PATCH v1 2/3] sis900: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/net/ethernet/sis/sis900.c | 23 +++
1 file changed, 7 insertions(+), 16 deletions(-)
diff --git a/drivers/net/ethernet/sis/sis900.c
b/drivers/net/ethernet/sis/sis900.c
index 81ed7589e33c..2af2c9816dfc 100644
--- a/drivers/net/ethernet/sis/sis900.c
+++ b/drivers/net/ethernet/sis/sis900.c
@@ -2493,11 +2493,9 @@ static void sis900_remove(struct pci_dev *pci_dev)
pci_release_regions(pci_dev);
}
-#ifdef CONFIG_PM
-
-static int sis900_suspend(struct pci_dev *pci_dev, pm_message_t state)
+static int __maybe_unused sis900_suspend(struct device *dev)
{
- struct net_device *net_dev = pci_get_drvdata(pci_dev);
+ struct net_device *net_dev = dev_get_drvdata(dev);
struct sis900_private *sis_priv = netdev_priv(net_dev);
void __iomem *ioaddr = sis_priv->ioaddr;
@@ -2510,22 +2508,17 @@ static int sis900_suspend(struct pci_dev *pci_dev,
pm_message_t state)
/* Stop the chip's Tx and Rx Status Machine */
sw32(cr, RxDIS | TxDIS | sr32(cr));
- pci_set_power_state(pci_dev, PCI_D3hot);
- pci_save_state(pci_dev);
-
return 0;
}
-static int sis900_resume(struct pci_dev *pci_dev)
+static int __maybe_unused sis900_resume(struct device *dev)
{
- struct net_device *net_dev = pci_get_drvdata(pci_dev);
+ struct net_device *net_dev = dev_get_drvdata(dev);
struct sis900_private *sis_priv = netdev_priv(net_dev);
void __iomem *ioaddr = sis_priv->ioaddr;
if(!netif_running(net_dev))
return 0;
- pci_restore_state(pci_dev);
- pci_set_power_state(pci_dev, PCI_D0);
sis900_init_rxfilter(net_dev);
@@ -2549,17 +2542,15 @@ static int sis900_resume(struct pci_dev *pci_dev)
return 0;
}
-#endif /* CONFIG_PM */
+
+static SIMPLE_DEV_PM_OPS(sis900_pm_ops, sis900_suspend, sis900_resume);
static struct pci_driver sis900_pci_driver = {
.name = SIS900_MODULE_NAME,
.id_table = sis900_pci_tbl,
.probe = sis900_probe,
.remove = sis900_remove,
-#ifdef CONFIG_PM
- .suspend= sis900_suspend,
- .resume = sis900_resume,
-#endif /* CONFIG_PM */
+ .driver.pm = _pm_ops,
};
static int __init sis900_init_module(void)
--
2.27.0
[PATCH v1 0/3] net: ethernet: use generic power management
Linux Kernel Mentee: Remove Legacy Power Management. The purpose of this patch series is to upgrade power management in net ethernet drivers. This has been done by upgrading .suspend() and .resume() callbacks. The upgrade makes sure that the involvement of PCI Core does not change the order of operations executed in a driver. Thus, does not change its behavior. In general, drivers with legacy PM, .suspend() and .resume() make use of PCI helper functions like pci_enable/disable_device_mem(), pci_set_power_state(), pci_save/restore_state(), pci_enable/disable_device(), etc. to complete their job. The conversion requires the removal of those function calls, change the callbacks' definition accordingly and make use of dev_pm_ops structure. All patches are compile-tested only. Test tools: - Compiler: gcc (GCC) 10.1.0 - allmodconfig build: make -j$(nproc) W=1 all Vaibhav Gupta (3): sc92031: use generic power management sis900: use generic power management tlan: use generic power management drivers/net/ethernet/silan/sc92031.c | 26 --- drivers/net/ethernet/sis/sis900.c| 23 +++-- drivers/net/ethernet/ti/tlan.c | 31 ++-- 3 files changed, 22 insertions(+), 58 deletions(-) -- 2.27.0
[PATCH v1 1/3] sc92031: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/net/ethernet/silan/sc92031.c | 26 +-
1 file changed, 9 insertions(+), 17 deletions(-)
diff --git a/drivers/net/ethernet/silan/sc92031.c
b/drivers/net/ethernet/silan/sc92031.c
index cb043eb1bdc1..f94078f8ebe5 100644
--- a/drivers/net/ethernet/silan/sc92031.c
+++ b/drivers/net/ethernet/silan/sc92031.c
@@ -1499,15 +1499,13 @@ static void sc92031_remove(struct pci_dev *pdev)
pci_disable_device(pdev);
}
-static int sc92031_suspend(struct pci_dev *pdev, pm_message_t state)
+static int __maybe_unused sc92031_suspend(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct sc92031_priv *priv = netdev_priv(dev);
- pci_save_state(pdev);
-
if (!netif_running(dev))
- goto out;
+ return 0;
netif_device_detach(dev);
@@ -1521,22 +1519,16 @@ static int sc92031_suspend(struct pci_dev *pdev,
pm_message_t state)
spin_unlock_bh(>lock);
-out:
- pci_set_power_state(pdev, pci_choose_state(pdev, state));
-
return 0;
}
-static int sc92031_resume(struct pci_dev *pdev)
+static int __maybe_unused sc92031_resume(struct device *dev_d)
{
- struct net_device *dev = pci_get_drvdata(pdev);
+ struct net_device *dev = dev_get_drvdata(dev_d);
struct sc92031_priv *priv = netdev_priv(dev);
- pci_restore_state(pdev);
- pci_set_power_state(pdev, PCI_D0);
-
if (!netif_running(dev))
- goto out;
+ return 0;
/* Interrupts already disabled by sc92031_suspend */
spin_lock_bh(>lock);
@@ -1553,7 +1545,6 @@ static int sc92031_resume(struct pci_dev *pdev)
else
netif_tx_disable(dev);
-out:
return 0;
}
@@ -1565,13 +1556,14 @@ static const struct pci_device_id
sc92031_pci_device_id_table[] = {
};
MODULE_DEVICE_TABLE(pci, sc92031_pci_device_id_table);
+static SIMPLE_DEV_PM_OPS(sc92031_pm_ops, sc92031_suspend, sc92031_resume);
+
static struct pci_driver sc92031_pci_driver = {
.name = SC92031_NAME,
.id_table = sc92031_pci_device_id_table,
.probe = sc92031_probe,
.remove = sc92031_remove,
- .suspend= sc92031_suspend,
- .resume = sc92031_resume,
+ .driver.pm = _pm_ops,
};
module_pci_driver(sc92031_pci_driver);
--
2.27.0
RE: [PATCH] exfat: retain 'VolumeFlags' properly
> Ping.. Hi Tetsuhiro, > > On 2020/07/15 19:06, Tetsuhiro Kohada wrote: > >> It looks complicated. It would be better to simply set/clear VOLUME DIRTY > >> bit. > > > > I think exfat_set_vol_flags() gets a little complicated, because it > > needs the followings (with bit operation) > > a) Set/Clear VOLUME_DIRTY. > > b) Set MEDIA_FAILUR. > > How about splitting these into separate functions as below? > > > exfat_set_volume_dirty() > exfat_set_vol_flags(sb, sbi->vol_flag | VOLUME_DIRTY); > > exfat_clear_volume_dirty() > exfat_set_vol_flags(sb, sbi->vol_flag & ~VOLUME_DIRTY); Looks good. > > exfat_set_media_failure() > exfat_set_vol_flags(sb, sbi->vol_flag | MEDIA_FAILURE); Where will this function be called? We don't need to create unused functions in advance... > > > The implementation is essentially the same for exfat_set_vol_flags(), but I > think the intention of the > operation will be easier to understand. Yes. Thanks! > > > BR > --- > Tetsuhiro Kohada
Re: [PATCH v18 3/3] Input: new da7280 haptic driver
Hello, On Wed, Jul 29, 2020 at 11:34:04PM -0700, Dmitry Torokhov wrote: > On Thu, Jul 30, 2020 at 08:16:31AM +0200, Uwe Kleine-König wrote: > > I suggested that some time ago with limited success, see > > https://lore.kernel.org/lkml/20200129115516.zsvxu56e6h7gh...@pathway.suse.cz/ > > . > > > > > In the meantime I would prefer using %d when we have integer > > > error. We should not see these error messages anyway ;) > > > > I don't agree. Error messages are supposed to be helpful and I prefer > > some casting between error pointer and error int over emitting bare > > numbers to the kernel log. (And additionally the uglyness might help to > > convince the vsprintf guys that %de might be a good idea after all :-) > > Sorry, but I do not think that I want to introduce this pointless > casting just to sway printk folks. Either we have proper way of > formatting error codes, or we do not and we stick with what was working > since forever. You got me wrong. Getting an argument to rediscuss %de is just a minor side reason for me. Getting an expressive error message is the relevant reason. There is also an inconsitence in what you suggest that I don't like. If in a probe function devm_clk_get fails it should say "-ENODEV" but if it's clk_enable that fails it should say "-5"? Also "we stick with what was working since forever" is a poor argument. Best regards Uwe -- Pengutronix e.K. | Uwe Kleine-König| Industrial Linux Solutions | https://www.pengutronix.de/ | signature.asc Description: PGP signature
[char-misc-next V2] mei: hdcp: fix mei_hdcp_verify_mprime() input paramter
wired_cmd_repeater_auth_stream_req_in has a variable
length array at the end. we use struct_size() overflow
macro to determine the size for the allocation and sending
size.
Fixes: c56967d674e3 (mei: hdcp: Replace one-element array with flexible-array
member)
Fixes: c56967d674e3 (mei: hdcp: Replace one-element array with flexible-array
member)
Cc: Ramalingam C
Cc: Gustavo A. R. Silva
Signed-off-by: Tomas Winkler
---
V2: Check for allocation failure.
drivers/misc/mei/hdcp/mei_hdcp.c | 40 +++-
1 file changed, 24 insertions(+), 16 deletions(-)
diff --git a/drivers/misc/mei/hdcp/mei_hdcp.c b/drivers/misc/mei/hdcp/mei_hdcp.c
index d1d3e025ca0e..f1205e0060db 100644
--- a/drivers/misc/mei/hdcp/mei_hdcp.c
+++ b/drivers/misc/mei/hdcp/mei_hdcp.c
@@ -546,38 +546,46 @@ static int mei_hdcp_verify_mprime(struct device *dev,
struct hdcp_port_data *data,
struct hdcp2_rep_stream_ready *stream_ready)
{
- struct wired_cmd_repeater_auth_stream_req_in
- verify_mprime_in = { { 0 } };
+ struct wired_cmd_repeater_auth_stream_req_in *verify_mprime_in;
struct wired_cmd_repeater_auth_stream_req_out
verify_mprime_out = { { 0 } };
struct mei_cl_device *cldev;
ssize_t byte;
+ size_t cmd_size;
if (!dev || !stream_ready || !data)
return -EINVAL;
cldev = to_mei_cl_device(dev);
- verify_mprime_in.header.api_version = HDCP_API_VERSION;
- verify_mprime_in.header.command_id = WIRED_REPEATER_AUTH_STREAM_REQ;
- verify_mprime_in.header.status = ME_HDCP_STATUS_SUCCESS;
- verify_mprime_in.header.buffer_len =
+ cmd_size = struct_size(verify_mprime_in, streams, data->k);
+ if (cmd_size == SIZE_MAX)
+ return -EINVAL;
+
+ verify_mprime_in = kzalloc(cmd_size, GFP_KERNEL);
+ if (!verify_mprime_in)
+ return -ENOMEM;
+
+ verify_mprime_in->header.api_version = HDCP_API_VERSION;
+ verify_mprime_in->header.command_id = WIRED_REPEATER_AUTH_STREAM_REQ;
+ verify_mprime_in->header.status = ME_HDCP_STATUS_SUCCESS;
+ verify_mprime_in->header.buffer_len =
WIRED_CMD_BUF_LEN_REPEATER_AUTH_STREAM_REQ_MIN_IN;
- verify_mprime_in.port.integrated_port_type = data->port_type;
- verify_mprime_in.port.physical_port = (u8)data->fw_ddi;
- verify_mprime_in.port.attached_transcoder = (u8)data->fw_tc;
+ verify_mprime_in->port.integrated_port_type = data->port_type;
+ verify_mprime_in->port.physical_port = (u8)data->fw_ddi;
+ verify_mprime_in->port.attached_transcoder = (u8)data->fw_tc;
+
+ memcpy(verify_mprime_in->m_prime, stream_ready->m_prime,
HDCP_2_2_MPRIME_LEN);
+ drm_hdcp_cpu_to_be24(verify_mprime_in->seq_num_m, data->seq_num_m);
- memcpy(verify_mprime_in.m_prime, stream_ready->m_prime,
- HDCP_2_2_MPRIME_LEN);
- drm_hdcp_cpu_to_be24(verify_mprime_in.seq_num_m, data->seq_num_m);
- memcpy(verify_mprime_in.streams, data->streams,
+ memcpy(verify_mprime_in->streams, data->streams,
array_size(data->k, sizeof(*data->streams)));
- verify_mprime_in.k = cpu_to_be16(data->k);
+ verify_mprime_in->k = cpu_to_be16(data->k);
- byte = mei_cldev_send(cldev, (u8 *)_mprime_in,
- sizeof(verify_mprime_in));
+ byte = mei_cldev_send(cldev, (u8 *)_mprime_in, cmd_size);
+ kfree(verify_mprime_in);
if (byte < 0) {
dev_dbg(dev, "mei_cldev_send failed. %zd\n", byte);
return byte;
--
2.25.4
Re: [PATCH 08/23] fs: don't change the address limit for ->write_iter in __kernel_write
On Wed, Jul 29, 2020 at 09:50:36PM +0100, Al Viro wrote: > On Tue, Jul 07, 2020 at 07:47:46PM +0200, Christoph Hellwig wrote: > > If we write to a file that implements ->write_iter there is no need > > to change the address limit if we send a kvec down. Implement that > > case, and prefer it over using plain ->write with a changed address > > limit if available. > > You are flipping the priorities of ->write and ->write_iter > for kernel_write(). Note by the end of the series (and what's been in linux-next for a while now) there is no order, as kernel_write only uses ->write_iter, so a few patches later this kinda becomes moot point. > Now, there are 4 instances of file_operations > where we have both. null_fops and zero_fops are fine either way - > ->write() and ->write_iter() do the same thing there (and arguably > removing ->write might be the right thing; the only reason I hesistate > is that writing to /dev/null *is* critical for many things, including > the proper mail delivery ;-) > > However, the other two (infinibarf and pcm) are different; there we > really have different semantics. I don't believe anything writes into > either under KERNEL_DS, but having kernel_write() and vfs_write() with > subtly different semantics is asking for trouble down the road. > > How about we remove ->write in null_fops/zero_fops and fail loudly if > *both* ->write() and ->write_iter() are present (in kernel_write(), > that is)? I'm fine with removing plain ->write for /dev/null and /dev/zero, as that seems the right thing to do. Failing the kernel ops if both are present sounds fine, I'm not sure about the loud part as it could be user triggered through splice. I'd go for the same kind of noticable not loud warning that we have for the lack of iter ops in kernel_read/write. > There's a similar situation on the read side - there we have /dev/null > with both ->read() and ->read_iter() (and there "remove ->read" is > obviously the right thing to do) *and* we have pcm crap, with different > semantics for ->read() and ->read_iter().
Re: [PATCH 22/23] fs: default to generic_file_splice_read for files having ->read_iter
On Thu, Jul 30, 2020 at 01:05:44AM +0100, Al Viro wrote:
> On Tue, Jul 07, 2020 at 07:48:00PM +0200, Christoph Hellwig wrote:
> > If a file implements the ->read_iter method, the iter based splice read
> > works and is always preferred over the ->read based one. Use it by
> > default in do_splice_to and remove all the direct assignment of
> > generic_file_splice_read to file_operations.
>
> The worst problem here is the assumption that all ->read_iter() instances
> will take pipe-backed destination; that's _not_ automatically true.
> In particular, it's almost certainly false for tap_read_iter() (as
> well as tun_chr_read_iter() in IFF_VNET_HDR case).
>
> Other potentially interesting cases: cuse and hugetlbfs.
>
> But in any case, that blind assertion ("iter based splice read works")
> really needs to be backed by something.
I think we need to fix that in the instances, as we really expect
->splice_read to just work instead of the caller knowing what could
work and what might not.
[PATCH v1] scsi: smartpqi: use generic power management
Drivers using legacy power management .suspen()/.resume() callbacks
have to manage PCI states and device's PM states themselves. They also
need to take care of standard configuration registers.
Switch to generic power management framework using a single
"struct dev_pm_ops" variable to take the unnecessary load from the driver.
This also avoids the need for the driver to directly call most of the PCI
helper functions and device power state control functions, as through
the generic framework PCI Core takes care of the necessary operations,
and drivers are required to do only device-specific jobs.
Signed-off-by: Vaibhav Gupta
---
drivers/scsi/smartpqi/smartpqi_init.c | 42 ++-
1 file changed, 29 insertions(+), 13 deletions(-)
diff --git a/drivers/scsi/smartpqi/smartpqi_init.c
b/drivers/scsi/smartpqi/smartpqi_init.c
index cd157f11eb22..dc8567236a23 100644
--- a/drivers/scsi/smartpqi/smartpqi_init.c
+++ b/drivers/scsi/smartpqi/smartpqi_init.c
@@ -8059,11 +8059,11 @@ static void pqi_process_module_params(void)
pqi_process_lockup_action_param();
}
-static __maybe_unused int pqi_suspend(struct pci_dev *pci_dev, pm_message_t
state)
+static __maybe_unused int pqi_suspend_late(struct device *dev, pm_message_t
state)
{
struct pqi_ctrl_info *ctrl_info;
- ctrl_info = pci_get_drvdata(pci_dev);
+ ctrl_info = dev_get_drvdata(dev);
pqi_disable_events(ctrl_info);
pqi_cancel_update_time_worker(ctrl_info);
@@ -8081,20 +8081,33 @@ static __maybe_unused int pqi_suspend(struct pci_dev
*pci_dev, pm_message_t stat
if (state.event == PM_EVENT_FREEZE)
return 0;
- pci_save_state(pci_dev);
- pci_set_power_state(pci_dev, pci_choose_state(pci_dev, state));
-
ctrl_info->controller_online = false;
ctrl_info->pqi_mode_enabled = false;
return 0;
}
-static __maybe_unused int pqi_resume(struct pci_dev *pci_dev)
+static __maybe_unused int pqi_suspend(struct device *dev)
+{
+ return pqi_suspend_late(dev, PMSG_SUSPEND);
+}
+
+static __maybe_unused int pqi_hibernate(struct device *dev)
+{
+ return pqi_suspend_late(dev, PMSG_HIBERNATE);
+}
+
+static __maybe_unused int pqi_freeze(struct device *dev)
+{
+ return pqi_suspend_late(dev, PMSG_FREEZE);
+}
+
+static __maybe_unused int pqi_resume(struct device *dev)
{
int rc;
struct pqi_ctrl_info *ctrl_info;
+ struct pci_dev *pci_dev = to_pci_dev(dev);
ctrl_info = pci_get_drvdata(pci_dev);
if (pci_dev->current_state != PCI_D0) {
@@ -8115,9 +8128,6 @@ static __maybe_unused int pqi_resume(struct pci_dev
*pci_dev)
return 0;
}
- pci_set_power_state(pci_dev, PCI_D0);
- pci_restore_state(pci_dev);
-
return pqi_ctrl_init_resume(ctrl_info);
}
@@ -8480,16 +8490,22 @@ static const struct pci_device_id pqi_pci_id_table[] = {
MODULE_DEVICE_TABLE(pci, pqi_pci_id_table);
+static const struct dev_pm_ops pqi_pm_ops = {
+ .suspend = pqi_suspend,
+ .resume = pqi_resume,
+ .freeze = pqi_freeze,
+ .thaw = pqi_resume,
+ .poweroff = pqi_hibernate,
+ .restore = pqi_resume,
+};
+
static struct pci_driver pqi_pci_driver = {
.name = DRIVER_NAME_SHORT,
.id_table = pqi_pci_id_table,
.probe = pqi_pci_probe,
.remove = pqi_pci_remove,
.shutdown = pqi_shutdown,
-#if defined(CONFIG_PM)
- .suspend = pqi_suspend,
- .resume = pqi_resume,
-#endif
+ .driver.pm = _pm_ops
};
static int __init pqi_init(void)
--
2.27.0
Re: [PATCH v9 0/4] driver core: add probe error check helper
On Tue, Jul 28, 2020 at 05:05:03PM +0200, Andrzej Hajda wrote: > Hi Greg, > > Apparently the patchset has no more comments. > > Could you take the patches to your tree? At least 1st and 2nd. All now queued up, thanks! greg k-h
Re: [PATCH 15/23] seq_file: switch over direct seq_read method calls to seq_read_iter
Al Viro writes:
> On Fri, Jul 17, 2020 at 11:09:13PM +0200, Thomas Gleixner wrote:
>>
>> Needs some thought and maybe some cocci help from Julia, but that's way
>> better than this brute force sed thing which results in malformed crap
>> like this:
>>
>> static const struct file_operations debug_stats_fops = {
>> .open = debug_stats_open,
>> .read_iter = seq_read_iter,
>> .llseek = seq_lseek,
>> .release= single_release,
>> };
>>
>> and proliferates the copy and paste voodoo programming.
>
> Better copy and paste than templates, IMO; at least the former is
> greppable; fucking DEFINE_..._ATRIBUTE is *NOT*, especially due
> to the use of ##.
Copy and paste itself is not the issue, but once the copy and paste orgy
starts you end up with more subtle bugs and silly differences than
copies. I spent enough time cleaning such crap up just to figure out
that once you've finished a full tree sweep you can start over.
grep for these things is a nuisance, but it's not rocket science to
figure it out. I rather have to figure that out than staring at a
gazillion of broken implementations.
Thanks,
tglx
Re: bpfilter logging write errors in dmesg
On Mon, Jul 27, 2020 at 04:50:13PM +0200, Christoph Hellwig wrote:
> Strange. Can you add this additional debugging patch:
Sorry Christoph,
didn't mean to leave you waiting. I got pulled into other stuff.
Christian
>
> diff --git a/fs/read_write.c b/fs/read_write.c
> index 4fb797822567a6..d0a8ada1efd954 100644
> --- a/fs/read_write.c
> +++ b/fs/read_write.c
> @@ -369,8 +369,10 @@ int rw_verify_area(int read_write, struct file *file,
> const loff_t *ppos, size_t
> int retval = -EINVAL;
>
> inode = file_inode(file);
> - if (unlikely((ssize_t) count < 0))
> + if (unlikely((ssize_t) count < 0)) {
> + printk("count invalid: %zd\n", count);
> return retval;
> + }
>
> /*
>* ranged mandatory locking does not apply to streams - it makes sense
> @@ -380,25 +382,35 @@ int rw_verify_area(int read_write, struct file *file,
> const loff_t *ppos, size_t
> loff_t pos = *ppos;
>
> if (unlikely(pos < 0)) {
> - if (!unsigned_offsets(file))
> + if (!unsigned_offsets(file)) {
> + printk("pos invalid: %lld\n", pos);
> return retval;
> + }
> if (count >= -pos) /* both values are in 0..LLONG_MAX */
> return -EOVERFLOW;
> } else if (unlikely((loff_t) (pos + count) < 0)) {
> - if (!unsigned_offsets(file))
> + if (!unsigned_offsets(file)) {
> + printk("pos+count invalid: %lld, %zd\n", pos,
> count);
> return retval;
> + }
> }
>
> if (unlikely(inode->i_flctx && mandatory_lock(inode))) {
> retval = locks_mandatory_area(inode, file, pos, pos +
> count - 1,
> read_write == READ ? F_RDLCK : F_WRLCK);
> - if (retval < 0)
> + if (retval < 0) {
> + if (retval == -EINVAL)
> + printk("locks_mandatory_area\n");
> return retval;
> + }
> }
> }
>
> - return security_file_permission(file,
> + retval = security_file_permission(file,
> read_write == READ ? MAY_READ : MAY_WRITE);
> + if (retval == -EINVAL)
> + printk("security_file_permission\n");
> + return retval;
> }
>
> static ssize_t new_sync_read(struct file *filp, char __user *buf, size_t
> len, loff_t *ppos)
KASAN: slab-out-of-bounds Read in ath9k_hif_usb_rx_cb (2)
Hello, syzbot found the following issue on: HEAD commit:ab4dc051 usb: mtu3: simplify mtu3_req_complete() git tree: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/usb.git usb-testing console output: https://syzkaller.appspot.com/x/log.txt?x=11c0666c90 kernel config: https://syzkaller.appspot.com/x/.config?x=fb6677a3d4f11788 dashboard link: https://syzkaller.appspot.com/bug?extid=6ecc26112e7241c454ef compiler: gcc (GCC) 10.1.0-syz 20200507 syz repro: https://syzkaller.appspot.com/x/repro.syz?x=171e600490 IMPORTANT: if you fix the issue, please add the following tag to the commit: Reported-by: syzbot+6ecc26112e7241c45...@syzkaller.appspotmail.com == BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:627 [inline] BUG: KASAN: slab-out-of-bounds in ath9k_hif_usb_rx_cb+0xd7d/0xf80 drivers/net/wireless/ath/ath9k/hif_usb.c:671 Read of size 4 at addr 8881cbf6c090 by task swapper/0/0 CPU: 0 PID: 0 Comm: swapper/0 Not tainted 5.8.0-rc7-syzkaller #0 Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 01/01/2011 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0xf6/0x16e lib/dump_stack.c:118 print_address_description.constprop.0+0x1a/0x210 mm/kasan/report.c:383 __kasan_report mm/kasan/report.c:513 [inline] kasan_report.cold+0x37/0x7c mm/kasan/report.c:530 ath9k_hif_usb_rx_stream drivers/net/wireless/ath/ath9k/hif_usb.c:627 [inline] ath9k_hif_usb_rx_cb+0xd7d/0xf80 drivers/net/wireless/ath/ath9k/hif_usb.c:671 __usb_hcd_giveback_urb+0x32d/0x560 drivers/usb/core/hcd.c:1650 usb_hcd_giveback_urb+0x367/0x410 drivers/usb/core/hcd.c:1716 dummy_timer+0x11f2/0x3240 drivers/usb/gadget/udc/dummy_hcd.c:1967 call_timer_fn+0x1ac/0x6e0 kernel/time/timer.c:1415 expire_timers kernel/time/timer.c:1460 [inline] __run_timers.part.0+0x54c/0x9e0 kernel/time/timer.c:1784 __run_timers kernel/time/timer.c:1756 [inline] run_timer_softirq+0x80/0x120 kernel/time/timer.c:1797 __do_softirq+0x222/0x95b kernel/softirq.c:292 asm_call_on_stack+0xf/0x20 arch/x86/entry/entry_64.S:711 __run_on_irqstack arch/x86/include/asm/irq_stack.h:22 [inline] run_on_irqstack_cond arch/x86/include/asm/irq_stack.h:48 [inline] do_softirq_own_stack+0xed/0x140 arch/x86/kernel/irq_64.c:77 invoke_softirq kernel/softirq.c:387 [inline] __irq_exit_rcu kernel/softirq.c:417 [inline] irq_exit_rcu+0x150/0x1f0 kernel/softirq.c:429 sysvec_apic_timer_interrupt+0x49/0xc0 arch/x86/kernel/apic/apic.c:1091 asm_sysvec_apic_timer_interrupt+0x12/0x20 arch/x86/include/asm/idtentry.h:585 RIP: 0010:native_irq_disable arch/x86/include/asm/irqflags.h:49 [inline] RIP: 0010:arch_local_irq_disable arch/x86/include/asm/irqflags.h:89 [inline] RIP: 0010:acpi_safe_halt+0x72/0x90 drivers/acpi/processor_idle.c:112 Code: 74 06 5b e9 e0 4c 8f fb e8 db 4c 8f fb e8 26 d8 94 fb e9 0c 00 00 00 e8 cc 4c 8f fb 0f 00 2d 05 63 74 00 e8 c0 4c 8f fb fb f4 e8 18 d2 94 fb 5b e9 b2 4c 8f fb 48 89 df e8 fa fb b8 fb eb ab RSP: 0018:87207c80 EFLAGS: 0293 RAX: RBX: RCX: RDX: 8722f840 RSI: 85b05d40 RDI: 85b05d2a RBP: 8881d8cca864 R08: R09: R10: 0001 R11: R12: 8881d8cca864 R13: 10e40f99 R14: 8881d8cca865 R15: 0001 acpi_idle_do_entry+0x15c/0x1b0 drivers/acpi/processor_idle.c:525 acpi_idle_enter+0x3f0/0xa50 drivers/acpi/processor_idle.c:651 cpuidle_enter_state+0xff/0x870 drivers/cpuidle/cpuidle.c:235 cpuidle_enter+0x4a/0xa0 drivers/cpuidle/cpuidle.c:346 call_cpuidle kernel/sched/idle.c:126 [inline] cpuidle_idle_call kernel/sched/idle.c:214 [inline] do_idle+0x3d6/0x5a0 kernel/sched/idle.c:276 cpu_startup_entry+0x14/0x20 kernel/sched/idle.c:372 start_kernel+0xa1b/0xa56 init/main.c:1043 secondary_startup_64+0xb6/0xc0 arch/x86/kernel/head_64.S:243 Allocated by task 0: (stack is not available) Freed by task 0: (stack is not available) The buggy address belongs to the object at 8881cbf6c000 which belongs to the cache kmalloc-1k of size 1024 The buggy address is located 144 bytes inside of 1024-byte region [8881cbf6c000, 8881cbf6c400) The buggy address belongs to the page: page:ea00072fda00 refcount:1 mapcount:0 mapping: index:0x0 head:ea00072fda00 order:3 compound_mapcount:0 compound_pincount:0 flags: 0x2010200(slab|head) raw: 02010200 dead0100 dead0122 8881da002280 raw: 80100010 0001 page dumped because: kasan: bad access detected Memory state around the buggy address: 8881cbf6bf80: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc 8881cbf6c000: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc >8881cbf6c080: fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc fc
Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default
Hi Anson, On Thu, 2020-07-30 at 02:11 +, Anson Huang wrote: > Hi, Philipp/Rob > > > Subject: Re: [PATCH V3 3/3] pci: imx: Select RESET_IMX7 by default > > > > On Wed, 2020-07-29 at 09:26 -0600, Rob Herring wrote: > > > On Mon, Jul 20, 2020 at 8:26 AM Anson Huang > > wrote: > > > > i.MX7 reset driver now supports module build and it is no longer > > > > built in by default, so i.MX PCI driver needs to select it > > > > explicitly due to it is NOT supporting loadable module currently. > > > > > > > > Signed-off-by: Anson Huang > > > > --- > > > > No change. > > > > --- > > > > drivers/pci/controller/dwc/Kconfig | 1 + > > > > 1 file changed, 1 insertion(+) > > > > > > > > diff --git a/drivers/pci/controller/dwc/Kconfig > > > > b/drivers/pci/controller/dwc/Kconfig > > > > index 044a376..bcf63ce 100644 > > > > --- a/drivers/pci/controller/dwc/Kconfig > > > > +++ b/drivers/pci/controller/dwc/Kconfig > > > > @@ -90,6 +90,7 @@ config PCI_EXYNOS > > > > > > > > config PCI_IMX6 > > > > bool "Freescale i.MX6/7/8 PCIe controller" > > > > + select RESET_IMX7 > > > > > > This will break as select will not cause all of RESET_IMX7's > > > dependencies to be met. It also doesn't scale. Are you going to do the > > > same thing for clocks, pinctrl, gpio, etc.? > > > > > > You should make the PCI driver work as a module. > > > > Oh, also PCI_IMX6 is used on (surprise) i.MX6, which doesn't need > > RESET_IMX7 at all. > > > > How about hiding the RESET_IMX7 option and setting it default y if > > PCI_IMX6 is enabled, as an interim solution? > > Like below, RESET_IMX7 is already default y when SOC_IMX7D, now added > PCI_IMX6, > let me know if it is OK for you, then I will send new patch for review. > > +++ b/drivers/reset/Kconfig > @@ -68,7 +68,7 @@ config RESET_IMX7 > tristate "i.MX7/8 Reset Driver" I was thinking something like tristate "i.MX7/8 Reset Driver" if COMPILE_TEST || !PCI_IMX6 > depends on HAS_IOMEM > depends on SOC_IMX7D || (ARM64 && ARCH_MXC) || COMPILE_TEST > - default y if SOC_IMX7D > + default y if (SOC_IMX7D || PCI_IMX6) Yes, although without the above I think it could still be disabled manually or via oldconfig. regards Philipp
Re: [patch V5 05/15] entry: Provide infrastructure for work before transitioning to guest mode
Qian Cai writes:
> On Wed, Jul 22, 2020 at 11:59:59PM +0200, Thomas Gleixner wrote:
> SR-IOV will start trigger a warning below in this commit,
>
> [ 765.434611] WARNING: CPU: 13 PID: 3377 at include/linux/entry-kvm.h:75
> kvm_arch_vcpu_ioctl_run+0xb52/0x1320 [kvm]
Yes, I'm a moron. Fixed it locally and failed to transfer the fixup when
merging it. Fix below.
> [ 768.221270] softirqs last disabled at (5093): []
> asm_call_on_stack+0x12/0x20
> [ 768.267273] ---[ end trace 8730450ad8cfee9f ]---
Can you pretty please trim your replies?
>> ---
>> V5: Rename exit -> xfer (Sean)
Thanks,
tglx
---
diff --git a/arch/x86/kvm/x86.c b/arch/x86/kvm/x86.c
index 82d4a9e88908..532597265c50 100644
--- a/arch/x86/kvm/x86.c
+++ b/arch/x86/kvm/x86.c
@@ -8682,7 +8682,7 @@ static int vcpu_run(struct kvm_vcpu *vcpu)
break;
}
- if (xfer_to_guest_mode_work_pending()) {
+ if (__xfer_to_guest_mode_work_pending()) {
srcu_read_unlock(>srcu, vcpu->srcu_idx);
r = xfer_to_guest_mode_handle_work(vcpu);
if (r)
Re: [PATCH V8 1/6] clk: imx6sl: Use BIT(x) to avoid shifting signed 32-bit value by 31 bits
On Thu, Jul 30, 2020 at 2:03 AM Anson Huang wrote: > > Use readl_relaxed() instead of __raw_readl(), and use BIT(x) > instead of (1 << X) to fix below build warning reported by kernel > test robot: > > drivers/clk/imx/clk-imx6sl.c:149:49: warning: Shifting signed 32-bit > value by 31 bits is undefined behaviour [shiftTooManyBitsSigned] > while (!(__raw_readl(anatop_base + PLL_ARM) & BM_PLL_ARM_LOCK)) > > Signed-off-by: Anson Huang > Reported-by: kernel test robot Acked-by: Arnd Bergmann
Re: [RFC PATCH] arm64: defconfig: Disable fine-grained task level IRQ time accounting
Hi Vladimir, On Wed Jul 29 2020, Vladimir Oltean wrote: > For more context, here is my original report of the issue: > https://lkml.org/lkml/2020/6/4/1062 > > Just like you, I could not reproduce the RCU stalls and system hang on a > 5.6-rt kernel, just on mainline and derivatives, using the plain > defconfig. > > The issue is not specific to Layerscape or i.MX8, but rather I was able > to see the same behavior on Marvell Armada 37xx as well as Qualcomm > MSM8976. > > So, while of course I agree that disabling IRQ time accounting for arm64 > isn't a real solution, it isn't by far an exaggerated proposal either. > Nonetheless, the patch is just a RFC and should be treated as such. We > are at a loss when it comes to debugging this any further and we would > appreciate some pointers. Yeah, sure. I'll try to reproduce this issue first. So it triggers with: * arm64 * mainline, not -rt kernel * opened serial console * irq accounting enabled Anything else? Thanks, Kurt signature.asc Description: PGP signature
Re: [PATCH] ASoC: fsl-asoc-card: Remove fsl_asoc_card_set_bias_level function
On Mon, Jul 27, 2020 at 8:58 AM Nicolin Chen wrote:
>
> On Sun, Jul 26, 2020 at 07:20:17PM +0800, Shengjiu Wang wrote:
> > With this case:
> > aplay -Dhw:x 16khz.wav 24khz.wav
> > There is sound distortion for 24khz.wav. The reason is that setting
> > PLL of WM8962 with set_bias_level function, the bias level is not
> > changed when 24khz.wav is played, then the PLL won't be reset, the
> > clock is not correct, so distortion happens.
> >
> > The resolution of this issue is to remove fsl_asoc_card_set_bias_level.
> > Move PLL configuration to hw_params and hw_free.
>
> Hmm...using set_bias_level() instead of hw_params/hw_free() was
> strongly suggested by Mark when I got imx-wm8962 machine driver
> upstream. So we will need his input here, although I personally
> don't have a problem with it...
>
> > After removing fsl_asoc_card_set_bias_level, also test WM8960 case,
> > it can work.
> >
> > Fixes: 708b4351f08c ("ASoC: fsl: Add Freescale Generic ASoC Sound Card with
> > ASRC support")
> > Signed-off-by: Shengjiu Wang
> > ---
> > sound/soc/fsl/fsl-asoc-card.c | 149 +++---
> > 1 file changed, 66 insertions(+), 83 deletions(-)
> >
> > diff --git a/sound/soc/fsl/fsl-asoc-card.c b/sound/soc/fsl/fsl-asoc-card.c
> > index 4848ba61d083..0517dbb3e908 100644
> > --- a/sound/soc/fsl/fsl-asoc-card.c
> > +++ b/sound/soc/fsl/fsl-asoc-card.c
> > @@ -73,6 +73,7 @@ struct cpu_priv {
> > * @codec_priv: CODEC private data
> > * @cpu_priv: CPU private data
> > * @card: ASoC card structure
> > + * @is_stream_in_use: flags for release resource in hw_free
>
> Would love to see something shorter... Could we reuse similar
> one below, borrowing from fsl_ssi driver?
>
> * @streams: Mask of current active streams: BIT(TX) and BIT(RX)
>
will send v2 for this change.
> > static int fsl_asoc_card_audmux_init(struct device_node *np,
> >struct fsl_asoc_card_priv *priv)
> > {
> > @@ -611,7 +600,6 @@ static int fsl_asoc_card_probe(struct platform_device
> > *pdev)
> > /* Diversify the card configurations */
> > if (of_device_is_compatible(np, "fsl,imx-audio-cs42888")) {
> > codec_dai_name = "cs42888";
> > - priv->card.set_bias_level = NULL;
>
> Can check if set_bias_level is still being used with this change.
[PATCH v4] usb: typec: tcpm: Migrate workqueue to RT priority for processing events
"tReceiverResponse 15 ms Section 6.6.2
The receiver of a Message requiring a response Shall respond
within tReceiverResponse in order to ensure that the
sender’s SenderResponseTimer does not expire."
When the cpu complex is busy running other lower priority
work items, TCPM's work queue sometimes does not get scheduled
on time to meet the above requirement from the spec.
Moving to kthread_work apis to run with real time priority.
Just lower than the default threaded irq priority,
MAX_USER_RT_PRIO/2 + 1. (Higher number implies lower priority).
Further, as observed in 1ff688209e2e, moving to hrtimers to
overcome scheduling latency while scheduling the delayed work.
TCPM has three work streams:
1. tcpm_state_machine
2. vdm_state_machine
3. event_work
tcpm_state_machine and vdm_state_machine both schedule work in
future i.e. delayed. Hence each of them have a corresponding
hrtimer, tcpm_state_machine_timer & vdm_state_machine_timer.
When work is queued right away kthread_queue_work is used.
Else, the relevant timer is programmed and made to queue
the kthread_work upon timer expiry.
kthread_create_worker only creates one kthread worker thread,
hence single threadedness of workqueue is retained.
Signed-off-by: Badhri Jagan Sridharan
---
Changes since v1:(Guenter's suggestions)
- Remove redundant call to hrtimer_cancel while calling
hrtimer_start.
Changes since v2:(Greg KH's suggestions)
- Rebase usb-next TOT.
633198cd2945b7 (HEAD -> usb-next-1) usb: typec: tcpm: Migrate workqueue to RT
priority for processing events
fa56dd9152ef95 (origin/usb-next) Merge tag 'usb-serial-5.9-rc1' of
https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into usb-next
25252919a1050e xhci: dbgtty: Make some functions static
b0e02550346e67 xhci: dbc: Make function xhci_dbc_ring_alloc() static
ca6377900974c3 Revert "usb: dwc2: override PHY input signals with usb role
switch support"
09df709cb5aeb2 Revert "usb: dwc2: don't use ID/Vbus detection if
usb-role-switch on STM32MP15 SoCs"
17a82716587e9d USB: iowarrior: fix up report size handling for some devices
e98ba8cc3f8a89 Merge tag 'usb-for-v5.9' of
git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next
c97793089b11f7 Merge 5.8-rc7 into usb-next
92ed301919932f (tag: v5.8-rc7, origin/usb-linus, origin/main) Linux 5.8-rc7
- Added reviewed-by tags.
Changes since v3:(Greg KH's suggestions)
- Rebase usb-next TOT
Resolve merge conflict with "usb: typec: tcpm: Move
mod_delayed_work(>vdm_state_machine) call into tcpm_queue_vdm()"
b8f151122648b0 (HEAD -> usb-next-1) usb: typec: tcpm: Migrate workqueue to RT
priority for processing events
ab4dc051d7b39c (origin/usb-testing, origin/usb-next) usb: mtu3: simplify
mtu3_req_complete()
f1e51e99ed498d usb: mtu3: clear dual mode of u3port when disable device
54402373c45e44 usb: mtu3: use MTU3_EP_WEDGE flag
bf77804b1cbdeb usb: mtu3: remove useless member @busy in mtu3_ep struct
ba428976a8827f usb: mtu3: remove repeated error log
dc4c1aa7eae99d usb: mtu3: add ->udc_set_speed()
1258450ef38af7 usb: mtu3: introduce a funtion to check maximum speed
---
drivers/usb/typec/tcpm/tcpm.c | 133 +++---
1 file changed, 89 insertions(+), 44 deletions(-)
diff --git a/drivers/usb/typec/tcpm/tcpm.c b/drivers/usb/typec/tcpm/tcpm.c
index 3ef37202ee3757..fe9dff0337b156 100644
--- a/drivers/usb/typec/tcpm/tcpm.c
+++ b/drivers/usb/typec/tcpm/tcpm.c
@@ -8,8 +8,10 @@
#include
#include
#include
+#include
#include
#include
+#include
#include
#include
#include
@@ -28,7 +30,8 @@
#include
#include
#include
-#include
+
+#include
#define FOREACH_STATE(S) \
S(INVALID_STATE), \
@@ -203,7 +206,7 @@ struct tcpm_port {
struct device *dev;
struct mutex lock; /* tcpm state machine lock */
- struct workqueue_struct *wq;
+ struct kthread_worker *wq;
struct typec_capability typec_caps;
struct typec_port *typec_port;
@@ -247,15 +250,17 @@ struct tcpm_port {
enum tcpm_state prev_state;
enum tcpm_state state;
enum tcpm_state delayed_state;
- unsigned long delayed_runtime;
+ ktime_t delayed_runtime;
unsigned long delay_ms;
spinlock_t pd_event_lock;
u32 pd_events;
- struct work_struct event_work;
- struct delayed_work state_machine;
- struct delayed_work vdm_state_machine;
+ struct kthread_work event_work;
+ struct hrtimer state_machine_timer;
+ struct kthread_work state_machine;
+ struct hrtimer vdm_state_machine_timer;
+ struct kthread_work vdm_state_machine;
bool state_machine_running;
struct completion tx_complete;
@@ -340,7 +345,7 @@ struct tcpm_port {
};
struct pd_rx_event {
- struct work_struct work;
+ struct kthread_work work;
struct tcpm_port *port;
struct pd_message msg;
};
[PATCH net] net: ll_temac: Use devm_platform_ioremap_resource_byname()
platform_get_resource() may fail and return NULL, so we had better
check its return value to avoid a NULL pointer dereference a bit later
in the code. Fix it to use devm_platform_ioremap_resource_byname()
instead of calling platform_get_resource_byname() and devm_ioremap().
Fixes: 8425c41d1ef7 ("net: ll_temac: Extend support to non-device-tree
platforms")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
drivers/net/ethernet/xilinx/ll_temac_main.c | 6 ++
1 file changed, 2 insertions(+), 4 deletions(-)
diff --git a/drivers/net/ethernet/xilinx/ll_temac_main.c
b/drivers/net/ethernet/xilinx/ll_temac_main.c
index 929244064abd..9a15f14daa47 100644
--- a/drivers/net/ethernet/xilinx/ll_temac_main.c
+++ b/drivers/net/ethernet/xilinx/ll_temac_main.c
@@ -1407,10 +1407,8 @@ static int temac_probe(struct platform_device *pdev)
}
/* map device registers */
- res = platform_get_resource(pdev, IORESOURCE_MEM, 0);
- lp->regs = devm_ioremap(>dev, res->start,
- resource_size(res));
- if (!lp->regs) {
+ lp->regs = devm_platform_ioremap_resource_byname(pdev, 0);
+ if (IS_ERR(lp->regs)) {
dev_err(>dev, "could not map TEMAC registers\n");
return -ENOMEM;
}
--
2.17.1
Re: [PATCH V7 1/6] clk: imx6sl: Use BIT(x) to avoid shifting signed 32-bit value by 31 bits
On Thu, Jul 30, 2020 at 3:14 AM Anson Huang wrote: > > Subject: Re: [PATCH V7 1/6] clk: imx6sl: Use BIT(x) to avoid shifting signed > > 32-bit value by 31 bits > > or you could read Documentation/process/submit-checklist.rst, > > where rule #1 says: > > > > 1) If you use a facility then #include the file that defines/declares > >that facility. Don't depend on other header files pulling in ones > >that you use. > > Understood, while I search "BIT()" in clk driver, most of the drivers does > NOT include > linux/bits.h even they use it. > > But OK, I will send V9 to include it. Ok, good. I have a patch series that adds it to a lot of files. Note that linux/bitops.h itself is a fairly big header with everything else that it includes, and it is included almost everywhere indirectly. When we change the other headers to not require linux/bitops.h any more, everything that uses BIT() needs to include linux/bits.h. Arnd
Re: [PATCH v3] usb: typec: tcpm: Migrate workqueue to RT priority for processing events
Hi Greg, Yes Guenter's suspicion is right. Mine was conflicting with Han's following patch: commit 5f2b8d87bca528616e04344d1fc4032dc5ec0f3d Author: Hans de Goede Date: Fri Jul 24 19:46:57 2020 +0200 usb: typec: tcpm: Move mod_delayed_work(>vdm_state_machine) call into tcpm_queue_vdm() All callers of tcpm_queue_vdm() immediately follow the tcpm_queue_vdm() vdm call with a: mod_delayed_work(port->wq, >vdm_state_machine, 0); Call, fold this into tcpm_queue_vdm() itself. Reviewed-by: Guenter Roeck Reviewed-by: Heikki Krogerus Signed-off-by: Hans de Goede Link: https://lore.kernel.org/r/20200724174702.61754-1-hdego...@redhat.com Signed-off-by: Greg Kroah-Hartman Fixed merged conflicts and sent out the v4 version of the patch. Added hdego...@redhat.com to CC as well. Thanks, Badhri On Wed, Jul 29, 2020 at 11:44 PM Greg Kroah-Hartman wrote: > > On Wed, Jul 29, 2020 at 07:24:57PM -0700, Badhri Jagan Sridharan wrote: > > "tReceiverResponse 15 ms Section 6.6.2 > > The receiver of a Message requiring a response Shall respond > > within tReceiverResponse in order to ensure that the > > sender’s SenderResponseTimer does not expire." > > > > When the cpu complex is busy running other lower priority > > work items, TCPM's work queue sometimes does not get scheduled > > on time to meet the above requirement from the spec. > > Moving to kthread_work apis to run with real time priority. > > Just lower than the default threaded irq priority, > > MAX_USER_RT_PRIO/2 + 1. (Higher number implies lower priority). > > > > Further, as observed in 1ff688209e2e, moving to hrtimers to > > overcome scheduling latency while scheduling the delayed work. > > > > TCPM has three work streams: > > 1. tcpm_state_machine > > 2. vdm_state_machine > > 3. event_work > > > > tcpm_state_machine and vdm_state_machine both schedule work in > > future i.e. delayed. Hence each of them have a corresponding > > hrtimer, tcpm_state_machine_timer & vdm_state_machine_timer. > > > > When work is queued right away kthread_queue_work is used. > > Else, the relevant timer is programmed and made to queue > > the kthread_work upon timer expiry. > > > > kthread_create_worker only creates one kthread worker thread, > > hence single threadedness of workqueue is retained. > > > > Signed-off-by: Badhri Jagan Sridharan > > Reviewed-by: Guenter Roeck > > Reviewed-by: Heikki Krogerus > > --- > > Changes since v1:(Guenter's suggestions) > > - Remove redundant call to hrtimer_cancel while calling > > hrtimer_start. > > > > Changes since v2:(Greg KH's suggestions) > > - Rebase usb-next TOT. > > 633198cd2945b7 (HEAD -> usb-next-1) usb: typec: tcpm: Migrate workqueue > > to RT priority for processing events > > fa56dd9152ef95 (origin/usb-next) Merge tag 'usb-serial-5.9-rc1' of > > https://git.kernel.org/pub/scm/linux/kernel/git/johan/usb-serial into > > usb-next > > 25252919a1050e xhci: dbgtty: Make some functions static > > b0e02550346e67 xhci: dbc: Make function xhci_dbc_ring_alloc() static > > ca6377900974c3 Revert "usb: dwc2: override PHY input signals with usb > > role switch support" > > 09df709cb5aeb2 Revert "usb: dwc2: don't use ID/Vbus detection if > > usb-role-switch on STM32MP15 SoCs" > > 17a82716587e9d USB: iowarrior: fix up report size handling for some > > devices > > e98ba8cc3f8a89 Merge tag 'usb-for-v5.9' of > > git://git.kernel.org/pub/scm/linux/kernel/git/balbi/usb into usb-next > > c97793089b11f7 Merge 5.8-rc7 into usb-next > > 92ed301919932f (tag: v5.8-rc7, origin/usb-linus, origin/main) Linux > > 5.8-rc7 > > > > Hm, still does not apply. I think it has something to do with other > patches that landed before yours, can you rebase again? > > thanks, > > greg k-h
Re: [PATCH 01/17] dt-bindings: clocks: imx8mp: Rename audiomix ids clocks to audio_blk_ctrl
On 20-07-29 12:47:26, Stephen Boyd wrote: > Quoting Abel Vesa (2020-07-29 05:07:47) > > In the reference manual the actual name is Audio BLK_CTRL. > > Lets make it more obvious here by renaming from audiomix to audio_blk_ctrl. > > And this is safe because there aren't any users of the defines? Yes, these defines are not used at all yet. This patchset introduces the first user.
Re: [PATCH 10/17] Documentation: bindings: clk: Add bindings for i.MX BLK_CTRL
On 20-07-29 12:49:41, Stephen Boyd wrote:
> Quoting Abel Vesa (2020-07-29 05:07:56)
> > diff --git a/Documentation/devicetree/bindings/clock/fsl,imx-blk-ctrl.yaml
> > b/Documentation/devicetree/bindings/clock/fsl,imx-blk-ctrl.yaml
> > new file mode 100644
> > index ..036d3d3
> > --- /dev/null
> > +++ b/Documentation/devicetree/bindings/clock/fsl,imx-blk-ctrl.yaml
> > @@ -0,0 +1,55 @@
> > +# SPDX-License-Identifier: (GPL-2.0-only)
> > +%YAML 1.2
> > +---
> > +$id:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fschemas%2Fclock%2Ffsl%2Cimx-blk-ctrl.yaml%23data=02%7C01%7Cabel.vesa%40nxp.com%7C30d7fec624c44b4f85e108d833f88a5e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637316489861505079sdata=63jhxp1rw%2BMYPlc%2BhwSjOwBvN%2Fikf5PXUKGjEXD5agM%3Dreserved=0
> > +$schema:
> > https://eur01.safelinks.protection.outlook.com/?url=http%3A%2F%2Fdevicetree.org%2Fmeta-schemas%2Fcore.yaml%23data=02%7C01%7Cabel.vesa%40nxp.com%7C30d7fec624c44b4f85e108d833f88a5e%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7C637316489861505079sdata=9%2FLcFRkMHXx1%2FHsEfFyhNmI73hDC2GUu2s%2BE9EQTUSE%3Dreserved=0
> [...]
> > +
> > +examples:
> > + - |
> > +audio-blk-ctrl: blk-ctrl@30e2 {
>
> clock-controller@30e2 {
Will change in the new version.
>
> > + compatible = "fsl,imx8mp-blk-ctrl", "syscon";
> > + reg = <0x30e2 0x1>;
> > + power-domains = <_pd>;
> > +
> > + #clock-cells = <1>;
> > + #reset-cells = <1>;
> > +};
Re: [PATCH] Revert "Bluetooth: btusb: Disable runtime suspend on Realtek devices"
Hi Kai-Heng, >> On Jul 30, 2020, at 07:17, Abhishek Pandit-Subedi >> wrote: >> >> This reverts commit 7ecacafc240638148567742cca41aa7144b4fe1e. >> >> Testing this change on a board with RTL8822CE, I found that enabling >> autosuspend has no effect on the stability of the system. The board >> continued working after autosuspend, suspend and reboot. > > The original issue was found on 8723DE. Do you have one to test with? > The rtw88 codebase has changed a lot and maybe it's already fixed in mainline. > Let me do some test and I'll report back. > >> >> The original commit makes it impossible to enable autosuspend on working >> systems so it should be reverted. Disabling autosuspend should be done >> via module param or udev in userspace instead. >> >> Signed-off-by: Abhishek Pandit-Subedi >> --- >> We have a few Chromebooks using the RTL 8822CE part over USB and they >> are running without problems with autosuspend enabled. While bringing up >> a new board, I found some power regressions that I was able to narrow >> down to this change so I'm requesting a revert. >> >> I tested this on Hp Chromebook 14a (running 4.14 kernel and 5.4 kernel) >> with this revert: >> * Enabled autosuspend, used it normally with a HID device >> * Suspended the Chromebook and verified it worked normally on resume >> * Rebooted the Chromebook and verified Bluetooth was working on next >> boot >> >> I didn't see the issue that was originally reported with this fix. For >> the original reporter, if you're still seeing this issue, there are >> other ways to disable autosuspend for your device: >> * set module param: enable_autosuspend=0 >> * change your kconfig so BT_HCIBTUSB_AUTOSUSPEND=n >> * use a udev rule to disable autosuspend for specific vid:pid >> >> Keeping this change in the kernel makes it impossible to enable >> autosuspend so it should be reverted. > > It's apparently a driver/firmware/hardware issue, so the fix should keep > inside the kernel. > However, the fix can be more precise and target only 8723DE. lets do that and lets do it quickly since the merge window is close. Otherwise I really have to revert that patch. Regards Marcel
Re: [PATCH v2 stable-5.4.y] Revert "dpaa_eth: fix usage as DSA master, try 3"
On Wed, Jun 24, 2020 at 03:45:17PM +0300, Vladimir Oltean wrote:
> From: Vladimir Oltean
>
> This reverts commit 40a904b1c2e57b22dd002dfce73688871cb0bac8.
>
> The patch is not wrong, but the Fixes: tag is. It should have been:
>
> Fixes: 060ad66f9795 ("dpaa_eth: change DMA device")
>
> which means that it's fixing a commit which was introduced in:
>
> git tag --contains 060ad66f97954
> v5.5
>
> which then means it should have not been backported to linux-5.4.y,
> where things _were_ working and now they're not.
>
> Reported-by: Joakim Tjernlund
> Signed-off-by: Vladimir Oltean
> ---
> Changes in v1:
> Adjusted the commit message from linux-4.19.y to linux-5.4.y
>
> Changes in v2:
> Fixed the sha1sum of the reverted commit.
>
> drivers/net/ethernet/freescale/dpaa/dpaa_eth.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> index 6683409fbd4a..4b21ae27a9fd 100644
> --- a/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> +++ b/drivers/net/ethernet/freescale/dpaa/dpaa_eth.c
> @@ -2796,7 +2796,7 @@ static int dpaa_eth_probe(struct platform_device *pdev)
> }
>
> /* Do this here, so we can be verbose early */
> - SET_NETDEV_DEV(net_dev, dev->parent);
> + SET_NETDEV_DEV(net_dev, dev);
> dev_set_drvdata(dev, net_dev);
>
> priv = netdev_priv(net_dev);
> --
> 2.25.1
>
Now queued up, thanks.
greg k-h
[PATCH net] net: gemini: Fix missing clk_disable_unprepare() in error path of gemini_ethernet_port_probe()
Fix the missing clk_disable_unprepare() before return
from gemini_ethernet_port_probe() in the error handling case.
Fixes: 4d5ae32f5e1e ("net: ethernet: Add a driver for Gemini gigabit ethernet")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
drivers/net/ethernet/cortina/gemini.c | 5 -
1 file changed, 4 insertions(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/cortina/gemini.c
b/drivers/net/ethernet/cortina/gemini.c
index 8d13ea370db1..66e67b24a887 100644
--- a/drivers/net/ethernet/cortina/gemini.c
+++ b/drivers/net/ethernet/cortina/gemini.c
@@ -2446,6 +2446,7 @@ static int gemini_ethernet_port_probe(struct
platform_device *pdev)
port->reset = devm_reset_control_get_exclusive(dev, NULL);
if (IS_ERR(port->reset)) {
dev_err(dev, "no reset\n");
+ clk_disable_unprepare(port->pclk);
return PTR_ERR(port->reset);
}
reset_control_reset(port->reset);
@@ -2501,8 +2502,10 @@ static int gemini_ethernet_port_probe(struct
platform_device *pdev)
IRQF_SHARED,
port_names[port->id],
port);
- if (ret)
+ if (ret) {
+ clk_disable_unprepare(port->pclk);
return ret;
+ }
ret = register_netdev(netdev);
if (!ret) {
--
2.17.1
Re: [PATCH v2] checkpatch: Fix the usage of capture group ( ... )
On Thu, 2020-07-30 at 07:58 +0200, Lukas Bulwahn wrote: > Hi Joe, > > did you see this quick fix to checkpatch.pl? Can you comment on the > commit and can we get a quick ack on that fix, please? Yes, in a bit. > General question on patches for ./scripts/checkpatch.pl: > How do they travel to Linus? Generally via Andrew Morton. > Do you pick those patches and provide them to Andrew Morton? > Or do you just ack them and we need to send them to Andrew Morton to get > them into mainline? Generally, ack and forward.
[PATCH] ACPI / APEI: do memory failure on the physical address reported by ARM processor error section
After the following commit applied, user-mode SEA is preferentially
processed by APEI. Do memory failure to recover.
But there are some problems:
1) The function apei_claim_sea() has processed an CPER, does not
mean that memory failure handling has done. Because the firmware-first
RAS error is reported by both producer and consumer. Mostly SEA uses
ARM processor error section to report as a consumer. (The producer could
be DDRC and cache, and use memory error section and other error section
to report). But memory failure handling for ARM processor error section
has not been supported. We should add it.
2) Some hardware platforms can't record physical address each time. But
they could always have reported a firmware-first RAS error using ARM
processor error section. Such platform should update firmware. Don't
report the RAS error when physical address is not recorded.
Fixes: 8fcc4ae6faf8 ("arm64: acpi: Make apei_claim_sea() synchronise with
APEI's irq work")
Signed-off-by: Xiaofei Tan
---
drivers/acpi/apei/ghes.c | 42 +++---
1 file changed, 39 insertions(+), 3 deletions(-)
diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c
index 81bf71b..07bfa28 100644
--- a/drivers/acpi/apei/ghes.c
+++ b/drivers/acpi/apei/ghes.c
@@ -466,6 +466,44 @@ static bool ghes_handle_memory_failure(struct
acpi_hest_generic_data *gdata,
return false;
}
+static bool ghes_handle_arm_hw_error(struct acpi_hest_generic_data *gdata, int
sev)
+{
+ struct cper_sec_proc_arm *err = acpi_hest_get_payload(gdata);
+ struct cper_arm_err_info *err_info;
+ bool queued = false;
+ int sec_sev, i;
+
+ log_arm_hw_error(err);
+
+ if (!IS_ENABLED(CONFIG_ACPI_APEI_MEMORY_FAILURE))
+ return false;
+
+ sec_sev = ghes_severity(gdata->error_severity);
+ if (sev != GHES_SEV_RECOVERABLE || sec_sev != GHES_SEV_RECOVERABLE)
+ return false;
+
+ err_info = (struct cper_arm_err_info *) (err + 1);
+ for (i = 0; i < err->err_info_num; i++, err_info++) {
+ unsigned long pfn;
+
+ if (!(err_info->validation_bits &
CPER_ARM_INFO_VALID_PHYSICAL_ADDR))
+ continue;
+
+ pfn = PHYS_PFN(err_info->physical_fault_addr);
+ if (!pfn_valid(pfn)) {
+ pr_warn(FW_WARN GHES_PFX
+ "Invalid address in generic error data:
0x%#llx\n",
+ err_info->physical_fault_addr);
+ continue;
+ }
+
+ memory_failure_queue(pfn, 0);
+ queued = true;
+ }
+
+ return queued;
+}
+
/*
* PCIe AER errors need to be sent to the AER driver for reporting and
* recovery. The GHES severities map to the following AER severities and
@@ -543,9 +581,7 @@ static bool ghes_do_proc(struct ghes *ghes,
ghes_handle_aer(gdata);
}
else if (guid_equal(sec_type, _SEC_PROC_ARM)) {
- struct cper_sec_proc_arm *err =
acpi_hest_get_payload(gdata);
-
- log_arm_hw_error(err);
+ queued = ghes_handle_arm_hw_error(gdata, sev);
} else {
void *err = acpi_hest_get_payload(gdata);
--
2.8.1
Re: [PATCH] s390/test_unwind: fix possible memleak in test_unwind()
On Thu, 2020-07-30 at 14:36 +0800, Wang Hai wrote:
> test_unwind() misses to call kfree(bt) in an error path.
> Add the missed function call to fix it.
>
> Fixes: 0610154650f1 ("s390/test_unwind: print verbose unwinding
> results")
> Reported-by: Hulk Robot
> Signed-off-by: Wang Hai
> ---
> arch/s390/lib/test_unwind.c | 1 +
> 1 file changed, 1 insertion(+)
>
> diff --git a/arch/s390/lib/test_unwind.c
> b/arch/s390/lib/test_unwind.c
> index 32b7a30b2485..b0b12b46bc57 100644
> --- a/arch/s390/lib/test_unwind.c
> +++ b/arch/s390/lib/test_unwind.c
> @@ -63,6 +63,7 @@ static noinline int test_unwind(struct task_struct
> *task, struct pt_regs *regs,
> break;
> if (state.reliable && !addr) {
> pr_err("unwind state reliable but addr is
> 0\n");
> + kfree(bt);
> return -EINVAL;
> }
> sprint_symbol(sym, addr);
Looks good to me, thanks!
Acked-by: Ilya Leoshkevich
[PATCH v2] drivers/net/wan/lapbether: Use needed_headroom instead of hard_header_len
In net/packet/af_packet.c, the function packet_snd first reserves a headroom of length (dev->hard_header_len + dev->needed_headroom). Then if the socket is a SOCK_DGRAM socket, it calls dev_hard_header, which calls dev->header_ops->create, to create the link layer header. If the socket is a SOCK_RAW socket, it "un-reserves" a headroom of length (dev->hard_header_len), and assumes the user to provide the appropriate link layer header. So according to the logic of af_packet.c, dev->hard_header_len should be the length of the header that would be created by dev->header_ops->create. However, this driver doesn't provide dev->header_ops, so logically dev->hard_header_len should be 0. So we should use dev->needed_headroom instead of dev->hard_header_len to request necessary headroom to be allocated. Signed-off-by: Xie He --- Patch v2 has no difference from v1. I re-submitted it because I want to find new reviewers, and I want to free new reviewers from the burden of reading the lengthy discussion and explanations in the v1 email threads. Summary of v1 discussion: Cong Wang referred me to Brian Norris, who did a similar change before. Brian Norris agreed with me on "hard_header_len vs needed_headroom", but was unfamiliar with X.25 drivers. --- drivers/net/wan/lapbether.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/drivers/net/wan/lapbether.c b/drivers/net/wan/lapbether.c index b2868433718f..34cf6db89912 100644 --- a/drivers/net/wan/lapbether.c +++ b/drivers/net/wan/lapbether.c @@ -305,6 +305,7 @@ static void lapbeth_setup(struct net_device *dev) dev->netdev_ops = _netdev_ops; dev->needs_free_netdev = true; dev->type= ARPHRD_X25; + dev->hard_header_len = 0; dev->mtu = 1000; dev->addr_len= 0; } @@ -331,7 +332,8 @@ static int lapbeth_new_device(struct net_device *dev) * then this driver prepends a length field of 2 bytes, * then the underlying Ethernet device prepends its own header. */ - ndev->hard_header_len = -1 + 3 + 2 + dev->hard_header_len; + ndev->needed_headroom = -1 + 3 + 2 + dev->hard_header_len + + dev->needed_headroom; lapbeth = netdev_priv(ndev); lapbeth->axdev = ndev; -- 2.25.1
Re: [PATCH] vgacon: fix out of bounds write to the scrollback buffer
On 30. 07. 20, 8:46, Jiri Slaby wrote:
> Hi, OTOH, you should have CCed all the (public) lists.
>
> On 30. 07. 20, 4:50, 张云海 wrote:
>> Zhang Xiao points out that the check should use > instead of >=,
>> otherwise the last line will be skip.
>> I agree with that, so I modify the patch.
>> Could you please verify that it is still correct and sufficient?
>
> IMO, yes, correct -- I was thinking about this yesterday too. Just an
> example: hypothetically, if we had:
> size_row = 1
> tail = 29
> size = 30
>
> data[29] would be the last accessible member. Writing to data + tail (as
> "29 + 1 > 30" doesn't hold, so the modified check would pass), i.e.
> data[29] is still OK. So yes, > is OK, >= would waste space and would be
> actually incorrect.
>
>> BTW, Zhang Xiao also points out that the check after the memcpy can be
>> remove.
>> I also think that was right, but vgacon_scrollback_cur->tail may keep
>> the value vgacon_scrollback_cur->size in some case. That is not a
>> problem in vgacon_scrollback_update because of the check before the
>> memcpy. However, that may break some other code which assumes that
>> vgacon_scrollback_cur->tail won't be vgacon_scrollback_cur->size. I do
>> not know if there are such code, and if it is the code actually should
>> check it too. But I still not remove the check in the patch to make sure
>> it won't breaks other code.
>
> As I wrote about this yesterday:
> ===
> I am also not sure the test I was pointing out on the top of this
> message would be of any use after the change. But maybe leave the code
> rest in peace.
> ===
>
> I would let it as is in this particular code. Especially because
> vgacon_scrolldelta takes ->tail into consideration and I was too lazy to
> study the code there. But if you are willing to study the code there and
> confirm the check is superfluous, feel free to remove it. Perhaps in a
> separate patch. I was actually testing with the check removed and didn't
> hit any issue (which means, in fact, exactly nothing).
>
>> From ad143ede24ff4e61292cc9c96000100aacd97259 Mon Sep 17 00:00:00 2001
>> From: Yunhai Zhang
>> Date: Tue, 28 Jul 2020 09:58:03 +0800
>> Subject: [PATCH] Fix for missing check in vgacon scrollback handling
>>
>> vgacon_scrollback_update() always left enbough room in the scrollback
>
> "leaves enough"
>
>> buffer for the next call, but if the console size changed that room
>> might not actually be enough, and so we need to re-check.
>
> Also, could you add reasoning why you are adding the check to the loop
> and not outside (for instance, use your reasoning with numbers or CSI M
> as an example).
>
> Could you add a sample output here, something like I had:
> ===
> This leads to random crashes or KASAN reports like:
> BUG: KASAN: slab-out-of-bounds in vgacon_scroll+0x57a/0x8ed
> ===
>
> It's then easier to google for when this happens to someone who runs
> non-patched kernels.
>
>> This fixes CVE-2020-14331.
>>
>> Reported-and-debugged-by: 张云海
>> Reported-and-debugged-by: Yang Yingliang
>> Reported-by: Kyungtae Kim
>> Fixes: 15bdab959c9b ([PATCH] vgacon: Add support for soft scrollback)
>> Cc: Linus Torvalds
>> Cc: Greg KH
>> Cc: Solar Designer
>> Cc: "Srivatsa S. Bhat"
>> Cc: Anthony Liguori
>> Cc: Yang Yingliang
>> Cc: Bartlomiej Zolnierkiewicz
>
> Oh, and we should:
> Cc: sta...@vger.kernel.org
>
>> Signed-off-by: Yunhai Zhang
>> ---
>> drivers/video/console/vgacon.c | 4
>> 1 file changed, 4 insertions(+)
>>
>> diff --git a/drivers/video/console/vgacon.c b/drivers/video/console/vgacon.c
>> index 998b0de1812f..37b5711cd958 100644
>> --- a/drivers/video/console/vgacon.c
>> +++ b/drivers/video/console/vgacon.c
>> @@ -251,6 +251,10 @@ static void vgacon_scrollback_update(struct vc_data *c,
>> int t, int count)
>> p = (void *) (c->vc_origin + t * c->vc_size_row);
>>
>> while (count--) {
>> +if ((vgacon_scrollback_cur->tail + c->vc_size_row) >
And git complains here:
.git/rebase-apply/patch:13: trailing whitespace.
if ((vgacon_scrollback_cur->tail + c->vc_size_row) >
warning: 1 line adds whitespace errors.
There is a space at the EOL.
>> +vgacon_scrollback_cur->size)
>> +vgacon_scrollback_cur->tail = 0;
>> +
>> scr_memcpyw(vgacon_scrollback_cur->data +
>> vgacon_scrollback_cur->tail,
>> p, c->vc_size_row);
>
> thanks,
>
--
js
[PATCH net] wl1251: fix always return 0 error
wl1251_event_ps_report() should not always return 0 because
wl1251_ps_set_mode() may fail. Change it to return 'ret'.
Fixes: f7ad1eed4d4b ("wl1251: retry power save entry")
Reported-by: Hulk Robot
Signed-off-by: Wang Hai
---
drivers/net/wireless/ti/wl1251/event.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/wireless/ti/wl1251/event.c
b/drivers/net/wireless/ti/wl1251/event.c
index 850864dbafa1..e6d426edab56 100644
--- a/drivers/net/wireless/ti/wl1251/event.c
+++ b/drivers/net/wireless/ti/wl1251/event.c
@@ -70,7 +70,7 @@ static int wl1251_event_ps_report(struct wl1251 *wl,
break;
}
- return 0;
+ return ret;
}
static void wl1251_event_mbox_dump(struct event_mailbox *mbox)
--
2.17.1
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Thu, Jul 30, 2020 at 4:00 AM Herbert Xu wrote: > > On Wed, Jul 29, 2020 at 06:04:57PM +0300, Andy Shevchenko wrote: > > On Wed, Jul 29, 2020 at 4:35 PM Waiman Long wrote: > > > On 7/29/20 8:28 AM, Herbert Xu wrote: > > > > ... > > > > > This patch series looks good to me. I just wonder if we should also move > > > ATOMIC64_INIT() to types.h for symmetry purpose. Anyway, > > > > Same question here. > > Yes I almost started doing it but at least one architecture (arc) > had a custom atomic64_t so I kept it out just to be on the safe > side. We may ask Synopsys folks to look at this as well. Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? > We certainly could do this as a follow-up patch. -- With Best Regards, Andy Shevchenko
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote: > > We may ask Synopsys folks to look at this as well. > Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? I don't think there is any technical difficulty. The custom atomic64_t simply adds an alignment requirement so the initialisor remains the same. I just didn't want to add more complexity to the existing patch. As a follow-up patch it should be quite straightforward. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
linux-next: build failure after merge of the vhost tree
Hi all,
After merging the vhost tree, today's linux-next build (x86_64
allmodconfig) failed like this:
drivers/virtio/virtio_vdpa.c: In function 'virtio_vdpa_get':
drivers/virtio/virtio_vdpa.c:60:32: warning: unused variable 'ops'
[-Wunused-variable]
60 | const struct vdpa_config_ops *ops = vdpa->config;
|^~~
drivers/virtio/virtio_vdpa.c: In function 'virtio_vdpa_finalize_features':
drivers/virtio/virtio_vdpa.c:301:9: error: void value not ignored as it ought
to be
301 | return vdpa_set_features(vdpa, vdev->features);
| ^~~
drivers/virtio/virtio_vdpa.c:296:32: warning: unused variable 'ops'
[-Wunused-variable]
296 | const struct vdpa_config_ops *ops = vdpa->config;
|^~~
drivers/virtio/virtio_vdpa.c:302:1: warning: control reaches end of non-void
function [-Wreturn-type]
302 | }
| ^
drivers/vhost/vdpa.c: In function 'vhost_vdpa_get_config':
drivers/vhost/vdpa.c:198:32: warning: unused variable 'ops' [-Wunused-variable]
198 | const struct vdpa_config_ops *ops = vdpa->config;
|^~~
drivers/vhost/vdpa.c: In function 'vhost_vdpa_set_features':
drivers/vhost/vdpa.c:284:6: error: void value not ignored as it ought to be
284 | if (vdpa_set_features(vdpa, features))
| ^
Caused by commit
d4ff0377f05b ("virtio_vdpa: legacy features handling")
I added the following (hack) patch for today.
From: Stephen Rothwell
Date: Thu, 30 Jul 2020 17:39:11 +1000
Subject: [PATCH] virtio_vdpa: fix legacy features handling
Signed-off-by: Stephen Rothwell
---
drivers/vhost/vdpa.c | 4 +---
drivers/virtio/virtio_vdpa.c | 5 ++---
2 files changed, 3 insertions(+), 6 deletions(-)
diff --git a/drivers/vhost/vdpa.c b/drivers/vhost/vdpa.c
index be9b3ed01a93..f7ef5f3ef0e3 100644
--- a/drivers/vhost/vdpa.c
+++ b/drivers/vhost/vdpa.c
@@ -195,7 +195,6 @@ static long vhost_vdpa_get_config(struct vhost_vdpa *v,
struct vhost_vdpa_config __user *c)
{
struct vdpa_device *vdpa = v->vdpa;
- const struct vdpa_config_ops *ops = vdpa->config;
struct vhost_vdpa_config config;
unsigned long size = offsetof(struct vhost_vdpa_config, buf);
u8 *buf;
@@ -281,8 +280,7 @@ static long vhost_vdpa_set_features(struct vhost_vdpa *v,
u64 __user *featurep)
if (features & ~vhost_vdpa_features[v->virtio_id])
return -EINVAL;
- if (vdpa_set_features(vdpa, features))
- return -EINVAL;
+ vdpa_set_features(vdpa, features);
return 0;
}
diff --git a/drivers/virtio/virtio_vdpa.c b/drivers/virtio/virtio_vdpa.c
index d0ae58d35392..73c5d49ff0df 100644
--- a/drivers/virtio/virtio_vdpa.c
+++ b/drivers/virtio/virtio_vdpa.c
@@ -57,7 +57,6 @@ static void virtio_vdpa_get(struct virtio_device *vdev,
unsigned offset,
void *buf, unsigned len)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
- const struct vdpa_config_ops *ops = vdpa->config;
vdpa_get_config(vdpa, offset, buf, len);
}
@@ -293,12 +292,12 @@ static u64 virtio_vdpa_get_features(struct virtio_device
*vdev)
static int virtio_vdpa_finalize_features(struct virtio_device *vdev)
{
struct vdpa_device *vdpa = vd_get_vdpa(vdev);
- const struct vdpa_config_ops *ops = vdpa->config;
/* Give virtio_ring a chance to accept features. */
vring_transport_features(vdev);
- return vdpa_set_features(vdpa, vdev->features);
+ vdpa_set_features(vdpa, vdev->features);
+ return 0;
}
static const char *virtio_vdpa_bus_name(struct virtio_device *vdev)
--
2.27.0
--
Cheers,
Stephen Rothwell
pgpMVVi46WfVx.pgp
Description: OpenPGP digital signature
Re: [PATCH 0/2] locking/qspinlock: Break qspinlock_types.h header loop
On Thu, Jul 30, 2020 at 10:51 AM Herbert Xu wrote: > On Thu, Jul 30, 2020 at 10:47:16AM +0300, Andy Shevchenko wrote: > > > > We may ask Synopsys folks to look at this as well. > > Vineet, any ideas if we may unify ATOMIC64_INIT() across the architectures? > > I don't think there is any technical difficulty. The custom > atomic64_t simply adds an alignment requirement so the initialisor > remains the same. > > I just didn't want to add more complexity to the existing patch. > As a follow-up patch it should be quite straightforward. Ah, I see what you mean. I considered the follow up patch as well, I thought there were some bigger impediments. -- With Best Regards, Andy Shevchenko
Re: [PATCH 1/1] x86/tsr: Fix tsc frequency enumeration failure on lightning mountain SoC
On Thu, Jul 30, 2020 at 9:16 AM Dilip Kota wrote: > > Frequency descriptor of Lightning Mountain SoC doesn't have all the > frequency entries so resulting in the below failure causing kernel hang. > > [0.00] Error MSR_FSB_FREQ index 15 is unknown > [0.00] tsc: Fast TSC calibration failed > > So, add all the frequency entries in the Lightning Mountain SoC frequency > descriptor. While at this, can you confirm (with maybe good description and documentation reference) that the numbers in that array are all correct? -- With Best Regards, Andy Shevchenko
Re: [PATCH v2] scsi: ufs: Fix possible infinite loop in ufshcd_hold
Hi Can, On Wed, 2020-07-29 at 18:53 +0800, Can Guo wrote: > Hi Stanley, > > On 2020-07-29 18:26, Stanley Chu wrote: > > Hi Can, > > > > On Wed, 2020-07-29 at 16:43 +0800, Can Guo wrote: > >> Hi Stanley, > >> > >> On 2020-07-29 10:40, Stanley Chu wrote: > >> > In ufshcd_suspend(), after clk-gating is suspended and link is set > >> > as Hibern8 state, ufshcd_hold() is still possibly invoked before > >> > ufshcd_suspend() returns. For example, MediaTek's suspend vops may > >> > issue UIC commands which would call ufshcd_hold() during the command > >> > issuing flow. > >> > > >> > Now if UFSHCD_CAP_HIBERN8_WITH_CLK_GATING capability is enabled, > >> > then ufshcd_hold() may enter infinite loops because there is no > >> > clk-ungating work scheduled or pending. In this case, ufshcd_hold() > >> > shall just bypass, and keep the link as Hibern8 state. > >> > > >> > >> The infinite loop is expected as ufshcd_hold is called again after > >> link is put to hibern8 state, so in QCOM's code, we never do this. > > > > Sadly MediaTek have to do this to make our UniPro to enter low-power > > mode. > > > >> The cap UFSHCD_CAP_HIBERN8_WITH_CLK_GATING means UIC link state > >> must not be HIBERN8 after ufshcd_hold(async=false) returns. > > > > If driver is not in PM scenarios, e.g., suspended, above statement > > shall > > be always followed. But two obvious violations are existed, > > > > 1. In ufshcd_suspend(), link is set as HIBERN8 behind ufshcd_hold() > > 2. In ufshcd_resume(), link is set back as Active before > > ufshcd_release() is invoked > > > > So as my understanding, special conditions are allowed in PM scenarios, > > and this is why "hba->clk_gating.is_suspended" is introduced. By this > > thought, I used "hba->clk_gating.is_suspended" in this patch as the > > mandatory condition to allow ufshcd_hold() usage in vendor suspend and > > resume callbacks. > > > > > >> Instead of bailing out from that loop, which makes the logic of > >> ufshcd_hold and clk gating even more complex, how about removing > >> ufshcd_hold/release from ufshcd_send_uic_cmd()? I think they are > >> redundant and we should never send DME cmds if clocks/powers are > >> not ready. I mean callers should make sure they are ready to send > >> DME cmds (and only callers know when), but not leave that job to > >> ufshcd_send_uic_cmd(). It is convenient to remove ufshcd_hold/ > >> release from ufshcd_send_uic_cmd() as there are not many places > >> sending DME cmds without holding the clocks, ufs_bsg.c is one. > >> And I have tested my idea on my setup, it worked well for me. > >> Another benefit is that it also allows us to use DME cmds > >> in clk gating/ungating contexts if we need to in the future. > >> > > > > Brilliant idea! But this may not solve problems if vendor callbacks > > need > > more than UIC commands in the future. > > > > This simple patch could make all vendor operations on UFSHCI in PM > > callbacks possible with UFSHCD_CAP_HIBERN8_WITH_CLK_GATING enabled, and > > again, it allows those operations in PM scenarios only. > > > > Other than UIC cmds, I can only think of device manangement cmds (like > query). > If device management cmds come into the way in the future, we fix it as > well. > I mean that is the right thing to do in my opinion - just like we don't > call > pm_runtime_get_sync() in ufshcd_send_uic_cmd(). > > I can understand that you want a simple/quick fix to get it work for you > once > for all, but from my point of view, debugging clk gating/ungating really > takes > huge efforts sometime (I've spent a lot of time on it). Some flash > vendors also > use it in their own driver widely which makes some failure scenes even > harder to > undertand/debug. So the first thing comes to my head is that we should > avoid > making it more complex or giving it more exceptions. > > From functionality point of view, it looks ok to me. It is just that I > cannot > predict it won't cause new problems since the clk gating/ungating > sequeces are > like magic in some use cases sometime. Thanks for the functionality review. I totally understand what you mentioned above about the clk-gating debugging because we also spent lots of time for issue analysis. I just finished some fault injection for this patch in our platform, the results are fine. The active window of this patch is limited: Starting from ufshcd_link_state_transition() in ufshcd_suspend to ufshcd_vops_resume() in ufshcd_resume() because the link is back to LINKUP state in MediaTek resume callback. So I was focus on injecting errors in our callbacks between this period and most of injected fails triggered host and device reset flow. For example, Suspend: UniPro PowerDownControl timeout Resume: hba_enable timeout Resume: UniPro PowerDownControl timeout Resume: HIBERN8 Exit timeout Hope these tests can ease your concerns. Thanks, Stanley Chu > > Thanks, > > Can Guo. > > >> Please let me know your idea, thanks. > >> > >> Can Guo. > >
[RFC PATCH 3/3] opp: Power on (virtual) power domains managed by the OPP core
dev_pm_opp_attach_genpd() allows attaching an arbitrary number of
power domains to an OPP table. In that case, the genpd core will
create a virtual device for each of the power domains.
At the moment, the OPP core only calls
dev_pm_genpd_set_performance_state() on these virtual devices.
It does not attempt to power on the power domains. Therefore
the required power domain might never get turned on.
So far, dev_pm_opp_attach_genpd() is only used in qcom-cpufreq-nvmem.c
to attach the CPR power domain to the CPU OPP table. The CPR driver
does not check if it was actually powered on so this did not cause
any problems. However, other drivers (e.g. rpmpd) might ignore the
performance state until the power domain is actually powered on.
Since these virtual devices are managed exclusively by the OPP core,
I would say that it should also be responsible to ensure they are
enabled. A similar approach is already used for regulators, see
commit 8d45719caaf5 ("opp: core: add regulators enable and disable").
This commit implements similar functionality for the virtual genpd
devices managed by the OPP core. The power domains are turned on
the first time dev_pm_opp_set_rate() is called. They are turned off
again when dev_pm_opp_set_rate(dev, 0) is called.
Signed-off-by: Stephan Gerhold
---
Related discussion:
https://lore.kernel.org/linux-arm-msm/20200426123140.ga190...@gerhold.net/
There would be also other ways to implement this, e.g. device links,
assuming that the device using the OPP table also makes use of runtime PM.
My first thought was that it would be most consistent to handle this like
regulators, bandwidth votes etc. RFC :)
---
drivers/opp/core.c | 55 ++
drivers/opp/opp.h | 1 +
2 files changed, 56 insertions(+)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index f93f551c911e..66ecffe12f01 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include "opp.h"
@@ -796,6 +797,26 @@ static int _set_required_opp(struct device *dev, struct
device *pd_dev,
return ret;
}
+static int _enable_required_opp(struct device *dev, struct device *pd_dev,
+ bool on)
+{
+ int ret;
+
+ if (on) {
+ ret = pm_runtime_get_sync(pd_dev);
+ if (ret < 0) {
+ pm_runtime_put_noidle(pd_dev);
+ dev_err(dev, "Failed to enable %s: %d\n",
+ dev_name(pd_dev), ret);
+ return ret;
+ }
+ } else {
+ pm_runtime_put(pd_dev);
+ }
+
+ return 0;
+}
+
/* This is only called for PM domain for now */
static int _set_required_opps(struct device *dev,
struct opp_table *opp_table,
@@ -803,6 +824,8 @@ static int _set_required_opps(struct device *dev,
{
struct opp_table **required_opp_tables = opp_table->required_opp_tables;
struct device **genpd_virt_devs = opp_table->genpd_virt_devs;
+ bool power_on = opp != NULL;
+ bool already_enabled = power_on == opp_table->genpd_virt_enabled;
struct device *pd_dev;
int i, ret = 0;
@@ -829,6 +852,20 @@ static int _set_required_opps(struct device *dev,
ret = _set_required_opp(dev, pd_dev, opp, i);
if (ret)
break;
+
+ if (likely(already_enabled))
+ continue;
+
+ ret = _enable_required_opp(dev, pd_dev, power_on);
+ if (ret)
+ break;
+ }
+
+ if (ret && !already_enabled) {
+ /* Rollback (skip current since it failed) */
+ for (i--; i >= 0; i--)
+ _enable_required_opp(dev, genpd_virt_devs[i],
+!power_on);
}
} else {
/* Scaling down? Set required OPPs in reverse order */
@@ -838,8 +875,26 @@ static int _set_required_opps(struct device *dev,
ret = _set_required_opp(dev, pd_dev, opp, i);
if (ret)
break;
+
+ if (likely(already_enabled))
+ continue;
+
+ ret = _enable_required_opp(dev, pd_dev, power_on);
+ if (ret)
+ break;
+ }
+
+ if (ret && !already_enabled) {
+ /* Rollback (skip current since it failed) */
+ for (i++; i < opp_table->required_opp_count; i++)
+ _enable_required_opp(dev, genpd_virt_devs[i],
+!power_on);
}
}
+
+
[RFC PATCH 1/3] opp: Reduce code duplication in _set_required_opps()
Move call to dev_pm_genpd_set_performance_state() to a separate
function so we can avoid duplicating the code for the single and
multiple genpd case.
Signed-off-by: Stephan Gerhold
---
drivers/opp/core.c | 40 +---
1 file changed, 21 insertions(+), 19 deletions(-)
diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 9d7fb45b1786..f7a476b55069 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -781,6 +781,21 @@ static int _set_opp_custom(const struct opp_table
*opp_table,
return opp_table->set_opp(data);
}
+static int _set_required_opp(struct device *dev, struct device *pd_dev,
+struct dev_pm_opp *opp, int i)
+{
+ unsigned int pstate = likely(opp) ? opp->required_opps[i]->pstate : 0;
+ int ret;
+
+ ret = dev_pm_genpd_set_performance_state(pd_dev, pstate);
+ if (ret) {
+ dev_err(dev, "Failed to set performance rate of %s: %d (%d)\n",
+ dev_name(pd_dev), pstate, ret);
+ }
+
+ return ret;
+}
+
/* This is only called for PM domain for now */
static int _set_required_opps(struct device *dev,
struct opp_table *opp_table,
@@ -788,22 +803,15 @@ static int _set_required_opps(struct device *dev,
{
struct opp_table **required_opp_tables = opp_table->required_opp_tables;
struct device **genpd_virt_devs = opp_table->genpd_virt_devs;
- unsigned int pstate;
+ struct device *pd_dev;
int i, ret = 0;
if (!required_opp_tables)
return 0;
/* Single genpd case */
- if (!genpd_virt_devs) {
- pstate = likely(opp) ? opp->required_opps[0]->pstate : 0;
- ret = dev_pm_genpd_set_performance_state(dev, pstate);
- if (ret) {
- dev_err(dev, "Failed to set performance state of %s: %d
(%d)\n",
- dev_name(dev), pstate, ret);
- }
- return ret;
- }
+ if (!genpd_virt_devs)
+ return _set_required_opp(dev, dev, opp, 0);
/* Multiple genpd case */
@@ -814,17 +822,11 @@ static int _set_required_opps(struct device *dev,
mutex_lock(_table->genpd_virt_dev_lock);
for (i = 0; i < opp_table->required_opp_count; i++) {
- pstate = likely(opp) ? opp->required_opps[i]->pstate : 0;
-
- if (!genpd_virt_devs[i])
- continue;
+ pd_dev = genpd_virt_devs[i];
- ret = dev_pm_genpd_set_performance_state(genpd_virt_devs[i],
pstate);
- if (ret) {
- dev_err(dev, "Failed to set performance rate of %s: %d
(%d)\n",
- dev_name(genpd_virt_devs[i]), pstate, ret);
+ ret = _set_required_opp(dev, pd_dev, opp, i);
+ if (ret)
break;
- }
}
mutex_unlock(_table->genpd_virt_dev_lock);
--
2.27.0
RE: [PATCH 2/2] ASoC: Intel: Add period size constraint on strago board
> > Is this patch required if you've already constrained the period sizes for the > platform driver in patch1? Yes or alsa will select 320 as default period size for it. Regards, Brent
Re: [PATCH v2] drivers/net/wan/lapbether: Use needed_headroom instead of hard_header_len
Hi Martin, I'm currently working on a plan to make all X.25 drivers (lapbether.c, x25_asy.c, hdlc_x25.c) to set dev->hard_header_len / dev->needed_headroom correctly. So that upper layers no longer need to guess how much headroom a X.25 device needs with a constant value (as they currently do). After studying af_packet.c, I found that X.25 drivers needed to set needed_headroom to reserve the headroom instead of using hard_header_len. Because hard_header_len should be the length of the header that would be created by dev_hard_header, and in this case it should be 0, according to the logic of af_packet.c. So my first step is to fix the settings in lapbether.c. Could you review this patch and extend your support via a "Reviewed-by" tag? If this can be fixed, I'll go on and fix other X.25 drivers. Thanks! It's very hard to find reviewers for X.25 code because it is relatively unmaintained by people. I hope I can do some of the maintenance work. I greatly appreciate your support!
Re: [PATCH] selinux: add tracepoint on denials
On 7/28/20 6:02 PM, Thiébaud Weksteen wrote: > On Tue, Jul 28, 2020 at 5:12 PM Paul Moore wrote: >> Perhaps it would be helpful if you provided an example of how one >> would be expected to use this new tracepoint? That would help put >> things in the proper perspective. > The best example is the one I provided in the commit message, that is > using perf (or a perf equivalent), to hook onto that tracepoint. > >> Well, to be honest, the very nature of this tracepoint is duplicating >> the AVC audit record with a focus on using perf to establish a full >> backtrace at the expense of reduced information. At least that is how >> it appears to me. > I see both methods as complementary. By default, the kernel itself can > do some reporting (i.e avc message) on which process triggered the > denial, what was the context, etc. This is useful even in production > and doesn't require any extra tooling. > The case for adding this tracepoint can be seen as advanced debugging. > That is, once an avc denial has been confirmed, a developer can use > this tracepoint to surface the userland stacktrace. It requires more > userland tools and symbols on the userland binaries. I think from development view you would like to have a better way to trap this events in userspace. One idea that I have is is to have more outcomes from a rule. We have today allow, dontaudit, auditallow i think it would be good to have signal sent too. "signal-xxx-allow" for some set of signals. SIGBUS, SIGSEGV, SIGABRT maybe. That will be a good way to pickup the problem with a debugger or generate a a core file. I have also done some selinux trace functions. I think they collide with this set, but I think I can rebase them upon yours and see if they give some more functionality. I see this functionality very much needed in some form.
[PATCH 5.7 12/20] tcp: allow at most one TLP probe per flight
From: Yuchung Cheng
[ Upstream commit 76be93fc0702322179bb0ea87295d820ee46ad14 ]
Previously TLP may send multiple probes of new data in one
flight. This happens when the sender is cwnd limited. After the
initial TLP containing new data is sent, the sender receives another
ACK that acks partial inflight. It may re-arm another TLP timer
to send more, if no further ACK returns before the next TLP timeout
(PTO) expires. The sender may send in theory a large amount of TLP
until send queue is depleted. This only happens if the sender sees
such irregular uncommon ACK pattern. But it is generally undesirable
behavior during congestion especially.
The original TLP design restrict only one TLP probe per inflight as
published in "Reducing Web Latency: the Virtue of Gentle Aggression",
SIGCOMM 2013. This patch changes TLP to send at most one probe
per inflight.
Note that if the sender is app-limited, TLP retransmits old data
and did not have this issue.
Signed-off-by: Yuchung Cheng
Signed-off-by: Neal Cardwell
Signed-off-by: Eric Dumazet
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
include/linux/tcp.h |4 +++-
net/ipv4/tcp_input.c | 11 ++-
net/ipv4/tcp_output.c | 13 -
3 files changed, 17 insertions(+), 11 deletions(-)
--- a/include/linux/tcp.h
+++ b/include/linux/tcp.h
@@ -217,6 +217,8 @@ struct tcp_sock {
} rack;
u16 advmss; /* Advertised MSS */
u8 compressed_ack;
+ u8 tlp_retrans:1, /* TLP is a retransmission */
+ unused:7;
u32 chrono_start; /* Start time in jiffies of a TCP chrono */
u32 chrono_stat[3]; /* Time in jiffies for chrono_stat stats */
u8 chrono_type:2, /* current chronograph type */
@@ -239,7 +241,7 @@ struct tcp_sock {
save_syn:1, /* Save headers of SYN packet */
is_cwnd_limited:1,/* forward progress limited by snd_cwnd? */
syn_smc:1; /* SYN includes SMC */
- u32 tlp_high_seq; /* snd_nxt at the time of TLP retransmit. */
+ u32 tlp_high_seq; /* snd_nxt at the time of TLP */
u32 tcp_tx_delay; /* delay (in usec) added to TX packets */
u64 tcp_wstamp_ns; /* departure time for next sent data packet */
--- a/net/ipv4/tcp_input.c
+++ b/net/ipv4/tcp_input.c
@@ -3506,10 +3506,8 @@ static void tcp_replace_ts_recent(struct
}
}
-/* This routine deals with acks during a TLP episode.
- * We mark the end of a TLP episode on receiving TLP dupack or when
- * ack is after tlp_high_seq.
- * Ref: loss detection algorithm in draft-dukkipati-tcpm-tcp-loss-probe.
+/* This routine deals with acks during a TLP episode and ends an episode by
+ * resetting tlp_high_seq. Ref: TLP algorithm in draft-ietf-tcpm-rack
*/
static void tcp_process_tlp_ack(struct sock *sk, u32 ack, int flag)
{
@@ -3518,7 +3516,10 @@ static void tcp_process_tlp_ack(struct s
if (before(ack, tp->tlp_high_seq))
return;
- if (flag & FLAG_DSACKING_ACK) {
+ if (!tp->tlp_retrans) {
+ /* TLP of new data has been acknowledged */
+ tp->tlp_high_seq = 0;
+ } else if (flag & FLAG_DSACKING_ACK) {
/* This DSACK means original and TLP probe arrived; no loss */
tp->tlp_high_seq = 0;
} else if (after(ack, tp->tlp_high_seq)) {
--- a/net/ipv4/tcp_output.c
+++ b/net/ipv4/tcp_output.c
@@ -2625,6 +2625,11 @@ void tcp_send_loss_probe(struct sock *sk
int pcount;
int mss = tcp_current_mss(sk);
+ /* At most one outstanding TLP */
+ if (tp->tlp_high_seq)
+ goto rearm_timer;
+
+ tp->tlp_retrans = 0;
skb = tcp_send_head(sk);
if (skb && tcp_snd_wnd_test(tp, skb, mss)) {
pcount = tp->packets_out;
@@ -2642,10 +2647,6 @@ void tcp_send_loss_probe(struct sock *sk
return;
}
- /* At most one outstanding TLP retransmission. */
- if (tp->tlp_high_seq)
- goto rearm_timer;
-
if (skb_still_in_host_queue(sk, skb))
goto rearm_timer;
@@ -2667,10 +2668,12 @@ void tcp_send_loss_probe(struct sock *sk
if (__tcp_retransmit_skb(sk, skb, 1))
goto rearm_timer;
+ tp->tlp_retrans = 1;
+
+probe_sent:
/* Record snd_nxt for loss detection. */
tp->tlp_high_seq = tp->snd_nxt;
-probe_sent:
NET_INC_STATS(sock_net(sk), LINUX_MIB_TCPLOSSPROBES);
/* Reset s.t. tcp_rearm_rto will restart timer from now */
inet_csk(sk)->icsk_pending = 0;
[PATCH 5.7 11/20] rxrpc: Fix sendmsg() returning EPIPE due to recvmsg() returning ENODATA
From: David Howells
[ Upstream commit 639f181f0ee20d3249dbc55f740f0167267180f0 ]
rxrpc_sendmsg() returns EPIPE if there's an outstanding error, such as if
rxrpc_recvmsg() indicating ENODATA if there's nothing for it to read.
Change rxrpc_recvmsg() to return EAGAIN instead if there's nothing to read
as this particular error doesn't get stored in ->sk_err by the networking
core.
Also change rxrpc_sendmsg() so that it doesn't fail with delayed receive
errors (there's no way for it to report which call, if any, the error was
caused by).
Fixes: 17926a79320a ("[AF_RXRPC]: Provide secure RxRPC sockets for use by
userspace and kernel both")
Signed-off-by: David Howells
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
net/rxrpc/recvmsg.c |2 +-
net/rxrpc/sendmsg.c |2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
--- a/net/rxrpc/recvmsg.c
+++ b/net/rxrpc/recvmsg.c
@@ -464,7 +464,7 @@ try_again:
list_empty(>recvmsg_q) &&
rx->sk.sk_state != RXRPC_SERVER_LISTENING) {
release_sock(>sk);
- return -ENODATA;
+ return -EAGAIN;
}
if (list_empty(>recvmsg_q)) {
--- a/net/rxrpc/sendmsg.c
+++ b/net/rxrpc/sendmsg.c
@@ -306,7 +306,7 @@ static int rxrpc_send_data(struct rxrpc_
/* this should be in poll */
sk_clear_bit(SOCKWQ_ASYNC_NOSPACE, sk);
- if (sk->sk_err || (sk->sk_shutdown & SEND_SHUTDOWN))
+ if (sk->sk_shutdown & SEND_SHUTDOWN)
return -EPIPE;
more = msg->msg_flags & MSG_MORE;
[PATCH 5.7 14/20] sctp: shrink stream outq only when new outcnt < old outcnt
From: Xin Long
[ Upstream commit 8f13399db22f909a35735bf8ae2f932e0c8f0e30 ]
It's not necessary to go list_for_each for outq->out_chunk_list
when new outcnt >= old outcnt, as no chunk with higher sid than
new (outcnt - 1) exists in the outqueue.
While at it, also move the list_for_each code in a new function
sctp_stream_shrink_out(), which will be used in the next patch.
Signed-off-by: Xin Long
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
net/sctp/stream.c | 21 ++---
1 file changed, 14 insertions(+), 7 deletions(-)
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -22,17 +22,11 @@
#include
#include
-/* Migrates chunks from stream queues to new stream queues if needed,
- * but not across associations. Also, removes those chunks to streams
- * higher than the new max.
- */
-static void sctp_stream_outq_migrate(struct sctp_stream *stream,
-struct sctp_stream *new, __u16 outcnt)
+static void sctp_stream_shrink_out(struct sctp_stream *stream, __u16 outcnt)
{
struct sctp_association *asoc;
struct sctp_chunk *ch, *temp;
struct sctp_outq *outq;
- int i;
asoc = container_of(stream, struct sctp_association, stream);
outq = >outqueue;
@@ -56,6 +50,19 @@ static void sctp_stream_outq_migrate(str
sctp_chunk_free(ch);
}
+}
+
+/* Migrates chunks from stream queues to new stream queues if needed,
+ * but not across associations. Also, removes those chunks to streams
+ * higher than the new max.
+ */
+static void sctp_stream_outq_migrate(struct sctp_stream *stream,
+struct sctp_stream *new, __u16 outcnt)
+{
+ int i;
+
+ if (stream->outcnt > outcnt)
+ sctp_stream_shrink_out(stream, outcnt);
if (new) {
/* Here we actually move the old ext stuff into the new
[PATCH 5.7 10/20] rtnetlink: Fix memory(net_device) leak when ->newlink fails
From: Weilong Chen [ Upstream commit cebb69754f37d68e1355a5e726fdac317bcda302 ] When vlan_newlink call register_vlan_dev fails, it might return error with dev->reg_state = NETREG_UNREGISTERED. The rtnl_newlink should free the memory. But currently rtnl_newlink only free the memory which state is NETREG_UNINITIALIZED. BUG: memory leak unreferenced object 0x8881051de000 (size 4096): comm "syz-executor139", pid 560, jiffies 4294745346 (age 32.445s) hex dump (first 32 bytes): 76 6c 61 6e 32 00 00 00 00 00 00 00 00 00 00 00 vlan2... 00 45 28 03 81 88 ff ff 00 00 00 00 00 00 00 00 .E(. backtrace: [<47527e31>] kmalloc_node include/linux/slab.h:578 [inline] [<47527e31>] kvmalloc_node+0x33/0xd0 mm/util.c:574 [<2b59e3bc>] kvmalloc include/linux/mm.h:753 [inline] [<2b59e3bc>] kvzalloc include/linux/mm.h:761 [inline] [<2b59e3bc>] alloc_netdev_mqs+0x83/0xd90 net/core/dev.c:9929 [<6076752a>] rtnl_create_link+0x2c0/0xa20 net/core/rtnetlink.c:3067 [<572b3be5>] __rtnl_newlink+0xc9c/0x1330 net/core/rtnetlink.c:3329 [] rtnl_newlink+0x66/0x90 net/core/rtnetlink.c:3397 [<52c7c0a9>] rtnetlink_rcv_msg+0x540/0x990 net/core/rtnetlink.c:5460 [<4b5cb379>] netlink_rcv_skb+0x12b/0x3a0 net/netlink/af_netlink.c:2469 [ ] netlink_unicast_kernel net/netlink/af_netlink.c:1303 [inline] [ ] netlink_unicast+0x4c6/0x690 net/netlink/af_netlink.c:1329 [ ] netlink_sendmsg+0x735/0xcc0 net/netlink/af_netlink.c:1918 [<9221ebf7>] sock_sendmsg_nosec net/socket.c:652 [inline] [<9221ebf7>] sock_sendmsg+0x109/0x140 net/socket.c:672 [<1c30ffe4>] sys_sendmsg+0x5f5/0x780 net/socket.c:2352 [ ] ___sys_sendmsg+0x11d/0x1a0 net/socket.c:2406 [<07297384>] __sys_sendmsg+0xeb/0x1b0 net/socket.c:2439 [<0eb29b11>] do_syscall_64+0x56/0xa0 arch/x86/entry/common.c:359 [<6839b4d0>] entry_SYSCALL_64_after_hwframe+0x44/0xa9 Fixes: cb626bf566eb ("net-sysfs: Fix reference count leak") Reported-by: Hulk Robot Signed-off-by: Weilong Chen Signed-off-by: David S. Miller Signed-off-by: Greg Kroah-Hartman --- net/core/rtnetlink.c |3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) --- a/net/core/rtnetlink.c +++ b/net/core/rtnetlink.c @@ -3337,7 +3337,8 @@ replay: */ if (err < 0) { /* If device is not registered at all, free it now */ - if (dev->reg_state == NETREG_UNINITIALIZED) + if (dev->reg_state == NETREG_UNINITIALIZED || + dev->reg_state == NETREG_UNREGISTERED) free_netdev(dev); goto out; }
[PATCH 5.7 13/20] AX.25: Prevent integer overflows in connect and sendmsg
From: Dan Carpenter
[ Upstream commit 17ad73e941b71f3bec7523ea4e9cbc3752461c2d ]
We recently added some bounds checking in ax25_connect() and
ax25_sendmsg() and we so we removed the AX25_MAX_DIGIS checks because
they were no longer required.
Unfortunately, I believe they are required to prevent integer overflows
so I have added them back.
Fixes: 8885bb0621f0 ("AX.25: Prevent out-of-bounds read in ax25_sendmsg()")
Fixes: 2f2a7ffad5c6 ("AX.25: Fix out-of-bounds read in ax25_connect()")
Signed-off-by: Dan Carpenter
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
net/ax25/af_ax25.c |5 -
1 file changed, 4 insertions(+), 1 deletion(-)
--- a/net/ax25/af_ax25.c
+++ b/net/ax25/af_ax25.c
@@ -1188,6 +1188,7 @@ static int __must_check ax25_connect(str
fsa->fsa_ax25.sax25_ndigis != 0) {
/* Valid number of digipeaters ? */
if (fsa->fsa_ax25.sax25_ndigis < 1 ||
+ fsa->fsa_ax25.sax25_ndigis > AX25_MAX_DIGIS ||
addr_len < sizeof(struct sockaddr_ax25) +
sizeof(ax25_address) * fsa->fsa_ax25.sax25_ndigis) {
err = -EINVAL;
@@ -1509,7 +1510,9 @@ static int ax25_sendmsg(struct socket *s
struct full_sockaddr_ax25 *fsa = (struct
full_sockaddr_ax25 *)usax;
/* Valid number of digipeaters ? */
- if (usax->sax25_ndigis < 1 || addr_len < sizeof(struct
sockaddr_ax25) +
+ if (usax->sax25_ndigis < 1 ||
+ usax->sax25_ndigis > AX25_MAX_DIGIS ||
+ addr_len < sizeof(struct sockaddr_ax25) +
sizeof(ax25_address) * usax->sax25_ndigis) {
err = -EINVAL;
goto out;
[PATCH 5.7 15/20] sctp: shrink stream outq when fails to do addstream reconf
From: Xin Long
[ Upstream commit 3ecdda3e9ad837cf9cb41b6faa11b1af3a5abc0c ]
When adding a stream with stream reconf, the new stream firstly is in
CLOSED state but new out chunks can still be enqueued. Then once gets
the confirmation from the peer, the state will change to OPEN.
However, if the peer denies, it needs to roll back the stream. But when
doing that, it only sets the stream outcnt back, and the chunks already
in the new stream don't get purged. It caused these chunks can still be
dequeued in sctp_outq_dequeue_data().
As its stream is still in CLOSE, the chunk will be enqueued to the head
again by sctp_outq_head_data(). This chunk will never be sent out, and
the chunks after it can never be dequeued. The assoc will be 'hung' in
a dead loop of sending this chunk.
To fix it, this patch is to purge these chunks already in the new
stream by calling sctp_stream_shrink_out() when failing to do the
addstream reconf.
Fixes: 11ae76e67a17 ("sctp: implement receiver-side procedures for the Reconf
Response Parameter")
Reported-by: Ying Xu
Signed-off-by: Xin Long
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
net/sctp/stream.c |6 --
1 file changed, 4 insertions(+), 2 deletions(-)
--- a/net/sctp/stream.c
+++ b/net/sctp/stream.c
@@ -1044,11 +1044,13 @@ struct sctp_chunk *sctp_process_strreset
nums = ntohs(addstrm->number_of_streams);
number = stream->outcnt - nums;
- if (result == SCTP_STRRESET_PERFORMED)
+ if (result == SCTP_STRRESET_PERFORMED) {
for (i = number; i < stream->outcnt; i++)
SCTP_SO(stream, i)->state = SCTP_STREAM_OPEN;
- else
+ } else {
+ sctp_stream_shrink_out(stream, number);
stream->outcnt = number;
+ }
*evp = sctp_ulpevent_make_stream_change_event(asoc, flags,
0, nums, GFP_ATOMIC);
[PATCH 5.7 03/20] dev: Defer free of skbs in flush_backlog
From: Subash Abhinov Kasiviswanathan
[ Upstream commit 7df5cb75cfb8acf96c7f2342530eb41e0c11f4c3 ]
IRQs are disabled when freeing skbs in input queue.
Use the IRQ safe variant to free skbs here.
Fixes: 145dd5f9c88f ("net: flush the softnet backlog in process context")
Signed-off-by: Subash Abhinov Kasiviswanathan
Signed-off-by: David S. Miller
Signed-off-by: Greg Kroah-Hartman
---
net/core/dev.c |2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -5504,7 +5504,7 @@ static void flush_backlog(struct work_st
skb_queue_walk_safe(>input_pkt_queue, skb, tmp) {
if (skb->dev->reg_state == NETREG_UNREGISTERING) {
__skb_unlink(skb, >input_pkt_queue);
- kfree_skb(skb);
+ dev_kfree_skb_irq(skb);
input_queue_head_incr(sd);
}
}
