Linux Documentation
 help / color / mirror / Atom feed
* [PATCH 6/8] trace_uprobe/sdt: Fix multiple update of same reference counter
From: Ravi Bangoria @ 2018-03-13 12:56 UTC (permalink / raw)
  To: mhiramat, oleg, peterz, srikar
  Cc: acme, ananth, akpm, alexander.shishkin, alexis.berlemont, corbet,
	dan.j.williams, gregkh, huawei.libin, hughd, jack, jglisse, jolsa,
	kan.liang, kirill.shutemov, kjlx, kstewart, linux-doc,
	linux-kernel, linux-mm, mhocko, milian.wolff, mingo, namhyung,
	naveen.n.rao, pc, pombredanne, rostedt, tglx, tmricht, willy,
	yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180313125603.19819-1-ravi.bangoria@linux.vnet.ibm.com>

For tiny binaries/libraries, different mmap regions points to the
same file portion. In such cases, we may increment reference counter
multiple times. But while de-registration, reference counter will get
decremented only by once leaving reference counter > 0 even if no one
is tracing on that marker.

Ensure increment and decrement happens in sync by keeping list of
mms in trace_uprobe. Increment reference counter only if mm is not
present in the list and decrement only if mm is present in the list.

Example

  # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4(0x10036)" > uprobe_events

Before patch:

  # perf stat -a -e sdt_tick:loop2
  # /tmp/tick
  # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
   0000000: 02                                       .

  # pkill perf
  # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
  0000000: 01                                       .

After patch:

  # perf stat -a -e sdt_tick:loop2
  # /tmp/tick
  # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
  0000000: 01                                       .

  # pkill perf
  # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
  0000000: 00                                       .

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 kernel/trace/trace_uprobe.c | 105 +++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 103 insertions(+), 2 deletions(-)

diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
index b6c9b48..9bf3f7a 100644
--- a/kernel/trace/trace_uprobe.c
+++ b/kernel/trace/trace_uprobe.c
@@ -50,6 +50,11 @@ struct trace_uprobe_filter {
 	struct list_head	perf_events;
 };
 
+struct sdt_mm_list {
+	struct mm_struct *mm;
+	struct sdt_mm_list *next;
+};
+
 /*
  * uprobe event core functions
  */
@@ -61,6 +66,8 @@ struct trace_uprobe {
 	char				*filename;
 	unsigned long			offset;
 	unsigned long			ref_ctr_offset;
+	struct sdt_mm_list		*sml;
+	struct rw_semaphore		sml_rw_sem;
 	unsigned long			nhit;
 	struct trace_probe		tp;
 };
@@ -274,6 +281,7 @@ static inline bool is_ret_probe(struct trace_uprobe *tu)
 	if (is_ret)
 		tu->consumer.ret_handler = uretprobe_dispatcher;
 	init_trace_uprobe_filter(&tu->filter);
+	init_rwsem(&tu->sml_rw_sem);
 	return tu;
 
 error:
@@ -921,6 +929,74 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
 	return trace_handle_return(s);
 }
 
+static bool sdt_check_mm_list(struct trace_uprobe *tu, struct mm_struct *mm)
+{
+	struct sdt_mm_list *tmp = tu->sml;
+
+	if (!tu->sml || !mm)
+		return false;
+
+	while (tmp) {
+		if (tmp->mm == mm)
+			return true;
+		tmp = tmp->next;
+	}
+
+	return false;
+}
+
+static void sdt_add_mm_list(struct trace_uprobe *tu, struct mm_struct *mm)
+{
+	struct sdt_mm_list *tmp;
+
+	tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
+	if (!tmp)
+		return;
+
+	tmp->mm = mm;
+	tmp->next = tu->sml;
+	tu->sml = tmp;
+}
+
+static void sdt_del_mm_list(struct trace_uprobe *tu, struct mm_struct *mm)
+{
+	struct sdt_mm_list *prev, *curr;
+
+	if (!tu->sml)
+		return;
+
+	if (tu->sml->mm == mm) {
+		curr = tu->sml;
+		tu->sml = tu->sml->next;
+		kfree(curr);
+		return;
+	}
+
+	prev = tu->sml;
+	curr = tu->sml->next;
+	while (curr) {
+		if (curr->mm == mm) {
+			prev->next = curr->next;
+			kfree(curr);
+			return;
+		}
+		prev = curr;
+		curr = curr->next;
+	}
+}
+
+static void sdt_flush_mm_list(struct trace_uprobe *tu)
+{
+	struct sdt_mm_list *next, *curr = tu->sml;
+
+	while (curr) {
+		next = curr->next;
+		kfree(curr);
+		curr = next;
+	}
+	tu->sml = NULL;
+}
+
 static bool sdt_valid_vma(struct trace_uprobe *tu, struct vm_area_struct *vma)
 {
 	unsigned long vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
@@ -989,17 +1065,25 @@ static void sdt_increment_ref_ctr(struct trace_uprobe *tu)
 	if (IS_ERR(info))
 		goto out;
 
+	down_write(&tu->sml_rw_sem);
 	while (info) {
+		if (sdt_check_mm_list(tu, info->mm))
+			goto cont;
+
 		down_write(&info->mm->mmap_sem);
 
 		vma = sdt_find_vma(info->mm, tu);
 		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
-		sdt_update_ref_ctr(info->mm, vaddr, 1);
+		if (!sdt_update_ref_ctr(info->mm, vaddr, 1))
+			sdt_add_mm_list(tu, info->mm);
 
 		up_write(&info->mm->mmap_sem);
+
+cont:
 		mmput(info->mm);
 		info = uprobe_free_map_info(info);
 	}
+	up_write(&tu->sml_rw_sem);
 
 out:
 	uprobe_end_dup_mmap();
@@ -1020,8 +1104,16 @@ void trace_uprobe_mmap_callback(struct vm_area_struct *vma)
 		    !trace_probe_is_enabled(&tu->tp))
 			continue;
 
+		down_write(&tu->sml_rw_sem);
+		if (sdt_check_mm_list(tu, vma->vm_mm))
+			goto cont;
+
 		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
-		sdt_update_ref_ctr(vma->vm_mm, vaddr, 1);
+		if (!sdt_update_ref_ctr(vma->vm_mm, vaddr, 1))
+			sdt_add_mm_list(tu, vma->vm_mm);
+
+cont:
+		up_write(&tu->sml_rw_sem);
 	}
 	mutex_unlock(&uprobe_lock);
 }
@@ -1038,7 +1130,11 @@ static void sdt_decrement_ref_ctr(struct trace_uprobe *tu)
 	if (IS_ERR(info))
 		goto out;
 
+	down_write(&tu->sml_rw_sem);
 	while (info) {
+		if (!sdt_check_mm_list(tu, info->mm))
+			goto cont;
+
 		down_write(&info->mm->mmap_sem);
 
 		vma = sdt_find_vma(info->mm, tu);
@@ -1046,9 +1142,14 @@ static void sdt_decrement_ref_ctr(struct trace_uprobe *tu)
 		sdt_update_ref_ctr(info->mm, vaddr, -1);
 
 		up_write(&info->mm->mmap_sem);
+		sdt_del_mm_list(tu, info->mm);
+
+cont:
 		mmput(info->mm);
 		info = uprobe_free_map_info(info);
 	}
+	sdt_flush_mm_list(tu);
+	up_write(&tu->sml_rw_sem);
 
 out:
 	uprobe_end_dup_mmap();
-- 
1.8.3.1

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

^ permalink raw reply related

* [PATCH 3/8] Uprobe: Rename map_info to uprobe_map_info
From: Ravi Bangoria @ 2018-03-13 12:55 UTC (permalink / raw)
  To: mhiramat, oleg, peterz, srikar
  Cc: acme, ananth, akpm, alexander.shishkin, alexis.berlemont, corbet,
	dan.j.williams, gregkh, huawei.libin, hughd, jack, jglisse, jolsa,
	kan.liang, kirill.shutemov, kjlx, kstewart, linux-doc,
	linux-kernel, linux-mm, mhocko, milian.wolff, mingo, namhyung,
	naveen.n.rao, pc, pombredanne, rostedt, tglx, tmricht, willy,
	yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180313125603.19819-1-ravi.bangoria@linux.vnet.ibm.com>

map_info is very generic name, rename it to uprobe_map_info.
Renaming will help to export this structure outside of the
file.

Also rename free_map_info() to uprobe_free_map_info() and
build_map_info() to uprobe_build_map_info().

No functionality changes.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 kernel/events/uprobes.c | 32 +++++++++++++++++---------------
 1 file changed, 17 insertions(+), 15 deletions(-)

diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index 535fd39..081b88c1 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -695,27 +695,29 @@ static void delete_uprobe(struct uprobe *uprobe)
 	put_uprobe(uprobe);
 }
 
