* [PATCH] fix mount option pasing to make inode cluster deletion default (again)
@ 2008-02-14 3:34 Eric Sandeen
2008-02-14 7:45 ` [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP Josef 'Jeff' Sipek
2008-02-18 14:38 ` [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
0 siblings, 2 replies; 10+ messages in thread
From: Eric Sandeen @ 2008-02-14 3:34 UTC (permalink / raw)
To: xfs-oss
mod xfs-linux-melb:xfs-kern:29683a /
git commit 574342f4ad450b33bc85ec53210b8aa8bfff2fcf
broke default options in such a way that empty inode clusters
are no longer deleted by default, because if no options are
given, we "goto done;" without setting the default
XFSMNT_IDELETE flag.
All this logic could probably be rearranged to make things
clearer, but for now I think this small patch fixes it:
Set IDELETE a.k.a. "noikeep" by default, but if dmapi is in
use, turn it back off (i.e. "ikeep") *unless* noikeep was
specifically requested.
Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
---
Index: linux-2.6.24/fs/xfs/linux-2.6/xfs_super.c
===================================================================
--- linux-2.6.24.orig/fs/xfs/linux-2.6/xfs_super.c
+++ linux-2.6.24/fs/xfs/linux-2.6/xfs_super.c
@@ -171,9 +171,10 @@ xfs_parseargs(
char *this_char, *value, *eov;
int dsunit, dswidth, vol_dsunit, vol_dswidth;
int iosize;
- int ikeep = 0;
+ int noikeep = 0; /* track _explicit_ requests */
args->flags |= XFSMNT_BARRIER;
+ args->flags |= XFSMNT_IDELETE; /* i.e. "noikeep" is default */
args->flags2 |= XFSMNT2_COMPAT_IOSIZE;
if (!options)
@@ -302,9 +303,9 @@ xfs_parseargs(
} else if (!strcmp(this_char, MNTOPT_NOBARRIER)) {
args->flags &= ~XFSMNT_BARRIER;
} else if (!strcmp(this_char, MNTOPT_IKEEP)) {
- ikeep = 1;
args->flags &= ~XFSMNT_IDELETE;
} else if (!strcmp(this_char, MNTOPT_NOIKEEP)) {
+ noikeep = 1; /* explicitly requested */
args->flags |= XFSMNT_IDELETE;
} else if (!strcmp(this_char, MNTOPT_LARGEIO)) {
args->flags2 &= ~XFSMNT2_COMPAT_IOSIZE;
@@ -410,8 +411,8 @@ xfs_parseargs(
* Note that if "ikeep" or "noikeep" mount options are
* supplied, then they are honored.
*/
- if (!(args->flags & XFSMNT_DMAPI) && !ikeep)
- args->flags |= XFSMNT_IDELETE;
+ if ((args->flags & XFSMNT_DMAPI) && !noikeep)
+ args->flags &= ~XFSMNT_IDELETE;
if ((args->flags & XFSMNT_NOALIGN) != XFSMNT_NOALIGN) {
if (dsunit) {
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-14 3:34 [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
@ 2008-02-14 7:45 ` Josef 'Jeff' Sipek
2008-02-15 16:19 ` Christoph Hellwig
2008-02-18 14:38 ` [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
1 sibling, 1 reply; 10+ messages in thread
From: Josef 'Jeff' Sipek @ 2008-02-14 7:45 UTC (permalink / raw)
To: xfs; +Cc: sandeen, Josef 'Jeff' Sipek
Change the *_IDELETE flags to *_IKEEP, and flip the logic as necessary.
This completely eliminates the no-no-no-idelete madness.
Additionally, "ikeep" or "noikeep" is always displayed in /proc/mounts
option string. This should help clear up any confusion about what the
current mode is.
Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
---
fs/xfs/linux-2.6/xfs_super.c | 11 ++++++-----
fs/xfs/xfs_clnt.h | 2 +-
fs/xfs/xfs_ialloc.c | 2 +-
fs/xfs/xfs_mount.h | 2 +-
fs/xfs/xfs_vfsops.c | 4 ++--
fs/xfs/xfsidbg.c | 2 +-
6 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index a0b1235..5c343a0 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -171,7 +171,7 @@ xfs_parseargs(
char *this_char, *value, *eov;
int dsunit, dswidth, vol_dsunit, vol_dswidth;
int iosize;
- int ikeep = 0;
+ int ikeep = 0; /* don't keep by default */
args->flags |= XFSMNT_BARRIER;
args->flags2 |= XFSMNT2_COMPAT_IOSIZE;
@@ -303,9 +303,9 @@ xfs_parseargs(
args->flags &= ~XFSMNT_BARRIER;
} else if (!strcmp(this_char, MNTOPT_IKEEP)) {
ikeep = 1;
- args->flags &= ~XFSMNT_IDELETE;
+ args->flags |= XFSMNT_IKEEP;
} else if (!strcmp(this_char, MNTOPT_NOIKEEP)) {
- args->flags |= XFSMNT_IDELETE;
+ args->flags &= ~XFSMNT_IKEEP;
} else if (!strcmp(this_char, MNTOPT_LARGEIO)) {
args->flags2 &= ~XFSMNT2_COMPAT_IOSIZE;
} else if (!strcmp(this_char, MNTOPT_NOLARGEIO)) {
@@ -411,7 +411,7 @@ xfs_parseargs(
* supplied, then they are honored.
*/
if (!(args->flags & XFSMNT_DMAPI) && !ikeep)
- args->flags |= XFSMNT_IDELETE;
+ args->flags &= ~XFSMNT_IKEEP;
if ((args->flags & XFSMNT_NOALIGN) != XFSMNT_NOALIGN) {
if (dsunit) {
@@ -446,6 +446,7 @@ xfs_showargs(
{
static struct proc_xfs_info xfs_info_set[] = {
/* the few simple ones we can get from the mount struct */
+ { XFS_MOUNT_IKEEP, "," MNTOPT_IKEEP },
{ XFS_MOUNT_WSYNC, "," MNTOPT_WSYNC },
{ XFS_MOUNT_INO64, "," MNTOPT_INO64 },
{ XFS_MOUNT_NOALIGN, "," MNTOPT_NOALIGN },
@@ -461,7 +462,7 @@ xfs_showargs(
};
static struct proc_xfs_info xfs_info_unset[] = {
/* the few simple ones we can get from the mount struct */
- { XFS_MOUNT_IDELETE, "," MNTOPT_IKEEP },
+ { XFS_MOUNT_IKEEP, "," MNTOPT_NOIKEEP },
{ XFS_MOUNT_COMPAT_IOSIZE, "," MNTOPT_LARGEIO },
{ XFS_MOUNT_BARRIER, "," MNTOPT_NOBARRIER },
{ XFS_MOUNT_SMALL_INUMS, "," MNTOPT_64BITINODE },
diff --git a/fs/xfs/xfs_clnt.h b/fs/xfs/xfs_clnt.h
index d16c1b9..d5d1e60 100644
--- a/fs/xfs/xfs_clnt.h
+++ b/fs/xfs/xfs_clnt.h
@@ -86,7 +86,7 @@ struct xfs_mount_args {
#define XFSMNT_NOUUID 0x01000000 /* Ignore fs uuid */
#define XFSMNT_DMAPI 0x02000000 /* enable dmapi/xdsm */
#define XFSMNT_BARRIER 0x04000000 /* use write barriers */
-#define XFSMNT_IDELETE 0x08000000 /* inode cluster delete */
+#define XFSMNT_IKEEP 0x08000000 /* inode cluster delete */
#define XFSMNT_SWALLOC 0x10000000 /* turn on stripe width
* allocation */
#define XFSMNT_DIRSYNC 0x40000000 /* sync creat,link,unlink,rename
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 1409c2d..badf745 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1053,7 +1053,7 @@ xfs_difree(
/*
* When an inode cluster is free, it becomes eligible for removal
*/
- if ((mp->m_flags & XFS_MOUNT_IDELETE) &&
+ if (!(mp->m_flags & XFS_MOUNT_IKEEP) &&
(rec.ir_freecount == XFS_IALLOC_INODES(mp))) {
*delete = 1;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 435d625..87ee8b8 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -366,7 +366,7 @@ typedef struct xfs_mount {
#define XFS_MOUNT_SMALL_INUMS (1ULL << 15) /* users wants 32bit inodes */
#define XFS_MOUNT_NOUUID (1ULL << 16) /* ignore uuid during mount */
#define XFS_MOUNT_BARRIER (1ULL << 17)
-#define XFS_MOUNT_IDELETE (1ULL << 18) /* delete empty inode clusters*/
+#define XFS_MOUNT_IKEEP (1ULL << 18) /* keep empty inode clusters*/
#define XFS_MOUNT_SWALLOC (1ULL << 19) /* turn on stripe width
* allocation */
#define XFS_MOUNT_RDONLY (1ULL << 20) /* read-only fs */
diff --git a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
index a0f287e..e809b1c 100644
--- a/fs/xfs/xfs_vfsops.c
+++ b/fs/xfs/xfs_vfsops.c
@@ -279,8 +279,8 @@ xfs_start_flags(
mp->m_readio_log = mp->m_writeio_log = ap->iosizelog;
}
- if (ap->flags & XFSMNT_IDELETE)
- mp->m_flags |= XFS_MOUNT_IDELETE;
+ if (ap->flags & XFSMNT_IKEEP)
+ mp->m_flags |= XFS_MOUNT_IKEEP;
if (ap->flags & XFSMNT_DIRSYNC)
mp->m_flags |= XFS_MOUNT_DIRSYNC;
if (ap->flags & XFSMNT_ATTR2)
diff --git a/fs/xfs/xfsidbg.c b/fs/xfs/xfsidbg.c
index aa029da..a875351 100644
--- a/fs/xfs/xfsidbg.c
+++ b/fs/xfs/xfsidbg.c
@@ -6282,7 +6282,7 @@ xfsidbg_xmount(xfs_mount_t *mp)
"SMALL_INUMS", /* 0x8000 */
"NOUUID", /* 0x10000 */
"BARRIER", /* 0x20000 */
- "IDELETE", /* 0x40000 */
+ "IKEEP", /* 0x40000 */
"SWALLOC", /* 0x80000 */
"RDONLY", /* 0x100000 */
"DIRSYNC", /* 0x200000 */
--
1.5.4.rc2.85.g9de45-dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-14 7:45 ` [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP Josef 'Jeff' Sipek
@ 2008-02-15 16:19 ` Christoph Hellwig
2008-02-15 16:23 ` Eric Sandeen
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2008-02-15 16:19 UTC (permalink / raw)
To: Josef 'Jeff' Sipek; +Cc: xfs, sandeen
On Thu, Feb 14, 2008 at 02:45:39AM -0500, Josef 'Jeff' Sipek wrote:
> Change the *_IDELETE flags to *_IKEEP, and flip the logic as necessary.
>
> This completely eliminates the no-no-no-idelete madness.
>
> Additionally, "ikeep" or "noikeep" is always displayed in /proc/mounts
> option string. This should help clear up any confusion about what the
> current mode is.
Looks fine to me, and I think the changed display in /proc/<pid>/mounts
is fine aswell.
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-15 16:19 ` Christoph Hellwig
@ 2008-02-15 16:23 ` Eric Sandeen
2008-02-16 13:22 ` Christoph Hellwig
0 siblings, 1 reply; 10+ messages in thread
From: Eric Sandeen @ 2008-02-15 16:23 UTC (permalink / raw)
To: Christoph Hellwig; +Cc: Josef 'Jeff' Sipek, xfs
Christoph Hellwig wrote:
> On Thu, Feb 14, 2008 at 02:45:39AM -0500, Josef 'Jeff' Sipek wrote:
>> Change the *_IDELETE flags to *_IKEEP, and flip the logic as necessary.
>>
>> This completely eliminates the no-no-no-idelete madness.
>>
>> Additionally, "ikeep" or "noikeep" is always displayed in /proc/mounts
>> option string. This should help clear up any confusion about what the
>> current mode is.
>
> Looks fine to me, and I think the changed display in /proc/<pid>/mounts
> is fine aswell.
>
IMHO if we want to display defaults, then we should probably change it
so that all defaults are displayed, and not make noikeep special in this
respect. (oh, and "noquota" is already there, too)
Doesn't matter much to me either way but it should be consistent across
all options, I think.
-Eric
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-15 16:23 ` Eric Sandeen
@ 2008-02-16 13:22 ` Christoph Hellwig
2008-02-17 21:30 ` Josef 'Jeff' Sipek
0 siblings, 1 reply; 10+ messages in thread
From: Christoph Hellwig @ 2008-02-16 13:22 UTC (permalink / raw)
To: Eric Sandeen; +Cc: Christoph Hellwig, Josef 'Jeff' Sipek, xfs
On Fri, Feb 15, 2008 at 10:23:43AM -0600, Eric Sandeen wrote:
> IMHO if we want to display defaults, then we should probably change it
> so that all defaults are displayed, and not make noikeep special in this
> respect. (oh, and "noquota" is already there, too)
>
> Doesn't matter much to me either way but it should be consistent across
> all options, I think.
True, it's probably better to kill the noikeep string.
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-16 13:22 ` Christoph Hellwig
@ 2008-02-17 21:30 ` Josef 'Jeff' Sipek
2008-02-18 5:33 ` Barry Naujok
0 siblings, 1 reply; 10+ messages in thread
From: Josef 'Jeff' Sipek @ 2008-02-17 21:30 UTC (permalink / raw)
To: hch, sandeen, xfs; +Cc: Josef 'Jeff' Sipek
Change the *_IDELETE flags to *_IKEEP, and flip the logic as necessary.
This completely eliminates the no-no-no-idelete madness.
Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
---
fs/xfs/linux-2.6/xfs_super.c | 10 +++++-----
fs/xfs/xfs_clnt.h | 2 +-
fs/xfs/xfs_ialloc.c | 2 +-
fs/xfs/xfs_mount.h | 2 +-
fs/xfs/xfs_vfsops.c | 4 ++--
fs/xfs/xfsidbg.c | 2 +-
6 files changed, 11 insertions(+), 11 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index a0b1235..8cabbc1 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -171,7 +171,7 @@ xfs_parseargs(
char *this_char, *value, *eov;
int dsunit, dswidth, vol_dsunit, vol_dswidth;
int iosize;
- int ikeep = 0;
+ int ikeep = 0; /* don't keep by default */
args->flags |= XFSMNT_BARRIER;
args->flags2 |= XFSMNT2_COMPAT_IOSIZE;
@@ -303,9 +303,9 @@ xfs_parseargs(
args->flags &= ~XFSMNT_BARRIER;
} else if (!strcmp(this_char, MNTOPT_IKEEP)) {
ikeep = 1;
- args->flags &= ~XFSMNT_IDELETE;
+ args->flags |= XFSMNT_IKEEP;
} else if (!strcmp(this_char, MNTOPT_NOIKEEP)) {
- args->flags |= XFSMNT_IDELETE;
+ args->flags &= ~XFSMNT_IKEEP;
} else if (!strcmp(this_char, MNTOPT_LARGEIO)) {
args->flags2 &= ~XFSMNT2_COMPAT_IOSIZE;
} else if (!strcmp(this_char, MNTOPT_NOLARGEIO)) {
@@ -411,7 +411,7 @@ xfs_parseargs(
* supplied, then they are honored.
*/
if (!(args->flags & XFSMNT_DMAPI) && !ikeep)
- args->flags |= XFSMNT_IDELETE;
+ args->flags &= ~XFSMNT_IKEEP;
if ((args->flags & XFSMNT_NOALIGN) != XFSMNT_NOALIGN) {
if (dsunit) {
@@ -446,6 +446,7 @@ xfs_showargs(
{
static struct proc_xfs_info xfs_info_set[] = {
/* the few simple ones we can get from the mount struct */
+ { XFS_MOUNT_IKEEP, "," MNTOPT_IKEEP },
{ XFS_MOUNT_WSYNC, "," MNTOPT_WSYNC },
{ XFS_MOUNT_INO64, "," MNTOPT_INO64 },
{ XFS_MOUNT_NOALIGN, "," MNTOPT_NOALIGN },
@@ -461,7 +462,6 @@ xfs_showargs(
};
static struct proc_xfs_info xfs_info_unset[] = {
/* the few simple ones we can get from the mount struct */
- { XFS_MOUNT_IDELETE, "," MNTOPT_IKEEP },
{ XFS_MOUNT_COMPAT_IOSIZE, "," MNTOPT_LARGEIO },
{ XFS_MOUNT_BARRIER, "," MNTOPT_NOBARRIER },
{ XFS_MOUNT_SMALL_INUMS, "," MNTOPT_64BITINODE },
diff --git a/fs/xfs/xfs_clnt.h b/fs/xfs/xfs_clnt.h
index d16c1b9..d5d1e60 100644
--- a/fs/xfs/xfs_clnt.h
+++ b/fs/xfs/xfs_clnt.h
@@ -86,7 +86,7 @@ struct xfs_mount_args {
#define XFSMNT_NOUUID 0x01000000 /* Ignore fs uuid */
#define XFSMNT_DMAPI 0x02000000 /* enable dmapi/xdsm */
#define XFSMNT_BARRIER 0x04000000 /* use write barriers */
-#define XFSMNT_IDELETE 0x08000000 /* inode cluster delete */
+#define XFSMNT_IKEEP 0x08000000 /* inode cluster delete */
#define XFSMNT_SWALLOC 0x10000000 /* turn on stripe width
* allocation */
#define XFSMNT_DIRSYNC 0x40000000 /* sync creat,link,unlink,rename
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 1409c2d..badf745 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1053,7 +1053,7 @@ xfs_difree(
/*
* When an inode cluster is free, it becomes eligible for removal
*/
- if ((mp->m_flags & XFS_MOUNT_IDELETE) &&
+ if (!(mp->m_flags & XFS_MOUNT_IKEEP) &&
(rec.ir_freecount == XFS_IALLOC_INODES(mp))) {
*delete = 1;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 435d625..87ee8b8 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -366,7 +366,7 @@ typedef struct xfs_mount {
#define XFS_MOUNT_SMALL_INUMS (1ULL << 15) /* users wants 32bit inodes */
#define XFS_MOUNT_NOUUID (1ULL << 16) /* ignore uuid during mount */
#define XFS_MOUNT_BARRIER (1ULL << 17)
-#define XFS_MOUNT_IDELETE (1ULL << 18) /* delete empty inode clusters*/
+#define XFS_MOUNT_IKEEP (1ULL << 18) /* keep empty inode clusters*/
#define XFS_MOUNT_SWALLOC (1ULL << 19) /* turn on stripe width
* allocation */
#define XFS_MOUNT_RDONLY (1ULL << 20) /* read-only fs */
diff --git a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
index a0f287e..e809b1c 100644
--- a/fs/xfs/xfs_vfsops.c
+++ b/fs/xfs/xfs_vfsops.c
@@ -279,8 +279,8 @@ xfs_start_flags(
mp->m_readio_log = mp->m_writeio_log = ap->iosizelog;
}
- if (ap->flags & XFSMNT_IDELETE)
- mp->m_flags |= XFS_MOUNT_IDELETE;
+ if (ap->flags & XFSMNT_IKEEP)
+ mp->m_flags |= XFS_MOUNT_IKEEP;
if (ap->flags & XFSMNT_DIRSYNC)
mp->m_flags |= XFS_MOUNT_DIRSYNC;
if (ap->flags & XFSMNT_ATTR2)
diff --git a/fs/xfs/xfsidbg.c b/fs/xfs/xfsidbg.c
index aa029da..a875351 100644
--- a/fs/xfs/xfsidbg.c
+++ b/fs/xfs/xfsidbg.c
@@ -6282,7 +6282,7 @@ xfsidbg_xmount(xfs_mount_t *mp)
"SMALL_INUMS", /* 0x8000 */
"NOUUID", /* 0x10000 */
"BARRIER", /* 0x20000 */
- "IDELETE", /* 0x40000 */
+ "IKEEP", /* 0x40000 */
"SWALLOC", /* 0x80000 */
"RDONLY", /* 0x100000 */
"DIRSYNC", /* 0x200000 */
--
1.5.4.rc2.85.g9de45-dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-17 21:30 ` Josef 'Jeff' Sipek
@ 2008-02-18 5:33 ` Barry Naujok
2008-02-18 6:26 ` Josef 'Jeff' Sipek
0 siblings, 1 reply; 10+ messages in thread
From: Barry Naujok @ 2008-02-18 5:33 UTC (permalink / raw)
To: Josef 'Jeff' Sipek, hch, sandeen, xfs
On Mon, 18 Feb 2008 08:30:01 +1100, Josef 'Jeff' Sipek
<jeffpc@josefsipek.net> wrote:
> Change the *_IDELETE flags to *_IKEEP, and flip the logic as necessary.
>
> This completely eliminates the no-no-no-idelete madness.
>
> Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
> ---
> fs/xfs/linux-2.6/xfs_super.c | 10 +++++-----
> fs/xfs/xfs_clnt.h | 2 +-
> fs/xfs/xfs_ialloc.c | 2 +-
> fs/xfs/xfs_mount.h | 2 +-
> fs/xfs/xfs_vfsops.c | 4 ++--
> fs/xfs/xfsidbg.c | 2 +-
> 6 files changed, 11 insertions(+), 11 deletions(-)
I _think_ the dmapi support logic is wrong:
> diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
> index a0b1235..8cabbc1 100644
> --- a/fs/xfs/linux-2.6/xfs_super.c
> +++ b/fs/xfs/linux-2.6/xfs_super.c
> @@ -171,7 +171,7 @@ xfs_parseargs(
> char *this_char, *value, *eov;
> int dsunit, dswidth, vol_dsunit, vol_dswidth;
> int iosize;
> - int ikeep = 0;
> + int ikeep = 0; /* don't keep by default */
int inokeep = 0;
> args->flags |= XFSMNT_BARRIER;
> args->flags2 |= XFSMNT2_COMPAT_IOSIZE;
> @@ -303,9 +303,9 @@ xfs_parseargs(
> args->flags &= ~XFSMNT_BARRIER;
> } else if (!strcmp(this_char, MNTOPT_IKEEP)) {
> ikeep = 1;
yank ikeep here
> - args->flags &= ~XFSMNT_IDELETE;
> + args->flags |= XFSMNT_IKEEP;
> } else if (!strcmp(this_char, MNTOPT_NOIKEEP)) {
> - args->flags |= XFSMNT_IDELETE;
> + args->flags &= ~XFSMNT_IKEEP;
add: inokeep = 1;
> } else if (!strcmp(this_char, MNTOPT_LARGEIO)) {
> args->flags2 &= ~XFSMNT2_COMPAT_IOSIZE;
> } else if (!strcmp(this_char, MNTOPT_NOLARGEIO)) {
> @@ -411,7 +411,7 @@ xfs_parseargs(
> * supplied, then they are honored.
> */
> if (!(args->flags & XFSMNT_DMAPI) && !ikeep)
> - args->flags |= XFSMNT_IDELETE;
> + args->flags &= ~XFSMNT_IKEEP;
if ((args->flags & XFSMNT_DMAPI) && !inokeep)
args->flags |= XFSMNT_IKEEP;
> if ((args->flags & XFSMNT_NOALIGN) != XFSMNT_NOALIGN) {
> if (dsunit) {
> @@ -446,6 +446,7 @@ xfs_showargs(
> {
> static struct proc_xfs_info xfs_info_set[] = {
> /* the few simple ones we can get from the mount struct */
> + { XFS_MOUNT_IKEEP, "," MNTOPT_IKEEP },
> { XFS_MOUNT_WSYNC, "," MNTOPT_WSYNC },
> { XFS_MOUNT_INO64, "," MNTOPT_INO64 },
> { XFS_MOUNT_NOALIGN, "," MNTOPT_NOALIGN },
> @@ -461,7 +462,6 @@ xfs_showargs(
> };
> static struct proc_xfs_info xfs_info_unset[] = {
> /* the few simple ones we can get from the mount struct */
> - { XFS_MOUNT_IDELETE, "," MNTOPT_IKEEP },
> { XFS_MOUNT_COMPAT_IOSIZE, "," MNTOPT_LARGEIO },
> { XFS_MOUNT_BARRIER, "," MNTOPT_NOBARRIER },
> { XFS_MOUNT_SMALL_INUMS, "," MNTOPT_64BITINODE },
> diff --git a/fs/xfs/xfs_clnt.h b/fs/xfs/xfs_clnt.h
> index d16c1b9..d5d1e60 100644
> --- a/fs/xfs/xfs_clnt.h
> +++ b/fs/xfs/xfs_clnt.h
> @@ -86,7 +86,7 @@ struct xfs_mount_args {
> #define XFSMNT_NOUUID 0x01000000 /* Ignore fs uuid */
> #define XFSMNT_DMAPI 0x02000000 /* enable dmapi/xdsm */
> #define XFSMNT_BARRIER 0x04000000 /* use write barriers */
> -#define XFSMNT_IDELETE 0x08000000 /* inode cluster delete */
> +#define XFSMNT_IKEEP 0x08000000 /* inode cluster delete */
> #define XFSMNT_SWALLOC 0x10000000 /* turn on stripe width
> * allocation */
> #define XFSMNT_DIRSYNC 0x40000000 /* sync creat,link,unlink,rename
> diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
> index 1409c2d..badf745 100644
> --- a/fs/xfs/xfs_ialloc.c
> +++ b/fs/xfs/xfs_ialloc.c
> @@ -1053,7 +1053,7 @@ xfs_difree(
> /*
> * When an inode cluster is free, it becomes eligible for removal
> */
> - if ((mp->m_flags & XFS_MOUNT_IDELETE) &&
> + if (!(mp->m_flags & XFS_MOUNT_IKEEP) &&
> (rec.ir_freecount == XFS_IALLOC_INODES(mp))) {
> *delete = 1;
> diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
> index 435d625..87ee8b8 100644
> --- a/fs/xfs/xfs_mount.h
> +++ b/fs/xfs/xfs_mount.h
> @@ -366,7 +366,7 @@ typedef struct xfs_mount {
> #define XFS_MOUNT_SMALL_INUMS (1ULL << 15) /* users wants 32bit inodes
> */
> #define XFS_MOUNT_NOUUID (1ULL << 16) /* ignore uuid during mount */
> #define XFS_MOUNT_BARRIER (1ULL << 17)
> -#define XFS_MOUNT_IDELETE (1ULL << 18) /* delete empty inode clusters*/
> +#define XFS_MOUNT_IKEEP (1ULL << 18) /* keep empty inode clusters*/
> #define XFS_MOUNT_SWALLOC (1ULL << 19) /* turn on stripe width
> * allocation */
> #define XFS_MOUNT_RDONLY (1ULL << 20) /* read-only fs */
> diff --git a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
> index a0f287e..e809b1c 100644
> --- a/fs/xfs/xfs_vfsops.c
> +++ b/fs/xfs/xfs_vfsops.c
> @@ -279,8 +279,8 @@ xfs_start_flags(
> mp->m_readio_log = mp->m_writeio_log = ap->iosizelog;
> }
> - if (ap->flags & XFSMNT_IDELETE)
> - mp->m_flags |= XFS_MOUNT_IDELETE;
> + if (ap->flags & XFSMNT_IKEEP)
> + mp->m_flags |= XFS_MOUNT_IKEEP;
> if (ap->flags & XFSMNT_DIRSYNC)
> mp->m_flags |= XFS_MOUNT_DIRSYNC;
> if (ap->flags & XFSMNT_ATTR2)
> diff --git a/fs/xfs/xfsidbg.c b/fs/xfs/xfsidbg.c
> index aa029da..a875351 100644
> --- a/fs/xfs/xfsidbg.c
> +++ b/fs/xfs/xfsidbg.c
> @@ -6282,7 +6282,7 @@ xfsidbg_xmount(xfs_mount_t *mp)
> "SMALL_INUMS", /* 0x8000 */
> "NOUUID", /* 0x10000 */
> "BARRIER", /* 0x20000 */
> - "IDELETE", /* 0x40000 */
> + "IKEEP", /* 0x40000 */
> "SWALLOC", /* 0x80000 */
> "RDONLY", /* 0x100000 */
> "DIRSYNC", /* 0x200000 */
^ permalink raw reply [flat|nested] 10+ messages in thread
* [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP
2008-02-18 5:33 ` Barry Naujok
@ 2008-02-18 6:26 ` Josef 'Jeff' Sipek
0 siblings, 0 replies; 10+ messages in thread
From: Josef 'Jeff' Sipek @ 2008-02-18 6:26 UTC (permalink / raw)
To: hch, sandeen, xfs, bnaujok; +Cc: Josef 'Jeff' Sipek
Change the *_IDELETE flags to *_IKEEP, and flip the logic as necessary.
This completely eliminates the no-no-no-idelete madness.
Signed-off-by: Josef 'Jeff' Sipek <jeffpc@josefsipek.net>
---
fs/xfs/linux-2.6/xfs_super.c | 14 +++++++-------
fs/xfs/xfs_clnt.h | 2 +-
fs/xfs/xfs_ialloc.c | 2 +-
fs/xfs/xfs_mount.h | 2 +-
fs/xfs/xfs_vfsops.c | 4 ++--
fs/xfs/xfsidbg.c | 2 +-
6 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/fs/xfs/linux-2.6/xfs_super.c b/fs/xfs/linux-2.6/xfs_super.c
index a0b1235..32b7f6e 100644
--- a/fs/xfs/linux-2.6/xfs_super.c
+++ b/fs/xfs/linux-2.6/xfs_super.c
@@ -171,7 +171,7 @@ xfs_parseargs(
char *this_char, *value, *eov;
int dsunit, dswidth, vol_dsunit, vol_dswidth;
int iosize;
- int ikeep = 0;
+ int dmapi_implies_ikeep = 1;
args->flags |= XFSMNT_BARRIER;
args->flags2 |= XFSMNT2_COMPAT_IOSIZE;
@@ -302,10 +302,10 @@ xfs_parseargs(
} else if (!strcmp(this_char, MNTOPT_NOBARRIER)) {
args->flags &= ~XFSMNT_BARRIER;
} else if (!strcmp(this_char, MNTOPT_IKEEP)) {
- ikeep = 1;
- args->flags &= ~XFSMNT_IDELETE;
+ args->flags |= XFSMNT_IKEEP;
} else if (!strcmp(this_char, MNTOPT_NOIKEEP)) {
- args->flags |= XFSMNT_IDELETE;
+ dmapi_implies_ikeep = 0;
+ args->flags &= ~XFSMNT_IKEEP;
} else if (!strcmp(this_char, MNTOPT_LARGEIO)) {
args->flags2 &= ~XFSMNT2_COMPAT_IOSIZE;
} else if (!strcmp(this_char, MNTOPT_NOLARGEIO)) {
@@ -410,8 +410,8 @@ xfs_parseargs(
* Note that if "ikeep" or "noikeep" mount options are
* supplied, then they are honored.
*/
- if (!(args->flags & XFSMNT_DMAPI) && !ikeep)
- args->flags |= XFSMNT_IDELETE;
+ if ((args->flags & XFSMNT_DMAPI) && dmapi_implies_ikeep)
+ args->flags |= XFSMNT_IKEEP;
if ((args->flags & XFSMNT_NOALIGN) != XFSMNT_NOALIGN) {
if (dsunit) {
@@ -446,6 +446,7 @@ xfs_showargs(
{
static struct proc_xfs_info xfs_info_set[] = {
/* the few simple ones we can get from the mount struct */
+ { XFS_MOUNT_IKEEP, "," MNTOPT_IKEEP },
{ XFS_MOUNT_WSYNC, "," MNTOPT_WSYNC },
{ XFS_MOUNT_INO64, "," MNTOPT_INO64 },
{ XFS_MOUNT_NOALIGN, "," MNTOPT_NOALIGN },
@@ -461,7 +462,6 @@ xfs_showargs(
};
static struct proc_xfs_info xfs_info_unset[] = {
/* the few simple ones we can get from the mount struct */
- { XFS_MOUNT_IDELETE, "," MNTOPT_IKEEP },
{ XFS_MOUNT_COMPAT_IOSIZE, "," MNTOPT_LARGEIO },
{ XFS_MOUNT_BARRIER, "," MNTOPT_NOBARRIER },
{ XFS_MOUNT_SMALL_INUMS, "," MNTOPT_64BITINODE },
diff --git a/fs/xfs/xfs_clnt.h b/fs/xfs/xfs_clnt.h
index d16c1b9..d5d1e60 100644
--- a/fs/xfs/xfs_clnt.h
+++ b/fs/xfs/xfs_clnt.h
@@ -86,7 +86,7 @@ struct xfs_mount_args {
#define XFSMNT_NOUUID 0x01000000 /* Ignore fs uuid */
#define XFSMNT_DMAPI 0x02000000 /* enable dmapi/xdsm */
#define XFSMNT_BARRIER 0x04000000 /* use write barriers */
-#define XFSMNT_IDELETE 0x08000000 /* inode cluster delete */
+#define XFSMNT_IKEEP 0x08000000 /* inode cluster delete */
#define XFSMNT_SWALLOC 0x10000000 /* turn on stripe width
* allocation */
#define XFSMNT_DIRSYNC 0x40000000 /* sync creat,link,unlink,rename
diff --git a/fs/xfs/xfs_ialloc.c b/fs/xfs/xfs_ialloc.c
index 1409c2d..badf745 100644
--- a/fs/xfs/xfs_ialloc.c
+++ b/fs/xfs/xfs_ialloc.c
@@ -1053,7 +1053,7 @@ xfs_difree(
/*
* When an inode cluster is free, it becomes eligible for removal
*/
- if ((mp->m_flags & XFS_MOUNT_IDELETE) &&
+ if (!(mp->m_flags & XFS_MOUNT_IKEEP) &&
(rec.ir_freecount == XFS_IALLOC_INODES(mp))) {
*delete = 1;
diff --git a/fs/xfs/xfs_mount.h b/fs/xfs/xfs_mount.h
index 435d625..87ee8b8 100644
--- a/fs/xfs/xfs_mount.h
+++ b/fs/xfs/xfs_mount.h
@@ -366,7 +366,7 @@ typedef struct xfs_mount {
#define XFS_MOUNT_SMALL_INUMS (1ULL << 15) /* users wants 32bit inodes */
#define XFS_MOUNT_NOUUID (1ULL << 16) /* ignore uuid during mount */
#define XFS_MOUNT_BARRIER (1ULL << 17)
-#define XFS_MOUNT_IDELETE (1ULL << 18) /* delete empty inode clusters*/
+#define XFS_MOUNT_IKEEP (1ULL << 18) /* keep empty inode clusters*/
#define XFS_MOUNT_SWALLOC (1ULL << 19) /* turn on stripe width
* allocation */
#define XFS_MOUNT_RDONLY (1ULL << 20) /* read-only fs */
diff --git a/fs/xfs/xfs_vfsops.c b/fs/xfs/xfs_vfsops.c
index a0f287e..e809b1c 100644
--- a/fs/xfs/xfs_vfsops.c
+++ b/fs/xfs/xfs_vfsops.c
@@ -279,8 +279,8 @@ xfs_start_flags(
mp->m_readio_log = mp->m_writeio_log = ap->iosizelog;
}
- if (ap->flags & XFSMNT_IDELETE)
- mp->m_flags |= XFS_MOUNT_IDELETE;
+ if (ap->flags & XFSMNT_IKEEP)
+ mp->m_flags |= XFS_MOUNT_IKEEP;
if (ap->flags & XFSMNT_DIRSYNC)
mp->m_flags |= XFS_MOUNT_DIRSYNC;
if (ap->flags & XFSMNT_ATTR2)
diff --git a/fs/xfs/xfsidbg.c b/fs/xfs/xfsidbg.c
index aa029da..a875351 100644
--- a/fs/xfs/xfsidbg.c
+++ b/fs/xfs/xfsidbg.c
@@ -6282,7 +6282,7 @@ xfsidbg_xmount(xfs_mount_t *mp)
"SMALL_INUMS", /* 0x8000 */
"NOUUID", /* 0x10000 */
"BARRIER", /* 0x20000 */
- "IDELETE", /* 0x40000 */
+ "IKEEP", /* 0x40000 */
"SWALLOC", /* 0x80000 */
"RDONLY", /* 0x100000 */
"DIRSYNC", /* 0x200000 */
--
1.5.4.rc2.85.g9de45-dirty
^ permalink raw reply related [flat|nested] 10+ messages in thread
* Re: [PATCH] fix mount option pasing to make inode cluster deletion default (again)
2008-02-14 3:34 [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
2008-02-14 7:45 ` [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP Josef 'Jeff' Sipek
@ 2008-02-18 14:38 ` Eric Sandeen
2008-02-20 23:39 ` Josef 'Jeff' Sipek
1 sibling, 1 reply; 10+ messages in thread
From: Eric Sandeen @ 2008-02-18 14:38 UTC (permalink / raw)
To: xfs-oss
Eric Sandeen wrote:
> mod xfs-linux-melb:xfs-kern:29683a /
> git commit 574342f4ad450b33bc85ec53210b8aa8bfff2fcf
>
> broke default options in such a way that empty inode clusters
> are no longer deleted by default, because if no options are
> given, we "goto done;" without setting the default
> XFSMNT_IDELETE flag.
>
> All this logic could probably be rearranged to make things
> clearer, but for now I think this small patch fixes it:
>
> Set IDELETE a.k.a. "noikeep" by default, but if dmapi is in
> use, turn it back off (i.e. "ikeep") *unless* noikeep was
> specifically requested.
>
> Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
While I like Jeff's patch in the long run... unless you are considering
submitting it for 2.6.25, might you consider this smaller patch for
2.6.25 to fix the inode reclamation problem in the short-term?
Thanks,
-Eric
> ---
>
>
> Index: linux-2.6.24/fs/xfs/linux-2.6/xfs_super.c
> ===================================================================
> --- linux-2.6.24.orig/fs/xfs/linux-2.6/xfs_super.c
> +++ linux-2.6.24/fs/xfs/linux-2.6/xfs_super.c
> @@ -171,9 +171,10 @@ xfs_parseargs(
> char *this_char, *value, *eov;
> int dsunit, dswidth, vol_dsunit, vol_dswidth;
> int iosize;
> - int ikeep = 0;
> + int noikeep = 0; /* track _explicit_ requests */
>
> args->flags |= XFSMNT_BARRIER;
> + args->flags |= XFSMNT_IDELETE; /* i.e. "noikeep" is default */
> args->flags2 |= XFSMNT2_COMPAT_IOSIZE;
>
> if (!options)
> @@ -302,9 +303,9 @@ xfs_parseargs(
> } else if (!strcmp(this_char, MNTOPT_NOBARRIER)) {
> args->flags &= ~XFSMNT_BARRIER;
> } else if (!strcmp(this_char, MNTOPT_IKEEP)) {
> - ikeep = 1;
> args->flags &= ~XFSMNT_IDELETE;
> } else if (!strcmp(this_char, MNTOPT_NOIKEEP)) {
> + noikeep = 1; /* explicitly requested */
> args->flags |= XFSMNT_IDELETE;
> } else if (!strcmp(this_char, MNTOPT_LARGEIO)) {
> args->flags2 &= ~XFSMNT2_COMPAT_IOSIZE;
> @@ -410,8 +411,8 @@ xfs_parseargs(
> * Note that if "ikeep" or "noikeep" mount options are
> * supplied, then they are honored.
> */
> - if (!(args->flags & XFSMNT_DMAPI) && !ikeep)
> - args->flags |= XFSMNT_IDELETE;
> + if ((args->flags & XFSMNT_DMAPI) && !noikeep)
> + args->flags &= ~XFSMNT_IDELETE;
>
> if ((args->flags & XFSMNT_NOALIGN) != XFSMNT_NOALIGN) {
> if (dsunit) {
>
>
^ permalink raw reply [flat|nested] 10+ messages in thread
* Re: [PATCH] fix mount option pasing to make inode cluster deletion default (again)
2008-02-18 14:38 ` [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
@ 2008-02-20 23:39 ` Josef 'Jeff' Sipek
0 siblings, 0 replies; 10+ messages in thread
From: Josef 'Jeff' Sipek @ 2008-02-20 23:39 UTC (permalink / raw)
To: Eric Sandeen; +Cc: xfs-oss
On Mon, Feb 18, 2008 at 08:38:28AM -0600, Eric Sandeen wrote:
> Eric Sandeen wrote:
> > mod xfs-linux-melb:xfs-kern:29683a /
> > git commit 574342f4ad450b33bc85ec53210b8aa8bfff2fcf
> >
> > broke default options in such a way that empty inode clusters
> > are no longer deleted by default, because if no options are
> > given, we "goto done;" without setting the default
> > XFSMNT_IDELETE flag.
> >
> > All this logic could probably be rearranged to make things
> > clearer, but for now I think this small patch fixes it:
> >
> > Set IDELETE a.k.a. "noikeep" by default, but if dmapi is in
> > use, turn it back off (i.e. "ikeep") *unless* noikeep was
> > specifically requested.
> >
> > Signed-off-by: Eric Sandeen <sandeen@sandeen.net>
>
> While I like Jeff's patch in the long run... unless you are considering
> submitting it for 2.6.25, might you consider this smaller patch for
> 2.6.25 to fix the inode reclamation problem in the short-term?
Yup. Either patch fixes a bug and so it should get sent out rather sooner
than later.
Josef 'Jeff' Sipek.
--
Si hoc legere scis nimium eruditionis habes.
^ permalink raw reply [flat|nested] 10+ messages in thread
end of thread, other threads:[~2008-02-20 23:39 UTC | newest]
Thread overview: 10+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2008-02-14 3:34 [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
2008-02-14 7:45 ` [PATCH 1/1] XFS: replace *_IDELETE with *_IKEEP Josef 'Jeff' Sipek
2008-02-15 16:19 ` Christoph Hellwig
2008-02-15 16:23 ` Eric Sandeen
2008-02-16 13:22 ` Christoph Hellwig
2008-02-17 21:30 ` Josef 'Jeff' Sipek
2008-02-18 5:33 ` Barry Naujok
2008-02-18 6:26 ` Josef 'Jeff' Sipek
2008-02-18 14:38 ` [PATCH] fix mount option pasing to make inode cluster deletion default (again) Eric Sandeen
2008-02-20 23:39 ` Josef 'Jeff' Sipek
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox