[ANNOUNCE] nilfs.org was expired
As we announced previously, we have fully moved the NILFS project site to nilfs.sourceforge.net. The prior address "www.nilfs.org" will be unavailable shortly. Please refer to nilfs.sourceforge.net hereafter. Thanks, Ryusuke Konishi -- To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
Hi Andreas,
On Sun, 31 Aug 2014 17:47:13 +0200, Andreas Rohner wrote:
> Under normal circumstances nilfs_sync_fs() writes out the super block,
> which causes a flush of the underlying block device. But this depends on
> the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the
> last segment crosses a segment boundary. So if only a small amount of
> data is written before the call to nilfs_sync_fs(), no flush of the
> block device occurs.
>
> In the above case an additional call to blkdev_issue_flush() is needed.
> To prevent unnecessary overhead, the new flag THE_NILFS_FLUSHED is
> introduced, which is cleared whenever new logs are written and set
> whenever the block device is flushed.
>
> Signed-off-by: Andreas Rohner
The patch looks good to me except that I feel the use of atomic
test-and-set bitwise operations something unfavorable (though it's
logically correct). I will try to send this to upstream as is unless
a comment comes to mind.
Thanks,
Ryusuke Konishi
> ---
> fs/nilfs2/file.c | 3 ++-
> fs/nilfs2/ioctl.c | 3 ++-
> fs/nilfs2/segment.c | 2 ++
> fs/nilfs2/super.c | 8
> fs/nilfs2/the_nilfs.h | 6 ++
> 5 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/fs/nilfs2/file.c b/fs/nilfs2/file.c
> index 2497815..7857460 100644
> --- a/fs/nilfs2/file.c
> +++ b/fs/nilfs2/file.c
> @@ -56,7 +56,8 @@ int nilfs_sync_file(struct file *file, loff_t start, loff_t
> end, int datasync)
> mutex_unlock(&inode->i_mutex);
>
> nilfs = inode->i_sb->s_fs_info;
> - if (!err && nilfs_test_opt(nilfs, BARRIER)) {
> + if (!err && nilfs_test_opt(nilfs, BARRIER) &&
> + !test_and_set_nilfs_flushed(nilfs)) {
> err = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
> if (err != -EIO)
> err = 0;
> diff --git a/fs/nilfs2/ioctl.c b/fs/nilfs2/ioctl.c
> index 422fb54..dc5d101 100644
> --- a/fs/nilfs2/ioctl.c
> +++ b/fs/nilfs2/ioctl.c
> @@ -1022,7 +1022,8 @@ static int nilfs_ioctl_sync(struct inode *inode, struct
> file *filp,
> return ret;
>
> nilfs = inode->i_sb->s_fs_info;
> - if (nilfs_test_opt(nilfs, BARRIER)) {
> + if (nilfs_test_opt(nilfs, BARRIER) &&
> + !test_and_set_nilfs_flushed(nilfs)) {
> ret = blkdev_issue_flush(inode->i_sb->s_bdev, GFP_KERNEL, NULL);
> if (ret == -EIO)
> return ret;
> diff --git a/fs/nilfs2/segment.c b/fs/nilfs2/segment.c
> index a1a1916..54a6be1 100644
> --- a/fs/nilfs2/segment.c
> +++ b/fs/nilfs2/segment.c
> @@ -1842,6 +1842,8 @@ static void nilfs_segctor_complete_write(struct
> nilfs_sc_info *sci)
> nilfs_segctor_clear_metadata_dirty(sci);
> } else
> clear_bit(NILFS_SC_SUPER_ROOT, &sci->sc_flags);
> +
> + clear_nilfs_flushed(nilfs);
> }
>
> static int nilfs_segctor_wait(struct nilfs_sc_info *sci)
> diff --git a/fs/nilfs2/super.c b/fs/nilfs2/super.c
> index 228f5bd..332fdf0 100644
> --- a/fs/nilfs2/super.c
> +++ b/fs/nilfs2/super.c
> @@ -310,6 +310,7 @@ int nilfs_commit_super(struct super_block *sb, int flag)
> nilfs->ns_sbsize));
> }
> clear_nilfs_sb_dirty(nilfs);
> + set_nilfs_flushed(nilfs);
> return nilfs_sync_super(sb, flag);
> }
>
> @@ -514,6 +515,13 @@ static int nilfs_sync_fs(struct super_block *sb, int
> wait)
> }
> up_write(&nilfs->ns_sem);
>
> + if (wait && !err && nilfs_test_opt(nilfs, BARRIER) &&
> + !test_and_set_nilfs_flushed(nilfs)) {
> + err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL);
> + if (err != -EIO)
> + err = 0;
> + }
> +
> return err;
> }
>
> diff --git a/fs/nilfs2/the_nilfs.h b/fs/nilfs2/the_nilfs.h
> index d01ead1..d12a8ce 100644
> --- a/fs/nilfs2/the_nilfs.h
> +++ b/fs/nilfs2/the_nilfs.h
> @@ -41,6 +41,7 @@ enum {
> THE_NILFS_DISCONTINUED, /* 'next' pointer chain has broken */
> THE_NILFS_GC_RUNNING, /* gc process is running */
> THE_NILFS_SB_DIRTY, /* super block is dirty */
> + THE_NILFS_FLUSHED, /* volatile data was flushed to disk */
> };
>
> /**
> @@ -202,6 +203,10 @@ struct the_nilfs {
> };
>
> #define THE_NILFS_FNS(bit, name) \
> +static inline int test_and_set_nilfs_##name(struct the_nilfs *nilfs) \
> +{\
> + return test_and_set_bit(THE_NILFS_##bit, &(nilfs)->ns_flags); \
> +}\
> static inline void set_nilfs_##name(struct the_nilfs *nilfs) \
> {\
> set_bit(THE_NILFS_##bit, &(nilfs)->ns_flags); \
> @@ -219,6 +224,7 @@ THE_NILFS_FNS(INIT, init)
> THE_NILFS_FNS(DISCONTINUED, discontinued)
> THE_NILFS_FNS(GC_RUNNING,
Re: [PATCH v2 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
Hi Ryusuke,
On 2014-09-01 19:59, Ryusuke Konishi wrote:
> On Sun, 31 Aug 2014 17:47:13 +0200, Andreas Rohner wrote:
>> Under normal circumstances nilfs_sync_fs() writes out the super block,
>> which causes a flush of the underlying block device. But this depends on
>> the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the
>> last segment crosses a segment boundary. So if only a small amount of
>> data is written before the call to nilfs_sync_fs(), no flush of the
>> block device occurs.
>>
>> In the above case an additional call to blkdev_issue_flush() is needed.
>> To prevent unnecessary overhead, the new flag THE_NILFS_FLUSHED is
>> introduced, which is cleared whenever new logs are written and set
>> whenever the block device is flushed.
>>
>> Signed-off-by: Andreas Rohner
>
> The patch looks good to me except that I feel the use of atomic
> test-and-set bitwise operations something unfavorable (though it's
> logically correct). I will try to send this to upstream as is unless
> a comment comes to mind.
I originally thought, that it is necessary to do it atomically to avoid
a race condition, but I am not so sure about that any more. I think the
only case we have to avoid is, to call set_nilfs_flushed() after
blkdev_issue_flush(), because this could race with the
clear_nilfs_flushed() from the segment construction. So this should also
work:
+ if (wait && !err && nilfs_test_opt(nilfs, BARRIER) &&
+ !nilfs_flushed(nilfs)) {
+ set_nilfs_flushed(nilfs);
+ err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL);
+ if (err != -EIO)
+ err = 0;
+ }
+
What do you think?
br,
Andreas Rohner
> Thanks,
> Ryusuke Konishi
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/1] nilfs2: add missing blkdev_issue_flush() to nilfs_sync_fs()
On 2014-09-01 20:43, Andreas Rohner wrote:
> Hi Ryusuke,
> On 2014-09-01 19:59, Ryusuke Konishi wrote:
>> On Sun, 31 Aug 2014 17:47:13 +0200, Andreas Rohner wrote:
>>> Under normal circumstances nilfs_sync_fs() writes out the super block,
>>> which causes a flush of the underlying block device. But this depends on
>>> the THE_NILFS_SB_DIRTY flag, which is only set if the pointer to the
>>> last segment crosses a segment boundary. So if only a small amount of
>>> data is written before the call to nilfs_sync_fs(), no flush of the
>>> block device occurs.
>>>
>>> In the above case an additional call to blkdev_issue_flush() is needed.
>>> To prevent unnecessary overhead, the new flag THE_NILFS_FLUSHED is
>>> introduced, which is cleared whenever new logs are written and set
>>> whenever the block device is flushed.
>>>
>>> Signed-off-by: Andreas Rohner
>>
>> The patch looks good to me except that I feel the use of atomic
>> test-and-set bitwise operations something unfavorable (though it's
>> logically correct). I will try to send this to upstream as is unless
>> a comment comes to mind.
>
> I originally thought, that it is necessary to do it atomically to avoid
> a race condition, but I am not so sure about that any more. I think the
> only case we have to avoid is, to call set_nilfs_flushed() after
> blkdev_issue_flush(), because this could race with the
> clear_nilfs_flushed() from the segment construction. So this should also
> work:
>
> +if (wait && !err && nilfs_test_opt(nilfs, BARRIER) &&
> +!nilfs_flushed(nilfs)) {
> + set_nilfs_flushed(nilfs);
> +err = blkdev_issue_flush(sb->s_bdev, GFP_KERNEL, NULL);
> +if (err != -EIO)
> +err = 0;
> +}
> +
On the other hand, it says in the comments to set_bit(), that it can be
reordered on architectures other than x86. test_and_set_bit() implies a
memory barrier on all architectures. But I don't think the processor
would reorder set_nilfs_flushed() after the external function call to
blkdev_issue_flush(), would it?
/**
* set_bit - Atomically set a bit in memory
* @nr: the bit to set
* @addr: the address to start counting from
*
* This function is atomic and may not be reordered. See __set_bit()
* if you do not require the atomic guarantees.
*
* Note: there are no guarantees that this function will not be reordered
* on non x86 architectures, so if you are writing portable code,
* make sure not to rely on its reordering guarantees.
*/
br,
Andreas Rohner
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] libmount: don't base GC startup on no-mtab context
When mtab is a symlink to /proc/mounts, libmount uses /run/mount/utab to
store filesystem-specific mount attributes, making this check
unnecessary. Worse, it now breaks nilfs_cleanerd under systemd 216+
since it always passes `-n` to the mount command.
Signed-off-by: Dan McGee
---
sbin/mount/mount_libmount.c | 6 --
1 file changed, 6 deletions(-)
diff --git a/sbin/mount/mount_libmount.c b/sbin/mount/mount_libmount.c
index c518475..b901654 100644
--- a/sbin/mount/mount_libmount.c
+++ b/sbin/mount/mount_libmount.c
@@ -404,12 +404,6 @@ static int nilfs_update_mount_state(struct
nilfs_mount_info *mi)
rungc = gc_ok && !mi->new_attrs.nogc;
old_attrs = (mi->mflags & MS_REMOUNT) ? &mi->old_attrs : NULL;
- if (mnt_context_is_nomtab(cxt)) {
- if (rungc)
- printf(_("%s not started\n"), NILFS_CLEANERD_NAME);
- return 0;
- }
-
if (rungc) {
if (mi->new_attrs.pp == ULONG_MAX)
mi->new_attrs.pp = mi->old_attrs.pp;
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH] lscp: always show snapshots, even if marked minor
When the average user types `lscp` and doesn't see the snapshot they
just tried to make, it can be very confusing. Add an additional check
to ensure snapshots are never omitted from lscp output.
Signed-off-by: Dan McGee
---
Thoughts? I notice this quite often due to a "snapshot-on-shutdown" job I have
running on my machine, and it is very odd to not see this snapshot listed next
time I boot up and run `lscp -r | less`.
bin/lscp.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/bin/lscp.c b/bin/lscp.c
index 2de81b6..6803657 100644
--- a/bin/lscp.c
+++ b/bin/lscp.c
@@ -159,7 +159,7 @@ static int lscp_forward_cpinfo(struct nilfs *nilfs,
break;
for (cpi = cpinfos; cpi < cpinfos + n; cpi++) {
- if (show_all || !nilfs_cpinfo_minor(cpi)) {
+ if (show_all || nilfs_cpinfo_snapshot(cpi) ||
!nilfs_cpinfo_minor(cpi)) {
lscp_print_cpinfo(cpi);
rest--;
}
--
2.1.0
--
To unsubscribe from this list: send the line "unsubscribe linux-nilfs" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
