Re: [PATCH v3] kernel: add panic_on_taint
On 05/09/20 at 09:57am, Rafael Aquini wrote: > Analogously to the introduction of panic_on_warn, this patch > introduces a kernel option named panic_on_taint in order to > provide a simple and generic way to stop execution and catch > a coredump when the kernel gets tainted by any given taint flag. > > This is useful for debugging sessions as it avoids rebuilding > the kernel to explicitly add calls to panic() or BUG() into > code sites that introduce the taint flags of interest. > Another, perhaps less frequent, use for this option would be > as a mean for assuring a security policy (in paranoid mode) > case where no single taint is allowed for the running system. > > Suggested-by: Qian Cai > Signed-off-by: Rafael Aquini > --- > Changelog: > * v2: get rid of unnecessary/misguided compiler hints (Luis) > * v2: enhance documentation text for the new kernel parameter (Randy) > * v3: drop sysctl interface, keep it only as a kernel parameter (Luis) > > Documentation/admin-guide/kdump/kdump.rst | 10 + > .../admin-guide/kernel-parameters.txt | 15 +++ > include/linux/kernel.h| 2 + > kernel/panic.c| 40 +++ > kernel/sysctl.c | 9 - > 5 files changed, 75 insertions(+), 1 deletion(-) > > diff --git a/Documentation/admin-guide/kdump/kdump.rst > b/Documentation/admin-guide/kdump/kdump.rst > index ac7e131d2935..de3cf6d377cc 100644 > --- a/Documentation/admin-guide/kdump/kdump.rst > +++ b/Documentation/admin-guide/kdump/kdump.rst > @@ -521,6 +521,16 @@ will cause a kdump to occur at the panic() call. In > cases where a user wants > to specify this during runtime, /proc/sys/kernel/panic_on_warn can be set to > 1 > to achieve the same behaviour. > > +Trigger Kdump on add_taint() > + > + > +The kernel parameter, panic_on_taint, calls panic() from within add_taint(), > +whenever the value set in this bitmask matches with the bit flag being set > +by add_taint(). This will cause a kdump to occur at the panic() call. > +In cases where a user wants to specify this during runtime, > +/proc/sys/kernel/panic_on_taint can be set to a respective bitmask value > +to achieve the same behaviour. > + > Contact > === > > diff --git a/Documentation/admin-guide/kernel-parameters.txt > b/Documentation/admin-guide/kernel-parameters.txt > index 7bc83f3d9bdf..4a69fe49a70d 100644 > --- a/Documentation/admin-guide/kernel-parameters.txt > +++ b/Documentation/admin-guide/kernel-parameters.txt > @@ -3404,6 +3404,21 @@ > panic_on_warn panic() instead of WARN(). Useful to cause kdump > on a WARN(). > > + panic_on_taint= [KNL] conditionally panic() in add_taint() > + Format: Changed it as 'Format: ' to be consistent with the existing other options? > + Specifies, as a string, the TAINT flag set that will > + compose a bitmask for calling panic() when the kernel > + gets tainted. > + See Documentation/admin-guide/tainted-kernels.rst for > + details on the taint flags that users can pick to > + compose the bitmask to assign to panic_on_taint. > + When the string is prefixed with a '-' the bitmask > + set in panic_on_taint will be mutually exclusive > + with the sysctl knob kernel.tainted, and any attempt > + to write to that sysctl will fail with -EINVAL for > + any taint value that masks with the flags set for > + this option. > + > crash_kexec_post_notifiers > Run kdump after running panic-notifiers and dumping > kmsg. This only for the users who doubt kdump always > diff --git a/include/linux/kernel.h b/include/linux/kernel.h > index 9b7a8d74a9d6..66bc102cb59a 100644 > --- a/include/linux/kernel.h > +++ b/include/linux/kernel.h > @@ -528,6 +528,8 @@ extern int panic_on_oops; > extern int panic_on_unrecovered_nmi; > extern int panic_on_io_nmi; > extern int panic_on_warn; > +extern unsigned long panic_on_taint; > +extern bool panic_on_taint_exclusive; > extern int sysctl_panic_on_rcu_stall; > extern int sysctl_panic_on_stackoverflow; > > diff --git a/kernel/panic.c b/kernel/panic.c > index b69ee9e76cb2..65c62f8a1de8 100644 > --- a/kernel/panic.c > +++ b/kernel/panic.c > @@ -25,6 +25,7 @@ > #include > #include > #include > +#include > #include > #include > #include > @@ -44,6 +45,8 @@ static int pause_on_oops_flag; > static DEFINE_SPINLOCK(pause_on_oops_lock); > bool crash_kexec_post_notifiers; > int panic_on_warn __read_mostly; > +unsigned long panic_on_taint; > +bool panic_on_taint_exclusive = false; > > int panic_timeout = CONFIG_PANIC_TIMEOUT; > EXPORT_SYMBOL_GPL(panic_
Re: [PATCH 05/20] tomoyo_write_control(): get rid of pointless access_ok()
On Sat, May 09, 2020 at 05:57:56PM -0700, Linus Torvalds wrote: > On Sat, May 9, 2020 at 5:51 PM Tetsuo Handa > wrote: > > > > I think that this access_ok() check helps reducing partial writes (either > > "whole amount was processed" or "not processed at all" unless -ENOMEM). > > No it doesn't. > > "access_ok()" only checks the range being a valid user address range. > > It doesn't actually help at all if the worry is "what if we take a > page fault in the middle". Because it simply doesn't check those > kinds of things. > > Now, if somebody passes actual invalid ranges (ie kernel addresses or > other crazy stuff), they only have themselves to blame. The invalid > range will be noticed when actually doing the user copy, and then > you'll get EFAULT there. But there's no point in trying to figure that > out early - it's only adding overhead, and it doesn't help any normal > case. It might be a good idea to add Documentation/what-access_ok-does_not ;-/ In addition to what you've mentioned, * access_ok() does not fault anything in; never had. * access_ok() does not verify that memory is readable/writable/there at all; never had, except for genuine 80386 and (maybe) some of the shittier 486 clones. * access_ok() does not protect you from the length being insanely large; even on i386 it can pass with length being a bit under 3Gb. If you count upon it to prevent kmalloc() complaints about insanely large allocation (yes, I've seen that excuse used), you are wrong. * on a bunch of architectures access_ok() never rejects anything, and no, that's _not_ MMU-less ones. sparc64, for example. Or s390, or parisc, etc.
Re: [PATCH net-next 3/4] net: phy: broadcom: add cable test support
Hi Michael, Thank you for the patch! Yet something to improve: [auto build test ERROR on net-next/master] [also build test ERROR on next-20200508] [cannot apply to net/master linus/master ipvs/master v5.7-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Michael-Walle/net-phy-broadcom-cable-tester-support/20200510-063955 base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git 2c674bec76d35b75c7c730f863424387c9e9633a config: i386-allyesconfig (attached as .config) compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0 reproduce: # save the attached .config to linux build tree make ARCH=i386 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot All errors (new ones prefixed by >>): drivers/net/phy/bcm-phy-lib.c: In function 'bcm_phy_cable_test_report_trans': >> drivers/net/phy/bcm-phy-lib.c:639:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_OK' undeclared (first use in this function); >> did you mean 'ETHTOOL_A_COALESCE_PKT_RATE_LOW'? return ETHTOOL_A_CABLE_RESULT_CODE_OK; ^~ ETHTOOL_A_COALESCE_PKT_RATE_LOW drivers/net/phy/bcm-phy-lib.c:639:10: note: each undeclared identifier is reported only once for each function it appears in >> drivers/net/phy/bcm-phy-lib.c:641:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_OPEN' undeclared (first use in this function); >> did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_OK'? return ETHTOOL_A_CABLE_RESULT_CODE_OPEN; ^~~~ ETHTOOL_A_CABLE_RESULT_CODE_OK >> drivers/net/phy/bcm-phy-lib.c:643:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT' undeclared (first use in this >> function); did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_OPEN'? return ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT; ^~ ETHTOOL_A_CABLE_RESULT_CODE_OPEN >> drivers/net/phy/bcm-phy-lib.c:645:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT' undeclared (first use in this >> function); did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT'? return ETHTOOL_A_CABLE_RESULT_CODE_CROSS_SHORT; ^~~ ETHTOOL_A_CABLE_RESULT_CODE_SAME_SHORT >> drivers/net/phy/bcm-phy-lib.c:649:10: error: >> 'ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC' undeclared (first use in this >> function); did you mean 'ETHTOOL_A_CABLE_RESULT_CODE_OPEN'? return ETHTOOL_A_CABLE_RESULT_CODE_UNSPEC; ^~ ETHTOOL_A_CABLE_RESULT_CODE_OPEN drivers/net/phy/bcm-phy-lib.c: In function 'bcm_phy_report_length': >> drivers/net/phy/bcm-phy-lib.c:681:2: error: implicit declaration of function >> 'ethnl_cable_test_fault_length' [-Werror=implicit-function-declaration] ethnl_cable_test_fault_length(phydev, pair, val); ^ drivers/net/phy/bcm-phy-lib.c: In function '_bcm_phy_cable_test_get_status': >> drivers/net/phy/bcm-phy-lib.c:719:2: error: implicit declaration of function >> 'ethnl_cable_test_result'; did you mean 'bcm_phy_cable_test_start'? >> [-Werror=implicit-function-declaration] ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A, ^~~ bcm_phy_cable_test_start >> drivers/net/phy/bcm-phy-lib.c:719:34: error: 'ETHTOOL_A_CABLE_PAIR_A' >> undeclared (first use in this function); did you mean 'ETHTOOL_A_PAUSE_MAX'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_A, ^~ ETHTOOL_A_PAUSE_MAX >> drivers/net/phy/bcm-phy-lib.c:721:34: error: 'ETHTOOL_A_CABLE_PAIR_B' >> undeclared (first use in this function); did you mean >> 'ETHTOOL_A_CABLE_PAIR_A'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_B, ^~ ETHTOOL_A_CABLE_PAIR_A >> drivers/net/phy/bcm-phy-lib.c:723:34: error: 'ETHTOOL_A_CABLE_PAIR_C' >> undeclared (first use in this function); did you mean >> 'ETHTOOL_A_CABLE_PAIR_B'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_C, ^~ ETHTOOL_A_CABLE_PAIR_B >> drivers/net/phy/bcm-phy-lib.c:725:34: error: 'ETHTOOL_A_CABLE_PAIR_D' >> undeclared (first use in this function); did you mean >> 'ETHTOOL_A_CABLE_PAIR_C'? ethnl_cable_test_result(phydev, ETHTOOL_A_CABLE_PAIR_D, ^~ ETHTOOL_A_CABLE_PAIR_C cc1: some warnings being treated as errors vim +639 drivers/net
Re: [PATCHES] uaccess simple access_ok() removals
On Sat, May 09, 2020 at 05:34:58PM -0700, Linus Torvalds wrote: > On Sat, May 9, 2020 at 4:41 PM Al Viro wrote: > > > > Individual patches in followups; if nobody screams - into #for-next > > it goes... > > Looks fine to me, although I only read your commit logs, I didn't > verify that what you stated was actually true (ie the whole "only used > for xyz" parts). > > But I'll take your word for it. Particularly the double-underscore > versions are getting rare (and presumably some of the other branches > you have make it rarer still). I have - FWIW, right now I'm going through the patch series for netdev; once I'm done with turning commit messages into something printable (and finish rereading the patches themselves), there it goes, hopefully tonight. Then a bunch of small branches + repost of csum one + some of the weird shit (comedi compat ioctls - the largest pile of __get_user() and __put_user() outside of arch/* is festering there). Then regset stuff + (probably) resurrection of i915 stuff. I've got a bunch of stuff around execve(), but that'll have to wait until I get through the recently posted execve-related patchsets... Then there's e.g. mempolicy compat stuff, etc., but I'd rather wait for several days before posting that, reviewers' bandwidth being finite...
[PATCH] ALSA: hda/realtek: Add quirk for Samsung Notebook
Some models of the Samsung Notebook 9 have very quiet and distorted
headphone output. This quirk changes the VREF value of the ALC298
codec NID 0x1a from default HIZ to new 100.
Signed-off-by: Mike Pozulp
---
sound/pci/hda/patch_realtek.c | 10 ++
1 file changed, 10 insertions(+)
diff --git a/sound/pci/hda/patch_realtek.c b/sound/pci/hda/patch_realtek.c
index 63e1a56f705b..9b14ed5c75d7 100644
--- a/sound/pci/hda/patch_realtek.c
+++ b/sound/pci/hda/patch_realtek.c
@@ -5923,6 +5923,7 @@ enum {
ALC294_FIXUP_ASUS_DUAL_SPK,
ALC285_FIXUP_THINKPAD_HEADSET_JACK,
ALC294_FIXUP_ASUS_HPE,
+ ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET,
};
static const struct hda_fixup alc269_fixups[] = {
@@ -7061,6 +7062,13 @@ static const struct hda_fixup alc269_fixups[] = {
.chained = true,
.chain_id = ALC294_FIXUP_ASUS_HEADSET_MIC
},
+ [ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET] = {
+ .type = HDA_FIXUP_VERBS,
+ .v.verbs = (const struct hda_verb[]) {
+ { 0x1a, AC_VERB_SET_PIN_WIDGET_CONTROL, 0xc5 },
+ { }
+ },
+ },
};
static const struct snd_pci_quirk alc269_fixup_tbl[] = {
@@ -7336,6 +7344,8 @@ static const struct snd_pci_quirk alc269_fixup_tbl[] = {
SND_PCI_QUIRK(0x1b7d, 0xa831, "Ordissimo EVE2 ",
ALC269VB_FIXUP_ORDISSIMO_EVE2), /* Also known as Malata PC-B1303 */
SND_PCI_QUIRK(0x1d72, 0x1901, "RedmiBook 14",
ALC256_FIXUP_ASUS_HEADSET_MIC),
SND_PCI_QUIRK(0x10ec, 0x118c, "Medion EE4254 MD62100",
ALC256_FIXUP_MEDION_HEADSET_NO_PRESENCE),
+ SND_PCI_QUIRK(0x144d, 0xc169, "Samsung Notebook 9 Pen
(NP930SBE-K01US)", ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET),
+ SND_PCI_QUIRK(0x144d, 0xc176, "Samsung Notebook 9 Pro
(NP930MBE-K04US)", ALC298_FIXUP_SAMSUNG_HEADPHONE_VERY_QUIET),
#if 0
/* Below is a quirk table taken from the old code.
--
2.24.1
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Thu, 2020-05-07 at 13:59 -0500, Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array
> member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> By making use of the mechanism above, we will get a compiler warning
> in case the flexible array does not occur last in the structure,
> which
> will help us prevent some kind of undefined behavior bugs from being
> inadvertently introduced[3] to the codebase from now on.
>
> Also, notice that, dynamic memory allocations won't be affected by
> this change:
>
> "Flexible array members have incomplete type, and so the sizeof
> operator
> may not be applied. As a quirk of the original implementation of
> zero-length arrays, sizeof evaluates to zero."[1]
>
> sizeof(flexible-array-member) triggers a warning because flexible
> array
> members have incomplete type[1]. There are some instances of code in
> which the sizeof operator is being incorrectly/erroneously applied to
> zero-length arrays and the result is zero. Such instances may be
> hiding
hmmm, we actually have some tooling that rely on this to identify such
0 length fields .. since the structs in this file are usually auto-
generated from the hw sepcs .. now i see that these tools are broken in
our CI with this patch applied.
I guess we will need to fix them, and fix our code auto-generation
tools.
overall i am ok with this patch. I will apply it to mlx5-next.
and submit it upstream soom.
> some bugs. So, this work (flexible-array member conversions) will
> also
> help to get completely rid of those sorts of issues.
>
> This issue was found with the help of Coccinelle.
>
> [1] https://gcc.gnu.org/onlinedocs/gcc/Zero-Length.html
> [2] https://github.com/KSPP/linux/issues/21
> [3] commit 76497732932f ("cxgb3/l2t: Fix undefined behaviour")
>
> Signed-off-by: Gustavo A. R. Silva
> ---
> include/linux/mlx5/driver.h |2 -
> include/linux/mlx5/mlx5_ifc.h | 66 +
> -
> include/linux/mlx5/qp.h |2 -
> 3 files changed, 35 insertions(+), 35 deletions(-)
>
> diff --git a/include/linux/mlx5/driver.h
> b/include/linux/mlx5/driver.h
> index 6f8f79ef829b..1a4ba36275de 100644
> --- a/include/linux/mlx5/driver.h
> +++ b/include/linux/mlx5/driver.h
> @@ -200,7 +200,7 @@ struct mlx5_rsc_debug {
> void *object;
> enum dbg_rsc_type type;
> struct dentry *root;
> - struct mlx5_field_desc fields[0];
> + struct mlx5_field_desc fields[];
> };
>
> enum mlx5_dev_event {
> diff --git a/include/linux/mlx5/mlx5_ifc.h
> b/include/linux/mlx5/mlx5_ifc.h
> index 69b27c7dfc3e..c55686ff6504 100644
> --- a/include/linux/mlx5/mlx5_ifc.h
> +++ b/include/linux/mlx5/mlx5_ifc.h
> @@ -1677,7 +1677,7 @@ struct mlx5_ifc_wq_bits {
>
> u8 reserved_at_140[0x4c0];
>
> - struct mlx5_ifc_cmd_pas_bits pas[0];
> + struct mlx5_ifc_cmd_pas_bits pas[];
> };
>
> struct mlx5_ifc_rq_num_bits {
> @@ -1895,7 +1895,7 @@ struct mlx5_ifc_resource_dump_menu_segment_bits
> {
> u8 reserved_at_20[0x10];
> u8 num_of_records[0x10];
>
> - struct mlx5_ifc_resource_dump_menu_record_bits record[0];
> + struct mlx5_ifc_resource_dump_menu_record_bits record[];
> };
>
> struct mlx5_ifc_resource_dump_resource_segment_bits {
> @@ -1907,7 +1907,7 @@ struct
> mlx5_ifc_resource_dump_resource_segment_bits {
>
> u8 index2[0x20];
>
> - u8 payload[0][0x20];
> + u8 payload[][0x20];
> };
>
> struct mlx5_ifc_resource_dump_terminate_segment_bits {
> @@ -2984,7 +2984,7 @@ struct mlx5_ifc_flow_context_bits {
>
> u8 reserved_at_1200[0x600];
>
> - union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits
> destination[0];
> + union mlx5_ifc_dest_format_struct_flow_counter_list_auto_bits
> destination[];
> };
>
> enum {
> @@ -3276,7 +3276,7 @@ struct mlx5_ifc_rqtc_bits {
>
> u8 reserved_at_e0[0x6a0];
>
> - struct mlx5_ifc_rq_num_bits rq_num[0];
> + struct mlx5_ifc_rq_num_bits rq_num[];
> };
>
> enum {
> @@ -3388,7 +3388,7 @@ struct mlx5_ifc_nic_vport_context_bits {
>
> u8 reserved_at_7e0[0x20];
>
> - u8 current_uc_mac_address[0][0x40];
> + u8 current_uc_mac_address[][0x40];
> };
>
> enum {
> @@ -4310,7 +4310,7 @@ struct mlx5_ifc_query_xrc_srq_out_bits {
>
> u8 reserved_at_280[0x600];
>
> - u8 pas[0][0x40];
> + u8 pas[][0x40];
> };
>
> struct mlx5_ifc_query_xrc_srq_in_bits {
> @@ -4588,7 +4588,7 @@ struct mlx5_ifc_query_srq_out_bits {
>
> u8 reserved_at_280[0x600];
>
> - u8 pas[0][0x40];
> + u8 pas[][0x40];
> };
>
> struct mlx
Re: [PATCH] IB/mlx4: Replace zero-length array with flexible-array
On Thu, 7 May 2020 13:59:21 -0500 Gustavo A. R. Silva wrote:
> The current codebase makes use of the zero-length array language
> extension to the C90 standard, but the preferred mechanism to declare
> variable-length types such as these ones is a flexible array member[1][2],
> introduced in C99:
>
> struct foo {
> int stuff;
> struct boo array[];
> };
>
> ...
Applied, thank you!
Re: [PATCH] net/mlx5: Replace zero-length array with flexible-array
On Sat, 9 May 2020 23:43:08 + Saeed Mahameed wrote: > > Saeed, I'm expecting you to take this and the mlx4 patch via your > > trees. > > Yes for the mlx5 patch, but usually Dave takes mlx4 patches directly. Ack, it said IB on it, but looks like the patch can as well be applied to net-next, so I took it.
Re: [PATCH v2] netprio_cgroup: Fix unlimited memory leak of v2 cgroups
On Fri, 8 May 2020 22:58:29 -0700 Jakub Kicinski wrote:
> On Sat, 9 May 2020 11:32:10 +0800 Zefan Li wrote:
> > If systemd is configured to use hybrid mode which enables the use of
> > both cgroup v1 and v2, systemd will create new cgroup on both the default
> > root (v2) and netprio_cgroup hierarchy (v1) for a new session and attach
> > task to the two cgroups. If the task does some network thing then the v2
> > cgroup can never be freed after the session exited.
> >
> > One of our machines ran into OOM due to this memory leak.
> >
> > In the scenario described above when sk_alloc() is called cgroup_sk_alloc()
> > thought it's in v2 mode, so it stores the cgroup pointer in sk->sk_cgrp_data
> > and increments the cgroup refcnt, but then sock_update_netprioidx() thought
> > it's in v1 mode, so it stores netprioidx value in sk->sk_cgrp_data, so the
> > cgroup refcnt will never be freed.
> >
> > Currently we do the mode switch when someone writes to the ifpriomap cgroup
> > control file. The easiest fix is to also do the switch when a task is
> > attached
> > to a new cgroup.
> >
> > Fixes: bd1060a1d671("sock, cgroup: add sock->sk_cgroup")
>
> ^ space missing here
>
> > Reported-by: Yang Yingliang
> > Tested-by: Yang Yingliang
> > Signed-off-by: Zefan Li
Fixed up the commit message and applied, thank you.
Re: [PATCH v2 5/5] drm/panel: add panel driver for Ilitek ili9341 panels
Sam Ravnborg 于2020年5月10日周日 上午4:06写道：
>
> Hi Dillon.
>
> On Fri, May 08, 2020 at 06:13:43PM +0800, dillon min wrote:
> > Hi Sam,
> >
> > Thanks for your comments, i will rework this panel driver after l3gd20
> > patch submission.
> >
> > Sam Ravnborg 于2020年5月8日周五 下午5:02写道：
> >
> > > Hi Dillon.
> > >
> > > Patch submissions starts to look fine.
> > >
> > > On Fri, May 08, 2020 at 12:13:14PM +0800, dillon.min...@gmail.com wrote:
> > > > From: dillon min
> > > >
> > > > This is a driver for 320x240 TFT panels, accepting a rgb input
> > > > streams that get adapted and scaled to the panel.
> > > This driver is, I suppose, prepared to be a driver for ILI9341 based
> > > panles, and as such not for a fixed resolution.
> > > I expect (hope) we in the future will see more panels added.
> > >
> > > As i checked ili9341 datasheets, this panel just support 240x320
> > resolution only. if i'm wrong , please correct me. thanks
> > https://cdn-shop.adafruit.com/datasheets/ILI9341.pdf
> >
> > This panel can support 9 different kinds of interface , "3/4-line Serial
> > Interface" have been supported by tiny/ili9341.c. i was verified it
> > but the performance on stm32f4 is very low.
> It had somehow escaped my mind we already have a driver for ili9341
> based panels.
> And we do not want onyl one driver for this IC.
> So please extent the current driver to match your panel as well as
> the original panel.
> And then also extend the current binding for ili9341, which
> you may have to convert to DT Schema in the process.
>
> This is more work now but when it is done we end up with a much better
> solution than if we introduce an additional driver for ili9341.
>
> Sorry for not catching this in the inital review.
>
> Sam
>
>
Hi Sam,
Yes, it's should be more reasonable to support different interface of
a panel in one driver.
i will add mipi dpi & dbi support to this panel in one driver just
like panel-simple.c(dpi & dsi)
one question, what to do next:
1 should i just add code to tiny/ili9341.c to support mipi dpi.
which means remove panel/panel-ilitek-ili9341.c
2 merge tiny/ili9341.c mipi dbi support to panel/panel-ilitek-ili9341.c ,
which means remove tiny/ili9341.c after this work done.
to extend dts binding for different interface. Ie.
original:
164 static const struct of_device_id ili9341_of_match[] = {
165 { .compatible = "adafruit,yx240qv29" },
166 { }
167 };
168 MODULE_DEVICE_TABLE(of, ili9341_of_match);
169
170 static const struct spi_device_id ili9341_id[] = {
171 { "yx240qv29", 0 },
172 { }
173 };
174 MODULE_DEVICE_TABLE(spi, ili9341_id);
i want to extend of_device_id and spi_device_id for driver to identify
dpi or dbi in probe.
"adafruit,yx240qv29" is for users who use original tiny drm driver
mipi dbi compatible
for panel's parallel mcu and parallel rgb interface.
static const struct of_device_id ili9341_of_match[] = {
{ .compatible = "adafruit,yx240qv29" },
{ .compatible = "ili9341,parallel-mcu" },
{ .compatible = "ili9341,parallel-rgb" },
{ }
};
static const struct spi_device_id ili9341_id[] = {
{ "yx240qv29", 0 },
{ "parallel-mcu", 0 },
{ "parallel-rgb", 0 },
{ }
};
static int ili9341_probe(struct spi_device *spi)
{
...
const struct spi_device_id *id = spi_get_device_id(spi);
if (!strcmp(id->name, "yx240qv29")) {
mipi dbi register
} else if (!strcmp(id->name, "parallel-rgb") ||
!strcmp(id->name, "parallel-mcu") {
mipi dpi register
} else {
failed handle
}
}
if this is a good idea to extend dts binding ?
thanks,
best regards.
Dillon
> >
> > currently, i just have stm32f429-disco in hands, with 18-bit parallel rgb
> > bus connected on this board. reference to panel-ilitek-ili9322 and
> > panel-ilitek-ili9881 driver, i have some plan to rewrite this driver.
> >
> > 1 add your below comments in.
> > 2 use dc-gpio, reset-gpio, rgb-interface-bits from dts to config panel
> > interface.
> > 3 drop regmap, porting drm_mipi_dbi's mipi_dbi_command to config panel
> > paramter. like tiny/ili9341.c
> > 4 support rgb-16, rgb-18 interface.
> > 5 use optional regulator or power gpio to control panel's power, as panel
> > power is always on for my board, so i can't test this part. could i add the
> > code which can't be tested?
> > 6 support rotation in panel config (currently , i rotate the screen by
> > kernel cmdline paramter)
> >
> > if you have any other common panel configuration should be add , please
> > inform me.
> >
> > thanks.
> >
> > >
> > > Some things to fix, see comments in the follwoing.
> > >
> > > Sam
> > >
> > > >
> > > > Signed-off-by: dillon min
> > > > ---
> > > > drivers/gpu/drm/panel/Kconfig| 8 +
> > > > drivers/gpu/drm/panel/Makefile | 1 +
> > > > drivers/gpu/drm/panel/panel-ilitek-ili9341.c | 561
> > > +++
> >
Re: [PATCH v3] kernel: add panic_on_taint
On 5/9/20 7:59 PM, Baoquan He wrote: > Read admin-guide/tainted-kernels.rst, but still do not get what 'G' means. I interpret 'G' as GPL (strictly it means that no proprietary module has been loaded). But I don't see why TAINT_PROPRIETARY_MODULE is the only taint flag that has a non-blank c_false character. It could just be blank also AFAICT. Then the 'G' would not be there to confuse us. :) -- ~Randy
Re: [PATCH v2 2/2] serial: lantiq: Make driver modular
Hi Rahul, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on tty/tty-testing] [also build test WARNING on next-20200508] [cannot apply to usb/usb-testing linus/master linux/master v5.7-rc4] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system. BTW, we also suggest to use '--base' option to specify the base tree in git format-patch, please see https://stackoverflow.com/a/37406982] url: https://github.com/0day-ci/linux/commits/Rahul-Tanwar/serial-lantiq-Make-UART-s-use-as-console-selectable/20200509-174007 base: https://git.kernel.org/pub/scm/linux/kernel/git/gregkh/tty.git tty-testing config: h8300-allyesconfig (attached as .config) compiler: h8300-linux-gcc (GCC) 9.3.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross ARCH=h8300 If you fix the issue, kindly add following tag as appropriate Reported-by: kbuild test robot All warnings (new ones prefixed by >>): >> WARNING: modpost: vmlinux.o(.data+0x172c14): Section mismatch in reference >> from the variable lqasc_driver to the variable .init.text:lqasc_probe The variable lqasc_driver references the variable __init lqasc_probe If the reference is valid then annotate the variable with or __refdata (see linux/init.h) or name the variable: --- 0-DAY CI Kernel Test Service, Intel Corporation https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org .config.gz Description: application/gzip
Re: [PATCH 3/5] exec: Remove recursion from search_binary_handler
On 2020/05/10 4:41, Eric W. Biederman wrote: > --- a/fs/binfmt_misc.c > +++ b/fs/binfmt_misc.c > @@ -234,10 +234,7 @@ static int load_misc_binary(struct linux_binprm *bprm) > if (retval < 0) > goto error; > > - retval = search_binary_handler(bprm); > - if (retval < 0) > - goto error; > - > + retval = 1; /* Search for the interpreter */ > ret: > dput(fmt->dentry); > return retval; Wouldn't this change cause if (fd_binary > 0) ksys_close(fd_binary); bprm->interp_flags = 0; bprm->interp_data = 0; not to be called when "Search for the interpreter" failed?
Re: [patch V4 part 5 02/31] x86/entry: Provide helpers for execute on irqstack
On Tue, May 5, 2020 at 10:19 PM Thomas Gleixner wrote: > > Device interrupt handlers and system vector handlers are executed on the > interrupt stack. The stack switch happens in the low level assembly entry > code. This conflicts with the efforts to consolidate the exit code in C to > ensure correctness vs. RCU and tracing. > > As there is no way to move #DB away from IST due to the MOV SS issue, the > requirements vs. #DB and NMI for switching to the interrupt stack do not > exist anymore. The only requirement is that interrupts are disabled. Hi, tglx and Andy Lutomirski, Is there any information about "no way to move #DB away from IST due to the MOV SS issue"? IST-based #DB results to ist_shift(for nested #DB) and debug_idt(for #NMI vs. #DB) which are somewhat ugly. If IST-less #DB should work, debug stack should be switched in software manner like interrupt stack. There was a "POP/MOV SS" CVE/issue about #BP which lead to moving #BP to IST-less by d8ba61ba58c8 (x86/entry/64: Don't use IST entry for #BP stack) #DB #BP are considered as #NMI due to their super-interrupt ability. But the kernel has much more control over #DB and #BP which can be disabled by putting the code snip into non-instrument sections like __entry noinstr etc. Is it possible to implement IST-less #DB? Thanks, Lai
Re: [RFC PATCH 0/7] mm: Get rid of vmalloc_sync_(un)mappings()
On Sat, May 9, 2020 at 2:57 PM Joerg Roedel wrote: > > Hi Andy, > > On Sat, May 09, 2020 at 12:05:29PM -0700, Andy Lutomirski wrote: > > So, unless I'm missing something here, there is an absolute maximum of > > 512 top-level entries that ever need to be synchronized. > > And here is where your assumption is wrong. On 32-bit PAE systems it is > not the top-level entries that need to be synchronized for vmalloc, but > the second-level entries. And dependent on the kernel configuration, > there are (in total, not only vmalloc) 1536, 1024, or 512 of these > second-level entries. How much of them are actually used for vmalloc > depends on the size of the system RAM (but is at least 64), because > the vmalloc area begins after the kernel direct-mapping (with an 8MB > unmapped hole). I spent some time looking at the code, and I'm guessing you're talking about the 3-level !SHARED_KERNEL_PMD case. I can't quite figure out what's going on. Can you explain what is actually going on that causes different mms/pgds to have top-level entries in the kernel range that point to different tables? Because I'm not seeing why this makes any sense. > > > Now, there's an additional complication. On x86_64, we have a rule: > > those entries that need to be synced start out null and may, during > > the lifetime of the system, change *once*. They are never unmapped or > > modified after being allocated. This means that those entries can > > only ever point to a page *table* and not to a ginormous page. So, > > even if the hardware were to support ginormous pages (which, IIRC, it > > doesn't), we would be limited to merely immense and not ginormous > > pages in the vmalloc range. On x86_32, I don't think we have this > > rule right now. And this means that it's possible for one of these > > pages to be unmapped or modified. > > The reason for x86-32 being different is that the address space is > orders of magnitude smaller than on x86-64. We just have 4 top-level > entries with PAE paging and can't afford to partition kernel-adress > space on that level like we do on x86-64. That is the reason the address > space is partitioned on the second (PMD) level, which is also the reason > vmalloc synchronization needs to happen on that level. And because > that's not enough yet, its also the page-table level to map huge-pages. Why does it need to be partitioned at all? The only thing that comes to mind is that the LDT range is per-mm. So I can imagine that the PAE case with a 3G user / 1G kernel split has to have the vmalloc range and the LDT range in the same top-level entry. Yuck. > > > So my suggestion is that just apply the x86_64 rule to x86_32 as well. > > The practical effect will be that 2-level-paging systems will not be > > able to use huge pages in the vmalloc range, since the rule will be > > that the vmalloc-relevant entries in the top-level table must point to > > page *tables* instead of huge pages. > > I could very well live with prohibiting huge-page ioremap mappings for > x86-32. But as I wrote before, this doesn't solve the problems I am > trying to address with this patch-set, or would only address them if > significant amount of total system memory is used. > > The pre-allocation solution would work for x86-64, it would only need > 256kb of preallocated memory for the vmalloc range to never synchronize > or fault again. I have thought about that and did the math before > writing this patch-set, but doing the math for 32 bit drove me away from > it for reasons written above. > If it's *just* the LDT that's a problem, we could plausibly shrink the user address range a little bit and put the LDT in the user portion. I suppose this could end up creating its own set of problems involving tracking which code owns which page tables. > And since a lot of the vmalloc_sync_(un)mappings problems I debugged > were actually related to 32-bit, I want a solution that works for 32 and > 64-bit x86 (at least until support for x86-32 is removed). And I think > this patch-set provides a solution that works well for both. I'm not fundamentally objecting to your patch set, but I do want to understand what's going on that needs this stuff. > > > Joerg
[PATCH] powerpc/powernv/pci: fix a RCU-list lock
It is unsafe to traverse tbl->it_group_list without the RCU read lock.
WARNING: suspicious RCU usage
5.7.0-rc4-next-20200508 #1 Not tainted
-
arch/powerpc/platforms/powernv/pci-ioda-tce.c:355 RCU-list traversed in
non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
3 locks held by qemu-kvm/4305:
#0: c00bc3fe6988 (&container->group_lock){}-{3:3}, at:
vfio_fops_unl_ioctl+0x108/0x410 [vfio]
#1: c0080fcc7400 (&vfio.iommu_drivers_lock){+.+.}-{3:3}, at:
vfio_fops_unl_ioctl+0x148/0x410 [vfio]
#2: c00bc3fe4d68 (&container->lock){+.+.}-{3:3}, at:
tce_iommu_attach_group+0x3c/0x4f0 [vfio_iommu_spapr_tce]
stack backtrace:
CPU: 4 PID: 4305 Comm: qemu-kvm Not tainted 5.7.0-rc4-next-20200508 #1
Call Trace:
[c010f29afa60] [c07154c8] dump_stack+0xfc/0x174 (unreliable)
[c010f29afab0] [c01d8ff0] lockdep_rcu_suspicious+0x140/0x164
[c010f29afb30] [c00dae2c]
pnv_pci_unlink_table_and_group+0x11c/0x200
[c010f29afb70] [c00d4a34] pnv_pci_ioda2_unset_window+0xc4/0x190
[c010f29afbf0] [c00d4b4c] pnv_ioda2_take_ownership+0x4c/0xd0
[c010f29afc30] [c0080fd60ee0] tce_iommu_attach_group+0x2c8/0x4f0
[vfio_iommu_spapr_tce]
[c010f29afcd0] [c0080fcc11a0] vfio_fops_unl_ioctl+0x238/0x410 [vfio]
[c010f29afd50] [c05430a8] ksys_ioctl+0xd8/0x130
[c010f29afda0] [c0543128] sys_ioctl+0x28/0x40
[c010f29afdc0] [c0038af4] system_call_exception+0x114/0x1e0
[c010f29afe20] [c000c8f0] system_call_common+0xf0/0x278
Signed-off-by: Qian Cai
---
arch/powerpc/platforms/powernv/pci-ioda-tce.c | 4
1 file changed, 4 insertions(+)
diff --git a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
index 5dc6847d5f4c..6be9cf292b4e 100644
--- a/arch/powerpc/platforms/powernv/pci-ioda-tce.c
+++ b/arch/powerpc/platforms/powernv/pci-ioda-tce.c
@@ -352,6 +352,8 @@ void pnv_pci_unlink_table_and_group(struct iommu_table *tbl,
/* Remove link to a group from table's list of attached groups */
found = false;
+
+ rcu_read_lock();
list_for_each_entry_rcu(tgl, &tbl->it_group_list, next) {
if (tgl->table_group == table_group) {
list_del_rcu(&tgl->next);
@@ -360,6 +362,8 @@ void pnv_pci_unlink_table_and_group(struct iommu_table *tbl,
break;
}
}
+ rcu_read_unlock();
+
if (WARN_ON(!found))
return;
--
2.21.0 (Apple Git-122.2)
Re: [PATCH v9 3/8] x86/split_lock: Introduce flag X86_FEATURE_SLD_FATAL and drop sld_state
On Fri, May 8, 2020 at 8:03 PM Xiaoyao Li wrote: > > Introduce a synthetic feature flag X86_FEATURE_SLD_FATAL, which means > kernel is in sld_fatal mode if set. > > Now sld_state is not needed any more that the state of SLD can be > inferred from X86_FEATURE_SPLIT_LOCK_DETECT and X86_FEATURE_SLD_FATAL. Is it too much to ask for Intel to actually allocate and define a CPUID bit that means "this CPU *always* sends #AC on a split lock"? This would be a pure documentation change, but it would make this architectural rather than something that each hypervisor needs to hack up. Meanwhile, I don't see why adding a cpufeature flag is worthwhile to avoid a less bizarre global variable. There's no performance issue here, and the old code looked a lot more comprehensible than the new code.
[PATCH v1] dt-bindings: serial: qca,ar9330-uart: Convert to json-schema
Convert the Qualcomm Atheros AR9330 High-Speed UART
Device Tree binding documentation to json-schema.
Signed-off-by: Oleksij Rempel
---
.../bindings/serial/qca,ar9330-uart.txt | 31
.../bindings/serial/qca,ar9330-uart.yaml | 50 +++
2 files changed, 50 insertions(+), 31 deletions(-)
delete mode 100644 Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
create mode 100644
Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
deleted file mode 100644
index 7d65126bd1d77..0
--- a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.txt
+++ /dev/null
@@ -1,31 +0,0 @@
-* Qualcomm Atheros AR9330 High-Speed UART
-
-Required properties:
-
-- compatible: Must be "qca,ar9330-uart"
-
-- reg: Specifies the physical base address of the controller and
- the length of the memory mapped region.
-
-- interrupts: Specifies the interrupt source of the parent interrupt
- controller. The format of the interrupt specifier depends on the
- parent interrupt controller.
-
-Additional requirements:
-
- Each UART port must have an alias correctly numbered in "aliases"
- node.
-
-Example:
-
- aliases {
- serial0 = &uart0;
- };
-
- uart0: uart@1802 {
- compatible = "qca,ar9330-uart";
- reg = <0x1802 0x14>;
-
- interrupt-parent = <&intc>;
- interrupts = <3>;
- };
diff --git a/Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
new file mode 100644
index 0..a344369285b6a
--- /dev/null
+++ b/Documentation/devicetree/bindings/serial/qca,ar9330-uart.yaml
@@ -0,0 +1,50 @@
+# SPDX-License-Identifier: (GPL-2.0-only OR BSD-2-Clause)
+%YAML 1.2
+---
+$id: http://devicetree.org/schemas/serial/qca,ar9330-uart.yaml#
+$schema: http://devicetree.org/meta-schemas/core.yaml#
+
+title: Qualcomm Atheros AR9330 High-Speed UART
+
+maintainers:
+ - Oleksij Rempel
+
+allOf:
+ - $ref: /schemas/serial.yaml#
+
+properties:
+ compatible:
+const: qca,ar9330-uart
+
+ reg:
+maxItems: 1
+
+ interrupts:
+maxItems: 1
+
+ clocks:
+maxItems: 1
+
+ clock-names:
+const: uart
+
+required:
+ - compatible
+ - reg
+ - interrupts
+ - clocks
+ - clock-names
+
+additionalProperties: false
+
+examples:
+ - |
+serial@1802 {
+ compatible = "qca,ar9330-uart";
+ reg = <0x1802 0x14>;
+ clocks = <&ref>;
+ clock-names = "uart";
+ interrupt-parent = <&intc>;
+ interrupts = <3>;
+};
+...
--
2.26.2
Re: [PATCH v9 8/8] x86/split_lock: Enable split lock detection initialization when running as an guest on KVM
On Fri, May 8, 2020 at 8:04 PM Xiaoyao Li wrote:
>
> When running as guest, enumerating feature split lock detection through
> CPU model is not easy since CPU model is configurable by host VMM.
>
> If running upon KVM, it can be enumerated through
> KVM_FEATURE_SPLIT_LOCK_DETECT,
This needs crystal clear documentation. What, exactly, is the host
telling the guest if it sets this flag?
> and if KVM_HINTS_SLD_FATAL is set, it
> needs to be set to sld_fatal mode.
This needs much better docs. Do you mean:
"If KVM_HINTS_SLD_FATAL is set, then the guest will get #AC if it does
a split-lock regardless of what is written to MSR_TEST_CTRL?"
>
> Signed-off-by: Xiaoyao Li
> ---
> arch/x86/include/asm/cpu.h | 2 ++
> arch/x86/kernel/cpu/intel.c | 12 ++--
> arch/x86/kernel/kvm.c | 3 +++
> 3 files changed, 15 insertions(+), 2 deletions(-)
>
> diff --git a/arch/x86/include/asm/cpu.h b/arch/x86/include/asm/cpu.h
> index a57f00f1d5b5..5d5b488b4b45 100644
> --- a/arch/x86/include/asm/cpu.h
> +++ b/arch/x86/include/asm/cpu.h
> @@ -42,12 +42,14 @@ unsigned int x86_model(unsigned int sig);
> unsigned int x86_stepping(unsigned int sig);
> #ifdef CONFIG_CPU_SUP_INTEL
> extern void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c);
> +extern void __init split_lock_setup(bool fatal);
> extern void switch_to_sld(unsigned long tifn);
> extern bool handle_user_split_lock(struct pt_regs *regs, long error_code);
> extern bool handle_guest_split_lock(unsigned long ip);
> extern bool split_lock_virt_switch(bool on);
> #else
> static inline void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c) {}
> +static inline void __init split_lock_setup(bool fatal) {}
> static inline void switch_to_sld(unsigned long tifn) {}
> static inline bool handle_user_split_lock(struct pt_regs *regs, long
> error_code)
> {
> diff --git a/arch/x86/kernel/cpu/intel.c b/arch/x86/kernel/cpu/intel.c
> index 1e2a74e8c592..02e24134b9b5 100644
> --- a/arch/x86/kernel/cpu/intel.c
> +++ b/arch/x86/kernel/cpu/intel.c
> @@ -996,12 +996,18 @@ static bool split_lock_verify_msr(bool on)
> return ctrl == tmp;
> }
>
> -static void __init split_lock_setup(void)
> +void __init split_lock_setup(bool fatal)
> {
> enum split_lock_detect_state state = sld_warn;
> char arg[20];
> int i, ret;
>
> + if (fatal) {
> + state = sld_fatal;
> + pr_info("forced on, sending SIGBUS on user-space
> split_locks\n");
> + goto set_cap;
> + }
> +
> if (!split_lock_verify_msr(false)) {
> pr_info("MSR access failed: Disabled\n");
> return;
> @@ -1037,6 +1043,7 @@ static void __init split_lock_setup(void)
> return;
> }
>
> +set_cap:
> setup_force_cpu_cap(X86_FEATURE_SPLIT_LOCK_DETECT);
> if (state == sld_fatal)
> setup_force_cpu_cap(X86_FEATURE_SLD_FATAL);
> @@ -1161,6 +1168,7 @@ void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
> const struct x86_cpu_id *m;
> u64 ia32_core_caps;
>
> + /* Note, paravirt support can enable SLD, e.g., see kvm_guest_init().
> */
> if (boot_cpu_has(X86_FEATURE_HYPERVISOR))
> return;
>
> @@ -1182,5 +1190,5 @@ void __init cpu_set_core_cap_bits(struct cpuinfo_x86 *c)
> return;
> }
>
> - split_lock_setup();
> + split_lock_setup(false);
> }
> diff --git a/arch/x86/kernel/kvm.c b/arch/x86/kernel/kvm.c
> index 6efe0410fb72..489ea89e2e8e 100644
> --- a/arch/x86/kernel/kvm.c
> +++ b/arch/x86/kernel/kvm.c
> @@ -670,6 +670,9 @@ static void __init kvm_guest_init(void)
> * overcommitted.
> */
> hardlockup_detector_disable();
> +
> + if (kvm_para_has_feature(KVM_FEATURE_SPLIT_LOCK_DETECT))
> + split_lock_setup(kvm_para_has_hint(KVM_HINTS_SLD_FATAL));
> }
>
> static noinline uint32_t __kvm_cpuid_base(void)
> --
> 2.18.2
>
[PATCH] powerpc/mm/book3s64/iommu: fix some RCU-list locks
It is safe to traverse mm->context.iommu_group_mem_list with either
mem_list_mutex or the RCU read lock held. Silence a few RCU-list false
positive warnings and fix a few missing RCU read locks.
arch/powerpc/mm/book3s64/iommu_api.c:330 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4305:
#0: c00bc3fe4d68 (&container->lock){+.+.}-{3:3}, at:
tce_iommu_ioctl.part.9+0xc7c/0x1870 [vfio_iommu_spapr_tce]
#1: c1501910 (mem_list_mutex){+.+.}-{3:3}, at: mm_iommu_get+0x50/0x190
arch/powerpc/mm/book3s64/iommu_api.c:132 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4305:
#0: c00bc3fe4d68 (&container->lock){+.+.}-{3:3}, at:
tce_iommu_ioctl.part.9+0xc7c/0x1870 [vfio_iommu_spapr_tce]
#1: c1501910 (mem_list_mutex){+.+.}-{3:3}, at:
mm_iommu_do_alloc+0x120/0x5f0
arch/powerpc/mm/book3s64/iommu_api.c:292 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4312:
#0: c00ecafe23c8 (&vcpu->mutex){+.+.}-{3:3}, at:
kvm_vcpu_ioctl+0xdc/0x950 [kvm]
#1: c00045e6c468 (&kvm->srcu){}-{0:0}, at:
kvmppc_h_put_tce+0x88/0x340 [kvm]
arch/powerpc/mm/book3s64/iommu_api.c:424 RCU-list traversed in non-reader
section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4312:
#0: c00ecafe23c8 (&vcpu->mutex){+.+.}-{3:3}, at:
kvm_vcpu_ioctl+0xdc/0x950 [kvm]
#1: c00045e6c468 (&kvm->srcu){}-{0:0}, at:
kvmppc_h_put_tce+0x88/0x340 [kvm]
Signed-off-by: Qian Cai
---
arch/powerpc/mm/book3s64/iommu_api.c | 10 --
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/arch/powerpc/mm/book3s64/iommu_api.c
b/arch/powerpc/mm/book3s64/iommu_api.c
index fa05bbd1f682..bf0108b6f445 100644
--- a/arch/powerpc/mm/book3s64/iommu_api.c
+++ b/arch/powerpc/mm/book3s64/iommu_api.c
@@ -129,7 +129,8 @@ static long mm_iommu_do_alloc(struct mm_struct *mm,
unsigned long ua,
mutex_lock(&mem_list_mutex);
- list_for_each_entry_rcu(mem2, &mm->context.iommu_group_mem_list, next) {
+ list_for_each_entry_rcu(mem2, &mm->context.iommu_group_mem_list, next,
+ lockdep_is_held(&mem_list_mutex)) {
/* Overlap? */
if ((mem2->ua < (ua + (entries << PAGE_SHIFT))) &&
(ua < (mem2->ua +
@@ -289,6 +290,7 @@ struct mm_iommu_table_group_mem_t *mm_iommu_lookup(struct
mm_struct *mm,
{
struct mm_iommu_table_group_mem_t *mem, *ret = NULL;
+ rcu_read_lock();
list_for_each_entry_rcu(mem, &mm->context.iommu_group_mem_list, next) {
if ((mem->ua <= ua) &&
(ua + size <= mem->ua +
@@ -297,6 +299,7 @@ struct mm_iommu_table_group_mem_t *mm_iommu_lookup(struct
mm_struct *mm,
break;
}
}
+ rcu_read_unlock();
return ret;
}
@@ -327,7 +330,8 @@ struct mm_iommu_table_group_mem_t *mm_iommu_get(struct
mm_struct *mm,
mutex_lock(&mem_list_mutex);
- list_for_each_entry_rcu(mem, &mm->context.iommu_group_mem_list, next) {
+ list_for_each_entry_rcu(mem, &mm->context.iommu_group_mem_list, next,
+ lockdep_is_held(&mem_list_mutex)) {
if ((mem->ua == ua) && (mem->entries == entries)) {
ret = mem;
++mem->used;
@@ -421,6 +425,7 @@ bool mm_iommu_is_devmem(struct mm_struct *mm, unsigned long
hpa,
struct mm_iommu_table_group_mem_t *mem;
unsigned long end;
+ rcu_read_lock();
list_for_each_entry_rcu(mem, &mm->context.iommu_group_mem_list, next) {
if (mem->dev_hpa == MM_IOMMU_TABLE_INVALID_HPA)
continue;
@@ -437,6 +442,7 @@ bool mm_iommu_is_devmem(struct mm_struct *mm, unsigned long
hpa,
return true;
}
}
+ rcu_read_unlock();
return false;
}
--
2.21.0 (Apple Git-122.2)
Re: [PATCH v3] kernel: add panic_on_taint
On 05/09/20 at 09:10pm, Randy Dunlap wrote: > On 5/9/20 7:59 PM, Baoquan He wrote: > > Read admin-guide/tainted-kernels.rst, but still do not get what 'G' means. > > I interpret 'G' as GPL (strictly it means that no proprietary module has > been loaded). But I don't see why TAINT_PROPRIETARY_MODULE is the only > taint flag that has a non-blank c_false character. It could just be blank > also AFAICT. Then the 'G' would not be there to confuse us. :) Yeah, seems c_false character is not so necessary. If no 'P' set, then it means no proprietary modules loaded naturally. We may need clean up the c_false in struct taint_flag, since c_true is enough to indicate what want to check.
[PATCH] powerpc/kvm/book3s64/vio: fix some RCU-list locks
It is unsafe to traverse kvm->arch.spapr_tce_tables and
stt->iommu_tables without the RCU read lock held. Also, add
cond_resched_rcu() in places with the RCU read lock held that could take
a while to finish.
arch/powerpc/kvm/book3s_64_vio.c:76 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
no locks held by qemu-kvm/4265.
stack backtrace:
CPU: 96 PID: 4265 Comm: qemu-kvm Not tainted 5.7.0-rc4-next-20200508+ #2
Call Trace:
[c000201a8690f720] [c0715948] dump_stack+0xfc/0x174 (unreliable)
[c000201a8690f770] [c01d9470] lockdep_rcu_suspicious+0x140/0x164
[c000201a8690f7f0] [c00810b9fb48]
kvm_spapr_tce_release_iommu_group+0x1f0/0x220 [kvm]
[c000201a8690f870] [c00810b8462c]
kvm_spapr_tce_release_vfio_group+0x54/0xb0 [kvm]
[c000201a8690f8a0] [c00810b84710] kvm_vfio_destroy+0x88/0x140 [kvm]
[c000201a8690f8f0] [c00810b7d488] kvm_put_kvm+0x370/0x600 [kvm]
[c000201a8690f990] [c00810b7e3c0] kvm_vm_release+0x38/0x60 [kvm]
[c000201a8690f9c0] [c05223f4] __fput+0x124/0x330
[c000201a8690fa20] [c0151cd8] task_work_run+0xb8/0x130
[c000201a8690fa70] [c01197e8] do_exit+0x4e8/0xfa0
[c000201a8690fb70] [c011a374] do_group_exit+0x64/0xd0
[c000201a8690fbb0] [c0132c90] get_signal+0x1f0/0x1200
[c000201a8690fcc0] [c0020690] do_notify_resume+0x130/0x3c0
[c000201a8690fda0] [c0038d64] syscall_exit_prepare+0x1a4/0x280
[c000201a8690fe20] [c000c8f8] system_call_common+0xf8/0x278
arch/powerpc/kvm/book3s_64_vio.c:368 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
2 locks held by qemu-kvm/4264:
#0: c000201ae2d000d8 (&vcpu->mutex){+.+.}-{3:3}, at:
kvm_vcpu_ioctl+0xdc/0x950 [kvm]
#1: c000200c9ed0c468 (&kvm->srcu){}-{0:0}, at:
kvmppc_h_put_tce+0x88/0x340 [kvm]
arch/powerpc/kvm/book3s_64_vio.c:108 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by qemu-kvm/4257:
#0: c000200b1b363a40 (&kv->lock){+.+.}-{3:3}, at:
kvm_vfio_set_attr+0x598/0x6c0 [kvm]
arch/powerpc/kvm/book3s_64_vio.c:146 RCU-list traversed in non-reader section!!
other info that might help us debug this:
rcu_scheduler_active = 2, debug_locks = 1
1 lock held by qemu-kvm/4257:
#0: c000200b1b363a40 (&kv->lock){+.+.}-{3:3}, at:
kvm_vfio_set_attr+0x598/0x6c0 [kvm]
Signed-off-by: Qian Cai
---
arch/powerpc/kvm/book3s_64_vio.c | 19 +++
1 file changed, 15 insertions(+), 4 deletions(-)
diff --git a/arch/powerpc/kvm/book3s_64_vio.c b/arch/powerpc/kvm/book3s_64_vio.c
index 50555ad1db93..4f5016bab723 100644
--- a/arch/powerpc/kvm/book3s_64_vio.c
+++ b/arch/powerpc/kvm/book3s_64_vio.c
@@ -73,6 +73,7 @@ extern void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
struct kvmppc_spapr_tce_iommu_table *stit, *tmp;
struct iommu_table_group *table_group = NULL;
+ rcu_read_lock();
list_for_each_entry_rcu(stt, &kvm->arch.spapr_tce_tables, list) {
table_group = iommu_group_get_iommudata(grp);
@@ -87,7 +88,9 @@ extern void kvm_spapr_tce_release_iommu_group(struct kvm *kvm,
kref_put(&stit->kref, kvm_spapr_tce_liobn_put);
}
}
+ cond_resched_rcu();
}
+ rcu_read_unlock();
}
extern long kvm_spapr_tce_attach_iommu_group(struct kvm *kvm, int tablefd,
@@ -105,12 +108,14 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm
*kvm, int tablefd,
if (!f.file)
return -EBADF;
+ rcu_read_lock();
list_for_each_entry_rcu(stt, &kvm->arch.spapr_tce_tables, list) {
if (stt == f.file->private_data) {
found = true;
break;
}
}
+ rcu_read_unlock();
fdput(f);
@@ -143,6 +148,7 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm
*kvm, int tablefd,
if (!tbl)
return -EINVAL;
+ rcu_read_lock();
list_for_each_entry_rcu(stit, &stt->iommu_tables, next) {
if (tbl != stit->tbl)
continue;
@@ -150,14 +156,17 @@ extern long kvm_spapr_tce_attach_iommu_group(struct kvm
*kvm, int tablefd,
if (!kref_get_unless_zero(&stit->kref)) {
/* stit is being destroyed */
iommu_tce_table_put(tbl);
+ rcu_read_unlock();
return -ENOTTY;
}
/*
* The table is already known to this KVM, we just increased
* its KVM reference counter and can return.
*/
+ rcu_read_unlock();
return 0;
}
+ rcu_read_unlock();
Re: [PATCH] net/sonic: Fix some resource leaks in error handling paths
> Is there a way to add a Fixes tag that would not invoke the -stable
> process? And was that what you had in mind?
Christophe Jaillet proposed to complete the exception handling also for this
function implementation.
I find that such a software correction is qualified for this tag.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/submitting-patches.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n183
Corresponding consequences can vary then according to the change management
of involved developers.
> I think 'undo_probe1' is both descriptive and consistent with commit
> 10e3cc180e64 ("net/sonic: Fix a resource leak in an error handling path in
> 'jazz_sonic_probe()'").
I can agree to this view (in principle).
By the way:
The referenced commit contains the tag “Fixes”.
https://lore.kernel.org/patchwork/patch/1231354/
https://lore.kernel.org/lkml/20200427061803.53857-1-christophe.jail...@wanadoo.fr/
> Your suggestion, 'free_dma' is also good.
Thanks for your positive feedback.
> But coming up with good alternatives is easy.
But the change acceptance can occasionally become harder.
> If every good alternative would be considered there would be no obvious way
> to get a patch merged.
I imagine that some alternatives can result in preferable solutions, can't they?
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/Documentation/process/coding-style.rst?id=e99332e7b4cda6e60f5b5916cf9943a79dbef902#n460
Regards,
Markus
Re: [PATCH net-next 4/4] net: phy: bcm54140: add cable diagnostics support
Hi Michael,
Thank you for the patch! Yet something to improve:
[auto build test ERROR on net-next/master]
[also build test ERROR on next-20200508]
[cannot apply to net/master linus/master ipvs/master v5.7-rc4]
[if your patch is applied to the wrong git tree, please drop us a note to help
improve the system. BTW, we also suggest to use '--base' option to specify the
base tree in git format-patch, please see https://stackoverflow.com/a/37406982]
url:
https://github.com/0day-ci/linux/commits/Michael-Walle/net-phy-broadcom-cable-tester-support/20200510-063955
base: https://git.kernel.org/pub/scm/linux/kernel/git/davem/net-next.git
2c674bec76d35b75c7c730f863424387c9e9633a
config: i386-allyesconfig (attached as .config)
compiler: gcc-7 (Ubuntu 7.5.0-6ubuntu2) 7.5.0
reproduce:
# save the attached .config to linux build tree
make ARCH=i386
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All error/warnings (new ones prefixed by >>):
>> drivers/net/phy/bcm54140.c:834:13: error: 'PHY_POLL_CABLE_TEST' undeclared
>> here (not in a function)
.flags = PHY_POLL_CABLE_TEST,
^~~
>> drivers/net/phy/bcm54140.c:846:4: error: 'struct phy_driver' has no member
>> named 'cable_test_start'
.cable_test_start = bcm_phy_cable_test_start_rdb,
^~~~
>> drivers/net/phy/bcm54140.c:846:23: error: initialization from incompatible
>> pointer type [-Werror=incompatible-pointer-types]
.cable_test_start = bcm_phy_cable_test_start_rdb,
^~~~
drivers/net/phy/bcm54140.c:846:23: note: (near initialization for
'bcm54140_drivers[0].set_loopback')
>> drivers/net/phy/bcm54140.c:847:4: error: 'struct phy_driver' has no member
>> named 'cable_test_get_status'
.cable_test_get_status = bcm_phy_cable_test_get_status_rdb,
^
>> drivers/net/phy/bcm54140.c:847:28: warning: excess elements in struct
>> initializer
.cable_test_get_status = bcm_phy_cable_test_get_status_rdb,
^
drivers/net/phy/bcm54140.c:847:28: note: (near initialization for
'bcm54140_drivers[0]')
cc1: some warnings being treated as errors
vim +/PHY_POLL_CABLE_TEST +834 drivers/net/phy/bcm54140.c
828
829 static struct phy_driver bcm54140_drivers[] = {
830 {
831 .phy_id = PHY_ID_BCM54140,
832 .phy_id_mask= BCM54140_PHY_ID_MASK,
833 .name = "Broadcom BCM54140",
> 834 .flags = PHY_POLL_CABLE_TEST,
835 .features = PHY_GBIT_FEATURES,
836 .config_init= bcm54140_config_init,
837 .did_interrupt = bcm54140_did_interrupt,
838 .ack_interrupt = bcm54140_ack_intr,
839 .config_intr= bcm54140_config_intr,
840 .probe = bcm54140_probe,
841 .suspend= genphy_suspend,
842 .resume = genphy_resume,
843 .soft_reset = genphy_soft_reset,
844 .get_tunable= bcm54140_get_tunable,
845 .set_tunable= bcm54140_set_tunable,
> 846 .cable_test_start = bcm_phy_cable_test_start_rdb,
> 847 .cable_test_get_status =
> bcm_phy_cable_test_get_status_rdb,
848 },
849 };
850 module_phy_driver(bcm54140_drivers);
851
---
0-DAY CI Kernel Test Service, Intel Corporation
https://lists.01.org/hyperkitty/list/kbuild-...@lists.01.org
.config.gz
Description: application/gzip
[rcu:dev.2020.05.07a] BUILD SUCCESS 1af6526eb52eb1fb840f5941e1b8691d49e03674
tree/branch: https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git dev.2020.05.07a branch HEAD: 1af6526eb52eb1fb840f5941e1b8691d49e03674 locking/osq_lock: Annotate a data race in osq_lock i386-tinyconfig vmlinux size: == TOTAL TEXT arch/x86/events/zhaoxin/built-in.* try_invoke_on_locked_down_task() == +1 0 353159365e72 rcu: Add KCSAN stubs 0 0 4f58820fd710 srcu: Add KCSAN stubs 0 0 2f0846956355 rcu: Mark rcu_state.ncpus to detect concurrent writes 0 0 314eeb43e5f2 rcu: Add *_ONCE() and data_race() to rcu_node ->exp_tasks pl 0 0 065a6db12a80 rcu: Add READ_ONCE and data_race() to rcu_node ->boost_tasks 0 0 b68c6146512d srcu: Add data_race() to ->srcu_lock_count and ->srcu_unlock 0 0 5822b8126ff0 rcu: Add WRITE_ONCE() to rcu_node ->boost_tasks 0 0 47fbb074536e rcu: Use data_race() for RCU CPU stall-warning prints 0 0 53965dbe5396 drm: Make drm_dp_mst_dsc_aux_for_port() safe for old compile 0 0 1fca4d12f463 rcu: Expedite first two FQS scans under callback-overload co 0 0 fcbcc0e70050 rcu: Fix the (t=0 jiffies) false positive 0 0 ddc465936643 Revert "rculist: Describe variadic macro argument in a Sphin 0 0 c28d5c09d09f rcu: Get rid of some doc warnings in update.c 0 0 62ae19511f1e rcu: Mark rcu_state.gp_seq to detect more concurrent writes 0 0 a66dbda7893f rcu: Replace assigned pointer ret value by corresponding boo 0 0 da44cd6c8e88 rcu: Replace 1 by true 0 0 29ffebc5fcc0 rcu: Convert ULONG_CMP_GE() to time_after() for jiffy compar 0 0 7b2413111a63 rcu: Convert rcu_initiate_boost() ULONG_CMP_GE() to time_aft 0 0 e2f3ccfa6200 rcu: Convert rcu_nohz_full_cpu() ULONG_CMP_LT() to time_befo +83 +84+136 +83 f736e0f1a55a Merge branches 'fixes.2020.04.27a', 'kfree_rcu.2020.04.27a', -224 -224 0 0 2d9d829af55c Merge branch 'kcsan-dev.2020.04.13c' into HEAD 0 0 0 0 99a5d03ba959 Merge branch 'lkmm-dev.2020.04.15b' into HEAD +38 +38 0 0 3123dcd3ef7f fork: Annotate a data race in vm_area_dup() 0 0 0 0 2ba74f25e0de x86/mm/pat: Mark an intentional data race 0 0 0 0 cd59625dedde rculist: Add ASSERT_EXCLUSIVE_ACCESS() to __list_splice_init 0 0 0 0 ca6e49239a17 locktorture: Use true and false to assign to bool variables 0 0 0 0 8c8786cd0247 srcu: Fix a typo in c
Re: USB Attached SCSI breakage due no udev involvement
On Sun, May 10, 2020 at 09:55:57AM +0700, Dio Putra wrote: > Hi, it's first time for me to report user-space breakage in here, so > i'm begging your pardon. > > I want to report that Linux 5.4 breaking my USB mount workflow due > udevadm monitor report here (I'm using vanilla kernel 5.4.39 on > Slackware64 Current and vanilla kernel 4.4.221 on Slackware64 14.2): Sorry, but what actually changed that you can see in the logs? What functionality broke? What used to work that no longer does work? And 4.4.221 is quite different from 5.4, is that the jump that you are seeing breakage in, or is it in some smaller jump? thanks, greg k-h
[PATCH v1 2/2] MIPS: ath79: ar9331: rename uart to serial node
schema violation was detected by the dtbs_check
Signed-off-by: Oleksij Rempel
---
arch/mips/boot/dts/qca/ar9331.dtsi | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/mips/boot/dts/qca/ar9331.dtsi
b/arch/mips/boot/dts/qca/ar9331.dtsi
index 6e1e17cbc1ee0..9267bc9e4cc09 100644
--- a/arch/mips/boot/dts/qca/ar9331.dtsi
+++ b/arch/mips/boot/dts/qca/ar9331.dtsi
@@ -59,7 +59,7 @@ ddr_ctrl: memory-controller@1800 {
#qca,ddr-wb-channel-cells = <1>;
};
- uart: uart@1802 {
+ uart: serial@1802 {
compatible = "qca,ar9330-uart";
reg = <0x1802 0x14>;
--
2.26.2
[PATCH v1 1/2] MIPS: ath79: ar9331_dpt_module: update led nodes
Fit led nodes to the latest naming schema.
Signed-off-by: Oleksij Rempel
---
arch/mips/boot/dts/qca/ar9331_dpt_module.dts | 6 --
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
b/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
index 969d3384b0dd0..8be54465207a7 100644
--- a/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
+++ b/arch/mips/boot/dts/qca/ar9331_dpt_module.dts
@@ -3,6 +3,7 @@
#include
#include
+#include
#include "ar9331.dtsi"
@@ -22,8 +23,9 @@ memory@0 {
leds {
compatible = "gpio-leds";
- system {
- label = "dpt-module:green:system";
+ led-0 {
+ function = LED_FUNCTION_STATUS;
+ color = ;
gpios = <&gpio 27 GPIO_ACTIVE_LOW>;
default-state = "off";
};
--
2.26.2
[PATCH V2] staging: vc04_services: vchiq_connected.c: Block comment alignment
Coding style issue
Signed-off-by: John Oldman
---
v1: Initial attempt.
V2: Resubmitted with shorter comment line, as suggested by Greg KH.
This patch clears the checkpatch.pl "Block comments should align the * on each
line" warning.
.../interface/vchiq_arm/vchiq_connected.c | 32 +--
1 file changed, 16 insertions(+), 16 deletions(-)
diff --git
a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
index 1640906e3929..993535bbc479 100644
--- a/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
+++ b/drivers/staging/vc04_services/interface/vchiq_arm/vchiq_connected.c
@@ -15,10 +15,10 @@ static intg_once_init;
static struct mutex g_connected_mutex;
/
-*
-* Function to initialize our lock.
-*
-***/
+ *
+ * Function to initialize our lock.
+ *
+ ***/
static void connected_init(void)
{
@@ -29,13 +29,13 @@ static void connected_init(void)
}
/
-*
-* This function is used to defer initialization until the vchiq stack is
-* initialized. If the stack is already initialized, then the callback will
-* be made immediately, otherwise it will be deferred until
-* vchiq_call_connected_callbacks is called.
-*
-***/
+ *
+ * This function is used to defer initialization until the vchiq stack is
+ * initialized. If the stack is already initialized, then the callback will
+ * be made immediately, otherwise it will be deferred until
+ * vchiq_call_connected_callbacks is called.
+ *
+ ***/
void vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback)
{
@@ -64,11 +64,11 @@ void
vchiq_add_connected_callback(VCHIQ_CONNECTED_CALLBACK_T callback)
}
/
-*
-* This function is called by the vchiq stack once it has been connected to
-* the videocore and clients can start to use the stack.
-*
-***/
+ *
+ * This function is called by the vchiq stack once it has been connected to
+ * the videocore and clients can start to use the stack.
+ *
+ ***/
void vchiq_call_connected_callbacks(void)
{
--
2.17.1
Re: [PATCH -next] arch/arm: use scnprintf() in l2x0_pmu_event_show()
On Sat, May 09, 2020 at 04:35:39PM +0800, Chen Zhou wrote: > snprintf() returns the number of bytes that would be written, > which may be greater than the the actual length to be written. > > show() methods should return the number of bytes printed into the > buffer. This is the return value of scnprintf(). > > Signed-off-by: Chen Zhou > --- > arch/arm/mm/cache-l2x0-pmu.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/arch/arm/mm/cache-l2x0-pmu.c b/arch/arm/mm/cache-l2x0-pmu.c > index 993fefdc167a..d20626451a2e 100644 > --- a/arch/arm/mm/cache-l2x0-pmu.c > +++ b/arch/arm/mm/cache-l2x0-pmu.c > @@ -343,7 +343,7 @@ static ssize_t l2x0_pmu_event_show(struct device *dev, > struct l2x0_event_attribute *lattr; > > lattr = container_of(attr, typeof(*lattr), attr); > - return snprintf(buf, PAGE_SIZE, "config=0x%x\n", lattr->config); > + return scnprintf(buf, PAGE_SIZE, "config=0x%x\n", lattr->config); This should just be replaced with sprintf() as there's no way such a string can go beyond PAGE_SIZE. And the "config=" part could be dropped as well, but now userspace probably requires it, that is sad. thanks, greg k-h
Re: [PATCH v4 1/5] block: revert back to synchronous request_queue removal
On Sat, May 09, 2020 at 03:10:54AM +, Luis Chamberlain wrote:
> Commit dc9edc44de6c ("block: Fix a blk_exit_rl() regression") merged on
> v4.12 moved the work behind blk_release_queue() into a workqueue after a
> splat floated around which indicated some work on blk_release_queue()
> could sleep in blk_exit_rl(). This splat would be possible when a driver
> called blk_put_queue() or blk_cleanup_queue() (which calls blk_put_queue()
> as its final call) from an atomic context.
>
> blk_put_queue() decrements the refcount for the request_queue kobject,
> and upon reaching 0 blk_release_queue() is called. Although blk_exit_rl()
> is now removed through commit db6d9952356 ("block: remove request_list code")
> on v5.0, we reserve the right to be able to sleep within blk_release_queue()
> context.
>
> The last reference for the request_queue must not be called from atomic
> context. *When* the last reference to the request_queue reaches 0 varies,
> and so let's take the opportunity to document when that is expected to
> happen and also document the context of the related calls as best as possible
> so we can avoid future issues, and with the hopes that the synchronous
> request_queue removal sticks.
>
> We revert back to synchronous request_queue removal because asynchronous
> removal creates a regression with expected userspace interaction with
> several drivers. An example is when removing the loopback driver, one
> uses ioctls from userspace to do so, but upon return and if successful,
> one expects the device to be removed. Likewise if one races to add another
> device the new one may not be added as it is still being removed. This was
> expected behavior before and it now fails as the device is still present
> and busy still. Moving to asynchronous request_queue removal could have
> broken many scripts which relied on the removal to have been completed if
> there was no error. Document this expectation as well so that this
> doesn't regress userspace again.
>
> Using asynchronous request_queue removal however has helped us find
> other bugs. In the future we can test what could break with this
> arrangement by enabling CONFIG_DEBUG_KOBJECT_RELEASE.
You are adding documenation and might_sleep() calls all over the place
in here, making the "real" change in the patch hard to pick out.
How about you split this up into 3 patches, one for documentation, one
for might_sleep() and one for the real change? Or maybe just 2 patches,
but what you have here seems excessive.
thanks,
greg k-h
Re: [PATCH v4 3/5] blktrace: fix debugfs use after free
On Sat, May 09, 2020 at 03:10:56AM +, Luis Chamberlain wrote:
> On commit 6ac93117ab00 ("blktrace: use existing disk debugfs directory")
> merged on v4.12 Omar fixed the original blktrace code for request-based
> drivers (multiqueue). This however left in place a possible crash, if you
> happen to abuse blktrace while racing to remove / add a device.
>
> We used to use asynchronous removal of the request_queue, and with that
> the issue was easier to reproduce. Now that we have reverted to
> synchronous removal of the request_queue, the issue is still possible to
> reproduce, its however just a bit more difficult.
>
> We essentially run two instances of break-blktrace which add/remove
> a loop device, and setup a blktrace and just never tear the blktrace
> down. We do this twice in parallel. This is easily reproduced with the
> break-blktrace run_0004.sh script.
>
> We can end up with two types of panics each reflecting where we
> race, one a failed blktrace setup:
>
> [ 252.426751] debugfs: Directory 'loop0' with parent 'block' already present!
> [ 252.432265] BUG: kernel NULL pointer dereference, address: 00a0
> [ 252.436592] #PF: supervisor write access in kernel mode
> [ 252.439822] #PF: error_code(0x0002) - not-present page
> [ 252.442967] PGD 0 P4D 0
> [ 252.444656] Oops: 0002 [#1] SMP NOPTI
> [ 252.446972] CPU: 10 PID: 1153 Comm: break-blktrace Tainted: GE
> 5.7.0-rc2-next-20200420+ #164
> [ 252.452673] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.13.0-1 04/01/2014
> [ 252.456343] RIP: 0010:down_write+0x15/0x40
> [ 252.458146] Code: eb ca e8 ae 22 8d ff cc cc cc cc cc cc cc cc cc cc cc cc
>cc cc 0f 1f 44 00 00 55 48 89 fd e8 52 db ff ff 31 c0 ba 01 00
>00 00 48 0f b1 55 00 75 0f 48 8b 04 25 c0 8b 01 00 48 89
>45 08 5d
> [ 252.463638] RSP: 0018:a626415abcc8 EFLAGS: 00010246
> [ 252.464950] RAX: RBX: 958c25f0f5c0 RCX:
> ff81
> [ 252.466727] RDX: 0001 RSI: ff81 RDI:
> 00a0
> [ 252.468482] RBP: 00a0 R08: R09:
> 0001
> [ 252.470014] R10: R11: 958d1f9227ff R12:
>
> [ 252.471473] R13: 958c25ea5380 R14: 8cce15f1 R15:
> 00a0
> [ 252.473346] FS: 7f2e69dee540() GS:958c2fc8()
> knlGS:
> [ 252.475225] CS: 0010 DS: ES: CR0: 80050033
> [ 252.476267] CR2: 00a0 CR3: 000427d10004 CR4:
> 00360ee0
> [ 252.477526] DR0: DR1: DR2:
>
> [ 252.478776] DR3: DR6: fffe0ff0 DR7:
> 0400
> [ 252.479866] Call Trace:
> [ 252.480322] simple_recursive_removal+0x4e/0x2e0
> [ 252.481078] ? debugfs_remove+0x60/0x60
> [ 252.481725] ? relay_destroy_buf+0x77/0xb0
> [ 252.482662] debugfs_remove+0x40/0x60
> [ 252.483518] blk_remove_buf_file_callback+0x5/0x10
> [ 252.484328] relay_close_buf+0x2e/0x60
> [ 252.484930] relay_open+0x1ce/0x2c0
> [ 252.485520] do_blk_trace_setup+0x14f/0x2b0
> [ 252.486187] __blk_trace_setup+0x54/0xb0
> [ 252.486803] blk_trace_ioctl+0x90/0x140
> [ 252.487423] ? do_sys_openat2+0x1ab/0x2d0
> [ 252.488053] blkdev_ioctl+0x4d/0x260
> [ 252.488636] block_ioctl+0x39/0x40
> [ 252.489139] ksys_ioctl+0x87/0xc0
> [ 252.489675] __x64_sys_ioctl+0x16/0x20
> [ 252.490380] do_syscall_64+0x52/0x180
> [ 252.491032] entry_SYSCALL_64_after_hwframe+0x44/0xa9
>
> And the other on the device removal:
>
> [ 128.528940] debugfs: Directory 'loop0' with parent 'block' already present!
> [ 128.615325] BUG: kernel NULL pointer dereference, address: 00a0
> [ 128.619537] #PF: supervisor write access in kernel mode
> [ 128.622700] #PF: error_code(0x0002) - not-present page
> [ 128.625842] PGD 0 P4D 0
> [ 128.627585] Oops: 0002 [#1] SMP NOPTI
> [ 128.629871] CPU: 12 PID: 544 Comm: break-blktrace Tainted: GE
>5.7.0-rc2-next-20200420+ #164
> [ 128.635595] Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS
> 1.13.0-1 04/01/2014
> [ 128.640471] RIP: 0010:down_write+0x15/0x40
> [ 128.643041] Code: eb ca e8 ae 22 8d ff cc cc cc cc cc cc cc cc cc cc cc cc
>cc cc 0f 1f 44 00 00 55 48 89 fd e8 52 db ff ff 31 c0 ba 01 00
>00 00 48 0f b1 55 00 75 0f 65 48 8b 04 25 c0 8b 01 00 48
> 89
>45 08 5d
> [ 128.650180] RSP: 0018:a9c3c05ebd78 EFLAGS: 00010246
> [ 128.651820] RAX: RBX: 8ae9a6370240 RCX:
> ff81
> [ 128.653942] RDX: 0001 RSI: ff81 RDI:
> 00a0
> [ 128.655720] RBP: 00a0 R08: 0002 R09:
> 8ae9afd2d3d0
> [ 128.657400] R10: 0056 R11: R12:
>
> [ 128.659099] R13: R14: 0003 R15:
Re: [PATCH 2/2] perf: Add missing metrics to POWER9 'cpi_breakdown'
On Fri, May 8, 2020 at 12:05 AM kajoljain wrote:
> On 5/7/20 9:58 PM, Paul A. Clarke wrote:
> > From: "Paul A. Clarke"
> >
> > Add the following metrics to the POWER9 'cpi_breakdown' metricgroup:
> > - ict_noslot_br_mpred_cpi
> > - ict_noslot_br_mpred_icmiss_cpi
> > - ict_noslot_cyc_other_cpi
> > - ict_noslot_disp_held_cpi
> > - ict_noslot_disp_held_hb_full_cpi
> > - ict_noslot_disp_held_issq_cpi
> > - ict_noslot_disp_held_other_cpi
> > - ict_noslot_disp_held_sync_cpi
> > - ict_noslot_disp_held_tbegin_cpi
> > - ict_noslot_ic_l2_cpi
> > - ict_noslot_ic_l3_cpi
> > - ict_noslot_ic_l3miss_cpi
> > - ict_noslot_ic_miss_cpi
> >
> > Signed-off-by: Paul A. Clarke
>
> Reviewed-by: Kajol Jain
Tested with:
https://lore.kernel.org/lkml/20200508053629.210324-3-irog...@google.com/
Tested-by: Ian Rogers
Thanks,
Ian
> Thanks,
> Kajol Jain
> > ---
> > .../arch/powerpc/power9/metrics.json | 143 ++
> > 1 file changed, 78 insertions(+), 65 deletions(-)
> >
> > diff --git a/tools/perf/pmu-events/arch/powerpc/power9/metrics.json
> > b/tools/perf/pmu-events/arch/powerpc/power9/metrics.json
> > index 811c2a8c1c9e..6169351a72c8 100644
> > --- a/tools/perf/pmu-events/arch/powerpc/power9/metrics.json
> > +++ b/tools/perf/pmu-events/arch/powerpc/power9/metrics.json
> > @@ -207,6 +207,84 @@
> > "MetricGroup": "cpi_breakdown",
> > "MetricName": "fxu_stall_cpi"
> > },
> > +{
> > +"BriefDescription": "Ict empty for this thread due to branch
> > mispred",
> > +"MetricExpr": "PM_ICT_NOSLOT_BR_MPRED/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_br_mpred_cpi"
> > +},
> > +{
> > +"BriefDescription": "Ict empty for this thread due to Icache Miss
> > and branch mispred",
> > +"MetricExpr": "PM_ICT_NOSLOT_BR_MPRED_ICMISS/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_br_mpred_icmiss_cpi"
> > +},
> > +{
> > +"BriefDescription": "ICT other stalls",
> > +"MetricExpr": "(PM_ICT_NOSLOT_CYC - PM_ICT_NOSLOT_IC_MISS -
> > PM_ICT_NOSLOT_BR_MPRED_ICMISS - PM_ICT_NOSLOT_BR_MPRED -
> > PM_ICT_NOSLOT_DISP_HELD)/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_cyc_other_cpi"
> > +},
> > +{
> > +"BriefDescription": "Cycles in which the NTC instruciton is held
> > at dispatch for any reason",
> > +"MetricExpr": "PM_ICT_NOSLOT_DISP_HELD/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_disp_held_cpi"
> > +},
> > +{
> > +"BriefDescription": "Ict empty for this thread due to dispatch
> > holds because the History Buffer was full. Could be GPR/VSR/VMR/FPR/CR/XVF",
> > +"MetricExpr": "PM_ICT_NOSLOT_DISP_HELD_HB_FULL/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_disp_held_hb_full_cpi"
> > +},
> > +{
> > +"BriefDescription": "Ict empty for this thread due to dispatch
> > hold on this thread due to Issue q full, BRQ full, XVCF Full, Count cache,
> > Link, Tar full",
> > +"MetricExpr": "PM_ICT_NOSLOT_DISP_HELD_ISSQ/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_disp_held_issq_cpi"
> > +},
> > +{
> > +"BriefDescription": "ICT_NOSLOT_DISP_HELD_OTHER_CPI",
> > +"MetricExpr": "(PM_ICT_NOSLOT_DISP_HELD -
> > PM_ICT_NOSLOT_DISP_HELD_HB_FULL - PM_ICT_NOSLOT_DISP_HELD_SYNC -
> > PM_ICT_NOSLOT_DISP_HELD_TBEGIN -
> > PM_ICT_NOSLOT_DISP_HELD_ISSQ)/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_disp_held_other_cpi"
> > +},
> > +{
> > +"BriefDescription": "Dispatch held due to a synchronizing
> > instruction at dispatch",
> > +"MetricExpr": "PM_ICT_NOSLOT_DISP_HELD_SYNC/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_disp_held_sync_cpi"
> > +},
> > +{
> > +"BriefDescription": "the NTC instruction is being held at dispatch
> > because it is a tbegin instruction and there is an older tbegin in the
> > pipeline that must complete before the younger tbegin can dispatch",
> > +"MetricExpr": "PM_ICT_NOSLOT_DISP_HELD_TBEGIN/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_disp_held_tbegin_cpi"
> > +},
> > +{
> > +"BriefDescription": "ICT_NOSLOT_IC_L2_CPI",
> > +"MetricExpr": "(PM_ICT_NOSLOT_IC_MISS - PM_ICT_NOSLOT_IC_L3 -
> > PM_ICT_NOSLOT_IC_L3MISS)/PM_RUN_INST_CMPL",
> > +"MetricGroup": "cpi_breakdown",
> > +"MetricName": "ict_noslot_ic_l2_cpi"
> > +},
> > +{
> > +"BriefDescription": "Ict empty for this thread due to icache
> > misses that were sourced from the lo
drivers/crypto/chelsio/chcr_ktls.c:1078: undefined reference to `tls_get_record'
tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
master
head: e99332e7b4cda6e60f5b5916cf9943a79dbef902
commit: 5a4b9fe7fece62ecab6fb28fe92362f83b41c33e cxgb4/chcr: complete record tx
handling
date: 9 weeks ago
config: alpha-randconfig-r036-20200510 (attached as .config)
compiler: alpha-linux-gcc (GCC) 9.3.0
reproduce:
wget
https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O
~/bin/make.cross
chmod +x ~/bin/make.cross
git checkout 5a4b9fe7fece62ecab6fb28fe92362f83b41c33e
# save the attached .config to linux build tree
COMPILER_INSTALL_PATH=$HOME/0day GCC_VERSION=9.3.0 make.cross
ARCH=alpha
If you fix the issue, kindly add following tag as appropriate
Reported-by: kbuild test robot
All errors (new ones prefixed by >>):
alpha-linux-ld: drivers/crypto/chelsio/chcr_ktls.o: in function
`tls_is_sk_tx_device_offloaded':
>> include/net/tls.h:431: undefined reference to `tls_validate_xmit_skb'
alpha-linux-ld: drivers/crypto/chelsio/chcr_ktls.o: in function
`chcr_ktls_xmit':
>> drivers/crypto/chelsio/chcr_ktls.c:1078: undefined reference to
>> `tls_get_record'
>> alpha-linux-ld: drivers/crypto/chelsio/chcr_ktls.c:1078: undefined reference
>> to `tls_get_record'
vim +1078 drivers/crypto/chelsio/chcr_ktls.c
996
997 /* nic tls TX handler */
998 int chcr_ktls_xmit(struct sk_buff *skb, struct net_device *dev)
999 {
1000 struct chcr_ktls_ofld_ctx_tx *tx_ctx;
1001 struct tcphdr *th = tcp_hdr(skb);
1002 int data_len, qidx, ret = 0, mss;
1003 struct tls_record_info *record;
1004 struct chcr_ktls_info *tx_info;
1005 u32 tls_end_offset, tcp_seq;
1006 struct tls_context *tls_ctx;
1007 struct sk_buff *local_skb;
1008 int new_connection_state;
1009 struct sge_eth_txq *q;
1010 struct adapter *adap;
1011 unsigned long flags;
1012
1013 tcp_seq = ntohl(th->seq);
1014
1015 mss = skb_is_gso(skb) ? skb_shinfo(skb)->gso_size :
skb->data_len;
1016
1017 /* check if we haven't set it for ktls offload */
1018 if (!skb->sk || !tls_is_sk_tx_device_offloaded(skb->sk))
1019 goto out;
1020
1021 tls_ctx = tls_get_ctx(skb->sk);
1022 if (unlikely(tls_ctx->netdev != dev))
1023 goto out;
1024
1025 tx_ctx = chcr_get_ktls_tx_context(tls_ctx);
1026 tx_info = tx_ctx->chcr_info;
1027
1028 if (unlikely(!tx_info))
1029 goto out;
1030
1031 /* check the connection state, we don't need to pass new
connection
1032 * state, state machine will check and update the new state if
it is
1033 * stuck due to responses not received from HW.
1034 * Start the tx handling only if state is KTLS_CONN_TX_READY.
1035 */
1036 new_connection_state =
chcr_ktls_update_connection_state(tx_info, 0);
1037 if (new_connection_state != KTLS_CONN_TX_READY)
1038 goto out;
1039
1040 /* don't touch the original skb, make a new skb to extract each
records
1041 * and send them separately.
1042 */
1043 local_skb = alloc_skb(0, GFP_KERNEL);
1044
1045 if (unlikely(!local_skb))
1046 return NETDEV_TX_BUSY;
1047
1048 adap = tx_info->adap;
1049 qidx = skb->queue_mapping;
1050 q = &adap->sge.ethtxq[qidx + tx_info->first_qset];
1051 cxgb4_reclaim_completed_tx(adap, &q->q, true);
1052 /* update tcb */
1053 ret = chcr_ktls_xmit_tcb_cpls(tx_info, q, ntohl(th->seq),
1054ntohl(th->ack_seq),
1055ntohs(th->window));
1056 if (ret) {
1057 dev_kfree_skb_any(local_skb);
1058 return NETDEV_TX_BUSY;
1059 }
1060
1061 /* copy skb contents into local skb */
1062 chcr_ktls_skb_copy(skb, local_skb);
1063
1064 /* go through the skb and send only one record at a time. */
1065 data_len = skb->data_len;
1066 /* TCP segments can be in received from host either complete or
partial.
1067 * chcr_end_part_handler will handle cases if complete record
or end
1068 * part of the record is received. Incase of partial end part
of record,
1069 * we will send the complete record again.
1070 */
1071 do {
1072 int i;
1073
1074 cxgb4_reclaim_completed_tx(adap, &q->q, true);
1075 /* lock taken */
1076 spin_lock_irqsave(&tx_ctx->base.lock, flags);
1077 /* fetch the tls record */
> 1078
Re: USB Attached SCSI breakage due no udev involvement
On 5/10/20 12:47 PM, Greg KH wrote:
> On Sun, May 10, 2020 at 09:55:57AM +0700, Dio Putra wrote:
>> Hi, it's first time for me to report user-space breakage in here, so
>> i'm begging your pardon.
>>
>> I want to report that Linux 5.4 breaking my USB mount workflow due
>> udevadm monitor report here (I'm using vanilla kernel 5.4.39 on
>> Slackware64 Current and vanilla kernel 4.4.221 on Slackware64 14.2):
>
>
>
> Sorry, but what actually changed that you can see in the logs?
Sorry, what do you mean? The dmesg log or the kernel changelogs?
>
> What functionality broke? What used to work that no longer does work?
>
Yes, it supposed that just work and kernel could talk with udev, not just
handled by the kernel.
Here's my personal conclusion:
[Slackware64-Current linux-5.4.39]$ sudo strace -p 2417
strace: Process 2417 attached
write(1, "KERNEL[672.150926] remove /dev"..., 84) = 84
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=0x01},
msg_namelen=128->12, msg_iov=[{iov_base="remove@/devices/pci:00/:"...,
iov_len=8192}], msg_iovlen=1, msg_control=[{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, cmsg_data={pid=0, uid=0, gid=0}}],
msg_controllen=32, msg_flags=0}, 0) = 269
write(1, "KERNEL[672.151622] remove /dev"..., 122) = 122
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=0x01},
msg_namelen=128->12, msg_iov=[{iov_base="remove@/devices/pci:00/:"...,
iov_len=8192}], msg_iovlen=1, msg_control=[{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, cmsg_data={pid=0, uid=0, gid=0}}],
msg_controllen=32, msg_flags=0}, 0) = 255
write(1, "KERNEL[672.152396] remove /dev"..., 138) = 138
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name={sa_family=AF_NETLINK, nl_pid=0, nl_groups=0x01},
msg_namelen=128->12, msg_iov=[{iov_base="remove@/devices/virtual/bdi/11:0"...,
iov_len=8192}], msg_iovlen=1, msg_control=[{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, cmsg_data={pid=0, uid=0, gid=0}}],
msg_controllen=32, msg_flags=0}, 0) = 107
write(1, "KERNEL[672.153191] remove /dev"..., 60) = 60
..
"So the summary of syscall on linux-5.4.39 it's something like: write -->
epoll_wait --> recvmsg (direct call from kernel only, no udev dependencies has
been called ever) --> write"
[Slackware64-14.2 linux-kernel-4.4.221] $ sudo strace -p 2102
strace: Process 2102 attached
write(1, "KERNEL[160.475104] remove /dev"..., 84) = 84
epoll_wait(3, [{EPOLLIN, {u32=4, u64=4}}], 4, -1) = 1
recvmsg(4, {msg_name(12)={sa_family=AF_NETLINK, pid=598, groups=0002},
msg_iov(1)=[{"libudev\0\376\355\312\376(\0\0\0(\0\0\0\375\0\0\0\5w\305\345\261\2Ge"...,
8192}], msg_controllen=32, [{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, {pid=2116, uid=0, gid=0}}], msg_flags=0}, 0) = 293
write(1, "UDEV [160.476746] remove /dev"..., 84) = 84
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=0001},
msg_iov(1)=[{"remove@/devices/pci:00/:"..., 8192}], msg_controllen=32,
[{cmsg_len=28, cmsg_level=SOL_SOCKET, cmsg_type=SCM_CREDENTIALS, {pid=0, uid=0,
gid=0}}], msg_flags=0}, 0) = 256
write(1, "KERNEL[160.478096] remove /dev"..., 76) = 76
epoll_wait(3, [{EPOLLIN, {u32=4, u64=4}}], 4, -1) = 1
recvmsg(4, {msg_name(12)={sa_family=AF_NETLINK, pid=598, groups=0002},
msg_iov(1)=[{"libudev\0\376\355\312\376(\0\0\0(\0\0\0\26\2\0\0\5w\305\345'\370\365\f"...,
8192}], msg_controllen=32, [{cmsg_len=28, cmsg_level=SOL_SOCKET,
cmsg_type=SCM_CREDENTIALS, {pid=2116, uid=0, gid=0}}], msg_flags=0}, 0) = 574
write(1, "UDEV [160.495353] remove /dev"..., 76) = 76
epoll_wait(3, [{EPOLLIN, {u32=5, u64=5}}], 4, -1) = 1
recvmsg(5, {msg_name(12)={sa_family=AF_NETLINK, pid=0, groups=0001},
msg_iov(1)=[{"add@/devices/pci:00/:00:"..., 8192}], msg_controllen=32,
[{cmsg_len=28, cmsg_level=SOL_SOCKET, cmsg_type=SCM_CREDENTIALS, {pid=0, uid=0,
gid=0}}], msg_flags=0}, 0) = 250
write(1, "KERNEL[161.109310] add /dev"..., 76) = 76
..
"So the summary of syscall on linux-4.4.221 it's something like: write -->
epoll_wait --> recvmsg (libudev has been called) --> write (UDEV has been
called) --> epoll_wait -> recvmsg (libudev has been called again)-> write"
> And 4.4.221 is quite different from 5.4, is that the jump that you are
> seeing breakage in, or is it in some smaller jump?
>
Yeah, I know it's really different between linux-4.4 and linux-5.4
> thanks,
>
> greg k-h
>
Re: [PATCH] tty: hvc: Fix data abort due to race in hvc_open
On Sat, May 09, 2020 at 06:30:56PM -0700, rana...@codeaurora.org wrote:
> On 2020-05-06 02:48, Greg KH wrote:
> > On Mon, Apr 27, 2020 at 08:26:01PM -0700, Raghavendra Rao Ananta wrote:
> > > Potentially, hvc_open() can be called in parallel when two tasks calls
> > > open() on /dev/hvcX. In such a scenario, if the
> > > hp->ops->notifier_add()
> > > callback in the function fails, where it sets the tty->driver_data to
> > > NULL, the parallel hvc_open() can see this NULL and cause a memory
> > > abort.
> > > Hence, serialize hvc_open and check if tty->private_data is NULL
> > > before
> > > proceeding ahead.
> > >
> > > The issue can be easily reproduced by launching two tasks
> > > simultaneously
> > > that does nothing but open() and close() on /dev/hvcX.
> > > For example:
> > > $ ./simple_open_close /dev/hvc0 & ./simple_open_close /dev/hvc0 &
> > >
> > > Signed-off-by: Raghavendra Rao Ananta
> > > ---
> > > drivers/tty/hvc/hvc_console.c | 16 ++--
> > > 1 file changed, 14 insertions(+), 2 deletions(-)
> > >
> > > diff --git a/drivers/tty/hvc/hvc_console.c
> > > b/drivers/tty/hvc/hvc_console.c
> > > index 436cc51c92c3..ebe26fe5ac09 100644
> > > --- a/drivers/tty/hvc/hvc_console.c
> > > +++ b/drivers/tty/hvc/hvc_console.c
> > > @@ -75,6 +75,8 @@ static LIST_HEAD(hvc_structs);
> > > */
> > > static DEFINE_MUTEX(hvc_structs_mutex);
> > >
> > > +/* Mutex to serialize hvc_open */
> > > +static DEFINE_MUTEX(hvc_open_mutex);
> > > /*
> > > * This value is used to assign a tty->index value to a hvc_struct
> > > based
> > > * upon order of exposure via hvc_probe(), when we can not match it
> > > to
> > > @@ -346,16 +348,24 @@ static int hvc_install(struct tty_driver
> > > *driver, struct tty_struct *tty)
> > > */
> > > static int hvc_open(struct tty_struct *tty, struct file * filp)
> > > {
> > > - struct hvc_struct *hp = tty->driver_data;
> > > + struct hvc_struct *hp;
> > > unsigned long flags;
> > > int rc = 0;
> > >
> > > + mutex_lock(&hvc_open_mutex);
> > > +
> > > + hp = tty->driver_data;
> > > + if (!hp) {
> > > + rc = -EIO;
> > > + goto out;
> > > + }
> > > +
> > > spin_lock_irqsave(&hp->port.lock, flags);
> > > /* Check and then increment for fast path open. */
> > > if (hp->port.count++ > 0) {
> > > spin_unlock_irqrestore(&hp->port.lock, flags);
> > > hvc_kick();
> > > - return 0;
> > > + goto out;
> > > } /* else count == 0 */
> > > spin_unlock_irqrestore(&hp->port.lock, flags);
> >
> > Wait, why isn't this driver just calling tty_port_open() instead of
> > trying to open-code all of this?
> >
> > Keeping a single mutext for open will not protect it from close, it will
> > just slow things down a bit. There should already be a tty lock held by
> > the tty core for open() to keep it from racing things, right?
> The tty lock should have been held, but not likely across ->install() and
> ->open() callbacks, thus resulting in a race between hvc_install() and
> hvc_open(),
How? The tty lock is held in install, and should not conflict with
open(), otherwise we would be seeing this happen in all tty drivers,
right?
> where hvc_install() sets a data and the hvc_open() clears it. hvc_open()
> doesn't
> check if the data was set to NULL and proceeds.
What data is being set that hvc_open is checking?
And you are not grabbing a lock in your install callback, you are only
serializing your open call here, I don't see how this is fixing anything
other than perhaps slowing down your codepaths.
As an arument why this isn't correct, can you answer why this same type
of change wouldn't be required for all tty drivers in the tree?
thanks,
greg k-h
Re: USB Attached SCSI breakage due no udev involvement
On Sun, May 10, 2020 at 01:48:24PM +0700, Dio Putra wrote: > On 5/10/20 12:47 PM, Greg KH wrote: > > On Sun, May 10, 2020 at 09:55:57AM +0700, Dio Putra wrote: > >> Hi, it's first time for me to report user-space breakage in here, so > >> i'm begging your pardon. > >> > >> I want to report that Linux 5.4 breaking my USB mount workflow due > >> udevadm monitor report here (I'm using vanilla kernel 5.4.39 on > >> Slackware64 Current and vanilla kernel 4.4.221 on Slackware64 14.2): > > > > > > > > Sorry, but what actually changed that you can see in the logs? > Sorry, what do you mean? The dmesg log or the kernel changelogs? Either, your message made them pretty impossible to compare with all of the line-wrapping :( > > What functionality broke? What used to work that no longer does work? > > > Yes, it supposed that just work and kernel could talk with udev, not just > handled by the kernel. I don't understand, what functionality changed? What exactly used to work that no longer does? Did you change anything else other than the kernel on your system? Did you change to a newer version of udev/systemd or anything else? thanks, greg k-h
Re: [PATCH V2] staging: vc04_services: vchiq_connected.c: Block comment alignment
On Sun, May 10, 2020 at 07:06:45AM +0100, John Oldman wrote: > Coding style issue > > Signed-off-by: John Oldman > --- > v1: Initial attempt. > V2: Resubmitted with shorter comment line, as suggested by Greg KH. > > This patch clears the checkpatch.pl "Block comments should align the * on > each line" warning. Why is this text not in the changelog area? Don't you think that it is better than the 3 words you currently put there? Please fix up and resend. thanks, greg k-h
