linux-kernel - Re: [PATCH 02/11] writeback: switch to per-bdi threads for flushing data

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-Id: <5DB1E671-D2D7-4052-977E-981EEDAD98FB@cam.ac.uk>
Date:	Wed, 20 May 2009 15:02:01 +0100
From:	Anton Altaparmakov <aia21@....ac.uk>
To:	Jens Axboe <jens.axboe@...cle.com>
Cc:	Christoph Hellwig <hch@...radead.org>,
	linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
	chris.mason@...cle.com, david@...morbit.com,
	akpm@...ux-foundation.org, jack@...e.cz,
	yanmin_zhang@...ux.intel.com, aia21@...tab.net,
	linux-ntfs-dev@...ts.sourceforge.net
Subject: Re: [PATCH 02/11] writeback: switch to per-bdi threads for flushing data

Hi,

On 20 May 2009, at 13:49, Jens Axboe wrote:
> On Wed, May 20 2009, Christoph Hellwig wrote:
>> Can you run the hunk below past Anton and get it upstream separately?
>> The code does indeed looks extremly fishy, but I'd rather not see it
>> go in a large unrelated patch..
>
> Yes, it really should go out of this patchset and into a prep patch.
> Anton, care to comment?
>
>> On Mon, May 18, 2009 at 02:19:43PM +0200, Jens Axboe wrote:
>>> diff --git a/fs/ntfs/super.c b/fs/ntfs/super.c
>>> index f76951d..c4cb157 100644
>>> --- a/fs/ntfs/super.c
>>> +++ b/fs/ntfs/super.c
>>> @@ -2373,39 +2373,13 @@ static void ntfs_put_super(struct  
>>> super_block *sb)
>>> 		vol->mftmirr_ino = NULL;
>>> 	}
>>> 	/*
>>> -	 * If any dirty inodes are left, throw away all mft data page  
>>> cache
>>> -	 * pages to allow a clean umount.  This should never happen any  
>>> more
>>> -	 * due to mft.c::ntfs_mft_writepage() cleaning all the dirty  
>>> pages as
>>> -	 * the underlying mft records are written out and cleaned.  If  
>>> it does,
>>> +	 * We should have no dirty inodes left, due to
>>> +	 * mft.c::ntfs_mft_writepage() cleaning all the dirty pages as
>>> +	 * the underlying mft records are written out and cleaned.
>>> 	 * happen anyway, we want to know...

You need to remove the above line, too.  It does not make sense to  
leave half a sentence there...

Otherwise you can apply this patch if you really want.  It is just a  
debug/bandaid.  I used to have problems where dirty inodes were left  
and I had put that in to allow the unmount to succeed properly.  I  
believe that should not happen any more as explained in the comment  
above but I left the fixup code as a sanity check that would produce  
output to the system log that people would hopefully report should my  
fix not be correct/sufficient...

Best regards,

Anton


>>>
>>> 	 */
>>> 	ntfs_commit_inode(vol->mft_ino);
>>> 	write_inode_now(vol->mft_ino, 1);
>>> -	if (sb_has_dirty_inodes(sb)) {
>>> -		const char *s1, *s2;
>>> -
>>> -		mutex_lock(&vol->mft_ino->i_mutex);
>>> -		truncate_inode_pages(vol->mft_ino->i_mapping, 0);
>>> -		mutex_unlock(&vol->mft_ino->i_mutex);
>>> -		write_inode_now(vol->mft_ino, 1);
>>> -		if (sb_has_dirty_inodes(sb)) {
>>> -			static const char *_s1 = "inodes";
>>> -			static const char *_s2 = "";
>>> -			s1 = _s1;
>>> -			s2 = _s2;
>>> -		} else {
>>> -			static const char *_s1 = "mft pages";
>>> -			static const char *_s2 = "They have been thrown "
>>> -					"away.  ";
>>> -			s1 = _s1;
>>> -			s2 = _s2;
>>> -		}
>>> -		ntfs_error(sb, "Dirty %s found at umount time.  %sYou should "
>>> -				"run chkdsk.  Please email "
>>> -				"linux-ntfs-dev@...ts.sourceforge.net and say "
>>> -				"that you saw this message.  Thank you.", s1,
>>> -				s2);
>>> -	}
>>> #endif /* NTFS_RW */
>>>
>>> 	iput(vol->mft_ino);

-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer, http://www.linux-ntfs.org/

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/