Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

From: David Miller
Date: Mon Apr 03 2017 - 12:16:22 EST


From: "Reshetova, Elena" <elena.reshetova@xxxxxxxxx>
Date: Mon, 3 Apr 2017 16:06:44 +0000

>> From: "Reshetova, Elena" <elena.reshetova@xxxxxxxxx>
>> Date: Mon, 3 Apr 2017 07:28:01 +0000
>>
>> >
>> >> From: Elena Reshetova <elena.reshetova@xxxxxxxxx>
>> >> Date: Mon, 20 Feb 2017 13:06:20 +0200
>> >>
>> >> > refcount_t type and corresponding API should be
>> >> > used instead of atomic_t when the variable is used as
>> >> > a reference counter. This allows to avoid accidental
>> >> > refcounter overflows that might lead to use-after-free
>> >> > situations.
>> >> >
>> >> > Signed-off-by: Elena Reshetova <elena.reshetova@xxxxxxxxx>
>> >> > Signed-off-by: Hans Liljestrand <ishkamiel@xxxxxxxxx>
>> >> > Signed-off-by: Kees Cook <keescook@xxxxxxxxxxxx>
>> >> > Signed-off-by: David Windsor <dwindsor@xxxxxxxxx>
>> >>
>> >> Acked-by: David S. Miller <davem@xxxxxxxxxxxxx>
>> >
>> > Hi David,
>> >
>> > Would you be able to propagate this patch further or should I send
>> > it (with your acked-by) once more to specific list/maintainer for
>> > the inclusion?
>>
>> I'm generally not happy with the refcount_t and the added overhead it
>> has compared to the existing atomic_t operations.
>>
>> I know it is going to make a difference for networking.
>>
>> I understand that this sparc case is a slow path, but I know that if
>> we just apply all of these refcount_t conversions, there will be no
>> work done to address the performance issues.
>
> I think we will have to address the performance problems in places where we can see it matters.
> The problem is that so far noone told us how to measure in any reasonable way the overhead neither in networking, not in mm changes.
> If this change is a slow path, why would it matter for *this particular patch*?

I think not having a way to avoid the functional call makes the facility
unusable as a core kernel facility.

You can't just say "oh well, just convert the slow paths, we'll solve the
fundamental performance issue later". Sorry, that is not how we do things.