Date:   Tue, 31 Jan 2023 15:06:38 +0100
From:   Peter Zijlstra <peterz@infradead.org>
To:     Christophe Leroy <christophe.leroy@csgroup.eu>
Cc:     Song Liu <song@kernel.org>,
        "linux-modules@vger.kernel.org" <linux-modules@vger.kernel.org>,
        "linux-kernel@vger.kernel.org" <linux-kernel@vger.kernel.org>,
        "hch@lst.de" <hch@lst.de>,
        "kernel-team@meta.com" <kernel-team@meta.com>,
        Luis Chamberlain <mcgrof@kernel.org>,
        Thomas Gleixner <tglx@linutronix.de>,
        Guenter Roeck <linux@roeck-us.net>
Subject: Re: [PATCH v4] module: replace module_layout with module_memory
Message-ID: <Y9kgblwd/wUZ1KCJ@hirez.programming.kicks-ass.net>
References: <20230130182109.3571524-1-song@kernel.org>
 <Y9jau76YwdCElzZ0@hirez.programming.kicks-ass.net>
 <1302e2b6-8980-e6e5-7433-6754634c98f3@csgroup.eu>
 <Y9kDyUaOe57IL/yF@hirez.programming.kicks-ass.net>
 <6420c572-3d0c-eb64-208e-26b38d3da873@csgroup.eu>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <6420c572-3d0c-eb64-208e-26b38d3da873@csgroup.eu>
Precedence: bulk

On Tue, Jan 31, 2023 at 12:14:48PM +0000, Christophe Leroy wrote:

> >> Something like
> >>
> >> 	return within(addr, mod, MOD_TEXT) || within(addr, mod, MOD_DATA) ||
> >> 	       within(addr, mod, MOD_RODATA) || within(addr, mod,
> >> MOD_RO_AFTER_INIT);
> > 
> > Urgh, how about?
> > 
> > 	for_each_mod_mem_type(type) {
> > 		if (!mod_mem_type_is_init(type) && within(addr, mod, type))
> > 			return true;
> > 	}
> > 	return false;
> > 
> > Then you have have a bunch of mod_mem_type_id_foo() filter functions
> > that are non-contiguous without having to endlessly repeat stuff
> > manually.
> 
> But that's un-readable.

"For all except init."

> You have to have the list of possible types in front of you in order to 
> understand what the function does. Which means that one day or another 
> someone will change the order of types in the enum, and it will break.

I really don't agree, if you do explicit type lists everywhere you have
to update each and every sites when you modify the enum.

If you make category helpers, like: data, text, init, then you only need
to update the helpers without having to worry about each site. Only if
you add an enum that doesn't fit the existing categories do you need to
do something new.