[v1] mei: use kvmalloc for read buffer

[char-misc-next] mei: use kvmalloc for read buffer

Posted by Alexander Usyskin 1 month, 2 weeks ago

Read buffer is allocated according to max message size,
reported by the firmware and may reach 64K in systems
with pxp client.
Contiguous 64k allocation may fail under memory pressure.
Read buffer is used as in-driver message storage and
not required to be contiguous.
Use kvmalloc to allow kernel to allocate non-contiguous
memory in this case.

Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
---
 drivers/misc/mei/client.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/drivers/misc/mei/client.c b/drivers/misc/mei/client.c
index 9d090fa07516..be011cef12e5 100644
--- a/drivers/misc/mei/client.c
+++ b/drivers/misc/mei/client.c
@@ -321,7 +321,7 @@ void mei_io_cb_free(struct mei_cl_cb *cb)
 		return;
 
 	list_del(&cb->list);
-	kfree(cb->buf.data);
+	kvfree(cb->buf.data);
 	kfree(cb->ext_hdr);
 	kfree(cb);
 }
@@ -497,7 +497,7 @@ struct mei_cl_cb *mei_cl_alloc_cb(struct mei_cl *cl, size_t length,
 	if (length == 0)
 		return cb;
 
-	cb->buf.data = kmalloc(roundup(length, MEI_SLOT_SIZE), GFP_KERNEL);
+	cb->buf.data = kvmalloc(roundup(length, MEI_SLOT_SIZE), GFP_KERNEL);
 	if (!cb->buf.data) {
 		mei_io_cb_free(cb);
 		return NULL;
-- 
2.43.0

Re: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Greg Kroah-Hartman 1 month, 2 weeks ago

On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> Read buffer is allocated according to max message size,
> reported by the firmware and may reach 64K in systems
> with pxp client.
> Contiguous 64k allocation may fail under memory pressure.
> Read buffer is used as in-driver message storage and
> not required to be contiguous.
> Use kvmalloc to allow kernel to allocate non-contiguous
> memory in this case.
> 
> Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> ---
>  drivers/misc/mei/client.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)

What about this thread:
	https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/

No attribution for the reporter?  Does it solve their problem?

And why such a short line-length in the changelog?

Also, where is this memory pressure coming from, what is the root cause
and what commit does this fix?  Stable backports needed?  Anything else?

And who inside Intel reviewed this before sending this out?  Why are
they not credited here properly?

thanks,

greg k-h

RE: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Usyskin, Alexander 1 month, 2 weeks ago

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Sunday, October 13, 2024 3:14 PM
> To: Usyskin, Alexander <alexander.usyskin@intel.com>
> Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> 
> On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > Read buffer is allocated according to max message size,
> > reported by the firmware and may reach 64K in systems
> > with pxp client.
> > Contiguous 64k allocation may fail under memory pressure.
> > Read buffer is used as in-driver message storage and
> > not required to be contiguous.
> > Use kvmalloc to allow kernel to allocate non-contiguous
> > memory in this case.
> >
> > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > ---
> >  drivers/misc/mei/client.c | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> What about this thread:
> 	https://lore.kernel.org/all/20240813084542.2921300-1-
> rohiagar@chromium.org/
> 
> No attribution for the reporter?  Does it solve their problem?
> 
This patch is a result from non-public bug report on ChromeOS.
Their symptoms are not the same as in the thread above.
But, as you rightly point out, this patch should also fix above problem.
I'll add attribution in v2.

> And why such a short line-length in the changelog?
I've tried to split logically with not passing 75 characters.
Will try to re-shuffle in v2.

> 
> Also, where is this memory pressure coming from, what is the root cause
> and what commit does this fix?  Stable backports needed?  Anything else?
> 
The ChromeOS is extremely short on memory by design and can trigger
this situation very easily.
I do not think that this patch fixes any commit - the problematic code exists
from the earliest versions of this driver.
As this problem reproduced only on ChromeOS I believe that no need
in wide backport, the ChromeOS can cherry-pick the patch.
From your experience, is this the right strategy? 

> And who inside Intel reviewed this before sending this out?  Why are
> they not credited here properly?
> 
My bad, lost the tested-by tag in patch preparations.
Will add in v2.

> thanks,
> 
> greg k-h

- - 
Thanks,
Sasha

Re: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Greg Kroah-Hartman 1 month, 2 weeks ago

On Sun, Oct 13, 2024 at 02:22:27PM +0000, Usyskin, Alexander wrote:
> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Sent: Sunday, October 13, 2024 3:14 PM
> > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > 
> > On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > > Read buffer is allocated according to max message size,
> > > reported by the firmware and may reach 64K in systems
> > > with pxp client.
> > > Contiguous 64k allocation may fail under memory pressure.
> > > Read buffer is used as in-driver message storage and
> > > not required to be contiguous.
> > > Use kvmalloc to allow kernel to allocate non-contiguous
> > > memory in this case.
> > >
> > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > ---
> > >  drivers/misc/mei/client.c | 4 ++--
> > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > 
> > What about this thread:
> > 	https://lore.kernel.org/all/20240813084542.2921300-1-
> > rohiagar@chromium.org/
> > 
> > No attribution for the reporter?  Does it solve their problem?
> > 
> This patch is a result from non-public bug report on ChromeOS.

Then make that bug report public as it was discussed in public already :)

