Re: [PATCH v2] ACPI: NFIT: Optimize nfit_mem_cmp() for efficiency
On Wed, Oct 18, 2023 at 01:17:31PM +0200, Rafael J. Wysocki wrote: > On Fri, Oct 13, 2023 at 2:22 PM Kuan-Wei Chiu wrote: > > > > The original code used conditional branching in the nfit_mem_cmp > > function to compare two values and return -1, 1, or 0 based on the > > result. However, the list_sort comparison function only needs results > > <0, >0, or =0. This patch optimizes the code to make the comparison > > branchless, improving efficiency and reducing code size. This change > > reduces the number of comparison operations from 1-2 to a single > > subtraction operation, thereby saving the number of instructions. > > > > Signed-off-by: Kuan-Wei Chiu > > --- > > v1 -> v2: > > - Add explicit type cast in case the sizes of u32 and int differ. > > > > drivers/acpi/nfit/core.c | 6 +- > > 1 file changed, 1 insertion(+), 5 deletions(-) > > > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > > index f96bf32cd368..563a32eba888 100644 > > --- a/drivers/acpi/nfit/core.c > > +++ b/drivers/acpi/nfit/core.c > > @@ -1138,11 +1138,7 @@ static int nfit_mem_cmp(void *priv, const struct > > list_head *_a, > > > > handleA = __to_nfit_memdev(a)->device_handle; > > handleB = __to_nfit_memdev(b)->device_handle; > > - if (handleA < handleB) > > - return -1; > > - else if (handleA > handleB) > > - return 1; > > - return 0; > > + return (int)handleA - (int)handleB; > > Are you sure that you are not losing bits in these conversions? I believe your concerns are valid. Perhaps this was a stupid mistake I made. Initially, I proposed this patch because I noticed that other parts of the Linux kernel, such as the sram_reserve_cmp() function in drivers/misc/sram.c, directly used subtraction for comparisons involving u32. However, this approach could potentially lead to issues when the size of int is 2 bytes instead of 4 bytes. I think maybe we should consider dropping this patch. I apologize for proposing an incorrect patch. Thanks, Kuan-Wei Chiu > > > } > > > > static int nfit_mem_init(struct acpi_nfit_desc *acpi_desc) > > --
[PATCH] tracefs/eventfs: Modify mismatched function name
No functional modification involved. fs/tracefs/event_inode.c:864: warning: expecting prototype for eventfs_remove(). Prototype was for eventfs_remove_dir() instead. Reported-by: Abaci Robot Closes: https://bugzilla.openanolis.cn/show_bug.cgi?id=6939 Signed-off-by: Jiapeng Chong --- fs/tracefs/event_inode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c index 1ccd100bc565..ba9d1cb0d24c 100644 --- a/fs/tracefs/event_inode.c +++ b/fs/tracefs/event_inode.c @@ -855,7 +855,7 @@ static void unhook_dentry(struct dentry **dentry, struct dentry **list) } } /** - * eventfs_remove - remove eventfs dir or file from list + * eventfs_remove_dir - remove eventfs dir or file from list * @ei: eventfs_inode to be removed. * * This function acquire the eventfs_mutex lock and call eventfs_remove_rec() -- 2.20.1.7.g153144c
Re: [PATCH 2/2] mm: multi-gen lru: fix stat count
On Wed, Oct 18, 2023 at 8:17 PM Huan Yang wrote:
>
> Hi Yu Zhao,
>
> Thanks for your reply.
>
> 在 2023/10/19 0:21, Yu Zhao 写道:
> > On Wed, Oct 18, 2023 at 2:22 AM Huan Yang wrote:
> >> For multi-gen lru reclaim in evict_folios, like shrink_inactive_list,
> >> gather folios which isolate to reclaim, and invoke shirnk_folio_list.
> >>
> >> But, when complete shrink, it not gather shrink reclaim stat into sc,
> >> we can't get info like nr_dirty\congested in reclaim, and then
> >> control writeback, dirty number and mark as LRUVEC_CONGESTED, or
> >> just bpf trace shrink and get correct sc stat.
> >>
> >> This patch fix this by simple copy code from shrink_inactive_list when
> >> end of shrink list.
> > MGLRU doesn't try to write back dirt file pages in the reclaim path --
> > it filters them out in sort_folio() and leaves them to the page
> Nice to know this, sort_folio() filters some folio indeed.
> But, I want to know, if we touch some folio in shrink_folio_list(), may some
> folio become dirty or writeback even if sort_folio() filter then?
Good question: in that case MGLRU still doesn't try to write those
folios back because isolate_folio() cleared PG_reclaim and
shrink_folio_list() checks PG_reclaim:
if (folio_test_dirty(folio)) {
/*
* Only kswapd can writeback filesystem folios
* to avoid risk of stack overflow. But avoid
* injecting inefficient single-folio I/O into
* flusher writeback as much as possible: only
* write folios when we've encountered many
* dirty folios, and when we've already scanned
* the rest of the LRU for clean folios and see
* the same dirty folios again (with the reclaim
* flag set).
*/
if (folio_is_file_lru(folio) &&
(!current_is_kswapd() ||
!folio_test_reclaim(folio) ||
!test_bit(PGDAT_DIRTY, >flags))) {
Re: [PATCH 1/2] tracing: mm: multigen-lru: fix mglru trace
在 2023/10/19 0:28, Yu Zhao 写道: On Wed, Oct 18, 2023 at 2:22 AM Huan Yang wrote: This patch add two reclaim stat: nr_promote: nr_pages shrink before promote by folio_update_gen. nr_demote: nr_pages NUMA demotion passed. The above isn't specific to MLGRU, so they should be in a separate patchset. OK, nr_demote isn't MGLRU, separate is good, but, if this, nr_demote isn't need by myself, I just add this when I see this code. :) Please see nr_promote and nr_scanned fix is MGLRU need? And then, use correct nr_scanned which evict_folios passed into trace_mm_vmscan_lru_shrink_inactive. Mistake info like this: ``` kswapd0-89[000]64.887613: mm_vmscan_lru_shrink_inactive: nid=0 nr_scanned=64 nr_reclaimed=0 nr_dirty=0 nr_writeback=0 nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0 nr_ref_keep=0 nr_unmap_fail=0 priority=4 flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC ``` Correct info like this: ``` <...>-9041 [006]43.738481: mm_vmscan_lru_shrink_inactive: nid=0 nr_scanned=13 nr_reclaimed=0 nr_dirty=0 nr_writeback=0 nr_congested=0 nr_immediate=0 nr_activate_anon=9 nr_activate_file=0 nr_ref_keep=0 nr_unmap_fail=0 nr_promote=4 nr_demote=0 priority=12 flags=RECLAIM_WB_ANON|RECLAIM_WB_ASYNC ``` Adding Jaewon & Kalesh to take a look. Thanks, Huan
Re: [PATCH 2/2] mm: multi-gen lru: fix stat count
Hi Yu Zhao, Thanks for your reply. 在 2023/10/19 0:21, Yu Zhao 写道: On Wed, Oct 18, 2023 at 2:22 AM Huan Yang wrote: For multi-gen lru reclaim in evict_folios, like shrink_inactive_list, gather folios which isolate to reclaim, and invoke shirnk_folio_list. But, when complete shrink, it not gather shrink reclaim stat into sc, we can't get info like nr_dirty\congested in reclaim, and then control writeback, dirty number and mark as LRUVEC_CONGESTED, or just bpf trace shrink and get correct sc stat. This patch fix this by simple copy code from shrink_inactive_list when end of shrink list. MGLRU doesn't try to write back dirt file pages in the reclaim path -- it filters them out in sort_folio() and leaves them to the page Nice to know this, sort_folio() filters some folio indeed. But, I want to know, if we touch some folio in shrink_folio_list(), may some folio become dirty or writeback even if sort_folio() filter then? writeback. (The page writeback is a dedicated component for this purpose). So there is nothing to fix.
Re: [PATCH 2/4] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PM7250B thermals
On 10/13/23 10:09, Luca Weiss wrote:
Configure the thermals for the CHARGER_SKIN_THERM and USB_CONN_THERM
thermistors connected to PM7250B.
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 66 ++
1 file changed, 66 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
index 2de0b8c26c35..7fe19b556e6a 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -134,6 +134,36 @@ afvdd_2p8: regulator-afvdd-2p8 {
enable-active-high;
vin-supply = <_bob>;
};
+
+ thermal-zones {
+ chg-skin-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <_adc_tm 0>;
+
+ trips {
+ active-config0 {
+ temperature = <125000>;
I guess looking at skin-temp-thermal in x13s dts for starters
is a good idea.. we should probably then adjust it to something
more pocketable..
Konrad
Re: [PATCH 4/4] arm64: dts: qcom: qcm6490-fairphone-fp5: Add PM7325 thermals
On 10/14/23 19:52, Luca Weiss wrote:
On Samstag, 14. Oktober 2023 01:13:29 CEST Konrad Dybcio wrote:
On 13.10.2023 10:09, Luca Weiss wrote:
Configure the thermals for the QUIET_THERM, CAM_FLASH_THERM, MSM_THERM
and RFC_CAM_THERM thermistors connected to PM7325.
With this PMIC the software communication to the ADC is going through
PMK7325 (= PMK8350).
Signed-off-by: Luca Weiss
---
arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts | 117
+ 1 file changed, 117 insertions(+)
diff --git a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts index
2c01f799a6b2..d0b1e4e507ff 100644
--- a/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
+++ b/arch/arm64/boot/dts/qcom/qcm6490-fairphone-fp5.dts
@@ -9,6 +9,7 @@
#define PM7250B_SID 8
#define PM7250B_SID1 9
+#include
#include
#include
#include
@@ -137,6 +138,20 @@ afvdd_2p8: regulator-afvdd-2p8 {
};
thermal-zones {
+ camera-thermal {
+ polling-delay-passive = <0>;
+ polling-delay = <0>;
+ thermal-sensors = <_adc_tm 2>;
+
+ trips {
+ active-config0 {
+ temperature = <125000>;
are
+ rear-cam-thermal {
+ temperature = <125000>;
you
+ sdm-skin-thermal {
+ temperature = <125000>;
sure
about these temps?
(email from my other address, quicker right now)
Well yes and no.
Yes as in those are the temps specified in downstream dtb.
No as in I'm 99% sure there's user space with definitely lower threshold that
actually does something in response to the temps.
I didn't look too much into this but does the kernel even do something when it
hits one of these trip points? I assume when there's a cooling device thing
specified then it can actually tell the driver to do something, but without
(and most drivers don't support this?) I'm assuming the kernel can't do much
anyways?
So e.g. when the temperature for the flash led is reached I'm assuming
downstream (+Android) either dims the led or turns it off? But I'd have to dig
quite a bit into the thermal setup there to check what it's really doing.
I think reaching "critical" shuts down the platform, unless something
registering the thermal zone explicitly overrides the behavior.
But for now I think it's okay to put this current thermal config into dts and
we'll improve it later when 1. I understand more and 2. maybe some useful
drivers support the cooling bits?
Yeah it's better than nothing, but ultimately we should probably move
the values that userspace daemon operates on here in the dt..
Konrad
Re: [PATCH] arm64: dts: qcom: msm8939-longcheer-l9100: Add proximity-near-level
Am Mittwoch, dem 18.10.2023 um 10:32 +0200 schrieb Konrad Dybcio: > > > On 10/17/23 22:03, André Apitzsch wrote: > > Am Dienstag, dem 17.10.2023 um 18:25 +0200 schrieb Konrad Dybcio: > > > > > > > > > On 10/16/23 22:18, André Apitzsch wrote: > > > > Consider an object near to the sensor when their distance is > > > > about > > > > 4 cm > > > > or below. > > > > > > > > Signed-off-by: André Apitzsch > > > > --- > > > Reviewed-by: Konrad Dybcio > > > > > > Out of interest, what is it set to by default? > > > > > > Konrad > > > > The default value is 0. > That much I could guess :) I was wondering if it meant more like "it > can detect movement from 1km away" or "disabled" > > Konrad I doubt that this sensor can detect anything from a distance of 50cm. The values returned by the sensor are all non-negative, i.e. zero means something like "disabled" as every raw proximity value would be considered 'near' to the sensor according to the description in iio/common.yaml. André
Re: [PATCH 5/5] modules: only allow symbol_get of EXPORT_SYMBOL_GPL modules
On Wed, Oct 18, 2023 at 07:31:46AM +0200, Christoph Hellwig wrote: > On Wed, Oct 18, 2023 at 01:30:18AM +0100, David Woodhouse wrote: > > > > But if we're going to tolerate the core kernel still exporting some > > stuff with EXPORT_SYMBOL, why isn't OK for a GPL-licensed module do to > > the same? Even an *in-tree* GPL-licensed module now can't export > > functionality with EXPORT_SYMBOL and have it used with symbol_get(). > > Anything using symbol_get is by intent very deeply internal for tightly > coupled modules working together, and thus not a non-GPL export. > > In fact the current series is just a stepping stone. Once some mess > in the kvm/vfio integration is fixed up we'll require a new explicit > EXPORT_SYMBOL variant as symbol_get wasn't ever intended to be used > on totally random symbols not exported for use by symbol_get. The later patches in the series also show we could resolves most uses through Kconfig and at build time, it really begs the question if we even need it for any real valid uses. Luis
Re: [PATCH] eventfs: Use ERR_CAST() in eventfs_create_events_dir()
On Wed, Oct 18, 2023 at 11:10:31AM -0700, Nathan Chancellor wrote:
> When building with clang and CONFIG_RANDSTRUCT_FULL=y, there is an error
> due to a cast in eventfs_create_events_dir():
>
> fs/tracefs/event_inode.c:734:10: error: casting from randomized structure
> pointer type 'struct dentry *' to 'struct eventfs_inode *'
> 734 | return (struct eventfs_inode *)dentry;
> |^
> 1 error generated.
>
> Use the ERR_CAST() function to resolve the error, as it was designed for
> this exact situation (casting an error pointer to another type).
>
> Closes: https://github.com/ClangBuiltLinux/linux/issues/1947
> Fixes: 5790b1fb3d67 ("eventfs: Remove eventfs_file and just use
> eventfs_inode")
> Signed-off-by: Nathan Chancellor
Yes, please. That's the correct method to do such casts. Thanks!
Reviewed-by: Kees Cook
--
Kees Cook
[PATCH] eventfs: Use ERR_CAST() in eventfs_create_events_dir()
When building with clang and CONFIG_RANDSTRUCT_FULL=y, there is an error
due to a cast in eventfs_create_events_dir():
fs/tracefs/event_inode.c:734:10: error: casting from randomized structure
pointer type 'struct dentry *' to 'struct eventfs_inode *'
734 | return (struct eventfs_inode *)dentry;
|^
1 error generated.
Use the ERR_CAST() function to resolve the error, as it was designed for
this exact situation (casting an error pointer to another type).
Closes: https://github.com/ClangBuiltLinux/linux/issues/1947
Fixes: 5790b1fb3d67 ("eventfs: Remove eventfs_file and just use eventfs_inode")
Signed-off-by: Nathan Chancellor
---
fs/tracefs/event_inode.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/tracefs/event_inode.c b/fs/tracefs/event_inode.c
index 1ccd100bc565..9f19b6608954 100644
--- a/fs/tracefs/event_inode.c
+++ b/fs/tracefs/event_inode.c
@@ -731,7 +731,7 @@ struct eventfs_inode *eventfs_create_events_dir(const char
*name, struct dentry
return NULL;
if (IS_ERR(dentry))
- return (struct eventfs_inode *)dentry;
+ return ERR_CAST(dentry);
ei = kzalloc(sizeof(*ei), GFP_KERNEL);
if (!ei)
---
base-commit: 5ddd8baa4857709b4e5d84b376d735152851955b
change-id: 20231018-ftrace-fix-clang-randstruct-0cb6899523ac
Best regards,
--
Nathan Chancellor
Re: [PATCH v6 3/4] remoteproc: zynqmp: add pm domains support
Good morning,
On Thu, Oct 12, 2023 at 09:22:28PM -0700, Tanmay Shah wrote:
> Use TCM pm domains extracted from device-tree
> to power on/off TCM using general pm domain framework.
>
> Signed-off-by: Tanmay Shah
> ---
>
> Changes in v6:
> - Remove spurious change
> - Handle errors in add_pm_domains function
> - Remove redundant code to handle errors from remove_pm_domains
>
> drivers/remoteproc/xlnx_r5_remoteproc.c | 262 ++--
> 1 file changed, 243 insertions(+), 19 deletions(-)
>
> diff --git a/drivers/remoteproc/xlnx_r5_remoteproc.c
> b/drivers/remoteproc/xlnx_r5_remoteproc.c
> index 4395edea9a64..04e95d880184 100644
> --- a/drivers/remoteproc/xlnx_r5_remoteproc.c
> +++ b/drivers/remoteproc/xlnx_r5_remoteproc.c
> @@ -16,6 +16,7 @@
> #include
> #include
> #include
> +#include
>
> #include "remoteproc_internal.h"
>
> @@ -102,6 +103,12 @@ static const struct mem_bank_data
> zynqmp_tcm_banks_lockstep[] = {
> * @rproc: rproc handle
> * @pm_domain_id: RPU CPU power domain id
> * @ipi: pointer to mailbox information
> + * @num_pm_dev: number of tcm pm domain devices for this core
> + * @pm_dev1: pm domain virtual devices for power domain framework
> + * @pm_dev_link1: pm domain device links after registration
> + * @pm_dev2: used only in lockstep mode. second core's pm domain virtual
> devices
> + * @pm_dev_link2: used only in lockstep mode. second core's pm device links
> after
> + * registration
> */
> struct zynqmp_r5_core {
> struct device *dev;
> @@ -111,6 +118,11 @@ struct zynqmp_r5_core {
> struct rproc *rproc;
> u32 pm_domain_id;
> struct mbox_info *ipi;
> + int num_pm_dev;
> + struct device **pm_dev1;
s/pm_dev1/pm_dev_core0
> + struct device_link **pm_dev_link1;
s/pm_dev_link1/pm_dev_core0_link;
> + struct device **pm_dev2;
s/pm_dev2/pm_dev_core1
> + struct device_link **pm_dev_link2;
s/pm_dev_link2/pm_dev_core1_link;
> };
>
> /**
> @@ -575,12 +587,21 @@ static int add_tcm_carveout_split_mode(struct rproc
> *rproc)
> bank_size = r5_core->tcm_banks[i]->size;
> pm_domain_id = r5_core->tcm_banks[i]->pm_domain_id;
>
> - ret = zynqmp_pm_request_node(pm_domain_id,
> - ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> - ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> - if (ret < 0) {
> - dev_err(dev, "failed to turn on TCM 0x%x",
> pm_domain_id);
> - goto release_tcm_split;
> + /*
> + * If TCM information is available in device-tree then
> + * in that case, pm domain framework will power on/off TCM.
> + * In that case pm_domain_id is set to 0. If hardcode
> + * bindings from driver is used, then only this driver will
> + * use pm_domain_id.
> + */
> + if (pm_domain_id) {
> + ret = zynqmp_pm_request_node(pm_domain_id,
> +
> ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> +
> ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> + if (ret < 0) {
> + dev_err(dev, "failed to turn on TCM 0x%x",
> pm_domain_id);
> + goto release_tcm_split;
> + }
This should go in the next patch.
> }
>
> dev_dbg(dev, "TCM carveout split mode %s addr=%llx, da=0x%x,
> size=0x%lx",
> @@ -646,13 +667,16 @@ static int add_tcm_carveout_lockstep_mode(struct rproc
> *rproc)
> for (i = 0; i < num_banks; i++) {
> pm_domain_id = r5_core->tcm_banks[i]->pm_domain_id;
>
> - /* Turn on each TCM bank individually */
> - ret = zynqmp_pm_request_node(pm_domain_id,
> - ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> - ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> - if (ret < 0) {
> - dev_err(dev, "failed to turn on TCM 0x%x",
> pm_domain_id);
> - goto release_tcm_lockstep;
> + if (pm_domain_id) {
> + /* Turn on each TCM bank individually */
> + ret = zynqmp_pm_request_node(pm_domain_id,
> +
> ZYNQMP_PM_CAPABILITY_ACCESS, 0,
> +
> ZYNQMP_PM_REQUEST_ACK_BLOCKING);
> + if (ret < 0) {
> + dev_err(dev, "failed to turn on TCM 0x%x",
> + pm_domain_id);
> + goto release_tcm_lockstep;
> + }
Same
> }
>
> bank_size = r5_core->tcm_banks[i]->size;
> @@ -687,7 +711,8 @@ static int add_tcm_carveout_lockstep_mode(struct rproc
>
Re: [PATCH v5 12/18] x86/sgx: Add EPC OOM path to forcefully reclaim EPC
On Wed, 18 Oct 2023 10:52:23 -0500, Michal Koutný wrote: On Wed, Oct 18, 2023 at 08:37:25AM -0700, Dave Hansen wrote: 1. Admin sets a limit 2. Enclave is created 3. Enclave hits limit, allocation fails I was actually about to suggest reorganizing the series to a part implementing this simple limiting and a subsequent part with the reclaim stuff for easier digestability. Nothing else matters. If the latter part is an unncessary overkill, it's even better. Ok. I'll take out max_write() callback and only implement non-preemptive misc.max for EPC. I can also separate OOEPC_killing enclaves out, which is not needed if we only block allocation at limit, no need killing one enclave to make space for another. This will simplify a lot. Thanks to all for your input! Haitao
Re: [PATCH v5 0/2] Return EADDRNOTAVAIL when func matches several symbols during kprobe creation
On Wed, 18 Oct 2023 17:40:28 +0300 Francis Laniel wrote: > Changes since: > v1: > * Use EADDRNOTAVAIL instead of adding a new error code. > * Correct also this behavior for sysfs kprobe. > v2: > * Count the number of symbols corresponding to function name and return > EADDRNOTAVAIL if higher than 1. > * Return ENOENT if above count is 0, as it would be returned later by while > registering the kprobe. > v3: > * Check symbol does not contain ':' before testing its uniqueness. > * Add a selftest to check this is not possible to install a kprobe for a non > unique symbol. > v5: > * No changes, just add linux-stable as recipient. So why is this adding stable? (and as Greg's form letter states, that's not how you do that) I don't see this as a fix but a new feature. -- Steve
Re: [PATCH 1/2] tracing: mm: multigen-lru: fix mglru trace
On Wed, Oct 18, 2023 at 2:22 AM Huan Yang wrote: > > This patch add two reclaim stat: > nr_promote: nr_pages shrink before promote by folio_update_gen. > nr_demote: nr_pages NUMA demotion passed. The above isn't specific to MLGRU, so they should be in a separate patchset. > And then, use correct nr_scanned which evict_folios passed into > trace_mm_vmscan_lru_shrink_inactive. > > Mistake info like this: > ``` > kswapd0-89[000]64.887613: mm_vmscan_lru_shrink_inactive: > nid=0 nr_scanned=64 nr_reclaimed=0 nr_dirty=0 nr_writeback=0 > nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0 > nr_ref_keep=0 nr_unmap_fail=0 priority=4 > flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC > ``` > Correct info like this: > ``` > <...>-9041 [006]43.738481: mm_vmscan_lru_shrink_inactive: > nid=0 nr_scanned=13 nr_reclaimed=0 nr_dirty=0 nr_writeback=0 > nr_congested=0 nr_immediate=0 nr_activate_anon=9 nr_activate_file=0 > nr_ref_keep=0 nr_unmap_fail=0 nr_promote=4 nr_demote=0 priority=12 > flags=RECLAIM_WB_ANON|RECLAIM_WB_ASYNC > ``` Adding Jaewon & Kalesh to take a look.
Re: [PATCH 2/2] mm: multi-gen lru: fix stat count
On Wed, Oct 18, 2023 at 2:22 AM Huan Yang wrote: > > For multi-gen lru reclaim in evict_folios, like shrink_inactive_list, > gather folios which isolate to reclaim, and invoke shirnk_folio_list. > > But, when complete shrink, it not gather shrink reclaim stat into sc, > we can't get info like nr_dirty\congested in reclaim, and then > control writeback, dirty number and mark as LRUVEC_CONGESTED, or > just bpf trace shrink and get correct sc stat. > > This patch fix this by simple copy code from shrink_inactive_list when > end of shrink list. MGLRU doesn't try to write back dirt file pages in the reclaim path -- it filters them out in sort_folio() and leaves them to the page writeback. (The page writeback is a dedicated component for this purpose). So there is nothing to fix.
Re: [PATCH v5 12/18] x86/sgx: Add EPC OOM path to forcefully reclaim EPC
On Wed, Oct 18, 2023 at 08:37:25AM -0700, Dave Hansen wrote: > 1. Admin sets a limit > 2. Enclave is created > 3. Enclave hits limit, allocation fails I was actually about to suggest reorganizing the series to a part implementing this simple limiting and a subsequent part with the reclaim stuff for easier digestability. > Nothing else matters. If the latter part is an unncessary overkill, it's even better. Thanks, Michal signature.asc Description: PGP signature
Re: [PATCH v5 12/18] x86/sgx: Add EPC OOM path to forcefully reclaim EPC
On 10/18/23 08:26, Haitao Huang wrote: > Maybe not in sense of killing something. My understanding memory.reclaim > does not necessarily invoke the OOM killer. But what I really intend to > say is we can have a separate knob for user to express the need for > reducing the current usage explicitly and keep "misc.max' non-preemptive > semantics intact. When we implement that new knob, then we can define > what kind of reclaim for that. Depending on vEPC implementation, it may > or may not involve killing VMs. But at least that semantics will be > explicit for user. I'm really worried that you're going for "perfect" semantics here. This is SGX. It's *NOT* getting heavy use and even fewer folks will ever apply cgroup controls to it. Can we please stick to simple, easily-coded rules here? I honestly don't think these corner cases matter all that much and there's been *WAY* too much traffic in this thread for what is ultimately not that complicated. Focus on *ONE* thing: 1. Admin sets a limit 2. Enclave is created 3. Enclave hits limit, allocation fails Nothing else matters. What if the admin lowers the limit on an already-created enclave? Nobody cares. Seriously. What about inducing reclaim? Nobody cares. What about vEPC? Doesn't matter, an enclave page is an enclave page.
Re: [PATCH v2 5/6] ACPI: NFIT: Replace acpi_driver with platform_driver
On 10/17/2023 8:24 PM, Dan Williams wrote: > Michal Wilczynski wrote: >> NFIT driver uses struct acpi_driver incorrectly to register itself. >> This is wrong as the instances of the ACPI devices are not meant >> to be literal devices, they're supposed to describe ACPI entry of a >> particular device. >> >> Use platform_driver instead of acpi_driver. In relevant places call >> platform devices instances pdev to make a distinction with ACPI >> devices instances. >> >> NFIT driver uses devm_*() family of functions extensively. This change >> has no impact on correct functioning of the whole devm_*() family of >> functions, since the lifecycle of the device stays the same. It is still >> being created during the enumeration, and destroyed on platform device >> removal. > I notice this verbiage has the same fundamental misunderstanding of devm > allocation lifetime as the acpi_nfit_init_interleave_set() discussion. > The devm allocation lifetime typically starts in driver->probe() and > ends either with driver->probe() failure, or the driver->remove() call. > Note that the driver->remove() call is invoked not only for > platform-device removal, but also driver "unbind" events. So the > "destroyed on platform device removal" is the least likely way that > these allocations are torn down given ACPI0012 devices are never > removed. > > Outside of that, my main concern about this patch is that I expect it > breaks unit tests. The infrastructure in > tools/testing/nvdimm/test/nfit.c emulates an ACPI0012 device that allows > for deeper regression testing given hardware is difficult to come by, > and because QEMU does not implement some of the tricky corner cases that > the unit tests cover. > > This needs to pass tests, but fair warning, > tools/testing/nvdimm/test/nfit.c does some non-idiomatic + "strange" > things to achieve deeper test coverage. So I expect that if this breaks > tests as I expect the effort needed to fix the emulation could be > significant. > > If you want to give running the tests a try the easiest would be to use > "run_qemu.sh" with --nfit-test option [1], or you can try to setup an > environment manually using the ndctl instructions [2]. > > [1]: https://github.com/pmem/run_qemu > [2]: https://github.com/pmem/ndctl#readme Thanks a lot ! I will run qemu tests and fix the verbiage, Michał
Re: [PATCH v5 12/18] x86/sgx: Add EPC OOM path to forcefully reclaim EPC
On Wed, 18 Oct 2023 08:55:12 -0500, Dave Hansen wrote: On 10/17/23 21:37, Haitao Huang wrote: Yes we can introduce misc.reclaim to give user a knob to forcefully reducing usage if that is really needed in real usage. The semantics would make force-kill VMs explicit to user. Do any other controllers do something like this? It seems odd. Maybe not in sense of killing something. My understanding memory.reclaim does not necessarily invoke the OOM killer. But what I really intend to say is we can have a separate knob for user to express the need for reducing the current usage explicitly and keep "misc.max' non-preemptive semantics intact. When we implement that new knob, then we can define what kind of reclaim for that. Depending on vEPC implementation, it may or may not involve killing VMs. But at least that semantics will be explicit for user. Thanks Haitao
Re: [PATCH v10 4/5] kprobes: freelist.h removed
Hi,
On Mon, 16 Oct 2023 22:23:14 +0900
Masami Hiramatsu (Google) wrote:
> Hi Peter,
>
> This freelist has been introduced by you, is it OK to remove this because no
> other user exists?
I'll pick this on my probes/for-next branch. If there is any issue reported,
I'll revert it.
Thank you,
>
> Thank you,
>
> On Sun, 15 Oct 2023 13:32:50 +0800
> "wuqiang.matt" wrote:
>
> > This patch will remove freelist.h from kernel source tree, since the
> > only use cases (kretprobe and rethook) are converted to objpool.
> >
> > Signed-off-by: wuqiang.matt
> > ---
> > include/linux/freelist.h | 129 ---
> > 1 file changed, 129 deletions(-)
> > delete mode 100644 include/linux/freelist.h
> >
> > diff --git a/include/linux/freelist.h b/include/linux/freelist.h
> > deleted file mode 100644
> > index fc1842b96469..
> > --- a/include/linux/freelist.h
> > +++ /dev/null
> > @@ -1,129 +0,0 @@
> > -/* SPDX-License-Identifier: GPL-2.0-only OR BSD-2-Clause */
> > -#ifndef FREELIST_H
> > -#define FREELIST_H
> > -
> > -#include
> > -
> > -/*
> > - * Copyright: came...@moodycamel.com
> > - *
> > - * A simple CAS-based lock-free free list. Not the fastest thing in the
> > world
> > - * under heavy contention, but simple and correct (assuming nodes are never
> > - * freed until after the free list is destroyed), and fairly speedy under
> > low
> > - * contention.
> > - *
> > - * Adapted from:
> > https://moodycamel.com/blog/2014/solving-the-aba-problem-for-lock-free-free-lists
> > - */
> > -
> > -struct freelist_node {
> > - atomic_trefs;
> > - struct freelist_node*next;
> > -};
> > -
> > -struct freelist_head {
> > - struct freelist_node*head;
> > -};
> > -
> > -#define REFS_ON_FREELIST 0x8000
> > -#define REFS_MASK 0x7FFF
> > -
> > -static inline void __freelist_add(struct freelist_node *node, struct
> > freelist_head *list)
> > -{
> > - /*
> > -* Since the refcount is zero, and nobody can increase it once it's
> > -* zero (except us, and we run only one copy of this method per node at
> > -* a time, i.e. the single thread case), then we know we can safely
> > -* change the next pointer of the node; however, once the refcount is
> > -* back above zero, then other threads could increase it (happens under
> > -* heavy contention, when the refcount goes to zero in between a load
> > -* and a refcount increment of a node in try_get, then back up to
> > -* something non-zero, then the refcount increment is done by the other
> > -* thread) -- so if the CAS to add the node to the actual list fails,
> > -* decrese the refcount and leave the add operation to the next thread
> > -* who puts the refcount back to zero (which could be us, hence the
> > -* loop).
> > -*/
> > - struct freelist_node *head = READ_ONCE(list->head);
> > -
> > - for (;;) {
> > - WRITE_ONCE(node->next, head);
> > - atomic_set_release(>refs, 1);
> > -
> > - if (!try_cmpxchg_release(>head, , node)) {
> > - /*
> > -* Hmm, the add failed, but we can only try again when
> > -* the refcount goes back to zero.
> > -*/
> > - if (atomic_fetch_add_release(REFS_ON_FREELIST - 1,
> > >refs) == 1)
> > - continue;
> > - }
> > - return;
> > - }
> > -}
> > -
> > -static inline void freelist_add(struct freelist_node *node, struct
> > freelist_head *list)
> > -{
> > - /*
> > -* We know that the should-be-on-freelist bit is 0 at this point, so
> > -* it's safe to set it using a fetch_add.
> > -*/
> > - if (!atomic_fetch_add_release(REFS_ON_FREELIST, >refs)) {
> > - /*
> > -* Oh look! We were the last ones referencing this node, and we
> > -* know we want to add it to the free list, so let's do it!
> > -*/
> > - __freelist_add(node, list);
> > - }
> > -}
> > -
> > -static inline struct freelist_node *freelist_try_get(struct freelist_head
> > *list)
> > -{
> > - struct freelist_node *prev, *next, *head =
> > smp_load_acquire(>head);
> > - unsigned int refs;
> > -
> > - while (head) {
> > - prev = head;
> > - refs = atomic_read(>refs);
> > - if ((refs & REFS_MASK) == 0 ||
> > - !atomic_try_cmpxchg_acquire(>refs, , refs+1)) {
> > - head = smp_load_acquire(>head);
> > - continue;
> > - }
> > -
> > - /*
> > -* Good, reference count has been incremented (it wasn't at
> > -* zero), which means we can read the next and not worry about
> > -* it changing between now and the time we do the CAS.
> > -*/
> > - next = READ_ONCE(head->next);
> > - if (try_cmpxchg_acquire(>head, , next)) {
> > - /*
> > -
Re: [PATCH v11 0/5] lib,kprobes: kretprobe scalability improvement
Hi, On Tue, 17 Oct 2023 21:56:49 +0800 "wuqiang.matt" wrote: > This patch series introduces a scalable and lockless ring-array based > object pool to improve scalability of kretprobed routines. > > v11: > *) patchset rebased to branch probes/core of linux-trace.git > *) objpool: objpool_fini optimized for better code readability > *) test_objpool: asynchronous releasing of objpool now covered > > wuqiang.matt (5): > lib: objpool added: ring-array based lockless MPMC > lib: objpool test module added > kprobes: kretprobe scalability improvement with objpool > kprobes: freelist.h removed > MAINTAINERS: objpool added OK, this version looks good to me. Acked-by: Masami Hiramatsu (Google) I'll pick this series on for-next branch. Thank you, > > MAINTAINERS | 7 + > include/linux/freelist.h | 129 > include/linux/kprobes.h | 11 +- > include/linux/objpool.h | 176 ++ > include/linux/rethook.h | 16 +- > kernel/kprobes.c | 93 +++--- > kernel/trace/fprobe.c| 32 +- > kernel/trace/rethook.c | 90 +++-- > lib/Kconfig.debug| 11 + > lib/Makefile | 4 +- > lib/objpool.c| 286 > lib/test_objpool.c | 689 +++ > 12 files changed, 1270 insertions(+), 274 deletions(-) > delete mode 100644 include/linux/freelist.h > create mode 100644 include/linux/objpool.h > create mode 100644 lib/objpool.c > create mode 100644 lib/test_objpool.c > > -- > 2.40.1 > -- Masami Hiramatsu (Google)
[tip:sched/core] [sched/numa] f169c62ff7: autonuma-benchmark.numa01.seconds -32.0% improvement
Hello,
kernel test robot noticed a -32.0% improvement of
autonuma-benchmark.numa01.seconds on:
commit: f169c62ff7cd1acf8bac8ae17bfeafa307d9e6fa ("sched/numa: Complete
scanning of inactive VMAs when there is no alternative")
https://git.kernel.org/cgit/linux/kernel/git/tip/tip.git sched/core
testcase: autonuma-benchmark
test machine: 96 threads 2 sockets Intel(R) Xeon(R) Platinum 8260L CPU @
2.40GHz (Cascade Lake) with 128G memory
parameters:
iterations: 4x
test: _HARD_BIND
cpufreq_governor: performance
Details are as below:
-->
The kernel config and materials to reproduce are available at:
https://download.01.org/0day-ci/archive/20231018/202310182223.6ef26fcb-oliver.s...@intel.com
=
compiler/cpufreq_governor/iterations/kconfig/rootfs/tbox_group/test/testcase:
gcc-12/performance/4x/x86_64-rhel-8.3/debian-11.1-x86_64-20220510.cgz/lkp-csl-2sp3/_HARD_BIND/autonuma-benchmark
commit:
b7a5b537c5 ("sched/numa: Complete scanning of partial VMAs regardless of PID
activity")
f169c62ff7 ("sched/numa: Complete scanning of inactive VMAs when there is no
alternative")
b7a5b537c55c088d f169c62ff7cd1acf8bac8ae17bf
---
%stddev %change %stddev
\ |\
0.00 ± 58% +3290.8% 0.06 ± 30% vmstat.procs.b
2.101e+10 ± 6% -47.7% 1.099e+10 ± 5% cpuidle..time
21679574 ± 6% -47.8% 11321089 ± 5% cpuidle..usage
2441 -25.2% 1826 ± 2% uptime.boot
25666 ± 5% -40.3% 15332 ± 3% uptime.idle
208968 ± 8% -11.6% 184676 ± 3% meminfo.Active
208280 ± 8% -11.7% 183990 ± 3% meminfo.Active(anon)
221680 ± 8% -11.7% 195741 ± 2% meminfo.Shmem
3154708 ± 7% -12.1%2773727 ± 4% numa-numastat.node0.local_node
3621021 ± 4% -12.9%3154008 ± 3% numa-numastat.node0.numa_hit
3424075 ± 5% -19.2%2767342 ± 3% numa-numastat.node1.local_node
3659250 ± 4% -15.8%3079449 ± 3% numa-numastat.node1.numa_hit
3620012 ± 4% -12.9%3153670 ± 3% numa-vmstat.node0.numa_hit
3153693 ± 7% -12.1%2773388 ± 4% numa-vmstat.node0.numa_local
3657708 ± 4% -15.8%3078660 ± 3% numa-vmstat.node1.numa_hit
3422533 ± 5% -19.2%2766552 ± 3% numa-vmstat.node1.numa_local
0.03 ± 31%+840.9% 0.29 ±179%
perf-sched.sch_delay.avg.ms.exit_to_user_mode_loop.exit_to_user_mode_prepare.syscall_exit_to_user_mode.do_syscall_64
2.34 ± 49% -83.7% 0.38 ±194%
perf-sched.sch_delay.max.ms.__cond_resched.down_write_killable.setup_arg_pages.load_elf_binary.search_binary_handler
948.50 ± 16% -30.3% 661.33 ± 20%
perf-sched.wait_and_delay.count.exit_to_user_mode_loop.exit_to_user_mode_prepare.syscall_exit_to_user_mode.do_syscall_64
1142 ± 9% -24.3% 865.00 ± 9%
perf-sched.wait_and_delay.count.pipe_read.vfs_read.ksys_read.do_syscall_64
9.20 ± 6% -2.86.43 ± 5% mpstat.cpu.all.idle%
0.00 ± 14% +0.00.03 ± 28% mpstat.cpu.all.iowait%
2.25-0.51.73mpstat.cpu.all.irq%
0.08 ± 3% -0.00.06 ± 2% mpstat.cpu.all.soft%
2.18 ± 4% +0.52.69 ± 2% mpstat.cpu.all.sys%
474.78 -32.0% 322.76 ± 2% autonuma-benchmark.numa01.seconds
2386 -25.7% 1774 ± 2%
autonuma-benchmark.time.elapsed_time
2386 -25.7% 1774 ± 2%
autonuma-benchmark.time.elapsed_time.max
1298583 ± 2% -24.3% 983395 ± 3%
autonuma-benchmark.time.involuntary_context_switches
3120169-6.0%2932947
autonuma-benchmark.time.minor_page_faults
8443+3.9% 8771
autonuma-benchmark.time.percent_of_cpu_this_job_got
197252 -23.2% 151486 ± 2% autonuma-benchmark.time.user_time
26055 +45.4% 37896 ± 8%
autonuma-benchmark.time.voluntary_context_switches
42526 ± 9% -15.6% 35892 ± 10% turbostat.C1
0.03+0.00.05 ± 20% turbostat.C1E%
21430219 ± 6% -48.2% 11099622 ± 5% turbostat.C6
9.14 ± 6% -2.76.40 ± 5% turbostat.C6%
8.80 ± 6% -31.1% 6.06 ± 5% turbostat.CPU%c1
2.31e+08 -24.9% 1.735e+08 ± 2% turbostat.IRQ
37846 ± 7% -29.3% 26768 ± 9% turbostat.POLL
283.70+3.1% 292.44turbostat.PkgWatt
63.32 +16.3% 73.65turbostat.RAMWatt
52079 ± 8% -11.6% 46034 ± 3% proc-vmstat.nr_active_anon
1560738-1.8%1532470proc-vmstat.nr_anon_pages
3000
Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
Hi! Le mercredi 18 octobre 2023, 09:30:20 EEST Masami Hiramatsu a écrit : > Hi Francis, > > On Thu, 31 Aug 2023 09:14:55 +0200 > > Francis Laniel wrote: > > Hi. > > > > Le mercredi 30 août 2023, 01:57:19 CEST Steven Rostedt a écrit : > > > On Fri, 25 Aug 2023 22:13:21 +0900 > > > > > > Masami Hiramatsu (Google) wrote: > > > > > Excellent catch! Thank you, I will apply this patch and send v4 > > > > > right > > > > > after. Regarding test, do you think I can add a test for the > > > > > EADDRNOTAVAIL case?> > > > > > > > > Hmm, in that case, you need to change something in tracefs/README so > > > > that > > > > we can identify the kernel has different behavior. Or we have to > > > > change > > > > this is a "Fix" for backporting. > > > > > > I prefer this to be a Fix and backported. > > > > This makes sense, I will send v5 to stable mailing list too! > > I missed this a while. did you send v5 ? I could not find in my mbox. Sorry, I took a bit of time before sending the v5 as I wanted to wait for Alessandro patchset to be merged first. As it seems more work is needed on his contribution I think we can go with this fix first. > Thank you, > > > > Thanks, > > > > > > -- Steve > > > > Best regards. Best regards.
[PATCH v5 1/2] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
Previously to this commit, if func matches several symbols, a kprobe, being
either sysfs or PMU, would only be installed for the first matching address.
This could lead to some misunderstanding when some BPF code was never called
because it was attached to a function which was indeed not called, because
the effectively called one has no kprobes attached.
So, this commit returns EADDRNOTAVAIL when func matches several symbols.
This way, user needs to use address to remove the ambiguity.
Suggested-by: Masami Hiramatsu
Signed-off-by: Francis Laniel
Link:
https://lore.kernel.org/lkml/20230819101105.b0c104ae4494a7d1f2eea...@kernel.org/
---
kernel/trace/trace_kprobe.c | 63 +
kernel/trace/trace_probe.h | 1 +
2 files changed, 64 insertions(+)
diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c
index 3d7a180a8427..a8fef6ab0872 100644
--- a/kernel/trace/trace_kprobe.c
+++ b/kernel/trace/trace_kprobe.c
@@ -705,6 +705,25 @@ static struct notifier_block trace_kprobe_module_nb = {
.priority = 1 /* Invoked after kprobe module callback */
};
+static int count_symbols(void *data, unsigned long unused)
+{
+ unsigned int *count = data;
+
+ (*count)++;
+
+ return 0;
+}
+
+static unsigned int number_of_same_symbols(char *func_name)
+{
+ unsigned int count;
+
+ count = 0;
+ kallsyms_on_each_match_symbol(count_symbols, func_name, );
+
+ return count;
+}
+
static int __trace_kprobe_create(int argc, const char *argv[])
{
/*
@@ -836,6 +855,31 @@ static int __trace_kprobe_create(int argc, const char
*argv[])
}
}
+ if (symbol && !strchr(symbol, ':')) {
+ unsigned int count;
+
+ count = number_of_same_symbols(symbol);
+ if (count > 1) {
+ /*
+* Users should use ADDR to remove the ambiguity of
+* using KSYM only.
+*/
+ trace_probe_log_err(0, NON_UNIQ_SYMBOL);
+ ret = -EADDRNOTAVAIL;
+
+ goto error;
+ } else if (count == 0) {
+ /*
+* We can return ENOENT earlier than when register the
+* kprobe.
+*/
+ trace_probe_log_err(0, BAD_PROBE_ADDR);
+ ret = -ENOENT;
+
+ goto error;
+ }
+ }
+
trace_probe_log_set_index(0);
if (event) {
ret = traceprobe_parse_event_name(, , gbuf,
@@ -1695,6 +1739,7 @@ static int unregister_kprobe_event(struct trace_kprobe
*tk)
}
#ifdef CONFIG_PERF_EVENTS
+
/* create a trace_kprobe, but don't add it to global lists */
struct trace_event_call *
create_local_trace_kprobe(char *func, void *addr, unsigned long offs,
@@ -1705,6 +1750,24 @@ create_local_trace_kprobe(char *func, void *addr,
unsigned long offs,
int ret;
char *event;
+ if (func) {
+ unsigned int count;
+
+ count = number_of_same_symbols(func);
+ if (count > 1)
+ /*
+* Users should use addr to remove the ambiguity of
+* using func only.
+*/
+ return ERR_PTR(-EADDRNOTAVAIL);
+ else if (count == 0)
+ /*
+* We can return ENOENT earlier than when register the
+* kprobe.
+*/
+ return ERR_PTR(-ENOENT);
+ }
+
/*
* local trace_kprobes are not added to dyn_event, so they are never
* searched in find_trace_kprobe(). Therefore, there is no concern of
diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h
index 02b432ae7513..850d9ecb6765 100644
--- a/kernel/trace/trace_probe.h
+++ b/kernel/trace/trace_probe.h
@@ -450,6 +450,7 @@ extern int traceprobe_define_arg_fields(struct
trace_event_call *event_call,
C(BAD_MAXACT, "Invalid maxactive number"),\
C(MAXACT_TOO_BIG, "Maxactive is too big"),\
C(BAD_PROBE_ADDR, "Invalid probed address or symbol"),\
+ C(NON_UNIQ_SYMBOL, "The symbol is not unique"),\
C(BAD_RETPROBE, "Retprobe address must be an function entry"), \
C(NO_TRACEPOINT,"Tracepoint is not found"), \
C(BAD_ADDR_SUFFIX, "Invalid probed address suffix"), \
--
2.34.1
[PATCH v5 0/2] Return EADDRNOTAVAIL when func matches several symbols during kprobe creation
Hi. In the kernel source code, it exists different functions which share the same name but which have, of course, different addresses as they can be defined in different modules: # Kernel was compiled with CONFIG_NTFS_FS and CONFIG_NTFS3_FS as built-in. root@vm-amd64:~# grep ntfs_file_write_iter /proc/kallsyms 814ce3c0 t __pfx_ntfs_file_write_iter 814ce3d0 t ntfs_file_write_iter 814fc8a0 t __pfx_ntfs_file_write_iter 814fc8b0 t ntfs_file_write_iter This can be source of troubles when you create a PMU kprobe for such a function, as it will only install one for the first address (e.g. 0x814ce3d0 in the above). This could lead to some troubles were BPF based tools does not report any event because the second function is not called: root@vm-amd64:/mnt# mount | grep /mnt /foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8) # ig is a tool which installs a PMU kprobe on ntfs_file_write_iter(). root@vm-amd64:/mnt# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo & [1] 207 root@vm-amd64:/mnt# dd if=./foo of=./bar count=3 3+0 records in 3+0 records out 1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00543323 s, 283 kB/s root@vm-amd64:/mnt# fg ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo ^Croot@vm-amd64:/mnt# more /tmp/foo RUNTIME.CONTAINERNAME RUNTIME.CONTAIN… PID COMM T BYTES OFFSETLAT FILE 214 dd R512 0766 foo 214 dd R512512 9 foo 214 dd As you can see in the above, only read events are reported and no write because the kprobe is installed for the old ntfs_file_write_iter() and not the ntfs3 one. The same behavior occurs with sysfs kprobe: root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter' > /sys/kernel/tracing/kprobe_events root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events p:probe/ntfs_file_write_iter ntfs_file_write_iter root@vm-amd64:/# mount | grep /mnt /foo.img on /mnt type ntfs3 (rw,relatime,uid=0,gid=0,iocharset=utf8) root@vm-amd64:/# perf record -e probe:ntfs_file_write_iter & [1] 210 root@vm-amd64:/# cd /mnt/ root@vm-amd64:/mnt# dd if=./foo of=./bar count=3 3+0 records in 3+0 records out 1536 bytes (1.5 kB, 1.5 KiB) copied, 0.00234793 s, 654 kB/s root@vm-amd64:/mnt# cd - / root@vm-amd64:/# fg perf record -e probe:ntfs_file_write_iter ^C[ perf record: Woken up 1 times to write data ] [ perf record: Captured and wrote 0.056 MB perf.data ] root@vm-amd64:/# perf report Error: The perf.data data has no samples! # To display the perf.data header info, please use --header/--header-only optio> # In this contribution, I modified the functions creating sysfs and PMU kprobes to test if the function name given as argument matches several symbols. In this case, these functions return EADDRNOTAVAIL to indicate the user to use addr and offs to remove this ambiguity. So, when the above BPF tool is run, the following error message is printed: root@vm-amd64:~# ig trace fsslower -m 0 -f ntfs3 --host &> /tmp/foo & [1] 228 root@vm-amd64:~# more /tmp/foo RUNTIME.CONTAINERNAME RUNTIME.CONTAIN… PID COMM T BYTES OFFSETLAT FILE Error: running gadget: running gadget: installing tracer: attaching kprobe: crea ting perf_kprobe PMU (arch-specific fallback for "ntfs_file_write_iter"): token ntfs_file_write_iter: opening perf event: cannot assign requested address And the same with sysfs kprobe: root@vm-amd64:/# echo 'p:probe/ntfs_file_write_iter ntfs_file_write_iter' > /sys/kernel/tracing/kprobe_events -bash: echo: write error: Cannot assign requested address Note that, this does not influence perf as it installs kprobes as offset on _text: root@vm-amd64:/# perf probe --add ntfs_file_write_iter Added new events: probe:ntfs_file_write_iter (on ntfs_file_write_iter) probe:ntfs_file_write_iter (on ntfs_file_write_iter) ... root@vm-amd64:/# cat /sys/kernel/tracing/kprobe_events p:probe/ntfs_file_write_iter _text+5039088 p:probe/ntfs_file_write_iter _text+5228752 Note that, this contribution is the conclusion of a previous RFC which intended to install a PMU kprobe for all matching symbols [1, 2]. If you see any way to improve this contribution, particularly if you have an idea to add tests or documentation for this behavior, please share your feedback. Changes since: v1: * Use EADDRNOTAVAIL instead of adding a new error code. * Correct also this behavior for sysfs kprobe. v2: * Count the number of symbols corresponding to function name and return EADDRNOTAVAIL if higher than 1. * Return ENOENT if above count is 0, as it would be returned later by while registering the kprobe. v3: * Check symbol does not contain ':' before testing its uniqueness. * Add a selftest to check this is not possible to install a kprobe
Re: [PATCH v5 12/18] x86/sgx: Add EPC OOM path to forcefully reclaim EPC
On 10/17/23 21:37, Haitao Huang wrote: > Yes we can introduce misc.reclaim to give user a knob to forcefully > reducing usage if that is really needed in real usage. The semantics > would make force-kill VMs explicit to user. Do any other controllers do something like this? It seems odd.
Re: [PATCH v2] ACPI: NFIT: Optimize nfit_mem_cmp() for efficiency
On Fri, Oct 13, 2023 at 2:22 PM Kuan-Wei Chiu wrote: > > The original code used conditional branching in the nfit_mem_cmp > function to compare two values and return -1, 1, or 0 based on the > result. However, the list_sort comparison function only needs results > <0, >0, or =0. This patch optimizes the code to make the comparison > branchless, improving efficiency and reducing code size. This change > reduces the number of comparison operations from 1-2 to a single > subtraction operation, thereby saving the number of instructions. > > Signed-off-by: Kuan-Wei Chiu > --- > v1 -> v2: > - Add explicit type cast in case the sizes of u32 and int differ. > > drivers/acpi/nfit/core.c | 6 +- > 1 file changed, 1 insertion(+), 5 deletions(-) > > diff --git a/drivers/acpi/nfit/core.c b/drivers/acpi/nfit/core.c > index f96bf32cd368..563a32eba888 100644 > --- a/drivers/acpi/nfit/core.c > +++ b/drivers/acpi/nfit/core.c > @@ -1138,11 +1138,7 @@ static int nfit_mem_cmp(void *priv, const struct > list_head *_a, > > handleA = __to_nfit_memdev(a)->device_handle; > handleB = __to_nfit_memdev(b)->device_handle; > - if (handleA < handleB) > - return -1; > - else if (handleA > handleB) > - return 1; > - return 0; > + return (int)handleA - (int)handleB; Are you sure that you are not losing bits in these conversions? > } > > static int nfit_mem_init(struct acpi_nfit_desc *acpi_desc) > --
Re: [PATCH v3] ACPI: NFIT: Use cleanup.h helpers instead of devm_*()
On Wed, Oct 18, 2023 at 6:28 AM Dan Williams wrote: > > Michal Wilczynski wrote: > > The new cleanup.h facilities that arrived in v6.5-rc1 can replace the > > the usage of devm semantics in acpi_nfit_init_interleave_set(). That > > routine appears to only be using devm to avoid goto statements. The > > new __free() annotation at variable declaration time can achieve the same > > effect more efficiently. > > > > There is no end user visible side effects of this patch, I was > > motivated to send this cleanup to practice using the new helpers. > > > > Suggested-by: Dave Jiang > > Suggested-by: Andy Shevchenko > > Reviewed-by: Dave Jiang > > Reviewed-by: Andy Shevchenko > > Signed-off-by: Michal Wilczynski > > --- > > > > Dan, would you like me to give you credit for the changelog changes > > with Co-developed-by tag ? > > Nope, this looks good to me, thanks for fixing it up. > > Reviewed-by: Dan Williams Are you going to apply it too, or should I take it?
Re: [PATCH] neighbor: tracing: Move pin6 inside CONFIG_IPV6=y section
Hello:
This patch was applied to netdev/net.git (main)
by David S. Miller :
On Mon, 16 Oct 2023 14:49:04 +0200 you wrote:
> When CONFIG_IPV6=n, and building with W=1:
>
> In file included from include/trace/define_trace.h:102,
>from include/trace/events/neigh.h:255,
>from net/core/net-traces.c:51:
> include/trace/events/neigh.h: In function
> ‘trace_event_raw_event_neigh_create’:
> include/trace/events/neigh.h:42:34: error: variable ‘pin6’ set but not
> used [-Werror=unused-but-set-variable]
>42 | struct in6_addr *pin6;
> | ^~~~
> include/trace/trace_events.h:402:11: note: in definition of macro
> ‘DECLARE_EVENT_CLASS’
> 402 | { assign; }
> \
> | ^~
> include/trace/trace_events.h:44:30: note: in expansion of macro ‘PARAMS’
>44 | PARAMS(assign), \
> | ^~
> include/trace/events/neigh.h:23:1: note: in expansion of macro
> ‘TRACE_EVENT’
>23 | TRACE_EVENT(neigh_create,
> | ^~~
> include/trace/events/neigh.h:41:9: note: in expansion of macro
> ‘TP_fast_assign’
>41 | TP_fast_assign(
> | ^~
> In file included from include/trace/define_trace.h:103,
>from include/trace/events/neigh.h:255,
>from net/core/net-traces.c:51:
> include/trace/events/neigh.h: In function ‘perf_trace_neigh_create’:
> include/trace/events/neigh.h:42:34: error: variable ‘pin6’ set but not
> used [-Werror=unused-but-set-variable]
>42 | struct in6_addr *pin6;
> | ^~~~
> include/trace/perf.h:51:11: note: in definition of macro
> ‘DECLARE_EVENT_CLASS’
>51 | { assign; }
> \
> | ^~
> include/trace/trace_events.h:44:30: note: in expansion of macro ‘PARAMS’
>44 | PARAMS(assign), \
> | ^~
> include/trace/events/neigh.h:23:1: note: in expansion of macro
> ‘TRACE_EVENT’
>23 | TRACE_EVENT(neigh_create,
> | ^~~
> include/trace/events/neigh.h:41:9: note: in expansion of macro
> ‘TP_fast_assign’
>41 | TP_fast_assign(
> | ^~
>
> [...]
Here is the summary with links:
- neighbor: tracing: Move pin6 inside CONFIG_IPV6=y section
https://git.kernel.org/netdev/net/c/2915240eddba
You are awesome, thank you!
--
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html
Re: [PATCH 2/2] arm64: dts: qcom: msm8939-longcheer-l9100: Enable RGB LED
On 10/13/23 22:51, André Apitzsch wrote: l9100 uses KTD2026 LED driver. Add it to the device tree. Signed-off-by: André Apitzsch --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH 1/2] arm64: dts: qcom: msm8916-longcheer-l8910: Enable RGB LED
On 10/13/23 22:51, André Apitzsch wrote: l8910 uses KTD2026 LED driver. Add it to the device tree. Tested-by: Stephan Gerhold Signed-off-by: André Apitzsch --- Reviewed-by: Konrad Dybcio Konrad
Re: [PATCH] arm64: dts: qcom: msm8939-longcheer-l9100: Add proximity-near-level
On 10/17/23 22:03, André Apitzsch wrote: Am Dienstag, dem 17.10.2023 um 18:25 +0200 schrieb Konrad Dybcio: On 10/16/23 22:18, André Apitzsch wrote: Consider an object near to the sensor when their distance is about 4 cm or below. Signed-off-by: André Apitzsch --- Reviewed-by: Konrad Dybcio Out of interest, what is it set to by default? Konrad The default value is 0. That much I could guess :) I was wondering if it meant more like "it can detect movement from 1km away" or "disabled" Konrad
[PATCH 0/2] some fix of multi-gen lru
For multi-gen lru shrink_inactive trace, here are some mistake: First, nr_scanned in evict_folios means all folios which it touched in `get_nr_to_scan`(so not means nr_taken). So, we may get some info from trace like this: ``` kswapd0-89[000]64.887613: mm_vmscan_lru_shrink_inactive: nid=0 nr_scanned=64 nr_reclaimed=0 nr_dirty=0 nr_writeback=0 nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0 nr_ref_keep=0 nr_unmap_fail=0 priority=4 flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC ``` Actually, we don't taken too much folios in shrink. This patch use difference between before sc->nr_scanned and after shrink to replace nr_taken and pass this to shrink_inactive tracing. Second, also like above info, we can't get nr_folios going to where, actually in multi-gen lru shrink, many keep by folio_update_gen when look around or other promote this folio. Third, evict_folios don't gather reclaim_stat info after shrink list, so, dirty\writeback\congested stat will miss, and we can't control LRUVEC_CONGESTED or reclaim_throttle. Patch 1 aim to resolve first and second, patch 2 resolve third. Huan Yang (2): tracing: mm: multigen-lru: fix mglru trace mm: multi-gen lru: fix stat count include/linux/vmstat.h| 2 ++ include/trace/events/vmscan.h | 8 - mm/vmscan.c | 61 --- 3 files changed, 65 insertions(+), 6 deletions(-) -- 2.34.1
[PATCH 2/2] mm: multi-gen lru: fix stat count
For multi-gen lru reclaim in evict_folios, like shrink_inactive_list,
gather folios which isolate to reclaim, and invoke shirnk_folio_list.
But, when complete shrink, it not gather shrink reclaim stat into sc,
we can't get info like nr_dirty\congested in reclaim, and then
control writeback, dirty number and mark as LRUVEC_CONGESTED, or
just bpf trace shrink and get correct sc stat.
This patch fix this by simple copy code from shrink_inactive_list when
end of shrink list.
Signed-off-by: Huan Yang
---
mm/vmscan.c | 35 +++
1 file changed, 35 insertions(+)
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 21099b9f21e0..88d1d586aea5 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -4593,6 +4593,41 @@ static int evict_folios(struct lruvec *lruvec, struct
scan_control *sc, int swap
*/
nr_taken = sc->nr_scanned - nr_taken;
+ /*
+* If dirty folios are scanned that are not queued for IO, it
+* implies that flushers are not doing their job. This can
+* happen when memory pressure pushes dirty folios to the end of
+* the LRU before the dirty limits are breached and the dirty
+* data has expired. It can also happen when the proportion of
+* dirty folios grows not through writes but through memory
+* pressure reclaiming all the clean cache. And in some cases,
+* the flushers simply cannot keep up with the allocation
+* rate. Nudge the flusher threads in case they are asleep.
+*/
+ if (unlikely(stat.nr_unqueued_dirty == nr_taken)) {
+ wakeup_flusher_threads(WB_REASON_VMSCAN);
+ /*
+* For cgroupv1 dirty throttling is achieved by waking up
+* the kernel flusher here and later waiting on folios
+* which are in writeback to finish (see shrink_folio_list()).
+*
+* Flusher may not be able to issue writeback quickly
+* enough for cgroupv1 writeback throttling to work
+* on a large system.
+*/
+ if (!writeback_throttling_sane(sc))
+ reclaim_throttle(pgdat, VMSCAN_THROTTLE_WRITEBACK);
+ }
+
+ sc->nr.dirty += stat.nr_dirty;
+ sc->nr.congested += stat.nr_congested;
+ sc->nr.unqueued_dirty += stat.nr_unqueued_dirty;
+ sc->nr.writeback += stat.nr_writeback;
+ sc->nr.immediate += stat.nr_immediate;
+ sc->nr.taken += nr_taken;
+ if (type)
+ sc->nr.file_taken += nr_taken;
+
sc->nr_reclaimed += total_reclaimed;
trace_mm_vmscan_lru_shrink_inactive(pgdat->node_id, nr_taken,
total_reclaimed, ,
--
2.34.1
[PATCH 1/2] tracing: mm: multigen-lru: fix mglru trace
This patch add two reclaim stat:
nr_promote: nr_pages shrink before promote by folio_update_gen.
nr_demote: nr_pages NUMA demotion passed.
And then, use correct nr_scanned which evict_folios passed into
trace_mm_vmscan_lru_shrink_inactive.
Mistake info like this:
```
kswapd0-89[000]64.887613: mm_vmscan_lru_shrink_inactive:
nid=0 nr_scanned=64 nr_reclaimed=0 nr_dirty=0 nr_writeback=0
nr_congested=0 nr_immediate=0 nr_activate_anon=0 nr_activate_file=0
nr_ref_keep=0 nr_unmap_fail=0 priority=4
flags=RECLAIM_WB_FILE|RECLAIM_WB_ASYNC
```
Correct info like this:
```
<...>-9041 [006]43.738481: mm_vmscan_lru_shrink_inactive:
nid=0 nr_scanned=13 nr_reclaimed=0 nr_dirty=0 nr_writeback=0
nr_congested=0 nr_immediate=0 nr_activate_anon=9 nr_activate_file=0
nr_ref_keep=0 nr_unmap_fail=0 nr_promote=4 nr_demote=0 priority=12
flags=RECLAIM_WB_ANON|RECLAIM_WB_ASYNC
```
Signed-off-by: Huan Yang
---
include/linux/vmstat.h| 2 ++
include/trace/events/vmscan.h | 8 +++-
mm/vmscan.c | 26 +-
3 files changed, 30 insertions(+), 6 deletions(-)
diff --git a/include/linux/vmstat.h b/include/linux/vmstat.h
index fed855bae6d8..ac2dd9168780 100644
--- a/include/linux/vmstat.h
+++ b/include/linux/vmstat.h
@@ -32,6 +32,8 @@ struct reclaim_stat {
unsigned nr_ref_keep;
unsigned nr_unmap_fail;
unsigned nr_lazyfree_fail;
+ unsigned nr_promote;
+ unsigned nr_demote;
};
enum writeback_stat_item {
diff --git a/include/trace/events/vmscan.h b/include/trace/events/vmscan.h
index 1a488c30afa5..9b403824a293 100644
--- a/include/trace/events/vmscan.h
+++ b/include/trace/events/vmscan.h
@@ -366,6 +366,8 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
__field(unsigned int, nr_activate1)
__field(unsigned long, nr_ref_keep)
__field(unsigned long, nr_unmap_fail)
+ __field(unsigned long, nr_promote)
+ __field(unsigned long, nr_demote)
__field(int, priority)
__field(int, reclaim_flags)
),
@@ -382,17 +384,21 @@ TRACE_EVENT(mm_vmscan_lru_shrink_inactive,
__entry->nr_activate1 = stat->nr_activate[1];
__entry->nr_ref_keep = stat->nr_ref_keep;
__entry->nr_unmap_fail = stat->nr_unmap_fail;
+ __entry->nr_promote = stat->nr_promote;
+ __entry->nr_demote = stat->nr_demote;
__entry->priority = priority;
__entry->reclaim_flags = trace_reclaim_flags(file);
),
- TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld
nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d
nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld priority=%d flags=%s",
+ TP_printk("nid=%d nr_scanned=%ld nr_reclaimed=%ld nr_dirty=%ld
nr_writeback=%ld nr_congested=%ld nr_immediate=%ld nr_activate_anon=%d"
+ " nr_activate_file=%d nr_ref_keep=%ld nr_unmap_fail=%ld nr_promote=%ld
nr_demote=%ld priority=%d flags=%s",
__entry->nid,
__entry->nr_scanned, __entry->nr_reclaimed,
__entry->nr_dirty, __entry->nr_writeback,
__entry->nr_congested, __entry->nr_immediate,
__entry->nr_activate0, __entry->nr_activate1,
__entry->nr_ref_keep, __entry->nr_unmap_fail,
+ __entry->nr_promote, __entry->nr_demote,
__entry->priority,
show_reclaim_flags(__entry->reclaim_flags))
);
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 2cc0cb41fb32..21099b9f21e0 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -1063,8 +1063,10 @@ static unsigned int shrink_folio_list(struct list_head
*folio_list,
/* folio_update_gen() tried to promote this page? */
if (lru_gen_enabled() && !ignore_references &&
- folio_mapped(folio) && folio_test_referenced(folio))
+ folio_mapped(folio) && folio_test_referenced(folio)) {
+ stat->nr_promote += nr_pages;
goto keep_locked;
+ }
/*
* The number of dirty pages determines if a node is marked
@@ -1193,6 +1195,7 @@ static unsigned int shrink_folio_list(struct list_head
*folio_list,
(thp_migration_supported() || !folio_test_large(folio))) {
list_add(>lru, _folios);
folio_unlock(folio);
+ stat->nr_demote += nr_pages;
continue;
}
@@ -4495,6 +4498,8 @@ static int evict_folios(struct lruvec *lruvec, struct
scan_control *sc, int swap
int type;
int scanned;
int reclaimed;
+ unsigned long nr_taken = sc->nr_scanned;
+ unsigned int total_reclaimed = 0;
LIST_HEAD(list);
LIST_HEAD(clean);
struct folio *folio;
@@ -4521,10 +4526,7 @@ static
Re: [PATCH 2/3] usb: typec: fsa4480: Add support to swap SBU orientation
Hi Luca, > > Shouldn't you loop through the endpoints? In any case: > > > > ep = fwnode_graph_get_next_endpoint(dev_fwnode(>client->dev, > > NULL)); > > The docs only mention one endpoint so I'm assuming just next_endpoint is > fine? I'm mostly concerned about what we may have in the future. If one day you have more than the one connection in your graph, then you have to be able to identify the endpoint you are after. But that may not be a problem in this case (maybe that "data-lanes" device property can be used to identify the correct endpoint?). We can worry about it then when/if we ever have another endpoint to deal with. thanks, -- heikki
Re: [PATCH v3 1/1] tracing/kprobes: Return EADDRNOTAVAIL when func matches several symbols
Hi Francis, On Thu, 31 Aug 2023 09:14:55 +0200 Francis Laniel wrote: > Hi. > > Le mercredi 30 août 2023, 01:57:19 CEST Steven Rostedt a écrit : > > On Fri, 25 Aug 2023 22:13:21 +0900 > > > > Masami Hiramatsu (Google) wrote: > > > > Excellent catch! Thank you, I will apply this patch and send v4 right > > > > after. Regarding test, do you think I can add a test for the > > > > EADDRNOTAVAIL case?> > > > Hmm, in that case, you need to change something in tracefs/README so that > > > we can identify the kernel has different behavior. Or we have to change > > > this is a "Fix" for backporting. > > > > I prefer this to be a Fix and backported. > > This makes sense, I will send v5 to stable mailing list too! I missed this a while. did you send v5 ? I could not find in my mbox. Thank you, > > > Thanks, > > > > -- Steve > > Best regards. > > -- Masami Hiramatsu (Google)
