swap: allowing a more flexible DISCARD policy V2

PATCH Applied Rafael Aquini 4 participants 11 messages May 26, 2013 – Aug 23, 2013

Subsystems (2)

Memory management MM swap

lore

Share & tools

lore

Rafael Aquini

author

2013-05-26 04:32 UTC #0 lore raw

cover swap: allowing a more flexible DISCARD policy V2

Considering the use cases where the swap device supports discard:
a) and can do it quickly;
b) but it's slow to do in small granularities (or concurrent with other I/O);
c) but the implementation is so horrendous that you don't even want to send one down;

And assuming that the sysadmin considers it useful to send the discards down at all, we would (probably) want the following solutions:

i. do the fine-grained discards for freed swap pages, if device is

     capable of doing so optimally;
 ii. do single-time (batched) swap area discards, either at swapon 
     or via something like fstrim (not implemented yet);

iii. allow doing both single-time and fine-grained discards; or
iv. turn it off completely (default behavior)

As implemented today, one can only enable/disable discards for swap, but one cannot select, for instance, solution (ii) on a swap device like (b) even though the single-time discard is regarded to be interesting, or necessary to the workload because it would imply (1), and the device is not capable of performing it optimally.

This patchset addresses the scenario depicted above by introducing a way to ensure the (probably) wanted solutions (i, ii, iii and iv) can be flexibly flagged through swapon(8) to allow a sysadmin to select the best suitable swap discard policy accordingly to system constraints.

Changeset from V1:
01 (kernel) swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES
* ensure backwards compatibility with older swapon(8) releases; (mkosaki)

02 (util-linux) swapon: allow a more flexible swap discard policy
* introduce discard policy selector as an optional argument of --discard option;
* rename user-visible discard policy names; (mkosaki, karel, jmoyer)

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

Rafael Aquini

author

2013-05-26 04:32 UTC #1 lore raw

PATCH 1/2 swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

This patch introduces SWAP_FLAG_DISCARD_PAGES and SWAP_FLAG_DISCARD_ONCE new flags to allow more flexibe swap discard policies being flagged through swapon(8). The default behavior is to keep both single-time, or batched, area discards (SWAP_FLAG_DISCARD_ONCE) and fine-grained discards for page-clusters (SWAP_FLAG_DISCARD_PAGES) enabled, in order to keep consistentcy with older kernel behavior, as well as maintain compatibility with older swapon(8). However, through the new introduced flags the best suitable discard policy can be selected accordingly to any given swap device constraint.

Signed-off-by Rafael Aquini <[email protected]>

---
 include/linux/swap.h | 13 +++++++++----
 mm/swapfile.c        | 55 +++++++++++++++++++++++++++++++++++++++++++++++-----
 2 files changed, 59 insertions(+), 9 deletions(-)

include/linux/swap.h +9 -4

index 1701ce4..33fa21f 100644
@@ -19,10 +19,13 @@ struct bio;
 #define SWAP_FLAG_PREFER	0x8000	/* set if swap priority specified */
 #define SWAP_FLAG_PRIO_MASK	0x7fff
 #define SWAP_FLAG_PRIO_SHIFT	0
-#define SWAP_FLAG_DISCARD	0x10000 /* discard swap cluster after use */
+#define SWAP_FLAG_DISCARD	0x10000 /* enable discard for swap */
+#define SWAP_FLAG_DISCARD_ONCE	0x20000 /* discard swap area at swapon-time */
+#define SWAP_FLAG_DISCARD_PAGES 0x40000 /* discard page-clusters after use */
 
 #define SWAP_FLAGS_VALID	(SWAP_FLAG_PRIO_MASK | SWAP_FLAG_PREFER | \
