linux-kernel - Re: [PATCH 1/2] Added flush_disk to factor out common buffer cache flushing code.

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: <1210183141.3067.13.camel@localhost.localdomain>
Date:	Wed, 07 May 2008 12:59:01 -0500
From:	James Bottomley <James.Bottomley@...senPartnership.com>
To:	Christoph Hellwig <hch@...radead.org>
Cc:	Andrew Patterson <andrew.patterson@...com>,
	linux-scsi@...r.kernel.org, linux-kernel@...r.kernel.org,
	viro@...iv.linux.org.uk, axboe@...nel.dk,
	andmike@...ux.vnet.ibm.com
Subject: Re: [PATCH 1/2] Added flush_disk to factor out common buffer cache
	flushing code.

On Tue, 2008-05-06 at 04:44 -0400, Christoph Hellwig wrote:
> On Mon, May 05, 2008 at 05:04:19PM -0600, Andrew Patterson wrote:
> > Added flush_disk to factor out common buffer cache flushing code.
> > 
> > We need to be able to flush the buffer cache for more than just when a
> > disk is changed, so we factor out common cache flush code in
> > check_disk_change() to an internal flush_disk() routine.  This routine
> > will then be used for both disk changes and disk resizes (in a later
> > patch).
> > 
> > Include the disk name in the text indicating that there are busy
> > inodes on the device and increase the KERN severity of the message.
> 
> This doesn't make much sense to me.  When a disk has grown there's no
> point in invalidating any buffers, and when it has shrunk it's too late
> already.  Also I suspect modern filesystems might be really allergic to
> this kind of under the hood actions.  That is if they use the bdev
> mapping at all, something that at least xfs and I think btrfs aswell
> don't do at all.

I agree on the grown disc case.  For the shrunk disk, we need at least
to invalidate the sectors that no-longer physically exist.

The two use cases for shrinking I can see are

     1. planned: the fs is already shrunk to within the new boundaries
        and all data is relocated, so invalidate is fine (any dirty
        buffers that might exist in the shrunk region are there only
        because they were relocated but not yet written to their
        original location).
     2. unplanned:  In this case, the fs is probably toast, so whether
        we invalidate or not isn't going to make a whole lot of
        difference; it's still going to try to read or write from
        sectors beyond the new size and get I/O errors.

Unfortunately, we don't seem to have a partial invalidation function for
the page cache and filesystem, so should we have one?

James


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