Refactor sys_swapon
Subsystems (2)
cover Refactor sys_swapon
This patch series refactors the sys_swapon function.
sys_swapon is currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.
One of these chunks of code was nearly identical to the part of sys_swapoff which is used in case of a failure return from try_to_unuse(), so this patch series also makes both share the same code.
As a side effect of all this refactoring, the compiled code gets a bit smaller:
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.after
Lightly tested on a x86_64 VM.
mm/swapfile.c | 360 +++++++++++++++++++++++++++---------------------- 1 files changed, 197 insertions(+), 163 deletions(-)
Cesar Eduardo Barros (24):
sys_swapon: use vzalloc instead of vmalloc/memset
sys_swapon: remove changelog from function comment
sys_swapon: do not depend on "type" after allocation
sys_swapon: separate swap_info allocation
sys_swapon: simplify error return from swap_info allocation
sys_swapon: simplify error flow in alloc_swap_info
sys_swapon: remove initial value of name variable
sys_swapon: move setting of error nearer use
sys_swapon: remove did_down variable
sys_swapon: remove bdev variable
sys_swapon: do only cleanup in the cleanup blocks
sys_swapon: use a single error label
sys_swapon: separate bdev claim and inode lock
sys_swapon: simplify error flow in claim_swapfile
sys_swapon: move setting of swapfilepages near use
sys_swapon: separate parsing of swapfile header
sys_swapon: simplify error flow in read_swap_header
sys_swapon: call swap_cgroup_swapon earlier
sys_swapon: separate parsing of bad blocks and extents
sys_swapon: simplify error flow in setup_swap_map_and_extents
sys_swapon: remove nr_good_pages variable
sys_swapon: move printk outside lock
sys_swapoff: change order to match sys_swapon
sys_swapon: separate final enabling of the swapfile
Signature · 10 lines
Cesar Eduardo Barros [email protected] [email protected] -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 1/24 sys_swapon: use vzalloc instead of vmalloc/memset
--- mm/swapfile.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
mm/swapfile.c +1 -2
@@ -2047,13 +2047,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) goto bad_swap;/* OK, set up the swap map and apply the bad block list */ - swap_map =vmalloc (maxpages); + swap_map =vzalloc (maxpages);if (!swap_map) { error = -ENOMEM ; goto bad_swap; } -memset (swap_map,0 , maxpages); nr_good_pages = maxpages -1 ;/* omit header page */ for (i =0 ; i < swap_header->info.nr_badpages; i++) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 2/24 sys_swapon: remove changelog from function comment
In reply to Cesar Eduardo BarrosChangelogs belong in the git history instead of in the source code.
Also, "The swapon system call" is redundant with
"SYSCALL_DEFINE2(swapon, ...)".
--- mm/swapfile.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
mm/swapfile.c +0 -5
@@ -1844,11 +1844,6 @@ static int __init max_swapfiles_check(void)late_initcall (max_swapfiles_check);#endif -/* -* Written 01/25/92 by Simmule Turner, heavily changed by Linus. -* -* The swapon system call -*/ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 3/24 sys_swapon: do not depend on "type" after allocation
In reply to Cesar Eduardo BarrosWithin sys_swapon, after the swap_info entry has been allocated, we always have type == p->type and swap_info[type] == p. Use this fact to reduce the dependency on the "type" local variable within the function, as a preparation to move the allocation of the swap_info entry to a separate function.
--- mm/swapfile.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
mm/swapfile.c +5 -5
@@ -1927,7 +1927,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)for (i =0 ; i < nr_swapfiles; i++) {struct swap_info_struct *q = swap_info[i]; -if (i == type || !q->swap_file) +if (q == p || !q->swap_file)continue ;if (mapping == q->swap_file->f_mapping) goto bad_swap;
@@ -2062,7 +2062,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } - error =swap_cgroup_swapon (type, maxpages); + error =swap_cgroup_swapon (p->type, maxpages);if (error) goto bad_swap;
@@ -2120,9 +2120,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } p->next = i;if (prev <0 ) - swap_list.head = swap_list.next = type; + swap_list.head = swap_list.next = p->type;else - swap_info[prev]->next = type; + swap_info[prev]->next = p->type;spin_unlock (&swap_lock);mutex_unlock (&swapon_mutex);atomic_inc (&proc_poll_event);
@@ -2136,7 +2136,7 @@ bad_swap:blkdev_put (bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); }destroy_swap_extents (p); -swap_cgroup_swapoff (type); +swap_cgroup_swapoff (p->type); bad_swap_2:spin_lock (&swap_lock); p->swap_file =NULL ;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 4/24 sys_swapon: separate swap_info allocation
In reply to Cesar Eduardo BarrosMove the swap_info allocation to its own function. Only code movement, no functional changes.
--- mm/swapfile.c | 57 +++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 37 insertions(+), 20 deletions(-)
mm/swapfile.c +37 -20
@@ -1844,33 +1844,15 @@ static int __init max_swapfiles_check(void)late_initcall (max_swapfiles_check);#endif -SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) +static struct swap_info_struct *alloc_swap_info (void ) {struct swap_info_struct *p; -char *name =NULL ; -struct block_device *bdev =NULL ; -struct file *swap_file =NULL ; -struct address_space *mapping;unsigned int type; -int i, prev;int error; -union swap_header *swap_header; -unsigned int nr_good_pages; -int nr_extents =0 ; -sector_t span; -unsigned long maxpages; -unsigned long swapfilepages; -unsigned char *swap_map =NULL ; -struct page *page =NULL ; -struct inode *inode =NULL ; -int did_down =0 ; - -if (!capable (CAP_SYS_ADMIN )) -return -EPERM ; p =kzalloc (sizeof (*p),GFP_KERNEL );if (!p) -return -ENOMEM ; +return ERR_PTR (-ENOMEM );spin_lock (&swap_lock);for (type =0 ; type < nr_swapfiles; type++) {
@@ -1906,6 +1888,41 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) p->next =-1 ;spin_unlock (&swap_lock); +return p; + +out: +return ERR_PTR (error); +} + +SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) +{ +struct swap_info_struct *p; +char *name =NULL ; +struct block_device *bdev =NULL ; +struct file *swap_file =NULL ; +struct address_space *mapping; +int i, prev; +int error; +union swap_header *swap_header; +unsigned int nr_good_pages; +int nr_extents =0 ; +sector_t span; +unsigned long maxpages; +unsigned long swapfilepages; +unsigned char *swap_map =NULL ; +struct page *page =NULL ; +struct inode *inode =NULL ; +int did_down =0 ; + +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + + p =alloc_swap_info (); +if (IS_ERR (p)) { + error =PTR_ERR (p); + goto out; + } + name =getname (specialfile); error =PTR_ERR (name);if (IS_ERR (name)) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 5/24 sys_swapon: simplify error return from swap_info allocation
In reply to Cesar Eduardo BarrosAt this point in sys_swapon, there is nothing to free. Return directly instead of jumping to the cleanup block at the end of the function.
--- mm/swapfile.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
mm/swapfile.c +2 -4
@@ -1918,10 +1918,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)return -EPERM ; p =alloc_swap_info (); -if (IS_ERR (p)) { - error =PTR_ERR (p); - goto out; - } +if (IS_ERR (p)) +return PTR_ERR (p); name =getname (specialfile); error =PTR_ERR (name);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 6/24 sys_swapon: simplify error flow in alloc_swap_info
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-)
mm/swapfile.c +1 -6
@@ -1848,7 +1848,6 @@ static struct swap_info_struct *alloc_swap_info(void) {struct swap_info_struct *p;unsigned int type; -int error; p =kzalloc (sizeof (*p),GFP_KERNEL );if (!p)
@@ -1859,11 +1858,10 @@ static struct swap_info_struct *alloc_swap_info(void)if (!(swap_info[type]->flags &SWP_USED ))break ; } - error = -EPERM ;if (type >=MAX_SWAPFILES ) {spin_unlock (&swap_lock);kfree (p); - goto out; +return ERR_PTR (-EPERM ); }if (type >= nr_swapfiles) { p->type = type;
@@ -1889,9 +1887,6 @@ static struct swap_info_struct *alloc_swap_info(void)spin_unlock (&swap_lock);return p; - -out: -return ERR_PTR (error); } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 7/24 sys_swapon: remove initial value of name variable
In reply to Cesar Eduardo BarrosNow there is nothing which jumps to the cleanup blocks before the name variable is set. There is no need to set it initially to NULL anymore.
--- mm/swapfile.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
mm/swapfile.c +1 -1
@@ -1892,7 +1892,7 @@ static struct swap_info_struct *alloc_swap_info(void) SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p; -char *name =NULL ; +char *name;struct block_device *bdev =NULL ;struct file *swap_file =NULL ;struct address_space *mapping;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 8/24 sys_swapon: move setting of error nearer use
In reply to Cesar Eduardo BarrosMove the setting of the error variable nearer the goto in a few places.
Avoids calling PTR_ERR() if not IS_ERR() in two places, and makes the error condition more explicit in two other places.
--- mm/swapfile.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)
mm/swapfile.c +6 -5
@@ -1917,14 +1917,14 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)return PTR_ERR (p); name =getname (specialfile); - error =PTR_ERR (name);if (IS_ERR (name)) { + error =PTR_ERR (name); name =NULL ; goto bad_swap_2; } swap_file =filp_open (name,O_RDWR |O_LARGEFILE ,0 ); - error =PTR_ERR (swap_file);if (IS_ERR (swap_file)) { + error =PTR_ERR (swap_file); swap_file =NULL ; goto bad_swap_2; }
@@ -1933,17 +1933,17 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) mapping = swap_file->f_mapping; inode = mapping->host; - error = -EBUSY ;for (i =0 ; i < nr_swapfiles; i++) {struct swap_info_struct *q = swap_info[i];if (q == p || !q->swap_file)continue ; -if (mapping == q->swap_file->f_mapping) +if (mapping == q->swap_file->f_mapping) { + error = -EBUSY ; goto bad_swap; + } } - error = -EINVAL ;if (S_ISBLK (inode->i_mode)) { bdev =I_BDEV (inode); error = blkdev_get(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL,
@@ -1968,6 +1968,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) goto bad_swap; } }else { + error = -EINVAL ; goto bad_swap; }
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 9/24 sys_swapon: remove did_down variable
In reply to Cesar Eduardo BarrosSince mutex_lock(&inode->i_mutex) is called just after setting inode, did_down is always equivalent to (inode && S_ISREG(inode->i_mode)).
Use this fact to remove the did_down variable.
--- mm/swapfile.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
mm/swapfile.c +1 -3
@@ -1907,7 +1907,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)unsigned char *swap_map =NULL ;struct page *page =NULL ;struct inode *inode =NULL ; -int did_down =0 ;if (!capable (CAP_SYS_ADMIN ))return -EPERM ;
@@ -1962,7 +1961,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) }else if (S_ISREG (inode->i_mode)) { p->bdev = inode->i_sb->s_bdev;mutex_lock (&inode->i_mutex); - did_down =1 ;if (IS_SWAPFILE (inode)) { error = -EBUSY ; goto bad_swap;
@@ -2163,7 +2161,7 @@ out: }if (name)putname (name); -if (did_down) { +if (inode &&S_ISREG (inode->i_mode)) {if (!error) inode->i_flags |=S_SWAPFILE ;mutex_unlock (&inode->i_mutex);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 10/24 sys_swapon: remove bdev variable
In reply to Cesar Eduardo BarrosThe bdev variable is always equivalent to (S_ISBLK(inode->i_mode) ? p->bdev : NULL), as long as it being set is moved to a bit earlier. Use this fact to remove the bdev variable.
--- mm/swapfile.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)
mm/swapfile.c +9 -10
@@ -1893,7 +1893,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;char *name; -struct block_device *bdev =NULL ;struct file *swap_file =NULL ;struct address_space *mapping;int i, prev;
@@ -1944,19 +1943,19 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) }if (S_ISBLK (inode->i_mode)) { - bdev =I_BDEV (inode); - error =blkdev_get (bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL , + p->bdev =I_BDEV (inode); + error =blkdev_get (p->bdev, +FMODE_READ |FMODE_WRITE |FMODE_EXCL , sys_swapon);if (error <0 ) { - bdev =NULL ; + p->bdev =NULL ; error = -EINVAL ; goto bad_swap; } - p->old_block_size =block_size (bdev); - error =set_blocksize (bdev,PAGE_SIZE ); + p->old_block_size =block_size (p->bdev); + error =set_blocksize (p->bdev,PAGE_SIZE );if (error <0 ) goto bad_swap; - p->bdev = bdev; p->flags |=SWP_BLKDEV ; }else if (S_ISREG (inode->i_mode)) { p->bdev = inode->i_sb->s_bdev;
@@ -2140,9 +2139,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error =0 ; goto out; bad_swap: -if (bdev) { -set_blocksize (bdev, p->old_block_size); -blkdev_put (bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); +if (S_ISBLK (inode->i_mode) && p->bdev) { +set_blocksize (p->bdev, p->old_block_size); +blkdev_put (p->bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); }destroy_swap_extents (p);swap_cgroup_swapoff (p->type);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 11/24 sys_swapon: do only cleanup in the cleanup blocks
In reply to Cesar Eduardo BarrosThe only way error is 0 in the cleanup blocks is when the function is returning successfully. In this case, the cleanup blocks were setting S_SWAPFILE in the S_ISREG case. But this is not a cleanup.
Move the setting of S_SWAPFILE to just before the "goto out;" to make this more clear. At this point, we do not need to test for inode because it will never be NULL.
--- mm/swapfile.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
mm/swapfile.c +3 -4
@@ -2136,6 +2136,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)atomic_inc (&proc_poll_event);wake_up_interruptible (&proc_poll_wait); +if (S_ISREG (inode->i_mode)) + inode->i_flags |=S_SWAPFILE ; error =0 ; goto out; bad_swap:
@@ -2160,11 +2162,8 @@ out: }if (name)putname (name); -if (inode &&S_ISREG (inode->i_mode)) { -if (!error) - inode->i_flags |=S_SWAPFILE ; +if (inode &&S_ISREG (inode->i_mode))mutex_unlock (&inode->i_mutex); - }return error; }
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 12/24 sys_swapon: use a single error label
In reply to Cesar Eduardo Barrossys_swapon currently has two error labels, bad_swap and bad_swap_2. bad_swap does the same as bad_swap_2 plus destroy_swap_extents() and swap_cgroup_swapoff(); both are noops in the places where bad_swap_2 is jumped to. With a single extra test for inode (matching the one in the S_ISREG case below), all the error paths in the function can go to bad_swap.
--- mm/swapfile.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
mm/swapfile.c +3 -4
@@ -1918,13 +1918,13 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)if (IS_ERR (name)) { error =PTR_ERR (name); name =NULL ; - goto bad_swap_2; + goto bad_swap; } swap_file =filp_open (name,O_RDWR |O_LARGEFILE ,0 );if (IS_ERR (swap_file)) { error =PTR_ERR (swap_file); swap_file =NULL ; - goto bad_swap_2; + goto bad_swap; } p->swap_file = swap_file;
@@ -2141,13 +2141,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error =0 ; goto out; bad_swap: -if (S_ISBLK (inode->i_mode) && p->bdev) { +if (inode &&S_ISBLK (inode->i_mode) && p->bdev) {set_blocksize (p->bdev, p->old_block_size);blkdev_put (p->bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); }destroy_swap_extents (p);swap_cgroup_swapoff (p->type); -bad_swap_2:spin_lock (&swap_lock); p->swap_file =NULL ; p->flags =0 ;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 13/24 sys_swapon: separate bdev claim and inode lock
In reply to Cesar Eduardo BarrosMove the code which claims the bdev (S_ISBLK) or locks the inode (S_ISREG) to a separate function. Only code movement, no functional changes.
--- mm/swapfile.c | 64 ++++++++++++++++++++++++++++++++++---------------------- 1 files changed, 39 insertions(+), 25 deletions(-)
mm/swapfile.c +39 -25
@@ -1889,6 +1889,43 @@ static struct swap_info_struct *alloc_swap_info(void)return p; } +static int claim_swapfile (struct swap_info_struct *p,struct inode *inode) +{ +int error; + +if (S_ISBLK (inode->i_mode)) { + p->bdev =I_BDEV (inode); + error =blkdev_get (p->bdev, +FMODE_READ |FMODE_WRITE |FMODE_EXCL , + sys_swapon); +if (error <0 ) { + p->bdev =NULL ; + error = -EINVAL ; + goto bad_swap; + } + p->old_block_size =block_size (p->bdev); + error =set_blocksize (p->bdev,PAGE_SIZE ); +if (error <0 ) + goto bad_swap; + p->flags |=SWP_BLKDEV ; + }else if (S_ISREG (inode->i_mode)) { + p->bdev = inode->i_sb->s_bdev; +mutex_lock (&inode->i_mutex); +if (IS_SWAPFILE (inode)) { + error = -EBUSY ; + goto bad_swap; + } + }else { + error = -EINVAL ; + goto bad_swap; + } + +return 0 ; + +bad_swap: +return error; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
@@ -1942,32 +1979,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } -if (S_ISBLK (inode->i_mode)) { - p->bdev =I_BDEV (inode); - error =blkdev_get (p->bdev, -FMODE_READ |FMODE_WRITE |FMODE_EXCL , - sys_swapon); -if (error <0 ) { - p->bdev =NULL ; - error = -EINVAL ; - goto bad_swap; - } - p->old_block_size =block_size (p->bdev); - error =set_blocksize (p->bdev,PAGE_SIZE ); -if (error <0 ) - goto bad_swap; - p->flags |=SWP_BLKDEV ; - }else if (S_ISREG (inode->i_mode)) { - p->bdev = inode->i_sb->s_bdev; -mutex_lock (&inode->i_mutex); -if (IS_SWAPFILE (inode)) { - error = -EBUSY ; - goto bad_swap; - } - }else { - error = -EINVAL ; + error =claim_swapfile (p, inode); +if (unlikely (error)) goto bad_swap; - } swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 14/24 sys_swapon: simplify error flow in claim_swapfile
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 20 ++++++-------------- 1 files changed, 6 insertions(+), 14 deletions(-)
mm/swapfile.c +6 -14
@@ -1900,30 +1900,22 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode) sys_swapon);if (error <0 ) { p->bdev =NULL ; - error = -EINVAL ; - goto bad_swap; +return -EINVAL ; } p->old_block_size =block_size (p->bdev); error =set_blocksize (p->bdev,PAGE_SIZE );if (error <0 ) - goto bad_swap; +return error; p->flags |=SWP_BLKDEV ; }else if (S_ISREG (inode->i_mode)) { p->bdev = inode->i_sb->s_bdev;mutex_lock (&inode->i_mutex); -if (IS_SWAPFILE (inode)) { - error = -EBUSY ; - goto bad_swap; - } - }else { - error = -EINVAL ; - goto bad_swap; - } +if (IS_SWAPFILE (inode)) +return -EBUSY ; + }else +return -EINVAL ;return 0 ; - -bad_swap: -return error; } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 15/24 sys_swapon: move setting of swapfilepages near use
In reply to Cesar Eduardo BarrosThere is no reason I can see to read inode->i_size long before it is needed. Move its read to just before it is needed, to reduce the variable lifetime.
--- mm/swapfile.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
mm/swapfile.c +1 -2
@@ -1975,8 +1975,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)if (unlikely (error)) goto bad_swap; - swapfilepages =i_size_read (inode) >>PAGE_SHIFT ; -/* * Read the swap header. */
@@ -2045,6 +2043,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error = -EINVAL ;if (!maxpages) goto bad_swap; + swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;if (swapfilepages && maxpages > swapfilepages) {printk (KERN_WARNING "Swap area shorter than signature indicates\n" );
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 16/24 sys_swapon: separate parsing of swapfile header
In reply to Cesar Eduardo BarrosMove the code which parses and checks the swapfile header (except for the bad block list) to a separate function. Only code movement, no functional changes.
--- mm/swapfile.c | 140 ++++++++++++++++++++++++++++++++------------------------- 1 files changed, 78 insertions(+), 62 deletions(-)
mm/swapfile.c +78 -62
@@ -1918,6 +1918,82 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode)return 0 ; } +static unsigned long read_swap_header (struct swap_info_struct *p, +union swap_header *swap_header, +struct inode *inode) +{ +int i; +unsigned long maxpages; +unsigned long swapfilepages; + +if (memcmp ("SWAPSPACE2" , swap_header->magic.magic,10 )) { +printk (KERN_ERR "Unable to find swap-space signature\n" ); + goto bad_swap; + } + +/* swap partition endianess hack... */ +if (swab32 (swap_header->info.version) ==1 ) { +swab32s (&swap_header->info.version); +swab32s (&swap_header->info.last_page); +swab32s (&swap_header->info.nr_badpages); +for (i =0 ; i < swap_header->info.nr_badpages; i++) +swab32s (&swap_header->info.badpages[i]); + } +/* Check the swap header's sub-version */ +if (swap_header->info.version !=1 ) { +printk (KERN_WARNING +"Unable to handle swap header version %d\n" , + swap_header->info.version); + goto bad_swap; + } + + p->lowest_bit =1 ; + p->cluster_next =1 ; + p->cluster_nr =0 ; + +/* +* Find out how many pages are allowed for a single swap +* device. There are two limiting factors: 1) the number of +* bits for the swap offset in the swp_entry_t type and +* 2) the number of bits in the a swap pte as defined by +* the different architectures. In order to find the +* largest possible bit mask a swap entry with swap type 0 +* and swap offset ~0UL is created, encoded to a swap pte, +* decoded to a swp_entry_t again and finally the swap +* offset is extracted. This will mask all the bits from +* the initial ~0UL mask that can't be encoded in either +* the swp_entry_t or the architecture definition of a +* swap pte. +*/ + maxpages =swp_offset (pte_to_swp_entry ( +swp_entry_to_pte (swp_entry (0 , ~0UL )))) +1 ; +if (maxpages > swap_header->info.last_page) { + maxpages = swap_header->info.last_page +1 ; +/* p->max is an unsigned int: don't overflow it */ +if ((unsigned int )maxpages ==0 ) + maxpages =UINT_MAX ; + } + p->highest_bit = maxpages -1 ; + +if (!maxpages) + goto bad_swap; + swapfilepages =i_size_read (inode) >>PAGE_SHIFT ; +if (swapfilepages && maxpages > swapfilepages) { +printk (KERN_WARNING +"Swap area shorter than signature indicates\n" ); + goto bad_swap; + } +if (swap_header->info.nr_badpages &&S_ISREG (inode->i_mode)) + goto bad_swap; +if (swap_header->info.nr_badpages >MAX_SWAP_BADPAGES ) + goto bad_swap; + +return maxpages; + +bad_swap: +return 0 ; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
@@ -1931,7 +2007,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)int nr_extents =0 ;sector_t span;unsigned long maxpages; -unsigned long swapfilepages;unsigned char *swap_map =NULL ;struct page *page =NULL ;struct inode *inode =NULL ;
@@ -1989,71 +2064,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } swap_header =kmap (page); -if (memcmp ("SWAPSPACE2" , swap_header->magic.magic,10 )) { -printk (KERN_ERR "Unable to find swap-space signature\n" ); + maxpages =read_swap_header (p, swap_header, inode); +if (unlikely (!maxpages)) { error = -EINVAL ; goto bad_swap; } -/* swap partition endianess hack... */ -if (swab32 (swap_header->info.version) ==1 ) { -swab32s (&swap_header->info.version); -swab32s (&swap_header->info.last_page); -swab32s (&swap_header->info.nr_badpages); -for (i =0 ; i < swap_header->info.nr_badpages; i++) -swab32s (&swap_header->info.badpages[i]); - } -/* Check the swap header's sub-version */ -if (swap_header->info.version !=1 ) { -printk (KERN_WARNING -"Unable to handle swap header version %d\n" , - swap_header->info.version); - error = -EINVAL ; - goto bad_swap; - } - - p->lowest_bit =1 ; - p->cluster_next =1 ; - p->cluster_nr =0 ; - -/* -* Find out how many pages are allowed for a single swap -* device. There are two limiting factors: 1) the number of -* bits for the swap offset in the swp_entry_t type and -* 2) the number of bits in the a swap pte as defined by -* the different architectures. In order to find the -* largest possible bit mask a swap entry with swap type 0 -* and swap offset ~0UL is created, encoded to a swap pte, -* decoded to a swp_entry_t again and finally the swap -* offset is extracted. This will mask all the bits from -* the initial ~0UL mask that can't be encoded in either -* the swp_entry_t or the architecture definition of a -* swap pte. -*/ - maxpages =swp_offset (pte_to_swp_entry ( -swp_entry_to_pte (swp_entry (0 , ~0UL )))) +1 ; -if (maxpages > swap_header->info.last_page) { - maxpages = swap_header->info.last_page +1 ; -/* p->max is an unsigned int: don't overflow it */ -if ((unsigned int )maxpages ==0 ) - maxpages =UINT_MAX ; - } - p->highest_bit = maxpages -1 ; - - error = -EINVAL ; -if (!maxpages) - goto bad_swap; - swapfilepages =i_size_read (inode) >>PAGE_SHIFT ; -if (swapfilepages && maxpages > swapfilepages) { -printk (KERN_WARNING -"Swap area shorter than signature indicates\n" ); - goto bad_swap; - } -if (swap_header->info.nr_badpages &&S_ISREG (inode->i_mode)) - goto bad_swap; -if (swap_header->info.nr_badpages >MAX_SWAP_BADPAGES ) - goto bad_swap; -/* OK, set up the swap map and apply the bad block list */ swap_map =vzalloc (maxpages);if (!swap_map) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 17/24 sys_swapon: simplify error flow in read_swap_header
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 15 ++++++--------- 1 files changed, 6 insertions(+), 9 deletions(-)
mm/swapfile.c +6 -9
@@ -1928,7 +1928,7 @@ static unsigned long read_swap_header(struct swap_info_struct *p,if (memcmp ("SWAPSPACE2" , swap_header->magic.magic,10 )) {printk (KERN_ERR "Unable to find swap-space signature\n" ); - goto bad_swap; +return 0 ; }/* swap partition endianess hack... */
@@ -1944,7 +1944,7 @@ static unsigned long read_swap_header(struct swap_info_struct *p,printk (KERN_WARNING "Unable to handle swap header version %d\n" , swap_header->info.version); - goto bad_swap; +return 0 ; } p->lowest_bit =1 ;
@@ -1976,22 +1976,19 @@ static unsigned long read_swap_header(struct swap_info_struct *p, p->highest_bit = maxpages -1 ;if (!maxpages) - goto bad_swap; +return 0 ; swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;if (swapfilepages && maxpages > swapfilepages) {printk (KERN_WARNING "Swap area shorter than signature indicates\n" ); - goto bad_swap; +return 0 ; }if (swap_header->info.nr_badpages &&S_ISREG (inode->i_mode)) - goto bad_swap; +return 0 ;if (swap_header->info.nr_badpages >MAX_SWAP_BADPAGES ) - goto bad_swap; +return 0 ;return maxpages; - -bad_swap: -return 0 ; } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 18/24 sys_swapon: call swap_cgroup_swapon earlier
In reply to Cesar Eduardo BarrosThe call to swap_cgroup_swapon is in the middle of loading the swap map and extents. As it only does memory allocation and does not depend on the swapfile layout (map/extents), it can be called earlier (or later).
Move it to just after the allocation of swap_map, since it is conceptually similar (allocates a map).
--- mm/swapfile.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
mm/swapfile.c +4 -4
@@ -2074,6 +2074,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) goto bad_swap; } + error =swap_cgroup_swapon (p->type, maxpages); +if (error) + goto bad_swap; + nr_good_pages = maxpages -1 ;/* omit header page */ for (i =0 ; i < swap_header->info.nr_badpages; i++) {
@@ -2088,10 +2092,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } - error =swap_cgroup_swapon (p->type, maxpages); -if (error) - goto bad_swap; -if (nr_good_pages) { swap_map[0 ] =SWAP_MAP_BAD ; p->max = maxpages;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 19/24 sys_swapon: separate parsing of bad blocks and extents
In reply to Cesar Eduardo BarrosMove the code which parses the bad block list and the extents to a separate function. Only code movement, no functional changes.
This change uses the fact that, after the success path, nr_good_pages == p->pages.
--- mm/swapfile.c | 83 +++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 54 insertions(+), 29 deletions(-)
mm/swapfile.c +54 -29
@@ -1991,6 +1991,54 @@ static unsigned long read_swap_header(struct swap_info_struct *p,return maxpages; } +static int setup_swap_map_and_extents (struct swap_info_struct *p, +union swap_header *swap_header, +unsigned char *swap_map, +unsigned long maxpages, +sector_t *span) +{ +int i; +int error; +unsigned int nr_good_pages; +int nr_extents; + + nr_good_pages = maxpages -1 ;/* omit header page */ + +for (i =0 ; i < swap_header->info.nr_badpages; i++) { +unsigned int page_nr = swap_header->info.badpages[i]; +if (page_nr ==0 || page_nr > swap_header->info.last_page) { + error = -EINVAL ; + goto bad_swap; + } +if (page_nr < maxpages) { + swap_map[page_nr] =SWAP_MAP_BAD ; + nr_good_pages--; + } + } + +if (nr_good_pages) { + swap_map[0 ] =SWAP_MAP_BAD ; + p->max = maxpages; + p->pages = nr_good_pages; + nr_extents =setup_swap_extents (p, span); +if (nr_extents <0 ) { + error = nr_extents; + goto bad_swap; + } + nr_good_pages = p->pages; + } +if (!nr_good_pages) { +printk (KERN_WARNING "Empty swap-file\n" ); + error = -EINVAL ; + goto bad_swap; + } + +return nr_extents; + +bad_swap: +return error; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
@@ -2001,7 +2049,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)int error;union swap_header *swap_header;unsigned int nr_good_pages; -int nr_extents =0 ; +int nr_extents;sector_t span;unsigned long maxpages;unsigned char *swap_map =NULL ;
@@ -2078,36 +2126,13 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)if (error) goto bad_swap; - nr_good_pages = maxpages -1 ;/* omit header page */ - -for (i =0 ; i < swap_header->info.nr_badpages; i++) { -unsigned int page_nr = swap_header->info.badpages[i]; -if (page_nr ==0 || page_nr > swap_header->info.last_page) { - error = -EINVAL ; - goto bad_swap; - } -if (page_nr < maxpages) { - swap_map[page_nr] =SWAP_MAP_BAD ; - nr_good_pages--; - } - } - -if (nr_good_pages) { - swap_map[0 ] =SWAP_MAP_BAD ; - p->max = maxpages; - p->pages = nr_good_pages; - nr_extents =setup_swap_extents (p, &span); -if (nr_extents <0 ) { - error = nr_extents; - goto bad_swap; - } - nr_good_pages = p->pages; - } -if (!nr_good_pages) { -printk (KERN_WARNING "Empty swap-file\n" ); - error = -EINVAL ; + nr_extents =setup_swap_map_and_extents (p, swap_header, swap_map, + maxpages, &span); +if (unlikely (nr_extents <0 )) { + error = nr_extents; goto bad_swap; } + nr_good_pages = p->pages;if (p->bdev) {if (blk_queue_nonrot (bdev_get_queue (p->bdev))) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 20/24 sys_swapon: simplify error flow in setup_swap_map_and_extents
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 19 +++++-------------- 1 files changed, 5 insertions(+), 14 deletions(-)
mm/swapfile.c +5 -14
@@ -1998,7 +1998,6 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p, sector_t *span) {int i; -int error;unsigned int nr_good_pages;int nr_extents;
@@ -2006,10 +2005,8 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p,for (i =0 ; i < swap_header->info.nr_badpages; i++) {unsigned int page_nr = swap_header->info.badpages[i]; -if (page_nr ==0 || page_nr > swap_header->info.last_page) { - error = -EINVAL ; - goto bad_swap; - } +if (page_nr ==0 || page_nr > swap_header->info.last_page) +return -EINVAL ;if (page_nr < maxpages) { swap_map[page_nr] =SWAP_MAP_BAD ; nr_good_pages--;
@@ -2021,22 +2018,16 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p, p->max = maxpages; p->pages = nr_good_pages; nr_extents =setup_swap_extents (p, span); -if (nr_extents <0 ) { - error = nr_extents; - goto bad_swap; - } +if (nr_extents <0 ) +return nr_extents; nr_good_pages = p->pages; }if (!nr_good_pages) {printk (KERN_WARNING "Empty swap-file\n" ); - error = -EINVAL ; - goto bad_swap; +return -EINVAL ; }return nr_extents; - -bad_swap: -return error; } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 21/24 sys_swapon: remove nr_good_pages variable
In reply to Cesar Eduardo BarrosIt still exists within setup_swap_map_and_extents(), but after it nr_good_pages == p->pages.
--- mm/swapfile.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
mm/swapfile.c +3 -5
@@ -2039,7 +2039,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)int i, prev;int error;union swap_header *swap_header; -unsigned int nr_good_pages;int nr_extents;sector_t span;unsigned long maxpages;
@@ -2123,7 +2122,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error = nr_extents; goto bad_swap; } - nr_good_pages = p->pages;if (p->bdev) {if (blk_queue_nonrot (bdev_get_queue (p->bdev))) {
@@ -2143,12 +2141,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) p->prio = --least_priority; p->swap_map = swap_map; p->flags |=SWP_WRITEOK ; - nr_swap_pages += nr_good_pages; - total_swap_pages += nr_good_pages; + nr_swap_pages += p->pages; + total_swap_pages += p->pages;printk (KERN_INFO "Adding %uk swap on %s. " "Priority:%d extents:%d across:%lluk %s%s\n" , - nr_good_pages<<(PAGE_SHIFT -10 ), name, p->prio, + p->pages<<(PAGE_SHIFT -10 ), name, p->prio, nr_extents, (unsigned long long )span<<(PAGE_SHIFT -10 ), (p->flags &SWP_SOLIDSTATE ) ?"SS" :"" , (p->flags &SWP_DISCARDABLE ) ?"D" :"" );
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 22/24 sys_swapon: move printk outside lock
In reply to Cesar Eduardo BarrosThe block in sys_swapon which does the final adjustments to the swap_info_struct and to swap_list is the same as the block which re-inserts it again at sys_swapoff on failure of try_to_unuse(). To be able to make both share the same code, move the printk() call in the middle of it to just after it.
--- mm/swapfile.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
mm/swapfile.c +8 -7
@@ -2144,13 +2144,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) nr_swap_pages += p->pages; total_swap_pages += p->pages; -printk (KERN_INFO "Adding %uk swap on %s. " -"Priority:%d extents:%d across:%lluk %s%s\n" , - p->pages<<(PAGE_SHIFT -10 ), name, p->prio, - nr_extents, (unsigned long long )span<<(PAGE_SHIFT -10 ), - (p->flags &SWP_SOLIDSTATE ) ?"SS" :"" , - (p->flags &SWP_DISCARDABLE ) ?"D" :"" ); -/* insert swap space into swap_list: */ prev =-1 ;for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) {
@@ -2164,6 +2157,14 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)else swap_info[prev]->next = p->type;spin_unlock (&swap_lock); + +printk (KERN_INFO "Adding %uk swap on %s. " +"Priority:%d extents:%d across:%lluk %s%s\n" , + p->pages<<(PAGE_SHIFT -10 ), name, p->prio, + nr_extents, (unsigned long long )span<<(PAGE_SHIFT -10 ), + (p->flags &SWP_SOLIDSTATE ) ?"SS" :"" , + (p->flags &SWP_DISCARDABLE ) ?"D" :"" ); +mutex_unlock (&swapon_mutex);atomic_inc (&proc_poll_event);wake_up_interruptible (&proc_poll_wait);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 23/24 sys_swapoff: change order to match sys_swapon
In reply to Cesar Eduardo BarrosThe block in sys_swapon which does the final adjustments to the swap_info_struct and to swap_list is the same as the block which re-inserts it again at sys_swapoff on failure of try_to_unuse(), except for the order of the operations within the lock. Since the order should not matter, arbitrarily change sys_swapoff to match sys_swapon, in preparation to making both share the same code.
--- mm/swapfile.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
mm/swapfile.c +4 -3
@@ -1625,6 +1625,10 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)spin_lock (&swap_lock);if (p->prio <0 ) p->prio = --least_priority; + p->flags |=SWP_WRITEOK ; + nr_swap_pages += p->pages; + total_swap_pages += p->pages; + prev =-1 ;for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) {if (p->prio >= swap_info[i]->prio)
@@ -1636,9 +1640,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) swap_list.head = swap_list.next = type; else swap_info[prev]->next = type; - nr_swap_pages += p->pages; - total_swap_pages += p->pages; - p->flags |=SWP_WRITEOK ;spin_unlock (&swap_lock); goto out_dput; }
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH 24/24 sys_swapon: separate final enabling of the swapfile
In reply to Cesar Eduardo BarrosThe block in sys_swapon which does the final adjustments to the swap_info_struct and to swap_list is the same as the block which re-inserts it again at sys_swapoff on failure of try_to_unuse(). Move this code to a separate function, and use it both in sys_swapon and sys_swapoff.
--- mm/swapfile.c | 84 ++++++++++++++++++++++++++++---------------------------- 1 files changed, 42 insertions(+), 42 deletions(-)
mm/swapfile.c +42 -42
@@ -1550,6 +1550,36 @@ bad_bmap: goto out; } +static void enable_swap_info (struct swap_info_struct *p,int prio, +unsigned char *swap_map) +{ +int i, prev; + +spin_lock (&swap_lock); +if (prio >=0 ) + p->prio = prio; +else + p->prio = --least_priority; + p->swap_map = swap_map; + p->flags |=SWP_WRITEOK ; + nr_swap_pages += p->pages; + total_swap_pages += p->pages; + +/* insert swap space into swap_list: */ + prev =-1 ; +for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) { +if (p->prio >= swap_info[i]->prio) +break ; + prev = i; + } + p->next = i; +if (prev <0 ) + swap_list.head = swap_list.next = p->type; +else + swap_info[prev]->next = p->type; +spin_unlock (&swap_lock); +} + SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) {struct swap_info_struct *p =NULL ;
@@ -1621,26 +1651,14 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) current->flags &= ~PF_OOM_ORIGIN ;if (err) { +/* +* reading p->prio and p->swap_map outside the lock is +* safe here because only sys_swapon and sys_swapoff +* change them, and there can be no other sys_swapon or +* sys_swapoff for this swap_info_struct at this point. +*/ /* re-insert swap space back into swap_list */ -spin_lock (&swap_lock); -if (p->prio <0 ) - p->prio = --least_priority; - p->flags |=SWP_WRITEOK ; - nr_swap_pages += p->pages; - total_swap_pages += p->pages; - - prev =-1 ; -for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) { -if (p->prio >= swap_info[i]->prio) -break ; - prev = i; - } - p->next = i; -if (prev <0 ) - swap_list.head = swap_list.next = type; -else - swap_info[prev]->next = type; -spin_unlock (&swap_lock); +enable_swap_info (p, p->prio, p->swap_map); goto out_dput; }
@@ -2037,7 +2055,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)char *name;struct file *swap_file =NULL ;struct address_space *mapping; -int i, prev; +int i; +int prio;int error;union swap_header *swap_header;int nr_extents;
@@ -2134,30 +2153,11 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) }mutex_lock (&swapon_mutex); -spin_lock (&swap_lock); + prio =-1 ;if (swap_flags &SWAP_FLAG_PREFER ) - p->prio = + prio = (swap_flags &SWAP_FLAG_PRIO_MASK ) >>SWAP_FLAG_PRIO_SHIFT ; -else - p->prio = --least_priority; - p->swap_map = swap_map; - p->flags |=SWP_WRITEOK ; - nr_swap_pages += p->pages; - total_swap_pages += p->pages; - -/* insert swap space into swap_list: */ - prev =-1 ; -for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) { -if (p->prio >= swap_info[i]->prio) -break ; - prev = i; - } - p->next = i; -if (prev <0 ) - swap_list.head = swap_list.next = p->type; -else - swap_info[prev]->next = p->type; -spin_unlock (&swap_lock); +enable_swap_info (p, prio, swap_map); printk(KERN_INFO "Adding %uk swap on %s. " "Priority:%d extents:%d across:%lluk %s%s\n",
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 12 lines · Cesar Eduardo Barros
This patch series refactors the
sys_swaponfunction.
sys_swaponis currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.One of these chunks of code was nearly identical to the part of
sys_swapoffwhich is used in case of a failure return fromtry_to_unuse(), so this patch series also makes both share the same code.
Quoted 8 lines
As a side effect of all this refactoring, the compiled code gets a bit smaller:
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.afterLightly tested on a
x86_64VM.
I have been working on reviewing/testing this set and I cannot get it to apply to Linus' tree, what is this set based on?
Thanks,
Eric
Em 01-03-2011 15:20, Eric B Munson escreveu:
Quoted 6 lines · Cesar Eduardo Barros
Quoted 2 lines
This patch series refactors the
sys_swaponfunction.I have been working on reviewing/testing this set and I cannot get it to apply to Linus' tree, what is this set based on?
According to the git tree from which I generated these patches, it was based on v2.6.38-rc4.
Commit 8074b26 (mm: fix refcounting in swapon) is what probably is causing you conflicts. I was planning to rebase and repost this patch series this weekend because of it.
I just did a quick rebase to Linus' current tree, and will post the whole set as a reply to this email. I have not even compile tested it, but the change is so small that, unless I made a typo when fixing the merge conflicts, it should work the same. The patches affected are 08 (context only), 10, and 13.
Signature · 10 lines
Cesar Eduardo Barros [email protected] [email protected] -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
v2 cover Refactor sys_swapon
This patch series refactors the sys_swapon function.
sys_swapon is currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.
One of these chunks of code was nearly identical to the part of sys_swapoff which is used in case of a failure return from try_to_unuse(), so this patch series also makes both share the same code.
As a side effect of all this refactoring, the compiled code gets a bit smaller (from v1 of this patch series):
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.after
The v1 of this patch series was lightly tested on a x86_64 VM.
Changes from v1:
Rebased from v2.6.38-rc4 to v2.6.38-rc7.
Cesar Eduardo Barros (24):
sys_swapon: use vzalloc instead of vmalloc/memset
sys_swapon: remove changelog from function comment
sys_swapon: do not depend on "type" after allocation
sys_swapon: separate swap_info allocation
sys_swapon: simplify error return from swap_info allocation
sys_swapon: simplify error flow in alloc_swap_info
sys_swapon: remove initial value of name variable
sys_swapon: move setting of error nearer use
sys_swapon: remove did_down variable
sys_swapon: remove bdev variable
sys_swapon: do only cleanup in the cleanup blocks
sys_swapon: use a single error label
sys_swapon: separate bdev claim and inode lock
sys_swapon: simplify error flow in claim_swapfile
sys_swapon: move setting of swapfilepages near use
sys_swapon: separate parsing of swapfile header
sys_swapon: simplify error flow in read_swap_header
sys_swapon: call swap_cgroup_swapon earlier
sys_swapon: separate parsing of bad blocks and extents
sys_swapon: simplify error flow in setup_swap_map_and_extents
sys_swapon: remove nr_good_pages variable
sys_swapon: move printk outside lock
sys_swapoff: change order to match sys_swapon
sys_swapon: separate final enabling of the swapfile
mm/swapfile.c | 360 +++++++++++++++++++++++++++++++-------------------------- 1 files changed, 197 insertions(+), 163 deletions(-)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 1/24 sys_swapon: use vzalloc instead of vmalloc/memset
--- mm/swapfile.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
mm/swapfile.c +1 -2
@@ -2047,13 +2047,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) goto bad_swap;/* OK, set up the swap map and apply the bad block list */ - swap_map =vmalloc (maxpages); + swap_map =vzalloc (maxpages);if (!swap_map) { error = -ENOMEM ; goto bad_swap; } -memset (swap_map,0 , maxpages); nr_good_pages = maxpages -1 ;/* omit header page */ for (i =0 ; i < swap_header->info.nr_badpages; i++) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 2/24 sys_swapon: remove changelog from function comment
In reply to Cesar Eduardo BarrosChangelogs belong in the git history instead of in the source code.
Also, "The swapon system call" is redundant with
"SYSCALL_DEFINE2(swapon, ...)".
--- mm/swapfile.c | 5 ----- 1 files changed, 0 insertions(+), 5 deletions(-)
mm/swapfile.c +0 -5
@@ -1844,11 +1844,6 @@ static int __init max_swapfiles_check(void)late_initcall (max_swapfiles_check);#endif -/* -* Written 01/25/92 by Simmule Turner, heavily changed by Linus. -* -* The swapon system call -*/ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 3/24 sys_swapon: do not depend on "type" after allocation
In reply to Cesar Eduardo BarrosWithin sys_swapon, after the swap_info entry has been allocated, we always have type == p->type and swap_info[type] == p. Use this fact to reduce the dependency on the "type" local variable within the function, as a preparation to move the allocation of the swap_info entry to a separate function.
--- mm/swapfile.c | 10 +++++----- 1 files changed, 5 insertions(+), 5 deletions(-)
mm/swapfile.c +5 -5
@@ -1927,7 +1927,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)for (i =0 ; i < nr_swapfiles; i++) {struct swap_info_struct *q = swap_info[i]; -if (i == type || !q->swap_file) +if (q == p || !q->swap_file)continue ;if (mapping == q->swap_file->f_mapping) goto bad_swap;
@@ -2062,7 +2062,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } - error =swap_cgroup_swapon (type, maxpages); + error =swap_cgroup_swapon (p->type, maxpages);if (error) goto bad_swap;
@@ -2120,9 +2120,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } p->next = i;if (prev <0 ) - swap_list.head = swap_list.next = type; + swap_list.head = swap_list.next = p->type;else - swap_info[prev]->next = type; + swap_info[prev]->next = p->type;spin_unlock (&swap_lock);mutex_unlock (&swapon_mutex);atomic_inc (&proc_poll_event);
@@ -2136,7 +2136,7 @@ bad_swap:blkdev_put (bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); }destroy_swap_extents (p); -swap_cgroup_swapoff (type); +swap_cgroup_swapoff (p->type); bad_swap_2:spin_lock (&swap_lock); p->swap_file =NULL ;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 4/24 sys_swapon: separate swap_info allocation
In reply to Cesar Eduardo BarrosMove the swap_info allocation to its own function. Only code movement, no functional changes.
--- mm/swapfile.c | 57 +++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 37 insertions(+), 20 deletions(-)
mm/swapfile.c +37 -20
@@ -1844,33 +1844,15 @@ static int __init max_swapfiles_check(void)late_initcall (max_swapfiles_check);#endif -SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) +static struct swap_info_struct *alloc_swap_info (void ) {struct swap_info_struct *p; -char *name =NULL ; -struct block_device *bdev =NULL ; -struct file *swap_file =NULL ; -struct address_space *mapping;unsigned int type; -int i, prev;int error; -union swap_header *swap_header; -unsigned int nr_good_pages; -int nr_extents =0 ; -sector_t span; -unsigned long maxpages; -unsigned long swapfilepages; -unsigned char *swap_map =NULL ; -struct page *page =NULL ; -struct inode *inode =NULL ; -int did_down =0 ; - -if (!capable (CAP_SYS_ADMIN )) -return -EPERM ; p =kzalloc (sizeof (*p),GFP_KERNEL );if (!p) -return -ENOMEM ; +return ERR_PTR (-ENOMEM );spin_lock (&swap_lock);for (type =0 ; type < nr_swapfiles; type++) {
@@ -1906,6 +1888,41 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) p->next =-1 ;spin_unlock (&swap_lock); +return p; + +out: +return ERR_PTR (error); +} + +SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) +{ +struct swap_info_struct *p; +char *name =NULL ; +struct block_device *bdev =NULL ; +struct file *swap_file =NULL ; +struct address_space *mapping; +int i, prev; +int error; +union swap_header *swap_header; +unsigned int nr_good_pages; +int nr_extents =0 ; +sector_t span; +unsigned long maxpages; +unsigned long swapfilepages; +unsigned char *swap_map =NULL ; +struct page *page =NULL ; +struct inode *inode =NULL ; +int did_down =0 ; + +if (!capable (CAP_SYS_ADMIN )) +return -EPERM ; + + p =alloc_swap_info (); +if (IS_ERR (p)) { + error =PTR_ERR (p); + goto out; + } + name =getname (specialfile); error =PTR_ERR (name);if (IS_ERR (name)) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 5/24 sys_swapon: simplify error return from swap_info allocation
In reply to Cesar Eduardo BarrosAt this point in sys_swapon, there is nothing to free. Return directly instead of jumping to the cleanup block at the end of the function.
--- mm/swapfile.c | 6 ++---- 1 files changed, 2 insertions(+), 4 deletions(-)
mm/swapfile.c +2 -4
@@ -1918,10 +1918,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)return -EPERM ; p =alloc_swap_info (); -if (IS_ERR (p)) { - error =PTR_ERR (p); - goto out; - } +if (IS_ERR (p)) +return PTR_ERR (p); name =getname (specialfile); error =PTR_ERR (name);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 6/24 sys_swapon: simplify error flow in alloc_swap_info
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 7 +------ 1 files changed, 1 insertions(+), 6 deletions(-)
mm/swapfile.c +1 -6
@@ -1848,7 +1848,6 @@ static struct swap_info_struct *alloc_swap_info(void) {struct swap_info_struct *p;unsigned int type; -int error; p =kzalloc (sizeof (*p),GFP_KERNEL );if (!p)
@@ -1859,11 +1858,10 @@ static struct swap_info_struct *alloc_swap_info(void)if (!(swap_info[type]->flags &SWP_USED ))break ; } - error = -EPERM ;if (type >=MAX_SWAPFILES ) {spin_unlock (&swap_lock);kfree (p); - goto out; +return ERR_PTR (-EPERM ); }if (type >= nr_swapfiles) { p->type = type;
@@ -1889,9 +1887,6 @@ static struct swap_info_struct *alloc_swap_info(void)spin_unlock (&swap_lock);return p; - -out: -return ERR_PTR (error); } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 7/24 sys_swapon: remove initial value of name variable
In reply to Cesar Eduardo BarrosNow there is nothing which jumps to the cleanup blocks before the name variable is set. There is no need to set it initially to NULL anymore.
--- mm/swapfile.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-)
mm/swapfile.c +1 -1
@@ -1892,7 +1892,7 @@ static struct swap_info_struct *alloc_swap_info(void) SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p; -char *name =NULL ; +char *name;struct block_device *bdev =NULL ;struct file *swap_file =NULL ;struct address_space *mapping;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 8/24 sys_swapon: move setting of error nearer use
In reply to Cesar Eduardo BarrosMove the setting of the error variable nearer the goto in a few places.
Avoids calling PTR_ERR() if not IS_ERR() in two places, and makes the error condition more explicit in two other places.
--- mm/swapfile.c | 11 ++++++----- 1 files changed, 6 insertions(+), 5 deletions(-)
mm/swapfile.c +6 -5
@@ -1917,14 +1917,14 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)return PTR_ERR (p); name =getname (specialfile); - error =PTR_ERR (name);if (IS_ERR (name)) { + error =PTR_ERR (name); name =NULL ; goto bad_swap_2; } swap_file =filp_open (name,O_RDWR |O_LARGEFILE ,0 ); - error =PTR_ERR (swap_file);if (IS_ERR (swap_file)) { + error =PTR_ERR (swap_file); swap_file =NULL ; goto bad_swap_2; }
@@ -1933,17 +1933,17 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) mapping = swap_file->f_mapping; inode = mapping->host; - error = -EBUSY ;for (i =0 ; i < nr_swapfiles; i++) {struct swap_info_struct *q = swap_info[i];if (q == p || !q->swap_file)continue ; -if (mapping == q->swap_file->f_mapping) +if (mapping == q->swap_file->f_mapping) { + error = -EBUSY ; goto bad_swap; + } } - error = -EINVAL ;if (S_ISBLK (inode->i_mode)) { bdev =bdgrab (I_BDEV (inode)); error = blkdev_get(bdev, FMODE_READ | FMODE_WRITE | FMODE_EXCL,
@@ -1968,6 +1968,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) goto bad_swap; } }else { + error = -EINVAL ; goto bad_swap; }
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 9/24 sys_swapon: remove did_down variable
In reply to Cesar Eduardo BarrosSince mutex_lock(&inode->i_mutex) is called just after setting inode, did_down is always equivalent to (inode && S_ISREG(inode->i_mode)).
Use this fact to remove the did_down variable.
--- mm/swapfile.c | 4 +--- 1 files changed, 1 insertions(+), 3 deletions(-)
mm/swapfile.c +1 -3
@@ -1907,7 +1907,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)unsigned char *swap_map =NULL ;struct page *page =NULL ;struct inode *inode =NULL ; -int did_down =0 ;if (!capable (CAP_SYS_ADMIN ))return -EPERM ;
@@ -1962,7 +1961,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) }else if (S_ISREG (inode->i_mode)) { p->bdev = inode->i_sb->s_bdev;mutex_lock (&inode->i_mutex); - did_down =1 ;if (IS_SWAPFILE (inode)) { error = -EBUSY ; goto bad_swap;
@@ -2163,7 +2161,7 @@ out: }if (name)putname (name); -if (did_down) { +if (inode &&S_ISREG (inode->i_mode)) {if (!error) inode->i_flags |=S_SWAPFILE ;mutex_unlock (&inode->i_mutex);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 10/24 sys_swapon: remove bdev variable
In reply to Cesar Eduardo BarrosThe bdev variable is always equivalent to (S_ISBLK(inode->i_mode) ? p->bdev : NULL), as long as it being set is moved to a bit earlier. Use this fact to remove the bdev variable.
--- mm/swapfile.c | 19 +++++++++---------- 1 files changed, 9 insertions(+), 10 deletions(-)
mm/swapfile.c +9 -10
@@ -1893,7 +1893,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;char *name; -struct block_device *bdev =NULL ;struct file *swap_file =NULL ;struct address_space *mapping;int i, prev;
@@ -1944,19 +1943,19 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) }if (S_ISBLK (inode->i_mode)) { - bdev =bdgrab (I_BDEV (inode)); - error =blkdev_get (bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL , + p->bdev =bdgrab (I_BDEV (inode)); + error =blkdev_get (p->bdev, +FMODE_READ |FMODE_WRITE |FMODE_EXCL , sys_swapon);if (error <0 ) { - bdev =NULL ; + p->bdev =NULL ; error = -EINVAL ; goto bad_swap; } - p->old_block_size =block_size (bdev); - error =set_blocksize (bdev,PAGE_SIZE ); + p->old_block_size =block_size (p->bdev); + error =set_blocksize (p->bdev,PAGE_SIZE );if (error <0 ) goto bad_swap; - p->bdev = bdev; p->flags |=SWP_BLKDEV ; }else if (S_ISREG (inode->i_mode)) { p->bdev = inode->i_sb->s_bdev;
@@ -2140,9 +2139,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error =0 ; goto out; bad_swap: -if (bdev) { -set_blocksize (bdev, p->old_block_size); -blkdev_put (bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); +if (S_ISBLK (inode->i_mode) && p->bdev) { +set_blocksize (p->bdev, p->old_block_size); +blkdev_put (p->bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); }destroy_swap_extents (p);swap_cgroup_swapoff (p->type);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 11/24 sys_swapon: do only cleanup in the cleanup blocks
In reply to Cesar Eduardo BarrosThe only way error is 0 in the cleanup blocks is when the function is returning successfully. In this case, the cleanup blocks were setting S_SWAPFILE in the S_ISREG case. But this is not a cleanup.
Move the setting of S_SWAPFILE to just before the "goto out;" to make this more clear. At this point, we do not need to test for inode because it will never be NULL.
--- mm/swapfile.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
mm/swapfile.c +3 -4
@@ -2136,6 +2136,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)atomic_inc (&proc_poll_event);wake_up_interruptible (&proc_poll_wait); +if (S_ISREG (inode->i_mode)) + inode->i_flags |=S_SWAPFILE ; error =0 ; goto out; bad_swap:
@@ -2160,11 +2162,8 @@ out: }if (name)putname (name); -if (inode &&S_ISREG (inode->i_mode)) { -if (!error) - inode->i_flags |=S_SWAPFILE ; +if (inode &&S_ISREG (inode->i_mode))mutex_unlock (&inode->i_mutex); - }return error; }
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 12/24 sys_swapon: use a single error label
In reply to Cesar Eduardo Barrossys_swapon currently has two error labels, bad_swap and bad_swap_2. bad_swap does the same as bad_swap_2 plus destroy_swap_extents() and swap_cgroup_swapoff(); both are noops in the places where bad_swap_2 is jumped to. With a single extra test for inode (matching the one in the S_ISREG case below), all the error paths in the function can go to bad_swap.
--- mm/swapfile.c | 7 +++---- 1 files changed, 3 insertions(+), 4 deletions(-)
mm/swapfile.c +3 -4
@@ -1918,13 +1918,13 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)if (IS_ERR (name)) { error =PTR_ERR (name); name =NULL ; - goto bad_swap_2; + goto bad_swap; } swap_file =filp_open (name,O_RDWR |O_LARGEFILE ,0 );if (IS_ERR (swap_file)) { error =PTR_ERR (swap_file); swap_file =NULL ; - goto bad_swap_2; + goto bad_swap; } p->swap_file = swap_file;
@@ -2141,13 +2141,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error =0 ; goto out; bad_swap: -if (S_ISBLK (inode->i_mode) && p->bdev) { +if (inode &&S_ISBLK (inode->i_mode) && p->bdev) {set_blocksize (p->bdev, p->old_block_size);blkdev_put (p->bdev,FMODE_READ |FMODE_WRITE |FMODE_EXCL ); }destroy_swap_extents (p);swap_cgroup_swapoff (p->type); -bad_swap_2:spin_lock (&swap_lock); p->swap_file =NULL ; p->flags =0 ;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 13/24 sys_swapon: separate bdev claim and inode lock
In reply to Cesar Eduardo BarrosMove the code which claims the bdev (S_ISBLK) or locks the inode (S_ISREG) to a separate function. Only code movement, no functional changes.
--- mm/swapfile.c | 64 ++++++++++++++++++++++++++++++++++---------------------- 1 files changed, 39 insertions(+), 25 deletions(-)
mm/swapfile.c +39 -25
@@ -1889,6 +1889,43 @@ static struct swap_info_struct *alloc_swap_info(void)return p; } +static int claim_swapfile (struct swap_info_struct *p,struct inode *inode) +{ +int error; + +if (S_ISBLK (inode->i_mode)) { + p->bdev =bdgrab (I_BDEV (inode)); + error =blkdev_get (p->bdev, +FMODE_READ |FMODE_WRITE |FMODE_EXCL , + sys_swapon); +if (error <0 ) { + p->bdev =NULL ; + error = -EINVAL ; + goto bad_swap; + } + p->old_block_size =block_size (p->bdev); + error =set_blocksize (p->bdev,PAGE_SIZE ); +if (error <0 ) + goto bad_swap; + p->flags |=SWP_BLKDEV ; + }else if (S_ISREG (inode->i_mode)) { + p->bdev = inode->i_sb->s_bdev; +mutex_lock (&inode->i_mutex); +if (IS_SWAPFILE (inode)) { + error = -EBUSY ; + goto bad_swap; + } + }else { + error = -EINVAL ; + goto bad_swap; + } + +return 0 ; + +bad_swap: +return error; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
@@ -1942,32 +1979,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } -if (S_ISBLK (inode->i_mode)) { - p->bdev =bdgrab (I_BDEV (inode)); - error =blkdev_get (p->bdev, -FMODE_READ |FMODE_WRITE |FMODE_EXCL , - sys_swapon); -if (error <0 ) { - p->bdev =NULL ; - error = -EINVAL ; - goto bad_swap; - } - p->old_block_size =block_size (p->bdev); - error =set_blocksize (p->bdev,PAGE_SIZE ); -if (error <0 ) - goto bad_swap; - p->flags |=SWP_BLKDEV ; - }else if (S_ISREG (inode->i_mode)) { - p->bdev = inode->i_sb->s_bdev; -mutex_lock (&inode->i_mutex); -if (IS_SWAPFILE (inode)) { - error = -EBUSY ; - goto bad_swap; - } - }else { - error = -EINVAL ; + error =claim_swapfile (p, inode); +if (unlikely (error)) goto bad_swap; - } swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Re: [PATCHv2 13/24] sys_swapon: separate bdev claim and inode lock
Nit pick below
Quoted 77 lines · Cesar Eduardo Barros
Move the code which claims the bdev (
S_ISBLK) or locks the inode (S_ISREG) to a separate function. Only code movement, no functional changes.Signed-off-by Cesar Eduardo Barros <[email protected]>--- mm/swapfile.c | 64 ++++++++++++++++++++++++++++++++++---------------------- 1 files changed, 39 insertions(+), 25 deletions(-)mm/swapfile.c +37
-16@@ -1889,6 +1889,43 @@ static struct swap_info_struct *alloc_swap_info(void)return p; } +static int claim_swapfile (struct swap_info_struct *p,struct inode *inode) +{ +int error; + +if (S_ISBLK (inode->i_mode)) { + p->bdev =bdgrab (I_BDEV (inode)); + error =blkdev_get (p->bdev, +FMODE_READ |FMODE_WRITE |FMODE_EXCL , + sys_swapon); +if (error <0 ) { + p->bdev =NULL ; + error = -EINVAL ; + goto bad_swap; + } + p->old_block_size =block_size (p->bdev); + error =set_blocksize (p->bdev,PAGE_SIZE ); +if (error <0 ) + goto bad_swap; + p->flags |=SWP_BLKDEV ; + }else if (S_ISREG (inode->i_mode)) { + p->bdev = inode->i_sb->s_bdev; +mutex_lock (&inode->i_mutex); +if (IS_SWAPFILE (inode)) { + error = -EBUSY ; + goto bad_swap; + } + }else { + error = -EINVAL ; + goto bad_swap; + } + +return 0 ; + +bad_swap: +return error; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;@@ -1942,32 +1979,9 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } -if (S_ISBLK (inode->i_mode)) { - p->bdev =bdgrab (I_BDEV (inode)); - error =blkdev_get (p->bdev, -FMODE_READ |FMODE_WRITE |FMODE_EXCL , - sys_swapon); -if (error <0 ) { - p->bdev =NULL ; - error = -EINVAL ; - goto bad_swap; - } - p->old_block_size =block_size (p->bdev); - error =set_blocksize (p->bdev,PAGE_SIZE ); -if (error <0 ) - goto bad_swap; - p->flags |=SWP_BLKDEV ; - }else if (S_ISREG (inode->i_mode)) {
mm/swapfile.c SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) · line 1983
Quoted 10 lines
mm/swapfile.c +2
-8- p->bdev = inode->i_sb->s_bdev; -mutex_lock (&inode->i_mutex); -if (IS_SWAPFILE (inode)) { - error = -EBUSY ; - goto bad_swap; - } - }else { - error = -EINVAL ; + error =claim_swapfile (p, inode); +if (unlikely (error))
As a personal preference, I don't use likely/unlikley unless I have a profiler telling me that the compiler got it wrong. Just a suggestion.
Quoted 8 lines
mm/swapfile.c +0
-1goto bad_swap; - } swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;Signature · 1 line
1.7.4
Re: [PATCHv2 13/24] sys_swapon: separate bdev claim and inode lock
Em 02-03-2011 18:40, Eric B Munson escreveu:
Quoted 7 lines
mm/swapfile.c SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) · line 1983
Quoted 4 lines
mm/swapfile.c +2
-2- }else { - error = -EINVAL ; + error =claim_swapfile (p, inode); +if (unlikely (error))As a personal preference, I don't use likely/unlikley unless I have a profiler telling me that the compiler got it wrong. Just a suggestion.
I tend to use them for paths which should never happen in normal operation (error paths mostly). But yeah, I am probably still overusing them - who says the error path is not normal in some cases? Old habits die hard...
And I added more unlikely() calls than are visible in the patches. Remember that every IS_ERR() counts as a unlikely() too.
Signature · 10 lines
Cesar Eduardo Barros [email protected] [email protected] -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 14/24 sys_swapon: simplify error flow in claim_swapfile
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 20 ++++++-------------- 1 files changed, 6 insertions(+), 14 deletions(-)
mm/swapfile.c +6 -14
@@ -1900,30 +1900,22 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode) sys_swapon);if (error <0 ) { p->bdev =NULL ; - error = -EINVAL ; - goto bad_swap; +return -EINVAL ; } p->old_block_size =block_size (p->bdev); error =set_blocksize (p->bdev,PAGE_SIZE );if (error <0 ) - goto bad_swap; +return error; p->flags |=SWP_BLKDEV ; }else if (S_ISREG (inode->i_mode)) { p->bdev = inode->i_sb->s_bdev;mutex_lock (&inode->i_mutex); -if (IS_SWAPFILE (inode)) { - error = -EBUSY ; - goto bad_swap; - } - }else { - error = -EINVAL ; - goto bad_swap; - } +if (IS_SWAPFILE (inode)) +return -EBUSY ; + }else +return -EINVAL ;return 0 ; - -bad_swap: -return error; } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 15/24 sys_swapon: move setting of swapfilepages near use
In reply to Cesar Eduardo BarrosThere is no reason I can see to read inode->i_size long before it is needed. Move its read to just before it is needed, to reduce the variable lifetime.
--- mm/swapfile.c | 3 +-- 1 files changed, 1 insertions(+), 2 deletions(-)
mm/swapfile.c +1 -2
@@ -1975,8 +1975,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)if (unlikely (error)) goto bad_swap; - swapfilepages =i_size_read (inode) >>PAGE_SHIFT ; -/* * Read the swap header. */
@@ -2045,6 +2043,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error = -EINVAL ;if (!maxpages) goto bad_swap; + swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;if (swapfilepages && maxpages > swapfilepages) {printk (KERN_WARNING "Swap area shorter than signature indicates\n" );
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 16/24 sys_swapon: separate parsing of swapfile header
In reply to Cesar Eduardo BarrosMove the code which parses and checks the swapfile header (except for the bad block list) to a separate function. Only code movement, no functional changes.
--- mm/swapfile.c | 140 ++++++++++++++++++++++++++++++++------------------------- 1 files changed, 78 insertions(+), 62 deletions(-)
mm/swapfile.c +78 -62
@@ -1918,6 +1918,82 @@ static int claim_swapfile(struct swap_info_struct *p, struct inode *inode)return 0 ; } +static unsigned long read_swap_header (struct swap_info_struct *p, +union swap_header *swap_header, +struct inode *inode) +{ +int i; +unsigned long maxpages; +unsigned long swapfilepages; + +if (memcmp ("SWAPSPACE2" , swap_header->magic.magic,10 )) { +printk (KERN_ERR "Unable to find swap-space signature\n" ); + goto bad_swap; + } + +/* swap partition endianess hack... */ +if (swab32 (swap_header->info.version) ==1 ) { +swab32s (&swap_header->info.version); +swab32s (&swap_header->info.last_page); +swab32s (&swap_header->info.nr_badpages); +for (i =0 ; i < swap_header->info.nr_badpages; i++) +swab32s (&swap_header->info.badpages[i]); + } +/* Check the swap header's sub-version */ +if (swap_header->info.version !=1 ) { +printk (KERN_WARNING +"Unable to handle swap header version %d\n" , + swap_header->info.version); + goto bad_swap; + } + + p->lowest_bit =1 ; + p->cluster_next =1 ; + p->cluster_nr =0 ; + +/* +* Find out how many pages are allowed for a single swap +* device. There are two limiting factors: 1) the number of +* bits for the swap offset in the swp_entry_t type and +* 2) the number of bits in the a swap pte as defined by +* the different architectures. In order to find the +* largest possible bit mask a swap entry with swap type 0 +* and swap offset ~0UL is created, encoded to a swap pte, +* decoded to a swp_entry_t again and finally the swap +* offset is extracted. This will mask all the bits from +* the initial ~0UL mask that can't be encoded in either +* the swp_entry_t or the architecture definition of a +* swap pte. +*/ + maxpages =swp_offset (pte_to_swp_entry ( +swp_entry_to_pte (swp_entry (0 , ~0UL )))) +1 ; +if (maxpages > swap_header->info.last_page) { + maxpages = swap_header->info.last_page +1 ; +/* p->max is an unsigned int: don't overflow it */ +if ((unsigned int )maxpages ==0 ) + maxpages =UINT_MAX ; + } + p->highest_bit = maxpages -1 ; + +if (!maxpages) + goto bad_swap; + swapfilepages =i_size_read (inode) >>PAGE_SHIFT ; +if (swapfilepages && maxpages > swapfilepages) { +printk (KERN_WARNING +"Swap area shorter than signature indicates\n" ); + goto bad_swap; + } +if (swap_header->info.nr_badpages &&S_ISREG (inode->i_mode)) + goto bad_swap; +if (swap_header->info.nr_badpages >MAX_SWAP_BADPAGES ) + goto bad_swap; + +return maxpages; + +bad_swap: +return 0 ; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
@@ -1931,7 +2007,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)int nr_extents =0 ;sector_t span;unsigned long maxpages; -unsigned long swapfilepages;unsigned char *swap_map =NULL ;struct page *page =NULL ;struct inode *inode =NULL ;
@@ -1989,71 +2064,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } swap_header =kmap (page); -if (memcmp ("SWAPSPACE2" , swap_header->magic.magic,10 )) { -printk (KERN_ERR "Unable to find swap-space signature\n" ); + maxpages =read_swap_header (p, swap_header, inode); +if (unlikely (!maxpages)) { error = -EINVAL ; goto bad_swap; } -/* swap partition endianess hack... */ -if (swab32 (swap_header->info.version) ==1 ) { -swab32s (&swap_header->info.version); -swab32s (&swap_header->info.last_page); -swab32s (&swap_header->info.nr_badpages); -for (i =0 ; i < swap_header->info.nr_badpages; i++) -swab32s (&swap_header->info.badpages[i]); - } -/* Check the swap header's sub-version */ -if (swap_header->info.version !=1 ) { -printk (KERN_WARNING -"Unable to handle swap header version %d\n" , - swap_header->info.version); - error = -EINVAL ; - goto bad_swap; - } - - p->lowest_bit =1 ; - p->cluster_next =1 ; - p->cluster_nr =0 ; - -/* -* Find out how many pages are allowed for a single swap -* device. There are two limiting factors: 1) the number of -* bits for the swap offset in the swp_entry_t type and -* 2) the number of bits in the a swap pte as defined by -* the different architectures. In order to find the -* largest possible bit mask a swap entry with swap type 0 -* and swap offset ~0UL is created, encoded to a swap pte, -* decoded to a swp_entry_t again and finally the swap -* offset is extracted. This will mask all the bits from -* the initial ~0UL mask that can't be encoded in either -* the swp_entry_t or the architecture definition of a -* swap pte. -*/ - maxpages =swp_offset (pte_to_swp_entry ( -swp_entry_to_pte (swp_entry (0 , ~0UL )))) +1 ; -if (maxpages > swap_header->info.last_page) { - maxpages = swap_header->info.last_page +1 ; -/* p->max is an unsigned int: don't overflow it */ -if ((unsigned int )maxpages ==0 ) - maxpages =UINT_MAX ; - } - p->highest_bit = maxpages -1 ; - - error = -EINVAL ; -if (!maxpages) - goto bad_swap; - swapfilepages =i_size_read (inode) >>PAGE_SHIFT ; -if (swapfilepages && maxpages > swapfilepages) { -printk (KERN_WARNING -"Swap area shorter than signature indicates\n" ); - goto bad_swap; - } -if (swap_header->info.nr_badpages &&S_ISREG (inode->i_mode)) - goto bad_swap; -if (swap_header->info.nr_badpages >MAX_SWAP_BADPAGES ) - goto bad_swap; -/* OK, set up the swap map and apply the bad block list */ swap_map =vzalloc (maxpages);if (!swap_map) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 17/24 sys_swapon: simplify error flow in read_swap_header
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 15 ++++++--------- 1 files changed, 6 insertions(+), 9 deletions(-)
mm/swapfile.c +6 -9
@@ -1928,7 +1928,7 @@ static unsigned long read_swap_header(struct swap_info_struct *p,if (memcmp ("SWAPSPACE2" , swap_header->magic.magic,10 )) {printk (KERN_ERR "Unable to find swap-space signature\n" ); - goto bad_swap; +return 0 ; }/* swap partition endianess hack... */
@@ -1944,7 +1944,7 @@ static unsigned long read_swap_header(struct swap_info_struct *p,printk (KERN_WARNING "Unable to handle swap header version %d\n" , swap_header->info.version); - goto bad_swap; +return 0 ; } p->lowest_bit =1 ;
@@ -1976,22 +1976,19 @@ static unsigned long read_swap_header(struct swap_info_struct *p, p->highest_bit = maxpages -1 ;if (!maxpages) - goto bad_swap; +return 0 ; swapfilepages =i_size_read (inode) >>PAGE_SHIFT ;if (swapfilepages && maxpages > swapfilepages) {printk (KERN_WARNING "Swap area shorter than signature indicates\n" ); - goto bad_swap; +return 0 ; }if (swap_header->info.nr_badpages &&S_ISREG (inode->i_mode)) - goto bad_swap; +return 0 ;if (swap_header->info.nr_badpages >MAX_SWAP_BADPAGES ) - goto bad_swap; +return 0 ;return maxpages; - -bad_swap: -return 0 ; } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 18/24 sys_swapon: call swap_cgroup_swapon earlier
In reply to Cesar Eduardo BarrosThe call to swap_cgroup_swapon is in the middle of loading the swap map and extents. As it only does memory allocation and does not depend on the swapfile layout (map/extents), it can be called earlier (or later).
Move it to just after the allocation of swap_map, since it is conceptually similar (allocates a map).
--- mm/swapfile.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-)
mm/swapfile.c +4 -4
@@ -2074,6 +2074,10 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) goto bad_swap; } + error =swap_cgroup_swapon (p->type, maxpages); +if (error) + goto bad_swap; + nr_good_pages = maxpages -1 ;/* omit header page */ for (i =0 ; i < swap_header->info.nr_badpages; i++) {
@@ -2088,10 +2092,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) } } - error =swap_cgroup_swapon (p->type, maxpages); -if (error) - goto bad_swap; -if (nr_good_pages) { swap_map[0 ] =SWAP_MAP_BAD ; p->max = maxpages;
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 19/24 sys_swapon: separate parsing of bad blocks and extents
In reply to Cesar Eduardo BarrosMove the code which parses the bad block list and the extents to a separate function. Only code movement, no functional changes.
This change uses the fact that, after the success path, nr_good_pages == p->pages.
--- mm/swapfile.c | 83 +++++++++++++++++++++++++++++++++++++-------------------- 1 files changed, 54 insertions(+), 29 deletions(-)
mm/swapfile.c +54 -29
@@ -1991,6 +1991,54 @@ static unsigned long read_swap_header(struct swap_info_struct *p,return maxpages; } +static int setup_swap_map_and_extents (struct swap_info_struct *p, +union swap_header *swap_header, +unsigned char *swap_map, +unsigned long maxpages, +sector_t *span) +{ +int i; +int error; +unsigned int nr_good_pages; +int nr_extents; + + nr_good_pages = maxpages -1 ;/* omit header page */ + +for (i =0 ; i < swap_header->info.nr_badpages; i++) { +unsigned int page_nr = swap_header->info.badpages[i]; +if (page_nr ==0 || page_nr > swap_header->info.last_page) { + error = -EINVAL ; + goto bad_swap; + } +if (page_nr < maxpages) { + swap_map[page_nr] =SWAP_MAP_BAD ; + nr_good_pages--; + } + } + +if (nr_good_pages) { + swap_map[0 ] =SWAP_MAP_BAD ; + p->max = maxpages; + p->pages = nr_good_pages; + nr_extents =setup_swap_extents (p, span); +if (nr_extents <0 ) { + error = nr_extents; + goto bad_swap; + } + nr_good_pages = p->pages; + } +if (!nr_good_pages) { +printk (KERN_WARNING "Empty swap-file\n" ); + error = -EINVAL ; + goto bad_swap; + } + +return nr_extents; + +bad_swap: +return error; +} + SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) {struct swap_info_struct *p;
@@ -2001,7 +2049,7 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)int error;union swap_header *swap_header;unsigned int nr_good_pages; -int nr_extents =0 ; +int nr_extents;sector_t span;unsigned long maxpages;unsigned char *swap_map =NULL ;
@@ -2078,36 +2126,13 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)if (error) goto bad_swap; - nr_good_pages = maxpages -1 ;/* omit header page */ - -for (i =0 ; i < swap_header->info.nr_badpages; i++) { -unsigned int page_nr = swap_header->info.badpages[i]; -if (page_nr ==0 || page_nr > swap_header->info.last_page) { - error = -EINVAL ; - goto bad_swap; - } -if (page_nr < maxpages) { - swap_map[page_nr] =SWAP_MAP_BAD ; - nr_good_pages--; - } - } - -if (nr_good_pages) { - swap_map[0 ] =SWAP_MAP_BAD ; - p->max = maxpages; - p->pages = nr_good_pages; - nr_extents =setup_swap_extents (p, &span); -if (nr_extents <0 ) { - error = nr_extents; - goto bad_swap; - } - nr_good_pages = p->pages; - } -if (!nr_good_pages) { -printk (KERN_WARNING "Empty swap-file\n" ); - error = -EINVAL ; + nr_extents =setup_swap_map_and_extents (p, swap_header, swap_map, + maxpages, &span); +if (unlikely (nr_extents <0 )) { + error = nr_extents; goto bad_swap; } + nr_good_pages = p->pages;if (p->bdev) {if (blk_queue_nonrot (bdev_get_queue (p->bdev))) {
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 20/24 sys_swapon: simplify error flow in setup_swap_map_and_extents
In reply to Cesar Eduardo BarrosSince there is no cleanup to do, there is no reason to jump to a label. Return directly instead.
--- mm/swapfile.c | 19 +++++-------------- 1 files changed, 5 insertions(+), 14 deletions(-)
mm/swapfile.c +5 -14
@@ -1998,7 +1998,6 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p, sector_t *span) {int i; -int error;unsigned int nr_good_pages;int nr_extents;
@@ -2006,10 +2005,8 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p,for (i =0 ; i < swap_header->info.nr_badpages; i++) {unsigned int page_nr = swap_header->info.badpages[i]; -if (page_nr ==0 || page_nr > swap_header->info.last_page) { - error = -EINVAL ; - goto bad_swap; - } +if (page_nr ==0 || page_nr > swap_header->info.last_page) +return -EINVAL ;if (page_nr < maxpages) { swap_map[page_nr] =SWAP_MAP_BAD ; nr_good_pages--;
@@ -2021,22 +2018,16 @@ static int setup_swap_map_and_extents(struct swap_info_struct *p, p->max = maxpages; p->pages = nr_good_pages; nr_extents =setup_swap_extents (p, span); -if (nr_extents <0 ) { - error = nr_extents; - goto bad_swap; - } +if (nr_extents <0 ) +return nr_extents; nr_good_pages = p->pages; }if (!nr_good_pages) {printk (KERN_WARNING "Empty swap-file\n" ); - error = -EINVAL ; - goto bad_swap; +return -EINVAL ; }return nr_extents; - -bad_swap: -return error; } SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 21/24 sys_swapon: remove nr_good_pages variable
In reply to Cesar Eduardo BarrosIt still exists within setup_swap_map_and_extents(), but after it nr_good_pages == p->pages.
--- mm/swapfile.c | 8 +++----- 1 files changed, 3 insertions(+), 5 deletions(-)
mm/swapfile.c +3 -5
@@ -2039,7 +2039,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)int i, prev;int error;union swap_header *swap_header; -unsigned int nr_good_pages;int nr_extents;sector_t span;unsigned long maxpages;
@@ -2123,7 +2122,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) error = nr_extents; goto bad_swap; } - nr_good_pages = p->pages;if (p->bdev) {if (blk_queue_nonrot (bdev_get_queue (p->bdev))) {
@@ -2143,12 +2141,12 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) p->prio = --least_priority; p->swap_map = swap_map; p->flags |=SWP_WRITEOK ; - nr_swap_pages += nr_good_pages; - total_swap_pages += nr_good_pages; + nr_swap_pages += p->pages; + total_swap_pages += p->pages;printk (KERN_INFO "Adding %uk swap on %s. " "Priority:%d extents:%d across:%lluk %s%s\n" , - nr_good_pages<<(PAGE_SHIFT -10 ), name, p->prio, + p->pages<<(PAGE_SHIFT -10 ), name, p->prio, nr_extents, (unsigned long long )span<<(PAGE_SHIFT -10 ), (p->flags &SWP_SOLIDSTATE ) ?"SS" :"" , (p->flags &SWP_DISCARDABLE ) ?"D" :"" );
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 22/24 sys_swapon: move printk outside lock
In reply to Cesar Eduardo BarrosThe block in sys_swapon which does the final adjustments to the swap_info_struct and to swap_list is the same as the block which re-inserts it again at sys_swapoff on failure of try_to_unuse(). To be able to make both share the same code, move the printk() call in the middle of it to just after it.
--- mm/swapfile.c | 15 ++++++++------- 1 files changed, 8 insertions(+), 7 deletions(-)
mm/swapfile.c +8 -7
@@ -2144,13 +2144,6 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) nr_swap_pages += p->pages; total_swap_pages += p->pages; -printk (KERN_INFO "Adding %uk swap on %s. " -"Priority:%d extents:%d across:%lluk %s%s\n" , - p->pages<<(PAGE_SHIFT -10 ), name, p->prio, - nr_extents, (unsigned long long )span<<(PAGE_SHIFT -10 ), - (p->flags &SWP_SOLIDSTATE ) ?"SS" :"" , - (p->flags &SWP_DISCARDABLE ) ?"D" :"" ); -/* insert swap space into swap_list: */ prev =-1 ;for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) {
@@ -2164,6 +2157,14 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)else swap_info[prev]->next = p->type;spin_unlock (&swap_lock); + +printk (KERN_INFO "Adding %uk swap on %s. " +"Priority:%d extents:%d across:%lluk %s%s\n" , + p->pages<<(PAGE_SHIFT -10 ), name, p->prio, + nr_extents, (unsigned long long )span<<(PAGE_SHIFT -10 ), + (p->flags &SWP_SOLIDSTATE ) ?"SS" :"" , + (p->flags &SWP_DISCARDABLE ) ?"D" :"" ); +mutex_unlock (&swapon_mutex);atomic_inc (&proc_poll_event);wake_up_interruptible (&proc_poll_wait);
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 23/24 sys_swapoff: change order to match sys_swapon
In reply to Cesar Eduardo BarrosThe block in sys_swapon which does the final adjustments to the swap_info_struct and to swap_list is the same as the block which re-inserts it again at sys_swapoff on failure of try_to_unuse(), except for the order of the operations within the lock. Since the order should not matter, arbitrarily change sys_swapoff to match sys_swapon, in preparation to making both share the same code.
--- mm/swapfile.c | 7 ++++--- 1 files changed, 4 insertions(+), 3 deletions(-)
mm/swapfile.c +4 -3
@@ -1625,6 +1625,10 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile)spin_lock (&swap_lock);if (p->prio <0 ) p->prio = --least_priority; + p->flags |=SWP_WRITEOK ; + nr_swap_pages += p->pages; + total_swap_pages += p->pages; + prev =-1 ;for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) {if (p->prio >= swap_info[i]->prio)
@@ -1636,9 +1640,6 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) swap_list.head = swap_list.next = type; else swap_info[prev]->next = type; - nr_swap_pages += p->pages; - total_swap_pages += p->pages; - p->flags |=SWP_WRITEOK ;spin_unlock (&swap_lock); goto out_dput; }
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
PATCH v2 24/24 sys_swapon: separate final enabling of the swapfile
In reply to Cesar Eduardo BarrosThe block in sys_swapon which does the final adjustments to the swap_info_struct and to swap_list is the same as the block which re-inserts it again at sys_swapoff on failure of try_to_unuse(). Move this code to a separate function, and use it both in sys_swapon and sys_swapoff.
--- mm/swapfile.c | 84 ++++++++++++++++++++++++++++---------------------------- 1 files changed, 42 insertions(+), 42 deletions(-)
mm/swapfile.c +42 -42
@@ -1550,6 +1550,36 @@ bad_bmap: goto out; } +static void enable_swap_info (struct swap_info_struct *p,int prio, +unsigned char *swap_map) +{ +int i, prev; + +spin_lock (&swap_lock); +if (prio >=0 ) + p->prio = prio; +else + p->prio = --least_priority; + p->swap_map = swap_map; + p->flags |=SWP_WRITEOK ; + nr_swap_pages += p->pages; + total_swap_pages += p->pages; + +/* insert swap space into swap_list: */ + prev =-1 ; +for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) { +if (p->prio >= swap_info[i]->prio) +break ; + prev = i; + } + p->next = i; +if (prev <0 ) + swap_list.head = swap_list.next = p->type; +else + swap_info[prev]->next = p->type; +spin_unlock (&swap_lock); +} + SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) {struct swap_info_struct *p =NULL ;
@@ -1621,26 +1651,14 @@ SYSCALL_DEFINE1(swapoff, const char __user *, specialfile) current->flags &= ~PF_OOM_ORIGIN ;if (err) { +/* +* reading p->prio and p->swap_map outside the lock is +* safe here because only sys_swapon and sys_swapoff +* change them, and there can be no other sys_swapon or +* sys_swapoff for this swap_info_struct at this point. +*/ /* re-insert swap space back into swap_list */ -spin_lock (&swap_lock); -if (p->prio <0 ) - p->prio = --least_priority; - p->flags |=SWP_WRITEOK ; - nr_swap_pages += p->pages; - total_swap_pages += p->pages; - - prev =-1 ; -for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) { -if (p->prio >= swap_info[i]->prio) -break ; - prev = i; - } - p->next = i; -if (prev <0 ) - swap_list.head = swap_list.next = type; -else - swap_info[prev]->next = type; -spin_unlock (&swap_lock); +enable_swap_info (p, p->prio, p->swap_map); goto out_dput; }
@@ -2037,7 +2055,8 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags)char *name;struct file *swap_file =NULL ;struct address_space *mapping; -int i, prev; +int i; +int prio;int error;union swap_header *swap_header;int nr_extents;
@@ -2134,30 +2153,11 @@ SYSCALL_DEFINE2(swapon, const char __user *, specialfile, int, swap_flags) }mutex_lock (&swapon_mutex); -spin_lock (&swap_lock); + prio =-1 ;if (swap_flags &SWAP_FLAG_PREFER ) - p->prio = + prio = (swap_flags &SWAP_FLAG_PRIO_MASK ) >>SWAP_FLAG_PRIO_SHIFT ; -else - p->prio = --least_priority; - p->swap_map = swap_map; - p->flags |=SWP_WRITEOK ; - nr_swap_pages += p->pages; - total_swap_pages += p->pages; - -/* insert swap space into swap_list: */ - prev =-1 ; -for (i = swap_list.head; i >=0 ; i = swap_info[i]->next) { -if (p->prio >= swap_info[i]->prio) -break ; - prev = i; - } - p->next = i; -if (prev <0 ) - swap_list.head = swap_list.next = p->type; -else - swap_info[prev]->next = p->type; -spin_unlock (&swap_lock); +enable_swap_info (p, prio, swap_map); printk(KERN_INFO "Adding %uk swap on %s. " "Priority:%d extents:%d across:%lluk %s%s\n",
Signature · 8 lines
1.7.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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 15 lines · Cesar Eduardo Barros
This patch series refactors the
sys_swaponfunction.
sys_swaponis currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.One of these chunks of code was nearly identical to the part of
sys_swapoffwhich is used in case of a failure return fromtry_to_unuse(), so this patch series also makes both share the same code.As a side effect of all this refactoring, the compiled code gets a bit smaller (from v1 of this patch series):
Quoted 8 lines
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.afterThe v1 of this patch series was lightly tested on a
x86_64VM.Changes from v1:
Rebased from v2.6.38-rc4 to v2.6.38-rc7.
I have tested this set on x86_64 by manually adding/removing swap files and partitions. Also I used the hugeadm program (fomr libhugetlbfs) to add temporary swap files on disk and using ram disks. All of this worked well.
This can be applied to the entire set.
Quoted 15 lines · Cesar Eduardo Barros
This patch series refactors the
sys_swaponfunction.
sys_swaponis currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.One of these chunks of code was nearly identical to the part of
sys_swapoffwhich is used in case of a failure return fromtry_to_unuse(), so this patch series also makes both share the same code.As a side effect of all this refactoring, the compiled code gets a bit smaller (from v1 of this patch series):
Quoted 8 lines
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.afterThe v1 of this patch series was lightly tested on a
x86_64VM.Changes from v1:
Rebased from v2.6.38-rc4 to v2.6.38-rc7.
Aside from my preference to avoid likely/unlikely unless necessary, I don't see any problems with this series.
For the entire series
Quoted 12 lines · Cesar Eduardo Barros
This patch series refactors the
sys_swaponfunction.
sys_swaponis currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.One of these chunks of code was nearly identical to the part of
sys_swapoffwhich is used in case of a failure return fromtry_to_unuse(), so this patch series also makes both share the same code.
Quoted 8 lines
As a side effect of all this refactoring, the compiled code gets a bit smaller (from v1 of this patch series):
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.afterThe v1 of this patch series was lightly tested on a
x86_64VM.
One more small suggestion, you should cc LKML on this series, as well as any of the other emails suggested by get_maintainer.pl.
Em 03-03-2011 13:15, Eric B Munson escreveu:
Quoted 17 lines · Cesar Eduardo Barros
Quoted 16 lines
This patch series refactors the
sys_swaponfunction.
sys_swaponis currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.One of these chunks of code was nearly identical to the part of
sys_swapoffwhich is used in case of a failure return fromtry_to_unuse(), so this patch series also makes both share the same code.As a side effect of all this refactoring, the compiled code gets a bit smaller (from v1 of this patch series):
Quoted 8 lines
Quoted 5 lines
text data bss dec hex filename 14012 944 276 15232 3b80 mm/swapfile.o.before 13941 944 276 15161 3b39 mm/swapfile.o.afterThe v1 of this patch series was lightly tested on a
x86_64VM.One more small suggestion, you should cc LKML on this series, as well as any of the other emails suggested by get_maintainer.pl.
Should I resend the whole patch series with the correct Cc:?
Signature · 10 lines
Cesar Eduardo Barros [email protected] [email protected] -- 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/ . Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/ Don't email: <a href=mailto:"[email protected]"> [email protected] </a>
Quoted 25 lines · Cesar Eduardo Barros
Em 03-03-2011 13:15, Eric B Munson escreveu:
Quoted 24 lines · Cesar Eduardo Barros
Quoted 22 lines
This patch series refactors the
sys_swaponfunction.
sys_swaponis currently a very large function, with 313 lines (more than 12 25-line screens), which can make it a bit hard to read. This patch series reduces this size by half, by extracting large chunks of related code to new helper functions.One of these chunks of code was nearly identical to the part of
sys_swapoffwhich is used in case of a failure return fromtry_to_unuse(), so this patch series also makes both share the same code.As a side effect of all this refactoring, the compiled code gets a bit smaller (from v1 of this patch series):
text data bss dec hex filename 14012 944 276 15232 3b80
mm/swapfile.o.before
13941 944 276 15161 3b39mm/swapfile.o.after
Quoted 8 lines
Quoted 5 lines
Quoted 1 line
The v1 of this patch series was lightly tested on a
x86_64VM.One more small suggestion, you should cc LKML on this series, as well as any
of the other emails suggested by get_maintainer.pl.Should I resend the whole patch series with the correct Cc:?
I would and you can add my Tested-by: and Acked-by: to each patch as well.
--
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/ .
Fight unfair telecom internet charges in Canada: sign http://stopthemeter.ca/
Don't email: <a href=mailto:"[email protected]"> [email protected] </a>