Linaro-mm-sig

linaro-mm-sig@lists.linaro.org

10 participants
3232 discussions

[PATCH v4a 00/38] timers: Use timer_shutdown*() before freeing timers

by Steven Rostedt

Back in April, I posted an RFC patch set to help mitigate a common issue where a timer gets armed just before it is freed, and when the timer goes off, it crashes in the timer code without any evidence of who the culprit was. I got side tracked and never finished up on that patch set. Since this type of crash is still our #1 crash we are seeing in the field, it has become a priority again to finish it. The last version of that patch set is here: https://lore.kernel.org/all/20221104054053.431922658@goodmis.org/ I'm calling this version 4a as it only has obvious changes were the timer that is being shutdown is in the same function where it will be freed or released, as this series should be "safe" for adding. I'll be calling the other patches 4b for the next merge window. Patch 1 fixes an issue with sunrpc/xprt where it incorrectly uses del_singleshot_timer_sync() for something that is not a oneshot timer. As this will be converted to shutdown, this needs to be fixed first. Patches 2-4 changes existing timer_shutdown() functions used locally in ARM and some drivers to better namespace names. Patch 5 implements the new timer_shutdown() and timer_shutdown_sync() functions that disable re-arming the timer after they are called. Patches 6-28 change all the locations where there's a kfree(), kfree_rcu(), kmem_cache_free() and one call_rcu() call where the RCU function frees the timer (the workqueue patch) in the same function as the del_timer{,_sync}() is called on that timer, and there's no extra exit path between the del_timer and freeing of the timer. Patches 29-32 add timer_shutdown*() on on-stack timers that are about to be released at the end of the function. Patches 33-37 add timer_shutdown*() on module timers in the module exit code. Patch 38 simply converts an open coded "shutdown" code into timer_shutdown(), as a way timer_shutdown() disables the timer is by setting that timer function to NULL. Linus, I sorted the patches this way to let you see which you would think is safe to go into this -rc. I honestly believe that they are all safe, but that's just my own opinion. This series is here: git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git timers-start Head SHA1: f58b516a65bac76f1bfa00126856d6c6c3d24a40 Steven Rostedt (Google) (38): SUNRPC/xprt: Use del_timer_sync() instead of del_singleshot_timer_sync() ARM: spear: Do not use timer namespace for timer_shutdown() function clocksource/drivers/arm_arch_timer: Do not use timer namespace for timer_shutdown() function clocksource/drivers/sp804: Do not use timer namespace for timer_shutdown() function timers: Add timer_shutdown_sync() and timer_shutdown() to be called before freeing timers timers: sh: Use timer_shutdown_sync() before freeing timer timers: block: Use timer_shutdown_sync() before freeing timer timers: ACPI: Use timer_shutdown_sync() before freeing timer timers: atm: Use timer_shutdown_sync() before freeing timer timers: Bluetooth: Use timer_shutdown_sync() before freeing timer timers: drm: Use timer_shutdown_sync() before freeing timer timers: HID: Use timer_shutdown_sync() before freeing timer timers: Input: Use timer_shutdown_sync() before freeing timer timers: mISDN: Use timer_shutdown_sync() before freeing timer timers: leds: Use timer_shutdown_sync() before freeing timer timers: media: Use timer_shutdown_sync() before freeing timer timers: net: Use timer_shutdown_sync() before freeing timer timers: usb: Use timer_shutdown_sync() before freeing timer timers: nfc: pn533: Use timer_shutdown_sync() before freeing timer timers: pcmcia: Use timer_shutdown_sync() before freeing timer timers: scsi: Use timer_shutdown_sync() and timer_shutdown() before freeing timer timers: tty: Use timer_shutdown_sync() before freeing timer timers: ext4: Use timer_shutdown_sync() before freeing timer timers: fs/nilfs2: Use timer_shutdown_sync() before freeing timer timers: ALSA: Use timer_shutdown_sync() before freeing timer timers: jbd2: Use timer_shutdown() before freeing timer timers: sched/psi: Use timer_shutdown_sync() before freeing timer timers: workqueue: Use timer_shutdown_sync() before freeing timer random: use timer_shutdown_sync() for on stack timers timers: dma-buf: Use timer_shutdown_sync() for on stack timers timers: drm: Use timer_shutdown_sync() for on stack timers timers: media: Use timer_shutdown_sync() for on stack timers timers: s390/cmm: Use timer_shutdown_sync() before a module is released timers: atm: Use timer_shutdown_sync() before a module is released timers: hangcheck: Use timer_shutdown_sync() before a module is released timers: ipmi: Use timer_shutdown_sync() before a module is released timers: Input: Use timer_shutdown_sync() before a module is released timers: PM: Use timer_shutdown_sync() ---- .../RCU/Design/Requirements/Requirements.rst | 2 +- Documentation/core-api/local_ops.rst | 2 +- Documentation/kernel-hacking/locking.rst | 5 ++ arch/arm/mach-spear/time.c | 8 +-- arch/s390/mm/cmm.c | 4 +- arch/sh/drivers/push-switch.c | 2 +- block/blk-iocost.c | 2 +- block/blk-iolatency.c | 2 +- block/blk-throttle.c | 2 +- block/kyber-iosched.c | 2 +- drivers/acpi/apei/ghes.c | 2 +- drivers/atm/idt77105.c | 4 +- drivers/atm/idt77252.c | 4 +- drivers/atm/iphase.c | 2 +- drivers/base/power/wakeup.c | 7 +-- drivers/block/drbd/drbd_main.c | 2 +- drivers/block/loop.c | 2 +- drivers/block/sunvdc.c | 2 +- drivers/bluetooth/hci_bcsp.c | 2 +- drivers/bluetooth/hci_h5.c | 4 +- drivers/bluetooth/hci_qca.c | 4 +- drivers/char/hangcheck-timer.c | 4 +- drivers/char/ipmi/ipmi_msghandler.c | 2 +- drivers/char/random.c | 2 +- drivers/clocksource/arm_arch_timer.c | 12 ++-- drivers/clocksource/timer-sp804.c | 6 +- drivers/dma-buf/st-dma-fence.c | 2 +- drivers/gpu/drm/gud/gud_pipe.c | 2 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/hid/hid-wiimote-core.c | 2 +- drivers/input/keyboard/locomokbd.c | 2 +- drivers/input/keyboard/omap-keypad.c | 2 +- drivers/input/mouse/alps.c | 2 +- drivers/input/serio/hil_mlc.c | 2 +- drivers/isdn/hardware/mISDN/hfcmulti.c | 5 +- drivers/isdn/mISDN/l1oip_core.c | 4 +- drivers/isdn/mISDN/timerdev.c | 4 +- drivers/leds/trigger/ledtrig-pattern.c | 2 +- drivers/leds/trigger/ledtrig-transient.c | 2 +- drivers/media/pci/ivtv/ivtv-driver.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 18 +++--- drivers/media/usb/s2255/s2255drv.c | 4 +- drivers/net/ethernet/intel/i40e/i40e_main.c | 7 +-- drivers/net/ethernet/marvell/sky2.c | 2 +- drivers/net/ethernet/sun/sunvnet.c | 2 +- drivers/net/usb/sierra_net.c | 2 +- drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 2 +- drivers/net/wireless/intersil/hostap/hostap_ap.c | 2 +- drivers/net/wireless/marvell/mwifiex/main.c | 2 +- drivers/net/wireless/microchip/wilc1000/hif.c | 6 +- drivers/nfc/pn533/pn533.c | 2 +- drivers/nfc/pn533/uart.c | 2 +- drivers/pcmcia/bcm63xx_pcmcia.c | 2 +- drivers/pcmcia/electra_cf.c | 2 +- drivers/pcmcia/omap_cf.c | 2 +- drivers/pcmcia/pd6729.c | 4 +- drivers/pcmcia/yenta_socket.c | 4 +- drivers/scsi/qla2xxx/qla_edif.c | 4 +- drivers/staging/media/atomisp/i2c/atomisp-lm3554.c | 2 +- drivers/tty/n_gsm.c | 2 +- drivers/tty/sysrq.c | 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/serial/garmin_gps.c | 2 +- drivers/usb/serial/mos7840.c | 2 +- fs/ext4/super.c | 2 +- fs/jbd2/journal.c | 2 + fs/nilfs2/segment.c | 2 +- include/linux/timer.h | 64 +++++++++++++++++++--- kernel/sched/psi.c | 1 + kernel/time/timer.c | 64 ++++++++++++---------- kernel/workqueue.c | 4 +- net/802/garp.c | 2 +- net/802/mrp.c | 2 +- net/bridge/br_multicast.c | 6 +- net/bridge/br_multicast_eht.c | 4 +- net/core/gen_estimator.c | 2 +- net/core/neighbour.c | 2 + net/ipv4/inet_timewait_sock.c | 1 + net/ipv4/ipmr.c | 2 +- net/ipv6/ip6mr.c | 2 +- net/mac80211/mesh_pathtbl.c | 2 +- net/netfilter/ipset/ip_set_list_set.c | 2 +- net/netfilter/ipvs/ip_vs_lblc.c | 2 +- net/netfilter/ipvs/ip_vs_lblcr.c | 2 +- net/netfilter/xt_LED.c | 2 +- net/rxrpc/conn_object.c | 2 +- net/sched/cls_flow.c | 2 +- net/sunrpc/svc.c | 2 +- net/sunrpc/xprt.c | 2 +- net/tipc/discover.c | 2 +- net/tipc/monitor.c | 2 +- sound/i2c/other/ak4117.c | 2 +- sound/synth/emux/emux.c | 2 +- 93 files changed, 227 insertions(+), 169 deletions(-)

