Re: [PATCH 02/16] f2fs: add on-disk layout
2012-10-12 (금), 21:05 +, Arnd Bergmann:
> On Friday 05 October 2012 20:56:44 김재극 wrote:
> > +struct f2fs_nat_entry {
> > + __u8 version;
> > + __le32 ino;
> > + __le32 block_addr;
> > +} __packed;
> > +
> > +struct f2fs_nat_block {
> > + struct f2fs_nat_entry entries[NAT_ENTRY_PER_BLOCK];
> > +} __packed;
>
> Using "__packed" on structure is rather inefficient on CPU architectures
> that cannot do aligned accesses. I would suggest you remove this
> attribute everywhere you can. The f2fs_nat_entry is particularly
> suboptimal because it is 9 bytes long, and I'm not sure if this
> can be reasonably changed to a multiple of four.
>
> In all other cases, you should try to lay out the structures
> so that each member is naturally aligned and you don't need any __packed
> attributes, in particular for those that are accessed a lot.
>
Ok, I'll check all the in-memory data structures.
Thank you for comments.
> Arnd
> --
> To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
> the body of a message to majord...@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
> Please read the FAQ at http://www.tux.org/lkml/
--
Jaegeuk Kim
Samsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
2012-10-12 (금), 21:05 +, Arnd Bergmann:
On Friday 05 October 2012 20:56:44 김재극 wrote:
+struct f2fs_nat_entry {
+ __u8 version;
+ __le32 ino;
+ __le32 block_addr;
+} __packed;
+
+struct f2fs_nat_block {
+ struct f2fs_nat_entry entries[NAT_ENTRY_PER_BLOCK];
+} __packed;
Using __packed on structure is rather inefficient on CPU architectures
that cannot do aligned accesses. I would suggest you remove this
attribute everywhere you can. The f2fs_nat_entry is particularly
suboptimal because it is 9 bytes long, and I'm not sure if this
can be reasonably changed to a multiple of four.
In all other cases, you should try to lay out the structures
so that each member is naturally aligned and you don't need any __packed
attributes, in particular for those that are accessed a lot.
Ok, I'll check all the in-memory data structures.
Thank you for comments.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
--
Jaegeuk Kim
Samsung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On Friday 05 October 2012 20:56:44 김재극 wrote:
> +struct f2fs_nat_entry {
> + __u8 version;
> + __le32 ino;
> + __le32 block_addr;
> +} __packed;
> +
> +struct f2fs_nat_block {
> + struct f2fs_nat_entry entries[NAT_ENTRY_PER_BLOCK];
> +} __packed;
Using "__packed" on structure is rather inefficient on CPU architectures
that cannot do aligned accesses. I would suggest you remove this
attribute everywhere you can. The f2fs_nat_entry is particularly
suboptimal because it is 9 bytes long, and I'm not sure if this
can be reasonably changed to a multiple of four.
In all other cases, you should try to lay out the structures
so that each member is naturally aligned and you don't need any __packed
attributes, in particular for those that are accessed a lot.
Arnd
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On Friday 05 October 2012 20:56:44 김재극 wrote:
+struct f2fs_nat_entry {
+ __u8 version;
+ __le32 ino;
+ __le32 block_addr;
+} __packed;
+
+struct f2fs_nat_block {
+ struct f2fs_nat_entry entries[NAT_ENTRY_PER_BLOCK];
+} __packed;
Using __packed on structure is rather inefficient on CPU architectures
that cannot do aligned accesses. I would suggest you remove this
attribute everywhere you can. The f2fs_nat_entry is particularly
suboptimal because it is 9 bytes long, and I'm not sure if this
can be reasonably changed to a multiple of four.
In all other cases, you should try to lay out the structures
so that each member is naturally aligned and you don't need any __packed
attributes, in particular for those that are accessed a lot.
Arnd
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 02/16] f2fs: add on-disk layout
[snip]
> > +/*
> > + * For superblock
> > + */
> > +struct f2fs_super_block {
> > + __le32 magic; /* Magic Number */
> > + __le16 major_ver; /* Major Version */
> > + __le16 minor_ver; /* Minor Version */
> > + __le32 log_sectorsize; /* log2 (Sector size in bytes) */
> > + __le32 log_sectors_per_block; /* log2 (Number of sectors per block */
> > + __le32 log_blocksize; /* log2 (Block size in bytes) */
>
> Why store log_blocksize on disk when it can be calculated from
> log_sectorsize and log_sectors_per_block? It may be handy to keep this
> in the in-memory superblock but keeping it on-disk means you need a
> consistency check and error code path when loading the superblock.
>
Yes, I added this for sanity check of superblock.
> > +struct f2fs_inode {
> > + __le16 i_mode; /* File mode */
> > + __le16 i_reserved; /* Reserved */
> > + __le32 i_uid; /* User ID */
> > + __le32 i_gid; /* Group ID */
> > + __le32 i_links; /* Links count */
> > + __le64 i_size; /* File size in bytes */
> > + __le64 i_blocks;/* File size in bytes */
>
> File size in blocks
>
> > +struct direct_node {
> > + __le32 addr[ADDRS_PER_BLOCK]; /* aray of data block address */
>
> s/aray/array/
>
> > +} __packed;
> > +
> > +struct indirect_node {
> > + __le32 nid[NIDS_PER_BLOCK]; /* aray of data block address */
>
> s/aray/array/
I'll change them.
Thank you.
>
> Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 02/16] f2fs: add on-disk layout
[snip]
+/*
+ * For superblock
+ */
+struct f2fs_super_block {
+ __le32 magic; /* Magic Number */
+ __le16 major_ver; /* Major Version */
+ __le16 minor_ver; /* Minor Version */
+ __le32 log_sectorsize; /* log2 (Sector size in bytes) */
+ __le32 log_sectors_per_block; /* log2 (Number of sectors per block */
+ __le32 log_blocksize; /* log2 (Block size in bytes) */
Why store log_blocksize on disk when it can be calculated from
log_sectorsize and log_sectors_per_block? It may be handy to keep this
in the in-memory superblock but keeping it on-disk means you need a
consistency check and error code path when loading the superblock.
Yes, I added this for sanity check of superblock.
+struct f2fs_inode {
+ __le16 i_mode; /* File mode */
+ __le16 i_reserved; /* Reserved */
+ __le32 i_uid; /* User ID */
+ __le32 i_gid; /* Group ID */
+ __le32 i_links; /* Links count */
+ __le64 i_size; /* File size in bytes */
+ __le64 i_blocks;/* File size in bytes */
File size in blocks
+struct direct_node {
+ __le32 addr[ADDRS_PER_BLOCK]; /* aray of data block address */
s/aray/array/
+} __packed;
+
+struct indirect_node {
+ __le32 nid[NIDS_PER_BLOCK]; /* aray of data block address */
s/aray/array/
I'll change them.
Thank you.
Stefan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 02/16] f2fs: add on-disk layout
Dear David Sterba,
David Sterba wrote:
> On Fri, Oct 05, 2012 at 08:56:44PM +0900, ? wrote:
> > +struct node_footer {
> > + __le32 nid; /* node id */
> > + __le32 ino; /* inode nunmber */
> > + __le32 cold:1; /* cold mark */
> > + __le32 fsync:1; /* fsync mark */
> > + __le32 dentry:1;/* dentry mark */
> > + __le32 offset:29; /* offset in inode's node space */
>
> A bitfield for a on-disk structure? This will have endianity issues,
> (but I don't know if you intend to support big-endian). It's not enough
> to use cpu_to_le* as in
>
> fill_node_footer(...) {
>
> rn->footer.offset = cpu_to_le32(ofs);
>
> }
>
> because the bitfield inside the structure will be already defined
> reversed. The cpu_to_le macro will only convert value of 'ofs' but will
> place it to different bits than it would on a little-endian arch.
>
> There are macros to define bitfields in an endian-neutral way (or do it
> by #ifdefs though it also involves duplicating the item names), or you
> can alternatively use two structs fr disk-only and memory-only access,
> the disk one stores __le32 with value combined of all and the in-memory
> gets set up properly and will look like your current version of the
> structure.
>
> (More about not using bitfields
> http://yarchive.net/comp/linux/bitfields.html)
I appreciate your kind feedback, but we should've avoided using bitfields
for on-disk structure. As you suggested, we'll revise them. The latter
approach would be good.
>
>
> david
Chul
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
RE: [PATCH 02/16] f2fs: add on-disk layout
Dear David Sterba,
David Sterba wrote:
On Fri, Oct 05, 2012 at 08:56:44PM +0900, ? wrote:
+struct node_footer {
+ __le32 nid; /* node id */
+ __le32 ino; /* inode nunmber */
+ __le32 cold:1; /* cold mark */
+ __le32 fsync:1; /* fsync mark */
+ __le32 dentry:1;/* dentry mark */
+ __le32 offset:29; /* offset in inode's node space */
A bitfield for a on-disk structure? This will have endianity issues,
(but I don't know if you intend to support big-endian). It's not enough
to use cpu_to_le* as in
fill_node_footer(...) {
rn-footer.offset = cpu_to_le32(ofs);
}
because the bitfield inside the structure will be already defined
reversed. The cpu_to_le macro will only convert value of 'ofs' but will
place it to different bits than it would on a little-endian arch.
There are macros to define bitfields in an endian-neutral way (or do it
by #ifdefs though it also involves duplicating the item names), or you
can alternatively use two structs fr disk-only and memory-only access,
the disk one stores __le32 with value combined of all and the in-memory
gets set up properly and will look like your current version of the
structure.
(More about not using bitfields
http://yarchive.net/comp/linux/bitfields.html)
I appreciate your kind feedback, but we should've avoided using bitfields
for on-disk structure. As you suggested, we'll revise them. The latter
approach would be good.
david
Chul
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
> +/*
> + * For superblock
> + */
> +struct f2fs_super_block {
> + __le32 magic; /* Magic Number */
> + __le16 major_ver; /* Major Version */
> + __le16 minor_ver; /* Minor Version */
> + __le32 log_sectorsize; /* log2 (Sector size in bytes) */
> + __le32 log_sectors_per_block; /* log2 (Number of sectors per block */
> + __le32 log_blocksize; /* log2 (Block size in bytes) */
Why store log_blocksize on disk when it can be calculated from
log_sectorsize and log_sectors_per_block? It may be handy to keep this
in the in-memory superblock but keeping it on-disk means you need a
consistency check and error code path when loading the superblock.
> +struct f2fs_inode {
> + __le16 i_mode; /* File mode */
> + __le16 i_reserved; /* Reserved */
> + __le32 i_uid; /* User ID */
> + __le32 i_gid; /* Group ID */
> + __le32 i_links; /* Links count */
> + __le64 i_size; /* File size in bytes */
> + __le64 i_blocks;/* File size in bytes */
File size in blocks
> +struct direct_node {
> + __le32 addr[ADDRS_PER_BLOCK]; /* aray of data block address */
s/aray/array/
> +} __packed;
> +
> +struct indirect_node {
> + __le32 nid[NIDS_PER_BLOCK]; /* aray of data block address */
s/aray/array/
Stefan
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
+/*
+ * For superblock
+ */
+struct f2fs_super_block {
+ __le32 magic; /* Magic Number */
+ __le16 major_ver; /* Major Version */
+ __le16 minor_ver; /* Minor Version */
+ __le32 log_sectorsize; /* log2 (Sector size in bytes) */
+ __le32 log_sectors_per_block; /* log2 (Number of sectors per block */
+ __le32 log_blocksize; /* log2 (Block size in bytes) */
Why store log_blocksize on disk when it can be calculated from
log_sectorsize and log_sectors_per_block? It may be handy to keep this
in the in-memory superblock but keeping it on-disk means you need a
consistency check and error code path when loading the superblock.
+struct f2fs_inode {
+ __le16 i_mode; /* File mode */
+ __le16 i_reserved; /* Reserved */
+ __le32 i_uid; /* User ID */
+ __le32 i_gid; /* Group ID */
+ __le32 i_links; /* Links count */
+ __le64 i_size; /* File size in bytes */
+ __le64 i_blocks;/* File size in bytes */
File size in blocks
+struct direct_node {
+ __le32 addr[ADDRS_PER_BLOCK]; /* aray of data block address */
s/aray/array/
+} __packed;
+
+struct indirect_node {
+ __le32 nid[NIDS_PER_BLOCK]; /* aray of data block address */
s/aray/array/
Stefan
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On Fri, Oct 05, 2012 at 08:56:44PM +0900, ? wrote:
> +struct node_footer {
> + __le32 nid; /* node id */
> + __le32 ino; /* inode nunmber */
> + __le32 cold:1; /* cold mark */
> + __le32 fsync:1; /* fsync mark */
> + __le32 dentry:1;/* dentry mark */
> + __le32 offset:29; /* offset in inode's node space */
A bitfield for a on-disk structure? This will have endianity issues,
(but I don't know if you intend to support big-endian). It's not enough
to use cpu_to_le* as in
fill_node_footer(...) {
rn->footer.offset = cpu_to_le32(ofs);
}
because the bitfield inside the structure will be already defined
reversed. The cpu_to_le macro will only convert value of 'ofs' but will
place it to different bits than it would on a little-endian arch.
There are macros to define bitfields in an endian-neutral way (or do it
by #ifdefs though it also involves duplicating the item names), or you
can alternatively use two structs fr disk-only and memory-only access,
the disk one stores __le32 with value combined of all and the in-memory
gets set up properly and will look like your current version of the
structure.
(More about not using bitfields http://yarchive.net/comp/linux/bitfields.html)
> + __le64 cp_ver; /* checkpoint version */
> + __le32 next_blkaddr;/* next node page block address */
> +} __packed;
> +
david
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
2012-10-05 (금), 19:46 +0200, Martin Steigerwald:
> Am Freitag, 5. Oktober 2012 schrieb 김재극:
> > This adds a header file describing the on-disk layout of f2fs.
> >
> > Signed-off-by: Changman Lee
> > Signed-off-by: Chul Lee
> > Signed-off-by: Jaegeuk Kim
> > ---
> > include/linux/f2fs_fs.h | 359
> > +++
> > 1 file changed, 359 insertions(+)
> > create mode 100644 include/linux/f2fs_fs.h
> >
> > diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> > new file mode 100644
> > index 000..b17eeec
> > --- /dev/null
> > +++ b/include/linux/f2fs_fs.h
> > @@ -0,0 +1,359 @@
> > +/**
> > + * include/linux/f2fs_fs.h
> […]
> > +/*
> > + * For superblock
> > + */
> > +struct f2fs_super_block {
> > + __le32 magic; /* Magic Number */
> > + __le16 major_ver; /* Major Version */
> > + __le16 minor_ver; /* Minor Version */
> > + __le32 log_sectorsize; /* log2 (Sector size in bytes) */
> > + __le32 log_sectors_per_block; /* log2 (Number of sectors per
> > block */
> > + __le32 log_blocksize; /* log2 (Block size in bytes) */
> > + __le32 log_blocks_per_seg; /* log2 (Number of blocks per segment) */
> > + __le32 log_segs_per_sec; /* log2 (Number of segments per section) */
> > + __le32 secs_per_zone; /* Number of sections per zone */
> > + __le32 checksum_offset; /* Checksum position in this super block */
> > + __le64 block_count; /* Total number of blocks */
> > + __le32 section_count; /* Total number of sections */
> > + __le32 segment_count; /* Total number of segments */
> > + __le32 segment_count_ckpt; /* Total number of segments
> > + in Checkpoint area */
> > + __le32 segment_count_sit; /* Total number of segments
> > +in Segment information table */
> > + __le32 segment_count_nat; /* Total number of segments
> > +in Node address table */
> > + /*Total number of segments in Segment summary area */
> > + __le32 segment_count_ssa;
> > + /* Total number of segments in Main area */
> > + __le32 segment_count_main;
> > + __le32 failure_safe_block_distance;
> > + __le64 segment0_blkaddr;/* Start block address of Segment 0
> > */
> > + __le64 start_segment_checkpoint; /* Start block address of ckpt */
> > + __le64 sit_blkaddr; /* Start block address of SIT */
> > + __le64 nat_blkaddr; /* Start block address of NAT */
> > + __le64 ssa_blkaddr; /* Start block address of SSA */
> > + __le64 main_blkaddr;/* Start block address of Main area */
> > + __le32 root_ino;/* Root directory inode number */
> > + __le32 node_ino;/* node inode number */
> > + __le32 meta_ino;/* meta inode number */
> > + __le32 volume_serial_number;/* VSN is optional field */
> > + __le16 volume_name[8]; /* Volume Name. 8 unicode characters */
> > +} __packed;
>
> Any reason why volume label is that short?
>
Noop.
> If superblock can occupy up to 4096 bytes, i.e. a block, I think a bit
> more space for volume label would fit in there. I count 128 bytes up
> to volume_name. Do other structures need to fit in superblock as well?
>
I think it cannot be a problem to give more bytes to the name space.
I'll do this.
> I believe users easily can hit a limit of 8 unicode characters even for
> USB sticks and SD cards.
> Nice to see a flash friendly filesystem which seems to take lots of the
> stuff in account I read at Linaro website about flash devices.
>
Thank you.
--
Jaegeuk Kim
Samsung
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
2012-10-05 (금), 14:09 -0700, Boaz Harrosh: > On 10/05/2012 10:54 AM, Dan Luedtke wrote: > > On Fri, 2012-10-05 at 20:56 +0900, 김재극 wrote: > >> + __le32 i_atime; /* Access time */ > >> + __le32 i_ctime; /* inode Change time */ > >> + __le32 i_mtime; /* Modification time */ > >> + __le32 i_btime; /* file creation time*/ > > > > It's probably to late, but have you considered using 64 bit timestamps? > > In LanyFS we use 64 bit and I'm very interested in reasoning for 32 bit > > timestamps. > > > > Yes and please don't make them millisecond resolution. This is a real > headache. > From the like of make to NFS to reboot/corruption recovery... the list is > endless > > Please make it nanoseconds resolution. > Ok, I'll take a look at this. Thanks, > > Regards > > > > Dan > > > > Thanks > Boaz > -- > To unsubscribe from this list: send the line "unsubscribe linux-kernel" in > the body of a message to majord...@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html > Please read the FAQ at http://www.tux.org/lkml/ -- Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
2012-10-05 (금), 14:09 -0700, Boaz Harrosh: On 10/05/2012 10:54 AM, Dan Luedtke wrote: On Fri, 2012-10-05 at 20:56 +0900, 김재극 wrote: + __le32 i_atime; /* Access time */ + __le32 i_ctime; /* inode Change time */ + __le32 i_mtime; /* Modification time */ + __le32 i_btime; /* file creation time*/ It's probably to late, but have you considered using 64 bit timestamps? In LanyFS we use 64 bit and I'm very interested in reasoning for 32 bit timestamps. Yes and please don't make them millisecond resolution. This is a real headache. From the like of make to NFS to reboot/corruption recovery... the list is endless Please make it nanoseconds resolution. Ok, I'll take a look at this. Thanks, Regards Dan Thanks Boaz -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ -- Jaegeuk Kim Samsung -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
2012-10-05 (금), 19:46 +0200, Martin Steigerwald:
Am Freitag, 5. Oktober 2012 schrieb 김재극:
This adds a header file describing the on-disk layout of f2fs.
Signed-off-by: Changman Lee cm224@samsung.com
Signed-off-by: Chul Lee chur@samsung.com
Signed-off-by: Jaegeuk Kim jaegeuk@samsung.com
---
include/linux/f2fs_fs.h | 359
+++
1 file changed, 359 insertions(+)
create mode 100644 include/linux/f2fs_fs.h
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
new file mode 100644
index 000..b17eeec
--- /dev/null
+++ b/include/linux/f2fs_fs.h
@@ -0,0 +1,359 @@
+/**
+ * include/linux/f2fs_fs.h
[…]
+/*
+ * For superblock
+ */
+struct f2fs_super_block {
+ __le32 magic; /* Magic Number */
+ __le16 major_ver; /* Major Version */
+ __le16 minor_ver; /* Minor Version */
+ __le32 log_sectorsize; /* log2 (Sector size in bytes) */
+ __le32 log_sectors_per_block; /* log2 (Number of sectors per
block */
+ __le32 log_blocksize; /* log2 (Block size in bytes) */
+ __le32 log_blocks_per_seg; /* log2 (Number of blocks per segment) */
+ __le32 log_segs_per_sec; /* log2 (Number of segments per section) */
+ __le32 secs_per_zone; /* Number of sections per zone */
+ __le32 checksum_offset; /* Checksum position in this super block */
+ __le64 block_count; /* Total number of blocks */
+ __le32 section_count; /* Total number of sections */
+ __le32 segment_count; /* Total number of segments */
+ __le32 segment_count_ckpt; /* Total number of segments
+ in Checkpoint area */
+ __le32 segment_count_sit; /* Total number of segments
+in Segment information table */
+ __le32 segment_count_nat; /* Total number of segments
+in Node address table */
+ /*Total number of segments in Segment summary area */
+ __le32 segment_count_ssa;
+ /* Total number of segments in Main area */
+ __le32 segment_count_main;
+ __le32 failure_safe_block_distance;
+ __le64 segment0_blkaddr;/* Start block address of Segment 0
*/
+ __le64 start_segment_checkpoint; /* Start block address of ckpt */
+ __le64 sit_blkaddr; /* Start block address of SIT */
+ __le64 nat_blkaddr; /* Start block address of NAT */
+ __le64 ssa_blkaddr; /* Start block address of SSA */
+ __le64 main_blkaddr;/* Start block address of Main area */
+ __le32 root_ino;/* Root directory inode number */
+ __le32 node_ino;/* node inode number */
+ __le32 meta_ino;/* meta inode number */
+ __le32 volume_serial_number;/* VSN is optional field */
+ __le16 volume_name[8]; /* Volume Name. 8 unicode characters */
+} __packed;
Any reason why volume label is that short?
Noop.
If superblock can occupy up to 4096 bytes, i.e. a block, I think a bit
more space for volume label would fit in there. I count 128 bytes up
to volume_name. Do other structures need to fit in superblock as well?
I think it cannot be a problem to give more bytes to the name space.
I'll do this.
I believe users easily can hit a limit of 8 unicode characters even for
USB sticks and SD cards.
Nice to see a flash friendly filesystem which seems to take lots of the
stuff in account I read at Linaro website about flash devices.
Thank you.
--
Jaegeuk Kim
Samsung
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On Fri, Oct 05, 2012 at 08:56:44PM +0900, ? wrote:
+struct node_footer {
+ __le32 nid; /* node id */
+ __le32 ino; /* inode nunmber */
+ __le32 cold:1; /* cold mark */
+ __le32 fsync:1; /* fsync mark */
+ __le32 dentry:1;/* dentry mark */
+ __le32 offset:29; /* offset in inode's node space */
A bitfield for a on-disk structure? This will have endianity issues,
(but I don't know if you intend to support big-endian). It's not enough
to use cpu_to_le* as in
fill_node_footer(...) {
rn-footer.offset = cpu_to_le32(ofs);
}
because the bitfield inside the structure will be already defined
reversed. The cpu_to_le macro will only convert value of 'ofs' but will
place it to different bits than it would on a little-endian arch.
There are macros to define bitfields in an endian-neutral way (or do it
by #ifdefs though it also involves duplicating the item names), or you
can alternatively use two structs fr disk-only and memory-only access,
the disk one stores __le32 with value combined of all and the in-memory
gets set up properly and will look like your current version of the
structure.
(More about not using bitfields http://yarchive.net/comp/linux/bitfields.html)
+ __le64 cp_ver; /* checkpoint version */
+ __le32 next_blkaddr;/* next node page block address */
+} __packed;
+
david
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On 10/05/2012 10:54 AM, Dan Luedtke wrote: > On Fri, 2012-10-05 at 20:56 +0900, 김재극 wrote: >> +__le32 i_atime; /* Access time */ >> +__le32 i_ctime; /* inode Change time */ >> +__le32 i_mtime; /* Modification time */ >> +__le32 i_btime; /* file creation time*/ > > It's probably to late, but have you considered using 64 bit timestamps? > In LanyFS we use 64 bit and I'm very interested in reasoning for 32 bit > timestamps. > Yes and please don't make them millisecond resolution. This is a real headache. >From the like of make to NFS to reboot/corruption recovery... the list is >endless Please make it nanoseconds resolution. > Regards > > Dan > Thanks Boaz -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On Fri, 2012-10-05 at 20:56 +0900, 김재극 wrote: > + __le32 i_atime; /* Access time */ > + __le32 i_ctime; /* inode Change time */ > + __le32 i_mtime; /* Modification time */ > + __le32 i_btime; /* file creation time*/ It's probably to late, but have you considered using 64 bit timestamps? In LanyFS we use 64 bit and I'm very interested in reasoning for 32 bit timestamps. Regards Dan -- Dan Luedtke http://www.danrl.de -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
Am Freitag, 5. Oktober 2012 schrieb 김재극:
> This adds a header file describing the on-disk layout of f2fs.
>
> Signed-off-by: Changman Lee
> Signed-off-by: Chul Lee
> Signed-off-by: Jaegeuk Kim
> ---
> include/linux/f2fs_fs.h | 359
> +++
> 1 file changed, 359 insertions(+)
> create mode 100644 include/linux/f2fs_fs.h
>
> diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
> new file mode 100644
> index 000..b17eeec
> --- /dev/null
> +++ b/include/linux/f2fs_fs.h
> @@ -0,0 +1,359 @@
> +/**
> + * include/linux/f2fs_fs.h
[…]
> +/*
> + * For superblock
> + */
> +struct f2fs_super_block {
> + __le32 magic; /* Magic Number */
> + __le16 major_ver; /* Major Version */
> + __le16 minor_ver; /* Minor Version */
> + __le32 log_sectorsize; /* log2 (Sector size in bytes) */
> + __le32 log_sectors_per_block; /* log2 (Number of sectors per block
> */
> + __le32 log_blocksize; /* log2 (Block size in bytes) */
> + __le32 log_blocks_per_seg; /* log2 (Number of blocks per segment) */
> + __le32 log_segs_per_sec; /* log2 (Number of segments per section) */
> + __le32 secs_per_zone; /* Number of sections per zone */
> + __le32 checksum_offset; /* Checksum position in this super block */
> + __le64 block_count; /* Total number of blocks */
> + __le32 section_count; /* Total number of sections */
> + __le32 segment_count; /* Total number of segments */
> + __le32 segment_count_ckpt; /* Total number of segments
> + in Checkpoint area */
> + __le32 segment_count_sit; /* Total number of segments
> +in Segment information table */
> + __le32 segment_count_nat; /* Total number of segments
> +in Node address table */
> + /*Total number of segments in Segment summary area */
> + __le32 segment_count_ssa;
> + /* Total number of segments in Main area */
> + __le32 segment_count_main;
> + __le32 failure_safe_block_distance;
> + __le64 segment0_blkaddr;/* Start block address of Segment 0 */
> + __le64 start_segment_checkpoint; /* Start block address of ckpt */
> + __le64 sit_blkaddr; /* Start block address of SIT */
> + __le64 nat_blkaddr; /* Start block address of NAT */
> + __le64 ssa_blkaddr; /* Start block address of SSA */
> + __le64 main_blkaddr;/* Start block address of Main area */
> + __le32 root_ino;/* Root directory inode number */
> + __le32 node_ino;/* node inode number */
> + __le32 meta_ino;/* meta inode number */
> + __le32 volume_serial_number;/* VSN is optional field */
> + __le16 volume_name[8]; /* Volume Name. 8 unicode characters */
> +} __packed;
Any reason why volume label is that short?
If superblock can occupy up to 4096 bytes, i.e. a block, I think a bit
more space for volume label would fit in there. I count 128 bytes up
to volume_name. Do other structures need to fit in superblock as well?
I believe users easily can hit a limit of 8 unicode characters even for
USB sticks and SD cards.
Nice to see a flash friendly filesystem which seems to take lots of the
stuff in account I read at Linaro website about flash devices.
--
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA B82F 991B EAAC A599 84C7
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
Am Freitag, 5. Oktober 2012 schrieb 김재극:
This adds a header file describing the on-disk layout of f2fs.
Signed-off-by: Changman Lee cm224@samsung.com
Signed-off-by: Chul Lee chur@samsung.com
Signed-off-by: Jaegeuk Kim jaegeuk@samsung.com
---
include/linux/f2fs_fs.h | 359
+++
1 file changed, 359 insertions(+)
create mode 100644 include/linux/f2fs_fs.h
diff --git a/include/linux/f2fs_fs.h b/include/linux/f2fs_fs.h
new file mode 100644
index 000..b17eeec
--- /dev/null
+++ b/include/linux/f2fs_fs.h
@@ -0,0 +1,359 @@
+/**
+ * include/linux/f2fs_fs.h
[…]
+/*
+ * For superblock
+ */
+struct f2fs_super_block {
+ __le32 magic; /* Magic Number */
+ __le16 major_ver; /* Major Version */
+ __le16 minor_ver; /* Minor Version */
+ __le32 log_sectorsize; /* log2 (Sector size in bytes) */
+ __le32 log_sectors_per_block; /* log2 (Number of sectors per block
*/
+ __le32 log_blocksize; /* log2 (Block size in bytes) */
+ __le32 log_blocks_per_seg; /* log2 (Number of blocks per segment) */
+ __le32 log_segs_per_sec; /* log2 (Number of segments per section) */
+ __le32 secs_per_zone; /* Number of sections per zone */
+ __le32 checksum_offset; /* Checksum position in this super block */
+ __le64 block_count; /* Total number of blocks */
+ __le32 section_count; /* Total number of sections */
+ __le32 segment_count; /* Total number of segments */
+ __le32 segment_count_ckpt; /* Total number of segments
+ in Checkpoint area */
+ __le32 segment_count_sit; /* Total number of segments
+in Segment information table */
+ __le32 segment_count_nat; /* Total number of segments
+in Node address table */
+ /*Total number of segments in Segment summary area */
+ __le32 segment_count_ssa;
+ /* Total number of segments in Main area */
+ __le32 segment_count_main;
+ __le32 failure_safe_block_distance;
+ __le64 segment0_blkaddr;/* Start block address of Segment 0 */
+ __le64 start_segment_checkpoint; /* Start block address of ckpt */
+ __le64 sit_blkaddr; /* Start block address of SIT */
+ __le64 nat_blkaddr; /* Start block address of NAT */
+ __le64 ssa_blkaddr; /* Start block address of SSA */
+ __le64 main_blkaddr;/* Start block address of Main area */
+ __le32 root_ino;/* Root directory inode number */
+ __le32 node_ino;/* node inode number */
+ __le32 meta_ino;/* meta inode number */
+ __le32 volume_serial_number;/* VSN is optional field */
+ __le16 volume_name[8]; /* Volume Name. 8 unicode characters */
+} __packed;
Any reason why volume label is that short?
If superblock can occupy up to 4096 bytes, i.e. a block, I think a bit
more space for volume label would fit in there. I count 128 bytes up
to volume_name. Do other structures need to fit in superblock as well?
I believe users easily can hit a limit of 8 unicode characters even for
USB sticks and SD cards.
Nice to see a flash friendly filesystem which seems to take lots of the
stuff in account I read at Linaro website about flash devices.
--
Martin 'Helios' Steigerwald - http://www.Lichtvoll.de
GPG: 03B0 0D6C 0040 0710 4AFA B82F 991B EAAC A599 84C7
--
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On Fri, 2012-10-05 at 20:56 +0900, 김재극 wrote: + __le32 i_atime; /* Access time */ + __le32 i_ctime; /* inode Change time */ + __le32 i_mtime; /* Modification time */ + __le32 i_btime; /* file creation time*/ It's probably to late, but have you considered using 64 bit timestamps? In LanyFS we use 64 bit and I'm very interested in reasoning for 32 bit timestamps. Regards Dan -- Dan Luedtke http://www.danrl.de -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH 02/16] f2fs: add on-disk layout
On 10/05/2012 10:54 AM, Dan Luedtke wrote: On Fri, 2012-10-05 at 20:56 +0900, 김재극 wrote: +__le32 i_atime; /* Access time */ +__le32 i_ctime; /* inode Change time */ +__le32 i_mtime; /* Modification time */ +__le32 i_btime; /* file creation time*/ It's probably to late, but have you considered using 64 bit timestamps? In LanyFS we use 64 bit and I'm very interested in reasoning for 32 bit timestamps. Yes and please don't make them millisecond resolution. This is a real headache. From the like of make to NFS to reboot/corruption recovery... the list is endless Please make it nanoseconds resolution. Regards Dan Thanks Boaz -- To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
