linux-fsdevel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Hugh Dickins <hughd@google.com>
To: Jakob Unterwurzacher <jakobunt@gmail.com>
Cc: linux-kernel@vger.kernel.org, linux-fsdevel@vger.kernel.org
Subject: Re: tmpfs returns incorrect data on concurrent pread() and truncate()
Date: Tue, 1 Nov 2016 16:51:30 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LSU.2.11.1611011602340.1362@eggly.anvils> (raw)
In-Reply-To: <18e9fa0f-ec31-9107-459c-ae1694503f87@gmail.com>

On Wed, 26 Oct 2016, Jakob Unterwurzacher wrote:

> tmpfs seems to be incorrectly returning 0-bytes when reading from
> a file that is concurrently being truncated.

That is an interesting observation, and you got me worried;
but in fact, it is not a tmpfs problem: if we call it a
problem at all, it's a VFS problem or a userspace problem.

You chose a ratio of 3 preads to 1 ftruncate in your program below:
let's call that the Unterwurzacher Ratio, 3 for tmpfs; YMMV, but for
me 4 worked well to show the same issue on ramfs, and 15 on ext4.

The Linux VFS does not serialize reads against writes or truncation
very strictly: it's unusual to need that serialization, and most
users prefer maximum speed to the additional locking, or intermediate
buffering, that would be required to avoid the issue you've seen.

> 
> This is causing crashes in gocryptfs, a cryptographic FUSE overlay,
> when it reads a nonce from disk that should absolutely positively
> never be all-zero.

I don't think that there will be much appetite for changing the
kernel's VFS to prevent that.  I hope that gocryptfs can provide
the serialization that it needs for itself, or otherwise handle
those zeroes without crashing.

(Though if something is free to truncate at an awkward moment,
then I imagine it can also mess things up by writing wrong data.)

> 
> I have written a reproducer in C that triggers this issue on
> both boxes I tested, Linux 4.7.2 and 3.16.7, both amd64.
> 
> It can be downloaded from here:
> 
>     https://gist.github.com/rfjakob/d01281c737db38075767f90bf03fc475
> 
> or, alternatively, I have attached it to this email at the bottom.
> 
> The reproducer:
> 1) Creates a 10MB file filled with 'x' at /dev/shm/x
> 2) Spawns a thread that truncates the file 3 bytes at a time
> 3) Spawns another thread that pread()s the file 1 byte at a time
>    starting from the top
> 4) Prints "wrong data" whenever the pread() gets something that
>    is not 'x' or an empty result.

Nice work, thank you, this helped me a lot.

Hugh

> 
> Example run:
> 
> $ gcc -Wall -lpthread truncate_read.c && ./a.out
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> wrong data: 0
> [...]
> 
> 
> Best regards,
> Jakob
> 
> 
> ---------------------------------------------------------------------
> truncate_read.c
> ------------------------------8<---------------------------------------
> 
> 
> // Compile and run:
> // gcc -Wall -lpthread truncate_read.c && ./a.out
> 
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <fcntl.h>
> #include <stdlib.h>
> #include <string.h>
> #include <unistd.h>
> #include <stdio.h>
> #include <pthread.h>
> 
> int fd;
> int l = 10*1024*1024;
> pthread_t tid[2];
> 
> void* read_thread(void *arg){
> 	int o,n;
> 	char b;
> 	for(o=l; o>0; o--) {
> 		b = 'a';
> 		n = pread(fd, &b, 1, o);
> 		if(n==0) {
> 			continue;
> 		}
> 		if(b != 'x') {
> 			printf("wrong data: %x\n", b);
> 		}
> 	}
> 	return NULL;
> }
> 
> void* truncate_thread(void *arg){
> 	// Parent = Truncater
> 	int o,n;
> 	// "3" seems to be the sweet spot to trigger the most errors.
> 	for(o=l; o>0; o-=3) {
> 		n = ftruncate(fd, o);
> 		if(n!=0) {
> 			perror("ftruncate err");
> 		}
> 	}
> 	return NULL;
> }
> 
> int main(int argc, char *argv[]) {
> 	fd = open("/dev/shm/x", O_RDWR | O_TRUNC | O_CREAT, 0666);
> 	if(fd < 0) {
> 		printf("open failed\n");
> 		exit(1);
> 	}
> 	char* x = malloc(l);
> 	memset(x, 'x', l);
> 	write(fd, x, l);
> 
> 	pthread_create(&(tid[0]), NULL, &read_thread, NULL);
> 	pthread_create(&(tid[1]), NULL, &truncate_thread, NULL);
> 
> 	void *res;
> 	pthread_join(tid[0], &res);
> 	pthread_join(tid[1], &res);
> 
> 	return 0;
> }

       reply	other threads:[~2016-11-01 23:51 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
     [not found] <18e9fa0f-ec31-9107-459c-ae1694503f87@gmail.com>
2016-11-01 23:51 ` Hugh Dickins [this message]
2016-11-02  1:01   ` tmpfs returns incorrect data on concurrent pread() and truncate() Dave Chinner
2016-11-02  1:38     ` Hugh Dickins
2016-11-02  4:01       ` Dave Chinner
2016-11-02 13:21       ` Theodore Ts'o
2016-11-03 23:45   ` Jakob Unterwurzacher

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LSU.2.11.1611011602340.1362@eggly.anvils \
    --to=hughd@google.com \
    --cc=jakobunt@gmail.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).