Re: [PATCH v1 1/1] s390/uv: Prealloc and use one work page

[Heiko Carstens <hca@linux.ibm.com>]

On Fri, Mar 28, 2025 at 01:51:06PM +0100, Harald Freudenberger wrote:
> > > +	work_page = (u8 *)__get_free_page(GFP_KERNEL);
> > > +	if (!work_page) {
> > > +		pr_warn("Failed to alloc a working memory page\n");
> > > +		return;
> > > +	}
> > > +
> > >  	uv_stor_base = memblock_alloc_try_nid(
> > 
> > Did you test this? I think this cannot work. When setup_uv() is called
> > the buddy allocator is not yet initialized.
> > Please use memblock_alloc_or_panic() instead.
> > 
> 
> I only compiled this and I wanted to test this today in my
> SEL environment. The patch is a suggestion and should trigger
> maybe some feedback.

Well, you got some feedback :)

> > > -	buf = kzalloc(sizeof(*buf), GFP_KERNEL);
> > > -	if (!buf)
> > > -		return -ENOMEM;
> > > +	mutex_lock(&work_page_lock);
> > > +	buf = (struct uv_secret_list *)work_page;
> > >  	rc = find_secret(secret_id, buf, secret);
> > > -	kfree(buf);
> > > +	mutex_unlock(&work_page_lock);
> > 
> > The commit message does not explain why memory allocation is not
> > acceptable. Usually this translates to non-sleepable context. If that
> > is the case, then using a mutex would be wrong. This needs to be
> > clarified.

But I'm really wondering about this part. Please clarify.