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

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

[Ming Lei]

This patch introduces PF_MEMALLOC_NOIO on process flag('flags' field of
'struct task_struct'), so that the flag can be set by one task
to avoid doing I/O inside memory allocation in the task's context.

The patch trys to solve one deadlock problem caused by block device,
and the problem may happen at least in the below situations:

- during block device runtime resume, if memory allocation with
GFP_KERNEL is called inside runtime resume callback of any one
of its ancestors(or the block device itself), the deadlock may be
triggered inside the memory allocation since it might not complete
until the block device becomes active and the involed page I/O finishes.
The situation is pointed out first by Alan Stern. It is not a good
approach to convert all GFP_KERNEL in the path into GFP_NOIO because
several subsystems may be involved(for example, PCI, USB and SCSI may
be involved for usb mass stoarage device)

- during error handling of usb mass storage deivce, USB bus reset
will be put on the device, so there shouldn't have any
memory allocation with GFP_KERNEL during USB bus reset, otherwise
the deadlock similar with above may be triggered. Unfortunately, any
usb device may include one mass storage interface in theory, so it
requires all usb interface drivers to handle the situation. In fact,
most usb drivers don't know how to handle bus reset on the device
and don't provide .pre_set() and .post_reset() callback at all, so
USB core has to unbind and bind driver for these devices. So it
is still not practical to resort to GFP_NOIO for solving the problem.

Also the introduced solution can be used by block subsystem or block
drivers too, for example, set the PF_MEMALLOC_NOIO flag before doing
actual I/O transfer.

Cc: Alan Stern <stern@rowland.harvard.edu>
Cc: Oliver Neukum <oneukum@suse.de>
Cc: Jiri Kosina <jiri.kosina@suse.com>
Cc: Andrew Morton <akpm@linux-foundation.org>
Cc: Mel Gorman <mel@csn.ul.ie>
Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
Cc: Michal Hocko <mhocko@suse.cz>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
Cc: linux-mm <linux-mm@kvack.org>
Signed-off-by: Minchan Kim <minchan@kernel.org>
Signed-off-by: Ming Lei <ming.lei@canonical.com>
---
 include/linux/sched.h |   11 +++++++++++
 mm/page_alloc.c       |   10 +++++++++-
 mm/vmscan.c           |   13 +++++++++++++
 3 files changed, 33 insertions(+), 1 deletion(-)

diff --git a/include/linux/sched.h b/include/linux/sched.h
index f6961c9..c149ae7 100644
--- a/include/linux/sched.h
+++ b/include/linux/sched.h
@@ -1811,6 +1811,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
 #define PF_FROZEN	0x00010000	/* frozen for system suspend */
 #define PF_FSTRANS	0x00020000	/* inside a filesystem transaction */
 #define PF_KSWAPD	0x00040000	/* I am kswapd */
+#define PF_MEMALLOC_NOIO 0x00080000	/* Allocating memory without IO involved */
 #define PF_LESS_THROTTLE 0x00100000	/* Throttle me less: I clean memory */
 #define PF_KTHREAD	0x00200000	/* I am a kernel thread */
 #define PF_RANDOMIZE	0x00400000	/* randomize virtual address space */
@@ -1848,6 +1849,16 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
 #define tsk_used_math(p) ((p)->flags & PF_USED_MATH)
 #define used_math() tsk_used_math(current)
 
+#define memalloc_noio() (current->flags & PF_MEMALLOC_NOIO)
+#define memalloc_noio_save(noio_flag) do { \
+	(noio_flag) = current->flags & PF_MEMALLOC_NOIO; \
+	current->flags |= PF_MEMALLOC_NOIO; \
+} while (0)
+#define memalloc_noio_restore(noio_flag) do { \
+	if (!(noio_flag)) \
+		current->flags &= ~PF_MEMALLOC_NOIO; \
+} while (0)
+
 /*
  * task->jobctl flags
  */
diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index 8e1be1c..e3746dd 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -2630,10 +2630,18 @@ retry_cpuset:
 	page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
 			zonelist, high_zoneidx, alloc_flags,
 			preferred_zone, migratetype);
-	if (unlikely(!page))
+	if (unlikely(!page)) {
+		/*
+		 * Resume, block IO and its error handling path
+		 * can deadlock because I/O on the device might not
+		 * complete.
+		 */
+		if (unlikely(memalloc_noio()))
+			gfp_mask &= ~GFP_IOFS;
 		page = __alloc_pages_slowpath(gfp_mask, order,
 				zonelist, high_zoneidx, nodemask,
 				preferred_zone, migratetype);
+	}
 
 	trace_mm_page_alloc(page, order, gfp_mask, migratetype);
 
