Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1455856 > unrolled thread

Re: 4.7.0-rc7 ext4 error in dx_probe

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: 4.7.0-rc7 ext4 error in dx_probe Török Edwin <edwin@etorok.net> - 2016-08-03 17:20 +0200
    Re: 4.7.0-rc7 ext4 error in dx_probe Johannes Stezenbach <js@sig21.net> - 2016-08-05 12:40 +0200
      Re: 4.7.0-rc7 ext4 error in dx_probe "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-08-05 19:10 +0200
        Re: 4.7.0-rc7 ext4 error in dx_probe Johannes Stezenbach <js@sig21.net> - 2016-08-05 20:20 +0200
          Re: 4.7.0-rc7 ext4 error in dx_probe "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-08-05 21:20 +0200
            Re: 4.7.0-rc7 ext4 error in dx_probe Theodore Ts'o <tytso@mit.edu> - 2016-08-08 06:00 +0200
              Re: 4.7.0-rc7 ext4 error in dx_probe "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-08-08 08:30 +0200
                Re: 4.7.0-rc7 ext4 error in dx_probe Theodore Ts'o <tytso@mit.edu> - 2016-08-08 18:10 +0200
                  Re: 4.7.0-rc7 ext4 error in dx_probe "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-08-08 19:00 +0200
                    Re: 4.7.0-rc7 ext4 error in dx_probe Török Edwin <edwin@etorok.net> - 2016-08-08 23:20 +0200
                      Re: 4.7.0-rc7 ext4 error in dx_probe "Darrick J. Wong" <darrick.wong@oracle.com> - 2016-08-09 04:40 +0200
                        Re: 4.7.0-rc7 ext4 error in dx_probe Török Edwin <edwin@etorok.net> - 2016-08-09 09:20 +0200

#1455856 — Re: 4.7.0-rc7 ext4 error in dx_probe

Hi,


