Linux-Block Archive on lore.kernel.org
 help / color / Atom feed
* [PATCH 3/3] xen/blkback: Use refcount_t for refcount
@ 2019-08-08 13:11 Chuhong Yuan
  2019-08-08 13:35 ` Roger Pau Monné
  0 siblings, 1 reply; 4+ messages in thread
From: Chuhong Yuan @ 2019-08-08 13:11 UTC (permalink / raw)
  To: unlisted-recipients:; (no To-header on input)
  Cc: Konrad Rzeszutek Wilk, Roger Pau Monné,
	Jens Axboe, xen-devel, linux-block, linux-kernel, Chuhong Yuan

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(-)

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);
 
-- 
2.20.1


^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] xen/blkback: Use refcount_t for refcount
  2019-08-08 13:11 [PATCH 3/3] xen/blkback: Use refcount_t for refcount Chuhong Yuan
@ 2019-08-08 13:35 ` Roger Pau Monné
  2019-08-09  1:12   ` Chuhong Yuan
  2019-08-13  6:10   ` Chuhong Yuan
  0 siblings, 2 replies; 4+ messages in thread
From: Roger Pau Monné @ 2019-08-08 13:35 UTC (permalink / raw)
  To: Chuhong Yuan
  Cc: Konrad Rzeszutek Wilk, Jens Axboe, xen-devel, linux-block, linux-kernel

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?

Roger.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] xen/blkback: Use refcount_t for refcount
  2019-08-08 13:35 ` Roger Pau Monné
@ 2019-08-09  1:12   ` Chuhong Yuan
  2019-08-13  6:10   ` Chuhong Yuan
  1 sibling, 0 replies; 4+ messages in thread
From: Chuhong Yuan @ 2019-08-09  1:12 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Konrad Rzeszutek Wilk, Jens Axboe, xen-devel, linux-block, LKML

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH 3/3] xen/blkback: Use refcount_t for refcount
  2019-08-08 13:35 ` Roger Pau Monné
  2019-08-09  1:12   ` Chuhong Yuan
@ 2019-08-13  6:10   ` Chuhong Yuan
  1 sibling, 0 replies; 4+ messages in thread
From: Chuhong Yuan @ 2019-08-13  6:10 UTC (permalink / raw)
  To: Roger Pau Monné
  Cc: Konrad Rzeszutek Wilk, Jens Axboe, xen-devel, linux-block, LKML

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.

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, back to index

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-08 13:11 [PATCH 3/3] xen/blkback: Use refcount_t for refcount Chuhong Yuan
2019-08-08 13:35 ` Roger Pau Monné
2019-08-09  1:12   ` Chuhong Yuan
2019-08-13  6:10   ` Chuhong Yuan

Linux-Block Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-block/0 linux-block/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-block linux-block/ https://lore.kernel.org/linux-block \
		linux-block@vger.kernel.org linux-block@archiver.kernel.org
	public-inbox-index linux-block


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-block


AGPL code for this site: git clone https://public-inbox.org/ public-inbox