On Wed, Nov 24, 2021 at 12:03:03PM +0100, Uladzislau Rezki (Sony) wrote: > Instead of invoking a synchronize_rcu() to free a pointer > after a grace period we can directly make use of new API > that does the same but in more efficient way. It isn't entirely new, kfree_rcu() has been around for ages and any of these call sites could have made use of it if they wanted. The kvfree_rcu() just adds the twist of transparently allocating memory. We really must ask in each case why the original author didn't use kfree_rcu().. > drivers/block/drbd/drbd_nl.c | 9 +++------ > drivers/block/drbd/drbd_receiver.c | 6 ++---- > drivers/block/drbd/drbd_state.c | 3 +-- > drivers/block/rnbd/rnbd-srv.c | 3 +-- > drivers/crypto/nx/nx-common-pseries.c | 3 +-- > drivers/infiniband/hw/hfi1/sdma.c | 3 +-- > drivers/ipack/carriers/tpci200.c | 3 +-- > drivers/mfd/dln2.c | 6 ++---- > drivers/misc/vmw_vmci/vmci_context.c | 6 ++---- > drivers/misc/vmw_vmci/vmci_event.c | 3 +-- > drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c | 3 +-- > drivers/net/ethernet/mellanox/mlx5/core/en/qos.c | 3 +-- > drivers/net/ethernet/mellanox/mlx5/core/fpga/tls.c | 3 +-- > drivers/net/ethernet/mellanox/mlxsw/core.c | 3 +-- > drivers/scsi/device_handler/scsi_dh_alua.c | 3 +-- > drivers/scsi/device_handler/scsi_dh_rdac.c | 3 +-- > drivers/staging/fwserial/fwserial.c | 3 +-- These all need to be split to single patches and ack'ed by experts. > diff --git a/drivers/block/drbd/drbd_nl.c b/drivers/block/drbd/drbd_nl.c > index 44ccf8b4f4b2..28f4d84945fd 100644 > +++ b/drivers/block/drbd/drbd_nl.c > @@ -1679,8 +1679,7 @@ int drbd_adm_disk_opts(struct sk_buff *skb, struct genl_info *info) > drbd_send_sync_param(peer_device); > } > > - synchronize_rcu(); > - kfree(old_disk_conf); > + kvfree_rcu(old_disk_conf); > kfree(old_plan); For instance, this, how do you know that old_plan isn't also RCU protected? > diff --git a/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > index dde1cf51d0ab..0619cb94f0e0 100644 > +++ b/drivers/net/ethernet/chelsio/cxgb4/cxgb4_main.c > @@ -7190,8 +7190,7 @@ static void remove_one(struct pci_dev *pdev) > } > pci_release_regions(pdev); > kfree(adapter->mbox_log); > - synchronize_rcu(); > - kfree(adapter); > + kvfree_rcu(adapter); > } And this, for instance, just looks crazy! There is only one RCU region in this file and it is not protecting an adaptor pointer, it is protecting a netdev. No idea what this is trying to do today even. Each case needs to be audited to make sure the synchronize_rcu() is only protecting the kfree and not other things as well. It is tricky stuff. I see you got an Ack for the infiniband peice, so feel free to send that file to the linux-rdma list. Jason
