| Message ID | 20170803081152.GC12521@dhcp22.suse.cz (mailing list archive) |
|---|---|
| State | Accepted |
| Headers | show |
On Thu, Aug 03, 2017 at 10:11:52AM +0200, Michal Hocko wrote: > > The GFP_ATOMIC|__GFP_NOMEMALLOC use in SELinux appears to be limited > > to security/selinux/avc.c, and digging a bit, I'm guessing commit > > fa1aa143ac4a copied the combination from 6290c2c43973 ("selinux: tag > > avc cache alloc as non-critical") and the avc_alloc_node() function. > > Thanks for the pointer. That makes much more sense now. Back in 2012 we > really didn't have a good way to distinguish non sleeping and atomic > with reserves allocations. > Yes, and GFP_NOWAIT is the right way to express that now. It'll still wake kswapd but it won't stall on direct reclaim and won't dip into reserves. Thanks Michal. > --- > From 6d506a75da83c0724ed399966971711f37d67411 Mon Sep 17 00:00:00 2001 > From: Michal Hocko <mhocko@suse.com> > Date: Thu, 3 Aug 2017 10:04:20 +0200 > Subject: [PATCH] selinux: replace GFP_ATOMIC | __GFP_NOMEMALLOC with > GFP_NOWAIT > > selinux avc code uses a weird combination of gfp flags. It asks for > GFP_ATOMIC which allows access to memory reserves while it requests > to not consume memory reserves by __GFP_NOMEMALLOC. This seems to be > copying the pattern from 6290c2c43973 ("selinux: tag avc cache alloc as > non-critical"). > > Back then (before d0164adc89f6 ("mm, page_alloc: distinguish between > being unable to sleep, unwilling to sleep and avoiding waking kswapd")) > we didn't have a good way to distinguish nowait and atomic allocations > so the construct made some sense. Now we do not have to play tricks > though and GFP_NOWAIT will provide the required semantic (aka do not > sleep and do not consume any memory reserves). > > Signed-off-by: Michal Hocko <mhocko@suse.com> Looks right to me Acked-by: Mel Gorman <mgorman@suse.de>
On 2017/08/03 17:11, Michal Hocko wrote: > [CC Mel] > > On Wed 02-08-17 17:45:56, Paul Moore wrote: >> On Wed, Aug 2, 2017 at 6:50 AM, Michal Hocko <mhocko@kernel.org> wrote: >>> Hi, >>> while doing something completely unrelated to selinux I've noticed a >>> really strange __GFP_NOMEMALLOC usage pattern in selinux, especially >>> GFP_ATOMIC | __GFP_NOMEMALLOC doesn't make much sense to me. GFP_ATOMIC >>> on its own allows to access memory reserves while the later flag tells >>> we cannot use memory reserves at all. The primary usecase for >>> __GFP_NOMEMALLOC is to override a global PF_MEMALLOC should there be a >>> need. >>> >>> It all leads to fa1aa143ac4a ("selinux: extended permissions for >>> ioctls") which doesn't explain this aspect so let me ask. Why is the >>> flag used at all? Moreover shouldn't GFP_ATOMIC be actually GFP_NOWAIT. >>> What makes this path important to access memory reserves? >> >> [NOTE: added the SELinux list to the CC line, please include that list >> when asking SELinux questions] > > Sorry about that. Will keep it in mind for next posts > >> The GFP_ATOMIC|__GFP_NOMEMALLOC use in SELinux appears to be limited >> to security/selinux/avc.c, and digging a bit, I'm guessing commit >> fa1aa143ac4a copied the combination from 6290c2c43973 ("selinux: tag >> avc cache alloc as non-critical") and the avc_alloc_node() function. > > Thanks for the pointer. That makes much more sense now. Back in 2012 we > really didn't have a good way to distinguish non sleeping and atomic > with reserves allocations. > >> I can't say that I'm an expert at the vm subsystem and the variety of >> different GFP_* flags, but your suggestion of moving to GFP_NOWAIT in >> security/selinux/avc.c seems reasonable and in keeping with the idea >> behind commit 6290c2c43973. > > What do you think about the following? I haven't tested it but it should > be rather straightforward. Why not at least __GFP_NOWARN ? And why not also __GFP_NOMEMALLOC ? http://lkml.kernel.org/r/201706302210.GCA05089.MFFOtQVJSOLHOF@I-love.SAKURA.ne.jp
On Thu 03-08-17 19:02:57, Tetsuo Handa wrote: > On 2017/08/03 17:11, Michal Hocko wrote: > > [CC Mel] > > > > On Wed 02-08-17 17:45:56, Paul Moore wrote: > >> On Wed, Aug 2, 2017 at 6:50 AM, Michal Hocko <mhocko@kernel.org> wrote: > >>> Hi, > >>> while doing something completely unrelated to selinux I've noticed a > >>> really strange __GFP_NOMEMALLOC usage pattern in selinux, especially > >>> GFP_ATOMIC | __GFP_NOMEMALLOC doesn't make much sense to me. GFP_ATOMIC > >>> on its own allows to access memory reserves while the later flag tells > >>> we cannot use memory reserves at all. The primary usecase for > >>> __GFP_NOMEMALLOC is to override a global PF_MEMALLOC should there be a > >>> need. > >>> > >>> It all leads to fa1aa143ac4a ("selinux: extended permissions for > >>> ioctls") which doesn't explain this aspect so let me ask. Why is the > >>> flag used at all? Moreover shouldn't GFP_ATOMIC be actually GFP_NOWAIT. > >>> What makes this path important to access memory reserves? > >> > >> [NOTE: added the SELinux list to the CC line, please include that list > >> when asking SELinux questions] > > > > Sorry about that. Will keep it in mind for next posts > > > >> The GFP_ATOMIC|__GFP_NOMEMALLOC use in SELinux appears to be limited > >> to security/selinux/avc.c, and digging a bit, I'm guessing commit > >> fa1aa143ac4a copied the combination from 6290c2c43973 ("selinux: tag > >> avc cache alloc as non-critical") and the avc_alloc_node() function. > > > > Thanks for the pointer. That makes much more sense now. Back in 2012 we > > really didn't have a good way to distinguish non sleeping and atomic > > with reserves allocations. > > > >> I can't say that I'm an expert at the vm subsystem and the variety of > >> different GFP_* flags, but your suggestion of moving to GFP_NOWAIT in > >> security/selinux/avc.c seems reasonable and in keeping with the idea > >> behind commit 6290c2c43973. > > > > What do you think about the following? I haven't tested it but it should > > be rather straightforward. > > Why not at least __GFP_NOWARN ? This would require an additional justification. > And why not also __GFP_NOMEMALLOC ? What would be the purpose of __GFP_NOMEMALLOC? In other words which context would set PF_NOMEMALLOC so that the flag would override it?
Michal Hocko wrote: > On Thu 03-08-17 19:02:57, Tetsuo Handa wrote: > > On 2017/08/03 17:11, Michal Hocko wrote: > > > [CC Mel] > > > > > > On Wed 02-08-17 17:45:56, Paul Moore wrote: > > >> On Wed, Aug 2, 2017 at 6:50 AM, Michal Hocko <mhocko@kernel.org> wrote: > > >>> Hi, > > >>> while doing something completely unrelated to selinux I've noticed a > > >>> really strange __GFP_NOMEMALLOC usage pattern in selinux, especially > > >>> GFP_ATOMIC | __GFP_NOMEMALLOC doesn't make much sense to me. GFP_ATOMIC > > >>> on its own allows to access memory reserves while the later flag tells > > >>> we cannot use memory reserves at all. The primary usecase for > > >>> __GFP_NOMEMALLOC is to override a global PF_MEMALLOC should there be a > > >>> need. > > >>> > > >>> It all leads to fa1aa143ac4a ("selinux: extended permissions for > > >>> ioctls") which doesn't explain this aspect so let me ask. Why is the > > >>> flag used at all? Moreover shouldn't GFP_ATOMIC be actually GFP_NOWAIT. > > >>> What makes this path important to access memory reserves? > > >> > > >> [NOTE: added the SELinux list to the CC line, please include that list > > >> when asking SELinux questions] > > > > > > Sorry about that. Will keep it in mind for next posts > > > > > >> The GFP_ATOMIC|__GFP_NOMEMALLOC use in SELinux appears to be limited > > >> to security/selinux/avc.c, and digging a bit, I'm guessing commit > > >> fa1aa143ac4a copied the combination from 6290c2c43973 ("selinux: tag > > >> avc cache alloc as non-critical") and the avc_alloc_node() function. > > > > > > Thanks for the pointer. That makes much more sense now. Back in 2012 we > > > really didn't have a good way to distinguish non sleeping and atomic > > > with reserves allocations. > > > > > >> I can't say that I'm an expert at the vm subsystem and the variety of > > >> different GFP_* flags, but your suggestion of moving to GFP_NOWAIT in > > >> security/selinux/avc.c seems reasonable and in keeping with the idea > > >> behind commit 6290c2c43973. > > > > > > What do you think about the following? I haven't tested it but it should > > > be rather straightforward. > > > > Why not at least __GFP_NOWARN ? > > This would require an additional justification. If allocation failure is not a problem, printing allocation failure messages is nothing but noisy. > > > And why not also __GFP_NOMEMALLOC ? > > What would be the purpose of __GFP_NOMEMALLOC? In other words which > context would set PF_NOMEMALLOC so that the flag would override it? > When allocating thread is selected as an OOM victim, it gets TIF_MEMDIE. Since that function might be called from !in_interrupt() context, it is possible that gfp_pfmemalloc_allowed() returns true due to TIF_MEMDIE and the OOM victim will dip into memory reserves even when allocation failure is not a problem. Thus, I think that majority of plain GFP_NOWAIT users want to use GFP_NOWAIT | __GFP_NOWARN | __GFP_NOMEMALLOC.
On Thu 03-08-17 19:44:46, Tetsuo Handa wrote: > Michal Hocko wrote: > > On Thu 03-08-17 19:02:57, Tetsuo Handa wrote: > > > On 2017/08/03 17:11, Michal Hocko wrote: > > > > [CC Mel] > > > > > > > > On Wed 02-08-17 17:45:56, Paul Moore wrote: > > > >> On Wed, Aug 2, 2017 at 6:50 AM, Michal Hocko <mhocko@kernel.org> wrote: > > > >>> Hi, > > > >>> while doing something completely unrelated to selinux I've noticed a > > > >>> really strange __GFP_NOMEMALLOC usage pattern in selinux, especially > > > >>> GFP_ATOMIC | __GFP_NOMEMALLOC doesn't make much sense to me. GFP_ATOMIC > > > >>> on its own allows to access memory reserves while the later flag tells > > > >>> we cannot use memory reserves at all. The primary usecase for > > > >>> __GFP_NOMEMALLOC is to override a global PF_MEMALLOC should there be a > > > >>> need. > > > >>> > > > >>> It all leads to fa1aa143ac4a ("selinux: extended permissions for > > > >>> ioctls") which doesn't explain this aspect so let me ask. Why is the > > > >>> flag used at all? Moreover shouldn't GFP_ATOMIC be actually GFP_NOWAIT. > > > >>> What makes this path important to access memory reserves? > > > >> > > > >> [NOTE: added the SELinux list to the CC line, please include that list > > > >> when asking SELinux questions] > > > > > > > > Sorry about that. Will keep it in mind for next posts > > > > > > > >> The GFP_ATOMIC|__GFP_NOMEMALLOC use in SELinux appears to be limited > > > >> to security/selinux/avc.c, and digging a bit, I'm guessing commit > > > >> fa1aa143ac4a copied the combination from 6290c2c43973 ("selinux: tag > > > >> avc cache alloc as non-critical") and the avc_alloc_node() function. > > > > > > > > Thanks for the pointer. That makes much more sense now. Back in 2012 we > > > > really didn't have a good way to distinguish non sleeping and atomic > > > > with reserves allocations. > > > > > > > >> I can't say that I'm an expert at the vm subsystem and the variety of > > > >> different GFP_* flags, but your suggestion of moving to GFP_NOWAIT in > > > >> security/selinux/avc.c seems reasonable and in keeping with the idea > > > >> behind commit 6290c2c43973. > > > > > > > > What do you think about the following? I haven't tested it but it should > > > > be rather straightforward. > > > > > > Why not at least __GFP_NOWARN ? > > > > This would require an additional justification. > > If allocation failure is not a problem, printing allocation failure messages > is nothing but noisy. That alone is not a sufficient justification. An allocation warning might still tell you that something is not configured properly. Note that I am not objecting that __GFP_NOWARN is wrong it should just not be added blindly withtout deep understanding of the code which I do not have. > > > And why not also __GFP_NOMEMALLOC ? > > > > What would be the purpose of __GFP_NOMEMALLOC? In other words which > > context would set PF_NOMEMALLOC so that the flag would override it? > > > > When allocating thread is selected as an OOM victim, it gets TIF_MEMDIE. > Since that function might be called from !in_interrupt() context, it is > possible that gfp_pfmemalloc_allowed() returns true due to TIF_MEMDIE and > the OOM victim will dip into memory reserves even when allocation failure > is not a problem. Yes this is possible but I do not see any major problem with that. I wouldn't add __GFP_NOMEMALLOC unless there is a real runaway of some sort that could be abused. > Thus, I think that majority of plain GFP_NOWAIT users want to use > GFP_NOWAIT | __GFP_NOWARN | __GFP_NOMEMALLOC.
On Thu, Aug 3, 2017 at 7:05 AM, Michal Hocko <mhocko@kernel.org> wrote: > On Thu 03-08-17 19:44:46, Tetsuo Handa wrote: >> Michal Hocko wrote: >> > On Thu 03-08-17 19:02:57, Tetsuo Handa wrote: >> > > On 2017/08/03 17:11, Michal Hocko wrote: >> > > > [CC Mel] >> > > > >> > > > On Wed 02-08-17 17:45:56, Paul Moore wrote: >> > > >> On Wed, Aug 2, 2017 at 6:50 AM, Michal Hocko <mhocko@kernel.org> wrote: >> > > >>> Hi, >> > > >>> while doing something completely unrelated to selinux I've noticed a >> > > >>> really strange __GFP_NOMEMALLOC usage pattern in selinux, especially >> > > >>> GFP_ATOMIC | __GFP_NOMEMALLOC doesn't make much sense to me. GFP_ATOMIC >> > > >>> on its own allows to access memory reserves while the later flag tells >> > > >>> we cannot use memory reserves at all. The primary usecase for >> > > >>> __GFP_NOMEMALLOC is to override a global PF_MEMALLOC should there be a >> > > >>> need. >> > > >>> >> > > >>> It all leads to fa1aa143ac4a ("selinux: extended permissions for >> > > >>> ioctls") which doesn't explain this aspect so let me ask. Why is the >> > > >>> flag used at all? Moreover shouldn't GFP_ATOMIC be actually GFP_NOWAIT. >> > > >>> What makes this path important to access memory reserves? >> > > >> >> > > >> [NOTE: added the SELinux list to the CC line, please include that list >> > > >> when asking SELinux questions] >> > > > >> > > > Sorry about that. Will keep it in mind for next posts >> > > > >> > > >> The GFP_ATOMIC|__GFP_NOMEMALLOC use in SELinux appears to be limited >> > > >> to security/selinux/avc.c, and digging a bit, I'm guessing commit >> > > >> fa1aa143ac4a copied the combination from 6290c2c43973 ("selinux: tag >> > > >> avc cache alloc as non-critical") and the avc_alloc_node() function. >> > > > >> > > > Thanks for the pointer. That makes much more sense now. Back in 2012 we >> > > > really didn't have a good way to distinguish non sleeping and atomic >> > > > with reserves allocations. >> > > > >> > > >> I can't say that I'm an expert at the vm subsystem and the variety of >> > > >> different GFP_* flags, but your suggestion of moving to GFP_NOWAIT in >> > > >> security/selinux/avc.c seems reasonable and in keeping with the idea >> > > >> behind commit 6290c2c43973. >> > > > >> > > > What do you think about the following? I haven't tested it but it should >> > > > be rather straightforward. >> > > >> > > Why not at least __GFP_NOWARN ? >> > >> > This would require an additional justification. >> >> If allocation failure is not a problem, printing allocation failure messages >> is nothing but noisy. > > That alone is not a sufficient justification. An allocation warning > might still tell you that something is not configured properly. Note > that I am not objecting that __GFP_NOWARN is wrong it should just not be > added blindly withtout deep understanding of the code which I do not > have. I understand the concern about noise from failed memory allocations, but I tend to agree with the argument that notification of these failures could be useful to admins/devs who are trying to diagnose problems; let's *not* use __GFP_NOWARN in the SELinux AVC code. >> > > And why not also __GFP_NOMEMALLOC ? >> > >> > What would be the purpose of __GFP_NOMEMALLOC? In other words which >> > context would set PF_NOMEMALLOC so that the flag would override it? >> > >> >> When allocating thread is selected as an OOM victim, it gets TIF_MEMDIE. >> Since that function might be called from !in_interrupt() context, it is >> possible that gfp_pfmemalloc_allowed() returns true due to TIF_MEMDIE and >> the OOM victim will dip into memory reserves even when allocation failure >> is not a problem. > > Yes this is possible but I do not see any major problem with that. > I wouldn't add __GFP_NOMEMALLOC unless there is a real runaway of some > sort that could be abused. Adding __GFP_NOMEMALLOC would not hurt anything would it? >> Thus, I think that majority of plain GFP_NOWAIT users want to use >> GFP_NOWAIT | __GFP_NOWARN | __GFP_NOMEMALLOC.
On Thu 03-08-17 14:17:26, Paul Moore wrote: > On Thu, Aug 3, 2017 at 7:05 AM, Michal Hocko <mhocko@kernel.org> wrote: > > On Thu 03-08-17 19:44:46, Tetsuo Handa wrote: [...] > >> When allocating thread is selected as an OOM victim, it gets TIF_MEMDIE. > >> Since that function might be called from !in_interrupt() context, it is > >> possible that gfp_pfmemalloc_allowed() returns true due to TIF_MEMDIE and > >> the OOM victim will dip into memory reserves even when allocation failure > >> is not a problem. > > > > Yes this is possible but I do not see any major problem with that. > > I wouldn't add __GFP_NOMEMALLOC unless there is a real runaway of some > > sort that could be abused. > > Adding __GFP_NOMEMALLOC would not hurt anything would it? I is not harmfull but I fail to see how it would be useful either and as such it just adds a pointless gfp flag and confusion to whoever tries to modify the code in future. Really the main purpose of __GFP_NOMEMALLOC is to override the process scope PF_MEMALLOC. As such it is quite a hack and the fewer users we have the better. Btw. Should I resend the patch or somebody will take it from this email thread?
On Fri, Aug 4, 2017 at 3:56 AM, Michal Hocko <mhocko@kernel.org> wrote: > On Thu 03-08-17 14:17:26, Paul Moore wrote: >> On Thu, Aug 3, 2017 at 7:05 AM, Michal Hocko <mhocko@kernel.org> wrote: >> > On Thu 03-08-17 19:44:46, Tetsuo Handa wrote: > [...] >> >> When allocating thread is selected as an OOM victim, it gets TIF_MEMDIE. >> >> Since that function might be called from !in_interrupt() context, it is >> >> possible that gfp_pfmemalloc_allowed() returns true due to TIF_MEMDIE and >> >> the OOM victim will dip into memory reserves even when allocation failure >> >> is not a problem. >> > >> > Yes this is possible but I do not see any major problem with that. >> > I wouldn't add __GFP_NOMEMALLOC unless there is a real runaway of some >> > sort that could be abused. >> >> Adding __GFP_NOMEMALLOC would not hurt anything would it? > > I is not harmfull but I fail to see how it would be useful either and as > such it just adds a pointless gfp flag and confusion to whoever tries to > modify the code in future. Really the main purpose of __GFP_NOMEMALLOC > is to override the process scope PF_MEMALLOC. As such it is quite a hack > and the fewer users we have the better. Okay, that is a viable explanation for me. > Btw. Should I resend the patch or somebody will take it from this email > thread? No, unless your mailer mangled the patch I should be able to pull it from this thread. However, I'm probably going to let this sit until early next week on the odd chance that anyone else wants to comment on the flag choice. I'll send another reply once I merge the patch.
On Fri 04-08-17 13:12:04, Paul Moore wrote: > On Fri, Aug 4, 2017 at 3:56 AM, Michal Hocko <mhocko@kernel.org> wrote: [...] > > Btw. Should I resend the patch or somebody will take it from this email > > thread? > > No, unless your mailer mangled the patch I should be able to pull it > from this thread. However, I'm probably going to let this sit until > early next week on the odd chance that anyone else wants to comment on > the flag choice. I'll send another reply once I merge the patch. OK, there is certainly no hurry for merging this. Thanks!
On Mon, Aug 7, 2017 at 2:58 AM, Michal Hocko <mhocko@kernel.org> wrote: > On Fri 04-08-17 13:12:04, Paul Moore wrote: >> On Fri, Aug 4, 2017 at 3:56 AM, Michal Hocko <mhocko@kernel.org> wrote: > [...] >> > Btw. Should I resend the patch or somebody will take it from this email >> > thread? >> >> No, unless your mailer mangled the patch I should be able to pull it >> from this thread. However, I'm probably going to let this sit until >> early next week on the odd chance that anyone else wants to comment on >> the flag choice. I'll send another reply once I merge the patch. > > OK, there is certainly no hurry for merging this. Thanks! > -- > Michal Hocko > SUSE Labs Merged into selinux/next with this patch description, and your sign-off (I had to munge the description a bit based on the thread). Are you okay with this, especially your sign-off? commit 476accbe2f6ef69caeebe99f52a286e12ac35aee Author: Michal Hocko <mhocko@kernel.org> Date: Thu Aug 3 10:11:52 2017 +0200 selinux: use GFP_NOWAIT in the AVC kmem_caches There is a strange __GFP_NOMEMALLOC usage pattern in SELinux, specifically GFP_ATOMIC | __GFP_NOMEMALLOC which doesn't make much sense. GFP_ATOMIC on its own allows to access memory reserves while __GFP_NOMEMALLOC dictates we cannot use memory reserves. Replace this with the much more sane GFP_NOWAIT in the AVC code as we can tolerate memory allocation failures in that code. Signed-off-by: Michal Hocko <mhocko@kernel.org> Acked-by: Mel Gorman <mgorman@suse.de> Signed-off-by: Paul Moore <paul@paul-moore.com>
On Tue 08-08-17 09:34:15, Paul Moore wrote: > On Mon, Aug 7, 2017 at 2:58 AM, Michal Hocko <mhocko@kernel.org> wrote: > > On Fri 04-08-17 13:12:04, Paul Moore wrote: > >> On Fri, Aug 4, 2017 at 3:56 AM, Michal Hocko <mhocko@kernel.org> wrote: > > [...] > >> > Btw. Should I resend the patch or somebody will take it from this email > >> > thread? > >> > >> No, unless your mailer mangled the patch I should be able to pull it > >> from this thread. However, I'm probably going to let this sit until > >> early next week on the odd chance that anyone else wants to comment on > >> the flag choice. I'll send another reply once I merge the patch. > > > > OK, there is certainly no hurry for merging this. Thanks! > > -- > > Michal Hocko > > SUSE Labs > > Merged into selinux/next with this patch description, and your > sign-off (I had to munge the description a bit based on the thread). > Are you okay with this, especially your sign-off? Yes. Thanks! > > commit 476accbe2f6ef69caeebe99f52a286e12ac35aee > Author: Michal Hocko <mhocko@kernel.org> > Date: Thu Aug 3 10:11:52 2017 +0200 > > selinux: use GFP_NOWAIT in the AVC kmem_caches > > There is a strange __GFP_NOMEMALLOC usage pattern in SELinux, > specifically GFP_ATOMIC | __GFP_NOMEMALLOC which doesn't make much > sense. GFP_ATOMIC on its own allows to access memory reserves while > __GFP_NOMEMALLOC dictates we cannot use memory reserves. Replace this > with the much more sane GFP_NOWAIT in the AVC code as we can tolerate > memory allocation failures in that code. > > Signed-off-by: Michal Hocko <mhocko@kernel.org> > Acked-by: Mel Gorman <mgorman@suse.de> > Signed-off-by: Paul Moore <paul@paul-moore.com> > > -- > paul moore > www.paul-moore.com
On Thu, Aug 10, 2017 at 3:02 AM, Michal Hocko <mhocko@kernel.org> wrote: > On Tue 08-08-17 09:34:15, Paul Moore wrote: >> On Mon, Aug 7, 2017 at 2:58 AM, Michal Hocko <mhocko@kernel.org> wrote: >> > On Fri 04-08-17 13:12:04, Paul Moore wrote: >> >> On Fri, Aug 4, 2017 at 3:56 AM, Michal Hocko <mhocko@kernel.org> wrote: >> > [...] >> >> > Btw. Should I resend the patch or somebody will take it from this email >> >> > thread? >> >> >> >> No, unless your mailer mangled the patch I should be able to pull it >> >> from this thread. However, I'm probably going to let this sit until >> >> early next week on the odd chance that anyone else wants to comment on >> >> the flag choice. I'll send another reply once I merge the patch. >> > >> > OK, there is certainly no hurry for merging this. Thanks! >> > -- >> > Michal Hocko >> > SUSE Labs >> >> Merged into selinux/next with this patch description, and your >> sign-off (I had to munge the description a bit based on the thread). >> Are you okay with this, especially your sign-off? > > Yes. Thanks! Great, thanks for the confirmation. I'll send this up during the next merge window.
diff --git a/security/selinux/avc.c b/security/selinux/avc.c index 4b4293194aee..b2c159e832b6 100644 --- a/security/selinux/avc.c +++ b/security/selinux/avc.c @@ -346,27 +346,26 @@ static struct avc_xperms_decision_node struct avc_xperms_decision_node *xpd_node; struct extended_perms_decision *xpd; - xpd_node = kmem_cache_zalloc(avc_xperms_decision_cachep, - GFP_ATOMIC | __GFP_NOMEMALLOC); + xpd_node = kmem_cache_zalloc(avc_xperms_decision_cachep, GFP_NOWAIT); if (!xpd_node) return NULL; xpd = &xpd_node->xpd; if (which & XPERMS_ALLOWED) { xpd->allowed = kmem_cache_zalloc(avc_xperms_data_cachep, - GFP_ATOMIC | __GFP_NOMEMALLOC); + GFP_NOWAIT); if (!xpd->allowed) goto error; } if (which & XPERMS_AUDITALLOW) { xpd->auditallow = kmem_cache_zalloc(avc_xperms_data_cachep, - GFP_ATOMIC | __GFP_NOMEMALLOC); + GFP_NOWAIT); if (!xpd->auditallow) goto error; } if (which & XPERMS_DONTAUDIT) { xpd->dontaudit = kmem_cache_zalloc(avc_xperms_data_cachep, - GFP_ATOMIC | __GFP_NOMEMALLOC); + GFP_NOWAIT); if (!xpd->dontaudit) goto error; } @@ -394,8 +393,7 @@ static struct avc_xperms_node *avc_xperms_alloc(void) { struct avc_xperms_node *xp_node; - xp_node = kmem_cache_zalloc(avc_xperms_cachep, - GFP_ATOMIC|__GFP_NOMEMALLOC); + xp_node = kmem_cache_zalloc(avc_xperms_cachep, GFP_NOWAIT); if (!xp_node) return xp_node; INIT_LIST_HEAD(&xp_node->xpd_head); @@ -548,7 +546,7 @@ static struct avc_node *avc_alloc_node(void) { struct avc_node *node; - node = kmem_cache_zalloc(avc_node_cachep, GFP_ATOMIC|__GFP_NOMEMALLOC); + node = kmem_cache_zalloc(avc_node_cachep, GFP_NOWAIT); if (!node) goto out;