lkml.org 
[lkml]   [2016]   [Aug]   [9]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: 4.7.0-rc7 ext4 error in dx_probe
    From
    Date
    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

    \
     
     \ /
      Last update: 2016-08-09 10:01    [W:3.934 / U:0.064 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site