* [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
@ 2017-05-21 0:45 Wei Yongjun
2017-05-21 7:48 ` Michal Hocko
0 siblings, 1 reply; 7+ messages in thread
From: Wei Yongjun @ 2017-05-21 0:45 UTC (permalink / raw)
To: jinqian, gregkh, lstoakes, jack, mhocko, christian.koenig
Cc: Wei Yongjun, linux-kernel
From: Wei Yongjun <weiyongjun1@huawei.com>
The function get_free_pipe_id_locked() is called from
goldfish_pipe_open() with a lock is held, so we should
use GFP_ATOMIC instead of GFP_KERNEL.
Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
---
drivers/platform/goldfish/goldfish_pipe.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
index 2de1e60..5f36721 100644
--- a/drivers/platform/goldfish/goldfish_pipe.c
+++ b/drivers/platform/goldfish/goldfish_pipe.c
@@ -704,7 +704,7 @@ static int get_free_pipe_id_locked(struct goldfish_pipe_dev *dev)
/* Reallocate the array */
u32 new_capacity = 2 * dev->pipes_capacity;
struct goldfish_pipe **pipes =
- kcalloc(new_capacity, sizeof(*pipes), GFP_KERNEL);
+ kcalloc(new_capacity, sizeof(*pipes), GFP_ATOMIC);
if (!pipes)
return -ENOMEM;
memcpy(pipes, dev->pipes, sizeof(*pipes) * dev->pipes_capacity);
^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
2017-05-21 0:45 [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock Wei Yongjun
@ 2017-05-21 7:48 ` Michal Hocko
2017-05-22 11:51 ` Michal Hocko
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2017-05-21 7:48 UTC (permalink / raw)
To: Wei Yongjun
Cc: jinqian, gregkh, lstoakes, jack, christian.koenig, Wei Yongjun,
linux-kernel
On Sun 21-05-17 00:45:46, Wei Yongjun wrote:
> From: Wei Yongjun <weiyongjun1@huawei.com>
>
> The function get_free_pipe_id_locked() is called from
> goldfish_pipe_open() with a lock is held, so we should
> use GFP_ATOMIC instead of GFP_KERNEL.
Why is GFP_NOWAIT insufficient? Does this path needs an access to memory
reserves?
> Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> ---
> drivers/platform/goldfish/goldfish_pipe.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
> index 2de1e60..5f36721 100644
> --- a/drivers/platform/goldfish/goldfish_pipe.c
> +++ b/drivers/platform/goldfish/goldfish_pipe.c
> @@ -704,7 +704,7 @@ static int get_free_pipe_id_locked(struct goldfish_pipe_dev *dev)
> /* Reallocate the array */
> u32 new_capacity = 2 * dev->pipes_capacity;
> struct goldfish_pipe **pipes =
> - kcalloc(new_capacity, sizeof(*pipes), GFP_KERNEL);
> + kcalloc(new_capacity, sizeof(*pipes), GFP_ATOMIC);
> if (!pipes)
> return -ENOMEM;
> memcpy(pipes, dev->pipes, sizeof(*pipes) * dev->pipes_capacity);
>
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
2017-05-21 7:48 ` Michal Hocko
@ 2017-05-22 11:51 ` Michal Hocko
2017-05-22 18:48 ` Alan Cox
0 siblings, 1 reply; 7+ messages in thread
From: Michal Hocko @ 2017-05-22 11:51 UTC (permalink / raw)
To: Wei Yongjun
Cc: jinqian, gregkh, lstoakes, jack, christian.koenig, Wei Yongjun,
linux-kernel
On Sun 21-05-17 09:48:36, Michal Hocko wrote:
> On Sun 21-05-17 00:45:46, Wei Yongjun wrote:
> > From: Wei Yongjun <weiyongjun1@huawei.com>
> >
> > The function get_free_pipe_id_locked() is called from
> > goldfish_pipe_open() with a lock is held, so we should
> > use GFP_ATOMIC instead of GFP_KERNEL.
>
> Why is GFP_NOWAIT insufficient? Does this path needs an access to memory
> reserves?
And now when looking at the code more deeply, wouldn't it be much better
to simply do the allocation outside of the spin lock and do assignments
with the lock held?
>
> > Signed-off-by: Wei Yongjun <weiyongjun1@huawei.com>
> > ---
> > drivers/platform/goldfish/goldfish_pipe.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/platform/goldfish/goldfish_pipe.c b/drivers/platform/goldfish/goldfish_pipe.c
> > index 2de1e60..5f36721 100644
> > --- a/drivers/platform/goldfish/goldfish_pipe.c
> > +++ b/drivers/platform/goldfish/goldfish_pipe.c
> > @@ -704,7 +704,7 @@ static int get_free_pipe_id_locked(struct goldfish_pipe_dev *dev)
> > /* Reallocate the array */
> > u32 new_capacity = 2 * dev->pipes_capacity;
> > struct goldfish_pipe **pipes =
> > - kcalloc(new_capacity, sizeof(*pipes), GFP_KERNEL);
> > + kcalloc(new_capacity, sizeof(*pipes), GFP_ATOMIC);
> > if (!pipes)
> > return -ENOMEM;
> > memcpy(pipes, dev->pipes, sizeof(*pipes) * dev->pipes_capacity);
> >
>
> --
> Michal Hocko
> SUSE Labs
--
Michal Hocko
SUSE Labs
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
2017-05-22 11:51 ` Michal Hocko
@ 2017-05-22 18:48 ` Alan Cox
2017-06-09 19:15 ` Jin Qian
0 siblings, 1 reply; 7+ messages in thread
From: Alan Cox @ 2017-05-22 18:48 UTC (permalink / raw)
To: Michal Hocko
Cc: Wei Yongjun, jinqian, gregkh, lstoakes, jack, christian.koenig,
Wei Yongjun, linux-kernel
On Mon, 22 May 2017 13:51:52 +0200
Michal Hocko <mhocko@kernel.org> wrote:
> On Sun 21-05-17 09:48:36, Michal Hocko wrote:
> > On Sun 21-05-17 00:45:46, Wei Yongjun wrote:
> > > From: Wei Yongjun <weiyongjun1@huawei.com>
> > >
> > > The function get_free_pipe_id_locked() is called from
> > > goldfish_pipe_open() with a lock is held, so we should
> > > use GFP_ATOMIC instead of GFP_KERNEL.
> >
> > Why is GFP_NOWAIT insufficient? Does this path needs an access to memory
> > reserves?
>
> And now when looking at the code more deeply, wouldn't it be much better
> to simply do the allocation outside of the spin lock and do assignments
> with the lock held?
That's far from trivial and certainly for backporting and an immediate
fix this seems better. The allocations are not that large and any fail
would be in open() not anywhere weird.
Alan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
2017-05-22 18:48 ` Alan Cox
@ 2017-06-09 19:15 ` Jin Qian
2017-06-13 6:02 ` Greg Kroah-Hartman
0 siblings, 1 reply; 7+ messages in thread
From: Jin Qian @ 2017-06-09 19:15 UTC (permalink / raw)
To: Alan Cox
Cc: Michal Hocko, Wei Yongjun, Greg Kroah-Hartman, lstoakes, jack,
christian.koenig, Wei Yongjun, linux-kernel
Reviewed-by: Jin Qian <jinqian@google.com>
Can we merge this to stable?
Thanks!
jin
On Mon, May 22, 2017 at 11:48 AM, Alan Cox <gnomes@lxorguk.ukuu.org.uk> wrote:
> On Mon, 22 May 2017 13:51:52 +0200
> Michal Hocko <mhocko@kernel.org> wrote:
>
>> On Sun 21-05-17 09:48:36, Michal Hocko wrote:
>> > On Sun 21-05-17 00:45:46, Wei Yongjun wrote:
>> > > From: Wei Yongjun <weiyongjun1@huawei.com>
>> > >
>> > > The function get_free_pipe_id_locked() is called from
>> > > goldfish_pipe_open() with a lock is held, so we should
>> > > use GFP_ATOMIC instead of GFP_KERNEL.
>> >
>> > Why is GFP_NOWAIT insufficient? Does this path needs an access to memory
>> > reserves?
>>
>> And now when looking at the code more deeply, wouldn't it be much better
>> to simply do the allocation outside of the spin lock and do assignments
>> with the lock held?
>
> That's far from trivial and certainly for backporting and an immediate
> fix this seems better. The allocations are not that large and any fail
> would be in open() not anywhere weird.
>
> Alan
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
2017-06-09 19:15 ` Jin Qian
@ 2017-06-13 6:02 ` Greg Kroah-Hartman
2017-06-13 18:45 ` Jin Qian
0 siblings, 1 reply; 7+ messages in thread
From: Greg Kroah-Hartman @ 2017-06-13 6:02 UTC (permalink / raw)
To: Jin Qian
Cc: Alan Cox, Michal Hocko, Wei Yongjun, lstoakes, jack,
christian.koenig, Wei Yongjun, linux-kernel
On Fri, Jun 09, 2017 at 12:15:42PM -0700, Jin Qian wrote:
> Reviewed-by: Jin Qian <jinqian@google.com>
>
> Can we merge this to stable?
I don't understand the question, what kernel(s) exactly do you want this
commit to be added to?
thanks,
greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
2017-06-13 6:02 ` Greg Kroah-Hartman
@ 2017-06-13 18:45 ` Jin Qian
0 siblings, 0 replies; 7+ messages in thread
From: Jin Qian @ 2017-06-13 18:45 UTC (permalink / raw)
To: Greg Kroah-Hartman
Cc: Alan Cox, Michal Hocko, Wei Yongjun, lstoakes, jack,
christian.koenig, Wei Yongjun, linux-kernel
I meant that the patch needs Cc: stable@vger.kernel.org.
Thanks,
jin
On Mon, Jun 12, 2017 at 11:02 PM, Greg Kroah-Hartman
<gregkh@linuxfoundation.org> wrote:
> On Fri, Jun 09, 2017 at 12:15:42PM -0700, Jin Qian wrote:
>> Reviewed-by: Jin Qian <jinqian@google.com>
>>
>> Can we merge this to stable?
>
> I don't understand the question, what kernel(s) exactly do you want this
> commit to be added to?
>
> thanks,
>
> greg k-h
^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2017-06-13 18:45 UTC | newest]
Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-05-21 0:45 [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock Wei Yongjun
2017-05-21 7:48 ` Michal Hocko
2017-05-22 11:51 ` Michal Hocko
2017-05-22 18:48 ` Alan Cox
2017-06-09 19:15 ` Jin Qian
2017-06-13 6:02 ` Greg Kroah-Hartman
2017-06-13 18:45 ` Jin Qian
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox