lkml.org 
[lkml]   [2007]   [Jun]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: [patch 2/2] ufd v1 - use unsequential O(1) fdmap
    On Sun, 3 Jun 2007, Ulrich Drepper wrote:

    > -----BEGIN PGP SIGNED MESSAGE-----
    > Hash: SHA1
    >
    > Davide Libenzi wrote:
    > > #define FD_UNSEQ_BASE (1U << 28)
    >
    > I agree with Ingo, no need for a second magic value. Use the same value
    > as FD_UNSEQ_ALLOC which will just mean this exact value should never be
    > used as a file descriptor.

    I explained this in my answer to Ingo...



    > If it's not too expensive, I would prefer to see fdmap_newfd to return
    > more or less random descriptor values. This way nobody can try to rely
    > on a specific sequence. Plus it might add a tiny bit of extra security.

    Random can be expensive. At the moment is FIFO. I'm missing though how
    this can be a security flaw, when the legacy one is exactly predictable.




    > While dup2() and fcntl() might seem like good candidates to introduce
    > the new functionality I think we should jump the gun and do it right.
    > There are both not the best fit, as can be seen by your description.
    > The parameter is now a hint. Additionally everybody who'd use
    > dup2/fcntl would have to issue a close syscall right after it. Finally,
    > I'm worried about accidental use of the new functionality. Not too
    > likely but it can happen. This will cause hard to debug problems.
    >
    > I think it's better to have a dedicated interface:
    >
    > int nonseqfd (int fd, int flags);
    >
    > The semantics would include returning the new descriptor, closing the
    > old descriptor (maybe this can be overwritten with a flags bit). I
    > guess I would also like to see the default of close_on_exit changed but
    > perhaps the caller can be required to set an appropriate bit in the
    > flags parameter.
    >
    > This approach is cleaner, no magic constants exported from the kernel
    > and it should be more efficient in general. It probably will also spark
    > reevaluating the choice of the interface for fdmap_newfd. I don't like
    > overloading a parameter to be used as a flag and a value at the same time.

    I can do a new syscall, no problem (I actually even slightly prefer). We
    cannot break dup2() and F_DUPFD though, so we have to handle those too.
    I was just trying to use Linus suggestion of using sus_dup2().



    > The flags parameter will also allow to specify the additional
    > functionality needed. For instance, by default descriptors allocated
    > this way *should* appear in /proc/self/fd. You mentioned web servers
    > which don't care about sequential allocation and are slowed down by the
    > current strict allocation. Those should have the descriptor appear in
    > /proc/self/fd. On the other hand, uses of the interfaces in, say,
    > glibc or valgrind should create invisible descriptor. Well, descriptors
    > visible in perhaps /proc/self/fd-private or so.

    Ohh, my last work yesterday was changing procfs/base.c to have them show
    up in there. We have quite a few flags available (31 or 63) to be
    assicated with each fd, so I guess we could use one for that.



    > BTW: those whose perfect knowledge of the English language, I guess
    > non-sequential is better than unsequential, right? This would require
    > renaming.

    Yeah :D



    > > repeat:
    > > + if (files->fd_count >= current->signal->rlim[RLIMIT_NOFILE].rlim_cur)
    > > + return -EMFILE;
    >
    > I haven't studied the entire patch completely. So, help me understand
    > this. ->fd_count is the exact count for the number of descriptors which
    > is open. This would make sense.

    Yes, that's the count of open fds.



    > But I hope everybody realizes that this is a change in the ABI. So far
    > RLIMIT_NOFILE specified the highest file descriptor number which could
    > be returned by an open call etc.
    >
    > There is a fine difference. Even if yo have just a couple of
    > descriptors open, you could not, for instance, dup2 to a descriptor
    > higher than RLIMIT_NOFILE. With this patch it seems possible, even for
    > processes not using non-sequential descriptors. This means programs
    > written on new kernels might not run on older kernels.
    >
    > I don't say that this is unacceptable. It is a change. If it can be
    > minimized this would be better but the new RLIMIT_NOFILE semantics is
    > certainly also correct according to POSIX.

    If you look a few lines below, there's also (this is inside the lagacy
    fd allocator BTW):

    if (fd >= current->signal->rlim[RLIMIT_NOFILE].rlim_cur)
    goto out;

    So the POSIX behaviour does not change. You cannot dup2() to an fd higher
    than RLIMIT_NOFILE (when using legacy fd allocation), *and* you cannot
    open more than RLIMIT_NOFILE files.
    This means that if you have RLIMIT_NOFILE==1000 and you have 999 files
    open in the nonsequential area, you can only open one file in the legacy
    area.



    - Davide

    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2007-06-03 20:57    [W:4.284 / U:0.764 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site