On Wed, 16 Apr 2014 16:17:26 +1000 NeilBrown <neilb@xxxxxxx> wrote: > On Wed, 16 Apr 2014 15:37:56 +1000 Dave Chinner <david@xxxxxxxxxxxxx> wrote: > > > On Wed, Apr 16, 2014 at 02:03:36PM +1000, NeilBrown wrote: > > > - /* > > > - * Given that we do not allow direct reclaim to call us, we should > > > - * never be called while in a filesystem transaction. > > > - */ > > > - if (WARN_ON(current->flags & PF_FSTRANS)) > > > - goto redirty; > > > > We still need to ensure this rule isn't broken. If it is, the > > filesystem will silently deadlock in delayed allocation rather than > > gracefully handle the problem with a warning.... > > Hmm... that might be tricky. The 'new' PF_FSTRANS can definitely be set when > xfs_vm_writepage is called and we really want the write to happen. > I don't suppose there is any other way to detect if a transaction is > happening? I've been thinking about this some more.... That code is in xfs_vm_writepage which is only called as ->writepage. xfs never calls that directly so it could only possibly be called during reclaim? We know that doesn't happen, but if it does then PF_MEMALLOC would be set, but PF_KSWAPD would not... and you already have a test for that. How about every time we set PF_FSTRANS, we store the corresponding xfs_trans_t in current->journal_info, and clear that field when PF_FSTRANS is cleared. Then xfs_vm_writepage can test for current->journal_info being clear. That is the field that several other filesystems use to keep track of the 'current' transaction. ?? I don't know what xfs_trans_t we would use in xfs_bmapi_allocate_worker, but I suspect you do :-) Thanks, NeilBrown
Attachment:
signature.asc
Description: PGP signature