linux-kernel - Re: [PATCH] staging: lustre: fix GFP

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-id: <52DCDBC2.3000504@samsung.com>
Date:	Mon, 20 Jan 2014 09:18:10 +0100
From:	Marek Szyprowski <m.szyprowski@...sung.com>
To:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>
Cc:	linux-kernel@...r.kernel.org, devel@...verdev.osuosl.org,
	Andreas Dilger <andreas.dilger@...el.com>,
	Peng Tao <tao.peng@....com>
Subject: Re: [PATCH] staging: lustre: fix GFP_ATOMIC macro usage

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@...sung.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

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/