->pid in filesystem code

->pid in filesystem code

[Ulrich Drepper]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

I looked briefly through the filesystem code.  That's all definitely out
of my league so I don't try to make a final call or change something.
Anyway, this is what I found, the owners of that code should probably
look at it.  Filesystems not mentioned are fine.  I've ignored uses of
- ->pid in print statements; it's ok, any maybe preferable, there.


cifs:

  apparently uses current->pid to keep track of locking.  This might
  mean that the current implementation is actually getting things very
  wrong, at least from the Unix semantics.  Locking happens on process
  basis.  I count 11 uses of ->pid, all suspicious.  Using this
  filesystem with NPTL seems to be risky in the moment.

coda:

  One use in upcall.c.  Seems fishy if it is assumed that the code can
  be executed by any process.  If it is only meant to be used by the
  userlevel part of CODA then it should be fine.  Might be good to
  add a comment, though.

intermezzo:

  Wow, don't know where to start.  A gazillion uses of ->pid.  Some are
  print statements but there are others where the value is assigned to
  elements of some internal data structures.  I think I would strongly
  suggest to avoid this filesystem when using NPTL until it is clear
  that there are no issues.

lockd:

  In clntproc.c the ->pid value is used to generate some kind of token.
  Again, the thread can go away and take the PID with it while the
  process remains.  Don't know whether this is a problem here.

nfs:

  Should be ok.  Only mentioned in nfsXproc.c where the PID of the
  server is returned to the client.

umsdos:

  The pid seems to be used for some kind of locking.  Might be that
  using ->pid is correct here.  In that case it needs comments.


There rest seems to be fine.  Including ext2/3 which use the ->pid value
for coloring.

- -- 
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/TlQm2ijCOnn/RHQRAmyWAKCBC+cPr3ebdoeiqpusTZPn6+3cVwCffBLS
6hWR3C2+8NKck8FxAAlZun8=
=9UyG
-----END PGP SIGNATURE-----

Re: ->pid in filesystem code

[viro]

On Thu, Aug 28, 2003 at 12:12:38PM -0700, Ulrich Drepper wrote:
> cifs:
> 
>   apparently uses current->pid to keep track of locking.  This might
>   mean that the current implementation is actually getting things very
>   wrong, at least from the Unix semantics.  Locking happens on process
>   basis.  I count 11 uses of ->pid, all suspicious.  Using this
>   filesystem with NPTL seems to be risky in the moment.

s/with NPTL//.  I'm fairly certain that fs/cifs went into the tree without
a review and what's more, in this case I have very strong suspicion that
it might have been deliberately obfuscated to scare potential reviewers off.
Whatever the cause might be, the code *is* obfuscated enough to make it very
hard to review and it certainly contains a lot of dubious stuff.

 
> intermezzo:
> 
>   Wow, don't know where to start.  A gazillion uses of ->pid.  Some are
>   print statements but there are others where the value is assigned to
>   elements of some internal data structures.  I think I would strongly
>   suggest to avoid this filesystem when using NPTL until it is clear
>   that there are no issues.

intermezzo needs a serious rewrite before it will be usable in 2.6.  Authors
had promised to do something about it, but so far it hadn't reached the Linus'
tree.
 
> umsdos:
> 
>   The pid seems to be used for some kind of locking.  Might be that
>   using ->pid is correct here.  In that case it needs comments.

Doesn't even build.  Will need a rewrite or removal - it had been rotting
for a *long* time.

->pid in filesystem code

[Steve French]

Yes the cifs vfs code was using current->pid in multiple places and in 
about five of the places the meaning
really was process id,  and thus presumably better to change this to 
current->tgid.   I have made the change
from pid to tgid for those places in the bk tree for the cifs vfs 
(http://cifs.bkbits.net/linux-2.5cifs) and will try them out.

The locking tests needed to be rerun anyway.