| Message ID | 20240325-faucht-kiesel-82c6c35504b3@brauner (mailing list archive) |
|---|---|
| State | New |
| Headers | show |
| Series | [1/2] ntfs3: serve as alias for the legacy ntfs driver | expand |
On Mon, Mar 25, 2024 at 09:34:38AM +0100, Christian Brauner wrote: > This causes visible changes for users that rely on ntfs3 to serve as an > alternative for the legacy ntfs driver. Print statements such as this > should probably be made conditional on a debug config option or similar. > > Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> > Cc: Johan Hovold <johan@kernel.org> > Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com > Signed-off-by: Christian Brauner <brauner@kernel.org> Tested-by: Johan Hovold <johan+linaro@kernel.org> I also see a ntfs3: Max link count 4000 message on mount which wasn't there with NTFS legacy. Is that benign and should be suppressed too perhaps? Johan
On Mon, Mar 25, 2024 at 11:12:00AM +0100, Johan Hovold wrote: > On Mon, Mar 25, 2024 at 09:34:38AM +0100, Christian Brauner wrote: > > This causes visible changes for users that rely on ntfs3 to serve as an > > alternative for the legacy ntfs driver. Print statements such as this > > should probably be made conditional on a debug config option or similar. > > > > Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> > > Cc: Johan Hovold <johan@kernel.org> > > Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com > > Signed-off-by: Christian Brauner <brauner@kernel.org> > > Tested-by: Johan Hovold <johan+linaro@kernel.org> > > I also see a > > ntfs3: Max link count 4000 > > message on mount which wasn't there with NTFS legacy. Is that benign > and should be suppressed too perhaps? We need a reply from the ntfs3 maintainers here.
On 25.03.24 13:05, Christian Brauner wrote: > On Mon, Mar 25, 2024 at 11:12:00AM +0100, Johan Hovold wrote: >> On Mon, Mar 25, 2024 at 09:34:38AM +0100, Christian Brauner wrote: >>> This causes visible changes for users that rely on ntfs3 to serve as an >>> alternative for the legacy ntfs driver. Print statements such as this >>> should probably be made conditional on a debug config option or similar. >>> >>> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> >>> Cc: Johan Hovold <johan@kernel.org> >>> Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com >>> Signed-off-by: Christian Brauner <brauner@kernel.org> >> >> Tested-by: Johan Hovold <johan+linaro@kernel.org> >> >> I also see a >> >> ntfs3: Max link count 4000 >> >> message on mount which wasn't there with NTFS legacy. Is that benign >> and should be suppressed too perhaps? > > We need a reply from the ntfs3 maintainers here. Those are known for taken their time to respond -- like we see here, as nothing happened for 10 days now. Which makes we wonder: should we at least bring the first patch from this series onto the track towards mainline? FWIW, quick side note: just noticed there was another report about the "Correct links count -> 1." messages: https://lore.kernel.org/all/6215a88a-7d78-4abb-911f-8a3e7033da3e@gmx.com/ Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) -- Everything you wanna know about Linux kernel regression tracking: https://linux-regtracking.leemhuis.info/about/#tldr If I did something stupid, please tell me, as explained on that page. #regzbot duplicate: https://lore.kernel.org/all/6215a88a-7d78-4abb-911f-8a3e7033da3e@gmx.com/ #regzbot poke
On 04.04.2024 11:06, Linux regression tracking (Thorsten Leemhuis) wrote: > On 25.03.24 13:05, Christian Brauner wrote: >> On Mon, Mar 25, 2024 at 11:12:00AM +0100, Johan Hovold wrote: >>> On Mon, Mar 25, 2024 at 09:34:38AM +0100, Christian Brauner wrote: >>>> This causes visible changes for users that rely on ntfs3 to serve as an >>>> alternative for the legacy ntfs driver. Print statements such as this >>>> should probably be made conditional on a debug config option or similar. >>>> >>>> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> >>>> Cc: Johan Hovold <johan@kernel.org> >>>> Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com >>>> Signed-off-by: Christian Brauner <brauner@kernel.org> >>> Tested-by: Johan Hovold <johan+linaro@kernel.org> >>> >>> I also see a >>> >>> ntfs3: Max link count 4000 >>> >>> message on mount which wasn't there with NTFS legacy. Is that benign >>> and should be suppressed too perhaps? >> We need a reply from the ntfs3 maintainers here. > Those are known for taken their time to respond -- like we see here, as > nothing happened for 10 days now. Which makes we wonder: should we at > least bring the first patch from this series onto the track towards > mainline? > > FWIW, quick side note: just noticed there was another report about the > "Correct links count -> 1." messages: > https://lore.kernel.org/all/6215a88a-7d78-4abb-911f-8a3e7033da3e@gmx.com/ > > Ciao, Thorsten (wearing his 'the Linux kernel's regression tracker' hat) > -- > Everything you wanna know about Linux kernel regression tracking: > https://linux-regtracking.leemhuis.info/about/#tldr > If I did something stupid, please tell me, as explained on that page. > > #regzbot duplicate: > https://lore.kernel.org/all/6215a88a-7d78-4abb-911f-8a3e7033da3e@gmx.com/ > #regzbot poke > Hello Christian, Johan, all, There is no problem in suppressing the output of any messages during mounting, like: diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c index cef5467fd928..4643b06b1550 100644 --- a/fs/ntfs3/super.c +++ b/fs/ntfs3/super.c @@ -1804,8 +1804,6 @@ static int __init init_ntfs_fs(void) { int err; - pr_info("ntfs3: Max link count %u\n", NTFS_LINK_MAX); - if (IS_ENABLED(CONFIG_NTFS3_FS_POSIX_ACL)) pr_info("ntfs3: Enabled Linux POSIX ACLs support\n"); if (IS_ENABLED(CONFIG_NTFS3_64BIT_CLUSTER)) Messages like this: diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index eb7a8c9fba01..8cc94a6a97ed 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -424,7 +424,6 @@ static struct inode *ntfs_read_mft(struct inode *inode, if (names != le16_to_cpu(rec->hard_links)) { /* Correct minor error on the fly. Do not mark inode as dirty. */ - ntfs_inode_warn(inode, "Correct links count -> %u.", names); rec->hard_links = cpu_to_le16(names); ni->mi.dirty = true; } can also be suppressed for the sake of seamless transition from a remote NTFS driver. However, I believe that file system corrections should be reported to the user.
On Thu, Apr 11, 2024 at 02:03:52PM +0300, Konstantin Komarov wrote: > On 04.04.2024 11:06, Linux regression tracking (Thorsten Leemhuis) wrote: > > On 25.03.24 13:05, Christian Brauner wrote: > >> On Mon, Mar 25, 2024 at 11:12:00AM +0100, Johan Hovold wrote: > >>> On Mon, Mar 25, 2024 at 09:34:38AM +0100, Christian Brauner wrote: > >>>> This causes visible changes for users that rely on ntfs3 to serve as an > >>>> alternative for the legacy ntfs driver. Print statements such as this > >>>> should probably be made conditional on a debug config option or similar. > >>>> > >>>> Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> > >>>> Cc: Johan Hovold <johan@kernel.org> > >>>> Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com > >>>> Signed-off-by: Christian Brauner <brauner@kernel.org> > >>> Tested-by: Johan Hovold <johan+linaro@kernel.org> > >>> > >>> I also see a > >>> > >>> ntfs3: Max link count 4000 > >>> > >>> message on mount which wasn't there with NTFS legacy. Is that benign > >>> and should be suppressed too perhaps? > >> We need a reply from the ntfs3 maintainers here. > There is no problem in suppressing the output of any messages during > mounting, like: > Messages like this: > > diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c > index eb7a8c9fba01..8cc94a6a97ed 100644 > --- a/fs/ntfs3/inode.c > +++ b/fs/ntfs3/inode.c > @@ -424,7 +424,6 @@ static struct inode *ntfs_read_mft(struct inode *inode, > if (names != le16_to_cpu(rec->hard_links)) { > /* Correct minor error on the fly. Do not mark inode as dirty. */ > - ntfs_inode_warn(inode, "Correct links count -> %u.", names); > rec->hard_links = cpu_to_le16(names); > ni->mi.dirty = true; > } > > can also be suppressed for the sake of seamless transition from a remote > NTFS driver. > However, I believe that file system corrections should be reported to > the user. A colleague of mine also tracked down a failed boot to the removal of the ntfs driver and reported seeing similar warnings with the ntfs3 driver. We're both accessing an NTFS partition on a Windows on Arm device, but it makes you wonder whether these warnings (corrections) are correct or indicative of a problem in the driver? Johan
On Mon, Apr 15, 2024 at 11:54:19AM +0200, Johan Hovold wrote: > On Thu, Apr 11, 2024 at 02:03:52PM +0300, Konstantin Komarov wrote: > > Messages like this: > > > > diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c > > index eb7a8c9fba01..8cc94a6a97ed 100644 > > --- a/fs/ntfs3/inode.c > > +++ b/fs/ntfs3/inode.c > > @@ -424,7 +424,6 @@ static struct inode *ntfs_read_mft(struct inode *inode, > > if (names != le16_to_cpu(rec->hard_links)) { > > /* Correct minor error on the fly. Do not mark inode as dirty. */ > > - ntfs_inode_warn(inode, "Correct links count -> %u.", names); > > rec->hard_links = cpu_to_le16(names); > > ni->mi.dirty = true; > > } > > > > can also be suppressed for the sake of seamless transition from a remote > > NTFS driver. > > However, I believe that file system corrections should be reported to > > the user. > > A colleague of mine also tracked down a failed boot to the removal of > the ntfs driver and reported seeing similar warnings with the ntfs3 > driver. > > We're both accessing an NTFS partition on a Windows on Arm device, but > it makes you wonder whether these warnings (corrections) are correct or > indicative of a problem in the driver? This is what I see with a recursive ls on that partition (I've added rec->hard_links in parentheses): [ 38.287555] ntfs3: nvme0n1p3: ino=2e1e7, Correct links count -> 1 (2). [ 38.288593] ntfs3: nvme0n1p3: ino=75ff, Correct links count -> 1 (2). [ 38.289887] ntfs3: nvme0n1p3: ino=1b4e1, Correct links count -> 1 (2). [ 38.290144] ntfs3: nvme0n1p3: ino=78c6, Correct links count -> 1 (2). [ 38.291313] ntfs3: nvme0n1p3: ino=8781b, Correct links count -> 1 (2). [ 38.291823] ntfs3: nvme0n1p3: ino=8781e, Correct links count -> 1 (2). [ 38.292289] ntfs3: nvme0n1p3: ino=87820, Correct links count -> 1 (2). [ 38.292978] ntfs3: nvme0n1p3: ino=87823, Correct links count -> 1 (2). [ 38.300531] ntfs3: nvme0n1p3: ino=a324, Correct links count -> 1 (2). [ 38.312235] ntfs3: nvme0n1p3: ino=882c3, Correct links count -> 1 (2). [ 43.286846] ntfs3: 5479 callbacks suppressed [ 43.286856] ntfs3: nvme0n1p3: ino=14aa, Correct links count -> 1 (2). [ 43.286998] ntfs3: nvme0n1p3: ino=14ac, Correct links count -> 1 (2). [ 43.287194] ntfs3: nvme0n1p3: ino=12cc2, Correct links count -> 1 (2). [ 43.287386] ntfs3: nvme0n1p3: ino=12ccd, Correct links count -> 1 (2). [ 43.287576] ntfs3: nvme0n1p3: ino=12d15, Correct links count -> 1 (2). [ 43.287667] ntfs3: nvme0n1p3: ino=12d19, Correct links count -> 1 (2). [ 43.287877] ntfs3: nvme0n1p3: ino=12d3a, Correct links count -> 1 (2). [ 43.288051] ntfs3: nvme0n1p3: ino=12d88, Correct links count -> 1 (2). [ 43.288265] ntfs3: nvme0n1p3: ino=874, Correct links count -> 1 (2). [ 43.288326] ntfs3: nvme0n1p3: ino=875, Correct links count -> 1 (2). [ 48.288211] ntfs3: 7735 callbacks suppressed [ 48.288220] ntfs3: nvme0n1p3: ino=33391, Correct links count -> 1 (2). [ 48.289115] ntfs3: nvme0n1p3: ino=347a4, Correct links count -> 1 (2). [ 48.290214] ntfs3: nvme0n1p3: ino=3553f, Correct links count -> 1 (2). [ 48.291193] ntfs3: nvme0n1p3: ino=35793, Correct links count -> 1 (2). [ 48.292818] ntfs3: nvme0n1p3: ino=358a0, Correct links count -> 1 (2). [ 48.293529] ntfs3: nvme0n1p3: ino=38f54, Correct links count -> 1 (2). [ 48.293901] ntfs3: nvme0n1p3: ino=391f6, Correct links count -> 1 (2). [ 48.294303] ntfs3: nvme0n1p3: ino=39324, Correct links count -> 1 (2). [ 48.294729] ntfs3: nvme0n1p3: ino=394a0, Correct links count -> 1 (2). [ 48.295063] ntfs3: nvme0n1p3: ino=3956a, Correct links count -> 1 (2). [ 53.289392] ntfs3: 11442 callbacks suppressed [ 53.289401] ntfs3: nvme0n1p3: ino=6293e, Correct links count -> 1 (2). [ 53.289972] ntfs3: nvme0n1p3: ino=61df1, Correct links count -> 1 (2). [ 53.290241] ntfs3: nvme0n1p3: ino=61df3, Correct links count -> 1 (2). [ 53.290578] ntfs3: nvme0n1p3: ino=61f3b, Correct links count -> 1 (2). [ 53.290881] ntfs3: nvme0n1p3: ino=62025, Correct links count -> 1 (2). [ 53.291045] ntfs3: nvme0n1p3: ino=629af, Correct links count -> 1 (2). [ 53.291181] ntfs3: nvme0n1p3: ino=61e3c, Correct links count -> 1 (2). [ 53.291463] ntfs3: nvme0n1p3: ino=61e22, Correct links count -> 1 (2). [ 53.291743] ntfs3: nvme0n1p3: ino=62882, Correct links count -> 1 (2). [ 53.292099] ntfs3: nvme0n1p3: ino=61b3d, Correct links count -> 1 (2). [ 58.291790] ntfs3: 5373 callbacks suppressed [ 58.291799] ntfs3: nvme0n1p3: ino=6d5a5, Correct links count -> 1 (2). [ 58.292106] ntfs3: nvme0n1p3: ino=6d7f6, Correct links count -> 1 (2). [ 58.292372] ntfs3: nvme0n1p3: ino=6db43, Correct links count -> 1 (2). [ 58.292653] ntfs3: nvme0n1p3: ino=72557, Correct links count -> 1 (2). [ 58.293244] ntfs3: nvme0n1p3: ino=728d8, Correct links count -> 1 (2). [ 58.294306] ntfs3: nvme0n1p3: ino=72c6e, Correct links count -> 1 (2). [ 58.294944] ntfs3: nvme0n1p3: ino=72ff7, Correct links count -> 1 (2). [ 58.295666] ntfs3: nvme0n1p3: ino=73271, Correct links count -> 1 (2). [ 58.296281] ntfs3: nvme0n1p3: ino=735fd, Correct links count -> 1 (2). [ 58.296991] ntfs3: nvme0n1p3: ino=73880, Correct links count -> 1 (2). [ 63.295009] ntfs3: 13921 callbacks suppressed [ 63.295017] ntfs3: nvme0n1p3: ino=6be65, Correct links count -> 1 (2). [ 63.295902] ntfs3: nvme0n1p3: ino=6c08e, Correct links count -> 1 (2). [ 63.296252] ntfs3: nvme0n1p3: ino=6c2e3, Correct links count -> 1 (2). [ 63.297581] ntfs3: nvme0n1p3: ino=6c610, Correct links count -> 1 (2). [ 63.298321] ntfs3: nvme0n1p3: ino=6c7f9, Correct links count -> 1 (2). [ 63.298730] ntfs3: nvme0n1p3: ino=6cb24, Correct links count -> 1 (2). [ 63.299079] ntfs3: nvme0n1p3: ino=6ceda, Correct links count -> 1 (2). [ 63.299408] ntfs3: nvme0n1p3: ino=6d288, Correct links count -> 1 (2). [ 63.299727] ntfs3: nvme0n1p3: ino=6d533, Correct links count -> 1 (2). [ 63.300080] ntfs3: nvme0n1p3: ino=6d77b, Correct links count -> 1 (2). [ 68.299457] ntfs3: 8228 callbacks suppressed [ 68.299467] ntfs3: nvme0n1p3: ino=3e248, Correct links count -> 1 (2). [ 68.301391] ntfs3: nvme0n1p3: ino=5d7b7, Correct links count -> 1 (2). [ 68.302440] ntfs3: nvme0n1p3: ino=5853d, Correct links count -> 1 (2). [ 68.303123] ntfs3: nvme0n1p3: ino=3ca2e, Correct links count -> 1 (2). [ 68.303722] ntfs3: nvme0n1p3: ino=59a98, Correct links count -> 1 (2). [ 68.304292] ntfs3: nvme0n1p3: ino=59a9b, Correct links count -> 1 (2). [ 68.304981] ntfs3: nvme0n1p3: ino=59a9e, Correct links count -> 1 (2). [ 68.305629] ntfs3: nvme0n1p3: ino=59aa1, Correct links count -> 1 (2). [ 68.306120] ntfs3: nvme0n1p3: ino=3214f, Correct links count -> 1 (2). [ 68.306539] ntfs3: nvme0n1p3: ino=2077b, Correct links count -> 1 (2). [ 73.302727] ntfs3: 8502 callbacks suppressed [ 73.302736] ntfs3: nvme0n1p3: ino=5aa99, Correct links count -> 1 (2). [ 73.303992] ntfs3: nvme0n1p3: ino=50ee9, Correct links count -> 1 (2). [ 73.304744] ntfs3: nvme0n1p3: ino=20420, Correct links count -> 1 (2). [ 73.305080] ntfs3: nvme0n1p3: ino=258c, Correct links count -> 1 (2). [ 73.305470] ntfs3: nvme0n1p3: ino=5a30d, Correct links count -> 1 (2). [ 73.307283] ntfs3: nvme0n1p3: ino=5a54c, Correct links count -> 1 (2). [ 73.307890] ntfs3: nvme0n1p3: ino=5c9de, Correct links count -> 1 (2). [ 73.308495] ntfs3: nvme0n1p3: ino=3d82d, Correct links count -> 1 (2). [ 73.309581] ntfs3: nvme0n1p3: ino=3d839, Correct links count -> 1 (2). [ 73.310016] ntfs3: nvme0n1p3: ino=25c77, Correct links count -> 1 (2). [ 78.304861] ntfs3: 11462 callbacks suppressed [ 78.304868] ntfs3: nvme0n1p3: ino=5c714, Correct links count -> 1 (2). [ 78.305579] ntfs3: nvme0n1p3: ino=57d11, Correct links count -> 1 (2). [ 78.306151] ntfs3: nvme0n1p3: ino=5842d, Correct links count -> 1 (2). [ 78.307412] ntfs3: nvme0n1p3: ino=34e6, Correct links count -> 1 (3). [ 78.307843] ntfs3: nvme0n1p3: ino=5bb23, Correct links count -> 1 (2). [ 78.308509] ntfs3: nvme0n1p3: ino=5c722, Correct links count -> 1 (2). [ 78.310018] ntfs3: nvme0n1p3: ino=5d761, Correct links count -> 1 (2). [ 78.310717] ntfs3: nvme0n1p3: ino=33d18, Correct links count -> 1 (3). [ 78.311179] ntfs3: nvme0n1p3: ino=5d75b, Correct links count -> 1 (3). [ 78.311605] ntfs3: nvme0n1p3: ino=5c708, Correct links count -> 1 (3). Johan
Hi, Had a look at ntfs3 code and it is corrupting your volume. Every such message you are seeing is damaging a file or directory on your volume. I would personally suggest you modify your /etc/fstab to mount read-only. If it is getting simple things like this wrong who knows what else it is doing incorrect... Best regards, Anton > On 15 Apr 2024, at 11:20, Johan Hovold <johan@kernel.org> wrote: > > On Mon, Apr 15, 2024 at 11:54:19AM +0200, Johan Hovold wrote: >> On Thu, Apr 11, 2024 at 02:03:52PM +0300, Konstantin Komarov wrote: > >>> Messages like this: >>> >>> diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c >>> index eb7a8c9fba01..8cc94a6a97ed 100644 >>> --- a/fs/ntfs3/inode.c >>> +++ b/fs/ntfs3/inode.c >>> @@ -424,7 +424,6 @@ static struct inode *ntfs_read_mft(struct inode *inode, >>> if (names != le16_to_cpu(rec->hard_links)) { >>> /* Correct minor error on the fly. Do not mark inode as dirty. */ >>> - ntfs_inode_warn(inode, "Correct links count -> %u.", names); >>> rec->hard_links = cpu_to_le16(names); >>> ni->mi.dirty = true; >>> } >>> >>> can also be suppressed for the sake of seamless transition from a remote >>> NTFS driver. >>> However, I believe that file system corrections should be reported to >>> the user. >> >> A colleague of mine also tracked down a failed boot to the removal of >> the ntfs driver and reported seeing similar warnings with the ntfs3 >> driver. >> >> We're both accessing an NTFS partition on a Windows on Arm device, but >> it makes you wonder whether these warnings (corrections) are correct or >> indicative of a problem in the driver? > > This is what I see with a recursive ls on that partition (I've added > rec->hard_links in parentheses): > > [ 38.287555] ntfs3: nvme0n1p3: ino=2e1e7, Correct links count -> 1 (2). > [ 38.288593] ntfs3: nvme0n1p3: ino=75ff, Correct links count -> 1 (2). > [ 38.289887] ntfs3: nvme0n1p3: ino=1b4e1, Correct links count -> 1 (2). > [ 38.290144] ntfs3: nvme0n1p3: ino=78c6, Correct links count -> 1 (2). > [ 38.291313] ntfs3: nvme0n1p3: ino=8781b, Correct links count -> 1 (2). > [ 38.291823] ntfs3: nvme0n1p3: ino=8781e, Correct links count -> 1 (2). > [ 38.292289] ntfs3: nvme0n1p3: ino=87820, Correct links count -> 1 (2). > [ 38.292978] ntfs3: nvme0n1p3: ino=87823, Correct links count -> 1 (2). > [ 38.300531] ntfs3: nvme0n1p3: ino=a324, Correct links count -> 1 (2). > [ 38.312235] ntfs3: nvme0n1p3: ino=882c3, Correct links count -> 1 (2). > [ 43.286846] ntfs3: 5479 callbacks suppressed > [ 43.286856] ntfs3: nvme0n1p3: ino=14aa, Correct links count -> 1 (2). > [ 43.286998] ntfs3: nvme0n1p3: ino=14ac, Correct links count -> 1 (2). > [ 43.287194] ntfs3: nvme0n1p3: ino=12cc2, Correct links count -> 1 (2). > [ 43.287386] ntfs3: nvme0n1p3: ino=12ccd, Correct links count -> 1 (2). > [ 43.287576] ntfs3: nvme0n1p3: ino=12d15, Correct links count -> 1 (2). > [ 43.287667] ntfs3: nvme0n1p3: ino=12d19, Correct links count -> 1 (2). > [ 43.287877] ntfs3: nvme0n1p3: ino=12d3a, Correct links count -> 1 (2). > [ 43.288051] ntfs3: nvme0n1p3: ino=12d88, Correct links count -> 1 (2). > [ 43.288265] ntfs3: nvme0n1p3: ino=874, Correct links count -> 1 (2). > [ 43.288326] ntfs3: nvme0n1p3: ino=875, Correct links count -> 1 (2). > [ 48.288211] ntfs3: 7735 callbacks suppressed > [ 48.288220] ntfs3: nvme0n1p3: ino=33391, Correct links count -> 1 (2). > [ 48.289115] ntfs3: nvme0n1p3: ino=347a4, Correct links count -> 1 (2). > [ 48.290214] ntfs3: nvme0n1p3: ino=3553f, Correct links count -> 1 (2). > [ 48.291193] ntfs3: nvme0n1p3: ino=35793, Correct links count -> 1 (2). > [ 48.292818] ntfs3: nvme0n1p3: ino=358a0, Correct links count -> 1 (2). > [ 48.293529] ntfs3: nvme0n1p3: ino=38f54, Correct links count -> 1 (2). > [ 48.293901] ntfs3: nvme0n1p3: ino=391f6, Correct links count -> 1 (2). > [ 48.294303] ntfs3: nvme0n1p3: ino=39324, Correct links count -> 1 (2). > [ 48.294729] ntfs3: nvme0n1p3: ino=394a0, Correct links count -> 1 (2). > [ 48.295063] ntfs3: nvme0n1p3: ino=3956a, Correct links count -> 1 (2). > [ 53.289392] ntfs3: 11442 callbacks suppressed > [ 53.289401] ntfs3: nvme0n1p3: ino=6293e, Correct links count -> 1 (2). > [ 53.289972] ntfs3: nvme0n1p3: ino=61df1, Correct links count -> 1 (2). > [ 53.290241] ntfs3: nvme0n1p3: ino=61df3, Correct links count -> 1 (2). > [ 53.290578] ntfs3: nvme0n1p3: ino=61f3b, Correct links count -> 1 (2). > [ 53.290881] ntfs3: nvme0n1p3: ino=62025, Correct links count -> 1 (2). > [ 53.291045] ntfs3: nvme0n1p3: ino=629af, Correct links count -> 1 (2). > [ 53.291181] ntfs3: nvme0n1p3: ino=61e3c, Correct links count -> 1 (2). > [ 53.291463] ntfs3: nvme0n1p3: ino=61e22, Correct links count -> 1 (2). > [ 53.291743] ntfs3: nvme0n1p3: ino=62882, Correct links count -> 1 (2). > [ 53.292099] ntfs3: nvme0n1p3: ino=61b3d, Correct links count -> 1 (2). > [ 58.291790] ntfs3: 5373 callbacks suppressed > [ 58.291799] ntfs3: nvme0n1p3: ino=6d5a5, Correct links count -> 1 (2). > [ 58.292106] ntfs3: nvme0n1p3: ino=6d7f6, Correct links count -> 1 (2). > [ 58.292372] ntfs3: nvme0n1p3: ino=6db43, Correct links count -> 1 (2). > [ 58.292653] ntfs3: nvme0n1p3: ino=72557, Correct links count -> 1 (2). > [ 58.293244] ntfs3: nvme0n1p3: ino=728d8, Correct links count -> 1 (2). > [ 58.294306] ntfs3: nvme0n1p3: ino=72c6e, Correct links count -> 1 (2). > [ 58.294944] ntfs3: nvme0n1p3: ino=72ff7, Correct links count -> 1 (2). > [ 58.295666] ntfs3: nvme0n1p3: ino=73271, Correct links count -> 1 (2). > [ 58.296281] ntfs3: nvme0n1p3: ino=735fd, Correct links count -> 1 (2). > [ 58.296991] ntfs3: nvme0n1p3: ino=73880, Correct links count -> 1 (2). > [ 63.295009] ntfs3: 13921 callbacks suppressed > [ 63.295017] ntfs3: nvme0n1p3: ino=6be65, Correct links count -> 1 (2). > [ 63.295902] ntfs3: nvme0n1p3: ino=6c08e, Correct links count -> 1 (2). > [ 63.296252] ntfs3: nvme0n1p3: ino=6c2e3, Correct links count -> 1 (2). > [ 63.297581] ntfs3: nvme0n1p3: ino=6c610, Correct links count -> 1 (2). > [ 63.298321] ntfs3: nvme0n1p3: ino=6c7f9, Correct links count -> 1 (2). > [ 63.298730] ntfs3: nvme0n1p3: ino=6cb24, Correct links count -> 1 (2). > [ 63.299079] ntfs3: nvme0n1p3: ino=6ceda, Correct links count -> 1 (2). > [ 63.299408] ntfs3: nvme0n1p3: ino=6d288, Correct links count -> 1 (2). > [ 63.299727] ntfs3: nvme0n1p3: ino=6d533, Correct links count -> 1 (2). > [ 63.300080] ntfs3: nvme0n1p3: ino=6d77b, Correct links count -> 1 (2). > [ 68.299457] ntfs3: 8228 callbacks suppressed > [ 68.299467] ntfs3: nvme0n1p3: ino=3e248, Correct links count -> 1 (2). > [ 68.301391] ntfs3: nvme0n1p3: ino=5d7b7, Correct links count -> 1 (2). > [ 68.302440] ntfs3: nvme0n1p3: ino=5853d, Correct links count -> 1 (2). > [ 68.303123] ntfs3: nvme0n1p3: ino=3ca2e, Correct links count -> 1 (2). > [ 68.303722] ntfs3: nvme0n1p3: ino=59a98, Correct links count -> 1 (2). > [ 68.304292] ntfs3: nvme0n1p3: ino=59a9b, Correct links count -> 1 (2). > [ 68.304981] ntfs3: nvme0n1p3: ino=59a9e, Correct links count -> 1 (2). > [ 68.305629] ntfs3: nvme0n1p3: ino=59aa1, Correct links count -> 1 (2). > [ 68.306120] ntfs3: nvme0n1p3: ino=3214f, Correct links count -> 1 (2). > [ 68.306539] ntfs3: nvme0n1p3: ino=2077b, Correct links count -> 1 (2). > [ 73.302727] ntfs3: 8502 callbacks suppressed > [ 73.302736] ntfs3: nvme0n1p3: ino=5aa99, Correct links count -> 1 (2). > [ 73.303992] ntfs3: nvme0n1p3: ino=50ee9, Correct links count -> 1 (2). > [ 73.304744] ntfs3: nvme0n1p3: ino=20420, Correct links count -> 1 (2). > [ 73.305080] ntfs3: nvme0n1p3: ino=258c, Correct links count -> 1 (2). > [ 73.305470] ntfs3: nvme0n1p3: ino=5a30d, Correct links count -> 1 (2). > [ 73.307283] ntfs3: nvme0n1p3: ino=5a54c, Correct links count -> 1 (2). > [ 73.307890] ntfs3: nvme0n1p3: ino=5c9de, Correct links count -> 1 (2). > [ 73.308495] ntfs3: nvme0n1p3: ino=3d82d, Correct links count -> 1 (2). > [ 73.309581] ntfs3: nvme0n1p3: ino=3d839, Correct links count -> 1 (2). > [ 73.310016] ntfs3: nvme0n1p3: ino=25c77, Correct links count -> 1 (2). > [ 78.304861] ntfs3: 11462 callbacks suppressed > [ 78.304868] ntfs3: nvme0n1p3: ino=5c714, Correct links count -> 1 (2). > [ 78.305579] ntfs3: nvme0n1p3: ino=57d11, Correct links count -> 1 (2). > [ 78.306151] ntfs3: nvme0n1p3: ino=5842d, Correct links count -> 1 (2). > [ 78.307412] ntfs3: nvme0n1p3: ino=34e6, Correct links count -> 1 (3). > [ 78.307843] ntfs3: nvme0n1p3: ino=5bb23, Correct links count -> 1 (2). > [ 78.308509] ntfs3: nvme0n1p3: ino=5c722, Correct links count -> 1 (2). > [ 78.310018] ntfs3: nvme0n1p3: ino=5d761, Correct links count -> 1 (2). > [ 78.310717] ntfs3: nvme0n1p3: ino=33d18, Correct links count -> 1 (3). > [ 78.311179] ntfs3: nvme0n1p3: ino=5d75b, Correct links count -> 1 (3). > [ 78.311605] ntfs3: nvme0n1p3: ino=5c708, Correct links count -> 1 (3). > > Johan
On Mon, Apr 15, 2024 at 11:32:50AM +0000, Anton Altaparmakov wrote: > Had a look at ntfs3 code and it is corrupting your volume. Every such > message you are seeing is damaging a file or directory on your volume. That's what I feared, thanks for confirming. > I would personally suggest you modify your /etc/fstab to mount > read-only. If it is getting simple things like this wrong who knows > what else it is doing incorrect... I fully agree and that's partly also why I asked Christian to make sure that the alias fs type is always mounted RO. But it seems we have a bigger problem here and should just restore the old ntfs driver for now. Johan
On Mon, Apr 15, 2024 at 01:42:01PM +0200, Johan Hovold wrote: > On Mon, Apr 15, 2024 at 11:32:50AM +0000, Anton Altaparmakov wrote: > > > Had a look at ntfs3 code and it is corrupting your volume. Every such > > message you are seeing is damaging a file or directory on your volume. > > That's what I feared, thanks for confirming. > > > I would personally suggest you modify your /etc/fstab to mount > > read-only. If it is getting simple things like this wrong who knows > > what else it is doing incorrect... > > I fully agree and that's partly also why I asked Christian to make sure > that the alias fs type is always mounted RO. > > But it seems we have a bigger problem here and should just restore the > old ntfs driver for now. Hey Linus, A brief summary: The removal of the legacy ntfs driver has caused a regression for some users that rely on the legacy ntfs driver to be available during boot. The affected user here is Johan (Cc'ed). In addition he's seeing dmesg warnings that he didn't see before with the legacy ntfs driver. I see the following options to resolve this: (1) Since the ntfs3 driver is supposed to serve as a drop-in replacement for the legacy ntfs driver we should to it the same way we did it for ext3 and ext4 where ext4 registers itself also for the ext3 driver. In other words, we would register ntfs3 as ntfs3 filesystem type and as legacy ntfs filesystem type. To make it fully compatible we also need to make sure it's persistently mounted read-only. (2) We revert the ntfs driver (for now) as we are in the middle of the cycle. If you decide (2) is the right way to go then I would suggest we try the removal once more next cycle with the proposed change in (1) in place if you're up for that. Christian
On Mon, 15 Apr 2024 at 07:16, Christian Brauner <brauner@kernel.org> wrote: > > (1) Since the ntfs3 driver is supposed to serve as a drop-in replacement > for the legacy ntfs driver we should to it the same way we did it > for ext3 and ext4 where ext4 registers itself also for the ext3 > driver. In other words, we would register ntfs3 as ntfs3 filesystem > type and as legacy ntfs filesystem type. I think that if just registering it under the same name solves the immediate issue, that's the one we should just go for. > To make it fully compatible > we also need to make sure it's persistently mounted read-only. My reaction to that is "only if it turns out we really need to". It sounds unlikely that somebody has an old ntfs setup and then tries to mount things rw which didn't use to work and things go sideways if that then suddenly works. But "unlikely" isn't "impossible", of course - it's just that I'd suggest we actually wait for that report to happen and ask what the heck they were doing and why they were doing that... Linus
On Mon, Apr 15, 2024 at 08:23:46AM -0700, Linus Torvalds wrote: > On Mon, 15 Apr 2024 at 07:16, Christian Brauner <brauner@kernel.org> wrote: > > > > (1) Since the ntfs3 driver is supposed to serve as a drop-in replacement > > for the legacy ntfs driver we should to it the same way we did it > > for ext3 and ext4 where ext4 registers itself also for the ext3 > > driver. In other words, we would register ntfs3 as ntfs3 filesystem > > type and as legacy ntfs filesystem type. > > I think that if just registering it under the same name solves the > immediate issue, that's the one we should just go for. I agree. > > To make it fully compatible > > we also need to make sure it's persistently mounted read-only. > > My reaction to that is "only if it turns out we really need to". Unfortunately, we do. It seems that ntfs3 has some bugs, and (according to Anton, the ntfs-classic maintainer) it's actually corrupting perfectly good filesystems. I'd expected that this kind of bug would have been shaken out by now (it's been in the kernel for over two and a half years!) but it seems like people just haven't been using it.
On Mon, Apr 15, 2024 at 08:23:46AM -0700, Linus Torvalds wrote: > On Mon, 15 Apr 2024 at 07:16, Christian Brauner <brauner@kernel.org> wrote: > > > > (1) Since the ntfs3 driver is supposed to serve as a drop-in replacement > > for the legacy ntfs driver we should to it the same way we did it > > for ext3 and ext4 where ext4 registers itself also for the ext3 > > driver. In other words, we would register ntfs3 as ntfs3 filesystem > > type and as legacy ntfs filesystem type. > > I think that if just registering it under the same name solves the > immediate issue, that's the one we should just go for. I also tend to agree, but... > > To make it fully compatible > > we also need to make sure it's persistently mounted read-only. > > My reaction to that is "only if it turns out we really need to". > > It sounds unlikely that somebody has an old ntfs setup and then tries > to mount things rw which didn't use to work and things go sideways if > that then suddenly works. > > But "unlikely" isn't "impossible", of course - it's just that I'd > suggest we actually wait for that report to happen and ask what the > heck they were doing and why they were doing that... I think the "ntfs" alias must always be mounted read-only because you can currently have an fstab entry which does not specify "ro" and this mount would suddenly become writeable when updating to 6.9 (possibly by a non-privileged user, etc). We also need to do something about the ntfs3 driver spamming the logs about broken corrections also when mounted read-only even if it doesn't eat your filesystem then. And it seems write-support should be disabled in the driver by default until someone has tracked down why listing a directory can currently corrupt your filesystem. Johan
On Mon, 15 Apr 2024 at 08:47, Johan Hovold <johan@kernel.org> wrote: > > I think the "ntfs" alias must always be mounted read-only because you > can currently have an fstab entry which does not specify "ro" and this > mount would suddenly become writeable when updating to 6.9 (possibly by > a non-privileged user, etc). Well, it would be fairly easy to do particularly if we just do it for the old legacy case. Of course, even the legacy case had that CONFIG_NTFS_RW option, so people who depended on _that_ would want to be able to remount... Linus
On Mon, Apr 15, 2024 at 08:51:13AM -0700, Linus Torvalds wrote: > On Mon, 15 Apr 2024 at 08:47, Johan Hovold <johan@kernel.org> wrote: > > > > I think the "ntfs" alias must always be mounted read-only because you > > can currently have an fstab entry which does not specify "ro" and this > > mount would suddenly become writeable when updating to 6.9 (possibly by > > a non-privileged user, etc). > > Well, it would be fairly easy to do particularly if we just do it for > the old legacy case. > > Of course, even the legacy case had that CONFIG_NTFS_RW option, so > people who depended on _that_ would want to be able to remount... Ah, right, I forgot about CONFIG_NTFS_RW as I've never enabled it. Judging from the now removed Kconfig entry perhaps not that many people did: The only supported operation is overwriting existing files, without changing the file length. No file or directory creation, deletion or renaming is possible. but I guess it still makes my argument above mostly moot. At least if we disable write support in ntfs3 by default for now... Johan
On Mon, Apr 15, 2024 at 06:06:03PM +0200, Johan Hovold wrote: > On Mon, Apr 15, 2024 at 08:51:13AM -0700, Linus Torvalds wrote: > > On Mon, 15 Apr 2024 at 08:47, Johan Hovold <johan@kernel.org> wrote: > > > > > > I think the "ntfs" alias must always be mounted read-only because you > > > can currently have an fstab entry which does not specify "ro" and this > > > mount would suddenly become writeable when updating to 6.9 (possibly by > > > a non-privileged user, etc). > > > > Well, it would be fairly easy to do particularly if we just do it for > > the old legacy case. > > > > Of course, even the legacy case had that CONFIG_NTFS_RW option, so > > people who depended on _that_ would want to be able to remount... > > Ah, right, I forgot about CONFIG_NTFS_RW as I've never enabled it. > > Judging from the now removed Kconfig entry perhaps not that many people > did: > > The only supported operation is overwriting existing files, > without changing the file length. No file or directory > creation, deletion or renaming is possible. > > but I guess it still makes my argument above mostly moot. > > At least if we disable write support in ntfs3 by default for now... I think we can disable write support in ntfs3 for now. I've picked up the patch to make ntfs3 serve I sent some time ago that Johan tested now. The only thing left is to disable write support for ntfs3 as legacy ntfs driver for now. I took a stab at this. The following two patches I'm appending _should_ be enough iiuc. Johan, please take a look and please test. This is also available on: https://git.kernel.org/pub/scm/linux/kernel/git/vfs/vfs.git vfs.fixes So feel free to pull from there and look there as well.
On Tue, Apr 16, 2024 at 12:38:56PM +0200, Christian Brauner wrote: > On Mon, Apr 15, 2024 at 06:06:03PM +0200, Johan Hovold wrote: > > Ah, right, I forgot about CONFIG_NTFS_RW as I've never enabled it. > > > > Judging from the now removed Kconfig entry perhaps not that many people > > did: > > > > The only supported operation is overwriting existing files, > > without changing the file length. No file or directory > > creation, deletion or renaming is possible. > > > > but I guess it still makes my argument above mostly moot. > > > > At least if we disable write support in ntfs3 by default for now... > > I think we can disable write support in ntfs3 for now. I've picked up > the patch to make ntfs3 serve I sent some time ago that Johan tested > now. Note that I actually meant that write support should be disabled completely in ntfs3 for now. After this first encounter I have zero confidence in that driver and pushing people towards using it (by removing the old, read-only one) is just gonna result in further corrupted filesystems. At least make sure it can't modify anything by default and mark write-support as experimental and broken or something as that's apparently what it is. > The only thing left is to disable write support for ntfs3 as legacy ntfs > driver for now. I took a stab at this. The following two patches > I'm appending _should_ be enough iiuc. Johan, please take a look and > please test. I skimmed them and gave them a quick spin. It seems that not specifying either "ro" or "rw" in fstab now results in a ro mount, but I can still specify "rw" explicitly (in fstab or command line) and end up with: /dev/nvme0n1p3 on /mnt/windows type ntfs (rw,relatime,uid=0,gid=0,iocharset=iso8859-1) For obvious reasons, I did not dare listing the root directory or write anything, but it looks like it's not read-only. Using just my naive temporary hack from yesterday: diff --git a/fs/ntfs3/super.c b/fs/ntfs3/super.c index 8d2e51bae2cb..26be6c6d1032 100644 --- a/fs/ntfs3/super.c +++ b/fs/ntfs3/super.c @@ -1177,6 +1177,9 @@ static int ntfs_fill_super(struct super_block *sb, struct fs_context *fc) sb->s_xattr = ntfs_xattr_handlers; sb->s_d_op = options->nocase ? &ntfs_dentry_ops : NULL; + ntfs_warn(sb, "ntfs3 driver is broken, mounting read only"); + sb->s_flags |= SB_RDONLY; + options->nls = ntfs_load_nls(options->nls_name); if (IS_ERR(options->nls)) { options->nls = NULL; seems to prevent also explicit rw mounts (but judging from your patches it is not necessarily sufficient to prevent all modifications). Johan
On 25.03.2024 11:34, Christian Brauner wrote: > This causes visible changes for users that rely on ntfs3 to serve as an > alternative for the legacy ntfs driver. Print statements such as this > should probably be made conditional on a debug config option or similar. > > Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> > Cc: Johan Hovold <johan@kernel.org> > Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com > Signed-off-by: Christian Brauner <brauner@kernel.org> > --- > fs/ntfs3/inode.c | 1 - > 1 file changed, 1 deletion(-) > > diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c > index eb7a8c9fba01..8cc94a6a97ed 100644 > --- a/fs/ntfs3/inode.c > +++ b/fs/ntfs3/inode.c > @@ -424,7 +424,6 @@ static struct inode *ntfs_read_mft(struct inode *inode, > > if (names != le16_to_cpu(rec->hard_links)) { > /* Correct minor error on the fly. Do not mark inode as dirty. */ > - ntfs_inode_warn(inode, "Correct links count -> %u.", names); > rec->hard_links = cpu_to_le16(names); > ni->mi.dirty = true; > } Hello Christian, all, The initial and true reason for multiple warnings is the disregard of short (DOS) names when counting hard links. This patch should fixes this bug: https://lore.kernel.org/ntfs3/0cb0b314-e4f6-40a2-9628-0fe7d905a676@paragon-software.com/T/#u There is no longer a need to suppress the message itself.
On Wed, Apr 17, 2024 at 07:07:06PM +0300, Konstantin Komarov wrote: > On 25.03.2024 11:34, Christian Brauner wrote: > > This causes visible changes for users that rely on ntfs3 to serve as an > > alternative for the legacy ntfs driver. Print statements such as this > > should probably be made conditional on a debug config option or similar. > The initial and true reason for multiple warnings is the disregard of > short (DOS) names when counting hard links. > > This patch should fixes this bug: > https://lore.kernel.org/ntfs3/0cb0b314-e4f6-40a2-9628-0fe7d905a676@paragon-software.com/T/#u As I just replied in that thread, I'm also seeing link counts being reduced from 3 to 1, that is, to not just be decremented by one due to the DOS name bug. Are you sure there are no further issues here and that the patch is indeed correct? I did not test the patch, which is white-space damaged, but if it addresses also the remaining warnings then the commit message would need to be updated as well. Johan
diff --git a/fs/ntfs3/inode.c b/fs/ntfs3/inode.c index eb7a8c9fba01..8cc94a6a97ed 100644 --- a/fs/ntfs3/inode.c +++ b/fs/ntfs3/inode.c @@ -424,7 +424,6 @@ static struct inode *ntfs_read_mft(struct inode *inode, if (names != le16_to_cpu(rec->hard_links)) { /* Correct minor error on the fly. Do not mark inode as dirty. */ - ntfs_inode_warn(inode, "Correct links count -> %u.", names); rec->hard_links = cpu_to_le16(names); ni->mi.dirty = true; }
This causes visible changes for users that rely on ntfs3 to serve as an alternative for the legacy ntfs driver. Print statements such as this should probably be made conditional on a debug config option or similar. Cc: "Matthew Wilcox (Oracle)" <willy@infradead.org> Cc: Johan Hovold <johan@kernel.org> Link: https://lore.kernel.org/r/Zf2zPf5TO5oYt3I3@hovoldconsulting.com Signed-off-by: Christian Brauner <brauner@kernel.org> --- fs/ntfs3/inode.c | 1 - 1 file changed, 1 deletion(-)