Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Fri, 3 Apr 2015, Drokin, Oleg wrote: > Hello! > > On Apr 2, 2015, at 6:18 AM, Julia Lawall wrote: > > >> Julia, I wonder if you happen to have a bunch of other patches to get rid > >> of the rest of OBD_ALLOC and OBD_FREE stuff by any chance? > > I can generate them again, but I wasn't clear on what was wanted. I would > > really prefer something where it is explicit at the call site that an > > assignment is taking place. If we can have x = obd_alloc(...) and > > obd_free(x,...) (I don't have time to look up the exact arguments at the > > moment), then I can take care of that). I still think it is too bad that > > this code won't benefit from rules written for more generic memory > > allocation functions, but if the extra debugging facility provided by > > these functions is useful, then I guess it is reasonable to keep it. > > Like I mentioned sometime last year - it's now pretty easy to replace the > memleak > detection with other in-kernel mechanisms some of which are in fact even > better > than what we have. And considering our mechanisms are totally broken now by > the mixup of > wrapped vs nonwrapped allocation/freeing - there's no point in holding to it > remaining at all. > The only last bit of useful functionality left, I imagine, is the ability to > redirect allocation > to regular kmalloc or to vmalloc based on the allocation size (there's kvfree > already for the > freeing part of it). > Other than that the wrappers could go away at any time now, I think. OK, thanks. julia -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
Hello! On Apr 2, 2015, at 6:18 AM, Julia Lawall wrote: >> Julia, I wonder if you happen to have a bunch of other patches to get rid of >> the rest of OBD_ALLOC and OBD_FREE stuff by any chance? > I can generate them again, but I wasn't clear on what was wanted. I would > really prefer something where it is explicit at the call site that an > assignment is taking place. If we can have x = obd_alloc(...) and > obd_free(x,...) (I don't have time to look up the exact arguments at the > moment), then I can take care of that). I still think it is too bad that > this code won't benefit from rules written for more generic memory > allocation functions, but if the extra debugging facility provided by > these functions is useful, then I guess it is reasonable to keep it. Like I mentioned sometime last year - it's now pretty easy to replace the memleak detection with other in-kernel mechanisms some of which are in fact even better than what we have. And considering our mechanisms are totally broken now by the mixup of wrapped vs nonwrapped allocation/freeing - there's no point in holding to it remaining at all. The only last bit of useful functionality left, I imagine, is the ability to redirect allocation to regular kmalloc or to vmalloc based on the allocation size (there's kvfree already for the freeing part of it). Other than that the wrappers could go away at any time now, I think. Bye, Oleg-- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
Hello! On Apr 2, 2015, at 6:18 AM, Julia Lawall wrote: Julia, I wonder if you happen to have a bunch of other patches to get rid of the rest of OBD_ALLOC and OBD_FREE stuff by any chance? I can generate them again, but I wasn't clear on what was wanted. I would really prefer something where it is explicit at the call site that an assignment is taking place. If we can have x = obd_alloc(...) and obd_free(x,...) (I don't have time to look up the exact arguments at the moment), then I can take care of that). I still think it is too bad that this code won't benefit from rules written for more generic memory allocation functions, but if the extra debugging facility provided by these functions is useful, then I guess it is reasonable to keep it. Like I mentioned sometime last year - it's now pretty easy to replace the memleak detection with other in-kernel mechanisms some of which are in fact even better than what we have. And considering our mechanisms are totally broken now by the mixup of wrapped vs nonwrapped allocation/freeing - there's no point in holding to it remaining at all. The only last bit of useful functionality left, I imagine, is the ability to redirect allocation to regular kmalloc or to vmalloc based on the allocation size (there's kvfree already for the freeing part of it). Other than that the wrappers could go away at any time now, I think. Bye, Oleg-- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Fri, 3 Apr 2015, Drokin, Oleg wrote: Hello! On Apr 2, 2015, at 6:18 AM, Julia Lawall wrote: Julia, I wonder if you happen to have a bunch of other patches to get rid of the rest of OBD_ALLOC and OBD_FREE stuff by any chance? I can generate them again, but I wasn't clear on what was wanted. I would really prefer something where it is explicit at the call site that an assignment is taking place. If we can have x = obd_alloc(...) and obd_free(x,...) (I don't have time to look up the exact arguments at the moment), then I can take care of that). I still think it is too bad that this code won't benefit from rules written for more generic memory allocation functions, but if the extra debugging facility provided by these functions is useful, then I guess it is reasonable to keep it. Like I mentioned sometime last year - it's now pretty easy to replace the memleak detection with other in-kernel mechanisms some of which are in fact even better than what we have. And considering our mechanisms are totally broken now by the mixup of wrapped vs nonwrapped allocation/freeing - there's no point in holding to it remaining at all. The only last bit of useful functionality left, I imagine, is the ability to redirect allocation to regular kmalloc or to vmalloc based on the allocation size (there's kvfree already for the freeing part of it). Other than that the wrappers could go away at any time now, I think. OK, thanks. julia -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Tue, 31 Mar 2015, Drokin, Oleg wrote:
>
> On Mar 31, 2015, at 11:57 AM, gre...@linuxfoundation.org wrote:
>
> > On Tue, Mar 31, 2015 at 05:15:23PM +0200, Julia Lawall wrote:
> >> On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
> >>
> >>> This patch replaces kzalloc and copy_from_user with memdup_user call
> >>> This change was detected with coccinelle tool
> >>>
> >>> Signed-off-by: Chaitanya Dhere
> >>> ---
> >>> drivers/staging/lustre/lustre/llite/file.c | 11 +++
> >>> 1 file changed, 3 insertions(+), 8 deletions(-)
> >>>
> >>> diff --git a/drivers/staging/lustre/lustre/llite/file.c
> >>> b/drivers/staging/lustre/lustre/llite/file.c
> >>> index 85e74d1..85b5567 100644
> >>> --- a/drivers/staging/lustre/lustre/llite/file.c
> >>> +++ b/drivers/staging/lustre/lustre/llite/file.c
> >>> @@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
> >>> unsigned long arg)
> >>> struct hsm_state_set*hss;
> >>> int rc;
> >>>
> >>> - hss = kzalloc(sizeof(*hss), GFP_NOFS);
> >>> - if (!hss)
> >>> - return -ENOMEM;
> >>> -
> >>> - if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
> >>> - OBD_FREE_PTR(hss);
> >>> - return -EFAULT;
> >>> - }
> >>> + hss = memdup_user((char *)arg, sizeof(*hss));
> >>
> >> memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
> >> __GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
> >> is specified. I don't know if this is a problem here.
> >
> > Yes, this is a filesystem, so this can't be changed, as we can't have
> > the allocation go out and ask for more filesystem accesses in the middle
> > of trying to do a filesystem access :)
>
> Technically in this place we are not really holding any locks or anything
> else of value to cause a deadlock,
> so we might be fine here.
> More importantly, I totally missed this OBD_ALLOC replacement with kzalloc
> when it happened.
> In theory all OBD_ALLOC() calls add up all allocated memory in a counter and
> then OBD_FREE() calls
> subtract freed memory (for a poor man's memory leak detection and tracing).
> Now since it's out of match, there should have been tons of very loud
> warnings about it, but I don't see
> any in my logs and I wonder why.
>
> Julia, I wonder if you happen to have a bunch of other patches to get rid of
> the rest of OBD_ALLOC and OBD_FREE stuff by any chance?
I can generate them again, but I wasn't clear on what was wanted. I would
really prefer something where it is explicit at the call site that an
assignment is taking place. If we can have x = obd_alloc(...) and
obd_free(x,...) (I don't have time to look up the exact arguments at the
moment), then I can take care of that). I still think it is too bad that
this code won't benefit from rules written for more generic memory
allocation functions, but if the extra debugging facility provided by
these functions is useful, then I guess it is reasonable to keep it.
julia
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Tue, 31 Mar 2015, Drokin, Oleg wrote:
On Mar 31, 2015, at 11:57 AM, gre...@linuxfoundation.org wrote:
On Tue, Mar 31, 2015 at 05:15:23PM +0200, Julia Lawall wrote:
On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
This patch replaces kzalloc and copy_from_user with memdup_user call
This change was detected with coccinelle tool
Signed-off-by: Chaitanya Dhere cvija...@visteon.com
---
drivers/staging/lustre/lustre/llite/file.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/file.c
b/drivers/staging/lustre/lustre/llite/file.c
index 85e74d1..85b5567 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
struct hsm_state_set*hss;
int rc;
- hss = kzalloc(sizeof(*hss), GFP_NOFS);
- if (!hss)
- return -ENOMEM;
-
- if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
- OBD_FREE_PTR(hss);
- return -EFAULT;
- }
+ hss = memdup_user((char *)arg, sizeof(*hss));
memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
__GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
is specified. I don't know if this is a problem here.
Yes, this is a filesystem, so this can't be changed, as we can't have
the allocation go out and ask for more filesystem accesses in the middle
of trying to do a filesystem access :)
Technically in this place we are not really holding any locks or anything
else of value to cause a deadlock,
so we might be fine here.
More importantly, I totally missed this OBD_ALLOC replacement with kzalloc
when it happened.
In theory all OBD_ALLOC() calls add up all allocated memory in a counter and
then OBD_FREE() calls
subtract freed memory (for a poor man's memory leak detection and tracing).
Now since it's out of match, there should have been tons of very loud
warnings about it, but I don't see
any in my logs and I wonder why.
Julia, I wonder if you happen to have a bunch of other patches to get rid of
the rest of OBD_ALLOC and OBD_FREE stuff by any chance?
I can generate them again, but I wasn't clear on what was wanted. I would
really prefer something where it is explicit at the call site that an
assignment is taking place. If we can have x = obd_alloc(...) and
obd_free(x,...) (I don't have time to look up the exact arguments at the
moment), then I can take care of that). I still think it is too bad that
this code won't benefit from rules written for more generic memory
allocation functions, but if the extra debugging facility provided by
these functions is useful, then I guess it is reasonable to keep it.
julia
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Mar 31, 2015, at 11:57 AM, gre...@linuxfoundation.org wrote:
> On Tue, Mar 31, 2015 at 05:15:23PM +0200, Julia Lawall wrote:
>> On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
>>
>>> This patch replaces kzalloc and copy_from_user with memdup_user call
>>> This change was detected with coccinelle tool
>>>
>>> Signed-off-by: Chaitanya Dhere
>>> ---
>>> drivers/staging/lustre/lustre/llite/file.c | 11 +++
>>> 1 file changed, 3 insertions(+), 8 deletions(-)
>>>
>>> diff --git a/drivers/staging/lustre/lustre/llite/file.c
>>> b/drivers/staging/lustre/lustre/llite/file.c
>>> index 85e74d1..85b5567 100644
>>> --- a/drivers/staging/lustre/lustre/llite/file.c
>>> +++ b/drivers/staging/lustre/lustre/llite/file.c
>>> @@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
>>> unsigned long arg)
>>> struct hsm_state_set*hss;
>>> int rc;
>>>
>>> - hss = kzalloc(sizeof(*hss), GFP_NOFS);
>>> - if (!hss)
>>> - return -ENOMEM;
>>> -
>>> - if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
>>> - OBD_FREE_PTR(hss);
>>> - return -EFAULT;
>>> - }
>>> + hss = memdup_user((char *)arg, sizeof(*hss));
>>
>> memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
>> __GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
>> is specified. I don't know if this is a problem here.
>
> Yes, this is a filesystem, so this can't be changed, as we can't have
> the allocation go out and ask for more filesystem accesses in the middle
> of trying to do a filesystem access :)
Technically in this place we are not really holding any locks or anything else
of value to cause a deadlock,
so we might be fine here.
More importantly, I totally missed this OBD_ALLOC replacement with kzalloc when
it happened.
In theory all OBD_ALLOC() calls add up all allocated memory in a counter and
then OBD_FREE() calls
subtract freed memory (for a poor man's memory leak detection and tracing).
Now since it's out of match, there should have been tons of very loud warnings
about it, but I don't see
any in my logs and I wonder why.
Julia, I wonder if you happen to have a bunch of other patches to get rid of
the rest of OBD_ALLOC and OBD_FREE stuff by any chance?
Bye,
Oleg--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Tue, Mar 31, 2015 at 05:15:23PM +0200, Julia Lawall wrote:
> On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
>
> > This patch replaces kzalloc and copy_from_user with memdup_user call
> > This change was detected with coccinelle tool
> >
> > Signed-off-by: Chaitanya Dhere
> > ---
> > drivers/staging/lustre/lustre/llite/file.c | 11 +++
> > 1 file changed, 3 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/staging/lustre/lustre/llite/file.c
> > b/drivers/staging/lustre/lustre/llite/file.c
> > index 85e74d1..85b5567 100644
> > --- a/drivers/staging/lustre/lustre/llite/file.c
> > +++ b/drivers/staging/lustre/lustre/llite/file.c
> > @@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
> > unsigned long arg)
> > struct hsm_state_set*hss;
> > int rc;
> >
> > - hss = kzalloc(sizeof(*hss), GFP_NOFS);
> > - if (!hss)
> > - return -ENOMEM;
> > -
> > - if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
> > - OBD_FREE_PTR(hss);
> > - return -EFAULT;
> > - }
> > + hss = memdup_user((char *)arg, sizeof(*hss));
>
> memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
> __GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
> is specified. I don't know if this is a problem here.
Yes, this is a filesystem, so this can't be changed, as we can't have
the allocation go out and ask for more filesystem accesses in the middle
of trying to do a filesystem access :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
> This patch replaces kzalloc and copy_from_user with memdup_user call
> This change was detected with coccinelle tool
>
> Signed-off-by: Chaitanya Dhere
> ---
> drivers/staging/lustre/lustre/llite/file.c | 11 +++
> 1 file changed, 3 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/staging/lustre/lustre/llite/file.c
> b/drivers/staging/lustre/lustre/llite/file.c
> index 85e74d1..85b5567 100644
> --- a/drivers/staging/lustre/lustre/llite/file.c
> +++ b/drivers/staging/lustre/lustre/llite/file.c
> @@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
> unsigned long arg)
> struct hsm_state_set*hss;
> int rc;
>
> - hss = kzalloc(sizeof(*hss), GFP_NOFS);
> - if (!hss)
> - return -ENOMEM;
> -
> - if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
> - OBD_FREE_PTR(hss);
> - return -EFAULT;
> - }
> + hss = memdup_user((char *)arg, sizeof(*hss));
memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
__GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
is specified. I don't know if this is a problem here.
julia
> + if (IS_ERR(hss))
> + return PTR_ERR(hss);
>
> rc = ll_hsm_state_set(inode, hss);
>
> --
> 1.7.9.5
>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
This patch replaces kzalloc and copy_from_user with memdup_user call
This change was detected with coccinelle tool
Signed-off-by: Chaitanya Dhere cvija...@visteon.com
---
drivers/staging/lustre/lustre/llite/file.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/file.c
b/drivers/staging/lustre/lustre/llite/file.c
index 85e74d1..85b5567 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
struct hsm_state_set*hss;
int rc;
- hss = kzalloc(sizeof(*hss), GFP_NOFS);
- if (!hss)
- return -ENOMEM;
-
- if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
- OBD_FREE_PTR(hss);
- return -EFAULT;
- }
+ hss = memdup_user((char *)arg, sizeof(*hss));
memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
__GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
is specified. I don't know if this is a problem here.
julia
+ if (IS_ERR(hss))
+ return PTR_ERR(hss);
rc = ll_hsm_state_set(inode, hss);
--
1.7.9.5
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Tue, Mar 31, 2015 at 05:15:23PM +0200, Julia Lawall wrote:
On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
This patch replaces kzalloc and copy_from_user with memdup_user call
This change was detected with coccinelle tool
Signed-off-by: Chaitanya Dhere cvija...@visteon.com
---
drivers/staging/lustre/lustre/llite/file.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/file.c
b/drivers/staging/lustre/lustre/llite/file.c
index 85e74d1..85b5567 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
struct hsm_state_set*hss;
int rc;
- hss = kzalloc(sizeof(*hss), GFP_NOFS);
- if (!hss)
- return -ENOMEM;
-
- if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
- OBD_FREE_PTR(hss);
- return -EFAULT;
- }
+ hss = memdup_user((char *)arg, sizeof(*hss));
memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
__GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
is specified. I don't know if this is a problem here.
Yes, this is a filesystem, so this can't be changed, as we can't have
the allocation go out and ask for more filesystem accesses in the middle
of trying to do a filesystem access :)
thanks,
greg k-h
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] staging: lustre: replace kzalloc with copy_from_user with memdup_user
On Mar 31, 2015, at 11:57 AM, gre...@linuxfoundation.org wrote:
On Tue, Mar 31, 2015 at 05:15:23PM +0200, Julia Lawall wrote:
On Tue, 31 Mar 2015, Dhere, Chaitanya (C.) wrote:
This patch replaces kzalloc and copy_from_user with memdup_user call
This change was detected with coccinelle tool
Signed-off-by: Chaitanya Dhere cvija...@visteon.com
---
drivers/staging/lustre/lustre/llite/file.c | 11 +++
1 file changed, 3 insertions(+), 8 deletions(-)
diff --git a/drivers/staging/lustre/lustre/llite/file.c
b/drivers/staging/lustre/lustre/llite/file.c
index 85e74d1..85b5567 100644
--- a/drivers/staging/lustre/lustre/llite/file.c
+++ b/drivers/staging/lustre/lustre/llite/file.c
@@ -2368,14 +2368,9 @@ ll_file_ioctl(struct file *file, unsigned int cmd,
unsigned long arg)
struct hsm_state_set*hss;
int rc;
- hss = kzalloc(sizeof(*hss), GFP_NOFS);
- if (!hss)
- return -ENOMEM;
-
- if (copy_from_user(hss, (char *)arg, sizeof(*hss))) {
- OBD_FREE_PTR(hss);
- return -EFAULT;
- }
+ hss = memdup_user((char *)arg, sizeof(*hss));
memdup_user will use the flag GFP_KERNEL, ie (__GFP_WAIT | __GFP_IO |
__GFP_FS), rather than the flag GFP_NOFS, ie (__GFP_WAIT | __GFP_IO), that
is specified. I don't know if this is a problem here.
Yes, this is a filesystem, so this can't be changed, as we can't have
the allocation go out and ask for more filesystem accesses in the middle
of trying to do a filesystem access :)
Technically in this place we are not really holding any locks or anything else
of value to cause a deadlock,
so we might be fine here.
More importantly, I totally missed this OBD_ALLOC replacement with kzalloc when
it happened.
In theory all OBD_ALLOC() calls add up all allocated memory in a counter and
then OBD_FREE() calls
subtract freed memory (for a poor man's memory leak detection and tracing).
Now since it's out of match, there should have been tons of very loud warnings
about it, but I don't see
any in my logs and I wonder why.
Julia, I wonder if you happen to have a bunch of other patches to get rid of
the rest of OBD_ALLOC and OBD_FREE stuff by any chance?
Bye,
Oleg--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
