Received-SPF: pass (google.com: domain of linux-kernel+bounces-116925-linux.lists.archive=gmail.com@vger.kernel.org designates 147.75.80.249 as permitted sender) client-ip=147.75.80.249;
Date: Mon, 25 Mar 2024 12:16:50 +0100
From: Heiko Carstens <hca@linux.ibm.com>
To: Baoquan He <bhe@redhat.com>
Cc: Christoph Hellwig <hch@infradead.org>,
        "Uladzislau Rezki (Sony)" <urezki@gmail.com>, linux-mm@kvack.org,
        Andrew Morton <akpm@linux-foundation.org>,
        LKML <linux-kernel@vger.kernel.org>,
        Lorenzo Stoakes <lstoakes@gmail.com>,
        Matthew Wilcox <willy@infradead.org>,
        Dave Chinner <david@fromorbit.com>, Guenter Roeck <linux@roeck-us.net>,
        Oleksiy Avramchenko <oleksiy.avramchenko@sony.com>,
        Vasily Gorbik <gor@linux.ibm.com>,
        Alexander Gordeev <agordeev@linux.ibm.com>,
        Christian Borntraeger <borntraeger@linux.ibm.com>,
        Sven Schnelle <svens@linux.ibm.com>, linux-s390@vger.kernel.org
Subject: Re: [PATCH 1/1] mm: vmalloc: Bail out early in find_vmap_area() if
 vmap is not init
Message-ID: <20240325111650.16056-A-hca@linux.ibm.com>
References: <20240323141544.4150-1-urezki@gmail.com>
 <ZgC38GfEZYpYGUU9@infradead.org>
 <20240325093959.9453-B-hca@linux.ibm.com>
 <ZgFNVtp3EsJRaSN0@MiWiFi-R3L-srv>
Precedence: bulk
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <ZgFNVtp3EsJRaSN0@MiWiFi-R3L-srv>

On Mon, Mar 25, 2024 at 06:09:26PM +0800, Baoquan He wrote:
> On 03/25/24 at 10:39am, Heiko Carstens wrote:
> > On Sun, Mar 24, 2024 at 04:32:00PM -0700, Christoph Hellwig wrote:
> > > On Sat, Mar 23, 2024 at 03:15:44PM +0100, Uladzislau Rezki (Sony) wrote:
> ......snip
> > > I guess this is ok as an urgend bandaid to get s390 booting again,
> > > but calling find_vmap_area before the vmap area is initialized
> > > seems an actual issue in the s390 mm init code.
> > > 
> > > Adding the s390 maintainers to see if they have and idea how this could
> > > get fixed in a better way.
> > 
> > I'm going to push the patch below to the s390 git tree later. This is not a
> > piece of art, but I wanted to avoid to externalize vmalloc's vmap_initialized,
> > or come up with some s390 specific change_page_attr_alias_early() variant where
> > sooner or later nobody remembers what "early" means.
> > 
> > So this seems to be "good enough".
..
> > Add a slab_is_available() check to change_page_attr_alias() in order to
> > avoid early calls into vmalloc code. slab_is_available() is not exactly
> > what is needed, but there is currently no other way to tell if the vmalloc
> > code is initialized or not, and there is no reason to expose
> > e.g. vmap_initialized from vmalloc to achieve the same.
> 
> If so, I would rather add a vmalloc_is_available() to achieve the same.
> The added code and the code comment definitely will confuse people and
> make people to dig why.

So after having given this a bit more thought I think Uladzislau's patch is
probably the best way to address this.

It seems to be better that the vmalloc code would just do the right thing,
regardless how early it is called, instead of adding yet another
subsystem_xyz_is_available() call.

Alternatively this could be addressed in s390 code with some sort of
"early" calls, but as already stated, sooner or later nobody would remember
what "early" means, and even if that would be remembered: would that
restriction still be valid?