Mike,

On 2018/10/17 3:34, Mike Snitzer wrote:
> On Thu, Aug 23 2018 at  6:12pm -0400,
> Damien Le Moal <damien.lem...@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 <jpitt...@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.

OK. Will send that shortly (and thanks for reminding me, I completely forgot
about this !).

Best regards.

-- 
Damien Le Moal
Western Digital Research

--
dm-devel mailing list
dm-devel@redhat.com
https://www.redhat.com/mailman/listinfo/dm-devel

Reply via email to