[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Comment on glibc2.28: Re: [PATCH glibc] Add file record locking supp

From: Samuel Thibault
Subject: Re: Comment on glibc2.28: Re: [PATCH glibc] Add file record locking support
Date: Mon, 3 Dec 2018 16:25:59 +0100
User-agent: NeoMutt/20170113 (1.7.2)

Svante Signell, le lun. 03 déc. 2018 16:01:37 +0100, a ecrit:
> On Sat, 2018-12-01 at 19:30 +0100, Samuel Thibault wrote:
> > I forgot here:
> Is this really needed?

Please quote a bit more than just that, otherwise we don't have any
actual context.

But actually, are you really talking about the mail you are quoting?
(which is about the GETLK part only, not 64bit vs 32bit questions.)

What is "this" in your sentence, exactly? The lines below? Why quoting
an unrelated sentence then?

> From git-fcntl64.diff:
> Index: glibc-2.28/sysdeps/mach/hurd/fcntl.c
> ===================================================================
> --- glibc-2.28.orig/sysdeps/mach/hurd/fcntl.c
> +++ glibc-2.28/sysdeps/mach/hurd/fcntl.c
> ...
> @@ -215,3 +210,4 @@ strong_alias (__libc_fcntl, __libc_fcntl
>  libc_hidden_def (__libc_fcntl64)
>  weak_alias (__libc_fcntl64, __fcntl64)
>  libc_hidden_weak (__fcntl64)
> +weak_alias (__fcntl64, fcntl64)
> Index: glibc-2.28/sysdeps/mach/hurd/fcntl64.c
> ===================================================================
> --- /dev/null
> +++ glibc-2.28/sysdeps/mach/hurd/fcntl64.c
> @@ -0,0 +1 @@
> +/* fcntl64 is defined in fcntl.c as an alias.  */

Well, yes? Like on Linux, fcntl and fcntl64 are just made the same, and
it's the cmd parameter which decides between 32bit vs 64bit.

Or are you saying that we do not need the fcntl64 alias?  Well, we do,
since it's declared and used by upstream commit

> Additionally, sysdeps/mach/hurd/flock.c calls __file_lock() and the 
> file-record-
> locking patches need to patch lib{diskfs,netfs,trivfs} file-lock.c files too.
> It would be much simpler to use sysdeps/posix/flock.c that calls fcntl()
> directly, and scratch sysdeps/mach/hurd/flock.c as well as 
> libdiskfs/file-lock.c
> libnetfs/file-lock.c
> libtrivfs/file-lock.c
> in due time.

It's not so simple: see my mail from 5th march 2015


flock is supposed to be per opened file, thus inherited through forks,
while fcntl/lockf to be per process, thus not inherited through forks.

In my mail I mentioned that we'd use the rendezvous port to distinguish
between both, but for now we'll defer that part and just pass NULL.

In other words, the steps would be:

- just keep flock as it is for now, make use fcntl use the RPC with
  rendezvous==NULL, and still make it per-process. tdb will be happy.
- add rendezvous support in the RPC. Then we can make the
  rendezvous!=NULL mean per-process, and make the rendezvous==NULL mean
- Then we can make flock.c use it instead of file_lock. Notice however
  that we can't just call fcntl like sysdeps/posix/flock.c does, since
  flock wants per-open lock, not per-process.
- then we can scratch file-lock.c (after we are fairly convinced that
  people have migrated to a libc that supports using the new locking

> What about libtreefs/s-file.c:treefs_S_file_lock() and it's hooks?

I don't see any users of this hook, and google doesn't either, so I'd
say we will be fine with scratching it at the same time as file-lock.C

> What it the main usage application of libtreefs?

I don't know.


reply via email to

[Prev in Thread] Current Thread [Next in Thread]