From: Artem Bityutskiy <[email protected]>
Exofs has the '->write_super()' handler and makes some use of the '->s_dirt'
superblock flag, but it really needs neither of them because it never sets
's_dirt' to one which means the VFS never calls its '->write_super()' handler.
Thus, remove both.
Note, I am trying to remove both 's_dirt' and 'write_super()' from VFS
altogether once all users are gone.
Signed-off-by: Artem Bityutskiy <[email protected]>
---
fs/exofs/super.c | 11 -----------
1 files changed, 0 insertions(+), 11 deletions(-)
diff --git a/fs/exofs/super.c b/fs/exofs/super.c
index 735ca06..6e1c515 100644
--- a/fs/exofs/super.c
+++ b/fs/exofs/super.c
@@ -400,8 +400,6 @@ static int exofs_sync_fs(struct super_block *sb, int wait)
ret = ore_write(ios);
if (unlikely(ret))
EXOFS_ERR("%s: ore_write failed.\n", __func__);
- else
- sb->s_dirt = 0;
unlock_super(sb);
@@ -412,14 +410,6 @@ out:
return ret;
}
-static void exofs_write_super(struct super_block *sb)
-{
- if (!(sb->s_flags & MS_RDONLY))
- exofs_sync_fs(sb, 1);
- else
- sb->s_dirt = 0;
-}
-
static void _exofs_print_device(const char *msg, const char *dev_path,
struct osd_dev *od, u64 pid)
{
@@ -942,7 +932,6 @@ static const struct super_operations exofs_sops = {
.write_inode = exofs_write_inode,
.evict_inode = exofs_evict_inode,
.put_super = exofs_put_super,
- .write_super = exofs_write_super,
.sync_fs = exofs_sync_fs,
.statfs = exofs_statfs,
};
--
1.7.7.6
On 06/04/2012 02:48 PM, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <[email protected]>
>
> Exofs has the '->write_super()' handler and makes some use of the '->s_dirt'
> superblock flag, but it really needs neither of them because it never sets
> 's_dirt' to one which means the VFS never calls its '->write_super()' handler.
> Thus, remove both.
>
Thanks Artem. I have seen your other FS conversions and thought I would
eventually need to also do it for exofs, thanks for beating me to it ;-)
I have removed all uses of sb->s_dirt = 1 cases around last year, by
writing the SB-info as part of the create/delete commands directly. So
I agree it is not needed anymore see here
1cea312 exofs: Write sbi->s_nextid as part of the Create command
I have one question though, which I did not understand at the time?
Today at exofs_write_super() we call exofs_sync_fs() (super_operations->sync_fs)
Who/when calls ->sync_fs() without the ->write_super() below.
But otherwise I agree that sb->s_dirt = 1 and ->write_super() are not needed
at all, for regular operations.
Should I take this for 3.6 through my tree. Or do you want my:
Ack-by: Boaz Harrosh <[email protected]>
> Note, I am trying to remove both 's_dirt' and 'write_super()' from VFS
> altogether once all users are gone.
>
> Signed-off-by: Artem Bityutskiy <[email protected]>
> ---
> fs/exofs/super.c | 11 -----------
> 1 files changed, 0 insertions(+), 11 deletions(-)
>
> diff --git a/fs/exofs/super.c b/fs/exofs/super.c
> index 735ca06..6e1c515 100644
> --- a/fs/exofs/super.c
> +++ b/fs/exofs/super.c
> @@ -400,8 +400,6 @@ static int exofs_sync_fs(struct super_block *sb, int wait)
> ret = ore_write(ios);
> if (unlikely(ret))
> EXOFS_ERR("%s: ore_write failed.\n", __func__);
> - else
> - sb->s_dirt = 0;
>
>
> unlock_super(sb);
> @@ -412,14 +410,6 @@ out:
> return ret;
> }
>
> -static void exofs_write_super(struct super_block *sb)
> -{
> - if (!(sb->s_flags & MS_RDONLY))
> - exofs_sync_fs(sb, 1);
> - else
> - sb->s_dirt = 0;
> -}
> -
> static void _exofs_print_device(const char *msg, const char *dev_path,
> struct osd_dev *od, u64 pid)
> {
> @@ -942,7 +932,6 @@ static const struct super_operations exofs_sops = {
> .write_inode = exofs_write_inode,
> .evict_inode = exofs_evict_inode,
> .put_super = exofs_put_super,
> - .write_super = exofs_write_super,
> .sync_fs = exofs_sync_fs,
> .statfs = exofs_statfs,
> };
On Mon, 2012-06-04 at 19:12 +0300, Boaz Harrosh wrote:
> I have one question though, which I did not understand at the time?
>
> Today at exofs_write_super() we call exofs_sync_fs() (super_operations->sync_fs)
> Who/when calls ->sync_fs() without the ->write_super() below.
Not sure I understood your question correctly, but:
o VFS calls '->sync_fs()' on
* sync(2)
* syncfs(2)
* when unmounting, before calling '->put_super()'
* when re-mounting, before calling '->remount_fs()'
* when freezing (happens when suspending the system)
* when the underlying block device is synced (see 'fsync_bdev()')
o '->write_super()' is called only from one place - from the
'sync_supers()' function (which is only used by the 'sync_supers'
kernel thread) if the superblock is dirty. This thread wakes up every
5 seconds and calls '->write_super()' for all dirty superblocks.
In case of exofs you never set 's_dirt' to non-zero, which means that
'exofs_write_super()' is never called. This means we can remove it and
this won't change anything for exofs.
Basically, in the ideal world where power-cuts and unclean reboots do
not exist, '->write_super()' is not needed because the superblock will
be synced on unmount. The 'write_super()' stuff is about making sure
that your dirty superblock stays dirty only for limited amount of time
(defined via /proc/sys/vm/dirty_writeback_centisecs) and then gets
synced so you have less chances end up with a not completely up-to-date
superblock.
--
Best Regards,
Artem Bityutskiy
On Mon, 2012-06-04 at 19:12 +0300, Boaz Harrosh wrote:
> Should I take this for 3.6 through my tree. Or do you want my:
Forgot to answer this question, sorry: if it is fine with you, please,
merge it through your tree.
--
Best Regards,
Artem Bityutskiy
On 06/05/2012 01:35 PM, Artem Bityutskiy wrote:
> On Mon, 2012-06-04 at 19:12 +0300, Boaz Harrosh wrote:
>> Should I take this for 3.6 through my tree. Or do you want my:
>
> Forgot to answer this question, sorry: if it is fine with you, please,
> merge it through your tree.
>
I will. It should appear on linux-next, next time I push new patches
destined for 3.6 Kernel
I conducted some tests and you are absolutely right. There is no effect
it is just dead code.
Thanks again
Boaz
On Mon, 2012-06-04 at 14:48 +0300, Artem Bityutskiy wrote:
> From: Artem Bityutskiy <[email protected]>
>
> Exofs has the '->write_super()' handler and makes some use of the '->s_dirt'
> superblock flag, but it really needs neither of them because it never sets
> 's_dirt' to one which means the VFS never calls its '->write_super()' handler.
> Thus, remove both.
>
> Note, I am trying to remove both 's_dirt' and 'write_super()' from VFS
> altogether once all users are gone.
>
> Signed-off-by: Artem Bityutskiy <[email protected]>
Al or Boaz, would you please consider picking this patch?
--
Best Regards,
Artem Bityutskiy
On 06/21/2012 03:29 PM, Artem Bityutskiy wrote:
> On Mon, 2012-06-04 at 14:48 +0300, Artem Bityutskiy wrote:
>> From: Artem Bityutskiy <[email protected]>
>>
>> Exofs has the '->write_super()' handler and makes some use of the '->s_dirt'
>> superblock flag, but it really needs neither of them because it never sets
>> 's_dirt' to one which means the VFS never calls its '->write_super()' handler.
>> Thus, remove both.
>>
>> Note, I am trying to remove both 's_dirt' and 'write_super()' from VFS
>> altogether once all users are gone.
>>
>> Signed-off-by: Artem Bityutskiy <[email protected]>
>
> Al or Boaz, would you please consider picking this patch?
>
I will
Sorry for the delay was sick and am only now coming back on.
It should all be included for the next merge window.
Thanks for doing this.
Boaz