Re: iunique() fails to return ino_t (after commit 866b04fccbf125cd)

!MAILaRCHIVE_VOTE_RePLACE
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]
To: Jeff Layton <jlayton@...>
Cc: Andrew Morton <akpm@...>, Christoph Hellwig <hch@...>, LKML <linux-kernel@...>
Date: Sunday, September 16, 2007 - 10:36 pm

On 9/17/07, Jeff Layton <jlayton@redhat.com> wrote:

That's a bad excuse! Such userspace programs are buggy, period.
Let's fix *them*. And, seriously, are you *really* talking of "supporting"
userspace programs whose even *sources* are no longer available ?

The standard is clear and simple -- calls from userspace programs (that
don't define LFS) to stat(2) on a file whose serial number is >2**32 must
see EOVERFLOW. This is *not* a kernel problem that needs "fixing".

Moreover, changing a kernel function such as iunique() (which was expressly
*written* to be used by filesystems to assign ino_t to inodes) to return only
values <= 2**32 to satisfy such buggy programs is just ... bad.

BTW, you might've as well changed the type of "res" in iunique() in that
patch to "unsigned int" too. What's the point of declaring it "ino_t" if we
never assign it any value in the (ino_t - unsigned int) set in the first place?



No, it doesn't ...


This is *unrelated*. It's completely immaterial how an inode managed to
get a serial number > 2**32. Whether it used iunique() running on a 64-bit
kernel, or it's own little implementation, or whatever. The *real* issue is with
the *userspace program* and not the kernel ... that's the whole point.

[ Also, you're assuming sizeof(long) == sizeof(int) for 32-bit kernels
  here, but okay, probably that's true for all supported targets ... ]



See below, for why such programs are buggy ...



iunique() was (before you changed it) expressly written as a "free"
implementation for filesystems to allow them to assign serial numbers
to inodes. Before your patch went in, it allowed such filesystems (regardless
of whether they're persistent storage based or not) to have more than 2**32
inodes. After the said commit, not anymore. And all that, to solve a "problem"
that never existed in the kernel in the first place!


  ^^^^^^^^^

Nobody's discussing inode number "uniqueness" here ... again, that's
unrelated.


It also unjustly limits the number of inodes on filesystems that use
iunique() ...


The point was that you still haven't eliminated EOVERFLOW for stat(2)
calls without LFS. And the point was also that *trying* to eliminate/avoid
the EOVERFLOW itself is a pointless activity, because there's nothing
wrong with that return.



Because anybody who:

1. stat(2)'s on a file whose serial number is > 2**32, *and*,
2. also does not define _FILE_OFFSET_BITS == 64, *and*,
3. also does not deal with / be prepared to see an EOVERFLOW return
   from stat(2),

is obviously asking for trouble. That's where the bug is ... plain and simple
reading of the standard.



No, please read the standard. There is nothing "invalid" about the inode-number-
not-fitting-into-st_ino-giving-an-EOVERFLOW reason.



It was never a problem (definitely not the kernel's) ... :-)


I'll turn this around. Please name one *real world* userspace application
for which your patch was a fix (so that I can go fix *that*, of course! :-)


Satyam
-
Previous message: [thread] [date] [author]
Next message: [thread] [date] [author]

Messages in current thread:
Re: iunique() fails to return ino_t (after commit 866b04fccb..., Satyam Sharma, (Sun Sep 16, 10:36 pm)