> > Also, where is this memory pressure coming from, what is the root cause
> > and what commit does this fix?  Stable backports needed?  Anything else?
> > 
> The ChromeOS is extremely short on memory by design and can trigger
> this situation very easily.

So normal allocations are failing?  That feels wrong, what caused this?

> I do not think that this patch fixes any commit - the problematic code exists
> from the earliest versions of this driver.
> As this problem reproduced only on ChromeOS I believe that no need
> in wide backport, the ChromeOS can cherry-pick the patch.
> From your experience, is this the right strategy? 

No.

RE: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Usyskin, Alexander 1 month, 1 week ago

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Sunday, October 13, 2024 6:08 PM
> To: Usyskin, Alexander <alexander.usyskin@intel.com>
> Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> 
> On Sun, Oct 13, 2024 at 02:22:27PM +0000, Usyskin, Alexander wrote:
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Sent: Sunday, October 13, 2024 3:14 PM
> > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > >
> > > On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > > > Read buffer is allocated according to max message size,
> > > > reported by the firmware and may reach 64K in systems
> > > > with pxp client.
> > > > Contiguous 64k allocation may fail under memory pressure.
> > > > Read buffer is used as in-driver message storage and
> > > > not required to be contiguous.
> > > > Use kvmalloc to allow kernel to allocate non-contiguous
> > > > memory in this case.
> > > >
> > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > ---
> > > >  drivers/misc/mei/client.c | 4 ++--
> > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > >
> > > What about this thread:
> > > 	https://lore.kernel.org/all/20240813084542.2921300-1-
> > > rohiagar@chromium.org/

[1] https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/
> > >
> > > No attribution for the reporter?  Does it solve their problem?
> > >
> > This patch is a result from non-public bug report on ChromeOS.
> 
> Then make that bug report public as it was discussed in public already :)
> 
Unfortunately, it is not my call.
For now, I'll anchor this on [1]

> > > Also, where is this memory pressure coming from, what is the root cause
> > > and what commit does this fix?  Stable backports needed?  Anything else?
> > >
> > The ChromeOS is extremely short on memory by design and can trigger
> > this situation very easily.
> 
> So normal allocations are failing?  That feels wrong, what caused this?

64K is order 4 allocation and may fail according to [1].

> > I do not think that this patch fixes any commit - the problematic code exists
> > from the earliest versions of this driver.
> > As this problem reproduced only on ChromeOS I believe that no need
> > in wide backport, the ChromeOS can cherry-pick the patch.
> > From your experience, is this the right strategy?
> 
> No.

Sure, I'll use
Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
where the first time such buffer allocated and add stable here in v2.

- - 
Thanks,
Sasha

Re: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Greg Kroah-Hartman 1 month, 1 week ago

On Mon, Oct 14, 2024 at 01:15:49PM +0000, Usyskin, Alexander wrote:
> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Sent: Sunday, October 13, 2024 6:08 PM
> > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > 
> > On Sun, Oct 13, 2024 at 02:22:27PM +0000, Usyskin, Alexander wrote:
> > > > -----Original Message-----
> > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Sent: Sunday, October 13, 2024 3:14 PM
> > > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > > >
> > > > On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > > > > Read buffer is allocated according to max message size,
> > > > > reported by the firmware and may reach 64K in systems
> > > > > with pxp client.
> > > > > Contiguous 64k allocation may fail under memory pressure.
> > > > > Read buffer is used as in-driver message storage and
> > > > > not required to be contiguous.
> > > > > Use kvmalloc to allow kernel to allocate non-contiguous
> > > > > memory in this case.
> > > > >
> > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > ---
> > > > >  drivers/misc/mei/client.c | 4 ++--
> > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > >
> > > > What about this thread:
> > > > 	https://lore.kernel.org/all/20240813084542.2921300-1-
> > > > rohiagar@chromium.org/
> 
> [1] https://lore.kernel.org/all/20240813084542.2921300-1-rohiagar@chromium.org/