-				 SWAP_FLAG_DISCARD)
+				 SWAP_FLAG_DISCARD | SWAP_FLAG_DISCARD_ONCE | \
+				 SWAP_FLAG_DISCARD_PAGES)
 
 static inline int current_is_kswapd(void)
 {

@@ -146,14 +149,16 @@ struct swap_extent {
 enum {
 	SWP_USED	= (1 << 0),	/* is slot in swap_info[] used? */
 	SWP_WRITEOK	= (1 << 1),	/* ok to write to this swap?	*/
-	SWP_DISCARDABLE = (1 << 2),	/* swapon+blkdev support discard */
+	SWP_DISCARDABLE = (1 << 2),	/* blkdev support discard */
 	SWP_DISCARDING	= (1 << 3),	/* now discarding a free cluster */
 	SWP_SOLIDSTATE	= (1 << 4),	/* blkdev seeks are cheap */
 	SWP_CONTINUED	= (1 << 5),	/* swap_map has count continuation */
 	SWP_BLKDEV	= (1 << 6),	/* its a block device */
 	SWP_FILE	= (1 << 7),	/* set after swap_activate success */
+	SWP_AREA_DISCARD = (1 << 8),	/* single-time swap area discards */
+	SWP_PAGE_DISCARD = (1 << 9),	/* freed swap page-cluster discards */
 					/* add others here before... */
-	SWP_SCANNING	= (1 << 8),	/* refcount in scan_swap_map */
+	SWP_SCANNING	= (1 << 10),	/* refcount in scan_swap_map */
 };
 
 #define SWAP_CLUSTER_MAX 32UL

mm/swapfile.c +50 -5

89 lines not on this page · raw message

Signature · 7 lines

1.8.1.4

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

KOSAKI Motohiro

2013-05-26 11:45 UTC #2 lore raw

Re: [PATCH 01/02] swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

mm/swapfile.c SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) · line 2160

Quoted 10 lines

mm/swapfile.c +10 -0

+                       /*
+                        * By flagging sys_swapon, a sysadmin can tell us to
+                        * either do sinle-time area discards only, or to just
+                        * perform discards for released swap page-clusters.
+                        * Now it's time to adjust the p->flags accordingly.
+                        */
+                       if (swap_flags & SWAP_FLAG_DISCARD_ONCE)
+                               p->flags &= ~SWP_PAGE_DISCARD;
+                       else if (swap_flags & SWAP_FLAG_DISCARD_PAGES)
+                               p->flags &= ~SWP_AREA_DISCARD;

When using old swapon(8), this code turn off both flags, right?

Rafael Aquini

author

2013-05-26 13:52 UTC #3 lore raw

Re: [PATCH 01/02] swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

Quoted 11 lines · KOSAKI Motohiro

mm/swapfile.c SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) · line 2160

Quoted 10 lines

mm/swapfile.c +10 -0

+                       /*
+                        * By flagging sys_swapon, a sysadmin can tell us to
+                        * either do sinle-time area discards only, or to just
+                        * perform discards for released swap page-clusters.
+                        * Now it's time to adjust the p->flags accordingly.
+                        */
+                       if (swap_flags & SWAP_FLAG_DISCARD_ONCE)
+                               p->flags &= ~SWP_PAGE_DISCARD;
+                       else if (swap_flags & SWAP_FLAG_DISCARD_PAGES)
+                               p->flags &= ~SWP_AREA_DISCARD;

Quoted 1 line

When using old swapon(8), this code turn off both flags, right?

As the flag that enables swap discards SWAP_FLAG_DISCARD remains meaning the same it meant before, when using old swapon(8) (SWP_PAGE_DISCARD|SWP_AREA_DISCARD) will remain flagged when discard is enabled, so we keep doing discards the same way we did before (at swapon, and for every released page-cluster).
The flags are removed orthogonally only when the new swapon(8) selects one of the particular discard policy available by using either SWAP_FLAG_DISCARD_ONCE, or SWAP_FLAG_DISCARD_PAGES flags.

KOSAKI Motohiro

2013-05-26 14:55 UTC #4 lore raw

Re: [PATCH 01/02] swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

Quoted 12 lines · Rafael Aquini

Quoted 11 lines · KOSAKI Motohiro

Quoted 10 lines

mm/swapfile.c +10 -0

