[PATCH 00/21] drop vmtruncate

[PATCH 00/21] drop vmtruncate

[Marco Stornelli]

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

-- 
1.7.3.4
---

Re: [PATCH 00/21] drop vmtruncate

[Lukáš Czerner]

On Fri, 31 Aug 2012, Marco Stornelli wrote:

> Date: Fri, 31 Aug 2012 15:50:20 +0200
> From: Marco Stornelli <marco.stornelli@gmail.com>
> To: Linux FS Devel <linux-fsdevel@vger.kernel.org>, linux-mm@kvack.org
> Cc: Linux Kernel <linux-kernel@vger.kernel.org>
> 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.
> 
> 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

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

Re: [PATCH 00/21] drop vmtruncate

[Marco Stornelli]

2012/9/6 Lukáš Czerner <lczerner@redhat.com>:
> On Fri, 31 Aug 2012, Marco Stornelli wrote:
>
>> Date: Fri, 31 Aug 2012 15:50:20 +0200
>> From: Marco Stornelli <marco.stornelli@gmail.com>
>> To: Linux FS Devel <linux-fsdevel@vger.kernel.org>, linux-mm@kvack.org
>> Cc: Linux Kernel <linux-kernel@vger.kernel.org>
>> 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.
>>
>> 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
>

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

Marco