Re: [RFC] ext3 freeze feature ver 0.2

To: Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Friday, February 8, 2008 - 6:48 am

Hi,


OK I would like to implement the freeze feature on VFS
as the filesystem independent ioctl so that it can be
available on filesystems that have already had write_super_lockfs()
and unlockfs().
The usage for the freeze ioctl is the following.
  int ioctl(int fd, int FIFREEZE, long *timeval);
    fd:file descriptor of mountpoint
    FIFREEZE:request cord for freeze
    timeval:timeout period (second)

And the unfreeze ioctl is the following.
  int ioctl(int fd, int FITHAW, NULL);
    fd:file descriptor of mountpoint
    FITHAW:Request cord for unfreeze

I think we need the timeout feature which thaws the filesystem
after lapse of specified time for a fail-safe in case the freezer
accesses the frozen filesystem and causes a deadlock.
I intend to implement the timeout feature on VFS.
(This is realized by registering the delayed work which calls
thaw_bdev() to the delayed work queue.)

Any comments are very welcome.

Cheers, Takashi
--

From: Andreas Dilger <adilger@...>

To: Takashi Sato <t-sato@...>

Cc: Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Friday, February 8, 2008 - 9:26 am

You may as well make the common ioctl the same as the XFS version,
both by number and parameters, so that applications which already
understand the XFS ioctl will work on other filesystems.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--

From: Christoph Hellwig <hch@...>

To: Andreas Dilger <adilger@...>

Cc: Takashi Sato <t-sato@...>, Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Friday, February 8, 2008 - 10:59 am

Yes.  In facy you should be able to lift the implementations of
XFS_IOC_FREEZE and XFS_IOC_THAW to generic code, there's nothing
XFS-specific in there.

--

To: Christoph Hellwig <hch@...>, Andreas Dilger <adilger@...>

Cc: Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Friday, February 15, 2008 - 7:51 am

Hi,


According to Documentation/ioctl-number.txt,
XFS_IOC_XXXs (_IOWR('X', aa, bb)) are defined for XFS like below.
From Documentation/ioctl-number.txt:
----------------------------------------------------------------------------
Code    Seq#    Include File            Comments
'X'     all     linux/xfs_fs.h
----------------------------------------------------------------------------
So XFS_IOC_FREEZE and XFS_IOC_THAW cannot be lifted to generic code simply.
I think we should create new generic numbers for freeze and thaw
like FIBMAP as followings.
linux/fs.h:
#define FIFREEZE _IO(0x00,3)
#define FITHAW   _IO(0x00,4)

And xfs_freeze calls XFS_IOC_FREEZE with a magic number 1, but what is 1?
Instead, I'd like the sec to timeout on freeze API in order to thaw
the filesystem automatically.  It can prevent a filesystem from staying
frozen forever.
(Because a freezer may cause a deadlock by accessing the frozen filesystem.)

Any comments are very welcome.

Cheers, Takashi 

--

From: Christoph Hellwig <hch@...>

To: Takashi Sato <t-sato@...>

Cc: Christoph Hellwig <hch@...>, Andreas Dilger <adilger@...>, Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Saturday, February 16, 2008 - 9:25 am

Actually we've lifted specific ioctls to the generic layer before all
the time in drivers.  That's the only way to make functionality that was
specific to a single driver (or in this case filesystem) generic.  If
the numbering issues confuses you make sure to add a big comment


Timeout based locking is generally a horrible idea, there's a reason
we don't have any primitives for that in the kernel :)
--

From: Eric Sandeen <sandeen@...>

To: Takashi Sato <t-sato@...>

Cc: Christoph Hellwig <hch@...>, Andreas Dilger <adilger@...>, Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Friday, February 15, 2008 - 10:24 am

It also says:

'f'     00-1F   linux/ext2_fs.h

and yet include/linux.h has:

#define	FS_IOC_GETFLAGS			_IOR('f', 1, long)
#define	FS_IOC_SETFLAGS			_IOW('f', 2, long)