Yes, it's a problem, I don't understand.

> > > >
> > > > No attribution for the reporter?  Does it solve their problem?
> > > >
> > > This patch is a result from non-public bug report on ChromeOS.
> > 
> > Then make that bug report public as it was discussed in public already :)
> > 
> Unfortunately, it is not my call.
> For now, I'll anchor this on [1]
> 
> > > > Also, where is this memory pressure coming from, what is the root cause
> > > > and what commit does this fix?  Stable backports needed?  Anything else?
> > > >
> > > The ChromeOS is extremely short on memory by design and can trigger
> > > this situation very easily.
> > 
> > So normal allocations are failing?  That feels wrong, what caused this?
> 
> 64K is order 4 allocation and may fail according to [1].

And what changed to cause this to suddenly be 64k?  And why can't we
allocate 64k at this point in time now?

> > > I do not think that this patch fixes any commit - the problematic code exists
> > > from the earliest versions of this driver.
> > > As this problem reproduced only on ChromeOS I believe that no need
> > > in wide backport, the ChromeOS can cherry-pick the patch.
> > > From your experience, is this the right strategy?
> > 
> > No.
> 
> Sure, I'll use
> Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> where the first time such buffer allocated and add stable here in v2.

So the problem has been there for years?  Why is it just now showing up?

thanks,

greg k-h

RE: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Usyskin, Alexander 1 month, 1 week ago

> -----Original Message-----
> From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> Sent: Monday, October 14, 2024 4:25 PM
> To: Usyskin, Alexander <alexander.usyskin@intel.com>
> Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> 
> On Mon, Oct 14, 2024 at 01:15:49PM +0000, Usyskin, Alexander wrote:
> > > -----Original Message-----
> > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > Sent: Sunday, October 13, 2024 6:08 PM
> > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > >
> > > On Sun, Oct 13, 2024 at 02:22:27PM +0000, Usyskin, Alexander wrote:
> > > > > -----Original Message-----
> > > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > Sent: Sunday, October 13, 2024 3:14 PM
> > > > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > > > >
> > > > > On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > > > > > Read buffer is allocated according to max message size,
> > > > > > reported by the firmware and may reach 64K in systems
> > > > > > with pxp client.
> > > > > > Contiguous 64k allocation may fail under memory pressure.
> > > > > > Read buffer is used as in-driver message storage and
> > > > > > not required to be contiguous.
> > > > > > Use kvmalloc to allow kernel to allocate non-contiguous
> > > > > > memory in this case.
> > > > > >
> > > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > > ---
> > > > > >  drivers/misc/mei/client.c | 4 ++--
> > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > >
> > > > > What about this thread:
> > > > > 	https://lore.kernel.org/all/20240813084542.2921300-1-
> > > > > rohiagar@chromium.org/
> >
> > [1] https://lore.kernel.org/all/20240813084542.2921300-1-
> rohiagar@chromium.org/
> 
> Yes, it's a problem, I don't understand.
> 
> > > > >
> > > > > No attribution for the reporter?  Does it solve their problem?
> > > > >
> > > > This patch is a result from non-public bug report on ChromeOS.
> > >
> > > Then make that bug report public as it was discussed in public already :)
> > >
> > Unfortunately, it is not my call.
> > For now, I'll anchor this on [1]
> >
> > > > > Also, where is this memory pressure coming from, what is the root
> cause
> > > > > and what commit does this fix?  Stable backports needed?  Anything
> else?
> > > > >
> > > > The ChromeOS is extremely short on memory by design and can trigger
> > > > this situation very easily.
> > >
> > > So normal allocations are failing?  That feels wrong, what caused this?
> >
> > 64K is order 4 allocation and may fail according to [1].
> 
> And what changed to cause this to suddenly be 64k?  And why can't we
> allocate 64k at this point in time now?
> 
> > > > I do not think that this patch fixes any commit - the problematic code
> exists
> > > > from the earliest versions of this driver.
> > > > As this problem reproduced only on ChromeOS I believe that no need
> > > > in wide backport, the ChromeOS can cherry-pick the patch.
> > > > From your experience, is this the right strategy?
> > >
> > > No.
> >
> > Sure, I'll use
> > Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> > where the first time such buffer allocated and add stable here in v2.
> 
> So the problem has been there for years?  Why is it just now showing up?
> 

