get_empty_filp() is not SMP safe. [f_version problem]

Manfred Spraul (masp0008@stud.uni-sb.de)
Thu, 12 Aug 1999 16:34:48 +0200


This is a multi-part message in MIME format.

------=_NextPart_000_004B_01BEE4E0.97BF3390
Content-Type: text/plain;
charset="iso-8859-1"
Content-Transfer-Encoding: 7bit

I noticed that get_empty_filp() is not smpsafe:
the line "f->f_version = ++event" can race.

Actually, it seems that f_version is completely broken:
* it's only read during read_dir.
* it's only set in get_empty_filp() and during file read/write operations,
ie. it seems that for directories, it is never updated.

I think it can be completely removed, because i_sem prevents parallel
accesses.

Additionally, I noticed several minor points:

* sys_pread()/sys_pwrite(), sys_readv()/sys_writev()
acquire the kernel lock, but they don't need it.
* [sparc64] has special versions of sys32_readv etc. in
sys_sparc32.c. They still contain the "old", i.e.
lock_kernel() synchonization. This should be changed
before 2.4
* mandantory file locks must be synchonized with pending
read/write calls.
* if f_version is removed, then sys_lseek() with default_lseek()
doesn't need the big kernel lock.

I've attached an [untested] patch which removes the superflous lock_kernel()
calls.

Btw, drivers/char/tty_io.c currently calls down(inode->i_sem) during
do_tty_write(). If you acquire i_sem in sys_write(), then you must delete
this line [should avoid one reboot :>)

--
    Manfred

------=_NextPart_000_004B_01BEE4E0.97BF3390 Content-Type: application/octet-stream; name="patch-read_write.c" Content-Transfer-Encoding: base64 Content-Disposition: attachment; filename="patch-read_write.c"

