linux-kernel - Re: [dm-devel] [PATCH] Fix over-zealous flush

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: <20110317122833.30077397@notabene.brown>
Date:	Thu, 17 Mar 2011 12:28:33 +1100
From:	NeilBrown <neilb@...e.de>
To:	Jeff Moyer <jmoyer@...hat.com>
Cc:	James Bottomley <James.Bottomley@...e.de>,
	device-mapper development <dm-devel@...hat.com>,
	Jens Axboe <axboe@...nel.dk>, linux-raid@...r.kernel.org,
	linux-kernel@...r.kernel.org,
	Christoph Hellwig <hch@...radead.org>,
	linux-fsdevel@...r.kernel.org
Subject: Re: [dm-devel] [PATCH] Fix over-zealous flush_disk when changing
 device size.

On Wed, 16 Mar 2011 16:30:22 -0400 Jeff Moyer <jmoyer@...hat.com> wrote:

> NeilBrown <neilb@...e.de> writes:
> 
> >> Synchronous notification of errors.  If we don't try to write everything
> >> back immediately after the size change, we don't see dirty pages in
> >> zapped regions until the writeout/page cache management takes it into
> >> its head to try to clean the pages.
> >> 
> >
> > So if you just want synchronous errors, I think you want:
> >     fsync_bdev()
> >
> > which calls sync_filesystem() if it can find a filesystem, else
> > sync_blockdev();  (sync_filesystem itself calls sync_blockdev too).
> 
> ... which deadlocks md.  ;-)  writeback_inodes_sb_nr is waiting for the
> flusher thread to write back the dirty data.  The flusher thread is
> stuck in md_write_start, here:
> 
>         wait_event(mddev->sb_wait,
>                    !test_bit(MD_CHANGE_PENDING, &mddev->flags));
> 
> This is after reverting your change, and replacing the flush_disk call
> in check_disk_size_change with a call to fsync_bdev.  I'm not familiar
> enough with md to really suggest a way forward.  Neil?

That would be quite easy to avoid.
Just call
   md_write_start()
before revalidate_disk, and
   md_write_end()
afterwards.
You wouldn't have a 'bio' to pass in - but it is rather ugly requiring
one anyway - I should fix that.
For testing, just pass in NULL, and change
	if (bio_data_dir(bi) != WRITE)
		return;
to
	if (bi && bio_data_dir(bi) != WRITE)
		return;

NeilBrown

--
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/