Re: [PATCH 2/3] dm zoned: metadata: use refcount_t for dm zoned reference counters

[Mike Snitzer <snitzer@redhat.com>]

On Thu, Aug 23 2018 at  6:12pm -0400,
Damien Le Moal <Damien.LeMoal@wdc.com> wrote:

> John,
> 
> On 2018/08/23 10:37, John Pittman wrote:
> > The API surrounding refcount_t should be used in place of atomic_t
> > when variables are being used as reference counters.  This API can
> > prevent issues such as counter overflows and use-after-free
> > conditions.  Within the dm zoned metadata stack, the atomic_t API
> > is used for mblk->ref and zone->refcount.  Change these to use
> > refcount_t, avoiding the issues mentioned.
> > 
> > Signed-off-by: John Pittman <jpittman@redhat.com>
> > ---
> >  drivers/md/dm-zoned-metadata.c | 25 +++++++++++++------------
> >  drivers/md/dm-zoned.h          |  2 +-
> >  2 files changed, 14 insertions(+), 13 deletions(-)
> > 
> > diff --git a/drivers/md/dm-zoned-metadata.c b/drivers/md/dm-zoned-metadata.c
> > index 969954915566..92e635749414 100644
> > --- a/drivers/md/dm-zoned-metadata.c
> > +++ b/drivers/md/dm-zoned-metadata.c
> > @@ -99,7 +99,7 @@ struct dmz_mblock {
> >  	struct rb_node		node;
> >  	struct list_head	link;
> >  	sector_t		no;
> > -	atomic_t		ref;
> > +	refcount_t		ref;
> 
> While reviewing your patch, I realized that this ref is always manipulated under
> the zmd->mblk_lock spinlock. So there is no need for it to be an atomic or a
> refcount. An unsigned int would do as well and be faster. My fault.
> 
> I will send a patch to go on top of yours to fix that.

Hi Damien,

Given what you've said I'm not seeing the point in the intermediate
refcount_t conversion. 

I'd rather you just send a patch that switches atomic_t to int.

Thanks,
Mike