Skip to contentSkip to conversation

ocfs2: return non-zero st_blocks for inline data [resend2]

PATCH New John Haxby 4 participants 7 messages Nov 24, 2015 – Dec 18, 2015 OCFS2

lore
Share & tools
lore

John Haxby

author
#0 lore raw

cover ocfs2: return non-zero st_blocks for inline data [resend2]

Hello All,

[Really sorry about this and I hope you're not getting fed up of
multiple copies of this message but the list on oss.oracle.com really doesn't like me.]

Some programs, and programmers, assume that if a file is occupying zero blocks (st_blocks == 0) then it contains no data and there's no point in reading it. Posix doesn't actually say anything about this, but it seems to be something a lot of people expect. Indeed, ext4, btrfs and ntfs-3d all seem to behave this way so that no one[1] has any unpleasant surprises.

This patch is almost exactly the same as commit 9206c561554c ("ext4:
return non-zero st_blocks for inline data") although I couldn't bring myself to include the typo in the comment :)

jch

[resend because rejected by list the first time.]
[1] tar, I'm looking at you, but you're not the only one.

John Haxby (1):
ocfs2: return non-zero st_blocks for inline data

 fs/ocfs2/file.c | 8 ++++++++
 1 file changed, 8 insertions(+)
Signature · 1 line
2.5.0
author
#1 lore raw

PATCH 1/1 ocfs2: return non-zero st_blocks for inline data

Some versions of tar assume that files with st_blocks == 0 do not contain any data and will skip reading them entirely. See also commit 9206c561554c ("ext4: return non-zero st_blocks for inline data").

---
 fs/ocfs2/file.c | 8 ++++++++
 1 file changed, 8 insertions(+)
fs/ocfs2/file.c +8 -0
index 0e5b451..d631279 100644
@@ -1302,6 +1302,14 @@ int ocfs2_getattr(struct vfsmount *mnt,
 	}
 
 	generic_fillattr(inode, stat);
+	/*
+	 * If there is inline data in the inode, the inode will normally not
+	 * have data blocks allocated (it may have an external xattr block).
+	 * Report at least one sector for such files, so tools like tar, rsync,
+	 * others don't incorrectly think the file is completely sparse.
+	 */
+	if (unlikely(OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL))
+		stat->blocks += (stat->size + 511)>>9;
 
 	/* We set the blksize from the cluster size for performance */
 	stat->blksize = osb->s_clustersize;
Signature · 1 line
2.5.0
#2 lore raw

PATCH 1/1 ocfs2: return non-zero st_blocks for inline data

Looks good for me.

Thanks
Gang

Quoted 37 lines
Quoted 1 line
Quoted 1 line

Some versions of tar assume that files with st_blocks == 0 do not contain any data and will skip reading them entirely. See also commit 9206c561554c ("ext4: return non-zero st_blocks for inline data").

---
 fs/ocfs2/file.c | 8 ++++++++
 1 file changed, 8 insertions(+)
fs/ocfs2/file.c +8 -0
index 0e5b451..d631279 100644
@@ -1302,6 +1302,14 @@ int ocfs2_getattr(struct vfsmount *mnt,
 	}
 
 	generic_fillattr(inode, stat);
+	/*
+	 * If there is inline data in the inode, the inode will normally not
+	 * have data blocks allocated (it may have an external xattr block).
+	 * Report at least one sector for such files, so tools like tar, rsync,
+	 * others don't incorrectly think the file is completely sparse.
+	 */
+	if (unlikely(OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL))
+		stat->blocks += (stat->size + 511)>>9;
 
 	/* We set the blksize from the cluster size for performance */
 	stat->blksize = osb->s_clustersize;
Signature · 7 lines
2.5.0


_______________________________________________
Ocfs2-devel mailing list
Ocfs2-devel at oss.oracle.com 
https://oss.oracle.com/mailman/listinfo/ocfs2-devel
#3 lore raw

PATCH 1/1 ocfs2: return non-zero st_blocks for inline data

In reply to John Haxby
Quoted 15 lines · John Haxby

Some versions of tar assume that files with st_blocks == 0 do not contain any data and will skip reading them entirely. See also commit 9206c561554c ("ext4: return non-zero st_blocks for inline data").

---
 fs/ocfs2/file.c | 8 ++++++++
 1 file changed, 8 insertions(+)
fs/ocfs2/file.c +0 -0
index 0e5b451..d631279 100644
@@ -1302,6 +1302,14 @@ int ocfs2_getattr(struct vfsmount *mnt,
 	}

fs/ocfs2/file.c int ocfs2_getattr(struct vfsmount *mnt, · line 1312

Quoted 10 lines
fs/ocfs2/file.c +8 -0
 
 	generic_fillattr(inode, stat);
+	/*
+	 * If there is inline data in the inode, the inode will normally not
+	 * have data blocks allocated (it may have an external xattr block).
+	 * Report at least one sector for such files, so tools like tar, rsync,
+	 * others don't incorrectly think the file is completely sparse.
+	 */
+	if (unlikely(OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL))
+		stat->blocks += (stat->size + 511)>>9;

From filesystem side, looks reasonable that data block is 0 for inlined-data file. This is like a hack to filesystem to fix tools issue. Indeed tar-1.26-27 have been fixed to not think file with st_blocks == 0 empty. But I am not sure why ext4 merge that fix.

Thanks,
Junxiao.

Quoted 4 lines
fs/ocfs2/file.c +0 -0
 	/* We set the blksize from the cluster size for performance */
 	stat->blksize = osb->s_clustersize;
 
author
#4 lore raw

PATCH 1/1 ocfs2: return non-zero st_blocks for inline data

Quoted 20 lines · Junxiao Bi
Quoted 16 lines · John Haxby

Some versions of tar assume that files with st_blocks == 0 do not contain any data and will skip reading them entirely. See also commit 9206c561554c ("ext4: return non-zero st_blocks for inline data").

---
fs/ocfs2/file.c | 8 ++++++++

1 file changed, 8 insertions(+)
fs/ocfs2/file.c +0 -0
index 0e5b451..d631279 100644
@@ -1302,6 +1302,14 @@ int ocfs2_getattr(struct vfsmount *mnt,
 	}
 
Quoted 13 lines

fs/ocfs2/file.c int ocfs2_getattr(struct vfsmount *mnt, · line 1312

Quoted 9 lines
fs/ocfs2/file.c +8 -0
 	generic_fillattr(inode, stat);
+	/*
+	 * If there is inline data in the inode, the inode will normally not
+	 * have data blocks allocated (it may have an external xattr block).
+	 * Report at least one sector for such files, so tools like tar, rsync,
+	 * others don't incorrectly think the file is completely sparse.
+	 */
+	if (unlikely(OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL))
+		stat->blocks += (stat->size + 511)>>9;

From filesystem side, looks reasonable that data block is 0 for inlined-data file. This is like a hack to filesystem to fix tools issue. Indeed tar-1.26-27 have been fixed to not think file with st_blocks == 0 empty. But I am not sure why ext4 merge that fix.

It?s not just tar and it?s not just ext4. Programmers not unreasonably assume that a file occupying zero blocks contains no data (where would you put it?)

ext4, btrfs and ntfs-3g all give inlined files a non-zero block size to avoid surprising programmers. There?s nothing in Posix that says what stat?s st_blocks so in this case it?s right for the file systems in question to stick to the principle of least surprise. In this case, it would be surprising if some small files suddenly started occupying no space while being non-empty. It?s not as though it would be consistent: some small files would occupy space and some would not. We want to present a consistent view of files to the user. It?s not as though we?re breaking du either: it already tells lies :)

Does that make sense now?

jch

Quoted 7 lines

Thanks,
Junxiao.

Quoted 3 lines
fs/ocfs2/file.c +0 -0
 	/* We set the blksize from the cluster size for performance */
 	stat->blksize = osb->s_clustersize;

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://oss.oracle.com/pipermail/ocfs2-devel/attachments/20151201/327439b3/attachment.html

#5 lore raw

PATCH 1/1 ocfs2: return non-zero st_blocks for inline data

Quoted 50 lines · John Haxby
Quoted 22 lines

On 1 Dec 2015, at 07:08, Junxiao Bi <[email protected] <mailto:[email protected]>> wrote:

Quoted 17 lines · John Haxby

Some versions of tar assume that files with st_blocks == 0 do not contain any data and will skip reading them entirely. See also commit 9206c561554c ("ext4: return non-zero st_blocks for inline data").

<mailto:[email protected]>>
---
fs/ocfs2/file.c | 8 ++++++++

1 file changed, 8 insertions(+)
fs/ocfs2/file.c +0 -0
index 0e5b451..d631279 100644
@@ -1302,6 +1302,14 @@ int ocfs2_getattr(struct vfsmount *mnt,
 }
 
Quoted 13 lines

fs/ocfs2/file.c int ocfs2_getattr(struct vfsmount *mnt, · line 1312

Quoted 9 lines
fs/ocfs2/file.c +8 -0
 generic_fillattr(inode, stat);
+/*
+ * If there is inline data in the inode, the inode will normally not
+ * have data blocks allocated (it may have an external xattr block).
+ * Report at least one sector for such files, so tools like tar, rsync,
+ * others don't incorrectly think the file is completely sparse.
+ */
+if (unlikely(OCFS2_I(inode)->ip_dyn_features & OCFS2_INLINE_DATA_FL))
+stat->blocks += (stat->size + 511)>>9;

From filesystem side, looks reasonable that data block is 0 for inlined-data file. This is like a hack to filesystem to fix tools issue. Indeed tar-1.26-27 have been fixed to not think file with st_blocks == 0 empty. But I am not sure why ext4 merge that fix.

It?s not just tar and it?s not just ext4. Programmers not unreasonably
assume that a file occupying zero blocks contains no data (where would you put it?)

ext4, btrfs and ntfs-3g all give inlined files a non-zero block size to
avoid surprising programmers. There?s nothing in Posix that says what
stat?s st_blocks so in this case it?s right for the file systems in question to stick to the principle of least surprise. In this case, it would be surprising if some small files suddenly started occupying no
space while being non-empty. It?s not as though it would be
consistent: some small files would occupy space and some would not. We want to present a consistent view of files to the user. It?s not as though we?re breaking du either: it already tells lies :)

Quoted 1 line

Does that make sense now?

OK. Thanks you for the explanation. We'd better not surprise programmers and keep align with other fs. So

Quoted 12 lines

jch

Quoted 7 lines

Thanks,
Junxiao.

Quoted 3 lines
fs/ocfs2/file.c +0 -0
 /* We set the blksize from the cluster size for performance */
 stat->blksize = osb->s_clustersize;