I suppose it is the combination of some fairly new FW that requests 64K buffer
for content-protection case, underpowered ChromeBook and ChromeOS running
content-protection flow.
All three conditions should be met to trigger this failure.

> thanks,
> 
> greg k-h

- - 
Thanks,
Sasha

Re: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Brian Geffon 1 month, 1 week ago

On Mon, Oct 14, 2024 at 02:43:31PM +0000, Usyskin, Alexander wrote:
> 
> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Sent: Monday, October 14, 2024 4:25 PM
> > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > 
> > On Mon, Oct 14, 2024 at 01:15:49PM +0000, Usyskin, Alexander wrote:
> > > > -----Original Message-----
> > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Sent: Sunday, October 13, 2024 6:08 PM
> > > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > > >
> > > > On Sun, Oct 13, 2024 at 02:22:27PM +0000, Usyskin, Alexander wrote:
> > > > > > -----Original Message-----
> > > > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > Sent: Sunday, October 13, 2024 3:14 PM
> > > > > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > > > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > > > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > > > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > > > > >
> > > > > > On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > > > > > > Read buffer is allocated according to max message size,
> > > > > > > reported by the firmware and may reach 64K in systems
> > > > > > > with pxp client.
> > > > > > > Contiguous 64k allocation may fail under memory pressure.
> > > > > > > Read buffer is used as in-driver message storage and
> > > > > > > not required to be contiguous.
> > > > > > > Use kvmalloc to allow kernel to allocate non-contiguous
> > > > > > > memory in this case.
> > > > > > >
> > > > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>

Tested-by: Brian Geffon <bgeffon@google.com>

> > > > > > > ---
> > > > > > >  drivers/misc/mei/client.c | 4 ++--
> > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > What about this thread:
> > > > > > 	https://lore.kernel.org/all/20240813084542.2921300-1-
> > > > > > rohiagar@chromium.org/
> > >
> > > [1] https://lore.kernel.org/all/20240813084542.2921300-1-
> > rohiagar@chromium.org/
> > 
> > Yes, it's a problem, I don't understand.
> > 
> > > > > >
> > > > > > No attribution for the reporter?  Does it solve their problem?
> > > > > >
> > > > > This patch is a result from non-public bug report on ChromeOS.
> > > >
> > > > Then make that bug report public as it was discussed in public already :)
> > > >
> > > Unfortunately, it is not my call.
> > > For now, I'll anchor this on [1]
> > >
> > > > > > Also, where is this memory pressure coming from, what is the root
> > cause
> > > > > > and what commit does this fix?  Stable backports needed?  Anything
> > else?
> > > > > >
> > > > > The ChromeOS is extremely short on memory by design and can trigger
> > > > > this situation very easily.
> > > >
> > > > So normal allocations are failing?  That feels wrong, what caused this?
> > >
> > > 64K is order 4 allocation and may fail according to [1].
> > 
> > And what changed to cause this to suddenly be 64k?  And why can't we
> > allocate 64k at this point in time now?
> > 
> > > > > I do not think that this patch fixes any commit - the problematic code
> > exists
> > > > > from the earliest versions of this driver.
> > > > > As this problem reproduced only on ChromeOS I believe that no need
> > > > > in wide backport, the ChromeOS can cherry-pick the patch.
> > > > > From your experience, is this the right strategy?
> > > >
> > > > No.
> > >
> > > Sure, I'll use
> > > Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> > > where the first time such buffer allocated and add stable here in v2.
> > 
> > So the problem has been there for years?  Why is it just now showing up?
> > 
> 
> I suppose it is the combination of some fairly new FW that requests 64K buffer
> for content-protection case, underpowered ChromeBook and ChromeOS running
> content-protection flow.
> All three conditions should be met to trigger this failure.

That's correct we've seen this on kernels as old as 5.4. I have
personally reproduced this issue and can confirm that vmalloc does fix
it.

> 
> > thanks,
> > 
> > greg k-h
> 
> - - 
> Thanks,
> Sasha
> 
>

