lkml.org 
[lkml]   [2008]   [May]   [16]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: Remove BKL from FAT/VFAT/MSDOS (v1) (was Re: Fw: Regression caused by bf726e "semaphore: fix,")


On Sat, 17 May 2008, OGAWA Hirofumi wrote:
> > static struct inode *fat_alloc_inode(struct super_block *sb)
> > {
> > struct msdos_inode_info *ei;
> > - ei = kmem_cache_alloc(fat_inode_cachep, GFP_KERNEL);
> > + ei = kmem_cache_alloc(fat_inode_cachep, GFP_NOFS);
> > if (!ei)
> > return NULL;
> > return &ei->vfs_inode;
>
> Um... do we need this? I think this path is not called from memory
> allocation path...

The issue isn't that *this* is called by memory allocation paths, but
whether this can hold the lock and then some memory allocation path calls
back to the filesystem two write something out - and deadlock.

In other words, the chain is something like this:

msdos_lookup
lock_super() ****
fat_build_inode ->
new_inode() ->
s->s_ops->alloc_inode = fat_alloc_inode ->
kmem_cache_alloc() ->
.. pageout ..
sync_inodes ->
(or any other writeout)
lock_super() ****

and now it deadlocks.

In other words, the kmem_cache_alloc() must _not_ be allowed to actually
cause a filesystem writeout, and that's what GFP_NOFS is all about.

That said, just removing the lock-kernel entirely obviously would not have
that problem either, but I feel safer at least keeping the same locking it
had, rather than removing locking entirely.

Linus


\
 
 \ /
  Last update: 2008-05-17 03:35    [W:0.054 / U:1.132 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site