* [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 2:12 ` Chris Mi
0 siblings, 0 replies; 35+ messages in thread
From: Chris Mi @ 2017-08-16 2:12 UTC (permalink / raw)
To: netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy,
linux-pm, ericvh, geliangtang, sparmaintainer, giometti, acme,
inki.dae, alex.williamson, rppt, teigland, viro, anna.schumaker,
weiyj.lk, chrism, marcos.souza.org, mike.marciniszyn, elder,
nbd-general, nhorman, nicolai.haehnle, david.binder, gregkh,
linux-usb, anil.gurumurthy, linux-kernel, varun, majd,
devesh.sharma, sameer.wadgaonkar, bhaktipriya96,
alexander.deucher, shaun.tancheff, akpm, matan, jlbec, kraxel,
Jason, timothy.sell, airlied, linux-wireless, pantelis.antoniou,
sudeep.dutt, neilb, edumazet, target-devel, linux-i2c,
daniel.vetter, jsarha, osandov, agk, drbd-dev, boris.brezillon,
thellstrom, dave, paul, leon, sainath.grandhi, gustavo.padovan,
james.smart, jonathanh, selvin.xavier, kgene,
linux-graphics-maintainer, andresx7, 3chas3, airlied, sean.hefty,
virtualization, hal.rosenstock, tj, mszeredi, hannes,
felipe.balbi, pali.rohar, tpmdd, linuxppc-dev, jani.nikula,
greybus-dev, nishants, swise, yuval.shaia, xiyou.wangcong,
evan.quan, lars.ellenberg, linux-arm-kernel, dwindsor,
linux-raid, syeh, bryan.thompson, ying.xue, Felix.Kuehling,
oneukum, fw, davem, minchan, kyungmin.park, monis, ebiederm, sre,
don.hiatt, leo.liu, jens.wiklander, hans.westgaard.ry,
alexander.shishkin, dennis.dalessandro, jasowang,
joonas.lahtinen, jiangshanlai, ast, fbarrat, mhocko,
alexandre.bounine, linux-mtd, amd-gfx, cgroups, jlayton,
frowand.list, elena.reshetova, f.fainelli, jejb, daniel,
linux-rdma, p.shailesh, ath10k, jgunthorpe, ccaulfie,
tomi.valkeinen, Monk.Liu, dgilbert, ira.weiny, david.kershner,
adobriyan, jon.maloy, keescook, arnd, intel-gfx, jhs, zhenyuw,
v9fs-developer, rmk+kernel, seanpaul, nab, Jerry.Zhang, eparis,
nicolas.dichtel, chris, mporter, rogerq, linux-nfs,
martin.petersen, linux-ppp, dm-devel, sw0312.kim,
fujita.tomonori, iommu, roman.kapl, ngupta, andrew.donnellan,
cyrille.pitchen, thierry.reding, colin.king, computersforpeace,
logang, christian.koenig, ocfs2-devel, rlove, jack, kvm, mst,
peterz, bigeasy, trond.myklebust, linux-remoteproc, amitkarwar,
bjorn.andersson, dhowells, linux-mm, ray.huang, jiri, peterhuewe,
linux1394-devel, lee.jones, john, devel, stephen,
mario.kleiner.de, manfred, oakad, linux-scsi, mawilcox, mfasheh,
richard, joro, jiangyilism, elfring, cluster-devel, javier,
jarkko.sakkinen, mingo, vdavydov.dev, kvalo, tipc-discussion,
ogerlitz, ishkamiel, devicetree, lizefan, huxm, mchehab, johan,
aditr, linux-block, robh+dt, Kai.Makisara, hare, rminnich,
linux-tegra, dsa, intel-gvt-dev, jy0922.shim, axboe, gbhat,
tomas.winkler, dedekind1, jbacik, jarno, vyasevich, krzk, sboyd,
jiri, afd, ashutosh.dixit, yishaih, rjw, hannes, Bart.VanAssche,
johannes, dwmw2, dasaratharaman.chandramouli
Using current TC code, it is very slow to insert a lot of rules.
In order to improve the rules update rate in TC,
we introduced the following two changes:
1) changed cls_flower to use IDR to manage the filters.
2) changed all act_xxx modules to use IDR instead of
a small hash table
But IDR has a limitation that it uses int. TC handle uses u32.
To make sure there is no regression, we also changed IDR to use
unsigned long. All clients of IDR are changed to use new IDR API.
Chris Mi (3):
idr: Use unsigned long instead of int
net/sched: Change cls_flower to use IDR
net/sched: Change act_api and act_xxx modules to use IDR
block/bsg.c | 8 +-
block/genhd.c | 12 +-
drivers/atm/nicstar.c | 11 +-
drivers/block/drbd/drbd_main.c | 31 +--
drivers/block/drbd/drbd_nl.c | 22 ++-
drivers/block/drbd/drbd_proc.c | 3 +-
drivers/block/drbd/drbd_receiver.c | 15 +-
drivers/block/drbd/drbd_state.c | 34 ++--
drivers/block/drbd/drbd_worker.c | 6 +-
drivers/block/loop.c | 17 +-
drivers/block/nbd.c | 20 +-
drivers/block/zram/zram_drv.c | 9 +-
drivers/char/tpm/tpm-chip.c | 10 +-
drivers/char/tpm/tpm.h | 2 +-
drivers/dca/dca-sysfs.c | 9 +-
drivers/firewire/core-cdev.c | 18 +-
drivers/firewire/core-device.c | 15 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
drivers/gpu/drm/drm_auth.c | 9 +-
drivers/gpu/drm/drm_connector.c | 10 +-
drivers/gpu/drm/drm_context.c | 20 +-
drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
drivers/gpu/drm/drm_drv.c | 6 +-
drivers/gpu/drm/drm_gem.c | 19 +-
drivers/gpu/drm/drm_info.c | 2 +-
drivers/gpu/drm/drm_mode_object.c | 11 +-
drivers/gpu/drm/drm_syncobj.c | 18 +-
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
drivers/gpu/drm/i915/gvt/display.c | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
drivers/gpu/drm/qxl/qxl_release.c | 14 +-
drivers/gpu/drm/sis/sis_mm.c | 8 +-
drivers/gpu/drm/tegra/drm.c | 10 +-
drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
drivers/gpu/drm/via/via_mm.c | 8 +-
drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
drivers/i2c/i2c-core-base.c | 19 +-
drivers/infiniband/core/cm.c | 8 +-
drivers/infiniband/core/cma.c | 12 +-
drivers/infiniband/core/rdma_core.c | 9 +-
drivers/infiniband/core/sa_query.c | 23 +--
drivers/infiniband/core/ucm.c | 7 +-
drivers/infiniband/core/ucma.c | 14 +-
drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
drivers/infiniband/hw/cxgb4/device.c | 18 +-
drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
drivers/infiniband/hw/hfi1/init.c | 9 +-
drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
drivers/infiniband/hw/mlx4/cm.c | 13 +-
drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
drivers/infiniband/hw/qib/qib_init.c | 9 +-
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
drivers/iommu/intel-svm.c | 9 +-
drivers/md/dm.c | 13 +-
drivers/memstick/core/memstick.c | 10 +-
drivers/memstick/core/ms_block.c | 9 +-
drivers/memstick/core/mspro_block.c | 12 +-
drivers/mfd/rtsx_pcr.c | 9 +-
drivers/misc/c2port/core.c | 7 +-
drivers/misc/cxl/context.c | 8 +-
drivers/misc/cxl/main.c | 15 +-
drivers/misc/mei/main.c | 8 +-
drivers/misc/mic/scif/scif_api.c | 11 +-
drivers/misc/mic/scif/scif_ports.c | 18 +-
drivers/misc/tifm_core.c | 9 +-
drivers/mtd/mtdcore.c | 9 +-
drivers/mtd/mtdcore.h | 2 +-
drivers/mtd/ubi/block.c | 7 +-
drivers/net/ppp/ppp_generic.c | 27 +--
drivers/net/tap.c | 10 +-
drivers/net/wireless/ath/ath10k/htt.h | 3 +-
drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
drivers/of/overlay.c | 15 +-
drivers/of/unittest.c | 25 ++-
drivers/power/supply/bq2415x_charger.c | 16 +-
drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
drivers/power/supply/ds2782_battery.c | 9 +-
drivers/powercap/powercap_sys.c | 8 +-
drivers/pps/pps.c | 10 +-
drivers/rapidio/rio_cm.c | 17 +-
drivers/remoteproc/remoteproc_core.c | 8 +-
drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
drivers/scsi/bfa/bfad_im.c | 8 +-
drivers/scsi/ch.c | 8 +-
drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
drivers/scsi/lpfc/lpfc_init.c | 11 +-
drivers/scsi/lpfc/lpfc_vport.c | 8 +-
drivers/scsi/sg.c | 10 +-
drivers/scsi/st.c | 8 +-
drivers/staging/greybus/uart.c | 22 +--
drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
drivers/target/iscsi/iscsi_target.c | 7 +-
drivers/target/iscsi/iscsi_target_login.c | 9 +-
drivers/target/target_core_device.c | 9 +-
drivers/target/target_core_user.c | 13 +-
drivers/tee/tee_shm.c | 8 +-
drivers/uio/uio.c | 9 +-
drivers/usb/class/cdc-acm.c | 24 +--
drivers/usb/core/devices.c | 2 +-
drivers/usb/core/hcd.c | 7 +-
drivers/usb/mon/mon_main.c | 3 +-
drivers/usb/serial/usb-serial.c | 11 +-
drivers/vfio/vfio.c | 15 +-
fs/dlm/lock.c | 9 +-
fs/dlm/lockspace.c | 6 +-
fs/dlm/recover.c | 10 +-
fs/nfs/nfs4client.c | 9 +-
fs/nfsd/nfs4state.c | 8 +-
fs/notify/inotify/inotify_fsnotify.c | 4 +-
fs/notify/inotify/inotify_user.c | 9 +-
fs/ocfs2/cluster/tcp.c | 10 +-
include/linux/idr.h | 26 +--
include/linux/of.h | 4 +-
include/linux/radix-tree.h | 2 +-
include/net/9p/9p.h | 2 +-
include/net/act_api.h | 76 +++-----
ipc/msg.c | 2 +-
ipc/sem.c | 2 +-
ipc/shm.c | 4 +-
ipc/util.c | 17 +-
kernel/bpf/syscall.c | 20 +-
kernel/cgroup/cgroup.c | 57 +++---
kernel/events/core.c | 10 +-
kernel/workqueue.c | 15 +-
lib/idr.c | 38 ++--
lib/radix-tree.c | 5 +-
mm/memcontrol.c | 11 +-
net/9p/client.c | 17 +-
net/9p/util.c | 14 +-
net/core/net_namespace.c | 23 ++-
net/mac80211/cfg.c | 23 +--
net/mac80211/iface.c | 3 +-
net/mac80211/main.c | 2 +-
net/mac80211/tx.c | 7 +-
net/mac80211/util.c | 3 +-
net/netlink/genetlink.c | 18 +-
net/qrtr/qrtr.c | 21 +-
net/rxrpc/conn_client.c | 15 +-
net/sched/act_api.c | 249 +++++++++++-------------
net/sched/act_bpf.c | 17 +-
net/sched/act_connmark.c | 16 +-
net/sched/act_csum.c | 16 +-
net/sched/act_gact.c | 16 +-
net/sched/act_ife.c | 20 +-
net/sched/act_ipt.c | 26 ++-
net/sched/act_mirred.c | 19 +-
net/sched/act_nat.c | 16 +-
net/sched/act_pedit.c | 18 +-
net/sched/act_police.c | 18 +-
net/sched/act_sample.c | 17 +-
net/sched/act_simple.c | 20 +-
net/sched/act_skbedit.c | 18 +-
net/sched/act_skbmod.c | 18 +-
net/sched/act_tunnel_key.c | 20 +-
net/sched/act_vlan.c | 22 +--
net/sched/cls_flower.c | 55 +++---
net/sctp/associola.c | 8 +-
net/tipc/server.c | 7 +-
172 files changed, 1256 insertions(+), 1113 deletions(-)
--
1.8.3.1
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 35+ messages in thread* [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 2:12 ` Chris Mi
0 siblings, 0 replies; 35+ messages in thread
From: Chris Mi @ 2017-08-16 2:12 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: lucho-OnYtXJJ0/fesTnJN9+BGXg,
sergey.senozhatsky.work-Re5JQEeQqe8AvxtiuMwx3w,
snitzer-H+wXaHxf7aLQT0dZR+AlfA, wsa-z923LK4zBo2bacvFa/9K2g,
markb-VPRAkNaXOzVWk0Htik3J/w, tom.leiming-Re5JQEeQqe8AvxtiuMwx3w,
stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB,
zhi.a.wang-ral2JQCrhuEAvxtiuMwx3w, nsekhar-l0cyMroinI0,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
bfields-uC3wQj2KruNg9hUCZPvPmw,
linux-sctp-u79uwXL29TY76Z2rM5mHXA, paulus-eUNUBHrolfbYtjvyW6yDsg,
jinpu.wang-EIkl63zCoXaH+58JC4qpiA, pshelar-LZ6Gd1LRuIk,
sumit.semwal-QSEj5FYQhm4dnm+yROfE0A, AlexBin.Xie-5C7GfCeVMHo,
david1.zhou-5C7GfCeVMHo,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
maximlevitsky-Re5JQEeQqe8AvxtiuMwx3w,
sudarsana.kalluru-h88ZbnxC6KDQT0dZR+AlfA,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dtwlin-Re5JQEeQqe8AvxtiuMwx3w, michel.daenzer-5C7GfCeVMHo,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
longman-H+wXaHxf7aLQT0dZR+AlfA,
niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w,
philipp.reisner-63ez5xqkn6DQT0dZR+AlfA,
shli-DgEjT+Ai2ygdnm+yROfE0A, linux-0h96xk9xTtrk1uMJSBkQmQ,
ohad-Ix1uc/W3ht7QT0dZR+AlfA, pmladek-IBi9RG/b67k,
dick.kennedy-dY08KVG/lbpWk0Htik3J/w, linux-
Using current TC code, it is very slow to insert a lot of rules.
In order to improve the rules update rate in TC,
we introduced the following two changes:
1) changed cls_flower to use IDR to manage the filters.
2) changed all act_xxx modules to use IDR instead of
a small hash table
But IDR has a limitation that it uses int. TC handle uses u32.
To make sure there is no regression, we also changed IDR to use
unsigned long. All clients of IDR are changed to use new IDR API.
Chris Mi (3):
idr: Use unsigned long instead of int
net/sched: Change cls_flower to use IDR
net/sched: Change act_api and act_xxx modules to use IDR
block/bsg.c | 8 +-
block/genhd.c | 12 +-
drivers/atm/nicstar.c | 11 +-
drivers/block/drbd/drbd_main.c | 31 +--
drivers/block/drbd/drbd_nl.c | 22 ++-
drivers/block/drbd/drbd_proc.c | 3 +-
drivers/block/drbd/drbd_receiver.c | 15 +-
drivers/block/drbd/drbd_state.c | 34 ++--
drivers/block/drbd/drbd_worker.c | 6 +-
drivers/block/loop.c | 17 +-
drivers/block/nbd.c | 20 +-
drivers/block/zram/zram_drv.c | 9 +-
drivers/char/tpm/tpm-chip.c | 10 +-
drivers/char/tpm/tpm.h | 2 +-
drivers/dca/dca-sysfs.c | 9 +-
drivers/firewire/core-cdev.c | 18 +-
drivers/firewire/core-device.c | 15 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
drivers/gpu/drm/drm_auth.c | 9 +-
drivers/gpu/drm/drm_connector.c | 10 +-
drivers/gpu/drm/drm_context.c | 20 +-
drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
drivers/gpu/drm/drm_drv.c | 6 +-
drivers/gpu/drm/drm_gem.c | 19 +-
drivers/gpu/drm/drm_info.c | 2 +-
drivers/gpu/drm/drm_mode_object.c | 11 +-
drivers/gpu/drm/drm_syncobj.c | 18 +-
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
drivers/gpu/drm/i915/gvt/display.c | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
drivers/gpu/drm/qxl/qxl_release.c | 14 +-
drivers/gpu/drm/sis/sis_mm.c | 8 +-
drivers/gpu/drm/tegra/drm.c | 10 +-
drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
drivers/gpu/drm/via/via_mm.c | 8 +-
drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
drivers/i2c/i2c-core-base.c | 19 +-
drivers/infiniband/core/cm.c | 8 +-
drivers/infiniband/core/cma.c | 12 +-
drivers/infiniband/core/rdma_core.c | 9 +-
drivers/infiniband/core/sa_query.c | 23 +--
drivers/infiniband/core/ucm.c | 7 +-
drivers/infiniband/core/ucma.c | 14 +-
drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
drivers/infiniband/hw/cxgb4/device.c | 18 +-
drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
drivers/infiniband/hw/hfi1/init.c | 9 +-
drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
drivers/infiniband/hw/mlx4/cm.c | 13 +-
drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
drivers/infiniband/hw/qib/qib_init.c | 9 +-
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
drivers/iommu/intel-svm.c | 9 +-
drivers/md/dm.c | 13 +-
drivers/memstick/core/memstick.c | 10 +-
drivers/memstick/core/ms_block.c | 9 +-
drivers/memstick/core/mspro_block.c | 12 +-
drivers/mfd/rtsx_pcr.c | 9 +-
drivers/misc/c2port/core.c | 7 +-
drivers/misc/cxl/context.c | 8 +-
drivers/misc/cxl/main.c | 15 +-
drivers/misc/mei/main.c | 8 +-
drivers/misc/mic/scif/scif_api.c | 11 +-
drivers/misc/mic/scif/scif_ports.c | 18 +-
drivers/misc/tifm_core.c | 9 +-
drivers/mtd/mtdcore.c | 9 +-
drivers/mtd/mtdcore.h | 2 +-
drivers/mtd/ubi/block.c | 7 +-
drivers/net/ppp/ppp_generic.c | 27 +--
drivers/net/tap.c | 10 +-
drivers/net/wireless/ath/ath10k/htt.h | 3 +-
drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
drivers/of/overlay.c | 15 +-
drivers/of/unittest.c | 25 ++-
drivers/power/supply/bq2415x_charger.c | 16 +-
drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
drivers/power/supply/ds2782_battery.c | 9 +-
drivers/powercap/powercap_sys.c | 8 +-
drivers/pps/pps.c | 10 +-
drivers/rapidio/rio_cm.c | 17 +-
drivers/remoteproc/remoteproc_core.c | 8 +-
drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
drivers/scsi/bfa/bfad_im.c | 8 +-
drivers/scsi/ch.c | 8 +-
drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
drivers/scsi/lpfc/lpfc_init.c | 11 +-
drivers/scsi/lpfc/lpfc_vport.c | 8 +-
drivers/scsi/sg.c | 10 +-
drivers/scsi/st.c | 8 +-
drivers/staging/greybus/uart.c | 22 +--
drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
drivers/target/iscsi/iscsi_target.c | 7 +-
drivers/target/iscsi/iscsi_target_login.c | 9 +-
drivers/target/target_core_device.c | 9 +-
drivers/target/target_core_user.c | 13 +-
drivers/tee/tee_shm.c | 8 +-
drivers/uio/uio.c | 9 +-
drivers/usb/class/cdc-acm.c | 24 +--
drivers/usb/core/devices.c | 2 +-
drivers/usb/core/hcd.c | 7 +-
drivers/usb/mon/mon_main.c | 3 +-
drivers/usb/serial/usb-serial.c | 11 +-
drivers/vfio/vfio.c | 15 +-
fs/dlm/lock.c | 9 +-
fs/dlm/lockspace.c | 6 +-
fs/dlm/recover.c | 10 +-
fs/nfs/nfs4client.c | 9 +-
fs/nfsd/nfs4state.c | 8 +-
fs/notify/inotify/inotify_fsnotify.c | 4 +-
fs/notify/inotify/inotify_user.c | 9 +-
fs/ocfs2/cluster/tcp.c | 10 +-
include/linux/idr.h | 26 +--
include/linux/of.h | 4 +-
include/linux/radix-tree.h | 2 +-
include/net/9p/9p.h | 2 +-
include/net/act_api.h | 76 +++-----
ipc/msg.c | 2 +-
ipc/sem.c | 2 +-
ipc/shm.c | 4 +-
ipc/util.c | 17 +-
kernel/bpf/syscall.c | 20 +-
kernel/cgroup/cgroup.c | 57 +++---
kernel/events/core.c | 10 +-
kernel/workqueue.c | 15 +-
lib/idr.c | 38 ++--
lib/radix-tree.c | 5 +-
mm/memcontrol.c | 11 +-
net/9p/client.c | 17 +-
net/9p/util.c | 14 +-
net/core/net_namespace.c | 23 ++-
net/mac80211/cfg.c | 23 +--
net/mac80211/iface.c | 3 +-
net/mac80211/main.c | 2 +-
net/mac80211/tx.c | 7 +-
net/mac80211/util.c | 3 +-
net/netlink/genetlink.c | 18 +-
net/qrtr/qrtr.c | 21 +-
net/rxrpc/conn_client.c | 15 +-
net/sched/act_api.c | 249 +++++++++++-------------
net/sched/act_bpf.c | 17 +-
net/sched/act_connmark.c | 16 +-
net/sched/act_csum.c | 16 +-
net/sched/act_gact.c | 16 +-
net/sched/act_ife.c | 20 +-
net/sched/act_ipt.c | 26 ++-
net/sched/act_mirred.c | 19 +-
net/sched/act_nat.c | 16 +-
net/sched/act_pedit.c | 18 +-
net/sched/act_police.c | 18 +-
net/sched/act_sample.c | 17 +-
net/sched/act_simple.c | 20 +-
net/sched/act_skbedit.c | 18 +-
net/sched/act_skbmod.c | 18 +-
net/sched/act_tunnel_key.c | 20 +-
net/sched/act_vlan.c | 22 +--
net/sched/cls_flower.c | 55 +++---
net/sctp/associola.c | 8 +-
net/tipc/server.c | 7 +-
172 files changed, 1256 insertions(+), 1113 deletions(-)
--
1.8.3.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply [flat|nested] 35+ messages in thread* [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 2:12 ` Chris Mi
0 siblings, 0 replies; 35+ messages in thread
From: Chris Mi @ 2017-08-16 2:12 UTC (permalink / raw)
To: netdev-u79uwXL29TY76Z2rM5mHXA
Cc: lucho-OnYtXJJ0/fesTnJN9+BGXg,
sergey.senozhatsky.work-Re5JQEeQqe8AvxtiuMwx3w,
snitzer-H+wXaHxf7aLQT0dZR+AlfA, wsa-z923LK4zBo2bacvFa/9K2g,
markb-VPRAkNaXOzVWk0Htik3J/w, tom.leiming-Re5JQEeQqe8AvxtiuMwx3w,
stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB,
zhi.a.wang-ral2JQCrhuEAvxtiuMwx3w, nsekhar-l0cyMroinI0,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
bfields-uC3wQj2KruNg9hUCZPvPmw,
linux-sctp-u79uwXL29TY76Z2rM5mHXA, paulus-eUNUBHrolfbYtjvyW6yDsg,
jinpu.wang-EIkl63zCoXaH+58JC4qpiA, pshelar-LZ6Gd1LRuIk,
sumit.semwal-QSEj5FYQhm4dnm+yROfE0A, AlexBin.Xie-5C7GfCeVMHo,
david1.zhou-5C7GfCeVMHo,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
maximlevitsky-Re5JQEeQqe8AvxtiuMwx3w,
sudarsana.kalluru-h88ZbnxC6KDQT0dZR+AlfA,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dtwlin-Re5JQEeQqe8AvxtiuMwx3w, michel.daenzer-5C7GfCeVMHo,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
longman-H+wXaHxf7aLQT0dZR+AlfA,
niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w,
philipp.reisner-63ez5xqkn6DQT0dZR+AlfA,
shli-DgEjT+Ai2ygdnm+yROfE0A, linux-0h96xk9xTtrk1uMJSBkQmQ,
ohad-Ix1uc/W3ht7QT0dZR+AlfA, pmladek-IBi9RG/b67k,
dick.kennedy-dY08KVG/lbpWk0Htik3J/w
Using current TC code, it is very slow to insert a lot of rules.
In order to improve the rules update rate in TC,
we introduced the following two changes:
1) changed cls_flower to use IDR to manage the filters.
2) changed all act_xxx modules to use IDR instead of
a small hash table
But IDR has a limitation that it uses int. TC handle uses u32.
To make sure there is no regression, we also changed IDR to use
unsigned long. All clients of IDR are changed to use new IDR API.
Chris Mi (3):
idr: Use unsigned long instead of int
net/sched: Change cls_flower to use IDR
net/sched: Change act_api and act_xxx modules to use IDR
block/bsg.c | 8 +-
block/genhd.c | 12 +-
drivers/atm/nicstar.c | 11 +-
drivers/block/drbd/drbd_main.c | 31 +--
drivers/block/drbd/drbd_nl.c | 22 ++-
drivers/block/drbd/drbd_proc.c | 3 +-
drivers/block/drbd/drbd_receiver.c | 15 +-
drivers/block/drbd/drbd_state.c | 34 ++--
drivers/block/drbd/drbd_worker.c | 6 +-
drivers/block/loop.c | 17 +-
drivers/block/nbd.c | 20 +-
drivers/block/zram/zram_drv.c | 9 +-
drivers/char/tpm/tpm-chip.c | 10 +-
drivers/char/tpm/tpm.h | 2 +-
drivers/dca/dca-sysfs.c | 9 +-
drivers/firewire/core-cdev.c | 18 +-
drivers/firewire/core-device.c | 15 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
drivers/gpu/drm/drm_auth.c | 9 +-
drivers/gpu/drm/drm_connector.c | 10 +-
drivers/gpu/drm/drm_context.c | 20 +-
drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
drivers/gpu/drm/drm_drv.c | 6 +-
drivers/gpu/drm/drm_gem.c | 19 +-
drivers/gpu/drm/drm_info.c | 2 +-
drivers/gpu/drm/drm_mode_object.c | 11 +-
drivers/gpu/drm/drm_syncobj.c | 18 +-
drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
drivers/gpu/drm/i915/gvt/display.c | 2 +-
drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
drivers/gpu/drm/qxl/qxl_release.c | 14 +-
drivers/gpu/drm/sis/sis_mm.c | 8 +-
drivers/gpu/drm/tegra/drm.c | 10 +-
drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
drivers/gpu/drm/via/via_mm.c | 8 +-
drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
drivers/i2c/i2c-core-base.c | 19 +-
drivers/infiniband/core/cm.c | 8 +-
drivers/infiniband/core/cma.c | 12 +-
drivers/infiniband/core/rdma_core.c | 9 +-
drivers/infiniband/core/sa_query.c | 23 +--
drivers/infiniband/core/ucm.c | 7 +-
drivers/infiniband/core/ucma.c | 14 +-
drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
drivers/infiniband/hw/cxgb4/device.c | 18 +-
drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
drivers/infiniband/hw/hfi1/init.c | 9 +-
drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
drivers/infiniband/hw/mlx4/cm.c | 13 +-
drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
drivers/infiniband/hw/qib/qib_init.c | 9 +-
drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
drivers/iommu/intel-svm.c | 9 +-
drivers/md/dm.c | 13 +-
drivers/memstick/core/memstick.c | 10 +-
drivers/memstick/core/ms_block.c | 9 +-
drivers/memstick/core/mspro_block.c | 12 +-
drivers/mfd/rtsx_pcr.c | 9 +-
drivers/misc/c2port/core.c | 7 +-
drivers/misc/cxl/context.c | 8 +-
drivers/misc/cxl/main.c | 15 +-
drivers/misc/mei/main.c | 8 +-
drivers/misc/mic/scif/scif_api.c | 11 +-
drivers/misc/mic/scif/scif_ports.c | 18 +-
drivers/misc/tifm_core.c | 9 +-
drivers/mtd/mtdcore.c | 9 +-
drivers/mtd/mtdcore.h | 2 +-
drivers/mtd/ubi/block.c | 7 +-
drivers/net/ppp/ppp_generic.c | 27 +--
drivers/net/tap.c | 10 +-
drivers/net/wireless/ath/ath10k/htt.h | 3 +-
drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
drivers/net/wireless/ath/ath10k/mac.c | 2 +-
drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
drivers/of/overlay.c | 15 +-
drivers/of/unittest.c | 25 ++-
drivers/power/supply/bq2415x_charger.c | 16 +-
drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
drivers/power/supply/ds2782_battery.c | 9 +-
drivers/powercap/powercap_sys.c | 8 +-
drivers/pps/pps.c | 10 +-
drivers/rapidio/rio_cm.c | 17 +-
drivers/remoteproc/remoteproc_core.c | 8 +-
drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
drivers/scsi/bfa/bfad_im.c | 8 +-
drivers/scsi/ch.c | 8 +-
drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
drivers/scsi/lpfc/lpfc_init.c | 11 +-
drivers/scsi/lpfc/lpfc_vport.c | 8 +-
drivers/scsi/sg.c | 10 +-
drivers/scsi/st.c | 8 +-
drivers/staging/greybus/uart.c | 22 +--
drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
drivers/target/iscsi/iscsi_target.c | 7 +-
drivers/target/iscsi/iscsi_target_login.c | 9 +-
drivers/target/target_core_device.c | 9 +-
drivers/target/target_core_user.c | 13 +-
drivers/tee/tee_shm.c | 8 +-
drivers/uio/uio.c | 9 +-
drivers/usb/class/cdc-acm.c | 24 +--
drivers/usb/core/devices.c | 2 +-
drivers/usb/core/hcd.c | 7 +-
drivers/usb/mon/mon_main.c | 3 +-
drivers/usb/serial/usb-serial.c | 11 +-
drivers/vfio/vfio.c | 15 +-
fs/dlm/lock.c | 9 +-
fs/dlm/lockspace.c | 6 +-
fs/dlm/recover.c | 10 +-
fs/nfs/nfs4client.c | 9 +-
fs/nfsd/nfs4state.c | 8 +-
fs/notify/inotify/inotify_fsnotify.c | 4 +-
fs/notify/inotify/inotify_user.c | 9 +-
fs/ocfs2/cluster/tcp.c | 10 +-
include/linux/idr.h | 26 +--
include/linux/of.h | 4 +-
include/linux/radix-tree.h | 2 +-
include/net/9p/9p.h | 2 +-
include/net/act_api.h | 76 +++-----
ipc/msg.c | 2 +-
ipc/sem.c | 2 +-
ipc/shm.c | 4 +-
ipc/util.c | 17 +-
kernel/bpf/syscall.c | 20 +-
kernel/cgroup/cgroup.c | 57 +++---
kernel/events/core.c | 10 +-
kernel/workqueue.c | 15 +-
lib/idr.c | 38 ++--
lib/radix-tree.c | 5 +-
mm/memcontrol.c | 11 +-
net/9p/client.c | 17 +-
net/9p/util.c | 14 +-
net/core/net_namespace.c | 23 ++-
net/mac80211/cfg.c | 23 +--
net/mac80211/iface.c | 3 +-
net/mac80211/main.c | 2 +-
net/mac80211/tx.c | 7 +-
net/mac80211/util.c | 3 +-
net/netlink/genetlink.c | 18 +-
net/qrtr/qrtr.c | 21 +-
net/rxrpc/conn_client.c | 15 +-
net/sched/act_api.c | 249 +++++++++++-------------
net/sched/act_bpf.c | 17 +-
net/sched/act_connmark.c | 16 +-
net/sched/act_csum.c | 16 +-
net/sched/act_gact.c | 16 +-
net/sched/act_ife.c | 20 +-
net/sched/act_ipt.c | 26 ++-
net/sched/act_mirred.c | 19 +-
net/sched/act_nat.c | 16 +-
net/sched/act_pedit.c | 18 +-
net/sched/act_police.c | 18 +-
net/sched/act_sample.c | 17 +-
net/sched/act_simple.c | 20 +-
net/sched/act_skbedit.c | 18 +-
net/sched/act_skbmod.c | 18 +-
net/sched/act_tunnel_key.c | 20 +-
net/sched/act_vlan.c | 22 +--
net/sched/cls_flower.c | 55 +++---
net/sctp/associola.c | 8 +-
net/tipc/server.c | 7 +-
172 files changed, 1256 insertions(+), 1113 deletions(-)
--
1.8.3.1
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 2:12 ` Chris Mi
` (2 preceding siblings ...)
(?)
@ 2017-08-16 3:05 ` David Miller
2017-08-16 3:38 ` Chris Mi
-1 siblings, 1 reply; 35+ messages in thread
From: David Miller @ 2017-08-16 3:05 UTC (permalink / raw)
To: chrism; +Cc: netdev
The CC: list you used for these postings was way too large.
Seriously, netdev by itself or with one or two _specific_ developers
added would have been more than sufficient.
If you are automating things using the MAINTAINERS file, always
look over the result by hand and ask yourself "Do I really have to
CC: everything listed in this thing?"
Thanks.
^ permalink raw reply [flat|nested] 35+ messages in thread* RE: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 3:05 ` David Miller
@ 2017-08-16 3:38 ` Chris Mi
0 siblings, 0 replies; 35+ messages in thread
From: Chris Mi @ 2017-08-16 3:38 UTC (permalink / raw)
To: David Miller; +Cc: netdev
Hi David,
Thanks for your suggestion. If needed, I could send the review request again
without so long CC: list. I surely will avoid such mistake in the future.
Thanks,
Chris
> -----Original Message-----
> From: David Miller [mailto:davem@davemloft.net]
> Sent: Wednesday, August 16, 2017 11:06 AM
> To: Chris Mi <chrism@mellanox.com>
> Cc: netdev@vger.kernel.org
> Subject: Re: [patch net-next 0/3] net/sched: Improve getting objects by
> indexes
>
>
> The CC: list you used for these postings was way too large.
>
> Seriously, netdev by itself or with one or two _specific_ developers added
> would have been more than sufficient.
>
> If you are automating things using the MAINTAINERS file, always look over
> the result by hand and ask yourself "Do I really have to
> CC: everything listed in this thing?"
>
> Thanks.
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 7:49 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev
Cc: aditr, stern, agk, alexander.shishkin, alexandre.bounine,
alexander.deucher, oakad, ast, elder, adobriyan, alex.williamson,
AlexBin.Xie, viro, amd-gfx, amitkarwar, andresx7,
andrew.donnellan, afd, akpm, anil.gurumurthy, anna.schumaker,
acme, arnd, dedekind1, ashutosh.dixit, ath10k, Bart.VanAssche,
bhaktipriya96, bjorn.andersson, boris.brezillon,
computersforpeace, bryan.thompson, cgroups, 3chas3, ccaulfie,
chris, david1.zhou, cluster-devel, colin.king, xiyou.wangcong,
cyrille.pitchen, daniel, daniel.vetter,
dasaratharaman.chandramouli, airlied, dsa, airlied, david.binder,
dhowells, david.kershner, dtwlin, dave, davem, teigland,
dwindsor, dwmw2, dennis.dalessandro, devel, devesh.sharma,
devicetree, dick.kennedy, dm-devel, don.hiatt, dgilbert,
dledford, drbd-dev, dri-devel, elena.reshetova, edumazet, eparis,
ericvh, ebiederm, evan.quan, felipe.balbi, Felix.Kuehling,
f.fainelli, fw, frowand.list, fbarrat, fujita.tomonori, gbhat,
geliangtang, kraxel, gregkh, greybus-dev, linux, gustavo.padovan,
hal.rosenstock, hannes, hare, ishkamiel, hans.westgaard.ry,
ray.huang, mingo, inki.dae, intel-gfx, intel-gvt-dev, iommu,
ira.weiny, jinpu.wang, jhs, jejb, james.smart, jani.nikula, jack,
jarkko.sakkinen, jarno, Jason, jgunthorpe, jasowang, javier,
bfields, jlayton, axboe, jens.wiklander, jiangyilism, jiri, jiri,
jlbec, joro, johan, johannes, hannes, john, jonathanh, jon.maloy,
joonas.lahtinen, jy0922.shim, jbacik, Jerry.Zhang, jsarha,
Kai.Makisara, kvalo, keescook, krzk, kgene, kvm, kyungmin.park,
jiangshanlai, lars.ellenberg, lucho, lee.jones, leo.liu, leon,
linux1394-devel, linux-arm-kernel, linux-atm-general,
linux-block, linux-i2c, linux-kernel, linux-mm, linux-mtd,
linux-nfs, linux-pm, linuxppc-dev, linux-ppp, linux-raid,
linux-rdma, linux-remoteproc, linux-samsung-soc, linux-scsi,
linux-sctp, linux-tegra, linux-usb, linux-wireless, logang, majd,
manfred, tpmdd, marcos.souza.org, marek.vasut, mario.kleiner.de,
markb, mfasheh, elfring, martin.petersen, matan, mawilcox,
mporter, mchehab, maximlevitsky, mst, mhocko, michel.daenzer,
mike.marciniszyn, rppt, snitzer, mszeredi, minchan, tom.leiming,
monis, Monk.Liu, nbd-general, neilb, nhorman, nab,
nicolai.haehnle, nicolas.dichtel, niranjana.vishwanathapura,
nishants, ngupta, ocfs2-devel, ohad, oneukum, osandov, ogerlitz,
pali.rohar, pantelis.antoniou, paulus, paul, peterhuewe, peterz,
pmladek, philipp.reisner, pshelar, rjw, richard, rlove, robh+dt,
giometti, rogerq, roman.kapl, rminnich, rmk+kernel,
sainath.grandhi, sameer.wadgaonkar, sean.hefty, seanpaul,
bigeasy, sre, nsekhar, selvin.xavier, sergey.senozhatsky.work,
sw0312.kim, p.shailesh, shli, shaun.tancheff, syeh,
sparmaintainer, stefanr, sboyd, stephen, swise,
sudarsana.kalluru, sudeep.dutt, sumit.semwal, target-devel, tj,
thierry.reding, thellstrom, timothy.sell, tipc-discussion,
tomas.winkler, tomi.valkeinen, tpmdd-devel, trond.myklebust,
v9fs-developer, varun, virtualization, vdavydov.dev, vyasevich,
linux-graphics-maintainer, longman, weiyj.lk, wsa, huxm,
ying.xue, yishaih, yuval.shaia, lizefan, zhenyuw, zhi.a.wang
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 7:49 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy,
linux-pm, ericvh, geliangtang, sparmaintainer, giometti, acme,
inki.dae, alex.williamson, rppt, teigland, viro, ishkamiel,
weiyj.lk, marcos.souza.org, mike.marciniszyn, elder, nbd-general,
nhorman, nicolai.haehnle, david.binder, gregkh, linux-usb,
anil.gurumurthy, linux-kernel, varun, majd, devesh.sharma,
sameer.wadgaonkar, bhaktipriya96, alexander.deucher,
shaun.tancheff, akpm, matan, jlbec, kraxel, Jason, timothy.sell,
airlied, linux-wireless, pantelis.antoniou, sudeep.dutt, neilb,
edumazet, target-devel, linux-i2c, daniel.vetter, jsarha,
osandov, agk, drbd-dev, boris.brezillon, thellstrom, dave, paul,
leon, sainath.grandhi, gustavo.padovan, james.smart, jonathanh,
selvin.xavier, kgene, linux-graphics-maintainer, andresx7,
3chas3, airlied, sean.hefty, virtualization, hal.rosenstock, tj,
mszeredi, hannes, felipe.balbi, pali.rohar, tpmdd, linuxppc-dev,
jani.nikula, greybus-dev, nishants, swise, yuval.shaia,
xiyou.wangcong, evan.quan, lars.ellenberg, linux-arm-kernel,
dwindsor, linux-raid, syeh, bryan.thompson, ying.xue,
Felix.Kuehling, oneukum, fw, anna.schumaker, minchan,
kyungmin.park, monis, ebiederm, sre, don.hiatt, leo.liu,
jens.wiklander, hans.westgaard.ry, alexander.shishkin,
dennis.dalessandro, jasowang, joonas.lahtinen, jiangshanlai, ast,
fbarrat, mhocko, alexandre.bounine, linux-mtd, amd-gfx, cgroups,
jlayton, frowand.list, elena.reshetova, f.fainelli, jejb, daniel,
linux-rdma, p.shailesh, ath10k, jgunthorpe, ccaulfie,
tomi.valkeinen, Monk.Liu, dgilbert, ira.weiny, david.kershner,
adobriyan, jon.maloy, keescook, arnd, intel-gfx, jhs, zhenyuw,
v9fs-developer, rmk+kernel, seanpaul, nab, Jerry.Zhang, eparis,
nicolas.dichtel, chris, mporter, rogerq, linux-nfs,
martin.petersen, linux-ppp, dm-devel, sw0312.kim,
fujita.tomonori, iommu, roman.kapl, ngupta, andrew.donnellan,
cyrille.pitchen, thierry.reding, colin.king, computersforpeace,
logang, davem, ocfs2-devel, rlove, jack, kvm, mst, peterz,
bigeasy, trond.myklebust, linux-remoteproc, amitkarwar,
bjorn.andersson, dhowells, linux-mm, ray.huang, jiri, peterhuewe,
linux1394-devel, lee.jones, john, devel, stephen,
mario.kleiner.de, manfred, oakad, linux-scsi, mawilcox, mfasheh,
richard, joro, jiangyilism, elfring, cluster-devel, javier,
jarkko.sakkinen, mingo, vdavydov.dev, kvalo, tipc-discussion,
ogerlitz, devicetree, lizefan, huxm, mchehab, johan, aditr,
linux-block, robh+dt, Kai.Makisara, hare, rminnich, linux-tegra,
dsa, intel-gvt-dev, jy0922.shim, axboe, gbhat, tomas.winkler,
dedekind1, jbacik, jarno, vyasevich, krzk, sboyd, jiri, afd,
ashutosh.dixit, yishaih, rjw, hannes, Bart.VanAssche, johannes,
dwmw2, dasaratharaman.chandramouli
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 7:49 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev-u79uwXL29TY76Z2rM5mHXA
Cc: lucho-OnYtXJJ0/fesTnJN9+BGXg,
sergey.senozhatsky.work-Re5JQEeQqe8AvxtiuMwx3w,
snitzer-H+wXaHxf7aLQT0dZR+AlfA, wsa-z923LK4zBo2bacvFa/9K2g,
markb-VPRAkNaXOzVWk0Htik3J/w, tom.leiming-Re5JQEeQqe8AvxtiuMwx3w,
stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB,
zhi.a.wang-ral2JQCrhuEAvxtiuMwx3w, nsekhar-l0cyMroinI0,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
bfields-uC3wQj2KruNg9hUCZPvPmw,
linux-sctp-u79uwXL29TY76Z2rM5mHXA, paulus-eUNUBHrolfbYtjvyW6yDsg,
jinpu.wang-EIkl63zCoXaH+58JC4qpiA, pshelar-LZ6Gd1LRuIk,
sumit.semwal-QSEj5FYQhm4dnm+yROfE0A, AlexBin.Xie-5C7GfCeVMHo,
david1.zhou-5C7GfCeVMHo,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
maximlevitsky-Re5JQEeQqe8AvxtiuMwx3w,
sudarsana.kalluru-h88ZbnxC6KDQT0dZR+AlfA,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dtwlin-Re5JQEeQqe8AvxtiuMwx3w, michel.daenzer-5C7GfCeVMHo,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
longman-H+wXaHxf7aLQT0dZR+AlfA,
niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w,
philipp.reisner-63ez5xqkn6DQT0dZR+AlfA,
shli-DgEjT+Ai2ygdnm+yROfE0A, linux-0h96xk9xTtrk1uMJSBkQmQ,
ohad-Ix1uc/W3ht7QT0dZR+AlfA, pmladek-IBi9RG/b67k,
dick.kennedy-dY08KVG/lbpWk0Htik3J/w, linu
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 7:49 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev-u79uwXL29TY76Z2rM5mHXA
Cc: lucho-OnYtXJJ0/fesTnJN9+BGXg,
sergey.senozhatsky.work-Re5JQEeQqe8AvxtiuMwx3w,
snitzer-H+wXaHxf7aLQT0dZR+AlfA, wsa-z923LK4zBo2bacvFa/9K2g,
markb-VPRAkNaXOzVWk0Htik3J/w, tom.leiming-Re5JQEeQqe8AvxtiuMwx3w,
stefanr-MtYdepGKPcBMYopoZt5u/LNAH6kLmebB,
zhi.a.wang-ral2JQCrhuEAvxtiuMwx3w, nsekhar-l0cyMroinI0,
dri-devel-PD4FTy7X32lNgt0PjOBp9y5qC8QIuHrW,
bfields-uC3wQj2KruNg9hUCZPvPmw,
linux-sctp-u79uwXL29TY76Z2rM5mHXA, paulus-eUNUBHrolfbYtjvyW6yDsg,
jinpu.wang-EIkl63zCoXaH+58JC4qpiA, pshelar-LZ6Gd1LRuIk,
sumit.semwal-QSEj5FYQhm4dnm+yROfE0A, AlexBin.Xie-5C7GfCeVMHo,
david1.zhou-5C7GfCeVMHo,
linux-samsung-soc-u79uwXL29TY76Z2rM5mHXA,
maximlevitsky-Re5JQEeQqe8AvxtiuMwx3w,
sudarsana.kalluru-h88ZbnxC6KDQT0dZR+AlfA,
marek.vasut-Re5JQEeQqe8AvxtiuMwx3w,
linux-atm-general-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
dtwlin-Re5JQEeQqe8AvxtiuMwx3w, michel.daenzer-5C7GfCeVMHo,
dledford-H+wXaHxf7aLQT0dZR+AlfA,
tpmdd-devel-5NWGOfrQmneRv+LV9MX5uipxlwaOVQ5f,
stern-nwvwT67g6+6dFdvTe/nMLpVzexx5G7lz,
longman-H+wXaHxf7aLQT0dZR+AlfA,
niranjana.vishwanathapura-ral2JQCrhuEAvxtiuMwx3w,
philipp.reisner-63ez5xqkn6DQT0dZR+AlfA,
shli-DgEjT+Ai2ygdnm+yROfE0A, linux-0h96xk9xTtrk1uMJSBkQmQ,
ohad-Ix1uc/W3ht7QT0dZR+AlfA, pmladek-IBi9RG/b67k,
dick.kennedy-dY08KVG/lbpWk0Htik3J/w
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 7:49 ` Christian König
` (2 preceding siblings ...)
(?)
@ 2017-08-16 8:16 ` Jiri Pirko
2017-08-16 8:31 ` Christian König
-1 siblings, 1 reply; 35+ messages in thread
From: Jiri Pirko @ 2017-08-16 8:16 UTC (permalink / raw)
To: Christian König; +Cc: Chris Mi, netdev
Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>Am 16.08.2017 um 04:12 schrieb Chris Mi:
>> Using current TC code, it is very slow to insert a lot of rules.
>>
>> In order to improve the rules update rate in TC,
>> we introduced the following two changes:
>> 1) changed cls_flower to use IDR to manage the filters.
>> 2) changed all act_xxx modules to use IDR instead of
>> a small hash table
>>
>> But IDR has a limitation that it uses int. TC handle uses u32.
>> To make sure there is no regression, we also changed IDR to use
>> unsigned long. All clients of IDR are changed to use new IDR API.
>
>WOW, wait a second. The idr change is touching a lot of drivers and to be
>honest doesn't looks correct at all.
>
>Just look at the first chunk of your modification:
>> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>> mutex_lock(&bsg_mutex);
>> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>> - if (ret < 0) {
>> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>> + GFP_KERNEL);
>> + if (ret) {
>> if (ret == -ENOSPC) {
>> printk(KERN_ERR "bsg: too many bsg devices\n");
>> ret = -EINVAL;
>The condition "if (ret)" will now always be true after the first allocation
>and so we always run into the error handling after that.
On success, idr_alloc returns 0.
>
>I've never read the bsg code before, but that's certainly not correct. And
>that incorrect pattern repeats over and over again in this code.
>
>Apart from that why the heck do you want to allocate more than 1<<31 handles?
tc action indexes for example. That is part of this patchset.
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 8:16 ` Jiri Pirko
@ 2017-08-16 8:31 ` Christian König
2017-08-16 8:39 ` Jiri Pirko
2017-08-16 14:28 ` David Laight
0 siblings, 2 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 8:31 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Chris Mi, netdev
Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
> Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>> Am 16.08.2017 um 04:12 schrieb Chris Mi:
>>> Using current TC code, it is very slow to insert a lot of rules.
>>>
>>> In order to improve the rules update rate in TC,
>>> we introduced the following two changes:
>>> 1) changed cls_flower to use IDR to manage the filters.
>>> 2) changed all act_xxx modules to use IDR instead of
>>> a small hash table
>>>
>>> But IDR has a limitation that it uses int. TC handle uses u32.
>>> To make sure there is no regression, we also changed IDR to use
>>> unsigned long. All clients of IDR are changed to use new IDR API.
>> WOW, wait a second. The idr change is touching a lot of drivers and to be
>> honest doesn't looks correct at all.
>>
>> Just look at the first chunk of your modification:
>>> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>>> mutex_lock(&bsg_mutex);
>>> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>>> - if (ret < 0) {
>>> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>>> + GFP_KERNEL);
>>> + if (ret) {
>>> if (ret == -ENOSPC) {
>>> printk(KERN_ERR "bsg: too many bsg devices\n");
>>> ret = -EINVAL;
>> The condition "if (ret)" will now always be true after the first allocation
>> and so we always run into the error handling after that.
> On success, idr_alloc returns 0.
Ah, I see. You change the idr_alloc to return the resulting index as
separate parameter.
You should explicit note that in the commit message, cause that is
something easily overlooked.
In general I strongly suggest to add a separate interface for allocating
unsigned long handles, use that for the while being and then move the
existing drivers over bit by bit.
A single patch which touches so many different driver is practically
impossible to review consequently.
>> I've never read the bsg code before, but that's certainly not correct. And
>> that incorrect pattern repeats over and over again in this code.
>>
>> Apart from that why the heck do you want to allocate more than 1<<31 handles?
> tc action indexes for example. That is part of this patchset.
Well, let me refine the question: Why does tc action indexes need more
than 31 bits? From an outside view that looks like pure overkill.
Regards,
Christian.
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 8:31 ` Christian König
@ 2017-08-16 8:39 ` Jiri Pirko
2017-08-16 8:55 ` Christian König
2017-08-16 14:28 ` David Laight
1 sibling, 1 reply; 35+ messages in thread
From: Jiri Pirko @ 2017-08-16 8:39 UTC (permalink / raw)
To: Christian König; +Cc: Chris Mi, netdev
Wed, Aug 16, 2017 at 10:31:35AM CEST, christian.koenig@amd.com wrote:
>Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
>> Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>> > Am 16.08.2017 um 04:12 schrieb Chris Mi:
>> > > Using current TC code, it is very slow to insert a lot of rules.
>> > >
>> > > In order to improve the rules update rate in TC,
>> > > we introduced the following two changes:
>> > > 1) changed cls_flower to use IDR to manage the filters.
>> > > 2) changed all act_xxx modules to use IDR instead of
>> > > a small hash table
>> > >
>> > > But IDR has a limitation that it uses int. TC handle uses u32.
>> > > To make sure there is no regression, we also changed IDR to use
>> > > unsigned long. All clients of IDR are changed to use new IDR API.
>> > WOW, wait a second. The idr change is touching a lot of drivers and to be
>> > honest doesn't looks correct at all.
>> >
>> > Just look at the first chunk of your modification:
>> > > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>> > > mutex_lock(&bsg_mutex);
>> > > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>> > > - if (ret < 0) {
>> > > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>> > > + GFP_KERNEL);
>> > > + if (ret) {
>> > > if (ret == -ENOSPC) {
>> > > printk(KERN_ERR "bsg: too many bsg devices\n");
>> > > ret = -EINVAL;
>> > The condition "if (ret)" will now always be true after the first allocation
>> > and so we always run into the error handling after that.
>> On success, idr_alloc returns 0.
>
>Ah, I see. You change the idr_alloc to return the resulting index as separate
>parameter.
>
>You should explicit note that in the commit message, cause that is something
>easily overlooked.
>
>In general I strongly suggest to add a separate interface for allocating
>unsigned long handles, use that for the while being and then move the
>existing drivers over bit by bit.
>
>A single patch which touches so many different driver is practically
>impossible to review consequently.
Understood. I think is is good to avoid having some "idr_alloc2". That
is why I suggested to do this in one go, to avoid "idr_alloc2" and then
patch to rename "idr_alloc2" to "idr_alloc" once nobody uses the original
"idr_alloc". In fact, if you do it driver, by driver, the review burden
would be the same, probably even bigger, you'll just have 100+ patches.
Why would it help?
I believe that the changes in drivers are trivial enough to have it in
one patch.
>
>> > I've never read the bsg code before, but that's certainly not correct. And
>> > that incorrect pattern repeats over and over again in this code.
>> >
>> > Apart from that why the heck do you want to allocate more than 1<<31 handles?
>> tc action indexes for example. That is part of this patchset.
>
>Well, let me refine the question: Why does tc action indexes need more than
>31 bits? From an outside view that looks like pure overkill.
That is current state, uapi. We have to live with it.
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 8:39 ` Jiri Pirko
@ 2017-08-16 8:55 ` Christian König
2017-08-16 9:31 ` Jiri Pirko
0 siblings, 1 reply; 35+ messages in thread
From: Christian König @ 2017-08-16 8:55 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Chris Mi, netdev
Am 16.08.2017 um 10:39 schrieb Jiri Pirko:
> Wed, Aug 16, 2017 at 10:31:35AM CEST, christian.koenig@amd.com wrote:
>> Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
>>> Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>>>> Am 16.08.2017 um 04:12 schrieb Chris Mi:
>>>>> Using current TC code, it is very slow to insert a lot of rules.
>>>>>
>>>>> In order to improve the rules update rate in TC,
>>>>> we introduced the following two changes:
>>>>> 1) changed cls_flower to use IDR to manage the filters.
>>>>> 2) changed all act_xxx modules to use IDR instead of
>>>>> a small hash table
>>>>>
>>>>> But IDR has a limitation that it uses int. TC handle uses u32.
>>>>> To make sure there is no regression, we also changed IDR to use
>>>>> unsigned long. All clients of IDR are changed to use new IDR API.
>>>> WOW, wait a second. The idr change is touching a lot of drivers and to be
>>>> honest doesn't looks correct at all.
>>>>
>>>> Just look at the first chunk of your modification:
>>>>> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>>>>> mutex_lock(&bsg_mutex);
>>>>> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>>>>> - if (ret < 0) {
>>>>> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>>>>> + GFP_KERNEL);
>>>>> + if (ret) {
>>>>> if (ret == -ENOSPC) {
>>>>> printk(KERN_ERR "bsg: too many bsg devices\n");
>>>>> ret = -EINVAL;
>>>> The condition "if (ret)" will now always be true after the first allocation
>>>> and so we always run into the error handling after that.
>>> On success, idr_alloc returns 0.
>> Ah, I see. You change the idr_alloc to return the resulting index as separate
>> parameter.
>>
>> You should explicit note that in the commit message, cause that is something
>> easily overlooked.
>>
>> In general I strongly suggest to add a separate interface for allocating
>> unsigned long handles, use that for the while being and then move the
>> existing drivers over bit by bit.
>>
>> A single patch which touches so many different driver is practically
>> impossible to review consequently.
> Understood. I think is is good to avoid having some "idr_alloc2". That
> is why I suggested to do this in one go, to avoid "idr_alloc2" and then
> patch to rename "idr_alloc2" to "idr_alloc" once nobody uses the original
> "idr_alloc". In fact, if you do it driver, by driver, the review burden
> would be the same, probably even bigger, you'll just have 100+ patches.
> Why would it help?
Because it would give each maintainer only the part of the change he is
interested in.
Current status of this change is that you send a mail with nearly 300
people on CC.
Do you really expect to get an reviewed-by or acked-by on this single
patch from all of them?
If yes then it somehow makes sense to send the patch bit by bit, if no
then it doesn't seem to make to much sense to CC them all individually.
>>>> I've never read the bsg code before, but that's certainly not correct. And
>>>> that incorrect pattern repeats over and over again in this code.
>>>>
>>>> Apart from that why the heck do you want to allocate more than 1<<31 handles?
>>> tc action indexes for example. That is part of this patchset.
>> Well, let me refine the question: Why does tc action indexes need more than
>> 31 bits? From an outside view that looks like pure overkill.
> That is current state, uapi. We have to live with it.
Is the range to allocate from part of the uapi or what is the issue here?
If the issue is that userspace can specify the handle then I suggest
that you use the radix tree directly instead of the idr wrapper around it.
Regards,
Christian.
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 8:55 ` Christian König
@ 2017-08-16 9:31 ` Jiri Pirko
2017-08-16 9:41 ` Christian König
0 siblings, 1 reply; 35+ messages in thread
From: Jiri Pirko @ 2017-08-16 9:31 UTC (permalink / raw)
To: Christian König; +Cc: Chris Mi, netdev
Wed, Aug 16, 2017 at 10:55:56AM CEST, christian.koenig@amd.com wrote:
>Am 16.08.2017 um 10:39 schrieb Jiri Pirko:
>> Wed, Aug 16, 2017 at 10:31:35AM CEST, christian.koenig@amd.com wrote:
>> > Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
>> > > Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
>> > > > Am 16.08.2017 um 04:12 schrieb Chris Mi:
>> > > > > Using current TC code, it is very slow to insert a lot of rules.
>> > > > >
>> > > > > In order to improve the rules update rate in TC,
>> > > > > we introduced the following two changes:
>> > > > > 1) changed cls_flower to use IDR to manage the filters.
>> > > > > 2) changed all act_xxx modules to use IDR instead of
>> > > > > a small hash table
>> > > > >
>> > > > > But IDR has a limitation that it uses int. TC handle uses u32.
>> > > > > To make sure there is no regression, we also changed IDR to use
>> > > > > unsigned long. All clients of IDR are changed to use new IDR API.
>> > > > WOW, wait a second. The idr change is touching a lot of drivers and to be
>> > > > honest doesn't looks correct at all.
>> > > >
>> > > > Just look at the first chunk of your modification:
>> > > > > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>> > > > > mutex_lock(&bsg_mutex);
>> > > > > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
>> > > > > - if (ret < 0) {
>> > > > > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
>> > > > > + GFP_KERNEL);
>> > > > > + if (ret) {
>> > > > > if (ret == -ENOSPC) {
>> > > > > printk(KERN_ERR "bsg: too many bsg devices\n");
>> > > > > ret = -EINVAL;
>> > > > The condition "if (ret)" will now always be true after the first allocation
>> > > > and so we always run into the error handling after that.
>> > > On success, idr_alloc returns 0.
>> > Ah, I see. You change the idr_alloc to return the resulting index as separate
>> > parameter.
>> >
>> > You should explicit note that in the commit message, cause that is something
>> > easily overlooked.
>> >
>> > In general I strongly suggest to add a separate interface for allocating
>> > unsigned long handles, use that for the while being and then move the
>> > existing drivers over bit by bit.
>> >
>> > A single patch which touches so many different driver is practically
>> > impossible to review consequently.
>> Understood. I think is is good to avoid having some "idr_alloc2". That
>> is why I suggested to do this in one go, to avoid "idr_alloc2" and then
>> patch to rename "idr_alloc2" to "idr_alloc" once nobody uses the original
>> "idr_alloc". In fact, if you do it driver, by driver, the review burden
>> would be the same, probably even bigger, you'll just have 100+ patches.
>> Why would it help?
>
>Because it would give each maintainer only the part of the change he is
>interested in.
>
>Current status of this change is that you send a mail with nearly 300 people
>on CC.
That was a mistake to cc all.
>
>Do you really expect to get an reviewed-by or acked-by on this single patch
>from all of them?
I don't. It is an API change, maintainers of the individual drivers are
not expected to review the patches like this.
>
>If yes then it somehow makes sense to send the patch bit by bit, if no then
>it doesn't seem to make to much sense to CC them all individually.
>
>> > > > I've never read the bsg code before, but that's certainly not correct. And
>> > > > that incorrect pattern repeats over and over again in this code.
>> > > >
>> > > > Apart from that why the heck do you want to allocate more than 1<<31 handles?
>> > > tc action indexes for example. That is part of this patchset.
>> > Well, let me refine the question: Why does tc action indexes need more than
>> > 31 bits? From an outside view that looks like pure overkill.
>> That is current state, uapi. We have to live with it.
>
>Is the range to allocate from part of the uapi or what is the issue here?
Yes.
>
>If the issue is that userspace can specify the handle then I suggest that you
>use the radix tree directly instead of the idr wrapper around it.
But why? idr is exactly the tool we need. Only signed int does not suit
us. In fact, it does not make sense idr is using signed int when it
uses radix tree with unsigned long under the hood.
>
>Regards,
>Christian.
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 9:31 ` Jiri Pirko
@ 2017-08-16 9:41 ` Christian König
0 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 9:41 UTC (permalink / raw)
To: Jiri Pirko; +Cc: Chris Mi, netdev
Am 16.08.2017 um 11:31 schrieb Jiri Pirko:
> [SNIP]
> I don't. It is an API change, maintainers of the individual drivers are
> not expected to review the patches like this.
Yeah, completely agree.
>> If yes then it somehow makes sense to send the patch bit by bit, if no then
>> it doesn't seem to make to much sense to CC them all individually.
>>
>>>>>> I've never read the bsg code before, but that's certainly not correct. And
>>>>>> that incorrect pattern repeats over and over again in this code.
>>>>>>
>>>>>> Apart from that why the heck do you want to allocate more than 1<<31 handles?
>>>>> tc action indexes for example. That is part of this patchset.
>>>> Well, let me refine the question: Why does tc action indexes need more than
>>>> 31 bits? From an outside view that looks like pure overkill.
>>> That is current state, uapi. We have to live with it.
>> Is the range to allocate from part of the uapi or what is the issue here?
> Yes.
A bit strange uapi design, but ok in this case that change actually
makes sense.
>> If the issue is that userspace can specify the handle then I suggest that you
>> use the radix tree directly instead of the idr wrapper around it.
> But why? idr is exactly the tool we need. Only signed int does not suit
> us. In fact, it does not make sense idr is using signed int when it
> uses radix tree with unsigned long under the hood.
Well it always depends on what you do and how to use it.
In amdgpu for example for have very very short lived objects and only
few of them are active at the same time.
The solution was not to use and idr, but rather 64bit identifiers and a
ring buffer with the last 128 entries.
But in your case changing the idr calling convention actually makes
sense (at least from the tn mile high view), feel free to add an
Acked-by: Christian König <christian.koenig@amd.com> on it.
Regards,
Christian.
^ permalink raw reply [flat|nested] 35+ messages in thread
* RE: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 8:31 ` Christian König
2017-08-16 8:39 ` Jiri Pirko
@ 2017-08-16 14:28 ` David Laight
1 sibling, 0 replies; 35+ messages in thread
From: David Laight @ 2017-08-16 14:28 UTC (permalink / raw)
To: 'Christian König', Jiri Pirko; +Cc: Chris Mi, netdev
From: Christian König
> Sent: 16 August 2017 09:32
> Am 16.08.2017 um 10:16 schrieb Jiri Pirko:
> > Wed, Aug 16, 2017 at 09:49:07AM CEST, christian.koenig@amd.com wrote:
> >> Am 16.08.2017 um 04:12 schrieb Chris Mi:
...
> >>> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> >>> - if (ret < 0) {
> >>> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> >>> + GFP_KERNEL);
> >>> + if (ret) {
> >>> if (ret == -ENOSPC) {
> >>> printk(KERN_ERR "bsg: too many bsg devices\n");
> >>> ret = -EINVAL;
> >> The condition "if (ret)" will now always be true after the first allocation
> >> and so we always run into the error handling after that.
> > On success, idr_alloc returns 0.
>
> Ah, I see. You change the idr_alloc to return the resulting index as
> separate parameter.
Returning values by reference typically generates considerably worse code
that using the function return value.
It isn't just the extra parameter, it can constrain the generated code
in other ways.
That is why ERR_PTR() and friends exist.
IMHO You need a really good reason to make this change.
David
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 7:49 ` Christian König
` (3 preceding siblings ...)
(?)
@ 2017-08-16 9:19 ` Chris Wilson
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp, paulus,
jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie, david1.zhou,
linux-samsung-soc, maximlevitsky, sudarsana.kalluru, marek.vasut,
linux-atm-general, dtwlin, michel.daenzer, dledford, tpmdd-devel,
stern, longman, niranjana.vishwanathapura, philipp.reisner, shli,
linux, ohad, pmladek, dick.kennedy, linux-pm, ericv
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 7:49 ` Christian König
` (4 preceding siblings ...)
(?)
@ 2017-08-16 9:19 ` Chris Wilson
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 7:49 ` Christian König
` (3 preceding siblings ...)
(?)
@ 2017-08-16 9:19 ` Chris Wilson
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: aditr, stern, agk, alexander.shishkin, alexandre.bounine,
alexander.deucher, oakad, ast, elder, adobriyan, alex.williamson,
AlexBin.Xie, viro, amd-gfx, amitkarwar, andresx7,
andrew.donnellan, afd, akpm, anil.gurumurthy, anna.schumaker,
acme, arnd, dedekind1, ashutosh.dixit, ath10k, Bart.VanAssche,
bhaktipriya96, bjorn.andersson, boris.brezillon,
computersforpeace, bryan.thompson, cgroups, 3chas3, ccaulfie,
david1.zhou, cluster-devel, colin.king, xiyou.wangcong,
cyrille.pitchen, daniel, daniel.vetter,
dasaratharaman.chandramouli, airlied, dsa, airlied, david.binder,
dhowells, david.kershner, dtwlin, dave, davem, teigland,
dwindsor, dwmw2, dennis.dalessandro, devel, devesh.sharma,
devicetree, dick.kennedy, dm-devel, don.hiatt, dgilbert,
dledford, drbd-dev, dri-devel, elena.reshetova, edumazet, eparis,
ericvh, ebiederm, evan.quan, felipe.balbi, Felix.Kuehling,
f.fainelli, fw, frowand.list, fbarrat, fujita.tomonori, gbhat,
geliangtang, kraxel, gregkh, greybus-dev, linux, gustavo.padovan,
hal.rosenstock, hannes, hare, ishkamiel, hans.westgaard.ry,
ray.huang, mingo, inki.dae, intel-gfx, intel-gvt-dev, iommu,
ira.weiny, jinpu.wang, jhs, jejb, james.smart, jani.nikula, jack,
jarkko.sakkinen, jarno, Jason, jgunthorpe, jasowang, javier,
bfields, jlayton, axboe, jens.wiklander, jiangyilism, jiri, jiri,
jlbec, joro, johan, johannes, hannes, john, jonathanh, jon.maloy,
joonas.lahtinen, jy0922.shim, jbacik, Jerry.Zhang, jsarha,
Kai.Makisara, kvalo, keescook, krzk, kgene, kvm, kyungmin.park,
jiangshanlai, lars.ellenberg, lucho, lee.jones, leo.liu, leon,
linux1394-devel, linux-arm-kernel, linux-atm-general,
linux-block, linux-i2c, linux-kernel, linux-mm, linux-mtd,
linux-nfs, linux-pm, linuxppc-dev, linux-ppp, linux-raid,
linux-rdma, linux-remoteproc, linux-samsung-soc, linux-scsi,
linux-sctp, linux-tegra, linux-usb, linux-wireless, logang, majd,
manfred, tpmdd, marcos.souza.org, marek.vasut, mario.kleiner.de,
markb, mfasheh, elfring, martin.petersen, matan, mawilcox,
mporter, mchehab, maximlevitsky, mst, mhocko, michel.daenzer,
mike.marciniszyn, rppt, snitzer, mszeredi, minchan, tom.leiming,
monis, Monk.Liu, nbd-general, neilb, nhorman, nab,
nicolai.haehnle, nicolas.dichtel, niranjana.vishwanathapura,
nishants, ngupta, ocfs2-devel, ohad, oneukum, osandov, ogerlitz,
pali.rohar, pantelis.antoniou, paulus, paul, peterhuewe, peterz,
pmladek, philipp.reisner, pshelar, rjw, richard, rlove, robh+dt,
giometti, rogerq, roman.kapl, rminnich, rmk+kernel,
sainath.grandhi, sameer.wadgaonkar, sean.hefty, seanpaul,
bigeasy, sre, nsekhar, selvin.xavier, sergey.senozhatsky.work,
sw0312.kim, p.shailesh, shli, shaun.tancheff, syeh,
sparmaintainer, stefanr, sboyd, stephen, swise,
sudarsana.kalluru, sudeep.dutt, sumit.semwal, target-devel, tj,
thierry.reding, thellstrom, timothy.sell, tipc-discussion,
tomas.winkler, tomi.valkeinen, tpmdd-devel, trond.myklebust,
v9fs-developer, varun, virtualization, vdavydov.dev, vyasevich,
linux-graphics-maintainer, longman, weiyj.lk, wsa, huxm,
ying.xue, yishaih, yuval.shaia, lizefan, zhenyuw, zhi.a.wang
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 9:19 ` Chris Wilson
0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy, linux-
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 9:19 ` Chris Wilson
0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy,
linux-pm, ericvh, geliangtang, sparmaintainer, giometti, acme,
inki.dae, alex.williamson, rppt, teigland, viro, ishkamiel,
weiyj.lk, marcos.souza.org, mike.marciniszyn, elder, nbd-general,
nhorman, nicolai.haehnle, david.binder, gregkh, linux-usb,
anil.gurumurthy, linux-kernel, varun, majd, devesh.sharma,
sameer.wadgaonkar, bhaktipriya96, alexander.deucher,
shaun.tancheff, akpm, matan, jlbec, kraxel, Jason, timothy.sell,
airlied, linux-wireless, pantelis.antoniou, sudeep.dutt, neilb,
edumazet, target-devel, linux-i2c, daniel.vetter, jsarha,
osandov, agk, drbd-dev, boris.brezillon, thellstrom, dave, paul,
leon, sainath.grandhi, gustavo.padovan, james.smart, jonathanh,
selvin.xavier, kgene, linux-graphics-maintainer, andresx7,
3chas3, airlied, sean.hefty, virtualization, hal.rosenstock, tj,
mszeredi, hannes, felipe.balbi, pali.rohar, tpmdd, linuxppc-dev,
jani.nikula, greybus-dev, nishants, swise, yuval.shaia,
xiyou.wangcong, evan.quan, lars.ellenberg, linux-arm-kernel,
dwindsor, linux-raid, syeh, bryan.thompson, Felix.Kuehling,
oneukum, fw, anna.schumaker, minchan, kyungmin.park, monis,
ebiederm, sre, don.hiatt, leo.liu, jens.wiklander,
hans.westgaard.ry, alexander.shishkin, dennis.dalessandro,
jasowang, joonas.lahtinen, jiangshanlai, ast, fbarrat, mhocko,
alexandre.bounine, linux-mtd, amd-gfx, cgroups, jlayton,
frowand.list, elena.reshetova, f.fainelli, jejb, daniel,
linux-rdma, p.shailesh, ath10k, jgunthorpe, ccaulfie,
tomi.valkeinen, Monk.Liu, dgilbert, ira.weiny, david.kershner,
adobriyan, jon.maloy, keescook, arnd, intel-gfx, jhs, zhenyuw,
v9fs-developer, rmk+kernel, seanpaul, nab, Jerry.Zhang, eparis,
nicolas.dichtel, Kai.Makisara, mporter, rogerq, linux-nfs,
martin.petersen, linux-ppp, dm-devel, sw0312.kim,
fujita.tomonori, iommu, roman.kapl, ngupta, andrew.donnellan,
cyrille.pitchen, ying.xue, colin.king, computersforpeace, logang,
davem, ocfs2-devel, rlove, jack, kvm, mst, peterz, bigeasy,
trond.myklebust, linux-remoteproc, amitkarwar, bjorn.andersson,
dhowells, linux-mm, ray.huang, jiri, peterhuewe, linux1394-devel,
lee.jones, john, devel, stephen, mario.kleiner.de, manfred,
oakad, linux-scsi, mawilcox, mfasheh, richard, joro, jiangyilism,
elfring, cluster-devel, javier, jarkko.sakkinen, mingo,
vdavydov.dev, kvalo, tipc-discussion, ogerlitz, devicetree,
lizefan, huxm, mchehab, johan, aditr, linux-block, robh+dt,
thierry.reding, hare, rminnich, linux-tegra, dsa, intel-gvt-dev,
jy0922.shim, axboe, gbhat, tomas.winkler, dedekind1, jbacik,
jarno, vyasevich, krzk, sboyd, jiri, afd, ashutosh.dixit,
yishaih, rjw, hannes, Bart.VanAssche, johannes, dwmw2,
dasaratharaman.chandramouli
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 9:19 ` Chris Wilson
0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy, linux-
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 9:19 ` Chris Wilson
0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: aditr, stern, agk, alexander.shishkin, alexandre.bounine,
alexander.deucher, oakad, ast, elder, adobriyan, alex.williamson,
AlexBin.Xie, viro, amd-gfx, amitkarwar, andresx7,
andrew.donnellan, afd, akpm, anil.gurumurthy, anna.schumaker,
acme, arnd, dedekind1, ashutosh.dixit, ath10k, Bart.VanAssche,
bhaktipriya96, bjorn.andersson, boris.brezillon,
computersforpeace, bryan.thompson, cgroups, 3chas3, ccaulfie,
david1.zhou, cluster-devel, colin.king, xiyou.wangcong,
cyrille.pitchen, daniel, daniel.vetter,
dasaratharaman.chandramouli, airlied, dsa, airlied, david.binder,
dhowells, david.kershner, dtwlin, dave, davem, teigland,
dwindsor, dwmw2, dennis.dalessandro, devel, devesh.sharma,
devicetree, dick.kennedy, dm-devel, don.hiatt, dgilbert,
dledford, drbd-dev, dri-devel, elena.reshetova, edumazet, eparis,
ericvh, ebiederm, evan.quan, felipe.balbi, Felix.Kuehling,
f.fainelli, fw, frowand.list, fbarrat, fujita.tomonori, gbhat,
geliangtang, kraxel, gregkh, greybus-dev, linux, gustavo.padovan,
hal.rosenstock, hannes, hare, ishkamiel, hans.westgaard.ry,
ray.huang, mingo, inki.dae, intel-gfx, intel-gvt-dev, iommu,
ira.weiny, jinpu.wang, jhs, jejb, james.smart, jani.nikula, jack,
jarkko.sakkinen, jarno, Jason, jgunthorpe, jasowang, javier,
bfields, jlayton, axboe, jens.wiklander, jiangyilism, jiri, jiri,
jlbec, joro, johan, johannes, hannes, john, jonathanh, jon.maloy,
joonas.lahtinen, jy0922.shim, jbacik, Jerry.Zhang, jsarha,
Kai.Makisara, kvalo, keescook, krzk, kgene, kvm, kyungmin.park,
jiangshanlai, lars.ellenberg, lucho, lee.jones, leo.liu, leon,
linux1394-devel, linux-arm-kernel, linux-atm-general,
linux-block, linux-i2c, linux-kernel, linux-mm, linux-mtd,
linux-nfs, linux-pm, linuxppc-dev, linux-ppp, linux-raid,
linux-rdma, linux-remoteproc, linux-samsung-soc, linux-scsi,
linux-sctp, linux-tegra, linux-usb, linux-wireless, logang, majd,
manfred, tpmdd, marcos.souza.org, marek.vasut, mario.kleiner.de,
markb, mfasheh, elfring, martin.petersen, matan, mawilcox,
mporter, mchehab, maximlevitsky, mst, mhocko, michel.daenzer,
mike.marciniszyn, rppt, snitzer, mszeredi, minchan, tom.leiming,
monis, Monk.Liu, nbd-general, neilb, nhorman, nab,
nicolai.haehnle, nicolas.dichtel, niranjana.vishwanathapura,
nishants, ngupta, ocfs2-devel, ohad, oneukum, osandov, ogerlitz,
pali.rohar, pantelis.antoniou, paulus, paul, peterhuewe, peterz,
pmladek, philipp.reisner, pshelar, rjw, richard, rlove, robh+dt,
giometti, rogerq, roman.kapl, rminnich, rmk+kernel,
sainath.grandhi, sameer.wadgaonkar, sean.hefty, seanpaul,
bigeasy, sre, nsekhar, selvin.xavier, sergey.senozhatsky.work,
sw0312.kim, p.shailesh, shli, shaun.tancheff, syeh,
sparmaintainer, stefanr, sboyd, stephen, swise,
sudarsana.kalluru, sudeep.dutt, sumit.semwal, target-devel, tj,
thierry.reding, thellstrom, timothy.sell, tipc-discussion,
tomas.winkler, tomi.valkeinen, tpmdd-devel, trond.myklebust,
v9fs-developer, varun, virtualization, vdavydov.dev, vyasevich,
linux-graphics-maintainer, longman, weiyj.lk, wsa, huxm,
ying.xue, yishaih, yuval.shaia, lizefan, zhenyuw, zhi.a.wang
Quoting Christian K=C3=B6nig (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
> =
> WOW, wait a second. The idr change is touching a lot of drivers and to =
> be honest doesn't looks correct at all.
> =
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, str=
uct device *parent,
> > =
> > mutex_lock(&bsg_mutex);
> > =
> > - ret =3D idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNE=
L);
> > - if (ret < 0) {
> > + ret =3D idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEV=
S,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret =3D=3D -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret =3D -EINVAL;
> The condition "if (ret)" will now always be true after the first =
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct. =
> And that incorrect pattern repeats over and over again in this code.
> =
> Apart from that why the heck do you want to allocate more than 1<<31 =
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=3D0 with end=3DINT_MAX
to maintain existing behaviour.
-Chris
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 9:19 ` Chris Wilson
0 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
_______________________________________________
ath10k mailing list
ath10k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath10k
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 7:49 ` Christian König
` (6 preceding siblings ...)
(?)
@ 2017-08-16 9:19 ` Chris Wilson
-1 siblings, 0 replies; 35+ messages in thread
From: Chris Wilson @ 2017-08-16 9:19 UTC (permalink / raw)
To: Christian König, Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp, paulus,
jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie, david1.zhou,
linux-samsung-soc, maximlevitsky, sudarsana.kalluru, marek.vasut,
linux-atm-general, dtwlin, michel.daenzer, dledford, tpmdd-devel,
stern, longman, niranjana.vishwanathapura, philipp.reisner, shli,
linux, ohad, pmladek, dick.kennedy, linux-pm, ericvh
Quoting Christian König (2017-08-16 08:49:07)
> Am 16.08.2017 um 04:12 schrieb Chris Mi:
> > Using current TC code, it is very slow to insert a lot of rules.
> >
> > In order to improve the rules update rate in TC,
> > we introduced the following two changes:
> > 1) changed cls_flower to use IDR to manage the filters.
> > 2) changed all act_xxx modules to use IDR instead of
> > a small hash table
> >
> > But IDR has a limitation that it uses int. TC handle uses u32.
> > To make sure there is no regression, we also changed IDR to use
> > unsigned long. All clients of IDR are changed to use new IDR API.
>
> WOW, wait a second. The idr change is touching a lot of drivers and to
> be honest doesn't looks correct at all.
>
> Just look at the first chunk of your modification:
> > @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
> >
> > mutex_lock(&bsg_mutex);
> >
> > - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> > - if (ret < 0) {
> > + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> > + GFP_KERNEL);
> > + if (ret) {
> > if (ret == -ENOSPC) {
> > printk(KERN_ERR "bsg: too many bsg devices\n");
> > ret = -EINVAL;
> The condition "if (ret)" will now always be true after the first
> allocation and so we always run into the error handling after that.
ret is now purely the error code, so it doesn't look that suspicious.
> I've never read the bsg code before, but that's certainly not correct.
> And that incorrect pattern repeats over and over again in this code.
>
> Apart from that why the heck do you want to allocate more than 1<<31
> handles?
And more to the point, arbitrarily changing the maximum to ULONG_MAX
where the ABI only supports U32_MAX is dangerous. Unless you do the
analysis otherwise, you have to replace all the end=0 with end=INT_MAX
to maintain existing behaviour.
-Chris
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
_______________________________________________
mailing list linux1394-devel@lists.sourceforge.net
https://lists.sourceforge.net/lists/listinfo/linux1394-devel
^ permalink raw reply [flat|nested] 35+ messages in thread
* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 2:12 ` Chris Mi
` (4 preceding siblings ...)
(?)
@ 2017-08-16 7:49 ` Christian König
-1 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp, paulus,
jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie, david1.zhou,
linux-samsung-soc, maximlevitsky, sudarsana.kalluru, marek.vasut,
linux-atm-general, dtwlin, michel.daenzer, dledford, tpmdd-devel,
stern, longman, niranjana.vishwanathapura, philipp.reisner, shli,
linux, ohad, pmladek, dick.kennedy, linux-pm, ericv
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 2:12 ` Chris Mi
` (5 preceding siblings ...)
(?)
@ 2017-08-16 7:49 ` Christian König
-1 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
stefanr, zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp,
paulus, jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie,
david1.zhou, linux-samsung-soc, maximlevitsky, sudarsana.kalluru,
marek.vasut, linux-atm-general, dtwlin, michel.daenzer, dledford,
tpmdd-devel, stern, longman, niranjana.vishwanathapura,
philipp.reisner, shli, linux, ohad, pmladek, dick.kennedy
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 2:12 ` Chris Mi
` (6 preceding siblings ...)
(?)
@ 2017-08-16 7:49 ` Christian König
-1 siblings, 0 replies; 35+ messages in thread
From: Christian König @ 2017-08-16 7:49 UTC (permalink / raw)
To: Chris Mi, netdev
Cc: lucho, sergey.senozhatsky.work, snitzer, wsa, markb, tom.leiming,
zhi.a.wang, nsekhar, dri-devel, bfields, linux-sctp, paulus,
jinpu.wang, pshelar, sumit.semwal, AlexBin.Xie, david1.zhou,
linux-samsung-soc, maximlevitsky, sudarsana.kalluru, marek.vasut,
linux-atm-general, dtwlin, michel.daenzer, dledford, tpmdd-devel,
stern, longman, niranjana.vishwanathapura, philipp.reisner, shli,
linux, ohad, pmladek, dick.kennedy, linux-pm, ericvh
Am 16.08.2017 um 04:12 schrieb Chris Mi:
> Using current TC code, it is very slow to insert a lot of rules.
>
> In order to improve the rules update rate in TC,
> we introduced the following two changes:
> 1) changed cls_flower to use IDR to manage the filters.
> 2) changed all act_xxx modules to use IDR instead of
> a small hash table
>
> But IDR has a limitation that it uses int. TC handle uses u32.
> To make sure there is no regression, we also changed IDR to use
> unsigned long. All clients of IDR are changed to use new IDR API.
WOW, wait a second. The idr change is touching a lot of drivers and to
be honest doesn't looks correct at all.
Just look at the first chunk of your modification:
> @@ -998,8 +999,9 @@ int bsg_register_queue(struct request_queue *q, struct device *parent,
>
> mutex_lock(&bsg_mutex);
>
> - ret = idr_alloc(&bsg_minor_idr, bcd, 0, BSG_MAX_DEVS, GFP_KERNEL);
> - if (ret < 0) {
> + ret = idr_alloc(&bsg_minor_idr, bcd, &idr_index, 0, BSG_MAX_DEVS,
> + GFP_KERNEL);
> + if (ret) {
> if (ret == -ENOSPC) {
> printk(KERN_ERR "bsg: too many bsg devices\n");
> ret = -EINVAL;
The condition "if (ret)" will now always be true after the first
allocation and so we always run into the error handling after that.
I've never read the bsg code before, but that's certainly not correct.
And that incorrect pattern repeats over and over again in this code.
Apart from that why the heck do you want to allocate more than 1<<31
handles?
Regards,
Christian.
>
> Chris Mi (3):
> idr: Use unsigned long instead of int
> net/sched: Change cls_flower to use IDR
> net/sched: Change act_api and act_xxx modules to use IDR
>
> block/bsg.c | 8 +-
> block/genhd.c | 12 +-
> drivers/atm/nicstar.c | 11 +-
> drivers/block/drbd/drbd_main.c | 31 +--
> drivers/block/drbd/drbd_nl.c | 22 ++-
> drivers/block/drbd/drbd_proc.c | 3 +-
> drivers/block/drbd/drbd_receiver.c | 15 +-
> drivers/block/drbd/drbd_state.c | 34 ++--
> drivers/block/drbd/drbd_worker.c | 6 +-
> drivers/block/loop.c | 17 +-
> drivers/block/nbd.c | 20 +-
> drivers/block/zram/zram_drv.c | 9 +-
> drivers/char/tpm/tpm-chip.c | 10 +-
> drivers/char/tpm/tpm.h | 2 +-
> drivers/dca/dca-sysfs.c | 9 +-
> drivers/firewire/core-cdev.c | 18 +-
> drivers/firewire/core-device.c | 15 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_bo_list.c | 8 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_ctx.c | 9 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_gem.c | 6 +-
> drivers/gpu/drm/amd/amdgpu/amdgpu_kms.c | 2 +-
> drivers/gpu/drm/drm_auth.c | 9 +-
> drivers/gpu/drm/drm_connector.c | 10 +-
> drivers/gpu/drm/drm_context.c | 20 +-
> drivers/gpu/drm/drm_dp_aux_dev.c | 11 +-
> drivers/gpu/drm/drm_drv.c | 6 +-
> drivers/gpu/drm/drm_gem.c | 19 +-
> drivers/gpu/drm/drm_info.c | 2 +-
> drivers/gpu/drm/drm_mode_object.c | 11 +-
> drivers/gpu/drm/drm_syncobj.c | 18 +-
> drivers/gpu/drm/exynos/exynos_drm_ipp.c | 25 ++-
> drivers/gpu/drm/i915/gvt/display.c | 2 +-
> drivers/gpu/drm/i915/gvt/kvmgt.c | 2 +-
> drivers/gpu/drm/i915/gvt/vgpu.c | 9 +-
> drivers/gpu/drm/i915/i915_debugfs.c | 6 +-
> drivers/gpu/drm/i915/i915_gem_context.c | 9 +-
> drivers/gpu/drm/qxl/qxl_cmd.c | 8 +-
> drivers/gpu/drm/qxl/qxl_release.c | 14 +-
> drivers/gpu/drm/sis/sis_mm.c | 8 +-
> drivers/gpu/drm/tegra/drm.c | 10 +-
> drivers/gpu/drm/tilcdc/tilcdc_slave_compat.c | 3 +-
> drivers/gpu/drm/vgem/vgem_fence.c | 12 +-
> drivers/gpu/drm/via/via_mm.c | 8 +-
> drivers/gpu/drm/virtio/virtgpu_kms.c | 5 +-
> drivers/gpu/drm/virtio/virtgpu_vq.c | 5 +-
> drivers/gpu/drm/vmwgfx/vmwgfx_resource.c | 9 +-
> drivers/i2c/i2c-core-base.c | 19 +-
> drivers/infiniband/core/cm.c | 8 +-
> drivers/infiniband/core/cma.c | 12 +-
> drivers/infiniband/core/rdma_core.c | 9 +-
> drivers/infiniband/core/sa_query.c | 23 +--
> drivers/infiniband/core/ucm.c | 7 +-
> drivers/infiniband/core/ucma.c | 14 +-
> drivers/infiniband/hw/cxgb3/iwch.c | 4 +-
> drivers/infiniband/hw/cxgb3/iwch.h | 4 +-
> drivers/infiniband/hw/cxgb4/device.c | 18 +-
> drivers/infiniband/hw/cxgb4/iw_cxgb4.h | 4 +-
> drivers/infiniband/hw/hfi1/init.c | 9 +-
> drivers/infiniband/hw/hfi1/vnic_main.c | 6 +-
> drivers/infiniband/hw/mlx4/cm.c | 13 +-
> drivers/infiniband/hw/ocrdma/ocrdma_main.c | 7 +-
> drivers/infiniband/hw/qib/qib_init.c | 9 +-
> drivers/infiniband/ulp/opa_vnic/opa_vnic_vema.c | 10 +-
> drivers/iommu/intel-svm.c | 9 +-
> drivers/md/dm.c | 13 +-
> drivers/memstick/core/memstick.c | 10 +-
> drivers/memstick/core/ms_block.c | 9 +-
> drivers/memstick/core/mspro_block.c | 12 +-
> drivers/mfd/rtsx_pcr.c | 9 +-
> drivers/misc/c2port/core.c | 7 +-
> drivers/misc/cxl/context.c | 8 +-
> drivers/misc/cxl/main.c | 15 +-
> drivers/misc/mei/main.c | 8 +-
> drivers/misc/mic/scif/scif_api.c | 11 +-
> drivers/misc/mic/scif/scif_ports.c | 18 +-
> drivers/misc/tifm_core.c | 9 +-
> drivers/mtd/mtdcore.c | 9 +-
> drivers/mtd/mtdcore.h | 2 +-
> drivers/mtd/ubi/block.c | 7 +-
> drivers/net/ppp/ppp_generic.c | 27 +--
> drivers/net/tap.c | 10 +-
> drivers/net/wireless/ath/ath10k/htt.h | 3 +-
> drivers/net/wireless/ath/ath10k/htt_tx.c | 22 ++-
> drivers/net/wireless/ath/ath10k/mac.c | 2 +-
> drivers/net/wireless/marvell/mwifiex/main.c | 13 +-
> drivers/net/wireless/marvell/mwifiex/wmm.c | 2 +-
> drivers/of/overlay.c | 15 +-
> drivers/of/unittest.c | 25 ++-
> drivers/power/supply/bq2415x_charger.c | 16 +-
> drivers/power/supply/bq27xxx_battery_i2c.c | 15 +-
> drivers/power/supply/ds2782_battery.c | 9 +-
> drivers/powercap/powercap_sys.c | 8 +-
> drivers/pps/pps.c | 10 +-
> drivers/rapidio/rio_cm.c | 17 +-
> drivers/remoteproc/remoteproc_core.c | 8 +-
> drivers/rpmsg/virtio_rpmsg_bus.c | 8 +-
> drivers/scsi/bfa/bfad_im.c | 8 +-
> drivers/scsi/ch.c | 8 +-
> drivers/scsi/lpfc/lpfc_crtn.h | 2 +-
> drivers/scsi/lpfc/lpfc_init.c | 11 +-
> drivers/scsi/lpfc/lpfc_vport.c | 8 +-
> drivers/scsi/sg.c | 10 +-
> drivers/scsi/st.c | 8 +-
> drivers/staging/greybus/uart.c | 22 +--
> drivers/staging/unisys/visorhba/visorhba_main.c | 7 +-
> drivers/target/iscsi/iscsi_target.c | 7 +-
> drivers/target/iscsi/iscsi_target_login.c | 9 +-
> drivers/target/target_core_device.c | 9 +-
> drivers/target/target_core_user.c | 13 +-
> drivers/tee/tee_shm.c | 8 +-
> drivers/uio/uio.c | 9 +-
> drivers/usb/class/cdc-acm.c | 24 +--
> drivers/usb/core/devices.c | 2 +-
> drivers/usb/core/hcd.c | 7 +-
> drivers/usb/mon/mon_main.c | 3 +-
> drivers/usb/serial/usb-serial.c | 11 +-
> drivers/vfio/vfio.c | 15 +-
> fs/dlm/lock.c | 9 +-
> fs/dlm/lockspace.c | 6 +-
> fs/dlm/recover.c | 10 +-
> fs/nfs/nfs4client.c | 9 +-
> fs/nfsd/nfs4state.c | 8 +-
> fs/notify/inotify/inotify_fsnotify.c | 4 +-
> fs/notify/inotify/inotify_user.c | 9 +-
> fs/ocfs2/cluster/tcp.c | 10 +-
> include/linux/idr.h | 26 +--
> include/linux/of.h | 4 +-
> include/linux/radix-tree.h | 2 +-
> include/net/9p/9p.h | 2 +-
> include/net/act_api.h | 76 +++-----
> ipc/msg.c | 2 +-
> ipc/sem.c | 2 +-
> ipc/shm.c | 4 +-
> ipc/util.c | 17 +-
> kernel/bpf/syscall.c | 20 +-
> kernel/cgroup/cgroup.c | 57 +++---
> kernel/events/core.c | 10 +-
> kernel/workqueue.c | 15 +-
> lib/idr.c | 38 ++--
> lib/radix-tree.c | 5 +-
> mm/memcontrol.c | 11 +-
> net/9p/client.c | 17 +-
> net/9p/util.c | 14 +-
> net/core/net_namespace.c | 23 ++-
> net/mac80211/cfg.c | 23 +--
> net/mac80211/iface.c | 3 +-
> net/mac80211/main.c | 2 +-
> net/mac80211/tx.c | 7 +-
> net/mac80211/util.c | 3 +-
> net/netlink/genetlink.c | 18 +-
> net/qrtr/qrtr.c | 21 +-
> net/rxrpc/conn_client.c | 15 +-
> net/sched/act_api.c | 249 +++++++++++-------------
> net/sched/act_bpf.c | 17 +-
> net/sched/act_connmark.c | 16 +-
> net/sched/act_csum.c | 16 +-
> net/sched/act_gact.c | 16 +-
> net/sched/act_ife.c | 20 +-
> net/sched/act_ipt.c | 26 ++-
> net/sched/act_mirred.c | 19 +-
> net/sched/act_nat.c | 16 +-
> net/sched/act_pedit.c | 18 +-
> net/sched/act_police.c | 18 +-
> net/sched/act_sample.c | 17 +-
> net/sched/act_simple.c | 20 +-
> net/sched/act_skbedit.c | 18 +-
> net/sched/act_skbmod.c | 18 +-
> net/sched/act_tunnel_key.c | 20 +-
> net/sched/act_vlan.c | 22 +--
> net/sched/cls_flower.c | 55 +++---
> net/sctp/associola.c | 8 +-
> net/tipc/server.c | 7 +-
> 172 files changed, 1256 insertions(+), 1113 deletions(-)
>
------------------------------------------------------------------------------
Check out the vibrant tech community on one of the world's most
engaging tech sites, Slashdot.org! http://sdm.link/slashdot
^ permalink raw reply [flat|nested] 35+ messages in thread* Re: [patch net-next 0/3] net/sched: Improve getting objects by indexes
2017-08-16 2:12 ` Chris Mi
@ 2017-08-16 21:51 ` Frank Rowand
-1 siblings, 0 replies; 35+ messages in thread
From: Frank Rowand @ 2017-08-16 21:51 UTC (permalink / raw)
To: Chris Mi; +Cc: arm-soc, linux-kernel, Pantelis Antoniou, Rob Herring
I deleted most of the distribution list. My email server rejects an email
with this many recipients.
On 08/15/17 19:12, Chris Mi wrote:
> IDR uses internally radix tree which uses unsigned long. It doesn't
> makes sense to have index as signed value.
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
< snip >
> drivers/of/overlay.c | 15 +++----
> drivers/of/unittest.c | 25 ++++++-----
< snip >
> include/linux/of.h | 4 +-
< snip >
Split the patch apart.
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index c0e4ee1..e5cfe01 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -373,10 +373,11 @@ static int of_free_overlay_info(struct of_overlay *ov)
> *
> * Returns the id of the created overlay, or a negative error number
> */
> -int of_overlay_create(struct device_node *tree)
> +int of_overlay_create(struct device_node *tree, unsigned long *id)
Added parameter *id, but never assigned a value to it.
> {
> struct of_overlay *ov;
> - int err, id;
> + unsigned long idr_index;
> + int err;
>
> /* allocate the overlay structure */
> ov = kzalloc(sizeof(*ov), GFP_KERNEL);
> @@ -390,12 +391,10 @@ int of_overlay_create(struct device_node *tree)
>
> mutex_lock(&of_mutex);
>
> - id = idr_alloc(&ov_idr, ov, 0, 0, GFP_KERNEL);
> - if (id < 0) {
> - err = id;
> + err = idr_alloc(&ov_idr, ov, &idr_index, 0, 0, GFP_KERNEL);
> + if (err)
> goto err_destroy_trans;
> - }
> - ov->id = id;
> + ov->id = idr_index;
>
> /* build the overlay info structures */
> err = of_build_overlay_info(ov, tree);
> @@ -430,7 +429,7 @@ int of_overlay_create(struct device_node *tree)
>
> mutex_unlock(&of_mutex);
>
> - return id;
> + return err;
>
> err_revert_overlay:
> err_abort_trans:
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 0107fc6..ac7cc76 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -1242,7 +1242,8 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr,
> int *overlay_id)
> {
> struct device_node *np = NULL;
> - int ret, id = -1;
> + unsigned long id = -1;
Assigns a negative value to an unsigned.
> + int ret;
>
> np = of_find_node_by_path(overlay_path(overlay_nr));
> if (np == NULL) {
> @@ -1252,17 +1253,14 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr,
> goto out;
> }
>
> - ret = of_overlay_create(np);
> - if (ret < 0) {
> + ret = of_overlay_create(np, &id);
> + if (ret) {
> unittest(0, "could not create overlay from \"%s\"\n",
> overlay_path(overlay_nr));
> goto out;
> }
> - id = ret;
> of_unittest_track_overlay(id);
>
> - ret = 0;
> -
> out:
> of_node_put(np);
>
> @@ -1442,6 +1440,7 @@ static void of_unittest_overlay_6(void)
> int ret, i, ov_id[2];
> int overlay_nr = 6, unittest_nr = 6;
> int before = 0, after = 1;
> + unsigned long id;
>
> /* unittest device must be in before state */
> for (i = 0; i < 2; i++) {
> @@ -1466,13 +1465,13 @@ static void of_unittest_overlay_6(void)
> return;
> }
>
> - ret = of_overlay_create(np);
> - if (ret < 0) {
> + ret = of_overlay_create(np, &id);
> + if (ret) {
> unittest(0, "could not create overlay from \"%s\"\n",
> overlay_path(overlay_nr + i));
> return;
> }
> - ov_id[i] = ret;
> + ov_id[i] = id;
> of_unittest_track_overlay(ov_id[i]);
> }
>
> @@ -2094,6 +2093,7 @@ static int __init overlay_data_add(int onum)
> int ret;
> u32 size;
> u32 size_from_header;
> + unsigned long id;
>
> for (k = 0, info = overlays; info; info++, k++) {
> if (k == onum)
> @@ -2138,13 +2138,12 @@ static int __init overlay_data_add(int onum)
> goto out_free_np_overlay;
> }
>
> - ret = of_overlay_create(info->np_overlay);
> - if (ret < 0) {
> + ret = of_overlay_create(info->np_overlay, &id);
> + if (ret) {
> pr_err("of_overlay_create() (ret=%d), %d\n", ret, onum);
> goto out_free_np_overlay;
> } else {
> - info->overlay_id = ret;
> - ret = 0;
> + info->overlay_id = id;
> }
>
> pr_debug("__dtb_overlay_begin applied, overlay id %d\n", ret);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 4a8a709..ceb14bf 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1307,7 +1307,7 @@ struct of_overlay_notify_data {
> #ifdef CONFIG_OF_OVERLAY
>
> /* ID based overlays; the API for external users */
> -int of_overlay_create(struct device_node *tree);
> +int of_overlay_create(struct device_node *tree, *unsigned long *id);
*unsigned long *id should be: unsigned long *id
How did you test this patch?
> int of_overlay_destroy(int id);
> int of_overlay_destroy_all(void);
>
> @@ -1316,7 +1316,7 @@ struct of_overlay_notify_data {
>
> #else
>
> -static inline int of_overlay_create(struct device_node *tree)
> +static inline int of_overlay_create(struct device_node *tree, unsigned long *id)
> {
> return -ENOTSUPP;
> }
^ permalink raw reply [flat|nested] 35+ messages in thread* [patch net-next 0/3] net/sched: Improve getting objects by indexes
@ 2017-08-16 21:51 ` Frank Rowand
0 siblings, 0 replies; 35+ messages in thread
From: Frank Rowand @ 2017-08-16 21:51 UTC (permalink / raw)
To: linux-arm-kernel
I deleted most of the distribution list. My email server rejects an email
with this many recipients.
On 08/15/17 19:12, Chris Mi wrote:
> IDR uses internally radix tree which uses unsigned long. It doesn't
> makes sense to have index as signed value.
>
> Signed-off-by: Chris Mi <chrism@mellanox.com>
> Signed-off-by: Jiri Pirko <jiri@mellanox.com>
> ---
< snip >
> drivers/of/overlay.c | 15 +++----
> drivers/of/unittest.c | 25 ++++++-----
< snip >
> include/linux/of.h | 4 +-
< snip >
Split the patch apart.
> diff --git a/drivers/of/overlay.c b/drivers/of/overlay.c
> index c0e4ee1..e5cfe01 100644
> --- a/drivers/of/overlay.c
> +++ b/drivers/of/overlay.c
> @@ -373,10 +373,11 @@ static int of_free_overlay_info(struct of_overlay *ov)
> *
> * Returns the id of the created overlay, or a negative error number
> */
> -int of_overlay_create(struct device_node *tree)
> +int of_overlay_create(struct device_node *tree, unsigned long *id)
Added parameter *id, but never assigned a value to it.
> {
> struct of_overlay *ov;
> - int err, id;
> + unsigned long idr_index;
> + int err;
>
> /* allocate the overlay structure */
> ov = kzalloc(sizeof(*ov), GFP_KERNEL);
> @@ -390,12 +391,10 @@ int of_overlay_create(struct device_node *tree)
>
> mutex_lock(&of_mutex);
>
> - id = idr_alloc(&ov_idr, ov, 0, 0, GFP_KERNEL);
> - if (id < 0) {
> - err = id;
> + err = idr_alloc(&ov_idr, ov, &idr_index, 0, 0, GFP_KERNEL);
> + if (err)
> goto err_destroy_trans;
> - }
> - ov->id = id;
> + ov->id = idr_index;
>
> /* build the overlay info structures */
> err = of_build_overlay_info(ov, tree);
> @@ -430,7 +429,7 @@ int of_overlay_create(struct device_node *tree)
>
> mutex_unlock(&of_mutex);
>
> - return id;
> + return err;
>
> err_revert_overlay:
> err_abort_trans:
> diff --git a/drivers/of/unittest.c b/drivers/of/unittest.c
> index 0107fc6..ac7cc76 100644
> --- a/drivers/of/unittest.c
> +++ b/drivers/of/unittest.c
> @@ -1242,7 +1242,8 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr,
> int *overlay_id)
> {
> struct device_node *np = NULL;
> - int ret, id = -1;
> + unsigned long id = -1;
Assigns a negative value to an unsigned.
> + int ret;
>
> np = of_find_node_by_path(overlay_path(overlay_nr));
> if (np == NULL) {
> @@ -1252,17 +1253,14 @@ static int of_unittest_apply_overlay(int overlay_nr, int unittest_nr,
> goto out;
> }
>
> - ret = of_overlay_create(np);
> - if (ret < 0) {
> + ret = of_overlay_create(np, &id);
> + if (ret) {
> unittest(0, "could not create overlay from \"%s\"\n",
> overlay_path(overlay_nr));
> goto out;
> }
> - id = ret;
> of_unittest_track_overlay(id);
>
> - ret = 0;
> -
> out:
> of_node_put(np);
>
> @@ -1442,6 +1440,7 @@ static void of_unittest_overlay_6(void)
> int ret, i, ov_id[2];
> int overlay_nr = 6, unittest_nr = 6;
> int before = 0, after = 1;
> + unsigned long id;
>
> /* unittest device must be in before state */
> for (i = 0; i < 2; i++) {
> @@ -1466,13 +1465,13 @@ static void of_unittest_overlay_6(void)
> return;
> }
>
> - ret = of_overlay_create(np);
> - if (ret < 0) {
> + ret = of_overlay_create(np, &id);
> + if (ret) {
> unittest(0, "could not create overlay from \"%s\"\n",
> overlay_path(overlay_nr + i));
> return;
> }
> - ov_id[i] = ret;
> + ov_id[i] = id;
> of_unittest_track_overlay(ov_id[i]);
> }
>
> @@ -2094,6 +2093,7 @@ static int __init overlay_data_add(int onum)
> int ret;
> u32 size;
> u32 size_from_header;
> + unsigned long id;
>
> for (k = 0, info = overlays; info; info++, k++) {
> if (k == onum)
> @@ -2138,13 +2138,12 @@ static int __init overlay_data_add(int onum)
> goto out_free_np_overlay;
> }
>
> - ret = of_overlay_create(info->np_overlay);
> - if (ret < 0) {
> + ret = of_overlay_create(info->np_overlay, &id);
> + if (ret) {
> pr_err("of_overlay_create() (ret=%d), %d\n", ret, onum);
> goto out_free_np_overlay;
> } else {
> - info->overlay_id = ret;
> - ret = 0;
> + info->overlay_id = id;
> }
>
> pr_debug("__dtb_overlay_begin applied, overlay id %d\n", ret);
> diff --git a/include/linux/of.h b/include/linux/of.h
> index 4a8a709..ceb14bf 100644
> --- a/include/linux/of.h
> +++ b/include/linux/of.h
> @@ -1307,7 +1307,7 @@ struct of_overlay_notify_data {
> #ifdef CONFIG_OF_OVERLAY
>
> /* ID based overlays; the API for external users */
> -int of_overlay_create(struct device_node *tree);
> +int of_overlay_create(struct device_node *tree, *unsigned long *id);
*unsigned long *id should be: unsigned long *id
How did you test this patch?
> int of_overlay_destroy(int id);
> int of_overlay_destroy_all(void);
>
> @@ -1316,7 +1316,7 @@ struct of_overlay_notify_data {
>
> #else
>
> -static inline int of_overlay_create(struct device_node *tree)
> +static inline int of_overlay_create(struct device_node *tree, unsigned long *id)
> {
> return -ENOTSUPP;
> }
^ permalink raw reply [flat|nested] 35+ messages in thread