uClibc declares posix_fadvise() even when the architecture does not provide
one. The static posix_fadvise() signature is not compatible with POSIX. Rename
the internal implementation to fix this. Fixes the following build failure
when building against uClibc:
e4defrag.c:189:2: warning: #warning Using locally defined posix_fadvise interface. [-Wcpp]
e4defrag.c:203:12: error: conflicting types for ‘posix_fadvise’
Signed-off-by: Baruch Siach <[email protected]>
---
misc/e4defrag.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/misc/e4defrag.c b/misc/e4defrag.c
index c6a5f0d..4e84a74 100644
--- a/misc/e4defrag.c
+++ b/misc/e4defrag.c
@@ -200,7 +200,8 @@ static struct frag_statistic_ino frag_rank[SHOW_FRAG_FILES];
* @len: area length.
* @advise: process flag.
*/
-static int posix_fadvise(int fd, loff_t offset, size_t len, int advise)
+#define posix_fadvise __posix_fadvise
+static int __posix_fadvise(int fd, loff_t offset, size_t len, int advise)
{
return syscall(__NR_fadvise64_64, fd, offset, len, advise);
}
--
1.8.5.2
On Wed, Jan 01, 2014 at 12:28:23PM +0200, Baruch Siach wrote:
> uClibc declares posix_fadvise() even when the architecture does not provide
> one. The static posix_fadvise() signature is not compatible with POSIX. Rename
> the internal implementation to fix this.
If the architecture doesn't provide posix_fadvise(), does that imply
that __NR_fadvise64_64 also doesn't exist?
Or do you mean that for some reason, uClibc is not providing
posix_fadvise on all architectures, even though the kernel supports it?
That seems wierd.
- Ted
Hi Ted,
On Wed, Jan 01, 2014 at 12:22:05PM -0500, Theodore Ts'o wrote:
> On Wed, Jan 01, 2014 at 12:28:23PM +0200, Baruch Siach wrote:
> > uClibc declares posix_fadvise() even when the architecture does not provide
> > one. The static posix_fadvise() signature is not compatible with POSIX. Rename
> > the internal implementation to fix this.
>
> If the architecture doesn't provide posix_fadvise(), does that imply
> that __NR_fadvise64_64 also doesn't exist?
>
> Or do you mean that for some reason, uClibc is not providing
> posix_fadvise on all architectures, even though the kernel supports it?
>
> That seems wierd.
The xtensa architecture has __NR_fadvise64_64 but not __NR_fadvise64. Should I
clarify this in the commit log?
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -
On Wed, Jan 01, 2014 at 07:31:46PM +0200, Baruch Siach wrote:
> > Or do you mean that for some reason, uClibc is not providing
> > posix_fadvise on all architectures, even though the kernel supports it?
> >
> > That seems wierd.
>
> The xtensa architecture has __NR_fadvise64_64 but not __NR_fadvise64. Should I
> clarify this in the commit log?
Is uClibc providing a posix_fadvise64()? Maybe the better fix would be
to try to use posix_fadvise64 if it is exists, with posix_fadvise()
and the locally defined posix_fadvise() as fallbacks.
If not, why is uClibc not providing any userspace access to
posix_fadvise, if the kernel suppots it, and it's insisting on
providing a function delcaration for it?
- Ted
Hi Ted,
On Wed, Jan 01, 2014 at 12:37:44PM -0500, Theodore Ts'o wrote:
> On Wed, Jan 01, 2014 at 07:31:46PM +0200, Baruch Siach wrote:
> > > Or do you mean that for some reason, uClibc is not providing
> > > posix_fadvise on all architectures, even though the kernel supports it?
> > >
> > > That seems wierd.
> >
> > The xtensa architecture has __NR_fadvise64_64 but not __NR_fadvise64. Should I
> > clarify this in the commit log?
>
> Is uClibc providing a posix_fadvise64()?
Yes. uClibc does provide posix_fadvise64() when __NR_fadvise64_64 is
available.
> Maybe the better fix would be to try to use posix_fadvise64 if it is exists,
> with posix_fadvise() and the locally defined posix_fadvise() as fallbacks.
OK. So you suggest to add another autoconf test for posix_fadvise64 and use
that as a fall back before trying direct syscall()?
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -
On Wed, Jan 01, 2014 at 09:08:00PM +0200, Baruch Siach wrote:
>
> Yes. uClibc does provide posix_fadvise64() when __NR_fadvise64_64 is
> available.
>
> > Maybe the better fix would be to try to use posix_fadvise64 if it is exists,
> > with posix_fadvise() and the locally defined posix_fadvise() as fallbacks.
>
> OK. So you suggest to add another autoconf test for posix_fadvise64 and use
> that as a fall back before trying direct syscall()?
Yes, I think that's the more general and hence the more correct
answer. In fact what we have right now is arguably wrong, since on a
32-bit platform where off_t is 32-bits, and which defines
posix_fadvise, we wouldn't correctly handle files that are larger than
2GB. So if posix_fadvise64 exists, we should be using in preference
to posix_fadvise(), even if both are provided by the C library.
Would you mind sending such a revised patch?
Many thanks!!
- Ted
Hi Ted,
On Wed, Jan 01, 2014 at 03:46:50PM -0500, Theodore Ts'o wrote:
> On Wed, Jan 01, 2014 at 09:08:00PM +0200, Baruch Siach wrote:
> > Yes. uClibc does provide posix_fadvise64() when __NR_fadvise64_64 is
> > available.
> >
> > > Maybe the better fix would be to try to use posix_fadvise64 if it is exists,
> > > with posix_fadvise() and the locally defined posix_fadvise() as fallbacks.
> >
> > OK. So you suggest to add another autoconf test for posix_fadvise64 and use
> > that as a fall back before trying direct syscall()?
>
> Yes, I think that's the more general and hence the more correct
> answer. In fact what we have right now is arguably wrong, since on a
> 32-bit platform where off_t is 32-bits, and which defines
> posix_fadvise, we wouldn't correctly handle files that are larger than
> 2GB. So if posix_fadvise64 exists, we should be using in preference
> to posix_fadvise(), even if both are provided by the C library.
>
> Would you mind sending such a revised patch?
Just sent. Thanks for reviewing.
Turns out that this is actually an uClibc bug. There used to be a
posix_fadvise definition for xtensa as a wrapper around the __NR_fadvise64_64
system call. This broke with uClibc commit ee84b8b400 (linux: posix_fadvise:
use new SYSCALL_ALIGN_64BIT). The patch I have sent can be used as a
workaround for now. I'll send a separate patch for uClibc later.
baruch
--
http://baruch.siach.name/blog/ ~. .~ Tk Open Systems
=}------------------------------------------------ooO--U--Ooo------------{=
- [email protected] - tel: +972.2.679.5364, http://www.tkos.co.il -