Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20;
From:   Thomas Gleixner <tglx@linutronix.de>
To:     Steven Rostedt <rostedt@goodmis.org>
Cc:     linux-kernel@vger.kernel.org,
        Linus Torvalds <torvalds@linux-foundation.org>,
        Stephen Boyd <sboyd@kernel.org>,
        Guenter Roeck <linux@roeck-us.net>,
        Anna-Maria Gleixner <anna-maria@linutronix.de>,
        Andrew Morton <akpm@linux-foundation.org>,
        Julia Lawall <Julia.Lawall@inria.fr>
Subject: Re: [PATCH v6 4/6] timers: Add timer_shutdown_sync() to be called
 before freeing timers
In-Reply-To: <20221114083635.60e23559@gandalf.local.home>
References: <20221110064101.429013735@goodmis.org>
 <20221110064147.343514404@goodmis.org> <875yfitpdu.ffs@tglx>
 <20221113191518.0a3b29b1@rorschach.local.home> <87sfims7ca.ffs@tglx>
 <20221114083635.60e23559@gandalf.local.home>
Date:   Mon, 14 Nov 2022 20:13:28 +0100
Message-ID: <87bkp9s61z.ffs@tglx>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

On Mon, Nov 14 2022 at 08:36, Steven Rostedt wrote:
> On Mon, 14 Nov 2022 01:33:25 +0100
> Thomas Gleixner <tglx@linutronix.de> wrote:
>>    https://lore.kernel.org/all/87v8vjiaih.ffs@tglx/
>> 
> I'm not sure what you mean by that. The idea is that once timer_shutdown()
> is called, we still warn on re-arming the timer.

That's the whole point. As Linus and I discussed in that thread:

   "That would mean, that we still check the function pointer for NULL
    without warning and just return. That would indeed be a good argument
    for not having the warning at all."

and as I demonstrated you on the example of the BT driver which you
"fixed" this is the only sensible way to handle this.

The warning does not buy us anything, unless you want to go and amend
all the usage sites which trigger it with 'if (mystruct->shutdown)'
conditionals.

It's very similar to the work->canceling logic for kthreads that Linus
mentioned in this thread which prevents that the work timer is rearmed
concurrently. The difference is that timer_shutdown() is a final
decision which renders the timer unusable unless it is explicitely
reinitialized.

But that's mostly a matter of documentation and it has to be made clear
that nothing in a shutdown path which has the BT pattern:

     timer_shutdown();
     destroy_workqueue();

relies on the timer being functional after the shutdown point. I'm
pretty sure that the vast majority of such use cases do not care, but
given the size of the driver zoo I'm also sure that you'll find at least
one which depends on the timer working accross teardown.

Thanks,

        tglx