[PATCH 2/2] mtd: spi-nor: intel-spi: add support for Intel Cannon Lake SPI flash
(apologies, resending without S/MIME signature) Now that SPI flash controllers without a software sequencer are supported, it's trivial to add support for CNL and its PCI ID. Signed-off-by: Jethro Beekman --- drivers/mtd/spi-nor/intel-spi-pci.c | 5 + drivers/mtd/spi-nor/intel-spi.c | 11 +++ include/linux/platform_data/intel-spi.h | 1 + 3 files changed, 17 insertions(+) diff --git a/drivers/mtd/spi-nor/intel-spi-pci.c b/drivers/mtd/spi-nor/intel-spi-pci.c index b83c4ab6..195a09d 100644 --- a/drivers/mtd/spi-nor/intel-spi-pci.c +++ b/drivers/mtd/spi-nor/intel-spi-pci.c @@ -20,6 +20,10 @@ static const struct intel_spi_boardinfo bxt_info = { .type = INTEL_SPI_BXT, }; +static const struct intel_spi_boardinfo cnl_info = { + .type = INTEL_SPI_CNL, +}; + static int intel_spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { @@ -67,6 +71,7 @@ static const struct pci_device_id intel_spi_pci_ids[] = { { PCI_VDEVICE(INTEL, 0x4b24), (unsigned long)_info }, { PCI_VDEVICE(INTEL, 0xa1a4), (unsigned long)_info }, { PCI_VDEVICE(INTEL, 0xa224), (unsigned long)_info }, + { PCI_VDEVICE(INTEL, 0xa324), (unsigned long)_info }, { }, }; MODULE_DEVICE_TABLE(pci, intel_spi_pci_ids); diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c index 195cdca..91b7851 100644 --- a/drivers/mtd/spi-nor/intel-spi.c +++ b/drivers/mtd/spi-nor/intel-spi.c @@ -108,6 +108,10 @@ #define BXT_FREG_NUM 12 #define BXT_PR_NUM 6 +#define CNL_PR 0x84 +#define CNL_FREG_NUM 6 +#define CNL_PR_NUM 5 + #define LVSCC0xc4 #define UVSCC0xc8 #define ERASE_OPCODE_SHIFT 8 @@ -344,6 +348,13 @@ static int intel_spi_init(struct intel_spi *ispi) ispi->erase_64k = true; break; +case INTEL_SPI_CNL: + ispi->sregs = NULL; + ispi->pregs = ispi->base + CNL_PR; + ispi->nregions = CNL_FREG_NUM; + ispi->pr_num = CNL_PR_NUM; + break; + default: return -EINVAL; } diff --git a/include/linux/platform_data/intel-spi.h b/include/linux/platform_data/intel-spi.h index ebb4f33..7f53a5c 100644 --- a/include/linux/platform_data/intel-spi.h +++ b/include/linux/platform_data/intel-spi.h @@ -13,6 +13,7 @@ enum intel_spi_type { INTEL_SPI_BYT = 1, INTEL_SPI_LPT, INTEL_SPI_BXT, + INTEL_SPI_CNL, }; /** -- 2.7.4
[PATCH 2/2] mtd: spi-nor: intel-spi: add support for Intel Cannon Lake SPI flash
Now that SPI flash controllers without a software sequencer are supported, it's trivial to add support for CNL and its PCI ID. Signed-off-by: Jethro Beekman --- drivers/mtd/spi-nor/intel-spi-pci.c | 5 + drivers/mtd/spi-nor/intel-spi.c | 11 +++ include/linux/platform_data/intel-spi.h | 1 + 3 files changed, 17 insertions(+) diff --git a/drivers/mtd/spi-nor/intel-spi-pci.c b/drivers/mtd/spi-nor/intel-spi-pci.c index b83c4ab6..195a09d 100644 --- a/drivers/mtd/spi-nor/intel-spi-pci.c +++ b/drivers/mtd/spi-nor/intel-spi-pci.c @@ -20,6 +20,10 @@ static const struct intel_spi_boardinfo bxt_info = { .type = INTEL_SPI_BXT, }; +static const struct intel_spi_boardinfo cnl_info = { + .type = INTEL_SPI_CNL, +}; + static int intel_spi_pci_probe(struct pci_dev *pdev, const struct pci_device_id *id) { @@ -67,6 +71,7 @@ static const struct pci_device_id intel_spi_pci_ids[] = { { PCI_VDEVICE(INTEL, 0x4b24), (unsigned long)_info }, { PCI_VDEVICE(INTEL, 0xa1a4), (unsigned long)_info }, { PCI_VDEVICE(INTEL, 0xa224), (unsigned long)_info }, + { PCI_VDEVICE(INTEL, 0xa324), (unsigned long)_info }, { }, }; MODULE_DEVICE_TABLE(pci, intel_spi_pci_ids); diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c index 195cdca..91b7851 100644 --- a/drivers/mtd/spi-nor/intel-spi.c +++ b/drivers/mtd/spi-nor/intel-spi.c @@ -108,6 +108,10 @@ #define BXT_FREG_NUM 12 #define BXT_PR_NUM 6 +#define CNL_PR0x84 +#define CNL_FREG_NUM 6 +#define CNL_PR_NUM 5 + #define LVSCC 0xc4 #define UVSCC 0xc8 #define ERASE_OPCODE_SHIFT 8 @@ -344,6 +348,13 @@ static int intel_spi_init(struct intel_spi *ispi) ispi->erase_64k = true; break; + case INTEL_SPI_CNL: + ispi->sregs = NULL; + ispi->pregs = ispi->base + CNL_PR; + ispi->nregions = CNL_FREG_NUM; + ispi->pr_num = CNL_PR_NUM; + break; + default: return -EINVAL; } diff --git a/include/linux/platform_data/intel-spi.h b/include/linux/platform_data/intel-spi.h index ebb4f33..7f53a5c 100644 --- a/include/linux/platform_data/intel-spi.h +++ b/include/linux/platform_data/intel-spi.h @@ -13,6 +13,7 @@ enum intel_spi_type { INTEL_SPI_BYT = 1, INTEL_SPI_LPT, INTEL_SPI_BXT, + INTEL_SPI_CNL, }; /** -- 2.7.4 smime.p7s Description: S/MIME Cryptographic Signature
[PATCH 1/2] mtd: spi-nor: intel-spi: support chips without software sequencer
Some flash controllers don't have a software sequencer. Avoid configuring the register addresses for it, and double check everywhere that its not accidentally trying to be used. Every use of `sregs` is now guarded by a check of `sregs` or `swseq_reg`. The check might be done in the calling function. Signed-off-by: Jethro Beekman --- drivers/mtd/spi-nor/intel-spi.c | 23 --- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/drivers/mtd/spi-nor/intel-spi.c b/drivers/mtd/spi-nor/intel-spi.c index 1ccf23f..195cdca 100644 --- a/drivers/mtd/spi-nor/intel-spi.c +++ b/drivers/mtd/spi-nor/intel-spi.c @@ -187,12 +187,16 @@ static void intel_spi_dump_regs(struct intel_spi *ispi) dev_dbg(ispi->dev, "PR(%d)=0x%08x\n", i, readl(ispi->pregs + PR(i))); - value = readl(ispi->sregs + SSFSTS_CTL); - dev_dbg(ispi->dev, "SSFSTS_CTL=0x%08x\n", value); - dev_dbg(ispi->dev, "PREOP_OPTYPE=0x%08x\n", - readl(ispi->sregs + PREOP_OPTYPE)); - dev_dbg(ispi->dev, "OPMENU0=0x%08x\n", readl(ispi->sregs + OPMENU0)); - dev_dbg(ispi->dev, "OPMENU1=0x%08x\n", readl(ispi->sregs + OPMENU1)); + if (ispi->sregs) { + value = readl(ispi->sregs + SSFSTS_CTL); + dev_dbg(ispi->dev, "SSFSTS_CTL=0x%08x\n", value); + dev_dbg(ispi->dev, "PREOP_OPTYPE=0x%08x\n", + readl(ispi->sregs + PREOP_OPTYPE)); + dev_dbg(ispi->dev, "OPMENU0=0x%08x\n", + readl(ispi->sregs + OPMENU0)); + dev_dbg(ispi->dev, "OPMENU1=0x%08x\n", + readl(ispi->sregs + OPMENU1)); + } if (ispi->info->type == INTEL_SPI_BYT) dev_dbg(ispi->dev, "BCR=0x%08x\n", readl(ispi->base + BYT_BCR)); @@ -367,6 +371,11 @@ static int intel_spi_init(struct intel_spi *ispi) !(uvscc & ERASE_64K_OPCODE_MASK)) ispi->erase_64k = false; + if (ispi->sregs == NULL && (ispi->swseq_reg || ispi->swseq_erase)) { + dev_err(ispi->dev, "software sequencer not supported, but required\n"); + return -EINVAL; + } + /* * Some controllers can only do basic operations using hardware * sequencer. All other operations are supposed to be carried out @@ -383,7 +392,7 @@ static int intel_spi_init(struct intel_spi *ispi) val = readl(ispi->base + HSFSTS_CTL); ispi->locked = !!(val & HSFSTS_CTL_FLOCKDN); - if (ispi->locked) { + if (ispi->locked && ispi->sregs) { /* * BIOS programs allowed opcodes and then locks down the * register. So read back what opcodes it decided to support. -- 2.7.4
Re: [PATCH v3 00/13] virtio-fs: shared file system for virtual machines
On Thu, Aug 29, 2019 at 6:01 PM Vivek Goyal wrote: > > On Thu, Aug 29, 2019 at 11:28:27AM +0200, Miklos Szeredi wrote: > > [..] > > There are miscellaneous changes, so needs to be thoroughly tested. > > Hi Miklos, > > First round of tests passed. Ran pjdfstests, blogbench and bunch of fio > jobs and everyting looks good. fsx-linux with cache=always revealed a couple of bugs in the argpages case. Pushed fixes for those too. Thanks, Miklos
Re: [PATCH v2 2/2] Bluetooth: btrtl: Add firmware version print
Hi Alex, > This patch is used to print fw version for debug convenience > > Signed-off-by: Alex Lu > --- > Changes in v2 > - Re-order the code so that no forward declaration is needed > > drivers/bluetooth/btrtl.c | 56 --- > 1 file changed, 35 insertions(+), 21 deletions(-) > > diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c > index b7487ab99eed..e32ef7c60a22 100644 > --- a/drivers/bluetooth/btrtl.c > +++ b/drivers/bluetooth/btrtl.c > @@ -178,6 +178,27 @@ static const struct id_table *btrtl_match_ic(u16 > lmp_subver, u16 hci_rev, > return _id_table[i]; > } > > +static struct sk_buff *btrtl_read_local_version(struct hci_dev *hdev) > +{ > + struct sk_buff *skb; > + > + skb = __hci_cmd_sync(hdev, HCI_OP_READ_LOCAL_VERSION, 0, NULL, > + HCI_INIT_TIMEOUT); > + if (IS_ERR(skb)) { > + rtl_dev_err(hdev, "HCI_OP_READ_LOCAL_VERSION failed (%ld)\n", > + PTR_ERR(skb)); > + return skb; > + } > + > + if (skb->len != sizeof(struct hci_rp_read_local_version)) { > + rtl_dev_err(hdev, "HCI_OP_READ_LOCAL_VERSION event length > mismatch\n"); > + kfree_skb(skb); > + return ERR_PTR(-EIO); > + } > + > + return skb; > +} > + > static int rtl_read_rom_version(struct hci_dev *hdev, u8 *version) > { > struct rtl_rom_version_evt *rom_version; > @@ -368,6 +389,8 @@ static int rtl_download_firmware(struct hci_dev *hdev, > int frag_len = RTL_FRAG_LEN; > int ret = 0; > int i; > + struct sk_buff *skb; > + struct hci_rp_read_local_version *rp; > > dl_cmd = kmalloc(sizeof(struct rtl_download_cmd), GFP_KERNEL); > if (!dl_cmd) > @@ -406,6 +429,18 @@ static int rtl_download_firmware(struct hci_dev *hdev, > data += RTL_FRAG_LEN; > } > > + skb = btrtl_read_local_version(hdev); > + if (IS_ERR(skb)) { > + ret = PTR_ERR(skb); > + rtl_dev_err(hdev, "read local version failed"); > + goto out; > + } > + > + rp = (struct hci_rp_read_local_version *)skb->data; > + rtl_dev_info(hdev, "rtl: fw version 0x%04x%04x", > + __le16_to_cpu(rp->hci_rev), __le16_to_cpu(rp->lmp_subver)); the rtl: prefix in the string is pointless. The rtl_dev_info already does that. So please remove that. And then send a patch that removes all the others from the rtl_dev_info users as well. Regards Marcel
Re: [PATCH] Makefile: Convert -Wimplicit-fallthrough to -Wimplicit-fallthrough=2
On 8/29/19 7:02 AM, Michal Suchanek wrote: > From gcc documentation: > > -Wimplicit-fallthrough=0 > disables the warning altogether. > -Wimplicit-fallthrough=1 > matches .* regular expression, any comment is used as fallthrough comment. > -Wimplicit-fallthrough=2 > case insensitively matches .*falls?[ \t-]*thr(ough|u).* regular expression. > -Wimplicit-fallthrough=3 > case sensitively matches one of the following regular expressions: >-fallthrough >@fallthrough@ >lint -fallthrough[ \t]* >[ \t.!]*(ELSE,? |INTENTIONAL(LY)? )? >FALL(S | |-)?THR(OUGH|U)[ \t.!]*(-[^\n\r]*)? >[ \t.!]*(Else,? |Intentional(ly)? )? >Fall((s | |-)[Tt]|t)hr(ough|u)[ \t.!]*(-[^\n\r]*)? >[ \t.!]*([Ee]lse,? |[Ii]ntentional(ly)? )? >fall(s | |-)?thr(ough|u)[ \t.!]*(-[^\n\r]*)? > -Wimplicit-fallthrough=4 > case sensitively matches one of the following regular expressions: >-fallthrough >@fallthrough@ >lint -fallthrough[ \t]* >[ \t]*FALLTHR(OUGH|U)[ \t]* > -Wimplicit-fallthrough=5 > doesn’t recognize any comments as fallthrough comments, only attributes > disable the warning. > > In particular the default value of 3 does not match the comments like > /* falls through to do foobar */ > generating suprious warnings on properly annotated code. > NACK How many of those case do you see? if any... In any case, those comments can be easily transformed to: /* falls through - to do foobar */ like in this case: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/commit/?id=d2d833e0bf2bad221a955626b942b38312630894 Also, notice that code in linux-next is already ahead... : https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/commit/?id=e2079e93f562c7f7a030eb7642017ee5eabaaa10 -- Gustavo
[PATCH v4] tpm: Parse event log from TPM2 ACPI table
For systems with a TPM2 chip which use ACPI to expose event logs, retrieve the crypto-agile event log from the TPM2 ACPI table. The TPM2 table is defined in section 7.3 of the TCG ACPI Specification (see link). The TPM2 table is used by SeaBIOS in place of the TCPA table when the system's TPM is version 2.0 to denote (among other metadata) the location of the crypto-agile log. Link: https://trustedcomputinggroup.org/resource/tcg-acpi-specification/ Signed-off-by: Jordan Hand --- drivers/char/tpm/eventlog/acpi.c | 60 ++-- 1 file changed, 41 insertions(+), 19 deletions(-) diff --git a/drivers/char/tpm/eventlog/acpi.c b/drivers/char/tpm/eventlog/acpi.c index 63ada5e53f13..38a8bcec1dd5 100644 --- a/drivers/char/tpm/eventlog/acpi.c +++ b/drivers/char/tpm/eventlog/acpi.c @@ -41,17 +41,23 @@ struct acpi_tcpa { }; }; +/* If an event log is present, the TPM2 ACPI table will contain the full + * trailer + */ + /* read binary bios log */ int tpm_read_log_acpi(struct tpm_chip *chip) { - struct acpi_tcpa *buff; + struct acpi_table_header *buff; + struct acpi_tcpa *tcpa; + struct acpi_tpm2_trailer *tpm2_trailer; acpi_status status; void __iomem *virt; u64 len, start; + int log_type; struct tpm_bios_log *log; - - if (chip->flags & TPM_CHIP_FLAG_TPM2) - return -ENODEV; + bool is_tpm2 = chip->flags & TPM_CHIP_FLAG_TPM2; + acpi_string table_sig; log = >log; @@ -61,26 +67,42 @@ int tpm_read_log_acpi(struct tpm_chip *chip) if (!chip->acpi_dev_handle) return -ENODEV; - /* Find TCPA entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ - status = acpi_get_table(ACPI_SIG_TCPA, 1, - (struct acpi_table_header **)); + /* Find TCPA or TPM2 entry in RSDT (ACPI_LOGICAL_ADDRESSING) */ + table_sig = is_tpm2 ? ACPI_SIG_TPM2 : ACPI_SIG_TCPA; + status = acpi_get_table(table_sig, 1, ); if (ACPI_FAILURE(status)) return -ENODEV; - switch(buff->platform_class) { - case BIOS_SERVER: - len = buff->server.log_max_len; - start = buff->server.log_start_addr; - break; - case BIOS_CLIENT: - default: - len = buff->client.log_max_len; - start = buff->client.log_start_addr; - break; + if (!is_tpm2) { + tcpa = (struct acpi_tcpa *)buff; + switch (tcpa->platform_class) { + case BIOS_SERVER: + len = tcpa->server.log_max_len; + start = tcpa->server.log_start_addr; + break; + case BIOS_CLIENT: + default: + len = tcpa->client.log_max_len; + start = tcpa->client.log_start_addr; + break; + } + log_type = EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + } else if (buff->length == + sizeof(struct acpi_table_tpm2) + + sizeof(struct acpi_tpm2_trailer)) { + tpm2_trailer = (struct acpi_tpm2_trailer *)buff; + + len = tpm2_trailer.minimum_log_length; + start = tpm2_trailer.log_address; + log_type = EFI_TCG2_EVENT_LOG_FORMAT_TCG_2; + } else { + return -ENODEV; } + if (!len) { - dev_warn(>dev, "%s: TCPA log area empty\n", __func__); + dev_warn(>dev, "%s: %s log area empty\n", +__func__, table_sig); return -EIO; } @@ -98,7 +120,7 @@ int tpm_read_log_acpi(struct tpm_chip *chip) memcpy_fromio(log->bios_event_log, virt, len); acpi_os_unmap_iomem(virt, len); - return EFI_TCG2_EVENT_LOG_FORMAT_TCG_1_2; + return log_type; err: kfree(log->bios_event_log); -- 2.20.1
RE: [PATCH v3 12/12] Drivers: hv: vmbus: Resume after fixing up old primary channels
> From: Dexuan Cui > Sent: Friday, August 30, 2019 9:37 PM > > Is the intent to proceed and use the new offer? > Yes, since this is not an error. > > I'll add a comment before the "Mismatched offer from the host" for this. Hi Michael, I'm going to make the below change in v4. --- a/drivers/hv/channel_mgmt.c +++ b/drivers/hv/channel_mgmt.c @@ -956,7 +956,13 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr) return; } - pr_debug("Mismatched offer from the host (relid=%d)\n", + /* +* This is not an error, since the host can also change the +* other field(s) of the offer, e.g. on WS RS5 (Build 17763), +* the offer->connection_id of the Mellanox VF vmbus device +* can change when the host reoffers the device upon resume. +*/ + pr_debug("vmbus offer changed: relid=%d\n", offer->child_relid); print_hex_dump_debug("Old vmbus offer: ", DUMP_PREFIX_OFFSET, @@ -965,6 +971,7 @@ static void vmbus_onoffer(struct vmbus_channel_message_header *hdr) print_hex_dump_debug("New vmbus offer: ", DUMP_PREFIX_OFFSET, 16, 4, offer, offer_sz, false); + /* Fix up the old channel. */ vmbus_setup_channel_state(oldchannel, offer); check_ready_for_resume_event(); Thanks, -- Dexuan
Re: Adding depends-on DT binding to break cyclic dependencies
On Fri, Aug 30, 2019 at 5:32 PM Saravana Kannan wrote: > > On Fri, Aug 30, 2019 at 7:35 AM Rob Herring wrote: > > > > On Thu, Aug 29, 2019 at 11:58 PM Saravana Kannan > > wrote: > > > > > > On Thu, Aug 29, 2019 at 9:28 AM Rob Herring wrote: > > > > > > > > On Thu, Aug 22, 2019 at 1:55 AM Saravana Kannan > > > > wrote: > > > > > > > > > > Hi Rob, > > > > > > > > > > Frank, Greg and I got together during ELC and had an extensive and > > > > > very productive discussion about my "postboot supplier state cleanup" > > > > > patch series [1]. The three of us are on the same page now -- the > > > > > series as it stands is the direction we want to go in, with some minor > > > > > refactoring, documentation and naming changes. > > > > > > > > > > However, one of the things Frank is concerned about (and Greg and I > > > > > agree) in the current patch series is that the "cyclic dependency > > > > > breaking" logic has been pushed off to individual drivers using the > > > > > edit_links() callback. > > > > > > > > I would think the core can detect this condition. There's nothing > > > > device specific once you have the dependency tree. You still need to > > > > know what device needs to probe first and the drivers are going to > > > > have that knowledge anyways. So wouldn't it be enough to allow probe > > > > to proceed for devices in the loop. > > > > > > The problem is that device links don't allow creating a cyclic > > > dependency graph -- for good reasons. The last link in the cycle would > > > be rejected. I don't think trying to change device link to allow > > > cyclic links is the right direction to go in nor is it a can of worms > > > I want to open. > > > > > > So, we'll need some other way of tracking the loop and then allowing > > > only those devices in a loop to attempt probing even if their > > > suppliers haven't probed. And then if a device ends up being in more > > > than one loop, things could get even more complicated. And after one > > > of the devices in the loop probes, we still need to somehow figure out > > > the "bad" link and delete it so the last "good" link can be made > > > before all the suppliers have their sync_state() called (because the > > > "good" link hasn't been created yet). That all gets pretty messy. If > > > we are never going to accept any DT changes, then I'd rather go with > > > edit_links() and keep the complexity within the one off weird hardware > > > where there are cycles instead of over complicating the driver core. > > > > If it is so complex, then it should not be in DT. > > I'm talking about existing simple DT bindings that says what > clocks/interconnects/regulators a device needs. Not sure what you mean > by "it should not be in DT". > > > Things like > > describing clocks at the gate/mux/divider level turned out to be too > > complex to get right or complete, so we avoid that. > > Right, and this patch series has nothing to do with describing complex > internals of a device. > > > > > > > Once 1 driver succeeds, then you > > > > can enforce the dependencies on the rest. > > > > > > > > > The concern being, there are going to be multiple device specific ad > > > > > hoc implementations to break a cyclic dependency. Also, if a device > > > > > can be part of a cyclic dependency, the driver for that device has to > > > > > check for specific system/products in which the device is part of a > > > > > cyclic dependency (because it might not always be part of a cycle), > > > > > and then potentially have cycle/product specific code to break the > > > > > cycle (since the cycle can be different on each system/product). > > > > > > > > > > One way to avoid all of the device/driver specific code and simplify > > > > > my patch series by a non-trivial amount would be by adding a > > > > > "depends-on" DT binding that can ONLY be used to break cycles. We can > > > > > document it as such and reject any attempts to use it for other > > > > > purposes. When a depends-on property is present in a device node, that > > > > > specific device's supplier list will be parsed ONLY from the > > > > > depends-on property and the other properties won't be parsed for > > > > > deriving dependency information for that device. > > > > > > > > Seems like only ignoring the dependencies with a cycle would be > > > > sufficient. > > > > > > No, we need to only ignore the "bad" dependency. We can't ignore all > > > the dependencies in the cycle because that would cause the suppliers > > > to clean up the hardware state before the consumers are ready. > > > > I misunderstood. I now see this is back to duplicating all the data > > that's already there which I've already said no to. > > What I proposed earlier was using depends-on for all devices. And I'm > glad you rejected it because I like my current set of patches better > than the earlier one. The question here is whether we can allow this > for the rare occasions where there is a cycle. Frank, Greg and I think > it was a
Re: [PATCH 2/9] perf/core: Add PERF_SAMPLE_CGROUP feature
Hello, On Sat, Aug 31, 2019 at 12:03:26PM +0900, Namhyung Kim wrote: > Hmm.. it looks hard to use fhandle as the identifier since perf > sampling is done in NMI context. AFAICS the encode_fh part seems ok > but getting dentry/inode from a kernfs_node seems not. > > I assume kernfs_node_id's ino and gen are same to its inode's. Then > we might use kernfs_node for encoding but not sure you like it ;-) Oh yeah, the whole cgroup id situation is kinda shitty and it's likely that it needs to be cleaned up a bit for this to be used widely. The issues are... * As identifiers, paths sucks. It's too big and unwieldy and can be rapidly reused for different instances. * ino is compact but can't be easily mapped to path from userland and also not unique. * The fhandle identifier - currently ino+gen - is better in that it's finite sized and compact and can be efficiently mapped to path from userspace. It's also mostly unique. However, the way gen is currently generated still has some chance of the same ID getting reused and it isn't easily accessible from inside the kernel right now. Eventually, where we wanna be at is having a single 64bit identifier which can be easily used everywhere. It should be pretty straight forward on 64bit machines - we can just use monotonically increasing id and use it for everything - ino, fhandle and internal cgroup id. On 32bit, it gets a bit complicated because ino is 32bit, so it'll need a custom allocator which bumps gen when the lower 32bit wraps and skips in-use inos. Once we have that, we can use that for cgrp->id and fhandle and derive ino from it. This is on the to-do list but obviously hasn't happened yet. If you wanna take on it, great, but, otherwise, what can be done now is either moving gen+ino generation into cgroup and tell kernfs to use it or copy gen+ino into cgroup for easier access. The former likely is the better approach given that it brings us closer to where we wanna be eventually. Thanks. -- tejun
Re: Oops (request_key_auth_describe) while running cve-2016-7042 from LTP
> On 30-Aug-2019, at 8:43 PM, David Howells wrote: > > Can you try this patch instead of Hillf’s? Works for me. Test ran fine without any problem. Tested-by: Sachin Sant Thanks -Sachin
Re: [PATCH v5 3/3] vfio/pci: make use of irq_update_devid and optimize irq ops
在 2019/8/31 上午4:06, Alex Williamson 写道: On Fri, 30 Aug 2019 16:42:06 +0800 Ben Luo wrote: When userspace (e.g. qemu) triggers a switch between KVM irqfd and userspace eventfd, only dev_id of irqaction (i.e. the "trigger" in this patch's context) will be changed, but a free-then-request-irq action is taken in current code. And, irq affinity setting in VM will also trigger a free-then-request-irq action, which actually changes nothing, but only need to bounce the irqbypass registraion in case that posted-interrupt is in use. This patch makes use of irq_update_devid() and optimize both cases above, which reduces the risk of losing interrupt and also cuts some overhead. Signed-off-by: Ben Luo --- drivers/vfio/pci/vfio_pci_intrs.c | 124 ++ 1 file changed, 87 insertions(+), 37 deletions(-) diff --git a/drivers/vfio/pci/vfio_pci_intrs.c b/drivers/vfio/pci/vfio_pci_intrs.c index 3fa3f72..d3a93d7 100644 --- a/drivers/vfio/pci/vfio_pci_intrs.c +++ b/drivers/vfio/pci/vfio_pci_intrs.c @@ -284,70 +284,120 @@ static int vfio_msi_enable(struct vfio_pci_device *vdev, int nvec, bool msix) static int vfio_msi_set_vector_signal(struct vfio_pci_device *vdev, int vector, int fd, bool msix) { + struct eventfd_ctx *trigger = NULL; struct pci_dev *pdev = vdev->pdev; - struct eventfd_ctx *trigger; int irq, ret; if (vector < 0 || vector >= vdev->num_ctx) return -EINVAL; + if (fd >= 0) { + trigger = eventfd_ctx_fdget(fd); + if (IS_ERR(trigger)) { + /* oops, going to disable this interrupt */ + dev_info(>dev, +"get ctx error on bad fd: %d for vector:%d\n", +fd, vector); I think a user could trigger this maliciously as a denial of service by simply providing a bogus file descriptor. The user is informed of the error by the return value, why do we need to spam the logs? Ah, you are right, I will remove this log in next version + } + } + irq = pci_irq_vector(pdev, vector); + /* +* 'trigger' is NULL or invalid, disable the interrupt +* 'trigger' is same as before, only bounce the bypass registration +* 'trigger' is a new invalid one, update it to irqaction and other s/invalid/valid/ sorry for typo :) +* data structures referencing to the old one; fallback to disable +* the interrupt on error +*/ if (vdev->ctx[vector].trigger) { - free_irq(irq, vdev->ctx[vector].trigger); + /* +* even if the trigger is unchanged we need to bounce the +* interrupt bypass connection to allow affinity changes in +* the guest to be realized. +*/ irq_bypass_unregister_producer(>ctx[vector].producer); - kfree(vdev->ctx[vector].name); - eventfd_ctx_put(vdev->ctx[vector].trigger); - vdev->ctx[vector].trigger = NULL; + + if (vdev->ctx[vector].trigger == trigger) { + /* avoid duplicated referencing to the same trigger */ + eventfd_ctx_put(trigger); + + } else if (trigger && !IS_ERR(trigger)) { + ret = irq_update_devid(irq, + vdev->ctx[vector].trigger, trigger); + if (unlikely(ret)) { + dev_info(>dev, +"update devid of %d (token %p) failed: %d\n", +irq, vdev->ctx[vector].trigger, ret); + eventfd_ctx_put(trigger); + free_irq(irq, vdev->ctx[vector].trigger); + kfree(vdev->ctx[vector].name); + eventfd_ctx_put(vdev->ctx[vector].trigger); + vdev->ctx[vector].trigger = NULL; + return ret; + } + eventfd_ctx_put(vdev->ctx[vector].trigger); + vdev->ctx[vector].producer.token = trigger; + vdev->ctx[vector].trigger = trigger; + + } else { + free_irq(irq, vdev->ctx[vector].trigger); + kfree(vdev->ctx[vector].name); + eventfd_ctx_put(vdev->ctx[vector].trigger); + vdev->ctx[vector].trigger = NULL; + } } if (fd < 0) return 0; + else if (IS_ERR(trigger)) + return PTR_ERR(trigger); - vdev->ctx[vector].name = kasprintf(GFP_KERNEL, "vfio-msi%s[%d](%s)", - msix ? "x" : "", vector, -
RE: [PATCH v3 12/12] Drivers: hv: vmbus: Resume after fixing up old primary channels
> From: Michael Kelley > Sent: Friday, August 23, 2019 1:25 PM > > From: Dexuan Cui Sent: Monday, August 19, 2019 6:52 PM > > @@ -890,6 +937,11 @@ static void vmbus_onoffer(struct > > vmbus_channel_message_header *hdr) > > false); > > print_hex_dump_debug("New vmbus offer: ", > DUMP_PREFIX_OFFSET, > > 16, 4, offer, offer_sz, false); > > + > > + vmbus_setup_channel_state(oldchannel, offer); > > + > > + check_ready_for_resume_event(); > > This is the error case where the new offer didn't match some aspect of > the old offer. Actually, this is not an error: besides the RELID, the host can also change the offer->connection_id when it re-offers a device to the guest: so far, I only see this host behavior for the VF vmbus device, and in this case, the first vmbus_setup_channel_state() in vmbus_onoffer() is used to do the fix-up: channel->sig_event = offer->connection_id; and later channel->sig_event is used in vmbus_set_event(). Despite the host behavior, it looks the VF vmbus device still works fine, so (IMO) this is not an error. I'll write a separate email to report this to Hyper-V team. > Is the intent to proceed and use the new offer? Yes, since this is not an error. I'll add a comment before the "Mismatched offer from the host" for this. BTW, the 3 debug lines here output nothing, unless we enable the output by cd /sys/kernel/debug/dynamic_debug/ echo 'file drivers/hv/channel_mgmt.c +p' > control . > I can see that check_ready_for_resume_event() has to be called in > the error case, otherwise the resume operation will hang forever, but > I'm not sure about setting up the channel state and then proceeding as > if all is good. > > + > > return; Thanks, -- Dexuan
Re: linux-next: build failure after merge of the pci tree
On 8/30/2019 6:00 PM, Bjorn Helgaas wrote: [+cc Krzysztof] On Thu, Aug 29, 2019 at 10:23 PM Stephen Rothwell wrote: Hi all, After merging the pci tree, today's linux-next build (x86_64 allmodconfig) failed like this: drivers/pci/controller/dwc/pcie-tegra194.c:24:10: fatal error: linux/pci-aspm.h: No such file or directory 24 | #include | ^~ Caused by commit 81564976b1a9 ("PCI: tegra: Add Tegra194 PCIe support") I have reverted that commit for todat. Thanks, Stephen. I *could* fix this by removing that include in the merge, since the contents of linux/pci-aspm.h were moved into linux/pci.h by https://git.kernel.org/cgit/linux/kernel/git/helgaas/pci.git/commit/?id=7ce2e76a0420 But as far as I can tell, pcie-tegra194.c doesn't actually require anything from linux/pci-aspm.h, so I'd rather amend the tegra194 commit https://git.kernel.org/cgit/linux/kernel/git/lpieralisi/pci.git/commit/?id=81564976b1a9 so it doesn't include pci-aspm.h in the first place. Thanks Bjorn for the reply. Yes. This header file is not required for now and can be removed. Is there any action required from my side for this? Bjorn
Re: [PATCH] staging: exfat: add exfat filesystem code to staging
On 2019/8/30 19:51, David Sterba wrote: > On Fri, Aug 30, 2019 at 10:06:25AM +0800, Chao Yu wrote: >> On 2019/8/29 23:43, Dan Carpenter wrote: p.s. There are 2947 (un)likely places in fs/ directory. >>> >>> I was complaining about you adding new pointless ones, not existing >>> ones. The likely/unlikely annotations are supposed to be functional and >>> not decorative. I explained this very clearly. >>> >>> Probably most of the annotations in fs/ are wrong but they are also >>> harmless except for the slight messiness. However there are definitely >>> some which are important so removing them all isn't a good idea. >> >> Hi Dan, >> >> Could you please pick up one positive example using likely and unlikely >> correctly? so we can follow the example, rather than removing them all >> blindly. > > Remove all of them and re-add with explanation if and how each is going > to make things better. If you can't reason about, prove by benchmarks or > point to inefficient asm code generated, then don't add them again. It seems the result of it is strongly related to arch and compiler, if we readd it after we just prove its gain only in one combination, I doubt we may suffer regression in other combination in between archs and comilers. It looks like we don't have any cheaper way to readd it? instead of verifying all/most combinations. > > The feedback I got from CPU and compiler people over the years is not to > bother using the annotations. CPUs are doing dynamic branch prediction > and compilers are applying tons of optimizations. > > GCC docs state about the builtin: "In general, you should prefer to use > actual profile feedback for this (-fprofile-arcs), as programmers are > notoriously bad at predicting how their programs actually perform." > (https://gcc.gnu.org/onlinedocs/gcc/Other-Builtins.html) Yes, I agree with this. Thanks a lot for sharing your experience. :) The removal change has done and been merged into Greg's tree, let's consider to readd it later if possible as you suggested. thanks, > . >
[PATCH v4] staging: rts5208: Fix checkpath warning
This patch fixes the following checkpath warning in the file drivers/staging/rts5208/rtsx_transport.c:546 WARNING: line over 80 characters + option = RTSX_SG_VALID | RTSX_SG_END | RTSX_SG_TRANS_DATA; Signed-off-by: P SAI PRASANTH --- Changes in v4: -Fix extra tab issue which was causing same blank line to be added and removed in git diff. drivers/staging/rts5208/rtsx_transport.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/drivers/staging/rts5208/rtsx_transport.c b/drivers/staging/rts5208/rtsx_transport.c index 8277d78..c1d99c4 100644 --- a/drivers/staging/rts5208/rtsx_transport.c +++ b/drivers/staging/rts5208/rtsx_transport.c @@ -541,10 +541,9 @@ static int rtsx_transfer_sglist_adma(struct rtsx_chip *chip, u8 card, dev_dbg(rtsx_dev(chip), "DMA addr: 0x%x, Len: 0x%x\n", (unsigned int)addr, len); + option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; if (j == (sg_cnt - 1)) - option = RTSX_SG_VALID | RTSX_SG_END | RTSX_SG_TRANS_DATA; - else - option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; + option |= RTSX_SG_END; rtsx_add_sg_tbl(chip, (u32)addr, (u32)len, option); -- 2.7.4
Re: [PATCH v3] staging: rts5208: Fix checkpath warning
On 19-08-30 19:58:09, Joe Perches wrote: > On Sat, 2019-08-31 at 07:55 +0530, P SAI PRASANTH wrote: > > This patch fixes the following checkpath warning > > in the file drivers/staging/rts5208/rtsx_transport.c:546 > > > > WARNING: line over 80 characters > > + option = RTSX_SG_VALID | RTSX_SG_END | > > RTSX_SG_TRANS_DATA; > [] > > diff --git a/drivers/staging/rts5208/rtsx_transport.c > > b/drivers/staging/rts5208/rtsx_transport.c > [] > > @@ -540,11 +540,10 @@ static int rtsx_transfer_sglist_adma(struct rtsx_chip > > *chip, u8 card, > > > > dev_dbg(rtsx_dev(chip), "DMA addr: 0x%x, Len: 0x%x\n", > > (unsigned int)addr, len); > > - > > + > > This same line delete then insert the same blank line > is very unusual. > > What did you use to create this patch? > > > + option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; > > if (j == (sg_cnt - 1)) > > - option = RTSX_SG_VALID | RTSX_SG_END | > > RTSX_SG_TRANS_DATA; > > - else > > - option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; > > + option |= RTSX_SG_END; > > > > rtsx_add_sg_tbl(chip, (u32)addr, (u32)len, option); > > > I have used vim for creating this patch. Upon checking, there is an extra tab on the new blankline which has been added. I'm sending an update immediately.
[PATCH vfs/for-next] vfs: fix vfs_get_single_reconf_super error handling
From: Eric Biggers syzbot reported an invalid free in debugfs_release_dentry(). The reproducer tries to mount debugfs with the 'dirsync' option, which is not allowed. The bug is that if reconfigure_super() fails in vfs_get_super(), deactivate_locked_super() is called, but also fs_context::root is left non-NULL which causes deactivate_super() to be called again later. Fix it by releasing fs_context::root in the error path. Reported-by: syzbot+5aca688dac0796c56...@syzkaller.appspotmail.com Fixes: e478b48498a7 ("vfs: Add a single-or-reconfig keying to vfs_get_super()") Cc: David Howells Signed-off-by: Eric Biggers --- fs/super.c | 5 - 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/fs/super.c b/fs/super.c index 0f913376fc4c..99195e15be05 100644 --- a/fs/super.c +++ b/fs/super.c @@ -1194,8 +1194,11 @@ int vfs_get_super(struct fs_context *fc, fc->root = dget(sb->s_root); if (keying == vfs_get_single_reconf_super) { err = reconfigure_super(fc); - if (err < 0) + if (err < 0) { + dput(fc->root); + fc->root = NULL; goto error; + } } } -- 2.23.0
[PATCH v2 2/2] touchscreen: goodix: define GPIO mapping for GPD P2 Max
The firmware of GPD P2 Max could not handle panel resets although code is present in DSDT. The kernel needs to take on this job instead, but the DSDT does not provide _DSD, rendering kernel helpless when trying to find the respective GPIO pins. Fortunately, this time GPD has proper DMI vendor / product strings that we could match against. We simply apply an acpi_gpio_mapping table when GPD P2 Max is matched. Additionally, the DSDT definition of the irq pin specifies a wrong polarity. The new quirk introduced in the previous patch (ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) is applied to correct this. Signed-off-by: Peter Cai --- v2: removed '#ifdef CONFIG_ACPI' as per suggestion. The #ifdef guards for CONFIG_DMI is kept for consistency with the other dmi_system_id definition in the same file. --- drivers/input/touchscreen/goodix.c | 31 ++ 1 file changed, 31 insertions(+) diff --git a/drivers/input/touchscreen/goodix.c b/drivers/input/touchscreen/goodix.c index 5178ea8b5f30..df476f7dcd95 100644 --- a/drivers/input/touchscreen/goodix.c +++ b/drivers/input/touchscreen/goodix.c @@ -144,6 +144,31 @@ static const struct dmi_system_id rotated_screen[] = { {} }; +static const struct acpi_gpio_params irq_gpios_default = { 0, 0, false }; +static const struct acpi_gpio_params reset_gpios_default = { 1, 0, false }; +static const struct acpi_gpio_mapping gpio_mapping_force_irq_active_high[] = { + { "irq-gpios", _gpios_default, 1, ACPI_GPIO_QUIRK_OVERRIDE_POLARITY }, + { "reset-gpios", _gpios_default, 1 }, + {} +}; + +/* + * Devices that need acpi_gpio_mapping to function correctly + */ +static const struct dmi_system_id need_gpio_mapping[] = { +#if defined(CONFIG_DMI) && defined(CONFIG_X86) + { + .ident = "GPD P2 Max", + .matches = { + DMI_MATCH(DMI_SYS_VENDOR, "GPD"), + DMI_MATCH(DMI_PRODUCT_NAME, "P2 MAX") + }, + .driver_data = _mapping_force_irq_active_high + }, +#endif + {} +}; + /** * goodix_i2c_read - read data from a register of the i2c slave device. * @@ -795,6 +820,12 @@ static int goodix_ts_probe(struct i2c_client *client, { struct goodix_ts_data *ts; int error; + struct dmi_system_id *dmi_match; + + dmi_match = dmi_first_match(need_gpio_mapping); + if (dmi_match) + devm_acpi_dev_add_driver_gpios(>dev, + dmi_match->driver_data); dev_dbg(>dev, "I2C Address: 0x%02x\n", client->addr); -- 2.23.0
[PATCH v2 1/2] gpio: acpi: add quirk to override GpioInt polarity
On GPD P2 Max, the firmware could not reset the touch panel correctly. The kernel needs to take on the job instead, but the GpioInt definition in DSDT specifies ActiveHigh while the GPIO pin should actually be ActiveLow. We need to override the polarity defined by DSDT. The GPIO driver already allows defining polarity in acpi_gpio_params, but the option is not applied to GpioInt. This patch adds a new quirk that enables the polarity specified in acpi_gpio_params to also be applied to GpioInt. Signed-off-by: Peter Cai --- v2: rebased to gpio/for-next, moved quirk out of the gpioint conditional. --- drivers/gpio/gpiolib-acpi.c | 9 + include/linux/gpio/consumer.h | 6 ++ 2 files changed, 15 insertions(+) diff --git a/drivers/gpio/gpiolib-acpi.c b/drivers/gpio/gpiolib-acpi.c index fdee8afa5339..ab16ea61a8fa 100644 --- a/drivers/gpio/gpiolib-acpi.c +++ b/drivers/gpio/gpiolib-acpi.c @@ -603,6 +603,15 @@ static int acpi_populate_gpio_lookup(struct acpi_resource *ares, void *data) lookup->info.flags = acpi_gpio_to_gpiod_flags(agpio); lookup->info.polarity = lookup->active_low; } + + /* +* Override the polarity specified by GpioInt if +* ACPI_GPIO_QUIRK_OVERRIDE_POLARITY is set. +*/ + if (lookup->info.quirks & ACPI_GPIO_QUIRK_OVERRIDE_POLARITY) { + dev_warn(>info.adev->dev, FW_BUG "Incorrect polarity specified by GpioInt, overriding.\n"); + lookup->info.polarity = lookup->active_low; + } } return 1; diff --git a/include/linux/gpio/consumer.h b/include/linux/gpio/consumer.h index b70af921c614..7e9f24ebb085 100644 --- a/include/linux/gpio/consumer.h +++ b/include/linux/gpio/consumer.h @@ -622,6 +622,12 @@ struct acpi_gpio_mapping { * get GpioIo type explicitly, this quirk may be used. */ #define ACPI_GPIO_QUIRK_ONLY_GPIOIOBIT(1) +/* + * Use the GPIO polarity (ActiveHigh / ActiveLow) from acpi_gpio_params + * for GpioInt as well. The default behavior is to use the one specified + * by GpioInt, which can be incorrect on some devices. + */ +#define ACPI_GPIO_QUIRK_OVERRIDE_POLARITY BIT(2) unsigned int quirks; }; -- 2.23.0
Re: [PATCH 2/9] perf/core: Add PERF_SAMPLE_CGROUP feature
Hi Tejun, On Wed, Aug 28, 2019 at 07:49:11AM -0700, Tejun Heo wrote: > On Wed, Aug 28, 2019 at 04:31:23PM +0900, Namhyung Kim wrote: > > @@ -958,6 +958,7 @@ struct perf_sample_data { > > u64 stack_user_size; > > > > u64 phys_addr; > > + u64 cgroup; > > Ditto, please use fhandle as the identifier. Hmm.. it looks hard to use fhandle as the identifier since perf sampling is done in NMI context. AFAICS the encode_fh part seems ok but getting dentry/inode from a kernfs_node seems not. I assume kernfs_node_id's ino and gen are same to its inode's. Then we might use kernfs_node for encoding but not sure you like it ;-) Thanks Namhyung
[PATCH 2/2] or1k: dts: Add ethoc device to SMP devicetree
This patch adds the ethoc device configuration to the OpenRISC basic SMP device tree config. This was tested with qemu. Signed-off-by: Stafford Horne --- arch/openrisc/boot/dts/simple_smp.dts | 6 ++ 1 file changed, 6 insertions(+) diff --git a/arch/openrisc/boot/dts/simple_smp.dts b/arch/openrisc/boot/dts/simple_smp.dts index defbb92714ec..71af0e117bfe 100644 --- a/arch/openrisc/boot/dts/simple_smp.dts +++ b/arch/openrisc/boot/dts/simple_smp.dts @@ -60,4 +60,10 @@ clock-frequency = <2000>; }; + enet0: ethoc@9200 { + compatible = "opencores,ethoc"; + reg = <0x9200 0x800>; + interrupts = <4>; + big-endian; + }; }; -- 2.21.0
[PATCH 0/2] OpenRISC ethoc device tree fixes
These patches fix up and add ethoc device support to the OpenRISC device tree definitions. These have been confirmed to work with qemu and can be tested as described on the qemu wiki: https://wiki.qemu.org/Documentation/Platforms/OpenRISC I plan to submit during the 5.4 merge window. Stafford Horne (2): or1k: dts: Fix ethoc network configuration in or1ksim devicetree or1k: dts: Add ethoc device to SMP devicetree arch/openrisc/boot/dts/or1ksim.dts| 5 +++-- arch/openrisc/boot/dts/simple_smp.dts | 6 ++ 2 files changed, 9 insertions(+), 2 deletions(-) -- 2.21.0
[PATCH 1/2] or1k: dts: Fix ethoc network configuration in or1ksim devicetree
This fixes several issues with the ethoc network device config. Fisrt off, the compatible property used an obsolete compatibility string; this caused the initialization to be skipped. Next, the register map was not given enough space to allocate ring descriptors, this caused module initialization to abort. Finally, we need to mark this device as big-endian as needed by openrisc. This was tested by me in qemu, the setup is documented on the qemu wiki: https://wiki.qemu.org/Documentation/Platforms/OpenRISC Signed-off-by: Stafford Horne --- arch/openrisc/boot/dts/or1ksim.dts | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/arch/openrisc/boot/dts/or1ksim.dts b/arch/openrisc/boot/dts/or1ksim.dts index d8aa8309c9d3..c0cb74e52f95 100644 --- a/arch/openrisc/boot/dts/or1ksim.dts +++ b/arch/openrisc/boot/dts/or1ksim.dts @@ -49,8 +49,9 @@ }; enet0: ethoc@9200 { - compatible = "opencores,ethmac-rtlsvn338"; - reg = <0x9200 0x100>; + compatible = "opencores,ethoc"; + reg = <0x9200 0x800>; interrupts = <4>; + big-endian; }; }; -- 2.21.0
Re: [PATCH v3] staging: rts5208: Fix checkpath warning
On Sat, 2019-08-31 at 07:55 +0530, P SAI PRASANTH wrote: > This patch fixes the following checkpath warning > in the file drivers/staging/rts5208/rtsx_transport.c:546 > > WARNING: line over 80 characters > + option = RTSX_SG_VALID | RTSX_SG_END | > RTSX_SG_TRANS_DATA; [] > diff --git a/drivers/staging/rts5208/rtsx_transport.c > b/drivers/staging/rts5208/rtsx_transport.c [] > @@ -540,11 +540,10 @@ static int rtsx_transfer_sglist_adma(struct rtsx_chip > *chip, u8 card, > > dev_dbg(rtsx_dev(chip), "DMA addr: 0x%x, Len: 0x%x\n", > (unsigned int)addr, len); > - > + This same line delete then insert the same blank line is very unusual. What did you use to create this patch? > + option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; > if (j == (sg_cnt - 1)) > - option = RTSX_SG_VALID | RTSX_SG_END | > RTSX_SG_TRANS_DATA; > - else > - option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; > + option |= RTSX_SG_END; > > rtsx_add_sg_tbl(chip, (u32)addr, (u32)len, option); >
RE: [PATCH v3 11/12] Drivers: hv: vmbus: Suspend after cleaning up hv_sock and sub channels
> From: Michael Kelley > Sent: Friday, August 23, 2019 1:17 PM > > From: Dexuan Cui Sent: Monday, August 19, 2019 6:52 PM > > > > diff --git a/drivers/hv/channel_mgmt.c b/drivers/hv/channel_mgmt.c > > @@ -499,6 +499,8 @@ static void vmbus_add_channel_work(struct > work_struct *work) > > return; > > > > err_deq_chan: > > + WARN_ON_ONCE(1); > > + > > Why this change? I was thinking maybe it's a debug statement that got > left in. This is indeed a debug statement. :-) I was not so sure if the error handling is absolutely correct there. I think I can remove this WARN_ON_ONCE() since almost all of the possible error paths have a pr_err(). We can make an extra patch to fix any bug in the existing error handling code. BTW, it should be pretty unlikely to reach the "err_deq_chan label" in practice. > > @@ -1021,6 +1044,11 @@ static void vmbus_onoffer_rescind(struct > > vmbus_channel_message_header *hdr) > > } > > mutex_unlock(_connection.channel_mutex); > > } > > + > > + /* The "channel" may have been freed. Do not access it any longer. */ > > + > > + if (clean_up_chan_for_suspend) > > + check_ready_for_suspend_event(); > > } > > Having to add the above lines twice is a bit clumsy, but the problem is > the overall structure of the vmbus_onoffer_rescind. The early return in > the case of a rescind_callback function is a bit weird, but I guess it makes > sense since from what I can tell, only uio and hv_sock have rescind callback > functions. Some minor restructuring might be warranted, but I don't feel > strongly about it. I agree. We should restructure vmbus_onoffer_rescind(), but I think we can do it in a separate patch. Here in this patch I'm focused on the minimal required change for hibernation. > > + /* > > +* Wait until all the sub-channels and hv_sock channels have been > > +* cleaned up. Sub-channels should be destroyed upon suspend, > otherwise > > +* they would conflict with the new sub-channels that will be created > > +* in the resume path. hv_sock channels should also be destroyed, but > > +* a hv_sock channel of an established hv_sock connection can not be > > +* really destroyed since it may still be referenced by the userspace > > +* application, so we just force the hv_sock channel to be rescinded > > +* by vmbus_force_channel_rescinded(), and the userspace application > > +* will thoroughly destroy the channel after hibernation. > > +*/ > > + if (atomic_read(_connection.nr_chan_close_on_suspend) > 0) > > At first glance, the above line seemed useless to me. You could just do the > wait_for_completion() unconditionally. But is the intent to handle the case > where the VM never had any sub-channels or hv_sock channels, and so > nr_chan_close_on_suspend never went above 0? Yes. > If so, a comment might be helpful. > > +wait_for_completion(_connection.ready_for_suspend_event); Ok. I'll add a commnt for this in v4. Thanks, -- Dexuan
Re: [PATCH v3] libata/ahci: Drop PCS quirk for Denverton and beyond
On 8/29/19 5:30 PM, Dan Williams wrote: > The Linux ahci driver has historically implemented a configuration fixup > for platforms / platform-firmware that fails to enable the ports prior > to OS hand-off at boot. The fixup was originally implemented way back > before ahci moved from drivers/scsi/ to drivers/ata/, and was updated in > 2007 via commit 49f290903935 "ahci: update PCS programming". The quirk > sets a port-enable bitmap in the PCS register at offset 0x92. > > This quirk could be applied generically up until the arrival of the > Denverton (DNV) platform. The DNV AHCI controller architecture supports > more than 6 ports and along with that the PCS register location and > format were updated to allow for more possible ports in the bitmap. DNV > AHCI expands the register to 32-bits and moves it to offset 0x94. > > As it stands there are no known problem reports with existing Linux > trying to set bits at offset 0x92 which indicates that the quirk is not > applicable. Likely it is not applicable on a wider range of platforms, > but it is difficult to discern which platforms if any still depend on > the quirk. > > Rather than try to fix the PCS quirk to consider the DNV register layout > instead require explicit opt-in. The assumption is that the OS driver > need not touch this register, and platforms can be added with a new > boad_ahci_pcs7 board-id when / if problematic platforms are found in the > future. The logic in ahci_intel_pcs_quirk() looks for all Intel AHCI > instances with "legacy" board-ids and otherwise skips the quirk if the > board was matched by class-code. Applied, thanks Dan. -- Jens Axboe
Re: [PATCH] riscv: move sifive_l2_cache.c to drivers/soc
Hi Mauro, On Thu, 22 Aug 2019, Mauro Carvalho Chehab wrote: > I'm wandering if we should at least add an entry for this one at > MAINTAINERS, pointing it to the EDAC mailing list. Something like: > > diff --git a/MAINTAINERS b/MAINTAINERS > index 7dfe381c8b43..1c3bc5aa3af0 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -5906,6 +5906,7 @@ M: Yash Shah > L: linux-e...@vger.kernel.org > S: Supported > F: drivers/edac/sifive_edac.c > +F: drivers/soc/sifive/ > > EDAC-SKYLAKE > M: Tony Luck There's already a MAINTAINERS entry that should cover drivers/soc/sifive. Probably it's not needed to add another one here. - Paul
Re: [PATCH v4 0/3] Optimize tlbflush path
Hi Atish, On Thu, 22 Aug 2019, Atish Patra wrote: > This series adds few optimizations to reduce the trap cost in the tlb > flush path. We should only make SBI calls to remote tlb flush only if > absolutely required. The patches look great. My understanding is that these optimization patches may actually be a partial workaround for the TLB flushing bug that we've been looking at for the last month or so, which can corrupt memory or crash the system. If that's the case, let's first root-cause the underlying bug. Otherwise we'll just be papering over the actual issue, which probably could still occur even with this series, correct? Since it contains no explicit fixes? - Paul
Re: [PATCH v3 03/11] net/mlx5e: Remove unlikely() from WARN*() condition
From: Saeed Mahameed Date: Thu, 29 Aug 2019 21:23:30 + > On Thu, 2019-08-29 at 19:50 +0300, Denis Efremov wrote: >> "unlikely(WARN_ON_ONCE(x))" is excessive. WARN_ON_ONCE() already uses >> unlikely() internally. >> >> Signed-off-by: Denis Efremov >> Cc: Boris Pismenny >> Cc: Saeed Mahameed >> Cc: Leon Romanovsky >> Cc: Joe Perches >> Cc: Andrew Morton >> Cc: net...@vger.kernel.org >> --- >> drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git >> a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c >> b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c >> index 7833ddef0427..e5222d17df35 100644 >> --- a/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c >> +++ b/drivers/net/ethernet/mellanox/mlx5/core/en_accel/ktls_tx.c >> @@ -408,7 +408,7 @@ struct sk_buff *mlx5e_ktls_handle_tx_skb(struct >> net_device *netdev, >> goto out; >> >> tls_ctx = tls_get_ctx(skb->sk); >> -if (unlikely(WARN_ON_ONCE(tls_ctx->netdev != netdev))) >> +if (WARN_ON_ONCE(tls_ctx->netdev != netdev)) >> goto err_out; >> >> priv_tx = mlx5e_get_ktls_tx_priv_ctx(tls_ctx); > > Acked-by: Saeed Mahameed > > Dave, you can take this one. Ok, applied to net-next as well as the UDP patch.
[GIT PULL] RISC-V updates for v5.3-rc7
Linus, The following changes since commit a55aa89aab90fae7c815b0551b07be37db359d76: Linux 5.3-rc6 (2019-08-25 12:01:23 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/riscv/linux.git tags/riscv/for-v5.3-rc7 for you to fetch changes up to a256f2e329df0773022d28df2c3d206b9aaf1e61: RISC-V: Fix FIXMAP area corruption on RV32 systems (2019-08-28 15:30:12 -0700) RISC-V updates for v5.3-rc7 One significant fix for 32-bit RISC-V systems: - Fix the RV32 memory map to prevent userspace from corrupting the FIXMAP area. Without this patch, the system can crash very early during the boot. Anup Patel (1): RISC-V: Fix FIXMAP area corruption on RV32 systems arch/riscv/include/asm/fixmap.h | 4 arch/riscv/include/asm/pgtable.h | 12 ++-- 2 files changed, 10 insertions(+), 6 deletions(-)
[GIT PULL] KVM fixes for Linux 5.3-rc7
Linus, The following changes since commit a55aa89aab90fae7c815b0551b07be37db359d76: Linux 5.3-rc6 (2019-08-25 12:01:23 -0700) are available in the Git repository at: git://git.kernel.org/pub/scm/virt/kvm/kvm tags/for-linus for you to fetch changes up to 75ee23b30dc712d80d2421a9a547e7ab6e379b44: KVM: x86: Don't update RIP or do single-step on faulting emulation (2019-08-27 20:59:04 +0200) KVM fixes for 5.3-rc7 PPC: - Fix bug which could leave locks locked in the host on return to a guest. x86: - Prevent infinitely looping emulation of a failing syscall while single stepping. - Do not crash the host when nesting is disabled. Alexey Kardashevskiy (1): KVM: PPC: Book3S: Fix incorrect guest-to-user-translation error handling Radim Krčmář (1): Merge tag 'kvm-ppc-fixes-5.3-1' of git://git.kernel.org/.../paulus/powerpc Sean Christopherson (1): KVM: x86: Don't update RIP or do single-step on faulting emulation Vitaly Kuznetsov (1): KVM: x86: hyper-v: don't crash on KVM_GET_SUPPORTED_HV_CPUID when kvm_intel.nested is disabled arch/powerpc/kvm/book3s_64_vio.c| 6 -- arch/powerpc/kvm/book3s_64_vio_hv.c | 6 -- arch/x86/kvm/hyperv.c | 5 - arch/x86/kvm/svm.c | 8 +--- arch/x86/kvm/vmx/vmx.c | 1 + arch/x86/kvm/x86.c | 9 + 6 files changed, 19 insertions(+), 16 deletions(-)
[PATCH v3] staging: rts5208: Fix checkpath warning
This patch fixes the following checkpath warning in the file drivers/staging/rts5208/rtsx_transport.c:546 WARNING: line over 80 characters + option = RTSX_SG_VALID | RTSX_SG_END | RTSX_SG_TRANS_DATA; Signed-off-by: P SAI PRASANTH --- Changes in v3: -Fixes the following error in da59abd45efc >> drivers/staging//rts5208/rtsx_transport.c:548:4: error: expected ';' before 'rtsx_add_sg_tbl' rtsx_add_sg_tbl(chip, (u32)addr, (u32)len, option); ^~~ drivers/staging/rts5208/rtsx_transport.c | 7 +++ 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/drivers/staging/rts5208/rtsx_transport.c b/drivers/staging/rts5208/rtsx_transport.c index 8277d78..3c67656 100644 --- a/drivers/staging/rts5208/rtsx_transport.c +++ b/drivers/staging/rts5208/rtsx_transport.c @@ -540,11 +540,10 @@ static int rtsx_transfer_sglist_adma(struct rtsx_chip *chip, u8 card, dev_dbg(rtsx_dev(chip), "DMA addr: 0x%x, Len: 0x%x\n", (unsigned int)addr, len); - + + option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; if (j == (sg_cnt - 1)) - option = RTSX_SG_VALID | RTSX_SG_END | RTSX_SG_TRANS_DATA; - else - option = RTSX_SG_VALID | RTSX_SG_TRANS_DATA; + option |= RTSX_SG_END; rtsx_add_sg_tbl(chip, (u32)addr, (u32)len, option); -- 2.7.4
[RFC PATCH v2] tools/power turbostat: Add support for Hygon Fam 18h (Dhyana) RAPL
Commit 9392bd98bba760be96ee ("tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL") and the commit 3316f99a9f1b68c578c5 ("tools/power turbostat: Also read package power on AMD F17h (Zen)") add AMD Fam 17h RAPL support. Hygon Family 18h(Dhyana) support RAPL in bit 14 of CPUID 0x8007 EDX, and has MSRs RAPL_PWR_UNIT/CORE_ENERGY_STAT/PKG_ENERGY_STAT. So add Hygon Dhyana Family 18h support for RAPL. Already tested on Hygon multi-node systems and it shows correct per-core energy usage and the total package power. Signed-off-by: Pu Wen Reviewed-by: Calvin Walton --- tools/power/x86/turbostat/turbostat.c | 9 +++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 1cd28eb..d42f3f5 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -60,6 +60,7 @@ unsigned int do_irtl_hsw; unsigned int units = 100; /* MHz etc */ unsigned int genuine_intel; unsigned int authentic_amd; +unsigned int hygon_genuine; unsigned int max_level, max_extended_level; unsigned int has_invariant_tsc; unsigned int do_nhm_platform_info; @@ -1714,7 +1715,7 @@ void get_apic_id(struct thread_data *t) if (!DO_BIC(BIC_X2APIC)) return; - if (authentic_amd) { + if (authentic_amd || hygon_genuine) { unsigned int topology_extensions; if (max_extended_level < 0x801e) @@ -3803,6 +3804,7 @@ double get_tdp_amd(unsigned int family) { switch (family) { case 0x17: + case 0x18: default: /* This is the max stock TDP of HEDT/Server Fam17h chips */ return 250.0; @@ -3982,6 +3984,7 @@ void rapl_probe_amd(unsigned int family, unsigned int model) switch (family) { case 0x17: /* Zen, Zen+ */ + case 0x18: /* Hygon Dhyana */ do_rapl = RAPL_AMD_F17H | RAPL_PER_CORE_ENERGY; if (rapl_joules) { BIC_PRESENT(BIC_Pkg_J); @@ -4018,7 +4021,7 @@ void rapl_probe(unsigned int family, unsigned int model) { if (genuine_intel) rapl_probe_intel(family, model); - if (authentic_amd) + if (authentic_amd || hygon_genuine) rapl_probe_amd(family, model); } @@ -4600,6 +4603,8 @@ void process_cpuid() genuine_intel = 1; else if (ebx == 0x68747541 && ecx == 0x444d4163 && edx == 0x69746e65) authentic_amd = 1; + else if (ebx == 0x6f677948 && ecx == 0x656e6975 && edx == 0x6e65476e) + hygon_genuine = 1; if (!quiet) fprintf(outf, "CPUID(0): %.4s%.4s%.4s ", -- 2.7.4
[RFC PATCH v2] tools/power turbostat: Fix caller parameter of get_tdp_amd()
Commit 9392bd98bba760be96ee ("tools/power turbostat: Add support for AMD Fam 17h (Zen) RAPL") add a function get_tdp_amd(), the parameter is CPU family. But the rapl_probe_amd() function use wrong model parameter. Fix the wrong caller parameter of get_tdp_amd() to use family. Cc: # v5.1+ Signed-off-by: Pu Wen Reviewed-by: Calvin Walton --- tools/power/x86/turbostat/turbostat.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/power/x86/turbostat/turbostat.c b/tools/power/x86/turbostat/turbostat.c index 75fc4fb..1cd28eb 100644 --- a/tools/power/x86/turbostat/turbostat.c +++ b/tools/power/x86/turbostat/turbostat.c @@ -4002,7 +4002,7 @@ void rapl_probe_amd(unsigned int family, unsigned int model) rapl_energy_units = ldexp(1.0, -(msr >> 8 & 0x1f)); rapl_power_units = ldexp(1.0, -(msr & 0xf)); - tdp = get_tdp_amd(model); + tdp = get_tdp_amd(family); rapl_joule_counter_range = 0x * rapl_energy_units / tdp; if (!quiet) -- 2.7.4
Re: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
From: Christoph Paasch Date: Fri, 30 Aug 2019 16:26:57 -0700 > (I don't see it in the stable-queue) I don't handle any stable branch before the most recent two, so this isn't my territory.
Re: [PATCH] riscv: kbuild: drop CONFIG_RISCV_ISA_C
Hi Charles, On Tue, 13 Aug 2019, Charles Papon wrote: > > Because it it the unix platform baseline as stated in the patch. > I know that, but i'm looking for arguments why RVC could't be kept as > an option, especialy it is only an optimisation option without > behavioral/code changes. > > That baseline make sense for heavy linux distributions, where you > expect everybody to compile with a baseline set of ISA extentions, to > make binary exchanges easier. > But for smaller systems, i do not see advantages having RVC forced. OK - I agree with you. Still, I think it would be good if we made this option depend on other more general kernel configuration parameters for smaller systems. Will think about this further. Thanks for commenting on this, and am looking forward to adding a VexRiscv system to our kernel tests - - Paul
RE: [PATCH v3 10/12] Drivers: hv: vmbus: Clean up hv_sock channels by force upon suspend
> From: Michael Kelley > Sent: Friday, August 23, 2019 1:02 PM > > From: Dexuan Cui Sent: Monday, August 19, 2019 6:52 PM > > > > Fake RESCIND_CHANNEL messages to clean up hv_sock channels by force for > > hibernation. There is no better method to clean up the channels since > > some of the channels may still be referenced by the userspace apps when > > hiberantin is triggered: in this case, the "rescind" fields of the > > s/hiberantin/hibernation/ Thanks! Will fix this in v4. > > @@ -2091,6 +2127,25 @@ static int vmbus_acpi_add(struct acpi_device > *device) > > > > static int vmbus_bus_suspend(struct device *dev) > > { > > + struct vmbus_channel *channel; > > + > > + while (atomic_read(_connection.offer_in_progress) != 0) { > > + /* > > +* We wait here until any channel offer is currently > > +* being processed. > > +*/ > > The wording of the comment is a bit off. Maybe > > /* >* We wait here until the completion of any channel >* offers that are currently in progress. >*/ Will use the better version this in v4. Thanks, -- Dexuan
[PATCH] net: stmmac: dwmac-sun8i: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized
In function sun8i_dwmac_set_syscon(), local variable "val" could be uninitialized if function regmap_field_read() returns -EINVAL. However, it will be used directly in the if statement, which is potentially unsafe. Signed-off-by: Yizhuo --- drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c | 7 ++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c index 4083019c547a..f97a4096f8fc 100644 --- a/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c +++ b/drivers/net/ethernet/stmicro/stmmac/dwmac-sun8i.c @@ -873,7 +873,12 @@ static int sun8i_dwmac_set_syscon(struct stmmac_priv *priv) int ret; u32 reg, val; - regmap_field_read(gmac->regmap_field, ); + ret = regmap_field_read(gmac->regmap_field, ); + if (ret) { + dev_err(priv->device, "Fail to read from regmap field.\n"); + return ret; + } + reg = gmac->variant->default_syscon_value; if (reg != val) dev_warn(priv->device, -- 2.17.1
Re: [PATCH 0/4] objtool,perf: Use shared x86 insn decoder
On Fri, 30 Aug 2019 16:48:45 -0300 Arnaldo Carvalho de Melo wrote: > Em Fri, Aug 30, 2019 at 02:31:09PM -0500, Josh Poimboeuf escreveu: > > On Fri, Aug 30, 2019 at 04:00:58PM -0300, Arnaldo Carvalho de Melo wrote: > > > I.e. we need to make sure that it always gets the x86 stuff, not > > > something that is tied to the host arch, with the patch below we get it > > > to work, please take a look. > > > > > > Probably this should go to the master copy, i.e. to the kernel sources, > > > no? Interesting approach. Hmm, but I would like "diff -I" trick just for short term solution. > > > > > > That or we'll have to ask the check-headers.sh and objtool sync-check > > > (hey, this should be unified, each project could provide just the list > > > of things it uses, but I digress) to ignore those lines... > > > > > > I.e. we want to decode intel_PT traces on other arches, ditto for > > > CoreSight (not affected here, but similar concept). > > > > > > will kick the full container build process now. > > > > Interesting, I didn't realize other arches would be using it. The patch > > Yeah, decoding CoreSight (aarch64) hardware traces on x86_64 should be > as possible as decoding Intel PT hardware traces on aarch64 :-) > > > looks good to me. > > > > Ideally there wouldn't be any differences between the headers, but if > > that's unavoidable then I guess we can just use the same 'diff -I' trick > > I'll go with this now, but... > > > we were using before in the check script(s). > > Masami? What do you think of applying the patch to the main kernel > sources so that building a decoder for x86 on any other arch becomes > possible? I think the build of kernel and user-space tools are different especially for "include/asm", since user-space tools may want to use all architecture features, but kernel needs only the architecture which it runs on. Maybe we need a special Makefile entries for the modules which depends on architecture dependent parts. e.g. x86-objs = insn.o inat.o ... arm64-objs = coresight.o ... and they should have different -I options ('-I arch/x86/include' or '-I arch/arm64/include') for compiling. I think this is better and scalable, if you use common (clone) files in the kernel tree. Thank you, -- Masami Hiramatsu
Re: [PATCH] arm: fix page faults in do_alignment
On 2019/8/30 21:35, Russell King - ARM Linux admin wrote: > On Fri, Aug 30, 2019 at 09:31:17PM +0800, Jing Xiangfeng wrote: >> The function do_alignment can handle misaligned address for user and >> kernel space. If it is a userspace access, do_alignment may fail on >> a low-memory situation, because page faults are disabled in >> probe_kernel_address. >> >> Fix this by using __copy_from_user stead of probe_kernel_address. >> >> Fixes: b255188 ("ARM: fix scheduling while atomic warning in alignment >> handling code") >> Signed-off-by: Jing Xiangfeng > > NAK. > > The "scheduling while atomic warning in alignment handling code" is > caused by fixing up the page fault while trying to handle the > mis-alignment fault generated from an instruction in atomic context. __might_sleep is called in the function __get_user which lead to that bug. And that bug is triggered in a kernel space. Page fault can not be generated. Right? > Your patch re-introduces that bug. > >> --- >> arch/arm/mm/alignment.c | 16 +--- >> 1 file changed, 13 insertions(+), 3 deletions(-) >> >> diff --git a/arch/arm/mm/alignment.c b/arch/arm/mm/alignment.c >> index 04b3643..2ccabd3 100644 >> --- a/arch/arm/mm/alignment.c >> +++ b/arch/arm/mm/alignment.c >> @@ -774,6 +774,7 @@ static ssize_t alignment_proc_write(struct file *file, >> const char __user *buffer >> unsigned long instr = 0, instrptr; >> int (*handler)(unsigned long addr, unsigned long instr, struct pt_regs >> *regs); >> unsigned int type; >> +mm_segment_t fs; >> unsigned int fault; >> u16 tinstr = 0; >> int isize = 4; >> @@ -784,16 +785,22 @@ static ssize_t alignment_proc_write(struct file *file, >> const char __user *buffer >> >> instrptr = instruction_pointer(regs); >> >> +fs = get_fs(); >> +set_fs(KERNEL_DS); >> if (thumb_mode(regs)) { >> u16 *ptr = (u16 *)(instrptr & ~1); >> -fault = probe_kernel_address(ptr, tinstr); >> +fault = __copy_from_user(tinstr, >> +(__force const void __user *)ptr, >> +sizeof(tinstr)); >> tinstr = __mem_to_opcode_thumb16(tinstr); >> if (!fault) { >> if (cpu_architecture() >= CPU_ARCH_ARMv7 && >> IS_T32(tinstr)) { >> /* Thumb-2 32-bit */ >> u16 tinst2 = 0; >> -fault = probe_kernel_address(ptr + 1, tinst2); >> +fault = __copy_from_user(tinst2, >> +(__force const void __user >> *)(ptr+1), >> +sizeof(tinst2)); >> tinst2 = __mem_to_opcode_thumb16(tinst2); >> instr = __opcode_thumb32_compose(tinstr, >> tinst2); >> thumb2_32b = 1; >> @@ -803,10 +810,13 @@ static ssize_t alignment_proc_write(struct file *file, >> const char __user *buffer >> } >> } >> } else { >> -fault = probe_kernel_address((void *)instrptr, instr); >> +fault = __copy_from_user(instr, >> +(__force const void __user *)instrptr, >> +sizeof(instr)); >> instr = __mem_to_opcode_arm(instr); >> } >> >> +set_fs(fs); >> if (fault) { >> type = TYPE_FAULT; >> goto bad_or_fault; >> -- >> 1.8.3.1 >> >> >
Re: [PATCH 4.14] tcp: fix tcp_rtx_queue_tail in case of empty retransmit queue
Hello, On 24/08/19 - 15:05:20, Jonathan Lemon wrote: > > > On 23 Aug 2019, at 23:03, Tim Froidcoeur wrote: > > > Commit 8c3088f895a0 ("tcp: be more careful in tcp_fragment()") > > triggers following stack trace: > > > > [25244.848046] kernel BUG at ./include/linux/skbuff.h:1406! > > [25244.859335] RIP: 0010:skb_queue_prev+0x9/0xc > > [25244.888167] Call Trace: > > [25244.889182] > > [25244.890001] tcp_fragment+0x9c/0x2cf > > [25244.891295] tcp_write_xmit+0x68f/0x988 > > [25244.892732] __tcp_push_pending_frames+0x3b/0xa0 > > [25244.894347] tcp_data_snd_check+0x2a/0xc8 > > [25244.895775] tcp_rcv_established+0x2a8/0x30d > > [25244.897282] tcp_v4_do_rcv+0xb2/0x158 > > [25244.898666] tcp_v4_rcv+0x692/0x956 > > [25244.899959] ip_local_deliver_finish+0xeb/0x169 > > [25244.901547] __netif_receive_skb_core+0x51c/0x582 > > [25244.903193] ? inet_gro_receive+0x239/0x247 > > [25244.904756] netif_receive_skb_internal+0xab/0xc6 > > [25244.906395] napi_gro_receive+0x8a/0xc0 > > [25244.907760] receive_buf+0x9a1/0x9cd > > [25244.909160] ? load_balance+0x17a/0x7b7 > > [25244.910536] ? vring_unmap_one+0x18/0x61 > > [25244.911932] ? detach_buf+0x60/0xfa > > [25244.913234] virtnet_poll+0x128/0x1e1 > > [25244.914607] net_rx_action+0x12a/0x2b1 > > [25244.915953] __do_softirq+0x11c/0x26b > > [25244.917269] ? handle_irq_event+0x44/0x56 > > [25244.918695] irq_exit+0x61/0xa0 > > [25244.919947] do_IRQ+0x9d/0xbb > > [25244.921065] common_interrupt+0x85/0x85 > > [25244.922479] > > > > tcp_rtx_queue_tail() (called by tcp_fragment()) can call > > tcp_write_queue_prev() on the first packet in the queue, which will trigger > > the BUG in tcp_write_queue_prev(), because there is no previous packet. > > > > This happens when the retransmit queue is empty, for example in case of a > > zero window. > > > > Patch is needed for 4.4, 4.9 and 4.14 stable branches. > > > > Fixes: 8c3088f895a0 ("tcp: be more careful in tcp_fragment()") > > Change-Id: I839bde7167ae59e2f7d916c913507372445765c5 > > Signed-off-by: Tim Froidcoeur > > Signed-off-by: Matthieu Baerts > > Reviewed-by: Christoph Paasch > > Acked-by: Jonathan Lemon just checking in, if the patch is getting picked up for -stable ? (I don't see it in the stable-queue) Thanks, Christoph
Re: [PATCH] checkpatch: Validate Fixes: tag using 'commit' checks
Hi Sean, On Fri, 30 Aug 2019 09:36:58 -0700 Sean Christopherson wrote: > > @@ -2803,10 +2805,15 @@ sub process { > ($id, $description) = git_commit_info($orig_commit, > $id, $orig_desc); > > - if (defined($id) && > -($short || $long || $space || $case || ($orig_desc > ne $description) || !$hasparens)) { > + > + if (!defined($id)) { > + if ($init_tag =~ /fixes:/i) { > + ERROR("GIT_COMMIT_ID", > + "Target SHA1 '$orig_commit' does > not exist\n" . $herecurr); > + } Unfortunately, git_commit_info() just returns the passed in $id (which is explicitly set earlier) if git is not available or you are not in a git repository (and that latter check is not entirely correct anyway). Also, what you really need to test is if the specified commit is an ancestor of the place in the maintainer's tree where this patch is to be applied. The commit may well exist in the developer's tree, but not be in the maintainer's tree :-( This will, however, catch the cases where the SHA1 has been mistyped, but we should encourage people not to type them anyway, instead generating them using "git log". -- Cheers, Stephen Rothwell pgpzv8UQyRPkw.pgp Description: OpenPGP digital signature
Re: [PATCH 1/2] w1: add 1-wire master driver for IP block found in SGI ASICs
Hi Thomas, I love your patch! Yet something to improve: [auto build test ERROR on linus/master] [cannot apply to v5.3-rc6 next-20190830] [if your patch is applied to the wrong git tree, please drop us a note to help improve the system] url: https://github.com/0day-ci/linux/commits/Thomas-Bogendoerfer/w1-add-1-wire-master-driver-for-IP-block-found-in-SGI-ASICs/20190830-122322 config: sh-allmodconfig (attached as .config) compiler: sh4-linux-gcc (GCC) 7.4.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 GCC_VERSION=7.4.0 make.cross ARCH=sh If you fix the issue, kindly add following tag Reported-by: kbuild test robot All errors (new ones prefixed by >>): In file included from drivers/w1/masters/sgi_w1.c:13:0: >> include/linux/platform_data/sgi-w1.h:9:10: fatal error: asm/sn/types.h: No >> such file or directory #include ^~~~ compilation terminated. vim +9 include/linux/platform_data/sgi-w1.h 8 > 9 #include 10 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH] riscv: add arch/riscv/Kbuild
On Wed, 21 Aug 2019, Masahiro Yamada wrote: > Use the standard obj-y form to specify the sub-directories under > arch/riscv/. No functional change intended. > > Signed-off-by: Masahiro Yamada Thanks, queued for v5.4-rc1. - Paul
Re: [PATCH] drivers/staging/exfat - by default, prohibit mount of fat/vfat
On Fri, 30 Aug 2019 09:45:03 -0700, Christoph Hellwig said: > On Fri, Aug 30, 2019 at 12:42:39PM -0400, Valdis KlÄtnieks wrote: > > Concerns have been raised about the exfat driver accidentally mounting > > fat/vfat file systems. Add an extra configure option to help prevent that. > > Just remove that code. This is exactly what I fear about this staging > crap, all kinds of half-a***ed patches instead of trying to get anything Explain how it's half-a**ed. You worry about accidental mounting, meanwhile down in the embedded space there are memory-constrained machines that don't want separate vfat and exfat drivers sitting around in memory. If you have a better patch that addresses both concerns, feel free to submit it. > done. Given that you signed up as the maintainer for this what is your > plan forward on it? What development did you on the code and what are > your next steps? Well, the *original* plan was to get it into the tree someplace so it can get review and updates from others. Given the amount of press the Microsoft announcement had, we were *hoping* there would be some momentum and people actually looking at the code and feeding me patches. I've gotten a half dozen already today Although if you prefer, it can just sit out-of-tree until I've got a perfect driver without input or review from anybody. But I can't think of *any* instance where that model has actually worked. pgp5SKFaJ926z.pgp Description: PGP signature
Re: linux-next: Tree for Aug 30 (exfat)
On Fri, 30 Aug 2019 09:52:15 -0700, Randy Dunlap said: > on x86_64: > when CONFIG_VFAT_FS is not set/enabled: > > ../drivers/staging/exfat/exfat_super.c:46:41: error: > �CONFIG_FAT_DEFAULT_IOCHARSET� undeclared here (not in a function); did you > mean �CONFIG_EXFAT_DEFAULT_IOCHARSET�? Thanks. I'll fix this tonight pgpxe_9ABOc_H.pgp Description: PGP signature
Re: [PATCH] net: stmmac: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized
From: Yizhuo Zhai Date: Fri, 30 Aug 2019 15:29:07 -0700 > Thanks for your feedback, this patch should work for v4.14. You must always submit patches against the current tree.
Re: Adding depends-on DT binding to break cyclic dependencies
On Fri, Aug 30, 2019 at 7:35 AM Rob Herring wrote: > > On Thu, Aug 29, 2019 at 11:58 PM Saravana Kannan wrote: > > > > On Thu, Aug 29, 2019 at 9:28 AM Rob Herring wrote: > > > > > > On Thu, Aug 22, 2019 at 1:55 AM Saravana Kannan > > > wrote: > > > > > > > > Hi Rob, > > > > > > > > Frank, Greg and I got together during ELC and had an extensive and > > > > very productive discussion about my "postboot supplier state cleanup" > > > > patch series [1]. The three of us are on the same page now -- the > > > > series as it stands is the direction we want to go in, with some minor > > > > refactoring, documentation and naming changes. > > > > > > > > However, one of the things Frank is concerned about (and Greg and I > > > > agree) in the current patch series is that the "cyclic dependency > > > > breaking" logic has been pushed off to individual drivers using the > > > > edit_links() callback. > > > > > > I would think the core can detect this condition. There's nothing > > > device specific once you have the dependency tree. You still need to > > > know what device needs to probe first and the drivers are going to > > > have that knowledge anyways. So wouldn't it be enough to allow probe > > > to proceed for devices in the loop. > > > > The problem is that device links don't allow creating a cyclic > > dependency graph -- for good reasons. The last link in the cycle would > > be rejected. I don't think trying to change device link to allow > > cyclic links is the right direction to go in nor is it a can of worms > > I want to open. > > > > So, we'll need some other way of tracking the loop and then allowing > > only those devices in a loop to attempt probing even if their > > suppliers haven't probed. And then if a device ends up being in more > > than one loop, things could get even more complicated. And after one > > of the devices in the loop probes, we still need to somehow figure out > > the "bad" link and delete it so the last "good" link can be made > > before all the suppliers have their sync_state() called (because the > > "good" link hasn't been created yet). That all gets pretty messy. If > > we are never going to accept any DT changes, then I'd rather go with > > edit_links() and keep the complexity within the one off weird hardware > > where there are cycles instead of over complicating the driver core. > > If it is so complex, then it should not be in DT. I'm talking about existing simple DT bindings that says what clocks/interconnects/regulators a device needs. Not sure what you mean by "it should not be in DT". > Things like > describing clocks at the gate/mux/divider level turned out to be too > complex to get right or complete, so we avoid that. Right, and this patch series has nothing to do with describing complex internals of a device. > > > > Once 1 driver succeeds, then you > > > can enforce the dependencies on the rest. > > > > > > > The concern being, there are going to be multiple device specific ad > > > > hoc implementations to break a cyclic dependency. Also, if a device > > > > can be part of a cyclic dependency, the driver for that device has to > > > > check for specific system/products in which the device is part of a > > > > cyclic dependency (because it might not always be part of a cycle), > > > > and then potentially have cycle/product specific code to break the > > > > cycle (since the cycle can be different on each system/product). > > > > > > > > One way to avoid all of the device/driver specific code and simplify > > > > my patch series by a non-trivial amount would be by adding a > > > > "depends-on" DT binding that can ONLY be used to break cycles. We can > > > > document it as such and reject any attempts to use it for other > > > > purposes. When a depends-on property is present in a device node, that > > > > specific device's supplier list will be parsed ONLY from the > > > > depends-on property and the other properties won't be parsed for > > > > deriving dependency information for that device. > > > > > > Seems like only ignoring the dependencies with a cycle would be > > > sufficient. > > > > No, we need to only ignore the "bad" dependency. We can't ignore all > > the dependencies in the cycle because that would cause the suppliers > > to clean up the hardware state before the consumers are ready. > > I misunderstood. I now see this is back to duplicating all the data > that's already there which I've already said no to. What I proposed earlier was using depends-on for all devices. And I'm glad you rejected it because I like my current set of patches better than the earlier one. The question here is whether we can allow this for the rare occasions where there is a cycle. Frank, Greg and I think it was a reasonable compromise when we talked about it in person. > > > > For example, consider a clock controller which has 2 clock > > > inputs from other clock controllers where one has a cycle and one > > > doesn't. Also consider it has a regulator
Re: [RESEND PATCH V3 3/8] perf/x86/intel: Support hardware TopDown metrics
> the same manner. It would greatly simplify the kernel implementation. I tried that originally. It was actually more complicated. You can't really do deltas on raw metrics, and a lot of the perf infrastructure is built around deltas. To do the regular reset and not lose precision over time internally you have to keep expanded counters anyways. And if you do that you can just expose them to user space too, and have everything in user space just work without any changes (except for the final output) -Andi
Re: [v2] net_sched: act_police: add 2 new attributes to support police 64bit rate and peakrate
On Fri, 2019-08-30 at 13:33 -0700, David Miller wrote: > From: "David Z. Dai" > Date: Fri, 30 Aug 2019 15:03:52 -0500 > > > I have the impression that last parameter num value should be larger > > than the attribute num value in 2nd parameter (TC_POLICE_RATE64 in this > > case). > > The argument in question is explicitly the "padding" value. > > Please explain in detail where you got the impression that the > argument has to be larger? In include/uapi/linux/pkt_sched.h header: For HTB: enum { TCA_HTB_UNSPEC, TCA_HTB_PARMS, TCA_HTB_INIT, TCA_HTB_CTAB, TCA_HTB_RTAB, TCA_HTB_DIRECT_QLEN, TCA_HTB_RATE64,/* <--- */ TCA_HTB_CEIL64,/* <--- */ TCA_HTB_PAD, /* <--- */ __TCA_HTB_MAX, }; /* TCA_HTB_RATE64,TCA_HTB_CEIL64 are declared *before* TCA_HTB_PAD */ For TBF: enum { TCA_TBF_UNSPEC, TCA_TBF_PARMS, TCA_TBF_RTAB, TCA_TBF_PTAB, TCA_TBF_RATE64, /* <--- */ TCA_TBF_PRATE64, /* <--- */ TCA_TBF_BURST, TCA_TBF_PBURST, TCA_TBF_PAD, /* <--- */ __TCA_TBF_MAX, }; /* TCA_TBF_RATE64, TCA_TBF_PRATE64 are declared *before* TCA_TBF_PAD */ For HTB, in net/sched/sch_htb.c file, htb_dump_class() routine: if ((cl->rate.rate_bytes_ps >= (1ULL << 32)) && nla_put_u64_64bit(skb, TCA_HTB_RATE64, cl->rate.rate_bytes_ps, TCA_HTB_PAD)) goto nla_put_failure; if ((cl->ceil.rate_bytes_ps >= (1ULL << 32)) && nla_put_u64_64bit(skb, TCA_HTB_CEIL64, cl->ceil.rate_bytes_ps, TCA_HTB_PAD)) goto nla_put_failure; For TBF, in net/sched/sch_tbf.c file, tbf_dump() routine: if (q->rate.rate_bytes_ps >= (1ULL << 32) && nla_put_u64_64bit(skb, TCA_TBF_RATE64, q->rate.rate_bytes_ps, TCA_TBF_PAD)) goto nla_put_failure; if (tbf_peak_present(q) && q->peak.rate_bytes_ps >= (1ULL << 32) && nla_put_u64_64bit(skb, TCA_TBF_PRATE64, q->peak.rate_bytes_ps, TCA_TBF_PAD)) goto nla_put_failure; The last parameter used TCA_TBF_PAD, TCA_TBF_PAD are all declared *after* those attributes. I am trying to keep it consistent in police part. That's where my impression is coming from. Now for suggestion/comment, do you think is it better to add a new PAD attribute in include/uapi/pkt_cls.h like this: enum { TCA_POLICE_UNSPEC, TCA_POLICE_TBF, TCA_POLICE_RATE, TCA_POLICE_PEAKRATE, TCA_POLICE_AVRATE, TCA_POLICE_RESULT, TCA_POLICE_TM, TCA_POLICE_PAD, TCA_POLICE_RATE64, /* <--- */ TCA_POLICE_PEAKRATE64, /* <--- */ TCA_POLICE_PAD2,/* <--- new PAD */ __TCA_POLICE_MAX #define TCA_POLICE_RESULT TCA_POLICE_RESULT #}; Then use this TCA_POLICE_PAD2 as the last parameter in nla_put_u64_64bit()? Thanks!
RE: [PATCH v3 08/12] Drivers: hv: vmbus: Ignore the offers when resuming from hibernation
> From: Michael Kelley > Sent: Friday, August 23, 2019 12:57 PM > > From: Dexuan Cui Sent: August 19, 2019 6:52 PM > > > > diff --git a/drivers/hv/connection.c b/drivers/hv/connection.c > > @@ -337,6 +337,33 @@ struct vmbus_channel *relid2channel(u32 relid) > > } > > > > /* > > + * find_primary_channel_by_offer - Get the channel object given the new > offer. > > + * This is only used in the resume path of hibernation. > > + */ > > +struct vmbus_channel * > > +find_primary_channel_by_offer(const struct vmbus_channel_offer_channel > *offer) > > +{ > > + struct vmbus_channel *channel; > > + const guid_t *inst1, *inst2; > > + > > + WARN_ON(!mutex_is_locked(_connection.channel_mutex)); > > + > > + /* Ignore sub-channel offers. */ > > + if (offer->offer.sub_channel_index != 0) > > + return NULL; > > + > > + list_for_each_entry(channel, _connection.chn_list, listentry) { > > + inst1 = >offermsg.offer.if_instance; > > + inst2 = >offer.if_instance; > > + > > + if (guid_equal(inst1, inst2)) > > + return channel; > > + } > > + > > + return NULL; > > +} > > Any particular reason this new function is in connection.c instead of > putting it in channel_mgmt.c where it is called? There is a similar function relid2channel(), which is in connection.c. Since the new function is only used in channel_mgmt.c. I'll move it to channel_mgmt.c in v4. Thanks, -- Dexuan
Re: [RFC][PATCH 1/1] Carry ima measurement log for arm64 via kexec_file_load
Stephen Boyd writes: > A lot of this code looks DT generic. Can it be moved out of the arch > layer to drivers/of/? Yes, if this code could be in drivers/of/ it would be great! Perhaps the DT generic functions could go in drivers/of/fdt.c, and ones dealing with IMA nodes/properties could go in drivers/of/fdt_ima.c? -- Thiago Jung Bauermann IBM Linux Technology Center
Re: WARNING: suspicious RCU usage in ext4_release_system_zone
On Fri, Aug 30, 2019 at 12:28:08PM -0700, syzbot wrote: > Hello, > > syzbot found the following crash on: > > HEAD commit:ed858b88 Add linux-next specific files for 20190826 > git tree: linux-next > console output: https://syzkaller.appspot.com/x/log.txt?x=121b506c60 > kernel config: https://syzkaller.appspot.com/x/.config?x=ee8373cd9733e305 > dashboard link: https://syzkaller.appspot.com/bug?extid=5bda120b4032f831c57f > compiler: gcc (GCC) 9.0.0 20181231 (experimental) > > Unfortunately, I don't have any reproducer for this crash yet. > > IMPORTANT: if you fix the bug, please add the following tag to the commit: > Reported-by: syzbot+5bda120b4032f831c...@syzkaller.appspotmail.com > > = > WARNING: suspicious RCU usage > 5.3.0-rc6-next-20190826 #73 Not tainted > - > fs/ext4/block_validity.c:333 suspicious rcu_dereference_check() usage! > #syz invalid There was already a fix applied between ed858b88 and latest linux-next: diff --git a/fs/ext4/block_validity.c b/fs/ext4/block_validity.c index 003dc1dc2da3..f7bc914a74df 100644 --- a/fs/ext4/block_validity.c +++ b/fs/ext4/block_validity.c @@ -330,11 +330,13 @@ void ext4_release_system_zone(struct super_block *sb) { struct ext4_system_blocks *system_blks; + rcu_read_lock(); system_blks = rcu_dereference(EXT4_SB(sb)->system_blks); rcu_assign_pointer(EXT4_SB(sb)->system_blks, NULL); if (system_blks) call_rcu(_blks->rcu, ext4_destroy_system_zone); + rcu_read_unlock(); }
Re: WARNING: ODEBUG bug in ext4_fill_super
On Fri, Aug 30, 2019 at 12:46:21PM -0700, 'Dmitry Vyukov' via syzkaller-bugs wrote: > On Fri, Aug 30, 2019 at 12:42 PM 'Nick Desaulniers' via syzkaller-bugs > wrote: > > > > Dmitry, > > Any idea how clang-built-linux got CC'ed on this? Is syzcaller > > running clang builds, yet? (this looks like a GCC build) > > syzbot always uses get_maintainers: > > $ ./scripts/get_maintainer.pl -f fs/ext4/super.c > "Theodore Ts'o" (maintainer:EXT4 FILE SYSTEM) > Andreas Dilger (maintainer:EXT4 FILE SYSTEM) > linux-e...@vger.kernel.org (open list:EXT4 FILE SYSTEM) > linux-kernel@vger.kernel.org (open list) > clang-built-li...@googlegroups.com (open list:CLANG/LLVM BUILD SUPPORT) > CLANG/LLVM BUILD SUPPORT L: clang-built-li...@googlegroups.com W: https://clangbuiltlinux.github.io/ B: https://github.com/ClangBuiltLinux/linux/issues C: irc://chat.freenode.net/clangbuiltlinux S: Supported K: \b(?i:clang|llvm)\b So, clang-built-linux has volunteered to maintain every file that contains the word "clang" or "llvm" anywhere in its contents :-) $ grep clang fs/ext4/super.c (void) ei; /* shut up clang warning if !CONFIG_LOCKDEP */ - Eric
Re: [RFC][PATCH 1/1] Carry ima measurement log for arm64 via kexec_file_load
Hello Prakhar, Answering this part from the cover letter: > The code is in most part same as powerpc, i want to get feedback as to > how/correct way to refactor the code so that cross architecture > partial helpers can be put in a common place. That's a great idea. If it could go to drivers/of/ as Stephen Boyd mentioned in the other email that would be great. More comments below. Prakhar Srivastava writes: > Carry ima measurement log for arm64 via kexec_file_load. > add support to kexec_file_load to pass the ima measurement log > > This patch adds entry for the ima measurement log in the > dtb which is then used in the kexec'ed session to fetch the > segment and then load the ima measurement log. > > Signed-off-by: Prakhar Srivastava > --- > arch/arm64/Kconfig | 7 + > arch/arm64/include/asm/ima.h | 31 > arch/arm64/include/asm/kexec.h | 4 + > arch/arm64/kernel/Makefile | 1 + > arch/arm64/kernel/ima_kexec.c | 219 + > arch/arm64/kernel/machine_kexec_file.c | 39 + > 6 files changed, 301 insertions(+) > create mode 100644 arch/arm64/include/asm/ima.h > create mode 100644 arch/arm64/kernel/ima_kexec.c > > diff --git a/arch/arm64/Kconfig b/arch/arm64/Kconfig > index 3adcec05b1f6..9e1b831e7baa 100644 > --- a/arch/arm64/Kconfig > +++ b/arch/arm64/Kconfig > @@ -964,6 +964,13 @@ config KEXEC_FILE > for kernel and initramfs as opposed to list of segments as > accepted by previous system call. > > +config HAVE_IMA_KEXEC > + bool "enable arch specific ima buffer pass" > + depends on KEXEC_FILE > + help > + This adds support to carry ima log to the next kernel in case > + of kexec_file_load > + > config KEXEC_VERIFY_SIG > bool "Verify kernel signature during kexec_file_load() syscall" > depends on KEXEC_FILE This Kconfig should be defined in arch/Kconfig so that both arm64 and powerpc can select it. > diff --git a/arch/arm64/kernel/Makefile b/arch/arm64/kernel/Makefile > index 478491f07b4f..9620f90bd0e1 100644 > --- a/arch/arm64/kernel/Makefile > +++ b/arch/arm64/kernel/Makefile > @@ -63,6 +63,7 @@ obj-$(CONFIG_CRASH_CORE)+= crash_core.o > obj-$(CONFIG_ARM_SDE_INTERFACE) += sdei.o > obj-$(CONFIG_ARM64_SSBD) += ssbd.o > obj-$(CONFIG_ARM64_PTR_AUTH) += pointer_auth.o > +obj-$(CONFIG_HAVE_IMA_KEXEC) += ima_kexec.o > > obj-y+= vdso/ probes/ > obj-$(CONFIG_COMPAT_VDSO)+= vdso32/ > diff --git a/arch/arm64/kernel/ima_kexec.c b/arch/arm64/kernel/ima_kexec.c > new file mode 100644 > index ..5ae0d776ec42 > --- /dev/null > +++ b/arch/arm64/kernel/ima_kexec.c > @@ -0,0 +1,219 @@ > +/* > + * Copyright (C) 2016 IBM Corporation > + * > + * Authors: > + * Thiago Jung Bauermann > + * > + * This program is free software; you can redistribute it and/or modify > + * it under the terms of the GNU General Public License as published by > + * the Free Software Foundation; either version 2 of the License, or > + * (at your option) any later version. > + */ The powerpc file was updated to use the SPDX tag and remove the paragraph above. Please update your file to match. > +#include > +#include > +#include > +#include > +#include > +#include > +#include > + > +static int get_addr_size_cells(int *addr_cells, int *size_cells) > +{ > + struct device_node *root; > + > + root = of_find_node_by_path("/"); > + if (!root) > + return -EINVAL; > + > + *addr_cells = of_n_addr_cells(root); > + *size_cells = of_n_size_cells(root); > + > + of_node_put(root); > + > + return 0; > +} > + > +static int do_get_kexec_buffer(const void *prop, int len, unsigned long > *addr, > +size_t *size) > +{ > + This spurious blank line only exists in the arm64 version. Should be removed. > + int ret, addr_cells, size_cells; > + > + ret = get_addr_size_cells(_cells, _cells); > + if (ret) > + return ret; > + > + if (len < 4 * (addr_cells + size_cells)) > + return -ENOENT; > + > + *addr = of_read_number(prop, addr_cells); > + *size = of_read_number(prop + 4 * addr_cells, size_cells); > + > + return 0; > +} > + > +/** > + * ima_get_kexec_buffer - get IMA buffer from the previous kernel > + * @addr:On successful return, set to point to the buffer contents. > + * @size:On successful return, set to the buffer size. > + * > + * Return: 0 on success, negative errno on error. > + */ > +int ima_get_kexec_buffer(void **addr, size_t *size) > +{ > + int ret, len; > + unsigned long tmp_addr; > + size_t tmp_size; > + const void *prop; > + > + prop = of_get_property(of_chosen, FDT_PROP_KEXEC_BUFFER, ); > + if (!prop) > + return -ENOENT; > + > + ret = do_get_kexec_buffer(prop, len, _addr, _size); > + if (ret) >
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message- > From: Brendan Higgins > > On Fri, Aug 30, 2019 at 11:22:43PM +, tim.b...@sony.com wrote: > > > -Original Message- > > > From: Brendan Higgins > > > > > > On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote: > > > > > > > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > > > > From: Joe Perches > > > > [] > > > > > IMHO %pV should be avoided if possible. Just because people are > > > > > doing it doesn't mean it should be used when it is not necessary. > > > > > > > > Well, as the guy that created %pV, I of course > > > > have a different opinion. > > > > > > > > > > then wouldn't it be easier to pass in the > > > > > > > kernel level as a separate parameter and then strip off all printk > > > > > > > headers like this: > > > > > > > > > > > > Depends on whether or not you care for overall > > > > > > object size. Consolidated formats with the > > > > > > embedded KERN_ like suggested are smaller > > > > > > overall object size. > > > > > > > > > > This is an argument I can agree with. I'm generally in favor of > > > > > things that lessen kernel size creep. :-) > > > > > > > > As am I. > > > > > > Sorry, to be clear, we are talking about the object size penalty due > > > to adding a single parameter to a function. Is that right? > > > > Not exactly. The argument is that pre-pending the different KERN_LEVEL > > strings onto format strings can result in several versions of nearly > > identical > strings > > being compiled into the object file. By parameterizing this (that is, > > adding > > '%s' into the format string, and putting the level into the string as an > argument), > > it prevents this duplication of format strings. > > > > I haven't seen the data on duplication of format strings, and how much this > > affects it, but little things can add up. Whether it matters in this case > depends > > on whether the format strings that kunit uses are also used elsewhere in > the kernel, > > and whether these same format strings are used with multiple kernel > message levels. > > -- Tim > > I thought this portion of the discussion was about whether Joe's version > of kunit_printk was better or my critique of his version of kunit_printk: > > Joe's: > > > > > -void kunit_printk(const char *level, > > > > > - const struct kunit *test, > > > > > - const char *fmt, ...) > > > > > +void kunit_printk(const struct kunit *test, const char *fmt, ...) > > > > > { > > > > > + char lvl[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0"; > > > > > struct va_format vaf; > > > > > va_list args; > > > > > + int kern_level; > > > > > > > > > > va_start(args, fmt); > > > > > > > > > > + while ((kern_level = printk_get_level(fmt)) != 0) { > > > > > + size_t size = printk_skip_level(fmt) - fmt; > > > > > + > > > > > + if (kern_level >= '0' && kern_level <= '7') { > > > > > + memcpy(lvl, fmt, size); > > > > > + lvl[size] = '\0'; > > > > > + } > > > > > + fmt += size; > > > > > + } > > > > > + > > > > > vaf.fmt = fmt; > > > > > vaf.va = > > > > > > > > > > - kunit_vprintk(test, level, ); > > > > > + printk("%s\t# %s %pV\n", lvl, test->name, ); > > > > > > > > > > va_end(args); > > > > > } > > Mine: > > void kunit_printk(const char *level, > > const struct kunit *test, > > const char *fmt, ...) > > { > > struct va_format vaf; > > va_list args; > > > > va_start(args, fmt); > > > > + fmt = printk_skip_headers(fmt); > > + > > vaf.fmt = fmt; > > vaf.va = > > > > - kunit_vprintk(test, level, ); > > + printk("%s\t# %s %pV\n", level, test->name, ); > > > > va_end(args); > > } > > I thought you and Joe were arguing that "Joe's" resulted in a smaller > object size than "Mine" (not to be confused with the actual patch I > presented here, which is what Sergey suggested I do on a different > thread). > > I really don't feel strongly about what Sergey suggested I do (which is > what this patch originally introduced), versus, what Joe suggested, > versus what I suggested in response to Joe (or any of the things > suggested on other threads). I just want to pick one, fix the breakage > in linux-next, and move on with my life. When in doubt, do what the sub-system maintainer says. I'd go with Sergey's suggestion. Maintainers often are juggling a host of issues, and weighing new features and usages of their system against their long-term plans for their sub-system. Sometimes they have time to communicate all the intricacies of their counter-proposals, and sometimes not. But they know their system best, and much more often than not provide sound advice. If you don't have a strong feeling about it, just do what they say. -- Tim
[PATCH] nvme-core: Fix subsystem instance mismatches
With multipath enabled (which is now default in many distros), nvme controllers and their respective namespaces can be numbered differently. For example: nvme0n1 might actually belong to controller nvme1, which is super confusing (and may have broken any scripts that rely on the numbering relation). To make matters worse, the mismatches can sometimes change from boot to boot so anyone dealing with the nvme control device has to inspect sysfs every boot to ensure they get the right one. The reason for this is that the X in nvmeXn1 is the subsystem's instance number (when multipath is enabled) which is distinct from the controller's instance and the subsystem instance is assigned from a separate IDA after the controller gets identified and this can race a bit when multiple controllers are being setup. To fix this, assign the subsystem's instance based on the instance number of the controller's instance that first created it. There should always be fewer subsystems than controllers so the should not be a need to create extra subsystems that overlap existing controllers. Signed-off-by: Logan Gunthorpe --- drivers/nvme/host/core.c | 10 +- 1 file changed, 1 insertion(+), 9 deletions(-) diff --git a/drivers/nvme/host/core.c b/drivers/nvme/host/core.c index d3d6b7bd6903..ca201b71ab49 100644 --- a/drivers/nvme/host/core.c +++ b/drivers/nvme/host/core.c @@ -81,7 +81,6 @@ EXPORT_SYMBOL_GPL(nvme_reset_wq); struct workqueue_struct *nvme_delete_wq; EXPORT_SYMBOL_GPL(nvme_delete_wq); -static DEFINE_IDA(nvme_subsystems_ida); static LIST_HEAD(nvme_subsystems); static DEFINE_MUTEX(nvme_subsystems_lock); @@ -2332,7 +2331,6 @@ static void nvme_release_subsystem(struct device *dev) struct nvme_subsystem *subsys = container_of(dev, struct nvme_subsystem, dev); - ida_simple_remove(_subsystems_ida, subsys->instance); kfree(subsys); } @@ -2461,12 +2459,7 @@ static int nvme_init_subsystem(struct nvme_ctrl *ctrl, struct nvme_id_ctrl *id) subsys = kzalloc(sizeof(*subsys), GFP_KERNEL); if (!subsys) return -ENOMEM; - ret = ida_simple_get(_subsystems_ida, 0, 0, GFP_KERNEL); - if (ret < 0) { - kfree(subsys); - return ret; - } - subsys->instance = ret; + subsys->instance = ctrl->instance; mutex_init(>lock); kref_init(>ref); INIT_LIST_HEAD(>ctrls); @@ -4074,7 +4067,6 @@ static int __init nvme_core_init(void) static void __exit nvme_core_exit(void) { - ida_destroy(_subsystems_ida); class_destroy(nvme_subsys_class); class_destroy(nvme_class); unregister_chrdev_region(nvme_chr_devt, NVME_MINORS); -- 2.20.1
Re: [PATCH 1/9] perf/core: Add PERF_RECORD_CGROUP event
On Fri, Aug 30, 2019 at 3:49 PM Namhyung Kim wrote: > > On Fri, Aug 30, 2019 at 4:35 PM Peter Zijlstra wrote: > > > > On Fri, Aug 30, 2019 at 12:46:51PM +0900, Namhyung Kim wrote: > > > Hi Peter, > > > > > > On Wed, Aug 28, 2019 at 6:45 PM Peter Zijlstra > > > wrote: > > > > > > > > On Wed, Aug 28, 2019 at 04:31:22PM +0900, Namhyung Kim wrote: > > > > > To support cgroup tracking, add CGROUP event to save a link between > > > > > cgroup path and inode number. The attr.cgroup bit was also added to > > > > > enable cgroup tracking from userspace. > > > > > > > > > > This event will be generated when a new cgroup becomes active. > > > > > Userspace might need to synthesize those events for existing cgroups. > > > > > > > > > > As aux_output change is also going on, I just added the bit here as > > > > > well to remove possible conflicts later. > > > > > > > > Why do we want this? > > > > > > I saw below [1] and thought you have the patch introduced aux_output > > > and it's gonna to be merged soon. > > > Also the tooling patches are already in the acme/perf/core > > > so I just wanted to avoid conflicts. > > > > > > Anyway, I'm ok with changing it. Will remove in v2. > > > > I seem to have confused both you and Arnaldo with this. This email > > questions the Changelog as a whole, not just the aux thing (I send a > > separate email for that). > > > > This Changelog utterly fails to explain to me _why_ we need/want cgroup > > tracking. So why do I want to review and possibly merge this? Changelog > > needs to answer this. > > OK. How about this? > > Systems running a large number of jobs in different cgroups want to > profiling such jobs precisely. This includes container hosting systems > widely used today. Currently perf supports namespace tracking but > the systems may not use (cgroup) namespace for their jobs. Also > it'd be more intuitive to see cgroup names (as they're given by user > or sysadmin) rather than numeric cgroup/namespace id even if they > use the namespaces. > In data centers you care about attributing samples to a job not such much to a process. A job may have multiple processes which may come and go. The cgroup on the other hand stays around for the entire lifetime of the job. It is much easier to map a cgroup name to a particular job than it is to map a pid back to a job name, especially for offline post-processing. Hope this clarifies why we would like this feature upstream. > > Thanks, > Namhyung
Re: [PATCH] net: bcmgenet: use ethtool_op_get_ts_info()
On 8/30/19 11:51 AM, Florian Fainelli wrote: > On 8/30/19 11:49 AM, Ryan M. Collins wrote: >> This change enables the use of SW timestamping on the Raspberry Pi 4. > > Finally the first bcmgenet patch that was tested on the Pi 4! > >> >> bcmgenet's transmit function bcmgenet_xmit() implements software >> timestamping. However the SOF_TIMESTAMPING_TX_SOFTWARE capability was >> missing and only SOF_TIMESTAMPING_RX_SOFTWARE was announced. By using >> ethtool_ops bcmgenet_ethtool_ops() as get_ts_info(), the >> SOF_TIMESTAMPING_TX_SOFTWARE capability is announced. >> >> Similar to commit a8f5cb9e7991 ("smsc95xx: use ethtool_op_get_ts_info()") >> >> Signed-off-by: Ryan M. Collins > > Acked-by: Florian Fainelli > Thanks Ryan! Acked-by: Doug Berger
Re: [PATCH v2] kunit: fix failure to build without printk
On Fri, 2019-08-30 at 16:37 -0700, Brendan Higgins wrote: > I thought you and Joe were arguing that "Joe's" resulted in a smaller > object size than "Mine" (not to be confused with the actual patch I > presented here, which is what Sergey suggested I do on a different > thread). > > I really don't feel strongly about what Sergey suggested I do (which is > what this patch originally introduced), versus, what Joe suggested, > versus what I suggested in response to Joe (or any of the things > suggested on other threads). I just want to pick one, fix the breakage > in linux-next, and move on with my life. Well, if we are voting, I vote for mine! ;) cheers, Joe
RE: [PATCH v3 02/12] x86/hyper-v: Implement hv_is_hibernation_supported()
> From: Michael Kelley > Sent: Friday, August 23, 2019 12:50 PM > > From: Dexuan Cui Sent: August 19, 2019 6:52 PM > > > > When a Linux VM runs on Hyper-V and hibernates, it must disable the > > memory hot-add/remove and balloon up/down capabilities in the hv_balloon > > driver. > > I'm unclear on the above statement. I think the requirement is that > ballooning must not be active when hibernation is initiated. Is hibernation > blocked in that case? If not, what happens? Ballooning (and hot-addition of memory) must not be active if the user wants the Linux VM to support hibernation, not just when hibernation is initiated. This is because ballooning and hot-addition of memory are incompatible with hibernation according to Hyper-V team, e.g. upon hibernation the balloon VSP doesn't save any info about ballooned-out pages (if any), so after Linux resumes, Linux balloon VSC expects that the VSP will return the pages if Linux is under memory pressure, but the VSP will never return the pages, since the VSP thinks it never stole the pages from the VM. So, if the user wants Linux VM to support hibernation, Linux must completely forbid ballooning and hot-addition of memory, and hence the only functionality of balloon VSC is reporting the memory pressure to the host. Ideally, when Linux detects that the user wants it to support hibernation, the balloon VSC should tell the VSP that it does not support ballooning and hot-addition; however, the current version of the VSP requires the VSC should support these capabilities, otherwise the capability negotiation fails and the VSC can not load at all, so in my changes to the VSC driver, I still report to the VSP that Linux supports the capabilities, but the VSC ignores the host's requests of balloon up/down and hot add, and returns an error to the VSP, when applicable. BTW, in the future the balloon VSP driver will allow the VSC to not support the capabilities of balloon up/down and hot add. The remaining problem is: how can Linux know the user wants Linux VM to support hibernation? The ACPI S4 state is not a must for hibernation to work, because Linux is able to hibernate as long as the system can shut down. My decision is that: we artificially use the presence of the virtual ACPI S4 state as the indicator of the user's intent of using hibernation. BTW, I believe the Windows team made the same decision. Once all the vmbus and VSC patches are accepted, I'll submit a patch to forbid hibernation if the virtual ACPI S4 state is absent, e.g. hv_is_hibernation_supported() is false. > > By default, Hyper-V does not enable the virtual ACPI S4 state for a VM; > > on recent Hyper-V hosts, the administrator is able to enable the virtual > > ACPI S4 state for a VM, so we hope to use the presence of the virtual ACPI > > "we hope" sounds very indefinite. :-(Does ACPI S4 have to be enabled for > hibernation to be initiated? Goes back to my first question Technically, we don't have to enable ACPI S4, but in practice, as I mentioned, I'll submit a patch to forbid hibernation if ACPI S4 is absent. > > > S4 state as a hint for hv_balloon to disable the aforementioned > > capabilities. > > > > The new API will be used by hv_balloon. I'll add my above explanation into the changelog in v4. Thanks, -- Dexuan
Re: [PATCH v2] kunit: fix failure to build without printk
On Fri, Aug 30, 2019 at 11:22:43PM +, tim.b...@sony.com wrote: > > -Original Message- > > From: Brendan Higgins > > > > On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote: > > > > > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > > > From: Joe Perches > > > [] > > > > IMHO %pV should be avoided if possible. Just because people are > > > > doing it doesn't mean it should be used when it is not necessary. > > > > > > Well, as the guy that created %pV, I of course > > > have a different opinion. > > > > > > > > then wouldn't it be easier to pass in the > > > > > > kernel level as a separate parameter and then strip off all printk > > > > > > headers like this: > > > > > > > > > > Depends on whether or not you care for overall > > > > > object size. Consolidated formats with the > > > > > embedded KERN_ like suggested are smaller > > > > > overall object size. > > > > > > > > This is an argument I can agree with. I'm generally in favor of > > > > things that lessen kernel size creep. :-) > > > > > > As am I. > > > > Sorry, to be clear, we are talking about the object size penalty due > > to adding a single parameter to a function. Is that right? > > Not exactly. The argument is that pre-pending the different KERN_LEVEL > strings onto format strings can result in several versions of nearly > identical strings > being compiled into the object file. By parameterizing this (that is, adding > '%s' into the format string, and putting the level into the string as an > argument), > it prevents this duplication of format strings. > > I haven't seen the data on duplication of format strings, and how much this > affects it, but little things can add up. Whether it matters in this case > depends > on whether the format strings that kunit uses are also used elsewhere in the > kernel, > and whether these same format strings are used with multiple kernel message > levels. > -- Tim I thought this portion of the discussion was about whether Joe's version of kunit_printk was better or my critique of his version of kunit_printk: Joe's: > > > > -void kunit_printk(const char *level, > > > > - const struct kunit *test, > > > > - const char *fmt, ...) > > > > +void kunit_printk(const struct kunit *test, const char *fmt, ...) > > > > { > > > > + char lvl[PRINTK_MAX_SINGLE_HEADER_LEN + 1] = "\0"; > > > > struct va_format vaf; > > > > va_list args; > > > > + int kern_level; > > > > > > > > va_start(args, fmt); > > > > > > > > + while ((kern_level = printk_get_level(fmt)) != 0) { > > > > + size_t size = printk_skip_level(fmt) - fmt; > > > > + > > > > + if (kern_level >= '0' && kern_level <= '7') { > > > > + memcpy(lvl, fmt, size); > > > > + lvl[size] = '\0'; > > > > + } > > > > + fmt += size; > > > > + } > > > > + > > > > vaf.fmt = fmt; > > > > vaf.va = > > > > > > > > - kunit_vprintk(test, level, ); > > > > + printk("%s\t# %s %pV\n", lvl, test->name, ); > > > > > > > > va_end(args); > > > > } Mine: > void kunit_printk(const char *level, > const struct kunit *test, > const char *fmt, ...) > { > struct va_format vaf; > va_list args; > > va_start(args, fmt); > > + fmt = printk_skip_headers(fmt); > + > vaf.fmt = fmt; > vaf.va = > > - kunit_vprintk(test, level, ); > + printk("%s\t# %s %pV\n", level, test->name, ); > > va_end(args); > } I thought you and Joe were arguing that "Joe's" resulted in a smaller object size than "Mine" (not to be confused with the actual patch I presented here, which is what Sergey suggested I do on a different thread). I really don't feel strongly about what Sergey suggested I do (which is what this patch originally introduced), versus, what Joe suggested, versus what I suggested in response to Joe (or any of the things suggested on other threads). I just want to pick one, fix the breakage in linux-next, and move on with my life. Cheers
Re: [PATCH v2] kunit: fix failure to build without printk
On Fri, 2019-08-30 at 23:22 +, tim.b...@sony.com wrote: > > -Original Message- > > From: Brendan Higgins > > > > On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote: > > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > > > From: Joe Perches > > > [] > > > > IMHO %pV should be avoided if possible. Just because people are > > > > doing it doesn't mean it should be used when it is not necessary. > > > > > > Well, as the guy that created %pV, I of course > > > have a different opinion. > > > > > > > > then wouldn't it be easier to pass in the > > > > > > kernel level as a separate parameter and then strip off all printk > > > > > > headers like this: > > > > > > > > > > Depends on whether or not you care for overall > > > > > object size. Consolidated formats with the > > > > > embedded KERN_ like suggested are smaller > > > > > overall object size. > > > > > > > > This is an argument I can agree with. I'm generally in favor of > > > > things that lessen kernel size creep. :-) > > > > > > As am I. > > > > Sorry, to be clear, we are talking about the object size penalty due > > to adding a single parameter to a function. Is that right? > > Not exactly. The argument is that pre-pending the different KERN_LEVEL > strings onto format strings can result in several versions of nearly > identical strings > being compiled into the object file. By parameterizing this (that is, adding > '%s' into the format string, and putting the level into the string as an > argument), > it prevents this duplication of format strings. > > I haven't seen the data on duplication of format strings, and how much this > affects it, but little things can add up. Whether it matters in this case > depends > on whether the format strings that kunit uses are also used elsewhere in the > kernel, > and whether these same format strings are used with multiple kernel message > levels. deduplication can matter as well, but so far there is little content with kunit_(err|warn|info(=) kunit/example-test.c: kunit_info(test, "initializing\n"); kunit/test.c: kunit_err(test, kunit/test.c: kunit_err(test, "%s", fragment->fragment); kunit/test.c: kunit_err(test, "\n"); kunit/test.c: kunit_err(test, "%s", buf); kunit/test.c: kunit_err(test, "failed to initialize: %d\n", ret); kunit/test.c: kunit_err(test, "test case timed out\n"); kunit/test.c: kunit_err(test, "internal error occurred preventing test case from running: %d\n", kunit/try-catch.c: kunit_err(test, "try timed out\n"); kunit/try-catch.c: kunit_err(test, "wake_up_process() was never called\n"); kunit/try-catch.c: kunit_err(test, "Unknown error: %d\n", exit_code); Of these, only two do match other kernel uses. "initializing\n", "failed to initialize: %d\n"
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message- > From: Joe Perches > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > From: Joe Perches > [] > > IMHO %pV should be avoided if possible. Just because people are > > doing it doesn't mean it should be used when it is not necessary. > > Well, as the guy that created %pV, I of course > have a different opinion. LOL. Well I stepped in that one. I don't have any data to support my position on this particular printk feature, but having worked for a while on stack size reduction for a few Sony products, I'm always a bit leery of recursive routines in the kernel. I vaguely recall some recursive printk routines giving me problems on a product that used a sub-4K stack configuration I did many years ago. I don't recall if it was specifically %pV or not. Anyway YMMV. -- Tim
Re: [PATCH] selftests/bpf: Fix a typo in test_offload.py
On 8/29/19 2:01 AM, Masanari Iida wrote: This patch fix a spelling typo in test_offload.py Signed-off-by: Masanari Iida Applied, thanks!
RE: [PATCH v2] kunit: fix failure to build without printk
> -Original Message- > From: Brendan Higgins > > On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote: > > > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > > From: Joe Perches > > [] > > > IMHO %pV should be avoided if possible. Just because people are > > > doing it doesn't mean it should be used when it is not necessary. > > > > Well, as the guy that created %pV, I of course > > have a different opinion. > > > > > > then wouldn't it be easier to pass in the > > > > > kernel level as a separate parameter and then strip off all printk > > > > > headers like this: > > > > > > > > Depends on whether or not you care for overall > > > > object size. Consolidated formats with the > > > > embedded KERN_ like suggested are smaller > > > > overall object size. > > > > > > This is an argument I can agree with. I'm generally in favor of > > > things that lessen kernel size creep. :-) > > > > As am I. > > Sorry, to be clear, we are talking about the object size penalty due > to adding a single parameter to a function. Is that right? Not exactly. The argument is that pre-pending the different KERN_LEVEL strings onto format strings can result in several versions of nearly identical strings being compiled into the object file. By parameterizing this (that is, adding '%s' into the format string, and putting the level into the string as an argument), it prevents this duplication of format strings. I haven't seen the data on duplication of format strings, and how much this affects it, but little things can add up. Whether it matters in this case depends on whether the format strings that kunit uses are also used elsewhere in the kernel, and whether these same format strings are used with multiple kernel message levels. -- Tim
[PATCH 1/2] pci: Convert to use built-in RCU list checking
CONFIG_PROVE_RCU_LIST requires list_for_each_entry_rcu() to pass a lockdep expression if using srcu or locking for protection. It can only check regular RCU protection, all other protection needs to be passed as lockdep expression. Signed-off-by: Joel Fernandes (Google) --- drivers/pci/controller/vmd.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c index 4575e0c6dc4b..127631d0c6da 100644 --- a/drivers/pci/controller/vmd.c +++ b/drivers/pci/controller/vmd.c @@ -718,7 +718,8 @@ static irqreturn_t vmd_irq(int irq, void *data) int idx; idx = srcu_read_lock(>srcu); - list_for_each_entry_rcu(vmdirq, >irq_list, node) + list_for_each_entry_rcu(vmdirq, >irq_list, node, + srcu_read_lock_held(>srcu)) generic_handle_irq(vmdirq->virq); srcu_read_unlock(>srcu, idx); -- 2.23.0.187.g17f5b7556c-goog
Re: [RESEND PATCH V3 3/8] perf/x86/intel: Support hardware TopDown metrics
Hi, On Mon, Aug 26, 2019 at 7:48 AM wrote: > > From: Kan Liang > > Intro > = > > Icelake has support for measuring the four top level TopDown metrics > directly in hardware. This is implemented by an additional "metrics" > register, and a new Fixed Counter 3 that measures pipeline "slots". > > Events > == > > We export four metric events as separate perf events, which map to > internal "metrics" counter register. Those events do not exist in > hardware, but can be allocated by the scheduler. > There is another approach possible for supporting Topdown-style counters. Instead of trying to abstract them as separate events to the user and then trying to put them back together in the kernel and then using slots to scale them as counts, we could just expose them as is, i.e., structured counter values. The kernel already handles structured counter configs and exports the fields on the config via sysfs and the perf tool picks them up and can encode any event. We could have a similar approach for a counter value. It could have fields, unit, types. Perf stat would pick them up in the same manner. It would greatly simplify the kernel implementation. You would need to publish an pseudo-event code for each group of metrics. Note that I am not advocating expose the raw counter value. That way you would maintain one event code -> one "counter" on hw. The reset on read would also work. It would generate only one rdmsr per read without forcing any grouping. You would not need any grouping, or using slots under the hood to scale. If PERF_METRICS gets extended, you can just add another pseudo event code or umask. The PERF_METRICS events do not make real sense in isolation. The SLOTS scaling is hard to interpret. You can never profiling on PERF_METRICS event so keeping them grouped is okay. > For the event mapping we use a special 0x00 event code, which is > reserved for fake events. The metric events start from umask 0x10. > > When setting up such events they point to the slots counter, and a > special callback, update_topdown_event(), reads the additional metrics > msr to generate the metrics. Then the metric is reported by multiplying > the metric (percentage) with slots. > > This multiplication allows to easily keep a running count, for example > when the slots counter overflows, and makes all the standard tools, such > as a perf stat, work. They can do deltas of the values without needing > to know about percentages. This also simplifies accumulating the counts > of child events, which otherwise would need to know how to average > percent values. > > All four metric events don't support sampling. Since they will be > handled specially for event update, a flag PERF_X86_EVENT_TOPDOWN is > introduced to indicate this case. > > The slots event can support both sampling and counting. > For counting, the flag is also applied. > For sampling, it will be handled normally as other normal events. > > Groups > == > > To avoid reading the METRICS register multiple times, the metrics and > slots value can only be updated by the first slots/metrics event in a > group. All active slots and metrics events will be updated one time. > > Reset > == > > The PERF_METRICS and Fixed counter 3 have to be reset for each read, > because: > - The 8bit metrics ratio values lose precision when the measurement > period gets longer. > - The PERF_METRICS may report wrong value if its delta was less than > 1/255 of SLOTS (Fixed counter 3). > > Also, for counting, the -max_period is the initial value of the SLOTS. > The huge initial value will definitely trigger the issue mentioned > above. Force initial value as 0 for topdown and slots event counting. > > NMI > == > > The METRICS register may be overflow. The bit 48 of STATUS register > will be set. If so, update all active slots and metrics events. > > The update_topdown_event() has to read two registers separately. The > values may be modify by a NMI. PMU has to be disabled before calling the > function. > > RDPMC > == > > RDPMC is temporarily disabled. The following patch will enable it. > > Originally-by: Andi Kleen > Signed-off-by: Kan Liang > --- > arch/x86/events/core.c | 10 ++ > arch/x86/events/intel/core.c | 230 ++- > arch/x86/events/perf_event.h | 17 +++ > arch/x86/include/asm/msr-index.h | 2 + > 4 files changed, 255 insertions(+), 4 deletions(-) > > diff --git a/arch/x86/events/core.c b/arch/x86/events/core.c > index 54534ff00940..1ae23db5c2d7 100644 > --- a/arch/x86/events/core.c > +++ b/arch/x86/events/core.c > @@ -76,6 +76,8 @@ u64 x86_perf_event_update(struct perf_event *event) > if (idx == INTEL_PMC_IDX_FIXED_BTS) > return 0; > > + if (is_topdown_count(event) && x86_pmu.update_topdown_event) > + return x86_pmu.update_topdown_event(event); > /* > * Careful: an NMI might modify the previous event value. > * > @@ -1003,6 +1005,10
[PATCH 2/2] ipc/sem: Convert to use built-in RCU list checking
CONFIG_PROVE_RCU_LIST requires list_for_each_entry_rcu() to pass a lockdep expression if using srcu or locking for protection. It can only check regular RCU protection, all other protection needs to be passed as lockdep expression. Signed-off-by: Joel Fernandes (Google) --- ipc/sem.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ipc/sem.c b/ipc/sem.c index 7da4504bcc7c..ec97a7072413 100644 --- a/ipc/sem.c +++ b/ipc/sem.c @@ -1852,7 +1852,8 @@ static struct sem_undo *__lookup_undo(struct sem_undo_list *ulp, int semid) { struct sem_undo *un; - list_for_each_entry_rcu(un, >list_proc, list_proc) { + list_for_each_entry_rcu(un, >list_proc, list_proc, + spin_is_locked(>lock)) { if (un->semid == semid) return un; } -- 2.23.0.187.g17f5b7556c-goog
[PATCH v2 6/6] x86: bug.h: use asm_inline in _BUG_FLAGS definitions
This helps preventing a BUG* or WARN* in some static inline from preventing that (or one of its callers) being inlined, so should allow gcc to make better informed inlining decisions. For example, with gcc 9.2, tcp_fastopen_no_cookie() vanishes from net/ipv4/tcp_fastopen.o. It does not itself have any BUG or WARN, but it calls dst_metric() which has a WARN_ON_ONCE - and despite that WARN_ON_ONCE vanishing since the condition is compile-time false, dst_metric() is apparently sufficiently "large" that when it gets inlined into tcp_fastopen_no_cookie(), the latter becomes too large for inlining. Overall, if one asks size(1), .text decreases a little and .data increases by about the same amount (x86-64 defconfig) $ size vmlinux.{before,after} textdata bss dec hex filename 197097265202600 1630280 26542606195020e vmlinux.before 197093305203068 1630280 265426781950256 vmlinux.after while bloat-o-meter says add/remove: 10/28 grow/shrink: 103/51 up/down: 3669/-2854 (815) ... Total: Before=14783683, After=14784498, chg +0.01% Signed-off-by: Rasmus Villemoes --- arch/x86/include/asm/bug.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/arch/x86/include/asm/bug.h b/arch/x86/include/asm/bug.h index 6804d6642767..facba9bc30ca 100644 --- a/arch/x86/include/asm/bug.h +++ b/arch/x86/include/asm/bug.h @@ -32,7 +32,7 @@ #define _BUG_FLAGS(ins, flags) \ do { \ - asm volatile("1:\t" ins "\n"\ + asm_inline volatile("1:\t" ins "\n" \ ".pushsection __bug_table,\"aw\"\n"\ "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \ "\t" __BUG_REL(%c0) "\t# bug_entry::file\n" \ @@ -49,7 +49,7 @@ do { \ #define _BUG_FLAGS(ins, flags) \ do { \ - asm volatile("1:\t" ins "\n"\ + asm_inline volatile("1:\t" ins "\n" \ ".pushsection __bug_table,\"aw\"\n"\ "2:\t" __BUG_REL(1b) "\t# bug_entry::bug_addr\n" \ "\t.word %c0""\t# bug_entry::flags\n" \ -- 2.20.1
[PATCH v2 5/6] x86: alternative.h: use asm_inline for all alternative variants
Most, if not all, uses of the alternative* family just provide one or two instructions in .text, but the string literal can be quite large, causing gcc to overestimate the size of the generated code. That in turn affects its decisions about inlining of the function containing the alternative() asm statement. gcc >= 9.1 allows one to overrule the estimated size by using "asm inline" instead of just "asm". So replace asm by the helper asm_inline, which for older gccs just expands to asm. Signed-off-by: Rasmus Villemoes --- arch/x86/include/asm/alternative.h | 14 +++--- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/arch/x86/include/asm/alternative.h b/arch/x86/include/asm/alternative.h index 094fbc9c0b1c..13adca37c99a 100644 --- a/arch/x86/include/asm/alternative.h +++ b/arch/x86/include/asm/alternative.h @@ -201,10 +201,10 @@ static inline int alternatives_text_reserved(void *start, void *end) * without volatile and memory clobber. */ #define alternative(oldinstr, newinstr, feature) \ - asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) : : : "memory") + asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, feature) : : : "memory") #define alternative_2(oldinstr, newinstr1, feature1, newinstr2, feature2) \ - asm volatile(ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2) ::: "memory") + asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, feature1, newinstr2, feature2) ::: "memory") /* * Alternative inline assembly with input. @@ -218,7 +218,7 @@ static inline int alternatives_text_reserved(void *start, void *end) * Leaving an unused argument 0 to keep API compatibility. */ #define alternative_input(oldinstr, newinstr, feature, input...) \ - asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ + asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ : : "i" (0), ## input) /* @@ -231,18 +231,18 @@ static inline int alternatives_text_reserved(void *start, void *end) */ #define alternative_input_2(oldinstr, newinstr1, feature1, newinstr2, \ feature2, input...) \ - asm volatile(ALTERNATIVE_2(oldinstr, newinstr1, feature1,\ + asm_inline volatile(ALTERNATIVE_2(oldinstr, newinstr1, feature1, \ newinstr2, feature2) \ : : "i" (0), ## input) /* Like alternative_input, but with a single output argument */ #define alternative_io(oldinstr, newinstr, feature, output, input...) \ - asm volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ + asm_inline volatile (ALTERNATIVE(oldinstr, newinstr, feature) \ : output : "i" (0), ## input) /* Like alternative_io, but for replacing a direct call with another one. */ #define alternative_call(oldfunc, newfunc, feature, output, input...) \ - asm volatile (ALTERNATIVE("call %P[old]", "call %P[new]", feature) \ + asm_inline volatile (ALTERNATIVE("call %P[old]", "call %P[new]", feature) \ : output : [old] "i" (oldfunc), [new] "i" (newfunc), ## input) /* @@ -253,7 +253,7 @@ static inline int alternatives_text_reserved(void *start, void *end) */ #define alternative_call_2(oldfunc, newfunc1, feature1, newfunc2, feature2, \ output, input...) \ - asm volatile (ALTERNATIVE_2("call %P[old]", "call %P[new1]", feature1,\ + asm_inline volatile (ALTERNATIVE_2("call %P[old]", "call %P[new1]", feature1,\ "call %P[new2]", feature2)\ : output, ASM_CALL_CONSTRAINT \ : [old] "i" (oldfunc), [new1] "i" (newfunc1), \ -- 2.20.1
[PATCH v2 2/6] lib/zstd/mem.h: replace __inline by inline
Currently, compiler_types.h #defines __inline as inline (and further #defines inline to automatically attach some attributes), so this does not change functionality. It serves as preparation for removing the #define of __inline. (Note that if ZSTD_STATIC is expanded somewhere where compiler_types.h has not yet been processed, both __inline and inline both refer to the compiler keyword, so again this does not change anything.) Signed-off-by: Rasmus Villemoes --- lib/zstd/mem.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/zstd/mem.h b/lib/zstd/mem.h index 3a0f34c8706c..739837a59ad6 100644 --- a/lib/zstd/mem.h +++ b/lib/zstd/mem.h @@ -27,7 +27,7 @@ /*- * Compiler specifics **/ -#define ZSTD_STATIC static __inline __attribute__((unused)) +#define ZSTD_STATIC static inline __attribute__((unused)) /*-** * Basic Types -- 2.20.1
[PATCH v2 3/6] compiler_types.h: don't #define __inline
The spellings __inline and __inline__ should be reserved for uses where one really wants to refer to the inline keyword, regardless of whether or not the spelling "inline" has been #defined to something else. Due to use of __inline__ in uapi headers, we can't easily get rid of the definition of __inline__. However, almost all users of __inline has been converted to inline, so we can get rid of that #define. The exception is include/acpi/platform/acintel.h. However, that header is only included when using the intel compiler (does anybody actually build the kernel with that?), and the ACPI_INLINE macro is only used in the definition of utterly trivial stub functions, where I doubt a small change of semantics (lack of __gnu_inline) changes anything. Signed-off-by: Rasmus Villemoes --- include/linux/compiler_types.h | 11 ++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index 599c27b56c29..ee49be6d6088 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -150,8 +150,17 @@ struct ftrace_likely_data { __maybe_unused notrace #endif +/* + * gcc provides both __inline__ and __inline as alternate spellings of + * the inline keyword, though the latter is undocumented. New kernel + * code should only use the inline spelling, but some existing code + * uses __inline__. Since we #define inline above, to ensure + * __inline__ has the same semantics, we need this #define. + * + * However, the spelling __inline is strictly reserved for referring + * to the bare keyword. + */ #define __inline__ inline -#define __inline inline /* * Rather then using noinline to prevent stack consumption, use -- 2.20.1
[PATCH v2 4/6] compiler-gcc.h: add asm_inline definition
This adds an asm_inline macro which expands to "asm inline" [1] when gcc is new enough (>= 9.1), and just asm for older gccs and other compilers. Using asm inline("foo") instead of asm("foo") overrules gcc's heuristic estimate of the size of the code represented by the asm() statement, and makes gcc use the minimum possible size instead. That can in turn affect gcc's inlining decisions. I wasn't sure whether to make this a function-like macro or not - this way, it can be combined with volatile as asm_inline volatile() but perhaps we'd prefer to spell that asm_inline_volatile() anyway. [1] Technically, asm __inline, since both inline and __inline__ are macros that attach various attributes, making gcc barf if one literally does "asm inline()". However, the third spelling __inline is available for referring to the bare keyword. Signed-off-by: Rasmus Villemoes --- include/linux/compiler-gcc.h | 4 include/linux/compiler_types.h | 4 2 files changed, 8 insertions(+) diff --git a/include/linux/compiler-gcc.h b/include/linux/compiler-gcc.h index d7ee4c6bad48..544b87b41b58 100644 --- a/include/linux/compiler-gcc.h +++ b/include/linux/compiler-gcc.h @@ -172,3 +172,7 @@ #endif #define __no_fgcse __attribute__((optimize("-fno-gcse"))) + +#if GCC_VERSION >= 90100 +#define asm_inline asm __inline +#endif diff --git a/include/linux/compiler_types.h b/include/linux/compiler_types.h index ee49be6d6088..ba8d81b716c7 100644 --- a/include/linux/compiler_types.h +++ b/include/linux/compiler_types.h @@ -198,6 +198,10 @@ struct ftrace_likely_data { #define asm_volatile_goto(x...) asm goto(x) #endif +#ifndef asm_inline +#define asm_inline asm +#endif + #ifndef __no_fgcse # define __no_fgcse #endif -- 2.20.1
[PATCH v2 1/6] staging: rtl8723bs: replace __inline by inline
Currently, __inline is #defined as inline in compiler_types.h, so this should not change functionality. It is preparation for removing said #define. While at it, change some "inline static" to the customary "static inline" order. Signed-off-by: Rasmus Villemoes --- drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 4 ++-- drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 2 +- drivers/staging/rtl8723bs/include/drv_types.h| 6 +++--- .../staging/rtl8723bs/include/osdep_service.h| 10 +- .../rtl8723bs/include/osdep_service_linux.h | 14 +++--- drivers/staging/rtl8723bs/include/rtw_mlme.h | 14 +++--- drivers/staging/rtl8723bs/include/rtw_recv.h | 16 drivers/staging/rtl8723bs/include/sta_info.h | 2 +- drivers/staging/rtl8723bs/include/wifi.h | 14 +++--- drivers/staging/rtl8723bs/include/wlan_bssdef.h | 2 +- 10 files changed, 42 insertions(+), 42 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c index ae7fb7046c93..3750fbaeec4f 100644 --- a/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c +++ b/drivers/staging/rtl8723bs/core/rtw_pwrctrl.c @@ -830,12 +830,12 @@ static void pwr_rpwm_timeout_handler(struct timer_list *t) _set_workitem(>rpwmtimeoutwi); } -static __inline void register_task_alive(struct pwrctrl_priv *pwrctrl, u32 tag) +static inline void register_task_alive(struct pwrctrl_priv *pwrctrl, u32 tag) { pwrctrl->alives |= tag; } -static __inline void unregister_task_alive(struct pwrctrl_priv *pwrctrl, u32 tag) +static inline void unregister_task_alive(struct pwrctrl_priv *pwrctrl, u32 tag) { pwrctrl->alives &= ~tag; } diff --git a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c index 76c50377f0fe..34e1ce1b0689 100644 --- a/drivers/staging/rtl8723bs/core/rtw_wlan_util.c +++ b/drivers/staging/rtl8723bs/core/rtw_wlan_util.c @@ -451,7 +451,7 @@ void set_channel_bwmode(struct adapter *padapter, unsigned char channel, unsigne mutex_unlock(&(adapter_to_dvobj(padapter)->setch_mutex)); } -__inline u8 *get_my_bssid(struct wlan_bssid_ex *pnetwork) +inline u8 *get_my_bssid(struct wlan_bssid_ex *pnetwork) { return pnetwork->MacAddress; } diff --git a/drivers/staging/rtl8723bs/include/drv_types.h b/drivers/staging/rtl8723bs/include/drv_types.h index 96346ce064aa..d3648f3b1de2 100644 --- a/drivers/staging/rtl8723bs/include/drv_types.h +++ b/drivers/staging/rtl8723bs/include/drv_types.h @@ -478,7 +478,7 @@ struct sdio_data intf_data; #define dvobj_to_pwrctl(dvobj) (&(dvobj->pwrctl_priv)) #define pwrctl_to_dvobj(pwrctl) container_of(pwrctl, struct dvobj_priv, pwrctl_priv) -__inline static struct device *dvobj_to_dev(struct dvobj_priv *dvobj) +static inline struct device *dvobj_to_dev(struct dvobj_priv *dvobj) { /* todo: get interface type from dvobj and the return the dev accordingly */ #ifdef RTW_DVOBJ_CHIP_HW_TYPE @@ -636,14 +636,14 @@ struct adapter { /* define RTW_DISABLE_FUNC(padapter, func) (atomic_add(_to_dvobj(padapter)->disable_func, (func))) */ /* define RTW_ENABLE_FUNC(padapter, func) (atomic_sub(_to_dvobj(padapter)->disable_func, (func))) */ -__inline static void RTW_DISABLE_FUNC(struct adapter *padapter, int func_bit) +static inline void RTW_DISABLE_FUNC(struct adapter *padapter, int func_bit) { int df = atomic_read(_to_dvobj(padapter)->disable_func); df |= func_bit; atomic_set(_to_dvobj(padapter)->disable_func, df); } -__inline static void RTW_ENABLE_FUNC(struct adapter *padapter, int func_bit) +static inline void RTW_ENABLE_FUNC(struct adapter *padapter, int func_bit) { int df = atomic_read(_to_dvobj(padapter)->disable_func); df &= ~(func_bit); diff --git a/drivers/staging/rtl8723bs/include/osdep_service.h b/drivers/staging/rtl8723bs/include/osdep_service.h index d2616af95ffa..81a9c19ecc6a 100644 --- a/drivers/staging/rtl8723bs/include/osdep_service.h +++ b/drivers/staging/rtl8723bs/include/osdep_service.h @@ -110,12 +110,12 @@ int _rtw_netif_rx(_nic_hdl ndev, struct sk_buff *skb); extern void _rtw_init_queue(struct __queue *pqueue); -static __inline void thread_enter(char *name) +static inline void thread_enter(char *name) { allow_signal(SIGTERM); } -__inline static void flush_signals_thread(void) +static inline void flush_signals_thread(void) { if (signal_pending (current)) { @@ -125,7 +125,7 @@ __inline static void flush_signals_thread(void) #define rtw_warn_on(condition) WARN_ON(condition) -__inline static int rtw_bug_check(void *parg1, void *parg2, void *parg3, void *parg4) +static inline int rtw_bug_check(void *parg1, void *parg2, void *parg3, void *parg4) { int ret = true; @@ -136,7 +136,7 @@ __inline static int rtw_bug_check(void *parg1, void *parg2, void *parg3, void *p #define _RND(sz, r)
[PATCH v2 0/6] make use of gcc 9's "asm inline()"
gcc 9 provides a way to override the otherwise crude heuristic that gcc uses to estimate the size of the code represented by an asm() statement. From the gcc docs If you use 'asm inline' instead of just 'asm', then for inlining purposes the size of the asm is taken as the minimum size, ignoring how many instructions GCC thinks it is. For compatibility with older compilers, we obviously want a #if new enough #define asm_inline asm inline #else #define asm_inline asm #endif But since we #define the identifier inline to attach some attributes, we have to use an alternate spelling of that keyword. gcc provides both __inline__ and __inline, and we currently #define both to inline, so they all have the same semantics. We have to free up one of __inline__ and __inline, and the latter is by far the easiest. The two x86 changes cause smaller code gen differences than I'd expect, but I think we do want the asm_inline thing available sooner or later, so this is just to get the ball rolling. Changes since v1: __inline instead of __inline__, making the diffstat 400 lines smaller. Probably no longer needs special handling (having Linus run a script to generate patch 1), so if the x86 folks want 5/6 and 6/6, perhaps the whole thing can be routed that way. Rasmus Villemoes (6): staging: rtl8723bs: replace __inline by inline lib/zstd/mem.h: replace __inline by inline compiler_types.h: don't #define __inline compiler-gcc.h: add asm_inline definition x86: alternative.h: use asm_inline for all alternative variants x86: bug.h: use asm_inline in _BUG_FLAGS definitions arch/x86/include/asm/alternative.h | 14 +++--- arch/x86/include/asm/bug.h | 4 ++-- drivers/staging/rtl8723bs/core/rtw_pwrctrl.c | 4 ++-- drivers/staging/rtl8723bs/core/rtw_wlan_util.c | 2 +- drivers/staging/rtl8723bs/include/drv_types.h| 6 +++--- .../staging/rtl8723bs/include/osdep_service.h| 10 +- .../rtl8723bs/include/osdep_service_linux.h | 14 +++--- drivers/staging/rtl8723bs/include/rtw_mlme.h | 14 +++--- drivers/staging/rtl8723bs/include/rtw_recv.h | 16 drivers/staging/rtl8723bs/include/sta_info.h | 2 +- drivers/staging/rtl8723bs/include/wifi.h | 14 +++--- drivers/staging/rtl8723bs/include/wlan_bssdef.h | 2 +- include/linux/compiler-gcc.h | 4 include/linux/compiler_types.h | 15 ++- lib/zstd/mem.h | 2 +- 15 files changed, 70 insertions(+), 53 deletions(-) -- 2.20.1
[PATCH] platform/chrome: chromeos_tbmc : Report wake events.
Mark chromeos_tbmc as wake capable and report wake events. This helps to abort suspend on seeing a tablet mode switch event when kernel is suspending. This also helps identifying if chroemos_tbmc is the wake source. Signed-off-by: Ravi Chandra Sadineni --- drivers/platform/chrome/chromeos_tbmc.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/platform/chrome/chromeos_tbmc.c b/drivers/platform/chrome/chromeos_tbmc.c index ce259ec9f990..d1cf8f3463ce 100644 --- a/drivers/platform/chrome/chromeos_tbmc.c +++ b/drivers/platform/chrome/chromeos_tbmc.c @@ -47,6 +47,7 @@ static __maybe_unused int chromeos_tbmc_resume(struct device *dev) static void chromeos_tbmc_notify(struct acpi_device *adev, u32 event) { + acpi_pm_wakeup_event(>dev); switch (event) { case 0x80: chromeos_tbmc_query_switch(adev, adev->driver_data); @@ -90,6 +91,7 @@ static int chromeos_tbmc_add(struct acpi_device *adev) dev_err(dev, "cannot register input device\n"); return ret; } + device_init_wakeup(dev, true); return 0; } -- 2.21.0
Re: [RFC PATCH 0/2] Add support for SBI version to 0.2
On Thu, 2019-08-29 at 03:59 -0700, h...@infradead.org wrote: > On Tue, Aug 27, 2019 at 10:19:42PM +, Atish Patra wrote: > > I did not understand this part. All the legacy SBI calls are > > defined as > > a separate extension ID not single extension. How did it break the > > backward compatibility ? > > Yes, sorry I mistead this. The way is is defined is rather > non-intuitive, but actually backwards compatible. > > > I think the confusion is because of legacy renaming. They are not > > single legacy extension. They are all separate extensions. The spec > > just called all those extensions as collectively as legacy. So I > > just > > tried to make the patch sync with the spec. > > > > If that's the source of confusion, I can rename it to sbi_0.1_x in > > stead of legacy. > > I think we actually need to fix the spec instead, even if it just the > naming and not the mechanism. > If I understood you clearly, you want to call it legacy in the spec and just say v0.1 extensions. The whole idea of marking them as legacy extensions to indicate that it would be obsolete in the future. But I am not too worried about the semantics here. So I am fine with just changing the text to v0.1 if that avoids confusion. > > > (1) actually board specific and have not place in a cpu > > > abstraction > > > layer: getchar/putchar, these should just never be > > > advertised in > > > a > > > non-legacy setup, and the drivers using them should not > > > probe > > > on a sbi 0.2+ system > > > > In that case, we have to update the drivers(earlycon-riscv-sbi & > > hvc_riscv_sbi) in kernel as well. Once these patches are merged, > > nobody > > will be able to use earlycon=sbi feature in mainline kernel. > > > > Personally, I am fine with it. But there were some interest during > > RISC-V workshop in keeping these for now for easy debugging and > > early > > bringup. > > The getchar/putchar calls unfortunately are fundamentally flawed, as > they mean the sbi can still access the console after the host has > taken > it over using its own drivers. Which will lead to bugs sooner or > later. > > And if you can bring up a console driver in opensbi it should be just > as trivial to bring up the kernel version. -- Regards, Atish
RE: [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF NIC
> -Original Message- > From: Jakub Kicinski > Sent: Friday, August 30, 2019 4:05 PM > To: Haiyang Zhang > Cc: sas...@kernel.org; linux-hyp...@vger.kernel.org; > net...@vger.kernel.org; KY Srinivasan ; Stephen > Hemminger ; o...@aepfle.de; vkuznets > ; da...@davemloft.net; linux- > ker...@vger.kernel.org; Mark Bloch > Subject: Re: [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF > NIC > > On Fri, 30 Aug 2019 03:45:38 +, Haiyang Zhang wrote: > > VF NIC may go down then come up during host servicing events. This > > causes the VF NIC offloading feature settings to roll back to the > > defaults. This patch can synchronize features from synthetic NIC to > > the VF NIC during ndo_set_features (ethtool -K), and > > netvsc_register_vf when VF comes back after host events. > > > > Signed-off-by: Haiyang Zhang > > Cc: Mark Bloch > > If we want to make this change in behaviour we should change net_failover > at the same time. I will check net_failover. Thanks.
Re: [PATCH net-next, 1/2] hv_netvsc: Allow scatter-gather feature to be tunable
On Fri, 30 Aug 2019 03:45:24 +, Haiyang Zhang wrote: > In a previous patch, the NETIF_F_SG was missing after the code changes. > That caused the SG feature to be "fixed". This patch includes it into > hw_features, so it is tunable again. > > Fixes:23312a3be999 ("netvsc: negotiate checksum and segmentation > parameters") ^ Looks like a tab sneaked in there. > Signed-off-by: Haiyang Zhang
Re: [PATCH net-next, 2/2] hv_netvsc: Sync offloading features to VF NIC
On Fri, 30 Aug 2019 03:45:38 +, Haiyang Zhang wrote: > VF NIC may go down then come up during host servicing events. This > causes the VF NIC offloading feature settings to roll back to the > defaults. This patch can synchronize features from synthetic NIC to > the VF NIC during ndo_set_features (ethtool -K), > and netvsc_register_vf when VF comes back after host events. > > Signed-off-by: Haiyang Zhang > Cc: Mark Bloch If we want to make this change in behaviour we should change net_failover at the same time.
Re: [PATCH v2] kunit: fix failure to build without printk
On Fri, Aug 30, 2019 at 3:46 PM Joe Perches wrote: > > On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > > From: Joe Perches > [] > > IMHO %pV should be avoided if possible. Just because people are > > doing it doesn't mean it should be used when it is not necessary. > > Well, as the guy that created %pV, I of course > have a different opinion. > > > > then wouldn't it be easier to pass in the > > > > kernel level as a separate parameter and then strip off all printk > > > > headers like this: > > > > > > Depends on whether or not you care for overall > > > object size. Consolidated formats with the > > > embedded KERN_ like suggested are smaller > > > overall object size. > > > > This is an argument I can agree with. I'm generally in favor of > > things that lessen kernel size creep. :-) > > As am I. Sorry, to be clear, we are talking about the object size penalty due to adding a single parameter to a function. Is that right?
[PATCH] ASoC: fsl_sai: Set SAI Channel Mode to Output Mode
>From SAI datasheet: CHMOD, configures if transmit data pins are configured for TDM mode or Output mode. * (0) TDM mode, transmit data pins are tri-stated when slots are masked or channels are disabled. * (1) Output mode, transmit data pins are never tri-stated and will output zero when slots are masked or channels are disabled. When data pins are tri-stated, there is noise on some channels when FS clock value is high and data is read while fsclk is transitioning from high to low. Fix this by setting CHMOD to Output Mode so that pins will output zero when slots are masked or channels are disabled. Cc: NXP Linux Team Signed-off-by: Cosmin-Gabriel Samoila Signed-off-by: Daniel Baluta --- sound/soc/fsl/fsl_sai.c | 15 --- sound/soc/fsl/fsl_sai.h | 2 ++ 2 files changed, 14 insertions(+), 3 deletions(-) diff --git a/sound/soc/fsl/fsl_sai.c b/sound/soc/fsl/fsl_sai.c index e896b577b1f7..b9daab0eb6eb 100644 --- a/sound/soc/fsl/fsl_sai.c +++ b/sound/soc/fsl/fsl_sai.c @@ -467,6 +467,12 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, val_cr4 |= FSL_SAI_CR4_FRSZ(slots); + /* +* set CHMOD to Output Mode so that transmit data pins will +* output zero when slots are masked or channels are disabled +*/ + val_cr4 |= FSL_SAI_CR4_CHMOD; + /* * For SAI master mode, when Tx(Rx) sync with Rx(Tx) clock, Rx(Tx) will * generate bclk and frame clock for Tx(Rx), we should set RCR4(TCR4), @@ -477,7 +483,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, if (!sai->is_slave_mode) { if (!sai->synchronous[TX] && sai->synchronous[RX] && !tx) { regmap_update_bits(sai->regmap, FSL_SAI_TCR4(ofs), - FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK, + FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK | + FSL_SAI_CR4_CHMOD_MASK, val_cr4); regmap_update_bits(sai->regmap, FSL_SAI_TCR5(ofs), FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK | @@ -486,7 +493,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, ~0UL - ((1 << channels) - 1)); } else if (!sai->synchronous[RX] && sai->synchronous[TX] && tx) { regmap_update_bits(sai->regmap, FSL_SAI_RCR4(ofs), - FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK, + FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK | + FSL_SAI_CR4_CHMOD_MASK, val_cr4); regmap_update_bits(sai->regmap, FSL_SAI_RCR5(ofs), FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK | @@ -497,7 +505,8 @@ static int fsl_sai_hw_params(struct snd_pcm_substream *substream, } regmap_update_bits(sai->regmap, FSL_SAI_xCR4(tx, ofs), - FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK, + FSL_SAI_CR4_SYWD_MASK | FSL_SAI_CR4_FRSZ_MASK | + FSL_SAI_CR4_CHMOD_MASK, val_cr4); regmap_update_bits(sai->regmap, FSL_SAI_xCR5(tx, ofs), FSL_SAI_CR5_WNW_MASK | FSL_SAI_CR5_W0W_MASK | diff --git a/sound/soc/fsl/fsl_sai.h b/sound/soc/fsl/fsl_sai.h index f96f8d97489d..1e3b4a6889a8 100644 --- a/sound/soc/fsl/fsl_sai.h +++ b/sound/soc/fsl/fsl_sai.h @@ -119,6 +119,8 @@ #define FSL_SAI_CR4_FRSZ_MASK (0x1f << 16) #define FSL_SAI_CR4_SYWD(x)(((x) - 1) << 8) #define FSL_SAI_CR4_SYWD_MASK (0x1f << 8) +#define FSL_SAI_CR4_CHMOD BIT(5) +#define FSL_SAI_CR4_CHMOD_MASK GENMASK(5, 5) #define FSL_SAI_CR4_MF BIT(4) #define FSL_SAI_CR4_FSEBIT(3) #define FSL_SAI_CR4_FSPBIT(1) -- 2.17.1
Re: [PATCH] printf: add support for printing symbolic error codes
On 31/08/2019 00.21, Joe Perches wrote: > On Sat, 2019-08-31 at 00:03 +0200, Rasmus Villemoes wrote: >> On 30/08/2019 23.53, Joe Perches wrote: diff --git a/lib/vsprintf.c b/lib/vsprintf.c >>> [] @@ -2178,8 +2204,6 @@ char *pointer(const char *fmt, char *buf, char *end, void *ptr, return flags_string(buf, end, ptr, spec, fmt); case 'O': return kobject_string(buf, end, ptr, spec, fmt); - case 'x': - return pointer_string(buf, end, ptr, spec); } /* default is to _not_ leak addresses, hash before printing */ >>> >>> why remove this? >>> >> >> The handling of %px is moved above the test for ptr being an ERR_PTR, so >> that %px, ptr continues to be (roughly) equivalent to %08lx, (long)ptr. > > Ah. > Pity the flow of the switch/case is disrupted. Agree, but I don't think it's that bad. > That now deserves a separate comment. You mean a comment like /* %px means the user explicitly wanted the pointer formatted as a hex value. */. Or do you want (the|an additional) comment somewhere inside the switch()? > But why not just extend check_pointer_msg? Partly because that would rely on all %p actually eventually passing ptr through to that (notably plain %p does not), partly because the way check_pointer_msg works means that it has to return a string for its caller to print - which is ok when the errcode is found, but breaks if it needs to format a decimal. It can't even snprintf() to a stack buffer and return that, because, well, you can't do that, and it would be a silly recursive snprintf anyway. OK, so perhaps you meant check_pointer() where it might be doable, but again, with plain %p and possibly others we don't get to that. Rasmus
Re: [PATCH 3/9] perf tools: Basic support for CGROUP event
Hi Arnaldo, On Fri, Aug 30, 2019 at 9:55 PM Arnaldo Carvalho de Melo wrote: > > Em Wed, Aug 28, 2019 at 04:31:24PM +0900, Namhyung Kim escreveu: > > Implement basic functionality to support cgroup tracking. Each cgroup > > can be identified by inode number which can be read from userspace > > too. The actual cgroup processing will come in the later patch. > > > > Cc: Adrian Hunter > > Signed-off-by: Namhyung Kim > > --- > > tools/include/uapi/linux/perf_event.h | 17 +++-- > > Try to have the synchronization of tools/include/ with the kernel to be > done in a separate patch, please, there is a number of projects such as > libbpf and in time libperf that will have a mirror repo outside the > kernel tree and that will get just the needed csets. OK, will do that in v2. Thanks, Namhyung
Re: [PATCH 1/9] perf/core: Add PERF_RECORD_CGROUP event
On Fri, Aug 30, 2019 at 4:35 PM Peter Zijlstra wrote: > > On Fri, Aug 30, 2019 at 12:46:51PM +0900, Namhyung Kim wrote: > > Hi Peter, > > > > On Wed, Aug 28, 2019 at 6:45 PM Peter Zijlstra wrote: > > > > > > On Wed, Aug 28, 2019 at 04:31:22PM +0900, Namhyung Kim wrote: > > > > To support cgroup tracking, add CGROUP event to save a link between > > > > cgroup path and inode number. The attr.cgroup bit was also added to > > > > enable cgroup tracking from userspace. > > > > > > > > This event will be generated when a new cgroup becomes active. > > > > Userspace might need to synthesize those events for existing cgroups. > > > > > > > > As aux_output change is also going on, I just added the bit here as > > > > well to remove possible conflicts later. > > > > > > Why do we want this? > > > > I saw below [1] and thought you have the patch introduced aux_output > > and it's gonna to be merged soon. > > Also the tooling patches are already in the acme/perf/core > > so I just wanted to avoid conflicts. > > > > Anyway, I'm ok with changing it. Will remove in v2. > > I seem to have confused both you and Arnaldo with this. This email > questions the Changelog as a whole, not just the aux thing (I send a > separate email for that). > > This Changelog utterly fails to explain to me _why_ we need/want cgroup > tracking. So why do I want to review and possibly merge this? Changelog > needs to answer this. OK. How about this? Systems running a large number of jobs in different cgroups want to profiling such jobs precisely. This includes container hosting systems widely used today. Currently perf supports namespace tracking but the systems may not use (cgroup) namespace for their jobs. Also it'd be more intuitive to see cgroup names (as they're given by user or sysadmin) rather than numeric cgroup/namespace id even if they use the namespaces. Thanks, Namhyung
Re: [PATCH v2] kunit: fix failure to build without printk
On Fri, 2019-08-30 at 21:58 +, tim.b...@sony.com wrote: > > From: Joe Perches [] > IMHO %pV should be avoided if possible. Just because people are > doing it doesn't mean it should be used when it is not necessary. Well, as the guy that created %pV, I of course have a different opinion. > > then wouldn't it be easier to pass in the > > > kernel level as a separate parameter and then strip off all printk > > > headers like this: > > > > Depends on whether or not you care for overall > > object size. Consolidated formats with the > > embedded KERN_ like suggested are smaller > > overall object size. > > This is an argument I can agree with. I'm generally in favor of > things that lessen kernel size creep. :-) As am I.
Re: [PATCH 0/3] tracing: fix minor build warnings
On Fri, 30 Aug 2019 18:01:50 -0400, Steven Rostedt wrote: > On Tue, 27 Aug 2019 22:25:46 -0700 > Jakub Kicinski wrote: > > > Hi! > > > > trace.o gets rebuild on every make run when tracing is enabled, > > which makes all warnings particularly noisy. This patchset fixes > > some low-hanging fruit on W=1 C=1 builds. > > > > Jakub Kicinski (3): > > tracing: correct kdoc formats > > I took the first one, but the two below, I wont take. Those changes > were added in preparation of the kernel access to tracing code. > > See: > > > http://lkml.kernel.org/r/1565805327-579-1-git-send-email-divya.i...@oracle.com Sounds good, thanks!
Re: WARNING: ODEBUG bug in ext4_fill_super
#syz invalid This has already been fixed in the latest linux-next
[PATCH] selftests: watchdog: cleanup whitespace in usage options
Convert hard spaces to tabs in usage options. Suggested-by: Shuah Khan Signed-off-by: George G. Davis --- tools/testing/selftests/watchdog/watchdog-test.c | 25 1 file changed, 13 insertions(+), 12 deletions(-) diff --git a/tools/testing/selftests/watchdog/watchdog-test.c b/tools/testing/selftests/watchdog/watchdog-test.c index c35989ffbc6b..3fff1ee20a7f 100644 --- a/tools/testing/selftests/watchdog/watchdog-test.c +++ b/tools/testing/selftests/watchdog/watchdog-test.c @@ -70,18 +70,19 @@ static void term(int sig) static void usage(char *progname) { printf("Usage: %s [options]\n", progname); - printf(" -f, --file Open watchdog device file\n"); - printf(" Default is /dev/watchdog\n"); - printf(" -b, --bootstatusGet last boot status (Watchdog/POR)\n"); - printf(" -d, --disable Turn off the watchdog timer\n"); - printf(" -e, --enableTurn on the watchdog timer\n"); - printf(" -h, --help Print the help message\n"); - printf(" -p, --pingrate=PSet ping rate to P seconds (default %d)\n", DEFAULT_PING_RATE); - printf(" -t, --timeout=T Set timeout to T seconds\n"); - printf(" -T, --gettimeoutGet the timeout\n"); - printf(" -n, --pretimeout=T Set the pretimeout to T seconds\n"); - printf(" -N, --getpretimeout Get the pretimeout\n"); - printf(" -L, --gettimeleft Get the time left until timer expires\n"); + printf(" -f, --file\t\tOpen watchdog device file\n"); + printf("\t\t\tDefault is /dev/watchdog\n"); + printf(" -b, --bootstatus\tGet last boot status (Watchdog/POR)\n"); + printf(" -d, --disable\t\tTurn off the watchdog timer\n"); + printf(" -e, --enable\t\tTurn on the watchdog timer\n"); + printf(" -h, --help\t\tPrint the help message\n"); + printf(" -p, --pingrate=P\tSet ping rate to P seconds (default %d)\n", + DEFAULT_PING_RATE); + printf(" -t, --timeout=T\tSet timeout to T seconds\n"); + printf(" -T, --gettimeout\tGet the timeout\n"); + printf(" -n, --pretimeout=T\tSet the pretimeout to T seconds\n"); + printf(" -N, --getpretimeout\tGet the pretimeout\n"); + printf(" -L, --gettimeleft\tGet the time left until timer expires\n"); printf("\n"); printf("Parameters are parsed left-to-right in real-time.\n"); printf("Example: %s -d -t 10 -p 5 -e\n", progname); -- 2.7.4
Re: [PATCH] arm: fix page faults in do_alignment
On Fri, Aug 30, 2019 at 04:02:48PM -0500, Eric W. Biederman wrote: > Russell King - ARM Linux admin writes: > > > On Fri, Aug 30, 2019 at 02:45:36PM -0500, Eric W. Biederman wrote: > >> Russell King - ARM Linux admin writes: > >> > >> > On Fri, Aug 30, 2019 at 09:31:17PM +0800, Jing Xiangfeng wrote: > >> >> The function do_alignment can handle misaligned address for user and > >> >> kernel space. If it is a userspace access, do_alignment may fail on > >> >> a low-memory situation, because page faults are disabled in > >> >> probe_kernel_address. > >> >> > >> >> Fix this by using __copy_from_user stead of probe_kernel_address. > >> >> > >> >> Fixes: b255188 ("ARM: fix scheduling while atomic warning in alignment > >> >> handling code") > >> >> Signed-off-by: Jing Xiangfeng > >> > > >> > NAK. > >> > > >> > The "scheduling while atomic warning in alignment handling code" is > >> > caused by fixing up the page fault while trying to handle the > >> > mis-alignment fault generated from an instruction in atomic context. > >> > > >> > Your patch re-introduces that bug. > >> > >> And the patch that fixed scheduling while atomic apparently introduced a > >> regression. Admittedly a regression that took 6 years to track down but > >> still. > > > > Right, and given the number of years, we are trading one regression for > > a different regression. If we revert to the original code where we > > fix up, we will end up with people complaining about a "new" regression > > caused by reverting the previous fix. Follow this policy and we just > > end up constantly reverting the previous revert. > > > > The window is very small - the page in question will have had to have > > instructions read from it immediately prior to the handler being entered, > > and would have had to be made "old" before subsequently being unmapped. > > > Rather than excessively complicating the code and making it even more > > inefficient (as in your patch), we could instead retry executing the > > instruction when we discover that the page is unavailable, which should > > cause the page to be paged back in. > > My patch does not introduce any inefficiencies. It onlys moves the > check for user_mode up a bit. My patch did duplicate the code. > > > If the page really is unavailable, the prefetch abort should cause a > > SEGV to be raised, otherwise the re-execution should replace the page. > > > > The danger to that approach is we page it back in, and it gets paged > > back out before we're able to read the instruction indefinitely. > > I would think either a little code duplication or a function that looks > at user_mode(regs) and picks the appropriate kind of copy to do would be > the best way to go. Because what needs to happen in the two cases for > reading the instruction are almost completely different. That is what I mean. I'd prefer to avoid that with the large chunk of code. How about instead adding a local replacement for probe_kernel_address() that just sorts out the reading, rather than duplicating all the code to deal with thumb fixup. > > However, as it's impossible for me to contact the submitter, anything > > I do will be poking about in the dark and without any way to validate > > that it does fix the problem, so I think apart from reviewing of any > > patches, there's not much I can do. > > I didn't realize your emails to him were bouncing. That is odd. Mine > don't appear to be. Hmm, so the fact I posted publically in reply to my reply with the MTA bounce message didn't give you a clue? -- RMK's Patch system: https://www.armlinux.org.uk/developer/patches/ FTTC broadband for 0.8mile line in suburbia: sync at 12.1Mbps down 622kbps up According to speedtest.net: 11.9Mbps down 500kbps up
Re: [PATCH] net: stmmac: Variable "val" in function sun8i_dwmac_set_syscon() could be uninitialized
Hi David: Thanks for your feedback, this patch should work for v4.14. On Fri, Feb 8, 2019 at 11:01 PM David Miller wrote: > > From: Yizhuo > Date: Thu, 7 Feb 2019 09:46:23 -0800 > > > In function sun8i_dwmac_set_syscon(), local variable "val" could > > be uninitialized if function regmap_read() returns -EINVAL. > > However, it will be used directly in the if statement, which > > is potentially unsafe. > > > > Signed-off-by: Yizhuo > > This doesn't apply to any of my trees. -- Kind Regards, Yizhuo Zhai Computer Science, Graduate Student University of California, Riverside
Re: [PATCH] mm/memcg: return value of the function mem_cgroup_from_css() is not checked
Our tool did not trace back the whole path, so, now we could say it might happen. On Thu, Aug 22, 2019 at 1:12 PM Michal Hocko wrote: > > On Thu 22-08-19 13:07:17, Yizhuo Zhai wrote: > > This will happen if variable "wb->memcg_css" is NULL. This case is reported > > by our analysis tool. > > Does your tool report the particular call path and conditions when that > happen? Or is it just a "it mignt happen" kinda thing? > > > Since the function mem_cgroup_wb_domain() is visible to the global, we > > cannot control caller's behavior. > > I am sorry but I do not understand what is this supposed to mean. > -- > Michal Hocko > SUSE Labs -- Kind Regards, Yizhuo Zhai Computer Science, Graduate Student University of California, Riverside
Re: [PATCH v2 3/3] nvme: fire discovery log page change events to userspace
On 8/30/2019 2:07 PM, Sagi Grimberg wrote: Yes we do, userspace should use it to order events. Does udev not handle that properly today? The problem is not ordering of events, its really about the fact that the chardev can be removed and reallocated for a different controller (could be a completely different discovery controller) by the time that userspace handles the event. The same is generally true for lot of kernel devices. We could reduce the chance by using the idr cyclic allocator. Well, it was raised by Hannes and James, so I'll ask them respond here because I don't mind having it this way. I personally think that this is a better approach than having a cyclic idr allocator. In general, I don't necessarily think that this is a good idea to have cyclic controller enumerations if we don't absolutely have to... We hit it right and left without the cyclic allocator, but that won't necessarily remove it. Perhaps we should have had a unique token assigned to the controller, and have the event pass the name and the token. The cli would then, if the token is present, validate it via an ioctl before proceeding with other ioctls. Where all the connection arguments were added we due to the reuse issue and then solving the question of how to verify and/or lookup the desired controller, by using the shotgun approach rather than being very pointed, which is what the name/token would do. This unique token is: trtype:traddr:trsvcid:host-traddr ... well yes :) though rather verbose. There is still a minute window as we're comparing values in sysfs, prior to opening the device, so technically something could change in that window between when we checked sysfs and when we open'd. We can certain check after we open the device to solve that issue. There is some elegance to a 32-bit token for the controller (can be an incrementing value) passed in the event and used when servicing the event that avoids a bunch of work. Doing either of these would eliminate what Hannes liked - looking for the discovery controller with those attributes. Although, I don't know that looking for it is all that meaningful.
Re: [PATCH] printf: add support for printing symbolic error codes
On Sat, 2019-08-31 at 00:03 +0200, Rasmus Villemoes wrote: > On 30/08/2019 23.53, Joe Perches wrote: > > > diff --git a/lib/vsprintf.c b/lib/vsprintf.c > > [] > > > @@ -2178,8 +2204,6 @@ char *pointer(const char *fmt, char *buf, char > > > *end, void *ptr, > > > return flags_string(buf, end, ptr, spec, fmt); > > > case 'O': > > > return kobject_string(buf, end, ptr, spec, fmt); > > > - case 'x': > > > - return pointer_string(buf, end, ptr, spec); > > > } > > > > > > /* default is to _not_ leak addresses, hash before printing */ > > > > why remove this? > > > > The handling of %px is moved above the test for ptr being an ERR_PTR, so > that %px, ptr continues to be (roughly) equivalent to %08lx, (long)ptr. Ah. Pity the flow of the switch/case is disrupted. That now deserves a separate comment. But why not just extend check_pointer_msg?
Re: [PATCH v1 2/4] perf vendor events intel: Update cascadelakex uncore events to v1.04
On 8/31/2019 2:22 AM, Arnaldo Carvalho de Melo wrote: Em Wed, Aug 28, 2019 at 01:59:30PM +0800, Jin Yao escreveu: From: Haiyan Song Not applying, please check, will apply the other 3 patches in the series, next time please try to collect some Acked-by in advance. - Arnaldo Hi Arnaldo, Sorry, any issue you found when applying this patch? Is it truncated by the email system? Or the applying is no problem at your side but this patch needs some Acked-by before applying, right? Thanks Jin Yao
[PATCH v3 10/16] ARM: mmp: define MMP_CHIPID by the means of CIU_REG()
A rather trivial cosmetic improvement. Signed-off-by: Lubomir Rintel --- arch/arm/mach-mmp/common.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/arch/arm/mach-mmp/common.c b/arch/arm/mach-mmp/common.c index 2ee08c78e8bc9..24c689a01ecb7 100644 --- a/arch/arm/mach-mmp/common.c +++ b/arch/arm/mach-mmp/common.c @@ -17,7 +17,7 @@ #include "common.h" -#define MMP_CHIPID (AXI_VIRT_BASE + 0x82c00) +#define MMP_CHIPID CIU_REG(0x00) unsigned int mmp_chip_id; EXPORT_SYMBOL(mmp_chip_id); -- 2.21.0
[PATCH v3 14/16] ARM: mmp: remove MMP3 USB PHY registers from regs-usb.h
Nothing in mach-mmp/ uses them and they belong to the PHY driver. Signed-off-by: Lubomir Rintel --- arch/arm/mach-mmp/regs-usb.h | 94 1 file changed, 94 deletions(-) diff --git a/arch/arm/mach-mmp/regs-usb.h b/arch/arm/mach-mmp/regs-usb.h index d9f08c1601542..ed0d1aa0ad6c9 100644 --- a/arch/arm/mach-mmp/regs-usb.h +++ b/arch/arm/mach-mmp/regs-usb.h @@ -121,100 +121,6 @@ #define UTMI_OTG_ADDON_OTG_ON (1 << 0) -/* For MMP3 USB Phy */ -#define USB2_PLL_REG0 0x4 -#define USB2_PLL_REG1 0x8 -#define USB2_TX_REG0 0x10 -#define USB2_TX_REG1 0x14 -#define USB2_TX_REG2 0x18 -#define USB2_RX_REG0 0x20 -#define USB2_RX_REG1 0x24 -#define USB2_RX_REG2 0x28 -#define USB2_ANA_REG0 0x30 -#define USB2_ANA_REG1 0x34 -#define USB2_ANA_REG2 0x38 -#define USB2_DIG_REG0 0x3C -#define USB2_DIG_REG1 0x40 -#define USB2_DIG_REG2 0x44 -#define USB2_DIG_REG3 0x48 -#define USB2_TEST_REG0 0x4C -#define USB2_TEST_REG1 0x50 -#define USB2_TEST_REG2 0x54 -#define USB2_CHARGER_REG0 0x58 -#define USB2_OTG_REG0 0x5C -#define USB2_PHY_MON0 0x60 -#define USB2_RESETVE_REG0 0x64 -#define USB2_ICID_REG0 0x78 -#define USB2_ICID_REG1 0x7C - -/* USB2_PLL_REG0 */ -/* This is for Ax stepping */ -#define USB2_PLL_FBDIV_SHIFT_MMP3 0 -#define USB2_PLL_FBDIV_MASK_MMP3 (0xFF << 0) - -#define USB2_PLL_REFDIV_SHIFT_MMP3 8 -#define USB2_PLL_REFDIV_MASK_MMP3 (0xF << 8) - -#define USB2_PLL_VDD12_SHIFT_MMP3 12 -#define USB2_PLL_VDD18_SHIFT_MMP3 14 - -/* This is for B0 stepping */ -#define USB2_PLL_FBDIV_SHIFT_MMP3_B0 0 -#define USB2_PLL_REFDIV_SHIFT_MMP3_B0 9 -#define USB2_PLL_VDD18_SHIFT_MMP3_B0 14 -#define USB2_PLL_FBDIV_MASK_MMP3_B00x01FF -#define USB2_PLL_REFDIV_MASK_MMP3_B0 0x3E00 - -#define USB2_PLL_CAL12_SHIFT_MMP3 0 -#define USB2_PLL_CALI12_MASK_MMP3 (0x3 << 0) - -#define USB2_PLL_VCOCAL_START_SHIFT_MMP3 2 - -#define USB2_PLL_KVCO_SHIFT_MMP3 4 -#define USB2_PLL_KVCO_MASK_MMP3(0x7<<4) - -#define USB2_PLL_ICP_SHIFT_MMP38 -#define USB2_PLL_ICP_MASK_MMP3 (0x7<<8) - -#define USB2_PLL_LOCK_BYPASS_SHIFT_MMP312 - -#define USB2_PLL_PU_PLL_SHIFT_MMP3 13 -#define USB2_PLL_PU_PLL_MASK (0x1 << 13) - -#define USB2_PLL_READY_MASK_MMP3 (0x1 << 15) - -/* USB2_TX_REG0 */ -#define USB2_TX_IMPCAL_VTH_SHIFT_MMP3 8 -#define USB2_TX_IMPCAL_VTH_MASK_MMP3 (0x7 << 8) - -#define USB2_TX_RCAL_START_SHIFT_MMP3 13 - -/* USB2_TX_REG1 */ -#define USB2_TX_CK60_PHSEL_SHIFT_MMP3 0 -#define USB2_TX_CK60_PHSEL_MASK_MMP3 (0xf << 0) - -#define USB2_TX_AMP_SHIFT_MMP3 4 -#define USB2_TX_AMP_MASK_MMP3 (0x7 << 4) - -#define USB2_TX_VDD12_SHIFT_MMP3 8 -#define USB2_TX_VDD12_MASK_MMP3(0x3 << 8) - -/* USB2_TX_REG2 */ -#define USB2_TX_DRV_SLEWRATE_SHIFT 10 - -/* USB2_RX_REG0 */ -#define USB2_RX_SQ_THRESH_SHIFT_MMP3 4 -#define USB2_RX_SQ_THRESH_MASK_MMP3(0xf << 4) - -#define USB2_RX_SQ_LENGTH_SHIFT_MMP3 10 -#define USB2_RX_SQ_LENGTH_MASK_MMP3(0x3 << 10) - -/* USB2_ANA_REG1*/ -#define USB2_ANA_PU_ANA_SHIFT_MMP3 14 - -/* USB2_OTG_REG0 */ -#define USB2_OTG_PU_OTG_SHIFT_MMP3 3 - /* fsic registers */ #define FSIC_MISC 0x4 #define FSIC_INT 0x28 -- 2.21.0
[PATCH v3 05/16] dt-bindings: phy-mmp3-usb: Add bindings
This is the PHY chip for USB OTG on MMP3 platform. Signed-off-by: Lubomir Rintel Reviewed-by: Rob Herring --- Changes since v2: - Add Rob's Reviewed-by tag Changes since v1: - s/usbphy@/usb-phy@/ - Dropped a reference to Documentation/phy.txt .../devicetree/bindings/phy/phy-mmp3-usb.txt| 13 + 1 file changed, 13 insertions(+) create mode 100644 Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt diff --git a/Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt b/Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt new file mode 100644 index 0..7183b9102f917 --- /dev/null +++ b/Documentation/devicetree/bindings/phy/phy-mmp3-usb.txt @@ -0,0 +1,13 @@ +Marvell MMP3 USB PHY + + +Required properties: +- compatible: must be "marvell,mmp3-usb-phy" +- #phy-cells: must be 0 + +Example: + usb-phy: usb-phy@d4207000 { + compatible = "marvell,mmp3-usb-phy"; + reg = <0xd4207000 0x40>; + #phy-cells = <0>; + }; -- 2.21.0