Re: [char-misc-next] mei: use kvmalloc for read buffer

Posted by Brian Geffon 1 month, 1 week ago

On Mon, Oct 14, 2024 at 02:43:31PM +0000, Usyskin, Alexander wrote:
> 
> > -----Original Message-----
> > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > Sent: Monday, October 14, 2024 4:25 PM
> > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > 
> > On Mon, Oct 14, 2024 at 01:15:49PM +0000, Usyskin, Alexander wrote:
> > > > -----Original Message-----
> > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > Sent: Sunday, October 13, 2024 6:08 PM
> > > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > > >
> > > > On Sun, Oct 13, 2024 at 02:22:27PM +0000, Usyskin, Alexander wrote:
> > > > > > -----Original Message-----
> > > > > > From: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > > > Sent: Sunday, October 13, 2024 3:14 PM
> > > > > > To: Usyskin, Alexander <alexander.usyskin@intel.com>
> > > > > > Cc: Weil, Oren jer <oren.jer.weil@intel.com>; Tomas Winkler
> > > > > > <tomasw@gmail.com>; linux-kernel@vger.kernel.org
> > > > > > Subject: Re: [char-misc-next] mei: use kvmalloc for read buffer
> > > > > >
> > > > > > On Sun, Oct 13, 2024 at 02:53:14PM +0300, Alexander Usyskin wrote:
> > > > > > > Read buffer is allocated according to max message size,
> > > > > > > reported by the firmware and may reach 64K in systems
> > > > > > > with pxp client.
> > > > > > > Contiguous 64k allocation may fail under memory pressure.
> > > > > > > Read buffer is used as in-driver message storage and
> > > > > > > not required to be contiguous.
> > > > > > > Use kvmalloc to allow kernel to allocate non-contiguous
> > > > > > > memory in this case.
> > > > > > >
> > > > > > > Signed-off-by: Alexander Usyskin <alexander.usyskin@intel.com>
> > > > > > > ---
> > > > > > >  drivers/misc/mei/client.c | 4 ++--
> > > > > > >  1 file changed, 2 insertions(+), 2 deletions(-)
> > > > > >
> > > > > > What about this thread:
> > > > > > 	https://lore.kernel.org/all/20240813084542.2921300-1-
> > > > > > rohiagar@chromium.org/
> > >
> > > [1] https://lore.kernel.org/all/20240813084542.2921300-1-
> > rohiagar@chromium.org/
> > 
> > Yes, it's a problem, I don't understand.
> > 
> > > > > >
> > > > > > No attribution for the reporter?  Does it solve their problem?
> > > > > >
> > > > > This patch is a result from non-public bug report on ChromeOS.
> > > >
> > > > Then make that bug report public as it was discussed in public already :)
> > > >
> > > Unfortunately, it is not my call.
> > > For now, I'll anchor this on [1]
> > >
> > > > > > Also, where is this memory pressure coming from, what is the root
> > cause
> > > > > > and what commit does this fix?  Stable backports needed?  Anything
> > else?
> > > > > >
> > > > > The ChromeOS is extremely short on memory by design and can trigger
> > > > > this situation very easily.
> > > >
> > > > So normal allocations are failing?  That feels wrong, what caused this?
> > >
> > > 64K is order 4 allocation and may fail according to [1].
> > 
> > And what changed to cause this to suddenly be 64k?  And why can't we
> > allocate 64k at this point in time now?
> > 
> > > > > I do not think that this patch fixes any commit - the problematic code
> > exists
> > > > > from the earliest versions of this driver.
> > > > > As this problem reproduced only on ChromeOS I believe that no need
> > > > > in wide backport, the ChromeOS can cherry-pick the patch.
> > > > > From your experience, is this the right strategy?
> > > >
> > > > No.
> > >
> > > Sure, I'll use
> > > Fixes: 3030dc056459 ("mei: add wrapper for queuing control commands.")
> > > where the first time such buffer allocated and add stable here in v2.
> > 
> > So the problem has been there for years?  Why is it just now showing up?
> > 
> 
> I suppose it is the combination of some fairly new FW that requests 64K buffer
> for content-protection case, underpowered ChromeBook and ChromeOS running
> content-protection flow.
> All three conditions should be met to trigger this failure.
> 
> > thanks,
> > 
> > greg k-h
> 
> - - 
> Thanks,
> Sasha
> 
>