[GIT PULL] MMC fixes for v.4.14-rc4
Hi Linus, Here's a PR with a couple of MMC fixes intended for v4.14-rc4. Details about the highlights are as usual found in the signed tag. Please pull this in! Kind regards Ulf Hansson The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff: Linux 4.14-rc3 (2017-10-01 14:54:54 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/ulfh/mmc.git tags/mmc-v4.14-rc3 for you to fetch changes up to bb16ea1742c8f35a9349b7508dc45d3a922db5f5: mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock (2017-10-04 10:50:36 +0200) MMC core: - Fix driver strength selection when selecting hs400es - Delete bounce buffer handling: This change fixes a problem related to how bounce buffers are being allocated. However, instead of trying to fix that, let's just remove the mmc bounce buffer code altogether, as it has practically no use. MMC host: - meson-gx: A couple of fixes related to clock/phase/tuning - sdhci-xenon: Fix clock resource by adding an optional bus clock Chanho Min (1): mmc: core: add driver strength selection when selecting hs400es Gregory CLEMENT (1): mmc: sdhci-xenon: Fix clock resource by adding an optional bus clock Jerome Brunet (3): mmc: meson-gx: make sure the clock is rounded down mmc: meson-gx: fix rx phase reset mmc: meson-gx: include tx phase in the tuning process Linus Walleij (1): mmc: Delete bounce buffer handling .../bindings/mmc/marvell,xenon-sdhci.txt | 12 +- drivers/mmc/core/block.c | 3 - drivers/mmc/core/mmc.c | 36 +++--- drivers/mmc/core/queue.c | 125 ++--- drivers/mmc/core/queue.h | 6 - drivers/mmc/host/cavium.c | 2 +- drivers/mmc/host/meson-gx-mmc.c| 26 - drivers/mmc/host/pxamci.c | 6 +- drivers/mmc/host/sdhci-xenon.c | 24 +++- drivers/mmc/host/sdhci-xenon.h | 1 + include/linux/mmc/host.h | 2 +- 11 files changed, 81 insertions(+), 162 deletions(-)
Re: [PATCH v5 13/14] platform/x86: wmi: create character devices when requested by drivers
On Fri, Oct 06, 2017 at 11:59:57PM -0500, Mario Limonciello wrote: > For WMI operations that are only Set or Query read or write sysfs > attributes created by WMI vendor drivers make sense. > > For other WMI operations that are run on Method, there needs to be a > way to guarantee to userspace that the results from the method call > belong to the data request to the method call. Sysfs attributes don't > work well in this scenario because two userspace processes may be > competing at reading/writing an attribute and step on each other's > data. > > When a WMI vendor driver declares an ioctl callback in the wmi_driver > the WMI bus driver will create a character device that maps to that > function. > > That character device will correspond to this path: > /dev/wmi/$driver > > The WMI bus driver will interpret the IOCTL calls, test them for > a valid instance and pass them on to the vendor driver to run. > > This creates an implicit policy that only driver per character > device. If a module matches multiple GUID's, the wmi_devices > will need to be all handled by the same wmi_driver if the same > character device is used. > > The WMI vendor drivers will be responsible for managing access to > this character device and proper locking on it. > > When a WMI vendor driver is unloaded the WMI bus driver will clean > up the character device. What prevents the vendor driver from being unloaded while the ioctl is being called in it? I don't see any protection here from that at all :( > +static long wmi_unlocked_ioctl(struct file *filp, unsigned int cmd, > +unsigned long arg) > +{ > + return match_ioctl(filp, cmd, arg, 0); > +} > + > +static long wmi_compat_ioctl(struct file *filp, unsigned int cmd, > + unsigned long arg) > +{ > + return match_ioctl(filp, cmd, arg, 1); > +} Why a compat ioctl at all? That's for older interfaces, not for brand new ones where you design the ioctl structures "correctly" to work on both 32 and 64 bits at the same time. That should not be needed at all. thanks, greg k-h
Re: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce userspace interface
On Fri, Oct 06, 2017 at 11:59:58PM -0500, Mario Limonciello wrote: > It's important for the driver to provide a R/W ioctl to ensure that > two competing userspace processes don't race to provide or read each > others data. > > This userspace character device will be used to perform SMBIOS calls > from any applications. > > It provides an ioctl that will allow passing the WMI calling > interface buffer between userspace and kernel space. > > This character device is intended to deprecate the dcdbas kernel module > and the interface that it provides to userspace. > > To use the character device the buffer needed for the machine will > also be needed. This information is exported to a sysfs attribute. > > The API for interacting with this interface is defined in documentation > as well as a uapi header provides the format of the structures. > > Signed-off-by: Mario Limonciello > --- > Documentation/ABI/testing/dell-smbios-wmi | 41 > .../ABI/testing/sysfs-platform-dell-smbios-wmi | 10 ++ > MAINTAINERS| 1 + > drivers/platform/x86/dell-smbios-wmi.c | 104 > ++--- > drivers/platform/x86/dell-smbios.h | 11 +-- > include/uapi/linux/dell-smbios.h | 42 + > 6 files changed, 188 insertions(+), 21 deletions(-) > create mode 100644 Documentation/ABI/testing/dell-smbios-wmi > create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > create mode 100644 include/uapi/linux/dell-smbios.h > > diff --git a/Documentation/ABI/testing/dell-smbios-wmi > b/Documentation/ABI/testing/dell-smbios-wmi > new file mode 100644 > index ..e067e955fcc9 > --- /dev/null > +++ b/Documentation/ABI/testing/dell-smbios-wmi > @@ -0,0 +1,41 @@ > +What:/dev/wmi/dell-smbios > +Date:November 2017 > +KernelVersion: 4.15 > +Contact: "Mario Limonciello" > +Description: > + Perform SMBIOS calls on supported Dell machines. > + through the Dell ACPI-WMI interface. > + > + IOCTL's and buffer formats are defined in: > + > + > + 1) To perform a call from userspace, you'll need to first > + determine the minimum size of the calling interface buffer > + for your machine. > + Platforms that contain larger buffers can return larger > + objects from the system firmware. > + Commonly this size is either 4k or 32k. > + > + To determine the size of the buffer, refer to: > + sysfs-platform-dell-smbios-wmi > + > + 2) After you've determined the minimum size of the calling > + interface buffer, you can allocate a structure that represents > + the structure documented above. > + > + 3) In the 'length' object store the size of the buffer you > + determined above and allocated. > + > + 4) In this buffer object, prepare as necessary for the SMBIOS > + call you're interested in. Typically SMBIOS buffers have > + "class", "select", and "input" defined to values that coincide > + with the data you are interested in. > + Documenting class/select/input values is outside of the scope > + of this documentation. Check with the libsmbios project for > + further documentation on these values. > + > + 6) Run the call by using ioctl() as described in the header. > + > + 7) The output will be returned in the buffer object. > + > + 8) Be sure to free up your allocated object. > diff --git a/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > b/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > new file mode 100644 > index ..6a0513703a3c > --- /dev/null > +++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > @@ -0,0 +1,10 @@ > +What:/sys/devices/platform//buffer_size > +Date:November 2017 > +KernelVersion: 4.15 > +Contact: "Mario Limonciello" > +Description: > + A read-only description of the size of a calling > + interface buffer that can be passed to Dell > + firmware. > + > + Commonly this size is either 4k or 32k. > diff --git a/MAINTAINERS b/MAINTAINERS > index 2a99ee9fd883..4940f3c7481b 100644 > --- a/MAINTAINERS > +++ b/MAINTAINERS > @@ -3986,6 +3986,7 @@ M: Mario Limonciello > L: platform-driver-...@vger.kernel.org > S: Maintained > F: drivers/platform/x86/dell-smbios-wmi.c > +F: include/uapi/linux/dell-smbios.h > > DELL LAPTOP DRIVER > M: Matthew Garrett > diff --git a/drivers/platform/x86/dell-smbios-wmi.c > b/drivers/platform/x86/dell-smbios-wmi.c > index 3de8abea38f8..2b78aba68755 100644 > --- a/drivers/platform/x86/dell-smbios-wmi.c > +++ b/drivers/platform/x86/dell-smbios-wmi.c
Re: [PATCH v5 10/14] platform/x86: dell-smbios: add filtering capability for requests
On Fri, Oct 06, 2017 at 11:59:54PM -0500, Mario Limonciello wrote: > There are some categories of tokens and SMBIOS calls that it makes > sense to protect userspace from accessing. These are calls that > may write to one time use fields or activate hardware debugging > capabilities. They are not intended for general purpose use. > > This same functionality may be be later extended to also intercept > calls that may cause kernel functionality to get out of sync if > the same functions are used by other drivers. > > Signed-off-by: Mario Limonciello > --- > drivers/platform/x86/dell-smbios.c | 76 > ++ > drivers/platform/x86/dell-smbios.h | 2 + > 2 files changed, 78 insertions(+) > > diff --git a/drivers/platform/x86/dell-smbios.c > b/drivers/platform/x86/dell-smbios.c > index 2f90ba5346bc..d1908f159be3 100644 > --- a/drivers/platform/x86/dell-smbios.c > +++ b/drivers/platform/x86/dell-smbios.c > @@ -32,6 +32,7 @@ struct calling_interface_structure { > struct calling_interface_token tokens[]; > } __packed; > > +static u32 da_supported_commands; > static int da_command_address; > static int da_command_code; > static int da_num_tokens; > @@ -45,6 +46,14 @@ struct smbios_device { > int (*call_fn)(struct calling_interface_buffer *); > }; > > +static u32 token_black[] = { > + 0x0175, 0x0176, 0x0195, 0x0196, 0x0197, 0x01DC, 0x01DD, 0x027D, 0x027E, > + 0x027F, 0x0280, 0x0281, 0x0282, 0x0283, 0x0284, 0x02E3, 0x02FF, 0x0300, > + 0x0301, 0x0302, 0x0325, 0x0326, 0x0332, 0x0333, 0x0334, 0x0335, 0x0350, > + 0x0363, 0x0368, 0x03F6, 0x03F7, 0x049E, 0x049F, 0x04A0, 0x04A1, 0x04A2, > + 0x04A3, 0x04E6, 0x04E7, 0x9000, 0x9001 > +}; Any hint as to what these values represent? > static LIST_HEAD(smbios_device_list); > > void dell_smbios_get_smm_address(int *address, int *code) > @@ -104,6 +113,65 @@ void dell_smbios_unregister_device(struct device *d) > } > EXPORT_SYMBOL_GPL(dell_smbios_unregister_device); > > +int dell_smbios_call_filter(struct device *d, > + struct calling_interface_buffer *buffer) > +{ > + int i; > + int j; > + u32 t; > + > + /* can't make calls over 30 */ > + if (buffer->class > 30) { > + dev_dbg(d, "buffer->class too big: %d\n", buffer->class); > + return -EINVAL; > + } > + > + /* supported calls on the particular system */ > + if (!(da_supported_commands & (1 << buffer->class))) { > + dev_dbg(d, "invalid command, supported commands: 0x%8x\n", > + da_supported_commands); > + return -EINVAL; > + } > + > + /* diagonstics, debugging information or write once */ > + if ((buffer->class == 01 && buffer->select == 07) || > + (buffer->class == 06 && buffer->select == 05) || > + (buffer->class == 11 && buffer->select == 03) || > + (buffer->class == 11 && buffer->select == 07) || > + (buffer->class == 11 && buffer->select == 11) || > + buffer->class == 19) { A structure of class/select that is not allowed might be easier to maintain over time, right? > + dev_dbg(d, "blacklisted command: %d/%d\n", > + buffer->class, buffer->select); > + return -EINVAL; > + } > + > + /* reading/writing tokens*/ > + if ((buffer->class == 0 && buffer->select < 3) || > + (buffer->class == 1 && buffer->select < 3)) { > + for (i = 0; i < da_num_tokens; i++) { > + if (da_tokens[i].location != buffer->input[0]) > + continue; > + /*blacklist reading and writing these */ "/* " ??? thanks, greg k-h
Re: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce userspace interface
On Sat, Oct 07, 2017 at 09:41:32AM +0200, Greg KH wrote: > And I still didn't see where you were verifying the list of commands in > this structure, was that in some other patch? Oh nevermind, that was a different patch, found it...
Re: [Patch v6 3/7] slimbus: qcom: Add Qualcomm Slimbus controller driver
Hi, On Fri, Oct 06, 2017 at 05:51:32PM +0200, srinivas.kandaga...@linaro.org wrote: > From: Sagar Dharia > > This controller driver programs manager, interface, and framer > devices for Qualcomm's slimbus HW block. > Manager component currently implements logical address setting, > and messaging interface. > Interface device reports bus synchronization information, and framer > device clocks the bus from the time it's woken up, until clock-pause > is executed by the manager device. > > Signed-off-by: Sagar Dharia > Signed-off-by: Srinivas Kandagatla > --- > .../devicetree/bindings/slimbus/slim-qcom-ctrl.txt | 43 ++ > drivers/slimbus/Kconfig| 9 + > drivers/slimbus/Makefile | 3 + > drivers/slimbus/slim-qcom-ctrl.c | 594 > + > drivers/slimbus/slim-qcom.h| 63 +++ > 5 files changed, 712 insertions(+) > create mode 100644 > Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt > create mode 100644 drivers/slimbus/slim-qcom-ctrl.c > create mode 100644 drivers/slimbus/slim-qcom.h > > diff --git a/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt > b/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt > new file mode 100644 > index 000..081110d > --- /dev/null > +++ b/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt > @@ -0,0 +1,43 @@ > +Qualcomm SLIMBUS controller > +This controller is used if applications processor driver controls slimbus > +master component. > + > +Required properties: > + > + - #address-cells - refer to > Documentation/devicetree/bindings/slimbus/bus.txt > + - #size-cells - refer to > Documentation/devicetree/bindings/slimbus/bus.txt > + > + - reg : Offset and length of the register region(s) for the device > + - reg-names : Register region name(s) referenced in reg above > + Required register resource entries are: > + "ctrl": Physical adderess of controller register blocks s/adderess/address/ > + - compatible : should be "qcom,-slim" for SOC specific compatible > or > + "qcom,slim" if using generic qcom SLIM IP. > + - interrupts : Interrupt number used by this controller > + - clocks : Interface and core clocks used by this slimbus controller > + - clock-names : Required clock-name entries are: > + "iface_clk" : Interface clock for this controller > + "core_clk" : Interrupt for controller core's BAM [...] > +static irqreturn_t msm_slim_interrupt(int irq, void *d) > +{ > + struct msm_slim_ctrl *dev = d; > + u32 stat = readl_relaxed(dev->base + MGR_INT_STAT); > + int err = 0, ret = IRQ_NONE; > + > + if (stat & MGR_INT_TX_MSG_SENT || stat & MGR_INT_TX_NACKED_2) { > + if (stat & MGR_INT_TX_MSG_SENT) > + writel_relaxed(MGR_INT_TX_MSG_SENT, > +dev->base + MGR_INT_CLR); > + if (stat & MGR_INT_TX_NACKED_2) { > + u32 mgr_stat = readl_relaxed(dev->base + MGR_STATUS); > + u32 mgr_ie_stat = readl_relaxed(dev->base + > + MGR_IE_STAT); > + u32 frm_stat = readl_relaxed(dev->base + FRM_STAT); > + u32 frm_cfg = readl_relaxed(dev->base + FRM_CFG); > + u32 frm_intr_stat = readl_relaxed(dev->base + > + FRM_INT_STAT); > + u32 frm_ie_stat = readl_relaxed(dev->base + > + FRM_IE_STAT); > + u32 intf_stat = readl_relaxed(dev->base + INTF_STAT); > + u32 intf_intr_stat = readl_relaxed(dev->base + > +INTF_INT_STAT); > + u32 intf_ie_stat = readl_relaxed(dev->base + > + INTF_IE_STAT); > + > + writel_relaxed(MGR_INT_TX_NACKED_2, dev->base + > +MGR_INT_CLR); > + dev_err(dev->dev, "TX Nack MGR:int:0x%x, stat:0x%x\n", > + stat, mgr_stat); > + dev_err(dev->dev, "TX Nack MGR:ie:0x%x\n", mgr_ie_stat); > + dev_err(dev->dev, "TX Nack FRM:int:0x%x, stat:0x%x\n", > + frm_intr_stat, frm_stat); > + dev_err(dev->dev, "TX Nack FRM:cfg:0x%x, ie:0x%x\n", > + frm_cfg, frm_ie_stat); > + dev_err(dev->dev, "TX Nack INTF:intr:0x%x, stat:0x%x\n", > + intf_intr_stat, intf_stat); > + dev_err(dev->dev, "TX Nack INTF:ie:0x%x\n", > + intf_ie_stat); > + err = -ENOTCONN; > + } > + /** This isn't really a kerneldoc comment… > + * Gu
Re: [PATCH] nvmem: imx-ocotp: read uniq CPU ID and export as system serial number
Hi Marcus, [auto build test ERROR on linus/master] [also build test ERROR on v4.14-rc3 next-20170929] [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/Marcus-Folkesson/nvmem-imx-ocotp-read-uniq-CPU-ID-and-export-as-system-serial-number/20171007-122234 config: i386-allmodconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): >> drivers/nvmem/imx-ocotp.c:31:29: fatal error: asm/system_info.h: No such >> file or directory #include ^ compilation terminated. vim +31 drivers/nvmem/imx-ocotp.c > 31 #include 32 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH v8 01/20] crypto: change transient busy return code to -EAGAIN
On Sat, Oct 7, 2017 at 6:05 AM, Herbert Xu wrote: > On Tue, Sep 05, 2017 at 03:38:40PM +0300, Gilad Ben-Yossef wrote: >> >> diff --git a/crypto/algif_hash.c b/crypto/algif_hash.c >> index 5e92bd2..3b3c154 100644 >> --- a/crypto/algif_hash.c >> +++ b/crypto/algif_hash.c >> @@ -39,6 +39,20 @@ struct algif_hash_tfm { >> bool has_key; >> }; >> >> +/* Previous versions of crypto_* ops used to return -EBUSY >> + * rather than -EAGAIN to indicate being tied up. The in >> + * kernel API changed but we don't want to break the user >> + * space API. As only the hash user interface exposed this >> + * error ever to the user, do the translation here. >> + */ >> +static inline int crypto_user_err(int err) >> +{ >> + if (err == -EAGAIN) >> + return -EBUSY; >> + >> + return err; > > I don't see the need to carry along this baggage. Does anyone > in user-space actually rely on EBUSY? I am not aware of anyone who does. I was just trying to avoid changing the user ABI. Shall I roll a new revision without this patch? Thanks, Gilad > > Cheers, > -- > Email: Herbert Xu > Home Page: http://gondor.apana.org.au/~herbert/ > PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- Gilad Ben-Yossef Chief Coffee Drinker "If you take a class in large-scale robotics, can you end up in a situation where the homework eats your dog?" -- Jean-Baptiste Queru
Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
On Sat 07-10-17 13:05:24, Tetsuo Handa wrote: > Johannes Weiner wrote: > > On Sat, Oct 07, 2017 at 11:21:26AM +0900, Tetsuo Handa wrote: > > > On 2017/10/05 19:36, Tetsuo Handa wrote: > > > > I don't want this patch backported. If you want to backport, > > > > "s/fatal_signal_pending/tsk_is_oom_victim/" is the safer way. > > > > > > If you backport this patch, you will see "complete depletion of memory > > > reserves" > > > and "extra OOM kills due to depletion of memory reserves" using below > > > reproducer. > > > > > > -- > > > #include > > > #include > > > #include > > > > > > static char *buffer; > > > > > > static int __init test_init(void) > > > { > > > set_current_oom_origin(); > > > buffer = vmalloc((1UL << 32) - 480 * 1048576); > > > > That's not a reproducer, that's a kernel module. It's not hard to > > crash the kernel from within the kernel. > > > > When did we agree that "reproducer" is "userspace program" ? > A "reproducer" is a program that triggers something intended. This way of argumentation is just ridiculous. I can construct whatever code to put kernel on knees and there is no way around it. The patch in question was supposed to mitigate a theoretical problem while it caused a real issue seen out there. That is a reason to revert the patch. Especially when a better mitigation has been put in place. You are right that replacing fatal_signal_pending by tsk_is_oom_victim would keep the original mitigation in pre-cd04ae1e2dc8 kernels but I would only agree to do that if the mitigated problem was real. And this doesn't seem to be the case. If any of the stable kernels regresses due to the revert I am willing to put a mitigation in place. > Year by year, people are spending efforts for kernel hardening. > It is silly to say that "It's not hard to crash the kernel from > within the kernel." when we can easily mitigate. This is true but we do not spread random hacks around for problems that are not real and there are better ways to address them. In this particular case cd04ae1e2dc8 was a better way to address the problem in general without spreading tsk_is_oom_victim all over the place. > Even with cd04ae1e2dc8, there is no point with triggering extra > OOM kills by needlessly consuming memory reserves. Yet again you are making unfounded claims and I am really fed up arguing discussing that any further. -- Michal Hocko SUSE Labs
Re: [Patch v6 4/7] slimbus: Add support for 'clock-pause' feature
Hi, some trivial comments below. On Fri, Oct 06, 2017 at 05:51:33PM +0200, srinivas.kandaga...@linaro.org wrote: > From: Sagar Dharia > > Per slimbus specification, a reconfiguration sequence known as > 'clock pause' needs to be broadcast over the bus while entering low- > power mode. Clock-pause is initiated by the controller driver. > To exit clock-pause, controller typically wakes up the framer device. > Since wakeup precedure is controller-specific, framework calls it via > controller's function pointer to invoke it. > > Signed-off-by: Sagar Dharia > Signed-off-by: Srinivas Kandagatla > --- [...] > @@ -429,6 +444,14 @@ void slim_return_tx(struct slim_controller *ctrl, int > err) > cur.cb(cur.ctx, err); > > up(&ctrl->tx_sem); > + if (!cur.clk_pause && (!cur.need_tid || err)) { > + /** This isn't really a kerneldoc comment. > + * remove runtime-pm vote if this was TX only, or > + * if there was error during this transaction > + */ > + pm_runtime_mark_last_busy(ctrl->dev.parent); > + pm_runtime_put_autosuspend(ctrl->dev.parent); > + } > } > EXPORT_SYMBOL_GPL(slim_return_tx); > [...] > +/** > + * slim_ctrl_clk_pause: Called by slimbus controller to enter/exit 'clock > pause' > + * Slimbus specification needs this sequence to turn-off clocks for the bus. > + * The sequence involves sending 3 broadcast messages (reconfiguration > + * sequence) to inform all devices on the bus. > + * To exit clock-pause, controller typically wakes up active framer device. > + * @ctrl: controller requesting bus to be paused or woken up > + * @wakeup: Wakeup this controller from clock pause. > + * @restart: Restart time value per spec used for clock pause. This value > + * isn't used when controller is to be woken up. > + * This API executes clock pause reconfiguration sequence if wakeup is false. > + * If wakeup is true, controller's wakeup is called. > + * For entering clock-pause, -EBUSY is returned if a message txn in pending. > + */ > +int slim_ctrl_clk_pause(struct slim_controller *ctrl, bool wakeup, u8 > restart) > +{ > + int i, ret = 0; > + unsigned long flags; > + struct slim_sched *sched = &ctrl->sched; > + struct slim_val_inf msg = {0, 0, NULL, NULL, NULL, NULL}; > + > + DEFINE_SLIM_BCAST_TXN(txn, SLIM_MSG_MC_BEGIN_RECONFIGURATION, > + 3, SLIM_LA_MANAGER, &msg); > + > + if (wakeup == false && restart > SLIM_CLK_UNSPECIFIED) > + return -EINVAL; > + > + mutex_lock(&sched->m_reconf); > + if (wakeup) { > + if (sched->clk_state == SLIM_CLK_ACTIVE) { > + mutex_unlock(&sched->m_reconf); > + return 0; > + } > + > + /** ditto > + * Fine-tune calculation based on clock gear, > + * message-bandwidth after bandwidth management > + */ > + ret = wait_for_completion_timeout(&sched->pause_comp, > + msecs_to_jiffies(100)); > + if (!ret) { > + mutex_unlock(&sched->m_reconf); > + pr_err("Previous clock pause did not finish"); > + return -ETIMEDOUT; > + } > + ret = 0; > + > + /** ditto > + * Slimbus framework will call controller wakeup > + * Controller should make sure that it sets active framer > + * out of clock pause > + */ > + if (sched->clk_state == SLIM_CLK_PAUSED && ctrl->wakeup) > + ret = ctrl->wakeup(ctrl); > + if (!ret) > + sched->clk_state = SLIM_CLK_ACTIVE; > + mutex_unlock(&sched->m_reconf); > + > + return ret; > + } [...] Thanks, Jonathan Neuschäfer signature.asc Description: PGP signature
Re: [PATCH] i2c: ismt: Separate I2C block read from SMBus block read
> Thank you! Is it safe to asume it will be backported to 4.4 stable > branch and upwards as the problematic commit did (once in mainline)? Yes, it has a Fixes: and a stable tag. That should do. > > Might have been good to CC the patch author of the problematic commit, > > too. > > Yes, of course! Stephen is CC now at least (kept the commit message > quoted at his convenience). So, I'll wait for his tests and then send it to Linus ASAP. I think it should be in rc5. signature.asc Description: PGP signature
Re: [Patch v6 5/7] slimbus: qcom: Add runtime-pm support using clock-pause feature
Hi, some more trivial comments below. On Fri, Oct 06, 2017 at 05:51:34PM +0200, srinivas.kandaga...@linaro.org wrote: > From: Sagar Dharia > > Slimbus HW mandates that clock-pause sequence has to be executed > before disabling relevant interface and core clocks. > Runtime-PM's autosuspend feature is used here to enter/exit low > power mode for Qualcomm's Slimbus controller. Autosuspend feature > enables driver to avoid changing power-modes too frequently since > entering clock-pause is an expensive sequence > > Signed-off-by: Sagar Dharia > Signed-off-by: Srinivas Kandagatla > --- [...] > +static int msm_clk_pause_wakeup(struct slim_controller *ctrl) > +{ > + struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl); > + > + clk_prepare_enable(dev->hclk); > + clk_prepare_enable(dev->rclk); > + enable_irq(dev->irq); > + > + writel_relaxed(1, dev->base + FRM_WAKEUP); > + /* Make sure framer wakeup write goes through before ISR fires */ > + mb(); > + /** This isn't really a kerneldoc comment. > + * HW Workaround: Currently, slave is reporting lost-sync messages > + * after slimbus comes out of clock pause. > + * Transaction with slave fail before slave reports that message > + * Give some time for that report to come > + * Slimbus wakes up in clock gear 10 at 24.576MHz. With each superframe > + * being 250 usecs, we wait for 5-10 superframes here to ensure > + * we get the message > + */ > + usleep_range(1250, 2500); > + return 0; > +} [...] > +#ifdef CONFIG_PM_SLEEP > +static int msm_slim_suspend(struct device *dev) > +{ > + int ret = 0; > + > + if (!pm_runtime_enabled(dev) || > + (!pm_runtime_suspended(dev))) { > + dev_dbg(dev, "system suspend"); > + ret = msm_slim_runtime_suspend(dev); > + } > + if (ret == -EISCONN) { > + /** ditto. Also, it looks misindented. > + * If the clock pause failed due to active channels, there is > + * a possibility that some audio stream is active during suspend. > + * (e.g. modem usecase during suspend) > + * We dont want to return suspend failure in that case so that > + * display and relevant components can still go to suspend. > + * If there is some other error, then it should prevent > + * system level suspend > + */ > + ret = 0; > + } > + return ret; > +} Thanks, Jonathan Neuschäfer signature.asc Description: PGP signature
[PULL REQUEST] i2c for 4.14
Linus, I2C has three driver fixes for the newly introduced drivers and one ID addition for the i801 driver. Please pull. Thanks, Wolfram The following changes since commit 9e66317d3c92ddaab330c125dfe9d06eee268aff: Linux 4.14-rc3 (2017-10-01 14:54:54 -0700) are available in the git repository at: git://git.kernel.org/pub/scm/linux/kernel/git/wsa/linux.git i2c/for-current-4.14 for you to fetch changes up to 25f2f440989c7079fdd8fccd54592cc077b63ae5: i2c: i2c-stm32f7: make structure stm32f7_setup static const (2017-10-05 14:44:57 +0200) Colin Ian King (1): i2c: i2c-stm32f7: make structure stm32f7_setup static const Jarkko Nikula (1): i2c: i801: Add support for Intel Cedar Fork Pierre-Yves MORDRET (1): i2c: stm32f7: fix setup structure Thomas Meyer (1): i2c: ensure termination of *_device_id tables with much appreciated quality assurance from Jean Delvare (1): (Rev.) i2c: i801: Add support for Intel Cedar Fork Documentation/i2c/busses/i2c-i801 | 1 + drivers/i2c/busses/Kconfig| 1 + drivers/i2c/busses/i2c-i801.c | 4 drivers/i2c/busses/i2c-sprd.c | 1 + drivers/i2c/busses/i2c-stm32f7.c | 17 +++-- 5 files changed, 14 insertions(+), 10 deletions(-) signature.asc Description: PGP signature
Re: [PATCH 4.9 086/104] arm64: kasan: avoid bad virt_to_pfn()
On Fri, Oct 06, 2017 at 07:13:22PM +0100, Mark Rutland wrote: > Hi Greg, > > On Fri, Oct 06, 2017 at 10:52:04AM +0200, Greg Kroah-Hartman wrote: > > 4.9-stable review patch. If anyone has any objections, please let me know. > > I'm a little confused as to why this is being backported, given it > wasn't Cc'd stable or marked as a fix. Ok, I'm now dropping it, sorry for the issues it raised. greg k-h
Re: [RFC PATCH -tip 0/5] kprobes: Abolish jprobe APIs
* Steven Rostedt wrote: > On Fri, 6 Oct 2017 13:49:59 +0900 > Masami Hiramatsu wrote: > > > Steve, could you write a documentation how to use ftrace callback? > > I think I should update the Documentation/kprobes.txt so that jprobe > > user can easily migrate on that. > > I decided to do this now. Here's a first draft. What do you think? > > -- Steve > > Using ftrace to hook to functions > = > > Copyright 2017 VMware Inc. >Author: Steven Rostedt > License: The GNU Free Documentation License, Version 1.2 >(dual licensed under the GPL v2) > > Written for: 4.14 > > Introduction > > > The ftrace infrastructure was originially created to attach hooks to the > beginning of functions in order to record and trace the flow of the kernel. > But hooks to the start of a function can have other use cases. Either > for live kernel patching, or for security monitoring. This document describes > how to use ftrace to implement your own function hooks. > > > The ftrace context > == > > WARNING: The ability to add a callback to almost any function within the > kernel comes with risks. A callback can be called from any context > (normal, softirq, irq, and NMI). Callbacks can also be called just before > going to idle, during CPU bring up and takedown, or going to user space. > This requires extra care to what can be done inside a callback. A callback > can be called outside the protective scope of RCU. > > The ftrace infrastructure has some protections agains recursions and RCU > but one must still be very careful how they use the callbacks. > > > The ftrace_ops structure > > > To register a function callback, a ftrace_ops is required. This structure > is used to tell ftrace what function should be called as the callback > as well as what protections the callback will perform and not require > ftrace to handle. So the text first starts talking about 'hooks' then uses the 'callback' terminology in the rest of th document. Could we please change it all to 'callback'? [ This is a pet peeve of mine as 'hook' gives me the cringe! ;-) ] Thanks, Ingo
Re: [PATCH 1/2] USB: serial: console: fix use-after-free on disconnect
Hi 2017-10-04 18:01 GMT+09:00 Johan Hovold : > A clean-up patch removing removing two redundant NULL-checks from the ^^ The word 'removing' was written twice. :) > console disconnect handler inadvertently also removed a third check. > This could lead to the struct usb_serial being prematurely freed by the > console code when a driver accepts but does not register any ports for > an interface which also lacks endpoint descriptors. > > Fixes: 0e517c93dc02 ("USB: serial: console: clean up sanity checks") > Cc: stable # 4.11 > Reported-by: Andrey Konovalov > Signed-off-by: Johan Hovold > --- > drivers/usb/serial/console.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/usb/serial/console.c b/drivers/usb/serial/console.c > index fdf89800ebc3..ed8ba3ef5c79 100644 > --- a/drivers/usb/serial/console.c > +++ b/drivers/usb/serial/console.c > @@ -265,7 +265,7 @@ static struct console usbcons = { > > void usb_serial_console_disconnect(struct usb_serial *serial) > { > - if (serial->port[0] == usbcons_info.port) { > + if (serial->port[0] && serial->port[0] == usbcons_info.port) { > usb_serial_console_exit(); > usb_serial_put(serial); > } > -- > 2.14.2 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-usb" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH 3.18 00/35] 3.18.74-stable review
On Fri, Oct 06, 2017 at 11:30:44AM -0600, Shuah Khan wrote: > On 10/06/2017 03:24 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 3.18.74 release. > > There are 35 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 Sun Oct 8 09:23:44 UTC 2017. > > Anything received after that time might be too late. > > > > The whole patch series can be found in one patch at: > > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.74-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-3.18.y > > and the diffstat can be found below. > > > > thanks, > > > > greg k-h > > > > Compiled and booted on my test system. No dmesg regressions. > No regressions in selftest run (non-root tests only) compared > to 3.18.73 selftests run. > > I started running selftests from Linux latest 4.14-rc3 on > stable releases. Ah, that's a great thing to do, thanks for that, and testing all of these kernels. greg k-h
Re: [PATCH 3/4] sched: WARN when migrating to an offline CPU
On Sat, Oct 07, 2017 at 02:07:26AM +, Levin, Alexander (Sasha Levin) wrote: > And quite a few lines of your added trace (lmk if you need more, or all): Yeah, could you please upload all of it somewhere? That chunk didn't include any hotplug bits at all.
Re: [PATCH tip/core/rcu 1/9] rcu: Provide GP ordering in face of migrations and delays
On Fri, Oct 06, 2017 at 08:31:05PM -0700, Paul E. McKenney wrote: > > > OK, I will bite... What do the smp_store_release() and the > > > smp_load_acquire() correspond to? I see just plain locking in > > > wait_for_completion() and complete(). > > > > They reflect the concept of complete() / wait_for_completion(). > > Fundamentally all it needs to do is pass the message of 'completion'. > > > > That is, if we were to go optimize our completion implementation, it > > would be impossible to be weaker than this and still correct. > > OK, though the model does not provide spinlocks, and there can be > differences in behavior between spinlocks and release-acquire. > But yes, in this case, it works. Sure; but the fundamental property here is that if we observe the complete() we must also observe everything that went before. The exact means of implementing that is irrelevant. > > > So I dropped that patch yesterday. The main thing I was missing was > > > that there is no ordering-free fastpath in wait_for_completion() and > > > complete(): Each unconditionally acquires the lock. So the smp_mb() > > > that I was trying to add doesn't need to be there. > > > > Going by the above, it never needs to be there, even if there was a > > lock-free fast-path. > > Given that wait_for_completion()/complete() both acquire the same lock, > yes, and agreed, if it were lockless but provided the release and > acquire ordering, then yes. I'm not sure I got the point across; so I'll try once more. Without providing this ordering the completion would be fundamentally broken. It _must_ provide this ordering. > But if it was instead structured like > wait_event()/wake_up(), there would be ordering only if the caller > supplied it. Right, wait_event()/wake_up() are different in that the 'condition' variable is external to the abstraction and thus it cannot help. All wait_event()/wake_up() can guarantee is that IFF it does a wakeup, the woken thread will observe the prior state of the waker. But given the actual condition is external and we might not hit the actual sleep case, there is no guarantees. > All that aside, paring the ordering down to the bare minimum is not > always the right approach. Why not? In what sort of cases does it go wobbly?
Re: [RFC PATCH -tip 0/5] kprobes: Abolish jprobe APIs
On Fri, 6 Oct 2017 11:34:30 -0400 Steven Rostedt wrote: > On Fri, 6 Oct 2017 13:49:59 +0900 > Masami Hiramatsu wrote: > > > Steve, could you write a documentation how to use ftrace callback? > > I think I should update the Documentation/kprobes.txt so that jprobe > > user can easily migrate on that. > > I decided to do this now. Here's a first draft. What do you think? Good! Thank you for writing this down. I just found some typo. > > -- Steve > > Using ftrace to hook to functions > = > > Copyright 2017 VMware Inc. >Author: Steven Rostedt > License: The GNU Free Documentation License, Version 1.2 >(dual licensed under the GPL v2) > > Written for: 4.14 > > Introduction > > > The ftrace infrastructure was originially created to attach hooks to the > beginning of functions in order to record and trace the flow of the kernel. > But hooks to the start of a function can have other use cases. Either > for live kernel patching, or for security monitoring. This document describes > how to use ftrace to implement your own function hooks. > > > The ftrace context > == > > WARNING: The ability to add a callback to almost any function within the > kernel comes with risks. A callback can be called from any context > (normal, softirq, irq, and NMI). Callbacks can also be called just before > going to idle, during CPU bring up and takedown, or going to user space. > This requires extra care to what can be done inside a callback. A callback > can be called outside the protective scope of RCU. > > The ftrace infrastructure has some protections agains recursions and RCU > but one must still be very careful how they use the callbacks. Q: As far as I know the ftrace handler is called under preempt-disabled, don't we need to mention that here? > > The ftrace_ops structure > > > To register a function callback, a ftrace_ops is required. This structure > is used to tell ftrace what function should be called as the callback > as well as what protections the callback will perform and not require > ftrace to handle. > > There are only two fields that are needed to be set when registering > an ftrace_ops with ftrace. The rest should be NULL. > > struct ftrace_ops ops = { >.func = my_callback_func, >.flags = MY_FTRACE_FLAGS >.private = any_private_data_structure, > }; > > Both .flags and .private are optional. Only .func is required. > > To enable tracing call: > > register_ftrace_function(&ops); > > To disable tracing call: > > unregister_ftrace_function(@ops); Is this &ops? > > > The callback function > = > > The prototype of the callback function is as follows (as of v4.14): ^ If we put this document under Documentation/, do we still need it? (since it should be updated along with the code) > void callback_func(unsigned long ip, unsigned long parent_ip, > struct ftrace_ops *op, struct pt_regs *regs); > > @ip - This is the instruction pointer of the function that is being traced. > (where the fentry or mcount is within the function) > > @parent_ip - This is the instruction pointer of the function that called the > the function being traced (where the call of the function occurred). > > @op - This is a pointer to ftrace_ops that was used to register the callback. > This can be used to pass data to the callback via the private pointer. > > @regs - If the FTRACE_OPS_FL_SAVE_REGS or FTRACE_OPS_FL_SAVE_REGS_IF_SUPPORTED > flags are set in the ftrace_ops structure, then this will be pointing > to the pt_regs structure like it would be if an breakpoint was placed > at the start of the function where ftrace was tracing. Otherwise it > either contains garbage, or NULL. > > > The ftrace FLAGS > > > The ftrace_ops flags are all defined and documented in include/linux/ftrace.h. > Some of the flags are used for internal infrastructure of ftrace, but the > ones that users should be aware of are the following: > > (All of these are prefixed with FTRACE_OPS_FL_) > > PER_CPU - When set, the callback can be enabled or disabled per cpu with the > following functions: > > void ftrace_function_local_enable(struct ftrace_ops *ops); > void ftrace_function_local_disable(struct ftrace_ops *ops); > > These two functions must be called with preemption disabled. > > SAVE_REGS - If the callback requires reading or modifying the pt_regs > passed to the callback, then it must set this flag. Registering > a ftrace_ops with this flag set on an architecture that does not > support passing of pt_regs to the callback, will fail. > > SAVE_REGS_IF_SUPPORTED
Re: [PATCH 3.18 00/35] 3.18.74-stable review
On Fri, Oct 06, 2017 at 07:03:27AM -0700, Guenter Roeck wrote: > On 10/06/2017 02:24 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 3.18.74 release. > > There are 35 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 Sun Oct 8 09:23:44 UTC 2017. > > Anything received after that time might be too late. > > > > Build results: > total: 136 pass: 134 fail: 2 > Failed builds: > mips:allmodconfig > mips:ar7_defconfig > Qemu test results: > total: 112 pass: 112 fail: 0 > > Build failures (mips:allmodconfig, mips:ar7_defconfig): > > arch/mips/kernel/setup.c: In function 'mips_parse_crashkernel': > arch/mips/kernel/setup.c:588:2: error: implicit declaration of function > 'memory_region_available' Thanks for the testing, the mips failure should now be resolved. greg k-h
Re: [PATCH 4.9 000/104] 4.9.54-stable review
On Fri, Oct 06, 2017 at 07:08:27AM -0700, Guenter Roeck wrote: > On 10/06/2017 01:50 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.9.54 release. > > There are 104 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 Sun Oct 8 08:37:55 UTC 2017. > > Anything received after that time might be too late. > > > > Build results: > total: 145 pass: 142 fail: 3 > Failed builds: > arm64:allmodconfig > mips:allmodconfig > mips:ar7_defconfig > Qemu test results: > total: 123 pass: 123 fail: 0 > > Build failures: > > arm64:allmodconfig: > > In file included from include/linux/topology.h:32:0, > from include/linux/sched.h:42, > from linux/kasan.h:4, > from arch/arm64/mm/kasan_init.c:14: > arch/arm64/mm/kasan_init.c: In function ‘kasan_init’: > arch/arm64/mm/kasan_init.c:156:28: error: implicit declaration of function > ‘lm_alias’ > > mips: > > arch/mips/kernel/setup.c: In function 'mips_parse_crashkernel': > arch/mips/kernel/setup.c:671:2: error: implicit declaration of function > 'memory_region_available' Both of these should now be resolved, thanks. greg k-h
Re: [PATCH 4.9 000/104] 4.9.54-stable review
On Fri, Oct 06, 2017 at 10:50:38AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 4.9.54 release. > There are 104 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 Sun Oct 8 08:37:55 UTC 2017. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.54-rc1.gz Ok, that had issues, so -rc2 is now out: kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.54-rc2.gz thanks, greg k-h
Re: [PATCH v2] ALSA: seq: resize buffer for overflow
On Fri, 06 Oct 2017 19:17:27 +0200, Mark Salyzyn wrote: > > Can not replicate, issue discovered in fuzzing. Stack trace below. > No functional or performance testing done regarding the fix. > > Trap at (reformatted): > > snd_seq_oss_readq_puts(struct seq_oss_readq *q, int dev, >unsigned char *data, int len) > { > union evrec rec; > int result; > > memset(&rec, 0, sizeof(rec)); > rec.c[0] = SEQ_MIDIPUTC; > rec.c[2] = dev; > > while (len-- > 0) { > rec.c[1] = *data++; // data is RBX HERE > > 'data' pointer just passed a page boundary, so the buffer supplied > was short. Caller must have been handed an ev->type equal to > SNDDRV_SEQ_EVENT_SYSEX, which resulted in handing off > ev->data.ext.ptr[ev->data.ext.len] buffer. Intuited that the source > of the event and buffer was referenced in > snd_midi_event_encode_byte() passing a larger length than the > allocated buffer. I doubt it came from snd_midi_event_encode_byte(). Judging from the call trace below, the event originated from the OSS sequencer write, i.e. it received an OSS event packet, and it was delivered again to another OSS sequencer port back via dummy client. If so, it should have received some EV_SYSEX packet, and it was processed via snd_seq_oss_synth_sysex(), and the encoded event was delivered. Now the question is how it triggers this Oops. I couldn't find any obvious cause, but one thing I noticed is a possible race when writing to OSS sequencer concurrently. Something wrong might happen. BTW, about your patch is buggy regarding the call kmalloc() with GFP_KERNEL inside spinlock. thanks, Takashi > BUG: unable to handle kernel paging request at c98ab000 > IP: [] snd_seq_oss_readq_puts+0xd5/0x170 > sound/core/seq/oss/seq_oss_readq.c:112 > PGD 1da091067 PUD 1da092067 > Oops: [#1] PREEMPT SMP KASAN > Dumping ftrace buffer: >(ftrace buffer empty) > Modules linked in: > CPU: 1 PID: 3264 Comm: > Hardware name: XX > task: 8801cdd9e000 task.stack: 8801ce648000 > RIP: 0010:[] [] > snd_seq_oss_readq_puts+0xd5/0x170 sound/core/seq/oss/seq_oss_readq.c:112 > RSP: 0018:8801ce64f1c0 EFLAGS: 00010246 > RAX: RBX: c98ab000 RCX: > RDX: RSI: 0001 RDI: 858e5780 > RBP: 8801ce64f260 R08: R09: > R10: R11: 110039cc9df2 R12: 3fa4 > R13: dc00 R14: 8801ce64f238 R15: c98ab001 > FS: 7fe3d3d9e700() GS:8801db30() knlGS: > CS: 0010 DS: ES: CR0: 80050033 > CR2: c98ab000 CR3: 0001d19b7000 CR4: 001406e0 > DR0: DR1: DR2: > DR3: DR6: fffe0ff0 DR7: 0400 > Stack: > 110039cc9e3b 8801ce64f1f8 8801d0b9aa00 41b58ab3 > 841daf3c 82e2fb30 0286 0005 > 838aa5d5 861962c0 dc00 8801ce64f260 > Call Trace: > [] send_midi_event sound/core/seq/oss/seq_oss_midi.c:616 > [inline] > [] snd_seq_oss_midi_input+0x8ce/0xa70 > sound/core/seq/oss/seq_oss_midi.c:535 > [] snd_seq_oss_event_input+0x15d/0x220 > sound/core/seq/oss/seq_oss_event.c:439 > [] snd_seq_deliver_single_event.constprop.11+0x310/0x7c0 > sound/core/seq/seq_clientmgr.c:621 > [] deliver_to_subscribers > sound/core/seq/seq_clientmgr.c:676 [inline] > [] snd_seq_deliver_event+0x316/0x740 > sound/core/seq/seq_clientmgr.c:807 > [] snd_seq_kernel_client_dispatch+0x11e/0x150 > sound/core/seq/seq_clientmgr.c:2314 > [] dummy_input+0x235/0x320 sound/core/seq/seq_dummy.c:104 > [] snd_seq_deliver_single_event.constprop.11+0x310/0x7c0 > sound/core/seq/seq_clientmgr.c:621 > [] snd_seq_deliver_event+0x12d/0x740 > sound/core/seq/seq_clientmgr.c:818 > [] snd_seq_dispatch_event+0x11d/0x520 > sound/core/seq/seq_clientmgr.c:892 > [] snd_seq_check_queue.part.3+0x38e/0x510 > sound/core/seq/seq_queue.c:285 > [] snd_seq_check_queue sound/core/seq/seq_queue.c:357 > [inline] > [] snd_seq_enqueue_event+0x32d/0x3d0 > sound/core/seq/seq_queue.c:363 > [] snd_seq_client_enqueue_event+0x204/0x3e0 > sound/core/seq/seq_clientmgr.c:951 > [] kernel_client_enqueue.part.10+0xb5/0xd0 > sound/core/seq/seq_clientmgr.c:2251 > [] kernel_client_enqueue > sound/core/seq/seq_clientmgr.c:2241 [inline] > [] snd_seq_kernel_client_enqueue_blocking+0xcf/0x110 > sound/core/seq/seq_clientmgr.c:2279 > [] insert_queue sound/core/seq/oss/seq_oss_rw.c:189 > [inline] > [] snd_seq_oss_write+0x538/0x850 > sound/core/seq/oss/seq_oss_rw.c:148 > [] odev_write+0x64/0x90 sound/core/seq/oss/seq_oss.c:177 > [] __vfs_write+0x103/0x680 fs/read_write.c:510 > [] vfs_write+0x170/0x4e0 fs/read_write.c:560 > [] SYSC_write fs/read_write.c:607 [inline] > [] SyS_write+0xd9/0x1b0 fs/read_write.c:599 > [] entry_SYSCALL_64_fastpath
Re: [PATCH 3.18 00/35] 3.18.74-stable review
On Fri, Oct 06, 2017 at 11:24:40AM +0200, Greg Kroah-Hartman wrote: > This is the start of the stable review cycle for the 3.18.74 release. > There are 35 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 Sun Oct 8 09:23:44 UTC 2017. > Anything received after that time might be too late. > > The whole patch series can be found in one patch at: > kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.74-rc1.gz As this also had build issues, -rc2 is now out: kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.74-rc2.gz thanks, greg k-h
[PATCH] fs/afs/flock and fs/locks: Fix possible sleep-in-atomic bugs in posix_lock_file
The kernel may sleep under a spinlock, and the function call paths are: afs_do_unlk (acquire the spinlock) posix_lock_file posix_lock_inode (fs/locks.c) locks_get_lock_context kmem_cache_alloc(GFP_KERNEL) --> may sleep afs_do_setlk (acquire the spinlock) posix_lock_file posix_lock_inode (fs/locks.c) locks_get_lock_context kmem_cache_alloc(GFP_KERNEL) --> may sleep To fix them, GFP_KERNEL is replaced with GFP_ATOMIC. These bugs are found by my static analysis tool and my code review. Signed-off-by: Jia-Ju Bai --- fs/locks.c |2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fs/locks.c b/fs/locks.c index 1bd71c4..975cc62 100644 --- a/fs/locks.c +++ b/fs/locks.c @@ -222,7 +222,7 @@ struct file_lock_list_struct { if (likely(ctx) || type == F_UNLCK) goto out; - ctx = kmem_cache_alloc(flctx_cache, GFP_KERNEL); + ctx = kmem_cache_alloc(flctx_cache, GFP_ATOMIC); if (!ctx) goto out; -- 1.7.9.5
Re: [PATCH 1/2] Revert "vmalloc: back off when the current task is killed"
Michal Hocko wrote: > On Sat 07-10-17 13:05:24, Tetsuo Handa wrote: > > Johannes Weiner wrote: > > > On Sat, Oct 07, 2017 at 11:21:26AM +0900, Tetsuo Handa wrote: > > > > On 2017/10/05 19:36, Tetsuo Handa wrote: > > > > > I don't want this patch backported. If you want to backport, > > > > > "s/fatal_signal_pending/tsk_is_oom_victim/" is the safer way. > > > > > > > > If you backport this patch, you will see "complete depletion of memory > > > > reserves" > > > > and "extra OOM kills due to depletion of memory reserves" using below > > > > reproducer. > > > > > > > > -- > > > > #include > > > > #include > > > > #include > > > > > > > > static char *buffer; > > > > > > > > static int __init test_init(void) > > > > { > > > > set_current_oom_origin(); > > > > buffer = vmalloc((1UL << 32) - 480 * 1048576); > > > > > > That's not a reproducer, that's a kernel module. It's not hard to > > > crash the kernel from within the kernel. > > > > > > > When did we agree that "reproducer" is "userspace program" ? > > A "reproducer" is a program that triggers something intended. > > This way of argumentation is just ridiculous. I can construct whatever > code to put kernel on knees and there is no way around it. But you don't distinguish between kernel module and userspace program. What you distinguish is "real" and "theoretical". And, more you reject with "ridiculous"/"theoretical", more I resist stronger. > > The patch in question was supposed to mitigate a theoretical problem > while it caused a real issue seen out there. That is a reason to > revert the patch. Especially when a better mitigation has been put > in place. You are right that replacing fatal_signal_pending by > tsk_is_oom_victim would keep the original mitigation in pre-cd04ae1e2dc8 > kernels but I would only agree to do that if the mitigated problem was > real. And this doesn't seem to be the case. If any of the stable kernels > regresses due to the revert I am willing to put a mitigation in place. The real issue here is that caller of vmalloc() was not ready to handle allocation failure. We addressed kmem_zalloc_greedy() case ( https://marc.info/?l=linux-mm&m=148844910724880 ) by 08b005f1333154ae rather than reverting fatal_signal_pending(). Removing fatal_signal_pending() in order to hide real issues is a random hack. > > > Year by year, people are spending efforts for kernel hardening. > > It is silly to say that "It's not hard to crash the kernel from > > within the kernel." when we can easily mitigate. > > This is true but we do not spread random hacks around for problems that > are not real and there are better ways to address them. In this > particular case cd04ae1e2dc8 was a better way to address the problem in > general without spreading tsk_is_oom_victim all over the place. Using tsk_is_oom_victim() is reasonable for vmalloc() because it is a memory allocation function which belongs to memory management subsystem. > > > Even with cd04ae1e2dc8, there is no point with triggering extra > > OOM kills by needlessly consuming memory reserves. > > Yet again you are making unfounded claims and I am really fed up > arguing discussing that any further. Kernel hardening changes are mostly addressing "theoretical" issues but we don't call them "ridiculous".
Re: [PATCH] irqchip/gicv3-its: Add missing changes to support 52bit physical address
On Sat, Oct 07 2017 at 12:13:24 am BST, Shanker Donthineni wrote: > The current ITS driver works fine as long as normal memory and GICR > regions are located within the lower 48bit (>=0 && <2^48) physical > address space. Some of the registers GICR_PEND/PROP, GICR_VPEND/VPROP > and GITS_CBASER are handled properly but not all when configuring > the hardware with 52bit physical address. > > This patch does the following changes to support 52bit PA. > -Handle 52bit PA in GITS_BASERn. > -Fix ITT_addr width to 52bits, bits[51:8]. > -Fix RDbase width to 52bits, bits[51:16]. > -Fix VPT_addr width to 52bits, bits[51:16]. > > Definition of the GITS_BASERn register when ITS PageSize is 64KB: > -Bits[47:16] of the register provide bits[47:16] of the table PA. > -Bits[15:12] of the register provide bits[51:48] of the table PA. > -Bits[15:00] of the base physical address are 0. > > Signed-off-by: Shanker Donthineni > --- > drivers/irqchip/irq-gic-v3-its.c | 24 +++- > include/linux/irqchip/arm-gic-v3.h | 3 +++ > 2 files changed, 22 insertions(+), 5 deletions(-) > > diff --git a/drivers/irqchip/irq-gic-v3-its.c > b/drivers/irqchip/irq-gic-v3-its.c > index e8d8934..e52c0da 100644 > --- a/drivers/irqchip/irq-gic-v3-its.c > +++ b/drivers/irqchip/irq-gic-v3-its.c > @@ -308,7 +308,7 @@ static void its_encode_size(struct its_cmd_block *cmd, u8 > size) > > static void its_encode_itt(struct its_cmd_block *cmd, u64 itt_addr) > { > - its_mask_encode(&cmd->raw_cmd[2], itt_addr >> 8, 50, 8); > + its_mask_encode(&cmd->raw_cmd[2], itt_addr >> 8, 51, 8); > } > > static void its_encode_valid(struct its_cmd_block *cmd, int valid) > @@ -318,7 +318,7 @@ static void its_encode_valid(struct its_cmd_block *cmd, > int valid) > > static void its_encode_target(struct its_cmd_block *cmd, u64 target_addr) > { > - its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 50, 16); > + its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16); > } > > static void its_encode_collection(struct its_cmd_block *cmd, u16 col) > @@ -358,7 +358,7 @@ static void its_encode_its_list(struct its_cmd_block > *cmd, u16 its_list) > > static void its_encode_vpt_addr(struct its_cmd_block *cmd, u64 vpt_pa) > { > - its_mask_encode(&cmd->raw_cmd[3], vpt_pa >> 16, 50, 16); > + its_mask_encode(&cmd->raw_cmd[3], vpt_pa >> 16, 51, 16); > } > > static void its_encode_vpt_size(struct its_cmd_block *cmd, u8 vpt_size) > @@ -1478,9 +1478,9 @@ static int its_setup_baser(struct its_node *its, struct > its_baser *baser, > u64 val = its_read_baser(its, baser); > u64 esz = GITS_BASER_ENTRY_SIZE(val); > u64 type = GITS_BASER_TYPE(val); > + u64 baser_phys, tmp; > u32 alloc_pages; > void *base; > - u64 tmp; > > retry_alloc_baser: > alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz); > @@ -1496,8 +1496,22 @@ static int its_setup_baser(struct its_node *its, > struct its_baser *baser, > if (!base) > return -ENOMEM; > > + baser_phys = virt_to_phys(base); > + > + /* Check if the physical address of the memory is above 48bits */ > + if (baser_phys & (~GITS_BASER_PHYS_MASK)) { > + /* 52bit PA is supported only when PageSize=64K */ > + if (psz != SZ_64K) { > + free_pages((unsigned long)base, order); > + return -EFAULT; > + } > + > + /* Convert 52bit PA to 48bit field */ > + baser_phys = GITS_BASER_64K_PHYS(baser_phys); > + } I'm not sure this is completely right. What guarantees that the memory you get is 64kB aligned? Your initial masking will do the wrong thing on a system with 16kB pages, and everything will break from that point on, as you're confusing the ITS page size with the CPU page size. This probably wants to be predicated with a 64kB CPU page size (which is the only valid configuration for 52bit PA anyway). Thanks, M. -- Jazz is not dead. It just smells funny.
Re: [PATCH 4.4 00/50] 4.4.91-stable review
On Fri, Oct 06, 2017 at 07:04:55AM -0700, Guenter Roeck wrote: > On 10/06/2017 01:52 AM, Greg Kroah-Hartman wrote: > > This is the start of the stable review cycle for the 4.4.91 release. > > There are 50 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 Sun Oct 8 08:36:32 UTC 2017. > > Anything received after that time might be too late. > > > Build results: > total: 145 pass: 145 fail: 0 > Qemu test results: > total: 116 pass: 116 fail: 0 > > Details are available at http://kerneltests.org/builders. Yeah, I got one tree right! :) thanks for letting me know. greg k-h
Re: [PATCH 3/3] mm: oom: show unreclaimable slab info when unreclaimable slabs > user memory
Hi Yang, [auto build test ERROR on mmotm/master] [also build test ERROR on v4.14-rc3 next-20170929] [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/Yang-Shi/oom-capture-unreclaimable-slab-info-in-oom-message/20171007-173639 base: git://git.cmpxchg.org/linux-mmotm.git master config: i386-tinyconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): mm/slab_common.o: In function `dump_unreclaimable_slab': >> slab_common.c:(.text+0x464): undefined reference to `get_slabinfo' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[BUG] fs/afs/flock: possible sleep-in-atomic bugs in afs_do_setlk
According to fs/afs/flock.c, the kernel may sleep under a spinlock, and the function call paths are: afs_do_setlk (acquire the spinlock: inode->i_lock) afs_vnode_fetch_status schedule --> may sleep afs_do_setlk (acquire the spinlock: inode->i_lock) wait_event_interruptible --> may sleep These bugs may be introduced by only considering "vnode->lock" but ignoring "inode->i_lock". A possible fix is to unlock "inode->i_lock" before calling afs_vnode_fetch_status and wait_event_interruptible, and lock "inode->i_lock" again after them. These bugs are found by my static analysis tool and my code review. Thanks, Jia-Ju Bai
Re: [RFC] yamldt v0.5, now a DTS compiler too
Hi Rob, > On Oct 6, 2017, at 16:55 , Rob Herring wrote: > > On Tue, Oct 3, 2017 at 12:39 PM, Pantelis Antoniou > wrote: >> Hi Rob, >> >> On Tue, 2017-10-03 at 12:13 -0500, Rob Herring wrote: >>> On Tue, Oct 3, 2017 at 9:13 AM, Pantelis Antoniou >>> wrote: Hi Rob, On Tue, 2017-10-03 at 08:18 -0500, Rob Herring wrote: > On Mon, Oct 2, 2017 at 2:46 PM, Pantelis Antoniou > wrote: >> Hi Rob, >> >> On Sun, 2017-10-01 at 17:00 -0500, Rob Herring wrote: >>> On Thu, Sep 28, 2017 at 2:58 PM, Pantelis Antoniou >>> wrote: Hello again, Significant progress has been made on yamldt and is now capable of not only generating yaml from DTS source but also compiling DTS sources and being almost fully compatible with DTC. >>> >>> Can you quantify "almost"? >>> Compiling the kernel's DTBs using yamldt is as simple as using a DTC=yamldt. >>> >>> Good. >>> Error reporting is accurate and validation against a YAML based schema works as well. In a short while I will begin posting patches with fixes on bindings and DTS files in the kernel. >>> >>> What I would like to see is the schema format posted for review. >>> >> >> I'm including the skeleton.yaml binding which is the template for >> the bindings and a board-example.yaml binding for a top level binding. >> >>> I would also like to see the bindings for top-level compatible strings >>> (aka boards) as an example. That's something that's simple enough that >>> I'd think we could agree on a format and start moving towards defining >>> board bindings that way. >>> >> >> Note there is some line wrapping I'm including a link >> to the github repo of the files: >> >> >> The skeleton.yaml >> >> https://raw.githubusercontent.com/pantoniou/yamldt/master/validate/bindings/skeleton.yaml >> >> %YAML 1.1 >> --- >> # The name of the binding is first >> # The anchor is put there for use by others >> skeleton: &skeleton > > This and "id" seem redundant. > Indeed. >>> >>> One other thing, "skeleton" is a bit weird as a key. It can't be >>> validated. All the other keys are standard words. I could write >>> "skeloton" by mistake and I guess I'd only find the mistake when >>> something inherits it. That's somewhat true with id, but we can at >>> least check "id" is present and that it's value is unique among all >>> other id values. >>> >> >> We can keep id and check that it matches the name of the enclosing node. >> That way you can be sure that there's no error. >> >> But it's a bit weird cause this is similar to declaring a function name >> with a typo. You won't find out until you use it. >> >> version: 1 >> >> id: skel-device >> >> title: > >>Skeleton Device >> >> maintainer: >>name: Skeleton Person >> >> description: > >>The Skeleton Device binding represents the SK11 device produced by >>the Skeleton Corporation. The binding can also support compatible >>clones made by second source vendors. >> >> # The class is an optional property that declares this >> # binding as part of a larger set >> # Multiple definitions are possible >> class: [ device, spi-device ] >> >> # This binding inherits property characteristics from the generic >> # spi-slave binding >> # Note that the notation is standard yaml reference >> inherits: *spi-slave >> >> # virtual bindings do not generate checkers >> virtual: true > > virtual is an overloaded term. > OK, what term should I use that this binding should not be instantiated as a checker, only be used by other bindings when inherited? >>> >>> checks: true? >>> >>> I'd really like to avoid having to decide and drop this, but I don't >>> really get why it is needed. >>> >> >> It is needed because otherwise checker filters will be generated for >> the template bindings that while they won't be executed they will be >> compiled and take up space in the schema. > > Size is not a problem I have. That can come later if needed. > >> # each property is defined by each name >> properties: >> >># The compatible property is a reserved name. The type is always >> "string" >># and should not be repeated device binding. >>compatible: >> category: required# required property >> type: strseq # is a sequence of strings >> >> description: > >>FX11 is a clone of the original SK11 device >> >> # v is always the name of the value of the property >> # np is passed to the checker and is the current >> # node pointer. We can access properties
Re: [Patch v6 2/7] slimbus: Add messaging APIs to slimbus framework
Thanks for the comments. On 07/10/17 07:42, Jonathan Neuschäfer wrote: Hi, On Fri, Oct 06, 2017 at 05:51:31PM +0200, srinivas.kandaga...@linaro.org wrote: From: Sagar Dharia Slimbus devices use value-element, and information elements to control device parameters (e.g. value element is used to represent gain for codec, information element is used to represent interrupt status for codec when codec interrupt fires). Messaging APIs are used to set/get these value and information elements. Slimbus specification uses 8-bit "transaction IDs" for messages where a read-value is anticipated. Framework uses a table of pointers to store those TIDs and responds back to the caller in O(1). Caller can opt to do synchronous, or asynchronous reads/writes. For asynchronous operations, the callback will be called from atomic context. TX and RX circular rings are used to allow queuing of multiple transfers per controller. Controller can choose size of these rings based of controller HW implementation. The buffers are coerently s/based of/based on/ s/coerently/coherently/ Yep.. will fix this in next version. mapped so that controller can utilize DMA operations for the transactions without remapping every transaction buffer. Statically allocated rings help to improve performance by avoiding overhead of dynamically allocating transactions on need basis. Signed-off-by: Sagar Dharia Tested-by: Naveen Kaje Signed-off-by: Srinivas Kandagatla --- [...] +int slim_xfer_msg(struct slim_controller *ctrl, + struct slim_device *sbdev, struct slim_val_inf *msg, + u8 mc) +{ + DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg); + struct slim_msg_txn *txn = &txn_stack; + int ret; + u16 sl, cur; + + ret = slim_val_inf_sanity(ctrl, msg, mc); + if (ret) + return ret; + + sl = slim_slicesize(msg->num_bytes); + + dev_dbg(&ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n", + msg->start_offset, msg->num_bytes, mc, sl); + + cur = slim_slicecodefromsize(sl); + txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4)); Shouldn't this be (cur | (1 << 3)? cur seems to be redundant TBH, the only difference between cur and sl is that the slim_slicesize() can give slice size to program for any lengths between 1-16 bytes. However the slim_slicecodefromsize() can only handle 1,2,3,4, 6,8,12,16 byte sizes. So we can delete slim_slicecodefromsize() call and function together. looks like it was a leftover from downstream. (Also, what does cur mean? Cursor? Current?) No Idea!! :-) it is supposed to return slice size as per number of bytes. + + switch (mc) { + case SLIM_MSG_MC_REQUEST_CHANGE_VALUE: + case SLIM_MSG_MC_CHANGE_VALUE: + case SLIM_MSG_MC_REQUEST_CLEAR_INFORMATION: + case SLIM_MSG_MC_CLEAR_INFORMATION: + txn->rl += msg->num_bytes; + default: + break; + } + + if (slim_tid_txn(txn->mt, txn->mc)) + txn->rl++; + + return slim_processtxn(ctrl, txn); +} +EXPORT_SYMBOL_GPL(slim_xfer_msg); [...] +/* + * slim_request_val_element: change and request a given value element name should be fixed here.. + * @sb: client handle requesting elemental message reads, writes. + * @msg: Input structure for start-offset, number of bytes to write. + * context: can sleep + * Returns: + * -EINVAL: Invalid parameters + * -ETIMEDOUT: If transmission of this message timed out (e.g. due to bus lines + * not being clocked or driven by controller) + * -ENOTCONN: If the transmitted message was not ACKed by destination device. Does rbuf contain the old value after this function finishes? Yep, device should send a reply value with the old value with matching tid. + */ +int slim_request_change_val_element(struct slim_device *sb, + struct slim_val_inf *msg) +{ + struct slim_controller *ctrl = sb->ctrl; + + if (!ctrl) + return -EINVAL; + + return slim_xfer_msg(ctrl, sb, msg, SLIM_MSG_MC_REQUEST_CHANGE_VALUE); +} +EXPORT_SYMBOL_GPL(slim_request_change_val_element); [...] +/** + * struct slim_pending: context of pending transfers + * @cb: callback for this transfer + * @ctx: contex for the callback function s/contex/context/ Will fix all these instances. + * @need_tid: True if this transfer need Transaction ID + */ +struct slim_pending { + void (*cb)(void *ctx, int err); + void *ctx; + bool need_tid; +}; Thanks, Jonathan Neuschäfer
Re: [Patch v6 1/7] slimbus: Device management on SLIMbus
Thanks for the comments. On 07/10/17 05:14, Jonathan Neuschäfer wrote: Hi, I have some more or less trivial comments below. On Fri, Oct 06, 2017 at 05:51:30PM +0200, srinivas.kandaga...@linaro.org wrote: From: Sagar Dharia SLIMbus (Serial Low Power Interchip Media Bus) is a specification developed by MIPI (Mobile Industry Processor Interface) alliance. SLIMbus is a 2-wire implementation, which is used to communicate with peripheral components like audio-codec. SLIMbus uses Time-Division-Multiplexing to accommodate multiple data channels, and control channel. Control channel has messages to do device-enumeration, messages to send/receive control-data to/from slimbus devices, messages for port/channel management, and messages to do bandwidth allocation. The framework supports multiple instances of the bus (1 controller per bus), and multiple slave devices per controller. This patch does device enumeration, logical address assignment, informing device when the device reports present/absent etc. Reporting present may need the driver to do the needful (e.g. turning on voltage regulators powering the device). Additionally device is probed when it reports present if that device doesn't need any such steps mentioned above. Signed-off-by: Sagar Dharia Signed-off-by: Srinivas Kandagatla --- [...] +SLIMbus example for Qualcomm's slimbus manager component: + + slim@2808 { + compatible = "qcom,slim-msm"; + reg = <0x2808 0x2000>, + interrupts = <0 33 0>; + clocks = <&lcc SLIMBUS_SRC>, <&lcc AUDIO_SLIMBUS_CLK>; + clock-names = "iface_clk", "core_clk"; + #address-cells = <2>; + #size-cells = <0>; + + codec: wcd9310@1{ + compatible = "slim217,60""; ^ spurious quote? + reg = <1 0>; + }; + }; diff --git a/Documentation/slimbus/summary b/Documentation/slimbus/summary new file mode 100644 index 000..e7f90bb --- /dev/null +++ b/Documentation/slimbus/summary Should this file have a .rst extension, like other Restructured Text files? Will try to sort this out in next version. @@ -0,0 +1,109 @@ +Overview of Linux kernel SLIMbus support + [...] +Device notifications to the driver: +--- +Since SLIMbus devices have mechanisms for reporting their presence, the +framework allows drivers to bind when corresponding devices report their +presence on the bus. +However, it is possible that the driver needs to be probed +first so that it can enable corresponding SLIMbus devie (e.g. power it up and/or s/devie/device/ I guess +take it out of reset). To support that behavior, the framework allows drivers +to probe first as well (e.g. using standard DeviceTree compatbility field). +This creates the necessity for the driver to know when the device is functional +(i.e. reported present). device_up callback is used for that reason when the +device reports present and is assigned a logical address by the controller. [...] +/** + * struct slim_addrt: slimbus address used internally by the slimbus framework. + * @valid: If the device is present. Valid is set to false when device reports + * absent. + * @eaddr: Enumeration address + * @laddr: It is possible that controller will set a predefined logical address + * rather than the one assigned by framework. (i.e. logical address may + * not be same as index into this table). This entry will store the + * logical address value for this enumeration address. + */ +struct slim_addrt { + boolvalid; + struct slim_eaddr eaddr; + u8 laddr; +}; I wonder if valid should be moved after eaddr, to reduce the need for padding. AFAICS, struct slim_eaddr is 6 bytes long and requires 2-byte alignment, so if valid is one byte long, there would be one byte of padding after it, slightly bloating struct slim_addrt, unnecessarily. Makes sense!! +/** + * struct slim_controller: Controls every instance of SLIMbus + * (similar to 'master' on SPI) + * 'Manager device' is responsible for device management, bandwidth + * allocation, channel setup, and port associations per channel. + * Device management means Logical address assignment/removal based on + * enumeration (report-present, report-absent) if a device. s/if a device/of a device/ ? Yep, will fix this in next version. + * Bandwidth allocation is done dynamically by the manager based on active + * channels on the bus, message-bandwidth requests made by slimbus devices. + * Based on current bandwidth usage, manager chooses a frequency to run + * the bus at (in steps of 'clock-gear', 1 through 10, each clock gear + * representing twice the frequency than the previous gear).
Re: [Patch v6 4/7] slimbus: Add support for 'clock-pause' feature
Thanks for the review comments On 07/10/17 09:06, Jonathan Neuschäfer wrote: Hi, some trivial comments below. On Fri, Oct 06, 2017 at 05:51:33PM +0200, srinivas.kandaga...@linaro.org wrote: From: Sagar Dharia Per slimbus specification, a reconfiguration sequence known as 'clock pause' needs to be broadcast over the bus while entering low- power mode. Clock-pause is initiated by the controller driver. To exit clock-pause, controller typically wakes up the framer device. Since wakeup precedure is controller-specific, framework calls it via controller's function pointer to invoke it. Signed-off-by: Sagar Dharia Signed-off-by: Srinivas Kandagatla --- [...] @@ -429,6 +444,14 @@ void slim_return_tx(struct slim_controller *ctrl, int err) cur.cb(cur.ctx, err); up(&ctrl->tx_sem); + if (!cur.clk_pause && (!cur.need_tid || err)) { + /** This isn't really a kerneldoc comment. Will fix all such instances in next version. +* remove runtime-pm vote if this was TX only, or +* if there was error during this transaction +*/ + pm_runtime_mark_last_busy(ctrl->dev.parent); + pm_runtime_put_autosuspend(ctrl->dev.parent); + } } EXPORT_SYMBOL_GPL(slim_return_tx); [...] +/** + * slim_ctrl_clk_pause: Called by slimbus controller to enter/exit 'clock pause' + * Slimbus specification needs this sequence to turn-off clocks for the bus. + * The sequence involves sending 3 broadcast messages (reconfiguration + * sequence) to inform all devices on the bus. + * To exit clock-pause, controller typically wakes up active framer device. + * @ctrl: controller requesting bus to be paused or woken up + * @wakeup: Wakeup this controller from clock pause. + * @restart: Restart time value per spec used for clock pause. This value + * isn't used when controller is to be woken up. + * This API executes clock pause reconfiguration sequence if wakeup is false. + * If wakeup is true, controller's wakeup is called. + * For entering clock-pause, -EBUSY is returned if a message txn in pending. + */ +int slim_ctrl_clk_pause(struct slim_controller *ctrl, bool wakeup, u8 restart) +{ + int i, ret = 0; + unsigned long flags; + struct slim_sched *sched = &ctrl->sched; + struct slim_val_inf msg = {0, 0, NULL, NULL, NULL, NULL}; + + DEFINE_SLIM_BCAST_TXN(txn, SLIM_MSG_MC_BEGIN_RECONFIGURATION, + 3, SLIM_LA_MANAGER, &msg); + + if (wakeup == false && restart > SLIM_CLK_UNSPECIFIED) + return -EINVAL; + + mutex_lock(&sched->m_reconf); + if (wakeup) { + if (sched->clk_state == SLIM_CLK_ACTIVE) { + mutex_unlock(&sched->m_reconf); + return 0; + } + + /** ditto +* Fine-tune calculation based on clock gear, +* message-bandwidth after bandwidth management +*/ + ret = wait_for_completion_timeout(&sched->pause_comp, + msecs_to_jiffies(100)); + if (!ret) { + mutex_unlock(&sched->m_reconf); + pr_err("Previous clock pause did not finish"); + return -ETIMEDOUT; + } + ret = 0; + + /** ditto +* Slimbus framework will call controller wakeup +* Controller should make sure that it sets active framer +* out of clock pause +*/ + if (sched->clk_state == SLIM_CLK_PAUSED && ctrl->wakeup) + ret = ctrl->wakeup(ctrl); + if (!ret) + sched->clk_state = SLIM_CLK_ACTIVE; + mutex_unlock(&sched->m_reconf); + + return ret; + } [...] Thanks, Jonathan Neuschäfer
Re: [Patch v6 3/7] slimbus: qcom: Add Qualcomm Slimbus controller driver
Thanks for the review comments On 07/10/17 08:45, Jonathan Neuschäfer wrote: Hi, On Fri, Oct 06, 2017 at 05:51:32PM +0200, srinivas.kandaga...@linaro.org wrote: From: Sagar Dharia This controller driver programs manager, interface, and framer devices for Qualcomm's slimbus HW block. Manager component currently implements logical address setting, and messaging interface. Interface device reports bus synchronization information, and framer device clocks the bus from the time it's woken up, until clock-pause is executed by the manager device. Signed-off-by: Sagar Dharia Signed-off-by: Srinivas Kandagatla --- .../devicetree/bindings/slimbus/slim-qcom-ctrl.txt | 43 ++ drivers/slimbus/Kconfig| 9 + drivers/slimbus/Makefile | 3 + drivers/slimbus/slim-qcom-ctrl.c | 594 + drivers/slimbus/slim-qcom.h| 63 +++ 5 files changed, 712 insertions(+) create mode 100644 Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt create mode 100644 drivers/slimbus/slim-qcom-ctrl.c create mode 100644 drivers/slimbus/slim-qcom.h diff --git a/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt b/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt new file mode 100644 index 000..081110d --- /dev/null +++ b/Documentation/devicetree/bindings/slimbus/slim-qcom-ctrl.txt @@ -0,0 +1,43 @@ +Qualcomm SLIMBUS controller +This controller is used if applications processor driver controls slimbus +master component. + +Required properties: + + - #address-cells - refer to Documentation/devicetree/bindings/slimbus/bus.txt + - #size-cells - refer to Documentation/devicetree/bindings/slimbus/bus.txt + + - reg : Offset and length of the register region(s) for the device + - reg-names : Register region name(s) referenced in reg above +Required register resource entries are: +"ctrl": Physical adderess of controller register blocks s/adderess/address/ Will fix this in next version. +} [...] +static void msm_slim_prg_slew(struct platform_device *pdev, + struct msm_slim_ctrl *dev) +{ + void __iomem *slew_reg; + + /* SLEW RATE register for this slimbus */ + dev->slew_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, +"slew"); + if (!dev->slew_mem) { + dev_warn(&pdev->dev, "no slimbus slew resource\n"); + return; + } + + slew_reg = devm_ioremap(&pdev->dev, dev->slew_mem->start, + resource_size(dev->slew_mem)); How often will the driver program a slew rate? This should be programmed only once after power on. If it's often, you'll have a "soft" memory leak over the life time of a SLIM controller instance, because the mappings for slew_reg will accumulate in the driver instance's devm area until they are all freed in the end (If I'm reading the code correctly). I think you'll either have to unmap slew_reg when this function returns (and not use devm), or cache slew_reg so that subsequent calls to msm_slim_prg_slew won't create more mappings. Yep .. I revisit this part once again before sending next version to see if we can do any better! + if (!slew_reg) { + dev_err(dev->dev, "slew register mapping failed"); + release_mem_region(dev->slew_mem->start, + resource_size(dev->slew_mem)); + dev->slew_mem = NULL; + return; + } + writel_relaxed(1, slew_reg); + /* Make sure slimbus-slew rate enabling goes through */ + wmb(); +} + +static int msm_slim_probe(struct platform_device *pdev) +{ + struct msm_slim_ctrl *dev; + struct slim_controller *ctrl; + struct resource *slim_mem; + struct resource *irq; + struct clk *hclk, *rclk; + int ret; + + hclk = devm_clk_get(&pdev->dev, "iface_clk"); + if (IS_ERR(hclk)) + return PTR_ERR(hclk); + + rclk = devm_clk_get(&pdev->dev, "core_clk"); + if (IS_ERR(rclk)) { + /* unlikely that this is probe-defer */ + dev_err(&pdev->dev, "rclk get failed:%ld\n", PTR_ERR(rclk)); + return PTR_ERR(rclk); + } + + ret = clk_set_rate(rclk, SLIM_ROOT_FREQ); + if (ret) { + dev_err(&pdev->dev, "ref-clock set-rate failed:%d\n", ret); + return ret; + } + + slim_mem = platform_get_resource_byname(pdev, IORESOURCE_MEM, "ctrl"); + if (!slim_mem) { + dev_err(&pdev->dev, "no slimbus physical memory resource\n"); + return -ENODEV; + } + + irq = platform_get_resource(pdev, IORESOURCE_IRQ, 0); + if (!irq) { + dev_err(&pdev->dev, "no slimbus IRQ resource\n"); + return -ENODEV; + } + + dev = de
Re: [Patch v6 6/7] regmap: add SLIMBUS support
Thanks for the review comments, On 07/10/17 06:02, Jonathan Neuschäfer wrote: Hi, On Fri, Oct 06, 2017 at 05:51:35PM +0200, srinivas.kandaga...@linaro.org wrote: From: Srinivas Kandagatla This patch adds support to read/write slimbus value elements. Currently it only supports byte read/write. Adding this support in regmap would give codec drivers more flexibility when there are more than 2 control interfaces like slimbus, i2c. Without this patch each codec driver has to directly call slimbus value element apis, and this could would get messy once we want to add i2c interface to it. Signed-off-by: Srinivas Kandagatla --- [...] +static int regmap_slimbus_byte_reg_read(void *context, unsigned int reg, + unsigned int *val) +{ + struct slim_device *slim = context; + struct slim_val_inf msg = {0,}; + + msg.start_offset = reg; + msg.num_bytes = 1; + msg.rbuf = (void *)val; + + return slim_request_val_element(slim, &msg); +} This looks like it won't work on big-endian systems. I know big endian is pretty uncommon in devices that will likely have SLIMBus, but it's better to be endian-independent. Yep, I agree! will fix it in next version. +static int regmap_slimbus_byte_reg_write(void *context, unsigned int reg, +unsigned int val) +{ + struct slim_device *slim = context; + struct slim_val_inf msg = {0,}; + + msg.start_offset = reg; + msg.num_bytes = 1; + msg.wbuf = (void *)&val; + + return slim_change_val_element(slim, &msg); +} dito +static struct regmap_bus regmap_slimbus_bus = { + .reg_write = regmap_slimbus_byte_reg_write, + .reg_read = regmap_slimbus_byte_reg_read, +}; Thanks, Jonathan Neuschäfer
Re: [Patch v6 5/7] slimbus: qcom: Add runtime-pm support using clock-pause feature
Thanks for the review comments On 07/10/17 09:22, Jonathan Neuschäfer wrote: Hi, some more trivial comments below. On Fri, Oct 06, 2017 at 05:51:34PM +0200, srinivas.kandaga...@linaro.org wrote: From: Sagar Dharia Slimbus HW mandates that clock-pause sequence has to be executed before disabling relevant interface and core clocks. Runtime-PM's autosuspend feature is used here to enter/exit low power mode for Qualcomm's Slimbus controller. Autosuspend feature enables driver to avoid changing power-modes too frequently since entering clock-pause is an expensive sequence Signed-off-by: Sagar Dharia Signed-off-by: Srinivas Kandagatla --- [...] +static int msm_clk_pause_wakeup(struct slim_controller *ctrl) +{ + struct msm_slim_ctrl *dev = slim_get_ctrldata(ctrl); + + clk_prepare_enable(dev->hclk); + clk_prepare_enable(dev->rclk); + enable_irq(dev->irq); + + writel_relaxed(1, dev->base + FRM_WAKEUP); + /* Make sure framer wakeup write goes through before ISR fires */ + mb(); + /** This isn't really a kerneldoc comment. Yep I agree will fix all such instances. +* HW Workaround: Currently, slave is reporting lost-sync messages +* after slimbus comes out of clock pause. +* Transaction with slave fail before slave reports that message +* Give some time for that report to come +* Slimbus wakes up in clock gear 10 at 24.576MHz. With each superframe +* being 250 usecs, we wait for 5-10 superframes here to ensure +* we get the message +*/ + usleep_range(1250, 2500); + return 0; +} [...] +#ifdef CONFIG_PM_SLEEP +static int msm_slim_suspend(struct device *dev) +{ + int ret = 0; + + if (!pm_runtime_enabled(dev) || + (!pm_runtime_suspended(dev))) { + dev_dbg(dev, "system suspend"); + ret = msm_slim_runtime_suspend(dev); + } + if (ret == -EISCONN) { + /** ditto. Also, it looks misindented. Yep. will fix this too. +* If the clock pause failed due to active channels, there is +* a possibility that some audio stream is active during suspend. +* (e.g. modem usecase during suspend) +* We dont want to return suspend failure in that case so that +* display and relevant components can still go to suspend. +* If there is some other error, then it should prevent +* system level suspend +*/ + ret = 0; + } + return ret; +} Thanks, Jonathan Neuschäfer
Re: [PATCH 2/2] pinctrl: rockchip: Fix the correct routing config for the gmac-m1 pins of rmii and rgmii
On Sat, Sep 30, 2017 at 5:07 PM, Heiko Stuebner wrote: > Am Samstag, 30. September 2017, 20:13:21 CEST schrieb David Wu: >> If the gmac-m1 optimization(bit10) is selected, the gpio function >> of gmac pins is not valid. We may use the rmii mode for gmac interface, >> the pins such as rx_d2, rx_d3, which the rgmii mode used, but rmii not >> used could be taken as gpio function. So gmac_rxd0m1 selects the bit2, >> and gmac_rxd0m3 select bit10 is more correct. >> >> Signed-off-by: David Wu > > the patch subject should mention the the rk3328 whose routing gets fixed > (like adding a simple "on rk3328" to it), otherwise > > Reviewed-by: Heiko Stuebner I added rk3328 to it, also assumed this Reviewed-by covers patch 1/2 as well and applied both with your tag. Yours, Linus Walleij
A question of "cond_resched_lock" called in atomic context
I have seen some drivers or file systems calls "cond_resched_lock" when holding a spinlock. An example is in fs/ocfs2/dlm/dlmdomain.c: dlm_migrate_all_locks (acquire the spinlock) cond_resched_lock I find that "cond_resched_lock" has two functions: "___might_sleep" and "__cond_resched_lock". I know that "__cond_resched_lock" is safe and okay to be called when holding a spinlock. However, I think "___might_sleep" can be removed, because it prints error messages in this situation, but it is safe in fact. Am I right? I am looking forward to your comments :) Thanks, Jia-Ju Bai
Re: [PATCH] fs/afs/flock and fs/locks: Fix possible sleep-in-atomic bugs in posix_lock_file
On Sat, 2017-10-07 at 17:55 +0800, Jia-Ju Bai wrote: > The kernel may sleep under a spinlock, and the function call paths are: > afs_do_unlk (acquire the spinlock) > posix_lock_file > posix_lock_inode (fs/locks.c) > locks_get_lock_context > kmem_cache_alloc(GFP_KERNEL) --> may sleep > > afs_do_setlk (acquire the spinlock) > posix_lock_file > posix_lock_inode (fs/locks.c) > locks_get_lock_context > kmem_cache_alloc(GFP_KERNEL) --> may sleep > > To fix them, GFP_KERNEL is replaced with GFP_ATOMIC. > These bugs are found by my static analysis tool and my code review. > > Signed-off-by: Jia-Ju Bai > --- > fs/locks.c |2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/locks.c b/fs/locks.c > index 1bd71c4..975cc62 100644 > --- a/fs/locks.c > +++ b/fs/locks.c > @@ -222,7 +222,7 @@ struct file_lock_list_struct { > if (likely(ctx) || type == F_UNLCK) > goto out; > > - ctx = kmem_cache_alloc(flctx_cache, GFP_KERNEL); > + ctx = kmem_cache_alloc(flctx_cache, GFP_ATOMIC); > if (!ctx) > goto out; > NAK This needs to be fixed in the AFS code. It should not be calling these functions with a spinlock held. -- Jeff Layton
Re: [PATCH v3 00/12] add pinmuxing support for pins in AXP209 and AXP813 PMICs
On Mon, Oct 2, 2017 at 2:08 PM, Quentin Schulz wrote: > The AXP209 and AXP813 PMICs have several pins (respectively 3 and 2) that can > be used either as GPIOs or for other purposes (ADC or LDO here). > > We already have a GPIO driver for the GPIO use of those pins on the AXP209. > Let's "upgrade" this driver to support all the functions these pins can have. > > Then we add support to this driver for the AXP813 which is slighlty different > (basically a different offset in a register and one less pin). > > I suggest patches 1 to 6 go through Linus's tree and 7 to 12 via Maxime or > Chen-Yu's tree. > > This version of the patchset is based on Chen-Yu's patchset for AXP813/818 > regulators[1]. > > v3: This is starting to look really good, but I see there are still comments. Keep the series going, we merge once Maxime and Chen-Yu are happy. Yours, Linus Walleij
Re: kprobes: propagate error from arm_kprobe_ftrace()
+++ Masami Hiramatsu [05/10/17 06:23 +]: Hi Jessica, On Wed, 4 Oct 2017 21:14:13 +0200 Jessica Yu wrote: Improve error handling when arming ftrace-based kprobes. Specifically, if we fail to arm a ftrace-based kprobe, register_kprobe()/enable_kprobe() should report an error instead of success. Previously, this has lead to confusing situations where register_kprobe() would return 0 indicating success, but the kprobe would not be functional if ftrace registration during the kprobe arming process had failed. We should therefore take any errors returned by ftrace into account and propagate this error so that we do not register/enable kprobes that cannot be armed. This can happen if, for example, register_ftrace_function() finds an IPMODIFY conflict (since kprobe_ftrace_ops has this flag set) and returns an error. Such a conflict is possible since livepatches also set the IPMODIFY flag for their ftrace_ops. arm_all_kprobes() keeps its current behavior and attempts to arm all kprobes. It returns the last encountered error and gives a warning if not all kprobes could be armed. This patch is based on Petr Mladek's original patchset (patches 2 and 3) back in 2015, which improved kprobes error handling, found here: https://lkml.org/lkml/2015/2/26/452 However, further work on this had been paused since then and the patches were not upstreamed. Ok, I have some comment. See below. Based-on-patches-by: Petr Mladek Signed-off-by: Jessica Yu --- kernel/kprobes.c | 87 +++- 1 file changed, 61 insertions(+), 26 deletions(-) diff --git a/kernel/kprobes.c b/kernel/kprobes.c index 2d28377a0e32..6e889be0d93c 100644 --- a/kernel/kprobes.c +++ b/kernel/kprobes.c @@ -979,18 +979,27 @@ static int prepare_kprobe(struct kprobe *p) } /* Caller must lock kprobe_mutex */ -static void arm_kprobe_ftrace(struct kprobe *p) +static int arm_kprobe_ftrace(struct kprobe *p) { - int ret; + int ret = 0; ret = ftrace_set_filter_ip(&kprobe_ftrace_ops, (unsigned long)p->addr, 0, 0); - WARN(ret < 0, "Failed to arm kprobe-ftrace at %p (%d)\n", p->addr, ret); - kprobe_ftrace_enabled++; - if (kprobe_ftrace_enabled == 1) { + if (WARN(ret < 0, "Failed to arm kprobe-ftrace at %p (%d)\n", p->addr, ret)) + return ret; + + if (kprobe_ftrace_enabled == 0) { ret = register_ftrace_function(&kprobe_ftrace_ops); - WARN(ret < 0, "Failed to init kprobe-ftrace (%d)\n", ret); + if (WARN(ret < 0, "Failed to init kprobe-ftrace (%d)\n", ret)) + goto err_ftrace; } + + kprobe_ftrace_enabled++; + return ret; + +err_ftrace: + ftrace_set_filter_ip(&kprobe_ftrace_ops, (unsigned long)p->addr, 1, 0); + return ret; } /* Caller must lock kprobe_mutex */ @@ -1009,22 +1018,23 @@ static void disarm_kprobe_ftrace(struct kprobe *p) } #else /* !CONFIG_KPROBES_ON_FTRACE */ #define prepare_kprobe(p) arch_prepare_kprobe(p) -#define arm_kprobe_ftrace(p) do {} while (0) +#define arm_kprobe_ftrace(p) (0) #define disarm_kprobe_ftrace(p)do {} while (0) #endif /* Arm a kprobe with text_mutex */ -static void arm_kprobe(struct kprobe *kp) +static int arm_kprobe(struct kprobe *kp) { - if (unlikely(kprobe_ftrace(kp))) { - arm_kprobe_ftrace(kp); - return; - } + if (unlikely(kprobe_ftrace(kp))) + return arm_kprobe_ftrace(kp); + cpus_read_lock(); mutex_lock(&text_mutex); __arm_kprobe(kp); mutex_unlock(&text_mutex); cpus_read_unlock(); + + return 0; } /* Disarm a kprobe with text_mutex */ @@ -1363,9 +1373,14 @@ static int register_aggr_kprobe(struct kprobe *orig_p, struct kprobe *p) if (ret == 0 && kprobe_disabled(ap) && !kprobe_disabled(p)) { ap->flags &= ~KPROBE_FLAG_DISABLED; - if (!kprobes_all_disarmed) + if (!kprobes_all_disarmed) { /* Arm the breakpoint again. */ - arm_kprobe(ap); + ret = arm_kprobe(ap); + if (ret) { + ap->flags |= KPROBE_FLAG_DISABLED; + list_del_rcu(&p->list); Nice catch :) this list_del_rcu() is important to keep error case behavior sane. + } + } } return ret; } @@ -1570,13 +1585,16 @@ int register_kprobe(struct kprobe *p) if (ret) goto out; + if (!kprobes_all_disarmed && !kprobe_disabled(p)) { + ret = arm_kprobe(p); + if (ret) + goto out; + } + No, this is no good. It is a small chance to hit kprobe on other CPUs before adding it to kprobe_table hashlist. In that case, we will see a stray breakpoint instruction. Ah yes,
Re: [PATCH 1/7] gpio: brcmstb: allow all instances to be wakeup sources
On Sat, Sep 30, 2017 at 5:40 AM, Doug Berger wrote: > This commit allows a wakeup parent interrupt to be shared between > instances. > > It also removes the redundant can_wake member of the private data > structure by using whether the parent_wake_irq has been defined to > indicate that the GPIO device can wake. > > Fixes: 19a7b6940b78 ("gpio: brcmstb: Add interrupt and wakeup source support") > Signed-off-by: Doug Berger Patch applied with Gregory's and Florian's ACKs. Yours, Linus Walleij
Re: [PATCH 0/7] gpio: brcmstb: improved interrupt and wake support
On Sat, Sep 30, 2017 at 5:40 AM, Doug Berger wrote: > This patch set collects a number of improvements to the GPIO driver > used by Broadcom Set-Top-Box devices. > > Primarily they are aimed at correcting problems with the interrupt > controller implementation, but they also extend the functionality for > waking on GPIO interrupts. I saw there was some mention of a v2 patch set inline, so I backed out the applied patch. Please send the v2 series and collect the ACKs from Gregory and Florian so I can apply it all! Yours, Linus Walleij
Re: [PATCH] Makefile: kselftest: fix grammar typo
2017-10-07 9:17 GMT+09:00 Randy Dunlap : > From: Randy Dunlap > > Correct typo in kselftest help text. > > Signed-off-by: Randy Dunlap > Cc: Shuah Khan > Cc: linux-kselft...@vger.kernel.org > Cc: Masahiro Yamada > Cc: Jiri Kosina > --- Applied to linux-kbuild/fixes. Thanks. -- Best Regards Masahiro Yamada
Re: [RESEND PATCH] gpio: omap: Fix lost edge interrupts
On Tue, Oct 3, 2017 at 6:17 PM, Grygorii Strashko wrote: > Now acking of edge irqs happens the following way: > - omap_gpio_irq_handler > - "isr" = read irq status > - omap_clear_gpio_irqbank(bank, isr_saved & ~level_mask); > ^ clear edge status, so irq can be accepted > - loop while "isr" > generic_handle_irq() > - handle_edge_irq() > - desc->irq_data.chip->irq_ack(&desc->irq_data); > - omap_gpio_ack_irq() > it might be that at this moment edge IRQ was triggered again and it will be > cleared and IRQ will be lost. > > Use handle_simple_irq and clear edge interrupts early without disabling them > in > omap_gpio_irq_handler to avoid loosing interrupts. > > [1] https://marc.info/?l=linux-omap&m=149004465313534&w=2 > Signed-off-by: Grygorii Strashko > Signed-off-by: Ladislav Michl Patch applied for fixes. I guess it is the right thing to do? Other maintainers need to tell me if I should hold it back. Yours, Linus Walleij
Re: [PATCH] Makefile: enable dochelp run from main make level
2017-10-03 8:44 GMT+09:00 Shuah Khan : > Change to enable dochelp run from main make level to make it easier to > use it. > > Signed-off-by: Shuah Khan > --- > Makefile | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/Makefile b/Makefile > index cf007a31d575..f99d1b36c437 100644 > --- a/Makefile > +++ b/Makefile > @@ -1454,7 +1454,7 @@ $(help-board-dirs): help-%: > > # Documentation targets > # --- > -DOC_TARGETS := xmldocs latexdocs pdfdocs htmldocs epubdocs cleandocs > linkcheckdocs > +DOC_TARGETS := xmldocs latexdocs pdfdocs htmldocs epubdocs cleandocs > linkcheckdocs dochelp > PHONY += $(DOC_TARGETS) > $(DOC_TARGETS): scripts_basic FORCE > $(Q)$(MAKE) $(build)=Documentation $@ > -- > 2.11.0 > The dochelp is invoked from "make help" of the main make level. Do you mean "make dochelp" is also necessary? -- Best Regards Masahiro Yamada
Re: [PATCH 2/3] mm: slabinfo: dump CONFIG_SLABINFO
Hi Yang, [auto build test ERROR on mmotm/master] [also build test ERROR on v4.14-rc3 next-20170929] [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/Yang-Shi/oom-capture-unreclaimable-slab-info-in-oom-message/20171007-173639 base: git://git.cmpxchg.org/linux-mmotm.git master config: sh-allnoconfig (attached as .config) compiler: sh4-linux-gnu-gcc (Debian 6.1.1-9) 6.1.1 20160705 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=sh All errors (new ones prefixed by >>): mm/slub.c: In function 'get_slabinfo': mm/slub.c:5864:14: error: implicit declaration of function 'node_nr_objs' [-Werror=implicit-function-declaration] nr_objs += node_nr_objs(n); ^~~~ >> mm/slub.c:5865:14: error: implicit declaration of function 'count_partial' >> [-Werror=implicit-function-declaration] nr_free += count_partial(n, count_free); ^ mm/slub.c:5865:31: error: 'count_free' undeclared (first use in this function) nr_free += count_partial(n, count_free); ^~ mm/slub.c:5865:31: note: each undeclared identifier is reported only once for each function it appears in cc1: some warnings being treated as errors vim +/count_partial +5865 mm/slub.c 57ed3eda9 Pekka J Enberg2008-01-01 5850 57ed3eda9 Pekka J Enberg2008-01-01 5851 /* 57ed3eda9 Pekka J Enberg2008-01-01 5852 * The /proc/slabinfo ABI 57ed3eda9 Pekka J Enberg2008-01-01 5853 */ 0d7561c61 Glauber Costa 2012-10-19 5854 void get_slabinfo(struct kmem_cache *s, struct slabinfo *sinfo) 57ed3eda9 Pekka J Enberg2008-01-01 5855 { 57ed3eda9 Pekka J Enberg2008-01-01 5856unsigned long nr_slabs = 0; 205ab99dd Christoph Lameter 2008-04-14 5857unsigned long nr_objs = 0; 205ab99dd Christoph Lameter 2008-04-14 5858unsigned long nr_free = 0; 57ed3eda9 Pekka J Enberg2008-01-01 5859int node; fa45dc254 Christoph Lameter 2014-08-06 5860struct kmem_cache_node *n; 57ed3eda9 Pekka J Enberg2008-01-01 5861 fa45dc254 Christoph Lameter 2014-08-06 5862for_each_kmem_cache_node(s, node, n) { c17fd13ec Wanpeng Li2013-07-04 5863nr_slabs += node_nr_slabs(n); c17fd13ec Wanpeng Li2013-07-04 @5864nr_objs += node_nr_objs(n); 205ab99dd Christoph Lameter 2008-04-14 @5865nr_free += count_partial(n, count_free); 57ed3eda9 Pekka J Enberg2008-01-01 5866} 57ed3eda9 Pekka J Enberg2008-01-01 5867 0d7561c61 Glauber Costa 2012-10-19 5868sinfo->active_objs = nr_objs - nr_free; 0d7561c61 Glauber Costa 2012-10-19 5869sinfo->num_objs = nr_objs; 0d7561c61 Glauber Costa 2012-10-19 5870sinfo->active_slabs = nr_slabs; 0d7561c61 Glauber Costa 2012-10-19 5871sinfo->num_slabs = nr_slabs; 0d7561c61 Glauber Costa 2012-10-19 5872sinfo->objects_per_slab = oo_objects(s->oo); 0d7561c61 Glauber Costa 2012-10-19 5873sinfo->cache_order = oo_order(s->oo); 57ed3eda9 Pekka J Enberg2008-01-01 5874 } 57ed3eda9 Pekka J Enberg2008-01-01 5875 :: The code at line 5865 was first introduced by commit :: 205ab99dd103e3dd5b0964dad8a16dfe2db69b2e slub: Update statistics handling for variable order slabs :: TO: Christoph Lameter :: CC: Pekka Enberg --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [BUG] fs/super: a possible sleep-in-atomic bug in put_super
Hello, On Fri, Oct 06, 2017 at 11:06:04AM +0200, Michal Hocko wrote: > On Fri 06-10-17 16:59:18, Jia-Ju Bai wrote: > > According to fs/super.c, the kernel may sleep under a spinlock. > > The function call path is: > > put_super (acquire the spinlock) > > __put_super > > destroy_super > > list_lru_destroy > > list_lru_unregister > > mutex_lock --> may sleep > > memcg_get_cache_ids > > down_read --> may sleep > > > > This bug is found by my static analysis tool and my code review. This is false-positive: by the time we get to destroy_super(), the lru lists have already been destroyed - see deactivate_locked_super() - so list_lru_destroy() will retrun right away without attempting to take any locks. That's why there's no lockdep warnings regarding this issue. I think we can move list_lru_destroy() to destroy_super_work() to suppress this warning. Not sure if it's really worth the trouble though. Thanks, Vladimir
RE: [PATCH v5 08/14] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens
> -Original Message- > From: Greg KH [mailto:g...@kroah.com] > Sent: Saturday, October 7, 2017 1:54 AM > To: Limonciello, Mario > Cc: dvh...@infradead.org; Andy Shevchenko ; > LKML ; platform-driver-...@vger.kernel.org; > Andy Lutomirski ; quasi...@google.com; > pali.ro...@gmail.com; r...@rjwysocki.net; mj...@google.com; h...@lst.de > Subject: Re: [PATCH v5 08/14] platform/x86: dell-smbios: Add a sysfs > interface for > SMBIOS tokens > > On Fri, Oct 06, 2017 at 11:59:52PM -0500, Mario Limonciello wrote: > > Currently userspace tools can access system tokens via the dcdbas > > kernel module and a SMI call that will cause the platform to execute > > SMM code. > > > > With a goal in mind of deprecating the dcdbas kernel module a different > > method for accessing these tokens from userspace needs to be created. > > > > This is intentionally marked to only be readable as root as it can > > contain sensitive information about the platform's configuration. > > > > MAINTAINERS was missing for this driver. Add myself and Pali to > > maintainers list for it. > > > > Signed-off-by: Mario Limonciello > > --- > > .../ABI/testing/sysfs-platform-dell-smbios | 16 ++ > > MAINTAINERS| 7 +++ > > drivers/platform/x86/dell-smbios.c | 64 > > ++ > > 3 files changed, 87 insertions(+) > > create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-dell-smbios > b/Documentation/ABI/testing/sysfs-platform-dell-smbios > > new file mode 100644 > > index ..d97f4bd5bd91 > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios > > @@ -0,0 +1,16 @@ > > +What: /sys/devices/platform//tokens > > +Date: November 2017 > > +KernelVersion: 4.15 > > +Contact: "Mario Limonciello" > > +Description: > > + A read-only description of Dell platform tokens > > + available on the machine. > > + > > + The tokens will be displayed in the following > > + machine readable format with each token on a > > + new line: > > + > > + ID Locationvalue > > + > > + For example token: > > + 5 5 3 > > That's more than "one value per file" which is what sysfs requires, so > this isn't acceptable, sorry. What's more acceptable to you to relay this information? Binary sysfs attribute and export the structure format in a uapi? or a collection of sysfs files? Eg: tokens/$x/id tokens/$x/location tokens/$s/value I'm guessing the latter is the better way to go. > > > +static ssize_t tokens_show(struct device *dev, > > + struct device_attribute *attr, char *buf) > > +{ > > + size_t off = 0; > > + int to_print; > > + int i; > > + > > + to_print = min(da_num_tokens, (int)(PAGE_SIZE - 1) / 15); > > + for (i = 0; i < to_print; i++) { > > + off += scnprintf(buf+off, PAGE_SIZE-off, "%04x\t%04x\t%04x\n", > > + da_tokens[i].tokenID, da_tokens[i].location, > > + da_tokens[i].value); > > Huge hint, if you are checking for the max size of the buffer for sysfs, > something is really wrong, and you need to redo your design. > > thanks, > > greg k-h
RE: [PATCH v5 13/14] platform/x86: wmi: create character devices when requested by drivers
> -Original Message- > From: Greg KH [mailto:g...@kroah.com] > Sent: Saturday, October 7, 2017 2:35 AM > To: Limonciello, Mario > Cc: dvh...@infradead.org; Andy Shevchenko ; > LKML ; platform-driver-...@vger.kernel.org; > Andy Lutomirski ; quasi...@google.com; > pali.ro...@gmail.com; r...@rjwysocki.net; mj...@google.com; h...@lst.de > Subject: Re: [PATCH v5 13/14] platform/x86: wmi: create character devices when > requested by drivers > > On Fri, Oct 06, 2017 at 11:59:57PM -0500, Mario Limonciello wrote: > > For WMI operations that are only Set or Query read or write sysfs > > attributes created by WMI vendor drivers make sense. > > > > For other WMI operations that are run on Method, there needs to be a > > way to guarantee to userspace that the results from the method call > > belong to the data request to the method call. Sysfs attributes don't > > work well in this scenario because two userspace processes may be > > competing at reading/writing an attribute and step on each other's > > data. > > > > When a WMI vendor driver declares an ioctl callback in the wmi_driver > > the WMI bus driver will create a character device that maps to that > > function. > > > > That character device will correspond to this path: > > /dev/wmi/$driver > > > > The WMI bus driver will interpret the IOCTL calls, test them for > > a valid instance and pass them on to the vendor driver to run. > > > > This creates an implicit policy that only driver per character > > device. If a module matches multiple GUID's, the wmi_devices > > will need to be all handled by the same wmi_driver if the same > > character device is used. > > > > The WMI vendor drivers will be responsible for managing access to > > this character device and proper locking on it. > > > > When a WMI vendor driver is unloaded the WMI bus driver will clean > > up the character device. > > What prevents the vendor driver from being unloaded while the ioctl is > being called in it? I don't see any protection here from that at all :( > In my driver I take a mutex that blocks unloading while running ioctl, but you mean you want one in the bus driver more generally, OK. > > +static long wmi_unlocked_ioctl(struct file *filp, unsigned int cmd, > > + unsigned long arg) > > +{ > > + return match_ioctl(filp, cmd, arg, 0); > > +} > > + > > +static long wmi_compat_ioctl(struct file *filp, unsigned int cmd, > > +unsigned long arg) > > +{ > > + return match_ioctl(filp, cmd, arg, 1); > > +} > > Why a compat ioctl at all? That's for older interfaces, not for brand > new ones where you design the ioctl structures "correctly" to work on > both 32 and 64 bits at the same time. That should not be needed at all. > > thanks, > > greg k-h I was trying to make sure that any other future vendor drivers had it for an option. I wasn't aware that new code shouldn't use it. OK, I'll remove it.
Re: [PATCH 2/3] mm: slabinfo: dump CONFIG_SLABINFO
Hi Yang, [auto build test ERROR on mmotm/master] [also build test ERROR on v4.14-rc3 next-20170929] [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/Yang-Shi/oom-capture-unreclaimable-slab-info-in-oom-message/20171007-173639 base: git://git.cmpxchg.org/linux-mmotm.git master config: openrisc-or1ksim_defconfig (attached as .config) compiler: or1k-linux-gcc (GCC) 5.4.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=openrisc All errors (new ones prefixed by >>): mm/slab_common.o: In function `slab_show': slab_common.c:(.text+0x1c4): undefined reference to `get_slabinfo' slab_common.c:(.text+0x1c4): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `get_slabinfo' >> slab_common.c:(.text+0x268): undefined reference to `slabinfo_show_stats' slab_common.c:(.text+0x268): relocation truncated to fit: R_OR1K_INSN_REL_26 against undefined symbol `slabinfo_show_stats' >> mm/slab_common.o:(.rodata+0xc): undefined reference to `slabinfo_write' --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH] PCI: iproc: Allow allocation of multiple MSIs
From: Sandor Bodo-Merle Add support for allocating multiple MSIs at the same time, so that the MSI_FLAG_MULTI_PCI_MSI flag can be added to the msi_domain_info structure. Avoid storing the hwirq in the low 5 bits of the message data, as it is used by the device. Also fix an endianness problem by using readl(). Signed-off-by: Sandor Bodo-Merle --- drivers/pci/host/pcie-iproc-msi.c | 19 --- 1 file changed, 12 insertions(+), 7 deletions(-) diff --git a/drivers/pci/host/pcie-iproc-msi.c b/drivers/pci/host/pcie-iproc-msi.c index 2d0f535a2f69..990fc906d73d 100644 --- a/drivers/pci/host/pcie-iproc-msi.c +++ b/drivers/pci/host/pcie-iproc-msi.c @@ -179,7 +179,7 @@ static struct irq_chip iproc_msi_irq_chip = { static struct msi_domain_info iproc_msi_domain_info = { .flags = MSI_FLAG_USE_DEF_DOM_OPS | MSI_FLAG_USE_DEF_CHIP_OPS | - MSI_FLAG_PCI_MSIX, + MSI_FLAG_MULTI_PCI_MSI | MSI_FLAG_PCI_MSIX, .chip = &iproc_msi_irq_chip, }; @@ -237,7 +237,7 @@ static void iproc_msi_irq_compose_msi_msg(struct irq_data *data, addr = msi->msi_addr + iproc_msi_addr_offset(msi, data->hwirq); msg->address_lo = lower_32_bits(addr); msg->address_hi = upper_32_bits(addr); - msg->data = data->hwirq; + msg->data = data->hwirq << 5; } static struct irq_chip iproc_msi_bottom_irq_chip = { @@ -251,7 +251,7 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain, void *args) { struct iproc_msi *msi = domain->host_data; - int hwirq; + int hwirq, i; mutex_lock(&msi->bitmap_lock); @@ -267,10 +267,14 @@ static int iproc_msi_irq_domain_alloc(struct irq_domain *domain, mutex_unlock(&msi->bitmap_lock); - irq_domain_set_info(domain, virq, hwirq, &iproc_msi_bottom_irq_chip, - domain->host_data, handle_simple_irq, NULL, NULL); + for (i = 0; i < nr_irqs; i++) { + irq_domain_set_info(domain, virq + i, hwirq + i, + &iproc_msi_bottom_irq_chip, + domain->host_data, handle_simple_irq, + NULL, NULL); + } - return 0; + return hwirq; } static void iproc_msi_irq_domain_free(struct irq_domain *domain, @@ -302,7 +306,8 @@ static inline u32 decode_msi_hwirq(struct iproc_msi *msi, u32 eq, u32 head) offs = iproc_msi_eq_offset(msi, eq) + head * sizeof(u32); msg = (u32 *)(msi->eq_cpu + offs); - hwirq = *msg & IPROC_MSI_EQ_MASK; + hwirq = readl(msg); + hwirq = (hwirq >> 5) + (hwirq & 0x1f); /* * Since we have multiple hwirq mapped to a single MSI vector, -- 2.15.0.rc0
Re: [PATCH 1/2] ARM: cpuidle: refine failure handling in init flow
On Mon, Sep 04, 2017 at 02:52:13PM +0800, Leo Yan wrote: > After applied Stefan Wahren patch ("ARM: cpuidle: Avoid memleak if init > fail") there have no memleak issue, but the code is not consistent to > handle initialization failure between driver registration and device > registration. And when device registration fails, it misses to > unregister the driver. > > So this patch is to refine failure handling in init flow, it adds two > 'goto' tags: when register device fails, it goto 'init_dev_fail' tag and > free 'dev' structure and unregister driver; when register driver fails, > it goto 'init_drv_fail' tag and free 'drv' structure. > > Cc: Daniel Lezcano > Cc: Stefan Wahren > Signed-off-by: Leo Yan Gentle ping... > --- > drivers/cpuidle/cpuidle-arm.c | 25 - > 1 file changed, 16 insertions(+), 9 deletions(-) > > diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c > index 52a7505..f419f6a 100644 > --- a/drivers/cpuidle/cpuidle-arm.c > +++ b/drivers/cpuidle/cpuidle-arm.c > @@ -86,10 +86,13 @@ static int __init arm_idle_init(void) > > for_each_possible_cpu(cpu) { > > + drv = NULL; > + dev = NULL; > + > drv = kmemdup(&arm_idle_driver, sizeof(*drv), GFP_KERNEL); > if (!drv) { > ret = -ENOMEM; > - goto out_fail; > + goto init_drv_fail; > } > > drv->cpumask = (struct cpumask *)cpumask_of(cpu); > @@ -104,13 +107,13 @@ static int __init arm_idle_init(void) > ret = dt_init_idle_driver(drv, arm_idle_state_match, 1); > if (ret <= 0) { > ret = ret ? : -ENODEV; > - goto init_fail; > + goto init_drv_fail; > } > > ret = cpuidle_register_driver(drv); > if (ret) { > pr_err("Failed to register cpuidle driver\n"); > - goto init_fail; > + goto init_drv_fail; > } > > /* > @@ -128,14 +131,14 @@ static int __init arm_idle_init(void) > > if (ret) { > pr_err("CPU %d failed to init idle CPU ops\n", cpu); > - goto out_fail; > + goto init_dev_fail; > } > > dev = kzalloc(sizeof(*dev), GFP_KERNEL); > if (!dev) { > pr_err("Failed to allocate cpuidle device\n"); > ret = -ENOMEM; > - goto out_fail; > + goto init_dev_fail; > } > dev->cpu = cpu; > > @@ -143,15 +146,19 @@ static int __init arm_idle_init(void) > if (ret) { > pr_err("Failed to register cpuidle device for CPU %d\n", > cpu); > - kfree(dev); > - goto out_fail; > + goto init_dev_fail; > } > } > > return 0; > -init_fail: > + > +init_dev_fail: > + kfree(dev); > + cpuidle_unregister_driver(drv); > + > +init_drv_fail: > kfree(drv); > -out_fail: > + > while (--cpu >= 0) { > dev = per_cpu(cpuidle_devices, cpu); > cpuidle_unregister_device(dev); > -- > 2.7.4 >
Re: [PATCH 2/2] ARM: cpuidle: replace cpuidle_get_driver with cpuidle_get_cpu_driver
On Mon, Sep 04, 2017 at 02:52:14PM +0800, Leo Yan wrote: > commit d50a7d8acd78 ("ARM: cpuidle: Support asymmetric idle definition") > supports multiple CPU idle driver so every CPU has its own driver. When > the initialization fails, the failure handling releases the resources > for every previous CPU; so it needs to retrieve every CPU device and > driver handler and unregister them. But the function > cpuidle_get_driver() can only return current CPU driver handler but not > the iterated CPU driver handler, so it cannot release resource properly. > > This patch is to replace cpuidle_get_driver() with > cpuidle_get_cpu_driver(), every CPU has its own device handler so this > function can get back correct driver handler for the CPU according to > the CPU device handler. By using this CPU driver handler we can release > resource properly. > > Cc: Daniel Lezcano > Cc: Stefan Wahren > Signed-off-by: Leo Yan > Fixes: d50a7d8acd78 ("ARM: cpuidle: Support asymmetric idle definition") Gentle ping. > --- > drivers/cpuidle/cpuidle-arm.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/drivers/cpuidle/cpuidle-arm.c b/drivers/cpuidle/cpuidle-arm.c > index f419f6a..ef34780 100644 > --- a/drivers/cpuidle/cpuidle-arm.c > +++ b/drivers/cpuidle/cpuidle-arm.c > @@ -161,9 +161,9 @@ static int __init arm_idle_init(void) > > while (--cpu >= 0) { > dev = per_cpu(cpuidle_devices, cpu); > + drv = cpuidle_get_cpu_driver(dev); > cpuidle_unregister_device(dev); > kfree(dev); > - drv = cpuidle_get_driver(); > cpuidle_unregister_driver(drv); > kfree(drv); > } > -- > 2.7.4 >
RE: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce userspace interface
> -Original Message- > From: Greg KH [mailto:g...@kroah.com] > Sent: Saturday, October 7, 2017 2:42 AM > To: Limonciello, Mario > Cc: dvh...@infradead.org; Andy Shevchenko ; > LKML ; platform-driver-...@vger.kernel.org; > Andy Lutomirski ; quasi...@google.com; > pali.ro...@gmail.com; r...@rjwysocki.net; mj...@google.com; h...@lst.de > Subject: Re: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce > userspace interface > > On Fri, Oct 06, 2017 at 11:59:58PM -0500, Mario Limonciello wrote: > > It's important for the driver to provide a R/W ioctl to ensure that > > two competing userspace processes don't race to provide or read each > > others data. > > > > This userspace character device will be used to perform SMBIOS calls > > from any applications. > > > > It provides an ioctl that will allow passing the WMI calling > > interface buffer between userspace and kernel space. > > > > This character device is intended to deprecate the dcdbas kernel module > > and the interface that it provides to userspace. > > > > To use the character device the buffer needed for the machine will > > also be needed. This information is exported to a sysfs attribute. > > > > The API for interacting with this interface is defined in documentation > > as well as a uapi header provides the format of the structures. > > > > Signed-off-by: Mario Limonciello > > --- > > Documentation/ABI/testing/dell-smbios-wmi | 41 > > .../ABI/testing/sysfs-platform-dell-smbios-wmi | 10 ++ > > MAINTAINERS| 1 + > > drivers/platform/x86/dell-smbios-wmi.c | 104 > > ++--- > > drivers/platform/x86/dell-smbios.h | 11 +-- > > include/uapi/linux/dell-smbios.h | 42 + > > 6 files changed, 188 insertions(+), 21 deletions(-) > > create mode 100644 Documentation/ABI/testing/dell-smbios-wmi > > create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios- > wmi > > create mode 100644 include/uapi/linux/dell-smbios.h > > > > diff --git a/Documentation/ABI/testing/dell-smbios-wmi > b/Documentation/ABI/testing/dell-smbios-wmi > > new file mode 100644 > > index ..e067e955fcc9 > > --- /dev/null > > +++ b/Documentation/ABI/testing/dell-smbios-wmi > > @@ -0,0 +1,41 @@ > > +What: /dev/wmi/dell-smbios > > +Date: November 2017 > > +KernelVersion: 4.15 > > +Contact: "Mario Limonciello" > > +Description: > > + Perform SMBIOS calls on supported Dell machines. > > + through the Dell ACPI-WMI interface. > > + > > + IOCTL's and buffer formats are defined in: > > + > > + > > + 1) To perform a call from userspace, you'll need to first > > + determine the minimum size of the calling interface buffer > > + for your machine. > > + Platforms that contain larger buffers can return larger > > + objects from the system firmware. > > + Commonly this size is either 4k or 32k. > > + > > + To determine the size of the buffer, refer to: > > + sysfs-platform-dell-smbios-wmi > > + > > + 2) After you've determined the minimum size of the calling > > + interface buffer, you can allocate a structure that represents > > + the structure documented above. > > + > > + 3) In the 'length' object store the size of the buffer you > > + determined above and allocated. > > + > > + 4) In this buffer object, prepare as necessary for the SMBIOS > > + call you're interested in. Typically SMBIOS buffers have > > + "class", "select", and "input" defined to values that coincide > > + with the data you are interested in. > > + Documenting class/select/input values is outside of the scope > > + of this documentation. Check with the libsmbios project for > > + further documentation on these values. > > + > > + 6) Run the call by using ioctl() as described in the header. > > + > > + 7) The output will be returned in the buffer object. > > + > > + 8) Be sure to free up your allocated object. > > diff --git a/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > b/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > > new file mode 100644 > > index ..6a0513703a3c > > --- /dev/null > > +++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios-wmi > > @@ -0,0 +1,10 @@ > > +What: /sys/devices/platform//buffer_size > > +Date: November 2017 > > +KernelVersion: 4.15 > > +Contact: "Mario Limonciello" > > +Description: > > + A read-only description of the size of a calling > > + interface buffer that can be passed to Dell > > + firmware. > > + > > + Commonly this size is either 4k or 32k. > > diff --git a/MAINTAINERS b/MAINTAINERS > > index 2a99
Re: [PATCH 0/2] Add support for Hi6220 coresight
Hi Stephen, Wei, On Thu, Aug 31, 2017 at 06:33:01PM -0700, Stephen Boyd wrote: > On 09/01, Leo Yan wrote: > > This patch series adds support for coresight on Hi6220; the first patch > > is to fix coresight PLL so can avoid system hang after we enable > > coresight, the second patch is to add DT binding according to coresight > > topology. > > > > The patch has been tested on Hikey; By using OpenCSD snapshot mode, it > > can successfully decode ETF and ETB trace data. > > > > I can take the first one and second one goes through arm-soc? Could you pick these two patches for Hi6220 coresight enabling for this merge window? Or need me resend these two patches? Thanks, Leo Yan
Re: [Patch v6 2/7] slimbus: Add messaging APIs to slimbus framework
On Sat, Oct 07, 2017 at 11:24:33AM +0100, Srinivas Kandagatla wrote: > Thanks for the comments. > > On 07/10/17 07:42, Jonathan Neuschäfer wrote: > > Hi, > > > > On Fri, Oct 06, 2017 at 05:51:31PM +0200, srinivas.kandaga...@linaro.org > > wrote: > > > From: Sagar Dharia [...] > > > +int slim_xfer_msg(struct slim_controller *ctrl, > > > + struct slim_device *sbdev, struct slim_val_inf *msg, > > > + u8 mc) > > > +{ > > > + DEFINE_SLIM_LDEST_TXN(txn_stack, mc, 6, sbdev->laddr, msg); > > > + struct slim_msg_txn *txn = &txn_stack; > > > + int ret; > > > + u16 sl, cur; > > > + > > > + ret = slim_val_inf_sanity(ctrl, msg, mc); > > > + if (ret) > > > + return ret; > > > + > > > + sl = slim_slicesize(msg->num_bytes); > > > + > > > + dev_dbg(&ctrl->dev, "SB xfer msg:os:%x, len:%d, MC:%x, sl:%x\n", > > > + msg->start_offset, msg->num_bytes, mc, sl); > > > + > > > + cur = slim_slicecodefromsize(sl); > > > + txn->ec = ((sl | (1 << 3)) | ((msg->start_offset & 0xFFF) << 4)); > > > > Shouldn't this be (cur | (1 << 3)? I misread the code here: I thought that cur was assigned the "compressed" message length (in the range 0..7) here, but that's not true, as slim_slicecodefromsize returns an "uncompressed" number. Thus cur is a "quantized"[1] version of msg->num_bytes. > cur seems to be redundant TBH, the only difference between cur and sl is > that the slim_slicesize() can give slice size to program for any lengths > between 1-16 bytes. However the slim_slicecodefromsize() can only handle > 1,2,3,4, 6,8,12,16 byte sizes. In any case, cur is only assigned and not used, as the code currently is. > So we can delete slim_slicecodefromsize() call and function together. > looks like it was a leftover from downstream. I agree. I don't know how it *might* be used, because I haven't read the SLIMbus spec, but it is unused here. > > (Also, what does cur mean? Cursor? Current?) > No Idea!! :-) it is supposed to return slice size as per number of bytes. Another problem solved by deleting slim_slicecodefromsize :-) (As a small side-note, I think slim_slicesize and slim_slicecodefromsize are named backwards: I would call sl, as used above, a "slice code", because it encodes the message length) > > > +/* > > > + * slim_request_val_element: change and request a given value element > > name should be fixed here.. Good catch. > > > + * @sb: client handle requesting elemental message reads, writes. > > > + * @msg: Input structure for start-offset, number of bytes to write. > > > + * context: can sleep > > > + * Returns: > > > + * -EINVAL: Invalid parameters > > > + * -ETIMEDOUT: If transmission of this message timed out (e.g. due to > > > bus lines > > > + * not being clocked or driven by controller) > > > + * -ENOTCONN: If the transmitted message was not ACKed by destination > > > device. > > > > Does rbuf contain the old value after this function finishes? > > > Yep, device should send a reply value with the old value with matching tid. I think you should document this in the comment to help readers. Thanks, Jonathan Neuschäfer [1]: https://en.wikipedia.org/wiki/Quantization signature.asc Description: PGP signature
Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
于 2017年10月5日 GMT+08:00 下午2:58:01, Kalle Valo 写到: >Icenowy Zheng writes: > >> 于 2017年10月4日 GMT+08:00 下午6:11:45, Maxime Ripard >> 写到: >>>On Wed, Oct 04, 2017 at 10:02:48AM +, Arend van Spriel wrote: On 10/4/2017 11:03 AM, Icenowy Zheng wrote: > > > 于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo > >>>写到: > > Icenowy Zheng writes: > > > > > Allwinner XR819 is a SDIO Wi-Fi chip, which has the >>>functionality to > > use > > > an out-of-band interrupt pin instead of SDIO in-band >interrupt. > > > > > > Add the device tree binding of this chip, in order to make it > > possible > > > to add this interrupt pin to device trees. > > > > > > Signed-off-by: Icenowy Zheng > > > Acked-by: Rob Herring > > > --- > > > Changes in v3: > > > - Renames the node name. > > > - Adds ACK from Rob. > > > Changes in v2: > > > - Removed status property in example. > > > - Added required property reg. > > > > > > .../bindings/net/wireless/allwinner,xr819.txt | 38 > > ++ > > > 1 file changed, 38 insertions(+) > > > create mode 100644 > > >>>Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt > > > > Like I asked already last time, AFAICS there is no upstream >xr819 > > wireless driver in drivers/net/wireless directory. Do we still >>>accept > > bindings like this for out-of-tree drivers? > > See esp8089. > > There's also no in-tree driver for it. The question is whether we should. The above might be a precedent, >>>but it may not necessarily be the way to go. The commit message for >esp8089 >>>seems to hint that there is intent to have an in-tree driver: """ Note that at this point there only is an out of tree driver for >>>this hardware, there is no clear timeline / path for merging this. >>>Still I believe it would be good to specify the binding for this in >>>tree now, so that any future migration to an in tree driver will not >>>cause compatiblity issues. Cc: Icenowy Zheng Signed-off-by: Hans de Goede Signed-off-by: Rob Herring """ Regardless the bindings are in principle independent of the kernel >>>and just describing hardware. I think there have been discussions to move >the bindings to their own repository, but apparently it was decided >>>otherwise. >>> >>>Yeah, I guess especially how it could be merged with the cw1200 >driver >>>would be very relevant to that commit log. >> >> The cw1200 driver seems to still have some legacy platform >> data. Maybe they should also be convert to DT. >> (Or maybe compatible = "allwinner,xr819" is enough, as >> xr819 is a specified variant of cw1200 family) > >Ah, so the upstream cw1200 driver supports xr819? Has anyone tested >that? Or does cw1200 more changes than just adding the DT support? I think the cw1200 driver currently lacks maintain, and the product is already discontinued by ST-E. > >-- >Kalle Valo > >___ >linux-arm-kernel mailing list >linux-arm-ker...@lists.infradead.org >http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
Re: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce userspace interface
On Sat, Oct 07, 2017 at 12:15:18PM +, mario.limoncie...@dell.com wrote: > > > + struct wmi_smbios_priv *priv; > > > + int ret = 0; > > > + size_t size; > > > + > > > + switch (cmd) { > > > + case DELL_WMI_SMBIOS_CMD: > > > + priv = dev_get_drvdata(&wdev->dev); > > > + if (!priv) > > > + return -ENODEV; > > > + size = sizeof(struct wmi_smbios_buffer); > > > + mutex_lock(&call_mutex); > > > + if (copy_from_user(priv->buf, input, size)) { Wait, how do you know that input is size big? > > > + dev_dbg(&wdev->dev, "Copy %lu from user failed\n", > > > + size); > > > + ret = -EFAULT; > > > + goto fail_smbios_cmd; > > > + } > > > + if (priv->buf->length < priv->buffer_size) { > > > + dev_err(&wdev->dev, > > > + "Buffer %lld too small, need at least %d\n", > > > + priv->buf->length, priv->buffer_size); > > > + ret = -EINVAL; > > > + goto fail_smbios_cmd; > > > + } > > > > No checking for too big of a length? Any other fields you should check > > for validity? Like too small? > > Too big is actually intentionally ignored. That seems "odd"... > I split the copy into two segments to check for this. > 1. First copy the size of the structure > (if userspace didn't allocate at least sizeof(struct wmi_smbios_buffer) > that's a problem) > 2. Verify the size claimed is "at least" what we internally are looking for. > 3. Copy the rest of the size internally needed. If userspace sent more it's > just not copied. > 4. When sending it back I only send back up to the "at least" internal size. That feels strange, are you sure this is correct? Why the odd two step process here? What if 'length' is set to an invalid value (too big or small), will you catch that correctly here? > > > + if (dell_smbios_call_filter(&wdev->dev, &priv->buf->std)) { > > > + dev_err(&wdev->dev, "Invalid call %d/%d:%8x\n", > > > + priv->buf->std.class, priv->buf->std.select, > > > + priv->buf->std.input[0]); > > > + ret = -EFAULT; > > > + goto fail_smbios_cmd; > > > + } > > > + size = priv->buffer_size - sizeof(struct wmi_smbios_buffer); > > > > What if size just went too small and wrapped around? :( > > > > Remember, "All input is evil". Go print that out and put it on the wall > > when you are designing this user/kernel api. You can trust no one, you > > have to validate _everything_. > > priv->buffer_size can't be set by userspace. Who sets it? Your structure naming here doesn't make it obvious which data is from the kernel and which from userspace, making this very hard to audit :( thanks, greg k-h
Re: [PATCH v5 13/14] platform/x86: wmi: create character devices when requested by drivers
On Sat, Oct 07, 2017 at 11:59:52AM +, mario.limoncie...@dell.com wrote: > > > > -Original Message- > > From: Greg KH [mailto:g...@kroah.com] > > Sent: Saturday, October 7, 2017 2:35 AM > > To: Limonciello, Mario > > Cc: dvh...@infradead.org; Andy Shevchenko ; > > LKML ; platform-driver-...@vger.kernel.org; > > Andy Lutomirski ; quasi...@google.com; > > pali.ro...@gmail.com; r...@rjwysocki.net; mj...@google.com; h...@lst.de > > Subject: Re: [PATCH v5 13/14] platform/x86: wmi: create character devices > > when > > requested by drivers > > > > On Fri, Oct 06, 2017 at 11:59:57PM -0500, Mario Limonciello wrote: > > > For WMI operations that are only Set or Query read or write sysfs > > > attributes created by WMI vendor drivers make sense. > > > > > > For other WMI operations that are run on Method, there needs to be a > > > way to guarantee to userspace that the results from the method call > > > belong to the data request to the method call. Sysfs attributes don't > > > work well in this scenario because two userspace processes may be > > > competing at reading/writing an attribute and step on each other's > > > data. > > > > > > When a WMI vendor driver declares an ioctl callback in the wmi_driver > > > the WMI bus driver will create a character device that maps to that > > > function. > > > > > > That character device will correspond to this path: > > > /dev/wmi/$driver > > > > > > The WMI bus driver will interpret the IOCTL calls, test them for > > > a valid instance and pass them on to the vendor driver to run. > > > > > > This creates an implicit policy that only driver per character > > > device. If a module matches multiple GUID's, the wmi_devices > > > will need to be all handled by the same wmi_driver if the same > > > character device is used. > > > > > > The WMI vendor drivers will be responsible for managing access to > > > this character device and proper locking on it. > > > > > > When a WMI vendor driver is unloaded the WMI bus driver will clean > > > up the character device. > > > > What prevents the vendor driver from being unloaded while the ioctl is > > being called in it? I don't see any protection here from that at all :( > > > > In my driver I take a mutex that blocks unloading while running ioctl, > but you mean you want one in the bus driver more generally, OK. No, you need to increment the module reference count before calling into it, which prevents anyone from even attempting to unload it. Much simpler than trying to rely on a mutex that is not obviously documented that this is what it does (hint, sharing a mutex across modules is not a good idea...) thanks, greg k-h
Re: [PATCH v5 08/14] platform/x86: dell-smbios: Add a sysfs interface for SMBIOS tokens
On Sat, Oct 07, 2017 at 11:56:04AM +, mario.limoncie...@dell.com wrote: > > -Original Message- > > From: Greg KH [mailto:g...@kroah.com] > > Sent: Saturday, October 7, 2017 1:54 AM > > To: Limonciello, Mario > > Cc: dvh...@infradead.org; Andy Shevchenko ; > > LKML ; platform-driver-...@vger.kernel.org; > > Andy Lutomirski ; quasi...@google.com; > > pali.ro...@gmail.com; r...@rjwysocki.net; mj...@google.com; h...@lst.de > > Subject: Re: [PATCH v5 08/14] platform/x86: dell-smbios: Add a sysfs > > interface for > > SMBIOS tokens > > > > On Fri, Oct 06, 2017 at 11:59:52PM -0500, Mario Limonciello wrote: > > > Currently userspace tools can access system tokens via the dcdbas > > > kernel module and a SMI call that will cause the platform to execute > > > SMM code. > > > > > > With a goal in mind of deprecating the dcdbas kernel module a different > > > method for accessing these tokens from userspace needs to be created. > > > > > > This is intentionally marked to only be readable as root as it can > > > contain sensitive information about the platform's configuration. > > > > > > MAINTAINERS was missing for this driver. Add myself and Pali to > > > maintainers list for it. > > > > > > Signed-off-by: Mario Limonciello > > > --- > > > .../ABI/testing/sysfs-platform-dell-smbios | 16 ++ > > > MAINTAINERS| 7 +++ > > > drivers/platform/x86/dell-smbios.c | 64 > > > ++ > > > 3 files changed, 87 insertions(+) > > > create mode 100644 Documentation/ABI/testing/sysfs-platform-dell-smbios > > > > > > diff --git a/Documentation/ABI/testing/sysfs-platform-dell-smbios > > b/Documentation/ABI/testing/sysfs-platform-dell-smbios > > > new file mode 100644 > > > index ..d97f4bd5bd91 > > > --- /dev/null > > > +++ b/Documentation/ABI/testing/sysfs-platform-dell-smbios > > > @@ -0,0 +1,16 @@ > > > +What:/sys/devices/platform//tokens > > > +Date:November 2017 > > > +KernelVersion: 4.15 > > > +Contact: "Mario Limonciello" > > > +Description: > > > + A read-only description of Dell platform tokens > > > + available on the machine. > > > + > > > + The tokens will be displayed in the following > > > + machine readable format with each token on a > > > + new line: > > > + > > > + ID Locationvalue > > > + > > > + For example token: > > > + 5 5 3 > > > > That's more than "one value per file" which is what sysfs requires, so > > this isn't acceptable, sorry. > > What's more acceptable to you to relay this information? > Binary sysfs attribute and export the structure format in a uapi? binary sysfs apis are for passing "raw" data to/from firmware/hardware to userspace, without the kernel knowing anything about the structure or format of the data. Putting the format in a structure in a uapi header kind of defeats that purpose :) > or a collection of sysfs files? > Eg: > tokens/$x/id > tokens/$x/location > tokens/$s/value > > I'm guessing the latter is the better way to go. Good guess :) thanks, greg k-h
[PATCH] Staging: rtl8723bs: core: rtw_cmd: Remove cast to pointer types in kfree
The cast to pointer types in kfree is not needed and can be dropped. Done using the following semantic by coccinelle. @r@ type T,P; T* x; @@ kfree( -(P *) x ) Signed-off-by: Srishti Sharma --- drivers/staging/rtl8723bs/core/rtw_cmd.c | 58 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/drivers/staging/rtl8723bs/core/rtw_cmd.c b/drivers/staging/rtl8723bs/core/rtw_cmd.c index 1843c44..e71e3ab 100644 --- a/drivers/staging/rtl8723bs/core/rtw_cmd.c +++ b/drivers/staging/rtl8723bs/core/rtw_cmd.c @@ -408,7 +408,7 @@ void rtw_free_cmd_obj(struct cmd_obj *pcmd) } /* free cmd_obj */ - kfree((unsigned char *)pcmd); + kfree(pcmd); } @@ -619,7 +619,7 @@ u8 rtw_sitesurvey_cmd(struct adapter *padapter, struct ndis_802_11_ssid *ssid, psurveyPara = rtw_zmalloc(sizeof(struct sitesurvey_parm)); if (psurveyPara == NULL) { - kfree((unsigned char *) ph2c); + kfree(ph2c); return _FAIL; } @@ -689,7 +689,7 @@ u8 rtw_setdatarate_cmd(struct adapter *padapter, u8 *rateset) pbsetdataratepara = rtw_zmalloc(sizeof(struct setdatarate_parm)); if (pbsetdataratepara == NULL) { - kfree((u8 *) ph2c); + kfree(ph2c); res = _FAIL; goto exit; } @@ -707,7 +707,7 @@ void rtw_getbbrfreg_cmdrsp_callback(struct adapter *padapter, struct cmd_obj *p { /* rtw_free_cmd_obj(pcmd); */ kfree((unsigned char *) pcmd->parmbuf); - kfree((unsigned char *) pcmd); + kfree(pcmd); } u8 rtw_createbss_cmd(struct adapter *padapter) @@ -847,7 +847,7 @@ u8 rtw_joinbss_cmd(struct adapter *padapter, struct wlan_network *pnetwork) psecnetwork = (struct wlan_bssid_ex *)&psecuritypriv->sec_bss; if (psecnetwork == NULL) { if (pcmd != NULL) - kfree((unsigned char *)pcmd); + kfree(pcmd); res = _FAIL; @@ -955,7 +955,7 @@ u8 rtw_disassoc_cmd(struct adapter *padapter, u32 deauth_timeout_ms, bool enqueu cmdobj = rtw_zmalloc(sizeof(*cmdobj)); if (cmdobj == NULL) { res = _FAIL; - kfree((u8 *)param); + kfree(param); goto exit; } init_h2fwcmd_w_parm_no_rsp(cmdobj, param, _DisConnect_CMD_); @@ -964,7 +964,7 @@ u8 rtw_disassoc_cmd(struct adapter *padapter, u32 deauth_timeout_ms, bool enqueu /* no need to enqueue, do the cmd hdl directly and free cmd parameter */ if (H2C_SUCCESS != disconnect_hdl(padapter, (u8 *)param)) res = _FAIL; - kfree((u8 *)param); + kfree(param); } exit: @@ -990,7 +990,7 @@ u8 rtw_setopmode_cmd(struct adapter *padapter, enum NDIS_802_11_NETWORK_INFRAST if (enqueue) { ph2c = rtw_zmalloc(sizeof(struct cmd_obj)); if (ph2c == NULL) { - kfree((u8 *)psetop); + kfree(psetop); res = _FAIL; goto exit; } @@ -999,7 +999,7 @@ u8 rtw_setopmode_cmd(struct adapter *padapter, enum NDIS_802_11_NETWORK_INFRAST res = rtw_enqueue_cmd(pcmdpriv, ph2c); } else{ setopmode_hdl(padapter, (u8 *)psetop); - kfree((u8 *)psetop); + kfree(psetop); } exit: return res; @@ -1042,15 +1042,15 @@ u8 rtw_setstakey_cmd(struct adapter *padapter, struct sta_info *sta, u8 unicast_ if (enqueue) { ph2c = rtw_zmalloc(sizeof(struct cmd_obj)); if (ph2c == NULL) { - kfree((u8 *) psetstakey_para); + kfree(psetstakey_para); res = _FAIL; goto exit; } psetstakey_rsp = rtw_zmalloc(sizeof(struct set_stakey_rsp)); if (psetstakey_rsp == NULL) { - kfree((u8 *) ph2c); - kfree((u8 *) psetstakey_para); + kfree(ph2c); + kfree(psetstakey_para); res = _FAIL; goto exit; } @@ -1061,7 +1061,7 @@ u8 rtw_setstakey_cmd(struct adapter *padapter, struct sta_info *sta, u8 unicast_ res = rtw_enqueue_cmd(pcmdpriv, ph2c); } else{ set_stakey_hdl(padapter, (u8 *)psetstakey_para); - kfree((u8 *) psetstakey_para); + kfree(psetstakey_para); } exit: return res; @@ -1091,15 +1091,15 @@ u8 rtw_clearstakey_cmd(struct adapter *padapter, struct sta_info *sta, u8 enqueu psetstakey_para = rtw_zmalloc(sizeof(struct set_stakey_parm)); if (psetstakey
Re: [PATCH] irqchip/gicv3-its: Add missing changes to support 52bit physical address
Hi Marc, Thanks for your quick review comments. On 10/07/2017 04:58 AM, Marc Zyngier wrote: > On Sat, Oct 07 2017 at 12:13:24 am BST, Shanker Donthineni > wrote: >> The current ITS driver works fine as long as normal memory and GICR >> regions are located within the lower 48bit (>=0 && <2^48) physical >> address space. Some of the registers GICR_PEND/PROP, GICR_VPEND/VPROP >> and GITS_CBASER are handled properly but not all when configuring >> the hardware with 52bit physical address. >> >> This patch does the following changes to support 52bit PA. >> -Handle 52bit PA in GITS_BASERn. >> -Fix ITT_addr width to 52bits, bits[51:8]. >> -Fix RDbase width to 52bits, bits[51:16]. >> -Fix VPT_addr width to 52bits, bits[51:16]. >> >> Definition of the GITS_BASERn register when ITS PageSize is 64KB: >> -Bits[47:16] of the register provide bits[47:16] of the table PA. >> -Bits[15:12] of the register provide bits[51:48] of the table PA. >> -Bits[15:00] of the base physical address are 0. >> >> Signed-off-by: Shanker Donthineni >> --- >> drivers/irqchip/irq-gic-v3-its.c | 24 +++- >> include/linux/irqchip/arm-gic-v3.h | 3 +++ >> 2 files changed, 22 insertions(+), 5 deletions(-) >> >> diff --git a/drivers/irqchip/irq-gic-v3-its.c >> b/drivers/irqchip/irq-gic-v3-its.c >> index e8d8934..e52c0da 100644 >> --- a/drivers/irqchip/irq-gic-v3-its.c >> +++ b/drivers/irqchip/irq-gic-v3-its.c >> @@ -308,7 +308,7 @@ static void its_encode_size(struct its_cmd_block *cmd, >> u8 size) >> >> static void its_encode_itt(struct its_cmd_block *cmd, u64 itt_addr) >> { >> -its_mask_encode(&cmd->raw_cmd[2], itt_addr >> 8, 50, 8); >> +its_mask_encode(&cmd->raw_cmd[2], itt_addr >> 8, 51, 8); >> } >> >> static void its_encode_valid(struct its_cmd_block *cmd, int valid) >> @@ -318,7 +318,7 @@ static void its_encode_valid(struct its_cmd_block *cmd, >> int valid) >> >> static void its_encode_target(struct its_cmd_block *cmd, u64 target_addr) >> { >> -its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 50, 16); >> +its_mask_encode(&cmd->raw_cmd[2], target_addr >> 16, 51, 16); >> } >> >> static void its_encode_collection(struct its_cmd_block *cmd, u16 col) >> @@ -358,7 +358,7 @@ static void its_encode_its_list(struct its_cmd_block >> *cmd, u16 its_list) >> >> static void its_encode_vpt_addr(struct its_cmd_block *cmd, u64 vpt_pa) >> { >> -its_mask_encode(&cmd->raw_cmd[3], vpt_pa >> 16, 50, 16); >> +its_mask_encode(&cmd->raw_cmd[3], vpt_pa >> 16, 51, 16); >> } >> >> static void its_encode_vpt_size(struct its_cmd_block *cmd, u8 vpt_size) >> @@ -1478,9 +1478,9 @@ static int its_setup_baser(struct its_node *its, >> struct its_baser *baser, >> u64 val = its_read_baser(its, baser); >> u64 esz = GITS_BASER_ENTRY_SIZE(val); >> u64 type = GITS_BASER_TYPE(val); >> +u64 baser_phys, tmp; >> u32 alloc_pages; >> void *base; >> -u64 tmp; >> >> retry_alloc_baser: >> alloc_pages = (PAGE_ORDER_TO_SIZE(order) / psz); >> @@ -1496,8 +1496,22 @@ static int its_setup_baser(struct its_node *its, >> struct its_baser *baser, >> if (!base) >> return -ENOMEM; >> >> +baser_phys = virt_to_phys(base); >> + >> +/* Check if the physical address of the memory is above 48bits */ >> +if (baser_phys & (~GITS_BASER_PHYS_MASK)) { >> +/* 52bit PA is supported only when PageSize=64K */ >> +if (psz != SZ_64K) { >> +free_pages((unsigned long)base, order); >> +return -EFAULT; >> +} >> + >> +/* Convert 52bit PA to 48bit field */ >> +baser_phys = GITS_BASER_64K_PHYS(baser_phys); >> +} > > I'm not sure this is completely right. What guarantees that the memory > you get is 64kB aligned? Your initial masking will do the wrong thing on > a system with 16kB pages, and everything will break from that point on, > as you're confusing the ITS page size with the CPU page size. > The first is check right, and the intention was to find out the upper bits of the PA (bits[51:12]) have non-zero value irrespective of PS 64K/16K/4KB. #define GITS_BASER_PHYS_MASKGENMASK_ULL(47, 12) (baser_phys & (~GITS_BASER_PHYS_MASK)) becomes (baser_phys & 0x ) We program GITS_BASERn PageSize based on 'psz' value. However the current code has a hidden bug, we assume memory allocation is aligned on 64K/16K when the ITS PageSize (variable of 'psz') is 64K/16K. We have two options to solve the problem. -fall-baclk to a lower ITS PageSize if the memory size is lower than 'psz'. -allocate minimum of 64K for all BASERn tables. Something like using the second method. retry_alloc_baser: alloc_pages = (min(PAGE_ORDER_TO_SIZE(order), 64K) / psz); baser_phys = virt_to_phys(base); /* Check if the physical address of the memory is above 48bits */ if (baser_phys & (~GITS_BASER_PHYS_MASK)) {
Re: [PATCH v3 1/2] dt-bindings: add device tree binding for Allwinner XR819 SDIO Wi-Fi
在 2017-10-05 14:58,Kalle Valo 写道: Icenowy Zheng writes: 于 2017年10月4日 GMT+08:00 下午6:11:45, Maxime Ripard 写到: On Wed, Oct 04, 2017 at 10:02:48AM +, Arend van Spriel wrote: On 10/4/2017 11:03 AM, Icenowy Zheng wrote: > > > 于 2017年10月4日 GMT+08:00 下午5:02:17, Kalle Valo 写到: > > Icenowy Zheng writes: > > > > > Allwinner XR819 is a SDIO Wi-Fi chip, which has the functionality to > > use > > > an out-of-band interrupt pin instead of SDIO in-band interrupt. > > > > > > Add the device tree binding of this chip, in order to make it > > possible > > > to add this interrupt pin to device trees. > > > > > > Signed-off-by: Icenowy Zheng > > > Acked-by: Rob Herring > > > --- > > > Changes in v3: > > > - Renames the node name. > > > - Adds ACK from Rob. > > > Changes in v2: > > > - Removed status property in example. > > > - Added required property reg. > > > > > > .../bindings/net/wireless/allwinner,xr819.txt | 38 > > ++ > > > 1 file changed, 38 insertions(+) > > > create mode 100644 > > Documentation/devicetree/bindings/net/wireless/allwinner,xr819.txt > > > > Like I asked already last time, AFAICS there is no upstream xr819 > > wireless driver in drivers/net/wireless directory. Do we still accept > > bindings like this for out-of-tree drivers? > > See esp8089. > > There's also no in-tree driver for it. The question is whether we should. The above might be a precedent, but it may not necessarily be the way to go. The commit message for esp8089 seems to hint that there is intent to have an in-tree driver: """ Note that at this point there only is an out of tree driver for this hardware, there is no clear timeline / path for merging this. Still I believe it would be good to specify the binding for this in tree now, so that any future migration to an in tree driver will not cause compatiblity issues. Cc: Icenowy Zheng Signed-off-by: Hans de Goede Signed-off-by: Rob Herring """ Regardless the bindings are in principle independent of the kernel and just describing hardware. I think there have been discussions to move the bindings to their own repository, but apparently it was decided otherwise. Yeah, I guess especially how it could be merged with the cw1200 driver would be very relevant to that commit log. The cw1200 driver seems to still have some legacy platform data. Maybe they should also be convert to DT. (Or maybe compatible = "allwinner,xr819" is enough, as xr819 is a specified variant of cw1200 family) Ah, so the upstream cw1200 driver supports xr819? Has anyone tested that? Or does cw1200 more changes than just adding the DT support? By doing some tests, XR819 is in the the CW1x60 family, which is not yet well supported by cw1200 driver. More work should be needed for support xr819 in cw1200 driver.
Re: [PATCH 3/3] mm: oom: show unreclaimable slab info when unreclaimable slabs > user memory
Hi Yang, [auto build test ERROR on mmotm/master] [also build test ERROR on v4.14-rc3 next-20170929] [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/Yang-Shi/oom-capture-unreclaimable-slab-info-in-oom-message/20171007-173639 base: git://git.cmpxchg.org/linux-mmotm.git master config: h8300-h8300h-sim_defconfig (attached as .config) compiler: h8300-linux-gcc (GCC) 6.2.0 reproduce: wget https://raw.githubusercontent.com/intel/lkp-tests/master/sbin/make.cross -O ~/bin/make.cross chmod +x ~/bin/make.cross # save the attached .config to linux build tree make.cross ARCH=h8300 All errors (new ones prefixed by >>): mm/slab_common.o: In function `dump_unreclaimable_slab': >> mm/slab_common.c:1298: undefined reference to `get_slabinfo' vim +1298 mm/slab_common.c 1272 1273 void dump_unreclaimable_slab(void) 1274 { 1275 struct kmem_cache *s, *s2; 1276 struct slabinfo sinfo; 1277 1278 /* 1279 * Here acquiring slab_mutex is risky since we don't prefer to get 1280 * sleep in oom path. But, without mutex hold, it may introduce a 1281 * risk of crash. 1282 * Use mutex_trylock to protect the list traverse, dump nothing 1283 * without acquiring the mutex. 1284 */ 1285 if (!mutex_trylock(&slab_mutex)) { 1286 pr_warn("excessive unreclaimable slab but cannot dump stats\n"); 1287 return; 1288 } 1289 1290 pr_info("Unreclaimable slab info:\n"); 1291 pr_info("Name Used Total\n"); 1292 1293 list_for_each_entry_safe(s, s2, &slab_caches, list) { 1294 if (!is_root_cache(s) || (s->flags & SLAB_RECLAIM_ACCOUNT)) 1295 continue; 1296 1297 memset(&sinfo, 0, sizeof(sinfo)); > 1298 get_slabinfo(s, &sinfo); 1299 1300 if (sinfo.num_objs > 0) 1301 pr_info("%-17s %10luKB %10luKB\n", cache_name(s), 1302 (sinfo.active_objs * s->size) / 1024, 1303 (sinfo.num_objs * s->size) / 1024); 1304 } 1305 mutex_unlock(&slab_mutex); 1306 } 1307 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH] kernel/power/swap: Delete five error messages for a failed memory allocation
From: Markus Elfring Date: Sat, 7 Oct 2017 14:54:06 +0200 Omit extra messages for a memory allocation failure in these functions. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- kernel/power/swap.c | 5 - 1 file changed, 5 deletions(-) diff --git a/kernel/power/swap.c b/kernel/power/swap.c index d7cdc426ee38..ba82113c05de 100644 --- a/kernel/power/swap.c +++ b/kernel/power/swap.c @@ -699,7 +699,6 @@ static int save_image_lzo(struct swap_map_handle *handle, data = vmalloc(sizeof(*data) * nr_threads); if (!data) { - printk(KERN_ERR "PM: Failed to allocate LZO data\n"); ret = -ENOMEM; goto out_clean; } @@ -708,7 +707,6 @@ static int save_image_lzo(struct swap_map_handle *handle, crc = kmalloc(sizeof(*crc), GFP_KERNEL); if (!crc) { - printk(KERN_ERR "PM: Failed to allocate crc\n"); ret = -ENOMEM; goto out_clean; } @@ -1190,14 +1188,12 @@ static int load_image_lzo(struct swap_map_handle *handle, page = vmalloc(sizeof(*page) * LZO_MAX_RD_PAGES); if (!page) { - printk(KERN_ERR "PM: Failed to allocate LZO page\n"); ret = -ENOMEM; goto out_clean; } data = vmalloc(sizeof(*data) * nr_threads); if (!data) { - printk(KERN_ERR "PM: Failed to allocate LZO data\n"); ret = -ENOMEM; goto out_clean; } @@ -1206,7 +1202,6 @@ static int load_image_lzo(struct swap_map_handle *handle, crc = kmalloc(sizeof(*crc), GFP_KERNEL); if (!crc) { - printk(KERN_ERR "PM: Failed to allocate crc\n"); ret = -ENOMEM; goto out_clean; } -- 2.14.2
RE: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce userspace interface
> -Original Message- > From: Greg KH [mailto:g...@kroah.com] > Sent: Saturday, October 7, 2017 7:37 AM > To: Limonciello, Mario > Cc: dvh...@infradead.org; andy.shevche...@gmail.com; linux- > ker...@vger.kernel.org; platform-driver-...@vger.kernel.org; l...@kernel.org; > quasi...@google.com; pali.ro...@gmail.com; r...@rjwysocki.net; > mj...@google.com; h...@lst.de > Subject: Re: [PATCH v5 14/14] platform/x86: dell-smbios-wmi: introduce > userspace interface > > On Sat, Oct 07, 2017 at 12:15:18PM +, mario.limoncie...@dell.com wrote: > > > > + struct wmi_smbios_priv *priv; > > > > + int ret = 0; > > > > + size_t size; > > > > + > > > > + switch (cmd) { > > > > + case DELL_WMI_SMBIOS_CMD: > > > > + priv = dev_get_drvdata(&wdev->dev); > > > > + if (!priv) > > > > + return -ENODEV; > > > > + size = sizeof(struct wmi_smbios_buffer); > > > > + mutex_lock(&call_mutex); > > > > + if (copy_from_user(priv->buf, input, size)) { > > Wait, how do you know that input is size big? How can you check this? I guess just read the first u64 for the data first (where length is stored). Or is there a better way? > > > > > + dev_dbg(&wdev->dev, "Copy %lu from user > > > > failed\n", > > > > + size); > > > > + ret = -EFAULT; > > > > + goto fail_smbios_cmd; > > > > + } > > > > + if (priv->buf->length < priv->buffer_size) { > > > > + dev_err(&wdev->dev, > > > > + "Buffer %lld too small, need at least > > > > %d\n", > > > > + priv->buf->length, priv->buffer_size); > > > > + ret = -EINVAL; > > > > + goto fail_smbios_cmd; > > > > + } > > > > > > No checking for too big of a length? Any other fields you should check > > > for validity? Like too small? > > > > Too big is actually intentionally ignored. > > That seems "odd"... > > > I split the copy into two segments to check for this. > > 1. First copy the size of the structure > > (if userspace didn't allocate at least sizeof(struct wmi_smbios_buffer) > > that's a > problem) > > 2. Verify the size claimed is "at least" what we internally are looking for. > > 3. Copy the rest of the size internally needed. If userspace sent more > > it's just > not copied. > > 4. When sending it back I only send back up to the "at least" internal size. > > That feels strange, are you sure this is correct? Why the odd two step > process here? > > What if 'length' is set to an invalid value (too big or small), will you > catch that correctly here? Too small is definitely caught, too big won't cause problems due to above logic. The remaining problem is if length doesn't really represent how much memory Userspace allocated. I'm not sure how you can actually check that. > > > > > + if (dell_smbios_call_filter(&wdev->dev, > > > > &priv->buf->std)) { > > > > + dev_err(&wdev->dev, "Invalid call %d/%d:%8x\n", > > > > + priv->buf->std.class, > > > > priv->buf->std.select, > > > > + priv->buf->std.input[0]); > > > > + ret = -EFAULT; > > > > + goto fail_smbios_cmd; > > > > + } > > > > + size = priv->buffer_size - sizeof(struct > > > > wmi_smbios_buffer); > > > > > > What if size just went too small and wrapped around? :( > > > > > > Remember, "All input is evil". Go print that out and put it on the wall > > > when you are designing this user/kernel api. You can trust no one, you > > > have to validate _everything_. > > > > priv->buffer_size can't be set by userspace. > > Who sets it? Your structure naming here doesn't make it obvious which > data is from the kernel and which from userspace, making this very hard > to audit :( It's set during the probe routine. I'll rename it to something more obvious. I'll also add some comments to the ioctl so it's more apparent.
Re: [for-next][PATCH 15/16] ftrace: Add freeing algorithm to free ftrace_mod_maps
On Fri, 6 Oct 2017 23:41:25 -0700 "Joel Fernandes (Google)" wrote: > Hi Steve, > > On Fri, Oct 6, 2017 at 11:07 AM, Steven Rostedt wrote: > > From: "Steven Rostedt (VMware)" > > > > The ftrace_mod_map is a descriptor to save module init function names in > > case they were traced, and the trace output needs to reference the function > > name from the function address. But after the function is unloaded, it > > the maps should be freed, as the rest of the function names are as well. > > Just checking for my understanding of this patch - wouldn't this also > mean that if there were any look ups of the init functions that may be > needed at trace output time, then those look ups wont be possible any > more after module is unloaded? Yes. That's true for all functions in the module. When a module is unloaded, all references to it in kallsyms is also freed. Try it on a current kernel. Trace a function in a module, then unload that module. The trace data will just show the ip hex address of the module function after that. > > I guess having a reference somehow on the ftrace_mod_map descriptor if > there are any entries in the trace buffer that need it can help > prevent that but it could be too expensive for not much return since > most likely the user wouldn't unload modules before trace collection > in normal usage. Right, I have thought about this, and I haven't come up with an inexpensive way to do this. As this has been the default operation of all module functions, and I haven't heard much complaining about it (I think I may have had a single complaint), I didn't put too much effort into it. I need to look at the approach that Jessica sent me. Perhaps there's ways to have all module function names be saved by a trace. But mapping which trace buffer has these names may be difficult. -- Steve
Re: [BUG] GPF on reboot of box
[ Replying from an actual computer this time ] On Fri, 6 Oct 2017 19:21:26 -0700 Nadav Amit wrote: > IIRC, there was a problem in rc1, which should be resolved in newer rcs. > If you need to run rc1, you can try to use the kernel parameter “nopcid". I noticed the bug when I rebased my work on top of rc3 and started testing that. I only mentioned rc1 because that's the first tagged release that had the bug in it. If it is fixed in a later release, then great! But I don't see that as the case (I haven't tested work on top of rc3). I'd like to have this fixed without having to do workarounds like adding nopcid to the kernel command line. If it is a bug for me, I'm sure it's going to be a bug for many others that wont know how to complain about it. -- Steve
Re: [PATCH v6 4/6] lib/dlock-list: Make sibling CPUs share the same linked list
Hi Waiman, [auto build test ERROR on linus/master] [also build test ERROR on v4.14-rc3] [cannot apply to next-20170929] [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/Waiman-Long/vfs-Use-dlock-list-for-SB-s-s_inodes-list/20171007-210724 config: i386-tinyconfig (attached as .config) compiler: gcc-6 (Debian 6.2.0-3) 6.2.0 20160901 reproduce: # save the attached .config to linux build tree make ARCH=i386 All error/warnings (new ones prefixed by >>): lib/dlock-list.c: In function 'cpu2idx_init': >> lib/dlock-list.c:75:16: error: assignment to expression with array type sibling_mask = topology_sibling_cpumask(cpu); ^ >> lib/dlock-list.c:76:7: warning: the address of 'sibling_mask' will always >> evaluate as 'true' [-Waddress] if (sibling_mask) { ^~~~ vim +75 lib/dlock-list.c 44 45 /* 46 * Initialize cpu2idx mapping table & nr_dlock_lists. 47 * 48 * It is possible that a dlock-list can be allocated before the cpu2idx is 49 * initialized. In this case, all the cpus are mapped to the first entry 50 * before initialization. 51 * 52 * All the sibling CPUs of a sibling group will map to the same dlock list so 53 * as to reduce the number of dlock lists to be maintained while minimizing 54 * cacheline contention. 55 * 56 * As the sibling masks are set up in the core initcall phase, this function 57 * has to be done in the postcore phase to get the right data. 58 */ 59 static int __init cpu2idx_init(void) 60 { 61 int idx, cpu; 62 cpumask_var_t sibling_mask; 63 static struct cpumask mask __initdata; 64 65 cpumask_clear(&mask); 66 idx = 0; 67 for_each_possible_cpu(cpu) { 68 int scpu; 69 70 if (cpumask_test_cpu(cpu, &mask)) 71 continue; 72 per_cpu(cpu2idx, cpu) = idx; 73 cpumask_set_cpu(cpu, &mask); 74 > 75 sibling_mask = topology_sibling_cpumask(cpu); > 76 if (sibling_mask) { 77 for_each_cpu(scpu, sibling_mask) { 78 per_cpu(cpu2idx, scpu) = idx; 79 cpumask_set_cpu(scpu, &mask); 80 } 81 } 82 idx++; 83 } 84 85 /* 86 * nr_dlock_lists can only be set after cpu2idx is properly 87 * initialized. 88 */ 89 smp_mb(); 90 nr_dlock_lists = idx; 91 pr_info("dlock-list: %d head entries per dlock list.\n", 92 nr_dlock_lists); 93 return 0; 94 } 95 postcore_initcall(cpu2idx_init); 96 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
[PATCH] Staging: media: atomisp: pci: Eliminate use of typedefs for struct
The use of typedefs for struct is discouraged, and hence can be eliminated. Done using the following semantic patch by coccinelle. @r1@ type T; @@ typedef struct {...} T; @script: python p@ T << r1.T; T1; @@ if T[-2:] == "_t" or T[-2:] == "_T": coccinelle.T1 = T[:-2] else: coccinelle.T1 = T print T, T1 @r2@ type r1.T; identifier p.T1; @@ - typedef struct + T1 { ... } - T ; @r3@ type r1.T; identifier p.T1; @@ - T + struct T1 Signed-off-by: Srishti Sharma --- .../media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c index d9178e8..6d9bceb 100644 --- a/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c +++ b/drivers/staging/media/atomisp/pci/atomisp2/css2400/runtime/spctrl/src/spctrl.c @@ -37,7 +37,7 @@ more details. #include "ia_css_spctrl.h" #include "ia_css_debug.h" -typedef struct { +struct spctrl_context_info { struct ia_css_sp_init_dmem_cfg dmem_config; uint32_tspctrl_config_dmem_addr; /** location of dmem_cfg in SP dmem */ uint32_tspctrl_state_dmem_addr; @@ -45,9 +45,9 @@ typedef struct { hrt_vaddresscode_addr; /* sp firmware location in host mem-DDR*/ uint32_tcode_size; char *program_name; /* used in case of PLATFORM_SIM */ -} spctrl_context_info; +}; -static spctrl_context_info spctrl_cofig_info[N_SP_ID]; +static struct spctrl_context_info spctrl_cofig_info[N_SP_ID]; static bool spctrl_loaded[N_SP_ID] = {0}; /* Load firmware */ -- 2.7.4
Re: [PATCH] platform/x86: mlx-platform: make a couple of structures static
On Thu, Oct 5, 2017 at 1:42 PM, Colin King wrote: > From: Colin Ian King > > The structures mlxplat_dev and mlxplat_hotplug are local to the source > and do not need to be in global scope, so make them static. > > Cleans up sparse warnings: > symbol 'mlxplat_dev' was not declared. Should it be static? > symbol 'mlxplat_hotplug' was not declared. Should it be static? Thanks for the patch. Since Vadim did some rather big driver changes I would like to hear from him how to proceed with this one: either I apply it now, or after we get Vadim's series in. -- With Best Regards, Andy Shevchenko
Re: [PATCH] RDMA/i40iw: Convert timers to use timer_setup()
On Fri, Oct 06, 2017 at 06:17:23PM -0500, Shiraz Saleem wrote: > On Wed, Oct 04, 2017 at 05:45:41PM -0700, Kees Cook wrote: > > In preparation for unconditionally passing the struct timer_list pointer to > > all timer callbacks, switch to using the new timer_setup() and from_timer() > > to pass the timer pointer explicitly. > > > > Cc: Faisal Latif > > Cc: Shiraz Saleem > > Cc: Doug Ledford > > Cc: Sean Hefty > > Cc: Hal Rosenstock > > Cc: linux-r...@vger.kernel.org > > Cc: Thomas Gleixner > > Signed-off-by: Kees Cook > > --- > > This requires commit 686fef928bba ("timer: Prepare to change timer > > callback argument type") in v4.14-rc3, but should be otherwise > > stand-alone. > > --- > > Patch looks ok. Did some minimal testing and looks good. > > Acked-by: Shiraz Saleem > Hi Kees, Sorry, I didnt notice this earlier, but, you made the change only to the stats timer to use the new timer init APIs. Can you do the same for the cm_timer and terminate_timer too for i40iw; so that things are consistent? [ssaleem@linbuild6081 i40iw]$ grep "setup_timer" * i40iw_cm.c: setup_timer(&cm_core->tcp_timer, i40iw_cm_timer_tick, i40iw_utils.c: setup_timer(&iwqp->terminate_timer, i40iw_terminate_timeout, i40iw_utils.c: setup_timer(&devstat->stats_timer, i40iw_hw_stats_timeout, Shiraz
[PATCH] kernel/trace: Delete five error messages for a failed memory allocation
From: Markus Elfring Date: Sat, 7 Oct 2017 16:10:06 +0200 Omit extra messages for a memory allocation failure in these functions. This issue was detected by using the Coccinelle software. Signed-off-by: Markus Elfring --- kernel/trace/trace_kprobe.c | 1 - kernel/trace/trace_probe.c| 5 ++--- kernel/trace/trace_selftest.c | 4 +--- kernel/trace/trace_uprobe.c | 2 -- 4 files changed, 3 insertions(+), 9 deletions(-) diff --git a/kernel/trace/trace_kprobe.c b/kernel/trace/trace_kprobe.c index 8a907e12b6b9..79a2ee2fc1f4 100644 --- a/kernel/trace/trace_kprobe.c +++ b/kernel/trace/trace_kprobe.c @@ -776,7 +776,6 @@ static int create_trace_kprobe(int argc, char **argv) } if (!parg->name) { - pr_info("Failed to allocate argument[%d] name.\n", i); ret = -ENOMEM; goto error; } diff --git a/kernel/trace/trace_probe.c b/kernel/trace/trace_probe.c index 52478f033f88..cf544d899eaa 100644 --- a/kernel/trace/trace_probe.c +++ b/kernel/trace/trace_probe.c @@ -545,10 +545,9 @@ int traceprobe_parse_probe_arg(char *arg, ssize_t *size, return -ENOSPC; } parg->comm = kstrdup(arg, GFP_KERNEL); - if (!parg->comm) { - pr_info("Failed to allocate memory for command '%s'.\n", arg); + if (!parg->comm) return -ENOMEM; - } + t = strchr(parg->comm, ':'); if (t) { arg[t - parg->comm] = '\0'; diff --git a/kernel/trace/trace_selftest.c b/kernel/trace/trace_selftest.c index cb917cebae29..6a2a57def182 100644 --- a/kernel/trace/trace_selftest.c +++ b/kernel/trace/trace_selftest.c @@ -250,10 +250,8 @@ static int trace_selftest_ops(struct trace_array *tr, int cnt) /* Add a dynamic probe */ dyn_ops = kzalloc(sizeof(*dyn_ops), GFP_KERNEL); - if (!dyn_ops) { - printk("MEMORY ERROR "); + if (!dyn_ops) goto out; - } dyn_ops->func = trace_selftest_test_dyn_func; diff --git a/kernel/trace/trace_uprobe.c b/kernel/trace/trace_uprobe.c index 4525e0271a53..9131f7372502 100644 --- a/kernel/trace/trace_uprobe.c +++ b/kernel/trace/trace_uprobe.c @@ -492,7 +492,6 @@ static int create_trace_uprobe(int argc, char **argv) tu->filename = kstrdup(filename, GFP_KERNEL); if (!tu->filename) { - pr_info("Failed to allocate filename.\n"); ret = -ENOMEM; goto error; } @@ -518,7 +517,6 @@ static int create_trace_uprobe(int argc, char **argv) } if (!parg->name) { - pr_info("Failed to allocate argument[%d] name.\n", i); ret = -ENOMEM; goto error; } -- 2.14.2
Re: [Part2 PATCH v5.1 12.2/31] crypto: ccp: Define SEV userspace ioctl and command id
On Fri, Oct 06, 2017 at 08:06:00PM -0500, Brijesh Singh wrote: > Add a include file which defines the ioctl and command id used for > issuing SEV platform management specific commands. > > Cc: Paolo Bonzini > Cc: "Radim Krčmář" > Cc: Borislav Petkov > Cc: Herbert Xu > Cc: Gary Hook > Cc: Tom Lendacky > Cc: linux-cry...@vger.kernel.org > Cc: k...@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Brijesh Singh > --- > include/uapi/linux/psp-sev.h | 115 > +++ > 1 file changed, 115 insertions(+) > create mode 100644 include/uapi/linux/psp-sev.h First of all, thanks for splitting the patch - it is much easier to review this way. Then, this patch should be 12.1, i.e., the first of the split because otherwise the previous one - which should be the next - fails building due to drivers/crypto/ccp/psp-dev.c:26:32: fatal error: uapi/linux/psp-sev.h: No such file or directory #include ^ Just swap them in their order. Also, those SEV commands should be __packed, see below. With that addressed: Reviewed-by: Borislav Petkov --- diff --git a/include/uapi/linux/psp-sev.h b/include/uapi/linux/psp-sev.h index a385bf2b8d2a..b63e116f18c1 100644 --- a/include/uapi/linux/psp-sev.h +++ b/include/uapi/linux/psp-sev.h @@ -53,7 +53,7 @@ struct sev_user_data_status { __u32 config; /* Out */ __u8 build; /* Out */ __u32 guest_count; /* Out */ -}; +} __packed; /** * struct sev_user_data_pek_csr - PEK_CSR command parameters @@ -64,7 +64,7 @@ struct sev_user_data_status { struct sev_user_data_pek_csr { __u64 address; /* In */ __u32 length; /* In/Out */ -}; +} __packed; /** * struct sev_user_data_cert_import - PEK_CERT_IMPORT command parameters @@ -79,7 +79,7 @@ struct sev_user_data_pek_cert_import { __u32 pek_cert_len; /* In */ __u64 oca_cert_address; /* In */ __u32 oca_cert_len; /* In */ -}; +} __packed; /** * struct sev_user_data_pdh_cert_export - PDH_CERT_EXPORT command parameters @@ -94,7 +94,7 @@ struct sev_user_data_pdh_cert_export { __u32 pdh_cert_len; /* In/Out */ __u64 cert_chain_address; /* In */ __u32 cert_chain_len; /* In/Out */ -}; +} __packed; /** * struct sev_issue_cmd - SEV ioctl parameters @@ -107,7 +107,7 @@ struct sev_issue_cmd { __u32 cmd; /* In */ __u64 data; /* In */ __u32 error;/* Out */ -}; +} __packed; #define SEV_IOC_TYPE 'S' #define SEV_ISSUE_CMD _IOWR(SEV_IOC_TYPE, 0x0, struct sev_issue_cmd) -- Regards/Gruss, Boris. SUSE Linux GmbH, GF: Felix Imendörffer, Jane Smithard, Graham Norton, HRB 21284 (AG Nürnberg) --
[PATCH] cifs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/cifs/cifsacl.c b/fs/cifs/cifsacl.c --- a/fs/cifs/cifsacl.c +++ b/fs/cifs/cifsacl.c @@ -370,7 +370,7 @@ sid_to_id(struct cifs_sb_info *cifs_sb, else is_group = false; - if (is_well_known_sid(psid, &unix_id, is_group) == false) + if (!is_well_known_sid(psid, &unix_id, is_group)) goto try_upcall_to_get_id; if (is_group) { diff -u -p a/fs/cifs/cifssmb.c b/fs/cifs/cifssmb.c --- a/fs/cifs/cifssmb.c +++ b/fs/cifs/cifssmb.c @@ -4508,7 +4508,7 @@ findFirstRetry: psrch_inf->unicode = false; psrch_inf->ntwrk_buf_start = (char *)pSMBr; - psrch_inf->smallBuf = 0; + psrch_inf->smallBuf = false; psrch_inf->srch_entries_start = (char *) &pSMBr->hdr.Protocol + le16_to_cpu(pSMBr->t2.DataOffset); @@ -4642,7 +4642,7 @@ int CIFSFindNext(const unsigned int xid, cifs_buf_release(psrch_inf->ntwrk_buf_start); psrch_inf->srch_entries_start = response_data; psrch_inf->ntwrk_buf_start = (char *)pSMB; - psrch_inf->smallBuf = 0; + psrch_inf->smallBuf = false; if (parms->EndofSearch) psrch_inf->endOfSearch = true; else diff -u -p a/fs/cifs/connect.c b/fs/cifs/connect.c --- a/fs/cifs/connect.c +++ b/fs/cifs/connect.c @@ -1081,7 +1081,7 @@ static int cifs_parse_security_flavors(c break; #endif case Opt_sec_none: - vol->nullauth = 1; + vol->nullauth = true; break; default: cifs_dbg(VFS, "bad security option: %s\n", value); @@ -1265,9 +1265,9 @@ cifs_parse_mount_options(const char *mou /* vol->retry default is 0 (i.e. "soft" limited retry not hard retry) */ /* default is always to request posix paths. */ - vol->posix_paths = 1; + vol->posix_paths = true; /* default to using server inode numbers where available */ - vol->server_ino = 1; + vol->server_ino = true; /* default is to use strict cifs caching semantics */ vol->strict_io = true; @@ -1333,10 +1333,10 @@ cifs_parse_mount_options(const char *mou /* Boolean values */ case Opt_user_xattr: - vol->no_xattr = 0; + vol->no_xattr = false; break; case Opt_nouser_xattr: - vol->no_xattr = 1; + vol->no_xattr = true; break; case Opt_forceuid: override_uid = 1; @@ -1351,22 +1351,22 @@ cifs_parse_mount_options(const char *mou override_gid = 0; break; case Opt_noblocksend: - vol->noblocksnd = 1; + vol->noblocksnd = true; break; case Opt_noautotune: - vol->noautotune = 1; + vol->noautotune = true; break; case Opt_hard: - vol->retry = 1; + vol->retry = true; break; case Opt_soft: - vol->retry = 0; + vol->retry = false; break; case Opt_perm: - vol->noperm = 0; + vol->noperm = false; break; case Opt_noperm: - vol->noperm = 1; + vol->noperm = true; break; case Opt_mapchars: vol->sfu_remap = true; @@ -1383,31 +1383,31 @@ cifs_parse_mount_options(const char *mou vol->remap = false; break; case Opt_sfu: - vol->sfu_emul = 1; + vol->sfu_emul = true; break; case Opt_nosfu: - vol->sfu_emul = 0; + vol->sfu_emul = false; break; case Opt_nodfs: - vol->nodfs = 1; + vol->nodfs = true; break; case Opt_posixpaths: - vol->posix_paths = 1; + vol->posix_paths = true; break; case Opt_noposixpaths: - vol->posix_pat
[PATCH] afs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/afs/cache.c b/fs/afs/cache.c --- a/fs/afs/cache.c +++ b/fs/afs/cache.c @@ -195,7 +195,7 @@ enum fscache_checkaux afs_vlocation_cach * VL record from the cache */ if (!vlocation->valid || vlocation->vldb.rtime == cvldb->rtime) { memcpy((uint8_t *)&vlocation->vldb.nservers, buffer, dlen); - vlocation->valid = 1; + vlocation->valid = true; _leave(" = SUCCESS [c->m]"); return FSCACHE_CHECKAUX_OKAY; } diff -u -p a/fs/afs/super.c b/fs/afs/super.c --- a/fs/afs/super.c +++ b/fs/afs/super.c @@ -211,7 +211,7 @@ static int afs_parse_options(struct afs_ break; case afs_opt_rwpath: - params->rwpath = 1; + params->rwpath = true; break; case afs_opt_vol: @@ -219,7 +219,7 @@ static int afs_parse_options(struct afs_ break; case afs_opt_autocell: - params->autocell = 1; + params->autocell = true; break; default: diff -u -p a/fs/afs/vlocation.c b/fs/afs/vlocation.c --- a/fs/afs/vlocation.c +++ b/fs/afs/vlocation.c @@ -154,7 +154,7 @@ out: printk(KERN_NOTICE "kAFS:" " Active volume no longer valid '%s'\n", vl->vldb.name); - vl->valid = 0; + vl->valid = false; ret = -ENOMEDIUM; }
[PATCH] selinux: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/security/selinux/ss/mls.c b/security/selinux/ss/mls.c --- a/security/selinux/ss/mls.c +++ b/security/selinux/ss/mls.c @@ -550,7 +550,7 @@ int mls_compute_sid(struct context *scon /* Fallthrough */ case AVTAB_CHANGE: - if ((tclass == policydb.process_class) || (sock == true)) + if ((tclass == policydb.process_class) || (sock)) /* Use the process MLS attributes. */ return mls_context_cpy(newcontext, scontext); else diff -u -p a/security/selinux/ss/services.c b/security/selinux/ss/services.c --- a/security/selinux/ss/services.c +++ b/security/selinux/ss/services.c @@ -1656,7 +1656,7 @@ static int security_compute_sid(u32 ssid } else if (cladatum && cladatum->default_role == DEFAULT_TARGET) { newcontext.role = tcontext->role; } else { - if ((tclass == policydb.process_class) || (sock == true)) + if ((tclass == policydb.process_class) || (sock)) newcontext.role = scontext->role; else newcontext.role = OBJECT_R_VAL; @@ -1668,7 +1668,7 @@ static int security_compute_sid(u32 ssid } else if (cladatum && cladatum->default_type == DEFAULT_TARGET) { newcontext.type = tcontext->type; } else { - if ((tclass == policydb.process_class) || (sock == true)) { + if ((tclass == policydb.process_class) || (sock)) { /* Use the type of process. */ newcontext.type = scontext->type; } else {
[PATCH] ext4: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/ext4/extents.c b/fs/ext4/extents.c --- a/fs/ext4/extents.c +++ b/fs/ext4/extents.c @@ -5242,7 +5242,7 @@ ext4_ext_shift_path_extents(struct ext4_ { int depth, err = 0; struct ext4_extent *ex_start, *ex_last; - bool update = 0; + bool update = false; depth = path->p_depth; while (depth >= 0) { @@ -5258,7 +5258,7 @@ ext4_ext_shift_path_extents(struct ext4_ goto out; if (ex_start == EXT_FIRST_EXTENT(path[depth].p_hdr)) - update = 1; + update = true; while (ex_start <= ex_last) { if (SHIFT == SHIFT_LEFT) {
[PATCH] xfs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/xfs/libxfs/xfs_bmap.c b/fs/xfs/libxfs/xfs_bmap.c --- a/fs/xfs/libxfs/xfs_bmap.c +++ b/fs/xfs/libxfs/xfs_bmap.c @@ -1490,14 +1490,14 @@ xfs_bmap_isaeof( int is_empty; int error; - bma->aeof = 0; + bma->aeof = false; error = xfs_bmap_last_extent(NULL, bma->ip, whichfork, &rec, &is_empty); if (error) return error; if (is_empty) { - bma->aeof = 1; + bma->aeof = true; return 0; } diff -u -p a/fs/xfs/libxfs/xfs_ialloc.c b/fs/xfs/libxfs/xfs_ialloc.c --- a/fs/xfs/libxfs/xfs_ialloc.c +++ b/fs/xfs/libxfs/xfs_ialloc.c @@ -1962,7 +1962,7 @@ xfs_difree_inobt( if (!(mp->m_flags & XFS_MOUNT_IKEEP) && rec.ir_free == XFS_INOBT_ALL_FREE && mp->m_sb.sb_inopblock <= XFS_INODES_PER_CHUNK) { - xic->deleted = 1; + xic->deleted = true; xic->first_ino = XFS_AGINO_TO_INO(mp, agno, rec.ir_startino); xic->alloc = xfs_inobt_irec_to_allocmask(&rec); @@ -1989,7 +1989,7 @@ xfs_difree_inobt( xfs_difree_inode_chunk(mp, agno, &rec, dfops); } else { - xic->deleted = 0; + xic->deleted = false; error = xfs_inobt_update(cur, &rec); if (error) { diff -u -p a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c --- a/fs/xfs/xfs_file.c +++ b/fs/xfs/xfs_file.c @@ -761,7 +761,7 @@ xfs_file_fallocate( enum xfs_prealloc_flags flags = 0; uintiolock = XFS_IOLOCK_EXCL; loff_t new_size = 0; - booldo_file_insert = 0; + booldo_file_insert = false; if (!S_ISREG(inode->i_mode)) return -EINVAL; @@ -822,7 +822,7 @@ xfs_file_fallocate( error = -EINVAL; goto out_unlock; } - do_file_insert = 1; + do_file_insert = true; } else { flags |= XFS_PREALLOC_SET; diff -u -p a/fs/xfs/xfs_log.c b/fs/xfs/xfs_log.c --- a/fs/xfs/xfs_log.c +++ b/fs/xfs/xfs_log.c @@ -2515,7 +2515,7 @@ next_lv: if (lv) vecp = lv->lv_iovecp; } - if (record_cnt == 0 && ordered == false) { + if (record_cnt == 0 && !ordered) { if (!lv) return 0; break; diff -u -p a/fs/xfs/xfs_mount.c b/fs/xfs/xfs_mount.c --- a/fs/xfs/xfs_mount.c +++ b/fs/xfs/xfs_mount.c @@ -704,7 +704,7 @@ xfs_mountfs( xfs_set_maxicount(mp); /* enable fail_at_unmount as default */ - mp->m_fail_unmount = 1; + mp->m_fail_unmount = true; error = xfs_sysfs_init(&mp->m_kobj, &xfs_mp_ktype, NULL, mp->m_fsname); if (error)
[PATCH] bfq: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/block/bfq-iosched.c b/block/bfq-iosched.c --- a/block/bfq-iosched.c +++ b/block/bfq-iosched.c @@ -4986,7 +4986,7 @@ static ssize_t bfq_low_latency_store(str if (__data > 1) __data = 1; - if (__data == 0 && bfqd->low_latency != 0) + if (__data == 0 && bfqd->low_latency) bfq_end_wr(bfqd); bfqd->low_latency = __data;
[PATCH] ima: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/security/integrity/ima/ima_fs.c b/security/integrity/ima/ima_fs.c --- a/security/integrity/ima/ima_fs.c +++ b/security/integrity/ima/ima_fs.c @@ -32,7 +32,7 @@ bool ima_canonical_fmt; static int __init default_canonical_fmt_setup(char *str) { #ifdef __BIG_ENDIAN - ima_canonical_fmt = 1; + ima_canonical_fmt = true; #endif return 1; } diff -u -p a/security/integrity/ima/ima_policy.c b/security/integrity/ima/ima_policy.c --- a/security/integrity/ima/ima_policy.c +++ b/security/integrity/ima/ima_policy.c @@ -196,9 +196,9 @@ static int __init policy_setup(char *str if ((strcmp(p, "tcb") == 0) && !ima_policy) ima_policy = DEFAULT_TCB; else if (strcmp(p, "appraise_tcb") == 0) - ima_use_appraise_tcb = 1; + ima_use_appraise_tcb = true; else if (strcmp(p, "secure_boot") == 0) - ima_use_secure_boot = 1; + ima_use_secure_boot = true; } return 1; @@ -207,7 +207,7 @@ __setup("ima_policy=", policy_setup); static int __init default_appraise_policy_setup(char *str) { - ima_use_appraise_tcb = 1; + ima_use_appraise_tcb = true; return 1; } __setup("ima_appraise_tcb", default_appraise_policy_setup);
Cocci spatch "boolinit" - v4.14-rc1
Bool initializations should use true and false. Bool tests don't need comparisons. Found by coccinelle spatch "misc/boolinit.cocci" Run against version v4.14-rc1 P.S. If you find this email unwanted, set up a procmail rule junking on the header: X-Patch: Cocci
[PATCH] f2fs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/f2fs/data.c b/fs/f2fs/data.c --- a/fs/f2fs/data.c +++ b/fs/f2fs/data.c @@ -419,7 +419,7 @@ next: bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page; /* set submitted = 1 as a return value */ - fio->submitted = 1; + fio->submitted = true; inc_page_count(sbi, WB_DATA_TYPE(bio_page));
[PATCH] configfs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/configfs/file.c b/fs/configfs/file.c --- a/fs/configfs/file.c +++ b/fs/configfs/file.c @@ -166,7 +166,7 @@ configfs_read_bin_file(struct file *file retval = -ETXTBSY; goto out; } - buffer->read_in_progress = 1; + buffer->read_in_progress = true; if (buffer->needs_read_fill) { /* perform first read with buf == NULL to get extent */ @@ -325,7 +325,7 @@ configfs_write_bin_file(struct file *fil len = -ETXTBSY; goto out; } - buffer->write_in_progress = 1; + buffer->write_in_progress = true; /* buffer grows? */ if (*ppos + count > buffer->bin_buffer_size) { @@ -429,8 +429,8 @@ static int check_perm(struct inode * ino } mutex_init(&buffer->mutex); buffer->needs_read_fill = 1; - buffer->read_in_progress = 0; - buffer->write_in_progress = 0; + buffer->read_in_progress = false; + buffer->write_in_progress = false; buffer->ops = ops; file->private_data = buffer; goto Done; @@ -488,10 +488,10 @@ static int configfs_release_bin_file(str ssize_t len = 0; int ret; - buffer->read_in_progress = 0; + buffer->read_in_progress = false; if (buffer->write_in_progress) { - buffer->write_in_progress = 0; + buffer->write_in_progress = false; len = bin_attr->write(item, buffer->bin_buffer, buffer->bin_buffer_size);
[PATCH] pstore: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/pstore/ftrace.c b/fs/pstore/ftrace.c --- a/fs/pstore/ftrace.c +++ b/fs/pstore/ftrace.c @@ -148,7 +148,7 @@ void pstore_unregister_ftrace(void) mutex_lock(&pstore_ftrace_lock); if (pstore_ftrace_enabled) { unregister_ftrace_function(&pstore_ftrace_ops); - pstore_ftrace_enabled = 0; + pstore_ftrace_enabled = false; } mutex_unlock(&pstore_ftrace_lock);
[PATCH] NFS: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/nfs/callback_proc.c b/fs/nfs/callback_proc.c --- a/fs/nfs/callback_proc.c +++ b/fs/nfs/callback_proc.c @@ -439,7 +439,7 @@ static bool referring_call_exists(struct uint32_t nrclists, struct referring_call_list *rclists) { - bool status = 0; + bool status = false; int i, j; struct nfs4_session *session; struct nfs4_slot_table *tbl; diff -u -p a/fs/nfs/dir.c b/fs/nfs/dir.c --- a/fs/nfs/dir.c +++ b/fs/nfs/dir.c @@ -253,7 +253,7 @@ int nfs_readdir_search_for_pos(struct nf desc->cache_entry_index = index; return 0; out_eof: - desc->eof = 1; + desc->eof = true; return -EBADCOOKIE; } @@ -307,7 +307,7 @@ int nfs_readdir_search_for_cookie(struct if (array->eof_index >= 0) { status = -EBADCOOKIE; if (*desc->dir_cookie == array->last_cookie) - desc->eof = 1; + desc->eof = true; } out: return status; @@ -761,7 +761,7 @@ int nfs_do_filldir(nfs_readdir_descripto ent = &array->array[i]; if (!dir_emit(desc->ctx, ent->string.name, ent->string.len, nfs_compat_user_ino64(ent->ino), ent->d_type)) { - desc->eof = 1; + desc->eof = true; break; } desc->ctx->pos++; @@ -773,7 +773,7 @@ int nfs_do_filldir(nfs_readdir_descripto ctx->duped = 1; } if (array->eof_index >= 0) - desc->eof = 1; + desc->eof = true; kunmap(desc->page); cache_page_release(desc); @@ -873,7 +873,7 @@ static int nfs_readdir(struct file *file if (res == -EBADCOOKIE) { res = 0; /* This means either end of directory */ - if (*desc->dir_cookie && desc->eof == 0) { + if (*desc->dir_cookie && !desc->eof) { /* Or that the server has 'lost' a cookie */ res = uncached_readdir(desc); if (res == 0) diff -u -p a/fs/nfs/nfs4client.c b/fs/nfs/nfs4client.c --- a/fs/nfs/nfs4client.c +++ b/fs/nfs/nfs4client.c @@ -793,7 +793,7 @@ nfs4_find_client_sessionid(struct net *n spin_lock(&nn->nfs_client_lock); list_for_each_entry(clp, &nn->nfs_client_list, cl_share_link) { - if (nfs4_cb_match_client(addr, clp, minorversion) == false) + if (!nfs4_cb_match_client(addr, clp, minorversion)) continue; if (!nfs4_has_session(clp)) diff -u -p a/fs/nfs/pnfs.c b/fs/nfs/pnfs.c --- a/fs/nfs/pnfs.c +++ b/fs/nfs/pnfs.c @@ -1513,7 +1513,7 @@ pnfs_lseg_range_match(const struct pnfs_ if ((range->iomode == IOMODE_RW && ls_range->iomode != IOMODE_RW) || (range->iomode != ls_range->iomode && -strict_iomode == true) || +strict_iomode) || !pnfs_lseg_range_intersecting(ls_range, range)) return 0;
[PATCH] proc: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/proc/generic.c b/fs/proc/generic.c --- a/fs/proc/generic.c +++ b/fs/proc/generic.c @@ -325,7 +325,7 @@ static int proc_register(struct proc_dir write_lock(&proc_subdir_lock); dp->parent = dir; - if (pde_subdir_insert(dir, dp) == false) { + if (!pde_subdir_insert(dir, dp)) { WARN(1, "proc_dir_entry '%s/%s' already registered\n", dir->name, dp->name); write_unlock(&proc_subdir_lock);
[PATCH] nfsd: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/nfsd/nfs4proc.c b/fs/nfsd/nfs4proc.c --- a/fs/nfsd/nfs4proc.c +++ b/fs/nfsd/nfs4proc.c @@ -230,7 +230,7 @@ do_open_lookup(struct svc_rqst *rqstp, s if (!*resfh) return nfserr_jukebox; fh_init(*resfh, NFS4_FHSIZE); - open->op_truncate = 0; + open->op_truncate = false; if (open->op_create) { /* FIXME: check session persistence and pnfs flags. @@ -360,7 +360,7 @@ nfsd4_open(struct svc_rqst *rqstp, struc if (open->op_create && open->op_claim_type != NFS4_OPEN_CLAIM_NULL) return nfserr_inval; - open->op_created = 0; + open->op_created = false; /* * RFC5661 18.51.3 * Before RECLAIM_COMPLETE done, server should deny new lock @@ -1108,8 +1108,8 @@ nfsd4_copy(struct svc_rqst *rqstp, struc else { copy->cp_res.wr_bytes_written = bytes; copy->cp_res.wr_stable_how = NFS_UNSTABLE; - copy->cp_consecutive = 1; - copy->cp_synchronous = 1; + copy->cp_consecutive = true; + copy->cp_synchronous = true; gen_boot_verifier(©->cp_res.wr_verifier, SVC_NET(rqstp)); status = nfs_ok; } @@ -2476,7 +2476,7 @@ bool nfsd4_spo_must_allow(struct svc_rqs if (!cstate->minorversion) return false; - if (cstate->spo_must_allowed == true) + if (cstate->spo_must_allowed) return true; opiter = resp->opcnt; diff -u -p a/fs/nfsd/nfssvc.c b/fs/nfsd/nfssvc.c --- a/fs/nfsd/nfssvc.c +++ b/fs/nfsd/nfssvc.c @@ -292,7 +292,7 @@ static int nfsd_startup_net(int nrservs, ret = lockd_up(net); if (ret) goto out_socks; - nn->lockd_up = 1; + nn->lockd_up = true; } ret = nfs4_state_start_net(net); @@ -305,7 +305,7 @@ static int nfsd_startup_net(int nrservs, out_lockd: if (nn->lockd_up) { lockd_down(net); - nn->lockd_up = 0; + nn->lockd_up = false; } out_socks: nfsd_shutdown_generic(); @@ -319,7 +319,7 @@ static void nfsd_shutdown_net(struct net nfs4_state_shutdown_net(net); if (nn->lockd_up) { lockd_down(net); - nn->lockd_up = 0; + nn->lockd_up = false; } nn->nfsd_net_up = false; nfsd_shutdown_generic(); diff -u -p a/fs/nfsd/vfs.c b/fs/nfsd/vfs.c --- a/fs/nfsd/vfs.c +++ b/fs/nfsd/vfs.c @@ -1379,7 +1379,7 @@ do_nfsd_create(struct svc_rqst *rqstp, s && d_inode(dchild)->i_atime.tv_sec == v_atime && d_inode(dchild)->i_size == 0 ) { if (created) - *created = 1; + *created = true; break; } case NFS4_CREATE_EXCLUSIVE4_1: @@ -1387,7 +1387,7 @@ do_nfsd_create(struct svc_rqst *rqstp, s && d_inode(dchild)->i_atime.tv_sec == v_atime && d_inode(dchild)->i_size == 0 ) { if (created) - *created = 1; + *created = true; goto set_attr; } /* fallthru */ @@ -1404,7 +1404,7 @@ do_nfsd_create(struct svc_rqst *rqstp, s goto out_nfserr; } if (created) - *created = 1; + *created = true; nfsd_check_ignore_resizing(iap);
[PATCH] exofs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/exofs/super.c b/fs/exofs/super.c --- a/fs/exofs/super.c +++ b/fs/exofs/super.c @@ -116,7 +116,7 @@ static int parse_options(char *options, EXOFS_MIN_PID); return -EINVAL; } - s_pid = 1; + s_pid = true; break; case Opt_to: if (match_int(&args[0], &option))
Re: [f2fs-dev] [PATCH] f2fs: Fix bool initialization/comparison
Isn't this bogus? "bool" type in Linux kernel is a typedef to "_Bool" and true/false is defined as 1 and 0 by enum at include/linux/stddef.h. On Sat, Oct 7, 2017 at 11:02 PM, Thomas Meyer wrote: > Bool initializations should use true and false. Bool tests don't need > comparisons. > > Signed-off-by: Thomas Meyer > --- > > diff -u -p a/fs/f2fs/data.c b/fs/f2fs/data.c > --- a/fs/f2fs/data.c > +++ b/fs/f2fs/data.c > @@ -419,7 +419,7 @@ next: > bio_page = fio->encrypted_page ? fio->encrypted_page : fio->page; > > /* set submitted = 1 as a return value */ > - fio->submitted = 1; > + fio->submitted = true; > > inc_page_count(sbi, WB_DATA_TYPE(bio_page)); > > > -- > Check out the vibrant tech community on one of the world's most > engaging tech sites, Slashdot.org! http://sdm.link/slashdot > ___ > Linux-f2fs-devel mailing list > linux-f2fs-de...@lists.sourceforge.net > https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel
Re: [PATCH 1/2] RAS/CEC: Use the right length for "cec_disable"
On Tue, Oct 3, 2017 at 11:04 AM, Borislav Petkov wrote: > On Mon, Oct 02, 2017 at 05:42:56PM +0200, Thomas Gleixner wrote: >> On Mon, 2 Oct 2017, Borislav Petkov wrote: >> > From: Nicolas Iooss >> > >> > parse_cec_param() compares a string with "cec_disable" using only 7 >> > characters of the 11-character-long string. Fix the length. >> > >> > Signed-off-by: Nicolas Iooss >> > Fixes: 011d82611172 ("RAS: Add a Corrected Errors Collector") >> > Link: >> > http://lkml.kernel.org/r/20170903075440.30250-1-nicolas.iooss_li...@m4x.org >> > Signed-off-by: Borislav Petkov >> > --- >> > drivers/ras/cec.c | 2 +- >> > 1 file changed, 1 insertion(+), 1 deletion(-) >> > >> > diff --git a/drivers/ras/cec.c b/drivers/ras/cec.c >> > index d0e5d6ee882c..586c296d1538 100644 >> > --- a/drivers/ras/cec.c >> > +++ b/drivers/ras/cec.c >> > @@ -523,7 +523,7 @@ int __init parse_cec_param(char *str) >> > if (*str == '=') >> > str++; >> > >> > - if (!strncmp(str, "cec_disable", 7)) >> > + if (!strncmp(str, "cec_disable", 11)) >> >> This kind of issue happens over and over. So if you really want to use >> strncmp() then this should be: >> >> #define CEC_DISABLE "cec_disable" >> >> if (!strncmp(str, CEC_DISABLE, strlen(CEC_DISABLE)) >> >> or we get a proper helper for that. Though in case of comparing some string >> against a constant string strncmp() has no real advantage over strcmp() as >> the comparison is guaranteed to be bound by the string constant. > > Right. > > Nicolas, wanna address that? > > Thx. Hi, the only difference I see between strncmp+strlen and strcmp is that the first option compares a prefix ("does the string begin with cec_disable?") and the second one checks that strings are equals. In parse_cec_param() it does not seem to matter. I have seen that Thomas Gleixner has already committed a modified patch which uses strcmp(), which looks good to me. Thanks! Nicolas
Re: [PATCH v6 4/6] lib/dlock-list: Make sibling CPUs share the same linked list
Hi Waiman, [auto build test ERROR on linus/master] [also build test ERROR on v4.14-rc3] [cannot apply to next-20170929] [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/Waiman-Long/vfs-Use-dlock-list-for-SB-s-s_inodes-list/20171007-210724 config: i386-randconfig-i0-201740 (attached as .config) compiler: gcc-4.8 (Debian 4.8.4-1) 4.8.4 reproduce: # save the attached .config to linux build tree make ARCH=i386 All errors (new ones prefixed by >>): lib/dlock-list.c: In function 'cpu2idx_init': >> lib/dlock-list.c:75:16: error: incompatible types when assigning to type >> 'cpumask_var_t' from type 'const struct cpumask *' sibling_mask = topology_sibling_cpumask(cpu); ^ lib/dlock-list.c:76:7: warning: the address of 'sibling_mask' will always evaluate as 'true' [-Waddress] if (sibling_mask) { ^ vim +75 lib/dlock-list.c 44 45 /* 46 * Initialize cpu2idx mapping table & nr_dlock_lists. 47 * 48 * It is possible that a dlock-list can be allocated before the cpu2idx is 49 * initialized. In this case, all the cpus are mapped to the first entry 50 * before initialization. 51 * 52 * All the sibling CPUs of a sibling group will map to the same dlock list so 53 * as to reduce the number of dlock lists to be maintained while minimizing 54 * cacheline contention. 55 * 56 * As the sibling masks are set up in the core initcall phase, this function 57 * has to be done in the postcore phase to get the right data. 58 */ 59 static int __init cpu2idx_init(void) 60 { 61 int idx, cpu; 62 cpumask_var_t sibling_mask; 63 static struct cpumask mask __initdata; 64 65 cpumask_clear(&mask); 66 idx = 0; 67 for_each_possible_cpu(cpu) { 68 int scpu; 69 70 if (cpumask_test_cpu(cpu, &mask)) 71 continue; 72 per_cpu(cpu2idx, cpu) = idx; 73 cpumask_set_cpu(cpu, &mask); 74 > 75 sibling_mask = topology_sibling_cpumask(cpu); 76 if (sibling_mask) { 77 for_each_cpu(scpu, sibling_mask) { 78 per_cpu(cpu2idx, scpu) = idx; 79 cpumask_set_cpu(scpu, &mask); 80 } 81 } 82 idx++; 83 } 84 85 /* 86 * nr_dlock_lists can only be set after cpu2idx is properly 87 * initialized. 88 */ 89 smp_mb(); 90 nr_dlock_lists = idx; 91 pr_info("dlock-list: %d head entries per dlock list.\n", 92 nr_dlock_lists); 93 return 0; 94 } 95 postcore_initcall(cpu2idx_init); 96 --- 0-DAY kernel test infrastructureOpen Source Technology Center https://lists.01.org/pipermail/kbuild-all Intel Corporation .config.gz Description: application/gzip
Re: [PATCH 3.18 00/35] 3.18.74-stable review
On 10/07/2017 02:40 AM, Greg Kroah-Hartman wrote: On Fri, Oct 06, 2017 at 11:24:40AM +0200, Greg Kroah-Hartman wrote: This is the start of the stable review cycle for the 3.18.74 release. There are 35 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 Sun Oct 8 09:23:44 UTC 2017. Anything received after that time might be too late. The whole patch series can be found in one patch at: kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.74-rc1.gz As this also had build issues, -rc2 is now out: kernel.org/pub/linux/kernel/v3.x/stable-review/patch-3.18.74-rc2.gz Build results: total: 136 pass: 136 fail: 0 Qemu test results: total: 112 pass: 112 fail: 0 Details are available at http://kerneltests.org/builders. Guenter
Re: [PATCH 4.9 000/104] 4.9.54-stable review
On 10/07/2017 02:39 AM, Greg Kroah-Hartman wrote: On Fri, Oct 06, 2017 at 10:50:38AM +0200, Greg Kroah-Hartman wrote: This is the start of the stable review cycle for the 4.9.54 release. There are 104 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 Sun Oct 8 08:37:55 UTC 2017. Anything received after that time might be too late. The whole patch series can be found in one patch at: kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.54-rc1.gz Ok, that had issues, so -rc2 is now out: kernel.org/pub/linux/kernel/v4.x/stable-review/patch-4.9.54-rc2.gz Build results: total: 145 pass: 145 fail: 0 Qemu test results: total: 123 pass: 123 fail: 0 Details are available at http://kerneltests.org/builders. Guenter
Re: [PATCH] Userfaultfd: Add description for UFFD_FEATURE_SIGBUS
Hello Prakash, On Fri, Oct 06, 2017 at 07:52:20PM -0700, Prakash Sangappa wrote: > Userfaultfd feature UFFD_FEATURE_SIGBUS was merged recently and should > be available in Linux 4.14 release. This patch is for the manpage > changes documenting this API. > > Documents the following commit: > > commit 2d6d6f5a09a96cc1fec7ed992b825e05f64cb50e > Author: Prakash Sangappa > Date: Wed Sep 6 16:23:39 2017 -0700 > > mm: userfaultfd: add feature to request for a signal delivery > > Signed-off-by: Prakash Sangappa > --- > man2/ioctl_userfaultfd.2 | 9 + > man2/userfaultfd.2 | 17 + > 2 files changed, 26 insertions(+) > > diff --git a/man2/ioctl_userfaultfd.2 b/man2/ioctl_userfaultfd.2 > index 60fd29b..cfc65ae 100644 > --- a/man2/ioctl_userfaultfd.2 > +++ b/man2/ioctl_userfaultfd.2 > @@ -196,6 +196,15 @@ with the > flag set, > .BR memfd_create (2), > and so on. > +.TP > +.B UFFD_FEATURE_SIGBUS > +Since Linux 4.14, If this feature bit is set, no page-fault events( space after events? > +.B UFFD_EVENT_PAGEFAULT > +) will be delivered, instead a > +.B SIGBUS > +signal will be sent to the faulting process. Applications using this > +feature will not require the use of a userfaultfd monitor for handling > +page-fault events. > .IP > The returned > .I ioctls > diff --git a/man2/userfaultfd.2 b/man2/userfaultfd.2 > index 1741ee3..a033742 100644 > --- a/man2/userfaultfd.2 > +++ b/man2/userfaultfd.2 > @@ -172,6 +172,23 @@ or > .BR ioctl (2) > operations to resolve the page fault. > .PP > +Starting from Linux 4.14, if application sets > +.B UFFD_FEATURE_SIGBUS > +feature bit using > +.B UFFDIO_API > +.BR ioctl (2) > +, no page fault notification will be forwarded to > +the user-space, instead a > +.B SIGBUS > +signal is delivered to the faulting process. With this feature, > +userfaultfd can be used for robustness purpose to simply catch > +any access to areas within the registered address range that do not > +have pages allocated, without having to deal with page-fault events. ",without having to listen to userfaultfd events." may be more clear. > +No userfaultd monitor will be required for handling page faults. For ^ typo: userfaultfd > +example, this feature can be useful for applications that want to > +prevent the kernel from automatically allocating pages and filling > +holes in sparse files when the hole is accessed thru mapped address. > +.PP Maybe also mention that "The UFFD_FEATURE_SIGBUS feature is implicitly inherited through fork() if used in combination with UFFD_FEATURE_FORK."
[PATCH] btrfs: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/btrfs/extent-tree.c b/fs/btrfs/extent-tree.c --- a/fs/btrfs/extent-tree.c +++ b/fs/btrfs/extent-tree.c @@ -6958,7 +6958,7 @@ static int __btrfs_free_extent(struct bt BUG_ON(!is_data && refs_to_drop != 1); if (is_data) - skinny_metadata = 0; + skinny_metadata = false; ret = lookup_extent_backref(trans, info, path, &iref, bytenr, num_bytes, parent, @@ -9311,7 +9311,7 @@ out: * don't have it in the radix (like when we recover after a power fail * or unmount) so we don't leak memory. */ - if (!for_reloc && root_dropped == false) + if (!for_reloc && !root_dropped) btrfs_add_dead_root(root); if (err && err != -EAGAIN) btrfs_handle_fs_error(fs_info, err, NULL); diff -u -p a/fs/btrfs/file.c b/fs/btrfs/file.c --- a/fs/btrfs/file.c +++ b/fs/btrfs/file.c @@ -2046,7 +2046,7 @@ int btrfs_sync_file(struct file *file, l struct btrfs_trans_handle *trans; struct btrfs_log_ctx ctx; int ret = 0, err; - bool full_sync = 0; + bool full_sync = false; u64 len; /* diff -u -p a/fs/btrfs/inode.c b/fs/btrfs/inode.c --- a/fs/btrfs/inode.c +++ b/fs/btrfs/inode.c @@ -4439,9 +4439,9 @@ int btrfs_truncate_inode_items(struct bt int err = 0; u64 ino = btrfs_ino(BTRFS_I(inode)); u64 bytes_deleted = 0; - bool be_nice = 0; - bool should_throttle = 0; - bool should_end = 0; + bool be_nice = false; + bool should_throttle = false; + bool should_end = false; BUG_ON(new_size > 0 && min_type != BTRFS_EXTENT_DATA_KEY); @@ -4451,7 +4451,7 @@ int btrfs_truncate_inode_items(struct bt */ if (!btrfs_is_free_space_inode(BTRFS_I(inode)) && test_bit(BTRFS_ROOT_REF_COWS, &root->state)) - be_nice = 1; + be_nice = true; path = btrfs_alloc_path(); if (!path) @@ -4657,7 +4657,7 @@ delete: } else { break; } - should_throttle = 0; + should_throttle = false; if (found_extent && (test_bit(BTRFS_ROOT_REF_COWS, &root->state) || @@ -4676,11 +4676,11 @@ delete: if (be_nice) { if (truncate_space_check(trans, root, extent_num_bytes)) { - should_end = 1; + should_end = true; } if (btrfs_should_throttle_delayed_refs(trans, fs_info)) - should_throttle = 1; + should_throttle = true; } }
[PATCH] ceph: Fix bool initialization/comparison
Bool initializations should use true and false. Bool tests don't need comparisons. Signed-off-by: Thomas Meyer --- diff -u -p a/fs/ceph/caps.c b/fs/ceph/caps.c --- a/fs/ceph/caps.c +++ b/fs/ceph/caps.c @@ -1711,7 +1711,7 @@ void ceph_check_caps(struct ceph_inode_i /* if we are unmounting, flush any unused caps immediately. */ if (mdsc->stopping) - is_delayed = 1; + is_delayed = true; spin_lock(&ci->i_ceph_lock); @@ -3185,8 +3185,8 @@ static void handle_cap_flush_ack(struct int dirty = le32_to_cpu(m->dirty); int cleaned = 0; bool drop = false; - bool wake_ci = 0; - bool wake_mdsc = 0; + bool wake_ci = false; + bool wake_mdsc = false; list_for_each_entry_safe(cf, tmp_cf, &ci->i_cap_flush_list, i_list) { if (cf->tid == flush_tid)