diff --git a/mm/vmscan.c b/mm/vmscan.c
index 1e9aa66..6647805 100644
--- a/mm/vmscan.c
+++ b/mm/vmscan.c
@@ -3298,6 +3298,19 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
 	};
 	unsigned long nr_slab_pages0, nr_slab_pages1;
 
+	if (unlikely(memalloc_noio())) {
+		sc.gfp_mask &= ~GFP_IOFS;
+		shrink.gfp_mask = sc.gfp_mask;
+		/*
+		 * We allow to reclaim only clean pages.
+		 * It can affect RECLAIM_SWAP and RECLAIM_WRITE mode
+		 * but this is really rare event and allocator can
+		 * fallback to other zones.
+		 */
+		sc.may_writepage = 0;
+		sc.may_swap = 0;
+	}
+
 	cond_resched();
 	/*
 	 * We need to be able to allocate from the reserves for RECLAIM_SWAP
-- 
1.7.9.5

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Andrew Morton]

On Tue, 16 Oct 2012 23:59:41 +0800
Ming Lei <ming.lei@canonical.com> wrote:

> This patch introduces PF_MEMALLOC_NOIO on process flag('flags' field of
> 'struct task_struct'), so that the flag can be set by one task
> to avoid doing I/O inside memory allocation in the task's context.
> 
> The patch trys to solve one deadlock problem caused by block device,
> and the problem may happen at least in the below situations:
> 
> - during block device runtime resume, if memory allocation with
> GFP_KERNEL is called inside runtime resume callback of any one
> of its ancestors(or the block device itself), the deadlock may be
> triggered inside the memory allocation since it might not complete
> until the block device becomes active and the involed page I/O finishes.
> The situation is pointed out first by Alan Stern. It is not a good
> approach to convert all GFP_KERNEL in the path into GFP_NOIO because
> several subsystems may be involved(for example, PCI, USB and SCSI may
> be involved for usb mass stoarage device)
> 
> - during error handling of usb mass storage deivce, USB bus reset
> will be put on the device, so there shouldn't have any
> memory allocation with GFP_KERNEL during USB bus reset, otherwise
> the deadlock similar with above may be triggered. Unfortunately, any
> usb device may include one mass storage interface in theory, so it
> requires all usb interface drivers to handle the situation. In fact,
> most usb drivers don't know how to handle bus reset on the device
> and don't provide .pre_set() and .post_reset() callback at all, so
> USB core has to unbind and bind driver for these devices. So it
> is still not practical to resort to GFP_NOIO for solving the problem.
> 
> Also the introduced solution can be used by block subsystem or block
> drivers too, for example, set the PF_MEMALLOC_NOIO flag before doing
> actual I/O transfer.

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.

> @@ -1848,6 +1849,16 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
>  #define tsk_used_math(p) ((p)->flags & PF_USED_MATH)
>  #define used_math() tsk_used_math(current)
>  
> +#define memalloc_noio() (current->flags & PF_MEMALLOC_NOIO)
> +#define memalloc_noio_save(noio_flag) do { \
> +	(noio_flag) = current->flags & PF_MEMALLOC_NOIO; \
> +	current->flags |= PF_MEMALLOC_NOIO; \
> +} while (0)
> +#define memalloc_noio_restore(noio_flag) do { \
> +	if (!(noio_flag)) \
> +		current->flags &= ~PF_MEMALLOC_NOIO; \
> +} while (0)

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

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:

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).

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ming Lei]

On Wed, Oct 17, 2012 at 4:19 AM, Andrew Morton
<akpm@linux-foundation.org> 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, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ming Lei]

On Wed, Oct 17, 2012 at 4:19 AM, Andrew Morton
<akpm@linux-foundation.org> 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.

OK, there are two examples of GFP_KERNEL allocation in subsystem
runtime resume path:

1), almost all devices in some pci platform
acpi_os_allocate
	<-acpi_ut_allocate
		<-ACPI_ALLOCATE_ZEROED
			<-acpi_evaluate_object
				<-__acpi_bus_set_power
					<-acpi_bus_set_power
						<-acpi_pci_set_power_state
							<-platform_pci_set_power_state
								<-pci_platform_power_transition
									<-__pci_complete_power_transition
										<-pci_set_power_state
											<-pci_restore_standard_config
												<-pci_pm_runtime_resume

2), all devices in usb subsystem
usb_get_status
	<-finish_port_resume
		<-usb_port_resume
			<-generic_resume
				<-usb_resume_device
					<-usb_resume_both
						<-usb_runtime_resume	

I also have many examples in which GFP_KERNEL allocation
is involved in runtime resume path of individual drivers.

The above two examples just show how difficult to solve the problem
by traditional way, :-)

Also as pointed by Oliver, network driver need memory allocation with
no io in iSCSI runtime resume situation too.

Thanks,
--
Ming Lei

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Kamezawa Hiroyuki]

(2012/10/17 0:59), Ming Lei wrote:
> This patch introduces PF_MEMALLOC_NOIO on process flag('flags' field of
> 'struct task_struct'), so that the flag can be set by one task
> to avoid doing I/O inside memory allocation in the task's context.
> 
> The patch trys to solve one deadlock problem caused by block device,
> and the problem may happen at least in the below situations:
> 
> - during block device runtime resume, if memory allocation with
> GFP_KERNEL is called inside runtime resume callback of any one
> of its ancestors(or the block device itself), the deadlock may be
> triggered inside the memory allocation since it might not complete
> until the block device becomes active and the involed page I/O finishes.
> The situation is pointed out first by Alan Stern. It is not a good
> approach to convert all GFP_KERNEL in the path into GFP_NOIO because
> several subsystems may be involved(for example, PCI, USB and SCSI may
> be involved for usb mass stoarage device)
> 
> - during error handling of usb mass storage deivce, USB bus reset
> will be put on the device, so there shouldn't have any
> memory allocation with GFP_KERNEL during USB bus reset, otherwise
> the deadlock similar with above may be triggered. Unfortunately, any
> usb device may include one mass storage interface in theory, so it
> requires all usb interface drivers to handle the situation. In fact,
> most usb drivers don't know how to handle bus reset on the device
> and don't provide .pre_set() and .post_reset() callback at all, so
> USB core has to unbind and bind driver for these devices. So it
> is still not practical to resort to GFP_NOIO for solving the problem.
> 
> Also the introduced solution can be used by block subsystem or block
> drivers too, for example, set the PF_MEMALLOC_NOIO flag before doing
> actual I/O transfer.
> 
> Cc: Alan Stern <stern@rowland.harvard.edu>
> Cc: Oliver Neukum <oneukum@suse.de>
> Cc: Jiri Kosina <jiri.kosina@suse.com>
> Cc: Andrew Morton <akpm@linux-foundation.org>
> Cc: Mel Gorman <mel@csn.ul.ie>
> Cc: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> Cc: Michal Hocko <mhocko@suse.cz>
> Cc: Ingo Molnar <mingo@redhat.com>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: "Rafael J. Wysocki" <rjw@sisk.pl>
> Cc: linux-mm <linux-mm@kvack.org>
> Signed-off-by: Minchan Kim <minchan@kernel.org>
> Signed-off-by: Ming Lei <ming.lei@canonical.com>
> ---
>   include/linux/sched.h |   11 +++++++++++
>   mm/page_alloc.c       |   10 +++++++++-
>   mm/vmscan.c           |   13 +++++++++++++
>   3 files changed, 33 insertions(+), 1 deletion(-)
> 
> diff --git a/include/linux/sched.h b/include/linux/sched.h
> index f6961c9..c149ae7 100644
> --- a/include/linux/sched.h
> +++ b/include/linux/sched.h
> @@ -1811,6 +1811,7 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
>   #define PF_FROZEN	0x00010000	/* frozen for system suspend */
>   #define PF_FSTRANS	0x00020000	/* inside a filesystem transaction */
>   #define PF_KSWAPD	0x00040000	/* I am kswapd */
> +#define PF_MEMALLOC_NOIO 0x00080000	/* Allocating memory without IO involved */
>   #define PF_LESS_THROTTLE 0x00100000	/* Throttle me less: I clean memory */
>   #define PF_KTHREAD	0x00200000	/* I am a kernel thread */
>   #define PF_RANDOMIZE	0x00400000	/* randomize virtual address space */
> @@ -1848,6 +1849,16 @@ extern void thread_group_times(struct task_struct *p, cputime_t *ut, cputime_t *
>   #define tsk_used_math(p) ((p)->flags & PF_USED_MATH)
>   #define used_math() tsk_used_math(current)
>   
> +#define memalloc_noio() (current->flags & PF_MEMALLOC_NOIO)
> +#define memalloc_noio_save(noio_flag) do { \
> +	(noio_flag) = current->flags & PF_MEMALLOC_NOIO; \
> +	current->flags |= PF_MEMALLOC_NOIO; \
> +} while (0)
> +#define memalloc_noio_restore(noio_flag) do { \
> +	if (!(noio_flag)) \
> +		current->flags &= ~PF_MEMALLOC_NOIO; \
> +} while (0)
> +
>   /*
>    * task->jobctl flags
>    */
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 8e1be1c..e3746dd 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -2630,10 +2630,18 @@ retry_cpuset:
>   	page = get_page_from_freelist(gfp_mask|__GFP_HARDWALL, nodemask, order,
>   			zonelist, high_zoneidx, alloc_flags,
>   			preferred_zone, migratetype);
> -	if (unlikely(!page))
> +	if (unlikely(!page)) {
> +		/*
> +		 * Resume, block IO and its error handling path
> +		 * can deadlock because I/O on the device might not
> +		 * complete.
> +		 */
> +		if (unlikely(memalloc_noio()))
> +			gfp_mask &= ~GFP_IOFS;
>   		page = __alloc_pages_slowpath(gfp_mask, order,
>   				zonelist, high_zoneidx, nodemask,
>   				preferred_zone, migratetype);
> +	}
>   
>   	trace_mm_page_alloc(page, order, gfp_mask, migratetype);
>   
> diff --git a/mm/vmscan.c b/mm/vmscan.c
> index 1e9aa66..6647805 100644
> --- a/mm/vmscan.c
> +++ b/mm/vmscan.c
> @@ -3298,6 +3298,19 @@ static int __zone_reclaim(struct zone *zone, gfp_t gfp_mask, unsigned int order)
>   	};
>   	unsigned long nr_slab_pages0, nr_slab_pages1;
>   
> +	if (unlikely(memalloc_noio())) {
> +		sc.gfp_mask &= ~GFP_IOFS;
> +		shrink.gfp_mask = sc.gfp_mask;
> +		/*
> +		 * We allow to reclaim only clean pages.
> +		 * It can affect RECLAIM_SWAP and RECLAIM_WRITE mode
> +		 * but this is really rare event and allocator can
> +		 * fallback to other zones.
> +		 */
> +		sc.may_writepage = 0;
> +		sc.may_swap = 0;

I think the idea is reasonable. I have a request.

In current implemententation of vmscan.c, it seems sc.may_writepage, sc.may_swap
are handled independent from gfp_mask. 

So, could you drop changes from this patch and handle these flags in another patch
if these flags should be unset if ~GFP_IOFS ?

I think try_to_free_page() path's sc.may_xxxx should be handled in the same way.

Thanks,
-Kame





--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ming Lei]

