From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <linux-fsdevel-owner@vger.kernel.org>
Received: from mail-qk0-f171.google.com ([209.85.220.171]:34073 "EHLO
	mail-qk0-f171.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org
	with ESMTP id S1753483AbcHQVzW (ORCPT
	<rfc822;linux-fsdevel@vger.kernel.org>);
	Wed, 17 Aug 2016 17:55:22 -0400
Received: by mail-qk0-f171.google.com with SMTP id t7so896342qkh.1
        for <linux-fsdevel@vger.kernel.org>; Wed, 17 Aug 2016 14:54:42 -0700 (PDT)
Message-ID: <1471470526.3196.153.camel@redhat.com>
Subject: Re: [glibc PATCH] fcntl: put F_OFD_* constants under #ifdef
 __USE_FILE_OFFSET64
From: Jeff Layton <jlayton@redhat.com>
To: Mike Frysinger <vapier@gentoo.org>
Cc: "Michael Kerrisk (man-pages)" <mtk.manpages@gmail.com>,
	libc-alpha@sourceware.org, linux-fsdevel@vger.kernel.org,
	Carlos O'Donell <carlos@redhat.com>,
	Yuriy Kolerov <Yuriy.Kolerov@synopsys.com>
Date: Wed, 17 Aug 2016 17:48:46 -0400
In-Reply-To: <20160817213522.GG21655@vapier.lan>
References: <1471445251-2450-1-git-send-email-jlayton@redhat.com>
	 <20160817184333.GC21655@vapier.lan> <1471461304.3196.101.camel@redhat.com>
	 <b470dd43-e77d-62ff-1b5d-2535febb2eb1@gmail.com>
	 <1471464343.3196.125.camel@redhat.com> <20160817203746.GF21655@vapier.lan>
	 <1471467478.3196.143.camel@redhat.com> <20160817213522.GG21655@vapier.lan>
Content-Type: text/plain; charset="UTF-8"
Mime-Version: 1.0
Content-Transfer-Encoding: 8bit
Sender: linux-fsdevel-owner@vger.kernel.org
List-ID: <linux-fsdevel.vger.kernel.org>

On Wed, 2016-08-17 at 14:35 -0700, Mike Frysinger wrote:
> On 17 Aug 2016 16:57, Jeff Layton wrote:
> > 
> > On Wed, 2016-08-17 at 13:37 -0700, Mike Frysinger wrote:
> > > 
> > > On 17 Aug 2016 16:05, Jeff Layton wrote:
> > > > 
> > > > 
> > > > The way it works now is that when you define
> > > > _FILE_OFFSET_BITS=64 and
> > > > call fcntl(fd, F_SETLK, fl) glibc swaps in a struct flock64 for
> > > > your
> > > > struct flock, and F_SETLK64 for the F_SETLK.
> > > 
> > > does it ?  doesn't seem like it does to me.  here's glibc's
> > > fcntl.c:
> > > 	io/fcntl.c - generic stub that sets ENOSYS
> > > 	sysdeps/unix/sysv/linux/fcntl.c - just calls syscall(fcntl)
> > > 	sysdeps/unix/sysv/linux/generic/wordsize-32/fcntl.c - just
> > > calls syscall(fcntl64)
> > > 	sysdeps/unix/sysv/linux/i386/fcntl.c - same as above
> > > 	<all the other 32-bit arches include the i386 file>
> > > 
> > 
> > Ok, I was being a little cavalier with my description. This is what
> > really happens (from x86 struct flock definition):
> > 
> > struct flock
> >   {
> >     short int l_type;   /* Type of lock: F_RDLCK, F_WRLCK, or
> > F_UNLCK.  */
> >     short int l_whence; /* Where `l_start' is relative to (like
> > `lseek').  */
> > #ifndef __USE_FILE_OFFSET64
> >     __off_t l_start;    /* Offset where the lock begins.  */
> >     __off_t l_len;      /* Size of the locked area; zero means
> > until EOF.  */
> > #else
> >     __off64_t l_start;  /* Offset where the lock begins.  */
> >     __off64_t l_len;    /* Size of the locked area; zero means
> > until EOF.  */
> > #endif
> >     __pid_t l_pid;      /* Process holding the lock.  */
> >   };
> > 
> > So, l_start and l_len get redefined into larger sizes when LFS is
> > enabled. The F_GETLK/F_SETLK/F_SETLKW are also redefined to their
> > *64
> > equivalents in that case using the preprocessor.
> 
> ah i forgot about that in the glibc header.  so it's not as grimm as
> i was thinking, and explains how glibc has been providing the API so
> the user doesn't have to explicitly pick the 64-bit types.
> 
> in order to do the same for OFD, we'd need to go the LFS route (i.e.
> add fcntl64 and transparent rewrites in glibc).  or we can just run
> with your idea ... i'm warmer to it now that i see we only have to
> tell the user "enable LFS support" rather than "use the flock64
> struct".
> -mike

Sorry yeah, I should have done a better job of explaining it, but it is
(necessarily) rather complex...

I figure that most people only haven't enabled LFS support in that
situation by mistake. I really have a hard time thinking of when you'd
want to explicitly _avoid_ using LFS support and still use OFD locks.

So yeah, I think what I proposed before would probably be fine. But now
that Michael pushed the issue, it's dawned on me that we may be able to
get away with supporting it better if we turn the compatability
mechanism on its head and use F_OFD_*32 constants in the non-LFS case.

I'm building a kernel with a test patch now. I'll either post that or a
revised version of the earlier one in another day or two once I've done
a bit more exploration of that approach.

Thanks everyone for having a look at this so far. It's been helpful...
-- 
Jeff Layton <jlayton@redhat.com>