[PATCH] block new writers on frozen filesystems

[PATCH] block new writers on frozen filesystems

[Christoph Hellwig]

When the lockfs patches went in an important bit got lost, the call in
generic_file_write to put newly incoming writers to sleep when a
filesystem is frozen.  Nathan added back the call in the now separate
XFS write patch, and the patch for the generic code is below:


Index: mm/filemap.c
===================================================================
RCS file: /cvs/linux-2.6-xfs/mm/filemap.c,v
retrieving revision 1.14
diff -u -p -r1.14 filemap.c
--- mm/filemap.c	5 Jan 2005 14:17:31 -0000	1.14
+++ mm/filemap.c	4 Feb 2005 21:35:53 -0000
@@ -2046,6 +2046,8 @@ __generic_file_aio_write_nolock(struct k
 	count = ocount;
 	pos = *ppos;
 
+	vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
+
 	/* We can write back this queue in page reclaim */
 	current->backing_dev_info = mapping->backing_dev_info;
 	written = 0;

Re: [PATCH] block new writers on frozen filesystems

[Andrew Morton]

Christoph Hellwig <hch@lst.de> wrote:
>
> When the lockfs patches went in an important bit got lost, the call in
> generic_file_write to put newly incoming writers to sleep when a
> filesystem is frozen.  Nathan added back the call in the now separate
> XFS write patch, and the patch for the generic code is below:
> 
> 
> Index: mm/filemap.c
> ===================================================================
> RCS file: /cvs/linux-2.6-xfs/mm/filemap.c,v
> retrieving revision 1.14
> diff -u -p -r1.14 filemap.c
> --- mm/filemap.c	5 Jan 2005 14:17:31 -0000	1.14
> +++ mm/filemap.c	4 Feb 2005 21:35:53 -0000
> @@ -2046,6 +2046,8 @@ __generic_file_aio_write_nolock(struct k
>  	count = ocount;
>  	pos = *ppos;
>  
> +	vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);

hm, I didn't pay much attention to this stuff.  Shouldn't the direct-io
code be waiting as well?  Are all paths which can write to the bdev supposed
to be blocked?  kjournald?

Re: [PATCH] block new writers on frozen filesystems

[Christoph Hellwig]

> > +	vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
> 
> hm, I didn't pay much attention to this stuff.  Shouldn't the direct-io
> code be waiting as well?  Are all paths which can write to the bdev supposed
> to be blocked?  kjournald?

This should block new data writers.  New metadata writes need to be
blocked in the individual filesystem, and only when the state is set
to SB_FREEZE_TRANS (a few steps later in freeze_bdev).

Currently only XFS deals with that part.

Re: [PATCH] block new writers on frozen filesystems

[Anton Altaparmakov]

On Sun, 13 Feb 2005, Christoph Hellwig wrote:
> > > +	vfs_check_frozen(inode->i_sb, SB_FREEZE_WRITE);
> > 
> > hm, I didn't pay much attention to this stuff.  Shouldn't the direct-io
> > code be waiting as well?  Are all paths which can write to the bdev supposed
> > to be blocked?  kjournald?
> 
> This should block new data writers.  New metadata writes need to be
> blocked in the individual filesystem, and only when the state is set
> to SB_FREEZE_TRANS (a few steps later in freeze_bdev).

Why this distinction?  At least on NTFS metadata is stored in normal files 
(everything is a file in NTFS) and the same readpage/writepage address 
space operations are used to access both...

Best regards,

	Anton
-- 
Anton Altaparmakov <aia21 at cam.ac.uk> (replace at with @)
Unix Support, Computing Service, University of Cambridge, CB2 3QH, UK
Linux NTFS maintainer / IRC: #ntfs on irc.freenode.net
WWW: http://linux-ntfs.sf.net/ & http://www-stu.christs.cam.ac.uk/~aia21/

Re: [PATCH] block new writers on frozen filesystems

[Christoph Hellwig]

On Sun, Feb 13, 2005 at 03:05:22PM +0000, Anton Altaparmakov wrote:
> > This should block new data writers.  New metadata writes need to be
> > blocked in the individual filesystem, and only when the state is set
> > to SB_FREEZE_TRANS (a few steps later in freeze_bdev).
> 
> Why this distinction?  At least on NTFS metadata is stored in normal files 
> (everything is a file in NTFS) and the same readpage/writepage address 
> space operations are used to access both...

Because flushing the data after you block new writes usually involved
writing out metadata aswell, so you can't block them at the same time
without deadlocking.