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

Heiko Carstens
2025-03-31T09:35:53Z

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.