+                       /*
+                        * By flagging sys_swapon, a sysadmin can tell us to
+                        * either do sinle-time area discards only, or to just
+                        * perform discards for released swap page-clusters.
+                        * Now it's time to adjust the p->flags accordingly.
+                        */
+                       if (swap_flags & SWAP_FLAG_DISCARD_ONCE)
+                               p->flags &= ~SWP_PAGE_DISCARD;
+                       else if (swap_flags & SWAP_FLAG_DISCARD_PAGES)
+                               p->flags &= ~SWP_AREA_DISCARD;

Quoted 4 lines

Quoted 1 line
When using old swapon(8), this code turn off both flags, right
As the flag that enables swap discards SWAP_FLAG_DISCARD remains meaning the same it meant before, when using old swapon(8) (SWP_PAGE_DISCARD|SWP_AREA_DISCARD)

But old swapon(8) don't use neigher SWAP_FLAG_DISCARD_ONCE nor SWAP_FLAG_DISCARD_PAGES. It uses only SWAP_FLAG_DISCARD. So, this condition disables both SWP_PAGE_DISCARD and SWP_AREA_DISCARD.

And you changed that SWP_DISCARDABLE is not checked in IO path at all.

mm/swapfile.c static unsigned long scan_swap_map(struct swap_info_struct *si, · line 215

Quoted 2 lines

mm/swapfile.c +1 -1

-               if (si->flags & SWP_DISCARDABLE) {
+               if (si->flags & SWP_PAGE_DISCARD) {

I suggest new swapon(8) don't pass SWP_DISCARDABLE and kernel handle SWP_DISCARDABLE as (SWAP_FLAG_DISCARD_ONCE | SWAP_FLAG_DISCARD_PAGES).

Optionally, warn SWP_DISCARDABLE is a good idea.

Quoted 6 lines

will remain flagged when discard is enabled, so we keep doing discards the same way we did before (at swapon, and for every released page-cluster).
The flags are removed orthogonally only when the new swapon(8) selects one of the particular discard policy available by using either SWAP_FLAG_DISCARD_ONCE, or SWAP_FLAG_DISCARD_PAGES flags.

Rafael Aquini

author

2013-05-26 15:30 UTC #5 lore raw

Re: [PATCH 01/02] swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

Quoted 13 lines · KOSAKI Motohiro

Quoted 12 lines · Rafael Aquini

Quoted 11 lines · KOSAKI Motohiro

Quoted 10 lines

mm/swapfile.c +10 -0

+                       /*
+                        * By flagging sys_swapon, a sysadmin can tell us to
+                        * either do sinle-time area discards only, or to just
+                        * perform discards for released swap page-clusters.
+                        * Now it's time to adjust the p->flags accordingly.
+                        */
+                       if (swap_flags & SWAP_FLAG_DISCARD_ONCE)
+                               p->flags &= ~SWP_PAGE_DISCARD;
+                       else if (swap_flags & SWAP_FLAG_DISCARD_PAGES)
+                               p->flags &= ~SWP_AREA_DISCARD;

Quoted 9 lines

Quoted 4 lines
Quoted 1 line
When using old swapon(8), this code turn off both flags, right
As the flag that enables swap discards SWAP_FLAG_DISCARD remains meaning the same it meant before, when using old swapon(8) (SWP_PAGE_DISCARD|SWP_AREA_DISCARD)
But old swapon(8) don't use neigher SWAP_FLAG_DISCARD_ONCE nor SWAP_FLAG_DISCARD_PAGES. It uses only SWAP_FLAG_DISCARD. So, this condition disables both SWP_PAGE_DISCARD and SWP_AREA_DISCARD.

This condition _only_ disables one of the new flags orthogonally if swapon(8) flags a policy to sys_swapon. As old swapon(8) can only flag SWAP_FLAG_DISCARD, the original behavior is kept. Nothing will change when one is using an old swapon(8) with this changeset.

Quoted 5 lines

And you changed that SWP_DISCARDABLE is not checked in IO path at all.
Quoted 2 lines
mm/swapfile.c +1 -1
-               if (si->flags & SWP_DISCARDABLE) {
+               if (si->flags & SWP_PAGE_DISCARD) {

And this is exactly what this change is about -- only enabling that particular I/O path if we've been told to discard swap page-clusters. Notice that having SWP_PAGE_DISCARD flagged already implies SWP_DISCARDABLE.

Quoted 2 lines

I suggest new swapon(8) don't pass SWP_DISCARDABLE and kernel handle SWP_DISCARDABLE as (SWAP_FLAG_DISCARD_ONCE | SWAP_FLAG_DISCARD_PAGES).

As the old swapon(8) case can only pass SWAP_FLAG_DISCARD along, this change would nullify the backwards compatibility, wouldn't it?

Quoted 10 lines

Optionally, warn SWP_DISCARDABLE is a good idea.
Quoted 6 lines
will remain flagged when discard is enabled, so we keep doing discards the same way we did before (at swapon, and for every released page-cluster).
The flags are removed orthogonally only when the new swapon(8) selects one of the particular discard policy available by using either SWAP_FLAG_DISCARD_ONCE, or SWAP_FLAG_DISCARD_PAGES flags.

KOSAKI Motohiro

Acked-by

2013-05-26 16:03 UTC #6 lore raw

Re: [PATCH 01/02] swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

Quoted 24 lines · Rafael Aquini

Quoted 13 lines · KOSAKI Motohiro
Quoted 12 lines · Rafael Aquini
Quoted 11 lines · KOSAKI Motohiro
Quoted 10 lines
mm/swapfile.c +10 -0
+                       /*
+                        * By flagging sys_swapon, a sysadmin can tell us to
+                        * either do sinle-time area discards only, or to just
+                        * perform discards for released swap page-clusters.
+                        * Now it's time to adjust the p->flags accordingly.
+                        */
+                       if (swap_flags & SWAP_FLAG_DISCARD_ONCE)
+                               p->flags &= ~SWP_PAGE_DISCARD;
+                       else if (swap_flags & SWAP_FLAG_DISCARD_PAGES)
+                               p->flags &= ~SWP_AREA_DISCARD;
Quoted 9 lines
Quoted 4 lines
Quoted 1 line
When using old swapon(8), this code turn off both flags, right
As the flag that enables swap discards SWAP_FLAG_DISCARD remains meaning the same it meant before, when using old swapon(8) (SWP_PAGE_DISCARD|SWP_AREA_DISCARD)
But old swapon(8) don't use neigher SWAP_FLAG_DISCARD_ONCE nor SWAP_FLAG_DISCARD_PAGES. It uses only SWAP_FLAG_DISCARD. So, this condition disables both SWP_PAGE_DISCARD and SWP_AREA_DISCARD.

Quoted 4 lines

This condition _only_ disables one of the new flags orthogonally if swapon(8) flags a policy to sys_swapon. As old swapon(8) can only flag SWAP_FLAG_DISCARD, the original behavior is kept. Nothing will change when one is using an old swapon(8) with this changeset.

Aha, got it. I misunderstood your code.
Thank you.

Acked-by KOSAKI Motohiro <[email protected]>

Shaohua Li

2013-06-28 02:15 UTC #7 lore raw

Re: [PATCH 01/02] swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES

In reply to Rafael Aquini

Quoted 8 lines · Rafael Aquini

This patch introduces SWAP_FLAG_DISCARD_PAGES and SWAP_FLAG_DISCARD_ONCE new flags to allow more flexibe swap discard policies being flagged through swapon(8). The default behavior is to keep both single-time, or batched, area discards (SWAP_FLAG_DISCARD_ONCE) and fine-grained discards for page-clusters (SWAP_FLAG_DISCARD_PAGES) enabled, in order to keep consistentcy with older kernel behavior, as well as maintain compatibility with older swapon(8). However, through the new introduced flags the best suitable discard policy can be selected accordingly to any given swap device constraint.

I'm sorry to response this thread so later. I thought if we just want to discard the swap partition once at swapon, we really should do it in swapon tool. The swapon tool can detect the swap device supports discard, any swap partition is empty at swapon, and we have ioctl to do discard in userspace, so we have no problem to do discard at the tool. If we don't want to do discard at all, let the tool handles the option. Kernel is not the place to handle the complexity.

Thanks,
Shaohua

Rafael Aquini

author

2013-05-26 04:32 UTC #8 lore raw

PATCH 2/2 swapon: allow a more flexible swap discard policy

In reply to Rafael Aquini

Introduce the necessary changes to swapon(8) allowing a sysadmin to leverage the new changes introduced to sys_swapon by "swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES", therefore allowing a more flexible set of choices when selection the discard policy for mounted swap areas. This patch introduces the following optional arguments to the already existent swapon(8) "--discard" option, in order to allow a discard type to be selected at swapon time:
* once : only single-time area discards are issued. (swapon)
* pages : discard freed pages before they are reused.
If no policy is selected both discard types are enabled. (default)

Signed-off-by Rafael Aquini <[email protected]>

---
 sys-utils/swapon.8 | 24 +++++++++++++------
 sys-utils/swapon.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 77 insertions(+), 17 deletions(-)

sys-utils/swapon.8 +17 -7

index 385bf5a..17f7970 100644
@@ -112,15 +112,25 @@ All devices marked as ``swap'' in
 are made available, except for those with the ``noauto'' option.
 Devices that are already being used as swap are silently skipped.
 .TP
-.B "\-d, \-\-discard"
-Discard freed swap pages before they are reused, if the swap
-device supports the discard or trim operation.  This may improve
-performance on some Solid State Devices, but often it does not.
+.B "\-d, \-\-discard\fR [\fIpolicy\fR]"
+Enable swap discards, if the swap backing device supports the discard or
+trim operation. This may improve performance on some Solid State Devices,
+but often it does not. The long option \-\-discard allows one to select
+between two available swap discard policies:
+.BI \-\-discard=once
+to perform a single-time discard operation for the whole swap area at swapon;
+or
+.BI \-\-discard=pages
+to discard freed swap pages before they are reused, while swapping.
+If no policy is selected, the default behavior is to enable both discard types.
 The
 .I /etc/fstab
-mount option
-.BI discard
-may be also used to enable discard flag.
+mount options
+.BI discard,
+.BI discard=once,
+or
+.BI discard=pages
+may be also used to enable discard flags.
 .TP
 .B "\-e, \-\-ifexists"
 Silently skip devices that do not exist.

sys-utils/swapon.c +60 ~~-10~~

123 lines not on this page · raw message

Signature · 7 lines

1.8.1.4

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

KOSAKI Motohiro

Acked-by on v1 2/2

2013-05-26 16:32 UTC #9 lore raw

Re: [PATCH 02/02] swapon: allow a more flexible swap discard policy

Quoted 11 lines · Rafael Aquini

Introduce the necessary changes to swapon(8) allowing a sysadmin to leverage the new changes introduced to sys_swapon by "swap: discard while swapping only if SWAP_FLAG_DISCARD_PAGES", therefore allowing a more flexible set of choices when selection the discard policy for mounted swap areas. This patch introduces the following optional arguments to the already existent swapon(8) "--discard" option, in order to allow a discard type to be selected at swapon time:
* once : only single-time area discards are issued. (swapon)
* pages : discard freed pages before they are reused.
If no policy is selected both discard types are enabled. (default)

Quoted 1 line

Signed-off-by Rafael Aquini <[email protected]>

Acked-by KOSAKI Motohiro <[email protected]>

Karel Zak

Applied

2013-08-23 11:03 UTC #10 lore raw

Re: [PATCH 02/02] swapon: allow a more flexible swap discard policy

In reply to Rafael Aquini

Quoted 3 lines · Rafael Aquini

 sys-utils/swapon.8 | 24 +++++++++++++------
 sys-utils/swapon.c | 70 ++++++++++++++++++++++++++++++++++++++++++++++--------
 2 files changed, 77 insertions(+), 17 deletions(-)

Applied, thanks.

Karel

Signature · 8 lines

 Karel Zak  <[email protected]>
 http://karelzak.blogspot.com

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