On Thu, 14 Dec 2017 10:09:58 +0800 "Yan, Zheng" <zyan@xxxxxxxxxx> wrote: > > > > On 14 Dec 2017, at 08:59, Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote: > > > > On Wed, 13 Dec 2017 11:58:36 +0800 "Yan, Zheng" <zyan@xxxxxxxxxx> wrote: > > > >> We recently got an Oops report: > >> > >> BUG: unable to handle kernel NULL pointer dereference at (null) > >> IP: jbd2__journal_start+0x38/0x1a2 > >> [...] > >> Call Trace: > >> ext4_page_mkwrite+0x307/0x52b > >> _ext4_get_block+0xd8/0xd8 > >> do_page_mkwrite+0x6e/0xd8 > >> handle_mm_fault+0x686/0xf9b > >> mntput_no_expire+0x1f/0x21e > >> __do_page_fault+0x21d/0x465 > >> dput+0x4a/0x2f7 > >> page_fault+0x22/0x30 > >> copy_user_generic_string+0x2c/0x40 > >> copy_page_to_iter+0x8c/0x2b8 > >> generic_file_read_iter+0x26e/0x845 > >> timerqueue_del+0x31/0x90 > >> ceph_read_iter+0x697/0xa33 [ceph] > >> hrtimer_cancel+0x23/0x41 > >> futex_wait+0x1c8/0x24d > >> get_futex_key+0x32c/0x39a > >> __vfs_read+0xe0/0x130 > >> vfs_read.part.1+0x6c/0x123 > >> handle_mm_fault+0x831/0xf9b > >> __fget+0x7e/0xbf > >> SyS_read+0x4d/0xb5 > >> > >> The reason is that page fault can happen when one filesystem copies > >> data from/to userspace, the filesystem may set current->journal_info. > >> If the userspace memory is mapped to a file on another filesystem, > >> the later filesystem may also want to use current->journal_info. > >> > > > > whoops. > > > > A cc:stable will be needed here... > > > > A filesystem doesn't "copy data from/to userspace". I assume here > > we're referring to a read() where the source is a pagecache page for > > filesystem A and the destination is a MAP_SHARED page in filesystem B? > > > > But in that case I don't see why filesystem A would have a live > > ->journal_info? It's just doing a read. > > > Background: when there are multiple cephfs clients read/write a file at time same time, read/write should go directly to object store daemon, using page cache is disabled. > > ceph_read_iter() uses current->journal_info to pass context information to ceph_readpages(). ceph_readpages() needs to know if its caller has already gotten capability of using page cache (distinguish read from readahead/fadvise). If not, it tries getting the capability by itself. I checked other filesystem, btrfs probably suffers similar problem for its readpages. (verify_parent_transid() uses current->journal_info and it can be called by by btrfs_get_extent()) > Ah. Well please let's get all that into the changelog. > > Can you explain why you chose these two sites? Rather than, for > > example, way up in handle_mm_fault()? And please answer this? > > It's hard to believe that a fault handler will alter ->journal_info if > > it is handling a read fault, so perhaps we only need to do this for a > > write fault? Although such an optimization probably isn't worthwhile. > > The whole thing is only about three instructions. > > > > >