-struct map_info {
-	struct map_info *next;
+struct uprobe_map_info {
+	struct uprobe_map_info *next;
 	struct mm_struct *mm;
 	unsigned long vaddr;
 };
 
-static inline struct map_info *free_map_info(struct map_info *info)
+static inline struct uprobe_map_info *
+uprobe_free_map_info(struct uprobe_map_info *info)
 {
-	struct map_info *next = info->next;
+	struct uprobe_map_info *next = info->next;
 	kfree(info);
 	return next;
 }
 
-static struct map_info *
-build_map_info(struct address_space *mapping, loff_t offset, bool is_register)
+static struct uprobe_map_info *
+uprobe_build_map_info(struct address_space *mapping, loff_t offset,
+		      bool is_register)
 {
 	unsigned long pgoff = offset >> PAGE_SHIFT;
 	struct vm_area_struct *vma;
-	struct map_info *curr = NULL;
-	struct map_info *prev = NULL;
-	struct map_info *info;
+	struct uprobe_map_info *curr = NULL;
+	struct uprobe_map_info *prev = NULL;
+	struct uprobe_map_info *info;
 	int more = 0;
 
  again:
@@ -729,7 +731,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
 			 * Needs GFP_NOWAIT to avoid i_mmap_rwsem recursion through
 			 * reclaim. This is optimistic, no harm done if it fails.
 			 */
-			prev = kmalloc(sizeof(struct map_info),
+			prev = kmalloc(sizeof(struct uprobe_map_info),
 					GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_NOWARN);
 			if (prev)
 				prev->next = NULL;
@@ -762,7 +764,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
 	}
 
 	do {
-		info = kmalloc(sizeof(struct map_info), GFP_KERNEL);
+		info = kmalloc(sizeof(struct uprobe_map_info), GFP_KERNEL);
 		if (!info) {
 			curr = ERR_PTR(-ENOMEM);
 			goto out;
@@ -774,7 +776,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
 	goto again;
  out:
 	while (prev)
-		prev = free_map_info(prev);
+		prev = uprobe_free_map_info(prev);
 	return curr;
 }
 
@@ -782,11 +784,11 @@ static inline struct map_info *free_map_info(struct map_info *info)
 register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new)
 {
 	bool is_register = !!new;
-	struct map_info *info;
+	struct uprobe_map_info *info;
 	int err = 0;
 
 	percpu_down_write(&dup_mmap_sem);
-	info = build_map_info(uprobe->inode->i_mapping,
+	info = uprobe_build_map_info(uprobe->inode->i_mapping,
 					uprobe->offset, is_register);
 	if (IS_ERR(info)) {
 		err = PTR_ERR(info);
@@ -825,7 +827,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
 		up_write(&mm->mmap_sem);
  free:
 		mmput(mm);
-		info = free_map_info(info);
+		info = uprobe_free_map_info(info);
 	}
  out:
 	percpu_up_write(&dup_mmap_sem);
-- 
1.8.3.1

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

^ permalink raw reply related

* [PATCH 2/8] mm: Prefix vma_ to vaddr_to_offset() and offset_to_vaddr()
From: Ravi Bangoria @ 2018-03-13 12:55 UTC (permalink / raw)
  To: mhiramat, oleg, peterz, srikar
  Cc: acme, ananth, akpm, alexander.shishkin, alexis.berlemont, corbet,
	dan.j.williams, gregkh, huawei.libin, hughd, jack, jglisse, jolsa,
	kan.liang, kirill.shutemov, kjlx, kstewart, linux-doc,
	linux-kernel, linux-mm, mhocko, milian.wolff, mingo, namhyung,
	naveen.n.rao, pc, pombredanne, rostedt, tglx, tmricht, willy,
	yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180313125603.19819-1-ravi.bangoria@linux.vnet.ibm.com>

No functionality changes.

Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
---
 include/linux/mm.h      |  4 ++--
 kernel/events/uprobes.c | 14 +++++++-------
 2 files changed, 9 insertions(+), 9 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 95909f2..d7ee526 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2275,13 +2275,13 @@ struct vm_unmapped_area_info {
 }
 
 static inline unsigned long
-offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
+vma_offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
 {
 	return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
 }
 
 static inline loff_t
-vaddr_to_offset(struct vm_area_struct *vma, unsigned long vaddr)
+vma_vaddr_to_offset(struct vm_area_struct *vma, unsigned long vaddr)
 {
 	return ((loff_t)vma->vm_pgoff << PAGE_SHIFT) + (vaddr - vma->vm_start);
 }
diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
index bd6f230..535fd39 100644
--- a/kernel/events/uprobes.c
+++ b/kernel/events/uprobes.c
@@ -748,7 +748,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
 		curr = info;
 
 		info->mm = vma->vm_mm;
-		info->vaddr = offset_to_vaddr(vma, offset);
+		info->vaddr = vma_offset_to_vaddr(vma, offset);
 	}
 	i_mmap_unlock_read(mapping);
 
@@ -807,7 +807,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
 			goto unlock;
 
 		if (vma->vm_start > info->vaddr ||
-		    vaddr_to_offset(vma, info->vaddr) != uprobe->offset)
+		    vma_vaddr_to_offset(vma, info->vaddr) != uprobe->offset)
 			goto unlock;
 
 		if (is_register) {
@@ -977,7 +977,7 @@ static int unapply_uprobe(struct uprobe *uprobe, struct mm_struct *mm)
 		    uprobe->offset >= offset + vma->vm_end - vma->vm_start)
 			continue;
 
-		vaddr = offset_to_vaddr(vma, uprobe->offset);
+		vaddr = vma_offset_to_vaddr(vma, uprobe->offset);
 		err |= remove_breakpoint(uprobe, mm, vaddr);
 	}
 	up_read(&mm->mmap_sem);
@@ -1023,7 +1023,7 @@ static void build_probe_list(struct inode *inode,
 	struct uprobe *u;
 
 	INIT_LIST_HEAD(head);
-	min = vaddr_to_offset(vma, start);
+	min = vma_vaddr_to_offset(vma, start);
 	max = min + (end - start) - 1;
 
 	spin_lock(&uprobes_treelock);
@@ -1076,7 +1076,7 @@ int uprobe_mmap(struct vm_area_struct *vma)
 	list_for_each_entry_safe(uprobe, u, &tmp_list, pending_list) {
 		if (!fatal_signal_pending(current) &&
 		    filter_chain(uprobe, UPROBE_FILTER_MMAP, vma->vm_mm)) {
-			unsigned long vaddr = offset_to_vaddr(vma, uprobe->offset);
+			unsigned long vaddr = vma_offset_to_vaddr(vma, uprobe->offset);
 			install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
 		}
 		put_uprobe(uprobe);
@@ -1095,7 +1095,7 @@ int uprobe_mmap(struct vm_area_struct *vma)
 
 	inode = file_inode(vma->vm_file);
 
-	min = vaddr_to_offset(vma, start);
+	min = vma_vaddr_to_offset(vma, start);
 	max = min + (end - start) - 1;
 
 	spin_lock(&uprobes_treelock);
@@ -1730,7 +1730,7 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
 	if (vma && vma->vm_start <= bp_vaddr) {
 		if (valid_vma(vma, false)) {
 			struct inode *inode = file_inode(vma->vm_file);
-			loff_t offset = vaddr_to_offset(vma, bp_vaddr);
+			loff_t offset = vma_vaddr_to_offset(vma, bp_vaddr);
 
 			uprobe = find_uprobe(inode, offset);
 		}
-- 
1.8.3.1

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

^ permalink raw reply related

* Re: [PATCH v2 7/8] [PATCH 7/8] drivers/hwmon: Add a generic PECI hwmon client driver
From: Jae Hyun Yoo @ 2018-03-13 18:56 UTC (permalink / raw)
  To: Stef van Os, joel, andrew, arnd, gregkh, jdelvare, linux, benh,
	andrew
  Cc: linux-hwmon, devicetree, linux-doc, openbmc, linux-kernel,
	linux-arm-kernel
In-Reply-To: <d9c3e768-ed57-a233-04fd-a05a64803da3@prodrive-technologies.com>

Hi Stef,

Thanks for sharing your time to test it.

That is expected result in v2. Previously in v1, it used delayed 
creation on core temperature group so it was okay if hwmon driver is 
registered when client CPU is powered down, but in v2, the driver should 
check resolved cores at probing time to prevent the delayed creation on 
core temperature group and indexing gap which breaks hwmon subsystem's 
common rule, so I added peci_detect() into peci_new_device() in PECI 
core driver to check online status of the client CPU when registering a 
new device.

You may need to use dynamic dtoverlay for loading/unloading a PECI hwmon 
driver according to the current client CPU power state. It means a PECI 
hwmon driver can be registered only when the client CPU is powered on. 
This design will be kept in v3 as well.

Thanks,
Jae

On 3/13/2018 2:32 AM, Stef van Os wrote:
> Hi Jae,
> 
> I tried version 1 and 2 of your PECI patch on our (AST2500 / Xeon E5 v4) 
> system. The V1 patchset works as expected (reading back temperature 0 
> until PECI is up), but the hwmon driver probe fails with version 2. It 
> communicates with the Xeon and assumes during kernel boot of the Aspeed 
> that PECI to the Xeon's is already up and running, but our system 
> enables the main Xeon supplies from AST2500 userspace.
> 
> If I load the hwmon driver as a module to load later on, the driver does 
> not call probe like e.g. a I2C driver on the I2C bus does. Am I using V2 
> wrongly?
> 
> BR,
> Stef
> 
> On 02/21/2018 05:16 PM, Jae Hyun Yoo wrote:
>> This commit adds a generic PECI hwmon client driver implementation.
>>
>> Signed-off-by: Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>
>> ---
>>   drivers/hwmon/Kconfig      |  10 +
>>   drivers/hwmon/Makefile     |   1 +
>>   drivers/hwmon/peci-hwmon.c | 928 
>> +++++++++++++++++++++++++++++++++++++++++++++
>>   3 files changed, 939 insertions(+)
>>   create mode 100644 drivers/hwmon/peci-hwmon.c
>>
>> diff --git a/drivers/hwmon/Kconfig b/drivers/hwmon/Kconfig
>> index ef23553ff5cb..f22e0c31f597 100644
>> --- a/drivers/hwmon/Kconfig
>> +++ b/drivers/hwmon/Kconfig
>> @@ -1246,6 +1246,16 @@ config SENSORS_NCT7904
>>         This driver can also be built as a module.  If so, the module
>>         will be called nct7904.
>> +config SENSORS_PECI_HWMON
>> +    tristate "PECI hwmon support"
>> +    depends on PECI
>> +    help
>> +      If you say yes here you get support for the generic PECI hwmon
>> +      driver.
>> +
>> +      This driver can also be built as a module.  If so, the module
>> +      will be called peci-hwmon.
>> +
>>   config SENSORS_NSA320
>>       tristate "ZyXEL NSA320 and compatible fan speed and temperature 
>> sensors"
>>       depends on GPIOLIB && OF
>> diff --git a/drivers/hwmon/Makefile b/drivers/hwmon/Makefile
>> index f814b4ace138..946f54b168e5 100644
>> --- a/drivers/hwmon/Makefile
>> +++ b/drivers/hwmon/Makefile
>> @@ -135,6 +135,7 @@ obj-$(CONFIG_SENSORS_NCT7802)    += nct7802.o
>>   obj-$(CONFIG_SENSORS_NCT7904)    += nct7904.o
>>   obj-$(CONFIG_SENSORS_NSA320)    += nsa320-hwmon.o
>>   obj-$(CONFIG_SENSORS_NTC_THERMISTOR)    += ntc_thermistor.o
>> +obj-$(CONFIG_SENSORS_PECI_HWMON)    += peci-hwmon.o
>>   obj-$(CONFIG_SENSORS_PC87360)    += pc87360.o
>>   obj-$(CONFIG_SENSORS_PC87427)    += pc87427.o
>>   obj-$(CONFIG_SENSORS_PCF8591)    += pcf8591.o
>> diff --git a/drivers/hwmon/peci-hwmon.c b/drivers/hwmon/peci-hwmon.c
>> new file mode 100644
>> index 000000000000..edd27744adcb
>> --- /dev/null
>> +++ b/drivers/hwmon/peci-hwmon.c
>> @@ -0,0 +1,928 @@
>> +// SPDX-License-Identifier: GPL-2.0
>> +// Copyright (c) 2018 Intel Corporation
>> +
>> +#include <linux/delay.h>
>> +#include <linux/hwmon.h>
>> +#include <linux/hwmon-sysfs.h>
>> +#include <linux/jiffies.h>
>> +#include <linux/module.h>
>> +#include <linux/of_device.h>
>> +#include <linux/peci.h>
>> +#include <linux/workqueue.h>
>> +
>> +#define DIMM_SLOT_NUMS_MAX    12  /* Max DIMM numbers (channel ranks 
>> x 2) */
>> +#define CORE_NUMS_MAX         28  /* Max core numbers (max on SKX 
>> Platinum) */
>> +#define TEMP_TYPE_PECI        6   /* Sensor type 6: Intel PECI */
>> +
>> +#define CORE_TEMP_ATTRS       5
>> +#define DIMM_TEMP_ATTRS       2
>> +#define ATTR_NAME_LEN         24
>> +
>> +#define DEFAULT_ATTR_GRP_NUMS 5
>> +
>> +#define UPDATE_INTERVAL_MIN   HZ
>> +#define DIMM_MASK_CHECK_DELAY msecs_to_jiffies(5000)
>> +
>> +enum sign {
>> +    POS,
>> +    NEG
>> +};
>> +
>> +struct temp_data {
>> +    bool valid;
>> +    s32  value;
>> +    unsigned long last_updated;
>> +};
>> +
>> +struct temp_group {
>> +    struct temp_data tjmax;
>> +    struct temp_data tcontrol;
>> +    struct temp_data tthrottle;
>> +    struct temp_data dts_margin;
>> +    struct temp_data die;
>> +    struct temp_data core[CORE_NUMS_MAX];
>> +    struct temp_data dimm[DIMM_SLOT_NUMS_MAX];
>> +};
>> +
>> +struct core_temp_group {
>> +    struct sensor_device_attribute sd_attrs[CORE_TEMP_ATTRS];
>> +    char attr_name[CORE_TEMP_ATTRS][ATTR_NAME_LEN];
>> +    struct attribute *attrs[CORE_TEMP_ATTRS + 1];
>> +    struct attribute_group attr_group;
>> +};
>> +
>> +struct dimm_temp_group {
>> +    struct sensor_device_attribute sd_attrs[DIMM_TEMP_ATTRS];
>> +    char attr_name[DIMM_TEMP_ATTRS][ATTR_NAME_LEN];
>> +    struct attribute *attrs[DIMM_TEMP_ATTRS + 1];
>> +    struct attribute_group attr_group;
>> +};
>> +
>> +struct peci_hwmon {
>> +    struct peci_client *client;
>> +    struct device *dev;
>> +    struct device *hwmon_dev;
>> +    struct workqueue_struct *work_queue;
>> +    struct delayed_work work_handler;
>> +    char name[PECI_NAME_SIZE];
>> +    struct temp_group temp;
>> +    u8 addr;
>> +    uint cpu_no;
>> +    u32 core_mask;
>> +    u32 dimm_mask;
>> +    const struct attribute_group *core_attr_groups[CORE_NUMS_MAX + 1];
>> +    const struct attribute_group *dimm_attr_groups[DIMM_SLOT_NUMS_MAX 
>> + 1];
>> +    uint global_idx;
>> +    uint core_idx;
>> +    uint dimm_idx;
>> +};
>> +
>> +enum label {
>> +    L_DIE,
>> +    L_DTS,
>> +    L_TCONTROL,
>> +    L_TTHROTTLE,
>> +    L_TJMAX,
>> +    L_MAX
>> +};
>> +
>> +static const char *peci_label[L_MAX] = {
>> +    "Die\n",
>> +    "DTS margin to Tcontrol\n",
>> +    "Tcontrol\n",
>> +    "Tthrottle\n",
>> +    "Tjmax\n",
>> +};
>> +
>> +static int send_peci_cmd(struct peci_hwmon *priv, enum peci_cmd cmd, 
>> void *msg)
>> +{
>> +    return peci_command(priv->client->adapter, cmd, msg);
>> +}
>> +
>> +static int need_update(struct temp_data *temp)
>> +{
>> +    if (temp->valid &&
>> +        time_before(jiffies, temp->last_updated + UPDATE_INTERVAL_MIN))
>> +        return 0;
>> +
>> +    return 1;
>> +}
>> +
>> +static s32 ten_dot_six_to_millidegree(s32 x)
>> +{
>> +    return ((((x) ^ 0x8000) - 0x8000) * 1000 / 64);
>> +}
>> +
>> +static int get_tjmax(struct peci_hwmon *priv)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    int rc;
>> +
>> +    if (!priv->temp.tjmax.valid) {
>> +        msg.addr = priv->addr;
>> +        msg.index = MBX_INDEX_TEMP_TARGET;
>> +        msg.param = 0;
>> +        msg.rx_len = 4;
>> +
>> +        rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +        if (rc < 0)
>> +            return rc;
>> +
>> +        priv->temp.tjmax.value = (s32)msg.pkg_config[2] * 1000;
>> +        priv->temp.tjmax.valid = true;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int get_tcontrol(struct peci_hwmon *priv)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    s32 tcontrol_margin;
>> +    int rc;
>> +
>> +    if (!need_update(&priv->temp.tcontrol))
>> +        return 0;
>> +
>> +    rc = get_tjmax(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    msg.addr = priv->addr;
>> +    msg.index = MBX_INDEX_TEMP_TARGET;
>> +    msg.param = 0;
>> +    msg.rx_len = 4;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    tcontrol_margin = msg.pkg_config[1];
>> +    tcontrol_margin = ((tcontrol_margin ^ 0x80) - 0x80) * 1000;
>> +
>> +    priv->temp.tcontrol.value = priv->temp.tjmax.value - 
>> tcontrol_margin;
>> +
>> +    if (!priv->temp.tcontrol.valid) {
>> +        priv->temp.tcontrol.last_updated = INITIAL_JIFFIES;
>> +        priv->temp.tcontrol.valid = true;
>> +    } else {
>> +        priv->temp.tcontrol.last_updated = jiffies;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int get_tthrottle(struct peci_hwmon *priv)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    s32 tthrottle_offset;
>> +    int rc;
>> +
>> +    if (!need_update(&priv->temp.tthrottle))
>> +        return 0;
>> +
>> +    rc = get_tjmax(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    msg.addr = priv->addr;
>> +    msg.index = MBX_INDEX_TEMP_TARGET;
>> +    msg.param = 0;
>> +    msg.rx_len = 4;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    tthrottle_offset = (msg.pkg_config[3] & 0x2f) * 1000;
>> +    priv->temp.tthrottle.value = priv->temp.tjmax.value - 
>> tthrottle_offset;
>> +
>> +    if (!priv->temp.tthrottle.valid) {
>> +        priv->temp.tthrottle.last_updated = INITIAL_JIFFIES;
>> +        priv->temp.tthrottle.valid = true;
>> +    } else {
>> +        priv->temp.tthrottle.last_updated = jiffies;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int get_die_temp(struct peci_hwmon *priv)
>> +{
>> +    struct peci_get_temp_msg msg;
>> +    int rc;
>> +
>> +    if (!need_update(&priv->temp.die))
>> +        return 0;
>> +
>> +    rc = get_tjmax(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    msg.addr = priv->addr;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_GET_TEMP, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    priv->temp.die.value = priv->temp.tjmax.value +
>> +                   ((s32)msg.temp_raw * 1000 / 64);
>> +
>> +    if (!priv->temp.die.valid) {
>> +        priv->temp.die.last_updated = INITIAL_JIFFIES;
>> +        priv->temp.die.valid = true;
>> +    } else {
>> +        priv->temp.die.last_updated = jiffies;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int get_dts_margin(struct peci_hwmon *priv)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    s32 dts_margin;
>> +    int rc;
>> +
>> +    if (!need_update(&priv->temp.dts_margin))
>> +        return 0;
>> +
>> +    msg.addr = priv->addr;
>> +    msg.index = MBX_INDEX_DTS_MARGIN;
>> +    msg.param = 0;
>> +    msg.rx_len = 4;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    dts_margin = (msg.pkg_config[1] << 8) | msg.pkg_config[0];
>> +
>> +    /**
>> +     * Processors return a value of DTS reading in 10.6 format
>> +     * (10 bits signed decimal, 6 bits fractional).
>> +     * Error codes:
>> +     *   0x8000: General sensor error
>> +     *   0x8001: Reserved
>> +     *   0x8002: Underflow on reading value
>> +     *   0x8003-0x81ff: Reserved
>> +     */
>> +    if (dts_margin >= 0x8000 && dts_margin <= 0x81ff)
>> +        return -1;
>> +
>> +    dts_margin = ten_dot_six_to_millidegree(dts_margin);
>> +
>> +    priv->temp.dts_margin.value = dts_margin;
>> +
>> +    if (!priv->temp.dts_margin.valid) {
>> +        priv->temp.dts_margin.last_updated = INITIAL_JIFFIES;
>> +        priv->temp.dts_margin.valid = true;
>> +    } else {
>> +        priv->temp.dts_margin.last_updated = jiffies;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int get_core_temp(struct peci_hwmon *priv, int core_index)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    s32 core_dts_margin;
>> +    int rc;
>> +
>> +    if (!need_update(&priv->temp.core[core_index]))
>> +        return 0;
>> +
>> +    rc = get_tjmax(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    msg.addr = priv->addr;
>> +    msg.index = MBX_INDEX_PER_CORE_DTS_TEMP;
>> +    msg.param = core_index;
>> +    msg.rx_len = 4;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    core_dts_margin = (msg.pkg_config[1] << 8) | msg.pkg_config[0];
>> +
>> +    /**
>> +     * Processors return a value of the core DTS reading in 10.6 format
>> +     * (10 bits signed decimal, 6 bits fractional).
>> +     * Error codes:
>> +     *   0x8000: General sensor error
>> +     *   0x8001: Reserved
>> +     *   0x8002: Underflow on reading value
>> +     *   0x8003-0x81ff: Reserved
>> +     */
>> +    if (core_dts_margin >= 0x8000 && core_dts_margin <= 0x81ff)
>> +        return -1;
>> +
>> +    core_dts_margin = ten_dot_six_to_millidegree(core_dts_margin);
>> +
>> +    priv->temp.core[core_index].value = priv->temp.tjmax.value +
>> +                        core_dts_margin;
>> +
>> +    if (!priv->temp.core[core_index].valid) {
>> +        priv->temp.core[core_index].last_updated = INITIAL_JIFFIES;
>> +        priv->temp.core[core_index].valid = true;
>> +    } else {
>> +        priv->temp.core[core_index].last_updated = jiffies;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static int get_dimm_temp(struct peci_hwmon *priv, int dimm_index)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    int channel = dimm_index / 2;
>> +    int dimm_order = dimm_index % 2;
>> +    int rc;
>> +
>> +    if (!need_update(&priv->temp.dimm[dimm_index]))
>> +        return 0;
>> +
>> +    msg.addr = priv->addr;
>> +    msg.index = MBX_INDEX_DDR_DIMM_TEMP;
>> +    msg.param = channel;
>> +    msg.rx_len = 4;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    priv->temp.dimm[dimm_index].value = msg.pkg_config[dimm_order] * 
>> 1000;
>> +
>> +    if (!priv->temp.dimm[dimm_index].valid) {
>> +        priv->temp.dimm[dimm_index].last_updated = INITIAL_JIFFIES;
>> +        priv->temp.dimm[dimm_index].valid = true;
>> +    } else {
>> +        priv->temp.dimm[dimm_index].last_updated = jiffies;
>> +    }
>> +
>> +    return 0;
>> +}
>> +
>> +static ssize_t show_tcontrol(struct device *dev,
>> +                 struct device_attribute *attr,
>> +                 char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    int rc;
>> +
>> +    rc = get_tcontrol(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.tcontrol.value);
>> +}
>> +
>> +static ssize_t show_tcontrol_margin(struct device *dev,
>> +                    struct device_attribute *attr,
>> +                    char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +    int rc;
>> +
>> +    rc = get_tcontrol(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", sensor_attr->index == POS ?
>> +                    priv->temp.tjmax.value -
>> +                    priv->temp.tcontrol.value :
>> +                    priv->temp.tcontrol.value -
>> +                    priv->temp.tjmax.value);
>> +}
>> +
>> +static ssize_t show_tthrottle(struct device *dev,
>> +                  struct device_attribute *attr,
>> +                  char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    int rc;
>> +
>> +    rc = get_tthrottle(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.tthrottle.value);
>> +}
>> +
>> +static ssize_t show_tjmax(struct device *dev,
>> +              struct device_attribute *attr,
>> +              char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    int rc;
>> +
>> +    rc = get_tjmax(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.tjmax.value);
>> +}
>> +
>> +static ssize_t show_die_temp(struct device *dev,
>> +                 struct device_attribute *attr,
>> +                 char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    int rc;
>> +
>> +    rc = get_die_temp(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.die.value);
>> +}
>> +
>> +static ssize_t show_dts_margin(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    int rc;
>> +
>> +    rc = get_dts_margin(priv);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.dts_margin.value);
>> +}
>> +
>> +static ssize_t show_core_temp(struct device *dev,
>> +                  struct device_attribute *attr,
>> +                  char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +    int core_index = sensor_attr->index;
>> +    int rc;
>> +
>> +    rc = get_core_temp(priv, core_index);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.core[core_index].value);
>> +}
>> +
>> +static ssize_t show_dimm_temp(struct device *dev,
>> +                  struct device_attribute *attr,
>> +                  char *buf)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(dev);
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +    int dimm_index = sensor_attr->index;
>> +    int rc;
>> +
>> +    rc = get_dimm_temp(priv, dimm_index);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    return sprintf(buf, "%d\n", priv->temp.dimm[dimm_index].value);
>> +}
>> +
>> +static ssize_t show_value(struct device *dev,
>> +              struct device_attribute *attr,
>> +              char *buf)
>> +{
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +
>> +    return sprintf(buf, "%d\n", sensor_attr->index);
>> +}
>> +
>> +static ssize_t show_label(struct device *dev,
>> +              struct device_attribute *attr,
>> +              char *buf)
>> +{
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +
>> +    return sprintf(buf, peci_label[sensor_attr->index]);
>> +}
>> +
>> +static ssize_t show_core_label(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   char *buf)
>> +{
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +
>> +    return sprintf(buf, "Core %d\n", sensor_attr->index);
>> +}
>> +
>> +static ssize_t show_dimm_label(struct device *dev,
>> +                   struct device_attribute *attr,
>> +                   char *buf)
>> +{
>> +    struct sensor_device_attribute *sensor_attr = 
>> to_sensor_dev_attr(attr);
>> +
>> +    char channel = 'A' + (sensor_attr->index / 2);
>> +    int index = sensor_attr->index % 2;
>> +
>> +    return sprintf(buf, "DIMM %d (%c%d)\n",
>> +               sensor_attr->index, channel, index);
>> +}
>> +
>> +/* Die temperature */
>> +static SENSOR_DEVICE_ATTR(temp1_label, 0444, show_label, NULL, L_DIE);
>> +static SENSOR_DEVICE_ATTR(temp1_input, 0444, show_die_temp, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(temp1_max, 0444, show_tcontrol, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(temp1_crit, 0444, show_tjmax, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(temp1_crit_hyst, 0444, 
>> show_tcontrol_margin, NULL,
>> +              POS);
>> +
>> +static struct attribute *die_temp_attrs[] = {
>> +    &sensor_dev_attr_temp1_label.dev_attr.attr,
>> +    &sensor_dev_attr_temp1_input.dev_attr.attr,
>> +    &sensor_dev_attr_temp1_max.dev_attr.attr,
>> +    &sensor_dev_attr_temp1_crit.dev_attr.attr,
>> +    &sensor_dev_attr_temp1_crit_hyst.dev_attr.attr,
>> +    NULL
>> +};
>> +
>> +static struct attribute_group die_temp_attr_group = {
>> +    .attrs = die_temp_attrs,
>> +};
>> +
>> +/* DTS margin temperature */
>> +static SENSOR_DEVICE_ATTR(temp2_label, 0444, show_label, NULL, L_DTS);
>> +static SENSOR_DEVICE_ATTR(temp2_input, 0444, show_dts_margin, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(temp2_min, 0444, show_value, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(temp2_lcrit, 0444, show_tcontrol_margin, 
>> NULL, NEG);
>> +
>> +static struct attribute *dts_margin_temp_attrs[] = {
>> +    &sensor_dev_attr_temp2_label.dev_attr.attr,
>> +    &sensor_dev_attr_temp2_input.dev_attr.attr,
>> +    &sensor_dev_attr_temp2_min.dev_attr.attr,
>> +    &sensor_dev_attr_temp2_lcrit.dev_attr.attr,
>> +    NULL
>> +};
>> +
>> +static struct attribute_group dts_margin_temp_attr_group = {
>> +    .attrs = dts_margin_temp_attrs,
>> +};
>> +
>> +/* Tcontrol temperature */
>> +static SENSOR_DEVICE_ATTR(temp3_label, 0444, show_label, NULL, 
>> L_TCONTROL);
>> +static SENSOR_DEVICE_ATTR(temp3_input, 0444, show_tcontrol, NULL, 0);
>> +static SENSOR_DEVICE_ATTR(temp3_crit, 0444, show_tjmax, NULL, 0);
>> +
>> +static struct attribute *tcontrol_temp_attrs[] = {
>> +    &sensor_dev_attr_temp3_label.dev_attr.attr,
>> +    &sensor_dev_attr_temp3_input.dev_attr.attr,
>> +    &sensor_dev_attr_temp3_crit.dev_attr.attr,
>> +    NULL
>> +};
>> +
>> +static struct attribute_group tcontrol_temp_attr_group = {
>> +    .attrs = tcontrol_temp_attrs,
>> +};
>> +
>> +/* Tthrottle temperature */
>> +static SENSOR_DEVICE_ATTR(temp4_label, 0444, show_label, NULL, 
>> L_TTHROTTLE);
>> +static SENSOR_DEVICE_ATTR(temp4_input, 0444, show_tthrottle, NULL, 0);
>> +
>> +static struct attribute *tthrottle_temp_attrs[] = {
>> +    &sensor_dev_attr_temp4_label.dev_attr.attr,
>> +    &sensor_dev_attr_temp4_input.dev_attr.attr,
>> +    NULL
>> +};
>> +
>> +static struct attribute_group tthrottle_temp_attr_group = {
>> +    .attrs = tthrottle_temp_attrs,
>> +};
>> +
>> +/* Tjmax temperature */
>> +static SENSOR_DEVICE_ATTR(temp5_label, 0444, show_label, NULL, L_TJMAX);
>> +static SENSOR_DEVICE_ATTR(temp5_input, 0444, show_tjmax, NULL, 0);
>> +
>> +static struct attribute *tjmax_temp_attrs[] = {
>> +    &sensor_dev_attr_temp5_label.dev_attr.attr,
>> +    &sensor_dev_attr_temp5_input.dev_attr.attr,
>> +    NULL
>> +};
>> +
>> +static struct attribute_group tjmax_temp_attr_group = {
>> +    .attrs = tjmax_temp_attrs,
>> +};
>> +
>> +static const struct attribute_group *
>> +default_attr_groups[DEFAULT_ATTR_GRP_NUMS + 1] = {
>> +    &die_temp_attr_group,
>> +    &dts_margin_temp_attr_group,
>> +    &tcontrol_temp_attr_group,
>> +    &tthrottle_temp_attr_group,
>> +    &tjmax_temp_attr_group,
>> +    NULL
>> +};
>> +
>> +/* Core temperature */
>> +static ssize_t (*const core_show_fn[CORE_TEMP_ATTRS]) (struct device 
>> *dev,
>> +        struct device_attribute *devattr, char *buf) = {
>> +    show_core_label,
>> +    show_core_temp,
>> +    show_tcontrol,
>> +    show_tjmax,
>> +    show_tcontrol_margin,
>> +};
>> +
>> +static const char *const core_suffix[CORE_TEMP_ATTRS] = {
>> +    "label",
>> +    "input",
>> +    "max",
>> +    "crit",
>> +    "crit_hyst",
>> +};
>> +
>> +static int check_resolved_cores(struct peci_hwmon *priv)
>> +{
>> +    struct peci_rd_pci_cfg_local_msg msg;
>> +    int rc;
>> +
>> +    if (!(priv->client->adapter->cmd_mask & 
>> BIT(PECI_CMD_RD_PCI_CFG_LOCAL)))
>> +        return -EINVAL;
>> +
>> +    /* Get the RESOLVED_CORES register value */
>> +    msg.addr = priv->addr;
>> +    msg.bus = 1;
>> +    msg.device = 30;
>> +    msg.function = 3;
>> +    msg.reg = 0xB4;
>> +    msg.rx_len = 4;
>> +
>> +    rc = send_peci_cmd(priv, PECI_CMD_RD_PCI_CFG_LOCAL, (void *)&msg);
>> +    if (rc < 0)
>> +        return rc;
>> +
>> +    priv->core_mask = msg.pci_config[3] << 24 |
>> +              msg.pci_config[2] << 16 |
>> +              msg.pci_config[1] << 8 |
>> +              msg.pci_config[0];
>> +
>> +    if (!priv->core_mask)
>> +        return -EAGAIN;
>> +
>> +    dev_dbg(priv->dev, "Scanned resolved cores: 0x%x\n", 
>> priv->core_mask);
>> +    return 0;
>> +}
>> +
>> +static int create_core_temp_group(struct peci_hwmon *priv, int core_no)
>> +{
>> +    struct core_temp_group *data;
>> +    int i;
>> +
>> +    data = devm_kzalloc(priv->dev, sizeof(struct core_temp_group),
>> +                GFP_KERNEL);
>> +    if (!data)
>> +        return -ENOMEM;
>> +
>> +    for (i = 0; i < CORE_TEMP_ATTRS; i++) {
>> +        snprintf(data->attr_name[i], ATTR_NAME_LEN,
>> +             "temp%d_%s", priv->global_idx, core_suffix[i]);
>> +        sysfs_attr_init(&data->sd_attrs[i].dev_attr.attr);
>> +        data->sd_attrs[i].dev_attr.attr.name = data->attr_name[i];
>> +        data->sd_attrs[i].dev_attr.attr.mode = 0444;
>> +        data->sd_attrs[i].dev_attr.show = core_show_fn[i];
>> +        if (i == 0 || i == 1) /* label or temp */
>> +            data->sd_attrs[i].index = core_no;
>> +        data->attrs[i] = &data->sd_attrs[i].dev_attr.attr;
>> +    }
>> +
>> +    data->attr_group.attrs = data->attrs;
>> +    priv->core_attr_groups[priv->core_idx++] = &data->attr_group;
>> +    priv->global_idx++;
>> +
>> +    return 0;
>> +}
>> +
>> +static int create_core_temp_groups(struct peci_hwmon *priv)
>> +{
>> +    int rc, i;
>> +
>> +    rc = check_resolved_cores(priv);
>> +    if (!rc) {
>> +        for (i = 0; i < CORE_NUMS_MAX; i++) {
>> +            if (priv->core_mask & BIT(i)) {
>> +                rc = create_core_temp_group(priv, i);
>> +                if (rc)
>> +                    return rc;
>> +            }
>> +        }
>> +
>> +        rc = sysfs_create_groups(&priv->hwmon_dev->kobj,
>> +                     priv->core_attr_groups);
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>> +/* DIMM temperature */
>> +static ssize_t (*const dimm_show_fn[DIMM_TEMP_ATTRS]) (struct device 
>> *dev,
>> +        struct device_attribute *devattr, char *buf) = {
>> +    show_dimm_label,
>> +    show_dimm_temp,
>> +};
>> +
>> +static const char *const dimm_suffix[DIMM_TEMP_ATTRS] = {
>> +    "label",
>> +    "input",
>> +};
>> +
>> +static int check_populated_dimms(struct peci_hwmon *priv)
>> +{
>> +    struct peci_rd_pkg_cfg_msg msg;
>> +    int i, rc, pass = 0;
>> +
>> +do_scan:
>> +    for (i = 0; i < (DIMM_SLOT_NUMS_MAX / 2); i++) {
>> +        msg.addr = priv->addr;
>> +        msg.index = MBX_INDEX_DDR_DIMM_TEMP;
>> +        msg.param = i; /* channel */
>> +        msg.rx_len = 4;
>> +
>> +        rc = send_peci_cmd(priv, PECI_CMD_RD_PKG_CFG, (void *)&msg);
>> +        if (rc < 0)
>> +            return rc;
>> +
>> +        if (msg.pkg_config[0]) /* DIMM #0 on the channel */
>> +            priv->dimm_mask |= BIT(i);
>> +
>> +        if (msg.pkg_config[1]) /* DIMM #1 on the channel */
>> +            priv->dimm_mask |= BIT(i + 1);
>> +    }
>> +
>> +    /* Do 2-pass scanning */
>> +    if (priv->dimm_mask && pass == 0) {
>> +        pass++;
>> +        goto do_scan;
>> +    }
>> +
>> +    if (!priv->dimm_mask)
>> +        return -EAGAIN;
>> +
>> +    dev_dbg(priv->dev, "Scanned populated DIMMs: 0x%x\n", 
>> priv->dimm_mask);
>> +    return 0;
>> +}
>> +
>> +static int create_dimm_temp_group(struct peci_hwmon *priv, int dimm_no)
>> +{
>> +    struct dimm_temp_group *data;
>> +    int i;
>> +
>> +    data = devm_kzalloc(priv->dev, sizeof(struct dimm_temp_group),
>> +                GFP_KERNEL);
>> +    if (!data)
>> +        return -ENOMEM;
>> +
>> +    for (i = 0; i < DIMM_TEMP_ATTRS; i++) {
>> +        snprintf(data->attr_name[i], ATTR_NAME_LEN,
>> +             "temp%d_%s", priv->global_idx, dimm_suffix[i]);
>> +        sysfs_attr_init(&data->sd_attrs[i].dev_attr.attr);
>> +        data->sd_attrs[i].dev_attr.attr.name = data->attr_name[i];
>> +        data->sd_attrs[i].dev_attr.attr.mode = 0444;
>> +        data->sd_attrs[i].dev_attr.show = dimm_show_fn[i];
>> +        data->sd_attrs[i].index = dimm_no;
>> +        data->attrs[i] = &data->sd_attrs[i].dev_attr.attr;
>> +    }
>> +
>> +    data->attr_group.attrs = data->attrs;
>> +    priv->dimm_attr_groups[priv->dimm_idx++] = &data->attr_group;
>> +    priv->global_idx++;
>> +
>> +    return 0;
>> +}
>> +
>> +static int create_dimm_temp_groups(struct peci_hwmon *priv)
>> +{
>> +    int rc, i;
>> +
>> +    rc = check_populated_dimms(priv);
>> +    if (!rc) {
>> +        for (i = 0; i < DIMM_SLOT_NUMS_MAX; i++) {
>> +            if (priv->dimm_mask & BIT(i)) {
>> +                rc = create_dimm_temp_group(priv, i);
>> +                if (rc)
>> +                    return rc;
>> +            }
>> +        }
>> +
>> +        rc = sysfs_create_groups(&priv->hwmon_dev->kobj,
>> +                     priv->dimm_attr_groups);
>> +        if (!rc)
>> +            dev_dbg(priv->dev, "Done DIMM temp group creation\n");
>> +    } else if (rc == -EAGAIN) {
>> +        queue_delayed_work(priv->work_queue, &priv->work_handler,
>> +                   DIMM_MASK_CHECK_DELAY);
>> +        dev_dbg(priv->dev, "Diferred DIMM temp group creation\n");
>> +    }
>> +
>> +    return rc;
>> +}
>> +
>> +static void create_dimm_temp_groups_delayed(struct work_struct *work)
>> +{
>> +    struct delayed_work *dwork = to_delayed_work(work);
>> +    struct peci_hwmon *priv = container_of(dwork, struct peci_hwmon,
>> +                           work_handler);
>> +    int rc;
>> +
>> +    rc = create_dimm_temp_groups(priv);
>> +    if (rc && rc != -EAGAIN)
>> +        dev_dbg(priv->dev, "Skipped to creat DIMM temp groups\n");
>> +}
>> +
>> +static int peci_hwmon_probe(struct peci_client *client)
>> +{
>> +    struct device *dev = &client->dev;
>> +    struct peci_hwmon *priv;
>> +    int rc;
>> +
>> +    if ((client->adapter->cmd_mask &
>> +        (BIT(PECI_CMD_GET_TEMP) | BIT(PECI_CMD_RD_PKG_CFG))) !=
>> +        (BIT(PECI_CMD_GET_TEMP) | BIT(PECI_CMD_RD_PKG_CFG))) {
>> +        dev_err(dev, "Client doesn't support temperature monitoring\n");
>> +        return -EINVAL;
>> +    }
>> +
>> +    priv = devm_kzalloc(dev, sizeof(*priv), GFP_KERNEL);
>> +    if (!priv)
>> +        return -ENOMEM;
>> +
>> +    dev_set_drvdata(dev, priv);
>> +    priv->client = client;
>> +    priv->dev = dev;
>> +    priv->addr = client->addr;
>> +    priv->cpu_no = priv->addr - PECI_BASE_ADDR;
>> +
>> +    snprintf(priv->name, PECI_NAME_SIZE, "peci_hwmon.cpu%d", 
>> priv->cpu_no);
>> +
>> +    priv->work_queue = create_singlethread_workqueue(priv->name);
>> +    if (!priv->work_queue)
>> +        return -ENOMEM;
>> +
>> +    priv->hwmon_dev = hwmon_device_register_with_groups(priv->dev,
>> +                                priv->name,
>> +                                priv,
>> +                               default_attr_groups);
>> +
>> +    rc = PTR_ERR_OR_ZERO(priv->hwmon_dev);
>> +    if (rc) {
>> +        dev_err(dev, "Failed to register peci hwmon\n");
>> +        return rc;
>> +    }
>> +
>> +    priv->global_idx = DEFAULT_ATTR_GRP_NUMS + 1;
>> +
>> +    rc = create_core_temp_groups(priv);
>> +    if (rc) {
>> +        dev_err(dev, "Failed to create core groups\n");
>> +        return rc;
>> +    }
>> +
>> +    INIT_DELAYED_WORK(&priv->work_handler, 
>> create_dimm_temp_groups_delayed);
>> +
>> +    rc = create_dimm_temp_groups(priv);
>> +    if (rc && rc != -EAGAIN)
>> +        dev_dbg(dev, "Skipped to creat DIMM temp groups\n");
>> +
>> +    dev_dbg(dev, "peci hwmon for CPU at 0x%x registered\n", priv->addr);
>> +
>> +    return 0;
>> +}
>> +
>> +static int peci_hwmon_remove(struct peci_client *client)
>> +{
>> +    struct peci_hwmon *priv = dev_get_drvdata(&client->dev);
>> +
>> +    cancel_delayed_work(&priv->work_handler);
>> +    destroy_workqueue(priv->work_queue);
>> +    sysfs_remove_groups(&priv->hwmon_dev->kobj, priv->core_attr_groups);
>> +    sysfs_remove_groups(&priv->hwmon_dev->kobj, priv->dimm_attr_groups);
>> +    hwmon_device_unregister(priv->hwmon_dev);
>> +
>> +    return 0;
>> +}
>> +
>> +static const struct of_device_id peci_of_table[] = {
>> +    { .compatible = "intel,peci-hwmon", },
>> +    { }
>> +};
>> +MODULE_DEVICE_TABLE(of, peci_of_table);
>> +
>> +static struct peci_driver peci_hwmon_driver = {
>> +    .probe  = peci_hwmon_probe,
>> +    .remove = peci_hwmon_remove,
>> +    .driver = {
>> +        .name           = "peci-hwmon",
>> +        .of_match_table = of_match_ptr(peci_of_table),
>> +    },
>> +};
>> +module_peci_driver(peci_hwmon_driver);
>> +
>> +MODULE_AUTHOR("Jae Hyun Yoo <jae.hyun.yoo@linux.intel.com>");
>> +MODULE_DESCRIPTION("PECI hwmon driver");
>> +MODULE_LICENSE("GPL v2");
>>
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH v3 4/4] tty: serial: msm_geni_serial: Add serial driver support for GENI based QUP
From: Karthik Ramasubramanian @ 2018-03-13 20:16 UTC (permalink / raw)
  To: Evan Green
  Cc: Jonathan Corbet, Andy Gross, David Brown, robh+dt, mark.rutland,
	wsa, gregkh, linux-doc, linux-arm-msm, devicetree, linux-i2c,
	linux-serial, jslaby, acourbot, Girish Mahadevan, Sagar Dharia,
	Doug Anderson
In-Reply-To: <CAE=gft4Hwqq2gx+b9AmiX1ZZBrSt1Ca2E48JV77wpcQQT0060w@mail.gmail.com>



On 3/2/2018 5:11 PM, Evan Green wrote:
>> +
>> +#ifdef CONFIG_CONSOLE_POLL
>> +#define RX_BYTES_PW 1
>> +#else
>> +#define RX_BYTES_PW 4
>> +#endif
> 
> This seems fishy to me. Does either setting work? If so, why not just
> have one value?
Yes, either one works. In the interrupt driven mode, sometimes due to
increased interrupt latency the RX FIFO may overflow if we use only 1
byte per FIFO word - given there are no flow control lines in the debug
uart. Hence using 4 bytes in the FIFO word will help to prevent the FIFO
overflow - especially in the case where commands are executed through
scripts.

In polling mode, using 1 byte per word helps to use the hardware to
buffer the data instead of software buffering especially when the
framework keeps reading one byte at a time.
> 
>> +static bool qcom_geni_serial_poll_bit(struct uart_port *uport,
>> +                               int offset, int bit_field, bool set)
>> +{
>> +       u32 reg;
>> +       struct qcom_geni_serial_port *port;
>> +       unsigned int baud;
>> +       unsigned int fifo_bits;
>> +       unsigned long timeout_us = 20000;
>> +
>> +       /* Ensure polling is not re-ordered before the prior writes/reads */
>> +       mb();
>> +
>> +       if (uport->private_data) {
>> +               port = to_dev_port(uport, uport);
>> +               baud = port->cur_baud;
>> +               if (!baud)
>> +                       baud = 115200;
>> +               fifo_bits = port->tx_fifo_depth * port->tx_fifo_width;
>> +               /*
>> +                * Total polling iterations based on FIFO worth of bytes to be
>> +                * sent at current baud .Add a little fluff to the wait.
>> +                */
>> +               timeout_us = ((fifo_bits * USEC_PER_SEC) / baud) + 500;
> 
> This fluff is a little mysterious, can it be explained at all? Do you
> think the fluff factor is in units of time (as you have it) or bits?
> Time makes sense I guess if we're worried about clock source
> differences.
The fluff is in micro-seconds and can help with unforeseen delays in
emulation platforms.
> 
>> +
>> +static void qcom_geni_serial_console_write(struct console *co, const char *s,
>> +                             unsigned int count)
>> +{
>> +       struct uart_port *uport;
>> +       struct qcom_geni_serial_port *port;
>> +       bool locked = true;
>> +       unsigned long flags;
>> +
>> +       WARN_ON(co->index < 0 || co->index >= GENI_UART_CONS_PORTS);
>> +
>> +       port = get_port_from_line(co->index);
>> +       if (IS_ERR(port))
>> +               return;
>> +
>> +       uport = &port->uport;
>> +       if (oops_in_progress)
>> +               locked = spin_trylock_irqsave(&uport->lock, flags);
>> +       else
>> +               spin_lock_irqsave(&uport->lock, flags);
>> +
>> +       if (locked) {
>> +               __qcom_geni_serial_console_write(uport, s, count);
>> +               spin_unlock_irqrestore(&uport->lock, flags);
> 
> I too am a little lost on the locking here. What exactly is the lock
> protecting? Looks like for the most part it's trying to synchronize
> with the ISR? What specifically in the ISR? I just wanted to go
> through and check to make sure whatever the shared resource is is
> appropriately protected.
The lock protects 2 simultaneous writers from putting the hardware in
the bad state. The output of the command entered in a shell can trigger
a write in the interrupt context while logging activity can trigger a
simultaneous write.
> 
>> +       }
>> +}
>> +
>> +static int handle_rx_console(struct uart_port *uport, u32 rx_bytes, bool drop)
>> +{
>> +       u32 i = rx_bytes;
>> +       u32 rx_fifo;
>> +       unsigned char *buf;
>> +       struct tty_port *tport;
>> +       struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
>> +
>> +       tport = &uport->state->port;
>> +       while (i > 0) {
>> +               int c;
>> +               int bytes = i > port->rx_bytes_pw ? port->rx_bytes_pw : i;
> 
> Please replace this with a min macro.
Ok.
> 
>> +static int qcom_geni_serial_handle_tx(struct uart_port *uport)
>> +{
>> +       int ret = 0;
>> +       struct qcom_geni_serial_port *port = to_dev_port(uport, uport);
>> +       struct circ_buf *xmit = &uport->state->xmit;
>> +       size_t avail;
>> +       size_t remaining;
>> +       int i = 0;
>> +       u32 status;
>> +       unsigned int chunk;
>> +       int tail;
>> +
>> +       chunk = uart_circ_chars_pending(xmit);
>> +       status = readl_relaxed(uport->membase + SE_GENI_TX_FIFO_STATUS);
>> +       /* Both FIFO and framework buffer are drained */
>> +       if ((chunk == port->xmit_size) && !status) {
>> +               port->xmit_size = 0;
>> +               uart_circ_clear(xmit);
>> +               qcom_geni_serial_stop_tx(uport);
>> +               goto out_write_wakeup;
>> +       }
>> +       chunk -= port->xmit_size;
>> +
>> +       avail = (port->tx_fifo_depth - port->tx_wm) * port->tx_bytes_pw;
>> +       tail = (xmit->tail + port->xmit_size) & (UART_XMIT_SIZE - 1);
>> +       if (chunk > (UART_XMIT_SIZE - tail))
>> +               chunk = UART_XMIT_SIZE - tail;
>> +       if (chunk > avail)
>> +               chunk = avail;
>> +
>> +       if (!chunk)
>> +               goto out_write_wakeup;
>> +
>> +       qcom_geni_serial_setup_tx(uport, chunk);
>> +
>> +       remaining = chunk;
>> +       while (i < chunk) {
>> +               unsigned int tx_bytes;
>> +               unsigned int buf = 0;
>> +               int c;
>> +
>> +               tx_bytes = min_t(size_t, remaining, (size_t)port->tx_bytes_pw);
>> +               for (c = 0; c < tx_bytes ; c++)
>> +                       buf |= (xmit->buf[tail + c] << (c * BITS_PER_BYTE));
>> +
>> +               writel_relaxed(buf, uport->membase + SE_GENI_TX_FIFOn);
>> +
>> +               i += tx_bytes;
>> +               tail = (tail + tx_bytes) & (UART_XMIT_SIZE - 1);
>> +               uport->icount.tx += tx_bytes;
>> +               remaining -= tx_bytes;
>> +       }
>> +       qcom_geni_serial_poll_tx_done(uport);
>> +       port->xmit_size += chunk;
>> +out_write_wakeup:
>> +       uart_write_wakeup(uport);
>> +       return ret;
>> +}
> 
> This function can't fail, please change the return type to void.
Ok.
> 
>> +
>> +static void qcom_geni_serial_shutdown(struct uart_port *uport)
>> +{
>> +       unsigned long flags;
>> +
>> +       /* Stop the console before stopping the current tx */
>> +       console_stop(uport->cons);
>> +
>> +       disable_irq(uport->irq);
>> +       free_irq(uport->irq, uport);
>> +       spin_lock_irqsave(&uport->lock, flags);
>> +       qcom_geni_serial_stop_tx(uport);
>> +       qcom_geni_serial_stop_rx(uport);
>> +       spin_unlock_irqrestore(&uport->lock, flags);
> 
> This is one part of where I'm confused. What are we protecting here
> with the lock? disable_irq waits for any pending ISRs to finish
> according to its comment, so you know you're not racing with the ISR.
In android, console shutdown can be invoked while console write happens.
This lock prevents shutdown from not interfering with the write and
vice-versa.
> 
>> +static void geni_serial_write_term_regs(struct uart_port *uport,
>> +               u32 tx_trans_cfg, u32 tx_parity_cfg, u32 rx_trans_cfg,
>> +               u32 rx_parity_cfg, u32 bits_per_char, u32 stop_bit_len,
>> +               u32 s_clk_cfg)
>> +{
>> +       writel_relaxed(tx_trans_cfg, uport->membase + SE_UART_TX_TRANS_CFG);
>> +       writel_relaxed(tx_parity_cfg, uport->membase + SE_UART_TX_PARITY_CFG);
>> +       writel_relaxed(rx_trans_cfg, uport->membase + SE_UART_RX_TRANS_CFG);
>> +       writel_relaxed(rx_parity_cfg, uport->membase + SE_UART_RX_PARITY_CFG);
>> +       writel_relaxed(bits_per_char, uport->membase + SE_UART_TX_WORD_LEN);
>> +       writel_relaxed(bits_per_char, uport->membase + SE_UART_RX_WORD_LEN);
>> +       writel_relaxed(stop_bit_len, uport->membase + SE_UART_TX_STOP_BIT_LEN);
>> +       writel_relaxed(s_clk_cfg, uport->membase + GENI_SER_M_CLK_CFG);
>> +       writel_relaxed(s_clk_cfg, uport->membase + GENI_SER_S_CLK_CFG);
>> +}
>> +
> 
> I agree with Stephen's comment, this should be inlined into the single
> place it's called from.
> 
> Thanks Karthik!
> -Evan
> 
Regards,
Karthik.
-- 
Qualcomm Innovation Center, Inc. is a member of the Code Aurora Forum,
a Linux Foundation Collaborative Project
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 1/8] Uprobe: Export vaddr <-> offset conversion functions
From: Jerome Glisse @ 2018-03-13 20:36 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: mhiramat, oleg, peterz, srikar, acme, ananth, akpm,
	alexander.shishkin, alexis.berlemont, corbet, dan.j.williams,
	gregkh, huawei.libin, hughd, jack, jolsa, kan.liang,
	kirill.shutemov, kjlx, kstewart, linux-doc, linux-kernel,
	linux-mm, mhocko, milian.wolff, mingo, namhyung, naveen.n.rao, pc,
	pombredanne, rostedt, tglx, tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-2-ravi.bangoria@linux.vnet.ibm.com>

On Tue, Mar 13, 2018 at 06:25:56PM +0530, Ravi Bangoria wrote:
> No functionality changes.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  include/linux/mm.h      | 12 ++++++++++++
>  kernel/events/uprobes.c | 10 ----------
>  2 files changed, 12 insertions(+), 10 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index ad06d42..95909f2 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2274,6 +2274,18 @@ struct vm_unmapped_area_info {
>  		return unmapped_area(info);
>  }
>  
> +static inline unsigned long
> +offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
> +{
> +	return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> +}
> +
> +static inline loff_t
> +vaddr_to_offset(struct vm_area_struct *vma, unsigned long vaddr)
> +{
> +	return ((loff_t)vma->vm_pgoff << PAGE_SHIFT) + (vaddr - vma->vm_start);
> +}
> +
>  /* truncate.c */
>  extern void truncate_inode_pages(struct address_space *, loff_t);
>  extern void truncate_inode_pages_range(struct address_space *,
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index ce6848e..bd6f230 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -130,16 +130,6 @@ static bool valid_vma(struct vm_area_struct *vma, bool is_register)
>  	return vma->vm_file && (vma->vm_flags & flags) == VM_MAYEXEC;
>  }
>  
> -static unsigned long offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
> -{
> -	return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
> -}
> -
> -static loff_t vaddr_to_offset(struct vm_area_struct *vma, unsigned long vaddr)
> -{
> -	return ((loff_t)vma->vm_pgoff << PAGE_SHIFT) + (vaddr - vma->vm_start);
> -}
> -
>  /**
>   * __replace_page - replace page in vma by new page.
>   * based on replace_page in mm/ksm.c
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 2/8] mm: Prefix vma_ to vaddr_to_offset() and offset_to_vaddr()
From: Jerome Glisse @ 2018-03-13 20:38 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: mhiramat, oleg, peterz, srikar, acme, ananth, akpm,
	alexander.shishkin, alexis.berlemont, corbet, dan.j.williams,
	gregkh, huawei.libin, hughd, jack, jolsa, kan.liang,
	kirill.shutemov, kjlx, kstewart, linux-doc, linux-kernel,
	linux-mm, mhocko, milian.wolff, mingo, namhyung, naveen.n.rao, pc,
	pombredanne, rostedt, tglx, tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-3-ravi.bangoria@linux.vnet.ibm.com>

On Tue, Mar 13, 2018 at 06:25:57PM +0530, Ravi Bangoria wrote:
> No functionality changes.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Doing this with coccinelle would have been nicer but this is small
enough to review without too much fatigue :)

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  include/linux/mm.h      |  4 ++--
>  kernel/events/uprobes.c | 14 +++++++-------
>  2 files changed, 9 insertions(+), 9 deletions(-)
> 
> diff --git a/include/linux/mm.h b/include/linux/mm.h
> index 95909f2..d7ee526 100644
> --- a/include/linux/mm.h
> +++ b/include/linux/mm.h
> @@ -2275,13 +2275,13 @@ struct vm_unmapped_area_info {
>  }
>  
>  static inline unsigned long
> -offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
> +vma_offset_to_vaddr(struct vm_area_struct *vma, loff_t offset)
>  {
>  	return vma->vm_start + offset - ((loff_t)vma->vm_pgoff << PAGE_SHIFT);
>  }
>  
>  static inline loff_t
> -vaddr_to_offset(struct vm_area_struct *vma, unsigned long vaddr)
> +vma_vaddr_to_offset(struct vm_area_struct *vma, unsigned long vaddr)
>  {
>  	return ((loff_t)vma->vm_pgoff << PAGE_SHIFT) + (vaddr - vma->vm_start);
>  }
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index bd6f230..535fd39 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -748,7 +748,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  		curr = info;
>  
>  		info->mm = vma->vm_mm;
> -		info->vaddr = offset_to_vaddr(vma, offset);
> +		info->vaddr = vma_offset_to_vaddr(vma, offset);
>  	}
>  	i_mmap_unlock_read(mapping);
>  
> @@ -807,7 +807,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  			goto unlock;
>  
>  		if (vma->vm_start > info->vaddr ||
> -		    vaddr_to_offset(vma, info->vaddr) != uprobe->offset)
> +		    vma_vaddr_to_offset(vma, info->vaddr) != uprobe->offset)
>  			goto unlock;
>  
>  		if (is_register) {
> @@ -977,7 +977,7 @@ static int unapply_uprobe(struct uprobe *uprobe, struct mm_struct *mm)
>  		    uprobe->offset >= offset + vma->vm_end - vma->vm_start)
>  			continue;
>  
> -		vaddr = offset_to_vaddr(vma, uprobe->offset);
> +		vaddr = vma_offset_to_vaddr(vma, uprobe->offset);
>  		err |= remove_breakpoint(uprobe, mm, vaddr);
>  	}
>  	up_read(&mm->mmap_sem);
> @@ -1023,7 +1023,7 @@ static void build_probe_list(struct inode *inode,
>  	struct uprobe *u;
>  
>  	INIT_LIST_HEAD(head);
> -	min = vaddr_to_offset(vma, start);
> +	min = vma_vaddr_to_offset(vma, start);
>  	max = min + (end - start) - 1;
>  
>  	spin_lock(&uprobes_treelock);
> @@ -1076,7 +1076,7 @@ int uprobe_mmap(struct vm_area_struct *vma)
>  	list_for_each_entry_safe(uprobe, u, &tmp_list, pending_list) {
>  		if (!fatal_signal_pending(current) &&
>  		    filter_chain(uprobe, UPROBE_FILTER_MMAP, vma->vm_mm)) {
> -			unsigned long vaddr = offset_to_vaddr(vma, uprobe->offset);
> +			unsigned long vaddr = vma_offset_to_vaddr(vma, uprobe->offset);
>  			install_breakpoint(uprobe, vma->vm_mm, vma, vaddr);
>  		}
>  		put_uprobe(uprobe);
> @@ -1095,7 +1095,7 @@ int uprobe_mmap(struct vm_area_struct *vma)
>  
>  	inode = file_inode(vma->vm_file);
>  
> -	min = vaddr_to_offset(vma, start);
> +	min = vma_vaddr_to_offset(vma, start);
>  	max = min + (end - start) - 1;
>  
>  	spin_lock(&uprobes_treelock);
> @@ -1730,7 +1730,7 @@ static struct uprobe *find_active_uprobe(unsigned long bp_vaddr, int *is_swbp)
>  	if (vma && vma->vm_start <= bp_vaddr) {
>  		if (valid_vma(vma, false)) {
>  			struct inode *inode = file_inode(vma->vm_file);
> -			loff_t offset = vaddr_to_offset(vma, bp_vaddr);
> +			loff_t offset = vma_vaddr_to_offset(vma, bp_vaddr);
>  
>  			uprobe = find_uprobe(inode, offset);
>  		}
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 3/8] Uprobe: Rename map_info to uprobe_map_info
From: Jerome Glisse @ 2018-03-13 20:39 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: mhiramat, oleg, peterz, srikar, acme, ananth, akpm,
	alexander.shishkin, alexis.berlemont, corbet, dan.j.williams,
	gregkh, huawei.libin, hughd, jack, jolsa, kan.liang,
	kirill.shutemov, kjlx, kstewart, linux-doc, linux-kernel,
	linux-mm, mhocko, milian.wolff, mingo, namhyung, naveen.n.rao, pc,
	pombredanne, rostedt, tglx, tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-4-ravi.bangoria@linux.vnet.ibm.com>

On Tue, Mar 13, 2018 at 06:25:58PM +0530, Ravi Bangoria wrote:
> map_info is very generic name, rename it to uprobe_map_info.
> Renaming will help to export this structure outside of the
> file.
> 
> Also rename free_map_info() to uprobe_free_map_info() and
> build_map_info() to uprobe_build_map_info().
> 
> No functionality changes.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Same coccinelle comments as previously.

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  kernel/events/uprobes.c | 32 +++++++++++++++++---------------
>  1 file changed, 17 insertions(+), 15 deletions(-)
> 
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 535fd39..081b88c1 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -695,27 +695,29 @@ static void delete_uprobe(struct uprobe *uprobe)
>  	put_uprobe(uprobe);
>  }
>  
> -struct map_info {
> -	struct map_info *next;
> +struct uprobe_map_info {
> +	struct uprobe_map_info *next;
>  	struct mm_struct *mm;
>  	unsigned long vaddr;
>  };
>  
> -static inline struct map_info *free_map_info(struct map_info *info)
> +static inline struct uprobe_map_info *
> +uprobe_free_map_info(struct uprobe_map_info *info)
>  {
> -	struct map_info *next = info->next;
> +	struct uprobe_map_info *next = info->next;
>  	kfree(info);
>  	return next;
>  }
>  
> -static struct map_info *
> -build_map_info(struct address_space *mapping, loff_t offset, bool is_register)
> +static struct uprobe_map_info *
> +uprobe_build_map_info(struct address_space *mapping, loff_t offset,
> +		      bool is_register)
>  {
>  	unsigned long pgoff = offset >> PAGE_SHIFT;
>  	struct vm_area_struct *vma;
> -	struct map_info *curr = NULL;
> -	struct map_info *prev = NULL;
> -	struct map_info *info;
> +	struct uprobe_map_info *curr = NULL;
> +	struct uprobe_map_info *prev = NULL;
> +	struct uprobe_map_info *info;
>  	int more = 0;
>  
>   again:
> @@ -729,7 +731,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  			 * Needs GFP_NOWAIT to avoid i_mmap_rwsem recursion through
>  			 * reclaim. This is optimistic, no harm done if it fails.
>  			 */
> -			prev = kmalloc(sizeof(struct map_info),
> +			prev = kmalloc(sizeof(struct uprobe_map_info),
>  					GFP_NOWAIT | __GFP_NOMEMALLOC | __GFP_NOWARN);
>  			if (prev)
>  				prev->next = NULL;
> @@ -762,7 +764,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  	}
>  
>  	do {
> -		info = kmalloc(sizeof(struct map_info), GFP_KERNEL);
> +		info = kmalloc(sizeof(struct uprobe_map_info), GFP_KERNEL);
>  		if (!info) {
>  			curr = ERR_PTR(-ENOMEM);
>  			goto out;
> @@ -774,7 +776,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  	goto again;
>   out:
>  	while (prev)
> -		prev = free_map_info(prev);
> +		prev = uprobe_free_map_info(prev);
>  	return curr;
>  }
>  
> @@ -782,11 +784,11 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  register_for_each_vma(struct uprobe *uprobe, struct uprobe_consumer *new)
>  {
>  	bool is_register = !!new;
> -	struct map_info *info;
> +	struct uprobe_map_info *info;
>  	int err = 0;
>  
>  	percpu_down_write(&dup_mmap_sem);
> -	info = build_map_info(uprobe->inode->i_mapping,
> +	info = uprobe_build_map_info(uprobe->inode->i_mapping,
>  					uprobe->offset, is_register);
>  	if (IS_ERR(info)) {
>  		err = PTR_ERR(info);
> @@ -825,7 +827,7 @@ static inline struct map_info *free_map_info(struct map_info *info)
>  		up_write(&mm->mmap_sem);
>   free:
>  		mmput(mm);
> -		info = free_map_info(info);
> +		info = uprobe_free_map_info(info);
>  	}
>   out:
>  	percpu_up_write(&dup_mmap_sem);
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 4/8] Uprobe: Export uprobe_map_info along with uprobe_{build/free}_map_info()
From: Jerome Glisse @ 2018-03-13 20:40 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: mhiramat, oleg, peterz, srikar, acme, ananth, akpm,
	alexander.shishkin, alexis.berlemont, corbet, dan.j.williams,
	gregkh, huawei.libin, hughd, jack, jolsa, kan.liang,
	kirill.shutemov, kjlx, kstewart, linux-doc, linux-kernel,
	linux-mm, mhocko, milian.wolff, mingo, namhyung, naveen.n.rao, pc,
	pombredanne, rostedt, tglx, tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-5-ravi.bangoria@linux.vnet.ibm.com>

On Tue, Mar 13, 2018 at 06:25:59PM +0530, Ravi Bangoria wrote:
> These exported data structure and functions will be used by other
> files in later patches.
> 
> No functionality changes.
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>

Reviewed-by: Jérôme Glisse <jglisse@redhat.com>

> ---
>  include/linux/uprobes.h |  9 +++++++++
>  kernel/events/uprobes.c | 14 +++-----------
>  2 files changed, 12 insertions(+), 11 deletions(-)
> 
> diff --git a/include/linux/uprobes.h b/include/linux/uprobes.h
> index 0a294e9..7bd2760 100644
> --- a/include/linux/uprobes.h
> +++ b/include/linux/uprobes.h
> @@ -109,12 +109,19 @@ enum rp_check {
>  	RP_CHECK_RET,
>  };
>  
> +struct address_space;
>  struct xol_area;
>  
>  struct uprobes_state {
>  	struct xol_area		*xol_area;
>  };
>  
> +struct uprobe_map_info {
> +	struct uprobe_map_info *next;
> +	struct mm_struct *mm;
> +	unsigned long vaddr;
> +};
> +
>  extern int set_swbp(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
>  extern int set_orig_insn(struct arch_uprobe *aup, struct mm_struct *mm, unsigned long vaddr);
>  extern bool is_swbp_insn(uprobe_opcode_t *insn);
> @@ -149,6 +156,8 @@ struct uprobes_state {
>  extern bool arch_uprobe_ignore(struct arch_uprobe *aup, struct pt_regs *regs);
>  extern void arch_uprobe_copy_ixol(struct page *page, unsigned long vaddr,
>  					 void *src, unsigned long len);
> +extern struct uprobe_map_info *uprobe_free_map_info(struct uprobe_map_info *info);
> +extern struct uprobe_map_info *uprobe_build_map_info(struct address_space *mapping, loff_t offset, bool is_register);
>  #else /* !CONFIG_UPROBES */
>  struct uprobes_state {
>  };
> diff --git a/kernel/events/uprobes.c b/kernel/events/uprobes.c
> index 081b88c1..e7830b8 100644
> --- a/kernel/events/uprobes.c
> +++ b/kernel/events/uprobes.c
> @@ -695,23 +695,15 @@ static void delete_uprobe(struct uprobe *uprobe)
>  	put_uprobe(uprobe);
>  }
>  
> -struct uprobe_map_info {
> -	struct uprobe_map_info *next;
> -	struct mm_struct *mm;
> -	unsigned long vaddr;
> -};
> -
> -static inline struct uprobe_map_info *
> -uprobe_free_map_info(struct uprobe_map_info *info)
> +struct uprobe_map_info *uprobe_free_map_info(struct uprobe_map_info *info)
>  {
>  	struct uprobe_map_info *next = info->next;
>  	kfree(info);
>  	return next;
>  }
>  
> -static struct uprobe_map_info *
> -uprobe_build_map_info(struct address_space *mapping, loff_t offset,
> -		      bool is_register)
> +struct uprobe_map_info *uprobe_build_map_info(struct address_space *mapping,
> +					      loff_t offset, bool is_register)
>  {
>  	unsigned long pgoff = offset >> PAGE_SHIFT;
>  	struct vm_area_struct *vma;
> -- 
> 1.8.3.1
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH] doc: fs: sysfs-pci: remove trailing whitespace
From: Kenny Ballou @ 2018-03-13 22:02 UTC (permalink / raw)
  To: linux-doc, corbet; +Cc: Kenny Ballou

Remove the trailing whitespace found in
`Documentation/filesystems/sysfs-pci.txt`.

Signed-off-by: Kenny Ballou <kballou@devnulllabs.io>
---
 Documentation/filesystems/sysfs-pci.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/sysfs-pci.txt b/Documentation/filesystems/sysfs-pci.txt
index 06f1d64c6f70..093bb0fa630f 100644
--- a/Documentation/filesystems/sysfs-pci.txt
+++ b/Documentation/filesystems/sysfs-pci.txt
@@ -67,11 +67,11 @@ don't support mmapping of certain resources, so be sure to check the return
 value from any attempted mmap.  The most notable of these are I/O port
 resources, which also provide read/write access.
 
-The 'enable' file provides a counter that indicates how many times the device 
+The 'enable' file provides a counter that indicates how many times the device
 has been enabled.  If the 'enable' file currently returns '4', and a '1' is
 echoed into it, it will then return '5'.  Echoing a '0' into it will decrease
 the count.  Even when it returns to 0, though, some of the initialisation
-may not be reversed.  
+may not be reversed.
 
 The 'rom' file is special in that it provides read-only access to the device's
 ROM file, if available.  It's disabled by default, however, so applications
-- 
2.16.1

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

^ permalink raw reply related

* [PATCH] doc: sysfs-pci: update enable 'rom' file instructions
From: Kenny Ballou @ 2018-03-13 21:58 UTC (permalink / raw)
  To: linux-doc; +Cc: bhelgaas, corbet, googlegot, Kenny Ballou

Update the instructions for enabling and disabling read access to the
special sysfs 'rom' file for PCI devices.  The function that interprets
user data for enabling and disabling the device actually expects _at
least_ 2 bytes, more than what is expressly implied by "1" or "0".

It should also be noted, that the code path for enabling read access to
the special file can receive any length of input as long as it doesn't
start with "0".  However, this is not added to the documentation as this
is, perhaps, not a "feature" of the file system we want to encourage.

This patch is submitted in place of accepting [pci: use kstrtobool over
ad hoc string parsing][0] as to not break existing functionality.
Similarly, this patch is in reference to a bugzilla report: [Sysfs PCI
rom file functionality does not match documentation][1].

[0]: https://www.spinics.net/lists/linux-pci/msg69073.html
[1]: https://bugzilla.kernel.org/show_bug.cgi?id=111301

Reported-by: googlegot@yahoo.com
Signed-off-by: Kenny Ballou <kballou@devnulllabs.io>
---
 Documentation/filesystems/sysfs-pci.txt | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Documentation/filesystems/sysfs-pci.txt b/Documentation/filesystems/sysfs-pci.txt
index 06f1d64c6f70..0cc9a32ebab0 100644
--- a/Documentation/filesystems/sysfs-pci.txt
+++ b/Documentation/filesystems/sysfs-pci.txt
@@ -75,8 +75,8 @@ may not be reversed.
 
 The 'rom' file is special in that it provides read-only access to the device's
 ROM file, if available.  It's disabled by default, however, so applications
-should write the string "1" to the file to enable it before attempting a read
-call, and disable it following the access by writing "0" to the file.  Note
+should write the string "1\n" to the file to enable it before attempting a read
+call, and disable it following the access by writing "0\n" to the file.  Note
 that the device must be enabled for a rom read to return data successfully.
 In the event a driver is not bound to the device, it can be enabled using the
 'enable' file, documented above.
-- 
2.16.1

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

^ permalink raw reply related

* Re: [PATCH V4] thermal: Add cooling device's statistics in sysfs
From: Zhang Rui @ 2018-03-14  8:01 UTC (permalink / raw)
  To: Viresh Kumar, Eduardo Valentin
  Cc: Vincent Guittot, linux-doc, linux-kernel, linux-pm
In-Reply-To: <1520924571.3766.8.camel@intel.com>

On 二, 2018-03-13 at 15:02 +0800, Zhang Rui wrote:
> Hi, Viresh,
> 
> I will queue it for 4.17, with just one minor fix below.
> 
I got the following warning from checkpatch.pl
---------------

WARNING: please write a paragraph that describes the config symbol
fully
#147: FILE: drivers/thermal/Kconfig:18:
+config THERMAL_STATISTICS

WARNING: Consider renaming function(s)
'thermal_cooling_device_total_trans_show' to 'total_trans_show'
#391: FILE: drivers/thermal/thermal_sysfs.c:901:
+}

WARNING: Consider renaming function(s)
'thermal_cooling_device_time_in_state_show' to 'time_in_state_show'
#395: FILE: drivers/thermal/thermal_sysfs.c:905:
+static DEVICE_ATTR(time_in_state, 0444,

WARNING: Consider renaming function(s)
'thermal_cooling_device_reset_store' to 'reset_store'
#397: FILE: drivers/thermal/thermal_sysfs.c:907:
+static DEVICE_ATTR(reset, 0200, NULL,
thermal_cooling_device_reset_store);

WARNING: Consider renaming function(s)
'thermal_cooling_device_trans_table_show' to 'trans_table_show'
#398: FILE: drivers/thermal/thermal_sysfs.c:908:
+static DEVICE_ATTR(trans_table, 0444,

total: 0 errors, 5 warnings, 366 lines checked


I'm okay with the first one because the description does not have to be
larger than 3 lines.
the last 4 warnings makes sense to me. I think we should rename the
function and use DEVICE_ATTR_RO() and DEVICE_ATTR_WO() instead.

what do you think?

thanks,
rui

> On 二, 2018-01-16 at 15:22 +0530, Viresh Kumar wrote:
> > 
> > This extends the sysfs interface for thermal cooling devices and
> > exposes
> > some pretty useful statistics. These statistics have proven to be
> > quite
> > useful specially while doing benchmarks related to the task
> > scheduler,
> > where we want to make sure that nothing has disrupted the test,
> > specially the cooling device which may have put constraints on the
> > CPUs.
> > The information exposed here tells us to what extent the CPUs were
> > constrained by the thermal framework.
> > 
> > The write-only "reset" file is used to reset the statistics.
> > 
> > The read-only "time_in_state" file shows the clock_t time spent by
> > the
> > device in the respective cooling states, and it prints one line per
> > cooling state.
> > 
> > The read-only "total_trans" file shows single positive integer
> > value
> > showing the total number of cooling state transitions the device
> > has
> > gone through since the time the cooling device is registered or the
> > time
> > when statistics were reset last.
> > 
> > The read-only "trans_table" file shows a two dimensional matrix,
> > where
> > an entry <i,j> (row i, column j) represents the number of
> > transitions
> > from State_i to State_j.
> > 
> > This is how the directory structure looks like for a single cooling
> > device:
> > 
> > $ ls -R /sys/class/thermal/cooling_device0/
> > /sys/class/thermal/cooling_device0/:
> > cur_state  max_state  power  stats  subsystem  type  uevent
> > 
> > /sys/class/thermal/cooling_device0/power:
> > autosuspend_delay_ms  runtime_active_time  runtime_suspended_time
> > control               runtime_status
> > 
> > /sys/class/thermal/cooling_device0/stats:
> > reset  time_in_state  total_trans  trans_table
> > 
> > This is tested on ARM 64-bit Hisilicon hikey620 board running
> > Ubuntu
> > and
> > ARM 64-bit Hisilicon hikey960 board running Android.
> > 
> > Signed-off-by: Viresh Kumar <viresh.kumar@linaro.org>
> [snip]
> 
> > 
> > +static void cooling_device_stats_setup(struct
> > thermal_cooling_device
> > *cdev)
> > +{
> > +	struct cooling_dev_stats *stats;
> > +	unsigned long states;
> > +	int var;
> > +
> > +	if (cdev->ops->get_max_state(cdev, &states))
> > +		return;
> > +
> > +	states++; /* Total number of states is highest state + 1
> > */
> > +
> > +	var = sizeof(*stats);
> > +	var += sizeof(*stats->time_in_state) * states;
> > +	var += sizeof(*stats->trans_table) * states * states;
> > +
> > +	stats = kzalloc(var, GFP_KERNEL);
> > +	if (!stats)
> > +		return;
> > +
> > +	stats->time_in_state = (ktime_t *)(stats + 1);
> > +	stats->trans_table = (unsigned int *)(stats->time_in_state 
> > +
> > states);
> > +	cdev->stats = stats;
> > +	stats->last_time = ktime_get();
> > +	stats->max_states = states;
> > +	cdev->stats = stats;
> > +
> cdev->stats is set twice here, I will remove the first one.
> 
> thanks,
> rui
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH V4] thermal: Add cooling device's statistics in sysfs
From: Viresh Kumar @ 2018-03-14  8:14 UTC (permalink / raw)
  To: Zhang Rui
  Cc: Eduardo Valentin, Vincent Guittot, linux-doc, linux-kernel,
	linux-pm
In-Reply-To: <1521014472.2087.32.camel@intel.com>

On 14-03-18, 16:01, Zhang Rui wrote:
> WARNING: please write a paragraph that describes the config symbol
> fully
> #147: FILE: drivers/thermal/Kconfig:18:
> +config THERMAL_STATISTICS
> 
> WARNING: Consider renaming function(s)
> 'thermal_cooling_device_total_trans_show' to 'total_trans_show'
> #391: FILE: drivers/thermal/thermal_sysfs.c:901:
> +}
> 
> WARNING: Consider renaming function(s)
> 'thermal_cooling_device_time_in_state_show' to 'time_in_state_show'
> #395: FILE: drivers/thermal/thermal_sysfs.c:905:
> +static DEVICE_ATTR(time_in_state, 0444,
> 
> WARNING: Consider renaming function(s)
> 'thermal_cooling_device_reset_store' to 'reset_store'
> #397: FILE: drivers/thermal/thermal_sysfs.c:907:
> +static DEVICE_ATTR(reset, 0200, NULL,
> thermal_cooling_device_reset_store);
> 
> WARNING: Consider renaming function(s)
> 'thermal_cooling_device_trans_table_show' to 'trans_table_show'
> #398: FILE: drivers/thermal/thermal_sysfs.c:908:
> +static DEVICE_ATTR(trans_table, 0444,
> 
> total: 0 errors, 5 warnings, 366 lines checked
> 
> 
> I'm okay with the first one because the description does not have to be
> larger than 3 lines.

Right.

> the last 4 warnings makes sense to me. I think we should rename the
> function and use DEVICE_ATTR_RO() and DEVICE_ATTR_WO() instead.
> 
> what do you think?

I got those warnings as well, and I quietly ignored them :)

I ignored the renaming part for the sake of consistency. The other existing
routines for similar purpose are named as:

thermal_cooling_device_type_show
thermal_cooling_device_max_state_show
thermal_cooling_device_cur_state_show
thermal_cooling_device_cur_state_store

for me it made more sense to follow that naming convention. And I didn't use the
_RO and _WO variants for the same reason.

Now here is what I propose now:

- You apply this patch as-is and ignore the warning.

- I will send few patches on top of that to do:
  - renaming of all such routines to shorter versions.
  - Use the _RO or _WO variants of the macro everywhere.

What do you say ?

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

^ permalink raw reply

* Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)
From: Masami Hiramatsu @ 2018-03-14 13:48 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-6-ravi.bangoria@linux.vnet.ibm.com>

Hi Ravi,

On Tue, 13 Mar 2018 18:26:00 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> Userspace Statically Defined Tracepoints[1] are dtrace style markers
> inside userspace applications. These markers are added by developer at
> important places in the code. Each marker source expands to a single
> nop instruction in the compiled code but there may be additional
> overhead for computing the marker arguments which expands to couple of
> instructions. In case the overhead is more, execution of it can be
> ommited by runtime if() condition when no one is tracing on the marker:
> 
>     if (reference_counter > 0) {
>         Execute marker instructions;
>     }
> 
> Default value of reference counter is 0. Tracer has to increment the
> reference counter before tracing on a marker and decrement it when
> done with the tracing.
> 
> Implement the reference counter logic in trace_uprobe, leaving core
> uprobe infrastructure as is, except one new callback from uprobe_mmap()
> to trace_uprobe.
> 
> trace_uprobe definition with reference counter will now be:
> 
>   <path>:<offset>[(ref_ctr_offset)]

Would you mean 
<path>:<offset>(<ref_ctr_offset>)
?

or use "[]" for delimiter?

Since,

> @@ -454,6 +458,26 @@ static int create_trace_uprobe(int argc, char **argv)
>  		goto fail_address_parse;
>  	}
>  
> +	/* Parse reference counter offset if specified. */
> +	rctr = strchr(arg, '(');

This seems you choose "()" for delimiter.

> +	if (rctr) {
> +		rctr_end = strchr(arg, ')');

		rctr_end = strchr(rctr, ')');

? since we are sure rctr != NULL.

> +		if (rctr > rctr_end || *(rctr_end + 1) != 0) {
> +			ret = -EINVAL;
> +			pr_info("Invalid reference counter offset.\n");
> +			goto fail_address_parse;
> +		}


Also

> +
> +		*rctr++ = 0;
> +		*rctr_end = 0;

Please consider to use '\0' for nul;

Thanks,



> +		ret = kstrtoul(rctr, 0, &ref_ctr_offset);
> +		if (ret) {
> +			pr_info("Invalid reference counter offset.\n");
> +			goto fail_address_parse;
> +		}
> +	}
> +
> +	/* Parse uprobe offset. */
>  	ret = kstrtoul(arg, 0, &offset);
>  	if (ret)
>  		goto fail_address_parse;
> @@ -488,6 +512,7 @@ static int create_trace_uprobe(int argc, char **argv)
>  		goto fail_address_parse;
>  	}
>  	tu->offset = offset;
> +	tu->ref_ctr_offset = ref_ctr_offset;
>  	tu->inode = inode;
>  	tu->filename = kstrdup(filename, GFP_KERNEL);
>  
> @@ -620,6 +645,8 @@ static int probes_seq_show(struct seq_file *m, void *v)
>  			break;
>  		}
>  	}
> +	if (tu->ref_ctr_offset)
> +		seq_printf(m, "(0x%lx)", tu->ref_ctr_offset);
>  
>  	for (i = 0; i < tu->tp.nr_args; i++)
>  		seq_printf(m, " %s=%s", tu->tp.args[i].name, tu->tp.args[i].comm);
> @@ -894,6 +921,139 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
>  	return trace_handle_return(s);
>  }
>  
> +static bool sdt_valid_vma(struct trace_uprobe *tu, struct vm_area_struct *vma)
> +{
> +	unsigned long vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> +
> +	return tu->ref_ctr_offset &&
> +		vma->vm_file &&
> +		file_inode(vma->vm_file) == tu->inode &&
> +		vma->vm_flags & VM_WRITE &&
> +		vma->vm_start <= vaddr &&
> +		vma->vm_end > vaddr;
> +}
> +
> +static struct vm_area_struct *
> +sdt_find_vma(struct mm_struct *mm, struct trace_uprobe *tu)
> +{
> +	struct vm_area_struct *tmp;
> +
> +	for (tmp = mm->mmap; tmp != NULL; tmp = tmp->vm_next)
> +		if (sdt_valid_vma(tu, tmp))
> +			return tmp;
> +
> +	return NULL;
> +}
> +
> +/*
> + * Reference count gate the invocation of probe. If present,
> + * by default reference count is 0. One needs to increment
> + * it before tracing the probe and decrement it when done.
> + */
> +static int
> +sdt_update_ref_ctr(struct mm_struct *mm, unsigned long vaddr, short d)
> +{
> +	void *kaddr;
> +	struct page *page;
> +	struct vm_area_struct *vma;
> +	int ret = 0;
> +	unsigned short orig = 0;
> +
> +	if (vaddr == 0)
> +		return -EINVAL;
> +
> +	ret = get_user_pages_remote(NULL, mm, vaddr, 1,
> +		FOLL_FORCE | FOLL_WRITE, &page, &vma, NULL);
> +	if (ret <= 0)
> +		return ret;
> +
> +	kaddr = kmap_atomic(page);
> +	memcpy(&orig, kaddr + (vaddr & ~PAGE_MASK), sizeof(orig));
> +	orig += d;
> +	memcpy(kaddr + (vaddr & ~PAGE_MASK), &orig, sizeof(orig));
> +	kunmap_atomic(kaddr);
> +
> +	put_page(page);
> +	return 0;
> +}
> +
> +static void sdt_increment_ref_ctr(struct trace_uprobe *tu)
> +{
> +	struct uprobe_map_info *info;
> +	struct vm_area_struct *vma;
> +	unsigned long vaddr;
> +
> +	uprobe_start_dup_mmap();
> +	info = uprobe_build_map_info(tu->inode->i_mapping,
> +				tu->ref_ctr_offset, false);
> +	if (IS_ERR(info))
> +		goto out;
> +
> +	while (info) {
> +		down_write(&info->mm->mmap_sem);
> +
> +		vma = sdt_find_vma(info->mm, tu);
> +		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> +		sdt_update_ref_ctr(info->mm, vaddr, 1);
> +
> +		up_write(&info->mm->mmap_sem);
> +		mmput(info->mm);
> +		info = uprobe_free_map_info(info);
> +	}
> +
> +out:
> +	uprobe_end_dup_mmap();
> +}
> +
> +/* Called with down_write(&vma->vm_mm->mmap_sem) */
> +void trace_uprobe_mmap_callback(struct vm_area_struct *vma)
> +{
> +	struct trace_uprobe *tu;
> +	unsigned long vaddr;
> +
> +	if (!(vma->vm_flags & VM_WRITE))
> +		return;
> +
> +	mutex_lock(&uprobe_lock);
> +	list_for_each_entry(tu, &uprobe_list, list) {
> +		if (!sdt_valid_vma(tu, vma) ||
> +		    !trace_probe_is_enabled(&tu->tp))
> +			continue;
> +
> +		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> +		sdt_update_ref_ctr(vma->vm_mm, vaddr, 1);
> +	}
> +	mutex_unlock(&uprobe_lock);
> +}
> +
> +static void sdt_decrement_ref_ctr(struct trace_uprobe *tu)
> +{
> +	struct vm_area_struct *vma;
> +	unsigned long vaddr;
> +	struct uprobe_map_info *info;
> +
> +	uprobe_start_dup_mmap();
> +	info = uprobe_build_map_info(tu->inode->i_mapping,
> +				tu->ref_ctr_offset, false);
> +	if (IS_ERR(info))
> +		goto out;
> +
> +	while (info) {
> +		down_write(&info->mm->mmap_sem);
> +
> +		vma = sdt_find_vma(info->mm, tu);
> +		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> +		sdt_update_ref_ctr(info->mm, vaddr, -1);
> +
> +		up_write(&info->mm->mmap_sem);
> +		mmput(info->mm);
> +		info = uprobe_free_map_info(info);
> +	}
> +
> +out:
> +	uprobe_end_dup_mmap();
> +}
> +
>  typedef bool (*filter_func_t)(struct uprobe_consumer *self,
>  				enum uprobe_filter_ctx ctx,
>  				struct mm_struct *mm);
> @@ -939,6 +1099,9 @@ typedef bool (*filter_func_t)(struct uprobe_consumer *self,
>  	if (ret)
>  		goto err_buffer;
>  
> +	if (tu->ref_ctr_offset)
> +		sdt_increment_ref_ctr(tu);
> +
>  	return 0;
>  
>   err_buffer:
> @@ -979,6 +1142,9 @@ typedef bool (*filter_func_t)(struct uprobe_consumer *self,
>  
>  	WARN_ON(!uprobe_filter_is_empty(&tu->filter));
>  
> +	if (tu->ref_ctr_offset)
> +		sdt_decrement_ref_ctr(tu);
> +
>  	uprobe_unregister(tu->inode, tu->offset, &tu->consumer);
>  	tu->tp.flags &= file ? ~TP_FLAG_TRACE : ~TP_FLAG_PROFILE;
>  
> @@ -1423,6 +1589,8 @@ static __init int init_uprobe_trace(void)
>  	/* Profile interface */
>  	trace_create_file("uprobe_profile", 0444, d_tracer,
>  				    NULL, &uprobe_profile_ops);
> +
> +	uprobe_mmap_callback = trace_uprobe_mmap_callback;
>  	return 0;
>  }
>  
> -- 
> 1.8.3.1
> 


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

^ permalink raw reply

* Re: [PATCH 8/8] trace_uprobe/sdt: Document about reference counter
From: Masami Hiramatsu @ 2018-03-14 13:50 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-9-ravi.bangoria@linux.vnet.ibm.com>

On Tue, 13 Mar 2018 18:26:03 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> No functionality changes.

Please consider to describe what is this change and why, here.

> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  Documentation/trace/uprobetracer.txt | 16 +++++++++++++---
>  kernel/trace/trace.c                 |  2 +-
>  2 files changed, 14 insertions(+), 4 deletions(-)
> 
> diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
> index bf526a7c..8fb13b0 100644
> --- a/Documentation/trace/uprobetracer.txt
> +++ b/Documentation/trace/uprobetracer.txt
> @@ -19,15 +19,25 @@ user to calculate the offset of the probepoint in the object.
>  
>  Synopsis of uprobe_tracer
>  -------------------------
> -  p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe
> -  r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe)
> -  -:[GRP/]EVENT                           : Clear uprobe or uretprobe event
> +  p[:[GRP/]EVENT] PATH:OFFSET[(REF_CTR_OFFSET)] [FETCHARGS]
> +  r[:[GRP/]EVENT] PATH:OFFSET[(REF_CTR_OFFSET)] [FETCHARGS]

Ah, OK in this context, [] means optional syntax :)

> +  -:[GRP/]EVENT
> +
> +  p : Set a uprobe
> +  r : Set a return uprobe (uretprobe)
> +  - : Clear uprobe or uretprobe event
>  
>    GRP           : Group name. If omitted, "uprobes" is the default value.
>    EVENT         : Event name. If omitted, the event name is generated based
>                    on PATH+OFFSET.
>    PATH          : Path to an executable or a library.
>    OFFSET        : Offset where the probe is inserted.
> +  REF_CTR_OFFSET: Reference counter offset. Optional field. Reference count
> +                  gate the invocation of probe. If present, by default
> +                  reference count is 0. Kernel needs to increment it before
> +                  tracing the probe and decrement it when done. This is
> +                  identical to semaphore in Userspace Statically Defined
> +                  Tracepoints (USDT).
>  
>    FETCHARGS     : Arguments. Each probe can have up to 128 args.
>     %REG         : Fetch register REG
> diff --git a/kernel/trace/trace.c b/kernel/trace/trace.c
> index 20a2300..2104d03 100644
> --- a/kernel/trace/trace.c
> +++ b/kernel/trace/trace.c
> @@ -4604,7 +4604,7 @@ static int tracing_trace_options_open(struct inode *inode, struct file *file)
>    "place (kretprobe): [<module>:]<symbol>[+<offset>]|<memaddr>\n"
>  #endif
>  #ifdef CONFIG_UPROBE_EVENTS
> -	"\t    place: <path>:<offset>\n"
> +  "   place (uprobe): <path>:<offset>[(ref_ctr_offset)]\n"
>  #endif
>  	"\t     args: <name>=fetcharg[:type]\n"
>  	"\t fetcharg: %<register>, @<address>, @<symbol>[+|-<offset>],\n"
> -- 
> 1.8.3.1
> 


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

^ permalink raw reply

* Re: [PATCH 7/8] perf probe: Support SDT markers having reference counter (semaphore)
From: Masami Hiramatsu @ 2018-03-14 14:09 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-8-ravi.bangoria@linux.vnet.ibm.com>

Hi Ravi,

This code logic looks good. I just have several small comments for style.

On Tue, 13 Mar 2018 18:26:02 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
> index e1dbc98..2cbe68a 100644
> --- a/tools/perf/util/probe-event.c
> +++ b/tools/perf/util/probe-event.c
> @@ -1832,6 +1832,12 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev)
>  			tp->offset = strtoul(fmt2_str, NULL, 10);
>  	}
>  
> +	if (tev->uprobes) {
> +		fmt2_str = strchr(p, '(');
> +		if (fmt2_str)
> +			tp->ref_ctr_offset = strtoul(fmt2_str + 1, NULL, 0);
> +	}
> +
>  	tev->nargs = argc - 2;
>  	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
>  	if (tev->args == NULL) {
> @@ -2054,15 +2060,22 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>  	}
>  
>  	/* Use the tp->address for uprobes */
> -	if (tev->uprobes)
> -		err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
> -	else if (!strncmp(tp->symbol, "0x", 2))
> +	if (tev->uprobes) {
> +		if (tp->ref_ctr_offset)
> +			err = strbuf_addf(&buf, "%s:0x%lx(0x%lx)", tp->module,
> +					  tp->address, tp->ref_ctr_offset);
> +		else
> +			err = strbuf_addf(&buf, "%s:0x%lx", tp->module,
> +					  tp->address);
> +	} else if (!strncmp(tp->symbol, "0x", 2)) {
>  		/* Absolute address. See try_to_find_absolute_address() */
>  		err = strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
>  				  tp->module ? ":" : "", tp->address);
> -	else
> +	} else {
>  		err = strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
>  				tp->module ? ":" : "", tp->symbol, tp->offset);
> +	}

What the purpose of this {}?

> +
>  	if (err)
>  		goto error;
>  
> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
> index 45b14f0..15a98c3 100644
> --- a/tools/perf/util/probe-event.h
> +++ b/tools/perf/util/probe-event.h
> @@ -27,6 +27,7 @@ struct probe_trace_point {
>  	char		*symbol;	/* Base symbol */
>  	char		*module;	/* Module name */
>  	unsigned long	offset;		/* Offset from symbol */
> +	unsigned long	ref_ctr_offset;	/* SDT reference counter offset */
>  	unsigned long	address;	/* Actual address of the trace point */
>  	bool		retprobe;	/* Return probe flag */
>  };
> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
> index 4ae1123..08ba3a6 100644
> --- a/tools/perf/util/probe-file.c
> +++ b/tools/perf/util/probe-file.c
> @@ -701,6 +701,12 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
>  		 : (unsigned long long)note->addr.a64[0];
>  }
>  
> +static unsigned long long sdt_note__get_ref_ctr_offset(struct sdt_note *note)
> +{
> +	return note->bit32 ? (unsigned long long)note->addr.a32[2]
> +		: (unsigned long long)note->addr.a64[2];
> +}

Could you please introduce an enum for specifying the index by name?

e.g.
enum {
	SDT_NOTE_IDX_ADDR = 0,
	SDT_NOTE_IDX_REFCTR = 2,
};

> +
>  static const char * const type_to_suffix[] = {
>  	":s64", "", "", "", ":s32", "", ":s16", ":s8",
>  	"", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
> @@ -776,14 +782,24 @@ static char *synthesize_sdt_probe_command(struct sdt_note *note,
>  {
>  	struct strbuf buf;
>  	char *ret = NULL, **args;
> -	int i, args_count;
> +	int i, args_count, err;
> +	unsigned long long ref_ctr_offset;
>  
>  	if (strbuf_init(&buf, 32) < 0)
>  		return NULL;
>  
> -	if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> +	ref_ctr_offset = sdt_note__get_ref_ctr_offset(note);
> +
> +	if (ref_ctr_offset)
> +		err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx(0x%llx)",
>  				sdtgrp, note->name, pathname,
> -				sdt_note__get_addr(note)) < 0)
> +				sdt_note__get_addr(note), ref_ctr_offset);
> +	else
> +		err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> +				sdtgrp, note->name, pathname,
> +				sdt_note__get_addr(note));

This can be minimized (and avoid repeating) by using 2 strbuf_addf()s, like

	err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
			sdtgrp, note->name, pathname,
			sdt_note__get_addr(note));
	if (ref_ctr_offset && !err < 0)
		err = strbuf_addf("(0x%llx)", ref_ctr_offset);


> +
> +	if (err < 0)
>  		goto error;
>  
>  	if (!note->args)
> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
> index 2de7705..76c7b54 100644
> --- a/tools/perf/util/symbol-elf.c
> +++ b/tools/perf/util/symbol-elf.c
> @@ -1928,6 +1928,16 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
>  		}
>  	}
>  
> +	/* Adjust reference counter offset */
> +	if (elf_section_by_name(*elf, &ehdr, &shdr, SDT_PROBES_SCN, NULL)) {
> +		if (shdr.sh_offset) {
> +			if (tmp->bit32)
> +				tmp->addr.a32[2] -= (shdr.sh_addr - shdr.sh_offset);
> +			else
> +				tmp->addr.a64[2] -= (shdr.sh_addr - shdr.sh_offset);

Here we should use enum above too.

Thank you,

> +		}
> +	}
> +
>  	list_add_tail(&tmp->note_list, sdt_notes);
>  	return 0;
>  
> diff --git a/tools/perf/util/symbol.h b/tools/perf/util/symbol.h
> index 70c16741..ad0c4f2 100644
> --- a/tools/perf/util/symbol.h
> +++ b/tools/perf/util/symbol.h
> @@ -384,6 +384,7 @@ struct sdt_note {
>  int cleanup_sdt_note_list(struct list_head *sdt_notes);
>  int sdt_notes__get_count(struct list_head *start);
>  
> +#define SDT_PROBES_SCN ".probes"
>  #define SDT_BASE_SCN ".stapsdt.base"
>  #define SDT_NOTE_SCN  ".note.stapsdt"
>  #define SDT_NOTE_TYPE 3
> -- 
> 1.8.3.1
> 


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

^ permalink raw reply

* Re: [PATCH 6/8] trace_uprobe/sdt: Fix multiple update of same reference counter
From: Masami Hiramatsu @ 2018-03-14 14:15 UTC (permalink / raw)
  To: Ravi Bangoria
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu
In-Reply-To: <20180313125603.19819-7-ravi.bangoria@linux.vnet.ibm.com>

On Tue, 13 Mar 2018 18:26:01 +0530
Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:

> For tiny binaries/libraries, different mmap regions points to the
> same file portion. In such cases, we may increment reference counter
> multiple times. But while de-registration, reference counter will get
> decremented only by once leaving reference counter > 0 even if no one
> is tracing on that marker.
> 
> Ensure increment and decrement happens in sync by keeping list of
> mms in trace_uprobe. Increment reference counter only if mm is not
> present in the list and decrement only if mm is present in the list.
> 
> Example
> 
>   # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4(0x10036)" > uprobe_events
> 
> Before patch:
> 
>   # perf stat -a -e sdt_tick:loop2
>   # /tmp/tick
>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>    0000000: 02                                       .
> 
>   # pkill perf
>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>   0000000: 01                                       .
> 
> After patch:
> 
>   # perf stat -a -e sdt_tick:loop2
>   # /tmp/tick
>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>   0000000: 01                                       .
> 
>   # pkill perf
>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>   0000000: 00                                       .
> 
> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
> ---
>  kernel/trace/trace_uprobe.c | 105 +++++++++++++++++++++++++++++++++++++++++++-
>  1 file changed, 103 insertions(+), 2 deletions(-)
> 
> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
> index b6c9b48..9bf3f7a 100644
> --- a/kernel/trace/trace_uprobe.c
> +++ b/kernel/trace/trace_uprobe.c
> @@ -50,6 +50,11 @@ struct trace_uprobe_filter {
>  	struct list_head	perf_events;
>  };
>  
> +struct sdt_mm_list {
> +	struct mm_struct *mm;
> +	struct sdt_mm_list *next;
> +};

Oh, please use struct list_head instead of defining your own pointer-chain :(

> +
>  /*
>   * uprobe event core functions
>   */
> @@ -61,6 +66,8 @@ struct trace_uprobe {
>  	char				*filename;
>  	unsigned long			offset;
>  	unsigned long			ref_ctr_offset;
> +	struct sdt_mm_list		*sml;
> +	struct rw_semaphore		sml_rw_sem;

BTW, is there any reason to use rw_semaphore? (mutex doesn't fit?)

Thank you,

>  	unsigned long			nhit;
>  	struct trace_probe		tp;
>  };
> @@ -274,6 +281,7 @@ static inline bool is_ret_probe(struct trace_uprobe *tu)
>  	if (is_ret)
>  		tu->consumer.ret_handler = uretprobe_dispatcher;
>  	init_trace_uprobe_filter(&tu->filter);
> +	init_rwsem(&tu->sml_rw_sem);
>  	return tu;
>  
>  error:
> @@ -921,6 +929,74 @@ static void uretprobe_trace_func(struct trace_uprobe *tu, unsigned long func,
>  	return trace_handle_return(s);
>  }
>  
> +static bool sdt_check_mm_list(struct trace_uprobe *tu, struct mm_struct *mm)
> +{
> +	struct sdt_mm_list *tmp = tu->sml;
> +
> +	if (!tu->sml || !mm)
> +		return false;
> +
> +	while (tmp) {
> +		if (tmp->mm == mm)
> +			return true;
> +		tmp = tmp->next;
> +	}
> +
> +	return false;
> +}
> +
> +static void sdt_add_mm_list(struct trace_uprobe *tu, struct mm_struct *mm)
> +{
> +	struct sdt_mm_list *tmp;
> +
> +	tmp = kzalloc(sizeof(*tmp), GFP_KERNEL);
> +	if (!tmp)
> +		return;
> +
> +	tmp->mm = mm;
> +	tmp->next = tu->sml;
> +	tu->sml = tmp;
> +}
> +
> +static void sdt_del_mm_list(struct trace_uprobe *tu, struct mm_struct *mm)
> +{
> +	struct sdt_mm_list *prev, *curr;
> +
> +	if (!tu->sml)
> +		return;
> +
> +	if (tu->sml->mm == mm) {
> +		curr = tu->sml;
> +		tu->sml = tu->sml->next;
> +		kfree(curr);
> +		return;
> +	}
> +
> +	prev = tu->sml;
> +	curr = tu->sml->next;
> +	while (curr) {
> +		if (curr->mm == mm) {
> +			prev->next = curr->next;
> +			kfree(curr);
> +			return;
> +		}
> +		prev = curr;
> +		curr = curr->next;
> +	}
> +}
> +
> +static void sdt_flush_mm_list(struct trace_uprobe *tu)
> +{
> +	struct sdt_mm_list *next, *curr = tu->sml;
> +
> +	while (curr) {
> +		next = curr->next;
> +		kfree(curr);
> +		curr = next;
> +	}
> +	tu->sml = NULL;
> +}
> +
>  static bool sdt_valid_vma(struct trace_uprobe *tu, struct vm_area_struct *vma)
>  {
>  	unsigned long vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> @@ -989,17 +1065,25 @@ static void sdt_increment_ref_ctr(struct trace_uprobe *tu)
>  	if (IS_ERR(info))
>  		goto out;
>  
> +	down_write(&tu->sml_rw_sem);
>  	while (info) {
> +		if (sdt_check_mm_list(tu, info->mm))
> +			goto cont;
> +
>  		down_write(&info->mm->mmap_sem);
>  
>  		vma = sdt_find_vma(info->mm, tu);
>  		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> -		sdt_update_ref_ctr(info->mm, vaddr, 1);
> +		if (!sdt_update_ref_ctr(info->mm, vaddr, 1))
> +			sdt_add_mm_list(tu, info->mm);
>  
>  		up_write(&info->mm->mmap_sem);
> +
> +cont:
>  		mmput(info->mm);
>  		info = uprobe_free_map_info(info);
>  	}
> +	up_write(&tu->sml_rw_sem);
>  
>  out:
>  	uprobe_end_dup_mmap();
> @@ -1020,8 +1104,16 @@ void trace_uprobe_mmap_callback(struct vm_area_struct *vma)
>  		    !trace_probe_is_enabled(&tu->tp))
>  			continue;
>  
> +		down_write(&tu->sml_rw_sem);
> +		if (sdt_check_mm_list(tu, vma->vm_mm))
> +			goto cont;
> +
>  		vaddr = vma_offset_to_vaddr(vma, tu->ref_ctr_offset);
> -		sdt_update_ref_ctr(vma->vm_mm, vaddr, 1);
> +		if (!sdt_update_ref_ctr(vma->vm_mm, vaddr, 1))
> +			sdt_add_mm_list(tu, vma->vm_mm);
> +
> +cont:
> +		up_write(&tu->sml_rw_sem);
>  	}
>  	mutex_unlock(&uprobe_lock);
>  }
> @@ -1038,7 +1130,11 @@ static void sdt_decrement_ref_ctr(struct trace_uprobe *tu)
>  	if (IS_ERR(info))
>  		goto out;
>  
> +	down_write(&tu->sml_rw_sem);
>  	while (info) {
> +		if (!sdt_check_mm_list(tu, info->mm))
> +			goto cont;
> +
>  		down_write(&info->mm->mmap_sem);
>  
>  		vma = sdt_find_vma(info->mm, tu);
> @@ -1046,9 +1142,14 @@ static void sdt_decrement_ref_ctr(struct trace_uprobe *tu)
>  		sdt_update_ref_ctr(info->mm, vaddr, -1);
>  
>  		up_write(&info->mm->mmap_sem);
> +		sdt_del_mm_list(tu, info->mm);
> +
> +cont:
>  		mmput(info->mm);
>  		info = uprobe_free_map_info(info);
>  	}
> +	sdt_flush_mm_list(tu);
> +	up_write(&tu->sml_rw_sem);
>  
>  out:
>  	uprobe_end_dup_mmap();
> -- 
> 1.8.3.1
> 


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

^ permalink raw reply

* [PATCH 00/16] remove eight obsolete architectures
From: Arnd Bergmann @ 2018-03-14 14:34 UTC (permalink / raw)
  To: linux-arch
  Cc: linux-kernel, Arnd Bergmann, linux-doc, linux-block, linux-ide,
	linux-input, netdev, linux-wireless, linux-pwm, linux-rtc,
	linux-spi, linux-usb, dri-devel, linux-fbdev, linux-watchdog,
	linux-fsdevel, linux-mm

Here is the collection of patches I have applied to my 'asm-generic' tree
on top of the 'metag' removal. This does not include any of the device
drivers, I'll send those separately to a someone different list of people.

The removal came out of a discussion that is now documented at
https://lwn.net/Articles/748074/

Following up from the state described there, I ended up removing the
mn10300, tile, blackfin and cris architectures directly, rather than
waiting, after consulting with the respective maintainers.

However, the unicore32 architecture is no longer part of the removal,
after its maintainer Xuetao Guan said that the port is still actively
being used and that he intends to keep working on it, and that he will
try to provide updated toolchain sources.

In the end, it seems that while the eight architectures are extremely
different, they all suffered the same fate: There was one company in
charge of an SoC line, a CPU microarchitecture and a software ecosystem,
which was more costly than licensing newer off-the-shelf CPU cores from
a third party (typically ARM, MIPS, or RISC-V). It seems that all the
SoC product lines are still around, but have not used the custom CPU
architectures for several years at this point.

      Arnd

Arnd Bergmann (14):
  arch: remove frv port
  arch: remove m32r port
  arch: remove score port
  arch: remove blackfin port
  arch: remove tile port
  procfs: remove CONFIG_HARDWALL dependency
  mm: remove blackfin MPU support
  mm: remove obsolete alloc_remap()
  treewide: simplify Kconfig dependencies for removed archs
  asm-generic: siginfo: remove obsolete #ifdefs
  Documentation: arch-support: remove obsolete architectures
  asm-generic: clean up asm/unistd.h
  recordmcount.pl: drop blackin and tile support
  ktest: remove obsolete architectures

David Howells (1):
  mn10300: Remove the architecture

Jesper Nilsson (1):
  CRIS: Drop support for the CRIS port

Dirstat only (full diffstat is over 100KB):

   6.3% arch/blackfin/mach-bf548/include/mach/
   4.5% arch/blackfin/mach-bf609/include/mach/
  26.3% arch/blackfin/
   4.1% arch/cris/arch-v32/
   5.6% arch/cris/include/arch-v32/arch/hwregs/iop/
   4.1% arch/cris/include/arch-v32/mach-a3/mach/hwregs/
   4.7% arch/cris/include/arch-v32/
   7.8% arch/cris/
   5.6% arch/frv/
   5.5% arch/m32r/
   7.0% arch/mn10300/
   7.6% arch/tile/include/
   6.4% arch/tile/kernel/
   0.0% Documentation/admin-guide/
   0.0% Documentation/blackfin/
   0.0% Documentation/cris/
   0.0% Documentation/devicetree/bindings/cris/
   0.0% Documentation/devicetree/bindings/interrupt-controller/
   2.8% Documentation/features/
   0.5% Documentation/frv/
   0.0% Documentation/ioctl/
   0.0% Documentation/mn10300/
   0.0% Documentation/
   0.0% block/
   0.0% crypto/
   0.0% drivers/ide/
   0.0% drivers/input/joystick/
   0.0% drivers/isdn/hisax/
   0.0% drivers/net/ethernet/davicom/
   0.0% drivers/net/ethernet/smsc/
   0.0% drivers/net/wireless/cisco/
   0.0% drivers/pci/
   0.0% drivers/pwm/
   0.0% drivers/rtc/
   0.0% drivers/spi/
   0.0% drivers/staging/speakup/
   0.0% drivers/usb/musb/
   0.0% drivers/video/console/
   0.0% drivers/watchdog/
   0.0% fs/minix/
   0.0% fs/proc/
   0.0% fs/
   0.0% include/asm-generic/
   0.0% include/linux/
   0.0% include/uapi/asm-generic/
   0.0% init/
   0.0% kernel/
   0.0% lib/
   0.0% mm/
   0.0% samples/blackfin/
   0.0% samples/kprobes/
   0.0% samples/
   0.0% scripts/mod/
   0.0% scripts/
   0.0% tools/arch/frv/include/uapi/asm/
   0.0% tools/arch/m32r/include/uapi/asm/
   0.0% tools/arch/mn10300/include/uapi/asm/
   0.0% tools/arch/score/include/uapi/asm/
   0.0% tools/arch/tile/include/asm/
   0.0% tools/arch/tile/include/uapi/asm/
   0.0% tools/include/asm-generic/
   0.0% tools/scripts/
   0.0% tools/testing/ktest/examples/
   0.0% tools/testing/ktest/

Cc: linux-doc@vger.kernel.org
Cc: linux-kernel@vger.kernel.org
Cc: linux-block@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: netdev@vger.kernel.org
Cc: linux-wireless@vger.kernel.org
Cc: linux-pwm@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: dri-devel@lists.freedesktop.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: linux-fsdevel@vger.kernel.org
Cc: linux-arch@vger.kernel.org
Cc: linux-mm@kvack.org
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* Re: [PATCH 5/8] trace_uprobe: Support SDT markers having reference count (semaphore)
From: Ravi Bangoria @ 2018-03-14 15:12 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180314224809.5ee4c8834bb366faa398e342@kernel.org>

Hi Masami,

On 03/14/2018 07:18 PM, Masami Hiramatsu wrote:
> Hi Ravi,
>
> On Tue, 13 Mar 2018 18:26:00 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> Userspace Statically Defined Tracepoints[1] are dtrace style markers
>> inside userspace applications. These markers are added by developer at
>> important places in the code. Each marker source expands to a single
>> nop instruction in the compiled code but there may be additional
>> overhead for computing the marker arguments which expands to couple of
>> instructions. In case the overhead is more, execution of it can be
>> ommited by runtime if() condition when no one is tracing on the marker:
>>
>>     if (reference_counter > 0) {
>>         Execute marker instructions;
>>     }
>>
>> Default value of reference counter is 0. Tracer has to increment the
>> reference counter before tracing on a marker and decrement it when
>> done with the tracing.
>>
>> Implement the reference counter logic in trace_uprobe, leaving core
>> uprobe infrastructure as is, except one new callback from uprobe_mmap()
>> to trace_uprobe.
>>
>> trace_uprobe definition with reference counter will now be:
>>
>>   <path>:<offset>[(ref_ctr_offset)]
> Would you mean 
> <path>:<offset>(<ref_ctr_offset>)
> ?
>
> or use "[]" for delimiter?

[] indicates optional field.

> Since,
>
>> @@ -454,6 +458,26 @@ static int create_trace_uprobe(int argc, char **argv)
>>  		goto fail_address_parse;
>>  	}
>>  
>> +	/* Parse reference counter offset if specified. */
>> +	rctr = strchr(arg, '(');
> This seems you choose "()" for delimiter.

Correct.

>> +	if (rctr) {
>> +		rctr_end = strchr(arg, ')');
> 		rctr_end = strchr(rctr, ')');
>
> ? since we are sure rctr != NULL.

Yes. we can use rctr instead of arg.

>> +		if (rctr > rctr_end || *(rctr_end + 1) != 0) {
>> +			ret = -EINVAL;
>> +			pr_info("Invalid reference counter offset.\n");
>> +			goto fail_address_parse;
>> +		}
>
> Also
>
>> +
>> +		*rctr++ = 0;
>> +		*rctr_end = 0;
> Please consider to use '\0' for nul;

Sure. Will change it.

Thanks for the review :)
Ravi

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

^ permalink raw reply

* Re: [PATCH 6/8] trace_uprobe/sdt: Fix multiple update of same reference counter
From: Ravi Bangoria @ 2018-03-14 15:15 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180314231540.b98c74a153255f59f54ebc46@kernel.org>



On 03/14/2018 07:45 PM, Masami Hiramatsu wrote:
> On Tue, 13 Mar 2018 18:26:01 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> For tiny binaries/libraries, different mmap regions points to the
>> same file portion. In such cases, we may increment reference counter
>> multiple times. But while de-registration, reference counter will get
>> decremented only by once leaving reference counter > 0 even if no one
>> is tracing on that marker.
>>
>> Ensure increment and decrement happens in sync by keeping list of
>> mms in trace_uprobe. Increment reference counter only if mm is not
>> present in the list and decrement only if mm is present in the list.
>>
>> Example
>>
>>   # echo "p:sdt_tick/loop2 /tmp/tick:0x6e4(0x10036)" > uprobe_events
>>
>> Before patch:
>>
>>   # perf stat -a -e sdt_tick:loop2
>>   # /tmp/tick
>>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>>    0000000: 02                                       .
>>
>>   # pkill perf
>>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>>   0000000: 01                                       .
>>
>> After patch:
>>
>>   # perf stat -a -e sdt_tick:loop2
>>   # /tmp/tick
>>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>>   0000000: 01                                       .
>>
>>   # pkill perf
>>   # dd if=/proc/`pgrep tick`/mem bs=1 count=1 skip=$(( 0x10020036 )) 2>/dev/null | xxd
>>   0000000: 00                                       .
>>
>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> ---
>>  kernel/trace/trace_uprobe.c | 105 +++++++++++++++++++++++++++++++++++++++++++-
>>  1 file changed, 103 insertions(+), 2 deletions(-)
>>
>> diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c
>> index b6c9b48..9bf3f7a 100644
>> --- a/kernel/trace/trace_uprobe.c
>> +++ b/kernel/trace/trace_uprobe.c
>> @@ -50,6 +50,11 @@ struct trace_uprobe_filter {
>>  	struct list_head	perf_events;
>>  };
>>  
>> +struct sdt_mm_list {
>> +	struct mm_struct *mm;
>> +	struct sdt_mm_list *next;
>> +};
> Oh, please use struct list_head instead of defining your own pointer-chain :(

Sure, will change it.

>> +
>>  /*
>>   * uprobe event core functions
>>   */
>> @@ -61,6 +66,8 @@ struct trace_uprobe {
>>  	char				*filename;
>>  	unsigned long			offset;
>>  	unsigned long			ref_ctr_offset;
>> +	struct sdt_mm_list		*sml;
>> +	struct rw_semaphore		sml_rw_sem;
> BTW, is there any reason to use rw_semaphore? (mutex doesn't fit?)

Hmm.. No specific reason.. will use a mutex instead.

Thanks for the review :)
Ravi

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

^ permalink raw reply

* Re: [PATCH 7/8] perf probe: Support SDT markers having reference counter (semaphore)
From: Ravi Bangoria @ 2018-03-14 15:21 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180314230909.52963a161210294ea2fc0420@kernel.org>



On 03/14/2018 07:39 PM, Masami Hiramatsu wrote:
> Hi Ravi,
>
> This code logic looks good. I just have several small comments for style.
>
> On Tue, 13 Mar 2018 18:26:02 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> diff --git a/tools/perf/util/probe-event.c b/tools/perf/util/probe-event.c
>> index e1dbc98..2cbe68a 100644
>> --- a/tools/perf/util/probe-event.c
>> +++ b/tools/perf/util/probe-event.c
>> @@ -1832,6 +1832,12 @@ int parse_probe_trace_command(const char *cmd, struct probe_trace_event *tev)
>>  			tp->offset = strtoul(fmt2_str, NULL, 10);
>>  	}
>>  
>> +	if (tev->uprobes) {
>> +		fmt2_str = strchr(p, '(');
>> +		if (fmt2_str)
>> +			tp->ref_ctr_offset = strtoul(fmt2_str + 1, NULL, 0);
>> +	}
>> +
>>  	tev->nargs = argc - 2;
>>  	tev->args = zalloc(sizeof(struct probe_trace_arg) * tev->nargs);
>>  	if (tev->args == NULL) {
>> @@ -2054,15 +2060,22 @@ char *synthesize_probe_trace_command(struct probe_trace_event *tev)
>>  	}
>>  
>>  	/* Use the tp->address for uprobes */
>> -	if (tev->uprobes)
>> -		err = strbuf_addf(&buf, "%s:0x%lx", tp->module, tp->address);
>> -	else if (!strncmp(tp->symbol, "0x", 2))
>> +	if (tev->uprobes) {
>> +		if (tp->ref_ctr_offset)
>> +			err = strbuf_addf(&buf, "%s:0x%lx(0x%lx)", tp->module,
>> +					  tp->address, tp->ref_ctr_offset);
>> +		else
>> +			err = strbuf_addf(&buf, "%s:0x%lx", tp->module,
>> +					  tp->address);
>> +	} else if (!strncmp(tp->symbol, "0x", 2)) {
>>  		/* Absolute address. See try_to_find_absolute_address() */
>>  		err = strbuf_addf(&buf, "%s%s0x%lx", tp->module ?: "",
>>  				  tp->module ? ":" : "", tp->address);
>> -	else
>> +	} else {
>>  		err = strbuf_addf(&buf, "%s%s%s+%lu", tp->module ?: "",
>>  				tp->module ? ":" : "", tp->symbol, tp->offset);
>> +	}
> What the purpose of this {}?

The starting if has multiple statements and thus it needs braces. So I added
braces is all other conditions.

>> +
>>  	if (err)
>>  		goto error;
>>  
>> diff --git a/tools/perf/util/probe-event.h b/tools/perf/util/probe-event.h
>> index 45b14f0..15a98c3 100644
>> --- a/tools/perf/util/probe-event.h
>> +++ b/tools/perf/util/probe-event.h
>> @@ -27,6 +27,7 @@ struct probe_trace_point {
>>  	char		*symbol;	/* Base symbol */
>>  	char		*module;	/* Module name */
>>  	unsigned long	offset;		/* Offset from symbol */
>> +	unsigned long	ref_ctr_offset;	/* SDT reference counter offset */
>>  	unsigned long	address;	/* Actual address of the trace point */
>>  	bool		retprobe;	/* Return probe flag */
>>  };
>> diff --git a/tools/perf/util/probe-file.c b/tools/perf/util/probe-file.c
>> index 4ae1123..08ba3a6 100644
>> --- a/tools/perf/util/probe-file.c
>> +++ b/tools/perf/util/probe-file.c
>> @@ -701,6 +701,12 @@ static unsigned long long sdt_note__get_addr(struct sdt_note *note)
>>  		 : (unsigned long long)note->addr.a64[0];
>>  }
>>  
>> +static unsigned long long sdt_note__get_ref_ctr_offset(struct sdt_note *note)
>> +{
>> +	return note->bit32 ? (unsigned long long)note->addr.a32[2]
>> +		: (unsigned long long)note->addr.a64[2];
>> +}
> Could you please introduce an enum for specifying the index by name?
>
> e.g.
> enum {
> 	SDT_NOTE_IDX_ADDR = 0,
> 	SDT_NOTE_IDX_REFCTR = 2,
> };

That will be good. Will change it.

>> +
>>  static const char * const type_to_suffix[] = {
>>  	":s64", "", "", "", ":s32", "", ":s16", ":s8",
>>  	"", ":u8", ":u16", "", ":u32", "", "", "", ":u64"
>> @@ -776,14 +782,24 @@ static char *synthesize_sdt_probe_command(struct sdt_note *note,
>>  {
>>  	struct strbuf buf;
>>  	char *ret = NULL, **args;
>> -	int i, args_count;
>> +	int i, args_count, err;
>> +	unsigned long long ref_ctr_offset;
>>  
>>  	if (strbuf_init(&buf, 32) < 0)
>>  		return NULL;
>>  
>> -	if (strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
>> +	ref_ctr_offset = sdt_note__get_ref_ctr_offset(note);
>> +
>> +	if (ref_ctr_offset)
>> +		err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx(0x%llx)",
>>  				sdtgrp, note->name, pathname,
>> -				sdt_note__get_addr(note)) < 0)
>> +				sdt_note__get_addr(note), ref_ctr_offset);
>> +	else
>> +		err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
>> +				sdtgrp, note->name, pathname,
>> +				sdt_note__get_addr(note));
> This can be minimized (and avoid repeating) by using 2 strbuf_addf()s, like
>
> 	err = strbuf_addf(&buf, "p:%s/%s %s:0x%llx",
> 			sdtgrp, note->name, pathname,
> 			sdt_note__get_addr(note));
> 	if (ref_ctr_offset && !err < 0)
> 		err = strbuf_addf("(0x%llx)", ref_ctr_offset);

Sure. Will change it.

>
>> +
>> +	if (err < 0)
>>  		goto error;
>>  
>>  	if (!note->args)
>> diff --git a/tools/perf/util/symbol-elf.c b/tools/perf/util/symbol-elf.c
>> index 2de7705..76c7b54 100644
>> --- a/tools/perf/util/symbol-elf.c
>> +++ b/tools/perf/util/symbol-elf.c
>> @@ -1928,6 +1928,16 @@ static int populate_sdt_note(Elf **elf, const char *data, size_t len,
>>  		}
>>  	}
>>  
>> +	/* Adjust reference counter offset */
>> +	if (elf_section_by_name(*elf, &ehdr, &shdr, SDT_PROBES_SCN, NULL)) {
>> +		if (shdr.sh_offset) {
>> +			if (tmp->bit32)
>> +				tmp->addr.a32[2] -= (shdr.sh_addr - shdr.sh_offset);
>> +			else
>> +				tmp->addr.a64[2] -= (shdr.sh_addr - shdr.sh_offset);
> Here we should use enum above too.

Sure.

Thanks for the review :)
Ravi

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

^ permalink raw reply

* Re: [PATCH 8/8] trace_uprobe/sdt: Document about reference counter
From: Ravi Bangoria @ 2018-03-14 15:22 UTC (permalink / raw)
  To: Masami Hiramatsu
  Cc: oleg, peterz, srikar, acme, ananth, akpm, alexander.shishkin,
	alexis.berlemont, corbet, dan.j.williams, gregkh, huawei.libin,
	hughd, jack, jglisse, jolsa, kan.liang, kirill.shutemov, kjlx,
	kstewart, linux-doc, linux-kernel, linux-mm, mhocko, milian.wolff,
	mingo, namhyung, naveen.n.rao, pc, pombredanne, rostedt, tglx,
	tmricht, willy, yao.jin, fengguang.wu, Ravi Bangoria
In-Reply-To: <20180314225021.64109239de8b14b0aec1e1c5@kernel.org>



On 03/14/2018 07:20 PM, Masami Hiramatsu wrote:
> On Tue, 13 Mar 2018 18:26:03 +0530
> Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com> wrote:
>
>> No functionality changes.
> Please consider to describe what is this change and why, here.

Will add in next version.

>> Signed-off-by: Ravi Bangoria <ravi.bangoria@linux.vnet.ibm.com>
>> ---
>>  Documentation/trace/uprobetracer.txt | 16 +++++++++++++---
>>  kernel/trace/trace.c                 |  2 +-
>>  2 files changed, 14 insertions(+), 4 deletions(-)
>>
>> diff --git a/Documentation/trace/uprobetracer.txt b/Documentation/trace/uprobetracer.txt
>> index bf526a7c..8fb13b0 100644
>> --- a/Documentation/trace/uprobetracer.txt
>> +++ b/Documentation/trace/uprobetracer.txt
>> @@ -19,15 +19,25 @@ user to calculate the offset of the probepoint in the object.
>>  
>>  Synopsis of uprobe_tracer
>>  -------------------------
>> -  p[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a uprobe
>> -  r[:[GRP/]EVENT] PATH:OFFSET [FETCHARGS] : Set a return uprobe (uretprobe)
>> -  -:[GRP/]EVENT                           : Clear uprobe or uretprobe event
>> +  p[:[GRP/]EVENT] PATH:OFFSET[(REF_CTR_OFFSET)] [FETCHARGS]
>> +  r[:[GRP/]EVENT] PATH:OFFSET[(REF_CTR_OFFSET)] [FETCHARGS]
> Ah, OK in this context, [] means optional syntax :)

Correct.

Thanks,
Ravi

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

^ permalink raw reply

* [PATCH 00/47] arch-removal: device drivers
From: Arnd Bergmann @ 2018-03-14 15:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnd Bergmann, linux, corbet, tj, gregkh, rjw, viresh.kumar,
	herbert, davem, bp, mchehab, linus.walleij, wsa, dmitry.torokhov,
	ulf.hansson, boris.brezillon, cyrille.pitchen, wg, mkl,
	alexandre.belloni, broonie, jic23, lars, jslaby, stern,
	b.zolnierkie, shli, linux-watchdog, linux-doc, linux-ide,
	linux-pm, linux-crypto, linux-edac, linux-gpio, linux-i2c,
	linux-input, linux-media, linux-mmc, linux-mtd, netdev, linux-can,
	linux-pwm, linux-rtc, linux-spi, linux-iio, linux-serial,
	linux-usb, linux-fbdev, linux-raid, alsa-devel

Hi driver maintainers,

I just posted one series with the removal of eight architectures,
see https://lkml.org/lkml/2018/3/14/505 for details, or
https://lwn.net/Articles/748074/ for more background.

These are the device drivers that go along with them. I have already
picked up the drivers for arch/metag/ into my tree, they were reviewed
earlier.

Please let me know if you have any concerns with the patch, or if you
prefer to pick up the patches in your respective trees.  I created
the patches with 'git format-patch -D', so they will not apply without
manually removing those files.

For anything else, I'd keep the removal patches in my asm-generic tree
and will send a pull request for 4.17 along with the actual arch removal.

       Arnd

Arnd Bergmann
  edac: remove tile driver
  net: tile: remove ethernet drivers
  net: adi: remove blackfin ethernet drivers
  net: 8390: remove m32r specific bits
  net: remove cris etrax ethernet driver
  net: smsc: remove m32r specific smc91x configuration
  raid: remove tile specific raid6 implementation
  rtc: remove tile driver
  rtc: remove bfin driver
  char: remove obsolete ds1302 rtc driver
  char: remove tile-srom.c
  char: remove blackfin OTP driver
  pcmcia: remove m32r drivers
  pcmcia: remove blackfin driver
  ASoC: remove blackfin drivers
  video/logo: remove obsolete logo files
  fbdev: remove blackfin drivers
  fbdev: s1d13xxxfb: remove m32r specific hacks
  crypto: remove blackfin CRC driver
  media: platform: remove blackfin capture driver
  media: platform: remove m32r specific arv driver
  cpufreq: remove blackfin driver
  cpufreq: remove cris specific drivers
  gpio: remove etraxfs driver
  pinctrl: remove adi2/blackfin drivers
  ata: remove bf54x driver
  input: keyboard: remove bf54x driver
  input: misc: remove blackfin rotary driver
  mmc: remove bfin_sdh driver
  can: remove bfin_can driver
  watchdog: remove bfin_wdt driver
  mtd: maps: remove bfin-async-flash driver
  mtd: nand: remove bf5xx_nand driver
  spi: remove blackfin related host drivers
  i2c: remove bfin-twi driver
  pwm: remobe pwm-bfin driver
  usb: host: remove tilegx platform glue
  usb: musb: remove blackfin port
  usb: isp1362: remove blackfin arch glue
  serial: remove cris/etrax uart drivers
  serial: remove blackfin drivers
  serial: remove m32r_sio driver
  serial: remove tile uart driver
  tty: remove bfin_jtag_comm and hvc_bfin_jtag drivers
  tty: hvc: remove tile driver
  staging: irda: remove bfin_sir driver
  staging: iio: remove iio-trig-bfin-timer driver

 .../devicetree/bindings/gpio/gpio-etraxfs.txt      |   22 -
 .../bindings/serial/axis,etraxfs-uart.txt          |   22 -
 Documentation/watchdog/watchdog-parameters.txt     |    5 -
 MAINTAINERS                                        |    8 -
 drivers/ata/Kconfig                                |    9 -
 drivers/ata/Makefile                               |    1 -
 drivers/ata/pata_bf54x.c                           | 1703 --------
 drivers/char/Kconfig                               |   48 -
 drivers/char/Makefile                              |    3 -
 drivers/char/bfin-otp.c                            |  237 --
 drivers/char/ds1302.c                              |  357 --
 drivers/char/tile-srom.c                           |  475 ---
 drivers/cpufreq/Makefile                           |    3 -
 drivers/cpufreq/blackfin-cpufreq.c                 |  217 -
 drivers/cpufreq/cris-artpec3-cpufreq.c             |   93 -
 drivers/cpufreq/cris-etraxfs-cpufreq.c             |   92 -
 drivers/crypto/Kconfig                             |    7 -
 drivers/crypto/Makefile                            |    1 -
 drivers/crypto/bfin_crc.c                          |  753 ----
 drivers/crypto/bfin_crc.h                          |  124 -
 drivers/edac/Kconfig                               |    8 -
 drivers/edac/Makefile                              |    2 -
 drivers/edac/tile_edac.c                           |  265 --
 drivers/gpio/Kconfig                               |    9 -
 drivers/gpio/Makefile                              |    1 -
 drivers/gpio/gpio-etraxfs.c                        |  475 ---
 drivers/i2c/busses/Kconfig                         |   18 -
 drivers/i2c/busses/Makefile                        |    1 -
 drivers/i2c/busses/i2c-bfin-twi.c                  |  737 ----
 drivers/input/keyboard/Kconfig                     |    9 -
 drivers/input/keyboard/Makefile                    |    1 -
 drivers/input/keyboard/bf54x-keys.c                |  396 --
 drivers/input/misc/Kconfig                         |    9 -
 drivers/input/misc/Makefile                        |    1 -
 drivers/input/misc/bfin_rotary.c                   |  294 --
 drivers/media/platform/Kconfig                     |   22 -
 drivers/media/platform/Makefile                    |    4 -
 drivers/media/platform/arv.c                       |  884 ----
 drivers/media/platform/blackfin/Kconfig            |   16 -
 drivers/media/platform/blackfin/Makefile           |    2 -
 drivers/media/platform/blackfin/bfin_capture.c     |  983 -----
 drivers/media/platform/blackfin/ppi.c              |  361 --
 drivers/mmc/host/Kconfig                           |   19 -
 drivers/mmc/host/Makefile                          |    1 -
 drivers/mmc/host/bfin_sdh.c                        |  679 ----
 drivers/mtd/maps/Kconfig                           |   10 -
 drivers/mtd/maps/Makefile                          |    1 -
 drivers/mtd/maps/bfin-async-flash.c                |  196 -
 drivers/mtd/nand/raw/Kconfig                       |   32 -
 drivers/mtd/nand/raw/Makefile                      |    1 -
 drivers/mtd/nand/raw/bf5xx_nand.c                  |  861 ----
 drivers/net/Makefile                               |    1 -
 drivers/net/can/Kconfig                            |    9 -
 drivers/net/can/Makefile                           |    1 -
 drivers/net/can/bfin_can.c                         |  784 ----
 drivers/net/cris/Makefile                          |    1 -
 drivers/net/cris/eth_v10.c                         | 1742 --------
 drivers/net/ethernet/8390/Kconfig                  |    3 +-
 drivers/net/ethernet/8390/ne.c                     |   23 +-
 drivers/net/ethernet/Kconfig                       |    2 -
 drivers/net/ethernet/Makefile                      |    2 -
 drivers/net/ethernet/adi/Kconfig                   |   66 -
 drivers/net/ethernet/adi/Makefile                  |    5 -
 drivers/net/ethernet/adi/bfin_mac.c                | 1881 ---------
 drivers/net/ethernet/adi/bfin_mac.h                |  104 -
 drivers/net/ethernet/smsc/Kconfig                  |    4 +-
 drivers/net/ethernet/smsc/smc91x.h                 |   26 -
 drivers/net/ethernet/tile/Kconfig                  |   18 -
 drivers/net/ethernet/tile/Makefile                 |   11 -
 drivers/net/ethernet/tile/tilegx.c                 | 2279 -----------
 drivers/net/ethernet/tile/tilepro.c                | 2397 -----------
 drivers/pcmcia/Kconfig                             |   26 -
 drivers/pcmcia/Makefile                            |    3 -
 drivers/pcmcia/bfin_cf_pcmcia.c                    |  316 --
 drivers/pcmcia/m32r_cfc.c                          |  786 ----
 drivers/pcmcia/m32r_cfc.h                          |   88 -
 drivers/pcmcia/m32r_pcc.c                          |  763 ----
 drivers/pcmcia/m32r_pcc.h                          |   66 -
 drivers/pinctrl/Kconfig                            |   19 -
 drivers/pinctrl/Makefile                           |    3 -
 drivers/pinctrl/pinctrl-adi2-bf54x.c               |  588 ---
 drivers/pinctrl/pinctrl-adi2-bf60x.c               |  517 ---
 drivers/pinctrl/pinctrl-adi2.c                     | 1114 -----
 drivers/pinctrl/pinctrl-adi2.h                     |   75 -
 drivers/pwm/Kconfig                                |    9 -
 drivers/pwm/Makefile                               |    1 -
 drivers/pwm/pwm-bfin.c                             |  157 -
 drivers/rtc/Kconfig                                |   17 -
 drivers/rtc/Makefile                               |    2 -
 drivers/rtc/rtc-bfin.c                             |  448 ---
 drivers/rtc/rtc-tile.c                             |  143 -
 drivers/spi/Kconfig                                |   19 -
 drivers/spi/Makefile                               |    3 -
 drivers/spi/spi-adi-v3.c                           |  984 -----
 drivers/spi/spi-bfin-sport.c                       |  919 -----
 drivers/spi/spi-bfin5xx.c                          | 1462 -------
 drivers/staging/iio/Kconfig                        |    1 -
 drivers/staging/iio/Makefile                       |    1 -
 drivers/staging/iio/trigger/Kconfig                |   19 -
 drivers/staging/iio/trigger/Makefile               |    5 -
 drivers/staging/iio/trigger/iio-trig-bfin-timer.c  |  292 --
 drivers/staging/iio/trigger/iio-trig-bfin-timer.h  |   25 -
 drivers/staging/irda/drivers/Kconfig               |   45 -
 drivers/staging/irda/drivers/Makefile              |    1 -
 drivers/staging/irda/drivers/bfin_sir.c            |  819 ----
 drivers/staging/irda/drivers/bfin_sir.h            |   93 -
 drivers/tty/Kconfig                                |   13 -
 drivers/tty/Makefile                               |    1 -
 drivers/tty/bfin_jtag_comm.c                       |  353 --
 drivers/tty/hvc/Kconfig                            |    9 -
 drivers/tty/hvc/Makefile                           |    2 -
 drivers/tty/hvc/hvc_bfin_jtag.c                    |  104 -
 drivers/tty/hvc/hvc_tile.c                         |  196 -
 drivers/tty/serial/Kconfig                         |  198 -
 drivers/tty/serial/Makefile                        |    6 -
 drivers/tty/serial/bfin_sport_uart.c               |  937 -----
 drivers/tty/serial/bfin_sport_uart.h               |   86 -
 drivers/tty/serial/bfin_uart.c                     | 1551 -------
 drivers/tty/serial/crisv10.c                       | 4248 --------------------
 drivers/tty/serial/crisv10.h                       |  133 -
 drivers/tty/serial/etraxfs-uart.c                  |  960 -----
 drivers/tty/serial/m32r_sio.c                      | 1053 -----
 drivers/tty/serial/m32r_sio_reg.h                  |  150 -
 drivers/tty/serial/tilegx.c                        |  689 ----
 drivers/usb/host/Kconfig                           |    1 +
 drivers/usb/host/ehci-hcd.c                        |    5 -
 drivers/usb/host/ehci-tilegx.c                     |  207 -
 drivers/usb/host/isp1362.h                         |   44 -
 drivers/usb/host/ohci-hcd.c                        |   18 -
 drivers/usb/host/ohci-tilegx.c                     |  196 -
 drivers/usb/musb/Kconfig                           |   10 +-
 drivers/usb/musb/Makefile                          |    1 -
 drivers/usb/musb/blackfin.c                        |  623 ---
 drivers/usb/musb/blackfin.h                        |   81 -
 drivers/usb/musb/musb_core.c                       |    5 -
 drivers/usb/musb/musb_core.h                       |   30 -
 drivers/usb/musb/musb_debugfs.c                    |    2 -
 drivers/usb/musb/musb_dma.h                        |   11 -
 drivers/usb/musb/musb_gadget.c                     |   35 -
 drivers/usb/musb/musb_regs.h                       |  182 -
 drivers/usb/musb/musbhsdma.c                       |    5 -
 drivers/usb/musb/musbhsdma.h                       |   64 -
 drivers/video/fbdev/Kconfig                        |  103 -
 drivers/video/fbdev/Makefile                       |    5 -
 drivers/video/fbdev/bf537-lq035.c                  |  891 ----
 drivers/video/fbdev/bf54x-lq043fb.c                |  764 ----
 drivers/video/fbdev/bfin-lq035q1-fb.c              |  864 ----
 drivers/video/fbdev/bfin-t350mcqb-fb.c             |  669 ---
 drivers/video/fbdev/bfin_adv7393fb.c               |  828 ----
 drivers/video/fbdev/bfin_adv7393fb.h               |  319 --
 drivers/video/fbdev/s1d13xxxfb.c                   |   10 -
 drivers/video/logo/Kconfig                         |   15 -
 drivers/video/logo/Makefile                        |    3 -
 drivers/video/logo/logo.c                          |   12 -
 drivers/video/logo/logo_blackfin_clut224.ppm       | 1127 ------
 drivers/video/logo/logo_blackfin_vga16.ppm         | 1127 ------
 drivers/video/logo/logo_m32r_clut224.ppm           | 1292 ------
 drivers/watchdog/Kconfig                           |   17 -
 drivers/watchdog/Makefile                          |    7 -
 drivers/watchdog/bfin_wdt.c                        |  476 ---
 include/linux/bfin_mac.h                           |   30 -
 include/linux/linux_logo.h                         |    3 -
 include/linux/platform_data/bfin_rotary.h          |  117 -
 include/linux/platform_data/pinctrl-adi2.h         |   40 -
 include/linux/raid/pq.h                            |    1 -
 include/linux/usb/musb.h                           |    7 -
 include/linux/usb/tilegx.h                         |   35 -
 include/media/blackfin/bfin_capture.h              |   39 -
 include/media/blackfin/ppi.h                       |   94 -
 lib/raid6/Makefile                                 |    6 -
 lib/raid6/algos.c                                  |    3 -
 lib/raid6/test/Makefile                            |    7 -
 lib/raid6/tilegx.uc                                |   87 -
 sound/soc/Kconfig                                  |    1 -
 sound/soc/Makefile                                 |    1 -
 sound/soc/blackfin/Kconfig                         |  205 -
 sound/soc/blackfin/Makefile                        |   40 -
 sound/soc/blackfin/bf5xx-ac97-pcm.c                |  480 ---
 sound/soc/blackfin/bf5xx-ac97.c                    |  388 --
 sound/soc/blackfin/bf5xx-ac97.h                    |   57 -
 sound/soc/blackfin/bf5xx-ad1836.c                  |  109 -
 sound/soc/blackfin/bf5xx-ad193x.c                  |  131 -
 sound/soc/blackfin/bf5xx-ad1980.c                  |  109 -
 sound/soc/blackfin/bf5xx-ad73311.c                 |  212 -
 sound/soc/blackfin/bf5xx-i2s-pcm.c                 |  373 --
 sound/soc/blackfin/bf5xx-i2s-pcm.h                 |   17 -
 sound/soc/blackfin/bf5xx-i2s.c                     |  391 --
 sound/soc/blackfin/bf5xx-sport.c                   | 1102 -----
 sound/soc/blackfin/bf5xx-sport.h                   |  174 -
 sound/soc/blackfin/bf5xx-ssm2602.c                 |  126 -
 sound/soc/blackfin/bf6xx-i2s.c                     |  239 --
 sound/soc/blackfin/bf6xx-sport.c                   |  425 --
 sound/soc/blackfin/bf6xx-sport.h                   |   82 -
 sound/soc/blackfin/bfin-eval-adau1373.c            |  173 -
 sound/soc/blackfin/bfin-eval-adau1701.c            |  113 -
 sound/soc/blackfin/bfin-eval-adau1x61.c            |  142 -
 sound/soc/blackfin/bfin-eval-adau1x81.c            |  129 -
 sound/soc/blackfin/bfin-eval-adav80x.c             |  145 -
 198 files changed, 7 insertions(+), 56230 deletions(-)
 delete mode 100644 Documentation/devicetree/bindings/gpio/gpio-etraxfs.txt
 delete mode 100644 Documentation/devicetree/bindings/serial/axis,etraxfs-uart.txt
 delete mode 100644 drivers/ata/pata_bf54x.c
 delete mode 100644 drivers/char/bfin-otp.c
 delete mode 100644 drivers/char/ds1302.c
 delete mode 100644 drivers/char/tile-srom.c
 delete mode 100644 drivers/cpufreq/blackfin-cpufreq.c
 delete mode 100644 drivers/cpufreq/cris-artpec3-cpufreq.c
 delete mode 100644 drivers/cpufreq/cris-etraxfs-cpufreq.c
 delete mode 100644 drivers/crypto/bfin_crc.c
 delete mode 100644 drivers/crypto/bfin_crc.h
 delete mode 100644 drivers/edac/tile_edac.c
 delete mode 100644 drivers/gpio/gpio-etraxfs.c
 delete mode 100644 drivers/i2c/busses/i2c-bfin-twi.c
 delete mode 100644 drivers/input/keyboard/bf54x-keys.c
 delete mode 100644 drivers/input/misc/bfin_rotary.c
 delete mode 100644 drivers/media/platform/arv.c
 delete mode 100644 drivers/media/platform/blackfin/Kconfig
 delete mode 100644 drivers/media/platform/blackfin/Makefile
 delete mode 100644 drivers/media/platform/blackfin/bfin_capture.c
 delete mode 100644 drivers/media/platform/blackfin/ppi.c
 delete mode 100644 drivers/mmc/host/bfin_sdh.c
 delete mode 100644 drivers/mtd/maps/bfin-async-flash.c
 delete mode 100644 drivers/mtd/nand/raw/bf5xx_nand.c
 delete mode 100644 drivers/net/can/bfin_can.c
 delete mode 100644 drivers/net/cris/Makefile
 delete mode 100644 drivers/net/cris/eth_v10.c
 delete mode 100644 drivers/net/ethernet/adi/Kconfig
 delete mode 100644 drivers/net/ethernet/adi/Makefile
 delete mode 100644 drivers/net/ethernet/adi/bfin_mac.c
 delete mode 100644 drivers/net/ethernet/adi/bfin_mac.h
 delete mode 100644 drivers/net/ethernet/tile/Kconfig
 delete mode 100644 drivers/net/ethernet/tile/Makefile
 delete mode 100644 drivers/net/ethernet/tile/tilegx.c
 delete mode 100644 drivers/net/ethernet/tile/tilepro.c
 delete mode 100644 drivers/pcmcia/bfin_cf_pcmcia.c
 delete mode 100644 drivers/pcmcia/m32r_cfc.c
 delete mode 100644 drivers/pcmcia/m32r_cfc.h
 delete mode 100644 drivers/pcmcia/m32r_pcc.c
 delete mode 100644 drivers/pcmcia/m32r_pcc.h
 delete mode 100644 drivers/pinctrl/pinctrl-adi2-bf54x.c
 delete mode 100644 drivers/pinctrl/pinctrl-adi2-bf60x.c
 delete mode 100644 drivers/pinctrl/pinctrl-adi2.c
 delete mode 100644 drivers/pinctrl/pinctrl-adi2.h
 delete mode 100644 drivers/pwm/pwm-bfin.c
 delete mode 100644 drivers/rtc/rtc-bfin.c
 delete mode 100644 drivers/rtc/rtc-tile.c
 delete mode 100644 drivers/spi/spi-adi-v3.c
 delete mode 100644 drivers/spi/spi-bfin-sport.c
 delete mode 100644 drivers/spi/spi-bfin5xx.c
 delete mode 100644 drivers/staging/iio/trigger/Kconfig
 delete mode 100644 drivers/staging/iio/trigger/Makefile
 delete mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.c
 delete mode 100644 drivers/staging/iio/trigger/iio-trig-bfin-timer.h
 delete mode 100644 drivers/staging/irda/drivers/bfin_sir.c
 delete mode 100644 drivers/staging/irda/drivers/bfin_sir.h
 delete mode 100644 drivers/tty/bfin_jtag_comm.c
 delete mode 100644 drivers/tty/hvc/hvc_bfin_jtag.c
 delete mode 100644 drivers/tty/hvc/hvc_tile.c
 delete mode 100644 drivers/tty/serial/bfin_sport_uart.c
 delete mode 100644 drivers/tty/serial/bfin_sport_uart.h
 delete mode 100644 drivers/tty/serial/bfin_uart.c
 delete mode 100644 drivers/tty/serial/crisv10.c
 delete mode 100644 drivers/tty/serial/crisv10.h
 delete mode 100644 drivers/tty/serial/etraxfs-uart.c
 delete mode 100644 drivers/tty/serial/m32r_sio.c
 delete mode 100644 drivers/tty/serial/m32r_sio_reg.h
 delete mode 100644 drivers/tty/serial/tilegx.c
 delete mode 100644 drivers/usb/host/ehci-tilegx.c
 delete mode 100644 drivers/usb/host/ohci-tilegx.c
 delete mode 100644 drivers/usb/musb/blackfin.c
 delete mode 100644 drivers/usb/musb/blackfin.h
 delete mode 100644 drivers/video/fbdev/bf537-lq035.c
 delete mode 100644 drivers/video/fbdev/bf54x-lq043fb.c
 delete mode 100644 drivers/video/fbdev/bfin-lq035q1-fb.c
 delete mode 100644 drivers/video/fbdev/bfin-t350mcqb-fb.c
 delete mode 100644 drivers/video/fbdev/bfin_adv7393fb.c
 delete mode 100644 drivers/video/fbdev/bfin_adv7393fb.h
 delete mode 100644 drivers/video/logo/logo_blackfin_clut224.ppm
 delete mode 100644 drivers/video/logo/logo_blackfin_vga16.ppm
 delete mode 100644 drivers/video/logo/logo_m32r_clut224.ppm
 delete mode 100644 drivers/watchdog/bfin_wdt.c
 delete mode 100644 include/linux/bfin_mac.h
 delete mode 100644 include/linux/platform_data/bfin_rotary.h
 delete mode 100644 include/linux/platform_data/pinctrl-adi2.h
 delete mode 100644 include/linux/usb/tilegx.h
 delete mode 100644 include/media/blackfin/bfin_capture.h
 delete mode 100644 include/media/blackfin/ppi.h
 delete mode 100644 lib/raid6/tilegx.uc
 delete mode 100644 sound/soc/blackfin/Kconfig
 delete mode 100644 sound/soc/blackfin/Makefile
 delete mode 100644 sound/soc/blackfin/bf5xx-ac97-pcm.c
 delete mode 100644 sound/soc/blackfin/bf5xx-ac97.c
 delete mode 100644 sound/soc/blackfin/bf5xx-ac97.h
 delete mode 100644 sound/soc/blackfin/bf5xx-ad1836.c
 delete mode 100644 sound/soc/blackfin/bf5xx-ad193x.c
 delete mode 100644 sound/soc/blackfin/bf5xx-ad1980.c
 delete mode 100644 sound/soc/blackfin/bf5xx-ad73311.c
 delete mode 100644 sound/soc/blackfin/bf5xx-i2s-pcm.c
 delete mode 100644 sound/soc/blackfin/bf5xx-i2s-pcm.h
 delete mode 100644 sound/soc/blackfin/bf5xx-i2s.c
 delete mode 100644 sound/soc/blackfin/bf5xx-sport.c
 delete mode 100644 sound/soc/blackfin/bf5xx-sport.h
 delete mode 100644 sound/soc/blackfin/bf5xx-ssm2602.c
 delete mode 100644 sound/soc/blackfin/bf6xx-i2s.c
 delete mode 100644 sound/soc/blackfin/bf6xx-sport.c
 delete mode 100644 sound/soc/blackfin/bf6xx-sport.h
 delete mode 100644 sound/soc/blackfin/bfin-eval-adau1373.c
 delete mode 100644 sound/soc/blackfin/bfin-eval-adau1701.c
 delete mode 100644 sound/soc/blackfin/bfin-eval-adau1x61.c
 delete mode 100644 sound/soc/blackfin/bfin-eval-adau1x81.c
 delete mode 100644 sound/soc/blackfin/bfin-eval-adav80x.c

-- 
Cc: linux@roeck-us.net
Cc: corbet@lwn.net
Cc: tj@kernel.org
Cc: gregkh@linuxfoundation.org
Cc: rjw@rjwysocki.net
Cc: viresh.kumar@linaro.org
Cc: herbert@gondor.apana.org.au
Cc: davem@davemloft.net
Cc: bp@alien8.de
Cc: mchehab@kernel.org
Cc: linus.walleij@linaro.org
Cc: wsa@the-dreams.de
Cc: dmitry.torokhov@gmail.com
Cc: ulf.hansson@linaro.org
Cc: boris.brezillon@bootlin.com
Cc: cyrille.pitchen@wedev4u.fr
Cc: wg@grandegger.com
Cc: mkl@pengutronix.de
Cc: alexandre.belloni@bootlin.com
Cc: broonie@kernel.org
Cc: jic23@kernel.org
Cc: lars@metafoo.de
Cc: jslaby@suse.com
Cc: stern@rowland.harvard.edu
Cc: b.zolnierkie@samsung.com
Cc: shli@kernel.org
Cc: linux-watchdog@vger.kernel.org
Cc: linux-doc@vger.kernel.org
Cc: linux-ide@vger.kernel.org
Cc: linux-pm@vger.kernel.org
Cc: linux-crypto@vger.kernel.org
Cc: linux-edac@vger.kernel.org
Cc: linux-gpio@vger.kernel.org
Cc: linux-i2c@vger.kernel.org
Cc: linux-input@vger.kernel.org
Cc: linux-media@vger.kernel.org
Cc: linux-mmc@vger.kernel.org
Cc: linux-mtd@lists.infradead.org
Cc: netdev@vger.kernel.org
Cc: linux-can@vger.kernel.org
Cc: linux-pwm@vger.kernel.org
Cc: linux-rtc@vger.kernel.org
Cc: linux-spi@vger.kernel.org
Cc: linux-iio@vger.kernel.org
Cc: linux-serial@vger.kernel.org
Cc: linux-usb@vger.kernel.org
Cc: linux-fbdev@vger.kernel.org
Cc: linux-raid@vger.kernel.org
Cc: alsa-devel@alsa-project.org
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply

* [PATCH 31/47] watchdog: remove bfin_wdt driver
From: Arnd Bergmann @ 2018-03-14 15:35 UTC (permalink / raw)
  To: linux-kernel
  Cc: Arnd Bergmann, Wim Van Sebroeck, Guenter Roeck, linux-watchdog,
	linux-doc
In-Reply-To: <20180314153603.3127932-1-arnd@arndb.de>

The blackfin architecture is getting removed, so this driver has
become obsolete.

Signed-off-by: Arnd Bergmann <arnd@arndb.de>
---
 Documentation/watchdog/watchdog-parameters.txt |   5 -
 drivers/watchdog/Kconfig                       |  17 -
 drivers/watchdog/Makefile                      |   7 -
 drivers/watchdog/bfin_wdt.c                    | 476 -------------------------
 4 files changed, 505 deletions(-)
 delete mode 100644 drivers/watchdog/bfin_wdt.c

diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
index beea975980f6..6d6200ea27b8 100644
--- a/Documentation/watchdog/watchdog-parameters.txt
+++ b/Documentation/watchdog/watchdog-parameters.txt
@@ -55,11 +55,6 @@ wdt_time: Watchdog time in seconds. (default=30)
 nowayout: Watchdog cannot be stopped once started
 	(default=kernel config parameter)
 -------------------------------------------------
-bfin_wdt:
-timeout: Watchdog timeout in seconds. (1<=timeout<=((2^32)/SCLK), default=20)
-nowayout: Watchdog cannot be stopped once started
-	(default=kernel config parameter)
--------------------------------------------------
 coh901327_wdt:
 margin: Watchdog margin in seconds (default 60s)
 -------------------------------------------------
diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
index 098e5ed4ee3d..f89f8869ca2a 100644
--- a/drivers/watchdog/Kconfig
+++ b/drivers/watchdog/Kconfig
@@ -815,23 +815,6 @@ config SPRD_WATCHDOG
 	  Say Y here to include watchdog timer supported
 	  by Spreadtrum system.
 
-# BLACKFIN Architecture
-
-config BFIN_WDT
-	tristate "Blackfin On-Chip Watchdog Timer"
-	depends on BLACKFIN
-	---help---
-	  If you say yes here you will get support for the Blackfin On-Chip
-	  Watchdog Timer. If you have one of these processors and wish to
-	  have watchdog support enabled, say Y, otherwise say N.
-
-	  To compile this driver as a module, choose M here: the
-	  module will be called bfin_wdt.
-
-# CRIS Architecture
-
-# FRV Architecture
-
 # X86 (i386 + ia64 + x86_64) Architecture
 
 config ACQUIRE_WDT
diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
index 0474d38aa854..e209824541b8 100644
--- a/drivers/watchdog/Makefile
+++ b/drivers/watchdog/Makefile
@@ -91,13 +91,6 @@ obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
 obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o
 obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
 
-# BLACKFIN Architecture
-obj-$(CONFIG_BFIN_WDT) += bfin_wdt.o
-
-# CRIS Architecture
-
-# FRV Architecture
-
 # X86 (i386 + ia64 + x86_64) Architecture
 obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
 obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
diff --git a/drivers/watchdog/bfin_wdt.c b/drivers/watchdog/bfin_wdt.c
deleted file mode 100644
index aa4d2e8a8ef9..000000000000
-- 
2.9.0

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

^ permalink raw reply related

* Re: [PATCH 31/47] watchdog: remove bfin_wdt driver
From: Guenter Roeck @ 2018-03-14 16:06 UTC (permalink / raw)
  To: Arnd Bergmann; +Cc: linux-kernel, Wim Van Sebroeck, linux-watchdog, linux-doc
In-Reply-To: <20180314153603.3127932-32-arnd@arndb.de>

On Wed, Mar 14, 2018 at 04:35:44PM +0100, Arnd Bergmann wrote:
> The blackfin architecture is getting removed, so this driver has
> become obsolete.
> 
> Signed-off-by: Arnd Bergmann <arnd@arndb.de>

Acked-by: Guenter Roeck <linux@roeck-us.net>

> ---
>  Documentation/watchdog/watchdog-parameters.txt |   5 -
>  drivers/watchdog/Kconfig                       |  17 -
>  drivers/watchdog/Makefile                      |   7 -
>  drivers/watchdog/bfin_wdt.c                    | 476 -------------------------
>  4 files changed, 505 deletions(-)
>  delete mode 100644 drivers/watchdog/bfin_wdt.c
> 
> diff --git a/Documentation/watchdog/watchdog-parameters.txt b/Documentation/watchdog/watchdog-parameters.txt
> index beea975980f6..6d6200ea27b8 100644
> --- a/Documentation/watchdog/watchdog-parameters.txt
> +++ b/Documentation/watchdog/watchdog-parameters.txt
> @@ -55,11 +55,6 @@ wdt_time: Watchdog time in seconds. (default=30)
>  nowayout: Watchdog cannot be stopped once started
>  	(default=kernel config parameter)
>  -------------------------------------------------
> -bfin_wdt:
> -timeout: Watchdog timeout in seconds. (1<=timeout<=((2^32)/SCLK), default=20)
> -nowayout: Watchdog cannot be stopped once started
> -	(default=kernel config parameter)
> --------------------------------------------------
>  coh901327_wdt:
>  margin: Watchdog margin in seconds (default 60s)
>  -------------------------------------------------
> diff --git a/drivers/watchdog/Kconfig b/drivers/watchdog/Kconfig
> index 098e5ed4ee3d..f89f8869ca2a 100644
> --- a/drivers/watchdog/Kconfig
> +++ b/drivers/watchdog/Kconfig
> @@ -815,23 +815,6 @@ config SPRD_WATCHDOG
>  	  Say Y here to include watchdog timer supported
>  	  by Spreadtrum system.
>  
> -# BLACKFIN Architecture
> -
> -config BFIN_WDT
> -	tristate "Blackfin On-Chip Watchdog Timer"
> -	depends on BLACKFIN
> -	---help---
> -	  If you say yes here you will get support for the Blackfin On-Chip
> -	  Watchdog Timer. If you have one of these processors and wish to
> -	  have watchdog support enabled, say Y, otherwise say N.
> -
> -	  To compile this driver as a module, choose M here: the
> -	  module will be called bfin_wdt.
> -
> -# CRIS Architecture
> -
> -# FRV Architecture
> -
>  # X86 (i386 + ia64 + x86_64) Architecture
>  
>  config ACQUIRE_WDT
> diff --git a/drivers/watchdog/Makefile b/drivers/watchdog/Makefile
> index 0474d38aa854..e209824541b8 100644
> --- a/drivers/watchdog/Makefile
> +++ b/drivers/watchdog/Makefile
> @@ -91,13 +91,6 @@ obj-$(CONFIG_UNIPHIER_WATCHDOG) += uniphier_wdt.o
>  obj-$(CONFIG_RTD119X_WATCHDOG) += rtd119x_wdt.o
>  obj-$(CONFIG_SPRD_WATCHDOG) += sprd_wdt.o
>  
> -# BLACKFIN Architecture
> -obj-$(CONFIG_BFIN_WDT) += bfin_wdt.o
> -
> -# CRIS Architecture
> -
> -# FRV Architecture
> -
>  # X86 (i386 + ia64 + x86_64) Architecture
>  obj-$(CONFIG_ACQUIRE_WDT) += acquirewdt.o
>  obj-$(CONFIG_ADVANTECH_WDT) += advantechwdt.o
> diff --git a/drivers/watchdog/bfin_wdt.c b/drivers/watchdog/bfin_wdt.c
> deleted file mode 100644
> index aa4d2e8a8ef9..000000000000
> -- 
> 2.9.0
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-doc" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply


This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox