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 10:12:54 +0200
From:   Sebastian Andrzej Siewior <bigeasy@linutronix.de>
To:     Tetsuo Handa <penguin-kernel@i-love.sakura.ne.jp>
Cc:     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>,
        Petr Mladek <pmladek@suse.com>,
        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: <20230626081254.XmorFrhs@linutronix.de>
References: <20230623171232.892937-1-bigeasy@linutronix.de>
 <20230623171232.892937-2-bigeasy@linutronix.de>
 <d9b7c170-ed0d-5d37-e099-20d233115943@I-love.SAKURA.ne.jp>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
In-Reply-To: <d9b7c170-ed0d-5d37-e099-20d233115943@I-love.SAKURA.ne.jp>
Precedence: bulk

On 2023-06-24 15:54:12 [+0900], Tetsuo Handa wrote:
> Why not to do the same on the end side?
>=20
>  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).

Looking at other locking primitives (spin_lock_unlock(),
mutex_unlock(),=E2=80=A6) that is what they do in the unlock path: lockdep
annotation followed by the actual operation. Therefore I would keep the
current ordering to remain in-sync with the other primitives.

Sebastian