Re: [RFC PATCH v1 1/3] mm: teach mm by current context info to not doI/O during memory allocation

From: Ming Lei
Date: Tue Oct 16 2012 - 21:54:02 EST


On Wed, Oct 17, 2012 at 4:19 AM, Andrew Morton
<akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
>
> The patch seems reasonable to me. I'd like to see some examples of
> these resume-time callsite which are performing the GFP_KERNEL
> allocations, please. You have found some kernel bugs, so those should
> be fully described.

There are two examples on 2/3 and 3/3 of the patchset, see below link:

http://marc.info/?l=linux-kernel&m=135040325717213&w=2
http://marc.info/?l=linux-kernel&m=135040327317222&w=2

Sorry for not Cc them to linux-mm because I am afraid of making noise
in mm list.

>
> This is just awful. Why oh why do we write code in macros when we have
> a nice C compiler?

The two helpers are following style of local_irq_save() and
local_irq_restore(), so that people can use them easily, that is
why I define them as macro instead of inline.

>
> These can all be done as nice, clean, type-safe, documented C
> functions. And if they can be done that way, they *should* be done
> that way!
>
> And I suggest that a better name for memalloc_noio_save() is
> memalloc_noio_set(). So this:

IMO, renaming as memalloc_noio_set() might not be better than _save
because the _set name doesn't indicate that the flag should be stored first.

>
> static inline unsigned memalloc_noio(void)
> {
> return current->flags & PF_MEMALLOC_NOIO;
> }
>
> static inline unsigned memalloc_noio_set(unsigned flags)
> {
> unsigned ret = memalloc_noio();
>
> current->flags |= PF_MEMALLOC_NOIO;
> return ret;
> }
>
> static inline unsigned memalloc_noio_restore(unsigned flags)
> {
> current->flags = (current->flags & ~PF_MEMALLOC_NOIO) | flags;
> }
>
> (I think that's correct? It's probably more efficient this way).

Yes, it is correct and more clean, and I will take it.

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