Re: [PATCH] Fix ext4 bitops

2008-02-04 Thread Heiko Carstens
> > > > | fs/ext4/mballoc.c: In function 'ext4_mb_generate_buddy':
> > > > | fs/ext4/mballoc.c:954: error: implicit declaration of function 
> > > > 'generic_find_next_le_bit'
> > > > 
> > > > The s390 specific bitops uses parts of the generic implementation.
> > > > Include the correct header.
> > > 
> > > That doesn't work:
> > > 
> > > fs/built-in.o: In function `ext4_mb_release_inode_pa':
> > > mballoc.c:(.text+0x95a8a): undefined reference to 
> > > `generic_find_next_le_bit'
> > > fs/built-in.o: In function `ext4_mb_init_cache':
> > > mballoc.c:(.text+0x967ea): undefined reference to 
> > > `generic_find_next_le_bit'
> > > 
> > > This still needs generic_find_next_le_bit which comes
> > > from lib/find_next_bit.c. That one doesn't get built on s390 since we
> > > don't set GENERIC_FIND_NEXT_BIT.
> > > Currently we have the lengthly patch below queued.
> > 
> > Similar issue on m68k. As Bastian also saw it on powerpc, I'm getting the
> > impression the ext4 people don't (compile) test on big endian machines?
> > 
> > Gr{oetje,eeting}s,
> > 
> 
> I have sent this patches to linux-arch expecting a review from
> different arch people. It is true that the patches are tested only on
> powerpc, x86-64, x86. That's the primary reason of me sending the
> patches to linux-arch.

Is there anything special I need to do so the ext4 code actually uses
ext2_find_next_bit() ? Haven't looked at the ext4 code, but I'd like to
test if the s390 implementation is ok.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/7] fallocate() implementation in i386, x86_64 and powerpc

2007-07-10 Thread Heiko Carstens
On Wed, Jul 11, 2007 at 12:10:34PM +1000, Stephen Rothwell wrote:
> On Wed, 11 Jul 2007 01:50:00 +0530 "Amit K. Arora" <[EMAIL PROTECTED]> wrote:
> >
> > --- linux-2.6.22.orig/arch/x86_64/ia32/sys_ia32.c
> > +++ linux-2.6.22/arch/x86_64/ia32/sys_ia32.c
> > @@ -879,3 +879,11 @@ asmlinkage long sys32_fadvise64(int fd, 
> > return sys_fadvise64_64(fd, ((u64)offset_hi << 32) | offset_lo,
> > len, advice);
> >  }
> > +
> > +asmlinkage long sys32_fallocate(int fd, int mode, unsigned offset_lo,
> > +   unsigned offset_hi, unsigned len_lo,
> > +   unsigned len_hi)
> 
> Please call this compat_sys_fallocate in line with the powerpc version -
> it gives us a hint that maybe we should think about how to consolidate
> them.  I know other stuff in that file is called sys32_ ... but it is time
> for a change :-)

Maybe it would be worth to finally consider this:

http://marc.info/?l=linux-kernel&m=118411511620432&w=2

?
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 1/7][TAKE5] fallocate() implementation on i386, x86_64 and powerpc

2007-06-26 Thread Heiko Carstens
> Index: linux-2.6.22-rc4/arch/powerpc/kernel/sys_ppc32.c
> ===
> --- linux-2.6.22-rc4.orig/arch/powerpc/kernel/sys_ppc32.c
> +++ linux-2.6.22-rc4/arch/powerpc/kernel/sys_ppc32.c
> @@ -773,6 +773,13 @@ asmlinkage int compat_sys_truncate64(con
>   return sys_truncate(path, (high << 32) | low);
>  }
> 
> +asmlinkage long compat_sys_fallocate(int fd, int mode, u32 offhi, u32 offlo,
> +  u32 lenhi, u32 lenlo)
> +{
> + return sys_fallocate(fd, mode, ((loff_t)offhi << 32) | offlo,
> +  ((loff_t)lenhi << 32) | lenlo);
> +}
> +
>  asmlinkage int compat_sys_ftruncate64(unsigned int fd, u32 reg4, unsigned 
> long high,
>unsigned long low)
>  {
> Index: linux-2.6.22-rc4/arch/x86_64/ia32/ia32entry.S
> ===
> --- linux-2.6.22-rc4.orig/arch/x86_64/ia32/ia32entry.S
> +++ linux-2.6.22-rc4/arch/x86_64/ia32/ia32entry.S
> @@ -719,4 +719,5 @@ ia32_sys_call_table:
>   .quad compat_sys_signalfd
>   .quad compat_sys_timerfd
>   .quad sys_eventfd
> + .quad sys_fallocate
>  ia32_syscall_end:

Btw. this is also (still?) broken. x86_64 needs a compat syscall here.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/7][TAKE5] fallocate() on s390(x)

