reiser4: port for Linux-4.1

PATCH In review Edward Shishkin 3 participants 22 messages Jun 26, 2015 – Apr 6, 2016 1 message not in the archive VFS

Share & tools

Edward Shishkin

author

2015-06-26 14:06 UTC #0 lore raw

PATCH 1/1 reiser4: port for Linux-4.1

. move generic_write_checks() up to the dispatcher; . cancel_dirty_pages(), invented by akpm and me in 2007, gone:
https://lkml.org/lkml/2015/2/20/241

Signed-off-by Edward Shishkin <[email protected]>

---
 fs/read_write.c                          |    3 ++-
 fs/reiser4/plugin/file/cryptcompress.c   |   19 ++++++++-----------
 fs/reiser4/plugin/file/file.c            |    7 -------
 fs/reiser4/plugin/file/file_conversion.c |   18 +++++++++++++++---
 include/linux/fs.h                       |    1 +
 5 files changed, 26 insertions(+), 22 deletions(-)

fs/read_write.c +2 -1

@@ -417,7 +417,7 @@ int rw_verify_area(int read_write, struc
 	return count > MAX_RW_COUNT ? MAX_RW_COUNT : count;
 }
 
-static ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos)
+ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos)
 {
 	struct iovec iov = { .iov_base = buf, .iov_len = len };
 	struct kiocb kiocb;

@@ -433,6 +433,7 @@ static ssize_t new_sync_read(struct file
 	*ppos = kiocb.ki_pos;
 	return ret;
 }
+EXPORT_SYMBOL(new_sync_read);
 
 ssize_t __vfs_read(struct file *file, char __user *buf, size_t count,
 		   loff_t *pos)

fs/reiser4/plugin/file/cryptcompress.c +8 ~~-11~~

@@ -1921,10 +1921,14 @@ static void checkout_page_cluster(struct
 		memcpy(tfm_stream_data(tc, INPUT_STREAM) + pg_to_off(i),
 		       data, in_page);
 		kunmap_atomic(data);
-
-		if (PageDirty(clust->pages[i]))
-			cancel_dirty_page(clust->pages[i], PAGE_CACHE_SIZE);
-
+		/*
+		 * modifications have been checked out and will be
+		 * committed later. Anyway, the dirty status of the
+		 * page is no longer relevant. However, the uptodate
+		 * status of the page is still relevant!
+		 */
+		if (TestClearPageDirty(clust->pages[i]))
+			account_page_cleaned(clust->pages[i], inode->i_mapping);
 		unlock_page(clust->pages[i]);
 
 		if (in_page < PAGE_CACHE_SIZE)

@@ -2875,13 +2879,6 @@ ssize_t write_cryptcompress(struct file
 	info = cryptcompress_inode_data(inode);
 	ctx = get_current_context();
 
-	result = generic_write_checks(file, &pos, &count, 0);
-  	if (unlikely(result != 0)) {
-		context_set_commit_async(ctx);
-		return result;
-	}
-  	if (unlikely(count == 0))
-		return 0;
 	result = file_remove_suid(file);
 	if (unlikely(result != 0)) {
 		context_set_commit_async(ctx);

fs/reiser4/plugin/file/file.c +0 -7

@@ -2101,13 +2101,6 @@ ssize_t write_unix_file(struct file *fil
 	assert("vs-947", !reiser4_inode_get_flag(inode, REISER4_NO_SD));
 	assert("vs-9471", (!reiser4_inode_get_flag(inode, REISER4_PART_MIXED)));
 
-	/* check amount of bytes to write and writing position */
-	result = generic_write_checks(file, pos, &count, 0);
-	if (result) {
-		context_set_commit_async(ctx);
-		return result;
-	}
-
 	result = file_remove_suid(file);
 	if (result) {
 		context_set_commit_async(ctx);

fs/reiser4/plugin/file/file_conversion.c +15 -3

@@ -28,6 +28,7 @@
  * are CS readers.
  */
 
+#include <linux/uio.h>
 #include "../../inode.h"
 #include "../cluster.h"
 #include "file.h"

@@ -519,19 +520,31 @@ static inline void done_dispatch_context
 ssize_t reiser4_write_dispatch(struct file *file, const char __user *buf,
 			       size_t count, loff_t *off)
 {
-	int result;
+	ssize_t result;
 	reiser4_context *ctx;
 	ssize_t written_old = 0; /* bytes written with initial plugin */
 	ssize_t written_new = 0; /* bytes written with new plugin */
 	struct dispatch_context cont;
 	struct inode * inode = file_inode(file);
 
+	struct iovec iov = { .iov_base = (void __user *)buf, .iov_len = count };
+	struct kiocb iocb;
+	struct iov_iter iter;
+
+	init_sync_kiocb(&iocb, file);
+	iocb.ki_pos = *off;
+	iov_iter_init(&iter, WRITE, &iov, 1, count);
+
 	ctx = reiser4_init_context(inode->i_sb);
 	if (IS_ERR(ctx))
 		return PTR_ERR(ctx);
 	current->backing_dev_info = inode_to_bdi(inode);
 	init_dispatch_context(&cont);
 	mutex_lock(&inode->i_mutex);
+
+	result = generic_write_checks(&iocb, &iter);
+	if (unlikely(result <= 0))
+		goto exit;
 	/**
 	 * First step.
 	 * Start write with initial file plugin.

@@ -556,8 +569,7 @@ ssize_t reiser4_write_dispatch(struct fi
 		warning("edward-1544",
 			"Inode %llu: file plugin conversion failed (%d)",
 			(unsigned long long)get_inode_oid(inode),
-			result);
-		context_set_commit_async(ctx);
+			(int)result);
 		goto exit;
 	}
 	reiser4_txn_restart(ctx);

include/linux/fs.h +1 -0

@@ -2605,6 +2605,7 @@ extern ssize_t generic_file_write_iter(s
 extern ssize_t generic_file_direct_write(struct kiocb *, struct iov_iter *, loff_t);
 extern ssize_t generic_perform_write(struct file *, struct iov_iter *, loff_t);
 
+ssize_t new_sync_read(struct file *filp, char __user *buf, size_t len, loff_t *ppos);
 ssize_t vfs_iter_read(struct file *file, struct iov_iter *iter, loff_t *ppos);
 ssize_t vfs_iter_write(struct file *file, struct iov_iter *iter, loff_t *ppos);

Edward Shishkin

author

2015-06-29 17:54 UTC #1 lore raw

Oh, bad
generic_write_checks() doesn't change offset now,
please, ignore this patch..

Thanks,
Edward.

Ivan Shapovalov

2015-06-30 07:13 UTC #2 lore raw

Quoted 3 lines · Edward Shishkin

Oh, bad
generic_write_checks() doesn't change offset now,
please, ignore this patch..

Hi,

it does change it, but in struct kiocb instance, but we keep using initial "off".

i'm trying to port file_operations' ->write() to ->write_iter() right now, FTR. The obvious fix (assign back ki_pos to off) is too ugly :)

Signature · 1 line

Ivan Shapovalov / intelfx /

Ivan Shapovalov

2015-06-30 08:06 UTC #4 lore raw

Quoted 14 lines · Edward Shishkin

Quoted 5 lines · Ivan Shapovalov
Quoted 3 lines · Edward Shishkin
Oh, bad
generic_write_checks() doesn't change offset now,
please, ignore this patch..
Hi,
Hello.
Quoted 4 lines
it does change it, but in struct kiocb instance, but we keep using initial "off".

Quoted 7 lines

Quoted 2 lines
i'm trying to port file_operations' ->write() to ->write_iter() right
Hmm. You'll need to modify ->write() of both file plugins - it's not simple, esp. for cryptcompress, which performs writes by chunks (4, 8, ... 64K)

Yeah, I understand. However, it doesn't seem too complex. It's simply iov_iter* instead of a char*+size_t. I just need to grok how does the page cache work (wrt. "faulting in" pages, flushing caches, error handling, etc).

I'm now trying to read the generic code together with btrfs' implementation, compare bit-by-bit and implement the same in reiser4.

Quoted 11 lines

Quoted 3 lines
now, FTR. The obvious fix (assign back ki_pos to off) is too ugly :)
may be just create a static inline function
reiser4_write_check() ?
{
     ...
     generic_write_check();
*off = iocb.ki_pos;
}

Sure, it will work, but right now, as everything in vfs moves towards *_iter methods, I guess there could be some sense in moving to those as well...

Yes, I understand that vfs is a fast-moving target, but why not?

Thanks,

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2015-06-30 08:58 UTC #5 lore raw

Quoted 35 lines · Ivan Shapovalov

Quoted 11 lines · Edward Shishkin
Quoted 5 lines · Ivan Shapovalov
Quoted 3 lines · Edward Shishkin
Oh, bad
generic_write_checks() doesn't change offset now,
please, ignore this patch..
Hi,
Hello.
Quoted 3 lines
it does change it, but in struct kiocb instance, but we keep using initial "off".
Quoted 6 lines
Quoted 2 lines
i'm trying to port file_operations' ->write() to ->write_iter() right
Hmm. You'll need to modify ->write() of both file plugins - it's not simple, esp. for cryptcompress, which performs writes by chunks (4, 8, ... 64K)
Yeah, I understand. However, it doesn't seem too complex. It's simply iov_iter* instead of a char*+size_t. I just need to grok how does the page cache work (wrt. "faulting in" pages, flushing caches, error handling, etc).
I'm now trying to read the generic code together with btrfs' implementation, compare bit-by-bit and implement the same in reiser4.
Quoted 9 lines
Quoted 2 lines
now, FTR. The obvious fix (assign back ki_pos to off) is too ugly :)
may be just create a static inline function
reiser4_write_check() ?
{
      ...
      generic_write_check();
*off = iocb.ki_pos;
}

Quoted 5 lines

Sure, it will work, but right now, as everything in vfs moves towards *_iter methods, I guess there could be some sense in moving to those as well...
Yes, I understand that vfs is a fast-moving target, but why not?

I afraid it'll get stuck in the queue "for review"..

BTW, we need to do something with the "precise discard extension":
http://sourceforge.net/projects/reiser4/files/patches/
It reports erase unit 512 bytes for my samsung SSD 840 EVO. You said that this is incorrect. If so, then how to retrieve correct discard parameters?

Thanks,
Edward.

Ivan Shapovalov

2015-07-01 23:35 UTC #6 lore raw

Quoted 47 lines · Edward Shishkin

Quoted 46 lines · Ivan Shapovalov
Quoted 12 lines · Edward Shishkin
Quoted 5 lines · Ivan Shapovalov
Quoted 3 lines · Edward Shishkin
Oh, bad
generic_write_checks() doesn't change offset now,
please, ignore this patch..
Hi,
Hello.
Quoted 4 lines
it does change it, but in struct kiocb instance, but we keep using
initial "off".
Quoted 6 lines
Quoted 2 lines
i'm trying to port file_operations' ->write() to ->write_iter() right
Hmm. You'll need to modify ->write() of both file plugins - it's not simple, esp. for cryptcompress, which performs writes by chunks (4, 8, ... 64K)
Yeah, I understand. However, it doesn't seem too complex. It's simply
iov_iter* instead of a char*+size_t. I just need to grok how does the
page cache work (wrt. "faulting in" pages, flushing caches, error handling, etc).
I'm now trying to read the generic code together with btrfs' implementation, compare bit-by-bit and implement the same in reiser4.
Quoted 10 lines
Quoted 3 lines
now, FTR. The obvious fix (assign back ki_pos to off) is too ugly
:)
may be just create a static inline function
reiser4_write_check() ?
{
      ...
      generic_write_check();
*off = iocb.ki_pos;
}
Sure, it will work, but right now, as everything in vfs moves towards
*_iter methods, I guess there could be some sense in moving to those as
well...

Quoted 3 lines

Quoted 1 line
Yes, I understand that vfs is a fast-moving target, but why not?
I afraid it'll get stuck in the queue "for review"..

Then let's say that we have an "experimental"/"unreviewed" branch :)

Quoted 6 lines

BTW, we need to do something with the "precise discard extension":
http://sourceforge.net/projects/reiser4/files/patches/
It reports erase unit 512 bytes for my samsung SSD 840 EVO. You said that this is incorrect. If so, then how to retrieve correct discard parameters?

I was wrong about usage of hdparm -I. The "limit" it says about is in fact "how many 512-byte blocks worth of LBA ranges can be given to the drive in a single ATA trim command"[1].

In fact, the standard (referenced below) doesn't seem to contain any references to the trim granularity, let alone to define any means to query it.

So, I guess, the kernel will never tell us correct values for ATA SSDs, and the only option is direct testing at mount time.

[1]: ATA8-ACS3 (nevar.pl/pliki/ATA8-ACS-3.pdf), 7.16.7.55
"IDENTIFY DEVICE" - "Input From the Device to the Host Data Structure" - "7.16.7.55 Word 105: Maximum number of 512-byte blocks of LBA Range Entries per DATA SET MANAGEMENT command".

Thanks,

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2015-07-04 07:53 UTC #7 lore raw

Quoted 60 lines · Ivan Shapovalov

Quoted 40 lines · Edward Shishkin
Quoted 39 lines · Ivan Shapovalov
Quoted 12 lines · Edward Shishkin
Quoted 5 lines · Ivan Shapovalov
Quoted 3 lines · Edward Shishkin
Oh, bad
generic_write_checks() doesn't change offset now,
please, ignore this patch..
Hi,
Hello.
Quoted 4 lines
it does change it, but in struct kiocb instance, but we keep using
initial "off".
Quoted 5 lines
Quoted 2 lines
i'm trying to port file_operations' ->write() to ->write_iter() right
Hmm. You'll need to modify ->write() of both file plugins - it's not simple, esp. for cryptcompress, which performs writes by chunks (4, 8, ... 64K)
Yeah, I understand. However, it doesn't seem too complex. It's simply
iov_iter* instead of a char*+size_t. I just need to grok how does the
page cache work (wrt. "faulting in" pages, flushing caches, error handling, etc).
I'm now trying to read the generic code together with btrfs' implementation, compare bit-by-bit and implement the same in reiser4.
Quoted 10 lines
Quoted 3 lines
now, FTR. The obvious fix (assign back ki_pos to off) is too ugly
:)
may be just create a static inline function
reiser4_write_check() ?
{
       ...
       generic_write_check();
*off = iocb.ki_pos;
}
Quoted 8 lines
Quoted 7 lines
Sure, it will work, but right now, as everything in vfs moves towards
*_iter methods, I guess there could be some sense in moving to those as
well...
Yes, I understand that vfs is a fast-moving target, but why not?
I afraid it'll get stuck in the queue "for review"..
Then let's say that we have an "experimental"/"unreviewed" branch :)
Quoted 5 lines
BTW, we need to do something with the "precise discard extension":
http://sourceforge.net/projects/reiser4/files/patches/
It reports erase unit 512 bytes for my samsung SSD 840 EVO. You said that this is incorrect. If so, then how to retrieve correct discard parameters?
I was wrong about usage of hdparm -I. The "limit" it says about is in fact "how many 512-byte blocks worth of LBA ranges can be given to the drive in a single ATA trim command"[1].

Quoted 6 lines

In fact, the standard (referenced below) doesn't seem to contain any references to the trim granularity, let alone to define any means to query it.
So, I guess, the kernel will never tell us correct values for ATA SSDs, and the only option is direct testing at mount time.

And how to test directly at mount time?
It seems that nobody cares about it..
E.g. I have the following (samsung SSD 840 EVO):

reiser4: sdb1: enable discard support (erase unit 512 bytes, alignment 0 bytes)

Thanks,
Edward.

Quoted 7 lines

[1]: ATA8-ACS3 (nevar.pl/pliki/ATA8-ACS-3.pdf), 7.16.7.55
"IDENTIFY DEVICE" - "Input From the Device to the Host Data Structure" - "7.16.7.55 Word 105: Maximum number of 512-byte blocks of LBA Range Entries per DATA SET MANAGEMENT command".
Thanks,

Ivan Shapovalov

2015-07-04 17:53 UTC #8 lore raw

Quoted 20 lines · Edward Shishkin

Quoted 18 lines · Ivan Shapovalov
Quoted 7 lines · Edward Shishkin
[...]
BTW, we need to do something with the "precise discard > > > extension":
http://sourceforge.net/projects/reiser4/files/patches/
It reports erase unit 512 bytes for my samsung SSD 840 EVO. You said that this is incorrect. If so, then how to retrieve correct
discard parameters?
I was wrong about usage of hdparm -I. The "limit" it says about is in fact "how many 512-byte blocks worth of LBA ranges can be given
to the drive in a single ATA trim command"[1].
In fact, the standard (referenced below) doesn't seem to contain any references to the trim granularity, let alone to define any means
to query it.

Quoted 6 lines

Quoted 3 lines
So, I guess, the kernel will never tell us correct values for ATA SSDs,
and the only option is direct testing at mount time.
And how to test directly at mount time?

Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero-filled
- if it did, infer alignnment from the first zero-filled block, infer granularity from the zero-filled region size.

Quoted 1 line

It seems that nobody cares about it..

It's just ATA interface does not provide necessary data.

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2015-07-04 18:33 UTC #9 lore raw

Quoted 25 lines · Ivan Shapovalov

Quoted 19 lines · Edward Shishkin
Quoted 18 lines · Ivan Shapovalov
Quoted 7 lines · Edward Shishkin
[...]
BTW, we need to do something with the "precise discard > > > extension":
http://sourceforge.net/projects/reiser4/files/patches/
It reports erase unit 512 bytes for my samsung SSD 840 EVO. You said that this is incorrect. If so, then how to retrieve correct
discard parameters?
I was wrong about usage of hdparm -I. The "limit" it says about is in fact "how many 512-byte blocks worth of LBA ranges can be given
to the drive in a single ATA trim command"[1].
In fact, the standard (referenced below) doesn't seem to contain any references to the trim granularity, let alone to define any means
to query it.
Quoted 5 lines
Quoted 3 lines
So, I guess, the kernel will never tell us correct values for ATA SSDs,
and the only option is direct testing at mount time.
And how to test directly at mount time?

Quoted 8 lines

Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero-filled
- if it did, infer alignnment from the first zero-filled block, infer granularity from the zero-filled region size.

mkfs seems to be more suitable for this funny business

Quoted 2 lines

Quoted 1 line
It seems that nobody cares about it..
It's just ATA interface does not provide necessary data.

OK, so our precise discard extension is waiting for the best times..

Thanks,
Edward.

Ivan Shapovalov

2015-07-05 13:08 UTC #10 lore raw

Quoted 15 lines · Edward Shishkin

Quoted 4 lines · Ivan Shapovalov
Quoted 3 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Quoted 8 lines
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero-filled
- if it did, infer alignnment from the first zero-filled block, infer granularity from the zero-filled region size.

Quoted 1 line

mkfs seems to be more suitable for this funny business

Yeah, sure. So... new superblock format with two extra fields?

Quoted 8 lines

Quoted 4 lines
Quoted 3 lines
It seems that nobody cares about it..
It's just ATA interface does not provide necessary data.
OK, so our precise discard extension is waiting for the best times..

I hope we'll be able to merge it eventually (esp. the final patch which accounts for non-idempotence of check-and-allocate-blocks).

Thanks,

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2015-07-05 13:46 UTC #11 lore raw

Quoted 14 lines · Ivan Shapovalov

Quoted 13 lines · Edward Shishkin
Quoted 11 lines · Ivan Shapovalov
Quoted 2 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero-filled
- if it did, infer alignnment from the first zero-filled block, infer granularity from the zero-filled region size.

Quoted 2 lines

Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?

The change will be forward and backward compatible, so there is no need in a new format. Just add two new fields to the superblock40.

Also people will want to "mount -o discard" when discard parameters are not known. In this case I think to provide them discard_extents(). which is currently merged. Otherwise use discard_precise_extents().

Thanks,
Edward.

Quoted 11 lines

Quoted 7 lines
Quoted 4 lines
Quoted 3 lines
It seems that nobody cares about it..
It's just ATA interface does not provide necessary data.
OK, so our precise discard extension is waiting for the best times..
I hope we'll be able to merge it eventually (esp. the final patch which accounts for non-idempotence of check-and-allocate-blocks).
Thanks,

Ivan Shapovalov

2015-07-05 15:11 UTC #12 lore raw

Quoted 18 lines · Edward Shishkin

Quoted 16 lines · Ivan Shapovalov
Quoted 15 lines · Edward Shishkin
Quoted 13 lines · Ivan Shapovalov
Quoted 2 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero
-filled
- if it did, infer alignnment from the first zero-filled
block,
infer granularity from the zero-filled region size.

Quoted 6 lines

Quoted 2 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
The change will be forward and backward compatible, so there is no need in a new format. Just add two new fields to the superblock40.

So there will be zeroes in case we mount a non-aware filesystem, correct?

Quoted 5 lines

Also people will want to "mount -o discard" when discard parameters are not known. In this case I think to provide them discard_extents().
which is currently merged. Otherwise use discard_precise_extents().

Yeah.
I'll do something with this after I finish porting ->write() to ->write_iter() (or if I run into heavy problems with that).

Thanks,

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2015-07-05 15:43 UTC #13 lore raw

Quoted 17 lines · Ivan Shapovalov

Quoted 16 lines · Edward Shishkin
Quoted 15 lines · Ivan Shapovalov
Quoted 14 lines · Edward Shishkin
Quoted 13 lines · Ivan Shapovalov
Quoted 2 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero
-filled
- if it did, infer alignnment from the first zero-filled
block,
infer granularity from the zero-filled region size.

Quoted 7 lines

Quoted 5 lines
Quoted 2 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
The change will be forward and backward compatible, so there is no need in a new format. Just add two new fields to the superblock40.
So there will be zeroes in case we mount a non-aware filesystem, correct?

Yup.
We have a "reserved" space of 428 bytes at the end of format40_disk_super_block for such purposes. This space is filled with zeros by mkfs. So "non-aware filesystems" including the ones that were created by old versions of mkfs will have zeros there.

Edward.

Quoted 10 lines

Quoted 5 lines
Also people will want to "mount -o discard" when discard parameters are not known. In this case I think to provide them discard_extents().
which is currently merged. Otherwise use discard_precise_extents().
Yeah.
I'll do something with this after I finish porting ->write() to ->write_iter() (or if I run into heavy problems with that).
Thanks,

A message that is not in the archive (sent off-list or not yet received): CADW=+3=J7Rt1yxtTfW=ZCLC40-D1FPCFR7KGSyp_YLgcRcH3FQ@mail.gmail.com

Ivan Shapovalov

2015-07-05 15:13 UTC #15 lore raw

Quoted 21 lines · Dušan Čolić

Quoted 20 lines · Ivan Shapovalov
Quoted 18 lines · Edward Shishkin
Quoted 14 lines · Ivan Shapovalov
Quoted 3 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero
-filled
- if it did, infer alignnment from the first zero-filled
block,
infer granularity from the zero-filled region size.

Quoted 7 lines

Quoted 4 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
But what happens when someone makes an image of whole partition and uses it
on new different ssd?

Maybe we can make a tunefs.reiser4 (just) for that purpose.

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2015-07-06 08:56 UTC #16 lore raw

Quoted 17 lines · Ivan Shapovalov

Quoted 16 lines · Dušan Čolić
Quoted 15 lines · Ivan Shapovalov
Quoted 14 lines · Edward Shishkin
Quoted 13 lines · Ivan Shapovalov
Quoted 2 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero
-filled
- if it did, infer alignnment from the first zero-filled
block,
infer granularity from the zero-filled region size.

Quoted 7 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
But what happens when someone makes an image of whole partition and uses it
on new different ssd?
Maybe we can make a tunefs.reiser4 (just) for that purpose.

tunefs utilities usually accept already existing file system as one of input arguments. And it can happen that file system doesn't have enough space for "probing". So the cleanest option would be to create a simple tool for probing an empty non-formatted partition, and supply a script, which:

1) probes an empty partition;
2) dd;
3) updates superblock of the newly created image (by tunefs, etc) with proper discard parameters.

Thanks,
Edward.
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Edward Shishkin

author

2016-02-09 17:53 UTC #17 lore raw

In reply to Ivan Shapovalov

Hi all,

There is a pending patch for precise discarsd, which I can not merge because it is incomplete. Just want to know, is there any progress on this?

So we found that calculating discard params by mkfs and storing them in superblock is a dirty option, so I suggest to reserve a contiguous area (1 MiB?) on disk at "mkfs -d" time (just mark it busy in the bitmap). And calculate erase unit size and offset at mount time. If calculation failed for some reasons, then use non-precise discard.

So, I think, we'll need one d32 field in disk superblock, which indicates number of reserved blocks (0 means no reservation) and a pair of definitions in disk_format40.h:

#define FORMAT40_FIRST_RESERVED_FOR_PROBING \
      ((REISER4_MASTER_OFFSET / PAGE_CACHE_SIZE) + 6)

#define FORMAT40_MAX_BLOCKS_RESERVED_FOR_PROBING

If any questions, or technical stoppers, then let me know.

Thanks,
Edward.

Quoted 17 lines · Ivan Shapovalov

Quoted 16 lines · Dušan Čolić
Quoted 15 lines · Ivan Shapovalov
Quoted 14 lines · Edward Shishkin
Quoted 13 lines · Ivan Shapovalov
Quoted 2 lines · Edward Shishkin
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero
-filled
- if it did, infer alignnment from the first zero-filled
block,
infer granularity from the zero-filled region size.

Quoted 7 lines

Quoted 6 lines
Quoted 3 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
But what happens when someone makes an image of whole partition and uses it
on new different ssd?
Maybe we can make a tunefs.reiser4 (just) for that purpose.

--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Ivan Shapovalov

2016-02-10 04:04 UTC #18 lore raw

Quoted 18 lines · Edward Shishkin

Hi all,
There is a pending patch for precise discarsd, which I can not merge because it is incomplete. Just want to know, is there any progress on this?
So we found that calculating discard params by mkfs and storing them in superblock is a dirty option, so I suggest to reserve a contiguous area (1 MiB?) on disk at "mkfs -d" time (just mark it busy in the bitmap). And calculate erase unit size and offset at mount time.
If calculation failed for some reasons, then use non-precise discard.
So, I think, we'll need one d32 field in disk superblock, which indicates
number of reserved blocks (0 means no reservation) and a pair of definitions in disk_format40.h:

Quoted 9 lines

#define FORMAT40_FIRST_RESERVED_FOR_PROBING \
((REISER4_MASTER_OFFSET / PAGE_CACHE_SIZE) + 6) #define FORMAT40_MAX_BLOCKS_RESERVED_FOR_PROBING
If any questions, or technical stoppers, then let me know.
Thanks,
Edward.

Hello Edward,

yes, finishing this is on top of my to-do list (but it is being dequeued at a very slow rate due to the university...).

I was under assumption of that we still want to go by path of probing parameters at mkfs time + having a special utility for rewriting these fields. But if you are now ok with probing at mount time, then that's great. I'll try to do something in nearest week or two.

Signature · 1 line

Ivan Shapovalov / intelfx /

Edward Shishkin

author

2016-02-10 09:01 UTC #19 lore raw

Quoted 25 lines · Ivan Shapovalov

Quoted 24 lines · Edward Shishkin
Hi all,
There is a pending patch for precise discarsd, which I can not merge because it is incomplete. Just want to know, is there any progress on this?
So we found that calculating discard params by mkfs and storing them in superblock is a dirty option, so I suggest to reserve a contiguous area (1 MiB?) on disk at "mkfs -d" time (just mark it busy in the bitmap). And calculate erase unit size and offset at mount time.
If calculation failed for some reasons, then use non-precise discard.
So, I think, we'll need one d32 field in disk superblock, which indicates
number of reserved blocks (0 means no reservation) and a pair of definitions in disk_format40.h:
#define FORMAT40_FIRST_RESERVED_FOR_PROBING \
        ((REISER4_MASTER_OFFSET / PAGE_CACHE_SIZE) + 6)
#define FORMAT40_MAX_BLOCKS_RESERVED_FOR_PROBING
If any questions, or technical stoppers, then let me know.

Quoted 7 lines

Quoted 3 lines
Thanks,
Edward.
Hello Edward,
yes, finishing this is on top of my to-do list (but it is being dequeued at a very slow rate due to the university...).

Ah, this is of highest priority, ofc.

Quoted 3 lines

I was under assumption of that we still want to go by path of probing parameters at mkfs time + having a special utility for rewriting these

I am afraid that 99% of users will be unaware of that utility..

Thanks,
Edward.

Quoted 3 lines

fields. But if you are now ok with probing at mount time, then that's great. I'll try to do something in nearest week or two.

Ivan Shapovalov

2016-04-06 16:54 UTC #20 lore raw

In reply to Edward Shishkin

Quoted 18 lines · Edward Shishkin

Hi all,
There is a pending patch for precise discarsd, which I can not merge because it is incomplete. Just want to know, is there any progress on this?
So we found that calculating discard params by mkfs and storing them in superblock is a dirty option, so I suggest to reserve a contiguous area (1 MiB?) on disk at "mkfs -d" time (just mark it busy in the bitmap). And calculate erase unit size and offset at mount time.
If calculation failed for some reasons, then use non-precise discard.
So, I think, we'll need one d32 field in disk superblock, which indicates
number of reserved blocks (0 means no reservation) and a pair of definitions in disk_format40.h:

Quoted 8 lines

#define FORMAT40_FIRST_RESERVED_FOR_PROBING \
((REISER4_MASTER_OFFSET / PAGE_CACHE_SIZE) + 6) #define FORMAT40_MAX_BLOCKS_RESERVED_FOR_PROBING
If any questions, or technical stoppers, then let me know.
Thanks,
Edward.

I guess it would be nicer to allocate the needed space dynamically at mount time. However, there is a problem: where to put the probing code in the fill_super() (I suppose?) sequence? We want the allocator to be intact, but nothing should be written to the filesystem so far.

--
Ivan Shapovalov / intelfx /

Quoted 36 lines · Ivan Shapovalov

Quoted 33 lines · Dušan Čolić
Quoted 23 lines
On 5 Jul 2015 15:12, "Ivan Shapovalov"<[email protected]>  wro te:
Quoted 20 lines · Edward Shishkin
Quoted 18 lines · Ivan Shapovalov
Quoted 16 lines · Edward Shishkin
Quoted 3 lines
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
    - discard N sectors from the contiguous space
    - check if anything in the discarded space became zero -filled
    - if it did, infer alignnment from the first zero- filled
block,
      infer granularity from the zero-filled region size.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
But what happens when someone makes an image of whole partition and
uses it
on new different ssd?
Maybe we can make a tunefs.reiser4 (just) for that purpose.

Edward Shishkin

author

2016-04-06 18:03 UTC #21 lore raw

Quoted 23 lines · Ivan Shapovalov

Quoted 22 lines · Edward Shishkin
Hi all,
There is a pending patch for precise discarsd, which I can not merge because it is incomplete. Just want to know, is there any progress on this?
So we found that calculating discard params by mkfs and storing them in superblock is a dirty option, so I suggest to reserve a contiguous area (1 MiB?) on disk at "mkfs -d" time (just mark it busy in the bitmap). And calculate erase unit size and offset at mount time.
If calculation failed for some reasons, then use non-precise discard.
So, I think, we'll need one d32 field in disk superblock, which indicates
number of reserved blocks (0 means no reservation) and a pair of definitions in disk_format40.h:
#define FORMAT40_FIRST_RESERVED_FOR_PROBING \
        ((REISER4_MASTER_OFFSET / PAGE_CACHE_SIZE) + 6)
#define FORMAT40_MAX_BLOCKS_RESERVED_FOR_PROBING

Quoted 6 lines

Quoted 4 lines
If any questions, or technical stoppers, then let me know.
Thanks,
Edward.
I guess it would be nicer to allocate the needed space dynamically at mount time.

And if no enough space, then switch to non-precise discard mode? OK. Since the probing code is for SSD drives, it shouldn't lead to long mounts in the case of big terabyte partitions, which are full of data.

Quoted 2 lines

However, there is a problem: where to put the probing code
in the fill_super() (I suppose?) sequence?

Put it in try_init_format40(), right after the sa_init_allocator():
seems to be a right place.

Thanks,
Edward.

Quoted 35 lines

We want the allocator to be
intact, but nothing should be written to the filesystem so far.
--
Ivan Shapovalov / intelfx /
Quoted 29 lines · Ivan Shapovalov
Quoted 27 lines · Dušan Čolić
Quoted 18 lines
On 5 Jul 2015 15:12, "Ivan Shapovalov"<[email protected]> wro te:
Quoted 16 lines · Edward Shishkin
Quoted 15 lines · Ivan Shapovalov
Quoted 14 lines · Edward Shishkin
Quoted 2 lines
[...]
And how to test directly at mount time?
Something along the lines of
- allocate 1 MiB of contiguous space
- fill it with non-zeros
- for N = 1, 2, 4, ...:
- discard N sectors from the contiguous space
- check if anything in the discarded space became zero
-filled
- if it did, infer alignnment from the first zero-
filled
block,
infer granularity from the zero-filled region size.
Quoted 7 lines
Quoted 3 lines
Quoted 1 line
mkfs seems to be more suitable for this funny business
Yeah, sure. So... new superblock format with two extra fields?
But what happens when someone makes an image of whole partition and
uses it
on new different ssd?
Maybe we can make a tunefs.reiser4 (just) for that purpose.

--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html

Ivan Shapovalov

2015-07-04 18:06 UTC #22 lore raw

In reply to Edward Shishkin

Quoted 21 lines · Edward Shishkin

Quoted 19 lines · Ivan Shapovalov
Quoted 8 lines · Edward Shishkin
[...]
BTW, we need to do something with the "precise discard extension":
http://sourceforge.net/projects/reiser4/files/patches/
It reports erase unit 512 bytes for my samsung SSD 840 EVO. You said that this is incorrect. If so, then how to retrieve correct
discard parameters?
I was wrong about usage of hdparm -I. The "limit" it says about is in fact "how many 512-byte blocks worth of LBA ranges can be given
to the drive in a single ATA trim command"[1].
In fact, the standard (referenced below) doesn't seem to contain any references to the trim granularity, let alone to define any means
to query it.

Quoted 6 lines

Quoted 3 lines
So, I guess, the kernel will never tell us correct values for ATA SSDs,
and the only option is direct testing at mount time.
And how to test directly at mount time?

Quoted 1 line

It seems that nobody cares about it..

It's just ATA interface does not provide necessary data.

Signature · 1 line

Ivan Shapovalov / intelfx /