3 years, 1 month

[RFC][PATCH v3 00/33] timers: Use timer_shutdown*() before freeing timers

by Steven Rostedt

Back in April, I posted an RFC patch set to help mitigate a common issue where a timer gets armed just before it is freed, and when the timer goes off, it crashes in the timer code without any evidence of who the culprit was. I got side tracked and never finished up on that patch set. Since this type of crash is still our #1 crash we are seeing in the field, it has become a priority again to finish it. This is v3 of that patch set. Thomas Gleixner posted an untested version that makes timer->function NULL as the flag that it is shutdown. I took that code, tested it (fixed it up), added more comments, and changed the name to timer_shutdown_sync(). I also converted it to use WARN_ON_ONCE() instead of just WARN_ON() as Linus asked for. I then created a trivial coccinelle script to find where del_timer*() is called before being freed, and converted them all to timer_shutdown*() (There was a couple that still used del_timer() instead of del_timer_sync()). I also updated DEBUG_OBJECTS_TIMERS to check from where the timer is ever armed, to calling of timer_shutdown_sync(), and it will trigger if a timer is freed in between. The current way is to only check if the timer is armed, but that means it only triggers if the race condition is hit, and with experience, it's not run on enough machines to catch all of them. By triggering it from the time the timer is armed to the time it is shutdown, it catches all potential cases even if the race condition is not hit. I went though the result of the cocinelle script, and updated the locations. Some locations were caught by DEBUG_OBJECTS_TIMERS as the coccinelle script only checked for timers being freed in the same function as the del_timer*(). Ideally, I would have the first patch go into this rc cycle, which is mostly non functional as it will allow the other patches to come in via the respective subsystems in the next merge window. Changes since v2: https://lore.kernel.org/all/20221027150525.753064657@goodmis.org/ - Talking with Thomas Gleixner, he wanted a better name space and to remove the "del_" portion of the API. - Since there's now a shutdown interface that does not synchronize, to keep it closer to del_timer() and del_timer_sync(), the API is now: timer_shutdown() - same as del_timer() but deactivates the timer. timer_shutdown_sync() - same as del_timer_sync() but deactivates the timer. - Added a few more locations that got converted. git://git.kernel.org/pub/scm/linux/kernel/git/rostedt/linux-trace.git trace/timers Head SHA1: 25106f0bb7968b3e8c746a7853f44b51840746c3 Steven Rostedt (Google) (33): timers: Add timer_shutdown_sync() and timer_shutdown() to be called before freeing timers timers: s390/cmm: Use timer_shutdown_sync() before freeing timer timers: sh: Use timer_shutdown_sync() before freeing timer timers: block: Use timer_shutdown_sync() before freeing timer timers: ACPI: Use timer_shutdown_sync() before freeing timer timers: atm: Use timer_shutdown_sync() before freeing timer timers: PM: Use timer_shutdown_sync() timers: Bluetooth: Use timer_shutdown_sync() before freeing timer timers: hangcheck: Use timer_shutdown_sync() before freeing timer timers: ipmi: Use timer_shutdown_sync() before freeing timer random: use timer_shutdown_sync() before freeing timer timers: dma-buf: Use timer_shutdown_sync() before freeing timer timers: drm: Use timer_shutdown_sync() before freeing timer timers: HID: Use timer_shutdown_sync() before freeing timer timers: Input: Use timer_shutdown_sync() before freeing timer timers: mISDN: Use timer_shutdown_sync() before freeing timer timers: leds: Use timer_shutdown_sync() before freeing timer timers: media: Use timer_shutdown_sync() before freeing timer timers: net: Use timer_shutdown_sync() before freeing timer timers: usb: Use timer_shutdown_sync() before freeing timer timers: cgroup: Use timer_shutdown_sync() before freeing timer timers: workqueue: Use timer_shutdown_sync() before freeing timer timers: nfc: pn533: Use timer_shutdown_sync() before freeing timer timers: pcmcia: Use timer_shutdown_sync() before freeing timer timers: scsi: Use timer_shutdown_sync() and timer_shutdown() before freeing timer timers: tty: Use timer_shutdown_sync() before freeing timer timers: ext4: Use timer_shutdown_sync() before freeing timer timers: fs/nilfs2: Use timer_shutdown_sync() before freeing timer timers: ALSA: Use timer_shutdown_sync() before freeing timer timers: jbd2: Use timer_shutdown() before freeing timer timers: sched/psi: Use timer_shutdown_sync() before freeing timer timers: x86/mce: Use __init_timer() for resetting timers timers: Expand DEBUG_OBJECTS_TIMER to check if it ever was used ---- .../RCU/Design/Requirements/Requirements.rst | 2 +- Documentation/core-api/local_ops.rst | 2 +- Documentation/kernel-hacking/locking.rst | 5 + arch/s390/mm/cmm.c | 4 +- arch/sh/drivers/push-switch.c | 2 +- arch/x86/kernel/cpu/mce/core.c | 14 ++- block/blk-iocost.c | 2 +- block/blk-iolatency.c | 2 +- block/blk-stat.c | 2 +- block/blk-throttle.c | 2 +- block/kyber-iosched.c | 2 +- drivers/acpi/apei/ghes.c | 2 +- drivers/atm/idt77105.c | 4 +- drivers/atm/idt77252.c | 4 +- drivers/atm/iphase.c | 2 +- drivers/base/power/wakeup.c | 7 +- drivers/block/drbd/drbd_main.c | 2 +- drivers/block/loop.c | 2 +- drivers/block/sunvdc.c | 2 +- drivers/bluetooth/hci_bcsp.c | 2 +- drivers/bluetooth/hci_h5.c | 2 +- drivers/bluetooth/hci_qca.c | 4 +- drivers/char/hangcheck-timer.c | 4 +- drivers/char/ipmi/ipmi_msghandler.c | 2 +- drivers/char/ipmi/ipmi_ssif.c | 4 +- drivers/char/random.c | 2 +- drivers/dma-buf/st-dma-fence.c | 2 +- drivers/gpu/drm/gud/gud_pipe.c | 2 +- drivers/gpu/drm/i915/i915_sw_fence.c | 2 +- drivers/hid/hid-wiimote-core.c | 2 +- drivers/input/keyboard/locomokbd.c | 2 +- drivers/input/keyboard/omap-keypad.c | 2 +- drivers/input/mouse/alps.c | 2 +- drivers/input/serio/hil_mlc.c | 2 +- drivers/input/serio/hp_sdc.c | 2 +- drivers/isdn/hardware/mISDN/hfcmulti.c | 6 +- drivers/isdn/mISDN/l1oip_core.c | 4 +- drivers/isdn/mISDN/timerdev.c | 4 +- drivers/leds/trigger/ledtrig-activity.c | 2 +- drivers/leds/trigger/ledtrig-heartbeat.c | 2 +- drivers/leds/trigger/ledtrig-pattern.c | 2 +- drivers/leds/trigger/ledtrig-transient.c | 2 +- drivers/media/pci/ivtv/ivtv-driver.c | 2 +- drivers/media/usb/pvrusb2/pvrusb2-hdw.c | 18 ++-- drivers/media/usb/s2255/s2255drv.c | 4 +- drivers/net/ethernet/intel/i40e/i40e_main.c | 6 +- drivers/net/ethernet/marvell/sky2.c | 2 +- drivers/net/ethernet/sun/sunvnet.c | 2 +- drivers/net/usb/sierra_net.c | 2 +- drivers/net/wireless/intel/iwlwifi/iwl-dbg-tlv.c | 2 +- drivers/net/wireless/intersil/hostap/hostap_ap.c | 2 +- drivers/net/wireless/marvell/mwifiex/main.c | 2 +- drivers/net/wireless/microchip/wilc1000/hif.c | 8 +- drivers/nfc/pn533/pn533.c | 2 +- drivers/nfc/pn533/uart.c | 2 +- drivers/pcmcia/bcm63xx_pcmcia.c | 2 +- drivers/pcmcia/electra_cf.c | 2 +- drivers/pcmcia/omap_cf.c | 2 +- drivers/pcmcia/pd6729.c | 4 +- drivers/pcmcia/yenta_socket.c | 4 +- drivers/scsi/qla2xxx/qla_edif.c | 4 +- drivers/scsi/scsi_lib.c | 1 + drivers/staging/media/atomisp/i2c/atomisp-lm3554.c | 2 +- drivers/tty/n_gsm.c | 2 +- drivers/tty/sysrq.c | 2 +- drivers/usb/gadget/udc/m66592-udc.c | 2 +- drivers/usb/serial/garmin_gps.c | 2 +- drivers/usb/serial/mos7840.c | 2 +- fs/ext4/super.c | 2 +- fs/jbd2/journal.c | 2 + fs/nilfs2/segment.c | 2 +- include/linux/timer.h | 100 +++++++++++++++++-- include/linux/workqueue.h | 4 +- kernel/cgroup/cgroup.c | 2 +- kernel/sched/psi.c | 1 + kernel/time/timer.c | 106 ++++++++++++++------- kernel/workqueue.c | 4 +- net/802/garp.c | 2 +- net/802/mrp.c | 2 +- net/bridge/br_multicast.c | 6 +- net/bridge/br_multicast_eht.c | 4 +- net/core/gen_estimator.c | 2 +- net/core/neighbour.c | 2 + net/ipv4/inet_connection_sock.c | 2 +- net/ipv4/inet_timewait_sock.c | 3 +- net/ipv4/ipmr.c | 2 +- net/ipv6/ip6mr.c | 2 +- net/mac80211/mesh_pathtbl.c | 2 +- net/netfilter/ipset/ip_set_list_set.c | 2 +- net/netfilter/ipvs/ip_vs_lblc.c | 2 +- net/netfilter/ipvs/ip_vs_lblcr.c | 2 +- net/netfilter/xt_LED.c | 2 +- net/rxrpc/conn_object.c | 2 +- net/sched/cls_flow.c | 2 +- net/sunrpc/svc.c | 2 +- net/sunrpc/xprt.c | 2 +- net/tipc/discover.c | 2 +- net/tipc/monitor.c | 2 +- sound/i2c/other/ak4117.c | 2 +- sound/synth/emux/emux.c | 2 +- 100 files changed, 310 insertions(+), 175 deletions(-)