as generic vfs ioctls.  These ioctls started out as
EXT2_IOC_SETFLAGS/EXT2_IOC_GETFLAGS but they were generically useful,


Looks like it's called "level" but it's probably a holdover, it doesn't

I'm still not very comfortable with the timeout; if you un-freeze on a
timer, how do you know that the work for which you needed the fileystem
frozen is complete?  How would you know if your snapshot was good if
there's a possibility that the fs unfroze while it was being taken?

Thanks,
--

From: <t-sato@...>

To: Eric Sandeen <sandeen@...>, Christoph Hellwig <hch@...>

Cc: Andreas Dilger <adilger@...>, Theodore Tso <tytso@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Date: Tuesday, February 19, 2008 - 7:27 am

Thank you for good information.


My following freeze ioctl never perform the timeout when 0 is specified
as timeval.  So, existent applications which don't expect the timeout
can stay frozen with 0.
 int ioctl(int fd, int FIFREEZE, long *timeval);
    fd:file descriptor of mountpoint
    FIFREEZE:request cord for freeze
    timeval:timeout period (second)

And how about adding the new ioctl to reset the timeval like below?
(Dmitri proposed this idea before.)
 int ioctl(int fd, int FIFREEZE_RESET_TIMEOUT, long *timeval);
    fd:file descriptor of mountpoint
    FIFREEZE_RESET_TIMEOUT:request cord for reset of timeout period 
    timeval:new timeout period
This is useful for the application to set the timeval more accurately.
For example, the freezer resets the timeval to 10 seconds every 5
seconds.  In this approach, even if the freezer causes a deadlock
by accessing the frozen filesystem, it will be solved by the timeout
in 10 seconds and the freezer can recognize that at the next reset
of timeval.

Any comments are very welcome.

Cheers, Takashi
--

To: <linux-ext4@...>, <linux-fsdevel@...>, <xfs@...>, <dm-devel@...>

Cc: linux-kernel@vger.kernel.org <linux-kernel@...>

Subject: [RFC] freeze feature ver 1.0

Date: Friday, March 7, 2008 - 5:13 am

Hi,

I have re-based my freeze patch from linux-2.6.25-rc3 to
linux-2.6.25-rc4.
There is no functional change from the previous version.
All of comments from ML have already been reflected in this patch.
The ioctls for the freeze feature are below.

o Freeze the filesystem
  int ioctl(int fd, int FIFREEZE, long *timeval)
    fd: The file descriptor of the mountpoint
    FIFREEZE: request code for the freeze
    timeval: the timeout period in seconds
             If it's 0 or 1, the timeout isn't set.
             This special case of "1" is implemented to keep
             the compatibility with XFS applications.
    Return value: 0 if the operation succeeds. Otherwise, -1

o Reset the timeout period
  This is useful for the application to set the timeval more accurately.
  For example, the freezer resets the timeval to 10 seconds every 5
  seconds.  In this approach, even if the freezer causes a deadlock
  by accessing the frozen filesystem, it will be solved by the timeout
  in 10 seconds and the freezer can recognize that at the next reset
  of timeval.
  int ioctl(int fd, int FIFREEZE_RESET_TIMEOUT, long *timeval)
    fd:file descriptor of mountpoint
    FIFREEZE_RESET_TIMEOUT: request code for reset of timeout period
    timeval: new timeout period in seconds
    Return value: 0 if the operation succeeds. Otherwise, -1
    Error number: If the filesystem has already been unfrozen,
                  errno is set to EINVAL.

o Unfreeze the filesystem
  int ioctl(int fd, int FITHAW, long *timeval)
    fd: The file descriptor of the mountpoint
    FITHAW: request code for unfreeze
    timeval: Ignored
    Return value: 0 if the operation succeeds. Otherwise, -1

Any comments are very welcome.

Cheers, Takashi

