Re: [PATCH] staging: lustre: fix GFP_ATOMIC macro usage

[Marek Szyprowski <m.szyprowski@samsung.com>]

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
-- 
Marek Szyprowski, PhD
Samsung R&D Institute Poland