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;
Date:   Mon, 26 Jun 2023 16:44:14 +0200
From:   Petr Mladek <pmladek@suse.com>
To:     Sebastian Andrzej Siewior <bigeasy@linutronix.de>
Cc:     Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>,
        linux-mm@kvack.org, linux-kernel@vger.kernel.org,
        "Luis Claudio R. Goncalves" <lgoncalv@redhat.com>,
        Andrew Morton <akpm@linux-foundation.org>,
        Boqun Feng <boqun.feng@gmail.com>,
        Ingo Molnar <mingo@redhat.com>,
        John Ogness <john.ogness@linutronix.de>,
        Mel Gorman <mgorman@techsingularity.net>,
        Michal Hocko <mhocko@suse.com>,
        Peter Zijlstra <peterz@infradead.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Waiman Long <longman@redhat.com>, Will Deacon <will@kernel.org>
Subject: Re: [PATCH v2 1/2] seqlock: Do the lockdep annotation before locking
 in do_write_seqcount_begin_nested()
Message-ID: <ZJmkPuqpW-wQAyNz@alley>
References: <20230623171232.892937-1-bigeasy@linutronix.de>
 <20230623171232.892937-2-bigeasy@linutronix.de>
 <d9b7c170-ed0d-5d37-e099-20d233115943@I-love.SAKURA.ne.jp>
 <20230626081254.XmorFrhs@linutronix.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20230626081254.XmorFrhs@linutronix.de>
Precedence: bulk

On Mon 2023-06-26 10:12:54, Sebastian Andrzej Siewior wrote:
> On 2023-06-24 15:54:12 [+0900], Tetsuo Handa wrote:
> > Why not to do the same on the end side?
> > 
> >  static inline void do_write_seqcount_end(seqcount_t *s)
> >  {
> > - 	seqcount_release(&s->dep_map, _RET_IP_);
> >  	do_raw_write_seqcount_end(s);
> > +	seqcount_release(&s->dep_map, _RET_IP_);
> >  }
> 
> I don't have a compelling argument for doing it. It is probably better
> to release the lock from lockdep's point of view and then really release
> it (so it can't be acquired before it is released).

If this is true then we should not change the ordering on the _begin
side either. I mean that we should call the lockdep code only
after the lock is taken. Anyway, both sides should be symmetric.

That said, lockdep is about chains of locks and not about timing.
We must not call lockdep annotation when the lock is still available
for a nested context. So the ordering is probably important only when
the lock might be taken from both normal and interrupt context.

Anyway, please do not do this change only because of printk().
IMHO, the current ordering is more logical and the printk() problem
should be solved another way.

Best Regards,
Petr