[PATCH v7 2/4] Input: goodix - Add regulators suppot
Goodix CTP controllers require AVDD28, VDDIO regulators for power-on
sequence.
The delay between these regualtor operations as per Power-on Timing
from datasheet[1] is 0 (T1 >= 0 usec).
So, enable and disable these regulators in proper order using normal
regulator functions without any delay in between.
[1] GT5663 Datasheet_English_20151106_Rev.01
Signed-off-by: Jagan Teki
---
drivers/input/touchscreen/goodix.c | 58 ++
1 file changed, 58 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c
b/drivers/input/touchscreen/goodix.c
index f57d82220a88..de5b80a08f41 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -27,6 +27,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -47,6 +48,8 @@ struct goodix_ts_data {
struct touchscreen_properties prop;
unsigned int max_touch_num;
unsigned int int_trigger_type;
+ struct regulator *avdd28;
+ struct regulator *vddio;
struct gpio_desc *gpiod_int;
struct gpio_desc *gpiod_rst;
u16 id;
@@ -532,6 +535,24 @@ static int goodix_get_gpio_config(struct goodix_ts_data
*ts)
return -EINVAL;
dev = >client->dev;
+ ts->avdd28 = devm_regulator_get(dev, "AVDD28");
+ if (IS_ERR(ts->avdd28)) {
+ error = PTR_ERR(ts->avdd28);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev,
+ "Failed to get AVDD28 regulator: %d\n", error);
+ return error;
+ }
+
+ ts->vddio = devm_regulator_get(dev, "VDDIO");
+ if (IS_ERR(ts->vddio)) {
+ error = PTR_ERR(ts->vddio);
+ if (error != -EPROBE_DEFER)
+ dev_err(dev,
+ "Failed to get VDDIO regulator: %d\n", error);
+ return error;
+ }
+
/* Get the interrupt GPIO pin number */
gpiod = devm_gpiod_get_optional(dev, GOODIX_GPIO_INT_NAME, GPIOD_IN);
if (IS_ERR(gpiod)) {
@@ -764,6 +785,17 @@ static void goodix_config_cb(const struct firmware *cfg,
void *ctx)
complete_all(>firmware_loading_complete);
}
+static void goodix_disable_regulator(void *arg)
+{
+ struct goodix_ts_data *ts = arg;
+
+ if (!IS_ERR(ts->vddio))
+ regulator_disable(ts->vddio);
+
+ if (!IS_ERR(ts->avdd28))
+ regulator_disable(ts->avdd28);
+}
+
static int goodix_ts_probe(struct i2c_client *client,
const struct i2c_device_id *id)
{
@@ -789,6 +821,32 @@ static int goodix_ts_probe(struct i2c_client *client,
if (error)
return error;
+ error = devm_add_action_or_reset(>dev,
+goodix_disable_regulator, ts);
+ if (error)
+ return error;
+
+ /* power the controller */
+ if (!IS_ERR(ts->avdd28)) {
+ error = regulator_enable(ts->avdd28);
+ if (error) {
+ dev_err(>dev,
+ "Failed to enable AVDD28 regulator: %d\n",
+ error);
+ return error;
+ }
+ }
+
+ if (!IS_ERR(ts->vddio)) {
+ error = regulator_enable(ts->vddio);
+ if (error) {
+ dev_err(>dev,
+ "Failed to enable VDDIO regulator: %d\n",
+ error);
+ return error;
+ }
+ }
+
if (ts->gpiod_int && ts->gpiod_rst) {
/* reset the controller */
error = goodix_reset(ts);
--
2.18.0.321.gffc6fa0e3
[PATCH v7 4/4] Input: goodix - Add GT5663 CTP support
GT5663 is capacitive touch controller with customized smart
wakeup gestures.
Add support for it by adding compatible and supported chip data.
The chip data on GT5663 is similar to GT1151, like
- config data register has 0x8050 address
- config data register max len is 240
- config data checksum has 16-bit
Signed-off-by: Jagan Teki
---
drivers/input/touchscreen/goodix.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/drivers/input/touchscreen/goodix.c
b/drivers/input/touchscreen/goodix.c
index de5b80a08f41..c558b091749c 100644
--- a/drivers/input/touchscreen/goodix.c
+++ b/drivers/input/touchscreen/goodix.c
@@ -219,6 +219,7 @@ static const struct goodix_chip_data
*goodix_get_chip_data(u16 id)
{
switch (id) {
case 1151:
+ case 5663:
case 5688:
return _chip_data;
@@ -1003,6 +1004,7 @@ MODULE_DEVICE_TABLE(acpi, goodix_acpi_match);
#ifdef CONFIG_OF
static const struct of_device_id goodix_of_match[] = {
{ .compatible = "goodix,gt1151" },
+ { .compatible = "goodix,gt5663" },
{ .compatible = "goodix,gt5688" },
{ .compatible = "goodix,gt911" },
{ .compatible = "goodix,gt9110" },
--
2.18.0.321.gffc6fa0e3
[PATCH v7 0/4] input: touchscreen: Add goodix GT5553 CTP support
This is v7 patchset for supporting goodix GT5553 CTP. Here is the previous version[1] Changes for v5: - rebase on linux-next Changes for v5: - document bindings for required regulators, which are need during power-on sequence - enable, disable required regulators as described in power-on sequence using normal regulator calls - update the proper commi messages Changes for v4: - document AVDD22, DVDD12, VDDIO as optional properties - use regulator bulk calls, for get, enable and disable functionalities Changes for v4: - devm_add_action_or_reset for disabling regulator Changes for v3: - add cover-letter - s/ADVV28/AVDD28 on commit head - fix few typo Changes for v2: - Rename vcc-supply with AVDD28-supply - disable regulator in remove - fix to setup regulator in probe code - add chipdata - drop example node in dt-bindings [1] https://patchwork.kernel.org/cover/10819645/ Jagan Teki (4): dt-bindings: input: touchscreen: goodix: Document regulator properties Input: goodix - Add regulators suppot dt-bindings: input: touchscreen: goodix: Add GT5663 compatible Input: goodix - Add GT5663 CTP support .../bindings/input/touchscreen/goodix.txt | 3 + drivers/input/touchscreen/goodix.c| 60 +++ 2 files changed, 63 insertions(+) -- 2.18.0.321.gffc6fa0e3
[PATCH v7 1/4] dt-bindings: input: touchscreen: goodix: Document regulator properties
Goodix CTP controllers support analog, digital and gpio regulator supplies on relevant controller pin configurations. Out of which AVDD28 and VDDIO regulators are required in few goodix CTP chips during power-on sequence. AVDD22, DVDD12 regulators have no relevant functionality described from datasheet [1]. So, document both AVDD28, VDDIO regulators into optional properties since few of the goodix chip do work without these regulator power-on sequence. [1] GT5663 Datasheet_English_20151106_Rev.01 Signed-off-by: Jagan Teki Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/input/touchscreen/goodix.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt index 8cf0b4d38a7e..255673250bbd 100644 --- a/Documentation/devicetree/bindings/input/touchscreen/goodix.txt +++ b/Documentation/devicetree/bindings/input/touchscreen/goodix.txt @@ -19,6 +19,8 @@ Optional properties: - irq-gpios : GPIO pin used for IRQ. The driver uses the interrupt gpio pin as output to reset the device. - reset-gpios : GPIO pin used for reset + - AVDD28-supply : Analog power supply regulator on AVDD28 pin + - VDDIO-supply: GPIO power supply regulator on VDDIO pin - touchscreen-inverted-x - touchscreen-inverted-y - touchscreen-size-x -- 2.18.0.321.gffc6fa0e3
Re: [PATCH v4 4/4] leds: lm3532: Introduce the lm3532 LED driver
On Wed, 20 Mar 2019, Jacek Anaszewski wrote:
> Hi Dan,
>
> I have one minor issue below.
>
> On 3/13/19 1:32 PM, Dan Murphy wrote:
> > Introduce the Texas Instruments LM3532 White LED driver.
> > The driver supports ALS configurability or manual brightness
> > control.
> >
> > The driver also supports associating LED strings with specific
> > control banks in a group or as individually controlled strings.
> >
> > Signed-off-by: Dan Murphy
> > ---
> >
> > v4 - Updated code to DT doc changes appending "ti," to TI specific
> > properties -
> > https://lore.kernel.org/patchwork/patch/1050124/
> >
> > v3 - Removed switch case for register setting in favor of an algorithim (v1
> > comment),
> > https://lore.kernel.org/patchwork/patch/1049024/
> > v2 - Added look up tables for als_avg, als_imp and ramp times, fixed
> > brightness
> > control when ALS is in control, added property value checks to ensure the
> > values
> > are not out of bounds - https://lore.kernel.org/patchwork/patch/1048807/
> >
> > drivers/leds/Kconfig | 10 +
> > drivers/leds/Makefile | 1 +
> > drivers/leds/leds-lm3532.c | 681 +
> > 3 files changed, 692 insertions(+)
> > create mode 100644 drivers/leds/leds-lm3532.c
[snip]
> > diff --git a/drivers/leds/leds-lm3532.c b/drivers/leds/leds-lm3532.c
> > new file mode 100644
> > index ..e8a614d71469
> > --- /dev/null
> > +++ b/drivers/leds/leds-lm3532.c
[snip 700 lines of churn]
Jacek could you please trim your replies please? It took way to long
to locate your one line comment in this 700 line patch.
> > +static int lm3532_remove(struct i2c_client *client)
> > +{
> > + struct lm3532_data *drvdata = i2c_get_clientdata(client);
> mutex_destroy(>lock);
--
Lee Jones [李琼斯]
Linaro Services Technical Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog
Re: ratelimit API: was: [RFC PATCH] printk: Introduce "store now but print later" prefix.
On 2019/03/21 0:25, Petr Mladek wrote: >> This requires serialization among threads using "rs". I already >> proposed ratelimit_reset() for memcg's OOM problem at >> https://lkml.kernel.org/r/201810180246.w9i2koi3011...@www262.sakura.ne.jp >> but it was not accepted. > > IMHO, the main problem was that the patch tried to work around > the ratelimit API weakness by a custom code. > > I believe that using an improved/extended ratelimit API with > a sane semantic would be more acceptable. > Michal, are you OK to use ratelimit_reset() in out_of_memory() if ratelimit_reset() is accepted? > >>> It means that it makes sense to enable the related >>> ratelimited messages again because they would describe >>> another problem. >> >> ___ratelimit() could also check number of not-yet-flushed >> printk() records (e.g. log_next_seq - console_seq <= $some_threshold). > > The number is almost useless without more information, for example, > how fast the consoles are, how many lines will get filtered > by a console_loglevel, if the console_sem owner is sleeping, > how many messages are being added by other CPUs. > > I believe that we do not really need it. The ratelimit_reset() > user should know when the messages can get skipped because > they describe the same situation again and again. If printk() becomes asynchronous (either my "synchronous by default + console_writer kernel thread" or John's "asynchronous by default + printk kernel thread" is accepted), there will be little delay between ___ratelimit() and ratelimit_reset() enough to make ratelimit_reset() unnecessary. Also, "number of not-yet-flushed printk() records" will become meaningful if printk() becomes asynchronous. But "how fast the consoles" depends on what consoles will be added/removed during messages are flushed, "how many lines will get filtered by a console_loglevel" depends on whether someone will change it during messages are flushed, "how many messages are being added by other CPUs during the console_sem owner is sleeping" (or "how many bytes which will need to be written to consoles") depends on stress at that moment. None of these example information will be reliable for making ___ratelimit() decision.
Re: [PATCH] mm/isolation: Remove redundant pfn_valid_within() in __first_valid_page()
On 03/21/2019 01:37 PM, Michal Hocko wrote:
> On Thu 21-03-19 11:03:18, Anshuman Khandual wrote:
>>
>>
>> On 03/21/2019 10:31 AM, Zi Yan wrote:
>>> On 20 Mar 2019, at 21:13, Anshuman Khandual wrote:
>>>
pfn_valid_within() calls pfn_valid() when CONFIG_HOLES_IN_ZONE making it
redundant for both definitions (w/wo CONFIG_MEMORY_HOTPLUG) of the helper
pfn_to_online_page() which either calls pfn_valid() or pfn_valid_within().
pfn_valid_within() being 1 when !CONFIG_HOLES_IN_ZONE is irrelevant either
way. This does not change functionality.
Fixes: 2ce13640b3f4 ("mm: __first_valid_page skip over offline pages")
>>>
>>> I would not say this patch fixes the commit 2ce13640b3f4 from 2017,
>>> because the pfn_valid_within() in pfn_to_online_page() was introduced by
>>> a recent commit b13bc35193d9e last month. :)
>>
>> Right, will update the tag with this commit.
>
> The patch is correct but I wouldn't bother to add Fixes tag at all. The
> current code is obviously not incorrect. Do you see any actual
Sure.
> performance issue?
>
No. Just from code inspection. pfn_valid() is anyways expensive on arm64
because of the memblock search so why to make it redundant as well.
Re: [PATCH -next] ubifs: remove unused function __ubifs_shash_final
Am Donnerstag, 21. März 2019, 08:54:55 CET schrieb Mukesh Ojha: > >> Acked-by: Mukesh Ojha > > I guess you mean Reviewed-by? > > As i am unsure about future scope of this func. i.e why Acked. Acked-by is usually something I expect from the code author or the person that owns the code. Thanks, //richard
[RFC] mm/hotplug: Make get_nid_for_pfn() work with HAVE_ARCH_PFN_VALID
Memory hot remove uses get_nid_for_pfn() while tearing down linked sysfs
entries between memory block and node. It first checks pfn validity with
pfn_valid_within() before fetching nid. With CONFIG_HOLES_IN_ZONE config
(arm64 has this enabled) pfn_valid_within() calls pfn_valid().
pfn_valid() is an arch implementation on arm64 (CONFIG_HAVE_ARCH_PFN_VALID)
which scans all mapped memblock regions with memblock_is_map_memory(). This
creates a problem in memory hot remove path which has already removed given
memory range from memory block with memblock_[remove|free] before arriving
at unregister_mem_sect_under_nodes().
During runtime memory hot remove get_nid_for_pfn() needs to validate that
given pfn has a struct page mapping so that it can fetch required nid. This
can be achieved just by looking into it's section mapping information. This
adds a new helper pfn_section_valid() for this purpose. Its same as generic
pfn_valid().
This maintains existing behaviour for deferred struct page init case.
Signed-off-by: Anshuman Khandual
---
This is a preparatory patch for memory hot-remove enablement on arm64. I
will appreciate some early feedback on this approach.
drivers/base/node.c| 15 ---
include/linux/mmzone.h | 9 +++--
2 files changed, 19 insertions(+), 5 deletions(-)
diff --git a/drivers/base/node.c b/drivers/base/node.c
index 86d6cd92ce3d..9e944b71e352 100644
--- a/drivers/base/node.c
+++ b/drivers/base/node.c
@@ -394,11 +394,20 @@ int unregister_cpu_under_node(unsigned int cpu, unsigned
int nid)
#ifdef CONFIG_MEMORY_HOTPLUG_SPARSE
static int __ref get_nid_for_pfn(unsigned long pfn)
{
- if (!pfn_valid_within(pfn))
- return -1;
#ifdef CONFIG_DEFERRED_STRUCT_PAGE_INIT
- if (system_state < SYSTEM_RUNNING)
+ if (system_state < SYSTEM_RUNNING) {
+ if (!pfn_valid_within(pfn))
+ return -1;
return early_pfn_to_nid(pfn);
+ }
+#endif
+
+#if defined(CONFIG_HAVE_ARCH_PFN_VALID) && defined(CONFIG_HOLES_IN_ZONE)
+ if (!pfn_section_valid(pfn))
+ return -1;
+#else
+ if (!pfn_valid_within(pfn))
+ return -1;
#endif
return pfn_to_nid(pfn);
}
diff --git a/include/linux/mmzone.h b/include/linux/mmzone.h
index 842f9189537b..9cf4cb95 100644
--- a/include/linux/mmzone.h
+++ b/include/linux/mmzone.h
@@ -1242,13 +1242,18 @@ static inline struct mem_section
*__pfn_to_section(unsigned long pfn)
extern int __highest_present_section_nr;
-#ifndef CONFIG_HAVE_ARCH_PFN_VALID
-static inline int pfn_valid(unsigned long pfn)
+static inline int pfn_section_valid(unsigned long pfn)
{
if (pfn_to_section_nr(pfn) >= NR_MEM_SECTIONS)
return 0;
return valid_section(__nr_to_section(pfn_to_section_nr(pfn)));
}
+
+#ifndef CONFIG_HAVE_ARCH_PFN_VALID
+static inline int pfn_valid(unsigned long pfn)
+{
+ return pfn_section_valid(pfn);
+}
#endif
static inline int pfn_present(unsigned long pfn)
--
2.20.1
Re: [PATCH] mm/isolation: Remove redundant pfn_valid_within() in __first_valid_page()
On Thu 21-03-19 11:03:18, Anshuman Khandual wrote:
>
>
> On 03/21/2019 10:31 AM, Zi Yan wrote:
> > On 20 Mar 2019, at 21:13, Anshuman Khandual wrote:
> >
> >> pfn_valid_within() calls pfn_valid() when CONFIG_HOLES_IN_ZONE making it
> >> redundant for both definitions (w/wo CONFIG_MEMORY_HOTPLUG) of the helper
> >> pfn_to_online_page() which either calls pfn_valid() or pfn_valid_within().
> >> pfn_valid_within() being 1 when !CONFIG_HOLES_IN_ZONE is irrelevant either
> >> way. This does not change functionality.
> >>
> >> Fixes: 2ce13640b3f4 ("mm: __first_valid_page skip over offline pages")
> >
> > I would not say this patch fixes the commit 2ce13640b3f4 from 2017,
> > because the pfn_valid_within() in pfn_to_online_page() was introduced by
> > a recent commit b13bc35193d9e last month. :)
>
> Right, will update the tag with this commit.
The patch is correct but I wouldn't bother to add Fixes tag at all. The
current code is obviously not incorrect. Do you see any actual
performance issue?
--
Michal Hocko
SUSE Labs
My name is Francis Aaron,I have a financial proposal which will benefit both of us.If you are interested,get back to me for more details.Thanks
[PATCH] vfs: Convert functionfs to fs_context
Signed-off-by: David Howells
cc: Felipe Balbi
cc: Michał Nazarewicz
cc: linux-...@vger.kernel.org
---
drivers/usb/gadget/function/f_fs.c | 232 ++--
1 file changed, 119 insertions(+), 113 deletions(-)
diff --git a/drivers/usb/gadget/function/f_fs.c
b/drivers/usb/gadget/function/f_fs.c
index 20413c276c61..1b8061cdf87c 100644
--- a/drivers/usb/gadget/function/f_fs.c
+++ b/drivers/usb/gadget/function/f_fs.c
@@ -17,6 +17,7 @@
#include
#include
#include
+#include
#include
#include
#include
@@ -1447,9 +1448,9 @@ struct ffs_sb_fill_data {
struct ffs_data *ffs_data;
};
-static int ffs_sb_fill(struct super_block *sb, void *_data, int silent)
+static int ffs_sb_fill(struct super_block *sb, struct fs_context *fc)
{
- struct ffs_sb_fill_data *data = _data;
+ struct ffs_sb_fill_data *data = fc->fs_private;
struct inode*inode;
struct ffs_data *ffs = data->ffs_data;
@@ -1482,147 +1483,152 @@ static int ffs_sb_fill(struct super_block *sb, void
*_data, int silent)
return 0;
}
-static int ffs_fs_parse_opts(struct ffs_sb_fill_data *data, char *opts)
-{
- ENTER();
+enum {
+ Opt_no_disconnect,
+ Opt_rmode,
+ Opt_fmode,
+ Opt_mode,
+ Opt_uid,
+ Opt_gid,
+};
- if (!opts || !*opts)
- return 0;
+static const struct fs_parameter_spec ffs_fs_param_specs[] = {
+ fsparam_bool("no_disconnect", Opt_no_disconnect),
+ fsparam_u32 ("rmode", Opt_rmode),
+ fsparam_u32 ("fmode", Opt_fmode),
+ fsparam_u32 ("mode",Opt_mode),
+ fsparam_u32 ("uid", Opt_uid),
+ fsparam_u32 ("gid", Opt_gid),
+ {}
+};
- for (;;) {
- unsigned long value;
- char *eq, *comma;
-
- /* Option limit */
- comma = strchr(opts, ',');
- if (comma)
- *comma = 0;
-
- /* Value limit */
- eq = strchr(opts, '=');
- if (unlikely(!eq)) {
- pr_err("'=' missing in %s\n", opts);
- return -EINVAL;
- }
- *eq = 0;
+static const struct fs_parameter_description ffs_fs_fs_parameters = {
+ .name = "kAFS",
+ .specs = ffs_fs_param_specs,
+};
- /* Parse value */
- if (kstrtoul(eq + 1, 0, )) {
- pr_err("%s: invalid value: %s\n", opts, eq + 1);
- return -EINVAL;
- }
+static int ffs_fs_parse_param(struct fs_context *fc, struct fs_parameter
*param)
+{
+ struct ffs_sb_fill_data *data = fc->fs_private;
+ struct fs_parse_result result;
+ int opt;
- /* Interpret option */
- switch (eq - opts) {
- case 13:
- if (!memcmp(opts, "no_disconnect", 13))
- data->no_disconnect = !!value;
- else
- goto invalid;
- break;
- case 5:
- if (!memcmp(opts, "rmode", 5))
- data->root_mode = (value & 0555) | S_IFDIR;
- else if (!memcmp(opts, "fmode", 5))
- data->perms.mode = (value & 0666) | S_IFREG;
- else
- goto invalid;
- break;
+ ENTER();
- case 4:
- if (!memcmp(opts, "mode", 4)) {
- data->root_mode = (value & 0555) | S_IFDIR;
- data->perms.mode = (value & 0666) | S_IFREG;
- } else {
- goto invalid;
- }
- break;
+ opt = fs_parse(fc, _fs_fs_parameters, param, );
+ if (opt < 0)
+ return opt;
- case 3:
- if (!memcmp(opts, "uid", 3)) {
- data->perms.uid = make_kuid(current_user_ns(),
value);
- if (!uid_valid(data->perms.uid)) {
- pr_err("%s: unmapped value: %lu\n",
opts, value);
- return -EINVAL;
- }
- } else if (!memcmp(opts, "gid", 3)) {
- data->perms.gid = make_kgid(current_user_ns(),
value);
- if (!gid_valid(data->perms.gid)) {
- pr_err("%s: unmapped value: %lu\n",
opts, value);
- return -EINVAL;
- }
- } else {
- goto invalid;
-
[PATCH v3 12/18] locking/lockdep: Remove unnecessary function pointer argument
check_prev_add() always has save_trace() as an input argument, which is
unnecessary, so remove it.
Signed-off-by: Yuyang Du
Reviewed-by: Bart Van Assche
---
kernel/locking/lockdep.c | 11 +--
1 file changed, 5 insertions(+), 6 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index eccfb0b..f46695a 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2193,8 +2193,7 @@ static inline void inc_chains(void)
*/
static int
check_prev_add(struct task_struct *curr, struct held_lock *prev,
- struct held_lock *next, int distance, struct stack_trace *trace,
- int (*save)(struct stack_trace *trace))
+ struct held_lock *next, int distance, struct stack_trace *trace)
{
struct lock_list *uninitialized_var(target_entry);
struct lock_list *entry;
@@ -2234,11 +2233,11 @@ static inline void inc_chains(void)
if (unlikely(!ret)) {
if (!trace->entries) {
/*
-* If @save fails here, the printing might trigger
+* If save_trace fails here, the printing might trigger
* a WARN but because of the !nr_entries it should
* not do bad things.
*/
- save(trace);
+ save_trace(trace);
}
print_circular_bug(, target_entry, next, prev, trace);
return 0;
@@ -2293,7 +2292,7 @@ static inline void inc_chains(void)
}
- if (!trace->entries && !save(trace))
+ if (!trace->entries && !save_trace(trace))
return 0;
/*
@@ -2358,7 +2357,7 @@ static inline void inc_chains(void)
* added:
*/
if (hlock->read != 2 && hlock->check) {
- int ret = check_prev_add(curr, hlock, next, distance,
, save_trace);
+ int ret = check_prev_add(curr, hlock, next, distance,
);
if (!ret)
return 0;
--
1.8.3.1
[PATCH v3 07/18] locking/lockdep: Use lockdep_init_task for task initiation consistently
Despite that there is a lockdep_init_task() which does nothing, lockdep
initiates tasks by assigning lockdep fields and does so inconsistently. Fix
this by using lockdep_init_task().
Signed-off-by: Yuyang Du
---
include/linux/lockdep.h | 7 ++-
init/init_task.c | 2 ++
kernel/fork.c| 3 ---
kernel/locking/lockdep.c | 11 ---
4 files changed, 16 insertions(+), 7 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 37706ad..267087e 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -282,6 +282,8 @@ struct held_lock {
extern asmlinkage void lockdep_sys_exit(void);
extern void lockdep_set_selftest_task(struct task_struct *task);
+extern inline void lockdep_init_task(struct task_struct *task);
+
extern void lockdep_off(void);
extern void lockdep_on(void);
@@ -406,6 +408,10 @@ static inline void lock_set_subclass(struct lockdep_map
*lock,
#else /* !CONFIG_LOCKDEP */
+static inline void lockdep_init_task(struct task_struct *task)
+{
+}
+
static inline void lockdep_off(void)
{
}
@@ -498,7 +504,6 @@ enum xhlock_context_t {
{ .name = (_name), .key = (void *)(_key), }
static inline void lockdep_invariant_state(bool force) {}
-static inline void lockdep_init_task(struct task_struct *task) {}
static inline void lockdep_free_task(struct task_struct *task) {}
#ifdef CONFIG_LOCK_STAT
diff --git a/init/init_task.c b/init/init_task.c
index 46dbf54..9460878 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -165,6 +165,8 @@ struct task_struct init_task
.softirqs_enabled = 1,
#endif
#ifdef CONFIG_LOCKDEP
+ .lockdep_depth = 0, /* no locks held yet */
+ .curr_chain_key = 0,
.lockdep_recursion = 0,
#endif
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
diff --git a/kernel/fork.c b/kernel/fork.c
index 77059b2..c0d2000 100644
--- a/kernel/fork.c
+++ b/kernel/fork.c
@@ -1869,9 +1869,6 @@ static __latent_entropy struct task_struct *copy_process(
p->pagefault_disabled = 0;
#ifdef CONFIG_LOCKDEP
- p->lockdep_depth = 0; /* no locks held yet */
- p->curr_chain_key = 0;
- p->lockdep_recursion = 0;
lockdep_init_task(p);
#endif
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 05c31d6..ef59651 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -358,6 +358,13 @@ static inline u64 iterate_chain_key(u64 key, u32 idx)
return k0 | (u64)k1 << 32;
}
+inline void lockdep_init_task(struct task_struct *task)
+{
+ task->lockdep_depth = 0; /* no locks held yet */
+ task->curr_chain_key = 0;
+ task->lockdep_recursion = 0;
+}
+
void lockdep_off(void)
{
current->lockdep_recursion++;
@@ -4496,9 +4503,7 @@ void lockdep_reset(void)
int i;
raw_local_irq_save(flags);
- current->curr_chain_key = 0;
- current->lockdep_depth = 0;
- current->lockdep_recursion = 0;
+ lockdep_init_task(current);
memset(current->held_locks, 0, MAX_LOCK_DEPTH*sizeof(struct held_lock));
nr_hardirq_chains = 0;
nr_softirq_chains = 0;
--
1.8.3.1
[PATCH v3 05/18] locking/lockdep: Print the right depth for chain key colission
Since chains are separated by irq context, so when printing a chain the
depth should be consistent with it.
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 7 ---
1 file changed, 4 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index eeea722..05c31d6 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2428,10 +2428,11 @@ static u64 print_chain_key_iteration(int class_idx, u64
chain_key)
struct held_lock *hlock;
u64 chain_key = 0;
int depth = curr->lockdep_depth;
- int i;
+ int i = get_first_held_lock(curr, hlock_next);
- printk("depth: %u\n", depth + 1);
- for (i = get_first_held_lock(curr, hlock_next); i < depth; i++) {
+ printk("depth: %u (irq_context %u)\n", depth - i + 1,
+ hlock_next->irq_context);
+ for (; i < depth; i++) {
hlock = curr->held_locks + i;
chain_key = print_chain_key_iteration(hlock->class_idx,
chain_key);
--
1.8.3.1
[PATCH v3 16/18] locking/lockdep: Combine check_noncircular and check_redundant
These two functions are essentially duplicates, combine them into
check_nonexistent(). Also update the comment on it.
No functional change.
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 31 +++
1 file changed, 11 insertions(+), 20 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index cd6792c..8202318 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1688,29 +1688,18 @@ unsigned long lockdep_count_backward_deps(struct
lock_class *class)
}
/*
- * Prove that the dependency graph starting at can not
- * lead to . Print an error and return 0 if it does.
+ * Prove that the dependency graph starting at can not
+ * lead to . If existent, there is a circle when adding
+ * a -> dependency.
+ *
+ * Print an error and return 0 if it does exist.
*/
static noinline int
-check_noncircular(struct lock_list *root, struct lock_class *target,
- struct lock_list **target_entry)
+check_nonexistent(struct lock_list *root, struct lock_class *target,
+ struct lock_list **target_entry)
{
int result;
- debug_atomic_inc(nr_cyclic_checks);
-
- result = __bfs_forwards(root, target, class_equal, target_entry);
-
- return result;
-}
-
-static noinline int
-check_redundant(struct lock_list *root, struct lock_class *target,
- struct lock_list **target_entry)
-{
- int result;
-
- debug_atomic_inc(nr_redundant_checks);
result = __bfs_forwards(root, target, class_equal, target_entry);
@@ -2246,7 +2235,8 @@ static inline void inc_chains(void)
*/
this.class = hlock_class(next);
this.parent = NULL;
- ret = check_noncircular(, hlock_class(prev), _entry);
+ debug_atomic_inc(nr_cyclic_checks);
+ ret = check_nonexistent(, hlock_class(prev), _entry);
if (unlikely(!ret)) {
if (!trace->entries) {
/*
@@ -2298,7 +2288,8 @@ static inline void inc_chains(void)
*/
this.class = hlock_class(prev);
this.parent = NULL;
- ret = check_redundant(, hlock_class(next), _entry);
+ debug_atomic_inc(nr_redundant_checks);
+ ret = check_nonexistent(, hlock_class(next), _entry);
if (!ret) {
debug_atomic_inc(nr_redundant);
return 2;
--
1.8.3.1
[PATCH v3 15/18] locking/lockdep: Avoid constant checks in __bfs by using offset reference
In search of a dependency in the lock graph, there is contant checks for
forward or backward search. Directly reference the field offset of the
struct that differentiates the type of search to avoid those checks.
No functional change.
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 33 +
1 file changed, 21 insertions(+), 12 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ad16793..cd6792c 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1376,11 +1376,25 @@ static inline int get_lock_depth(struct lock_list
*child)
return depth;
}
+/*
+ * Return the forward or backward dependency list.
+ *
+ * @lock: the lock_list to get its class's dependency list
+ * @offset: the offset to struct lock_class to determine whether it is
+ * locks_after or locks_before
+ */
+static inline struct list_head *get_dep_list(struct lock_list *lock, int
offset)
+{
+ void *lock_class = lock->class;
+
+ return lock_class + offset;
+}
+
static int __bfs(struct lock_list *source_entry,
void *data,
int (*match)(struct lock_list *entry, void *data),
struct lock_list **target_entry,
-int forward)
+int offset)
{
struct lock_list *entry;
struct lock_list *lock;
@@ -1394,11 +1408,7 @@ static int __bfs(struct lock_list *source_entry,
goto exit;
}
- if (forward)
- head = _entry->class->locks_after;
- else
- head = _entry->class->locks_before;
-
+ head = get_dep_list(source_entry, offset);
if (list_empty(head))
goto exit;
@@ -1412,10 +1422,7 @@ static int __bfs(struct lock_list *source_entry,
goto exit;
}
- if (forward)
- head = >class->locks_after;
- else
- head = >class->locks_before;
+ head = get_dep_list(lock, offset);
DEBUG_LOCKS_WARN_ON(!irqs_disabled());
@@ -1448,7 +1455,8 @@ static inline int __bfs_forwards(struct lock_list
*src_entry,
int (*match)(struct lock_list *entry, void *data),
struct lock_list **target_entry)
{
- return __bfs(src_entry, data, match, target_entry, 1);
+ return __bfs(src_entry, data, match, target_entry,
+offsetof(struct lock_class, locks_after));
}
@@ -1457,7 +1465,8 @@ static inline int __bfs_backwards(struct lock_list
*src_entry,
int (*match)(struct lock_list *entry, void *data),
struct lock_list **target_entry)
{
- return __bfs(src_entry, data, match, target_entry, 0);
+ return __bfs(src_entry, data, match, target_entry,
+offsetof(struct lock_class, locks_before));
}
--
1.8.3.1
[PATCH v3 06/18] locking/lockdep: Update obsolete struct field description
The lock_chain struct definition has outdated comment, update it and add
struct member description.
Signed-off-by: Yuyang Du
---
include/linux/lockdep.h | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 79c3873..37706ad 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -199,10 +199,16 @@ struct lock_list {
};
/*
- * We record lock dependency chains, so that we can cache them:
+ * struct lock_chain - we record lock dependency chains so that we can cache
them
+ *
+ * @irq_context: the same as irq_context in held_lock below
+ * @depth: the number of held locks in this chain
+ * @base:the index in chain_hlocks for this chain
+ * @entry: the collided lock chains in lock_chain hash list
+ * @chain_key: the hash key of this lock_chain
*/
struct lock_chain {
- /* see BUILD_BUG_ON()s in lookup_chain_cache() */
+ /* see BUILD_BUG_ON()s in add_chain_cache() */
unsigned intirq_context : 2,
depth : 6,
base: 24;
--
1.8.3.1
[PATCH v3 18/18] locking/lockdep: Add explanation to lock usage rules in lockdep design doc
The rules that if violated a deacklock may happen are explained in more detail concerning both irqs and circular dependencies. Signed-off-by: Yuyang Du --- Documentation/locking/lockdep-design.txt | 35 +++- 1 file changed, 25 insertions(+), 10 deletions(-) diff --git a/Documentation/locking/lockdep-design.txt b/Documentation/locking/lockdep-design.txt index 1dcceaa..83803c6 100644 --- a/Documentation/locking/lockdep-design.txt +++ b/Documentation/locking/lockdep-design.txt @@ -105,14 +105,24 @@ Unused locks (e.g., mutexes) cannot be part of the cause of an error. Single-lock state rules: +A lock is irq-safe means it was ever used in an irq context, while a lock +is irq-unsafe means it was ever acquired with irq enabled. + A softirq-unsafe lock-class is automatically hardirq-unsafe as well. The -following states are exclusive, and only one of them is allowed to be -set for any lock-class: +following states must be exclusive: only one of them is allowed to be set +for any lock-class based on its usage: + + or + or - and - and +This is because if a lock can be used in irq (safe) then it cannot be ever +acquired with irq enabled (unsafe). Otherwise, a deadlock may happen. For +example, in the scenario that after this lock was acquired but before +released, if the context is interrupted this lock will be attempted to +acquire twice, which creates a deadlock, sometimes referred to as lock +recursion deadlock. -The validator detects and reports lock usage that violate these +The validator detects and reports lock usage that violates these single-lock state rules. Multi-lock dependency rules: @@ -121,15 +131,20 @@ Multi-lock dependency rules: The same lock-class must not be acquired twice, because this could lead to lock recursion deadlocks. -Furthermore, two locks may not be taken in different order: +Furthermore, two locks can not be taken in inverse order: -> -> -because this could lead to lock inversion deadlocks. (The validator -finds such dependencies in arbitrary complexity, i.e. there can be any -other locking sequence between the acquire-lock operations, the -validator will still track all dependencies between locks.) +because it could lead to a deadlock - sometimes referred to as lock +inversion deadlock - as attempts to acquire the two locks form a circle +which could lead to two contexts waiting for each other permanently, namely +the two contexts are holding one lock while waiting for acquiring the other +in an inverse order. The validator will find such circle in arbitrary +complexity. In other words, there can be any number of locking sequences +between two acquire-lock operations (holding one lock while acquiring +another); the validator will still find whether these locks can be acquired +in a circular fashion. Furthermore, the following usage based lock dependencies are not allowed between any two lock-classes: -- 1.8.3.1
[PATCH v3 17/18] locking/lockdep: Update comments on dependency search
The breadth-first search is implemented as flat-out non-recursive now, but the comments are still describing it as recursive, update the comments in that regard. Signed-off-by: Yuyang Du --- kernel/locking/lockdep.c | 21 ++--- 1 file changed, 10 insertions(+), 11 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 8202318..9df2b1a 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1390,6 +1390,10 @@ static inline struct list_head *get_dep_list(struct lock_list *lock, int offset) return lock_class + offset; } +/* + * Forward- or backward-dependency search, used for both circular dependency + * checking and hardirq-unsafe/softirq-unsafe checking. + */ static int __bfs(struct lock_list *source_entry, void *data, int (*match)(struct lock_list *entry, void *data), @@ -1471,12 +1475,6 @@ static inline int __bfs_backwards(struct lock_list *src_entry, } /* - * Recursive, forwards-direction lock-dependency checking, used for - * both noncyclic checking and for hardirq-unsafe/softirq-unsafe - * checking. - */ - -/* * Print a dependency chain entry (this is only done when a deadlock * has been detected): */ @@ -2177,7 +2175,7 @@ static inline void inc_chains(void) /* * There was a chain-cache miss, and we are about to add a new dependency - * to a previous lock. We recursively validate the following rules: + * to a previous lock. We validate the following rules: * * - would the adding of the -> dependency create a *circular dependency in the graph? [== circular deadlock] @@ -2227,11 +2225,12 @@ static inline void inc_chains(void) /* * Prove that the new -> dependency would not * create a circular dependency in the graph. (We do this by -* forward-recursing into the graph starting at , and -* checking whether we can reach .) +* a breadth-first search into the graph starting at , +* which checks whether we can reach .) * -* We are using global variables to control the recursion, to -* keep the stackframe size of the recursive functions low: +* The search is limited by the size of the circular queue (i.e., +* MAX_CIRCULAR_QUEUE_SIZE) which keeps track of a breadth of nodes +* in the graph whose neighbours are to be checked. */ this.class = hlock_class(next); this.parent = NULL; -- 1.8.3.1
[PATCH v3 11/18] locking/lockdep: Update comment
An out-of-nowhere comment is removed. While at it, add more explanatory comments. Such a trivial patch! Signed-off-by: Yuyang Du --- kernel/locking/lockdep.c | 12 +--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index c7aec9f..eccfb0b 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2720,10 +2720,16 @@ static int validate_chain(struct task_struct *curr, * - is softirq-safe, if this lock is hardirq-unsafe * * And check whether the new lock's dependency graph -* could lead back to the previous lock. +* could lead back to the previous lock: * -* any of these scenarios could lead to a deadlock. If -* All validations +* - within the current held-lock stack +* - across our accumulated lock dependency records +* +* any of these scenarios could lead to a deadlock. +*/ + /* +* The simple case: does the current hold the same lock +* already? */ int ret = check_deadlock(curr, hlock, hlock->read); -- 1.8.3.1
[PATCH v3 13/18] locking/lockdep: Change type of the element field in circular_queue
The element field is an array in struct circular_queue to keep track of locks
in the search. Making it the same type as the locks avoids type cast. Also
fix a typo and elaborate the comment above struct circular_queue.
No functional change.
Signed-off-by: Yuyang Du
Reviewed-by: Bart Van Assche
---
kernel/locking/lockdep.c | 23 +--
1 file changed, 13 insertions(+), 10 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index f46695a..8167d69 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1272,13 +1272,16 @@ static int add_lock_to_list(struct lock_class *this,
#define CQ_MASK(MAX_CIRCULAR_QUEUE_SIZE-1)
/*
- * The circular_queue and helpers is used to implement the
- * breadth-first search(BFS)algorithem, by which we can build
- * the shortest path from the next lock to be acquired to the
- * previous held lock if there is a circular between them.
+ * The circular_queue and helpers are used to implement the graph
+ * breadth-first search (BFS) algorithm, by which we can determine whether
+ * there is a path from the next lock to be acquired to a previous held
+ * lock, which indicates that adding the -> lock dependency
+ * produces a circle in the lock dependency graph. Breadth-first search
+ * instead of depth-first search is used for finding the shortest circular
+ * path.
*/
struct circular_queue {
- unsigned long element[MAX_CIRCULAR_QUEUE_SIZE];
+ struct lock_list *element[MAX_CIRCULAR_QUEUE_SIZE];
unsigned int front, rear;
};
@@ -1304,7 +1307,7 @@ static inline int __cq_full(struct circular_queue *cq)
return ((cq->rear + 1) & CQ_MASK) == cq->front;
}
-static inline int __cq_enqueue(struct circular_queue *cq, unsigned long elem)
+static inline int __cq_enqueue(struct circular_queue *cq, struct lock_list
*elem)
{
if (__cq_full(cq))
return -1;
@@ -1314,7 +1317,7 @@ static inline int __cq_enqueue(struct circular_queue *cq,
unsigned long elem)
return 0;
}
-static inline int __cq_dequeue(struct circular_queue *cq, unsigned long *elem)
+static inline int __cq_dequeue(struct circular_queue *cq, struct lock_list
**elem)
{
if (__cq_empty(cq))
return -1;
@@ -1392,12 +1395,12 @@ static int __bfs(struct lock_list *source_entry,
goto exit;
__cq_init(cq);
- __cq_enqueue(cq, (unsigned long)source_entry);
+ __cq_enqueue(cq, source_entry);
while (!__cq_empty(cq)) {
struct lock_list *lock;
- __cq_dequeue(cq, (unsigned long *));
+ __cq_dequeue(cq, );
if (!lock->class) {
ret = -2;
@@ -1421,7 +1424,7 @@ static int __bfs(struct lock_list *source_entry,
goto exit;
}
- if (__cq_enqueue(cq, (unsigned long)entry)) {
+ if (__cq_enqueue(cq, entry)) {
ret = -1;
goto exit;
}
--
1.8.3.1
Re: [PATCH] PM / arch: x86: Rework the MSR_IA32_ENERGY_PERF_BIAS handling
On Thu, Mar 21, 2019 at 01:00:52AM +0100, Rafael J. Wysocki wrote: > + * The Performance and Energy Bias Hint (EPB) allows software to specify its > + * preference with respect to the power-performance tradeoffs present in the > + * processor. Generally, the EPB is expected to be set by user space through > + * the generic MSR interface (with the help of the x86_energy_perf_policy > tool), > + * but there are two reasons for the kernel to touch it. So I hate /dev/msr with a passion. It is absolute atrocious crap. Please, just make a real interface for this if it is important.
[PATCH v3 09/18] locking/lockdep: Change the range of class_idx in held_lock struct
held_lock->class_idx is used to point to the class of the held lock. The
index is shifted by 1 to make index 0 mean no class, which results in class
index shifting back and forth but is not worth doing so.
The reason is: (1) there will be no "no-class" held_lock to begin with, and
(2) index 0 seems to be used for error checking, but if something wrong
indeed happended, the index can't be counted on to distinguish it as that
something won't set the class_idx to 0 on purpose to tell us it is wrong.
Therefore, change the index to start from 0. This saves a lot of
back-and-forth shifts and save a slot back to lock_classes.
Since index 0 is now used for lock class, we change the initial chain key to
-1 to avoid key collision, which is due to the fact that __jhash_mix(0, 0, 0) =
0.
Actually, the initial chain key can be any arbitrary value other than 0.
In addition, we maintain a bitmap to keep track of the used lock classes,
and we check the validity of the held lock against that bitmap.
Signed-off-by: Yuyang Du
---
include/linux/lockdep.h | 14 ++--
kernel/locking/lockdep.c | 59
2 files changed, 46 insertions(+), 27 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 64d4565..0859d80 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -218,13 +218,8 @@ struct lock_chain {
};
#define MAX_LOCKDEP_KEYS_BITS 13
-/*
- * Subtract one because we offset hlock->class_idx by 1 in order
- * to make 0 mean no class. This avoids overflowing the class_idx
- * bitfield and hitting the BUG in hlock_class().
- */
-#define MAX_LOCKDEP_KEYS ((1UL << MAX_LOCKDEP_KEYS_BITS) - 1)
-#define INITIAL_CHAIN_KEY 0
+#define MAX_LOCKDEP_KEYS (1UL << MAX_LOCKDEP_KEYS_BITS)
+#define INITIAL_CHAIN_KEY -1
struct held_lock {
/*
@@ -249,6 +244,11 @@ struct held_lock {
u64 waittime_stamp;
u64 holdtime_stamp;
#endif
+ /*
+* class_idx is zero-indexed; it points to the element in
+* lock_classes this held lock instance belongs to. class_idx is in
+* the range from 0 to (MAX_LOCKDEP_KEYS-1) inclusive.
+*/
unsigned intclass_idx:MAX_LOCKDEP_KEYS_BITS;
/*
* The lock-stack is unified in that the lock chains of interrupt
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index c6363f7..f16d2f5 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -150,17 +150,28 @@ static inline int debug_locks_off_graph_unlock(void)
static
#endif
struct lock_class lock_classes[MAX_LOCKDEP_KEYS];
+static DECLARE_BITMAP(lock_classes_in_use, MAX_LOCKDEP_KEYS);
static inline struct lock_class *hlock_class(struct held_lock *hlock)
{
- if (!hlock->class_idx) {
+ unsigned int class_idx = hlock->class_idx;
+
+ /* Don't re-read hlock->class_idx, can't use READ_ONCE() on bitfield */
+ barrier();
+
+ if (!test_bit(class_idx, lock_classes_in_use)) {
/*
* Someone passed in garbage, we give up.
*/
DEBUG_LOCKS_WARN_ON(1);
return NULL;
}
- return lock_classes + hlock->class_idx - 1;
+
+ /*
+* At this point, if the passed hlock->class_idx is still garbage,
+* we just have to live with it
+*/
+ return lock_classes + class_idx;
}
#ifdef CONFIG_LOCK_STAT
@@ -604,19 +615,22 @@ static void print_lock(struct held_lock *hlock)
/*
* We can be called locklessly through debug_show_all_locks() so be
* extra careful, the hlock might have been released and cleared.
+*
+* If this indeed happens, lets pretend it does not hurt to continue
+* to print the lock unless the hlock class_idx does not point to a
+* registered class. The rationale here is: since we don't attempt
+* to distinguish whether we are in this situation, if it just
+* happened we can't count on class_idx to tell either.
*/
- unsigned int class_idx = hlock->class_idx;
+ struct lock_class *lock = hlock_class(hlock);
- /* Don't re-read hlock->class_idx, can't use READ_ONCE() on bitfields:
*/
- barrier();
-
- if (!class_idx || (class_idx - 1) >= MAX_LOCKDEP_KEYS) {
+ if (!lock) {
printk(KERN_CONT "\n");
return;
}
printk(KERN_CONT "%p", hlock->instance);
- print_lock_name(lock_classes + class_idx - 1);
+ print_lock_name(lock);
printk(KERN_CONT ", at: %pS\n", (void *)hlock->acquire_ip);
}
@@ -871,7 +885,7 @@ static bool check_lock_chain_key(struct lock_chain *chain)
int i;
for (i = chain->base; i < chain->base + chain->depth; i++)
- chain_key = iterate_chain_key(chain_key,
[PATCH v3 10/18] locking/lockdep: Remove unused argument in validate_chain() and check_deadlock()
The lockdep_map argument in them is not used, remove it.
Signed-off-by: Yuyang Du
Reviewed-by: Bart Van Assche
---
kernel/locking/lockdep.c | 16
1 file changed, 8 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index f16d2f5..c7aec9f 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -2134,8 +2134,7 @@ static inline void inc_chains(void)
* Returns: 0 on deadlock detected, 1 on OK, 2 on recursive read
*/
static int
-check_deadlock(struct task_struct *curr, struct held_lock *next,
- struct lockdep_map *next_instance, int read)
+check_deadlock(struct task_struct *curr, struct held_lock *next, int read)
{
struct held_lock *prev;
struct held_lock *nest = NULL;
@@ -2698,8 +2697,9 @@ static inline int lookup_chain_cache_add(struct
task_struct *curr,
return 1;
}
-static int validate_chain(struct task_struct *curr, struct lockdep_map *lock,
- struct held_lock *hlock, int chain_head, u64 chain_key)
+static int validate_chain(struct task_struct *curr,
+ struct held_lock *hlock,
+ int chain_head, u64 chain_key)
{
/*
* Trylock needs to maintain the stack of held locks, but it
@@ -2725,7 +2725,7 @@ static int validate_chain(struct task_struct *curr,
struct lockdep_map *lock,
* any of these scenarios could lead to a deadlock. If
* All validations
*/
- int ret = check_deadlock(curr, hlock, lock, hlock->read);
+ int ret = check_deadlock(curr, hlock, hlock->read);
if (!ret)
return 0;
@@ -2756,8 +2756,8 @@ static int validate_chain(struct task_struct *curr,
struct lockdep_map *lock,
}
#else
static inline int validate_chain(struct task_struct *curr,
- struct lockdep_map *lock, struct held_lock *hlock,
- int chain_head, u64 chain_key)
+struct held_lock *hlock,
+int chain_head, u64 chain_key)
{
return 1;
}
@@ -3733,7 +3733,7 @@ static int __lock_acquire(struct lockdep_map *lock,
unsigned int subclass,
WARN_ON_ONCE(!hlock_class(hlock)->key);
}
- if (!validate_chain(curr, lock, hlock, chain_head, chain_key))
+ if (!validate_chain(curr, hlock, chain_head, chain_key))
return 0;
curr->curr_chain_key = chain_key;
--
1.8.3.1
[PATCH v3 14/18] locking/lockdep: Change the return type of __cq_dequeue()
With the change, we can slightly adjust the code to iterate the queue in BFS
search, which simplifies the code. No functional change.
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 21 +
1 file changed, 13 insertions(+), 8 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 8167d69..ad16793 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1317,14 +1317,21 @@ static inline int __cq_enqueue(struct circular_queue
*cq, struct lock_list *elem
return 0;
}
-static inline int __cq_dequeue(struct circular_queue *cq, struct lock_list
**elem)
+/*
+ * Dequeue an element from the circular_queue, return the lock if the queue
+ * is not empty, or NULL if otherwise
+ */
+static inline struct lock_list * __cq_dequeue(struct circular_queue *cq)
{
+ struct lock_list * lock;
+
if (__cq_empty(cq))
- return -1;
+ return NULL;
- *elem = cq->element[cq->front];
+ lock = cq->element[cq->front];
cq->front = (cq->front + 1) & CQ_MASK;
- return 0;
+
+ return lock;
}
static inline unsigned int __cq_get_elem_count(struct circular_queue *cq)
@@ -1376,6 +1383,7 @@ static int __bfs(struct lock_list *source_entry,
int forward)
{
struct lock_list *entry;
+ struct lock_list *lock;
struct list_head *head;
struct circular_queue *cq = _cq;
int ret = 1;
@@ -1397,10 +1405,7 @@ static int __bfs(struct lock_list *source_entry,
__cq_init(cq);
__cq_enqueue(cq, source_entry);
- while (!__cq_empty(cq)) {
- struct lock_list *lock;
-
- __cq_dequeue(cq, );
+ while ((lock = __cq_dequeue(cq))) {
if (!lock->class) {
ret = -2;
--
1.8.3.1
[PATCH v3 08/18] locking/lockdep: Define INITIAL_CHAIN_KEY for chain keys to start with
Chain keys are computed using Jenkins hash function, which needs an initial
hash to start with. Dedicate a macro to make this clear and configurable. A
later patch changes this initial chain key.
Signed-off-by: Yuyang Du
---
include/linux/lockdep.h | 1 +
init/init_task.c | 2 +-
kernel/locking/lockdep.c | 18 +-
3 files changed, 11 insertions(+), 10 deletions(-)
diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h
index 267087e..64d4565 100644
--- a/include/linux/lockdep.h
+++ b/include/linux/lockdep.h
@@ -224,6 +224,7 @@ struct lock_chain {
* bitfield and hitting the BUG in hlock_class().
*/
#define MAX_LOCKDEP_KEYS ((1UL << MAX_LOCKDEP_KEYS_BITS) - 1)
+#define INITIAL_CHAIN_KEY 0
struct held_lock {
/*
diff --git a/init/init_task.c b/init/init_task.c
index 9460878..ff3e8bf 100644
--- a/init/init_task.c
+++ b/init/init_task.c
@@ -166,7 +166,7 @@ struct task_struct init_task
#endif
#ifdef CONFIG_LOCKDEP
.lockdep_depth = 0, /* no locks held yet */
- .curr_chain_key = 0,
+ .curr_chain_key = INITIAL_CHAIN_KEY,
.lockdep_recursion = 0,
#endif
#ifdef CONFIG_FUNCTION_GRAPH_TRACER
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index ef59651..c6363f7 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -361,7 +361,7 @@ static inline u64 iterate_chain_key(u64 key, u32 idx)
inline void lockdep_init_task(struct task_struct *task)
{
task->lockdep_depth = 0; /* no locks held yet */
- task->curr_chain_key = 0;
+ task->curr_chain_key = INITIAL_CHAIN_KEY;
task->lockdep_recursion = 0;
}
@@ -867,7 +867,7 @@ static bool class_lock_list_valid(struct lock_class *c,
struct list_head *h)
static bool check_lock_chain_key(struct lock_chain *chain)
{
#ifdef CONFIG_PROVE_LOCKING
- u64 chain_key = 0;
+ u64 chain_key = INITIAL_CHAIN_KEY;
int i;
for (i = chain->base; i < chain->base + chain->depth; i++)
@@ -2433,7 +2433,7 @@ static u64 print_chain_key_iteration(int class_idx, u64
chain_key)
print_chain_keys_held_locks(struct task_struct *curr, struct held_lock
*hlock_next)
{
struct held_lock *hlock;
- u64 chain_key = 0;
+ u64 chain_key = INITIAL_CHAIN_KEY;
int depth = curr->lockdep_depth;
int i = get_first_held_lock(curr, hlock_next);
@@ -2453,7 +2453,7 @@ static u64 print_chain_key_iteration(int class_idx, u64
chain_key)
static void print_chain_keys_chain(struct lock_chain *chain)
{
int i;
- u64 chain_key = 0;
+ u64 chain_key = INITIAL_CHAIN_KEY;
int class_id;
printk("depth: %u\n", chain->depth);
@@ -2757,7 +2757,7 @@ static void check_chain_key(struct task_struct *curr)
#ifdef CONFIG_DEBUG_LOCKDEP
struct held_lock *hlock, *prev_hlock = NULL;
unsigned int i;
- u64 chain_key = 0;
+ u64 chain_key = INITIAL_CHAIN_KEY;
for (i = 0; i < curr->lockdep_depth; i++) {
hlock = curr->held_locks + i;
@@ -2781,7 +2781,7 @@ static void check_chain_key(struct task_struct *curr)
if (prev_hlock && (prev_hlock->irq_context !=
hlock->irq_context))
- chain_key = 0;
+ chain_key = INITIAL_CHAIN_KEY;
chain_key = iterate_chain_key(chain_key, hlock->class_idx);
prev_hlock = hlock;
}
@@ -3694,14 +3694,14 @@ static int __lock_acquire(struct lockdep_map *lock,
unsigned int subclass,
/*
* How can we have a chain hash when we ain't got no keys?!
*/
- if (DEBUG_LOCKS_WARN_ON(chain_key != 0))
+ if (DEBUG_LOCKS_WARN_ON(chain_key != INITIAL_CHAIN_KEY))
return 0;
chain_head = 1;
}
hlock->prev_chain_key = chain_key;
if (separate_irq_context(curr, hlock)) {
- chain_key = 0;
+ chain_key = INITIAL_CHAIN_KEY;
chain_head = 1;
}
chain_key = iterate_chain_key(chain_key, class_idx);
@@ -4543,7 +4543,7 @@ static void remove_class_from_lock_chain(struct
pending_free *pf,
return;
recalc:
- chain_key = 0;
+ chain_key = INITIAL_CHAIN_KEY;
for (i = chain->base; i < chain->base + chain->depth; i++)
chain_key = iterate_chain_key(chain_key, chain_hlocks[i] + 1);
if (chain->depth && chain->chain_key == chain_key)
--
1.8.3.1
[PATCH v3 03/18] locking/lockdep: Adjust lock usage bit character checks
The lock usage bit characters are defined and determined with tricks. Use a
macro and add some explanation to make it a bit clearer. Then adjust the
logic to check the usage, which optimizes the code a bit.
No functional change.
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 21 -
kernel/locking/lockdep_internals.h | 1 +
2 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 41137ad..db0473d 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -514,15 +514,26 @@ static inline unsigned long lock_flag(enum lock_usage_bit
bit)
static char get_usage_char(struct lock_class *class, enum lock_usage_bit bit)
{
+ /*
+* The usage character defaults to '.' (i.e., irqs disabled and not in
+* irq context), which is the safest usage category.
+*/
char c = '.';
- if (class->usage_mask & lock_flag(bit + 2))
+ /*
+* The order of the following usage checks matters, which will
+* result in the outcome character as follows:
+*
+* - '+': irq is enabled and not in irq context
+* - '-': in irq context and irq is disabled
+* - '?': in irq context and irq is enabled
+*/
+ if (class->usage_mask & lock_flag(bit + LOCK_USAGE_TO_ENABLED_STEP)) {
c = '+';
- if (class->usage_mask & lock_flag(bit)) {
- c = '-';
- if (class->usage_mask & lock_flag(bit + 2))
+ if (class->usage_mask & lock_flag(bit))
c = '?';
- }
+ } else if (class->usage_mask & lock_flag(bit))
+ c = '-';
return c;
}
diff --git a/kernel/locking/lockdep_internals.h
b/kernel/locking/lockdep_internals.h
index d4c1974..2fd31d5 100644
--- a/kernel/locking/lockdep_internals.h
+++ b/kernel/locking/lockdep_internals.h
@@ -25,6 +25,7 @@ enum lock_usage_bit {
#define LOCK_USAGE_READ_MASK 1
#define LOCK_USAGE_DIR_MASK 2
#define LOCK_USAGE_STATE_MASK (~(LOCK_USAGE_READ_MASK | LOCK_USAGE_DIR_MASK))
+#define LOCK_USAGE_TO_ENABLED_STEP 2
/*
* Usage-state bitmasks:
--
1.8.3.1
[PATCH v3 02/18] locking/lockdep: Add description and explanation in lockdep design doc
More words are added to lockdep design document regarding key concepts,
which helps people understand the design as well as read the reports.
Signed-off-by: Yuyang Du
---
Documentation/locking/lockdep-design.txt | 80
1 file changed, 60 insertions(+), 20 deletions(-)
diff --git a/Documentation/locking/lockdep-design.txt
b/Documentation/locking/lockdep-design.txt
index 49f58a0..1dcceaa 100644
--- a/Documentation/locking/lockdep-design.txt
+++ b/Documentation/locking/lockdep-design.txt
@@ -15,51 +15,91 @@ tens of thousands of) instantiations. For example a lock in
the inode
struct is one class, while each inode has its own instantiation of that
lock class.
-The validator tracks the 'state' of lock-classes, and it tracks
-dependencies between different lock-classes. The validator maintains a
-rolling proof that the state and the dependencies are correct.
-
-Unlike an lock instantiation, the lock-class itself never goes away: when
-a lock-class is used for the first time after bootup it gets registered,
-and all subsequent uses of that lock-class will be attached to this
-lock-class.
+The validator tracks the 'usage state' of lock-classes, and it tracks the
+dependencies between different lock-classes. The dependency can be
+understood as lock order, where L1 -> L2 suggests L1 depends on L2, which
+can also be expressed as a forward dependency (L1 -> L2) or a backward
+dependency (L2 <- L1). From lockdep's perspective, the two locks (L1 and L2)
+are not necessarily related as opposed to in some modules an order must be
+followed. Here it just means that order ever happened. The validator
+maintains a continuing effort to prove that the lock usages and their
+dependencies are correct or the validator will shoot a splat if they are
+potentially incorrect.
+
+Unlike a lock instance, a lock-class itself never goes away: when a
+lock-class's instance is used for the first time after bootup the class gets
+registered, and all (subsequent) instances of that lock-class will be mapped
+to the lock-class.
State
-
-The validator tracks lock-class usage history into 4 * nSTATEs + 1 separate
-state bits:
+The validator tracks lock-class usage history and divides the usage into
+(4 usages * n STATEs + 1) categories:
+Where the 4 usages can be:
- 'ever held in STATE context'
- 'ever held as readlock in STATE context'
- 'ever held with STATE enabled'
- 'ever held as readlock with STATE enabled'
-Where STATE can be either one of (kernel/locking/lockdep_states.h)
- - hardirq
- - softirq
+Where the n STATEs are coded in kernel/locking/lockdep_states.h and as of
+now they include:
+- hardirq
+- softirq
+Where the last 1 category is:
- 'ever used' [ == !unused]
-When locking rules are violated, these state bits are presented in the
-locking error messages, inside curlies. A contrived example:
+When locking rules are violated, these usage bits are presented in the
+locking error messages, inside curlies, with a total of 2 * n STATEs bits.
+See a contrived example:
modprobe/2287 is trying to acquire lock:
-(_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
+(_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
but task is already holding lock:
-(_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24
+(_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
-The bit position indicates STATE, STATE-read, for each of the states listed
-above, and the character displayed in each indicates:
+For a given lock, the bit positions from left to right indicate the usage
+of the lock and readlock (if exists), for each of the n STATEs listed
+above respectively, and the character displayed at each bit position
+indicates:
'.' acquired while irqs disabled and not in irq context
'-' acquired in irq context
'+' acquired with irqs enabled
'?' acquired in irq context with irqs enabled.
-Unused mutexes cannot be part of the cause of an error.
+The bits are illustrated with an example:
+
+(_locks[i].lock){-.-.}, at: [] mutex_lock+0x21/0x24
+
+ ||| \-> softirq disabled and not in softirq context
+ || \--> acquired in softirq context
+ | \---> hardirq disabled and not in hardirq context
+ \> acquired in hardirq context
+
+
+For a given STATE, whether the lock is ever acquired in that STATE context
+and whether that STATE is enabled yields four possible cases as shown in the
+table below. It is worth noting that the bit character is able to indicate
+which exact case is for the lock as of the reporting time.
+
+ ---
+ | | irq enabled | irq disabled |
+ ---
+ | ever in irq | ? | - |
+ ---
+ | never in
[PATCH v3 04/18] locking/lockdep: Remove useless conditional macro
Since #defined(CONFIG_PROVE_LOCKING) is used in the scope of #ifdef
CONFIG_PROVE_LOCKING, it can be removed.
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index db0473d..eeea722 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1678,7 +1678,7 @@ unsigned long lockdep_count_backward_deps(struct
lock_class *class)
return result;
}
-#if defined(CONFIG_TRACE_IRQFLAGS) && defined(CONFIG_PROVE_LOCKING)
+#if defined(CONFIG_TRACE_IRQFLAGS)
/*
* Forwards and backwards subgraph searching, for the purposes of
* proving that two subgraphs can be connected by a new dependency
@@ -2056,7 +2056,7 @@ static inline void inc_chains(void)
nr_process_chains++;
}
-#endif
+#endif /* CONFIG_TRACE_IRQFLAGS */
static void
print_deadlock_scenario(struct held_lock *nxt, struct held_lock *prv)
@@ -2738,7 +2738,7 @@ static inline int validate_chain(struct task_struct *curr,
{
return 1;
}
-#endif
+#endif /* CONFIG_PROVE_LOCKING */
/*
* We are building curr_chain_key incrementally, so double-check
--
1.8.3.1
[PATCH v3 00/18] locking/lockdep: Add comments and make some code
Hi Peter, I recently looked at some system hang issues. While at it, I tried to use and understand lockdep. These patches are made as a result. I believe they should have helped me, so hopefully they do for others as well. Many thanks to Bart, Joe, and Peter for their valuable comments. Change from v2: - Removed indent adjustments only patch - Removed unnecessary if to else-if patch - Made changes according to comments - Added another doc addition patch Change from v1: - Rebased the patch series. - Added more no-functional-change patches. - Removed zapped locks in lock chains printing patch, which was a band-aid. The real problem was recently fixed by Bart. Thanks, Yuyang -- Yuyang Du (18): locking/lockdep: Change all print_*() return type to void locking/lockdep: Add description and explanation in lockdep design doc locking/lockdep: Adjust lock usage bit character checks locking/lockdep: Remove useless conditional macro locking/lockdep: Print the right depth for chain key colission locking/lockdep: Update obsolete struct field description locking/lockdep: Use lockdep_init_task for task initiation consistently locking/lockdep: Define INITIAL_CHAIN_KEY for chain keys to start with locking/lockdep: Change the range of class_idx in held_lock struct locking/lockdep: Remove unused argument in validate_chain() and check_deadlock() locking/lockdep: Update comment locking/lockdep: Remove unnecessary function pointer argument locking/lockdep: Change type of the element field in circular_queue locking/lockdep: Change the return type of __cq_dequeue() locking/lockdep: Avoid constant checks in __bfs by using offset reference locking/lockdep: Combine check_noncircular and check_redundant locking/lockdep: Update comments on dependency search locking/lockdep: Add explanation to lock usage rules in lockdep design doc Documentation/locking/lockdep-design.txt | 115 ++-- include/linux/lockdep.h | 30 +- init/init_task.c | 2 + kernel/fork.c| 3 - kernel/locking/lockdep.c | 487 +-- kernel/locking/lockdep_internals.h | 1 + 6 files changed, 378 insertions(+), 260 deletions(-) -- 1.8.3.1
[PATCH v3 01/18] locking/lockdep: Change all print_*() return type to void
Since none of the print_*() function's return value is necessary, change
their return type to void. No functional change.
In cases where an invariable return value is used, this change slightly
improves readability, i.e.:
print_x();
return 0;
is definitely better than:
return print_x(); /* where print_x() always returns 0 */
Signed-off-by: Yuyang Du
---
kernel/locking/lockdep.c | 203 ---
1 file changed, 103 insertions(+), 100 deletions(-)
diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c
index 34cdcbe..41137ad 100644
--- a/kernel/locking/lockdep.c
+++ b/kernel/locking/lockdep.c
@@ -1430,17 +1430,15 @@ static inline int __bfs_backwards(struct lock_list
*src_entry,
* Print a dependency chain entry (this is only done when a deadlock
* has been detected):
*/
-static noinline int
+static noinline void
print_circular_bug_entry(struct lock_list *target, int depth)
{
if (debug_locks_silent)
- return 0;
+ return;
printk("\n-> #%u", depth);
print_lock_name(target->class);
printk(KERN_CONT ":\n");
print_stack_trace(>trace, 6);
-
- return 0;
}
static void
@@ -1497,7 +1495,7 @@ static inline int __bfs_backwards(struct lock_list
*src_entry,
* When a circular dependency is detected, print the
* header first:
*/
-static noinline int
+static noinline void
print_circular_bug_header(struct lock_list *entry, unsigned int depth,
struct held_lock *check_src,
struct held_lock *check_tgt)
@@ -1505,7 +1503,7 @@ static inline int __bfs_backwards(struct lock_list
*src_entry,
struct task_struct *curr = current;
if (debug_locks_silent)
- return 0;
+ return;
pr_warn("\n");
pr_warn("==\n");
@@ -1523,8 +1521,6 @@ static inline int __bfs_backwards(struct lock_list
*src_entry,
pr_warn("\nthe existing dependency chain (in reverse order) is:\n");
print_circular_bug_entry(entry, depth);
-
- return 0;
}
static inline int class_equal(struct lock_list *entry, void *data)
@@ -1532,11 +1528,11 @@ static inline int class_equal(struct lock_list *entry,
void *data)
return entry->class == data;
}
-static noinline int print_circular_bug(struct lock_list *this,
- struct lock_list *target,
- struct held_lock *check_src,
- struct held_lock *check_tgt,
- struct stack_trace *trace)
+static noinline void print_circular_bug(struct lock_list *this,
+ struct lock_list *target,
+ struct held_lock *check_src,
+ struct held_lock *check_tgt,
+ struct stack_trace *trace)
{
struct task_struct *curr = current;
struct lock_list *parent;
@@ -1544,10 +1540,10 @@ static noinline int print_circular_bug(struct lock_list
*this,
int depth;
if (!debug_locks_off_graph_unlock() || debug_locks_silent)
- return 0;
+ return;
if (!save_trace(>trace))
- return 0;
+ return;
depth = get_lock_depth(target);
@@ -1569,21 +1565,17 @@ static noinline int print_circular_bug(struct lock_list
*this,
printk("\nstack backtrace:\n");
dump_stack();
-
- return 0;
}
-static noinline int print_bfs_bug(int ret)
+static noinline void print_bfs_bug(int ret)
{
if (!debug_locks_off_graph_unlock())
- return 0;
+ return;
/*
* Breadth-first-search failed, graph got corrupted?
*/
WARN(1, "lockdep bfs error:%d\n", ret);
-
- return 0;
}
static int noop_count(struct lock_list *entry, void *data)
@@ -1766,7 +1758,7 @@ static void print_lock_class_header(struct lock_class
*class, int depth)
*/
static void __used
print_shortest_lock_dependencies(struct lock_list *leaf,
- struct lock_list *root)
+struct lock_list *root)
{
struct lock_list *entry = leaf;
int depth;
@@ -1788,8 +1780,6 @@ static void print_lock_class_header(struct lock_class
*class, int depth)
entry = get_lock_parent(entry);
depth--;
} while (entry && (depth >= 0));
-
- return;
}
static void
@@ -1848,7 +1838,7 @@ static void print_lock_class_header(struct lock_class
*class, int depth)
printk("\n *** DEADLOCK ***\n\n");
}
-static int
+static void
print_bad_irq_dependency(struct task_struct *curr,
struct lock_list *prev_root,
struct lock_list *next_root,
@@ -1861,7 +1851,7 @@ static
Re: [PATCH -next] ubifs: remove unused function __ubifs_shash_final
On 3/21/2019 1:48 AM, Richard Weinberger wrote:
Am Mittwoch, 20. März 2019, 21:05:37 CET schrieb Mukesh Ojha:
On 3/20/2019 7:39 PM, Yue Haibing wrote:
From: YueHaibing
There is no callers in tree, and can be removed.
Signed-off-by: YueHaibing
---
fs/ubifs/auth.c | 18 --
1 file changed, 18 deletions(-)
diff --git a/fs/ubifs/auth.c b/fs/ubifs/auth.c
index 5bf5fd0..2a40ccce 100644
--- a/fs/ubifs/auth.c
+++ b/fs/ubifs/auth.c
@@ -147,24 +147,6 @@ struct shash_desc *__ubifs_hash_get_desc(const struct
ubifs_info *c)
}
/**
- * __ubifs_shash_final - finalize shash
- * @c: UBIFS file-system description object
- * @desc: the descriptor
- * @out: the output hash
- *
- * Simple wrapper around crypto_shash_final(), safe to be called with
- * disabled authentication.
- */
-int __ubifs_shash_final(const struct ubifs_info *c, struct shash_desc *desc,
- u8 *out)
-{
- if (ubifs_authenticated(c))
- return crypto_shash_final(desc, out);
-
- return 0;
-}
-
-/**
* ubifs_bad_hash - Report hash mismatches
* @c: UBIFS file-system description object
* @node: the node
Looks fine to be removed.
Acked-by: Mukesh Ojha
I guess you mean Reviewed-by?
As i am unsure about future scope of this func. i.e why Acked.
-Mukesh
Thanks,
//richard
[LKP] [hugetlbfs] 2284cf59cb: BUG:KASAN:global-out-of-bounds_in_f
FYI, we noticed the following commit (built with gcc-5):
commit: 2284cf59cbcec2f17e50139e2db6d6d761521cd3 ("hugetlbfs: Convert to
fs_context")
https://git.kernel.org/cgit/linux/kernel/git/viro/vfs.git R48
in testcase: rcutorture
with following parameters:
runtime: 300s
test: cpuhotplug
torture_type: rcu
test-description: rcutorture is rcutorture kernel module load/unload test.
test-url: https://www.kernel.org/doc/Documentation/RCU/torture.txt
on test machine: qemu-system-x86_64 -enable-kvm -cpu SandyBridge -smp 2 -m 8G
caused below changes (please refer to attached dmesg/kmsg for entire
log/backtrace):
+-+++
| |
0ecab105a8 | 2284cf59cb |
+-+++
| boot_successes |
4 | 0 |
| boot_failures |
51 | 4 |
| kobject(#):tried_to_init_an_initialized_object,something_is_seriously_wrong |
42 ||
| BUG:KASAN:double-free_or_invalid-free_in_k |
43 ||
| BUG:KASAN:use-after-free_in_t |
13 ||
| BUG:KASAN:user-memory-access_in_s |
1 ||
| BUG:unable_to_handle_kernel |
17 ||
| Oops:#[##] |
17 ||
| RIP:string |
2 ||
| Kernel_panic-not_syncing:Fatal_exception|
18 ||
| BUG:KASAN:slab-out-of-bounds_in_t |
3 ||
| general_protection_fault:#[##] |
1 ||
| WARNING:at_kernel/locking/lockdep.c:#lock_downgrade |
2 ||
| RIP:lock_downgrade |
2 ||
| BUG:kernel_in_stage |
3 ||
| BUG:KASAN:user-memory-access_in_t |
19 ||
| RIP:ttm_mem_global_init[ttm]|
16 ||
| BUG:kernel_hang_in_boot-around-mounting-root_stage |
5 ||
| BUG:KASAN:global-out-of-bounds_in_f |
0 | 4 |
+-+++
[5.777052] BUG: KASAN: global-out-of-bounds in
fs_validate_description+0xeb/0x3c0
[5.778184] Read of size 8 at addr afade9b0 by task swapper/1
[5.778184]
[5.778184] CPU: 0 PID: 1 Comm: swapper Not tainted 5.0.0-rc2-00037-g2284cf5
#2
[5.778184] Call Trace:
[5.778184] print_address_description+0x1dd/0x290
[5.778184] ? fs_validate_description+0xeb/0x3c0
[5.778184] ? fs_validate_description+0xeb/0x3c0
[5.778184] kasan_report+0x134/0x1a4
[5.778184] ? f_dupfd+0xa0/0xf0
[5.778184] ? fs_validate_description+0xeb/0x3c0
[5.778184] fs_validate_description+0xeb/0x3c0
[5.778184] ? kmem_cache_create_usercopy+0xa2/0x2f0
[5.778184] register_filesystem+0x23/0xc0
[5.778184] init_hugetlbfs_fs+0xc3/0x286
[5.778184] ? init_ramfs_fs+0x7c/0x7c
[5.778184] do_one_initcall+0xb3/0x300
[5.778184] ? initcall_blacklisted+0x120/0x120
[5.778184] ? check_flags+0x1d0/0x270
[5.778184] ? __lock_is_held+0x37/0xd0
[5.778184] kernel_init_freeable+0x418/0x66c
[5.778184] ? rest_init+0x140/0x140
[5.778184] kernel_init+0xf/0x120
[5.778184] ? _raw_spin_unlock_irq+0x29/0x40
[5.778184] ? rest_init+0x140/0x140
[5.778184] ret_from_fork+0x24/0x30
[5.778184]
[5.778184] The buggy address belongs to the variable:
[5.778184] hugetlb_param_specs+0x70/0xc0
[5.778184]
[5.778184] Memory state around the buggy address:
[5.778184] afade880: 00 01 fa fa fa fa fa fa 05 fa fa fa fa fa fa
fa
[5.778184] afade900: 04 fa fa fa fa fa fa fa 00 00 00 00 00 00 00
00
[5.778184] >afade980: 00 00 00 00 00 00 fa fa fa fa fa fa 00 00 00
00
[5.778184] ^
[5.778184] afadea00: 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00
00
[5.778184] afadea80: 00 00 00 00 00 00 00 00 fa fa
Re: [PATCH v12 1/4] PM / Domains: Add a generic data pointer to the genpd_power_state struct
On 27/02/2019 20:58, Ulf Hansson wrote:
> Let's add a data pointer to the genpd_power_state struct, to allow a genpd
> backend driver to store per state specific data. To introduce the pointer,
> we need to change the way genpd deals with freeing of the corresponding
> allocated data.
>
> More precisely, let's clarify the responsibility of whom that shall free
> the data, by adding a ->free_states() callback to the struct
> generic_pm_domain. The one allocating the data shall assign the callback,
> to allow genpd to invoke it from genpd_remove().
>
> Cc: Lina Iyer
> Co-developed-by: Lina Iyer
> Signed-off-by: Ulf Hansson
> ---
>
> Changes in v12:
> - None.
>
> ---
> drivers/base/power/domain.c | 12 ++--
> include/linux/pm_domain.h | 4 +++-
> 2 files changed, 13 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c
> index 2c334c01fc43..03885c003c6a 100644
> --- a/drivers/base/power/domain.c
> +++ b/drivers/base/power/domain.c
> @@ -1685,6 +1685,12 @@ int pm_genpd_remove_subdomain(struct generic_pm_domain
> *genpd,
> }
> EXPORT_SYMBOL_GPL(pm_genpd_remove_subdomain);
>
> +static void genpd_free_default_power_state(struct genpd_power_state *states,
> +unsigned int state_count)
> +{
> + kfree(states);
> +}
> +
> static int genpd_set_default_power_state(struct generic_pm_domain *genpd)
> {
> struct genpd_power_state *state;
> @@ -1695,7 +1701,7 @@ static int genpd_set_default_power_state(struct
> generic_pm_domain *genpd)
>
> genpd->states = state;
> genpd->state_count = 1;
> - genpd->free = state;
> + genpd->free_states = genpd_free_default_power_state;
>
> return 0;
> }
> @@ -1811,7 +1817,9 @@ static int genpd_remove(struct generic_pm_domain *genpd)
> list_del(>gpd_list_node);
> genpd_unlock(genpd);
> cancel_work_sync(>power_off_work);
> - kfree(genpd->free);
> + if (genpd->free_states)
Is this test necessary as the free_states function is initialized with
the genpd_set_default_power_state() in any case?
> + genpd->free_states(genpd->states, genpd->state_count);
> +
> pr_debug("%s: removed %s\n", __func__, genpd->name);
>
> return 0;
> diff --git a/include/linux/pm_domain.h b/include/linux/pm_domain.h
> index 1ed5874bcee0..8e1399231753 100644
> --- a/include/linux/pm_domain.h
> +++ b/include/linux/pm_domain.h
> @@ -69,6 +69,7 @@ struct genpd_power_state {
> s64 residency_ns;
> struct fwnode_handle *fwnode;
> ktime_t idle_time;
> + void *data;
> };
>
> struct genpd_lock_ops;
> @@ -110,9 +111,10 @@ struct generic_pm_domain {
> struct device *dev);
> unsigned int flags; /* Bit field of configs for genpd */
> struct genpd_power_state *states;
> + void (*free_states)(struct genpd_power_state *states,
> + unsigned int state_count);
> unsigned int state_count; /* number of states */
> unsigned int state_idx; /* state that genpd will go to when off */
> - void *free; /* Free the state that was allocated for default */
> ktime_t on_time;
> ktime_t accounting_time;
> const struct genpd_lock_ops *lock_ops;
>
--
Facebook |
Twitter |
Re: [PATCH] of: Drop redundant check in linker section OF match table
On 3/21/2019 1:25 AM, Frank Rowand wrote:
On 3/20/19 3:49 AM, Mukesh Ojha wrote:
Existing check of `fn` against NULL inside OF match table
is redundant. Remove the check.
Signed-off-by: Mukesh Ojha
Cc: Rob Herring
Cc: Frank Rowand
Cc: Pantelis Antoniou
Cc: devicet...@vger.kernel.org
---
include/linux/of.h | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/include/linux/of.h b/include/linux/of.h
index e240992..b86c00a 100644
--- a/include/linux/of.h
+++ b/include/linux/of.h
@@ -1283,13 +1283,13 @@ static inline int of_get_available_child_count(const
struct device_node *np)
static const struct of_device_id __of_table_##name \
__used __section(__##table##_of_table) \
= { .compatible = compat, \
-.data = (fn == (fn_type)NULL) ? fn : fn }
+.data = fn }
#else
#define _OF_DECLARE(table, name, compat, fn, fn_type) \
static const struct of_device_id __of_table_##name \
__attribute__((unused)) \
= { .compatible = compat, \
-.data = (fn == (fn_type)NULL) ? fn : fn }
+.data = fn }
#endif
typedef int (*of_init_fn_2)(struct device_node *, struct device_node *);
The check is not redundant and does serve a purpose.
The purpose is not very obvious on the surface, but it is checking that
the function fn() is of the proper type.
Frank,
Thanks for taking out time to explain the stuff, Yeah some miscellaneous
driver can do this type of mistake.
-Mukesh
An example of a compiler warning with a bad function type is created
by applying the following patch:
drivers/of/unittest.c:62:1: warning: comparison of distinct pointer types
lacks a cast [enabled by default]
Note that you need to have CONFIG_UNITTEST enabled to compile unittest.c.
Line 62 is
OF_DECLARE_1(__unittest_of_table, unittest_setup_2_bad, "unittest_compat",
---
drivers/of/unittest.c | 17 +
1 file changed, 17 insertions(+)
Index: b/drivers/of/unittest.c
===
--- a/drivers/of/unittest.c
+++ b/drivers/of/unittest.c
@@ -45,6 +45,23 @@ static struct unittest_results {
failed; \
})
+struct of_device_id __unittest_of_table;
+
+static void __init unittest_setup_1_good(struct device_node *np)
+{
+}
+
+static void __init unittest_setup_2_bad(struct device_node *np_1,
+ struct device_node *np_2)
+{
+}
+
+OF_DECLARE_1(__unittest_of_table, unittest_setup_1_good, "unittest_compat",
+ unittest_setup_1_good);
+
+OF_DECLARE_1(__unittest_of_table, unittest_setup_2_bad, "unittest_compat",
+ unittest_setup_2_bad);
+
static void __init of_unittest_find_node_by_name(void)
{
struct device_node *np;
Re: [PATCH v2] staging: rtlwifi: Fix potential NULL pointer dereference of kzalloc
On Tue, Mar 19, 2019 at 03:15:08PM -0500, Aditya Pakki wrote: > phydm.internal is allocated using kzalloc which is used multiple > times without a check for NULL pointer. This patch avoids such a > scenario. > > -- > v1: Patch collision with different things, fix as per Greg > Signed-off-by: Aditya Pakki > --- Gar... Sorry, no, that's not quite right. It should be: blah blah blah... Signed-off-by: you --- V1: Patch collision with different things, fix as per Greg Try applying your patch with `cat raw_email.txt | git am` and you'll see the problem with the commit message. regards, dan carpenter
Re: [PATCH] staging: rtl8712: uninitialized memory in read_bbreg_hdl()
On Thu, Mar 21, 2019 at 09:26:38AM +0300, Dan Carpenter wrote:
> Colin King reported a bug in read_bbreg_hdl():
>
> memcpy(pcmd->rsp, (u8 *), pcmd->rspsz);
>
> The problem is that "val" is uninitialized.
>
> This code is obviously not useful, but so far as I can tell
> "pcmd->cmdcode" is never GEN_CMD_CODE(_Read_BBREG) so it's not harmful
> either. For now the easiest fix is to just call r8712_free_cmd_obj()
> and return.
>
> Fixes: 2865d42c78a9 ("staging: r8712u: Add the new driver to the mainline
> kernel")
> Reported-by: Colin Ian King
> Signed-off-by: Dan Carpenter
> ---
> drivers/staging/rtl8712/rtl8712_cmd.c | 10 +-
> drivers/staging/rtl8712/rtl8712_cmd.h | 2 +-
> 2 files changed, 2 insertions(+), 10 deletions(-)
>
> diff --git a/drivers/staging/rtl8712/rtl8712_cmd.h
> b/drivers/staging/rtl8712/rtl8712_cmd.h
> index 92fb77666d44..1ef86b8c592f 100644
> --- a/drivers/staging/rtl8712/rtl8712_cmd.h
> +++ b/drivers/staging/rtl8712/rtl8712_cmd.h
> @@ -140,7 +140,7 @@ enum rtl8712_h2c_cmd {
> static struct _cmd_callback cmd_callback[] = {
> {GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
> {GEN_CMD_CODE(_Write_MACREG), NULL},
> - {GEN_CMD_CODE(_Read_BBREG), _getbbrfreg_cmdrsp_callback},
> + {GEN_CMD_CODE(_Read_BBREG), NULL},
> {GEN_CMD_CODE(_Write_BBREG), NULL},
> {GEN_CMD_CODE(_Read_RFREG), _getbbrfreg_cmdrsp_callback},
^
> {GEN_CMD_CODE(_Write_RFREG), NULL}, /*5*/
The other place that calls r8712_getbbrfreg_cmdrsp_callback() is
read_rfreg_hdl(). For GEN_CMD_CODE(_Read_RFREG) we don't allocate
the ->rsp pointer so we can't kfree() it. The read_rfreg_hdl()
functions calls r8712_free_cmd_obj() which kfrees it. But fortunately
that is dead code.
This code is obviously staging code...
It would be fairly straight forward to get rid of the cmd_callback[]
array.
regards,
dan carpenter
Re: [PATCH 1/3] mm/sparse: Clean up the obsolete code comment
Hi all, On 03/20/19 at 05:58am, Matthew Wilcox wrote: > On Wed, Mar 20, 2019 at 02:36:58PM +0200, Mike Rapoport wrote: > > There are more than a thousand -EEXIST in the kernel, I really doubt all of > > them mean "File exists" ;-) > > And yet that's what the user will see if it's ever printed with perror() > or similar. We're pretty bad at choosing errnos; look how abused > ENOSPC is: When I tried to change -EEXIST to -EBUSY, seems the returned value will return back over the whole path. And -EEXIST is checked explicitly several times during the path. acpi_memory_enable_device -> __add_pages .. -> __add_section -> sparse_add_one_section Only look into hotplug path triggered by ACPI event, there are also device memory and ballon memory paths I haven't checked carefully because not familiar with them. So from the checking, I tend to agree with Oscar and Mike. There have been so many places to use '-EEXIST' to indicate that stuffs checked have been existing. We can't deny it's inconsistent with term explanation text. While the defense is that -EEXIST is more precise to indicate a static instance has been present when we want to create it, but -EBUSY is a little blizarre. I would rather see -EBUSY is used on a device. When want to stop it or destroy it, need check if it's busy or not. #define EBUSY 16 /* Device or resource busy */ #define EEXIST 17 /* File exists */ Obviously saying resource busy or not, it violates semanics in any language. So many people use EEXIST instead, isn't it the obsolete text's fault? Personal opinion. Thanks Baoquan > > $ errno ENOSPC > ENOSPC 28 No space left on device > > net/sunrpc/auth_gss/gss_rpc_xdr.c: return -ENOSPC; > > ... that's an authentication failure, not "I've run out of disc space".
Re: [PATCHv2] x86/boot/KASLR: skip the specified crashkernel reserved region
On Wed, Mar 13, 2019 at 12:19:31PM +0800, Pingfan Liu wrote:
I tested it in Qemu test with 12G memory, and set crashkernel=6G@6G.
Without this PATCH, it successed to reserve memory just 4 times(total
10 times).
With this PATCH, it successed to reserve memory 15 times(total 15
times).
So I think if you post new version, you can add:
Tested-by: Chao Fan
Thanks,
Chao Fan
>crashkernel=x@y option may fail to reserve the required memory region if
>KASLR puts kernel into the region. To avoid this uncertainty, making KASLR
>skip the required region.
>
>Signed-off-by: Pingfan Liu
>Cc: Thomas Gleixner
>Cc: Ingo Molnar
>Cc: Borislav Petkov
>Cc: "H. Peter Anvin"
>Cc: Baoquan He
>Cc: Will Deacon
>Cc: Nicolas Pitre
>Cc: Pingfan Liu
>Cc: Chao Fan
>Cc: "Kirill A. Shutemov"
>Cc: Ard Biesheuvel
>Cc: linux-kernel@vger.kernel.org
>---
>v1 -> v2: fix some trival format
>
> arch/x86/boot/compressed/kaslr.c | 26 --
> 1 file changed, 24 insertions(+), 2 deletions(-)
>
>diff --git a/arch/x86/boot/compressed/kaslr.c
>b/arch/x86/boot/compressed/kaslr.c
>index 9ed9709..e185318 100644
>--- a/arch/x86/boot/compressed/kaslr.c
>+++ b/arch/x86/boot/compressed/kaslr.c
>@@ -109,6 +109,7 @@ enum mem_avoid_index {
> MEM_AVOID_BOOTPARAMS,
> MEM_AVOID_MEMMAP_BEGIN,
> MEM_AVOID_MEMMAP_END = MEM_AVOID_MEMMAP_BEGIN + MAX_MEMMAP_REGIONS - 1,
>+ MEM_AVOID_CRASHKERNEL,
> MEM_AVOID_MAX,
> };
>
>@@ -240,6 +241,25 @@ static void parse_gb_huge_pages(char *param, char *val)
> }
> }
>
>+/* parse crashkernel=x@y option */
>+static void mem_avoid_crashkernel_simple(char *option)
>+{
>+ unsigned long long crash_size, crash_base;
>+ char *cur = option;
>+
>+ crash_size = memparse(option, );
>+ if (option == cur)
>+ return;
>+
>+ if (*cur == '@') {
>+ option = cur + 1;
>+ crash_base = memparse(option, );
>+ if (option == cur)
>+ return;
>+ mem_avoid[MEM_AVOID_CRASHKERNEL].start = crash_base;
>+ mem_avoid[MEM_AVOID_CRASHKERNEL].size = crash_size;
>+ }
>+}
>
> static void handle_mem_options(void)
> {
>@@ -250,7 +270,7 @@ static void handle_mem_options(void)
> u64 mem_size;
>
> if (!strstr(args, "memmap=") && !strstr(args, "mem=") &&
>- !strstr(args, "hugepages"))
>+ !strstr(args, "hugepages") && !strstr(args, "crashkernel="))
> return;
>
> tmp_cmdline = malloc(len + 1);
>@@ -286,6 +306,8 @@ static void handle_mem_options(void)
> goto out;
>
> mem_limit = mem_size;
>+ } else if (strstr(param, "crashkernel")) {
>+ mem_avoid_crashkernel_simple(val);
> }
> }
>
>@@ -414,7 +436,7 @@ static void mem_avoid_init(unsigned long input, unsigned
>long input_size,
>
> /* We don't need to set a mapping for setup_data. */
>
>- /* Mark the memmap regions we need to avoid */
>+ /* Mark the regions we need to avoid */
> handle_mem_options();
>
> #ifdef CONFIG_X86_VERBOSE_BOOTUP
>--
>2.7.4
>
>
>
lening aanvragen
-- Goedendag, Wij zijn Simple Federal Credit Union die leningen verstrekt per postadvertentie. Wij bieden verschillende soorten leningen of projecten leningen (korte en lange termijn leningen, persoonlijke leningen, leningen aan bedrijven, enz.) Met 3% rente. dus als u financiële problemen heeft en u de lening nodig hebt, neemt u dan contact met ons op als u geïnteresseerd bent. Onze e-mail: i...@simplefederalcreditunion.com Je volledige naam: Uw land- en thuisadres: Telefoonnummer: Hoeveelheid benodigde lening: Looptijd: We wachten op uw reactie met betrekking tot de gevulde mal, hoewel onze e-mails. Bedankt voor je antwoord. Eric
[PATCH] staging: rtl8712: uninitialized memory in read_bbreg_hdl()
Colin King reported a bug in read_bbreg_hdl():
memcpy(pcmd->rsp, (u8 *), pcmd->rspsz);
The problem is that "val" is uninitialized.
This code is obviously not useful, but so far as I can tell
"pcmd->cmdcode" is never GEN_CMD_CODE(_Read_BBREG) so it's not harmful
either. For now the easiest fix is to just call r8712_free_cmd_obj()
and return.
Fixes: 2865d42c78a9 ("staging: r8712u: Add the new driver to the mainline
kernel")
Reported-by: Colin Ian King
Signed-off-by: Dan Carpenter
---
drivers/staging/rtl8712/rtl8712_cmd.c | 10 +-
drivers/staging/rtl8712/rtl8712_cmd.h | 2 +-
2 files changed, 2 insertions(+), 10 deletions(-)
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.h
b/drivers/staging/rtl8712/rtl8712_cmd.h
index 92fb77666d44..1ef86b8c592f 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.h
+++ b/drivers/staging/rtl8712/rtl8712_cmd.h
@@ -140,7 +140,7 @@ enum rtl8712_h2c_cmd {
static struct _cmd_callbackcmd_callback[] = {
{GEN_CMD_CODE(_Read_MACREG), NULL}, /*0*/
{GEN_CMD_CODE(_Write_MACREG), NULL},
- {GEN_CMD_CODE(_Read_BBREG), _getbbrfreg_cmdrsp_callback},
+ {GEN_CMD_CODE(_Read_BBREG), NULL},
{GEN_CMD_CODE(_Write_BBREG), NULL},
{GEN_CMD_CODE(_Read_RFREG), _getbbrfreg_cmdrsp_callback},
{GEN_CMD_CODE(_Write_RFREG), NULL}, /*5*/
diff --git a/drivers/staging/rtl8712/rtl8712_cmd.c
b/drivers/staging/rtl8712/rtl8712_cmd.c
index 1920d02f7c9f..8c36acedf507 100644
--- a/drivers/staging/rtl8712/rtl8712_cmd.c
+++ b/drivers/staging/rtl8712/rtl8712_cmd.c
@@ -147,17 +147,9 @@ static u8 write_macreg_hdl(struct _adapter *padapter, u8
*pbuf)
static u8 read_bbreg_hdl(struct _adapter *padapter, u8 *pbuf)
{
- u32 val;
- void (*pcmd_callback)(struct _adapter *dev, struct cmd_obj *pcmd);
struct cmd_obj *pcmd = (struct cmd_obj *)pbuf;
- if (pcmd->rsp && pcmd->rspsz > 0)
- memcpy(pcmd->rsp, (u8 *), pcmd->rspsz);
- pcmd_callback = cmd_callback[pcmd->cmdcode].callback;
- if (!pcmd_callback)
- r8712_free_cmd_obj(pcmd);
- else
- pcmd_callback(padapter, pcmd);
+ r8712_free_cmd_obj(pcmd);
return H2C_SUCCESS;
}
--
2.17.1
Re: [PATCH v2] ALSA: hda/realtek: Enable headset MIC of Acer AIO with ALC286
On Thu, 21 Mar 2019 03:37:33 +0100,
Kailang wrote:
>
> Yes, you could use Reviewed-by.
OK, applied now. Thanks.
Takashi
>
> -Original Message-
> From: Jian-Hong Pan
> Sent: Wednesday, March 20, 2019 1:06 PM
> To: Takashi Iwai
> Cc: Kailang ; alsa-de...@alsa-project.org; Linux
> Upstreaming Team ; Linux Kernel
>
> Subject: Re: [PATCH v2] ALSA: hda/realtek: Enable headset MIC of Acer AIO
> with ALC286
>
> Takashi Iwai 於 2019年3月15日 週五 下午7:24寫道：
> >
> > On Fri, 15 Mar 2019 10:51:09 +0100,
> > Jian-Hong Pan wrote:
> > >
> > > Some Acer AIO desktops like Veriton Z6860G, Z4860G and Z4660G cannot
> > > record sound from headset MIC. This patch adds the
> > > ALC286_FIXUP_ACER_AIO_HEADSET_MIC quirk to fix this issue.
> > >
> > > Signed-off-by: Jian-Hong Pan
> > > ---
> > > v2: According to Realtek's suggestion, change the COEF 0x4f from 0xd429 to
> > > 0x5029. Thanks to Realtek!
> >
> > It'd be nicer if we get either Acked-by or Reviewed-by tag from
> > Realtek. Kailang?
>
> Hi Kailang,
>
> May we have your Acked-by or Reviewed-by tag for this patch?
> Signed-off will also be great!
>
> Thank you
> Jian-Hong Pan
>
> > thanks,
> >
> > Takashi
> >
> > >
> > > sound/pci/hda/patch_realtek.c | 17 ++---
> > > 1 file changed, 14 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/sound/pci/hda/patch_realtek.c
> > > b/sound/pci/hda/patch_realtek.c index 384719d5c44e..191830d4fa40
> > > 100644
> > > --- a/sound/pci/hda/patch_realtek.c
> > > +++ b/sound/pci/hda/patch_realtek.c
> > > @@ -5687,6 +5687,7 @@ enum {
> > > ALC225_FIXUP_DELL_WYSE_AIO_MIC_NO_PRESENCE,
> > > ALC225_FIXUP_WYSE_AUTO_MUTE,
> > > ALC225_FIXUP_WYSE_DISABLE_MIC_VREF,
> > > + ALC286_FIXUP_ACER_AIO_HEADSET_MIC,
> > > };
> > >
> > > static const struct hda_fixup alc269_fixups[] = { @@ -6685,6
> > > +6686,16 @@ static const struct hda_fixup alc269_fixups[] = {
> > > .chained = true,
> > > .chain_id = ALC269_FIXUP_HEADSET_MODE_NO_HP_MIC
> > > },
> > > + [ALC286_FIXUP_ACER_AIO_HEADSET_MIC] = {
> > > + .type = HDA_FIXUP_VERBS,
> > > + .v.verbs = (const struct hda_verb[]) {
> > > + { 0x20, AC_VERB_SET_COEF_INDEX, 0x4f },
> > > + { 0x20, AC_VERB_SET_PROC_COEF, 0x5029 },
> > > + { }
> > > + },
> > > + .chained = true,
> > > + .chain_id = ALC286_FIXUP_ACER_AIO_MIC_NO_PRESENCE
> > > + },
> > > };
> > >
> > > static const struct snd_pci_quirk alc269_fixup_tbl[] = { @@ -6701,9
> > > +6712,9 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
> > > SND_PCI_QUIRK(0x1025, 0x079b, "Acer Aspire V5-573G",
> > > ALC282_FIXUP_ASPIRE_V5_PINS),
> > > SND_PCI_QUIRK(0x1025, 0x102b, "Acer Aspire C24-860",
> > > ALC286_FIXUP_ACER_AIO_MIC_NO_PRESENCE),
> > > SND_PCI_QUIRK(0x1025, 0x106d, "Acer Cloudbook 14",
> > > ALC283_FIXUP_CHROME_BOOK),
> > > - SND_PCI_QUIRK(0x1025, 0x128f, "Acer Veriton Z6860G",
> > > ALC286_FIXUP_ACER_AIO_MIC_NO_PRESENCE),
> > > - SND_PCI_QUIRK(0x1025, 0x1290, "Acer Veriton Z4860G",
> > > ALC286_FIXUP_ACER_AIO_MIC_NO_PRESENCE),
> > > - SND_PCI_QUIRK(0x1025, 0x1291, "Acer Veriton Z4660G",
> > > ALC286_FIXUP_ACER_AIO_MIC_NO_PRESENCE),
> > > + SND_PCI_QUIRK(0x1025, 0x128f, "Acer Veriton Z6860G",
> > > ALC286_FIXUP_ACER_AIO_HEADSET_MIC),
> > > + SND_PCI_QUIRK(0x1025, 0x1290, "Acer Veriton Z4860G",
> > > ALC286_FIXUP_ACER_AIO_HEADSET_MIC),
> > > + SND_PCI_QUIRK(0x1025, 0x1291, "Acer Veriton Z4660G",
> > > + ALC286_FIXUP_ACER_AIO_HEADSET_MIC),
> > > SND_PCI_QUIRK(0x1025, 0x1330, "Acer TravelMate X514-51T",
> > > ALC255_FIXUP_ACER_HEADSET_MIC),
> > > SND_PCI_QUIRK(0x1028, 0x0470, "Dell M101z",
> > > ALC269_FIXUP_DELL_M101Z),
> > > SND_PCI_QUIRK(0x1028, 0x054b, "Dell XPS one 2710",
> > > ALC275_FIXUP_DELL_XPS),
> > > --
> > > 2.20.1
> > >
> > >
>
> --Please consider the environment before printing this e-mail.
Re: general protection fault in __x86_indirect_thunk_rbx
On Thu, Mar 21, 2019 at 7:08 AM NeilBrown wrote:
>
> On Wed, Mar 20 2019, syzbot wrote:
>
> > syzbot has bisected this bug to:
> >
> > commit dee160df820de41ff2f59a715643680822a0ab06
> > Author: NeilBrown
> > Date: Mon Nov 5 01:30:47 2018 +
> >
> > locks: use properly initialized file_lock when unlocking.
>
> This commit did not make it to mainline.
> It was replaced by
>
> Commit d6367d624137 ("fs/locks: use properly initialized file_lock when
> unlocking.")
>
> which added a call to locks_init_lock() in flock_make_lock().
>
> NeilBrown
Hi Neil,
You are talking to robot. Robot does not understand English prose ;)
Here is info on how to talk to it:
> See https://goo.gl/tpsmEJ for more information about syzbot.
In this case I think we need to say:
#syz fix: fs/locks: use properly initialized file_lock when unlocking.
> > bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13f39fd720
> > start commit: dee160df locks: use properly initialized file_lock when un..
> > git tree: linux-next
> > final crash:https://syzkaller.appspot.com/x/report.txt?x=100b9fd720
> > console output: https://syzkaller.appspot.com/x/log.txt?x=17f39fd720
> > kernel config: https://syzkaller.appspot.com/x/.config?x=caa433e1c8778437
> > dashboard link: https://syzkaller.appspot.com/bug?extid=83e12cf81d2c14842146
> > syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15bd944740
> > C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1121d82b40
> >
> > Reported-by: syzbot+83e12cf81d2c14842...@syzkaller.appspotmail.com
> > Fixes: dee160df ("locks: use properly initialized file_lock when
> > unlocking.")
>
> --
> You received this message because you are subscribed to the Google Groups
> "syzkaller-bugs" group.
> To unsubscribe from this group and stop receiving emails from it, send an
> email to syzkaller-bugs+unsubscr...@googlegroups.com.
> To view this discussion on the web visit
> https://groups.google.com/d/msgid/syzkaller-bugs/87lg187oao.fsf%40notabene.neil.brown.name.
> For more options, visit https://groups.google.com/d/optout.
Re: general protection fault in __x86_indirect_thunk_rbx
On Wed, Mar 20 2019, syzbot wrote:
> syzbot has bisected this bug to:
>
> commit dee160df820de41ff2f59a715643680822a0ab06
> Author: NeilBrown
> Date: Mon Nov 5 01:30:47 2018 +
>
> locks: use properly initialized file_lock when unlocking.
This commit did not make it to mainline.
It was replaced by
Commit d6367d624137 ("fs/locks: use properly initialized file_lock when
unlocking.")
which added a call to locks_init_lock() in flock_make_lock().
NeilBrown
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=13f39fd720
> start commit: dee160df locks: use properly initialized file_lock when un..
> git tree: linux-next
> final crash:https://syzkaller.appspot.com/x/report.txt?x=100b9fd720
> console output: https://syzkaller.appspot.com/x/log.txt?x=17f39fd720
> kernel config: https://syzkaller.appspot.com/x/.config?x=caa433e1c8778437
> dashboard link: https://syzkaller.appspot.com/bug?extid=83e12cf81d2c14842146
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=15bd944740
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1121d82b40
>
> Reported-by: syzbot+83e12cf81d2c14842...@syzkaller.appspotmail.com
> Fixes: dee160df ("locks: use properly initialized file_lock when
> unlocking.")
signature.asc
Description: PGP signature
Re: [PATCH v5 09/14] soc: mediatek: Add basic_clk_name to scp_power_data
On Tue, Mar 19, 2019 at 4:02 PM Weiyi Lu wrote:
>
> Try to stop extending the clk_id or clk_names if there are
> more and more new BASIC clocks. To get its own clocks by the
> basic_clk_name of each power domain.
>
> Signed-off-by: Weiyi Lu
> ---
> drivers/soc/mediatek/mtk-scpsys.c | 27 +++
> 1 file changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/soc/mediatek/mtk-scpsys.c
> b/drivers/soc/mediatek/mtk-scpsys.c
> index 6bf846cb1893..c6360de4e41e 100644
> --- a/drivers/soc/mediatek/mtk-scpsys.c
> +++ b/drivers/soc/mediatek/mtk-scpsys.c
> @@ -118,6 +118,8 @@ static const char * const clk_names[] = {
> * @bus_prot_mask: The mask for single step bus protection.
> * @clk_id: The basic clock needs to be enabled before enabling certain
> * power domains.
> + * @basic_clk_name: provide the same purpose with field "clk_id"
> + * by declaring basic clock prefix name rather than clk_id.
> * @caps: The flag for active wake-up action.
> */
> struct scp_domain_data {
> @@ -128,6 +130,7 @@ struct scp_domain_data {
> u32 sram_pdn_ack_bits;
> u32 bus_prot_mask;
> enum clk_id clk_id[MAX_CLKS];
> + const char *basic_clk_name[MAX_CLKS];
Either call them basic_clk_id/basic_clk_name, or clk_id/clk_name.
> u8 caps;
> };
>
> @@ -499,16 +502,24 @@ static struct scp *init_scp(struct platform_device
> *pdev,
>
> scpd->data = data;
>
> - for (j = 0; j < MAX_CLKS && data->clk_id[j]; j++) {
> - struct clk *c = clk[data->clk_id[j]];
> + if (data->clk_id[0]) {
Since it's either clk_id or basic_clk_name, I wonder if it'd be good
to add a WARN_ON here, e.g.
WARN_ON(data->basic_clk_name[0]);
> + for (j = 0; j < MAX_CLKS && data->clk_id[j]; j++) {
> + struct clk *c = clk[data->clk_id[j]];
>
> - if (IS_ERR(c)) {
> - dev_err(>dev, "%s: clk unavailable\n",
> - data->name);
> - return ERR_CAST(c);
> - }
> + if (IS_ERR(c)) {
> + dev_err(>dev,
> + "%s: clk unavailable\n",
> + data->name);
> + return ERR_CAST(c);
> + }
>
> - scpd->clk[j] = c;
> + scpd->clk[j] = c;
> + }
> + } else if (data->basic_clk_name[0]) {
> + for (j = 0; j < MAX_CLKS &&
> + data->basic_clk_name[j]; j++)
> + scpd->clk[j] = devm_clk_get(>dev,
> + data->basic_clk_name[j]);
> }
>
> genpd->name = data->name;
> --
> 2.18.0
>
Re: [PATCH] thunderbolt: Fix to check the return value of kmemdup
On Tue, Mar 19, 2019 at 01:38:34PM -0500, Aditya Pakki wrote: > uuid in add_switch is allocted via kmemdup which can fail. The patch > logs the error in such a scenario. > > Signed-off-by: Aditya Pakki > --- > drivers/thunderbolt/icm.c | 1 + > 1 file changed, 1 insertion(+) > > diff --git a/drivers/thunderbolt/icm.c b/drivers/thunderbolt/icm.c > index e3fc920af682..8df8057cd79e 100644 > --- a/drivers/thunderbolt/icm.c > +++ b/drivers/thunderbolt/icm.c > @@ -473,6 +473,7 @@ static void add_switch(struct tb_switch *parent_sw, u64 > route, > goto out; > > sw->uuid = kmemdup(uuid, sizeof(*uuid), GFP_KERNEL); > + WARN_ONCE(!sw->uuid, "%s: sw->uuid = NULL", __func__); I don't think it is good idea to continue here since uuid is kind of needed. Maybe do the same as above (goto out) with a useful error message along the lines of "cannot allocate memory for switch" or so. Remember to use tb_switch_put() in that case.