3 years, 1 month

[PATCH v3] dma-buf: fix racing conflict of dma_heap_add()

by Dawei Li

Racing conflict could be: task A task B list_for_each_entry strcmp(h->name)) list_for_each_entry strcmp(h->name) kzalloc kzalloc ...... ..... device_create device_create list_add list_add The root cause is that task B has no idea about the fact someone else(A) has inserted heap with same name when it calls list_add, so a potential collision occurs. v1: https://lore.kernel.org/all/TYCP286MB2323950197F60FC3473123B7CA349@TYCP286M… v1->v2: Narrow down locking scope, check the existence of heap before insertion, as suggested by Andrew Davis. v2->v3: Remove double checking. Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework") base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7 Signed-off-by: Dawei Li <set_pte_at(a)outlook.com> --- drivers/dma-buf/dma-heap.c | 29 +++++++++++++++-------------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 8f5848aa144f..7a25e98259ea 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -233,18 +233,6 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info) return ERR_PTR(-EINVAL); } - /* check the name is unique */ - mutex_lock(&heap_list_lock); - list_for_each_entry(h, &heap_list, list) { - if (!strcmp(h->name, exp_info->name)) { - mutex_unlock(&heap_list_lock); - pr_err("dma_heap: Already registered heap named %s\n", - exp_info->name); - return ERR_PTR(-EINVAL); - } - } - mutex_unlock(&heap_list_lock); - heap = kzalloc(sizeof(*heap), GFP_KERNEL); if (!heap) return ERR_PTR(-ENOMEM); @@ -283,13 +271,26 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info) err_ret = ERR_CAST(dev_ret); goto err2; } - /* Add heap to the list */ + mutex_lock(&heap_list_lock); + /* check the name is unique */ + list_for_each_entry(h, &heap_list, list) { + if (!strcmp(h->name, exp_info->name)) { + mutex_unlock(&heap_list_lock); + pr_err("dma_heap: Already registered heap named %s\n", + exp_info->name); + err_ret = ERR_PTR(-EINVAL); + goto err3; + } + } + + /* Add heap to the list */ list_add(&heap->list, &heap_list); mutex_unlock(&heap_list_lock); return heap; - +err3: + device_destroy(dma_heap_class, heap->heap_devt); err2: cdev_del(&heap->heap_cdev); err1: -- 2.25.1

