Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1437715 > unrolled thread
| Started by | weiyj_lk@163.com |
|---|---|
| First post | 2016-07-06 15:30 +0200 |
| Last post | 2016-07-06 21:10 +0200 |
| Articles | 6 — 3 participants |
Back to article view | Back to linux.kernel
[PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) weiyj_lk@163.com - 2016-07-06 15:30 +0200
Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) Mike Marshall <hubcap@omnibond.com> - 2016-07-06 19:30 +0200
Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) Joe Perches <joe@perches.com> - 2016-07-06 19:50 +0200
Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) Mike Marshall <hubcap@omnibond.com> - 2016-07-06 20:10 +0200
Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) Joe Perches <joe@perches.com> - 2016-07-06 20:20 +0200
Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) Mike Marshall <hubcap@omnibond.com> - 2016-07-06 21:10 +0200
| From | weiyj_lk@163.com |
|---|---|
| Date | 2016-07-06 15:30 +0200 |
| Subject | [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) |
| Message-ID | <rRV3P-1jb-15@gated-at.bofh.it> |
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);
[toc] | [next] | [standalone]
| From | Mike Marshall <hubcap@omnibond.com> |
|---|---|
| Date | 2016-07-06 19:30 +0200 |
| Message-ID | <rRYO5-3Ee-7@gated-at.bofh.it> |
| In reply to | #1437715 |
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);
>
>
>
>
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-07-06 19:50 +0200 |
| Subject | Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) |
| Message-ID | <rRZ7s-3Lq-3@gated-at.bofh.it> |
| In reply to | #1437862 |
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);
[toc] | [prev] | [next] | [standalone]
| From | Mike Marshall <hubcap@omnibond.com> |
|---|---|
| Date | 2016-07-06 20:10 +0200 |
| Message-ID | <rRZqO-47a-23@gated-at.bofh.it> |
| In reply to | #1437869 |
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);
>
[toc] | [prev] | [next] | [standalone]
| From | Joe Perches <joe@perches.com> |
|---|---|
| Date | 2016-07-06 20:20 +0200 |
| Subject | Re: [PATCH -next] orangefs: use vzalloc() instead of vmalloc()/memset(0) |
| Message-ID | <rRZAt-4ax-1@gated-at.bofh.it> |
| In reply to | #1437895 |
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
[toc] | [prev] | [next] | [standalone]
| From | Mike Marshall <hubcap@omnibond.com> |
|---|---|
| Date | 2016-07-06 21:10 +0200 |
| Message-ID | <rS0mR-4Im-7@gated-at.bofh.it> |
| In reply to | #1437896 |
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
[toc] | [prev] | [standalone]
Back to top | Article view | linux.kernel
csiph-web