Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
From: Andrew Morton <[EMAIL PROTECTED]> Date: Tue, 19 Jun 2007 12:33:16 -0700 > Only x86_64 and ia64 are fixed. Would it be correct to assume that the > other CONFIG_COMPAT architectures also need to be fixed? Only platforms which compat to "i386" have the issue wrt. the alignment of "u64" types, which is "4" on i386 and "8" on the platforms that compat to them. This has been mentioned at least 3 times in this thread. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
Andrew Morton writes:
> On Mon, 18 Jun 2007 12:21:47 +0400
> Vasily Tarasov <[EMAIL PROTECTED]> wrote:
>
> > From: Vasily Tarasov <[EMAIL PROTECTED]>
> >
> > This patch should be applied after Arnd Bergmann's patch,
> > that intoduces new compat types:
> > http://lkml.org/lkml/2007/6/15/98
> >
> > OpenVZ Linux kernel team has discovered the problem
> > with 32bit quota tools working on 64bit architectures.
> > In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl()
> > with
> > the comment "sys_quotactl seems to be 32/64bit clean, enable it for 32bit"
> > However this isn't right. Look at if_dqblk structure:
> >
> > struct if_dqblk {
> > __u64 dqb_bhardlimit;
> > __u64 dqb_bsoftlimit;
> > __u64 dqb_curspace;
> > __u64 dqb_ihardlimit;
> > __u64 dqb_isoftlimit;
> > __u64 dqb_curinodes;
> > __u64 dqb_btime;
> > __u64 dqb_itime;
> > __u32 dqb_valid;
> > };
> >
> > For 32 bit quota tools sizeof(if_dqblk) == 0x44.
> > But for 64 bit kernel its size is 0x48, 'cause of alignment!
> > Thus we got a problem. Attached patch reintroduce sys32_quotactl()
> > function,
> > that handles this and related situations.
> >
> > Signed-off-by: Vasily Tarasov <[EMAIL PROTECTED]>
> >
> > ---
> >
> > In OpenVZ technology 32 bit Virtual Environments over
> > 64 bit OS are common, hence we have customers, that complains on this bad
> > quota
> > behaviour:
> >
> > # /usr/bin/quota
> > quota: error while getting quota from /dev/sda1 for 0: Success
> >
> > The reason is caused above.
>
> Only x86_64 and ia64 are fixed. Would it be correct to assume that the
> other CONFIG_COMPAT architectures also need to be fixed?
I complained about this very issue when a previous version of
this patch was submitted last week, and Arnd explained that
non-x86 doesn't have a problem here because alignof(u64) is the
same in 32- and 64-bit modes.
However, the fact that the patch description talks about 32-
and 64-bit machines _in_general_, while the patch clearly only
handles x86-32 on x86-64 and ia64, is itself a bug. A more
precise patch description and a better comment in the code
is in order, I think.
/Mikael
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Mon, 18 Jun 2007 12:21:47 +0400
Vasily Tarasov <[EMAIL PROTECTED]> wrote:
> From: Vasily Tarasov <[EMAIL PROTECTED]>
>
> This patch should be applied after Arnd Bergmann's patch,
> that intoduces new compat types:
> http://lkml.org/lkml/2007/6/15/98
>
> OpenVZ Linux kernel team has discovered the problem
> with 32bit quota tools working on 64bit architectures.
> In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl() with
> the comment "sys_quotactl seems to be 32/64bit clean, enable it for 32bit"
> However this isn't right. Look at if_dqblk structure:
>
> struct if_dqblk {
> __u64 dqb_bhardlimit;
> __u64 dqb_bsoftlimit;
> __u64 dqb_curspace;
> __u64 dqb_ihardlimit;
> __u64 dqb_isoftlimit;
> __u64 dqb_curinodes;
> __u64 dqb_btime;
> __u64 dqb_itime;
> __u32 dqb_valid;
> };
>
> For 32 bit quota tools sizeof(if_dqblk) == 0x44.
> But for 64 bit kernel its size is 0x48, 'cause of alignment!
> Thus we got a problem. Attached patch reintroduce sys32_quotactl() function,
> that handles this and related situations.
>
> Signed-off-by: Vasily Tarasov <[EMAIL PROTECTED]>
>
> ---
>
> In OpenVZ technology 32 bit Virtual Environments over
> 64 bit OS are common, hence we have customers, that complains on this bad
> quota
> behaviour:
>
> # /usr/bin/quota
> quota: error while getting quota from /dev/sda1 for 0: Success
>
> The reason is caused above.
Only x86_64 and ia64 are fixed. Would it be correct to assume that the
other CONFIG_COMPAT architectures also need to be fixed?
> --- linux-2.6.22-rc4-fixed/arch/x86_64/ia32/ia32entry.S.orig 2007-06-14
> 15:55:24.0 +0400
> +++ linux-2.6.22-rc4-fixed/arch/x86_64/ia32/ia32entry.S 2007-06-14
> 16:22:52.0 +0400
> @@ -526,7 +526,7 @@ ia32_sys_call_table:
> .quad sys_init_module
> .quad sys_delete_module
> .quad quiet_ni_syscall /* 130 get_kernel_syms */
> - .quad sys_quotactl
> + .quad sys32_quotactl
> .quad sys_getpgid
> .quad sys_fchdir
> .quad quiet_ni_syscall /* bdflush */
> --- linux-2.6.22-rc4-fixed/arch/ia64/ia32/ia32_entry.S.orig 2007-06-14
> 15:55:24.0 +0400
> +++ linux-2.6.22-rc4-fixed/arch/ia64/ia32/ia32_entry.S2007-06-14
> 16:22:52.0 +0400
> @@ -304,7 +304,7 @@ ia32_syscall_table:
> data8 sys_ni_syscall/* init_module */
> data8 sys_ni_syscall/* delete_module */
> data8 sys_ni_syscall/* get_kernel_syms */ /* 130 */
> - data8 sys_quotactl
> + data8 sys32_quotactl
> data8 sys_getpgid
> data8 sys_fchdir
> data8 sys_ni_syscall/* sys_bdflush */
> --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0
> +0400
> +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-18 12:00:56.0 +0400
> @@ -10,12 +10,14 @@
> #include
> #include
> #include
> +#include
> #include
> #include
> #include
> #include
> #include
> #include
> +#include
>
> /* Check validity of generic quotactl commands */
> static int generic_quotactl_valid(struct super_block *sb, int type, int cmd,
> qid_t id)
> @@ -384,3 +386,119 @@ asmlinkage long sys_quotactl(unsigned in
>
> return ret;
> }
> +
> +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64)
> +/*
> + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64)
> + * and is necessary due to alignment problems.
> + */
> +struct compat_if_dqblk {
> + compat_u64 dqb_bhardlimit;
> + compat_u64 dqb_bsoftlimit;
> + compat_u64 dqb_curspace;
> + compat_u64 dqb_ihardlimit;
> + compat_u64 dqb_isoftlimit;
> + compat_u64 dqb_curinodes;
> + compat_u64 dqb_btime;
> + compat_u64 dqb_itime;
> + compat_uint_t dqb_valid;
> +};
> +
> +/* XFS structures */
> +struct compat_fs_qfilestat {
> + compat_u64 dqb_bhardlimit;
> + compat_u64 qfs_nblks;
> + compat_uint_t qfs_nextents;
> +};
> +
> +struct compat_fs_quota_stat {
> + __s8qs_version;
> + __u16 qs_flags;
> + __s8qs_pad;
> + struct compat_fs_qfilestat qs_uquota;
> + struct compat_fs_qfilestat qs_gquota;
> + compat_uint_t qs_incoredqs;
> + compat_int_tqs_btimelimit;
> + compat_int_tqs_itimelimit;
> + compat_int_tqs_rtbtimelimit;
> + __u16 qs_bwarnlimit;
> + __u16 qs_iwarnlimit;
> +};
> +
> +asmlinkage long sys32_quotactl(unsigned int cmd, const char __user *special,
> + qid_t id, void __user *addr)
> +{
> + unsigned int cmds;
> + struct if_dqblk __user *dqblk;
> + struct compat_if_dqblk __user *compat_dqblk;
> + struct fs_quota_stat __user *fsqstat;
> + struct compat_fs_quota_stat __user *compat_fsqstat;
> + compat_uint_t data;
> + u16 xdata;
> + long ret;
> +
> + cmds = cmd >> SUBCMDSHIFT;
> +
> + switch (cmds) {
> + case
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Mon, 18 Jun 2007 12:21:47 +0400
Vasily Tarasov [EMAIL PROTECTED] wrote:
From: Vasily Tarasov [EMAIL PROTECTED]
This patch should be applied after Arnd Bergmann's patch,
that intoduces new compat types:
http://lkml.org/lkml/2007/6/15/98
OpenVZ Linux kernel team has discovered the problem
with 32bit quota tools working on 64bit architectures.
In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl() with
the comment sys_quotactl seems to be 32/64bit clean, enable it for 32bit
However this isn't right. Look at if_dqblk structure:
struct if_dqblk {
__u64 dqb_bhardlimit;
__u64 dqb_bsoftlimit;
__u64 dqb_curspace;
__u64 dqb_ihardlimit;
__u64 dqb_isoftlimit;
__u64 dqb_curinodes;
__u64 dqb_btime;
__u64 dqb_itime;
__u32 dqb_valid;
};
For 32 bit quota tools sizeof(if_dqblk) == 0x44.
But for 64 bit kernel its size is 0x48, 'cause of alignment!
Thus we got a problem. Attached patch reintroduce sys32_quotactl() function,
that handles this and related situations.
Signed-off-by: Vasily Tarasov [EMAIL PROTECTED]
---
In OpenVZ technology 32 bit Virtual Environments over
64 bit OS are common, hence we have customers, that complains on this bad
quota
behaviour:
# /usr/bin/quota
quota: error while getting quota from /dev/sda1 for 0: Success
The reason is caused above.
Only x86_64 and ia64 are fixed. Would it be correct to assume that the
other CONFIG_COMPAT architectures also need to be fixed?
--- linux-2.6.22-rc4-fixed/arch/x86_64/ia32/ia32entry.S.orig 2007-06-14
15:55:24.0 +0400
+++ linux-2.6.22-rc4-fixed/arch/x86_64/ia32/ia32entry.S 2007-06-14
16:22:52.0 +0400
@@ -526,7 +526,7 @@ ia32_sys_call_table:
.quad sys_init_module
.quad sys_delete_module
.quad quiet_ni_syscall /* 130 get_kernel_syms */
- .quad sys_quotactl
+ .quad sys32_quotactl
.quad sys_getpgid
.quad sys_fchdir
.quad quiet_ni_syscall /* bdflush */
--- linux-2.6.22-rc4-fixed/arch/ia64/ia32/ia32_entry.S.orig 2007-06-14
15:55:24.0 +0400
+++ linux-2.6.22-rc4-fixed/arch/ia64/ia32/ia32_entry.S2007-06-14
16:22:52.0 +0400
@@ -304,7 +304,7 @@ ia32_syscall_table:
data8 sys_ni_syscall/* init_module */
data8 sys_ni_syscall/* delete_module */
data8 sys_ni_syscall/* get_kernel_syms */ /* 130 */
- data8 sys_quotactl
+ data8 sys32_quotactl
data8 sys_getpgid
data8 sys_fchdir
data8 sys_ni_syscall/* sys_bdflush */
--- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0
+0400
+++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-18 12:00:56.0 +0400
@@ -10,12 +10,14 @@
#include linux/slab.h
#include asm/current.h
#include asm/uaccess.h
+#include asm/compat.h
#include linux/kernel.h
#include linux/security.h
#include linux/syscalls.h
#include linux/buffer_head.h
#include linux/capability.h
#include linux/quotaops.h
+#include linux/types.h
/* Check validity of generic quotactl commands */
static int generic_quotactl_valid(struct super_block *sb, int type, int cmd,
qid_t id)
@@ -384,3 +386,119 @@ asmlinkage long sys_quotactl(unsigned in
return ret;
}
+
+#if defined(CONFIG_X86_64) || defined(CONFIG_IA64)
+/*
+ * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64)
+ * and is necessary due to alignment problems.
+ */
+struct compat_if_dqblk {
+ compat_u64 dqb_bhardlimit;
+ compat_u64 dqb_bsoftlimit;
+ compat_u64 dqb_curspace;
+ compat_u64 dqb_ihardlimit;
+ compat_u64 dqb_isoftlimit;
+ compat_u64 dqb_curinodes;
+ compat_u64 dqb_btime;
+ compat_u64 dqb_itime;
+ compat_uint_t dqb_valid;
+};
+
+/* XFS structures */
+struct compat_fs_qfilestat {
+ compat_u64 dqb_bhardlimit;
+ compat_u64 qfs_nblks;
+ compat_uint_t qfs_nextents;
+};
+
+struct compat_fs_quota_stat {
+ __s8qs_version;
+ __u16 qs_flags;
+ __s8qs_pad;
+ struct compat_fs_qfilestat qs_uquota;
+ struct compat_fs_qfilestat qs_gquota;
+ compat_uint_t qs_incoredqs;
+ compat_int_tqs_btimelimit;
+ compat_int_tqs_itimelimit;
+ compat_int_tqs_rtbtimelimit;
+ __u16 qs_bwarnlimit;
+ __u16 qs_iwarnlimit;
+};
+
+asmlinkage long sys32_quotactl(unsigned int cmd, const char __user *special,
+ qid_t id, void __user *addr)
+{
+ unsigned int cmds;
+ struct if_dqblk __user *dqblk;
+ struct compat_if_dqblk __user *compat_dqblk;
+ struct fs_quota_stat __user *fsqstat;
+ struct compat_fs_quota_stat __user *compat_fsqstat;
+ compat_uint_t data;
+ u16 xdata;
+ long ret;
+
+ cmds = cmd SUBCMDSHIFT;
+
+ switch (cmds) {
+ case
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
Andrew Morton writes:
On Mon, 18 Jun 2007 12:21:47 +0400
Vasily Tarasov [EMAIL PROTECTED] wrote:
From: Vasily Tarasov [EMAIL PROTECTED]
This patch should be applied after Arnd Bergmann's patch,
that intoduces new compat types:
http://lkml.org/lkml/2007/6/15/98
OpenVZ Linux kernel team has discovered the problem
with 32bit quota tools working on 64bit architectures.
In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl()
with
the comment sys_quotactl seems to be 32/64bit clean, enable it for 32bit
However this isn't right. Look at if_dqblk structure:
struct if_dqblk {
__u64 dqb_bhardlimit;
__u64 dqb_bsoftlimit;
__u64 dqb_curspace;
__u64 dqb_ihardlimit;
__u64 dqb_isoftlimit;
__u64 dqb_curinodes;
__u64 dqb_btime;
__u64 dqb_itime;
__u32 dqb_valid;
};
For 32 bit quota tools sizeof(if_dqblk) == 0x44.
But for 64 bit kernel its size is 0x48, 'cause of alignment!
Thus we got a problem. Attached patch reintroduce sys32_quotactl()
function,
that handles this and related situations.
Signed-off-by: Vasily Tarasov [EMAIL PROTECTED]
---
In OpenVZ technology 32 bit Virtual Environments over
64 bit OS are common, hence we have customers, that complains on this bad
quota
behaviour:
# /usr/bin/quota
quota: error while getting quota from /dev/sda1 for 0: Success
The reason is caused above.
Only x86_64 and ia64 are fixed. Would it be correct to assume that the
other CONFIG_COMPAT architectures also need to be fixed?
I complained about this very issue when a previous version of
this patch was submitted last week, and Arnd explained that
non-x86 doesn't have a problem here because alignof(u64) is the
same in 32- and 64-bit modes.
However, the fact that the patch description talks about 32-
and 64-bit machines _in_general_, while the patch clearly only
handles x86-32 on x86-64 and ia64, is itself a bug. A more
precise patch description and a better comment in the code
is in order, I think.
/Mikael
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
From: Andrew Morton [EMAIL PROTECTED] Date: Tue, 19 Jun 2007 12:33:16 -0700 Only x86_64 and ia64 are fixed. Would it be correct to assume that the other CONFIG_COMPAT architectures also need to be fixed? Only platforms which compat to i386 have the issue wrt. the alignment of u64 types, which is 4 on i386 and 8 on the platforms that compat to them. This has been mentioned at least 3 times in this thread. - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 2007-06-15 at 17:24 +0200, Arnd Bergmann wrote:
> On Friday 15 June 2007, Vasily Tarasov wrote:
> > I just noticed that we can not avoid the addition of packed attribute.
> > Look, for example:
> >
> > struct if_dqblk {
> > __u64 dqb_bhardlimit;
> > __u64 dqb_bsoftlimit;
> > __u64 dqb_curspace;
> > __u64 dqb_ihardlimit;
> > __u64 dqb_isoftlimit;
> > __u64 dqb_curinodes;
> > __u64 dqb_btime;
> > __u64 dqb_itime;
> > __u32 dqb_valid;
> > };
> >
> > sizeof(if_dqblk) = 0x48
> > On 32 bit: 0x44
> >
> > If I replace __u64/__u32 with compat equivalents - it will not help!
> > alligned attribute can _only_ _increase_ the size of structure, but not
> > decrease it.
>
> No, the gcc documentation isn't quite clear there, see the discussion about
> compat_u64 and compat_s64 types. It actually does the right thing when
> you use 'typedef __u64 __attribute__((aligned(64))) compat_64', as my
> patch does.
>
> Arnd <><
>
Wow!... Thank you for the explanation.
I'll resend the patch soon.
Vasily
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 2007-06-15 at 17:24 +0200, Arnd Bergmann wrote:
On Friday 15 June 2007, Vasily Tarasov wrote:
I just noticed that we can not avoid the addition of packed attribute.
Look, for example:
struct if_dqblk {
__u64 dqb_bhardlimit;
__u64 dqb_bsoftlimit;
__u64 dqb_curspace;
__u64 dqb_ihardlimit;
__u64 dqb_isoftlimit;
__u64 dqb_curinodes;
__u64 dqb_btime;
__u64 dqb_itime;
__u32 dqb_valid;
};
sizeof(if_dqblk) = 0x48
On 32 bit: 0x44
If I replace __u64/__u32 with compat equivalents - it will not help!
alligned attribute can _only_ _increase_ the size of structure, but not
decrease it.
No, the gcc documentation isn't quite clear there, see the discussion about
compat_u64 and compat_s64 types. It actually does the right thing when
you use 'typedef __u64 __attribute__((aligned(64))) compat_64', as my
patch does.
Arnd
Wow!... Thank you for the explanation.
I'll resend the patch soon.
Vasily
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Friday 15 June 2007, Vasily Tarasov wrote:
> I just noticed that we can not avoid the addition of packed attribute.
> Look, for example:
>
> struct if_dqblk {
> __u64 dqb_bhardlimit;
> __u64 dqb_bsoftlimit;
> __u64 dqb_curspace;
> __u64 dqb_ihardlimit;
> __u64 dqb_isoftlimit;
> __u64 dqb_curinodes;
> __u64 dqb_btime;
> __u64 dqb_itime;
> __u32 dqb_valid;
> };
>
> sizeof(if_dqblk) = 0x48
> On 32 bit: 0x44
>
> If I replace __u64/__u32 with compat equivalents - it will not help!
> alligned attribute can _only_ _increase_ the size of structure, but not
> decrease it.
No, the gcc documentation isn't quite clear there, see the discussion about
compat_u64 and compat_s64 types. It actually does the right thing when
you use 'typedef __u64 __attribute__((aligned(64))) compat_64', as my
patch does.
Arnd <><
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 2007-06-15 at 12:43 +0200, Arnd Bergmann wrote:
> On Friday 15 June 2007, Mikael Pettersson wrote:
> > > --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14
> > > 15:55:26.0 +0400
> > > +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400
> > ...
> > > +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64)
> > > +/*
> > > + * This code works only for 32 bit quota tools over 64 bit OS (x86_64,
> > > ia64)
> > > + * and is necessary due to alignment problems.
> > > + */
> >
> > The #ifdef looks way too arch-specific. And isn't there a shared
> > compat.c module somewhere that this should go into?
> >
>
> Only x86_64 and ia64 have this particular problem, the other architectures,
> and hopefully all future 64 bit platforms with 32 bit user space use
> the same alignment rules in elf32 and elf64.
>
> Still, the patch should be converted to use the compat_u64 type and not
> add an 'attribute((packed))' so that you _can_ use the same code on all
> architectures. See my 'Introduce compat_u64 and compat_s64 types' patch
> that I just posted in another thread.
>
> Arnd <><
>
Hello,
I just noticed that we can not avoid the addition of packed attribute.
Look, for example:
struct if_dqblk {
__u64 dqb_bhardlimit;
__u64 dqb_bsoftlimit;
__u64 dqb_curspace;
__u64 dqb_ihardlimit;
__u64 dqb_isoftlimit;
__u64 dqb_curinodes;
__u64 dqb_btime;
__u64 dqb_itime;
__u32 dqb_valid;
};
sizeof(if_dqblk) = 0x48
On 32 bit: 0x44
If I replace __u64/__u32 with compat equivalents - it will not help!
alligned attribute can _only_ _increase_ the size of structure, but not
decrease it.
So we should use packed or just use the array of ints: int[2].
Vasily
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 15 Jun 2007 12:43:01 +0200, Arnd Bergmann wrote: > On Friday 15 June 2007, Mikael Pettersson wrote: > > > --- linux-2.6.22-rc4-fixed/fs/quota.c.orig=A0=A0=A0=A02007-06-14 15:55:= > 26.0 +0400 > > > +++ linux-2.6.22-rc4-fixed/fs/quota.c=A02007-06-14 19:50:13.0 += > 0400 > > ... > > > +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) > > > +/* > > > + * This code works only for 32 bit quota tools over 64 bit OS (x86_64,= > ia64) > > > + * and is necessary due to alignment problems. > > > + */ > >=20 > > The #ifdef looks way too arch-specific. And isn't there a shared > > compat.c module somewhere that this should go into? > >=20 > > Only x86_64 and ia64 have this particular problem, the other architectures, > and hopefully all future 64 bit platforms with 32 bit user space use > the same alignment rules in elf32 and elf64. Ah yes, alignof(u64) is the same in 32- and 64-bit modes on !x86, thus they don't have a problem here. Thanks for explaining that. I consider this is an essential piece of information that should be included in the patch. (In a comment in the code, not buried in some commit log.) /Mikael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 2007-06-15 at 12:43 +0200, Arnd Bergmann wrote: > On Friday 15 June 2007, Mikael Pettersson wrote: > > > --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 > > > 15:55:26.0 +0400 > > > +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 > > ... > > > +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) > > > +/* > > > + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, > > > ia64) > > > + * and is necessary due to alignment problems. > > > + */ > > > > The #ifdef looks way too arch-specific. And isn't there a shared > > compat.c module somewhere that this should go into? > > > > Only x86_64 and ia64 have this particular problem, the other architectures, > and hopefully all future 64 bit platforms with 32 bit user space use > the same alignment rules in elf32 and elf64. > > Still, the patch should be converted to use the compat_u64 type and not > add an 'attribute((packed))' so that you _can_ use the same code on all > architectures. See my 'Introduce compat_u64 and compat_s64 types' patch > that I just posted in another thread. > > Arnd <>< > Agree, it'll be the most clean solution. Is it ok, if I'll send a patch against (current kernel + Arnd's patch)? Thanks, Vasily - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Friday 15 June 2007, Mikael Pettersson wrote: > > --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0 > > +0400 > > +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 > ... > > +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) > > +/* > > + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, > > ia64) > > + * and is necessary due to alignment problems. > > + */ > > The #ifdef looks way too arch-specific. And isn't there a shared > compat.c module somewhere that this should go into? > Only x86_64 and ia64 have this particular problem, the other architectures, and hopefully all future 64 bit platforms with 32 bit user space use the same alignment rules in elf32 and elf64. Still, the patch should be converted to use the compat_u64 type and not add an 'attribute((packed))' so that you _can_ use the same code on all architectures. See my 'Introduce compat_u64 and compat_s64 types' patch that I just posted in another thread. Arnd <>< - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
You can find a half-year back discussions of this patch: First attempt: http://lkml.org/lkml/2006/10/19/123 Second attempt: http://lkml.org/lkml/2006/10/25/57 I think they will answer your questions. Thank you, Vasily On Fri, 2007-06-15 at 12:03 +0200, Mikael Pettersson wrote: > On Fri, 15 Jun 2007 13:01:48 +0400, Vasily Tarasov wrote: > > OpenVZ Linux kernel team has discovered the problem > > with 32bit quota tools working on 64bit architectures. > > In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl() > > with > > the comment "sys_quotactl seems to be 32/64bit clean, enable it for 32bit" > > However this isn't right. Look at if_dqblk structure: > > Your patch only converts ia32 on x86-64 or ia64. > What about ppc32-on-ppc64 and sparc32-on-sparc64? > And, I guess, mips32-on-mips64? > > > --- linux-2.6.22-rc4-fixed/fs/quota.c.orig 2007-06-14 15:55:26.0 > > +0400 > > +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 > > +0400 > ... > > +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) > > +/* > > + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, > > ia64) > > + * and is necessary due to alignment problems. > > + */ > > The #ifdef looks way too arch-specific. And isn't there a shared > compat.c module somewhere that this should go into? > > /Mikael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 15 Jun 2007 13:01:48 +0400, Vasily Tarasov wrote: > OpenVZ Linux kernel team has discovered the problem > with 32bit quota tools working on 64bit architectures. > In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl() with > the comment "sys_quotactl seems to be 32/64bit clean, enable it for 32bit" > However this isn't right. Look at if_dqblk structure: Your patch only converts ia32 on x86-64 or ia64. What about ppc32-on-ppc64 and sparc32-on-sparc64? And, I guess, mips32-on-mips64? > --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0 > +0400 > +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 ... > +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) > +/* > + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64) > + * and is necessary due to alignment problems. > + */ The #ifdef looks way too arch-specific. And isn't there a shared compat.c module somewhere that this should go into? /Mikael - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 15 Jun 2007 13:01:48 +0400, Vasily Tarasov wrote: OpenVZ Linux kernel team has discovered the problem with 32bit quota tools working on 64bit architectures. In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl() with the comment sys_quotactl seems to be 32/64bit clean, enable it for 32bit However this isn't right. Look at if_dqblk structure: Your patch only converts ia32 on x86-64 or ia64. What about ppc32-on-ppc64 and sparc32-on-sparc64? And, I guess, mips32-on-mips64? --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0 +0400 +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 ... +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) +/* + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64) + * and is necessary due to alignment problems. + */ The #ifdef looks way too arch-specific. And isn't there a shared compat.c module somewhere that this should go into? /Mikael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
You can find a half-year back discussions of this patch: First attempt: http://lkml.org/lkml/2006/10/19/123 Second attempt: http://lkml.org/lkml/2006/10/25/57 I think they will answer your questions. Thank you, Vasily On Fri, 2007-06-15 at 12:03 +0200, Mikael Pettersson wrote: On Fri, 15 Jun 2007 13:01:48 +0400, Vasily Tarasov wrote: OpenVZ Linux kernel team has discovered the problem with 32bit quota tools working on 64bit architectures. In 2.6.10 kernel sys32_quotactl() function was replaced by sys_quotactl() with the comment sys_quotactl seems to be 32/64bit clean, enable it for 32bit However this isn't right. Look at if_dqblk structure: Your patch only converts ia32 on x86-64 or ia64. What about ppc32-on-ppc64 and sparc32-on-sparc64? And, I guess, mips32-on-mips64? --- linux-2.6.22-rc4-fixed/fs/quota.c.orig 2007-06-14 15:55:26.0 +0400 +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 ... +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) +/* + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64) + * and is necessary due to alignment problems. + */ The #ifdef looks way too arch-specific. And isn't there a shared compat.c module somewhere that this should go into? /Mikael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Friday 15 June 2007, Mikael Pettersson wrote: --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0 +0400 +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 ... +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) +/* + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64) + * and is necessary due to alignment problems. + */ The #ifdef looks way too arch-specific. And isn't there a shared compat.c module somewhere that this should go into? Only x86_64 and ia64 have this particular problem, the other architectures, and hopefully all future 64 bit platforms with 32 bit user space use the same alignment rules in elf32 and elf64. Still, the patch should be converted to use the compat_u64 type and not add an 'attribute((packed))' so that you _can_ use the same code on all architectures. See my 'Introduce compat_u64 and compat_s64 types' patch that I just posted in another thread. Arnd - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 2007-06-15 at 12:43 +0200, Arnd Bergmann wrote: On Friday 15 June 2007, Mikael Pettersson wrote: --- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14 15:55:26.0 +0400 +++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400 ... +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) +/* + * This code works only for 32 bit quota tools over 64 bit OS (x86_64, ia64) + * and is necessary due to alignment problems. + */ The #ifdef looks way too arch-specific. And isn't there a shared compat.c module somewhere that this should go into? Only x86_64 and ia64 have this particular problem, the other architectures, and hopefully all future 64 bit platforms with 32 bit user space use the same alignment rules in elf32 and elf64. Still, the patch should be converted to use the compat_u64 type and not add an 'attribute((packed))' so that you _can_ use the same code on all architectures. See my 'Introduce compat_u64 and compat_s64 types' patch that I just posted in another thread. Arnd Agree, it'll be the most clean solution. Is it ok, if I'll send a patch against (current kernel + Arnd's patch)? Thanks, Vasily - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 15 Jun 2007 12:43:01 +0200, Arnd Bergmann wrote: On Friday 15 June 2007, Mikael Pettersson wrote: --- linux-2.6.22-rc4-fixed/fs/quota.c.orig=A0=A0=A0=A02007-06-14 15:55:= 26.0 +0400 +++ linux-2.6.22-rc4-fixed/fs/quota.c=A02007-06-14 19:50:13.0 += 0400 ... +#if defined(CONFIG_X86_64) || defined(CONFIG_IA64) +/* + * This code works only for 32 bit quota tools over 64 bit OS (x86_64,= ia64) + * and is necessary due to alignment problems. + */ =20 The #ifdef looks way too arch-specific. And isn't there a shared compat.c module somewhere that this should go into? =20 Only x86_64 and ia64 have this particular problem, the other architectures, and hopefully all future 64 bit platforms with 32 bit user space use the same alignment rules in elf32 and elf64. Ah yes, alignof(u64) is the same in 32- and 64-bit modes on !x86, thus they don't have a problem here. Thanks for explaining that. I consider this is an essential piece of information that should be included in the patch. (In a comment in the code, not buried in some commit log.) /Mikael - To unsubscribe from this list: send the line unsubscribe linux-kernel in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Fri, 2007-06-15 at 12:43 +0200, Arnd Bergmann wrote:
On Friday 15 June 2007, Mikael Pettersson wrote:
--- linux-2.6.22-rc4-fixed/fs/quota.c.orig2007-06-14
15:55:26.0 +0400
+++ linux-2.6.22-rc4-fixed/fs/quota.c 2007-06-14 19:50:13.0 +0400
...
+#if defined(CONFIG_X86_64) || defined(CONFIG_IA64)
+/*
+ * This code works only for 32 bit quota tools over 64 bit OS (x86_64,
ia64)
+ * and is necessary due to alignment problems.
+ */
The #ifdef looks way too arch-specific. And isn't there a shared
compat.c module somewhere that this should go into?
Only x86_64 and ia64 have this particular problem, the other architectures,
and hopefully all future 64 bit platforms with 32 bit user space use
the same alignment rules in elf32 and elf64.
Still, the patch should be converted to use the compat_u64 type and not
add an 'attribute((packed))' so that you _can_ use the same code on all
architectures. See my 'Introduce compat_u64 and compat_s64 types' patch
that I just posted in another thread.
Arnd
Hello,
I just noticed that we can not avoid the addition of packed attribute.
Look, for example:
struct if_dqblk {
__u64 dqb_bhardlimit;
__u64 dqb_bsoftlimit;
__u64 dqb_curspace;
__u64 dqb_ihardlimit;
__u64 dqb_isoftlimit;
__u64 dqb_curinodes;
__u64 dqb_btime;
__u64 dqb_itime;
__u32 dqb_valid;
};
sizeof(if_dqblk) = 0x48
On 32 bit: 0x44
If I replace __u64/__u32 with compat equivalents - it will not help!
alligned attribute can _only_ _increase_ the size of structure, but not
decrease it.
So we should use packed or just use the array of ints: int[2].
Vasily
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Re: [PATCH] diskquota: 32bit quota tools on 64bit architectures
On Friday 15 June 2007, Vasily Tarasov wrote:
I just noticed that we can not avoid the addition of packed attribute.
Look, for example:
struct if_dqblk {
__u64 dqb_bhardlimit;
__u64 dqb_bsoftlimit;
__u64 dqb_curspace;
__u64 dqb_ihardlimit;
__u64 dqb_isoftlimit;
__u64 dqb_curinodes;
__u64 dqb_btime;
__u64 dqb_itime;
__u32 dqb_valid;
};
sizeof(if_dqblk) = 0x48
On 32 bit: 0x44
If I replace __u64/__u32 with compat equivalents - it will not help!
alligned attribute can _only_ _increase_ the size of structure, but not
decrease it.
No, the gcc documentation isn't quite clear there, see the discussion about
compat_u64 and compat_s64 types. It actually does the right thing when
you use 'typedef __u64 __attribute__((aligned(64))) compat_64', as my
patch does.
Arnd
-
To unsubscribe from this list: send the line unsubscribe linux-kernel in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
