Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1614922 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to  refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-04-03 09:30 +0200
    Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t  to refcount_t David Miller <davem@davemloft.net> - 2017-04-03 15:20 +0200
      RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to  refcount_t "Reshetova, Elena" <elena.reshetova@intel.com> - 2017-04-03 18:10 +0200
        Re: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t  to refcount_t David Miller <davem@davemloft.net> - 2017-04-03 18:20 +0200

#1614922 — RE: [PATCH 3/4] sparc: convert mdesc_handle.refcnt from atomic_t to refcount_t

> From: Elena Reshetova <elena.reshetova@intel.com>
> 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@intel.com>
> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> > Signed-off-by: Kees Cook <keescook@chromium.org>
> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> 
> Acked-by: David S. Miller <davem@davemloft.net>

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?

Best Regards,
Elena

[toc] | [next] | [standalone]

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

From: "Reshetova, Elena" <elena.reshetova@intel.com>
Date: Mon, 3 Apr 2017 07:28:01 +0000

> 
>> From: Elena Reshetova <elena.reshetova@intel.com>
>> 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@intel.com>
>> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>> 
>> Acked-by: David S. Miller <davem@davemloft.net>
> 
> 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.

So I'm reluctant to ACK in any way these refcount_t conversions,
sorry.

[toc] | [prev] | [next] | [standalone]

#1615371

> From: "Reshetova, Elena" <elena.reshetova@intel.com>
> Date: Mon, 3 Apr 2017 07:28:01 +0000
> 
> >
> >> From: Elena Reshetova <elena.reshetova@intel.com>
> >> 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@intel.com>
> >> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
> >> > Signed-off-by: Kees Cook <keescook@chromium.org>
> >> > Signed-off-by: David Windsor <dwindsor@gmail.com>
> >>
> >> Acked-by: David S. Miller <davem@davemloft.net>
> >
> > 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*?

Best Regards,
Elena.
 
> So I'm reluctant to ACK in any way these refcount_t conversions,
> sorry.

[toc] | [prev] | [next] | [standalone]

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

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

>> From: "Reshetova, Elena" <elena.reshetova@intel.com>
>> Date: Mon, 3 Apr 2017 07:28:01 +0000
>> 
>> >
>> >> From: Elena Reshetova <elena.reshetova@intel.com>
>> >> 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@intel.com>
>> >> > Signed-off-by: Hans Liljestrand <ishkamiel@gmail.com>
>> >> > Signed-off-by: Kees Cook <keescook@chromium.org>
>> >> > Signed-off-by: David Windsor <dwindsor@gmail.com>
>> >>
>> >> Acked-by: David S. Miller <davem@davemloft.net>
>> >
>> > 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.

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web