Message ID | 20160314175732.GL17923@kvack.org |
---|---|
State | Superseded, archived |
Headers | show |
No response on this patch yet.... Given that it's clearly a bug, and we've got a test case that reproduces it, why isn't this being applied? -ben On Mon, Mar 14, 2016 at 01:57:32PM -0400, Benjamin LaHaise wrote: > This patch is a follow up to the email "ext4 bug: getdents uninterruptible > for 117 seconds". When doing the initial readdir on an ext4 filesystem > that grew a directory to 497MB (the filesystem image can be downloaded at > http://www.kvack.org/~bcrl/ext4/ext4-readdir.img.xz), the first getdents64() > system call blocked for 117 seconds. Begin to address this issue by making > the ext4 readdir() operation interruptible by fatal signals so that it is > possible to abort a process that is stuck on this operation. > > fs/ext4/dir.c | 3 +++ > fs/ext4/namei.c | 4 ++++ > 2 files changed, 7 insertions(+) > > diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c > index 33f5e2a..a3e32e8 100644 > --- a/fs/ext4/dir.c > +++ b/fs/ext4/dir.c > @@ -553,6 +553,9 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx) > info->curr_node = rb_first(&info->root); > > while (1) { > + if (fatal_signal_pending(current)) > + return -ERESTARTSYS; > + > /* > * Fill the rbtree if we have no more entries, > * or the inode has changed since we last read in the > diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c > index 48e4b89..8097cd1 100644 > --- a/fs/ext4/namei.c > +++ b/fs/ext4/namei.c > @@ -959,6 +959,10 @@ static int htree_dirblock_to_tree(struct file *dir_file, > bh = ext4_read_dirblock(dir, block, DIRENT); > if (IS_ERR(bh)) > return PTR_ERR(bh); > + if (fatal_signal_pending(current)) { > + brelse(bh); > + return -ERESTARTSYS; > + } > > de = (struct ext4_dir_entry_2 *) bh->b_data; > top = (struct ext4_dir_entry_2 *) ((char *) de + > -- > "Thought is the essence of where you are now."
On Fri, Apr 08, 2016 at 02:18:41PM -0400, Benjamin LaHaise wrote: > No response on this patch yet.... Given that it's clearly a bug, and we've > got a test case that reproduces it, why isn't this being applied? Sorry, I forgot to ack the your patch. I did apply it and it went to Linus during the merge window. Unfortunately, it casued a regression, and Linus reverted it. What went to Linus was changed slightly, because I was trying to fix another potential case where we could end up looping for a very long time. Unfortunately I screwed up the modification patch and the regression tests didn't catch the problem. - Ted -- 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
diff --git a/fs/ext4/dir.c b/fs/ext4/dir.c index 33f5e2a..a3e32e8 100644 --- a/fs/ext4/dir.c +++ b/fs/ext4/dir.c @@ -553,6 +553,9 @@ static int ext4_dx_readdir(struct file *file, struct dir_context *ctx) info->curr_node = rb_first(&info->root); while (1) { + if (fatal_signal_pending(current)) + return -ERESTARTSYS; + /* * Fill the rbtree if we have no more entries, * or the inode has changed since we last read in the diff --git a/fs/ext4/namei.c b/fs/ext4/namei.c index 48e4b89..8097cd1 100644 --- a/fs/ext4/namei.c +++ b/fs/ext4/namei.c @@ -959,6 +959,10 @@ static int htree_dirblock_to_tree(struct file *dir_file, bh = ext4_read_dirblock(dir, block, DIRENT); if (IS_ERR(bh)) return PTR_ERR(bh); + if (fatal_signal_pending(current)) { + brelse(bh); + return -ERESTARTSYS; + } de = (struct ext4_dir_entry_2 *) bh->b_data; top = (struct ext4_dir_entry_2 *) ((char *) de +