2007-06-26 Thread Heiko Carstens
> Index: linux-2.6.22-rc4/arch/s390/kernel/syscalls.S
> ===
> --- linux-2.6.22-rc4.orig/arch/s390/kernel/syscalls.S 2007-06-11 
> 16:16:01.0 -0700
> +++ linux-2.6.22-rc4/arch/s390/kernel/syscalls.S  2007-06-11 
> 16:27:29.0 -0700
> @@ -322,6 +322,7 @@
>  SYSCALL(sys_getcpu,sys_getcpu,sys_getcpu_wrapper)
>  SYSCALL(sys_epoll_pwait,sys_epoll_pwait,compat_sys_epoll_pwait_wrapper)
>  SYSCALL(sys_utimes,sys_utimes,compat_sys_utimes_wrapper)
> +SYSCALL(s390_fallocate,sys_fallocate,sys_fallocate_wrapper)
>  NI_SYSCALL   /* 314 
> sys_fallocate */

You need to remove the NI_SYSCALL line. Otherwise all following entries
will be wrong.

>  SYSCALL(sys_utimensat,sys_utimensat,compat_sys_utimensat_wrapper)/* 315 
> */
>  SYSCALL(sys_signalfd,sys_signalfd,compat_sys_signalfd_wrapper)
> Index: linux-2.6.22-rc4/include/asm-s390/unistd.h
> ===
> --- linux-2.6.22-rc4.orig/include/asm-s390/unistd.h   2007-06-11 
> 16:16:01.0 -0700
> +++ linux-2.6.22-rc4/include/asm-s390/unistd.h2007-06-11 
> 16:27:29.0 -0700
> @@ -256,7 +256,8 @@
>  #define __NR_signalfd316
>  #define __NR_timerfd 317
>  #define __NR_eventfd 318
> -#define NR_syscalls 319
> +#define __NR_fallocate   319
> +#define NR_syscalls 320

Erm... no. You use slot 314 in the syscall table but assign number 319.
That won't work. Please use 314 for both.
I assume this got broken when updating to newer kernel versions.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/5] fallocate system call

2007-04-27 Thread Heiko Carstens
On Fri, Apr 27, 2007 at 04:43:28PM +0200, Jörn Engel wrote:
> On Fri, 27 April 2007 14:10:03 +0200, Heiko Carstens wrote:
> > 
> > After long discussions where at least two possible implementations
> > were suggested that would work on _all_ architectures you chose one
> > which doesn't and causes extra effort.
> 
> I believe the long discussion also showed that every possible
> implementation has drawbacks.  To me this one appeared to be the best of
> many bad choices.

If one insists to have fd at first argument, what is wrong with having
u32 arguments only? It's not that this syscall comes even close to
what can be considered performance critical...

> Is this implementation worse than we thought?

It adds userspace overhead for one architecture. Every *trace and
*libc needs special handling on s390 for this syscall. I would
prefer to avoid this.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 0/5] fallocate system call

2007-04-27 Thread Heiko Carstens
On Thu, Apr 26, 2007 at 11:20:56PM +0530, Amit K. Arora wrote:
> Based on the discussion, this new patchset uses following as the
> interface for fallocate() system call:
> 
>  asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t len)
> 
> It seems that only s390 architecture has a problem with such a layout of
> arguments in fallocate(). Thus for s390, we plan to have a wrapper
> (say, sys_s390_fallocate()) for the sys_fallocate(), which will get
> called by glibc when an application issues a fallocate() system call
> on s390. The s390 arch specific changes will be part of a separate
> patch (PATCH 2/5). It will be great if some s390 expert can verify the
> patch, since I have not been able to test it on s390 so far.

After long discussions where at least two possible implementations
were suggested that would work on _all_ architectures you chose one
which doesn't and causes extra effort.

> It was also noted that minor changes might be required to strace code
> to take care of "different arguments on s390" issue.

This is not limited to strace...

Besides that the s390 backend looks ok.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Interface for the new fallocate() system call

