drop vmtruncate

PATCH Superseded Marco Stornelli 2 participants 3 messages Aug 31, 2012 – Sep 7, 2012

Subsystems (2)

Memory management VFS

lore

Share & tools

lore

Marco Stornelli

author

2012-08-31 13:50 UTC #0 lore raw

cover drop vmtruncate

Hi all,

with this patch series I try to clean the vmtruncate code. The theory of operation:

old new
vmtruncate() => inode_newsize_ok+truncate_setsize+fs truncate

Where vmtruncate was used without any error check, the code now is:

if (inode_newsize_ok() == 0) {
	truncate_setsize();
	fs truncate();
}

So, performance and semantic nothing change at all. I think that maybe in some point we can skip inode_newsize_ok (where the error check of vmtruncate wasn't used) but since there is a swap check in case of no-extension, maybe it's better to avoid regressions. After this clean, of course, each fs can clean in a deeply way.

With these patches even the inode truncate callback is deleted.

Any comments/feedback/bugs are welcome.

Marco Stornelli (21):
ufs: drop vmtruncate
sysv: drop vmtruncate
reiserfs: drop vmtruncate
procfs: drop vmtruncate
omfs: drop vmtruncate
ocfs2: drop vmtruncate
adfs: drop vmtruncate
affs: drop vmtruncate
bfs: drop vmtruncate
hfs: drop vmtruncate
hpfs: drop vmtruncate
jfs: drop vmtruncate
hfsplus: drop vmtruncate
hostfs: drop vmtruncate
logfs: drop vmtruncate
minix: drop vmtruncate
ncpfs: drop vmtruncate
nilfs2: drop vmtruncate
ntfs: drop vmtruncate
vfs: drop vmtruncate
mm: drop vmtruncate

 fs/adfs/inode.c         |    5 +++--
 fs/affs/file.c          |    8 +++++---
 fs/affs/inode.c         |    5 ++++-
 fs/bfs/file.c           |    5 +++--
 fs/hfs/inode.c          |   19 +++++++++++++------
 fs/hfsplus/inode.c      |   19 +++++++++++++------
 fs/hostfs/hostfs_kern.c |    8 +++++---
 fs/hpfs/file.c          |    8 +++++---
 fs/hpfs/inode.c         |    5 ++++-
 fs/jfs/file.c           |    6 ++++--
 fs/jfs/inode.c          |   13 +++++++++----
 fs/libfs.c              |    2 --
 fs/logfs/readwrite.c    |   10 ++++++++--
 fs/minix/file.c         |    6 ++++--
 fs/minix/inode.c        |    7 +++++--
 fs/ncpfs/inode.c        |    4 +++-
 fs/nilfs2/file.c        |    1 -
 fs/nilfs2/inode.c       |   18 +++++++++++++-----
 fs/nilfs2/recovery.c    |    7 +++++--
 fs/ntfs/file.c          |    8 +++++---
 fs/ntfs/inode.c         |   11 +++++++++--
 fs/ntfs/inode.h         |    4 ++++
 fs/ocfs2/file.c         |    3 ++-
 fs/omfs/file.c          |   12 ++++++++----
 fs/proc/base.c          |    3 ++-
 fs/proc/generic.c       |    3 ++-
 fs/proc/proc_sysctl.c   |    3 ++-
 fs/reiserfs/file.c      |    3 +--
 fs/reiserfs/inode.c     |   15 +++++++++++----
 fs/reiserfs/reiserfs.h  |    1 +
 fs/sysv/file.c          |    5 +++--
 fs/sysv/itree.c         |    7 +++++--
 fs/ufs/inode.c          |    5 +++--
 include/linux/fs.h      |    1 -
 include/linux/mm.h      |    1 -
 mm/truncate.c           |   23 -----------------------
 36 files changed, 164 insertions(+), 100 deletions(-)

Signature · 8 lines

1.7.3.4
---

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

Lukáš Czerner

2012-09-06 17:04 UTC #1 lore raw

Quoted 27 lines · Marco Stornelli

Date: Fri, 31 Aug 2012 15:50:20 +0200
From: Marco Stornelli <[email protected]>
To: Linux FS Devel <[email protected]>, [email protected]
Cc: Linux Kernel <[email protected]>
Subject: [PATCH 00/21] drop vmtruncate
Hi all,
with this patch series I try to clean the vmtruncate code. The theory of operation:
old new
vmtruncate() => inode_newsize_ok+truncate_setsize+fs truncate
Where vmtruncate was used without any error check, the code now is:
if (inode_newsize_ok() == 0) {
	truncate_setsize();
	fs truncate();
}
So, performance and semantic nothing change at all. I think that maybe in some point we can skip inode_newsize_ok (where the error check of vmtruncate wasn't used) but since there is a swap check in case of no-extension, maybe it's better to avoid regressions. After this clean, of course, each fs can clean in a deeply way.

Quoted 3 lines

With these patches even the inode truncate callback is deleted.
Any comments/feedback/bugs are welcome.

Could you explain the reason behind this change a little bit more ? This does not make any sense to me since you're replacing vmtruncate() which does basically

if (inode_newsize_ok() == 0) {
	truncate_setsize();
	fs truncate();
}

as you mentioned above by exactly the same thing but doing it within the file system. It does not seem like an improvement to me ... how is this a clean up ?

Thanks!
-Lukas

Quoted 53 lines

 fs/adfs/inode.c         |    5 +++--
 fs/affs/file.c          |    8 +++++---
 fs/affs/inode.c         |    5 ++++-
 fs/bfs/file.c           |    5 +++--
 fs/hfs/inode.c          |   19 +++++++++++++------
 fs/hfsplus/inode.c      |   19 +++++++++++++------
 fs/hostfs/hostfs_kern.c |    8 +++++---
 fs/hpfs/file.c          |    8 +++++---
 fs/hpfs/inode.c         |    5 ++++-
 fs/jfs/file.c           |    6 ++++--
 fs/jfs/inode.c          |   13 +++++++++----
 fs/libfs.c              |    2 --
 fs/logfs/readwrite.c    |   10 ++++++++--
 fs/minix/file.c         |    6 ++++--
 fs/minix/inode.c        |    7 +++++--
 fs/ncpfs/inode.c        |    4 +++-
 fs/nilfs2/file.c        |    1 -
 fs/nilfs2/inode.c       |   18 +++++++++++++-----
 fs/nilfs2/recovery.c    |    7 +++++--
 fs/ntfs/file.c          |    8 +++++---
 fs/ntfs/inode.c         |   11 +++++++++--
 fs/ntfs/inode.h         |    4 ++++
 fs/ocfs2/file.c         |    3 ++-
 fs/omfs/file.c          |   12 ++++++++----
 fs/proc/base.c          |    3 ++-
 fs/proc/generic.c       |    3 ++-
 fs/proc/proc_sysctl.c   |    3 ++-
 fs/reiserfs/file.c      |    3 +--
 fs/reiserfs/inode.c     |   15 +++++++++++----

Quoted 10 lines

 fs/reiserfs/reiserfs.h  |    1 +
 fs/sysv/file.c          |    5 +++--
 fs/sysv/itree.c         |    7 +++++--
 fs/ufs/inode.c          |    5 +++--
 include/linux/fs.h      |    1 -
 include/linux/mm.h      |    1 -
 mm/truncate.c           |   23 -----------------------
 36 files changed, 164 insertions(+), 100 deletions(-)

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

Marco Stornelli

author

2012-09-07 06:29 UTC #2 lore raw

2012/9/6 Lukáš Czerner <[email protected]>:

Quoted 42 lines · Marco Stornelli

Quoted 28 lines
Date: Fri, 31 Aug 2012 15:50:20 +0200
From: Marco Stornelli <[email protected]>
To: Linux FS Devel <[email protected]>, [email protected]
Cc: Linux Kernel <[email protected]>
Subject: [PATCH 00/21] drop vmtruncate
Hi all,
with this patch series I try to clean the vmtruncate code. The theory of operation:
old new
vmtruncate() => inode_newsize_ok+truncate_setsize+fs truncate
Where vmtruncate was used without any error check, the code now is:
if (inode_newsize_ok() == 0) {
      truncate_setsize();
      fs truncate();
}
So, performance and semantic nothing change at all. I think that maybe in some point we can skip inode_newsize_ok (where the error check of vmtruncate wasn't used) but since there is a swap check in case of no-extension, maybe it's better to avoid regressions. After this clean, of course, each fs can clean in a deeply way.
Quoted 3 lines
With these patches even the inode truncate callback is deleted.
Any comments/feedback/bugs are welcome.
Could you explain the reason behind this change a little bit more ? This does not make any sense to me since you're replacing vmtruncate() which does basically
if (inode_newsize_ok() == 0) {
        truncate_setsize();
        fs truncate();
}

Quoted 7 lines

as you mentioned above by exactly the same thing but doing it within the file system. It does not seem like an improvement to me ... how is this a clean up ?
Thanks!
-Lukas

First of all we have one function less in our stack :) Vmtruncate (see comments) is deprecated, so it's better to remove it completly. In this way we can remove even the truncate call back in inode operations (so save 4byte/8byte per struct for the pointer). The first goal of this cleaning activity, however, is remove a "deprecated" function to have a code much more readable. As I said, this patch series is only a *first* cleanup, each fs can of course clean its code in a deeply way. As you can see, the patch span over several fs, to be *safe* I preferred to use a conservative approach. Where vmtruncate was called without error check, as I said, maybe we can remove inode_newsize_ok(), but since in this way we skip a check, I preferred that approach. It seems that for NTFS and Raiserfs it's ok.

Marco