Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
I meant that the patch needs Cc: sta...@vger.kernel.org. Thanks, jin On Mon, Jun 12, 2017 at 11:02 PM, Greg Kroah-Hartman wrote: > On Fri, Jun 09, 2017 at 12:15:42PM -0700, Jin Qian wrote: >> Reviewed-by: Jin Qian >> >> 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
Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
On Fri, Jun 09, 2017 at 12:15:42PM -0700, Jin Qian wrote: > Reviewed-by: Jin Qian > > 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
Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
Reviewed-by: Jin Qian Can we merge this to stable? Thanks! jin On Mon, May 22, 2017 at 11:48 AM, Alan Cox wrote: > On Mon, 22 May 2017 13:51:52 +0200 > Michal Hocko 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 >> > > >> > > 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
Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
On Mon, 22 May 2017 13:51:52 +0200 Michal Hocko 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 > > > > > > 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
Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
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 > > > > 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 > > --- > > 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
Re: [PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
On Sun 21-05-17 00:45:46, Wei Yongjun wrote: > From: Wei Yongjun > > 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 > --- > 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
[PATCH] goldfish_pipe: use GFP_ATOMIC under spin lock
From: Wei Yongjun 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 --- 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);
