[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: ->journal_info sharing



[ext3-users redhat com: CC me on replies, I am not subscribed.]

Stephen C. Tweedie writes:
 > Hi,
 > 
 > On Thu, Sep 26, 2002 at 02:05:57PM +0400, Nikita Danilov wrote:
 >  
 > > It seem that in the presence of several jornalling file systems in the
 > > kernel some mechanism is needed for proper sharing of the ->journal_info
 > > pointer in the task_struct.
 > 
 > Yep.
 > 
 > > One journalled file system can indirectly call another during page fault
 > > in copy_from_user, quota file modification, or
 > > 
 > > __alloc_pages()->balance_classzone()->shrink_cache()->vm_writeback()
 > > 
 > > sequence. (Actually, I don't understand when prevents ext3 from
 > > re-entering itself in such a case.)
 > 
 > Ext3 just has to be very, very careful either to be prepared for the
 > recursion in advance, or to avoid it by controlling when objects
 > appear dirty to the VM above, or to detect the recusion and back out,
 > marking the object still-dirty so that it gets flushed again later.
 > The ext3_writepage() code, for example, exits with a noop and does a
 > SetPageDirty if it detects a recursion.
 > 
 > > struct task_struct {
 > >   ...
 > >   void *journal_info;
 > > };
 > > 
 > > with 
 > > 
 > > struct fs_activation;
 > > 
 > > struct fs_activation {
 > >   struct super_block   *super;
 > >   struct fs_activation *parent;
 > > };
 > 
 > Well, the jbd layer is in theory usable in cases where there's no
 > super_block (eg. a journaled soft raid5 layer), so a generic "void *
 > owner" might be more appropriate.  And having to allocate a new object
 > each time makes little sense: for ext3, we already have lightweight
 > per-task handles, so embedding the fs_activation in that would make
 > more sense for me, and having a pointer to the parent in the
 > activation object makes little sense if that object is already in the
 > parent.

Sorry for being vague, by "parent" I meant parent activation---previous
entrance into file system code, that called new one, rather than file
system specific object into which fs_activation is embedded.

No allocation is necessary, on the entry file system just does:

foofs_enter(struct super_block *super, foofs_handle *handle)
{
   handle->fs_activation.owner = super;
   handle->fs_activation.parent = current->journal_info;
   current->journal_info = &handle->fs_activation;
   ...
}

and on the exit

foofs_exit(foofs_handle *handle)
{
   ...
   current->journal_info = handle->fs_activation.parent;
}

->parent pointer is useful, for example, in

ext3 -> reiser4 -> ext3 

call sequence; ext3 would probably like to detect that there is another
ext3 activation already on the stack and avoid starting new handle (to
avoid deadlocks on the journal space, or something like this I guess).

reiser4 can or cannot proceed in 

reiser4 -> ext3 -> reiser4

situation depending on whether topmost activation acquired locks, and so
on.


 > 
 > So I'd just say let's do
 > 
 > struct fs_activation {
 > 	void * owner;
 > }
 > 
 > and then whatever object we currently point journal_info at can simply
 > be required to have a fs_activation object inside it as the first
 > field in the struct.
 > 
 > If we do that, then the "owner *" cookie is an adequate uniqifier, and
 > we don't need any other pointers in the fs_activation itself because a
 > simple cast is enough to get us to the parent object.
 > 
 > Cheers,
 >  Stephen

Nikita.





[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]