Date:   Tue, 28 Feb 2023 12:44:38 -0800
From:   Luis Chamberlain <mcgrof@kernel.org>
To:     Jason Baron <jbaron@akamai.com>
Cc:     jim.cromie@gmail.com, gregkh@linuxfoundation.org,
        linux-kernel@vger.kernel.org, linux-modules@vger.kernel.org,
        Peter Zijlstra <peterz@infradead.org>
Subject: Re: [PATCH v2 2/2] dyndbg: use the module notifier callbacks
Message-ID: <Y/5ntkYoKHvfU9S8@bombadil.infradead.org>
References: <cover.1677612539.git.jbaron@akamai.com>
 <a775dcc14a10d0b3df34e087ee29ddb1d62fb517.1677612539.git.jbaron@akamai.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <a775dcc14a10d0b3df34e087ee29ddb1d62fb517.1677612539.git.jbaron@akamai.com>
Sender: Luis Chamberlain <mcgrof@infradead.org>
Precedence: bulk

On Tue, Feb 28, 2023 at 02:35:02PM -0500, Jason Baron wrote:
> As part of Jim Cromie's new dynamic debug classmap feature, the new code
> tries to toggle a jump label from dynamic_debug_setup(). However,
> dynamic_debug_setup() is called before the 'module_notify_list' notifier
> chain is invoked. And jump labels are initialized via the module notifier
> chain. Note this is an issue for a new feature not yet merged and doesn't
> affect any existing codepaths.

I think we can summarize this to "in preperation for some future work where
ordering matters with respect to jump labels" or something like that.

Because that is then making it specific to the future use case and
creates the current justification.

> We could just move dynamic_debug_setup() earlier in load_module(). But
> let's instead ensure the ordering via the 'priority' in the module list
> notifier.

"becuase the notifier for jump labels jump_label_module_nb uses a
priority of 1" or something like that would be nice to get added.

> This brings dynamic debug more in line with other subsystems and
> pulls code out of the core module code.

This should be the main reason for this change, as explained in the
commit log. A secondary benefit would be it fixes the first future bug
mentioned.

With those changes I can take this into modules-next to start getting
this tested sooner rather than later.

  Luis