On Mon, Dec 17, 2018 at 2:30 AM Jiri Pirko <j...@resnulli.us> wrote:
>
> Sun, Dec 16, 2018 at 07:52:18PM CET, xiyou.wangc...@gmail.com wrote:
> >On Sun, Dec 16, 2018 at 8:32 AM Vlad Buslov <vla...@mellanox.com> wrote:
> >>
> >> On Thu 13 Dec 2018 at 23:32, Cong Wang <xiyou.wangc...@gmail.com> wrote:
> >> > On Tue, Dec 11, 2018 at 2:19 AM Vlad Buslov <vla...@mellanox.com> wrote:
> >> >>
> >> >> As a part of the effort to remove dependency on rtnl lock, cls API is
> >> >> being
> >> >> converted to use fine-grained locking mechanisms instead of global rtnl
> >> >> lock. However, chain_head_change callback for ingress Qdisc is a
> >> >> sleeping
> >> >> function and cannot be executed while holding a spinlock.
> >> >
> >> >
> >> > Why does it have to be a spinlock not a mutex?
> >> >
> >> > I've read your cover letter and this changelog, I don't find any
> >> > answer.
> >>
> >> My initial implementation used mutex. However, it was changed to
> >> spinlock by Jiri's request during internal review.
> >>
> >
> >So what's the answer to my question? :)
>
> Yeah, my concern agains mutexes was that it would be needed to have one
> per every block and per every chain. I find it quite heavy and I believe
> it is better to use spinlock in those cases. This patch is a side effect
> of that. Do you think it would be better to have mutexes instead of
> spinlocks?
My only concern with spinlock is we have to go async as we
can't block. This is almost always error-prone especially
when dealing with tcf block. I had to give up with spinlock
for idrinfo->lock, please take a look at:
commit 95278ddaa15cfa23e4a06ee9ed7b6ee0197c500b
Author: Cong Wang <xiyou.wangc...@gmail.com>
Date: Tue Oct 2 12:50:19 2018 -0700
net_sched: convert idrinfo->lock from spinlock to a mutex
There are indeed some cases in kernel we do take multiple
mutex'es, for example,
/*
* bd_mutex locking:
*
* mutex_lock(part->bd_mutex)
* mutex_lock_nested(whole->bd_mutex, 1)
*/
So, how heavy are they comparing with spinlocks?
Thanks.
