Re: kmalloc without GFP_xxx?

[Steven Rostedt <rostedt@goodmis.org>]

On Wed, 29 Jun 2005, Timur Tabi wrote:

> Denis Vlasenko wrote:
>
> > This is why I always use _irqsave. Less error prone.
>
> No, it's just bad programming.  How hard can it be to see which spinlocks are being used
> by your ISR and which ones aren't?  Only the ones that your ISR touches should have
> _irqsave.  It's really quite simple.

What about my argument that spin_lock is actually a longer latency on an
SMP system?  That is, if you grab a spin_lock and task on another CPU
tries to grab it and starts to spin. It will spin while the first task is
servicing interrupts.  It can be even worst with the following scenario:

task 1:
  spin_lock(&non_irq_lock);

task 2:

  spin_lock_irqsave(&some_irq_used_lock);
  spin_lock(&non_irq_lock);

Here we see that task 2 can spin with interrupts off, while the first task
is servicing an interrupt, and God forbid if the IRQ handler sends some
kind of SMP signal to the CPU running task 2 since that would be a
deadlock.  Granted, this is a hypothetical situation, but makes using
spin_lock with interrupts enabled a little scary.


> > This is more or less what I meant. Why think about each kmalloc and when you
> > eventually did get it right: "Aha, we _sometimes_ get called from spinlocked code,
> > GFP_ATOMIC then" - you still do atomic alloc even if cases when you
> > were _not_ called from locked code! Thus you needed to think longer and got
> > code which is worse.
>
> So you're saying that you're the kind of programmer who makes more mistakes the longer you
> think about something?????
>
> Using GFP_ATOMIC increases the probability that you won't be able to allocate the memory
> you need, and it also increases the probability that some other module that really needs
> GFP_ATOMIC will also be unable to allocate the memory it needs.  Please tell me, how is
> this considered good programming?

I believe he was trying to say that there might be a function that is
called by both an interrupt and non interrupt (schedulable) code.  That
means that the code would always need to do a GFP_ATOMIC and yes, it would
mean that there's a higher chance that it would fail.  So if you have some
function that's used by lots of schedulable code and that same function is
seldom used by an interrupt, then you either need to maintain two versions
of the function (one with GFP_ATOMIC and one without) or always use
GFP_ATOMIC which would mean the the majority user would suffer
unsuccessful allocations more often.

-- Steve