| Message ID | 20190808131100.24751-1-hslester96@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | None | expand |
On Thu, Aug 8, 2019 at 9:35 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > > On Thu, Aug 08, 2019 at 09:11:00PM +0800, Chuhong Yuan wrote: > > Reference counters are preferred to use refcount_t instead of > > atomic_t. > > This is because the implementation of refcount_t can prevent > > overflows and detect possible use-after-free. > > So convert atomic_t ref counters to refcount_t. > > Thanks! > > I think there are more reference counters in blkback than > the one you fixed. There's also an inflight field in xen_blkif_ring, > and a pendcnt in pending_req which look like possible candidates to > switch to use refcount_t, have you looked into switching those two > also? > I will switch those two in next version. > Roger.
On Thu, Aug 8, 2019 at 9:35 PM Roger Pau Monné <roger.pau@citrix.com> wrote: > > On Thu, Aug 08, 2019 at 09:11:00PM +0800, Chuhong Yuan wrote: > > Reference counters are preferred to use refcount_t instead of > > atomic_t. > > This is because the implementation of refcount_t can prevent > > overflows and detect possible use-after-free. > > So convert atomic_t ref counters to refcount_t. > > Thanks! > > I think there are more reference counters in blkback than > the one you fixed. There's also an inflight field in xen_blkif_ring, > and a pendcnt in pending_req which look like possible candidates to > switch to use refcount_t, have you looked into switching those two > also? > It seems that xen_blkif_ring::inflight is 0-based and cannot be directly converted to refcount_t. This is because the implementation of refcount_t will warn on increasing a 0 ref count. Therefore I only convert pending_req::pendcnt in v2. > Roger.
diff --git a/drivers/block/xen-blkback/common.h b/drivers/block/xen-blkback/common.h index 1d3002d773f7..9db5f3586fb4 100644 --- a/drivers/block/xen-blkback/common.h +++ b/drivers/block/xen-blkback/common.h @@ -35,6 +35,7 @@ #include <linux/wait.h> #include <linux/io.h> #include <linux/rbtree.h> +#include <linux/refcount.h> #include <asm/setup.h> #include <asm/pgalloc.h> #include <asm/hypervisor.h> @@ -309,7 +310,7 @@ struct xen_blkif { struct xen_vbd vbd; /* Back pointer to the backend_info. */ struct backend_info *be; - atomic_t refcnt; + refcount_t refcnt; /* for barrier (drain) requests */ struct completion drain_complete; atomic_t drain; @@ -362,10 +363,10 @@ struct pending_req { (_v)->bdev->bd_part->nr_sects : \ get_capacity((_v)->bdev->bd_disk)) -#define xen_blkif_get(_b) (atomic_inc(&(_b)->refcnt)) +#define xen_blkif_get(_b) (refcount_inc(&(_b)->refcnt)) #define xen_blkif_put(_b) \ do { \ - if (atomic_dec_and_test(&(_b)->refcnt)) \ + if (refcount_dec_and_test(&(_b)->refcnt)) \ schedule_work(&(_b)->free_work);\ } while (0) diff --git a/drivers/block/xen-blkback/xenbus.c b/drivers/block/xen-blkback/xenbus.c index 3ac6a5d18071..ecc5f9c5bf3f 100644 --- a/drivers/block/xen-blkback/xenbus.c +++ b/drivers/block/xen-blkback/xenbus.c @@ -169,7 +169,7 @@ static struct xen_blkif *xen_blkif_alloc(domid_t domid) return ERR_PTR(-ENOMEM); blkif->domid = domid; - atomic_set(&blkif->refcnt, 1); + refcount_set(&blkif->refcnt, 1); init_completion(&blkif->drain_complete); INIT_WORK(&blkif->free_work, xen_blkif_deferred_free);
Reference counters are preferred to use refcount_t instead of atomic_t. This is because the implementation of refcount_t can prevent overflows and detect possible use-after-free. So convert atomic_t ref counters to refcount_t. Signed-off-by: Chuhong Yuan <hslester96@gmail.com> --- drivers/block/xen-blkback/common.h | 7 ++++--- drivers/block/xen-blkback/xenbus.c | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-)