Re: [PATCH net v2 1/2] Revert "tuntap: add missing xdp flush"
On 2/22/2018 9:24 AM, Jason Wang wrote: This reverts commit 762c330d670e3d4b795cf7a8d761866fdd1eef49. The reason is we try to batch packets for devmap which causes calling xdp_do_flush() under the process context. Simply disable premmption s/under/in/. Disabling preemption. may not work since process may move among processors which lead xdp_do_flush() to miss some flushes on some processors. So simply revert the patch, a follow-up path will add the xdp flush correctly. Reported-by: Christoffer Dall Fixes: 762c330d670e ("tuntap: add missing xdp flush") Signed-off-by: Jason Wang [...] MBR, Sergei
Re: [PATCH v4 3/4] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP
On 02/22/2018 04:53 AM, Doug Anderson wrote: > Hi, > > On Mon, Feb 19, 2018 at 8:36 AM, Marc Zyngier wrote: >>> + interrupts = ; >> >> Please do not use IRQ_TYPE_NONE, ever. It doesn't exist in the GIC >> binding. Set it to the actual trigger value. >> > >>> + interrupts = ; >> >> Same here. > > Thanks for the review Marc! > > > Andy: If I'm reading everything correctly you're the one who would > collect these patches and apply them. Is that right? Do they look OK > to you in general? Would you prefer that Rajendra send out a v5 with > the fixes pointed out by Marc, or would you prefer to fix them up > yourself when applying? Is now a good time or would you prefer to > wait? I just fixed up to remove all instances of IRQ_TYPE_NONE and sent a v5 out. > > Thanks! :) > > -Doug > > ___ > linux-arm-kernel mailing list > linux-arm-ker...@lists.infradead.org > http://lists.infradead.org/mailman/listinfo/linux-arm-kernel > -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH 08/20] lightnvm: complete geo structure with maxoc*
> On 22 Feb 2018, at 08.45, Matias Bjørling wrote: > > On 02/21/2018 10:26 AM, Javier González wrote: >> Complete the generic geometry structure with the maxoc and maxocpu >> felds, present in the 2.0 spec. >> Signed-off-by: Javier González >> --- >> drivers/nvme/host/lightnvm.c | 4 >> include/linux/lightnvm.h | 2 ++ >> 2 files changed, 6 insertions(+) >> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c >> index cca32da05316..9c1f8225c4e1 100644 >> --- a/drivers/nvme/host/lightnvm.c >> +++ b/drivers/nvme/host/lightnvm.c >> @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id, >> dev_geo->c.ws_min = sec_per_pg; >> dev_geo->c.ws_opt = sec_per_pg; >> dev_geo->c.mw_cunits = 8; /* default to MLC safe values */ >> +dev_geo->c.maxoc = dev_geo->all_luns; /* default to 1 chunk per LUN */ >> +dev_geo->c.maxocpu = 1; /* default to 1 chunk per LUN */ > > One can't assume that it is 1 open chunk per lun. If you need this for > specific hardware, make a quirk for it. > Which default you want for 1.2 if not specified then? I use 1 because it has been the implicit default until now. Javier signature.asc Description: Message signed with OpenPGP
Re: [PATCH 04/20] lightnvm: add minor version to generic geometry
> On 22 Feb 2018, at 08.34, Matias Bjørling wrote: > > On 02/21/2018 10:26 AM, Javier González wrote: >> Separate the version between major and minor on the generic geometry. >> Also, add a "subversion" entry to sysfs to expose the minor version >> without breaking user space. >> Signed-off-by: Javier González >> --- >> drivers/lightnvm/core.c | 4 ++-- >> drivers/nvme/host/lightnvm.c | 25 - >> include/linux/lightnvm.h | 3 ++- >> 3 files changed, 24 insertions(+), 8 deletions(-) >> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c >> index 42596afdf64c..dc9ec6baff45 100644 >> --- a/drivers/lightnvm/core.c >> +++ b/drivers/lightnvm/core.c >> @@ -897,8 +897,8 @@ static int nvm_init(struct nvm_dev *dev) >> goto err; >> } >> - pr_debug("nvm: ver:%u nvm_vendor:%x\n", >> -dev_geo->ver_id, >> +pr_debug("nvm: ver:%u.%u nvm_vendor:%x\n", >> +dev_geo->major_ver_id, dev_geo->minor_ver_id, >> dev_geo->c.vmnt); >> ret = nvm_core_init(dev); >> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c >> index 598abba66f52..71b4ac57a668 100644 >> --- a/drivers/nvme/host/lightnvm.c >> +++ b/drivers/nvme/host/lightnvm.c >> @@ -296,7 +296,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id, >> } >> /* 1.2 spec. only reports a single version id - unfold */ >> -dev_geo->ver_id = id->ver_id; >> +dev_geo->major_ver_id = id->ver_id; >> +dev_geo->minor_ver_id = 2; >> dev_geo->nr_chnls = src->num_ch; >> dev_geo->nr_luns = src->num_lun; >> @@ -377,7 +378,14 @@ static void nvme_nvm_set_addr_20(struct nvm_addr_format >> *dst, >> static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, >> struct nvm_dev_geo *dev_geo) >> { >> -dev_geo->ver_id = id->mjr; >> +dev_geo->major_ver_id = id->mjr; >> +dev_geo->minor_ver_id = id->mnr; >> + >> +if (!(dev_geo->major_ver_id == 2 && dev_geo->minor_ver_id == 0)) { >> +pr_err("nvm: OCSSD version not supported (v%d.%d)\n", >> +dev_geo->major_ver_id, dev_geo->minor_ver_id); >> +return -EINVAL; >> +} >> dev_geo->nr_chnls = le16_to_cpu(id->num_grp); >> dev_geo->nr_luns = le16_to_cpu(id->num_pu); >> @@ -913,7 +921,11 @@ static ssize_t nvm_dev_attr_show(struct device *dev, >> attr = &dattr->attr; >> if (strcmp(attr->name, "version") == 0) { >> -return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->ver_id); >> +return scnprintf(page, PAGE_SIZE, "%u\n", >> +dev_geo->major_ver_id); >> +} else if (strcmp(attr->name, "subversion") == 0) { >> +return scnprintf(page, PAGE_SIZE, "%u\n", >> +dev_geo->minor_ver_id); >> } else if (strcmp(attr->name, "media_capabilities") == 0) { >> return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.mccap); >> } else if (strcmp(attr->name, "read_typ") == 0) { >> @@ -1055,6 +1067,7 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev, >>/* general attributes */ >> static NVM_DEV_ATTR_RO(version); >> +static NVM_DEV_ATTR_RO(subversion); >> static NVM_DEV_ATTR_RO(media_capabilities); >>static NVM_DEV_ATTR_RO(read_typ); >> @@ -1085,6 +1098,7 @@ static NVM_DEV_ATTR_12_RO(max_phys_secs); >>static struct attribute *nvm_dev_attrs_12[] = { >> &dev_attr_version.attr, >> +&dev_attr_subversion.attr, >> &dev_attr_media_capabilities.attr, >> &dev_attr_vendor_opcode.attr, >> @@ -1134,6 +1148,7 @@ static NVM_DEV_ATTR_20_RO(reset_max); >>static struct attribute *nvm_dev_attrs_20[] = { >> &dev_attr_version.attr, >> +&dev_attr_subversion.attr, >> &dev_attr_media_capabilities.attr, >> &dev_attr_groups.attr, >> @@ -1167,7 +1182,7 @@ int nvme_nvm_register_sysfs(struct nvme_ns *ns) >> if (!ndev) >> return -EINVAL; >> - switch (dev_geo->ver_id) { >> +switch (dev_geo->major_ver_id) { >> case 1: >> return sysfs_create_group(&disk_to_dev(ns->disk)->kobj, >> &nvm_dev_attr_group_12); >> @@ -1184,7 +1199,7 @@ void nvme_nvm_unregister_sysfs(struct nvme_ns *ns) >> struct nvm_dev *ndev = ns->ndev; >> struct nvm_dev_geo *dev_geo = &ndev->dev_geo; >> - switch (dev_geo->ver_id) { >> +switch (dev_geo->major_ver_id) { >> case 1: >> sysfs_remove_group(&disk_to_dev(ns->disk)->kobj, >> &nvm_dev_attr_group_12); >> diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h >> index 18e3751b1632..5af0b8837095 100644 >> --- a/include/linux/lightnvm.h >> +++ b/include/linux/lightnvm.h >> @@ -242,7 +242,8 @@ struct nvm_common_geo { >> /* Device identified geometry */ >> struct nvm_dev_geo { >> /* device reported version */ >>
Re: [PATCH net v2 2/2] tuntap: correctly add the missing xdp flush
Hello! On 2/22/2018 9:24 AM, Jason Wang wrote: Commit 762c330d670e ("tuntap: add missing xdp flush") tries to fix the devmap stall caused by missed xdp flush by counting the pending xdp redirected packets and flush when it exceeds NAPI_POLL_WEIGHT or MSG_MORE is clear. This may lead BUG() since xdp_do_flush() was Lead to BUG(). called under process context with preemption enabled. Simply disable s/under/in the/? preemption may silent the warning but be not enough since process may Silence. move between different CPUS during a batch which cause xdp_do_flush() misses some CPU where the process run previously. Consider the several fallouts, that commit was reverted. To fix the issue correctly, we can simply calling xdp_do_flush() immediately after xdp_do_redirect(), Call. a side effect is that this removes any possibility of batching which could be addressed in the future. Reported-by: Christoffer Dall Fixes: 762c330d670e ("tuntap: add missing xdp flush") Signed-off-by: Jason Wang [...] MBR, Sergei
Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
Hi Dan, On Jo, 2018-02-22 at 10:43 +0300, Dan Carpenter wrote: > On Wed, Feb 21, 2018 at 11:01:50PM +0200, Daniel Baluta wrote: > > > > On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira > > wrote: > > > > > > This patch fixes the checkpatch.pl warning: > > > > > > drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions > > > 'S_IWUSR' are not preferred. Consider using octal permissions '0200'. > > > > > > ... Why this "..." :)? > > Commit subject could be done better. It is pretty obvious from the code that > > we change S_IWUSR with 0200. > > > > Better message: > > > > iio:dummy: Fix poke_evN file permissions > Please stop telling people to say "Fix" when it's not a bug fix... > I agree with you that here "Fix" is an overstatement. > Also who cares? The commit message is perfectly clear. I do care about newcomers really learning on how to write a proper commit message. The commit messsage should really say why the patch is needed, no what the patch does. After several trivial patches newcomers will get into more serious stuff and I wouldn't want to see a patch with commit message like this: iio: Change bit 1 of status register but one that looks like this: iio: Set power up on chip probe thanks, Daniel.
Re: [PATCH 09/20] lightnvm: use generic identify structure
> On 22 Feb 2018, at 08.47, Matias Bjørling wrote: > > On 02/21/2018 10:26 AM, Javier González wrote: >> Create a generic identify structure to collect the identify information >> before knowing the spec. version. This forces different version paths to >> cast the structure to their spec structure, thus making the code less >> error prone and more maintainable. >> Signed-off-by: Javier González >> --- >> drivers/nvme/host/lightnvm.c | 32 >> 1 file changed, 20 insertions(+), 12 deletions(-) >> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c >> index 9c1f8225c4e1..70dc4740f0d3 100644 >> --- a/drivers/nvme/host/lightnvm.c >> +++ b/drivers/nvme/host/lightnvm.c >> @@ -170,6 +170,12 @@ struct nvme_nvm_id12 { >> __u8resv2[2880]; >> } __packed; >> +/* Generic identification structure */ >> +struct nvme_nvm_id { >> +__u8ver_id; >> +__u8resv[4095]; >> +} __packed; >> + >> struct nvme_nvm_bb_tbl { >> __u8tblid[4]; >> __le16 verid; >> @@ -279,9 +285,10 @@ static void nvme_nvm_set_addr_12(struct >> nvm_addr_format_12 *dst, >> dst->sec_mask = ((1ULL << dst->sec_len) - 1) << dst->sec_offset; >> } >> -static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id, >> +static int nvme_nvm_setup_12(struct nvme_nvm_id *gen_id, >> struct nvm_dev_geo *dev_geo) >> { >> +struct nvme_nvm_id12 *id = (struct nvme_nvm_id12 *)gen_id; >> struct nvme_nvm_id12_grp *src; >> int sec_per_pg, sec_per_pl, pg_per_blk; >> @@ -380,9 +387,11 @@ static void nvme_nvm_set_addr_20(struct >> nvm_addr_format *dst, >> dst->sec_mask = ((1ULL << dst->sec_len) - 1) << dst->sec_offset; >> } >> -static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, >> +static int nvme_nvm_setup_20(struct nvme_nvm_id *gen_id, >> struct nvm_dev_geo *dev_geo) >> { >> +struct nvme_nvm_id20 *id = (struct nvme_nvm_id20 *)gen_id; >> + >> dev_geo->major_ver_id = id->mjr; >> dev_geo->minor_ver_id = id->mnr; >> @@ -427,19 +436,19 @@ static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, >> static int nvme_nvm_identity(struct nvm_dev *nvmdev) >> { >> struct nvme_ns *ns = nvmdev->q->queuedata; >> -struct nvme_nvm_id12 *id; >> +struct nvme_nvm_id *nvme_nvm_id; >> struct nvme_nvm_command c = {}; >> int ret; >> c.identity.opcode = nvme_nvm_admin_identity; >> c.identity.nsid = cpu_to_le32(ns->head->ns_id); >> - id = kmalloc(sizeof(struct nvme_nvm_id12), GFP_KERNEL); >> -if (!id) >> +nvme_nvm_id = kmalloc(sizeof(struct nvme_nvm_id), GFP_KERNEL); >> +if (!nvme_nvm_id) >> return -ENOMEM; >> ret = nvme_submit_sync_cmd(ns->ctrl->admin_q, (struct nvme_command *)&c, >> -id, sizeof(struct nvme_nvm_id12)); >> +nvme_nvm_id, sizeof(struct nvme_nvm_id)); >> if (ret) { >> ret = -EIO; >> goto out; >> @@ -449,22 +458,21 @@ static int nvme_nvm_identity(struct nvm_dev *nvmdev) >> * The 1.2 and 2.0 specifications share the first byte in their geometry >> * command to make it possible to know what version a device implements. >> */ >> -switch (id->ver_id) { >> +switch (nvme_nvm_id->ver_id) { >> case 1: >> -ret = nvme_nvm_setup_12(id, &nvmdev->dev_geo); >> +ret = nvme_nvm_setup_12(nvme_nvm_id, &nvmdev->dev_geo); >> break; >> case 2: >> -ret = nvme_nvm_setup_20((struct nvme_nvm_id20 *)id, >> -&nvmdev->dev_geo); >> +ret = nvme_nvm_setup_20(nvme_nvm_id, &nvmdev->dev_geo); >> break; >> default: >> dev_err(ns->ctrl->device, "OCSSD revision not supported (%d)\n", >> -id->ver_id); >> +nvme_nvm_id->ver_id); >> ret = -EINVAL; >> } >>out: >> -kfree(id); >> +kfree(nvme_nvm_id); >> return ret; >> } >> > > Thanks for another way to represent it. I want to keep the original > path. If we are going that down road, then one should maybe look into > unifying the "three" data structures, and have the version as the base > property and the others in each their sub data structure. That's what this structure aims to do, since the version is shared. But you call if you want to cast unrelated structures back an forth. I'll revert it... Javier signature.asc Description: Message signed with OpenPGP
Re: [PATCH 11/20] lightnvm: pblk: check for supported version
On 02/21/2018 10:26 AM, Javier González wrote: At this point, only 1.2 spec is supported, thus check for it. Also, since device-side L2P is only supported in the 1.2 spec, make sure to only check its value under 1.2. Signed-off-by: Javier González --- drivers/lightnvm/pblk-init.c | 10 -- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index 56b4afc27add..ec39800eea42 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -990,9 +990,15 @@ static void *pblk_init(struct nvm_tgt_dev *dev, struct gendisk *tdisk, struct pblk *pblk; int ret; - if (dev->geo.c.dom & NVM_RSP_L2P) { + if (geo->c.version != NVM_OCSSD_SPEC_12) { + pr_err("pblk: OCSSD version not supported (%u)\n", + geo->c.version); + return ERR_PTR(-EINVAL); + } + + if (geo->c.version == NVM_OCSSD_SPEC_12 && geo->c.dom & NVM_RSP_L2P) { pr_err("pblk: host-side L2P table not supported. (%x)\n", - dev->geo.c.dom); + geo->c.dom); return ERR_PTR(-EINVAL); } Looks good to me. I'll pick up when rebased.
[PATCH v9 3/7] PCI/ERR: add mutex to synchronize recovery
This patch protects pci_do_recovery with mutex. Signed-off-by: Oza Pawandeep diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c index a532fe0..8318c84 100644 --- a/drivers/pci/pcie/pcie-err.c +++ b/drivers/pci/pcie/pcie-err.c @@ -20,6 +20,8 @@ #include #include "portdrv.h" +static DEFINE_MUTEX(pci_err_recovery_lock); + struct aer_broadcast_data { enum pci_channel_state state; enum pci_ers_result result; @@ -283,6 +285,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity) pci_ers_result_t status, result = PCI_ERS_RESULT_RECOVERED; enum pci_channel_state state; + mutex_lock(&pci_err_recovery_lock); + if (severity == AER_FATAL) state = pci_channel_io_frozen; else @@ -326,9 +330,11 @@ void pci_do_recovery(struct pci_dev *dev, int severity) report_resume); dev_info(&dev->dev, "Device recovery successful\n"); + mutex_unlock(&pci_err_recovery_lock); return; failed: /* TODO: Should kernel panic here? */ dev_info(&dev->dev, "Device recovery failed\n"); + mutex_unlock(&pci_err_recovery_lock); } -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc., a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH v9 2/7] PCI/AER: factor out error reporting from AER
This patch factors out error reporting callbacks, which are currently tightly coupled with AER. DPC should be able to register callbacks and attmept recovery when DPC trigger event occurs. Signed-off-by: Oza Pawandeep diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index fcd8191..a5a79f0 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -342,6 +342,9 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev, void pci_enable_acs(struct pci_dev *dev); +/* PCI error reporting and recovery */ +void pci_do_recovery(struct pci_dev *dev, int severity); + #ifdef CONFIG_PCIEASPM void pcie_aspm_init_link_state(struct pci_dev *pdev); void pcie_aspm_exit_link_state(struct pci_dev *pdev); diff --git a/drivers/pci/pcie/Makefile b/drivers/pci/pcie/Makefile index 223e4c3..d669497 100644 --- a/drivers/pci/pcie/Makefile +++ b/drivers/pci/pcie/Makefile @@ -6,7 +6,7 @@ # Build PCI Express ASPM if needed obj-$(CONFIG_PCIEASPM) += aspm.o -pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o +pcieportdrv-y := portdrv_core.o portdrv_pci.o portdrv_bus.o pcie-err.o pcieportdrv-$(CONFIG_ACPI) += portdrv_acpi.o obj-$(CONFIG_PCIEPORTBUS) += pcieportdrv.o diff --git a/drivers/pci/pcie/aer/aerdrv.h b/drivers/pci/pcie/aer/aerdrv.h index 5449e5c..bc9db53 100644 --- a/drivers/pci/pcie/aer/aerdrv.h +++ b/drivers/pci/pcie/aer/aerdrv.h @@ -76,36 +76,6 @@ struct aer_rpc { */ }; -struct aer_broadcast_data { - enum pci_channel_state state; - enum pci_ers_result result; -}; - -static inline pci_ers_result_t merge_result(enum pci_ers_result orig, - enum pci_ers_result new) -{ - if (new == PCI_ERS_RESULT_NO_AER_DRIVER) - return PCI_ERS_RESULT_NO_AER_DRIVER; - - if (new == PCI_ERS_RESULT_NONE) - return orig; - - switch (orig) { - case PCI_ERS_RESULT_CAN_RECOVER: - case PCI_ERS_RESULT_RECOVERED: - orig = new; - break; - case PCI_ERS_RESULT_DISCONNECT: - if (new == PCI_ERS_RESULT_NEED_RESET) - orig = PCI_ERS_RESULT_NEED_RESET; - break; - default: - break; - } - - return orig; -} - extern struct bus_type pcie_port_bus_type; void aer_isr(struct work_struct *work); void aer_print_error(struct pci_dev *dev, struct aer_err_info *info); diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index 306bf2f..410d4b8 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -23,6 +23,7 @@ #include #include #include "aerdrv.h" +#include "../../pci.h" #definePCI_EXP_AER_FLAGS (PCI_EXP_DEVCTL_CERE | PCI_EXP_DEVCTL_NFERE | \ PCI_EXP_DEVCTL_FERE | PCI_EXP_DEVCTL_URRE) @@ -230,191 +231,6 @@ static bool find_source_device(struct pci_dev *parent, return true; } -static int report_error_detected(struct pci_dev *dev, void *data) -{ - pci_ers_result_t vote; - const struct pci_error_handlers *err_handler; - struct aer_broadcast_data *result_data; - result_data = (struct aer_broadcast_data *) data; - - device_lock(&dev->dev); - dev->error_state = result_data->state; - - if (!dev->driver || - !dev->driver->err_handler || - !dev->driver->err_handler->error_detected) { - if (result_data->state == pci_channel_io_frozen && - dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) { - /* -* In case of fatal recovery, if one of down- -* stream device has no driver. We might be -* unable to recover because a later insmod -* of a driver for this device is unaware of -* its hw state. -*/ - pci_printk(KERN_DEBUG, dev, "device has %s\n", - dev->driver ? - "no AER-aware driver" : "no driver"); - } - - /* -* If there's any device in the subtree that does not -* have an error_detected callback, returning -* PCI_ERS_RESULT_NO_AER_DRIVER prevents calling of -* the subsequent mmio_enabled/slot_reset/resume -* callbacks of "any" device in the subtree. All the -* devices in the subtree are left in the error state -* without recovery. -*/ - - if (dev->hdr_type != PCI_HEADER_TYPE_BRIDGE) - vote = PCI_ERS_RESULT_NO_AER_DRIVER; - else - vote = PCI_ERS_RESULT_NONE; - } else { - err_handler = dev->driver->err_handler; - vote =
Re: [PATCH v7 0/5] Add Intel IOMMU debugfs support
On Tue, 2018-02-20 at 14:25 -0800, Jacob Pan wrote: > I didn't know about chipsec but reading the code seems to rely on an > out-of-tree kernel module. I don't think it matches what we need here. Yes good indeed, I had forgot about that. Maybe the userland part is still useful, but there's definitely a need for (protected) access to privileged memory (and access to /dev/mem is less practical than debugfs, I guess). Regards, -- Yves-Alexis signature.asc Description: This is a digitally signed message part
[PATCH v9 4/7] PCI/DPC: Unify and plumb error handling into DPC
Current DPC driver does not do recovery, e.g. calling end-point's driver's callbacks, which sanitize the sw. DPC driver implements link_reset callback, and calls pci_do_recovery. Signed-off-by: Oza Pawandeep diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index a5a79f0..124f42e 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -343,6 +343,8 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev, void pci_enable_acs(struct pci_dev *dev); /* PCI error reporting and recovery */ +#define DPC_FATAL 4 + void pci_do_recovery(struct pci_dev *dev, int severity); #ifdef CONFIG_PCIEASPM diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c index 38e40c6..208b427 100644 --- a/drivers/pci/pcie/pcie-dpc.c +++ b/drivers/pci/pcie/pcie-dpc.c @@ -13,6 +13,7 @@ #include #include "../pci.h" #include "aer/aerdrv.h" +#include "portdrv.h" struct dpc_dev { struct pcie_device *dev; @@ -45,6 +46,60 @@ struct dpc_dev { "Memory Request Completion Timeout", /* Bit Position 18 */ }; +static int find_dpc_dev_iter(struct device *device, void *data) +{ + struct pcie_port_service_driver *service_driver; + struct device **dev; + + dev = (struct device **) data; + + if (device->bus == &pcie_port_bus_type && device->driver) { + service_driver = to_service_driver(device->driver); + if (service_driver->service == PCIE_PORT_SERVICE_DPC) { + *dev = device; + return 1; + } + } + + return 0; +} + +static struct device *pci_find_dpc_dev(struct pci_dev *pdev) +{ + struct device *dev = NULL; + + device_for_each_child(&pdev->dev, &dev, find_dpc_dev_iter); + + return dev; +} + +static int find_dpc_service_iter(struct device *device, void *data) +{ + struct pcie_port_service_driver *service_driver, **drv; + + drv = (struct pcie_port_service_driver **) data; + + if (device->bus == &pcie_port_bus_type && device->driver) { + service_driver = to_service_driver(device->driver); + if (service_driver->service == PCIE_PORT_SERVICE_DPC) { + *drv = service_driver; + return 1; + } + } + + return 0; +} + +struct pcie_port_service_driver *pci_find_dpc_service(struct pci_dev *dev) +{ + struct pcie_port_service_driver *drv = NULL; + + device_for_each_child(&dev->dev, &drv, find_dpc_service_iter); + + return drv; +} +EXPORT_SYMBOL(pci_find_dpc_service); + static int dpc_wait_rp_inactive(struct dpc_dev *dpc) { unsigned long timeout = jiffies + HZ; @@ -82,12 +137,25 @@ static void dpc_wait_link_inactive(struct dpc_dev *dpc) dev_warn(dev, "Link state not disabled for DPC event\n"); } -static void dpc_work(struct work_struct *work) +/** + * dpc_reset_link - reset link DPC routine + * @dev: pointer to Root Port's pci_dev data structure + * + * Invoked by Port Bus driver when performing link reset at Root Port. + */ +static pci_ers_result_t dpc_reset_link(struct pci_dev *pdev) { - struct dpc_dev *dpc = container_of(work, struct dpc_dev, work); - struct pci_dev *dev, *temp, *pdev = dpc->dev->port; struct pci_bus *parent = pdev->subordinate; - u16 cap = dpc->cap_pos, ctl; + struct pci_dev *dev, *temp; + struct dpc_dev *dpc; + struct pcie_device *pciedev; + struct device *devdpc; + u16 cap, ctl; + + devdpc = pci_find_dpc_dev(pdev); + pciedev = to_pcie_device(devdpc); + dpc = get_service_data(pciedev); + cap = dpc->cap_pos; pci_lock_rescan_remove(); list_for_each_entry_safe_reverse(dev, temp, &parent->devices, @@ -104,21 +172,31 @@ static void dpc_work(struct work_struct *work) dpc_wait_link_inactive(dpc); if (dpc->rp_extensions && dpc_wait_rp_inactive(dpc)) - return; + return PCI_ERS_RESULT_DISCONNECT; if (dpc->rp_extensions && dpc->rp_pio_status) { pci_write_config_dword(pdev, cap + PCI_EXP_DPC_RP_PIO_STATUS, dpc->rp_pio_status); dpc->rp_pio_status = 0; } - pci_write_config_word(pdev, cap + PCI_EXP_DPC_STATUS, + pci_write_config_word(pdev, dpc->cap_pos + PCI_EXP_DPC_STATUS, PCI_EXP_DPC_STATUS_TRIGGER | PCI_EXP_DPC_STATUS_INTERRUPT); pci_read_config_word(pdev, cap + PCI_EXP_DPC_CTL, &ctl); pci_write_config_word(pdev, cap + PCI_EXP_DPC_CTL, - ctl | PCI_EXP_DPC_CTL_INT_EN); + ctl | PCI_EXP_DPC_CTL_INT_EN); + + return PCI_ERS_RESULT_RECOVERED; } +static void dpc_work(struct work_struct *work) +{ + struct dpc_dev *dpc = container_of(work, struct dpc_dev, work); + struct pci_dev *pdev = dpc->dev->port; + + /* From DPC p
Re: [PATCH v3 1/7] ARM: sun8i: smp: Add support for A83T
Hello Chen-Yu, On Tue, 20 Feb 2018 11:32:03 +0800 Chen-Yu Tsai wrote: > On Mon, Feb 19, 2018 at 4:18 PM, Mylène Josserand > wrote: > > Add the support for A83T. > > > > A83T SoC has an additional register than A80 to handle CPU configurations: > > R_CPUS_CFG. Information about the register comes from Allwinner's BSP > > driver. > > An important difference is the Power Off Gating register for clusters > > which is BIT(4) in case of SUN9I-A80 and BIT(0) in case of SUN8I-A83T. > > > > Signed-off-by: Mylène Josserand > > --- > > arch/arm/mach-sunxi/Kconfig | 2 +- > > arch/arm/mach-sunxi/mc_smp.c | 239 > > +++ > > The same high-level comments as Maxime. Splitting the patch > will make this much easier to understand. Yep, I will do it in next version. > > > 2 files changed, 198 insertions(+), 43 deletions(-) > > > > diff --git a/arch/arm/mach-sunxi/Kconfig b/arch/arm/mach-sunxi/Kconfig > > index ce53ceaf4cc5..a0ad35c41c02 100644 > > --- a/arch/arm/mach-sunxi/Kconfig > > +++ b/arch/arm/mach-sunxi/Kconfig > > @@ -51,7 +51,7 @@ config MACH_SUN9I > > config ARCH_SUNXI_MC_SMP > > bool > > depends on SMP > > - default MACH_SUN9I > > + default y if MACH_SUN9I || MACH_SUN8I > > select ARM_CCI400_PORT_CTRL > > select ARM_CPU_SUSPEND > > > > diff --git a/arch/arm/mach-sunxi/mc_smp.c b/arch/arm/mach-sunxi/mc_smp.c > > index 11e46c6efb90..fec592bf68b4 100644 > > --- a/arch/arm/mach-sunxi/mc_smp.c > > +++ b/arch/arm/mach-sunxi/mc_smp.c > > @@ -55,20 +55,29 @@ > > #define CPUCFG_CX_RST_CTRL_L2_RST BIT(8) > > #define CPUCFG_CX_RST_CTRL_CX_RST(n) BIT(4 + (n)) > > #define CPUCFG_CX_RST_CTRL_CORE_RST(n) BIT(n) > > +#define CPUCFG_CX_RST_CTRL_CORE_RST_ALL(0xf << 0) > > > > #define PRCM_CPU_PO_RST_CTRL(c)(0x4 + 0x4 * (c)) > > #define PRCM_CPU_PO_RST_CTRL_CORE(n) BIT(n) > > #define PRCM_CPU_PO_RST_CTRL_CORE_ALL 0xf > > #define PRCM_PWROFF_GATING_REG(c) (0x100 + 0x4 * (c)) > > -#define PRCM_PWROFF_GATING_REG_CLUSTER BIT(4) > > +/* The power off register for clusters are different from SUN9I and SUN8I > > */ > > +#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN8I BIT(0) > > +#define PRCM_PWROFF_GATING_REG_CLUSTER_SUN9I BIT(4) > > #define PRCM_PWROFF_GATING_REG_CORE(n) BIT(n) > > #define PRCM_PWR_SWITCH_REG(c, cpu)(0x140 + 0x10 * (c) + 0x4 * (cpu)) > > #define PRCM_CPU_SOFT_ENTRY_REG0x164 > > > > +/* R_CPUCFG registers, specific to SUN8I */ > > +#define R_CPUCFG_CLUSTER_PO_RST_CTRL(c)(0x30 + (c) * 0x4) > > +#define R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(n) BIT(n) > > +#define R_CPUCFG_CPU_SOFT_ENTRY_REG0x01a4 > > + > > #define CPU0_SUPPORT_HOTPLUG_MAGIC00xFA50392F > > #define CPU0_SUPPORT_HOTPLUG_MAGIC10x790DCA3A > > > > static void __iomem *cpucfg_base; > > +static void __iomem *r_cpucfg_base; > > static void __iomem *prcm_base; > > static void __iomem *sram_b_smp_base; > > > > @@ -157,6 +166,16 @@ static int sunxi_cpu_powerup(unsigned int cpu, > > unsigned int cluster) > > reg &= ~PRCM_CPU_PO_RST_CTRL_CORE(cpu); > > writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster)); > > > > + if (r_cpucfg_base) { > > + /* assert cpu power-on reset */ > > + reg = readl(r_cpucfg_base + > > +R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster)); > > + reg &= ~(R_CPUCFG_CLUSTER_PO_RST_CTRL_CORE(cpu)); > > + writel(reg, r_cpucfg_base + > > + R_CPUCFG_CLUSTER_PO_RST_CTRL(cluster)); > > + udelay(10); > > + } > > + > > /* Cortex-A7: hold L1 reset disable signal low */ > > if (!sunxi_core_is_cortex_a15(cpu, cluster)) { > > reg = readl(cpucfg_base + CPUCFG_CX_CTRL_REG0(cluster)); > > @@ -180,17 +199,37 @@ static int sunxi_cpu_powerup(unsigned int cpu, > > unsigned int cluster) > > /* open power switch */ > > sunxi_cpu_power_switch_set(cpu, cluster, true); > > > > + /* Handle A83T bit swap */ > > + if (of_machine_is_compatible("allwinner,sun8i-a83t")) { > > + if (cpu == 0) > > + cpu = 4; > > + } > > + > > /* clear processor power gate */ > > reg = readl(prcm_base + PRCM_PWROFF_GATING_REG(cluster)); > > reg &= ~PRCM_PWROFF_GATING_REG_CORE(cpu); > > writel(reg, prcm_base + PRCM_PWROFF_GATING_REG(cluster)); > > udelay(20); > > > > + if (of_machine_is_compatible("allwinner,sun8i-a83t")) { > > + if (cpu == 4) > > + cpu = 0; > > + } > > + > > /* de-assert processor power-on reset */ > > reg = readl(prcm_base + PRCM_CPU_PO_RST_CTRL(cluster)); > > reg |= PRCM_CPU_PO_RST_CTRL_CORE(cpu); > > writel(reg, prcm_base + PRCM_CPU_PO_RST_CTRL(cluster)); > > > > + if (r_cpucfg_base) { > > + reg = readl(r_cpucfg_ba
Re: [PATCH 10/20] lightnvm: pblk: rename ppaf* to addrf*
On 02/21/2018 10:26 AM, Javier González wrote: In preparation for 2.0 support in pblk, rename variables referring to the address format to addrf and reserve ppaf for the 1.2 path. Signed-off-by: Javier González --- drivers/lightnvm/pblk-init.c | 8 drivers/lightnvm/pblk-sysfs.c | 4 ++-- drivers/lightnvm/pblk.h | 16 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index 6bf51ef8f516..56b4afc27add 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -80,7 +80,7 @@ static size_t pblk_trans_map_size(struct pblk *pblk) { int entry_size = 8; - if (pblk->ppaf_bitsize < 32) + if (pblk->addrf_len < 32) entry_size = 4; return entry_size * pblk->rl.nr_secs; @@ -198,7 +198,7 @@ static int pblk_set_addrf_12(struct nvm_geo *geo, return dst->blk_offset + src->blk_len; } -static int pblk_set_ppaf(struct pblk *pblk) +static int pblk_set_addrf(struct pblk *pblk) { struct nvm_tgt_dev *dev = pblk->dev; struct nvm_geo *geo = &dev->geo; @@ -210,7 +210,7 @@ static int pblk_set_ppaf(struct pblk *pblk) return -EINVAL; } - pblk->ppaf_bitsize = pblk_set_addrf_12(geo, (void *)&pblk->ppaf); + pblk->addrf_len = pblk_set_addrf_12(geo, (void *)&pblk->addrf); return 0; } @@ -319,7 +319,7 @@ static int pblk_core_init(struct pblk *pblk) if (!pblk->r_end_wq) goto free_bb_wq; - if (pblk_set_ppaf(pblk)) + if (pblk_set_addrf(pblk)) goto free_r_end_wq; if (pblk_rwb_init(pblk)) diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c index 81288aa9162a..d3b50741b691 100644 --- a/drivers/lightnvm/pblk-sysfs.c +++ b/drivers/lightnvm/pblk-sysfs.c @@ -117,12 +117,12 @@ static ssize_t pblk_sysfs_ppaf(struct pblk *pblk, char *page) struct nvm_addr_format_12 *geo_ppaf; ssize_t sz = 0; - ppaf = (struct nvm_addr_format_12 *)&pblk->ppaf; + ppaf = (struct nvm_addr_format_12 *)&pblk->addrf; geo_ppaf = (struct nvm_addr_format_12 *)&geo->c.addrf; sz = snprintf(page, PAGE_SIZE, "pblk:(s:%d)ch:%d/%d,lun:%d/%d,blk:%d/%d,pg:%d/%d,pl:%d/%d,sec:%d/%d\n", - pblk->ppaf_bitsize, + pblk->addrf_len, ppaf->ch_offset, ppaf->ch_len, ppaf->lun_offset, ppaf->lun_len, ppaf->blk_offset, ppaf->blk_len, diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h index 4f7a365436f1..46b29a492f74 100644 --- a/drivers/lightnvm/pblk.h +++ b/drivers/lightnvm/pblk.h @@ -570,8 +570,8 @@ struct pblk { struct pblk_line_mgmt l_mg; /* Line management */ struct pblk_line_meta lm; /* Line metadata */ - struct nvm_addr_format ppaf; - int ppaf_bitsize; + struct nvm_addr_format addrf; + int addrf_len; struct pblk_rb rwb; @@ -948,7 +948,7 @@ static inline struct ppa_addr addr_to_gen_ppa(struct pblk *pblk, u64 paddr, u64 line_id) { struct nvm_addr_format_12 *ppaf = - (struct nvm_addr_format_12 *)&pblk->ppaf; + (struct nvm_addr_format_12 *)&pblk->addrf; struct ppa_addr ppa; ppa.ppa = 0; @@ -966,7 +966,7 @@ static inline u64 pblk_dev_ppa_to_line_addr(struct pblk *pblk, struct ppa_addr p) { struct nvm_addr_format_12 *ppaf = - (struct nvm_addr_format_12 *)&pblk->ppaf; + (struct nvm_addr_format_12 *)&pblk->addrf; u64 paddr; paddr = (u64)p.g.ch << ppaf->ch_offset; @@ -991,7 +991,7 @@ static inline struct ppa_addr pblk_ppa32_to_ppa64(struct pblk *pblk, u32 ppa32) ppa64.c.is_cached = 1; } else { struct nvm_addr_format_12 *ppaf = - (struct nvm_addr_format_12 *)&pblk->ppaf; + (struct nvm_addr_format_12 *)&pblk->addrf; ppa64.g.ch = (ppa32 & ppaf->ch_mask) >> ppaf->ch_offset; ppa64.g.lun = (ppa32 & ppaf->lun_mask) >> ppaf->lun_offset; @@ -1015,7 +1015,7 @@ static inline u32 pblk_ppa64_to_ppa32(struct pblk *pblk, struct ppa_addr ppa64) ppa32 |= 1U << 31; } else { struct nvm_addr_format_12 *ppaf = - (struct nvm_addr_format_12 *)&pblk->ppaf; + (struct nvm_addr_format_12 *)&pblk->addrf; ppa32 |= ppa64.g.ch << ppaf->ch_offset; ppa32 |= ppa64.g.lun << ppaf->lun_offset; @@ -1033,7 +1033,7 @@ static inline struct ppa_addr pblk_trans_map_get(struct pblk *pblk, { struct ppa_addr ppa; - if (pblk->ppaf_bitsize <
Re: [PATCH 03/20] lightnvm: fix capabilities for 2.0 sysfs
> On 22 Feb 2018, at 08.28, Matias Bjørling wrote: > > On 02/21/2018 10:26 AM, Javier González wrote: >> Both 1.2 and 2.0 specs define a field for media and controller >> capabilities. Also, 1.2 defines a separate field dedicated to device >> capabilities. >> In 2.0 sysfs, this values have been mixed. Revert them to the right >> value. >> Signed-off-by: Javier González >> --- >> drivers/nvme/host/lightnvm.c | 18 +- >> 1 file changed, 9 insertions(+), 9 deletions(-) >> diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c >> index 969bb874850c..598abba66f52 100644 >> --- a/drivers/nvme/host/lightnvm.c >> +++ b/drivers/nvme/host/lightnvm.c >> @@ -914,8 +914,8 @@ static ssize_t nvm_dev_attr_show(struct device *dev, >> if (strcmp(attr->name, "version") == 0) { >> return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->ver_id); >> -} else if (strcmp(attr->name, "capabilities") == 0) { >> -return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.cap); >> +} else if (strcmp(attr->name, "media_capabilities") == 0) { >> +return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.mccap); >> } else if (strcmp(attr->name, "read_typ") == 0) { >> return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.trdt); >> } else if (strcmp(attr->name, "read_max") == 0) { >> @@ -993,8 +993,8 @@ static ssize_t nvm_dev_attr_show_12(struct device *dev, >> return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.tbem); >> } else if (strcmp(attr->name, "multiplane_modes") == 0) { >> return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mpos); >> -} else if (strcmp(attr->name, "media_capabilities") == 0) { >> -return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mccap); >> +} else if (strcmp(attr->name, "capabilities") == 0) { >> +return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.cap); >> } else if (strcmp(attr->name, "max_phys_secs") == 0) { >> return scnprintf(page, PAGE_SIZE, "%u\n", NVM_MAX_VLBA); >> } else { >> @@ -1055,7 +1055,7 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev, >>/* general attributes */ >> static NVM_DEV_ATTR_RO(version); >> -static NVM_DEV_ATTR_RO(capabilities); >> +static NVM_DEV_ATTR_RO(media_capabilities); >>static NVM_DEV_ATTR_RO(read_typ); >> static NVM_DEV_ATTR_RO(read_max); >> @@ -1080,12 +1080,12 @@ static NVM_DEV_ATTR_12_RO(prog_max); >> static NVM_DEV_ATTR_12_RO(erase_typ); >> static NVM_DEV_ATTR_12_RO(erase_max); >> static NVM_DEV_ATTR_12_RO(multiplane_modes); >> -static NVM_DEV_ATTR_12_RO(media_capabilities); >> +static NVM_DEV_ATTR_12_RO(capabilities); >> static NVM_DEV_ATTR_12_RO(max_phys_secs); >>static struct attribute *nvm_dev_attrs_12[] = { >> &dev_attr_version.attr, >> -&dev_attr_capabilities.attr, >> +&dev_attr_media_capabilities.attr, >> &dev_attr_vendor_opcode.attr, >> &dev_attr_device_mode.attr, >> @@ -1108,7 +1108,7 @@ static struct attribute *nvm_dev_attrs_12[] = { >> &dev_attr_erase_typ.attr, >> &dev_attr_erase_max.attr, >> &dev_attr_multiplane_modes.attr, >> -&dev_attr_media_capabilities.attr, >> +&dev_attr_capabilities.attr, >> &dev_attr_max_phys_secs.attr, >> NULL, >> @@ -1134,7 +1134,7 @@ static NVM_DEV_ATTR_20_RO(reset_max); >>static struct attribute *nvm_dev_attrs_20[] = { >> &dev_attr_version.attr, >> -&dev_attr_capabilities.attr, >> +&dev_attr_media_capabilities.attr, >> &dev_attr_groups.attr, >> &dev_attr_punits.attr, > > With the mccap changes, it should make sense to keep the capabilities > as is. The change adds mccap, but sysfs points to cap, which is wrong. This patch is needed. Otherwise, we change the name of mccap to cap, which is _very_ confusing to people familiar to both specs. We can change the name of mccap to cap in a future spec revision. Javier signature.asc Description: Message signed with OpenPGP
[PATCH v9 5/7] PCI/AER: Unify aer error defines at single space
This patch moves AER error defines to drivers/pci/pci.h. So that it unifies the error repoting codes at single place along with dpc Signed-off-by: Oza Pawandeep diff --git a/drivers/acpi/apei/ghes.c b/drivers/acpi/apei/ghes.c index 1efefe9..7ae9bb3 100644 --- a/drivers/acpi/apei/ghes.c +++ b/drivers/acpi/apei/ghes.c @@ -56,6 +56,7 @@ #include #include "apei-internal.h" +#include "../../pci/pci.h" #define GHES_PFX "GHES: " diff --git a/drivers/pci/pci.h b/drivers/pci/pci.h index 124f42e..b0e63b5 100644 --- a/drivers/pci/pci.h +++ b/drivers/pci/pci.h @@ -343,7 +343,11 @@ static inline resource_size_t pci_resource_alignment(struct pci_dev *dev, void pci_enable_acs(struct pci_dev *dev); /* PCI error reporting and recovery */ -#define DPC_FATAL 4 +#define AER_NONFATAL 0 +#define AER_FATAL 1 +#define AER_CORRECTABLE2 + +#define DPC_FATAL 4 void pci_do_recovery(struct pci_dev *dev, int severity); diff --git a/drivers/pci/pcie/aer/aerdrv_errprint.c b/drivers/pci/pcie/aer/aerdrv_errprint.c index 6a352e6..4c59f37 100644 --- a/drivers/pci/pcie/aer/aerdrv_errprint.c +++ b/drivers/pci/pcie/aer/aerdrv_errprint.c @@ -19,6 +19,7 @@ #include #include "aerdrv.h" +#include "../../pci.h" #include #define AER_AGENT_RECEIVER 0 diff --git a/include/linux/aer.h b/include/linux/aer.h index 8f87bbe..3eac8ed 100644 --- a/include/linux/aer.h +++ b/include/linux/aer.h @@ -11,10 +11,6 @@ #include #include -#define AER_NONFATAL 0 -#define AER_FATAL 1 -#define AER_CORRECTABLE2 - struct pci_dev; struct aer_header_log_regs { diff --git a/include/ras/ras_event.h b/include/ras/ras_event.h index 9c68986..d75c75b 100644 --- a/include/ras/ras_event.h +++ b/include/ras/ras_event.h @@ -13,6 +13,7 @@ #include #include #include +#include "../../../drivers/pci/pci.h" /* * MCE Extended Error Log trace event -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc., a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH v9 6/7] PCI: Unify wait for link active into generic pci
Clients such as pciehp, dpc are using pcie_wait_link_active, which waits till the link becomes active or inactive. Made generic function and moved it to drivers/pci/pci.c Signed-off-by: Oza Pawandeep diff --git a/drivers/pci/hotplug/pciehp_hpc.c b/drivers/pci/hotplug/pciehp_hpc.c index 18a42f8..a133b8b 100644 --- a/drivers/pci/hotplug/pciehp_hpc.c +++ b/drivers/pci/hotplug/pciehp_hpc.c @@ -231,25 +231,12 @@ bool pciehp_check_link_active(struct controller *ctrl) return ret; } -static void __pcie_wait_link_active(struct controller *ctrl, bool active) +static bool pcie_wait_link_active(struct controller *ctrl) { - int timeout = 1000; - - if (pciehp_check_link_active(ctrl) == active) - return; - while (timeout > 0) { - msleep(10); - timeout -= 10; - if (pciehp_check_link_active(ctrl) == active) - return; - } - ctrl_dbg(ctrl, "Data Link Layer Link Active not %s in 1000 msec\n", - active ? "set" : "cleared"); -} + struct pci_dev *pdev = ctrl_dev(ctrl); + bool active = true; -static void pcie_wait_link_active(struct controller *ctrl) -{ - __pcie_wait_link_active(ctrl, true); + return pci_wait_for_link(pdev, active); } static bool pci_bus_check_dev(struct pci_bus *bus, int devfn) diff --git a/drivers/pci/pci.c b/drivers/pci/pci.c index f6a4dd1..5440696 100644 --- a/drivers/pci/pci.c +++ b/drivers/pci/pci.c @@ -4176,6 +4176,40 @@ static int pci_pm_reset(struct pci_dev *dev, int probe) return 0; } +/** + * pci__wait_for_link - Wait for link till its active/inactive + * @pdev: Bridge device + * @active: waiting for active or inactive ? + * + * Use this to wait till link becomes active or inactive. + */ +bool pci_wait_for_link(struct pci_dev *pdev, bool active) +{ + int timeout = 1000; + bool ret; + u16 lnk_status; + + pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status); + ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA); + + while ((ret != active) && (timeout > 0)) { + msleep(10); + timeout -= 10; + pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status); + ret = !!(lnk_status & PCI_EXP_LNKSTA_DLLLA); + } + + if (ret == active) + return true; + + dev_printk(KERN_DEBUG, &pdev->dev, + "Data Link Layer Link Active not %s in 1000 msec\n", + active ? "set" : "cleared"); + + return false; +} +EXPORT_SYMBOL(pci_wait_for_link); + void pci_reset_secondary_bus(struct pci_dev *dev) { u16 ctrl; diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c index 208b427..fce4518 100644 --- a/drivers/pci/pcie/pcie-dpc.c +++ b/drivers/pci/pcie/pcie-dpc.c @@ -122,19 +122,10 @@ static int dpc_wait_rp_inactive(struct dpc_dev *dpc) static void dpc_wait_link_inactive(struct dpc_dev *dpc) { - unsigned long timeout = jiffies + HZ; struct pci_dev *pdev = dpc->dev->port; - struct device *dev = &dpc->dev->device; - u16 lnk_status; + bool active = false; - pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status); - while (lnk_status & PCI_EXP_LNKSTA_DLLLA && - !time_after(jiffies, timeout)) { - msleep(10); - pcie_capability_read_word(pdev, PCI_EXP_LNKSTA, &lnk_status); - } - if (lnk_status & PCI_EXP_LNKSTA_DLLLA) - dev_warn(dev, "Link state not disabled for DPC event\n"); + pci_wait_for_link(pdev, active); } /** diff --git a/include/linux/pci.h b/include/linux/pci.h index 024a1be..cb674c3 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -1195,6 +1195,7 @@ int pci_add_ext_cap_save_buffer(struct pci_dev *dev, int pci_request_selected_regions(struct pci_dev *, int, const char *); int pci_request_selected_regions_exclusive(struct pci_dev *, int, const char *); void pci_release_selected_regions(struct pci_dev *, int); +bool pci_wait_for_link(struct pci_dev *pdev, bool active); /* drivers/pci/bus.c */ struct pci_bus *pci_bus_get(struct pci_bus *bus); -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc., a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH v9 7/7] PCI/DPC: Enumerate the devices after DPC trigger event
Implement error_resume callback in DPC so, after DPC trigger event enumerates the devices beneath. Signed-off-by: Oza Pawandeep diff --git a/drivers/pci/pcie/pcie-dpc.c b/drivers/pci/pcie/pcie-dpc.c index fce4518..59c01c7 100644 --- a/drivers/pci/pcie/pcie-dpc.c +++ b/drivers/pci/pcie/pcie-dpc.c @@ -129,6 +129,23 @@ static void dpc_wait_link_inactive(struct dpc_dev *dpc) } /** + * dpc_error_resume - enumerate the devices beneath + * @dev: pointer to Root Port's pci_dev data structure + * + * Invoked by Port Bus driver during nonfatal recovery. + */ +static void dpc_error_resume(struct pci_dev *pdev) +{ + bool active = true; + + if (pci_wait_for_link(pdev, active)) { + pci_lock_rescan_remove(); + pci_rescan_bus(pdev->bus); + pci_unlock_rescan_remove(); + } +} + +/** * dpc_reset_link - reset link DPC routine * @dev: pointer to Root Port's pci_dev data structure * @@ -366,6 +383,7 @@ static void dpc_remove(struct pcie_device *dev) .service= PCIE_PORT_SERVICE_DPC, .probe = dpc_probe, .remove = dpc_remove, + .error_resume = dpc_error_resume, .reset_link = dpc_reset_link, }; diff --git a/drivers/pci/pcie/pcie-err.c b/drivers/pci/pcie/pcie-err.c index 6844347..4950f49 100644 --- a/drivers/pci/pcie/pcie-err.c +++ b/drivers/pci/pcie/pcie-err.c @@ -236,6 +236,7 @@ static pci_ers_result_t reset_link(struct pci_dev *dev, int severity) * @state: error state * @error_mesg: message to print * @cb: callback to be broadcasted + * @severity: error severity * * Invoked during error recovery process. Once being invoked, the content * of error severity will be broadcasted to all downstream drivers in a @@ -244,7 +245,8 @@ static pci_ers_result_t reset_link(struct pci_dev *dev, int severity) static pci_ers_result_t broadcast_error_message(struct pci_dev *dev, enum pci_channel_state state, char *error_mesg, - int (*cb)(struct pci_dev *, void *)) + int (*cb)(struct pci_dev *, void *), + int severity) { struct aer_broadcast_data result_data; @@ -256,6 +258,15 @@ static pci_ers_result_t broadcast_error_message(struct pci_dev *dev, result_data.result = PCI_ERS_RESULT_RECOVERED; if (dev->hdr_type == PCI_HEADER_TYPE_BRIDGE) { + /* If DPC is triggered, call resume error hanlder +* because, at this point we can safely assume that +* link recovery has happened. +*/ + if ((severity == DPC_FATAL) && + (cb == report_resume)) { + cb(dev, NULL); + return PCI_ERS_RESULT_RECOVERED; + } /* * If the error is reported by a bridge, we think this error * is related to the downstream link of the bridge, so we @@ -305,7 +316,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity) status = broadcast_error_message(dev, state, "error_detected", - report_error_detected); + report_error_detected, + severity); if ((severity == AER_FATAL) || (severity == DPC_FATAL)) { @@ -321,7 +333,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity) status = broadcast_error_message(dev, state, "mmio_enabled", - report_mmio_enabled); + report_mmio_enabled, + severity); if (status == PCI_ERS_RESULT_NEED_RESET) { /* @@ -332,7 +345,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity) status = broadcast_error_message(dev, state, "slot_reset", - report_slot_reset); + report_slot_reset, + severity); } if (status != PCI_ERS_RESULT_RECOVERED) @@ -342,7 +356,8 @@ void pci_do_recovery(struct pci_dev *dev, int severity) broadcast_error_message(dev, state, "resume", - report_resume); + report_resume, + severity); dev_info(&dev->dev, "Device recovery successful\n"); mutex_unlock(&pci_err_recovery_lock); -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc., a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 09/20] lightnvm: use generic identify structure
On 02/21/2018 10:26 AM, Javier González wrote: Create a generic identify structure to collect the identify information before knowing the spec. version. This forces different version paths to cast the structure to their spec structure, thus making the code less error prone and more maintainable. Signed-off-by: Javier González --- drivers/nvme/host/lightnvm.c | 32 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index 9c1f8225c4e1..70dc4740f0d3 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -170,6 +170,12 @@ struct nvme_nvm_id12 { __u8resv2[2880]; } __packed; +/* Generic identification structure */ +struct nvme_nvm_id { + __u8ver_id; + __u8resv[4095]; +} __packed; + struct nvme_nvm_bb_tbl { __u8tblid[4]; __le16 verid; @@ -279,9 +285,10 @@ static void nvme_nvm_set_addr_12(struct nvm_addr_format_12 *dst, dst->sec_mask = ((1ULL << dst->sec_len) - 1) << dst->sec_offset; } -static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id, +static int nvme_nvm_setup_12(struct nvme_nvm_id *gen_id, struct nvm_dev_geo *dev_geo) { + struct nvme_nvm_id12 *id = (struct nvme_nvm_id12 *)gen_id; struct nvme_nvm_id12_grp *src; int sec_per_pg, sec_per_pl, pg_per_blk; @@ -380,9 +387,11 @@ static void nvme_nvm_set_addr_20(struct nvm_addr_format *dst, dst->sec_mask = ((1ULL << dst->sec_len) - 1) << dst->sec_offset; } -static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, +static int nvme_nvm_setup_20(struct nvme_nvm_id *gen_id, struct nvm_dev_geo *dev_geo) { + struct nvme_nvm_id20 *id = (struct nvme_nvm_id20 *)gen_id; + dev_geo->major_ver_id = id->mjr; dev_geo->minor_ver_id = id->mnr; @@ -427,19 +436,19 @@ static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, static int nvme_nvm_identity(struct nvm_dev *nvmdev) { struct nvme_ns *ns = nvmdev->q->queuedata; - struct nvme_nvm_id12 *id; + struct nvme_nvm_id *nvme_nvm_id; struct nvme_nvm_command c = {}; int ret; c.identity.opcode = nvme_nvm_admin_identity; c.identity.nsid = cpu_to_le32(ns->head->ns_id); - id = kmalloc(sizeof(struct nvme_nvm_id12), GFP_KERNEL); - if (!id) + nvme_nvm_id = kmalloc(sizeof(struct nvme_nvm_id), GFP_KERNEL); + if (!nvme_nvm_id) return -ENOMEM; ret = nvme_submit_sync_cmd(ns->ctrl->admin_q, (struct nvme_command *)&c, - id, sizeof(struct nvme_nvm_id12)); + nvme_nvm_id, sizeof(struct nvme_nvm_id)); if (ret) { ret = -EIO; goto out; @@ -449,22 +458,21 @@ static int nvme_nvm_identity(struct nvm_dev *nvmdev) * The 1.2 and 2.0 specifications share the first byte in their geometry * command to make it possible to know what version a device implements. */ - switch (id->ver_id) { + switch (nvme_nvm_id->ver_id) { case 1: - ret = nvme_nvm_setup_12(id, &nvmdev->dev_geo); + ret = nvme_nvm_setup_12(nvme_nvm_id, &nvmdev->dev_geo); break; case 2: - ret = nvme_nvm_setup_20((struct nvme_nvm_id20 *)id, - &nvmdev->dev_geo); + ret = nvme_nvm_setup_20(nvme_nvm_id, &nvmdev->dev_geo); break; default: dev_err(ns->ctrl->device, "OCSSD revision not supported (%d)\n", - id->ver_id); + nvme_nvm_id->ver_id); ret = -EINVAL; } out: - kfree(id); + kfree(nvme_nvm_id); return ret; } Thanks for another way to represent it. I want to keep the original path. If we are going that down road, then one should maybe look into unifying the "three" data structures, and have the version as the base property and the others in each their sub data structure.
[PATCH v9 0/7] Address error and recovery for AER and DPC
This patch set brings in error handling support for DPC The current implementation of AER and error message broadcasting to the EP driver is tightly coupled and limited to AER service driver. It is important to factor out broadcasting and other link handling callbacks. So that not only when AER gets triggered, but also when DPC get triggered (for e.g. ERR_FATAL), callbacks are handled appropriately. DPC should enumerate the devices after recovering the link, which is achieved by implementing error_resume callback. Changes since v8: Fixed Kbuild errors. Changes since v7: Rebased the code on pci master. > https://kernel.googlesource.com/pub/scm/linux/kernel/git/helgaas/pci Changes since v6: Sinan's and Stefan's comments implemented. > reordered patch 6 and 7 > cleaned up Changes since v5: Sinan's and Keith's comments incorporated. > made separate patch for mutex > unified error repotting codes into driver/pci/pci.h > got rid of wait link active/inactive and made generic function in driver/pci/pci.c Changes since v4: Bjorn's comments incorporated. > Renamed only do_recovery. > moved the things more locally to drivers/pci/pci.h Changes since v3: Bjorn's comments incorporated. > Made separate patch renaming generic pci_err.c > Introduce pci_err.h to contain all the error types and recovery > removed all the dependencies on pci.h Changes since v2: Based on feedback from Keith: " When DPC is triggered due to receipt of an uncorrectable error Message, the Requester ID from the Message is recorded in the DPC Error Source ID register and that Message is discarded and not forwarded Upstream. " Removed the patch where AER checks if DPC service is active Changes since v1: Kbuild errors fixed: > pci_find_dpc_dev made static > ras_event.h updated > pci_find_aer_service call with CONFIG check > pci_find_dpc_service call with CONFIG check Oza Pawandeep (7): PCI/AER: Rename error recovery to generic pci naming PCI/AER: factor out error reporting from AER PCI/ERR: add mutex to synchronize recovery PCI/DPC: Unify and plumb error handling into DPC PCI/AER: Unify aer error defines at single space PCI/DPC: Enumerate the devices after DPC trigger event PCI: Unify wait for link active into generic pci drivers/acpi/apei/ghes.c | 1 + drivers/pci/hotplug/pciehp_hpc.c | 21 +- drivers/pci/pci.c | 39 +++- drivers/pci/pci.h | 11 + drivers/pci/pcie/Makefile | 2 +- drivers/pci/pcie/aer/aerdrv.h | 30 --- drivers/pci/pcie/aer/aerdrv_core.c | 293 +- drivers/pci/pcie/aer/aerdrv_errprint.c | 1 + drivers/pci/pcie/pcie-dpc.c| 115 ++- drivers/pci/pcie/pcie-err.c| 366 + drivers/pci/pcie/portdrv.h | 2 + include/linux/aer.h| 4 - include/linux/pci.h| 1 + 13 files changed, 534 insertions(+), 352 deletions(-) create mode 100644 drivers/pci/pcie/pcie-err.c -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc., a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
[PATCH v9 1/7] PCI/AER: Rename error recovery to generic pci naming
This patch renames error recovery to generic name with pci prefix Signed-off-by: Oza Pawandeep diff --git a/drivers/pci/pcie/aer/aerdrv_core.c b/drivers/pci/pcie/aer/aerdrv_core.c index a4bfea5..306bf2f 100644 --- a/drivers/pci/pcie/aer/aerdrv_core.c +++ b/drivers/pci/pcie/aer/aerdrv_core.c @@ -478,7 +478,7 @@ static pci_ers_result_t reset_link(struct pci_dev *dev) } /** - * do_recovery - handle nonfatal/fatal error recovery process + * pci_do_recovery - handle nonfatal/fatal error recovery process * @dev: pointer to a pci_dev data structure of agent detecting an error * @severity: error severity type * @@ -486,7 +486,7 @@ static pci_ers_result_t reset_link(struct pci_dev *dev) * error detected message to all downstream drivers within a hierarchy in * question and return the returned code. */ -static void do_recovery(struct pci_dev *dev, int severity) +static void pci_do_recovery(struct pci_dev *dev, int severity) { pci_ers_result_t status, result = PCI_ERS_RESULT_RECOVERED; enum pci_channel_state state; @@ -566,7 +566,7 @@ static void handle_error_source(struct pcie_device *aerdev, pci_write_config_dword(dev, pos + PCI_ERR_COR_STATUS, info->status); } else - do_recovery(dev, info->severity); + pci_do_recovery(dev, info->severity); } #ifdef CONFIG_ACPI_APEI_PCIEAER @@ -631,7 +631,7 @@ static void aer_recover_work_func(struct work_struct *work) } cper_print_aer(pdev, entry.severity, entry.regs); if (entry.severity != AER_CORRECTABLE) - do_recovery(pdev, entry.severity); + pci_do_recovery(pdev, entry.severity); pci_dev_put(pdev); } } -- Qualcomm Datacenter Technologies, Inc. as an affiliate of Qualcomm Technologies, Inc., a Qualcomm Technologies, Inc. is a member of the Code Aurora Forum, a Linux Foundation Collaborative Project.
Re: [PATCH 08/20] lightnvm: complete geo structure with maxoc*
On 02/21/2018 10:26 AM, Javier González wrote: Complete the generic geometry structure with the maxoc and maxocpu felds, present in the 2.0 spec. Signed-off-by: Javier González --- drivers/nvme/host/lightnvm.c | 4 include/linux/lightnvm.h | 2 ++ 2 files changed, 6 insertions(+) diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index cca32da05316..9c1f8225c4e1 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -318,6 +318,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id, dev_geo->c.ws_min = sec_per_pg; dev_geo->c.ws_opt = sec_per_pg; dev_geo->c.mw_cunits = 8;/* default to MLC safe values */ + dev_geo->c.maxoc = dev_geo->all_luns; /* default to 1 chunk per LUN */ + dev_geo->c.maxocpu = 1; /* default to 1 chunk per LUN */ One can't assume that it is 1 open chunk per lun. If you need this for specific hardware, make a quirk for it. dev_geo->c.mccap = le32_to_cpu(src->mccap); @@ -405,6 +407,8 @@ static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, dev_geo->c.ws_min = le32_to_cpu(id->ws_min); dev_geo->c.ws_opt = le32_to_cpu(id->ws_opt); dev_geo->c.mw_cunits = le32_to_cpu(id->mw_cunits); + dev_geo->c.maxoc = le32_to_cpu(id->maxoc); + dev_geo->c.maxocpu = le32_to_cpu(id->maxocpu); dev_geo->c.mccap = le32_to_cpu(id->mccap); diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h index ccc5faa63cb7..e1c4292ea33d 100644 --- a/include/linux/lightnvm.h +++ b/include/linux/lightnvm.h @@ -215,6 +215,8 @@ struct nvm_common_geo { u32 ws_min; /* minimum write size */ u32 ws_opt; /* optimal write size */ u32 mw_cunits; /* distance required for successful read */ + u32 maxoc; /* maximum open chunks */ + u32 maxocpu;/* maximum open chunks per parallel unit */ /* device capabilities */ u32 mccap;
[BUGFIX PATCH v2.1] tracing: probeevent: Fix to support minus offset from symbol
In Documentation/trace/kprobetrace.txt, it says @SYM[+|-offs] : Fetch memory at SYM +|- offs (SYM should be a data symbol) However, the parser doesn't parse minus offset correctly, since commit 2fba0c8867af ("tracing/kprobes: Fix probe offset to be unsigned") drops minus ("-") offset support for kprobe probe address usage. This fixes the traceprobe_split_symbol_offset() to parse minus offset again with checking the offset range, and add a minus offset check in kprobe probe address usage. Fixes: 2fba0c8867af ("tracing/kprobes: Fix probe offset to be unsigned") Signed-off-by: Masami Hiramatsu --- Changes in v2.1: - Fix to ensure the offset value is less than LONG_MAX. - Fail if the offset is minus when traceprobe_split_symbol_offset() is used for the probe address. - Add Fixes tag. --- kernel/trace/trace_kprobe.c |4 ++-- kernel/trace/trace_probe.c | 15 ++- kernel/trace/trace_probe.h |2 +- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index 5ce9b8cf7be3..b5b1d8aa47d6 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -667,7 +667,7 @@ static int create_trace_kprobe(int argc, char **argv) char *symbol = NULL, *event = NULL, *group = NULL; int maxactive = 0; char *arg; - unsigned long offset = 0; + long offset = 0; void *addr = NULL; char buf[MAX_EVENT_NAME_LEN]; @@ -755,7 +755,7 @@ static int create_trace_kprobe(int argc, char **argv) symbol = argv[1]; /* TODO: support .init module functions */ ret = traceprobe_split_symbol_offset(symbol, &offset); - if (ret) { + if (ret || offset < 0) { pr_info("Failed to parse either an address or a symbol.\n"); return ret; } diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c index d59357308677..ec3856147fdd 100644 --- a/kernel/trace/trace_probe.c +++ b/kernel/trace/trace_probe.c @@ -320,21 +320,26 @@ static fetch_func_t get_fetch_size_function(const struct fetch_type *type, } /* Split symbol and offset. */ -int traceprobe_split_symbol_offset(char *symbol, unsigned long *offset) +int traceprobe_split_symbol_offset(char *symbol, long *offset) { + unsigned long ul; char *tmp; int ret; if (!offset) return -EINVAL; - tmp = strchr(symbol, '+'); + tmp = strpbrk(symbol, "+-"); if (tmp) { - /* skip sign because kstrtoul doesn't accept '+' */ - ret = kstrtoul(tmp + 1, 0, offset); + ret = kstrtoul(tmp + 1, 0, &ul); if (ret) return ret; - + if (ul > LONG_MAX) + return -E2BIG; + if (*tmp == '-') + *offset = -ul; + else + *offset = ul; *tmp = '\0'; } else *offset = 0; diff --git a/kernel/trace/trace_probe.h b/kernel/trace/trace_probe.h index 0745f895f780..75daff22ccea 100644 --- a/kernel/trace/trace_probe.h +++ b/kernel/trace/trace_probe.h @@ -365,7 +365,7 @@ extern int traceprobe_conflict_field_name(const char *name, extern void traceprobe_update_arg(struct probe_arg *arg); extern void traceprobe_free_probe_arg(struct probe_arg *arg); -extern int traceprobe_split_symbol_offset(char *symbol, unsigned long *offset); +extern int traceprobe_split_symbol_offset(char *symbol, long *offset); /* Sum up total data length for dynamic arraies (strings) */ static nokprobe_inline int
Re: [PATCH 01/20] lightnvm: simplify geometry structure.
> On 22 Feb 2018, at 08.25, Matias Bjørling wrote: > > On 02/21/2018 10:26 AM, Javier González wrote: >> Currently, the device geometry is stored redundantly in the nvm_id and >> nvm_geo structures at a device level. Moreover, when instantiating >> targets on a specific number of LUNs, these structures are replicated >> and manually modified to fit the instance channel and LUN partitioning. >> Instead, create a generic geometry around two base structures: >> nvm_dev_geo, which describes the geometry of the whole device and >> nvm_geo, which describes the geometry of the instance. Since these share >> a big part of the geometry, create a nvm_common_geo structure that keeps >> the static geoometry values that are shared across instances. >> As we introduce support for 2.0, these structures allow to abstract >> spec. specific values and present a common geometry to targets. >> Signed-off-by: Javier González >> --- >> drivers/lightnvm/core.c | 137 +++- >> drivers/lightnvm/pblk-core.c | 16 +- >> drivers/lightnvm/pblk-gc.c | 2 +- >> drivers/lightnvm/pblk-init.c | 123 +++--- >> drivers/lightnvm/pblk-read.c | 2 +- >> drivers/lightnvm/pblk-recovery.c | 14 +- >> drivers/lightnvm/pblk-rl.c | 2 +- >> drivers/lightnvm/pblk-sysfs.c| 39 +++-- >> drivers/lightnvm/pblk-write.c| 2 +- >> drivers/lightnvm/pblk.h | 93 +-- >> drivers/nvme/host/lightnvm.c | 339 >> +++ >> include/linux/lightnvm.h | 204 --- >> 12 files changed, 514 insertions(+), 459 deletions(-) >> diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c >> index 689c97b97775..42596afdf64c 100644 >> --- a/drivers/lightnvm/core.c >> +++ b/drivers/lightnvm/core.c >> @@ -111,6 +111,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, >> int lun_begin, >> static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear) >> { >> struct nvm_dev *dev = tgt_dev->parent; >> +struct nvm_dev_geo *dev_geo = &dev->dev_geo; >> struct nvm_dev_map *dev_map = tgt_dev->map; >> int i, j; >> @@ -122,7 +123,7 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev >> *tgt_dev, int clear) >> if (clear) { >> for (j = 0; j < ch_map->nr_luns; j++) { >> int lun = j + lun_offs[j]; >> -int lunid = (ch * dev->geo.nr_luns) + lun; >> +int lunid = (ch * dev_geo->nr_luns) + lun; >> WARN_ON(!test_and_clear_bit(lunid, >> dev->lun_map)); >> @@ -143,19 +144,20 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct >> nvm_dev *dev, >>u16 lun_begin, u16 lun_end, >>u16 op) >> { >> +struct nvm_dev_geo *dev_geo = &dev->dev_geo; >> struct nvm_tgt_dev *tgt_dev = NULL; >> struct nvm_dev_map *dev_rmap = dev->rmap; >> struct nvm_dev_map *dev_map; >> struct ppa_addr *luns; >> int nr_luns = lun_end - lun_begin + 1; >> int luns_left = nr_luns; >> -int nr_chnls = nr_luns / dev->geo.nr_luns; >> -int nr_chnls_mod = nr_luns % dev->geo.nr_luns; >> -int bch = lun_begin / dev->geo.nr_luns; >> -int blun = lun_begin % dev->geo.nr_luns; >> +int nr_chnls = nr_luns / dev_geo->nr_luns; >> +int nr_chnls_mod = nr_luns % dev_geo->nr_luns; >> +int bch = lun_begin / dev_geo->nr_luns; >> +int blun = lun_begin % dev_geo->nr_luns; >> int lunid = 0; >> int lun_balanced = 1; >> -int prev_nr_luns; >> +int sec_per_lun, prev_nr_luns; >> int i, j; >> nr_chnls = (nr_chnls_mod == 0) ? nr_chnls : nr_chnls + 1; >> @@ -173,15 +175,15 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct >> nvm_dev *dev, >> if (!luns) >> goto err_luns; >> - prev_nr_luns = (luns_left > dev->geo.nr_luns) ? >> -dev->geo.nr_luns : luns_left; >> +prev_nr_luns = (luns_left > dev_geo->nr_luns) ? >> +dev_geo->nr_luns : luns_left; >> for (i = 0; i < nr_chnls; i++) { >> struct nvm_ch_map *ch_rmap = &dev_rmap->chnls[i + bch]; >> int *lun_roffs = ch_rmap->lun_offs; >> struct nvm_ch_map *ch_map = &dev_map->chnls[i]; >> int *lun_offs; >> -int luns_in_chnl = (luns_left > dev->geo.nr_luns) ? >> -dev->geo.nr_luns : luns_left; >> +int luns_in_chnl = (luns_left > dev_geo->nr_luns) ? >> +dev_geo->nr_luns : luns_left; >> if (lun_balanced && prev_nr_luns != luns_in_chnl) >> lun_balanced = 0; >> @@ -215,18 +217,22 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct >> nvm_dev *dev, >> if (!tgt_dev) >>
Re: [PATCH v2] iio:dummy: Replace S_IWUSR by 0200
On Wed, Feb 21, 2018 at 11:01:50PM +0200, Daniel Baluta wrote: > On Wed, Feb 21, 2018 at 9:28 PM, Rodrigo Siqueira > wrote: > > This patch fixes the checkpatch.pl warning: > > > > drivers/iio/dummy/iio_dummy_evgen.c:151: WARNING: Symbolic permissions > > 'S_IWUSR' are not preferred. Consider using octal permissions '0200'. > > > ... Why this "..." :)? > > Commit subject could be done better. It is pretty obvious from the code that > we change S_IWUSR with 0200. > > Better message: > > iio:dummy: Fix poke_evN file permissions Please stop telling people to say "Fix" when it's not a bug fix... Also who cares? The commit message is perfectly clear. regards, dan carpenter
Re: [PATCH 07/20] lightnvm: rename sect_* to sec_*
On 02/21/2018 10:26 AM, Javier González wrote: Rename abbreviations for sector from sect_* to sec_* as most of the code uses this format and it is confusing when using the different structures. Signed-off-by: Javier González --- drivers/lightnvm/pblk-init.c | 8 drivers/lightnvm/pblk-sysfs.c | 4 ++-- drivers/lightnvm/pblk.h | 8 drivers/nvme/host/lightnvm.c | 8 include/linux/lightnvm.h | 8 5 files changed, 18 insertions(+), 18 deletions(-) diff --git a/drivers/lightnvm/pblk-init.c b/drivers/lightnvm/pblk-init.c index 455fc63a9409..6bf51ef8f516 100644 --- a/drivers/lightnvm/pblk-init.c +++ b/drivers/lightnvm/pblk-init.c @@ -179,16 +179,16 @@ static int pblk_set_addrf_12(struct nvm_geo *geo, dst->blk_len = src->blk_len; dst->pg_len = src->pg_len; dst->pln_len = src->pln_len; - dst->sect_len = src->sect_len; + dst->sec_len = src->sec_len; - dst->sect_offset = 0; - dst->pln_offset = dst->sect_len; + dst->sec_offset = 0; + dst->pln_offset = dst->sec_len; dst->ch_offset = dst->pln_offset + dst->pln_len; dst->lun_offset = dst->ch_offset + dst->ch_len; dst->pg_offset = dst->lun_offset + dst->lun_len; dst->blk_offset = dst->pg_offset + dst->pg_len; - dst->sec_mask = ((1ULL << dst->sect_len) - 1) << dst->sect_offset; + dst->sec_mask = ((1ULL << dst->sec_len) - 1) << dst->sec_offset; dst->pln_mask = ((1ULL << dst->pln_len) - 1) << dst->pln_offset; dst->ch_mask = ((1ULL << dst->ch_len) - 1) << dst->ch_offset; dst->lun_mask = ((1ULL << dst->lun_len) - 1) << dst->lun_offset; diff --git a/drivers/lightnvm/pblk-sysfs.c b/drivers/lightnvm/pblk-sysfs.c index 5eb21a279361..81288aa9162a 100644 --- a/drivers/lightnvm/pblk-sysfs.c +++ b/drivers/lightnvm/pblk-sysfs.c @@ -128,7 +128,7 @@ static ssize_t pblk_sysfs_ppaf(struct pblk *pblk, char *page) ppaf->blk_offset, ppaf->blk_len, ppaf->pg_offset, ppaf->pg_len, ppaf->pln_offset, ppaf->pln_len, - ppaf->sect_offset, ppaf->sect_len); + ppaf->sec_offset, ppaf->sec_len); sz += snprintf(page + sz, PAGE_SIZE - sz, "device:ch:%d/%d,lun:%d/%d,blk:%d/%d,pg:%d/%d,pl:%d/%d,sec:%d/%d\n", @@ -137,7 +137,7 @@ static ssize_t pblk_sysfs_ppaf(struct pblk *pblk, char *page) geo_ppaf->blk_offset, geo_ppaf->blk_len, geo_ppaf->pg_offset, geo_ppaf->pg_len, geo_ppaf->pln_offset, geo_ppaf->pln_len, - geo_ppaf->sect_offset, geo_ppaf->sect_len); + geo_ppaf->sec_offset, geo_ppaf->sec_len); return sz; } diff --git a/drivers/lightnvm/pblk.h b/drivers/lightnvm/pblk.h index 1f32284b0aec..4f7a365436f1 100644 --- a/drivers/lightnvm/pblk.h +++ b/drivers/lightnvm/pblk.h @@ -957,7 +957,7 @@ static inline struct ppa_addr addr_to_gen_ppa(struct pblk *pblk, u64 paddr, ppa.g.lun = (paddr & ppaf->lun_mask) >> ppaf->lun_offset; ppa.g.ch = (paddr & ppaf->ch_mask) >> ppaf->ch_offset; ppa.g.pl = (paddr & ppaf->pln_mask) >> ppaf->pln_offset; - ppa.g.sec = (paddr & ppaf->sec_mask) >> ppaf->sect_offset; + ppa.g.sec = (paddr & ppaf->sec_mask) >> ppaf->sec_offset; return ppa; } @@ -973,7 +973,7 @@ static inline u64 pblk_dev_ppa_to_line_addr(struct pblk *pblk, paddr |= (u64)p.g.lun << ppaf->lun_offset; paddr |= (u64)p.g.pg << ppaf->pg_offset; paddr |= (u64)p.g.pl << ppaf->pln_offset; - paddr |= (u64)p.g.sec << ppaf->sect_offset; + paddr |= (u64)p.g.sec << ppaf->sec_offset; return paddr; } @@ -998,7 +998,7 @@ static inline struct ppa_addr pblk_ppa32_to_ppa64(struct pblk *pblk, u32 ppa32) ppa64.g.blk = (ppa32 & ppaf->blk_mask) >> ppaf->blk_offset; ppa64.g.pg = (ppa32 & ppaf->pg_mask) >> ppaf->pg_offset; ppa64.g.pl = (ppa32 & ppaf->pln_mask) >> ppaf->pln_offset; - ppa64.g.sec = (ppa32 & ppaf->sec_mask) >> ppaf->sect_offset; + ppa64.g.sec = (ppa32 & ppaf->sec_mask) >> ppaf->sec_offset; } return ppa64; @@ -1022,7 +1022,7 @@ static inline u32 pblk_ppa64_to_ppa32(struct pblk *pblk, struct ppa_addr ppa64) ppa32 |= ppa64.g.blk << ppaf->blk_offset; ppa32 |= ppa64.g.pg << ppaf->pg_offset; ppa32 |= ppa64.g.pl << ppaf->pln_offset; - ppa32 |= ppa64.g.sec << ppaf->sect_offset; + ppa32 |= ppa64.g.sec << ppaf->sec_offset; } return ppa32; diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index bd97672e0b4f..cca32da05316 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -262,21 +262,21 @@ static void nvme_nvm_set_addr_12(struct nvm_addr_format_12 *dst, dst->blk_len = src-
Re: [PATCH V15 06/22] mmc: block: Add blk-mq support
On 21/02/18 22:50, Dmitry Osipenko wrote: > On 29.11.2017 16:41, Adrian Hunter wrote: >> Define and use a blk-mq queue. Discards and flushes are processed >> synchronously, but reads and writes asynchronously. In order to support >> slow DMA unmapping, DMA unmapping is not done until after the next request >> is started. That means the request is not completed until then. If there is >> no next request then the completion is done by queued work. > > Hello, > > I'm using (running linux-next and doing some upstream development for) some > old > NVIDIA Tegra tablet that has built-in (internal) and external MMC's and with > the > blk-mq being enabled I'm observing a soft lockup. The lockup seems is > reproducible quite reliably by running fsck on any MMC partition, sometimes > kernels lockups on boot during probing partitions table (weirdly only when > both > SDHCI's are present, i.e. internal storage enabled in DT and external SD is > inserted/enabled) and it also lockups pretty quickly in a case of just a > general > use. Reverting mmc/ commits up to 1bec43a3b18 ("Remove option not to use > blk-mq") and disabling CONFIG_MMC_MQ_DEFAULT makes everything working fine > again. There is also a third SDHCI populated with built-in WiFi/Bluetooth SDIO > and I'm observing odd MMC timeouts with the blk-mq enabled, disabling > CONFIG_MMC_MQ_DEFAULT fixes these timeouts as well. > > Any thoughts? SDIO (unless it is a combo card) should be unaffected by changes to the block driver. I don't have any ideas. Adding more NVIDIA people. > > WiFi issue > > > [ 38.247006] mmc2: Timeout waiting for hardware interrupt. > [ 38.247027] brcmfmac: brcmf_escan_timeout: timer expired > [ 38.247036] mmc2: sdhci: SDHCI REGISTER DUMP === > [ 38.247047] mmc2: sdhci: Sys addr: 0x | Version: 0x0001 > [ 38.247055] mmc2: sdhci: Blk size: 0x7008 | Blk cnt: 0x > [ 38.247062] mmc2: sdhci: Argument: 0x2108 | Trn mode: 0x0013 > [ 38.247070] mmc2: sdhci: Present: 0x01d7 | Host ctl: 0x0013 > [ 38.247077] mmc2: sdhci: Power: 0x0001 | Blk gap: 0x > [ 38.247084] mmc2: sdhci: Wake-up: 0x | Clock:0x0007 > [ 38.247091] mmc2: sdhci: Timeout: 0x000e | Int stat: 0x > [ 38.247098] mmc2: sdhci: Int enab: 0x02ff000b | Sig enab: 0x02fc000b > [ 38.247105] mmc2: sdhci: AC12 err: 0x | Slot int: 0x > [ 38.247112] mmc2: sdhci: Caps: 0x61ff30b0 | Caps_1: 0x > [ 38.247119] mmc2: sdhci: Cmd: 0x353a | Max curr: 0x0001 > [ 38.247126] mmc2: sdhci: Resp[0]: 0x1800 | Resp[1]: 0x08002db5 > [ 38.247133] mmc2: sdhci: Resp[2]: 0x16da8000 | Resp[3]: 0x0400 > [ 38.247139] mmc2: sdhci: Host ctl2: 0x > [ 38.247146] mmc2: sdhci: ADMA Err: 0x | ADMA Ptr: 0x17c47200 > [ 38.247152] mmc2: sdhci: > [ 38.247250] brcmfmac: brcmf_sdio_readframes: read 520 bytes from channel 1 > failed: -84 > [ 38.247274] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame, > send NAK > [ 40.807019] brcmfmac: brcmf_sdio_bus_rxctl: resumed on timeout > [ 40.807042] brcmfmac: brcmf_notify_escan_complete: Scan abort failed > [ 48.487007] mmc2: Timeout waiting for hardware interrupt. > [ 48.487057] mmc2: sdhci: SDHCI REGISTER DUMP === > [ 48.487096] mmc2: sdhci: Sys addr: 0x | Version: 0x0001 > [ 48.487128] mmc2: sdhci: Blk size: 0x7040 | Blk cnt: 0x0001 > [ 48.487160] mmc2: sdhci: Argument: 0x2140 | Trn mode: 0x0013 > [ 48.487191] mmc2: sdhci: Present: 0x01d7 | Host ctl: 0x0013 > [ 48.487221] mmc2: sdhci: Power: 0x0001 | Blk gap: 0x > [ 48.487251] mmc2: sdhci: Wake-up: 0x | Clock:0x0007 > [ 48.487281] mmc2: sdhci: Timeout: 0x000e | Int stat: 0x > [ 48.487313] mmc2: sdhci: Int enab: 0x02ff000b | Sig enab: 0x02fc000b > [ 48.487343] mmc2: sdhci: AC12 err: 0x | Slot int: 0x > [ 48.487374] mmc2: sdhci: Caps: 0x61ff30b0 | Caps_1: 0x > [ 48.487404] mmc2: sdhci: Cmd: 0x353a | Max curr: 0x0001 > [ 48.487435] mmc2: sdhci: Resp[0]: 0x1000 | Resp[1]: 0x08002db5 > [ 48.487466] mmc2: sdhci: Resp[2]: 0x16da8000 | Resp[3]: 0x0400 > [ 48.487493] mmc2: sdhci: Host ctl2: 0x > [ 48.487525] mmc2: sdhci: ADMA Err: 0x | ADMA Ptr: 0x17c47200 > [ 48.487552] mmc2: sdhci: > [ 48.487749] brcmfmac: brcmf_sdio_readframes: read 480 bytes from channel 1 > failed: -84 > [ 48.487822] brcmfmac: brcmf_sdio_rxfail: abort command, terminate frame, > send NAK > > > Soft lockup issue > > > # fsck -f /dev/disk/by-uuid/6768309f-3545-49d5-9ac7-d5be24d35ef2 > fsck из util-linux 2.30.2 > e2fsck 1.43.9 (8-Feb-2018) > Проход 1: Прове
Re: [PATCH 04/20] lightnvm: add minor version to generic geometry
On 02/21/2018 10:26 AM, Javier González wrote: Separate the version between major and minor on the generic geometry. Also, add a "subversion" entry to sysfs to expose the minor version without breaking user space. Signed-off-by: Javier González --- drivers/lightnvm/core.c | 4 ++-- drivers/nvme/host/lightnvm.c | 25 - include/linux/lightnvm.h | 3 ++- 3 files changed, 24 insertions(+), 8 deletions(-) diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c index 42596afdf64c..dc9ec6baff45 100644 --- a/drivers/lightnvm/core.c +++ b/drivers/lightnvm/core.c @@ -897,8 +897,8 @@ static int nvm_init(struct nvm_dev *dev) goto err; } - pr_debug("nvm: ver:%u nvm_vendor:%x\n", - dev_geo->ver_id, + pr_debug("nvm: ver:%u.%u nvm_vendor:%x\n", + dev_geo->major_ver_id, dev_geo->minor_ver_id, dev_geo->c.vmnt); ret = nvm_core_init(dev); diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index 598abba66f52..71b4ac57a668 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -296,7 +296,8 @@ static int nvme_nvm_setup_12(struct nvme_nvm_id12 *id, } /* 1.2 spec. only reports a single version id - unfold */ - dev_geo->ver_id = id->ver_id; + dev_geo->major_ver_id = id->ver_id; + dev_geo->minor_ver_id = 2; dev_geo->nr_chnls = src->num_ch; dev_geo->nr_luns = src->num_lun; @@ -377,7 +378,14 @@ static void nvme_nvm_set_addr_20(struct nvm_addr_format *dst, static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, struct nvm_dev_geo *dev_geo) { - dev_geo->ver_id = id->mjr; + dev_geo->major_ver_id = id->mjr; + dev_geo->minor_ver_id = id->mnr; + + if (!(dev_geo->major_ver_id == 2 && dev_geo->minor_ver_id == 0)) { + pr_err("nvm: OCSSD version not supported (v%d.%d)\n", + dev_geo->major_ver_id, dev_geo->minor_ver_id); + return -EINVAL; + } dev_geo->nr_chnls = le16_to_cpu(id->num_grp); dev_geo->nr_luns = le16_to_cpu(id->num_pu); @@ -913,7 +921,11 @@ static ssize_t nvm_dev_attr_show(struct device *dev, attr = &dattr->attr; if (strcmp(attr->name, "version") == 0) { - return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->ver_id); + return scnprintf(page, PAGE_SIZE, "%u\n", + dev_geo->major_ver_id); + } else if (strcmp(attr->name, "subversion") == 0) { + return scnprintf(page, PAGE_SIZE, "%u\n", + dev_geo->minor_ver_id); } else if (strcmp(attr->name, "media_capabilities") == 0) { return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.mccap); } else if (strcmp(attr->name, "read_typ") == 0) { @@ -1055,6 +1067,7 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev, /* general attributes */ static NVM_DEV_ATTR_RO(version); +static NVM_DEV_ATTR_RO(subversion); static NVM_DEV_ATTR_RO(media_capabilities); static NVM_DEV_ATTR_RO(read_typ); @@ -1085,6 +1098,7 @@ static NVM_DEV_ATTR_12_RO(max_phys_secs); static struct attribute *nvm_dev_attrs_12[] = { &dev_attr_version.attr, + &dev_attr_subversion.attr, &dev_attr_media_capabilities.attr, &dev_attr_vendor_opcode.attr, @@ -1134,6 +1148,7 @@ static NVM_DEV_ATTR_20_RO(reset_max); static struct attribute *nvm_dev_attrs_20[] = { &dev_attr_version.attr, + &dev_attr_subversion.attr, &dev_attr_media_capabilities.attr, &dev_attr_groups.attr, @@ -1167,7 +1182,7 @@ int nvme_nvm_register_sysfs(struct nvme_ns *ns) if (!ndev) return -EINVAL; - switch (dev_geo->ver_id) { + switch (dev_geo->major_ver_id) { case 1: return sysfs_create_group(&disk_to_dev(ns->disk)->kobj, &nvm_dev_attr_group_12); @@ -1184,7 +1199,7 @@ void nvme_nvm_unregister_sysfs(struct nvme_ns *ns) struct nvm_dev *ndev = ns->ndev; struct nvm_dev_geo *dev_geo = &ndev->dev_geo; - switch (dev_geo->ver_id) { + switch (dev_geo->major_ver_id) { case 1: sysfs_remove_group(&disk_to_dev(ns->disk)->kobj, &nvm_dev_attr_group_12); diff --git a/include/linux/lightnvm.h b/include/linux/lightnvm.h index 18e3751b1632..5af0b8837095 100644 --- a/include/linux/lightnvm.h +++ b/include/linux/lightnvm.h @@ -242,7 +242,8 @@ struct nvm_common_geo { /* Device identified geometry */ struct nvm_dev_geo { /* device reported version */ - u8 ver_id; + u8 major_ver_id; + u8 minor_ver_id; /* full device geometry */ u16 nr_chnls; Looks fairly good to me. Did you
Re: [PATCH v3 1/3] Kconfig : Remove HAS_IOMEM dependency for Graphics support
On 02/21/2018 05:39 PM, Farhan Ali wrote: > > > On 02/21/2018 07:11 AM, Christian Borntraeger wrote: >> >> >> On 02/21/2018 01:07 PM, Cornelia Huck wrote: >> [...] > But if you need to enable PCI to get IOMEM, I wonder why this patch here > is needed at all? The Graphics menu / VT dummy console should be > available in the config if IOMEM is enabled anyway? That is a good question. With CONFIG_PCI=y I can select virtio-gpu and dummy-console. IIRC the issue was that with patch 3 we can have the situation where we have CONFIG_VT = y and CONFIG_DUMMY_CONSOLE=n and this will crash early during boot as conswitchp is NULL. > > Yes, VT layer initializes very early in the boot process and looks for a > console. If it can't find any it will crash. I believe that was the whole > point of having the dummy console. > >>> >>> So in practice, CONFIG_VT depends on "there's a console available, even >>> if it's only the dummy one"? >> >> Yes. Maybe we should simple move dummy_console outside of >> drivers/video/console/Kconfig >> into something that is always available. >> > > I agree, but where should it go? consoles are kinda tightly tied to > video/Graphics Support. > >>> This patches goal was to always enable dummy console even without PCI, but it obviously fails to do so. >>> > This patch should enable the dummy console even without PCI but we won't have > DRM/Virtio GPU. All right. This patch then makes sure that VT does not crash since it guarantees that DUMMY_CONSOLE is available. If the user want to have virtio-gpu it needs to enable PCI due to the dependencies, but this is a different issue that we can fix later. So the patch is fine as long as you take care of these things - add missing HAS_IOMEM dependency to VGA_CONSOLE - extend "if HAS_IOMEM" in drivers/video/Kconfig to cover config items previously covered by this dependency (it should start before config HAVE_FB_ATMEL and end after config HDMI) as requested by Bartlomiej
[PATCH] fscache: fix a kernel BUG at fs/fscache/operation.c:69!
From: Lei Xue There is a potential race in fscache operation enqueuing for reading and copying multiple pages from cachefiles to netfs. Under some heavy load system, it will happen very often. If this race occurs, an oops similar to the following is seen: kernel BUG at fs/fscache/operation.c:69! invalid opcode: [#1] SMP … #0 [883fff0838d8] machine_kexec at 81051beb #1 [883fff083938] crash_kexec at 810f2542 #2 [883fff083a08] oops_end at 8163e1a8 #3 [883fff083a30] die at 8101859b #4 [883fff083a60] do_trap at 8163d860 #5 [883fff083ab0] do_invalid_op at 81015204 #6 [883fff083b60] invalid_op at 8164701e [exception RIP: fscache_enqueue_operation+246] RIP: a0b793c6 RSP: 883fff083c18 RFLAGS: 00010046 RAX: 0019 RBX: 8832ed1a9ec0 RCX: 0006 RDX: RSI: 0046 RDI: 0046 RBP: 883fff083c20 R8: 0086 R9: 178f R10: 816aeb00 R11: 883fff08392e R12: 8802f0525620 R13: 88407ffc01d8 R14: R15: 0003 ORIG_RAX: CS: 0010 SS: #7 [883fff083c10] fscache_enqueue_operation at a0b793c6 #8 [883fff083c28] cachefiles_read_waiter at a0b15a48 #9 [883fff083c48] __wake_up_common at 810af028 Signed-off-by: Lei Xue --- fs/cachefiles/rdwr.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/cachefiles/rdwr.c b/fs/cachefiles/rdwr.c index 883bc7bb12c5..9d5d13e150fb 100644 --- a/fs/cachefiles/rdwr.c +++ b/fs/cachefiles/rdwr.c @@ -58,9 +58,9 @@ static int cachefiles_read_waiter(wait_queue_entry_t *wait, unsigned mode, spin_lock(&object->work_lock); list_add_tail(&monitor->op_link, &monitor->op->to_do); + fscache_enqueue_retrieval(monitor->op); spin_unlock(&object->work_lock); - fscache_enqueue_retrieval(monitor->op); return 0; } -- 2.14.3 (Apple Git-98)
Re: [PATCH 03/20] lightnvm: fix capabilities for 2.0 sysfs
On 02/21/2018 10:26 AM, Javier González wrote: Both 1.2 and 2.0 specs define a field for media and controller capabilities. Also, 1.2 defines a separate field dedicated to device capabilities. In 2.0 sysfs, this values have been mixed. Revert them to the right value. Signed-off-by: Javier González --- drivers/nvme/host/lightnvm.c | 18 +- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index 969bb874850c..598abba66f52 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -914,8 +914,8 @@ static ssize_t nvm_dev_attr_show(struct device *dev, if (strcmp(attr->name, "version") == 0) { return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->ver_id); - } else if (strcmp(attr->name, "capabilities") == 0) { - return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.cap); + } else if (strcmp(attr->name, "media_capabilities") == 0) { + return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.mccap); } else if (strcmp(attr->name, "read_typ") == 0) { return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.trdt); } else if (strcmp(attr->name, "read_max") == 0) { @@ -993,8 +993,8 @@ static ssize_t nvm_dev_attr_show_12(struct device *dev, return scnprintf(page, PAGE_SIZE, "%u\n", dev_geo->c.tbem); } else if (strcmp(attr->name, "multiplane_modes") == 0) { return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mpos); - } else if (strcmp(attr->name, "media_capabilities") == 0) { - return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.mccap); + } else if (strcmp(attr->name, "capabilities") == 0) { + return scnprintf(page, PAGE_SIZE, "0x%08x\n", dev_geo->c.cap); } else if (strcmp(attr->name, "max_phys_secs") == 0) { return scnprintf(page, PAGE_SIZE, "%u\n", NVM_MAX_VLBA); } else { @@ -1055,7 +1055,7 @@ static ssize_t nvm_dev_attr_show_20(struct device *dev, /* general attributes */ static NVM_DEV_ATTR_RO(version); -static NVM_DEV_ATTR_RO(capabilities); +static NVM_DEV_ATTR_RO(media_capabilities); static NVM_DEV_ATTR_RO(read_typ); static NVM_DEV_ATTR_RO(read_max); @@ -1080,12 +1080,12 @@ static NVM_DEV_ATTR_12_RO(prog_max); static NVM_DEV_ATTR_12_RO(erase_typ); static NVM_DEV_ATTR_12_RO(erase_max); static NVM_DEV_ATTR_12_RO(multiplane_modes); -static NVM_DEV_ATTR_12_RO(media_capabilities); +static NVM_DEV_ATTR_12_RO(capabilities); static NVM_DEV_ATTR_12_RO(max_phys_secs); static struct attribute *nvm_dev_attrs_12[] = { &dev_attr_version.attr, - &dev_attr_capabilities.attr, + &dev_attr_media_capabilities.attr, &dev_attr_vendor_opcode.attr, &dev_attr_device_mode.attr, @@ -1108,7 +1108,7 @@ static struct attribute *nvm_dev_attrs_12[] = { &dev_attr_erase_typ.attr, &dev_attr_erase_max.attr, &dev_attr_multiplane_modes.attr, - &dev_attr_media_capabilities.attr, + &dev_attr_capabilities.attr, &dev_attr_max_phys_secs.attr, NULL, @@ -1134,7 +1134,7 @@ static NVM_DEV_ATTR_20_RO(reset_max); static struct attribute *nvm_dev_attrs_20[] = { &dev_attr_version.attr, - &dev_attr_capabilities.attr, + &dev_attr_media_capabilities.attr, &dev_attr_groups.attr, &dev_attr_punits.attr, With the mccap changes, it should make sense to keep the capabilities as is.
Re: [PATCH 02/20] lightnvm: add controller capabilities to 2.0
On 02/21/2018 10:26 AM, Javier González wrote: Assign missing mccap value on 2.0 path Signed-off-by: Javier González --- drivers/nvme/host/lightnvm.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/drivers/nvme/host/lightnvm.c b/drivers/nvme/host/lightnvm.c index c81e64cc20d7..969bb874850c 100644 --- a/drivers/nvme/host/lightnvm.c +++ b/drivers/nvme/host/lightnvm.c @@ -392,6 +392,8 @@ static int nvme_nvm_setup_20(struct nvme_nvm_id20 *id, dev_geo->c.ws_opt = le32_to_cpu(id->ws_opt); dev_geo->c.mw_cunits = le32_to_cpu(id->mw_cunits); + dev_geo->c.mccap = le32_to_cpu(id->mccap); + dev_geo->c.trdt = le32_to_cpu(id->trdt); dev_geo->c.trdm = le32_to_cpu(id->trdm); dev_geo->c.tprt = le32_to_cpu(id->twrt); The mccap field between 1.2 and 2.0 is orthogonal. They represent two different capabilities fields. The mccap has to be interpreted if used.
Re: [PATCH 01/13] metag: Remove arch/metag/
On Wed, Feb 21, 2018 at 07:52:08PM -0800, Guenter Roeck wrote: > On 02/21/2018 03:38 PM, James Hogan wrote: > > The earliest Meta architecture port of Linux I have a record of was an > > import of a Meta port of Linux v2.4.1 in February 2004, which was worked > > on significantly over the next few years by Graham Whaley, Will Newton, > > Matt Fleming, myself and others. > > > > Eventually the port was merged into mainline in v3.9 in March 2013, not > > long after Imagination Technologies bought MIPS Technologies and shifted > > its CPU focus over to the MIPS architecture. > > > > As a result, though the port was maintained for a while, kept on life > > support for a while longer, and useful for testing a few specific > > drivers for which I don't have ready access to the equivalent MIPS > > hardware, it is now essentially dead with no users. > > > > It is also stuck using an out-of-tree toolchain based on GCC 4.2.4 which > > is no longer maintained, now struggles to build modern kernels due to > > toolchain bugs, and doesn't itself build with a modern GCC. The latest > > buildroot port is still using an old uClibc snapshot which is no longer > > served, and the latest uClibc doesn't build with GCC 4.2.4. > > > > So lets call it a day and drop the Meta architecture port from the > > kernel. RIP Meta. > > > > Signed-off-by: James Hogan > > Link: > > https://lkml.kernel.org/r/95906b76-6ce1-3f84-eaba-c29b4ae95...@roeck-us.net > > Cc: Guenter Roeck > > Cc: linux-me...@vger.kernel.org > > FWIW: > > Reviewed-by: Guenter Roeck Thanks > > Did you drop the definition of CPUHP_AP_PERF_METAG_STARTING ? > I browsed through the patches but didn't find where it was dropped. Hmm, somehow that one slipped through my grepping. Thanks for pointing out. > > Also, how did you generate this patch, and can you try to apply it yourself ? > I tried to apply it, but neither git am nor patch worked for me. This patch > also doesn't show up on patchwork.kernel.org which is odd. I presume because I used -D to git format-patch, which is apparently only intended for human consumption (the full patch is 778K). I've pushed this version of the series here: git://git.kernel.org/pub/scm/linux/kernel/git/jhogan/metag.git tag: metag_remove_v1 Cheers James signature.asc Description: Digital signature
Re: [PATCH 01/20] lightnvm: simplify geometry structure.
On 02/21/2018 10:26 AM, Javier González wrote: Currently, the device geometry is stored redundantly in the nvm_id and nvm_geo structures at a device level. Moreover, when instantiating targets on a specific number of LUNs, these structures are replicated and manually modified to fit the instance channel and LUN partitioning. Instead, create a generic geometry around two base structures: nvm_dev_geo, which describes the geometry of the whole device and nvm_geo, which describes the geometry of the instance. Since these share a big part of the geometry, create a nvm_common_geo structure that keeps the static geoometry values that are shared across instances. As we introduce support for 2.0, these structures allow to abstract spec. specific values and present a common geometry to targets. Signed-off-by: Javier González --- drivers/lightnvm/core.c | 137 +++- drivers/lightnvm/pblk-core.c | 16 +- drivers/lightnvm/pblk-gc.c | 2 +- drivers/lightnvm/pblk-init.c | 123 +++--- drivers/lightnvm/pblk-read.c | 2 +- drivers/lightnvm/pblk-recovery.c | 14 +- drivers/lightnvm/pblk-rl.c | 2 +- drivers/lightnvm/pblk-sysfs.c| 39 +++-- drivers/lightnvm/pblk-write.c| 2 +- drivers/lightnvm/pblk.h | 93 +-- drivers/nvme/host/lightnvm.c | 339 +++ include/linux/lightnvm.h | 204 --- 12 files changed, 514 insertions(+), 459 deletions(-) diff --git a/drivers/lightnvm/core.c b/drivers/lightnvm/core.c index 689c97b97775..42596afdf64c 100644 --- a/drivers/lightnvm/core.c +++ b/drivers/lightnvm/core.c @@ -111,6 +111,7 @@ static void nvm_release_luns_err(struct nvm_dev *dev, int lun_begin, static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear) { struct nvm_dev *dev = tgt_dev->parent; + struct nvm_dev_geo *dev_geo = &dev->dev_geo; struct nvm_dev_map *dev_map = tgt_dev->map; int i, j; @@ -122,7 +123,7 @@ static void nvm_remove_tgt_dev(struct nvm_tgt_dev *tgt_dev, int clear) if (clear) { for (j = 0; j < ch_map->nr_luns; j++) { int lun = j + lun_offs[j]; - int lunid = (ch * dev->geo.nr_luns) + lun; + int lunid = (ch * dev_geo->nr_luns) + lun; WARN_ON(!test_and_clear_bit(lunid, dev->lun_map)); @@ -143,19 +144,20 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct nvm_dev *dev, u16 lun_begin, u16 lun_end, u16 op) { + struct nvm_dev_geo *dev_geo = &dev->dev_geo; struct nvm_tgt_dev *tgt_dev = NULL; struct nvm_dev_map *dev_rmap = dev->rmap; struct nvm_dev_map *dev_map; struct ppa_addr *luns; int nr_luns = lun_end - lun_begin + 1; int luns_left = nr_luns; - int nr_chnls = nr_luns / dev->geo.nr_luns; - int nr_chnls_mod = nr_luns % dev->geo.nr_luns; - int bch = lun_begin / dev->geo.nr_luns; - int blun = lun_begin % dev->geo.nr_luns; + int nr_chnls = nr_luns / dev_geo->nr_luns; + int nr_chnls_mod = nr_luns % dev_geo->nr_luns; + int bch = lun_begin / dev_geo->nr_luns; + int blun = lun_begin % dev_geo->nr_luns; int lunid = 0; int lun_balanced = 1; - int prev_nr_luns; + int sec_per_lun, prev_nr_luns; int i, j; nr_chnls = (nr_chnls_mod == 0) ? nr_chnls : nr_chnls + 1; @@ -173,15 +175,15 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct nvm_dev *dev, if (!luns) goto err_luns; - prev_nr_luns = (luns_left > dev->geo.nr_luns) ? - dev->geo.nr_luns : luns_left; + prev_nr_luns = (luns_left > dev_geo->nr_luns) ? + dev_geo->nr_luns : luns_left; for (i = 0; i < nr_chnls; i++) { struct nvm_ch_map *ch_rmap = &dev_rmap->chnls[i + bch]; int *lun_roffs = ch_rmap->lun_offs; struct nvm_ch_map *ch_map = &dev_map->chnls[i]; int *lun_offs; - int luns_in_chnl = (luns_left > dev->geo.nr_luns) ? - dev->geo.nr_luns : luns_left; + int luns_in_chnl = (luns_left > dev_geo->nr_luns) ? + dev_geo->nr_luns : luns_left; if (lun_balanced && prev_nr_luns != luns_in_chnl) lun_balanced = 0; @@ -215,18 +217,22 @@ static struct nvm_tgt_dev *nvm_create_tgt_dev(struct nvm_dev *dev, if (!tgt_dev) goto err_ch; - memcpy(&tgt_dev->geo, &dev->geo, sizeof(struct nvm_geo)); /* Target device only owns a portion of the physical device */ tgt_dev->geo.nr_chnls = nr_chnls;
Re: mmotm 2018-02-21-14-48 uploaded (mm/page_alloc.c on UML)
On Wed 21-02-18 15:58:41, Randy Dunlap wrote: > On 02/21/2018 02:48 PM, a...@linux-foundation.org wrote: > > The mm-of-the-moment snapshot 2018-02-21-14-48 has been uploaded to > > > >http://www.ozlabs.org/~akpm/mmotm/ > > > > mmotm-readme.txt says > > > > README for mm-of-the-moment: > > > > http://www.ozlabs.org/~akpm/mmotm/ > > > > This is a snapshot of my -mm patch queue. Uploaded at random hopefully > > more than once a week. > > > > You will need quilt to apply these patches to the latest Linus release (4.x > > or 4.x-rcY). The series file is in broken-out.tar.gz and is duplicated in > > http://ozlabs.org/~akpm/mmotm/series > > > > The file broken-out.tar.gz contains two datestamp files: .DATE and > > .DATE--mm-dd-hh-mm-ss. Both contain the string -mm-dd-hh-mm-ss, > > followed by the base kernel version against which this patch series is to > > be applied. > > um (or uml) defconfig on i386 and/or x86_64: > > ../mm/page_alloc.c: In function 'memmap_init_zone': > ../mm/page_alloc.c:5450:5: error: implicit declaration of function > 'memblock_next_valid_pfn' [-Werror=implicit-function-declaration] > pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1; > ^ > > > probably (?): > From: Eugeniu Rosca > Subject: mm: page_alloc: skip over regions of invalid pfns on UMA Yes. Steven has already reported the same [1]. There are two possible ways around this. Either provide and empty stub or use ifdef around memblock_next_valid_pfn. I would use the later because it is less confusing. We really do not want memblock_next_valid_pfn to be used outside of memblock aware code. [1] http://lkml.kernel.org/r/20180222143057.3a1b3...@canb.auug.org.au diff --git a/mm/page_alloc.c b/mm/page_alloc.c index 4334d3a9c6a2..2836bc9e0999 100644 --- a/mm/page_alloc.c +++ b/mm/page_alloc.c @@ -5446,8 +5446,9 @@ void __meminit memmap_init_zone(unsigned long size, int nid, unsigned long zone, * end_pfn), such that we hit a valid pfn (or end_pfn) * on our next iteration of the loop. */ - if (IS_ENABLED(CONFIG_HAVE_MEMBLOCK)) - pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1; +#ifdef CONFIG_HAVE_MEMBLOCK + pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1; +#endif continue; } if (!early_pfn_in_nid(pfn, nid)) -- Michal Hocko SUSE Labs
Re: [PATCH] dt-bindings: fpga: Consolidate bridge properties
On 21.2.2018 18:33, Moritz Fischer wrote: > Consolidate bridge properties in a single file, instead of duplicating > the same optional property over and over again. > > Signed-off-by: Moritz Fischer > Cc: Alan Tull > Cc: Rob Herring > --- > .../devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt | 5 + > .../devicetree/bindings/fpga/altera-freeze-bridge.txt | 5 + > .../devicetree/bindings/fpga/altera-hps2fpga-bridge.txt | 5 + > Documentation/devicetree/bindings/fpga/fpga-bridge.txt| 15 > +++ > .../devicetree/bindings/fpga/xilinx-pr-decoupler.txt | 8 ++-- > 5 files changed, 20 insertions(+), 18 deletions(-) > create mode 100644 Documentation/devicetree/bindings/fpga/fpga-bridge.txt > > diff --git > a/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > index 817a8d4bf903..5dd0ff0f7b4e 100644 > --- a/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > +++ b/Documentation/devicetree/bindings/fpga/altera-fpga2sdram-bridge.txt > @@ -3,10 +3,7 @@ Altera FPGA To SDRAM Bridge Driver > Required properties: > - compatible : Should contain "altr,socfpga-fpga2sdram-bridge" > > -Optional properties: > -- bridge-enable : 0 if driver should disable bridge at startup > - 1 if driver should enable bridge at startup > - Default is to leave bridge in current state. > +See Documentation/devicetree/bindings/fpga/fpga-bridge.txt for generic > bindings. > > Example: > fpga_bridge3: fpga-bridge@ffc25080 { > diff --git a/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt > b/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt > index f8e288c71b2d..8b26fbcff3c6 100644 > --- a/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt > +++ b/Documentation/devicetree/bindings/fpga/altera-freeze-bridge.txt > @@ -10,10 +10,7 @@ Required properties: > - compatible : Should contain "altr,freeze-bridge-controller" > - regs : base address and size for freeze bridge module > > -Optional properties: > -- bridge-enable : 0 if driver should disable bridge at startup > - 1 if driver should enable bridge at startup > - Default is to leave bridge in current state. > +See Documentation/devicetree/bindings/fpga/fpga-bridge.txt for generic > bindings. > > Example: > freeze-controller@10450 { > diff --git > a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > index 6406f9337eeb..68cce3945b10 100644 > --- a/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > +++ b/Documentation/devicetree/bindings/fpga/altera-hps2fpga-bridge.txt > @@ -9,10 +9,7 @@ Required properties: > - resets : Phandle and reset specifier for this bridge's reset > - clocks : Clocks used by this module. > > -Optional properties: > -- bridge-enable : 0 if driver should disable bridge at startup. > - 1 if driver should enable bridge at startup. > - Default is to leave bridge in its current state. > +See Documentation/devicetree/bindings/fpga/fpga-bridge.txt for generic > bindings. > > Example: > fpga_bridge0: fpga-bridge@ff40 { > diff --git a/Documentation/devicetree/bindings/fpga/fpga-bridge.txt > b/Documentation/devicetree/bindings/fpga/fpga-bridge.txt > new file mode 100644 > index ..82607b23a287 > --- /dev/null > +++ b/Documentation/devicetree/bindings/fpga/fpga-bridge.txt > @@ -0,0 +1,15 @@ > +FPGA Bridge Device Tree Binding > + > +Moritz Fischer 2018 (Consolidated from previous bindings) nit: I would remove your name and year - it is visible from git log. > + > +Optional properties: > +- bridge-enable : 0 if driver should disable bridge at startup > + 1 if driver should enable bridge at startup > + Default is to leave bridge in current state. > + > +Example: > + fpga_bridge3: fpga-bridge@ffc25080 { > + compatible = "altr,socfpga-fpga2sdram-bridge"; > + reg = <0xffc25080 0x4>; > + bridge-enable = <0>; > + }; > diff --git a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt > b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt > index 8dcfba926bc7..4284d293fa61 100644 > --- a/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt > +++ b/Documentation/devicetree/bindings/fpga/xilinx-pr-decoupler.txt > @@ -18,12 +18,8 @@ Required properties: > - clocks : input clock to IP > - clock-names: should contain "aclk" > > -Optional properties: > -- bridge-enable : 0 if driver should disable bridge at startup > - 1 if driver should enable
Re: linux-next: build failure after merge of the akpm-current tree
On Thu 22-02-18 14:30:57, Stephen Rothwell wrote: > Hi Andrew, > > [As reported by Randy for uml ...] > > After merging the akpm-current tree, today's linux-next build (sparc > defconfig) failed like this: > > /home/sfr/next/next/mm/page_alloc.c: In function 'memmap_init_zone': > /home/sfr/next/next/mm/page_alloc.c:5450:11: error: implicit declaration of > function 'memblock_next_valid_pfn'; did you mean 'memblock_virt_alloc_low'? > [-Werror=implicit-function-declaration] > pfn = memblock_next_valid_pfn(pfn, end_pfn) - 1; >^~~ >memblock_virt_alloc_low This is interesting. I thought that IS_ENABLED(CONFIG_HAVE_MEMBLOCK) would have the same meaning as ifdef CONFIG_HAVE_MEMBLOCK so the branch will never be considered. If that is not the case then I would rather reintroduce that ifdef. We already have those in the function anyway. -- Michal Hocko SUSE Labs
Re: [PATCH 4.15 000/163] 4.15.5-stable review
On Wed, Feb 21, 2018 at 01:13:46PM -0700, Shuah Khan wrote: > On 02/21/2018 05:47 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.15.5 release. > > There are 163 patches in this series, all will be posted as a response > > to this one. If anyone has any issues with these being applied, please > > let me know. > > > > Responses should be made by Fri Feb 23 12:44:46 UTC 2018. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > > > https://www.kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.15.5-rc1.gz > > or in the git tree and branch at: > > > > git://git.kernel.org/pub/scm/linux/kernel/git/stable/linux-stable-rc.git > > linux-4.15.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Compiled and booted on my test system. No dmesg regressions. Thanks for testing all of these and letting me know. greg k-h
[RFC tip/locking/lockdep v5 04/17] lockdep: Introduce lock_list::dep
To add recursive read locks into the dependency graph, we need to store the types of dependencies for the BFS later. There are four kinds of dependencies: * Non-recursive -> Non-recursive dependencies(NN) e.g. write_lock(prev) -> write_lock(next), we can also write this as "prev --(NN)--> next". * Recursive -> Non-recursive dependencies(RN) e.g. read_lock(prev) -> write_lock(next), we can also write this as "prev --(RN)--> next". * Non-recursive -> recursive dependencies(NR) e.g. write_lock(prev) -> read_lock(next), we can also write this as "prev --(NR)--> next". * Recursive -> recursive dependencies(RR) e.g. read_lock(prev) -> read_lock(next), we can also write this as "prev --(RR)--> next". Given a pair of two locks, four kinds of dependencies could all exist between them, so we use 4 bit for the presence of each kind(stored in lock_list::dep). Helper functions and marcos are also introduced to convert a pair of locks into ::dep bit and maintain the addition of different kinds of dependencies. Signed-off-by: Boqun Feng --- include/linux/lockdep.h | 2 ++ kernel/locking/lockdep.c | 48 +--- 2 files changed, 47 insertions(+), 3 deletions(-) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index 6fc77d4dbdcd..ab1e5a7d8864 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -187,6 +187,8 @@ struct lock_list { struct lock_class *class; struct stack_trace trace; int distance; + /* bitmap of different dependencies from head to this */ + u16 dep; /* * The parent field is used to implement breadth-first search, and the diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 5e6bf8d6954d..acd25bfc336d 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -859,7 +859,7 @@ static struct lock_list *alloc_list_entry(void) * Add a new dependency to the head of the list: */ static int add_lock_to_list(struct lock_class *this, struct list_head *head, - unsigned long ip, int distance, + unsigned long ip, int distance, unsigned int dep, struct stack_trace *trace) { struct lock_list *entry; @@ -872,6 +872,7 @@ static int add_lock_to_list(struct lock_class *this, struct list_head *head, return 0; entry->class = this; + entry->dep = dep; entry->distance = distance; entry->trace = *trace; /* @@ -1012,6 +1013,33 @@ static inline bool bfs_error(enum bfs_result res) return res < 0; } +#define DEP_NN_BIT 0 +#define DEP_RN_BIT 1 +#define DEP_NR_BIT 2 +#define DEP_RR_BIT 3 + +#define DEP_NN_MASK (1U << (DEP_NN_BIT)) +#define DEP_RN_MASK (1U << (DEP_RN_BIT)) +#define DEP_NR_MASK (1U << (DEP_NR_BIT)) +#define DEP_RR_MASK (1U << (DEP_RR_BIT)) + +static inline unsigned int __calc_dep_bit(int prev, int next) +{ + if (prev == 2 && next != 2) + return DEP_RN_BIT; + if (prev != 2 && next == 2) + return DEP_NR_BIT; + if (prev == 2 && next == 2) + return DEP_RR_BIT; + else + return DEP_NN_BIT; +} + +static inline unsigned int calc_dep(int prev, int next) +{ + return 1U << __calc_dep_bit(prev, next); +} + static enum bfs_result __bfs(struct lock_list *source_entry, void *data, int (*match)(struct lock_list *entry, void *data), @@ -1921,6 +1949,16 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, if (entry->class == hlock_class(next)) { if (distance == 1) entry->distance = 1; + entry->dep |= calc_dep(prev->read, next->read); + } + } + + /* Also, update the reverse dependency in @next's ->locks_before list */ + list_for_each_entry(entry, &hlock_class(next)->locks_before, entry) { + if (entry->class == hlock_class(prev)) { + if (distance == 1) + entry->distance = 1; + entry->dep |= calc_dep(next->read, prev->read); return 1; } } @@ -1948,14 +1986,18 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, */ ret = add_lock_to_list(hlock_class(next), &hlock_class(prev)->locks_after, - next->acquire_ip, distance, trace); + next->acquire_ip, distance, + calc_dep(prev->read, next->read), + trace); if (!ret) return 0; ret = ad
[RFC tip/locking/lockdep v5 02/17] lockdep: Make __bfs() visit every dependency until a match
Currently, __bfs() will do a breadth-first search in the dependency graph and visit each lock class in the graph exactly once, so for example, in the following graph: A -> B |^ || +--> C a __bfs() call starts at A, will visit B through dependency A -> B and visit C through dependency A -> C and that's it, IOW, __bfs() will not visit dependency C -> B. This is OK for now, as we only have strong dependencies in the dependency graph, so whenever there is a traverse path from A to B in __bfs(), it means A has strong dependency to B(IOW, B depends on A strongly). So no need to visit all dependencies in the graph. However, as we are going to add recursive-read lock into the dependency graph, afterwards, not all the paths mean strong dependencies, in the same example above, dependency A -> B may be a weak dependency and traverse A -> C -> B may be a strong dependency path. And with the old way of __bfs()(i.e. visiting every lock class exactly once), we will miss the strong dependency path, which will result into failing to find a deadlock. To cure this for the future, we need to find a way for __bfs() to visit each dependency, rather than each class, exactly once in the search until we find a match. The solution is simple: We used to mark lock_class::lockdep_dependency_gen_id to indicate a class has been visited in __bfs(), now we change the semantics a little bit: we now mark lock_class::lockdep_dependency_gen_id to indicate _all the dependencies_ in its lock_{after,before} have been visited in the __bfs()(note we only take one direction in a __bfs() search). In this way, every dependency is guaranteed to be visited until we find a match. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 61 +++- 1 file changed, 34 insertions(+), 27 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 9b2e318bcc81..c80f8276ceaa 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -948,24 +948,20 @@ static inline unsigned int __cq_get_elem_count(struct circular_queue *cq) return (cq->rear - cq->front) & CQ_MASK; } -static inline void mark_lock_accessed(struct lock_list *lock, - struct lock_list *parent) +static inline void mark_lock_list_accessed(struct lock_class *class) { - unsigned long nr; + class->dep_gen_id = lockdep_dependency_gen_id; +} - nr = lock - list_entries; - WARN_ON(nr >= nr_list_entries); /* Out-of-bounds, input fail */ +static inline void visit_lock_entry(struct lock_list *lock, + struct lock_list *parent) +{ lock->parent = parent; - lock->class->dep_gen_id = lockdep_dependency_gen_id; } -static inline unsigned long lock_accessed(struct lock_list *lock) +static inline unsigned long lock_list_accessed(struct lock_class *class) { - unsigned long nr; - - nr = lock - list_entries; - WARN_ON(nr >= nr_list_entries); /* Out-of-bounds, input fail */ - return lock->class->dep_gen_id == lockdep_dependency_gen_id; + return class->dep_gen_id == lockdep_dependency_gen_id; } static inline struct lock_list *get_lock_parent(struct lock_list *child) @@ -1054,6 +1050,18 @@ static enum bfs_result __bfs(struct lock_list *source_entry, goto exit; } + /* +* If we have visited all the dependencies from this @lock to +* others(iow, if we have visited all lock_list entries in +* @lock->class->locks_{after,before}, we skip, otherwise go +* and visit all the dependencies in the list and mark this +* list accessed. +*/ + if (lock_list_accessed(lock->class)) + continue; + else + mark_lock_list_accessed(lock->class); + if (forward) head = &lock->class->locks_after; else @@ -1062,23 +1070,22 @@ static enum bfs_result __bfs(struct lock_list *source_entry, DEBUG_LOCKS_WARN_ON(!irqs_disabled()); list_for_each_entry_rcu(entry, head, entry) { - if (!lock_accessed(entry)) { - unsigned int cq_depth; - mark_lock_accessed(entry, lock); - if (match(entry, data)) { - *target_entry = entry; - ret = BFS_RMATCH; - goto exit; - } + unsigned int cq_depth; - if (__cq_enqueue(cq, (unsigned long)entry)) { - ret = BFS_EQUEUEFULL; - g
[RFC tip/locking/lockdep v5 05/17] lockdep: Extend __bfs() to work with multiple kinds of dependencies
Now we have four kinds of dependencies in the dependency graph, and not all the pathes carry strong dependencies, for example: Given lock A, B, C, if we have: CPU1CPU2 = == write_lock(A); read_lock(B); read_lock(B); write_lock(C); then we have dependencies A--(NR)-->B, and B--(RN)-->C, (NR and RN are to indicate the dependency kind), A actually doesn't have strong dependency to C(IOW, C doesn't depend on A), to see this, let's say we have a third CPU3 doing: CPU3: = write_lock(C); write_lock(A); , this is not a deadlock. However if we change the read_lock() on CPU2 to a write_lock(), it's a deadlock then. So A --(NR)--> B --(RN)--> C is not a strong dependency path but A --(NR)--> B --(NN)-->C is a strong dependency path. We can generalize this as: If a path of dependencies doesn't have two adjacent dependencies as (*R)--L-->(R*), where L is some lock, it is a strong dependency path, otherwise it's not. Now our mission is to make __bfs() traverse only the strong dependency paths, which is simple: we record whether we have -(*R)-> at the current tail of the path in lock_list::is_rr, and whenever we pick a dependency in the traverse, we 1) make sure we don't pick a -(R*)-> dependency if our current tail is -(*R)-> and 2) greedily pick a -(*N)-> as hard as possible. With this extension for __bfs(), we now need to initialize the root of __bfs() properly(with a correct ->is_rr), to do so, we introduce some helper functions, which also cleans up a little bit for the __bfs() root initialization code. Signed-off-by: Boqun Feng --- include/linux/lockdep.h | 2 + kernel/locking/lockdep.c | 116 --- 2 files changed, 101 insertions(+), 17 deletions(-) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index ab1e5a7d8864..a1f91f8680bd 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -189,6 +189,8 @@ struct lock_list { int distance; /* bitmap of different dependencies from head to this */ u16 dep; + /* used by BFS to record whether this is picked as a recursive read */ + u16 is_rr; /* * The parent field is used to implement breadth-first search, and the diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index acd25bfc336d..07bcfaac6fe2 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1040,6 +1040,89 @@ static inline unsigned int calc_dep(int prev, int next) return 1U << __calc_dep_bit(prev, next); } +/* + * return -1 if no proper dependency could be picked + * return 0 if a -(*N)-> dependency could be picked + * return 1 if only a -(*R)-> dependency could be picked + * + * N: non-recursive lock + * R: recursive read lock + */ +static inline int pick_dep(u16 is_rr, u16 cap_dep) +{ + if (is_rr) { /* could only pick -(N*)-> */ + if (cap_dep & DEP_NN_MASK) + return 0; + else if (cap_dep & DEP_NR_MASK) + return 1; + else + return -1; + } else { + if (cap_dep & DEP_NN_MASK || cap_dep & DEP_RN_MASK) + return 0; + else + return 1; + } +} + +/* + * Initialize a lock_list entry @lock belonging to @class as the root for a BFS + * search. + */ +static inline void __bfs_init_root(struct lock_list *lock, + struct lock_class *class) +{ + lock->class = class; + lock->parent = NULL; + lock->is_rr = 0; +} + +/* + * Initialize a lock_list entry @lock based on a lock acquisition @hlock as the + * root for a BFS search. + */ +static inline void bfs_init_root(struct lock_list *lock, +struct held_lock *hlock) +{ + __bfs_init_root(lock, hlock_class(hlock)); + lock->is_rr = (hlock->read == 2); +} + +/* + * Breadth-First Search to find a strong path in the dependency graph. + * + * @source_entry: the source of the path we are searching for. + * @data: data used for the second parameter of @match function + * @match: match function for the search + * @target_entry: pointer to the target of a matched path + * @forward: direction of path, the lockdep dependency forward or backward + * + * We may have multiple edges(considering different kinds of dependencies, e.g. + * -(NR)-> and -(RN)->) between two nodes in the dependency graph, which may + * undermine the normal BFS algorithm, however, we are lucky because: in the + * search, for each pair of adjacent nodes, we can pick the edge greedily: + * + * Say we have nodes L0, L1 and L2, and we already pick edge from L0
[RFC tip/locking/lockdep v5 08/17] lockdep: Fix recursive read lock related safe->unsafe detection
There are four cases for recursive read lock realted deadlocks: (--(X..Y)--> means a strong dependency path starts with a --(X*)--> dependency and ends with a --(*Y)-- dependency.) 1. An irq-safe lock L1 has a dependency --(*..*)--> to an irq-unsafe lock L2. 2. An irq-read-safe lock L1 has a dependency --(N..*)--> to an irq-unsafe lock L2. 3. An irq-safe lock L1 has a dependency --(*..N)--> to an irq-read-unsafe lock L2. 4. An irq-read-safe lock L1 has a dependency --(N..N)--> to an irq-read-unsafe lock L2. The current check_usage() only checks 1) and 2), so this patch adds checks for 3) and 4) and makes sure when find_usage_{back,for}wards find an irq-read-{,un}safe lock, the traverse path should ends at a dependency --(*N)-->. Note when we search backwards, --(*N)--> indicates a real dependency --(N*)-->. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 17 - 1 file changed, 16 insertions(+), 1 deletion(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 0b0ad3db78b4..bd3eef664f9d 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1504,7 +1504,14 @@ check_redundant(struct lock_list *root, struct held_lock *target, static inline int usage_match(struct lock_list *entry, void *bit) { - return entry->class->usage_mask & (1 << (enum lock_usage_bit)bit); + enum lock_usage_bit ub = (enum lock_usage_bit)bit; + + + if (ub & 1) + return entry->class->usage_mask & (1 << ub) && + !entry->is_rr; + else + return entry->class->usage_mask & (1 << ub); } @@ -1815,6 +1822,10 @@ static int check_irq_usage(struct task_struct *curr, struct held_lock *prev, exclusive_bit(bit), state_name(bit))) return 0; + if (!check_usage(curr, prev, next, bit, + exclusive_bit(bit) + 1, state_name(bit))) + return 0; + bit++; /* _READ */ /* @@ -1827,6 +1838,10 @@ static int check_irq_usage(struct task_struct *curr, struct held_lock *prev, exclusive_bit(bit), state_name(bit))) return 0; + if (!check_usage(curr, prev, next, bit, + exclusive_bit(bit) + 1, state_name(bit))) + return 0; + return 1; } -- 2.16.1
[RFC tip/locking/lockdep v5 07/17] lockdep: Adjust check_redundant() for recursive read change
As we have four kinds of dependencies now, check_redundant() should only report redundant if we have a dependency path which is equal or _stronger_ than the current dependency. For example if in check_prev_add() we have: prev->read == 2 && next->read != 2 , we should only report redundant if we find a path like: prev--(RN)-->--(*N)-->next and if we have: prev->read == 2 && next->read == 2 , we could report redundant if we find a path like: prev--(RN)-->--(*N)-->next or prev--(RN)-->--(*R)-->next To do so, we need to pass the recursive-read status of @next into check_redundant(). This patch changes the parameter of check_redundant() and the match function to achieve this. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 13 - 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index e1be088a34c4..0b0ad3db78b4 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1338,9 +1338,12 @@ print_circular_bug_header(struct lock_list *entry, unsigned int depth, return 0; } -static inline int class_equal(struct lock_list *entry, void *data) +static inline int hlock_equal(struct lock_list *entry, void *data) { - return entry->class == data; + struct held_lock *hlock = (struct held_lock *)data; + + return hlock_class(hlock) == entry->class && + (hlock->read == 2 || !entry->is_rr); } static inline int hlock_conflict(struct lock_list *entry, void *data) @@ -1480,14 +1483,14 @@ check_noncircular(struct lock_list *root, struct held_lock *target, } static noinline enum bfs_result -check_redundant(struct lock_list *root, struct lock_class *target, +check_redundant(struct lock_list *root, struct held_lock *target, struct lock_list **target_entry) { enum bfs_result result; debug_atomic_inc(nr_redundant_checks); - result = __bfs_forwards(root, target, class_equal, target_entry); + result = __bfs_forwards(root, target, hlock_equal, target_entry); return result; } @@ -2060,7 +2063,7 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, * Is the -> link redundant? */ bfs_init_root(&this, prev); - ret = check_redundant(&this, hlock_class(next), &target_entry); + ret = check_redundant(&this, next, &target_entry); if (ret == BFS_RMATCH) { debug_atomic_inc(nr_redundant); return 2; -- 2.16.1
[RFC tip/locking/lockdep v5 17/17] MAINTAINERS: Add myself as a LOCKING PRIMITIVES reviewer
Recursive read lock detection work touches most core part of lockdep, so add myself as a dedicated reviewer to help people find me if any of my code introduces problems or misunderstandings, also if they need my help on parsing logs related to recursive read locks. Besides, I'd like to provide any help for lock related code. Signed-off-by: Boqun Feng --- MAINTAINERS | 1 + 1 file changed, 1 insertion(+) diff --git a/MAINTAINERS b/MAINTAINERS index 9a7f76eadae9..197dc9c5162f 100644 --- a/MAINTAINERS +++ b/MAINTAINERS @@ -8239,6 +8239,7 @@ F:Documentation/admin-guide/LSM/LoadPin.rst LOCKING PRIMITIVES M: Peter Zijlstra M: Ingo Molnar +R: Boqun Feng L: linux-kernel@vger.kernel.org T: git git://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git locking/core S: Maintained -- 2.16.1
[RFC tip/locking/lockdep v5 13/17] lockdep/selftest: Add more recursive read related test cases
Add those four test cases: 1. X --(NR)--> Y --(NR)--> Z --(NR)--> X is deadlock. 2. X --(NN)--> Y --(RR)--> Z --(NR)--> X is deadlock. 3. X --(NN)--> Y --(RR)--> Z --(RN)--> X is not deadlock. 4. X --(NR)--> Y --(RR)--> Z --(NN)--> X is not deadlock. Those self testcases are valuable for the development of supporting recursive read related deadlock detection. Signed-off-by: Boqun Feng --- lib/locking-selftest.c | 161 + 1 file changed, 161 insertions(+) diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c index c2f06b423da8..6b7a28d84fc4 100644 --- a/lib/locking-selftest.c +++ b/lib/locking-selftest.c @@ -1033,6 +1033,133 @@ GENERATE_PERMUTATIONS_3_EVENTS(irq_inversion_soft_wlock) #undef E2 #undef E3 +/* + * write-read / write-read / write-read deadlock even if read is recursive + */ + +#define E1() \ + \ + WL(X1); \ + RL(Y1); \ + RU(Y1); \ + WU(X1); + +#define E2() \ + \ + WL(Y1); \ + RL(Z1); \ + RU(Z1); \ + WU(Y1); + +#define E3() \ + \ + WL(Z1); \ + RL(X1); \ + RU(X1); \ + WU(Z1); + +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(W1R2_W2R3_W3R1) + +#undef E1 +#undef E2 +#undef E3 + +/* + * write-write / read-read / write-read deadlock even if read is recursive + */ + +#define E1() \ + \ + WL(X1); \ + WL(Y1); \ + WU(Y1); \ + WU(X1); + +#define E2() \ + \ + RL(Y1); \ + RL(Z1); \ + RU(Z1); \ + RU(Y1); + +#define E3() \ + \ + WL(Z1); \ + RL(X1); \ + RU(X1); \ + WU(Z1); + +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(W1W2_R2R3_W3R1) + +#undef E1 +#undef E2 +#undef E3 + +/* + * write-write / read-read / read-write is not deadlock when read is recursive + */ + +#define E1() \ + \ + WL(X1); \ + WL(Y1); \ + WU(Y1); \ + WU(X1); + +#define E2() \ + \ + RL(Y1); \ + RL(Z1); \ + RU(Z1); \ + RU(Y1); + +#define E3() \ + \ + RL(Z1); \ + WL(X1); \ + WU(X1); \ + RU(Z1); + +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(W1R2_R2R3_W3W1) + +#undef E1 +#undef E2 +#undef E3 + +/* + * write-read / read-read / write-write is not deadlock when read is recursive + */ + +#define E1() \ + \ + WL(X1); \ + RL(Y1); \ + RU(Y1); \ + WU(X1); + +#define E2() \ + \ + RL(Y1); \ + RL(Z1); \ + RU(Z1); \ + RU(Y1); + +#define E3() \ + \ + WL(Z1); \ + WL(X1); \ + WU(X1); \ + WU(Z1); + +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(W1W2_R2R3_R3W1) + +#undef E1 +#undef E2 +#undef E3 /* * read-lock / write-lock recursion that is actually safe. */ @@ -1258,6 +1385,19 @@ static inline void print_testname(const char *testname) dotest(name##_##nr, FAILURE, LOCKTYPE_RWLOCK); \ pr_cont("\n"); +#define DO_TESTCASE_1RR(desc, name, nr)\ + print_testname(desc"/"#nr); \ + pr_cont(" |"); \ + dotest(name##_##nr, SUCCESS, LOCKTYPE_RWLOCK); \ + pr_cont("\n"); + +#define DO_TESTCASE_1RRB(desc, name, nr) \ + print_testna
[RFC tip/locking/lockdep v5 16/17] lockdep: Documention for recursive read lock detection reasoning
As now we support recursive read lock deadlock detection, add related explanation in the Documentation/lockdep/lockdep-desgin.txt: * Definition of recursive read locks, non-recursive locks, strong dependency path and notions of -(**)->. * Lockdep's assumption. * Informal proof of recursive read lock deadlock detection. Signed-off-by: Boqun Feng Cc: Randy Dunlap --- Documentation/locking/lockdep-design.txt | 170 +++ 1 file changed, 170 insertions(+) diff --git a/Documentation/locking/lockdep-design.txt b/Documentation/locking/lockdep-design.txt index 382bc25589c2..fd8a8d97ce58 100644 --- a/Documentation/locking/lockdep-design.txt +++ b/Documentation/locking/lockdep-design.txt @@ -284,3 +284,173 @@ Run the command and save the output, then compare against the output from a later run of this command to identify the leakers. This same output can also help you find situations where runtime lock initialization has been omitted. + +Recursive Read Deadlock Detection: +-- +Lockdep now is equipped with deadlock detection for recursive read locks. + +Recursive read locks, as their name indicates, are the locks able to be +acquired recursively. Unlike non-recursive read locks, recursive read locks +only get blocked by current write lock *holders* other than write lock +*waiters*, for example: + + TASK A: TASK B: + + read_lock(X); + + write_lock(X); + + read_lock(X); + +is not a deadlock for recursive read locks, as while the task B is waiting for +the lock X, the second read_lock() doesn't need to wait because it's a recursive +read lock. + +Note that a lock can be a write lock(exclusive lock), a non-recursive read lock +(non-recursive shared lock) or a recursive read lock(recursive shared lock), +depending on the API used to acquire it(more specifically, the value of the +'read' parameter for lock_acquire(...)). In other words, a single lock instance +has three types of acquisition depending on the acquisition functions: +exclusive, non-recursive read, and recursive read. + +That said, recursive read locks could introduce deadlocks too, considering the +following: + + TASK A: TASK B: + + read_lock(X); + read_lock(Y); + write_lock(Y); + write_lock(X); + +, neither task could get the write locks because the corresponding read locks +are held by each other. + +Lockdep could detect recursive read lock related deadlocks. The dependencies(edges) +in the lockdep graph are classified into four categories: + +1) -(NN)->: non-recursive to non-recursive dependency, non-recursive locks include +non-recursive read locks, write locks and exclusive locks(e.g. spinlock_t). + They are treated equally in deadlock detection. "X -(NN)-> Y" means +X -> Y and both X and Y are non-recursive locks. + +2) -(RN)->: recursive to non-recursive dependency, recursive locks means recursive read + locks. "X -(RN)-> Y" means X -> Y and X is recursive read lock and +Y is non-recursive lock. + +3) -(NR)->: non-recursive to recursive dependency, "X -(NR)-> Y" means X -> Y and X is +non-recursive lock and Y is recursive lock. + +4) -(RR)->: recursive to recursive dependency, "X -(RR)-> Y" means X -> Y and both X +and Y are recursive locks. + +Note that given two locks, they may have multiple dependencies between them, for example: + + TASK A: + + read_lock(X); + write_lock(Y); + ... + + TASK B: + + write_lock(X); + write_lock(Y); + +, we have both X -(RN)-> Y and X -(NN)-> Y in the dependency graph. + +And obviously a non-recursive lock can block the corresponding recursive lock, +and vice versa. Besides a non-recursive lock may block the other non-recursive +lock of the same instance(e.g. a write lock may block a corresponding +non-recursive read lock and vice versa). + +We use -(*N)-> for edges that is either -(RN)-> or -(NN)->, the similar for -(N*)->, +-(*R)-> and -(R*)-> + +A "path" is a series of conjunct dependency edges in the graph. And we define a +"strong" path, which indicates the strong dependency throughout each dependency +in the path, as the path that doesn't have two conjunct edges(dependencies) as +-(*R)-> and -(R*)->. IOW, a "strong" path is a path from a lock walking to another +through the lock dependencies, and if X -> Y -> Z in the path(where X, Y, Z are +locks), if the walk from X to Y is through a -(NR)-> or -(RR)-> dependency, the +walk from Y to Z must not be through a -(RN)-> or -(RR)-> dependency, otherwise +it's not a strong path. + +We now prove that if a strong path forms a circle, then we have a potential deadlock. +By "forms a circle", it means for a set of locks A0,A1...An, there is a path from +A0 to An: + + A0 -> A1 -> ... ->
[RFC tip/locking/lockdep v5 06/17] lockdep: Support deadlock detection for recursive read in check_noncircular()
Currently, lockdep only has limit support for deadlock detection for recursive read locks. The basic idea of the detection is: Since we make __bfs() able to traverse only the strong dependency paths, so we report a circular deadlock if we could find a circle of a strong dependency path. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 16 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 07bcfaac6fe2..e1be088a34c4 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -1343,6 +1343,14 @@ static inline int class_equal(struct lock_list *entry, void *data) return entry->class == data; } +static inline int hlock_conflict(struct lock_list *entry, void *data) +{ + struct held_lock *hlock = (struct held_lock *)data; + + return hlock_class(hlock) == entry->class && + (hlock->read != 2 || !entry->is_rr); +} + static noinline int print_circular_bug(struct lock_list *this, struct lock_list *target, struct held_lock *check_src, @@ -1455,18 +1463,18 @@ unsigned long lockdep_count_backward_deps(struct lock_class *class) } /* - * Prove that the dependency graph starting at can not + * Prove that the dependency graph starting at can not * lead to . Print an error and return BFS_RMATCH if it does. */ static noinline enum bfs_result -check_noncircular(struct lock_list *root, struct lock_class *target, +check_noncircular(struct lock_list *root, struct held_lock *target, struct lock_list **target_entry) { enum bfs_result result; debug_atomic_inc(nr_cyclic_checks); - result = __bfs_forwards(root, target, class_equal, target_entry); + result = __bfs_forwards(root, target, hlock_conflict, target_entry); return result; } @@ -1994,7 +2002,7 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, * keep the stackframe size of the recursive functions low: */ bfs_init_root(&this, next); - ret = check_noncircular(&this, hlock_class(prev), &target_entry); + ret = check_noncircular(&this, prev, &target_entry); if (unlikely(ret == BFS_RMATCH)) { if (!trace->entries) { /* -- 2.16.1
[PATCH 4.14, 4.9] crypto: talitos - fix Kernel Oops on hashing an empty file
Upstream 87a81dce53b1ea61acaeefa5191a0376a2d1d721 Performing the hash of an empty file leads to a kernel Oops [ 44.504600] Unable to handle kernel paging request for data at address 0x000c [ 44.512819] Faulting instruction address: 0xc02d2be8 [ 44.524088] Oops: Kernel access of bad area, sig: 11 [#1] [ 44.529171] BE PREEMPT CMPC885 [ 44.532232] CPU: 0 PID: 491 Comm: md5sum Not tainted 4.15.0-rc8-00211-g3a968610b6ea #81 [ 44.540814] NIP: c02d2be8 LR: c02d2984 CTR: [ 44.545812] REGS: c6813c90 TRAP: 0300 Not tainted (4.15.0-rc8-00211-g3a968610b6ea) [ 44.554223] MSR: 9032 CR: 48222822 XER: 2000 [ 44.560855] DAR: 000c DSISR: c000 [ 44.560855] GPR00: c02d28fc c6813d40 c6828000 c646fa40 0001 0001 0001 [ 44.560855] GPR08: 004c c000bfcc 28222822 100280d4 10020008 [ 44.560855] GPR16: 0020 10024008 c646f9f0 c6179a10 [ 44.560855] GPR24: 0001 c62f0018 c6179a10 c6367a30 c62f c646f9c0 [ 44.598542] NIP [c02d2be8] ahash_process_req+0x448/0x700 [ 44.603751] LR [c02d2984] ahash_process_req+0x1e4/0x700 [ 44.608868] Call Trace: [ 44.611329] [c6813d40] [c02d28fc] ahash_process_req+0x15c/0x700 (unreliable) [ 44.618302] [c6813d90] [c02060c4] hash_recvmsg+0x11c/0x210 [ 44.623716] [c6813db0] [c0331354] ___sys_recvmsg+0x98/0x138 [ 44.629226] [c6813eb0] [c03332c0] __sys_recvmsg+0x40/0x84 [ 44.634562] [c6813f10] [c03336c0] SyS_socketcall+0xb8/0x1d4 [ 44.640073] [c6813f40] [c000d1ac] ret_from_syscall+0x0/0x38 [ 44.645530] Instruction dump: [ 44.648465] 38c1 7f63db78 4e800421 7c791b78 54690ffe 0f09 80ff0190 2f87 [ 44.656122] 40befe50 2f990001 409e0210 813f01bc <8129000c> b39e003a 7d29c214 913e003c This patch fixes that Oops by checking if src is NULL. Fixes: 6a1e8d14156d4 ("crypto: talitos - making mapping helpers more generic") Cc: Signed-off-by: Christophe Leroy --- drivers/crypto/talitos.c | 5 + 1 file changed, 5 insertions(+) diff --git a/drivers/crypto/talitos.c b/drivers/crypto/talitos.c index dff88838dce7..42913116620a 100644 --- a/drivers/crypto/talitos.c +++ b/drivers/crypto/talitos.c @@ -1124,6 +1124,11 @@ int talitos_sg_map(struct device *dev, struct scatterlist *src, struct talitos_private *priv = dev_get_drvdata(dev); bool is_sec1 = has_ftr_sec1(priv); + if (!src) { + *ptr = zero_entry; + return 1; + } + to_talitos_ptr_len(ptr, len, is_sec1); to_talitos_ptr_ext_set(ptr, 0, is_sec1); -- 2.13.3
[RFC tip/locking/lockdep v5 15/17] lockdep: Reduce the size of lock_list
We actually only need 4 bits for lock_list::dep and 1 bit for lock_list::is_rr, besides lock_list::distance should always be no greater than MAX_LOCKDEP_DEPTH(which is 48 right now), so a u16 will fit, this patch then reduces the sizes of those fields to save space for lock_list structure, as a result we can reduce the size increment introduced by recursive read lock detection and keep the lock_list the same size as before. Suggested-by: Peter Zijlstra Signed-off-by: Boqun Feng --- include/linux/lockdep.h | 6 +++--- kernel/locking/lockdep.c | 11 ++- 2 files changed, 9 insertions(+), 8 deletions(-) diff --git a/include/linux/lockdep.h b/include/linux/lockdep.h index a1f91f8680bd..3fce8dbf5091 100644 --- a/include/linux/lockdep.h +++ b/include/linux/lockdep.h @@ -186,11 +186,11 @@ struct lock_list { struct list_headentry; struct lock_class *class; struct stack_trace trace; - int distance; + u16 distance; /* bitmap of different dependencies from head to this */ - u16 dep; + u8 dep; /* used by BFS to record whether this is picked as a recursive read */ - u16 is_rr; + boolis_rr; /* * The parent field is used to implement breadth-first search, and the diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 1b981dc4c061..e8b83b36669c 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -874,7 +874,7 @@ static struct lock_list *alloc_list_entry(void) * Add a new dependency to the head of the list: */ static int add_lock_to_list(struct lock_class *this, struct list_head *head, - unsigned long ip, int distance, unsigned int dep, + unsigned long ip, u16 distance, unsigned int dep, struct stack_trace *trace) { struct lock_list *entry; @@ -1063,7 +1063,7 @@ static inline unsigned int calc_dep(int prev, int next) * N: non-recursive lock * R: recursive read lock */ -static inline int pick_dep(u16 is_rr, u16 cap_dep) +static inline int pick_dep(bool is_rr, u8 cap_dep) { if (is_rr) { /* could only pick -(N*)-> */ if (cap_dep & DEP_NN_MASK) @@ -1148,7 +1148,8 @@ static enum bfs_result __bfs(struct lock_list *source_entry, struct list_head *head; struct circular_queue *cq = &lock_cq; enum bfs_result ret = BFS_RNOMATCH; - int is_rr, next_is_rr; + bool is_rr; + int next_is_rr; if (match(source_entry, data)) { *target_entry = source_entry; @@ -1204,7 +1205,7 @@ static enum bfs_result __bfs(struct lock_list *source_entry, next_is_rr = pick_dep(is_rr, entry->dep); if (next_is_rr < 0) continue; - entry->is_rr = next_is_rr; + entry->is_rr = !!next_is_rr; visit_lock_entry(entry, lock); if (match(entry, data)) { @@ -2153,7 +2154,7 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next) goto out_bug; for (;;) { - int distance = curr->lockdep_depth - depth + 1; + u16 distance = curr->lockdep_depth - depth + 1; hlock = curr->held_locks + depth - 1; if (hlock->check) { -- 2.16.1
[RFC tip/locking/lockdep v5 01/17] lockdep: Demagic the return value of BFS
__bfs() could return four magic numbers: 1: search succeeds, but none match. 0: search succeeds, find one match. -1: search fails because of the cq is full. -2: search fails because a invalid node is found. This patch cleans things up by using a enum type for the return value of __bfs() and its friends, this improves the code readability of the code, and further, could help if we want to extend the BFS. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 136 --- 1 file changed, 80 insertions(+), 56 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 89b5f83f1969..9b2e318bcc81 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -984,21 +984,52 @@ static inline int get_lock_depth(struct lock_list *child) } return depth; } +/* + * Return values of a bfs search: + * + * BFS_E* indicates an error + * BFS_R* indicates a result(match or not) + * + * BFS_EINVALIDNODE: Find a invalid node in the graph. + * + * BFS_EQUEUEFULL: The queue is full while doing the bfs. + * + * BFS_RMATCH: Find the matched node in the graph, and put that node * into + **@target_entry. + * + * BFS_RNOMATCH: Haven't found the matched node and keep *@target_entry + * _unchanged_. + */ +enum bfs_result { + BFS_EINVALIDNODE = -2, + BFS_EQUEUEFULL = -1, + BFS_RMATCH = 0, + BFS_RNOMATCH = 1, +}; -static int __bfs(struct lock_list *source_entry, -void *data, -int (*match)(struct lock_list *entry, void *data), -struct lock_list **target_entry, -int forward) +/* + * bfs_result < 0 means error + */ + +static inline bool bfs_error(enum bfs_result res) +{ + return res < 0; +} + +static enum bfs_result __bfs(struct lock_list *source_entry, +void *data, +int (*match)(struct lock_list *entry, void *data), +struct lock_list **target_entry, +int forward) { struct lock_list *entry; struct list_head *head; struct circular_queue *cq = &lock_cq; - int ret = 1; + enum bfs_result ret = BFS_RNOMATCH; if (match(source_entry, data)) { *target_entry = source_entry; - ret = 0; + ret = BFS_RMATCH; goto exit; } @@ -1019,7 +1050,7 @@ static int __bfs(struct lock_list *source_entry, __cq_dequeue(cq, (unsigned long *)&lock); if (!lock->class) { - ret = -2; + ret = BFS_EINVALIDNODE; goto exit; } @@ -1036,12 +1067,12 @@ static int __bfs(struct lock_list *source_entry, mark_lock_accessed(entry, lock); if (match(entry, data)) { *target_entry = entry; - ret = 0; + ret = BFS_RMATCH; goto exit; } if (__cq_enqueue(cq, (unsigned long)entry)) { - ret = -1; + ret = BFS_EQUEUEFULL; goto exit; } cq_depth = __cq_get_elem_count(cq); @@ -1054,19 +1085,21 @@ static int __bfs(struct lock_list *source_entry, return ret; } -static inline int __bfs_forwards(struct lock_list *src_entry, - void *data, - int (*match)(struct lock_list *entry, void *data), - struct lock_list **target_entry) +static inline enum bfs_result +__bfs_forwards(struct lock_list *src_entry, + void *data, + int (*match)(struct lock_list *entry, void *data), + struct lock_list **target_entry) { return __bfs(src_entry, data, match, target_entry, 1); } -static inline int __bfs_backwards(struct lock_list *src_entry, - void *data, - int (*match)(struct lock_list *entry, void *data), - struct lock_list **target_entry) +static inline enum bfs_result +__bfs_backwards(struct lock_list *src_entry, + void *data, + int (*match)(struct lock_list *entry, void *data), + struct lock_list **target_entry) { return __bfs(src_entry, data, match, target_entry, 0); @@ -1299,13 +1332,13 @@ unsigned long lockdep_count_backward_deps(struct lock_class *class) /* * Prove that the dependency graph starting at can not - * lead to . Print an error and return 0 if it does. + * lead to . Print an error and return BFS_RMATCH if it d
[RFC tip/locking/lockdep v5 12/17] lockdep/selftest: Unleash irq_read_recursion2 and add more
Now since we can handle recursive read related irq inversion deadlocks correctly, uncomment the irq_read_recursion2 and add more testcases. Signed-off-by: Boqun Feng --- lib/locking-selftest.c | 59 -- 1 file changed, 47 insertions(+), 12 deletions(-) diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c index 700f9aa19db6..c2f06b423da8 100644 --- a/lib/locking-selftest.c +++ b/lib/locking-selftest.c @@ -1052,20 +1052,28 @@ GENERATE_PERMUTATIONS_3_EVENTS(irq_inversion_soft_wlock) #define E3() \ \ IRQ_ENTER();\ - RL(A); \ + LOCK(A);\ L(B); \ U(B); \ - RU(A); \ + UNLOCK(A); \ IRQ_EXIT(); /* - * Generate 12 testcases: + * Generate 24 testcases: */ #include "locking-selftest-hardirq.h" -GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_hard) +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_hard_rlock) + +#include "locking-selftest-wlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_hard_wlock) #include "locking-selftest-softirq.h" -GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft) +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft_rlock) + +#include "locking-selftest-wlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft_wlock) #undef E1 #undef E2 @@ -1079,8 +1087,8 @@ GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft) \ IRQ_DISABLE(); \ L(B); \ - WL(A); \ - WU(A); \ + LOCK(A);\ + UNLOCK(A); \ U(B); \ IRQ_ENABLE(); @@ -1097,13 +1105,21 @@ GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion_soft) IRQ_EXIT(); /* - * Generate 12 testcases: + * Generate 24 testcases: */ #include "locking-selftest-hardirq.h" -// GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion2_hard) +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion2_hard_rlock) + +#include "locking-selftest-wlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion2_hard_wlock) #include "locking-selftest-softirq.h" -// GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion2_soft) +#include "locking-selftest-rlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion2_soft_rlock) + +#include "locking-selftest-wlock.h" +GENERATE_PERMUTATIONS_3_EVENTS(irq_read_recursion2_soft_wlock) #ifdef CONFIG_DEBUG_LOCK_ALLOC # define I_SPINLOCK(x) lockdep_reset_lock(&lock_##x.dep_map) @@ -1256,6 +1272,25 @@ static inline void print_testname(const char *testname) dotest(name##_rlock_##nr, SUCCESS, LOCKTYPE_RWLOCK);\ pr_cont("\n"); +#define DO_TESTCASE_2RW(desc, name, nr)\ + print_testname(desc"/"#nr); \ + pr_cont(" |"); \ + dotest(name##_wlock_##nr, FAILURE, LOCKTYPE_RWLOCK);\ + dotest(name##_rlock_##nr, SUCCESS, LOCKTYPE_RWLOCK);\ + pr_cont("\n"); + +#define DO_TESTCASE_2x2RW(desc, name, nr) \ + DO_TESTCASE_2RW("hard-"desc, name##_hard, nr) \ + DO_TESTCASE_2RW("soft-"desc, name##_soft, nr) \ + +#define DO_TESTCASE_6x2x2RW(desc, name)\ + DO_TESTCASE_2x2RW(desc, name, 123); \ + DO_TESTCASE_2x2RW(desc, name, 132); \ + DO_TESTCASE_2x2RW(desc, name, 213); \ + DO_TESTCASE_2x2RW(desc, name, 231); \ + DO_TESTCASE_2x2RW(desc, name, 312); \ + DO_TESTCASE_2x2RW(desc, name, 321); + #define DO_TESTCASE_6(desc, name) \ print_testname(desc); \ dotest(name##_spin, FAILURE, LOCKTYPE_SPIN);\ @@ -2114,8 +2149,8 @@ void locking_selftest(void) DO_TESTCASE_6x6("safe-A + unsafe-B #2", irqsafe4); DO_TESTCASE_6x6RW("irq lock-inversion", irq_inversion); - DO_TESTCASE_6x2("irq read-recursion", irq_read_recursion); -// DO_TESTCASE_6x2B("irq read-recursion #2", irq_read_recursion2); + DO_TESTCASE_6x2x2RW("irq read-recursion", irq_read_recursion); + DO_TESTCASE_6x2x2RW("irq read-recursion #2", irq_read_recursion2); ww_tests(); -- 2.16.1
[RFC tip/locking/lockdep v5 10/17] lockdep/selftest: Add a R-L/L-W test case specific to chain cache behavior
As our chain cache doesn't differ read/write locks, so even we can detect a read-lock/lock-write deadlock in check_noncircular(), we can still be fooled if a read-lock/lock-read case(which is not a deadlock) comes first. So introduce this test case to test specific to the chain cache behavior on detecting recursive read lock related deadlocks. Signed-off-by: Boqun Feng --- lib/locking-selftest.c | 47 +++ 1 file changed, 47 insertions(+) diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c index b5c1293ce147..700f9aa19db6 100644 --- a/lib/locking-selftest.c +++ b/lib/locking-selftest.c @@ -395,6 +395,49 @@ static void rwsem_ABBA1(void) MU(Y1); // should fail } +/* + * read_lock(A) + * spin_lock(B) + * spin_lock(B) + * write_lock(A) + * + * This test case is aimed at poking whether the chain cache prevents us from + * detecting a read-lock/lock-write deadlock: if the chain cache doesn't differ + * read/write locks, the following case may happen + * + * { read_lock(A)->lock(B) dependency exists } + * + * P0: + * lock(B); + * read_lock(A); + * + * { Not a deadlock, B -> A is added in the chain cache } + * + * P1: + * lock(B); + * write_lock(A); + * + * { B->A found in chain cache, not reported as a deadlock } + * + */ +static void rlock_chaincache_ABBA1(void) +{ + RL(X1); + L(Y1); + U(Y1); + RU(X1); + + L(Y1); + RL(X1); + RU(X1); + U(Y1); + + L(Y1); + WL(X1); + WU(X1); + U(Y1); // should fail +} + /* * read_lock(A) * spin_lock(B) @@ -2055,6 +2098,10 @@ void locking_selftest(void) pr_cont(" |"); dotest(rwsem_ABBA3, FAILURE, LOCKTYPE_RWSEM); + print_testname("chain cached mixed R-L/L-W ABBA"); + pr_cont(" |"); + dotest(rlock_chaincache_ABBA1, FAILURE, LOCKTYPE_RWLOCK); + printk(" --\n"); /* -- 2.16.1
[RFC tip/locking/lockdep v5 14/17] Revert "locking/lockdep/selftests: Fix mixed read-write ABBA tests"
This reverts commit d82fed75294229abc9d757f08a4817febae6c4f4. Since we now could handle mixed read-write deadlock detection well, the self tests could be detected as expected, no need to use this work-around. Signed-off-by: Boqun Feng --- lib/locking-selftest.c | 8 1 file changed, 8 deletions(-) diff --git a/lib/locking-selftest.c b/lib/locking-selftest.c index 6b7a28d84fc4..79270288fa28 100644 --- a/lib/locking-selftest.c +++ b/lib/locking-selftest.c @@ -2266,14 +2266,6 @@ void locking_selftest(void) print_testname("mixed read-lock/lock-write ABBA"); pr_cont(" |"); dotest(rlock_ABBA1, FAILURE, LOCKTYPE_RWLOCK); -#ifdef CONFIG_PROVE_LOCKING - /* -* Lockdep does indeed fail here, but there's nothing we can do about -* that now. Don't kill lockdep for it. -*/ - unexpected_testcase_failures--; -#endif - pr_cont(" |"); dotest(rwsem_ABBA1, FAILURE, LOCKTYPE_RWSEM); -- 2.16.1
[RFC tip/locking/lockdep v5 09/17] lockdep: Add recursive read locks into dependency graph
Since we have all the fundamental to handle recursive read locks, we now add them into the dependency graph. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 16 +--- 1 file changed, 1 insertion(+), 15 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index bd3eef664f9d..254f90bade54 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -2038,16 +2038,6 @@ check_prev_add(struct task_struct *curr, struct held_lock *prev, if (!check_prev_add_irq(curr, prev, next)) return 0; - /* -* For recursive read-locks we do all the dependency checks, -* but we dont store read-triggered dependencies (only -* write-triggered dependencies). This ensures that only the -* write-side dependencies matter, and that if for example a -* write-lock never takes any other locks, then the reads are -* equivalent to a NOP. -*/ - if (next->read == 2 || prev->read == 2) - return 1; /* * Is the -> dependency already present? * @@ -2151,11 +2141,7 @@ check_prevs_add(struct task_struct *curr, struct held_lock *next) int distance = curr->lockdep_depth - depth + 1; hlock = curr->held_locks + depth - 1; - /* -* Only non-recursive-read entries get new dependencies -* added: -*/ - if (hlock->read != 2 && hlock->check) { + if (hlock->check) { int ret = check_prev_add(curr, hlock, next, distance, &trace, save_trace); if (!ret) return 0; -- 2.16.1
[RFC tip/locking/lockdep v5 11/17] lockdep: Take read/write status in consideration when generate chainkey
Currently, the chainkey of a lock chain is a hash sum of the class_idx of all the held locks, the read/write status are not taken in to consideration while generating the chainkey. This could result into a problem, if we have: P1() { read_lock(B); lock(A); } P2() { lock(A); read_lock(B); } P3() { lock(A); write_lock(B); } , and P1(), P2(), P3() run one by one. And when running P2(), lockdep detects such a lock chain A -> B is not a deadlock, then it's added in the chain cache, and then when running P3(), even if it's a deadlock, we could miss it because of the hit of chain cache. This could be confirmed by self testcase "chain cached mixed R-L/L-W ". To resolve this, we use concept"hlock_id" to generate the chainkey, the hlock_id is a tuple (hlock->class_idx, hlock->read), which fits in a u16 type. With this, the chainkeys are different is the lock sequences have the same locks but different read/write status. Besides, since we use "hlock_id" to generate chainkeys, the chain_hlocks array now store the "hlock_id"s rather than lock_class indexes. Signed-off-by: Boqun Feng --- kernel/locking/lockdep.c | 56 +++- 1 file changed, 36 insertions(+), 20 deletions(-) diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index 254f90bade54..1b981dc4c061 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -307,6 +307,21 @@ static struct hlist_head classhash_table[CLASSHASH_SIZE]; static struct hlist_head chainhash_table[CHAINHASH_SIZE]; +/* + * the id chain_hlocks + */ +static inline u16 hlock_id(struct held_lock *hlock) +{ + BUILD_BUG_ON(MAX_LOCKDEP_KEYS_BITS + 2 > 16); + + return (hlock->class_idx | (hlock->read << MAX_LOCKDEP_KEYS_BITS)); +} + +static inline unsigned int chain_hlock_class_idx(u16 hlock_id) +{ + return hlock_id & MAX_LOCKDEP_KEYS; +} + /* * The hash key of the lock dependency chains is a hash itself too: * it's a hash of all locks taken up to that lock, including that lock. @@ -2191,7 +2206,10 @@ static u16 chain_hlocks[MAX_LOCKDEP_CHAIN_HLOCKS]; struct lock_class *lock_chain_get_class(struct lock_chain *chain, int i) { - return lock_classes + chain_hlocks[chain->base + i]; + u16 chain_hlock = chain_hlocks[chain->base + i]; + unsigned int class_idx = chain_hlock_class_idx(chain_hlock); + + return lock_classes + class_idx - 1; } /* @@ -2217,12 +2235,12 @@ static inline int get_first_held_lock(struct task_struct *curr, /* * Returns the next chain_key iteration */ -static u64 print_chain_key_iteration(int class_idx, u64 chain_key) +static u64 print_chain_key_iteration(u16 hlock_id, u64 chain_key) { - u64 new_chain_key = iterate_chain_key(chain_key, class_idx); + u64 new_chain_key = iterate_chain_key(chain_key, hlock_id); - printk(" class_idx:%d -> chain_key:%016Lx", - class_idx, + printk(" hlock_id:%d -> chain_key:%016Lx", + (unsigned int)hlock_id, (unsigned long long)new_chain_key); return new_chain_key; } @@ -2238,12 +2256,12 @@ print_chain_keys_held_locks(struct task_struct *curr, struct held_lock *hlock_ne printk("depth: %u\n", depth + 1); for (i = get_first_held_lock(curr, hlock_next); i < depth; i++) { hlock = curr->held_locks + i; - chain_key = print_chain_key_iteration(hlock->class_idx, chain_key); + chain_key = print_chain_key_iteration(hlock_id(hlock), chain_key); print_lock(hlock); } - print_chain_key_iteration(hlock_next->class_idx, chain_key); + print_chain_key_iteration(hlock_id(hlock_next), chain_key); print_lock(hlock_next); } @@ -2251,14 +2269,14 @@ static void print_chain_keys_chain(struct lock_chain *chain) { int i; u64 chain_key = 0; - int class_id; + u16 hlock_id; printk("depth: %u\n", chain->depth); for (i = 0; i < chain->depth; i++) { - class_id = chain_hlocks[chain->base + i]; - chain_key = print_chain_key_iteration(class_id + 1, chain_key); + hlock_id = chain_hlocks[chain->base + i]; + chain_key = print_chain_key_iteration(hlock_id, chain_key); - print_lock_name(lock_classes + class_id); + print_lock_name(lock_classes + chain_hlock_class_idx(hlock_id) - 1); printk("\n"); } } @@ -2307,7 +2325,7 @@ static int check_no_collision(struct task_struct *curr, } for (j = 0; j < chain->depth - 1; j++, i++) { - id = curr->held_locks[i].class_idx - 1; + id = hlock_id(&curr->held_locks[i]); if (DEBUG_LOCKS_WARN_ON(chain_hlocks[chain->base + j] != id)) {
[RFC tip/locking/lockdep v5 03/17] lockdep: Redefine LOCK_*_STATE* bits
There are three types of lock acquisitions: write, non-recursive read and recursive read, among which write locks and non-recursive read locks have no difference from a viewpoint for deadlock detections, because a write acquisition of the corresponding lock on an independent CPU or task makes a non-recursive read lock act as a write lock in the sense of deadlock. So we could treat them as the same type(named as "non-recursive lock") in lockdep. As in the irq lock inversion detection(safe->unsafe deadlock detection), we used to differ write lock with read lock(non-recursive and recursive ones), such a classification could be improved as non-recursive read lock behaves the same as write lock, so this patch redefines the meanings of LOCK_{USED_IN, ENABLED}_STATE*. old: LOCK_* : stands for write lock LOCK_*_READ: stands for read lock(non-recursive and recursive) new: LOCK_* : stands for non-recursive(write lock and non-recursive read lock) LOCK_*_RR: stands for recursive read lock Such a change is needed for a future improvement on recursive read related irq inversion deadlock detection. Signed-off-by: Boqun Feng --- Documentation/locking/lockdep-design.txt | 6 +++--- kernel/locking/lockdep.c | 28 ++-- kernel/locking/lockdep_internals.h | 16 kernel/locking/lockdep_proc.c| 12 ++-- 4 files changed, 31 insertions(+), 31 deletions(-) diff --git a/Documentation/locking/lockdep-design.txt b/Documentation/locking/lockdep-design.txt index 9de1c158d44c..382bc25589c2 100644 --- a/Documentation/locking/lockdep-design.txt +++ b/Documentation/locking/lockdep-design.txt @@ -30,9 +30,9 @@ State The validator tracks lock-class usage history into 4n + 1 separate state bits: - 'ever held in STATE context' -- 'ever held as readlock in STATE context' +- 'ever held as recursive readlock in STATE context' - 'ever held with STATE enabled' -- 'ever held as readlock with STATE enabled' +- 'ever held as recurisve readlock with STATE enabled' Where STATE can be either one of (kernel/locking/lockdep_states.h) - hardirq @@ -51,7 +51,7 @@ locking error messages, inside curlies. A contrived example: (&sio_locks[i].lock){-.-...}, at: [] mutex_lock+0x21/0x24 -The bit position indicates STATE, STATE-read, for each of the states listed +The bit position indicates STATE, STATE-RR, for each of the states listed above, and the character displayed in each indicates: '.' acquired while irqs disabled and not in irq context diff --git a/kernel/locking/lockdep.c b/kernel/locking/lockdep.c index c80f8276ceaa..5e6bf8d6954d 100644 --- a/kernel/locking/lockdep.c +++ b/kernel/locking/lockdep.c @@ -448,10 +448,10 @@ DEFINE_PER_CPU(struct lockdep_stats, lockdep_stats); */ #define __USAGE(__STATE) \ - [LOCK_USED_IN_##__STATE] = "IN-"__stringify(__STATE)"-W", \ - [LOCK_ENABLED_##__STATE] = __stringify(__STATE)"-ON-W", \ - [LOCK_USED_IN_##__STATE##_READ] = "IN-"__stringify(__STATE)"-R",\ - [LOCK_ENABLED_##__STATE##_READ] = __stringify(__STATE)"-ON-R", + [LOCK_USED_IN_##__STATE] = "IN-"__stringify(__STATE), \ + [LOCK_ENABLED_##__STATE] = __stringify(__STATE)"-ON", \ + [LOCK_USED_IN_##__STATE##_RR] = "IN-"__stringify(__STATE)"-RR", \ + [LOCK_ENABLED_##__STATE##_RR] = __stringify(__STATE)"-ON-RR", static const char *usage_str[] = { @@ -492,7 +492,7 @@ void get_usage_chars(struct lock_class *class, char usage[LOCK_USAGE_CHARS]) #define LOCKDEP_STATE(__STATE) \ usage[i++] = get_usage_char(class, LOCK_USED_IN_##__STATE); \ - usage[i++] = get_usage_char(class, LOCK_USED_IN_##__STATE##_READ); + usage[i++] = get_usage_char(class, LOCK_USED_IN_##__STATE##_RR); #include "lockdep_states.h" #undef LOCKDEP_STATE @@ -1645,7 +1645,7 @@ static const char *state_names[] = { static const char *state_rnames[] = { #define LOCKDEP_STATE(__STATE) \ - __stringify(__STATE)"-READ", + __stringify(__STATE)"-RR", #include "lockdep_states.h" #undef LOCKDEP_STATE }; @@ -3039,14 +3039,14 @@ static int mark_irqflags(struct task_struct *curr, struct held_lock *hlock) * mark the lock as used in these contexts: */ if (!hlock->trylock) { - if (hlock->read) { + if (hlock->read == 2) { if (curr->hardirq_context) if (!mark_lock(curr, hlock, - LOCK_USED_IN_HARDIRQ_READ)) + LOCK_USED_IN_HARDIRQ_RR)) return 0; if (curr->softirq_context) if (!mark_lock(curr, hlock, - LOCK_US
[RFC tip/locking/lockdep v5 00/17] lockdep: Support deadlock detection for recursive read locks
Hi Ingo and Peter, This is V5 for recursive read lock support in lockdep. The changes since V4 are quite trivial: * Fix some wording issues in patch #16 as pointed out by Randy Dunlap I added the explanation about reasoning in patch #16 in V4, which will help understand this whole series. This patchset is based on v4.16-rc2. Changes since V3: * Reduce the unnecessary cost in structure lock_list as suggested by Peter Zijlstra * Add documentation for explanation of the reasoning in recursive read lock deadlock detection. * Add comments before bfs() describing the greedy search we use in BFS for strong dependency paths. * Add myself as a reviewer for LOCKING PRIMITIVES so that if there is any performance regression, log misunderstanding or false positives, people know who to blame^Wask help from. V1: https://marc.info/?l=linux-kernel&m=150393341825453 V2: https://marc.info/?l=linux-kernel&m=150468649417950 V3: https://marc.info/?l=linux-kernel&m=150637795424969 V4: https://marc.info/?l=linux-kernel&m=151550860121565 As Peter pointed out: https://marc.info/?l=linux-kernel&m=150349072023540 The lockdep current has a limit support for recursive read locks, the deadlock case as follow could not be detected: read_lock(A); lock(B); lock(B); write_lock(A); I got some inspiration from Gautham R Shenoy: https://lwn.net/Articles/332801/ , and came up with this series. The basic idea is: * Add recursive read locks into the graph * Classify dependencies into -(RR)->, -(NR)->, -(RN)->, -(NN)->, where R stands for recursive read lock, N stands for other locks(i.e. non-recursive read locks and write locks). * Define strong dependency paths as the paths of dependencies don't have two adjacent dependencies as -(*R)-> and -(R*)->. * Extend __bfs() to only traverse on strong dependency paths. * If __bfs() finds a strong dependency circle, then a deadlock is reported. The whole series consists of 17 patches: 1. Do a clean up on the return value of __bfs() and its friends. 2. Make __bfs() able to visit every dependency until a match is found. The old version of __bfs() could only visit each lock class once, and this is insufficient if we are going to add recursive read locks into the dependency graph. 3. Redefine LOCK*_STATE*, now LOCK*_STATE_RR stand for recursive read lock only and LOCK*_STATE stand for write lock and non-recursive read lock. 4-5 Extend __bfs() to be able to traverse the stong dependency patchs after recursive read locks added into the graph. 6-8 Adjust check_redundant(), check_noncircular() and check_irq_usage() with recursive read locks into consideration. 9. Finally add recursive read locks into the dependency graph. 10-11 Adjust lock cache chain key generation with recursive read locks into consideration, and provide a test case. 12-13 Add more test cases. 14. Revert commit d82fed752942 ("locking/lockdep/selftests: Fix mixed read-write ABBA tests"), 15. Reduce the extra size cost of lock_list to zero 16. Add documentation for recursive read lock deadlock detection reasoning 17. Add myself as a LOCKING PRIMITIVES reviewer. This series passed all the lockdep selftest cases(including those I introduce). Test and comments are welcome! Regards, Boqun
[RFC PATCH] drivers/peci: peci_match_id() can be static
Fixes: 99f5d2b99ecd ("drivers/peci: Add support for PECI bus driver core") Signed-off-by: Fengguang Wu --- peci-core.c |4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/peci/peci-core.c b/drivers/peci/peci-core.c index d976c73..4709b8c 100644 --- a/drivers/peci/peci-core.c +++ b/drivers/peci/peci-core.c @@ -770,8 +770,8 @@ peci_of_match_device(const struct of_device_id *matches, } #endif -const struct peci_device_id *peci_match_id(const struct peci_device_id *id, - struct peci_client *client) +static const struct peci_device_id *peci_match_id(const struct peci_device_id *id, + struct peci_client *client) { if (!(id && client)) return NULL;
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
Hi Jae, Thank you for the patch! Perhaps something to improve: [auto build test WARNING on linus/master] [also build test WARNING on v4.16-rc2 next-20180221] [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/Jae-Hyun-Yoo/PECI-device-driver-introduction/20180222-054545 reproduce: # apt-get install sparse make ARCH=x86_64 allmodconfig make C=1 CF=-D__CHECK_ENDIAN__ sparse warnings: (new ones prefixed by >>) >> drivers/peci/peci-core.c:773:29: sparse: symbol 'peci_match_id' was not >> declared. Should it be Please review and possibly fold the followup patch. --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation
Re: Use higher-order pages in vmalloc
On Wed 21-02-18 09:01:29, Matthew Wilcox wrote: > On Wed, Feb 21, 2018 at 08:16:22AM -0800, Dave Hansen wrote: > > On 02/21/2018 07:42 AM, Matthew Wilcox wrote: > > > This prompted me to write a patch I've been meaning to do for a while, > > > allocating large pages if they're available to satisfy vmalloc. I thought > > > it would save on touching multiple struct pages, but it turns out that > > > the checking code we currently have in the free_pages path requires you > > > to have initialised all of the tail pages (maybe we can make that code > > > conditional ...) > > > > What the concept here? If we can use high-order pages for vmalloc() at > > the moment, we *should* use them? > > Right. It helps with fragmentation if we can keep higher-order > allocations together. Hmm, wouldn't it help if we made vmalloc pages migrateable instead? That would help the compaction and get us to a lower fragmentation longterm without playing tricks in the allocation path. > > One of the coolest things about vmalloc() is that it can do large > > allocations without consuming large (high-order) pages, so it has very > > few side-effects compared to doing a bunch of order-0 allocations. This > > patch seems to propose removing that cool thing. Even trying the > > high-order allocation could kick off a bunch of reclaim and compaction > > that was not there previously. > > Yes, that's one of the debatable things. It'd be nice to have a GFP > flag that stopped after calling get_page_from_freelist() and didn't try > to do compaction or reclaim. GFP_NOWAIT, you mean? > > If you could take this an only _opportunistically_ allocate large pages, > > it could be a more universal win. You could try to make sure that no > > compaction or reclaim is done for the large allocation. Or, maybe you > > only try it if there are *only* high-order pages in the allocator that > > would have been broken down into order-0 *anyway*. > > > > I'm not sure it's worth it, though. I don't see a lot of folks > > complaining about vmalloc()'s speed or TLB impact. > > No, I'm not sure it's worth it either, although Konstantin's mail > suggesting improvements in fork speed were possible by avoiding vmalloc > reminded me that I'd been meaning to give this a try. Maybe we should consider kvmalloc for the kernel stack? -- Michal Hocko SUSE Labs
Re: [PATCH 4.9 34/77] powerpc: fix build errors in stable tree
On Thu, Feb 22, 2018 at 12:01:21PM +1100, Michael Ellerman wrote: > Greg Kroah-Hartman writes: > > > 4.9-stable review patch. If anyone has any objections, please let me know. > > > > -- > > > > From: Nicholas Piggin > > > > This is just the first chunk of commit > > 222f20f140623ef6033491d0103ee0875fe87d35 upstream. > > That should have been back ported in full? > > Oh I see now Greg took my 4.4 series and applied it to 4.9 but this one > didn't apply. > > So I guess this patch is OK for now, but we do need a full back port of > 222f20f to 4.9. Sure, that would be great to have, I'll gladly take those patches :) thanks, greg k-h
Re: [PATCH RFC tools/lkmm 10/12] tools/memory-model: Add a S lock-based external-view litmus test
On Wed, Feb 21, 2018 at 09:42:08PM -0800, Daniel Lustig wrote: > On 2/21/2018 9:27 PM, Boqun Feng wrote: > > On Wed, Feb 21, 2018 at 08:13:57PM -0800, Paul E. McKenney wrote: > >> On Thu, Feb 22, 2018 at 11:23:49AM +0800, Boqun Feng wrote: > >>> On Tue, Feb 20, 2018 at 03:25:10PM -0800, Paul E. McKenney wrote: > From: Alan Stern > > This commit adds a litmus test in which P0() and P1() form a lock-based S > litmus test, with the addition of P2(), which observes P0()'s and P1()'s > accesses with a full memory barrier but without the lock. This litmus > test asks whether writes carried out by two different processes under the > same lock will be seen in order by a third process not holding that lock. > The answer to this question is "yes" for all architectures supporting > >>> > >>> Hmm.. it this true? Our spin_lock() is RCpc because of PowerPC, so > >>> spin_lock()+spin_unlock() pairs don't provide transitivity, and that's > >>> why we have smp_mb__after_unlock_lock(). Is there something I'm missing? > >>> Or there is an upcomming commit to switch PowerPC's lock implementation? > >> > >> The PowerPC lock implementation's unlock-lock pair does not order writes > >> from the previous critical section against reads from the later critical > >> section, but it does order other combinations of reads and writes. > > > > Ah.. right! Thanks for the explanation ;-) > > > >> Some have apparently said that RISC-V 's unlock-lock pair also does not > >> order writes from the previous critical section against writes from the > >> later critical section. And no, I don't claim to have yet gotten my > >> head around RISC-V memory ordering. ;-) > >> > > > > Me neither. Now I remember this: we have a off-list(accidentally) > > discussion about this, and IIRC at that moment riscv people confirmed > > that riscv's unlock-lock pair doesn't order write->write, but that was > > before their memory model draft posted for discussions, so things may > > change now... > > > > Besides, I think the smp_mb() on P2 can be relaxed to smp_rmb(), no? > > > > Regards, > > Boqun > > > >>Thanx, Paul > >> > > As a matter of fact, the RISC-V memory model committee is debating this > exact question at the moment. Now that I see this thread I'll have to > go back and catch up on what I've missed, but at least I can be on cc > from this point on to answer any RISC-V questions that come up from > here on. > Hi Daniel ;-) > (Is there some place I should add my name as a RISC-V memory model > contact, so I don't miss threads like this in the future?) > You can submit a patch to add yourself as a Maintainer or Reviewer for LKMM, Patch #6 in this series is a good example: https://marc.info/?l=linux-kernel&m=151916916630299&w=2 , and "get_maintainer.pl" will help people find you for memory model stuffs. > And yes, if we go with a purely RCpc interpretation of acquire and > release, then I don't believe the writes in the previous critical > section would be ordered with the writes in the subsequent critical > section. That's really all the argument boils down to. We'd like I think atomics in Linux kernel(and in LKMM) are purely RCpc, right? Alan and Andrea? And we are not going to change it, are we? If atomics in Linux kernel are purely RCpc, then it cerntainly makes sense for riscv to provide purely RCpc atomics. > to support RCpc if we can since that's all some software needs, but > we also obviously want to make sure we can support RCsc and these > kinds of LKMM subtleties efficiently too when needed. So we have a I think the problem here is that locks in Linux kernel are more strict than RCpc but weaker than RCsc, and there is not a well-known concept for the semantics of locks in Linux kernel AFAIK. Regards, Boqun > few encoding details that we're still finalizing, because questions > like this one are still popping up :) > > Let me know if you had other RISC-V-specific questions I can help > answer. > > Dan signature.asc Description: PGP signature
Re: [PATCH v2 1/8] [PATCH 1/8] drivers/peci: Add support for PECI bus driver core
On Wed, Feb 21, 2018 at 12:42:30PM -0800, Jae Hyun Yoo wrote: > On 2/21/2018 9:58 AM, Greg KH wrote: > > On Wed, Feb 21, 2018 at 08:15:59AM -0800, Jae Hyun Yoo wrote: > > > This commit adds driver implementation for PECI bus into linux > > > driver framework. > > > > > > Signed-off-by: Jae Hyun Yoo > > > --- > > > > Why is there no other Intel developers willing to review and sign off on > > this patch? Please get their review first before asking us to do their > > work for them :) > > > > thanks, > > > > greg k-h > > > > Hi Greg, > > This patch set got our internal review process. Sorry if it's code quality > is under your expectation but it's the reason why I'm asking you to review > the code. Could you please share your time to review it? Nope. If no other Intel developer thinks it is good enough to put their name on it as part of their review process, why should I? Again, please use the resources you have, to fix the obvious problems in your code, BEFORE asking the community to do that work for you. greg k-h
[PATCH] srcu: remove never used variable
From: Zhouyi Zhou In function srcu_gp_end, the variable idxnext is never used after assign, remove it and its assign statement. Signed-off-by: Zhouyi Zhou --- kernel/rcu/srcutree.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/kernel/rcu/srcutree.c b/kernel/rcu/srcutree.c index d5cea81..1241715 100644 --- a/kernel/rcu/srcutree.c +++ b/kernel/rcu/srcutree.c @@ -531,7 +531,6 @@ static void srcu_gp_end(struct srcu_struct *sp) unsigned long flags; unsigned long gpseq; int idx; - int idxnext; unsigned long mask; struct srcu_data *sdp; struct srcu_node *snp; @@ -555,7 +554,6 @@ static void srcu_gp_end(struct srcu_struct *sp) /* Initiate callback invocation as needed. */ idx = rcu_seq_ctr(gpseq) % ARRAY_SIZE(snp->srcu_have_cbs); - idxnext = (idx + 1) % ARRAY_SIZE(snp->srcu_have_cbs); rcu_for_each_node_breadth_first(sp, snp) { spin_lock_irq_rcu_node(snp); cbs = false; -- 2.1.4
Re: [PATCH v2 01/17] tracing: probeevent: Fix to support minus offset from symbol
On Wed, 21 Feb 2018 20:53:41 -0500 Steven Rostedt wrote: > On Thu, 22 Feb 2018 08:41:53 +0900 > Masami Hiramatsu wrote: > > > Hi Steve, > > > > Could you pick this separately since this is a bugfix? > > > > Yes definitely. I'll have to run my tests on it too, and I'll send it > as urgent (current -rc release). > > Although you didn't mark it for stable. Which commit does it fix? Wait, it actually fixes the 2fba0c8867af ("tracing/kprobes: Fix probe offset to be unsigned") but I found that I was doing the opposite thing... OK, I'll update this to add a sanity check for kprobe.offset. Thank you, -- Masami Hiramatsu
[v2 0/3] Patches to address some limitations in OS microcode loading.
Patch series to address limitations of OS microcode loading. Review comments from Boris: Changes since v1: Patch 1/3 - Check for revision to avoid duplicate microcode load in early load - Added inline comments. Changes since v1: Patch 2/3 - Change to native_wbinvd for early load. Changes since v1: Patch 3/3 - Check for return code of stop_machine - When microcode file load fails, stop on first error. - If any of the present CPUs are offline, then stop reload. This is just for being paranoid. - Added more comments in commit log and inline in file. - split some functionality from reload_store() per Boris's comments. What's not done from review: TBD: - Load microcode file only once. Added comments in source for future cleanup. - Removing ucd->errors. (Gives a count of failed loads) Ashok Raj (3): x86/microcode/intel: Check microcode revision before updating sibling threads x86/microcode/intel: Perform a cache flush before ucode update. x86/microcode: Quiesce all threads before a microcode update. arch/x86/kernel/cpu/microcode/core.c | 207 ++ arch/x86/kernel/cpu/microcode/intel.c | 43 ++- 2 files changed, 222 insertions(+), 28 deletions(-) -- 2.7.4
[v2 2/3] x86/microcode/intel: Perform a cache flush before ucode update.
Microcode updates can be safer if the caches are clean. Some of the issues around in certain Broadwell parts can be addressed by doing a full cache flush. Signed-off-by: Ashok Raj Cc: X86 ML Cc: LKML Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Tony Luck Cc: Andi Kleen Cc: Boris Petkov Cc: Tom Lendacky Cc: Arjan Van De Ven --- arch/x86/kernel/cpu/microcode/intel.c | 14 ++ 1 file changed, 14 insertions(+) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 137c9f5..50e48c4 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -601,6 +601,13 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early) return UCODE_OK; } + /* +* Microcode updates can be safer if the caches are clean. +* Some of the issues around in certain Broadwell parts +* can be addressed by doing a full cache flush. +*/ + native_wbinvd(); + /* write microcode via MSR 0x79 */ native_wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits); @@ -817,6 +824,13 @@ static enum ucode_state apply_microcode_intel(int cpu) return UCODE_OK; } + /* +* Microcode updates can be safer if the caches are clean. +* Some of the issues around in certain Broadwell parts +* can be addressed by doing a full cache flush. +*/ + wbinvd(); + /* write microcode via MSR 0x79 */ wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits); -- 2.7.4
[v2 3/3] x86/microcode: Quiesce all threads before a microcode update.
Microcode updates during OS load always assumed the other hyper-thread was "quiet", but Linux never really did this. We've recently received several issues on this, where things did not go well at scale deployments, and the Intel microcode team asked us to make sure the system is in a quiet state during these updates. Such updates are rare events, so we use stop_machine() to ensure the whole system is quiet. The most robust solution is to have all the CPUs wait in stop_machine() rendezvous before ucode update, and ensure we stay there until all the CPUs have completed the microcode load. stop_machine ensures that interrupts are disabled while waiting, and a spin_lock serializes that only 1 cpu can perform the update. Remember microcode resources are shared between the HT siblings of the same core. So updating microcode on one thread automatically is good enough for the sibling thread. Microcode update ensures that the version being updated is greater than what is reported by the CPU. This ensures we do not perform a redundant load on the HT sibling when one isn't necessary. During early boot, we bring up one CPU at a time. Hence we only end up loading for one of the sibling thread and subsequent sibling would already have the latest copy of the microcode. Signed-off-by: Ashok Raj Cc: X86 ML Cc: LKML Cc: Tom Lendacky Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Tony Luck Cc: Andi Kleen Cc: Boris Petkov Cc: Arjan Van De Ven Changes from V1: - Check for return code of stop_machine - When microcode file load fails, stop on first error. - If any of the present CPUs are offline, then stop reload. - Added more comments in commitlog and inline in file. - split some functionality from reload_store() per Boris's comments. What's not done from review: TBD: - Load microcode file only once. - Removing ucd->errors. (Gives a count of failed loads) --- arch/x86/kernel/cpu/microcode/core.c | 207 ++ arch/x86/kernel/cpu/microcode/intel.c | 1 + 2 files changed, 183 insertions(+), 25 deletions(-) diff --git a/arch/x86/kernel/cpu/microcode/core.c b/arch/x86/kernel/cpu/microcode/core.c index aa1b9a4..20d2cbb 100644 --- a/arch/x86/kernel/cpu/microcode/core.c +++ b/arch/x86/kernel/cpu/microcode/core.c @@ -31,6 +31,10 @@ #include #include #include +#include +#include +#include +#include #include #include @@ -489,30 +493,185 @@ static void __exit microcode_dev_exit(void) /* fake device for request_firmware */ static struct platform_device *microcode_pdev; -static enum ucode_state reload_for_cpu(int cpu) +static struct ucode_update_param { + spinlock_t ucode_lock; /* Serialize microcode updates */ + atomic_t count; /* # of CPUs that attempt to load ucode */ + atomic_t errors; /* # of CPUs ucode load failed */ + atomic_t enter; /* ucode rendezvous count */ + inttimeout; /* Timeout to wait for all cpus to enter */ +} uc_data; + +static int do_ucode_update(int cpu, struct ucode_update_param *ucd) { - struct ucode_cpu_info *uci = ucode_cpu_info + cpu; - enum ucode_state ustate; + enum ucode_state retval = 0; - if (!uci->valid) - return UCODE_OK; + spin_lock(&ucd->ucode_lock); + retval = microcode_ops->apply_microcode(cpu); + spin_unlock(&ucd->ucode_lock); + if (retval > UCODE_NFOUND) { + atomic_inc(&ucd->errors); + pr_warn("microcode update to CPU %d failed\n", cpu); + } + atomic_inc(&ucd->count); + + return retval; +} + +/* + * Wait for upto 1sec for all CPUs + * to show up in the rendezvous function + */ +#define MAX_UCODE_RENDEZVOUS 10 /* nanosec */ +#define SPINUNIT 100/* 100ns */ + +/* + * Each cpu waits for 1sec max. + */ +static int microcode_wait_timedout(int *time_out, void *data) +{ + struct ucode_update_param *ucd = data; + if (*time_out < SPINUNIT) { + pr_err("Not all CPUs entered ucode update handler %d CPUs missed rendezvous\n", + (num_online_cpus() - atomic_read(&ucd->enter))); + return 1; + } + *time_out -= SPINUNIT; + touch_nmi_watchdog(); + return 0; +} + +/* + * All cpus enter here before a ucode load upto 1 sec. + * If not all cpus showed up, we abort the ucode update + * and return. ucode update is serialized with the spinlock + */ +static int microcode_load_rendezvous(void *data) +{ + int cpu = smp_processor_id(); + struct ucode_update_param *ucd = data; + int timeout = MAX_UCODE_RENDEZVOUS; + int total_cpus = num_online_cpus(); + + /* +* Wait for all cpu's to arrive +*/ + atomic_dec(&ucd->enter); + while(atomic_read(&ucd->enter)) { + if (microcode_wait_timedout(&timeout, ucd)) + return 1; + ndelay(SPINUNIT); + } + + do_ucode_update(cpu,
[v2 1/3] x86/microcode/intel: Check microcode revision before updating sibling threads
After updating microcode on one of the threads in the core, the thread sibling automatically gets the update since the microcode resources are shared. Check the ucode revision on the CPU before performing a ucode update. Signed-off-by: Ashok Raj Cc: X86 ML Cc: LKML Cc: Thomas Gleixner Cc: Ingo Molnar Cc: Tony Luck Cc: Andi Kleen Cc: Boris Petkov Cc: Tom Lendacky Cc: Arjan Van De Ven Updates: v2: Address Boris's to cleanup apply_microcode_intel v3: Fixups per Ingo: Spell Checks --- arch/x86/kernel/cpu/microcode/intel.c | 28 +--- 1 file changed, 25 insertions(+), 3 deletions(-) diff --git a/arch/x86/kernel/cpu/microcode/intel.c b/arch/x86/kernel/cpu/microcode/intel.c index 09b95a7..137c9f5 100644 --- a/arch/x86/kernel/cpu/microcode/intel.c +++ b/arch/x86/kernel/cpu/microcode/intel.c @@ -589,6 +589,18 @@ static int apply_microcode_early(struct ucode_cpu_info *uci, bool early) if (!mc) return 0; + rev = intel_get_microcode_revision(); + + /* +* Its possible the microcode got updated +* because its sibling update was done earlier. +* Skip the update in that case. +*/ + if (rev >= mc->hdr.rev) { + uci->cpu_sig.rev = rev; + return UCODE_OK; + } + /* write microcode via MSR 0x79 */ native_wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits); @@ -776,7 +788,7 @@ static enum ucode_state apply_microcode_intel(int cpu) { struct microcode_intel *mc; struct ucode_cpu_info *uci; - struct cpuinfo_x86 *c; + struct cpuinfo_x86 *c = &cpu_data(cpu); static int prev_rev; u32 rev; @@ -793,6 +805,18 @@ static enum ucode_state apply_microcode_intel(int cpu) return UCODE_NFOUND; } + rev = intel_get_microcode_revision(); + /* +* Its possible the microcode got updated +* because its sibling update was done earlier. +* Skip the update in that case. +*/ + if (rev >= mc->hdr.rev) { + uci->cpu_sig.rev = rev; + c->microcode = rev; + return UCODE_OK; + } + /* write microcode via MSR 0x79 */ wrmsrl(MSR_IA32_UCODE_WRITE, (unsigned long)mc->bits); @@ -813,8 +837,6 @@ static enum ucode_state apply_microcode_intel(int cpu) prev_rev = rev; } - c = &cpu_data(cpu); - uci->cpu_sig.rev = rev; c->microcode = rev; -- 2.7.4
[PATCH net v2 1/2] Revert "tuntap: add missing xdp flush"
This reverts commit 762c330d670e3d4b795cf7a8d761866fdd1eef49. The reason is we try to batch packets for devmap which causes calling xdp_do_flush() under the process context. Simply disable premmption may not work since process may move among processors which lead xdp_do_flush() to miss some flushes on some processors. So simply revert the patch, a follow-up path will add the xdp flush correctly. Reported-by: Christoffer Dall Fixes: 762c330d670e ("tuntap: add missing xdp flush") Signed-off-by: Jason Wang --- drivers/net/tun.c | 15 --- 1 file changed, 15 deletions(-) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index b52258c..2823a4a 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -181,7 +181,6 @@ struct tun_file { struct tun_struct *detached; struct ptr_ring tx_ring; struct xdp_rxq_info xdp_rxq; - int xdp_pending_pkts; }; struct tun_flow_entry { @@ -1662,7 +1661,6 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, case XDP_REDIRECT: get_page(alloc_frag->page); alloc_frag->offset += buflen; - ++tfile->xdp_pending_pkts; err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); if (err) goto err_redirect; @@ -1984,11 +1982,6 @@ static ssize_t tun_chr_write_iter(struct kiocb *iocb, struct iov_iter *from) result = tun_get_user(tun, tfile, NULL, from, file->f_flags & O_NONBLOCK, false); - if (tfile->xdp_pending_pkts) { - tfile->xdp_pending_pkts = 0; - xdp_do_flush_map(); - } - tun_put(tun); return result; } @@ -2325,13 +2318,6 @@ static int tun_sendmsg(struct socket *sock, struct msghdr *m, size_t total_len) ret = tun_get_user(tun, tfile, m->msg_control, &m->msg_iter, m->msg_flags & MSG_DONTWAIT, m->msg_flags & MSG_MORE); - - if (tfile->xdp_pending_pkts >= NAPI_POLL_WEIGHT || - !(m->msg_flags & MSG_MORE)) { - tfile->xdp_pending_pkts = 0; - xdp_do_flush_map(); - } - tun_put(tun); return ret; } @@ -3163,7 +3149,6 @@ static int tun_chr_open(struct inode *inode, struct file * file) sock_set_flag(&tfile->sk, SOCK_ZEROCOPY); memset(&tfile->tx_ring, 0, sizeof(tfile->tx_ring)); - tfile->xdp_pending_pkts = 0; return 0; } -- 2.7.4
[PATCH net v2 2/2] tuntap: correctly add the missing xdp flush
Commit 762c330d670e ("tuntap: add missing xdp flush") tries to fix the devmap stall caused by missed xdp flush by counting the pending xdp redirected packets and flush when it exceeds NAPI_POLL_WEIGHT or MSG_MORE is clear. This may lead BUG() since xdp_do_flush() was called under process context with preemption enabled. Simply disable preemption may silent the warning but be not enough since process may move between different CPUS during a batch which cause xdp_do_flush() misses some CPU where the process run previously. Consider the several fallouts, that commit was reverted. To fix the issue correctly, we can simply calling xdp_do_flush() immediately after xdp_do_redirect(), a side effect is that this removes any possibility of batching which could be addressed in the future. Reported-by: Christoffer Dall Fixes: 762c330d670e ("tuntap: add missing xdp flush") Signed-off-by: Jason Wang --- drivers/net/tun.c | 1 + 1 file changed, 1 insertion(+) diff --git a/drivers/net/tun.c b/drivers/net/tun.c index 2823a4a..a363ea2 100644 --- a/drivers/net/tun.c +++ b/drivers/net/tun.c @@ -1662,6 +1662,7 @@ static struct sk_buff *tun_build_skb(struct tun_struct *tun, get_page(alloc_frag->page); alloc_frag->offset += buflen; err = xdp_do_redirect(tun->dev, &xdp, xdp_prog); + xdp_do_flush_map(); if (err) goto err_redirect; rcu_read_unlock(); -- 2.7.4
[PATCH ipmi/kcs_bmc v4] ipmi: kcs_bmc: make the code be more clean
Modify the file read and write operation API's parameter declaration to meet the name convention. Rename the helper function by following the to_() name style. Update the poll API with the new type '__poll_t', this is new commit from linux-4.16-rc1. Correct the header file's comment style for 'SPDX-License-Identifier'. Correct the header file's macro defination end comment which is old file name. Remove the space between the comment words and colon by referring other modules. Signed-off-by: Haiyue Wang --- v3 -> v4: - Correct the header file's macro defination end comment which is old file name, I forgot to change it after changing the file name. - Remove the space between the comment words and colon by referring other modules. v2 -> v3: - Make the commit message be more understandable. v1 -> v2: - Add 'SPDX-License-Identifier' style for header files modification. --- drivers/char/ipmi/kcs_bmc.c| 32 +--- drivers/char/ipmi/kcs_bmc.h| 36 +++- drivers/char/ipmi/kcs_bmc_aspeed.c | 4 +++- include/uapi/linux/ipmi_bmc.h | 8 +--- 4 files changed, 44 insertions(+), 36 deletions(-) diff --git a/drivers/char/ipmi/kcs_bmc.c b/drivers/char/ipmi/kcs_bmc.c index 6476bfb..fbfc05e 100644 --- a/drivers/char/ipmi/kcs_bmc.c +++ b/drivers/char/ipmi/kcs_bmc.c @@ -1,5 +1,7 @@ // SPDX-License-Identifier: GPL-2.0 -// Copyright (c) 2015-2018, Intel Corporation. +/* + * Copyright (c) 2015-2018, Intel Corporation. + */ #define pr_fmt(fmt) "kcs-bmc: " fmt @@ -242,14 +244,14 @@ int kcs_bmc_handle_event(struct kcs_bmc *kcs_bmc) } EXPORT_SYMBOL(kcs_bmc_handle_event); -static inline struct kcs_bmc *file_to_kcs_bmc(struct file *filp) +static inline struct kcs_bmc *to_kcs_bmc(struct file *filp) { return container_of(filp->private_data, struct kcs_bmc, miscdev); } static int kcs_bmc_open(struct inode *inode, struct file *filp) { - struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp); + struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp); int ret = 0; spin_lock_irq(&kcs_bmc->lock); @@ -262,25 +264,25 @@ static int kcs_bmc_open(struct inode *inode, struct file *filp) return ret; } -static unsigned int kcs_bmc_poll(struct file *filp, poll_table *wait) +static __poll_t kcs_bmc_poll(struct file *filp, poll_table *wait) { - struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp); - unsigned int mask = 0; + struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp); + __poll_t mask = 0; poll_wait(filp, &kcs_bmc->queue, wait); spin_lock_irq(&kcs_bmc->lock); if (kcs_bmc->data_in_avail) - mask |= POLLIN; + mask |= EPOLLIN; spin_unlock_irq(&kcs_bmc->lock); return mask; } -static ssize_t kcs_bmc_read(struct file *filp, char *buf, - size_t count, loff_t *offset) +static ssize_t kcs_bmc_read(struct file *filp, char __user *buf, + size_t count, loff_t *ppos) { - struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp); + struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp); bool data_avail; size_t data_len; ssize_t ret; @@ -339,10 +341,10 @@ static ssize_t kcs_bmc_read(struct file *filp, char *buf, return ret; } -static ssize_t kcs_bmc_write(struct file *filp, const char *buf, -size_t count, loff_t *offset) +static ssize_t kcs_bmc_write(struct file *filp, const char __user *buf, +size_t count, loff_t *ppos) { - struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp); + struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp); ssize_t ret; /* a minimum response size '3' : netfn + cmd + ccode */ @@ -378,7 +380,7 @@ static ssize_t kcs_bmc_write(struct file *filp, const char *buf, static long kcs_bmc_ioctl(struct file *filp, unsigned int cmd, unsigned long arg) { - struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp); + struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp); long ret = 0; spin_lock_irq(&kcs_bmc->lock); @@ -410,7 +412,7 @@ static long kcs_bmc_ioctl(struct file *filp, unsigned int cmd, static int kcs_bmc_release(struct inode *inode, struct file *filp) { - struct kcs_bmc *kcs_bmc = file_to_kcs_bmc(filp); + struct kcs_bmc *kcs_bmc = to_kcs_bmc(filp); spin_lock_irq(&kcs_bmc->lock); kcs_bmc->running = 0; diff --git a/drivers/char/ipmi/kcs_bmc.h b/drivers/char/ipmi/kcs_bmc.h index c19501d..eb9ea4c 100644 --- a/drivers/char/ipmi/kcs_bmc.h +++ b/drivers/char/ipmi/kcs_bmc.h @@ -1,31 +1,33 @@ -// SPDX-License-Identifier: GPL-2.0 -// Copyright (c) 2015-2018, Intel Corporation. +/* SPDX-License-Identifier: GPL-2.0 */ +/* + * Copyright (c) 2015-2018, Intel Corporation. + */ #ifndef __KCS_BMC_H__ #define __KCS_BMC_H__ #include -/* Different phases of the KCS BMC module : - * KCS_PHASE_IDLE :
Re: [PATCH v4 3/4] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP
On 02/19/2018 10:06 PM, Marc Zyngier wrote: > On Fri, 16 Feb 2018 11:35:02 +0530 > Rajendra Nayak wrote: > >> Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files >> >> Signed-off-by: Rajendra Nayak >> Reviewed-by: Doug Anderson >> --- >> arch/arm64/boot/dts/qcom/Makefile | 1 + >> arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 15 ++ >> arch/arm64/boot/dts/qcom/sdm845.dtsi| 277 >> >> 3 files changed, 293 insertions(+) >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts >> create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi [...] >> + >> +soc: soc { >> +#address-cells = <1>; >> +#size-cells = <1>; >> +ranges = <0 0 0 0x>; >> +compatible = "simple-bus"; >> + >> +intc: interrupt-controller@17a0 { >> +compatible = "arm,gic-v3"; >> +#address-cells = <1>; >> +#size-cells = <1>; >> +ranges; >> +#interrupt-cells = <3>; >> +interrupt-controller; >> +#redistributor-regions = <1>; >> +redistributor-stride = <0x0 0x2>; >> +reg = <0x17a0 0x1>, /* GICD */ >> + <0x17a6 0x10>;/* GICR * 8 */ >> +interrupts = ; >> + >> +gic-its@17a4 { >> +compatible = "arm,gic-v3-its"; >> +msi-controller; >> +#msi-cells = <1>; >> +reg = <0x17a4 0x2>; >> +status = "disabled"; >> +}; >> +}; >> + >> +gcc: clock-controller@10 { >> +compatible = "qcom,gcc-sdm845"; >> +reg = <0x10 0x1f>; >> +#clock-cells = <1>; >> +#reset-cells = <1>; >> +}; >> + >> +tlmm: pinctrl@340 { >> +compatible = "qcom,sdm845-pinctrl"; >> +reg = <0x0340 0xc0>; >> +interrupts = ; > > Please do not use IRQ_TYPE_NONE, ever. It doesn't exist in the GIC > binding. Set it to the actual trigger value. Thanks Marc for the review. I fixed these up and did a respin. -- Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH] netlink: put module reference if dump start fails
Hi, On Wed, Feb 21, 2018 at 04:41:05PM +0100, Jason A. Donenfeld wrote: Fixes: 41c87425a1ac ("netlink: do not set cb_running if dump's start() errs") I think you Would better to resend it. Bo,
Re: [PATCH v13 7/8] ARM: dts: imx: Add TDA19971 HDMI Receiver to GW54xx
On Thu, Feb 15, 2018 at 01:19:17PM -0800, Tim Harvey wrote: > On Thu, Feb 15, 2018 at 10:36 AM, Hans Verkuil wrote: > > On 15/02/18 18:55, Tim Harvey wrote: > >> The GW54xx has a front-panel microHDMI connector routed to a TDA19971 > >> which is connected the the IPU CSI when using IMX6Q. > > > > I assume that this and the next patch go through another subsystem for arm > > and/or imx? > > > > Regards, > > > > Hans > > > > Hans, > > Yes - Shawn should pick up the two dts patches: > 0007-ARM-dts-imx-Add-TDA19971-HDMI-Receiver-to-GW54xx.patch > 0008-ARM-dts-imx-Add-TDA19971-HDMI-Receiver-to-GW551x.patch > > Shawn you've seen these before but haven't ack'd them - are they good > to merge to your imx tree? Yes. I will pick up the dts patches after Hans merges driver part. Shawn
[PATCH v5 2/4] dt-bindings: qcom: Add SDM845 bindings
Add a SoC string 'sdm845' for the qualcomm SDM845 SoC Signed-off-by: Rajendra Nayak Reviewed-by: Douglas Anderson Reviewed-by: Rob Herring --- Documentation/devicetree/bindings/arm/qcom.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/qcom.txt b/Documentation/devicetree/bindings/arm/qcom.txt index 0ed4d39d7fe1..ee532e705d6c 100644 --- a/Documentation/devicetree/bindings/arm/qcom.txt +++ b/Documentation/devicetree/bindings/arm/qcom.txt @@ -26,6 +26,7 @@ The 'SoC' element must be one of the following strings: msm8996 mdm9615 ipq8074 + sdm845 The 'board' element must be one of the following strings: -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
Re: [PATCH v1] drm/simple_kms_helper: Fix NULL pointer dereference with no active CRTC
On 02/22/2018 08:09 AM, Oleksandr Andrushchenko wrote: From: Oleksandr Andrushchenko It is possible that drm_simple_kms_plane_atomic_check called with no CRTC set, e.g. when user-space application sets CRTC_ID/FB_ID to 0 before doing any actual drawing. This leads to NULL pointer dereference because in this case new CRTC state is NULL and must be checked before accessing. Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Daniel Vetter --- Changes since initial: - re-worked checks for null CRTC as suggested by Daniel Vetter --- drivers/gpu/drm/drm_simple_kms_helper.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 9ca8a4a59b74..4a1dbd88b1ec 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -121,12 +121,6 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, pipe = container_of(plane, struct drm_simple_display_pipe, plane); crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, &pipe->crtc); - if (!crtc_state->enable) - return 0; /* nothing to check when disabling or disabled */ - - if (crtc_state->enable) - drm_mode_get_hv_timing(&crtc_state->mode, - &clip.x2, &clip.y2); ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state, &clip, @@ -137,7 +131,9 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, return ret; if (!plane_state->visible) - return -EINVAL; Daniel, I have put your R-b tag, but I had removed suggested "WARN_ON(crtc_state && crtc_state->enable);" here as it fires each time when crtc_state is not NULL. Please let me know if this is not ok and you want me to remove your R-b tag. + return 0; + + drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2); if (!pipe->funcs || !pipe->funcs->check) return 0; Thank you, Oleksandr
[PATCH v5 4/4] arm64: dts: sdm845: Add serial console support
Add the qup uart node and geni se instance needed to support the serial console on the MTP. Signed-off-by: Rajendra Nayak --- arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 39 + arch/arm64/boot/dts/qcom/sdm845.dtsi| 39 + 2 files changed, 78 insertions(+) diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts index 979ab49913f1..2a1ed55b703e 100644 --- a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts @@ -12,4 +12,43 @@ / { model = "Qualcomm Technologies, Inc. SDM845 MTP"; compatible = "qcom,sdm845-mtp"; + + aliases { + serial0 = &qup_uart2; + }; + + chosen { + stdout-path = "serial0"; + }; +}; + +&soc { + geni-se@ac { + serial@a84000 { + status = "okay"; + }; + }; + + pinctrl@340 { + qup-uart2-default { + pinconf_tx { + pins = "gpio4"; + drive-strength = <2>; + bias-disable; + }; + + pinconf_rx { + pins = "gpio5"; + drive-strength = <2>; + bias-pull-up; + }; + }; + + qup-uart2-sleep { + pinconf { + pins = "gpio4", "gpio5"; + bias-pull-down; + }; + }; + }; }; diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi index da3d6ac906c8..0acd6c51115d 100644 --- a/arch/arm64/boot/dts/qcom/sdm845.dtsi +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -6,6 +6,7 @@ */ #include +#include / { interrupt-parent = <&intc>; @@ -195,6 +196,20 @@ #gpio-cells = <2>; interrupt-controller; #interrupt-cells = <2>; + + qup_uart2_default: qup-uart2-default { + pinmux { + function = "qup9"; + pins = "gpio4", "gpio5"; + }; + }; + + qup_uart2_sleep: qup-uart2-sleep { + pinmux { + function = "gpio"; + pins = "gpio4", "gpio5"; + }; + }; }; timer@17c9 { @@ -273,5 +288,29 @@ #interrupt-cells = <4>; cell-index = <0>; }; + + geni-se@ac { + compatible = "qcom,geni-se-qup"; + #address-cells = <1>; + #size-cells = <1>; + ranges; + reg = <0xac 0x6000>; + clocks = <&gcc GCC_QUPV3_WRAP_1_M_AHB_CLK>, +<&gcc GCC_QUPV3_WRAP_1_S_AHB_CLK>; + clock-names = "m-ahb", "s-ahb"; + + qup_uart2: serial@a84000 { + compatible = "qcom,geni-debug-uart"; + reg = <0xa84000 0x4000>; + reg-names = "se-phys"; + clock-names = "se-clk"; + clocks = <&gcc GCC_QUPV3_WRAP1_S1_CLK>; + pinctrl-names = "default", "sleep"; + pinctrl-0 = <&qup_uart2_default>; + pinctrl-1 = <&qup_uart2_sleep>; + interrupts = ; + status = "disabled"; + }; + }; }; }; -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v5 3/4] arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP
Add a skeletal sdm845 SoC dtsi and MTP board dts/dtsi files Signed-off-by: Rajendra Nayak Reviewed-by: Doug Anderson --- arch/arm64/boot/dts/qcom/Makefile | 1 + arch/arm64/boot/dts/qcom/sdm845-mtp.dts | 15 ++ arch/arm64/boot/dts/qcom/sdm845.dtsi| 277 3 files changed, 293 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi diff --git a/arch/arm64/boot/dts/qcom/Makefile b/arch/arm64/boot/dts/qcom/Makefile index 55ec5ee7f7e8..9319e74b8906 100644 --- a/arch/arm64/boot/dts/qcom/Makefile +++ b/arch/arm64/boot/dts/qcom/Makefile @@ -6,3 +6,4 @@ dtb-$(CONFIG_ARCH_QCOM) += msm8916-mtp.dtb dtb-$(CONFIG_ARCH_QCOM)+= msm8992-bullhead-rev-101.dtb dtb-$(CONFIG_ARCH_QCOM)+= msm8994-angler-rev-101.dtb dtb-$(CONFIG_ARCH_QCOM)+= msm8996-mtp.dtb +dtb-$(CONFIG_ARCH_QCOM)+= sdm845-mtp.dtb diff --git a/arch/arm64/boot/dts/qcom/sdm845-mtp.dts b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts new file mode 100644 index ..979ab49913f1 --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sdm845-mtp.dts @@ -0,0 +1,15 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SDM845 MTP board device tree source + * + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +/dts-v1/; + +#include "sdm845.dtsi" + +/ { + model = "Qualcomm Technologies, Inc. SDM845 MTP"; + compatible = "qcom,sdm845-mtp"; +}; diff --git a/arch/arm64/boot/dts/qcom/sdm845.dtsi b/arch/arm64/boot/dts/qcom/sdm845.dtsi new file mode 100644 index ..da3d6ac906c8 --- /dev/null +++ b/arch/arm64/boot/dts/qcom/sdm845.dtsi @@ -0,0 +1,277 @@ +// SPDX-License-Identifier: GPL-2.0 +/* + * SDM845 SoC device tree source + * + * Copyright (c) 2018, The Linux Foundation. All rights reserved. + */ + +#include + +/ { + interrupt-parent = <&intc>; + + #address-cells = <2>; + #size-cells = <2>; + + chosen { }; + + memory@8000 { + device_type = "memory"; + /* We expect the bootloader to fill in the size */ + reg = <0 0x8000 0 0>; + }; + + cpus { + #address-cells = <2>; + #size-cells = <0>; + + CPU0: cpu@0 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x0>; + enable-method = "psci"; + next-level-cache = <&L2_0>; + L2_0: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + L3_0: l3-cache { + compatible = "cache"; + }; + }; + }; + + CPU1: cpu@100 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x100>; + enable-method = "psci"; + next-level-cache = <&L2_100>; + L2_100: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU2: cpu@200 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x200>; + enable-method = "psci"; + next-level-cache = <&L2_200>; + L2_200: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU3: cpu@300 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x300>; + enable-method = "psci"; + next-level-cache = <&L2_300>; + L2_300: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU4: cpu@400 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x400>; + enable-method = "psci"; + next-level-cache = <&L2_400>; + L2_400: l2-cache { + compatible = "cache"; + next-level-cache = <&L3_0>; + }; + }; + + CPU5: cpu@500 { + device_type = "cpu"; + compatible = "qcom,kryo385"; + reg = <0x0 0x500>; +
[PATCH v5 0/4] Add DTS for SDM845 SoC and MTP
These are basic device tree files needed to boot a SDM845 MTP board to a ramfs based serial console shell Bindings are based on whats proposed for pinctrl/serial/clock drivers for SDM845 SoC pinctrl: https://patchwork.kernel.org/patch/10157143/ (This is now pulled in by Linus Walleij for 4.17) clocks: https://lkml.org/lkml/2018/1/31/209 (under review) serial: https://patchwork.ozlabs.org/cover/860251/ (under review) 'PATCH 4/4' is based on v2 of serial patches, will need an update if v3 (still in the works) has further binding updates Since 'PATCH 3/4' also adds an ITS node and keeps it disabled, we also depend on https://lkml.org/lkml/2018/1/29/383 changes in v5: * Removed all instances of IRQ_TYPE_NONE changes in v4: * pull config changes to uart pins * License in device tree files is still GPL-2.0 changes in v3: * split the pinmux/pinconf nodes across SoC/Board files * Fixes for issues reported with 'make dtbs W=2' * other minor fixes based on review changes in v2: * dropped cpu-map * dropped GIC_CPU_MASK_SIMPLE() * Added new cpu compatible for kryo385 * added ITS node, marked as disabled Rajendra Nayak (4): dt-bindings: arm: Document kryo385 cpu dt-bindings: qcom: Add SDM845 bindings arm64: dts: sdm845: Add minimal dts/dtsi files for sdm845 SoC and MTP arm64: dts: sdm845: Add serial console support Documentation/devicetree/bindings/arm/cpus.txt | 1 + Documentation/devicetree/bindings/arm/qcom.txt | 1 + arch/arm64/boot/dts/qcom/Makefile | 1 + arch/arm64/boot/dts/qcom/sdm845-mtp.dts| 54 + arch/arm64/boot/dts/qcom/sdm845.dtsi | 316 + 5 files changed, 373 insertions(+) create mode 100644 arch/arm64/boot/dts/qcom/sdm845-mtp.dts create mode 100644 arch/arm64/boot/dts/qcom/sdm845.dtsi -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v5 1/4] dt-bindings: arm: Document kryo385 cpu
Document the compatible string for the Kryo385 cpus found in qualcomm SoCs. Signed-off-by: Rajendra Nayak Reviewed-by: Rob Herring Reviewed-by: Douglas Anderson --- Documentation/devicetree/bindings/arm/cpus.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/Documentation/devicetree/bindings/arm/cpus.txt b/Documentation/devicetree/bindings/arm/cpus.txt index a0009b72e9be..19611ccb61d9 100644 --- a/Documentation/devicetree/bindings/arm/cpus.txt +++ b/Documentation/devicetree/bindings/arm/cpus.txt @@ -184,6 +184,7 @@ described below. "nvidia,tegra186-denver" "qcom,krait" "qcom,kryo" + "qcom,kryo385" "qcom,scorpion" - enable-method Value type: -- QUALCOMM INDIA, on behalf of Qualcomm Innovation Center, Inc. is a member of Code Aurora Forum, hosted by The Linux Foundation
[PATCH v1] drm/simple_kms_helper: Fix NULL pointer dereference with no active CRTC
From: Oleksandr Andrushchenko It is possible that drm_simple_kms_plane_atomic_check called with no CRTC set, e.g. when user-space application sets CRTC_ID/FB_ID to 0 before doing any actual drawing. This leads to NULL pointer dereference because in this case new CRTC state is NULL and must be checked before accessing. Signed-off-by: Oleksandr Andrushchenko Reviewed-by: Daniel Vetter --- Changes since initial: - re-worked checks for null CRTC as suggested by Daniel Vetter --- drivers/gpu/drm/drm_simple_kms_helper.c | 10 +++--- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/drivers/gpu/drm/drm_simple_kms_helper.c b/drivers/gpu/drm/drm_simple_kms_helper.c index 9ca8a4a59b74..4a1dbd88b1ec 100644 --- a/drivers/gpu/drm/drm_simple_kms_helper.c +++ b/drivers/gpu/drm/drm_simple_kms_helper.c @@ -121,12 +121,6 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, pipe = container_of(plane, struct drm_simple_display_pipe, plane); crtc_state = drm_atomic_get_new_crtc_state(plane_state->state, &pipe->crtc); - if (!crtc_state->enable) - return 0; /* nothing to check when disabling or disabled */ - - if (crtc_state->enable) - drm_mode_get_hv_timing(&crtc_state->mode, - &clip.x2, &clip.y2); ret = drm_atomic_helper_check_plane_state(plane_state, crtc_state, &clip, @@ -137,7 +131,9 @@ static int drm_simple_kms_plane_atomic_check(struct drm_plane *plane, return ret; if (!plane_state->visible) - return -EINVAL; + return 0; + + drm_mode_get_hv_timing(&crtc_state->mode, &clip.x2, &clip.y2); if (!pipe->funcs || !pipe->funcs->check) return 0; -- 2.7.4
Re: [PATCH v4 00/16] R-Car DU: Convert LVDS code to bridge driver
On 02/20/18 15:10, Laurent Pinchart wrote: > Hello, > > This patch series addresses a design mistake that dates back from the initial > DU support. Support for the LVDS encoders, which are IP cores separate from > the DU, was bundled in the DU driver. Worse, both the DU and LVDS were > described through a single DT node. > > To fix the, patches 01/16 and 02/16 define new DT bindings for the LVDS > encoders, and deprecate their description inside the DU bindings. To retain > backward compatibility with existing DT, patches 03/16 to 08/16 then patch the > device tree at runtime to convert the legacy bindings to the new ones. > > With the DT side addressed, patch 09/16 converts the LVDS support code to a > separate bridge driver. Patches 11/16 to 16/16 then update all the device tree > sources to the new DU and LVDS encoders bindings. > > I decided to go for live DT patching in patch 08/16 because implementing > support for both the legacy and new bindings in the driver would have been > very intrusive, and prevented further cleanups. This version relies more > heavily on overlays to avoid touching the internals of the OF core compared to > v2, even if manual fixes to the device tree are still needed. > > Compared to v3, this series uses the OF changeset API to update properties > instead of accessing the internals of the property structure. This removes the > local implementation of functions to look up nodes by path and update > properties. In order to do this, I pulled in Pantelis' patch series titled > "[PATCH v2 0/5] of: dynamic: Changesets helpers & fixes" at Rob's request, and > rebased it while taking two small review comments into account. Wait a minute! Why are you putting a patch set to modify core devicetree in the middle of a driver series. Please pull it out to a separate series. I'll try to look at the patches, as they are in this series, sometime tomorrow. I have a vague memory of unresolved issues from the last time they were proposed. Thanks, Frank > > Rob, I'd like this series to be merged in v4.17. As the changeset helpers are > now a dependency, I'd need you to merge them early (ideally on top of > v4.16-rc1) and provide a stable branch, or get your ack to merge them through > Dave's tree if they don't conflict with what you have and will queue for > v4.17. > > This version also drops the small fix to the Porter board device tree that has > been queued for v4.17 already. > > Compared to v2, the biggest change is in patch 03/16. Following Rob's and > Frank's reviews it was clear that modifying the unflattened DT structure of > the overlay before applying it wasn't popular. I have thus decided to use one > overlay source per SoC to move as much of the DT changes to the overlay as > possible, and only perform manual modifications (that are still needed as some > of the information is board-specific) on the system DT after applying the > overlay. As a result the overlay is parsed and applied without being modified. > > Compared to v1, this series update the r8a7792 and r8a7794 device tree sources > and incorporate review feedback as described by the changelogs of individual > patches. > > > Laurent Pinchart (11): > dt-bindings: display: renesas: Add R-Car LVDS encoder DT bindings > dt-bindings: display: renesas: Deprecate LVDS support in the DU > bindings > drm: rcar-du: Fix legacy DT to create LVDS encoder nodes > drm: rcar-du: Convert LVDS encoder code to bridge driver > ARM: dts: r8a7790: Convert to new LVDS DT bindings > ARM: dts: r8a7791: Convert to new LVDS DT bindings > ARM: dts: r8a7792: Convert to new DU DT bindings > ARM: dts: r8a7793: Convert to new LVDS DT bindings > ARM: dts: r8a7794: Convert to new DU DT bindings > arm64: dts: renesas: r8a7795: Convert to new LVDS DT bindings > arm64: dts: renesas: r8a7796: Convert to new LVDS DT bindings > > Pantelis Antoniou (5): > of: dynamic: Add __of_node_dupv() > of: changesets: Introduce changeset helper methods > of: changeset: Add of_changeset_node_move method > of: unittest: changeset helpers > i2c: demux: Use changeset helpers for clarity > > .../bindings/display/bridge/renesas,lvds.txt | 56 +++ > .../devicetree/bindings/display/renesas,du.txt | 31 +- > MAINTAINERS| 1 + > arch/arm/boot/dts/r8a7790-lager.dts| 22 +- > arch/arm/boot/dts/r8a7790.dtsi | 64 ++- > arch/arm/boot/dts/r8a7791-koelsch.dts | 10 +- > arch/arm/boot/dts/r8a7791-porter.dts | 16 +- > arch/arm/boot/dts/r8a7791.dtsi | 36 +- > arch/arm/boot/dts/r8a7792.dtsi | 1 - > arch/arm/boot/dts/r8a7793-gose.dts | 10 +- > arch/arm/boot/dts/r8a7793.dtsi | 37 +- > arch/arm/boot/dts/r8a7794.dtsi | 1 - > .../boot/dts/renesas/r8a7795-es1-salvator-x.dts| 3 +- > arch/arm64/boot/dts/renesas/r8a7
Re: [PATCH] doc: process: Add "Root-caused-by" and "Suggested-by"
On Wed, Feb 21, 2018 at 8:43 PM, Randy Dunlap wrote: > On 02/21/2018 04:37 PM, Kees Cook wrote: >> As recently pointed out by Linus, "Root-caused-by" is a good tag to include >> since it can indicate significantly more work than "just" a Reported-by. >> This adds it and "Suggested-by" (which was also missing) to the documented >> list of tags. Additionally updates checkpatch.pl to match the process docs. >> >> Signed-off-by: Kees Cook >> --- >> Documentation/process/5.Posting.rst | 7 +++ >> scripts/checkpatch.pl | 2 ++ >> 2 files changed, 9 insertions(+) > > I would still rather see Co-developed-by: in both the docs and in checkpatch. > :( Hm? It is in docs. This syncs the process doc to checkpatch... -Kees > >> diff --git a/Documentation/process/5.Posting.rst >> b/Documentation/process/5.Posting.rst >> index 645fa9c7388a..2ff01f76f02a 100644 >> --- a/Documentation/process/5.Posting.rst >> +++ b/Documentation/process/5.Posting.rst >> @@ -234,6 +234,13 @@ The tags in common use are: >> people who test our code and let us know when things do not work >> correctly. >> >> + - Suggested-by: names a person who suggested the solution, but may not >> + have constructed the full patch. A weaker version of `Co-Developed-by`. >> + >> + - Root-caused-by: names a person who diagnosed the root cause of a >> + problem. This usually indicates significantly more work than a simple >> + `Reported-by`. >> + >> - Cc: the named person received a copy of the patch and had the >> opportunity to comment on it. >> >> diff --git a/scripts/checkpatch.pl b/scripts/checkpatch.pl >> index 3d4040322ae1..a1ab82e70b54 100755 >> --- a/scripts/checkpatch.pl >> +++ b/scripts/checkpatch.pl >> @@ -464,9 +464,11 @@ our $logFunctions = qr{(?x: >> our $signature_tags = qr{(?xi: >> Signed-off-by:| >> Acked-by:| >> + Co-Developed-by:| >> Tested-by:| >> Reviewed-by:| >> Reported-by:| >> + Root-caused-by:| >> Suggested-by:| >> To:| >> Cc: >> > > > -- > ~Randy -- Kees Cook Pixel Security
[PATCH 1/2] cpufreq: Reorder cpufreq_online() a bit
Ideally the de-allocation of resources should happen in the exact opposite order in which they were allocated. It helps maintain the code in long term, even if nothing really breaks with incorrect ordering. The same wasn't followed in cpufreq_online() and it has some inconsistencies. For example, the symlinks were created from within the locked region while they are removed only after putting the locks. Also ->exit() should have been called only after the symlinks are removed and the lock is dropped, as that was the case when ->init() was first called. Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index de33ebf008ad..8814c572e263 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1327,14 +1327,14 @@ static int cpufreq_online(unsigned int cpu) return 0; out_exit_policy: + for_each_cpu(j, policy->real_cpus) + remove_cpu_dev_symlink(policy, get_cpu_device(j)); + up_write(&policy->rwsem); if (cpufreq_driver->exit) cpufreq_driver->exit(policy); - for_each_cpu(j, policy->real_cpus) - remove_cpu_dev_symlink(policy, get_cpu_device(j)); - out_free_policy: cpufreq_policy_free(policy); return ret; -- 2.15.0.194.g9af6a3dea062
[PATCH 2/2] cpufreq: Validate cpufreq table from core
By design, it is responsibility of the cpufreq drivers to call cpufreq_frequency_table_cpuinfo() from their ->init() callback to validate the cpufreq table. But what if the cpufreq driver is buggy and fails to do so properly? It may then result in unexpected behavior from the driver or the cpufreq core at a later point of time. It would be better if the core can validate the cpufreq table at the initial stage. This commit adds another routine cpufreq_table_validate_and_sort() and calls it right after calling the ->init() callback of the driver and destroys the cpufreq policy if the table is invalid. For the moment the validation of the table happens twice, once from the driver and then from the core. The individual drivers would be updated separately to drop table validation if they don't need it for other purposes. The cpufreq table is marked "sorted" or "unsorted" from the new helper now instead of cpufreq_table_validate_and_show(), as we should be doing that only after validating the table (which the drivers wouldn't do going forward). Signed-off-by: Viresh Kumar --- drivers/cpufreq/cpufreq.c| 13 + drivers/cpufreq/freq_table.c | 16 +++- include/linux/cpufreq.h | 1 + 3 files changed, 25 insertions(+), 5 deletions(-) diff --git a/drivers/cpufreq/cpufreq.c b/drivers/cpufreq/cpufreq.c index 8814c572e263..239063fb6afc 100644 --- a/drivers/cpufreq/cpufreq.c +++ b/drivers/cpufreq/cpufreq.c @@ -1219,6 +1219,10 @@ static int cpufreq_online(unsigned int cpu) goto out_free_policy; } + ret = cpufreq_table_validate_and_sort(policy); + if (ret) + goto out_exit_policy; + down_write(&policy->rwsem); if (new_policy) { @@ -1249,7 +1253,7 @@ static int cpufreq_online(unsigned int cpu) policy->cur = cpufreq_driver->get(policy->cpu); if (!policy->cur) { pr_err("%s: ->get() failed\n", __func__); - goto out_exit_policy; + goto out_destroy_policy; } } @@ -1296,7 +1300,7 @@ static int cpufreq_online(unsigned int cpu) if (new_policy) { ret = cpufreq_add_dev_interface(policy); if (ret) - goto out_exit_policy; + goto out_destroy_policy; cpufreq_stats_create_table(policy); @@ -1311,7 +1315,7 @@ static int cpufreq_online(unsigned int cpu) __func__, cpu, ret); /* cpufreq_policy_free() will notify based on this */ new_policy = false; - goto out_exit_policy; + goto out_destroy_policy; } up_write(&policy->rwsem); @@ -1326,12 +1330,13 @@ static int cpufreq_online(unsigned int cpu) return 0; -out_exit_policy: +out_destroy_policy: for_each_cpu(j, policy->real_cpus) remove_cpu_dev_symlink(policy, get_cpu_device(j)); up_write(&policy->rwsem); +out_exit_policy: if (cpufreq_driver->exit) cpufreq_driver->exit(policy); diff --git a/drivers/cpufreq/freq_table.c b/drivers/cpufreq/freq_table.c index 6d007f824ca7..10e119ae66dd 100644 --- a/drivers/cpufreq/freq_table.c +++ b/drivers/cpufreq/freq_table.c @@ -362,10 +362,24 @@ int cpufreq_table_validate_and_show(struct cpufreq_policy *policy, return ret; policy->freq_table = table; - return set_freq_table_sorted(policy); + return 0; } EXPORT_SYMBOL_GPL(cpufreq_table_validate_and_show); +int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy) +{ + int ret; + + if (!policy->freq_table) + return 0; + + ret = cpufreq_frequency_table_cpuinfo(policy, policy->freq_table); + if (ret) + return ret; + + return set_freq_table_sorted(policy); +} + MODULE_AUTHOR("Dominik Brodowski "); MODULE_DESCRIPTION("CPUfreq frequency table helpers"); MODULE_LICENSE("GPL"); diff --git a/include/linux/cpufreq.h b/include/linux/cpufreq.h index 21e8d248d956..1fe49724da9e 100644 --- a/include/linux/cpufreq.h +++ b/include/linux/cpufreq.h @@ -962,6 +962,7 @@ extern struct freq_attr cpufreq_freq_attr_scaling_boost_freqs; extern struct freq_attr *cpufreq_generic_attr[]; int cpufreq_table_validate_and_show(struct cpufreq_policy *policy, struct cpufreq_frequency_table *table); +int cpufreq_table_validate_and_sort(struct cpufreq_policy *policy); unsigned int cpufreq_generic_get(unsigned int cpu); int cpufreq_generic_init(struct cpufreq_policy *policy, -- 2.15.0.194.g9af6a3dea062
[PATCH] cpufreq: mediatek: Convert pr_warn() to pr_debug()
With multi-platform build images, this shows a message on non mediatek platforms, which is unnecessary. Convert pr_warn() to pr_debug() here. Signed-off-by: Viresh Kumar --- drivers/cpufreq/mediatek-cpufreq.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/cpufreq/mediatek-cpufreq.c b/drivers/cpufreq/mediatek-cpufreq.c index 8c043c28..84d658d57029 100644 --- a/drivers/cpufreq/mediatek-cpufreq.c +++ b/drivers/cpufreq/mediatek-cpufreq.c @@ -578,7 +578,7 @@ static int __init mtk_cpufreq_driver_init(void) match = of_match_node(mtk_cpufreq_machines, np); of_node_put(np); if (!match) { - pr_warn("Machine is not compatible with mtk-cpufreq\n"); + pr_debug("Machine is not compatible with mtk-cpufreq\n"); return -ENODEV; } -- 2.15.0.194.g9af6a3dea062
Re: [PATCH v12 01/22] selftests/x86: Move protecton key selftest to arch neutral directory
* Ram Pai wrote: > cc: Dave Hansen > cc: Florian Weimer > Signed-off-by: Ram Pai > --- > tools/testing/selftests/vm/Makefile |1 + > tools/testing/selftests/vm/pkey-helpers.h | 223 > tools/testing/selftests/vm/protection_keys.c | 1407 > + > tools/testing/selftests/x86/Makefile |2 +- > tools/testing/selftests/x86/pkey-helpers.h| 223 > tools/testing/selftests/x86/protection_keys.c | 1407 > - > 6 files changed, 1632 insertions(+), 1631 deletions(-) > create mode 100644 tools/testing/selftests/vm/pkey-helpers.h > create mode 100644 tools/testing/selftests/vm/protection_keys.c > delete mode 100644 tools/testing/selftests/x86/pkey-helpers.h > delete mode 100644 tools/testing/selftests/x86/protection_keys.c Acked-by: Ingo Molnar Thanks, Ingo
[PATCH v1 1/5] dt-bindings: soc: update MT2712 power dt-bindings
Add new power domains(MFG_SC1/MFG_SC2/MFG_SC3) for MT2712 according to ECO design change. Signed-off-by: Weiyi Lu --- include/dt-bindings/power/mt2712-power.h | 3 +++ 1 file changed, 3 insertions(+) diff --git a/include/dt-bindings/power/mt2712-power.h b/include/dt-bindings/power/mt2712-power.h index 92b46d772fae..2c147817efc2 100644 --- a/include/dt-bindings/power/mt2712-power.h +++ b/include/dt-bindings/power/mt2712-power.h @@ -22,5 +22,8 @@ #define MT2712_POWER_DOMAIN_USB5 #define MT2712_POWER_DOMAIN_USB2 6 #define MT2712_POWER_DOMAIN_MFG7 +#define MT2712_POWER_DOMAIN_MFG_SC18 +#define MT2712_POWER_DOMAIN_MFG_SC29 +#define MT2712_POWER_DOMAIN_MFG_SC310 #endif /* _DT_BINDINGS_POWER_MT2712_POWER_H */ -- 2.12.5
[PATCH v1 3/5] dt-bindings: clock: add clocks for MT2712
add new clocks according to ECO design change Signed-off-by: Weiyi Lu --- include/dt-bindings/clock/mt2712-clk.h | 294 + 1 file changed, 151 insertions(+), 143 deletions(-) diff --git a/include/dt-bindings/clock/mt2712-clk.h b/include/dt-bindings/clock/mt2712-clk.h index 48a8e797a617..0690f24391b3 100644 --- a/include/dt-bindings/clock/mt2712-clk.h +++ b/include/dt-bindings/clock/mt2712-clk.h @@ -81,148 +81,154 @@ #define CLK_TOP_F_BUS_PLL2 42 #define CLK_TOP_APLL1 43 #define CLK_TOP_APLL1_D2 44 -#define CLK_TOP_APLL1_D4 45 -#define CLK_TOP_APLL1_D8 46 -#define CLK_TOP_APLL1_D16 47 -#define CLK_TOP_APLL2 48 -#define CLK_TOP_APLL2_D2 49 -#define CLK_TOP_APLL2_D4 50 -#define CLK_TOP_APLL2_D8 51 -#define CLK_TOP_APLL2_D16 52 -#define CLK_TOP_LVDSPLL53 -#define CLK_TOP_LVDSPLL_D2 54 -#define CLK_TOP_LVDSPLL_D4 55 -#define CLK_TOP_LVDSPLL_D8 56 -#define CLK_TOP_LVDSPLL2 57 -#define CLK_TOP_LVDSPLL2_D258 -#define CLK_TOP_LVDSPLL2_D459 -#define CLK_TOP_LVDSPLL2_D860 -#define CLK_TOP_ETHERPLL_125M 61 -#define CLK_TOP_ETHERPLL_50M 62 -#define CLK_TOP_CVBS 63 -#define CLK_TOP_CVBS_D264 -#define CLK_TOP_SYS_26M65 -#define CLK_TOP_MMPLL 66 -#define CLK_TOP_MMPLL_D2 67 -#define CLK_TOP_VENCPLL68 -#define CLK_TOP_VENCPLL_D2 69 -#define CLK_TOP_VCODECPLL 70 -#define CLK_TOP_VCODECPLL_D2 71 -#define CLK_TOP_TVDPLL 72 -#define CLK_TOP_TVDPLL_D2 73 -#define CLK_TOP_TVDPLL_D4 74 -#define CLK_TOP_TVDPLL_D8 75 -#define CLK_TOP_TVDPLL_429M76 -#define CLK_TOP_TVDPLL_429M_D2 77 -#define CLK_TOP_TVDPLL_429M_D4 78 -#define CLK_TOP_MSDCPLL79 -#define CLK_TOP_MSDCPLL_D2 80 -#define CLK_TOP_MSDCPLL_D4 81 -#define CLK_TOP_MSDCPLL2 82 -#define CLK_TOP_MSDCPLL2_D283 -#define CLK_TOP_MSDCPLL2_D484 -#define CLK_TOP_CLK26M_D2 85 -#define CLK_TOP_D2A_ULCLK_6P5M 86 -#define CLK_TOP_VPLL3_DPIX 87 -#define CLK_TOP_VPLL_DPIX 88 -#define CLK_TOP_LTEPLL_FS26M 89 -#define CLK_TOP_DMPLL 90 -#define CLK_TOP_DSI0_LNTC 91 -#define CLK_TOP_DSI1_LNTC 92 -#define CLK_TOP_LVDSTX3_CLKDIG_CTS 93 -#define CLK_TOP_LVDSTX_CLKDIG_CTS 94 -#define CLK_TOP_CLKRTC_EXT 95 -#define CLK_TOP_CLKRTC_INT 96 -#define CLK_TOP_CSI0 97 -#define CLK_TOP_CVBSPLL98 -#define CLK_TOP_AXI_SEL99 -#define CLK_TOP_MEM_SEL100 -#define CLK_TOP_MM_SEL 101 -#define CLK_TOP_PWM_SEL102 -#define CLK_TOP_VDEC_SEL 103 -#define CLK_TOP_VENC_SEL 104 -#define CLK_TOP_MFG_SEL105 -#define CLK_TOP_CAMTG_SEL 106 -#define CLK_TOP_UART_SEL 107 -#define CLK_TOP_SPI_SEL108 -#define CLK_TOP_USB20_SEL 109 -#define CLK_TOP_USB30_SEL 110 -#define CLK_TOP_MSDC50_0_HCLK_SEL 111 -#define CLK_TOP_MSDC50_0_SEL 112 -#define CLK_TOP_MSDC30_1_SEL 113 -#define CLK_TOP_MSDC30_2_SEL 114 -#define CLK_TOP_MSDC30_3_SEL 115 -#define CLK_TOP_AUDIO_SEL 116 -#define CLK_TOP_AUD_INTBUS_SEL 117 -#define CLK_TOP_PMICSPI_SEL118 -#define CLK_TOP_DPILVDS1_SEL 119 -#define CLK_TOP_ATB_SEL120 -#define CLK_TOP_NR_SEL 121 -#define CLK_TOP_NFI2X_SEL 122 -#define CLK_TOP_IRDA_SEL 123 -#define CLK_TOP_CCI400_SEL 124 -#define CLK_TOP_AUD_1_SEL 125 -#define CLK_TOP_AUD_2_SEL 126 -#define CLK_TOP_MEM_MFG_IN_AS_SEL 127 -#define CLK_TOP_AXI_MFG_IN_AS_SEL 128 -#define CLK_TOP_SCAM_SEL 129 -#define CLK_TOP_NFIECC_SEL 130 -#define CLK_TOP_PE2_MAC_P0_SEL 131 -#define CLK_TOP_PE2_MAC_P1_SEL 132 -#define CLK_TOP_DPILVDS_SEL133 -#define CLK_TOP_MSDC50_3_HCLK_SEL 134 -#define CLK_TOP_HDCP_SEL 135 -#define CLK_TOP_HDCP_24M_SEL 136 -#define CLK_TOP_RTC_SEL137 -#define CLK_TOP_SPINOR_SEL 138 -#define CLK_TOP_APLL_SEL 139 -#define CLK_TOP_APLL2_SEL 140 -#define CLK_TOP_A1SYS_HP_SEL 141 -#define CLK_TOP_A2SYS_HP_SEL 142 -#define CLK_TOP_ASM_L_SEL 143 -#define
[PATCH v1 4/5] arm64: dts: add clock device nodes of MT2712
add new clocks according to ECO design change Signed-off-by: Weiyi Lu --- arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 28 1 file changed, 28 insertions(+) diff --git a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi index fdf66f4fe7c3..d7688bc9db1b 100644 --- a/arch/arm64/boot/dts/mediatek/mt2712e.dtsi +++ b/arch/arm64/boot/dts/mediatek/mt2712e.dtsi @@ -199,6 +199,34 @@ clock-output-names = "clkaud_ext_i_2"; }; + clki2si0_mck_i: oscillator@6 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <3000>; + clock-output-names = "clki2si0_mck_i"; + }; + + clki2si1_mck_i: oscillator@7 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <3000>; + clock-output-names = "clki2si1_mck_i"; + }; + + clki2si2_mck_i: oscillator@8 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <3000>; + clock-output-names = "clki2si2_mck_i"; + }; + + clktdmin_mclk_i: oscillator@9 { + compatible = "fixed-clock"; + #clock-cells = <0>; + clock-frequency = <3000>; + clock-output-names = "clktdmin_mclk_i"; + }; + timer { compatible = "arm,armv8-timer"; interrupt-parent = <&gic>; -- 2.12.5
[PATCH v1 0/5] update Mediatek MT2712 clock and scpsys support
This series is based on v4.16-rc1 and composed of scpsys control (PATCH 1-2) and clock control (PATCH 3-5). Basically, all changes are for the ECO design change of MT2712. Weiyi Lu (5): dt-bindings: soc: update MT2712 power dt-bindings soc: mediatek: update power domain data of MT2712 dt-bindings: clock: add clocks for MT2712 arm64: dts: add clock device nodes of MT2712 clk: mediatek: update clock driver of MT2712 arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 28 +++ drivers/clk/mediatek/clk-mt2712.c | 69 +-- drivers/soc/mediatek/mtk-scpsys.c | 42 - include/dt-bindings/clock/mt2712-clk.h| 294 +++--- include/dt-bindings/power/mt2712-power.h | 3 + 5 files changed, 277 insertions(+), 159 deletions(-) -- 2.12.5
[PATCH v1 2/5] soc: mediatek: update power domain data of MT2712
1. split MFG power domain into MFG/MFG_SC1/MFG_SC2/MFG_SC3 according to MT2712 ECO design change 2. add subdomain support for MT2712 Signed-off-by: Weiyi Lu --- drivers/soc/mediatek/mtk-scpsys.c | 42 +-- 1 file changed, 40 insertions(+), 2 deletions(-) diff --git a/drivers/soc/mediatek/mtk-scpsys.c b/drivers/soc/mediatek/mtk-scpsys.c index 59bd749c2f25..edf8fd6c2c85 100644 --- a/drivers/soc/mediatek/mtk-scpsys.c +++ b/drivers/soc/mediatek/mtk-scpsys.c @@ -664,12 +664,48 @@ static const struct scp_domain_data scp_domain_data_mt2712[] = { .name = "mfg", .sta_mask = PWR_STATUS_MFG, .ctl_offs = SPM_MFG_PWR_CON, - .sram_pdn_bits = GENMASK(11, 8), - .sram_pdn_ack_bits = GENMASK(19, 16), + .sram_pdn_bits = GENMASK(8, 8), + .sram_pdn_ack_bits = GENMASK(16, 16), .clk_id = {CLK_MFG}, .bus_prot_mask = BIT(14) | BIT(21) | BIT(23), .active_wakeup = true, }, + [MT2712_POWER_DOMAIN_MFG_SC1] = { + .name = "mfg_sc1", + .sta_mask = BIT(22), + .ctl_offs = 0x02c0, + .sram_pdn_bits = GENMASK(8, 8), + .sram_pdn_ack_bits = GENMASK(16, 16), + .clk_id = {CLK_NONE}, + .active_wakeup = true, + }, + [MT2712_POWER_DOMAIN_MFG_SC2] = { + .name = "mfg_sc2", + .sta_mask = BIT(23), + .ctl_offs = 0x02c4, + .sram_pdn_bits = GENMASK(8, 8), + .sram_pdn_ack_bits = GENMASK(16, 16), + .clk_id = {CLK_NONE}, + .active_wakeup = true, + }, + [MT2712_POWER_DOMAIN_MFG_SC3] = { + .name = "mfg_sc3", + .sta_mask = BIT(30), + .ctl_offs = 0x01f8, + .sram_pdn_bits = GENMASK(8, 8), + .sram_pdn_ack_bits = GENMASK(16, 16), + .clk_id = {CLK_NONE}, + .active_wakeup = true, + }, +}; + +static const struct scp_subdomain scp_subdomain_mt2712[] = { + {MT2712_POWER_DOMAIN_MM, MT2712_POWER_DOMAIN_VDEC}, + {MT2712_POWER_DOMAIN_MM, MT2712_POWER_DOMAIN_VENC}, + {MT2712_POWER_DOMAIN_MM, MT2712_POWER_DOMAIN_ISP}, + {MT2712_POWER_DOMAIN_MFG, MT2712_POWER_DOMAIN_MFG_SC1}, + {MT2712_POWER_DOMAIN_MFG_SC1, MT2712_POWER_DOMAIN_MFG_SC2}, + {MT2712_POWER_DOMAIN_MFG_SC2, MT2712_POWER_DOMAIN_MFG_SC3}, }; /* @@ -905,6 +941,8 @@ static const struct scp_soc_data mt2701_data = { static const struct scp_soc_data mt2712_data = { .domains = scp_domain_data_mt2712, .num_domains = ARRAY_SIZE(scp_domain_data_mt2712), + .subdomains = scp_subdomain_mt2712, + .num_subdomains = ARRAY_SIZE(scp_subdomain_mt2712), .regs = { .pwr_sta_offs = SPM_PWR_STATUS, .pwr_sta2nd_offs = SPM_PWR_STATUS_2ND -- 2.12.5
[PATCH v1 0/5] update Mediatek MT2712 clock and scpsys support
This series is based on v4.16-rc1 and composed of scpsys control (PATCH 1-2) and clock control (PATCH 3-5). Basically, all changes are for the ECO design change of MT2712. Weiyi Lu (5): dt-bindings: soc: update MT2712 power dt-bindings soc: mediatek: update power domain data of MT2712 dt-bindings: clock: add clocks for MT2712 arm64: dts: add clock device nodes of MT2712 clk: mediatek: update clock driver of MT2712 arch/arm64/boot/dts/mediatek/mt2712e.dtsi | 28 +++ drivers/clk/mediatek/clk-mt2712.c | 69 +-- drivers/soc/mediatek/mtk-scpsys.c | 42 - include/dt-bindings/clock/mt2712-clk.h| 294 +++--- include/dt-bindings/power/mt2712-power.h | 3 + 5 files changed, 277 insertions(+), 159 deletions(-)
[PATCH v1 5/5] clk: mediatek: update clock driver of MT2712
According to ECO design change, 1. add new clock mux data and change some 2. add new clock gate data and clock factor data 3. change status register offset of infra subsystem Signed-off-by: Weiyi Lu --- drivers/clk/mediatek/clk-mt2712.c | 69 +++ 1 file changed, 55 insertions(+), 14 deletions(-) diff --git a/drivers/clk/mediatek/clk-mt2712.c b/drivers/clk/mediatek/clk-mt2712.c index 498d13799388..d4a7497a2417 100644 --- a/drivers/clk/mediatek/clk-mt2712.c +++ b/drivers/clk/mediatek/clk-mt2712.c @@ -141,6 +141,8 @@ static const struct mtk_fixed_factor top_divs[] = { 1), FACTOR(CLK_TOP_APLL1_D2, "apll1_d2", "apll1_ck", 1, 2), + FACTOR(CLK_TOP_APLL1_D3, "apll1_d3", "apll1_ck", 1, + 3), FACTOR(CLK_TOP_APLL1_D4, "apll1_d4", "apll1_ck", 1, 4), FACTOR(CLK_TOP_APLL1_D8, "apll1_d8", "apll1_ck", 1, @@ -625,7 +627,7 @@ static const char * const ether_125m_parents[] = { static const char * const ether_50m_parents[] = { "clk26m", "etherpll_50m", - "univpll_d26", + "apll1_d3", "univpll3_d4" }; @@ -686,7 +688,7 @@ static const char * const i2c_parents[] = { static const char * const msdc0p_aes_parents[] = { "clk26m", - "msdcpll_ck", + "syspll_d2", "univpll_d3", "vcodecpll_ck" }; @@ -719,6 +721,17 @@ static const char * const aud_apll2_parents[] = { "clkaud_ext_i_2" }; +static const char * const apll1_ref_parents[] = { + "clkaud_ext_i_2", + "clkaud_ext_i_1", + "clki2si0_mck_i", + "clki2si1_mck_i", + "clki2si2_mck_i", + "clktdmin_mclk_i", + "clki2si2_mck_i", + "clktdmin_mclk_i" +}; + static const char * const audull_vtx_parents[] = { "d2a_ulclk_6p5m", "clkaud_ext_i_0" @@ -884,6 +897,10 @@ static struct mtk_composite top_muxes[] = { aud_apll1_parents, 0x134, 0, 1), MUX(CLK_TOP_AUD_APLL2_SEL, "aud_apll2_sel", aud_apll2_parents, 0x134, 1, 1), + MUX(CLK_TOP_APLL1_REF_SEL, "apll1_ref_sel", + apll1_ref_parents, 0x134, 4, 3), + MUX(CLK_TOP_APLL2_REF_SEL, "apll2_ref_sel", + apll1_ref_parents, 0x134, 7, 3), MUX(CLK_TOP_DA_AUDULL_VTX_6P5M_SEL, "audull_vtx_sel", audull_vtx_parents, 0x134, 31, 1), }; @@ -932,36 +949,56 @@ static const struct mtk_clk_divider top_adj_divs[] = { DIV_ADJ(CLK_TOP_APLL_DIV7, "apll_div7", "i2si3_sel", 0x128, 24, 8), }; -static const struct mtk_gate_regs top_cg_regs = { +static const struct mtk_gate_regs top0_cg_regs = { .set_ofs = 0x120, .clr_ofs = 0x120, .sta_ofs = 0x120, }; -#define GATE_TOP(_id, _name, _parent, _shift) {\ +static const struct mtk_gate_regs top1_cg_regs = { + .set_ofs = 0x424, + .clr_ofs = 0x424, + .sta_ofs = 0x424, +}; + +#define GATE_TOP0(_id, _name, _parent, _shift) { \ .id = _id, \ .name = _name, \ .parent_name = _parent, \ - .regs = &top_cg_regs, \ + .regs = &top0_cg_regs, \ .shift = _shift,\ .ops = &mtk_clk_gate_ops_no_setclr, \ } +#define GATE_TOP1(_id, _name, _parent, _shift) { \ + .id = _id, \ + .name = _name, \ + .parent_name = _parent, \ + .regs = &top1_cg_regs, \ + .shift = _shift,\ + .ops = &mtk_clk_gate_ops_no_setclr_inv, \ + } + static const struct mtk_gate top_clks[] = { - GATE_TOP(CLK_TOP_APLL_DIV_PDN0, "apll_div_pdn0", "i2so1_sel", 0), - GATE_TOP(CLK_TOP_APLL_DIV_PDN1, "apll_div_pdn1", "i2so2_sel", 1), - GATE_TOP(CLK_TOP_APLL_DIV_PDN2, "apll_div_pdn2", "i2so3_sel", 2), - GATE_TOP(CLK_TOP_APLL_DIV_PDN3, "apll_div_pdn3", "tdmo0_sel", 3), - GATE_TOP(CLK_TOP_APLL_DIV_PDN4, "apll_div_pdn4", "tdmo1_sel", 4), - GATE_TOP(CLK_TOP_APLL_DIV_PDN5, "apll_div_pdn5", "i2si1_sel", 5), - GATE_TOP(CLK_TOP_APLL_DIV_PDN6, "apll_div_pdn6", "i2si2_sel", 6), - GATE_TOP(CLK_TOP_APLL_DIV_PDN7, "apll_div_pdn7", "i2si3_sel", 7), + /* TOP0 */ + GATE_TOP0(CLK_TOP_APLL_DIV_PDN0, "apll_div_pdn0", "i2so1_sel", 0), + GATE_TOP0(CLK_TOP_APLL_DIV_PDN1, "apll_div_pdn1", "i2so2_sel", 1), + GATE_TOP0(CLK_TOP_APLL_DIV_PDN2, "apll_div_pdn2", "i2so3_sel", 2), + GATE_TOP0(CLK_TOP_APLL_DIV_PDN3, "apll_div_pdn3", "tdmo0_sel", 3), + GATE_TOP0(CLK_TOP_APLL_DIV_PDN4, "apll_div_pdn4", "tdmo1_sel", 4), + GATE_TOP0(CLK_TOP_APLL_DIV_PDN5, "apll_div_pdn5", "i2si1_sel", 5), + GATE_TOP0(CLK_TOP_APLL_DIV_PDN6, "apll_div_pdn6",
Re: [PATCH RFC tools/lkmm 10/12] tools/memory-model: Add a S lock-based external-view litmus test
On 2/21/2018 9:27 PM, Boqun Feng wrote: > On Wed, Feb 21, 2018 at 08:13:57PM -0800, Paul E. McKenney wrote: >> On Thu, Feb 22, 2018 at 11:23:49AM +0800, Boqun Feng wrote: >>> On Tue, Feb 20, 2018 at 03:25:10PM -0800, Paul E. McKenney wrote: From: Alan Stern This commit adds a litmus test in which P0() and P1() form a lock-based S litmus test, with the addition of P2(), which observes P0()'s and P1()'s accesses with a full memory barrier but without the lock. This litmus test asks whether writes carried out by two different processes under the same lock will be seen in order by a third process not holding that lock. The answer to this question is "yes" for all architectures supporting >>> >>> Hmm.. it this true? Our spin_lock() is RCpc because of PowerPC, so >>> spin_lock()+spin_unlock() pairs don't provide transitivity, and that's >>> why we have smp_mb__after_unlock_lock(). Is there something I'm missing? >>> Or there is an upcomming commit to switch PowerPC's lock implementation? >> >> The PowerPC lock implementation's unlock-lock pair does not order writes >> from the previous critical section against reads from the later critical >> section, but it does order other combinations of reads and writes. > > Ah.. right! Thanks for the explanation ;-) > >> Some have apparently said that RISC-V 's unlock-lock pair also does not >> order writes from the previous critical section against writes from the >> later critical section. And no, I don't claim to have yet gotten my >> head around RISC-V memory ordering. ;-) >> > > Me neither. Now I remember this: we have a off-list(accidentally) > discussion about this, and IIRC at that moment riscv people confirmed > that riscv's unlock-lock pair doesn't order write->write, but that was > before their memory model draft posted for discussions, so things may > change now... > > Besides, I think the smp_mb() on P2 can be relaxed to smp_rmb(), no? > > Regards, > Boqun > >> Thanx, Paul >> As a matter of fact, the RISC-V memory model committee is debating this exact question at the moment. Now that I see this thread I'll have to go back and catch up on what I've missed, but at least I can be on cc from this point on to answer any RISC-V questions that come up from here on. (Is there some place I should add my name as a RISC-V memory model contact, so I don't miss threads like this in the future?) And yes, if we go with a purely RCpc interpretation of acquire and release, then I don't believe the writes in the previous critical section would be ordered with the writes in the subsequent critical section. That's really all the argument boils down to. We'd like to support RCpc if we can since that's all some software needs, but we also obviously want to make sure we can support RCsc and these kinds of LKMM subtleties efficiently too when needed. So we have a few encoding details that we're still finalizing, because questions like this one are still popping up :) Let me know if you had other RISC-V-specific questions I can help answer. Dan
Re: [PATCH 4/4] fs/dcache: Avoid the try_lock loops in dentry_kill()
On Fri, Feb 16, 2018 at 10:03:15AM -0800, Linus Torvalds wrote: > On Fri, Feb 16, 2018 at 7:09 AM, John Ogness > wrote: > > dentry_kill() holds dentry->d_lock and needs to acquire both > > dentry->d_inode->i_lock and dentry->d_parent->d_lock. This cannot be > > done with spin_lock() operations because it's the reverse of the > > regular lock order. To avoid ABBA deadlocks it is done with two > > trylock loops. > > > > Trylock loops are problematic in two scenarios: > > I don't mind this patch series per se (although I would really like Al > to ack it), but this particular patch I hate. I'm not happy about the previous patch either. Why do we need the users of that thing to deal with retries? And I'm not even sure we want to bother with retries on inode change inside dentry_kill() itself - just unlock, return dentry and let the caller handle that. Callers *must* handle "need to drop another one" anyway, so...
RE: [PATCH] efivarfs: Limit the rate for non-root to read files
> - just make it return -EAGAIN instead of sleeping (which probably > just works fine and doesn't break anything and is simple) It is very simple. But it does break things :-(. If I read one of these files using "dd bs=1", that used to read the whole file (while generating lots of SMI). With the -EAGAIN it just reads 100 bytes and says: dd: error reading 'DefSetup-e8a99903-302c-4851-a6be-ab2731873b2f': Resource temporarily unavailable 100+0 records in 100+0 records out 100 bytes copied, 0.153943 s, 0.6 kB/s > - add a per-user mutex, and do the usleep inside of it, so that > anybody who tries to do a thousand threads will just be serialized by > the mutex. > > Note that the mutex needs to be per-user, because otherwise it will be > a DoS for the other users. I can try that tomorrow (adding the per-user mutex to struct user_struct right next to the ratelimit I added. -Tony
Re: [PATCH] ocxl: Add get_metadata IOCTL to share OCXL information to userspace
On Thu, Feb 22, 2018 at 2:51 PM, Alastair D'Silva wrote: > On Thu, 2018-02-22 at 14:46 +1100, Balbir Singh wrote: > >> lpc_size could be added. It's currently useless to the library, but >> > doesn't >> > hurt. The one which was giving me troubles on a previous version of >> > this >> > patch was the lpc numa node ID, since that was experimental code >> > and felt >> > out of place considering what's been upstreamed in skiboot and >> > linux so far. >> > >> >> Yeah, I think metadata will evolve for a while till it settle's down. >> Since ocxl_ioctl_get_metadata is exposed via uapi, a newer program >> calling an older kernel will never work, since the size of that >> struct >> will always be larger than what the OS supports and our >> copy_to_user() >> will fail. The other option is for the user program to try all >> possible versions till one succeeds, that is bad as well. I think >> there are a few ways around it, if we care about this combination. >> >> Balbir Singh. >> > > We have a number of reserved members at the end of the struct which can > be re-purposed for future information (with a corresponding bump of the > version number). Good point, agreed Balbir Singh.
Re: [PATCH 4/4] fs/dcache: Avoid the try_lock loops in dentry_kill()
On Tue, Feb 20, 2018 at 12:34:57AM +0100, John Ogness wrote: > Implementation 3: The same as implementation 2 but using if's to > support branch prediction. This approach is probably the most > complicated to understand but will be the fastest. > > /* >* Lock the inode. Might drop dentry->d_lock temporarily >* which allows inode to change. Start over if that happens. >*/ > int ret = dentry_lock_inode(dentry); > if (unlikely(ret != LOCK_FAST)) { > if (ret == LOCK_FAILED) > goto again; > /* > * Recheck refcount as it might have been > * incremented while d_lock was dropped. > */ > if (dentry->d_lockref.count != 1) > goto drop_ref; > } Implementation 4: screw the tristate, move the loop inside dentry_lock_inode(). > If lock_parent() returns a non-NULL, it is returning > dentry->d_parent. So the return value is really just a boolean and the > locked parent is the parent of the dentry. The function is a little bit > tricky because it could return NULL (lock failed) even if the dentry has > a non-NULL d_parent. So any caller using a tristate return variation of > lock_parent() must rely on the return value instead of a non-NULL > dentry->d_parent. dentry always has non-NULL ->d_parent; it might point to dentry itself, but it's never NULL. > if (!dentry->d_lockref.count) { > - struct dentry *parent = lock_parent(dentry); > + int ret = lock_parent(dentry); > + parent = dentry->d_parent; > if (likely(!dentry->d_lockref.count)) { > __dentry_kill(dentry); > dput(parent); Broken. In IS_ROOT case you'll hit an extra dput() on dentry itself. dput(NULL) is no-op; this, OTOH, isn't.