| Message ID | 1389948416-26390-1-git-send-email-m.szyprowski@samsung.com |
|---|---|
| State | Accepted |
| Commit | 1ebf5b72dc8bfbe5e7866ee1a2eb935fc7d050e6 |
| Headers | show |
Hello, On 2014-01-17 15:33, Greg Kroah-Hartman wrote: > On Fri, Jan 17, 2014 at 09:46:56AM +0100, Marek Szyprowski wrote: > > GFP_ATOMIC is not a single gfp flag, but a macro which expands to the other > > flags and LACK of __GFP_WAIT flag. To check if caller wanted to perform an > > atomic allocation, the code must test __GFP_WAIT flag presence. > > > > Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> > > --- > > .../lustre/include/linux/libcfs/libcfs_private.h | 2 +- > > 1 file changed, 1 insertion(+), 1 deletion(-) > > > > diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h > > index d0d942c..dddccca1 100644 > > --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h > > +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h > > @@ -120,7 +120,7 @@ do { \ > > do { \ > > LASSERT(!in_interrupt() || \ > > ((size) <= LIBCFS_VMALLOC_SIZE && \ > > - ((mask) & GFP_ATOMIC)) != 0); \ > > + ((mask) & __GFP_WAIT) == 0)); \ > > } while (0) > > What a horrible assert, can't we just remove this entirely? > in_interrupt() usually should never be checked, if so, the code is doing > something wrong. And __GFP flags shouldn't be used on their own. Well, I've prepared this patch when I was checking kernel sources for incorrect GFP_ATOMIC usage. I agree that drivers should use generic memory allocation methods instead of inventing their own stuff. Feel free to ignore my patch in favor of removing this custom allocator at all. Best regards
diff --git a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h index d0d942c..dddccca1 100644 --- a/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h +++ b/drivers/staging/lustre/include/linux/libcfs/libcfs_private.h @@ -120,7 +120,7 @@ do { \ do { \ LASSERT(!in_interrupt() || \ ((size) <= LIBCFS_VMALLOC_SIZE && \ - ((mask) & GFP_ATOMIC)) != 0); \ + ((mask) & __GFP_WAIT) == 0)); \ } while (0) #define LIBCFS_ALLOC_POST(ptr, size) \
GFP_ATOMIC is not a single gfp flag, but a macro which expands to the other flags and LACK of __GFP_WAIT flag. To check if caller wanted to perform an atomic allocation, the code must test __GFP_WAIT flag presence. Signed-off-by: Marek Szyprowski <m.szyprowski@samsung.com> --- .../lustre/include/linux/libcfs/libcfs_private.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)