2007-03-30 Thread Heiko Carstens
On Fri, Mar 30, 2007 at 12:44:49PM +0200, Jörn Engel wrote:
> On Fri, 30 March 2007 19:15:58 +1000, Paul Mackerras wrote:
> > It does mean extra unnecessary work for 64-bit platforms, though...
> 
> Wouldn't that work be confined to fallocate()?  If I understand Heiko
> correctly, the alternative would slow s390 down for every syscall,
> including more performance-critical ones.

That is correct.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Interface for the new fallocate() system call

2007-03-30 Thread Heiko Carstens
On Fri, Mar 30, 2007 at 02:14:17AM -0500, Jakub Jelinek wrote:
> On Thu, Mar 29, 2007 at 10:10:10AM -0700, Andrew Morton wrote:
> > > Platform: s390
> > > --
> > > s390 prefers following layout:
> > > 
> > >int fallocate(int fd, loff_t offset, loff_t len, int mode)
> > > 
> > > For details on why and how "int, int, loff_t, loff_t" is a problem on
> > > s390, please see Heiko's mail on 16th March. Here is the link:
> > > http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg133595.html
> > > 
> > > Platform: ppc, arm
> > > --
> > > ppc (32 bit) has a problem with "int, loff_t, loff_t, int" layout,
> > > since this will result in a pad between fd and offset, making seven
> > > arguments total - which is not supported by ppc32. It supports only
> > > 6 arguments. Thus the desired layout by ppc32 is:
> > > 
> > >int fallocate(int fd, int mode, loff_t offset, loff_t len)
> > > 
> > > Even ARM prefers above kind of layout. For details please see the
> > > definition of sys_arm_sync_file_range().
> > 
> > This is a clean-looking option.  Can s390 be changed to support seven-arg
> > syscalls?
> 
> Wouldn't
> int fallocate(loff_t offset, loff_t len, int fd, int mode)
> work on both s390 and ppc/arm?  glibc will certainly wrap it and
> reorder the arguments as needed, so there is no need to keep fd first.

That would be fine for s390.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Interface for the new fallocate() system call

2007-03-29 Thread Heiko Carstens
> > Even ARM prefers above kind of layout. For details please see the
> > definition of sys_arm_sync_file_range().
> 
> This is a clean-looking option.  Can s390 be changed to support seven-arg
> syscalls?
> 
> > Option of loff_t => high u32 + low u32
> > --
> > Matthew and Russell have suggested another option of breaking each
> > "loff_t" into two "u32"s. This will result in 6 arguments in total.
> > 
> > Following think that this is a good alternative:
> > Matthew Wilcox, Russell King, Heiko Carstens
> > 
> > Following do not like this idea:
> > Chris Wedgwood
> 
> It's a bit weird-looking, but the six-32-bit-args approach is simple
> enought to understand and implement.  Presumably the glibc wrapper
> would hide that detail from everyone.

s390 can be changed to support seven-arg syscalls. But that would require
creating an additional stackframe in *libc to save original register
contents and in addition it would make our syscall hotpath slower.
That is because we have to take care of an additional register that might
contain user space passed contents and needs to be put on the kernel stack.
If possible I'd prefer the six-32-bit-args approach.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: Interface for the new fallocate() system call

2007-03-29 Thread Heiko Carstens
On Thu, Mar 29, 2007 at 07:01:54PM +0200, Jan Engelhardt wrote:
> Hi,
> 
> On Mar 29 2007 17:21, Amit K. Arora wrote:
> >
> >We need to come up with the best possible layout of arguments for the
> >fallocate() system call. Various architectures have different
> >requirements for how the arguments should look like. Since the mail
> >chain has become huge, here is the summary of various inputs received
> >so far.
> 
> >s390 prefers following layout:
> >   int fallocate(int fd, loff_t offset, loff_t len, int mode)
> >For details on why and how "int, int, loff_t, loff_t" is a problem on
> >s390, please see Heiko's mail on 16th March. Here is the link:
> >http://www.mail-archive.com/linux-kernel@vger.kernel.org/msg133595.html
> 
> Quoting that...
>   |len -> r6 + second halve on stack
> 
> Then, is not this a gcc glitch? (IMO, it should put all of "len" on the 
> stack)

It _does_ put all of "len" on the stack. That is what I tried to explain
in the section that follows what you quoted.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] sys_fallocate() system call

