| Message ID | aaf41f35-b702-b391-1cff-de4688b3bb65@paragon-software.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | fs/ntfs3: Various fixes for xattr and files | expand |
On Fri, Oct 22, 2021 at 06:54:31PM +0300, Konstantin Komarov wrote: > If size of file was reduced, we still kept allocated blocks. > This commit makes ntfs3 work as other fs like btrfs. > https://bugzilla.kernel.org/show_bug.cgi?id=214719 Link: https://bugzilla.kernel.org/show_bug.cgi?id=214719 > Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation") > > Reported-by: Ganapathi Kamath Add <hgkamath@hotmail.com> I also added to loop here. Ganapathi can you test if this patch fix your problem? > Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> > --- > fs/ntfs3/file.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c > index 43b1451bff53..3ac0482c6880 100644 > --- a/fs/ntfs3/file.c > +++ b/fs/ntfs3/file.c > @@ -494,7 +494,7 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size) > > down_write(&ni->file.run_lock); > err = attr_set_size(ni, ATTR_DATA, NULL, 0, &ni->file.run, new_size, > - &new_valid, true, NULL); > + &new_valid, ni->mi.sbi->options->prealloc, NULL); > up_write(&ni->file.run_lock); > > if (new_valid < ni->i_valid) > -- > 2.33.0 > >
On Sun, Oct 24, 2021 at 08:35:57AM +0000, Ganapathi Kamath wrote: > Hellom > > While compiling, first time around, I got the below. Yeah. Patch was meant for ntfs3/master... > fs/ntfs3/file.c: In function 'ntfs_truncate': > fs/ntfs3/file.c:498:60: error: invalid type argument of '->' (have 'struct ntfs_mount_options') > 498 | &new_valid, ni->mi.sbi->options->prealloc, NULL); > | ^~ > make[2]: *** [scripts/Makefile.build:277: fs/ntfs3/file.o] Error 1 > make[1]: *** [scripts/Makefile.build:540: fs/ntfs3] Error 2 > > So, in the file > fs/ntfs3/file.c > I changed > ni->mi.sbi->options->prealloc > to > ni->mi.sbi->options.prealloc but with your change it also applies to rc3. Your change was correct and it was totally ok to test top of rc3. > I don't really follow/understand the code, to understand what exactly > the logic is, except that you are trying to set boolean > 'keep_prealloc' call-argument for attr_set_size() by using the > ntfs_mount_option bit-field 'prealloc' which is to "Preallocate space > when file is growing", prototyped in the file fs/ntfs3/ntfs_fs.h > > * Built new 5.15.0-0.rc3.20211001git4de593fb965f.30.fc35.x86_64 > kernel. (4 hrs on my machine) I do not know if you used -j flag when making. Just if you did not know about it use: make -j8 This will example use 8 threads for compiling. You choose number based on how many threads you have in your processor. > * I was able to include patch into rpmbuild of kernel src patch, with > aforementioned correction * first reconfirmed/verified bug on old > kernel > * installed newly built kernel > * attempt reproduction no success meaning bug not present on new > kernel, patch/fix makes file size on overwrite to be as expected. > > note, I am not an expert, and as a user, I don't know 100% what > correct behavior should be, only what seems reasonable expected > behavior, But you are experts, so please excuse me for reiterating > what you know. NTFS is a filesystem that was designed by microsoft for > windows, and the way its fs-driver must update is so that on-disk > structures is suitable for windows in-kernel structures. A kernel > driver for linux, only adapts on disk-ntfs structures to something > suitable for linux in-kernel structures, but must update the on disk > structures the way Windows expects/designed it to. Ntfs file structure allows actually many things which even Windows does not understand. This is new driver and it will be evolving that user can decide what he wants. We will also have to define good defaults so that user get good experience. > So if you defended old behavior, I wouldn't know. So its your call, > to decide if it is a bug, and whether your patch fixes. On my side, > my machine is one I work on. patch seems to fix claimed bug. So I hope > there is no side effect, nothing corrupts or becomes unstable. This was bug. Thanks for reporting and testing it. We really appriciate it. This is new driver and there will be bugs so early users who report bugs and are even willing to test patches are gold mine for us. We will also add tags to patch: Reported-by: Ganapathi Kamath <hgkamath@hotmail.com> Tested-by: Ganapathi Kamath <hgkamath@hotmail.com> if it ok to you. This way if you report new bugs to kernel people will know you are good reporter as you have also tested what you have reported. If you wanna know more about these tags see [1]. [1]: https://www.kernel.org/doc/html/latest/process/submitting-patches.html#using-reported-by-tested-by-reviewed-by-suggested-by-and-fixes Argillander > That was fast fix. congrats. > > Log: > [root@sirius gana]# > [root@sirius gana]# mount -t ntfs3 /dev/sda17 /mnt/a17/ > [root@sirius gana]# > [root@sirius gana]# rm -f /mnt/a17/test1.bin /mnt/a17/test2.bin > [root@sirius gana]# dd if=/dev/zero of=/mnt/a17/test2.bin bs=1M count=3000 > 3000+0 records in > 3000+0 records out > 3145728000 bytes (3.1 GB, 2.9 GiB) copied, 5.40015 s, 583 MB/s > [root@sirius gana]# dd if=/dev/zero of=/mnt/a17/test1.bin bs=1M count=6000 > 6000+0 records in > 6000+0 records out > 6291456000 bytes (6.3 GB, 5.9 GiB) copied, 16.1809 s, 389 MB/s > [root@sirius gana]# ls -ls /mnt/a17/test1.bin /mnt/a17/test2.bin > 6144000 -rw-r--r--. 1 root root 6291456000 Oct 24 13:42 /mnt/a17/test1.bin > 3072000 -rw-r--r--. 1 root root 3145728000 Oct 24 13:41 /mnt/a17/test2.bin > [root@sirius gana]# cp /mnt/a17/test2.bin /mnt/a17/test1.bin > cp: overwrite '/mnt/a17/test1.bin'? y > [root@sirius gana]# ls -ls /mnt/a17/test1.bin /mnt/a17/test2.bin > 3072000 -rw-r--r--. 1 root root 3145728000 Oct 24 13:42 /mnt/a17/test1.bin > 3072000 -rw-r--r--. 1 root root 3145728000 Oct 24 13:41 /mnt/a17/test2.bin > [root@sirius gana]# stat /mnt/a17/test1.bin > File: /mnt/a17/test1.bin > Size: 3145728000 Blocks: 6144000 IO Block: 4096 regular file > Device: 10301h/66305d Inode: 44 Links: 1 > Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) > Context: system_u:object_r:unlabeled_t:s0 > Access: 2021-10-24 13:41:59.265503300 +0530 > Modify: 2021-10-24 13:42:44.738904000 +0530 > Change: 2021-10-24 13:42:44.738904000 +0530 > Birth: 2021-10-24 13:41:59.265503300 +0530 > [root@sirius gana]# stat /mnt/a17/test2.bin > File: /mnt/a17/test2.bin > Size: 3145728000 Blocks: 6144000 IO Block: 4096 regular file > Device: 10301h/66305d Inode: 43 Links: 1 > Access: (0644/-rw-r--r--) Uid: ( 0/ root) Gid: ( 0/ root) > Context: system_u:object_r:unlabeled_t:s0 > Access: 2021-10-24 13:42:40.610776900 +0530 > Modify: 2021-10-24 13:41:52.684315600 +0530 > Change: 2021-10-24 13:41:52.684315600 +0530 > Birth: 2021-10-24 13:41:47.284266100 +0530
diff --git a/fs/ntfs3/file.c b/fs/ntfs3/file.c index 43b1451bff53..3ac0482c6880 100644 --- a/fs/ntfs3/file.c +++ b/fs/ntfs3/file.c @@ -494,7 +494,7 @@ static int ntfs_truncate(struct inode *inode, loff_t new_size) down_write(&ni->file.run_lock); err = attr_set_size(ni, ATTR_DATA, NULL, 0, &ni->file.run, new_size, - &new_valid, true, NULL); + &new_valid, ni->mi.sbi->options->prealloc, NULL); up_write(&ni->file.run_lock); if (new_valid < ni->i_valid)
If size of file was reduced, we still kept allocated blocks. This commit makes ntfs3 work as other fs like btrfs. https://bugzilla.kernel.org/show_bug.cgi?id=214719 Fixes: 4342306f0f0d ("fs/ntfs3: Add file operations and implementation") Reported-by: Ganapathi Kamath Signed-off-by: Konstantin Komarov <almaz.alexandrovich@paragon-software.com> --- fs/ntfs3/file.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)