Re: [PATCH] VM: Fix the gfp_mask in invalidate_complete_page2

Andrew Morton wrote:
On Fri, 06 Oct 2006 18:19:27 -0400
Trond Myklebust <Trond.Myklebust@xxxxxxxxxx> wrote:


On Fri, 2006-10-06 at 18:16 -0400, Trond Myklebust wrote:

Yeah using mapping_gfp_mask(mapping) sounds like a better option.

Revised patch is attached...


Well, it wasn't attached, but I can simulate it.

invalidate_complete_page() wants to be called from inside spinlocks by
drop_pagecache(), so if we wanted to pull the same trick there we'd need to
pass a new flag into invalidate_inode_pages().
That seems abit broken (wrt performance) that drop_pagecache_sb() holds
the fairly popular inode_lock while it invalidate pages...
Nobody else seem to...


It's not 100% clear what the gfp_t _means_ in the try_to_release_page()
context. Callees will rarely want to allocate memory (true?). So it
conveys two concepts:

a) can sleep. (__GFP_WAIT). That's fairly straightforward

b) can take fs locks (__GFP_FS). This is less clear. By passing down
__GFP_FS we're telling the callee that it's OK to take i_mutex, even
lock_page(). That sounds pretty unsafe in this context, particularly
the latter, as we're already holding a page lock.

So perhaps the safer and more appropriate solution here is to pass in a
bare __GFP_WAIT.
I agree... __GFP_WAIT does seem to be a bit more straightforward...
either way is find.. as long as it cause NFS to flush its pages...

steved.

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

Relevant Pages

  • Re: [PATCH] VM: Fix the gfp_mask in invalidate_complete_page2
    ... Revised patch is attached... ... invalidate_complete_pagewants to be called from inside spinlocks by ... can take fs locks. ... That sounds pretty unsafe in this context, ...
    (Linux-Kernel)
  • Re: a few usb issues related to edge cases
    ... in a particular context where the polling is supposed to be used - in the ... The current USB code can be run fine without real locks, ... struct mtx { ...
    (freebsd-current)
  • Re: CCriticalSection - does my thread already have a lock?
    ... I have several worker threads waiting on an IOCP. ... hunts for the appropriate context, validates that the context is still ... locks the context (which could wait on another thread that is already ... IOCP M fires, thread X wakes up (but a timeslice happens, and the thread ...
    (microsoft.public.vc.mfc)
  • Re: skipping locks, mutex_owned, usb
    ... of 'infinite' recursion because mtx_ownedalways returns false. ... mtx_trylock'should' return when we actually act as if no locks ... special context of after panic. ... Skipped locking seems like it should be left silent. ...
    (freebsd-arch)
  • Re: skipping locks, mutex_owned, usb
    ... ukbd_poll(keyboard_t *kbd, int on) ... of 'infinite' recursion because mtx_ownedalways returns false. ... I skip all lock/unlock/etc operations in the post-panic context. ... mtx_trylock'should' return when we actually act as if no locks ...
    (freebsd-arch)