2007-03-19 Thread Heiko Carstens
On Mon, Mar 19, 2007 at 02:54:04PM +0530, Amit K. Arora wrote:
> On Fri, Mar 16, 2007 at 04:21:03PM +0100, Heiko Carstens wrote:
> > On Fri, Mar 16, 2007 at 08:01:01PM +0530, Amit K. Arora wrote:
> > >  asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t 
> > > len)
> > > 
> > > Currently we have two modes FA_ALLOCATE and FA_DEALLOCATE, for
> > > preallocation and deallocation of preallocated blocks respectively. More
> > > modes can be added, when required. And these modes can be renamed, since
> > > I am sure these are no way the best ones ! :)
> > > 
> Yes, the problem was adding compat wrapper for this. I will appreciate
> your help in writing it. Only thing is that we might have to wait till
> the order of the arguments is decided upon. Thanks!

There is probably not much choice. If you want to stay with the loff_t
arguments it won't work on 31-bit s390 or 32-bit powerpc dependent on the
order of the arguments.
So you should go for what Matthew Wilcox suggested:

asmlinkage long sys_fallocate(int fd, int mode, u32 off_low, u32 off_high,
  u32 len_low, u32 len_high);

That way it will work an all architectures and in addition no architecture
has to do some magic to combine the splitted 64 bit arguments in compat
mode.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] sys_fallocate() system call

2007-03-17 Thread Heiko Carstens
On Sat, Mar 17, 2007 at 05:07:06AM -0600, Matthew Wilcox wrote:
> On Sat, Mar 17, 2007 at 08:59:05PM +1100, Paul Mackerras wrote:
> > ... but wouldn't work on 32-bit powerpc. :(  We would end up with a
> > pad argument between fd and offset, giving 7 arguments in all
> > (counting the loff_t's as 2), but we only support 6.
> 
> Ditto mips and parisc.

Can't be. Or: mips supports 7 arguments and parisc doesn't pad.
Otherwise they couldn't have wired up

sys_sync_file_range(int fd, loff_t offset, loff_t nbytes, unsigned int flags)

But from what I read, it's currently not possible for 32-bit powerpc to
wire up the already present sync_file_range system call.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] sys_fallocate() system call

2007-03-16 Thread Heiko Carstens
> on s390, and thus the delay. While I try to get it right on s390(x), we
> thought of posting this patch, so that we can save some time. Parallely
> we will work on getting the patch work on s390, and probably it will
> come as a separate patch.
> 
> +asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t len)
> +{

There is something here that will not work on s390 (31bit): the arguments
would end up in:
fd -> r2
mode -> r3
offset -> r4 + r5
len -> r6 + second halve on stack

But the s390 ABI says that a long long will be put into two consecutive
registers if the first register is smaller than 6, or it will be put
completely on the stack. So both 32 bit parts of len will end up on the
stack. That would make it a syscall with seven arguments which we currently
don't support on s390. There is no way to access the second half of len
from kernel space and that is why it is not working for you.
So you either rearrange the parameters or convert the loff_t's to pointers.

e.g.

asmlinkage long sys_fallocate(int fd, loff_t offset, loff_t len, int mode)

would work even on s390 ;)
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [RFC][PATCH] sys_fallocate() system call

2007-03-16 Thread Heiko Carstens
On Fri, Mar 16, 2007 at 08:01:01PM +0530, Amit K. Arora wrote:
> First of all, thanks for the overwhelming response!
> 
> Based on the suggestions received, I have added a new parameter to the
> sys_fallocate() system call - an interger called "mode", just after the
> "fd". Now the system call looks like this:
> 
>  asmlinkage long sys_fallocate(int fd, int mode, loff_t offset, loff_t len)
> 
> Currently we have two modes FA_ALLOCATE and FA_DEALLOCATE, for
> preallocation and deallocation of preallocated blocks respectively. More
> modes can be added, when required. And these modes can be renamed, since
> I am sure these are no way the best ones ! :)
> 
> Attached below is the patch which implements this system call. It has
> been currently implemented and tested on i386, ppc64 and x86_64
> architectures. I am facing some problems while trying to implement this
> on s390, and thus the delay. While I try to get it right on s390(x), we
> thought of posting this patch, so that we can save some time. Parallely
> we will work on getting the patch work on s390, and probably it will
> come as a separate patch.

What's the problem you face on s390? If it's just the compat wrapper, you
may look at sys_sync_file_range_wrapper. Or I will send a patch if needed.
-
To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at  http://vger.kernel.org/majordomo-info.html