On Wed, Oct 17, 2012 at 1:14 PM, Kamezawa Hiroyuki
<kamezawa.hiroyu@jp.fujitsu.com> wrote:
>
> I think the idea is reasonable. I have a request.

Thanks for your comment.

>
> In current implemententation of vmscan.c, it seems sc.may_writepage, sc.may_swap
> are handled independent from gfp_mask.
>
> So, could you drop changes from this patch and handle these flags in another patch
> if these flags should be unset if ~GFP_IOFS ?

OK, I agree. In theory,  mm should make sure no I/O is involved if
memory allocation
users passes ~GFP_IOFS.

>
> I think try_to_free_page() path's sc.may_xxxx should be handled in the same way.

Yes, alloc_page_buffers() and dma_alloc_from_contiguous may drop into
the path, so gfp flag should be changed in try_to_free_page() too.


Thanks,
--
Ming Lei

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Andrew Morton]

On Wed, 17 Oct 2012 09:54:09 +0800
Ming Lei <ming.lei@canonical.com> wrote:

> On Wed, Oct 17, 2012 at 4:19 AM, Andrew Morton
> <akpm@linux-foundation.org> 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.

Don't worry about mailing list noise ;)

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

local_irq_save() and local_irq_restore() were mistakes :( It's silly to
write what appears to be a C function and then have it operate like
Pascal (warning: I last wrote some Pascal in 66 B.C.).

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

You could add __must_check to the function definition to ensure that
all callers save its return value.

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>

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

[Ming Lei]

On Thu, Oct 18, 2012 at 7:54 AM, Andrew Morton
<akpm@linux-foundation.org> wrote:
>
> local_irq_save() and local_irq_restore() were mistakes :( It's silly to
> write what appears to be a C function and then have it operate like
> Pascal (warning: I last wrote some Pascal in 66 B.C.).

Considered that spin_lock_irqsave/spin_unlock_irqrestore also follow
the style, kernel guys have been accustomed to the usage, I am
inclined to keep that as macro, :-)

>> 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.
>
> You could add __must_check to the function definition to ensure that
> all callers save its return value.

Yes, we can do that, but the function name is not better than _save
from readability.

Thanks,
--
Ming Lei

--
To unsubscribe, send a message with 'unsubscribe linux-mm' in
the body to majordomo@kvack.org.  For more info on Linux MM,
see: http://www.linux-mm.org/ .
Don't email: <a href=mailto:"dont@kvack.org"> email@kvack.org </a>