MIME-Version: 1.0
References: <20211213123737.9147-1-wander@redhat.com> <20211213123737.9147-2-wander@redhat.com>
 <YboeqlSsoFRbjEv2@linutronix.de>
In-Reply-To: <YboeqlSsoFRbjEv2@linutronix.de>
From:   Wander Costa <wcosta@redhat.com>
Date:   Thu, 16 Dec 2021 17:21:07 -0300
Message-ID: <CAAq0SUkamkGvcBWpSwuZ6brFw=7qnmiE8huX6xEE643ohBVuZQ@mail.gmail.com>
Subject: Re: [PATCH v2 1/2] block: Avoid sleeping function called from invalid
 context bug
To:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     Wander Lairson Costa <wander@redhat.com>,
        open list <linux-kernel@vger.kernel.org>,
        Steven Rostedt <rostedt@goodmis.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        linux-rt-users@vger.kernel.org
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

On Wed, Dec 15, 2021 at 1:58 PM Sebastian Andrzej Siewior
<bigeasy@linutronix.de> wrote:
>
> On 2021-12-13 09:37:36 [-0300], Wander Lairson Costa wrote:
> > ---
> >  block/blk-cgroup.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/block/blk-cgroup.c b/block/blk-cgroup.c
> > index 663aabfeba18..0a532bb3003c 100644
> > --- a/block/blk-cgroup.c
> > +++ b/block/blk-cgroup.c
> > @@ -1911,7 +1911,7 @@ void blk_cgroup_bio_start(struct bio *bio)
> >       struct blkg_iostat_set *bis;
> >       unsigned long flags;
> >
> > -     cpu = get_cpu();
> > +     cpu = get_cpu_light();
> >       bis = per_cpu_ptr(bio->bi_blkg->iostat_cpu, cpu);
> >       flags = u64_stats_update_begin_irqsave(&bis->sync);
> >
> > @@ -1928,7 +1928,7 @@ void blk_cgroup_bio_start(struct bio *bio)
> >       u64_stats_update_end_irqrestore(&bis->sync, flags);
> >       if (cgroup_subsys_on_dfl(io_cgrp_subsys))
> >               cgroup_rstat_updated(bio->bi_blkg->blkcg->css.cgroup, cpu);
> > -     put_cpu();
> > +     put_cpu_light();
> >  }
>
> Are you sure patch and backtrace match? There is also
> u64_stats_update_begin_irqsave() which disables preemption on RT. So by
> doing what you are suggesting, you only avoid disabling preemption in
> cgroup_rstat_updated() which acquires a raw_spinlock_t.
>

You're right. I double-checked and noticed my tree didn't have the
call to u64_stats_update_begin_irqsave. Only patch 2 is necessary
indeed.

> Sebastian
>