I have just encountered a similar problem after I've recently upgraded to 4.7.0:
[Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: inode #13295: comm python: Directory index failed checksum
[Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
[Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem read-only
[Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): ext4_journal_check_start:56: Detected aborted journal

I've rebooted in single-user mode, fsck fixed the filesystem, and rebooted, filesystem is rw again now.

inode #13295 seems to be this and I can list it now:
stat /usr/lib64/python3.4/site-packages
  File: '/usr/lib64/python3.4/site-packages'
  Size: 12288     	Blocks: 24         IO Block: 4096   directory
Device: fd01h/64769d	Inode: 13295       Links: 180
Access: (0755/drwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
Access: 2016-05-09 11:29:44.056661988 +0300
Modify: 2016-08-01 00:34:24.029779875 +0300
Change: 2016-08-01 00:34:24.029779875 +0300
 Birth: -

The filesystem was /, I only noticed it was readonly after several hours when I tried to install something:
/dev/mapper/vg--ssd-root on / type ext4 (rw,noatime,errors=remount-ro,data=ordered)

$ uname -a
Linux bolt 4.7.0-gentoo-rr #1 SMP Thu Jul 28 11:28:56 EEST 2016 x86_64 AMD FX(tm)-8350 Eight-Core Processor AuthenticAMD GNU/Linux

FWIW I've been using ext4 for years and this is the first time I see this message.
Prior to 4.7 I was on 4.6.1 -> 4.6.2 -> 4.6.3 -> 4.6.4.

The kernel is from gentoo-sources + a patch for enabling AMD LWP (I had that patch since 4.6.3 and its not related to I/O).

If I see this message again what should I do to obtain more information to trace down the root cause?

Best regards,
-- 
Edwin Török | Co-founder and Lead Developer

Skylable open-source object storage: reliable, fast, secure
http://www.skylable.com

[toc] | [next] | [standalone]

#1457008

On Wed, Aug 03, 2016 at 05:50:26PM +0300, Török Edwin wrote:
> I have just encountered a similar problem after I've recently upgraded to 4.7.0:
> [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: inode #13295: comm python: Directory index failed checksum
> [Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
> [Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem read-only
> [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): ext4_journal_check_start:56: Detected aborted journal
> 
> I've rebooted in single-user mode, fsck fixed the filesystem, and rebooted, filesystem is rw again now.
> 
> inode #13295 seems to be this and I can list it now:
> stat /usr/lib64/python3.4/site-packages
>   File: '/usr/lib64/python3.4/site-packages'
>   Size: 12288     	Blocks: 24         IO Block: 4096   directory
> Device: fd01h/64769d	Inode: 13295       Links: 180
> Access: (0755/drwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
> Access: 2016-05-09 11:29:44.056661988 +0300
> Modify: 2016-08-01 00:34:24.029779875 +0300
> Change: 2016-08-01 00:34:24.029779875 +0300
>  Birth: -
> 
> The filesystem was /, I only noticed it was readonly after several hours when I tried to install something:
> /dev/mapper/vg--ssd-root on / type ext4 (rw,noatime,errors=remount-ro,data=ordered)
> 
> $ uname -a
> Linux bolt 4.7.0-gentoo-rr #1 SMP Thu Jul 28 11:28:56 EEST 2016 x86_64 AMD FX(tm)-8350 Eight-Core Processor AuthenticAMD GNU/Linux
> 
> FWIW I've been using ext4 for years and this is the first time I see this message.
> Prior to 4.7 I was on 4.6.1 -> 4.6.2 -> 4.6.3 -> 4.6.4.
> 
> The kernel is from gentoo-sources + a patch for enabling AMD LWP (I had that patch since 4.6.3 and its not related to I/O).
> 
> If I see this message again what should I do to obtain more information to trace down the root cause?

It just happened again to me, this time hitting /usr/sbin/
on root fs.  Meanwhile I ran memtest86 7.0 for two nights,
it didn't find anything.  I'm using hibernate regularly
and I think so this only happened after a few hibernate/resume
cycles, but no idea if that means anything.
Now I'm back at 4.4.16 to see if it reproduces.

Johannes

[toc] | [prev] | [next] | [standalone]

#1457216

On Fri, Aug 05, 2016 at 12:35:44PM +0200, Johannes Stezenbach wrote:
> On Wed, Aug 03, 2016 at 05:50:26PM +0300, Török Edwin wrote:
> > I have just encountered a similar problem after I've recently upgraded to 4.7.0:
> > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: inode #13295: comm python: Directory index failed checksum
> > [Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
> > [Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem read-only
> > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): ext4_journal_check_start:56: Detected aborted journal
> > 
> > I've rebooted in single-user mode, fsck fixed the filesystem, and rebooted, filesystem is rw again now.
> > 
> > inode #13295 seems to be this and I can list it now:
> > stat /usr/lib64/python3.4/site-packages
> >   File: '/usr/lib64/python3.4/site-packages'
> >   Size: 12288     	Blocks: 24         IO Block: 4096   directory
> > Device: fd01h/64769d	Inode: 13295       Links: 180
> > Access: (0755/drwxr-xr-x)  Uid: (    0/    root)   Gid: (    0/    root)
> > Access: 2016-05-09 11:29:44.056661988 +0300
> > Modify: 2016-08-01 00:34:24.029779875 +0300
> > Change: 2016-08-01 00:34:24.029779875 +0300
> >  Birth: -
> > 
> > The filesystem was /, I only noticed it was readonly after several hours when I tried to install something:
> > /dev/mapper/vg--ssd-root on / type ext4 (rw,noatime,errors=remount-ro,data=ordered)
> > 
> > $ uname -a
> > Linux bolt 4.7.0-gentoo-rr #1 SMP Thu Jul 28 11:28:56 EEST 2016 x86_64 AMD FX(tm)-8350 Eight-Core Processor AuthenticAMD GNU/Linux
> > 
> > FWIW I've been using ext4 for years and this is the first time I see this message.
> > Prior to 4.7 I was on 4.6.1 -> 4.6.2 -> 4.6.3 -> 4.6.4.
> > 
> > The kernel is from gentoo-sources + a patch for enabling AMD LWP (I had that patch since 4.6.3 and its not related to I/O).
> > 
> > If I see this message again what should I do to obtain more information to trace down the root cause?
> 
> It just happened again to me, this time hitting /usr/sbin/
> on root fs.  Meanwhile I ran memtest86 7.0 for two nights,
> it didn't find anything.  I'm using hibernate regularly
> and I think so this only happened after a few hibernate/resume
> cycles, but no idea if that means anything.
> Now I'm back at 4.4.16 to see if it reproduces.

When you're back on 4.7, can you apply this patch[1] to see if it fixes
the problem?  I speculate that the new parallel dir lookup code enables
multiple threads to be verifying the same directory block buffer at the
same time.

--D

[1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23

> 
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1457231

On Fri, Aug 05, 2016 at 10:02:28AM -0700, Darrick J. Wong wrote:
> On Fri, Aug 05, 2016 at 12:35:44PM +0200, Johannes Stezenbach wrote:
> > On Wed, Aug 03, 2016 at 05:50:26PM +0300, Török Edwin wrote:
> > > I have just encountered a similar problem after I've recently upgraded to 4.7.0:
> > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: inode #13295: comm python: Directory index failed checksum
> > > [Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
> > > [Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem read-only
> > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): ext4_journal_check_start:56: Detected aborted journal
> > 
> > It just happened again to me, this time hitting /usr/sbin/
> > on root fs.  Meanwhile I ran memtest86 7.0 for two nights,
> > it didn't find anything.  I'm using hibernate regularly
> > and I think so this only happened after a few hibernate/resume
> > cycles, but no idea if that means anything.
> > Now I'm back at 4.4.16 to see if it reproduces.
> 
> When you're back on 4.7, can you apply this patch[1] to see if it fixes
> the problem?  I speculate that the new parallel dir lookup code enables
> multiple threads to be verifying the same directory block buffer at the
> same time.
> 
> [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23

I added the patch, rebuilt and rebooted.  It will take some time
before I'll report back since the issue is so hard to reproduce.

Thanks,
Johannes

[toc] | [prev] | [next] | [standalone]

#1457254

On Fri, Aug 05, 2016 at 08:11:36PM +0200, Johannes Stezenbach wrote:
> On Fri, Aug 05, 2016 at 10:02:28AM -0700, Darrick J. Wong wrote:
> > On Fri, Aug 05, 2016 at 12:35:44PM +0200, Johannes Stezenbach wrote:
> > > On Wed, Aug 03, 2016 at 05:50:26PM +0300, Török Edwin wrote:
> > > > I have just encountered a similar problem after I've recently upgraded to 4.7.0:
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): dx_probe:740: inode #13295: comm python: Directory index failed checksum
> > > > [Wed Aug  3 11:08:57 2016] Aborting journal on device dm-1-8.
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs (dm-1): Remounting filesystem read-only
> > > > [Wed Aug  3 11:08:57 2016] EXT4-fs error (device dm-1): ext4_journal_check_start:56: Detected aborted journal
> > > 
> > > It just happened again to me, this time hitting /usr/sbin/
> > > on root fs.  Meanwhile I ran memtest86 7.0 for two nights,
> > > it didn't find anything.  I'm using hibernate regularly
> > > and I think so this only happened after a few hibernate/resume
> > > cycles, but no idea if that means anything.
> > > Now I'm back at 4.4.16 to see if it reproduces.
> > 
> > When you're back on 4.7, can you apply this patch[1] to see if it fixes
> > the problem?  I speculate that the new parallel dir lookup code enables
> > multiple threads to be verifying the same directory block buffer at the
> > same time.
> > 
> > [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23
> 
> I added the patch, rebuilt and rebooted.  It will take some time
> before I'll report back since the issue is so hard to reproduce.

FWIW I could trigger it reliably by running a bunch of directory traversal
programs simultaneously on the same directory.  I have a script that fires
up multiple mutts pointing to the Maildirs for the high traffic Linux lists.

--D

> 
> Thanks,
> Johannes
> --
> To unsubscribe from this list: send the line "unsubscribe linux-ext4" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html

[toc] | [prev] | [next] | [standalone]

#1457521

On Fri, Aug 05, 2016 at 12:15:48PM -0700, Darrick J. Wong wrote:
> > > [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23
> > 
> > I added the patch, rebuilt and rebooted.  It will take some time
> > before I'll report back since the issue is so hard to reproduce.
> 
> FWIW I could trigger it reliably by running a bunch of directory traversal
> programs simultaneously on the same directory.  I have a script that fires
> up multiple mutts pointing to the Maildirs for the high traffic Linux lists.

Hmm, I wonder if we should request that this patch be backported to
-stable.  Darrick, what do you think?

						- Ted

[toc] | [prev] | [next] | [standalone]

#1457554

On Sun, Aug 07, 2016 at 11:56:34PM -0400, Theodore Ts'o wrote:
> On Fri, Aug 05, 2016 at 12:15:48PM -0700, Darrick J. Wong wrote:
> > > > [1] https://git.kernel.org/cgit/linux/kernel/git/torvalds/linux.git/commit/fs/ext4/inode.c?id=b47820edd1634dc1208f9212b7ecfb4230610a23
> > > 
> > > I added the patch, rebuilt and rebooted.  It will take some time
> > > before I'll report back since the issue is so hard to reproduce.
> > 
> > FWIW I could trigger it reliably by running a bunch of directory traversal
> > programs simultaneously on the same directory.  I have a script that fires
> > up multiple mutts pointing to the Maildirs for the high traffic Linux lists.
> 
> Hmm, I wonder if we should request that this patch be backported to
> -stable.  Darrick, what do you think?

Seems like an excellent idea.

I have one lingering concern -- is it a bug that two processes could be
computing the checksum of a buffer simultaneously?  I would have thought ext4
would serialize that kind of buffer_head access...

--D
> 
> 						- Ted

[toc] | [prev] | [next] | [standalone]

#1457883

On Sun, Aug 07, 2016 at 11:28:10PM -0700, Darrick J. Wong wrote:
> 
> I have one lingering concern -- is it a bug that two processes could be
> computing the checksum of a buffer simultaneously?  I would have thought ext4
> would serialize that kind of buffer_head access...

Do we know how this is happening?  We've always depended on the VFS to
provide this exclusion.  The only way we should be modifying the
buffer_head at the same time if two CPU's are trying to modify the
directory at the same time, and that should _never_ be happening, even
with the new directory parallism code, unless the file system has
given permission and intends to do its own fine-grained locking.

      		     	     	       	   - Ted

[toc] | [prev] | [next] | [standalone]

#1457916

On Mon, Aug 08, 2016 at 12:08:18PM -0400, Theodore Ts'o wrote:
> On Sun, Aug 07, 2016 at 11:28:10PM -0700, Darrick J. Wong wrote:
> > 
> > I have one lingering concern -- is it a bug that two processes could be
> > computing the checksum of a buffer simultaneously?  I would have thought ext4
> > would serialize that kind of buffer_head access...
> 
> Do we know how this is happening?  We've always depended on the VFS to
> provide this exclusion.  The only way we should be modifying the
> buffer_head at the same time if two CPU's are trying to modify the
> directory at the same time, and that should _never_ be happening, even
> with the new directory parallism code, unless the file system has
> given permission and intends to do its own fine-grained locking.

It's a combination of two things, I think.  The first is that the
checksum calculation routine (temporarily) set the checksum field to
zero during the computation, which of course is a no-no.  The patch
fixes that problem and should go in.

The second problem is that we now can have multiple lookups at the same
time, which means that there can be more than one CPU calling into
dx_probe on the same directory blocks at the same time.  There isn't any
locking on the buffer heads between readers, so we can end up with
ext4_read_dirblock racing with itself to verify the block.  It's perhaps
a little inefficient for multiple threads to be checksumming the same
block, but only turns deadly if you combine it with the first problem.

--D

> 
>       		     	     	       	   - Ted

[toc] | [prev] | [next] | [standalone]

#1458242

On 2016-08-08 19:55, Darrick J. Wong wrote:
> On Mon, Aug 08, 2016 at 12:08:18PM -0400, Theodore Ts'o wrote:
>> On Sun, Aug 07, 2016 at 11:28:10PM -0700, Darrick J. Wong wrote:
>>>
>>> I have one lingering concern -- is it a bug that two processes could be
>>> computing the checksum of a buffer simultaneously?  I would have thought ext4
>>> would serialize that kind of buffer_head access...
>>
>> Do we know how this is happening?  We've always depended on the VFS to
>> provide this exclusion.  The only way we should be modifying the
>> buffer_head at the same time if two CPU's are trying to modify the
>> directory at the same time, and that should _never_ be happening, even
>> with the new directory parallism code, unless the file system has
>> given permission and intends to do its own fine-grained locking.
> 
> It's a combination of two things, I think.  The first is that the
> checksum calculation routine (temporarily) set the checksum field to
> zero during the computation, which of course is a no-no.  The patch
> fixes that problem and should go in.

Thanks a lot for the patch.
I wrote a small testcase (see below) that triggers the problem quite soon on my box with kernel 4.7.0, and seems to have survived so far with kernel 4.7.0+patch.
When it failed it printed something like "readdir: Bad message".

The drop caches part is quite important for triggering the bug, and might explain why this bug was hard to reproduce: IIUC this race condition can happen only
if 2+ threads/processes try to access the same directory, and the directory's inode is not in the cache (i.e. was never cached, or got kicked out of the cache).


/*
 $ gcc trigger.c -o trigger -pthread
 $ ./trigger
*/

#include <stdio.h>
#include <sys/types.h>
#include <sys/stat.h>
#include <dirent.h>
#include <string.h>
#include <stdlib.h>
#include <errno.h>
#include <pthread.h>
#include <unistd.h>
#include <fcntl.h>

#define FILES 100000
#define THREADS 16
#define LOOPS 1000

static void die(const char *msg)
{
	perror(msg);
	exit(EXIT_FAILURE);
}

static void* list(void* arg)
{
	for(int i=0;i<LOOPS;i++) {
		DIR *d = opendir(".");
		if (!d) {
			die("opendir");
		}
		errno = 0;
		while(readdir(d)) {}
		if (errno) {
			die("readdir");
		}
		closedir(d);
		FILE *f = fopen("/proc/sys/vm/drop_caches", "w");
		if (f) {
			fputs("3", f);
			fclose(f);
		}
	}
	return NULL;
}

int main()
{
	pthread_t t[THREADS];

	if(mkdir("ext4test", 0755) < 0 && errno != EEXIST)
		die("mkdir");
	if(chdir("ext4test") < 0)
		die("chdir");
	for (unsigned i=0;i < FILES;i++) {
		char name[16];
		snprintf(name, sizeof(name), "%d", i); 
		int fd = open(name, O_WRONLY|O_CREAT, 0600);
		if (fd < 0)
			die("open");
		close(fd);
	}
	for (unsigned i=0;i < sizeof(t)/sizeof(t[0]); i++) {
		pthread_create(&t[i], NULL,list, NULL);
	}
	for (unsigned i=0;i < sizeof(t)/sizeof(t[0]); i++) {
		pthread_join(t[i], NULL);
	}
	return 0;
}



-- 
Edwin Török | Co-founder and Lead Developer

Skylable open-source object storage: reliable, fast, secure
http://www.skylable.com

[toc] | [prev] | [next] | [standalone]

#1458374

On Tue, Aug 09, 2016 at 12:13:01AM +0300, Török Edwin wrote:
> On 2016-08-08 19:55, Darrick J. Wong wrote:
> > On Mon, Aug 08, 2016 at 12:08:18PM -0400, Theodore Ts'o wrote:
> >> On Sun, Aug 07, 2016 at 11:28:10PM -0700, Darrick J. Wong wrote:
> >>>
> >>> I have one lingering concern -- is it a bug that two processes could be
> >>> computing the checksum of a buffer simultaneously?  I would have thought ext4
> >>> would serialize that kind of buffer_head access...
> >>
> >> Do we know how this is happening?  We've always depended on the VFS to
> >> provide this exclusion.  The only way we should be modifying the
> >> buffer_head at the same time if two CPU's are trying to modify the
> >> directory at the same time, and that should _never_ be happening, even
> >> with the new directory parallism code, unless the file system has
> >> given permission and intends to do its own fine-grained locking.
> > 
> > It's a combination of two things, I think.  The first is that the
> > checksum calculation routine (temporarily) set the checksum field to
> > zero during the computation, which of course is a no-no.  The patch
> > fixes that problem and should go in.
> 
> Thanks a lot for the patch.
> I wrote a small testcase (see below) that triggers the problem quite soon on
> my box with kernel 4.7.0, and seems to have survived so far with kernel
> 4.7.0+patch.
> When it failed it printed something like "readdir: Bad message".
> 
> The drop caches part is quite important for triggering the bug, and might
> explain why this bug was hard to reproduce: IIUC this race condition can
> happen only if 2+ threads/processes try to access the same directory, and the
> directory's inode is not in the cache (i.e. was never cached, or got kicked
> out of the cache).

Could you formulate this into an xfstest, please?  It would be very useful to
have this as a regression test.

(Or attach a Signed-off-by and I'll take care of it eventually.)

--D
> 
> 
> /*
>  $ gcc trigger.c -o trigger -pthread
>  $ ./trigger
> */
> 
> #include <stdio.h>
> #include <sys/types.h>
> #include <sys/stat.h>
> #include <dirent.h>
> #include <string.h>
> #include <stdlib.h>
> #include <errno.h>
> #include <pthread.h>
> #include <unistd.h>
> #include <fcntl.h>
> 
> #define FILES 100000
> #define THREADS 16
> #define LOOPS 1000
> 
> static void die(const char *msg)
> {
> 	perror(msg);
> 	exit(EXIT_FAILURE);
> }
> 
> static void* list(void* arg)
> {
> 	for(int i=0;i<LOOPS;i++) {
> 		DIR *d = opendir(".");
> 		if (!d) {
> 			die("opendir");
> 		}
> 		errno = 0;
> 		while(readdir(d)) {}
> 		if (errno) {
> 			die("readdir");
> 		}
> 		closedir(d);
> 		FILE *f = fopen("/proc/sys/vm/drop_caches", "w");
> 		if (f) {
> 			fputs("3", f);
> 			fclose(f);
> 		}
> 	}
> 	return NULL;
> }
> 
> int main()
> {
> 	pthread_t t[THREADS];
> 
> 	if(mkdir("ext4test", 0755) < 0 && errno != EEXIST)
> 		die("mkdir");
> 	if(chdir("ext4test") < 0)
> 		die("chdir");
> 	for (unsigned i=0;i < FILES;i++) {
> 		char name[16];
> 		snprintf(name, sizeof(name), "%d", i); 
> 		int fd = open(name, O_WRONLY|O_CREAT, 0600);
> 		if (fd < 0)
> 			die("open");
> 		close(fd);
> 	}
> 	for (unsigned i=0;i < sizeof(t)/sizeof(t[0]); i++) {
> 		pthread_create(&t[i], NULL,list, NULL);
> 	}
> 	for (unsigned i=0;i < sizeof(t)/sizeof(t[0]); i++) {
> 		pthread_join(t[i], NULL);
> 	}
> 	return 0;
> }
> 
> 
> 
> -- 
> Edwin Török | Co-founder and Lead Developer
> 
> Skylable open-source object storage: reliable, fast, secure
> http://www.skylable.com

[toc] | [prev] | [next] | [standalone]

#1458462

On 2016-08-09 05:37, Darrick J. Wong wrote:
> On Tue, Aug 09, 2016 at 12:13:01AM +0300, Török Edwin wrote:
>> On 2016-08-08 19:55, Darrick J. Wong wrote:
>>> On Mon, Aug 08, 2016 at 12:08:18PM -0400, Theodore Ts'o wrote:
>>>> On Sun, Aug 07, 2016 at 11:28:10PM -0700, Darrick J. Wong wrote:
>>>>>
>>>>> I have one lingering concern -- is it a bug that two processes could be
>>>>> computing the checksum of a buffer simultaneously?  I would have thought ext4
>>>>> would serialize that kind of buffer_head access...
>>>>
>>>> Do we know how this is happening?  We've always depended on the VFS to
>>>> provide this exclusion.  The only way we should be modifying the
>>>> buffer_head at the same time if two CPU's are trying to modify the
>>>> directory at the same time, and that should _never_ be happening, even
>>>> with the new directory parallism code, unless the file system has
>>>> given permission and intends to do its own fine-grained locking.
>>>
>>> It's a combination of two things, I think.  The first is that the
>>> checksum calculation routine (temporarily) set the checksum field to
>>> zero during the computation, which of course is a no-no.  The patch
>>> fixes that problem and should go in.
>>
>> Thanks a lot for the patch.
>> I wrote a small testcase (see below) that triggers the problem quite soon on
>> my box with kernel 4.7.0, and seems to have survived so far with kernel
>> 4.7.0+patch.
>> When it failed it printed something like "readdir: Bad message".
>>
>> The drop caches part is quite important for triggering the bug, and might
>> explain why this bug was hard to reproduce: IIUC this race condition can
>> happen only if 2+ threads/processes try to access the same directory, and the
>> directory's inode is not in the cache (i.e. was never cached, or got kicked
>> out of the cache).
> 
> Could you formulate this into an xfstest, please?  It would be very useful to
> have this as a regression test.
> 
> (Or attach a Signed-off-by and I'll take care of it eventually.)

I've attached a signed-off-by line and a copyright header (feel free to add yourself in the copyright header too):

Signed-off-by: Török Edwin <edwin@etorok.net>

>> /*
>>  $ gcc trigger.c -o trigger -pthread
>>  $ ./trigger
>> */

/*
 * Test concurrent readdir on uncached inodes
 *
 * Copyright (C) 2016 Skylable Ltd.
 *
 * This program is free software; you can redistribute it and/or
 * modify it under the terms of the GNU General Public License
 * as published by the Free Software Foundation; either version 2
 * of the License, or (at your option) any later version.
 *
 * This program is distributed in the hope that it will be useful,
 * but WITHOUT ANY WARRANTY; without even the implied warranty of
 * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
 * GNU General Public License for more details.
 *
 * You should have received a copy of the GNU General Public License
 * along with this program; if not, write to the Free Software
 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 */

>>
>> #include <stdio.h>
>> #include <sys/types.h>
>> #include <sys/stat.h>
>> #include <dirent.h>
>> #include <string.h>
>> #include <stdlib.h>
>> #include <errno.h>
>> #include <pthread.h>
>> #include <unistd.h>
>> #include <fcntl.h>
>>
>> #define FILES 100000
>> #define THREADS 16
>> #define LOOPS 1000
>>
>> static void die(const char *msg)
>> {
>> 	perror(msg);
>> 	exit(EXIT_FAILURE);
>> }
>>
>> static void* list(void* arg)
>> {
>> 	for(int i=0;i<LOOPS;i++) {
>> 		DIR *d = opendir(".");
>> 		if (!d) {
>> 			die("opendir");
>> 		}
>> 		errno = 0;
>> 		while(readdir(d)) {}
>> 		if (errno) {
>> 			die("readdir");
>> 		}
>> 		closedir(d);
>> 		FILE *f = fopen("/proc/sys/vm/drop_caches", "w");
>> 		if (f) {
>> 			fputs("3", f);
>> 			fclose(f);
>> 		}
>> 	}
>> 	return NULL;
>> }
>>
>> int main()
>> {
>> 	pthread_t t[THREADS];
>>
>> 	if(mkdir("ext4test", 0755) < 0 && errno != EEXIST)
>> 		die("mkdir");
>> 	if(chdir("ext4test") < 0)
>> 		die("chdir");
>> 	for (unsigned i=0;i < FILES;i++) {
>> 		char name[16];
>> 		snprintf(name, sizeof(name), "%d", i); 
>> 		int fd = open(name, O_WRONLY|O_CREAT, 0600);
>> 		if (fd < 0)
>> 			die("open");
>> 		close(fd);
>> 	}
>> 	for (unsigned i=0;i < sizeof(t)/sizeof(t[0]); i++) {
>> 		pthread_create(&t[i], NULL,list, NULL);
>> 	}
>> 	for (unsigned i=0;i < sizeof(t)/sizeof(t[0]); i++) {
>> 		pthread_join(t[i], NULL);
>> 	}
>> 	return 0;
>> }
>>
>>
>>
>> -- 
>> Edwin Török | Co-founder and Lead Developer
>>
>> Skylable open-source object storage: reliable, fast, secure
>> http://www.skylable.com
> 


-- 
Edwin Török | Co-founder and Lead Developer

Skylable open-source object storage: reliable, fast, secure
http://www.skylable.com

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web