3 years, 1 month

[PATCH] drm/msm: Remove exclusive-fence hack

by Rob Clark

From: Rob Clark <robdclark(a)chromium.org> The workaround was initially necessary due to dma_resv having only a single exclusive fence slot, yet whe don't necessarily know what order the gpu scheduler will schedule jobs. Unfortunately this workaround also has the result of forcing implicit sync, even when userspace does not want it. However, since commit 047a1b877ed4 ("dma-buf & drm/amdgpu: remove dma_resv workaround") the workaround is no longer needed. So remove it. This effectively reverts commit f1b3f696a084 ("drm/msm: Don't break exclusive fence ordering") Signed-off-by: Rob Clark <robdclark(a)chromium.org> --- drivers/gpu/drm/msm/msm_gem_submit.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/drivers/gpu/drm/msm/msm_gem_submit.c b/drivers/gpu/drm/msm/msm_gem_submit.c index 5599d93ec0d2..cc48f73adadf 100644 --- a/drivers/gpu/drm/msm/msm_gem_submit.c +++ b/drivers/gpu/drm/msm/msm_gem_submit.c @@ -334,8 +334,7 @@ static int submit_fence_sync(struct msm_gem_submit *submit, bool no_implicit) if (ret) return ret; - /* exclusive fences must be ordered */ - if (no_implicit && !write) + if (no_implicit) continue; ret = drm_sched_job_add_implicit_dependencies(&submit->base, -- 2.38.1

