* [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0)
@ 2016-07-06 13:23 weiyj_lk
2016-07-06 17:28 ` Mike Marshall
0 siblings, 1 reply; 6+ messages in thread
From: weiyj_lk @ 2016-07-06 13:23 UTC (permalink / raw)
To: Mike Marshall; +Cc: Wei Yongjun, linux-kernel
From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
Use vzalloc() instead of vmalloc() and memset(0).
Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
---
fs/orangefs/devorangefs-req.c | 4 +---
1 file changed, 1 insertion(+), 3 deletions(-)
diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
index a287a66..e839073 100644
--- a/fs/orangefs/devorangefs-req.c
+++ b/fs/orangefs/devorangefs-req.c
@@ -442,14 +442,12 @@ static ssize_t orangefs_devreq_write_iter(struct kiocb *iocb,
if (op->downcall.type != ORANGEFS_VFS_OP_READDIR)
goto wakeup;
- op->downcall.trailer_buf =
- vmalloc(op->downcall.trailer_size);
+ op->downcall.trailer_buf = vzalloc(op->downcall.trailer_size);
if (op->downcall.trailer_buf == NULL) {
gossip_err("%s: failed trailer vmalloc.\n",
__func__);
goto Enomem;
}
- memset(op->downcall.trailer_buf, 0, op->downcall.trailer_size);
n = copy_from_iter(op->downcall.trailer_buf,
op->downcall.trailer_size,
iter);
^ permalink raw reply related [flat|nested] 6+ messages in thread* Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0)
2016-07-06 13:23 [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) weiyj_lk
@ 2016-07-06 17:28 ` Mike Marshall
2016-07-06 17:49 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Mike Marshall @ 2016-07-06 17:28 UTC (permalink / raw)
To: weiyj_lk, Mike Marshall; +Cc: Wei Yongjun, LKML
Wei...
I have applied your patch and tested it, it seems good.
The gossip statement still references vmalloc, I used
"git commit --amend" to fix that for my test. I'll do the
same when I push to kernel.org too, if you're OK with that,
unless you want to fix it yourself and resend the patch...
Thanks!
-Mike
On Wed, Jul 6, 2016 at 9:23 AM, <weiyj_lk@163.com> wrote:
> From: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
>
> Use vzalloc() instead of vmalloc() and memset(0).
>
> Signed-off-by: Wei Yongjun <yongjun_wei@trendmicro.com.cn>
> ---
> fs/orangefs/devorangefs-req.c | 4 +---
> 1 file changed, 1 insertion(+), 3 deletions(-)
>
> diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
> index a287a66..e839073 100644
> --- a/fs/orangefs/devorangefs-req.c
> +++ b/fs/orangefs/devorangefs-req.c
> @@ -442,14 +442,12 @@ static ssize_t orangefs_devreq_write_iter(struct kiocb *iocb,
> if (op->downcall.type != ORANGEFS_VFS_OP_READDIR)
> goto wakeup;
>
> - op->downcall.trailer_buf =
> - vmalloc(op->downcall.trailer_size);
> + op->downcall.trailer_buf = vzalloc(op->downcall.trailer_size);
> if (op->downcall.trailer_buf == NULL) {
> gossip_err("%s: failed trailer vmalloc.\n",
> __func__);
> goto Enomem;
> }
> - memset(op->downcall.trailer_buf, 0, op->downcall.trailer_size);
> n = copy_from_iter(op->downcall.trailer_buf,
> op->downcall.trailer_size,
> iter);
>
>
>
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0)
2016-07-06 17:28 ` Mike Marshall
@ 2016-07-06 17:49 ` Joe Perches
2016-07-06 18:00 ` Mike Marshall
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-07-06 17:49 UTC (permalink / raw)
To: Mike Marshall, weiyj_lk; +Cc: Wei Yongjun, LKML
On Wed, 2016-07-06 at 13:28 -0400, Mike Marshall wrote:
> Wei...
>
> I have applied your patch and tested it, it seems good.
>
> The gossip statement still references vmalloc, I used
> "git commit --amend" to fix that for my test. I'll do the
> same when I push to kernel.org too, if you're OK with that,
> unless you want to fix it yourself and resend the patch...
Better still would be to remove the gossip_err() altogether
as v.alloc already does a dump_stack on OOM.
> > diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
[]
> > @@ -442,14 +442,12 @@ static ssize_t orangefs_devreq_write_iter(struct kiocb *iocb,
> > if (op->downcall.type != ORANGEFS_VFS_OP_READDIR)
> > goto wakeup;
> >
> > - op->downcall.trailer_buf =
> > - vmalloc(op->downcall.trailer_size);
> > + op->downcall.trailer_buf = vzalloc(op->downcall.trailer_size);
> > if (op->downcall.trailer_buf == NULL) {
> > gossip_err("%s: failed trailer vmalloc.\n",
> > __func__);
> > goto Enomem;
> > }
> > - memset(op->downcall.trailer_buf, 0, op->downcall.trailer_size);
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0)
2016-07-06 17:49 ` Joe Perches
@ 2016-07-06 18:00 ` Mike Marshall
2016-07-06 18:11 ` Joe Perches
0 siblings, 1 reply; 6+ messages in thread
From: Mike Marshall @ 2016-07-06 18:00 UTC (permalink / raw)
To: Joe Perches, Mike Marshall; +Cc: weiyj_lk, Wei Yongjun, LKML
Hi Joe...
I looked around at other code to see how they were using vzalloc...
even if it is just the caller of the function that uses vzalloc, most
fs's that use vzalloc print an error message when vzalloc fails... I sure do
like to follow the flow of our kernel module all in one place
with the gossip statements...
-Mike
On Wed, Jul 6, 2016 at 1:49 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2016-07-06 at 13:28 -0400, Mike Marshall wrote:
>> Wei...
>>
>> I have applied your patch and tested it, it seems good.
>>
>> The gossip statement still references vmalloc, I used
>> "git commit --amend" to fix that for my test. I'll do the
>> same when I push to kernel.org too, if you're OK with that,
>> unless you want to fix it yourself and resend the patch...
>
> Better still would be to remove the gossip_err() altogether
> as v.alloc already does a dump_stack on OOM.
>> > diff --git a/fs/orangefs/devorangefs-req.c b/fs/orangefs/devorangefs-req.c
> []
>> > @@ -442,14 +442,12 @@ static ssize_t orangefs_devreq_write_iter(struct kiocb *iocb,
>> > if (op->downcall.type != ORANGEFS_VFS_OP_READDIR)
>> > goto wakeup;
>> >
>> > - op->downcall.trailer_buf =
>> > - vmalloc(op->downcall.trailer_size);
>> > + op->downcall.trailer_buf = vzalloc(op->downcall.trailer_size);
>> > if (op->downcall.trailer_buf == NULL) {
>> > gossip_err("%s: failed trailer vmalloc.\n",
>> > __func__);
>> > goto Enomem;
>> > }
>> > - memset(op->downcall.trailer_buf, 0, op->downcall.trailer_size);
>
^ permalink raw reply [flat|nested] 6+ messages in thread* Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0)
2016-07-06 18:00 ` Mike Marshall
@ 2016-07-06 18:11 ` Joe Perches
2016-07-06 19:01 ` Mike Marshall
0 siblings, 1 reply; 6+ messages in thread
From: Joe Perches @ 2016-07-06 18:11 UTC (permalink / raw)
To: Mike Marshall; +Cc: weiyj_lk, Wei Yongjun, LKML
On Wed, 2016-07-06 at 14:00 -0400, Mike Marshall wrote:
> Hi Joe...
Hi Mike.
> I looked around at other code to see how they were using vzalloc...
> even if it is just the caller of the function that uses vzalloc, most
> fs's that use vzalloc print an error message when vzalloc fails...
That's incorrect.
There are 72 v.alloc calls in fs/
2 of those calls have a specific OOM message.
There's 1 in reiserfs (old and unsupported) and 1 in orangefs
> I sure do
> like to follow the flow of our kernel module all in one place
> with the gossip statements...
I don't see how that matters here as there's
already an OOM on v.alloc failure.
cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0)
2016-07-06 18:11 ` Joe Perches
@ 2016-07-06 19:01 ` Mike Marshall
0 siblings, 0 replies; 6+ messages in thread
From: Mike Marshall @ 2016-07-06 19:01 UTC (permalink / raw)
To: Joe Perches, Mike Marshall; +Cc: weiyj_lk, Wei Yongjun, LKML
Joe> That's incorrect.
<blush>... you are right. They mostly return -ENOMEM, but they
mostly don't print an error message. I like printing an error message
when I resolve to an error... a whole bunch of that function is sanity
checking input from userspace, with a specific error message for
any particular failure.
Joe> I don't see how that matters here as there's
Joe> already an OOM on v.alloc failure.
I'd find that if needed, but the first place I'd look when investigating
an Orangefs failure would be the Orangefs logs...
-Mike
On Wed, Jul 6, 2016 at 2:11 PM, Joe Perches <joe@perches.com> wrote:
> On Wed, 2016-07-06 at 14:00 -0400, Mike Marshall wrote:
>> Hi Joe...
>
> Hi Mike.
>
>> I looked around at other code to see how they were using vzalloc...
>> even if it is just the caller of the function that uses vzalloc, most
>> fs's that use vzalloc print an error message when vzalloc fails...
>
> That's incorrect.
>
> There are 72 v.alloc calls in fs/
> 2 of those calls have a specific OOM message.
> There's 1 in reiserfs (old and unsupported) and 1 in orangefs
>
>> I sure do
>> like to follow the flow of our kernel module all in one place
>> with the gossip statements...
>
> I don't see how that matters here as there's
> already an OOM on v.alloc failure.
>
> cheers, Joe
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-07-06 19:01 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-06 13:23 [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) weiyj_lk
2016-07-06 17:28 ` Mike Marshall
2016-07-06 17:49 ` Joe Perches
2016-07-06 18:00 ` Mike Marshall
2016-07-06 18:11 ` Joe Perches
2016-07-06 19:01 ` Mike Marshall
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.