| Message ID | 1530846750-6686-8-git-send-email-shan.hai@oracle.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: > Return non-zero blocks for inline data even though the inode has > no external blocks, otherwise the "ls -ls" would show zero blocks > occupied by the file. Does this match the behaviour of existing file systems with inline data? What do btrfs, ext4 or gfs2 report? -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: > Return non-zero blocks for inline data even though the inode has > no external blocks, otherwise the "ls -ls" would show zero blocks > occupied by the file. > Is there any issue you ran into while leaving inodes with zero blocks allocated? Inodes should actually report the real amount of allocated blocks, not fake it. Inodes with inlined data should actually report 0 blocks, otherwise, many applications which actually relies on the amount of allocated blocks for each file will misbehave. > Signed-off-by: Shan Hai <shan.hai@oracle.com> > --- > fs/xfs/xfs_iops.c | 8 +++++++- > 1 file changed, 7 insertions(+), 1 deletion(-) > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > index 0fa29f39d658..63be1355a473 100644 > --- a/fs/xfs/xfs_iops.c > +++ b/fs/xfs/xfs_iops.c > @@ -500,8 +500,14 @@ xfs_vn_getattr( > stat->atime = inode->i_atime; > stat->mtime = inode->i_mtime; > stat->ctime = inode->i_ctime; > - stat->blocks = > + > + if (xfs_sb_version_hasinlinedata(&mp->m_sb) && > + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) == XFS_DINODE_FMT_LOCAL) { > + stat->blocks = BTOBB(XFS_ISIZE(ip)); > + } else { > + stat->blocks = > XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks); > + } > > if (ip->i_d.di_version == 3) { > if (request_mask & STATX_BTIME) { > -- > 2.11.0 > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018年07月11日 21:08, Carlos Maiolino wrote: > On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: >> Return non-zero blocks for inline data even though the inode has >> no external blocks, otherwise the "ls -ls" would show zero blocks >> occupied by the file. >> > Is there any issue you ran into while leaving inodes with zero blocks allocated? > Inodes should actually report the real amount of allocated blocks, not fake it. > Inodes with inlined data should actually report 0 blocks, otherwise, many > applications which actually relies on the amount of allocated blocks for each > file will misbehave. > Man ls(1) reads: -s, --size print the allocated size of each file, in blocks So the 'ls -ls' would report 0 blocks when the data is inlined, a file holds data but it consumes 0 blocks, how is it possible :), this patch fixes this problem. Thanks Shan Hai >> Signed-off-by: Shan Hai <shan.hai@oracle.com> >> --- >> fs/xfs/xfs_iops.c | 8 +++++++- >> 1 file changed, 7 insertions(+), 1 deletion(-) >> >> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c >> index 0fa29f39d658..63be1355a473 100644 >> --- a/fs/xfs/xfs_iops.c >> +++ b/fs/xfs/xfs_iops.c >> @@ -500,8 +500,14 @@ xfs_vn_getattr( >> stat->atime = inode->i_atime; >> stat->mtime = inode->i_mtime; >> stat->ctime = inode->i_ctime; >> - stat->blocks = >> + >> + if (xfs_sb_version_hasinlinedata(&mp->m_sb) && >> + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) == XFS_DINODE_FMT_LOCAL) { >> + stat->blocks = BTOBB(XFS_ISIZE(ip)); >> + } else { >> + stat->blocks = >> XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks); >> + } >> >> if (ip->i_d.di_version == 3) { >> if (request_mask & STATX_BTIME) { >> -- >> 2.11.0 >> >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018年07月12日 09:03, Shan Hai wrote: > > > On 2018年07月11日 21:08, Carlos Maiolino wrote: >> On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: >>> Return non-zero blocks for inline data even though the inode has >>> no external blocks, otherwise the "ls -ls" would show zero blocks >>> occupied by the file. >>> >> Is there any issue you ran into while leaving inodes with zero blocks >> allocated? >> Inodes should actually report the real amount of allocated blocks, >> not fake it. >> Inodes with inlined data should actually report 0 blocks, otherwise, >> many >> applications which actually relies on the amount of allocated blocks >> for each >> file will misbehave. >> > > Man ls(1) reads: > > -s, --size > print the allocated size of each file, in blocks > > So the 'ls -ls' would report 0 blocks when the data is inlined, a file > holds data > but it consumes 0 blocks, how is it possible :), this patch fixes this > problem. > This patch is inspired by the upstream commit 9206c561554c9 (ext4: return non-zero st_blocks for inline data), please refer it for details. Thanks Shan Hai > Thanks > Shan Hai >>> Signed-off-by: Shan Hai <shan.hai@oracle.com> >>> --- >>> fs/xfs/xfs_iops.c | 8 +++++++- >>> 1 file changed, 7 insertions(+), 1 deletion(-) >>> >>> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c >>> index 0fa29f39d658..63be1355a473 100644 >>> --- a/fs/xfs/xfs_iops.c >>> +++ b/fs/xfs/xfs_iops.c >>> @@ -500,8 +500,14 @@ xfs_vn_getattr( >>> stat->atime = inode->i_atime; >>> stat->mtime = inode->i_mtime; >>> stat->ctime = inode->i_ctime; >>> - stat->blocks = >>> + >>> + if (xfs_sb_version_hasinlinedata(&mp->m_sb) && >>> + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) == XFS_DINODE_FMT_LOCAL) { >>> + stat->blocks = BTOBB(XFS_ISIZE(ip)); >>> + } else { >>> + stat->blocks = >>> XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks); >>> + } >>> if (ip->i_d.di_version == 3) { >>> if (request_mask & STATX_BTIME) { >>> -- >>> 2.11.0 >>> >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 12, 2018 at 09:13:46AM +0800, Shan Hai wrote: > > > On 2018年07月12日 09:03, Shan Hai wrote: > > > > > > On 2018年07月11日 21:08, Carlos Maiolino wrote: > > > On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: > > > > Return non-zero blocks for inline data even though the inode has > > > > no external blocks, otherwise the "ls -ls" would show zero blocks > > > > occupied by the file. > > > > > > > Is there any issue you ran into while leaving inodes with zero > > > blocks allocated? > > > Inodes should actually report the real amount of allocated blocks, > > > not fake it. > > > Inodes with inlined data should actually report 0 blocks, otherwise, > > > many > > > applications which actually relies on the amount of allocated blocks > > > for each > > > file will misbehave. > > > > > > > Man ls(1) reads: > > > > -s, --size > > print the allocated size of each file, in blocks > > > > So the 'ls -ls' would report 0 blocks when the data is inlined, a file > > holds data > > but it consumes 0 blocks, how is it possible :), this patch fixes this > > problem. > > > > This patch is inspired by the > upstream commit 9206c561554c9 (ext4: return non-zero st_blocks for inline > data), > please refer it for details. The fact that we're following precedent set by ext4 is worth mentioning in the commit message. --D > Thanks > Shan Hai > > > Thanks > > Shan Hai > > > > Signed-off-by: Shan Hai <shan.hai@oracle.com> > > > > --- > > > > fs/xfs/xfs_iops.c | 8 +++++++- > > > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > > > > > diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c > > > > index 0fa29f39d658..63be1355a473 100644 > > > > --- a/fs/xfs/xfs_iops.c > > > > +++ b/fs/xfs/xfs_iops.c > > > > @@ -500,8 +500,14 @@ xfs_vn_getattr( > > > > stat->atime = inode->i_atime; > > > > stat->mtime = inode->i_mtime; > > > > stat->ctime = inode->i_ctime; > > > > - stat->blocks = > > > > + > > > > + if (xfs_sb_version_hasinlinedata(&mp->m_sb) && > > > > + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) == XFS_DINODE_FMT_LOCAL) { > > > > + stat->blocks = BTOBB(XFS_ISIZE(ip)); > > > > + } else { > > > > + stat->blocks = > > > > XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks); > > > > + } > > > > if (ip->i_d.di_version == 3) { > > > > if (request_mask & STATX_BTIME) { > > > > -- > > > > 2.11.0 > > > > > > > > -- > > > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > > > the body of a message to majordomo@vger.kernel.org > > > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > > > -- > > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > > the body of a message to majordomo@vger.kernel.org > > More majordomo info at http://vger.kernel.org/majordomo-info.html > > -- > To unsubscribe from this list: send the line "unsubscribe linux-xfs" in > the body of a message to majordomo@vger.kernel.org > More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On 2018年07月12日 09:31, Darrick J. Wong wrote: > On Thu, Jul 12, 2018 at 09:13:46AM +0800, Shan Hai wrote: >> >> On 2018年07月12日 09:03, Shan Hai wrote: >>> >>> On 2018年07月11日 21:08, Carlos Maiolino wrote: >>>> On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: >>>>> Return non-zero blocks for inline data even though the inode has >>>>> no external blocks, otherwise the "ls -ls" would show zero blocks >>>>> occupied by the file. >>>>> >>>> Is there any issue you ran into while leaving inodes with zero >>>> blocks allocated? >>>> Inodes should actually report the real amount of allocated blocks, >>>> not fake it. >>>> Inodes with inlined data should actually report 0 blocks, otherwise, >>>> many >>>> applications which actually relies on the amount of allocated blocks >>>> for each >>>> file will misbehave. >>>> >>> Man ls(1) reads: >>> >>> -s, --size >>> print the allocated size of each file, in blocks >>> >>> So the 'ls -ls' would report 0 blocks when the data is inlined, a file >>> holds data >>> but it consumes 0 blocks, how is it possible :), this patch fixes this >>> problem. >>> >> This patch is inspired by the >> upstream commit 9206c561554c9 (ext4: return non-zero st_blocks for inline >> data), >> please refer it for details. > The fact that we're following precedent set by ext4 is worth mentioning > in the commit message. OK, I will include it in the next version of the patch. Thanks Shan Hai > --D > >> Thanks >> Shan Hai >> >>> Thanks >>> Shan Hai >>>>> Signed-off-by: Shan Hai <shan.hai@oracle.com> >>>>> --- >>>>> fs/xfs/xfs_iops.c | 8 +++++++- >>>>> 1 file changed, 7 insertions(+), 1 deletion(-) >>>>> >>>>> diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c >>>>> index 0fa29f39d658..63be1355a473 100644 >>>>> --- a/fs/xfs/xfs_iops.c >>>>> +++ b/fs/xfs/xfs_iops.c >>>>> @@ -500,8 +500,14 @@ xfs_vn_getattr( >>>>> stat->atime = inode->i_atime; >>>>> stat->mtime = inode->i_mtime; >>>>> stat->ctime = inode->i_ctime; >>>>> - stat->blocks = >>>>> + >>>>> + if (xfs_sb_version_hasinlinedata(&mp->m_sb) && >>>>> + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) == XFS_DINODE_FMT_LOCAL) { >>>>> + stat->blocks = BTOBB(XFS_ISIZE(ip)); >>>>> + } else { >>>>> + stat->blocks = >>>>> XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks); >>>>> + } >>>>> if (ip->i_d.di_version == 3) { >>>>> if (request_mask & STATX_BTIME) { >>>>> -- >>>>> 2.11.0 >>>>> >>>>> -- >>>>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >>>>> the body of a message to majordomo@vger.kernel.org >>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html >>> -- >>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >>> the body of a message to majordomo@vger.kernel.org >>> More majordomo info at http://vger.kernel.org/majordomo-info.html >> -- >> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in >> the body of a message to majordomo@vger.kernel.org >> More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Thu, Jul 12, 2018 at 09:46:20AM +0800, Shan Hai wrote: > > > On 2018年07月12日 09:31, Darrick J. Wong wrote: > > On Thu, Jul 12, 2018 at 09:13:46AM +0800, Shan Hai wrote: > > > > > > On 2018年07月12日 09:03, Shan Hai wrote: > > > > > > > > On 2018年07月11日 21:08, Carlos Maiolino wrote: > > > > > On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: > > > > > > Return non-zero blocks for inline data even though the inode has > > > > > > no external blocks, otherwise the "ls -ls" would show zero blocks > > > > > > occupied by the file. > > > > > > > > > > > Is there any issue you ran into while leaving inodes with zero > > > > > blocks allocated? > > > > > Inodes should actually report the real amount of allocated blocks, > > > > > not fake it. > > > > > Inodes with inlined data should actually report 0 blocks, otherwise, > > > > > many > > > > > applications which actually relies on the amount of allocated blocks > > > > > for each > > > > > file will misbehave. > > > > > > > > > Man ls(1) reads: > > > > > > > > -s, --size > > > > print the allocated size of each file, in blocks > > > > > > > > So the 'ls -ls' would report 0 blocks when the data is inlined, a file > > > > holds data > > > > but it consumes 0 blocks, how is it possible :), It is possible because the file doesn't consume data blocks at all. > > > This patch is inspired by the > > > upstream commit 9206c561554c9 (ext4: return non-zero st_blocks for inline > > > data), > > > please refer it for details. > > The fact that we're following precedent set by ext4 is worth mentioning > > in the commit message. The fact another filesystem use this trick, doesn't necessarily means it's correct. Ext4 added it to workaround a issue with tar, which actually skip zero blocks files. I honestly think it is wrong, we are working around a user space problem, which is wrongly assuming a 0-block file is empty. A quick search led me to this thread from tar project: http://www.mail-archive.com/bug-tar@gnu.org/msg04209.html which well, from that time, they were already aware that always assuming a zero-block file is empty, was not safe. Reporting a used block in a file that is storing data inlined in the inode is prone for space usage accounting error IMO. Suppose you have a 4K block-size filesystem, you create 100k files into it with inlined data. So now you have 100k files reporting to be using a single FSB, wich converts into a bit less than 400MB. How will you make sure the accounting is correct? You can't really mark any blocks used in the filesystem, so, tools like `df` (or any other tool based on statfs() ), will report these 400MB as free. At the same time, tools like `ls`, `du`, etc which relies on per-file stat, will report these 400MB used. I do see advantages on reporting a single block use, but IMO, it will cause more confusion than good. Anyway, as I said, my opinion only, I do really think reporting a single block used by inlined files is wrong, and if we are going to do that, we should at least properly document this is being done to workaround user space issues, while, in the meantime, it might create others. Cheers.
On 2018年07月12日 17:08, Carlos Maiolino wrote: > On Thu, Jul 12, 2018 at 09:46:20AM +0800, Shan Hai wrote: >> >> On 2018年07月12日 09:31, Darrick J. Wong wrote: >>> On Thu, Jul 12, 2018 at 09:13:46AM +0800, Shan Hai wrote: >>>> On 2018年07月12日 09:03, Shan Hai wrote: >>>>> On 2018年07月11日 21:08, Carlos Maiolino wrote: >>>>>> On Fri, Jul 06, 2018 at 11:12:28AM +0800, Shan Hai wrote: >>>>>>> Return non-zero blocks for inline data even though the inode has >>>>>>> no external blocks, otherwise the "ls -ls" would show zero blocks >>>>>>> occupied by the file. >>>>>>> >>>>>> Is there any issue you ran into while leaving inodes with zero >>>>>> blocks allocated? >>>>>> Inodes should actually report the real amount of allocated blocks, >>>>>> not fake it. >>>>>> Inodes with inlined data should actually report 0 blocks, otherwise, >>>>>> many >>>>>> applications which actually relies on the amount of allocated blocks >>>>>> for each >>>>>> file will misbehave. >>>>>> >>>>> Man ls(1) reads: >>>>> >>>>> -s, --size >>>>> print the allocated size of each file, in blocks >>>>> >>>>> So the 'ls -ls' would report 0 blocks when the data is inlined, a file >>>>> holds data >>>>> but it consumes 0 blocks, how is it possible :), > It is possible because the file doesn't consume data blocks at all. > >>>> This patch is inspired by the >>>> upstream commit 9206c561554c9 (ext4: return non-zero st_blocks for inline >>>> data), >>>> please refer it for details. >>> The fact that we're following precedent set by ext4 is worth mentioning >>> in the commit message. > The fact another filesystem use this trick, doesn't necessarily means it's > correct. Ext4 added it to workaround a issue with tar, which actually skip zero > blocks files. I honestly think it is wrong, we are working around a user space > problem, which is wrongly assuming a 0-block file is empty. The assumption which deemed 0 block files as empty holds true until the kernel broke the rules and introduced inline data feature, so the user space should not be blamed in my opinion. > A quick search led me to this thread from tar project: > > http://www.mail-archive.com/bug-tar@gnu.org/msg04209.html > > which well, from that time, they were already aware that always assuming a > zero-block file is empty, was not safe. > > > > Reporting a used block in a file that is storing data inlined in the inode is > prone for space usage accounting error IMO. > > Suppose you have a 4K block-size filesystem, you create 100k files into it with > inlined data. > > So now you have 100k files reporting to be using a single FSB, wich converts > into a bit less than 400MB. > > How will you make sure the accounting is correct? You can't really mark any > blocks used in the filesystem, so, tools like `df` (or any other tool based on > statfs() ), will report these 400MB as free. > > At the same time, tools like `ls`, `du`, etc which relies on per-file stat, will > report these 400MB used. > > > I do see advantages on reporting a single block use, but IMO, it will cause more > confusion than good. > > Anyway, as I said, my opinion only, I do really think reporting a single block > used by inlined files is wrong, and if we are going to do that, we should at > least properly document this is being done to workaround user space issues, > while, in the meantime, it might create others. Faking block usage count is not quite correct one but the sad fact is that there are applications out there which are designed and implemented on the assumption that non-zero length files consume at least a block, so I don't think breaking the user space suddenly by reporting 0 blocks for inline data is not a correct solution either. I will put a big fat note in inline comment to emphasize this issue. Any other better ideas? Thanks Shan Hai > Cheers. > -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
> > > > > > > Inodes with inlined data should actually report 0 blocks, otherwise, > > > > > > > many > > > > > > > applications which actually relies on the amount of allocated blocks > > > > > > > for each > > > > > > > file will misbehave. > > > > > > > > > > > > > Man ls(1) reads: > > > > > > > > > > > > -s, --size > > > > > > print the allocated size of each file, in blocks > > > > > > > > > > > > So the 'ls -ls' would report 0 blocks when the data is inlined, a file > > > > > > holds data > > > > > > but it consumes 0 blocks, how is it possible :), > > It is possible because the file doesn't consume data blocks at all. > > > > > > > This patch is inspired by the > > > > > upstream commit 9206c561554c9 (ext4: return non-zero st_blocks for inline > > > > > data), > > > > > please refer it for details. > > > > The fact that we're following precedent set by ext4 is worth mentioning > > > > in the commit message. > > The fact another filesystem use this trick, doesn't necessarily means it's > > correct. Ext4 added it to workaround a issue with tar, which actually skip zero > > blocks files. I honestly think it is wrong, we are working around a user space > > problem, which is wrongly assuming a 0-block file is empty. > > The assumption which deemed 0 block files as empty holds true until the > kernel broke the > rules and introduced inline data feature, so the user space should not be > blamed in my opinion. Did we? I couldn't find any interface, design, or whatever saying that userspace can safely assume a zero block file don't have data there and can be ignored, in fact, the link I posted previously already shows how tar people were not sure if they should assume a zero block file could be ignored. So, in fact, we didn't break anything, because as far as I know, nobody and no standard said an existing zero block file could be safely ignored, assuming it contains no data at all. Such thing might exist, but I'm not aware of. > > > A quick search led me to this thread from tar project: > > > > http://www.mail-archive.com/bug-tar@gnu.org/msg04209.html > > > > > > Anyway, as I said, my opinion only, I do really think reporting a single block > > used by inlined files is wrong, and if we are going to do that, we should at > > least properly document this is being done to workaround user space issues, > > while, in the meantime, it might create others. > > Faking block usage count is not quite correct one but the sad fact is that > there are applications > out there which are designed and implemented on the assumption that non-zero > length files > consume at least a block, so I don't think breaking the user space suddenly > by reporting 0 blocks > for inline data is not a correct solution either. > I understand, and yeah, we fell into some kind of chicken-egg problem, I still do think though there are not that much applications out there assuming zero block files are empty, and working around bad coded application which make assumptions they are not supposed to make, is as bad as breaking applications for not following standards, which will end up punishing applications which do not make assumptions they should. The biggest drawback I can see by reporting 0 blocks allocated for inline files, might be while estimating space needed for a copy, or a backup for example, where, if the target filesystem does not support inlined data, 0 blocks files will occupy at least a block on the target filesystem, but the same holds true when source and target filesystems have different block sizes. Anyway, that's all the arguments I have against faking block usage for inlined files, but the final decision is not mine :) Cheers
On Fri, Jul 13, 2018 at 02:39:55PM +0200, Carlos Maiolino wrote: > Did we? I couldn't find any interface, design, or whatever saying that userspace > can safely assume a zero block file don't have data there and can be ignored, > in fact, the link I posted previously already shows how tar people were not sure > if they should assume a zero block file could be ignored. So, in fact, we didn't > break anything, because as far as I know, nobody and no standard said an > existing zero block file could be safely ignored, assuming it contains no data > at all. Such thing might exist, but I'm not aware of. You are right that there is no interface. But breaking existing userspace simply isn't an option either, especially when it could lead to data loss and corruption. -- To unsubscribe from this list: send the line "unsubscribe linux-xfs" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Jul 17, 2018 at 06:57:41AM -0700, Christoph Hellwig wrote: > On Fri, Jul 13, 2018 at 02:39:55PM +0200, Carlos Maiolino wrote: > > Did we? I couldn't find any interface, design, or whatever saying that userspace > > can safely assume a zero block file don't have data there and can be ignored, > > in fact, the link I posted previously already shows how tar people were not sure > > if they should assume a zero block file could be ignored. So, in fact, we didn't > > break anything, because as far as I know, nobody and no standard said an > > existing zero block file could be safely ignored, assuming it contains no data > > at all. Such thing might exist, but I'm not aware of. > > You are right that there is no interface. But breaking existing > userspace simply isn't an option either, especially when it could > lead to data loss and corruption. Fair enough.
diff --git a/fs/xfs/xfs_iops.c b/fs/xfs/xfs_iops.c index 0fa29f39d658..63be1355a473 100644 --- a/fs/xfs/xfs_iops.c +++ b/fs/xfs/xfs_iops.c @@ -500,8 +500,14 @@ xfs_vn_getattr( stat->atime = inode->i_atime; stat->mtime = inode->i_mtime; stat->ctime = inode->i_ctime; - stat->blocks = + + if (xfs_sb_version_hasinlinedata(&mp->m_sb) && + XFS_IFORK_FORMAT(ip, XFS_DATA_FORK) == XFS_DINODE_FMT_LOCAL) { + stat->blocks = BTOBB(XFS_ISIZE(ip)); + } else { + stat->blocks = XFS_FSB_TO_BB(mp, ip->i_d.di_nblocks + ip->i_delayed_blks); + } if (ip->i_d.di_version == 3) { if (request_mask & STATX_BTIME) {
Return non-zero blocks for inline data even though the inode has no external blocks, otherwise the "ls -ls" would show zero blocks occupied by the file. Signed-off-by: Shan Hai <shan.hai@oracle.com> --- fs/xfs/xfs_iops.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)