3 years, 1 month

[PATCH v2] dma-buf: fix racing conflict of dma_heap_add()

by Dawei Li

Racing conflict could be: task A task B list_for_each_entry strcmp(h->name)) list_for_each_entry strcmp(h->name) kzalloc kzalloc ...... ..... device_create device_create list_add list_add The root cause is that task B has no idea about the fact someone else(A) has inserted heap with same name when it calls list_add, so a potential collision occurs. v1: https://lore.kernel.org/all/TYCP286MB2323950197F60FC3473123B7CA349@TYCP286M… v1->v2: Narrow down locking scope, check the existence of heap before insertion, as suggested by Andrew Davis. Fixes: c02a81fba74f ("dma-buf: Add dma-buf heaps framework") base-commit: 447fb14bf07905b880c9ed1ea92c53d6dd0649d7 Signed-off-by: Dawei Li <set_pte_at(a)outlook.com> --- drivers/dma-buf/dma-heap.c | 37 ++++++++++++++++++++++++++++--------- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/drivers/dma-buf/dma-heap.c b/drivers/dma-buf/dma-heap.c index 8f5848aa144f..1c787a147e3a 100644 --- a/drivers/dma-buf/dma-heap.c +++ b/drivers/dma-buf/dma-heap.c @@ -216,9 +216,21 @@ const char *dma_heap_get_name(struct dma_heap *heap) return heap->name; } +static inline bool dma_heap_exist(const char *name) +{ + struct dma_heap *h; + + list_for_each_entry(h, &heap_list, list) { + if (!strcmp(h->name, name)) + return true; + } + + return false; +} + struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info) { - struct dma_heap *heap, *h, *err_ret; + struct dma_heap *heap, *err_ret; struct device *dev_ret; unsigned int minor; int ret; @@ -235,13 +247,11 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info) /* check the name is unique */ mutex_lock(&heap_list_lock); - list_for_each_entry(h, &heap_list, list) { - if (!strcmp(h->name, exp_info->name)) { - mutex_unlock(&heap_list_lock); - pr_err("dma_heap: Already registered heap named %s\n", - exp_info->name); - return ERR_PTR(-EINVAL); - } + if (dma_heap_exist(exp_info->name)) { + mutex_unlock(&heap_list_lock); + pr_err("dma_heap: Already registered heap named %s\n", + exp_info->name); + return ERR_PTR(-EINVAL); } mutex_unlock(&heap_list_lock); @@ -283,13 +293,22 @@ struct dma_heap *dma_heap_add(const struct dma_heap_export_info *exp_info) err_ret = ERR_CAST(dev_ret); goto err2; } + /* Add heap to the list */ mutex_lock(&heap_list_lock); + if (unlikely(dma_heap_exist(exp_info->name))) { + mutex_unlock(&heap_list_lock); + pr_err("dma_heap: Already registered heap named %s\n", + exp_info->name); + err_ret = ERR_PTR(-EINVAL); + goto err3; + } list_add(&heap->list, &heap_list); mutex_unlock(&heap_list_lock); return heap; - +err3: + device_destroy(dma_heap_class, heap->heap_devt); err2: cdev_del(&heap->heap_cdev); err1: -- 2.25.1

