Re: [PATCH net] sctp: use gfp insteaad of GFP_NOWAIT in idr_alloc_cyclic when sctp_assoc_set_id

[Eric Dumazet]

On dim., 2016-03-06 at 13:52 +0800, Xin Long wrote:
> On Sun, Mar 6, 2016 at 12:42 AM, Eric Dumazet <eric.duma...@gmail.com> wrote:
> > On sam., 2016-03-05 at 23:59 +0800, Xin Long wrote:
> >
> > Are you sure idr_alloc(... GFP_KERNEL) makes sense inside spin_lock_bh()
> > section ?
> >
> > idr_alloc() has :
> >
> > might_sleep_if(gfpflags_allow_blocking(gfp_mask));
> >
> > A debug kernel (CONFIG_DEBUG_ATOMIC_SLEEP=y) should probably complain at
> > this point ?
> >
> OK, I got you, does it make sense to you if I just change GFP_NOWAIT
> to GFP_ATOMIC
> when sctp_assoc_set_id call idr_alloc_cyclic()?  which also can avoid
> this call trace in my
> test.
> 
> >
> >

What is the problem of being not able to allocate memory at this point ?

If really it bothers you (although we have thousands of other places it
can happen), maybe add __GFP_NOWARN

But whatever flag we use here, idr_alloc() _can_ fail.

diff --git a/net/sctp/associola.c b/net/sctp/associola.c
index 2bf8ec92dde4..2ae3874e3696 100644
--- a/net/sctp/associola.c
+++ b/net/sctp/associola.c
@@ -1606,7 +1606,8 @@ int sctp_assoc_set_id(struct sctp_association *asoc, 
gfp_t gfp)
                idr_preload(gfp);
        spin_lock_bh(&sctp_assocs_id_lock);
        /* 0 is not a valid assoc_id, must be >= 1 */
-       ret = idr_alloc_cyclic(&sctp_assocs_id, asoc, 1, 0, GFP_NOWAIT);
+       ret = idr_alloc_cyclic(&sctp_assocs_id, asoc, 1, 0,
+                              (gfp & ~__GFP_DIRECT_RECLAIM) | __GFP_NOWARN);
        spin_unlock_bh(&sctp_assocs_id_lock);
        if (preload)
                idr_preload_end();