Signed-off-by: Takashi Sato &lt;t-sato@yk.jp.nec.com&gt;
---
diff -uprN -X linux-2.6.25-rc4-freeze/Documentation/dontdiff linux-2.6.25-rc4/drivers/md/dm.c linux-2.6.25-rc4-freeze/dr
ivers/md/dm.c
--- linux-2.6.25-rc4/drivers/md/dm.c	2008-03-05 13:33:54.000000...[ message continues ]

To: <linux-ext4@...>, <linux-fsdevel@...>

Cc: <linux-kernel@...>

Subject: [RFC] ext3 freeze feature ver 0.2

Date: Tuesday, February 26, 2008 - 4:20 am

Hi,

I have improved the following two points in my ext3 freeze feature.
o Add the new ioctl to reset the timeout period as above
The usage is as below.
int ioctl(int fd, int FIFREEZE_RESET_TIMEOUT, long *timeval);
fd:file descriptor of mountpoint
FIFREEZE_RESET_TIMEOUT:request code for reset of timeout period
timeval:new timeout period
Return value: 0 if the operation succeeds. Otherwise, -1
Error number: If the filesystem has already been unfrozen,
it sets EINVAL to errno.
I have made sure the following two results with this ioctl.
- After the deadlock occurred by accessing the frozen filesystem,
it could be solved by the reset timeout.
- And the freezer could recognize that from the error number (EINVAL)
at the next reset of timeval.

o Elevate XFS ioctl numbers (XFS_IOC_FREEZE and XFS_IOC_THAW) to the VFS
As Andreas Dilger and Christoph Hellwig advised me, I have elevated
them to include/linux/fs.h as below.
#define FIFREEZE _IOWR('X', 119, int)
#define FITHAW _IOWR('X', 120, int)
The ioctl numbers used by XFS applications don't need to be changed.
But my following ioctl for the freeze needs the parameter
as the timeout period. So if XFS applications don't want the timeout
feature as the current implementation, the parameter needs to be
changed 1 (level?) into 0.

I haven't changed the following ioctls from the previous version.
int ioctl(int fd, int cmd, long *timeval)
fd: The file descriptor of the mountpoint
cmd: FIFREEZE for the freeze or FITHAW for the unfreeze
timeval: The timeout value expressed in seconds
If it's 0, the timeout isn't set.
Return value: 0 if the operation succeeds. Otherwise, -1

Any comments are very welcome.

Cheers, Takashi

Signed-off-by: Takashi Sato &lt;t-sato@yk.jp.nec.com&gt;
---
diff -uprN -X /home/sho/pub/MC/freeze-set/dontdiff linux-2.6.25-rc3.org/drivers/md/dm.c linux-2.6.25-rc3-freeze/drive...[ message continues ]

From: Eric Sandeen <sandeen@...>

To: Takashi Sato <t-sato@...>

Cc: <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Subject: Re: [RFC] ext3 freeze feature ver 0.2

Date: Tuesday, February 26, 2008 - 12:39 pm

So, existing xfs applications calling the xfs ioctl now will behave
differently, right?  We can only keep the same ioctl number if the
calling semantics are the same.  Keeping the same number but changing
the semantics is harmful, IMHO....

-Eric
--

From: Andreas Dilger <adilger@...>

To: Eric Sandeen <sandeen@...>

Cc: Takashi Sato <t-sato@...>, <linux-ext4@...>, <linux-fsdevel@...>, <linux-kernel@...>

Subject: Re: [RFC] ext3 freeze feature ver 0.2

Date: Tuesday, February 26, 2008 - 1:08 pm

Do we know what this parameter was supposed to mean?

We could special case "1" if needed to keep compatibility (documenting
this clearly), either making it == 0, or some very long timeout (1h
or whatever).  A relatively minor wart I think.

Cheers, Andreas
--
Andreas Dilger
Sr. Staff Engineer, Lustre Group
Sun Microsystems of Canada, Inc.

--