3 years, 1 month

[PATCH] drm/radeon: Replace kmap() with kmap_local_page()

by Fabio M. De Francesco

The use of kmap() is being deprecated in favor of kmap_local_page(). There are two main problems with kmap(): (1) It comes with an overhead as the mapping space is restricted and protected by a global lock for synchronization and (2) it also requires global TLB invalidation when the kmap’s pool wraps and it might block when the mapping space is fully utilized until a slot becomes available. With kmap_local_page() the mappings are per thread, CPU local, can take page faults, and can be called from any context (including interrupts). It is faster than kmap() in kernels with HIGHMEM enabled. Furthermore, the tasks can be preempted and, when they are scheduled to run again, the kernel virtual addresses are restored and still valid. Therefore, replace kmap() with kmap_local_page() in radeon_ttm_gtt_read(). Cc: "Venkataramanan, Anirudh" <anirudh.venkataramanan(a)intel.com> Suggested-by: Ira Weiny <ira.weiny(a)intel.com> Signed-off-by: Fabio M. De Francesco <fmdefrancesco(a)gmail.com> --- drivers/gpu/drm/radeon/radeon_ttm.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/drivers/gpu/drm/radeon/radeon_ttm.c b/drivers/gpu/drm/radeon/radeon_ttm.c index d33fec488713..bdb4c0e0736b 100644 --- a/drivers/gpu/drm/radeon/radeon_ttm.c +++ b/drivers/gpu/drm/radeon/radeon_ttm.c @@ -869,11 +869,11 @@ static ssize_t radeon_ttm_gtt_read(struct file *f, char __user *buf, page = rdev->gart.pages[p]; if (page) { - ptr = kmap(page); + ptr = kmap_local_page(page); ptr += off; r = copy_to_user(buf, ptr, cur_size); - kunmap(rdev->gart.pages[p]); + kunmap_local(ptr); } else r = clear_user(buf, cur_size); -- 2.37.3

3 years, 1 month

[PATCH] dma-buf: fix racing conflict of dma_heap_add()

by Dawei Li

3 years, 1 month

[PATCH v2] dma-buf: cma_heap: Fix typo in comment

by Mark-PK Tsai

Remove duplicated "by" from comment in cma_heap_allocate(). Signed-off-by: Mark-PK Tsai <mark-pk.tsai(a)mediatek.com> --- drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 28fb04eccdd0..cd386ce639f3 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -316,7 +316,7 @@ static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, kunmap_atomic(vaddr); /* * Avoid wasting time zeroing memory if the process - * has been killed by by SIGKILL + * has been killed by SIGKILL */ if (fatal_signal_pending(current)) goto free_cma; -- 2.18.0

3 years, 1 month

[PATCH v3] dma-buf: cma_heap: Remove duplicated 'by' in comment

by Mark-PK Tsai

Remove duplicated 'by' from comment in cma_heap_allocate(). Signed-off-by: Mark-PK Tsai <mark-pk.tsai(a)mediatek.com> --- drivers/dma-buf/heaps/cma_heap.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/dma-buf/heaps/cma_heap.c b/drivers/dma-buf/heaps/cma_heap.c index 28fb04eccdd0..cd386ce639f3 100644 --- a/drivers/dma-buf/heaps/cma_heap.c +++ b/drivers/dma-buf/heaps/cma_heap.c @@ -316,7 +316,7 @@ static struct dma_buf *cma_heap_allocate(struct dma_heap *heap, kunmap_atomic(vaddr); /* * Avoid wasting time zeroing memory if the process - * has been killed by by SIGKILL + * has been killed by SIGKILL */ if (fatal_signal_pending(current)) goto free_cma; -- 2.18.0

3 years, 1 month

etnaviv OOPS, NULL pointer dereference on Linux 6.0.2

by Francesco Dolcini

Hello all, I got the following Oops, on a Apalis iMX6 Dual with 512MB RAM, running glmark2 tests with the system under memory pressure (OOM Killer!). It's not something systematic and I cannot tell if this is a regression or not, any suggestion? The system just froze afterward. [ 0.000000] Booting Linux on physical CPU 0x0 [ 0.000000] Linux version 6.0.2-6.1.0-devel+git.dab08f7eecdf (oe-user@oe-host) (arm-tdx-linux-gnueabi-gcc (GCC) 11.3.0, GNU ld (GNU Binutils) 2.38.20220708) #1 SMP Sat Oct 15 06:02:59 UTC 2022 [ 0.000000] CPU: ARMv7 Processor [412fc09a] revision 10 (ARMv7), cr=10c5387d [ 0.000000] CPU: PIPT / VIPT nonaliasing data cache, VIPT aliasing instruction cache [ 0.000000] OF: fdt: Machine model: Toradex Apalis iMX6Q/D Module on Ixora Carrier Board V1.1 ... [ 1.749471] etnaviv etnaviv: bound 130000.gpu (ops gpu_ops) [ 1.750527] etnaviv etnaviv: bound 134000.gpu (ops gpu_ops) [ 1.751522] etnaviv etnaviv: bound 2204000.gpu (ops gpu_ops) [ 1.751566] etnaviv-gpu 130000.gpu: model: GC2000, revision: 5108 [ 1.753141] etnaviv-gpu 134000.gpu: model: GC320, revision: 5007 [ 1.753392] etnaviv-gpu 2204000.gpu: model: GC355, revision: 1215 [ 1.753421] etnaviv-gpu 2204000.gpu: Ignoring GPU with VG and FE2.0 [ 1.756559] [drm] Initialized etnaviv 1.3.0 20151214 for etnaviv on minor 0 ... [ 480.994256] Out of memory: Killed process 1740 (Qt5_CinematicEx) total-vm:242656kB, anon-rss:105212kB, file-rss:9864kB, shmem-rss:1304kB, UID:0 pgtables:192kB oom_score_adj:0 [ 481.068691] 8<--- cut here --- [ 481.072037] Unable to handle kernel NULL pointer dereference at virtual address 00000004 [ 481.080366] [00000004] *pgd=00000000 [ 481.083994] Internal error: Oops: 805 [#1] SMP ARM [ 481.088813] Modules linked in: 8021q imx_sdma virt_dma coda_vpu v4l2_jpeg imx_vdoa dw_hdmi_ahb_audio fuse [ 481.098458] CPU: 1 PID: 1755 Comm: QSGRenderThread Not tainted 6.0.2-6.1.0-devel+git.dab08f7eecdf #1 [ 481.107619] Hardware name: Freescale i.MX6 Quad/DualLite (Device Tree) [ 481.114157] PC is at etnaviv_gem_free_object+0x40/0x128 [ 481.119412] LR is at lock_is_held_type+0xa4/0x15c [ 481.124138] pc : [<c0787f90>] lr : [<c0e46250>] psr: 60030113 [ 481.130421] sp : e1155da8 ip : 00000000 fp : 0000000c [ 481.135670] r10: c34ef400 r9 : c262066c r8 : 00000122 [ 481.140916] r7 : c2153000 r6 : c2153000 r5 : 00000870 r4 : c25f24a0 [ 481.147460] r3 : 00000000 r2 : 00000000 r1 : 00000100 r0 : 00000000 [ 481.153997] Flags: nZCv IRQs on FIQs on Mode SVC_32 ISA ARM Segment none [ 481.161143] Control: 10c5387d Table: 2caf004a DAC: 00000051 [ 481.166896] Register r0 information: NULL pointer [ 481.171615] Register r1 information: non-paged memory [ 481.176694] Register r2 information: NULL pointer [ 481.181429] Register r3 information: NULL pointer [ 481.181441] Register r4 information: slab kmalloc-128 start c25f2480 pointer offset 32 size 128 [ 481.186173] Register r5 information: non-paged memory [ 481.186181] Register r6 information: slab kmalloc-512 start c2153000 pointer offset 0 size 512 [ 481.199953] Register r7 information: slab kmalloc-512 start c2153000 pointer offset 0 size 512 [ 481.199975] Register r8 information: non-paged memory [ 481.199983] Register r9 information: slab kmalloc-2k start c2620000 pointer offset 1644 size 2048 [ 481.222276] Register r10 information: slab kmalloc-1k start c34ef400 pointer offset 0 size 1024 [ 481.222297] Register r11 information: non-paged memory [ 481.245038] Register r12 information: NULL pointer [ 481.245056] Process QSGRenderThread (pid: 1755, stack limit = 0xd30acffa) [ 481.245070] Stack: (0xe1155da8 to 0xe1156000) [ 481.245084] 5da0: c0787f50 fffffff4 00000870 c8102400 c212f000 c2620000 [ 481.245094] 5dc0: c262066c c34ef400 0000000c c078693c 00000003 00000000 00000000 c07e2960 [ 481.245103] 5de0: c2153000 042beef1 c2153a00 c2620000 c8102400 c8102940 c34ef5e4 c07324ec [ 481.245112] 5e00: c262066c c34ef400 0000000c c2153a00 c2620000 c34ef400 e1155e6c c0732a5c [ 481.245121] 5e20: c00c642e 0000000c c212f000 0000000c c0f6d968 e1155e6c c34ef400 c0723448 [ 481.245130] 5e40: 0000e280 00000001 c12b5820 c212f000 aed13e60 e1155e6c 0000002e c2f39000 [ 481.245138] 5e60: c0732e44 00000051 00000000 00000000 00000000 0000000c c212f000 c212f7a0 [ 481.318348] 5e80: 00000000 c018b61c c212f000 c16e0d20 c03514d8 c156155c 60070013 c0193790 [ 481.318369] 5ea0: b28a3000 00000254 c6d00280 00000001 00000000 042beef1 00000009 00004000 [ 481.318378] 5ec0: c212f000 c3caf280 00000001 c2f39000 00000028 c03514f0 00000000 00000000 [ 481.342916] 5ee0: c03513f0 c212f7a0 00000000 042beef1 aed13e60 c00c642e c2f39001 c0100080 [ 481.342928] 5f00: aed13e60 c212f000 c2f39000 c25b8710 00000009 c0342234 00000000 042beef1 [ 481.342938] 5f20: c36866e0 80000007 c212f000 b28a311c c3686680 c36866e0 e1155fb0 80000007 [ 481.342948] 5f40: c212f000 c0e516b0 aefd7cd0 c01d34b0 000001e0 00000000 00000000 00000000 [ 481.342958] 5f60: 00000193 00000007 c160fd90 b28a311c e1155fb0 c0e51500 0000021c 042beef1 [ 481.342969] 5f80: adf87818 aed13e90 aed13e60 c00c642e 00000036 c01002b4 c212f000 00000036 [ 481.342978] 5fa0: adf87818 c0100080 aed13e90 aed13e60 00000009 c00c642e aed13e60 aed13e40 [ 481.342988] 5fc0: aed13e90 aed13e60 c00c642e 00000036 00000001 0000021c 00870000 adf87818 [ 481.408411] 5fe0: 00000036 aed13e28 b6088089 b6001ae6 60070030 00000009 00000000 00000000 [ 481.408431] etnaviv_gem_free_object from etnaviv_gem_prime_import_sg_table+0x12c/0x160 [ 481.408469] etnaviv_gem_prime_import_sg_table from drm_gem_prime_import_dev+0x98/0x150 [ 481.408509] drm_gem_prime_import_dev from drm_gem_prime_fd_to_handle+0x188/0x1f8 [ 481.408528] drm_gem_prime_fd_to_handle from drm_ioctl+0x1e8/0x3a0 [ 481.408545] drm_ioctl from sys_ioctl+0x530/0xdbc [ 481.408571] sys_ioctl from ret_fast_syscall+0x0/0x1c [ 481.408587] Exception stack(0xe1155fa8 to 0xe1155ff0) [ 481.408599] 5fa0: aed13e90 aed13e60 00000009 c00c642e aed13e60 aed13e40 [ 481.408608] 5fc0: aed13e90 aed13e60 c00c642e 00000036 00000001 0000021c 00870000 adf87818 [ 481.477625] 5fe0: 00000036 aed13e28 b6088089 b6001ae6 [ 481.477641] Code: e5962174 e59f80e0 e3a01c01 e1a07006 (e5823004) [ 481.477819] ---[ end trace 0000000000000000 ]--- Francesco

3 years, 1 month

Jump to page:

2025

2024

2023

2022

2021

2020

2019

2018

2017

2016

2015

2014

2013

2012

2011

Linaro-mm-sig