LS0tIHJlYWRfd3JpdGUuYy5vbGQJVGh1IEF1ZyAxMiAxNToyNToxOSAxOTk5CisrKyByZWFkX3dy aXRlLmMJVGh1IEF1ZyAxMiAxNjowOTozOSAxOTk5CkBAIC0xMjAsNiArMTIwLDcgQEAKIAogCXJl dCA9IC1FQkFERjsKIAlmaWxlID0gZmdldChmZCk7CisJLyogRklYTUU6IHJlYWQgJiBtYW5kYW50 b3J5IGZpbGUgbG9ja3MgbXVzdCBiZSBzeW5jaG9uaXplZCAqLwogCWlmIChmaWxlKSB7CiAJCWlm IChmaWxlLT5mX21vZGUgJiBGTU9ERV9SRUFEKSB7CiAJCQlyZXQgPSBsb2Nrc192ZXJpZnlfYXJl YShGTE9DS19WRVJJRllfUkVBRCwgZmlsZS0+Zl9kZW50cnktPmRfaW5vZGUsCkBAIC0xNDMsNiAr MTQ0LDggQEAKIAogCXJldCA9IC1FQkFERjsKIAlmaWxlID0gZmdldChmZCk7CisJLyogRklYTUU6 IHdyaXRlICYgdHJ1bmNhdGUgJiBtYW5kYW50b3J5IGZpbGUgbG9ja3MgJiBwYXJhbGxlbCB3cml0 ZQorCQlvcGVyYXRpb25zIG11c3QgYmUgc3luY2hvbml6ZWQgKi8KIAlpZiAoZmlsZSkgewogCQlp ZiAoZmlsZS0+Zl9tb2RlICYgRk1PREVfV1JJVEUpIHsKIAkJCXN0cnVjdCBpbm9kZSAqaW5vZGUg PSBmaWxlLT5mX2RlbnRyeS0+ZF9pbm9kZTsKQEAgLTIxMSw3ICsyMTQsOSBAQAogCSAqIGlvdmVj J3MgbmF0aXZlbHkKIAkgKi8KIAlpZiAoaW5vZGUtPmlfc29jaykgeworCQlsb2NrX2tlcm5lbCgp OwogCQlyZXQgPSBzb2NrX3JlYWR2X3dyaXRldih0eXBlLGlub2RlLGZpbGUsaW92LGNvdW50LHRv dF9sZW4pOworCQl1bmxvY2tfa2VybmVsKCk7CiAJCWdvdG8gb3V0OwogCX0KIApAQCAtMjYwLDgg KzI2NSw2IEBACiAJc3RydWN0IGZpbGUgKiBmaWxlOwogCXNzaXplX3QgcmV0OwogCi0JbG9ja19r ZXJuZWwoKTsKLQogCXJldCA9IC1FQkFERjsKIAlmaWxlID0gZmdldChmZCk7CiAJaWYgKCFmaWxl KQpAQCAtMjcxLDcgKzI3NCw2IEBACiAJZnB1dChmaWxlKTsKIAogYmFkX2ZpbGU6Ci0JdW5sb2Nr X2tlcm5lbCgpOwogCXJldHVybiByZXQ7CiB9CiAKQEAgLTI4MSw4ICsyODMsNiBAQAogCXN0cnVj dCBmaWxlICogZmlsZTsKIAlzc2l6ZV90IHJldDsKIAotCWxvY2tfa2VybmVsKCk7Ci0KIAlyZXQg PSAtRUJBREY7CiAJZmlsZSA9IGZnZXQoZmQpOwogCWlmICghZmlsZSkKQEAgLTI5Myw3ICsyOTMs NiBAQAogCWZwdXQoZmlsZSk7CiAKIGJhZF9maWxlOgotCXVubG9ja19rZXJuZWwoKTsKIAlyZXR1 cm4gcmV0OwogfQogCkBAIC0zMDgsMTQgKzMwNywxMyBAQAogCXN0cnVjdCBmaWxlICogZmlsZTsK IAlzc2l6ZV90ICgqcmVhZCkoc3RydWN0IGZpbGUgKiwgY2hhciAqLCBzaXplX3QsIGxvZmZfdCAq KTsKIAotCWxvY2tfa2VybmVsKCk7Ci0KIAlyZXQgPSAtRUJBREY7CiAJZmlsZSA9IGZnZXQoZmQp OwogCWlmICghZmlsZSkKIAkJZ290byBiYWRfZmlsZTsKIAlpZiAoIShmaWxlLT5mX21vZGUgJiBG TU9ERV9SRUFEKSkKIAkJZ290byBvdXQ7CisJLyogRklYTUU6IHN5bmNob25pemF0aW9uICovCiAJ cmV0ID0gbG9ja3NfdmVyaWZ5X2FyZWEoRkxPQ0tfVkVSSUZZX1JFQUQsIGZpbGUtPmZfZGVudHJ5 LT5kX2lub2RlLAogCQkJCWZpbGUsIHBvcywgY291bnQpOwogCWlmIChyZXQpCkBAIC0zMjksNyAr MzI3LDYgQEAKIG91dDoKIAlmcHV0KGZpbGUpOwogYmFkX2ZpbGU6Ci0JdW5sb2NrX2tlcm5lbCgp OwogCXJldHVybiByZXQ7CiB9CiAKQEAgLTM0MCwxNCArMzM3LDEzIEBACiAJc3RydWN0IGZpbGUg KiBmaWxlOwogCXNzaXplX3QgKCp3cml0ZSkoc3RydWN0IGZpbGUgKiwgY29uc3QgY2hhciAqLCBz aXplX3QsIGxvZmZfdCAqKTsKIAotCWxvY2tfa2VybmVsKCk7Ci0KIAlyZXQgPSAtRUJBREY7CiAJ ZmlsZSA9IGZnZXQoZmQpOwogCWlmICghZmlsZSkKIAkJZ290byBiYWRfZmlsZTsKIAlpZiAoIShm aWxlLT5mX21vZGUgJiBGTU9ERV9XUklURSkpCiAJCWdvdG8gb3V0OworCS8qIEZJWE1FOiBzeW5j aG9uaXphdGlvbiAqLwogCXJldCA9IGxvY2tzX3ZlcmlmeV9hcmVhKEZMT0NLX1ZFUklGWV9XUklU RSwgZmlsZS0+Zl9kZW50cnktPmRfaW5vZGUsCiAJCQkJZmlsZSwgcG9zLCBjb3VudCk7CiAJaWYg KHJldCkKQEAgLTM2Miw2ICszNTgsNSBAQAogb3V0OgogCWZwdXQoZmlsZSk7CiBiYWRfZmlsZToK LQl1bmxvY2tfa2VybmVsKCk7CiAJcmV0dXJuIHJldDsKIH0K

------=_NextPart_000_004B_01BEE4E0.97BF3390--

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