Re: kjournald() with DIO

[Badari Pulavarty <pbadari@us.ibm.com>]

On Thu, 2005-09-15 at 13:35 -0700, Andrew Morton wrote:
> Andrea Arcangeli <andrea@suse.de> wrote:
> >
> >  > Probably we could simulate ->releasepage(page) by using
> >  > ->invalidatepage(page, PAGE_CACHE_SIZE), but that's relying on
> >  > side-effects, or would need new semantics defined, or something.
> > 
> >  The problem is that one thing is the memory reclaim, one thing is the
> >  invalidate_inode_pages and one thing is the truncate.
> > 
> >  They need three different behaviours.
> 
> Yup.
> 
> >  But we've only two API to ask the fs to get rid of the buffers, one is
> >  destroying dirty buffers, the other is non-blocking, we might need one
> >  that is blocking and it isn't destroying dirty buffers.
> > 
> >  The "wait/gfpmask" parameter to releasepage is ignored by ext3, perhaps
> >  we should change the semantics of that bit, and have it passed as
> >  GFP_KERNEL only by the invalidate_inode_pages.
> 
> That would work.
> 
> >  > I made releasepage() nonblocking to avoid blocking processes in the memory
> >  > reclaim paths.  Given that it's best-effort, it may as well just trylock
> >  > everything it needs and give up if that doesn't work out.
> > 
> >  I agree the memory reclaim can remain nonblocking.
> > 
> >  However note that the failure here is in try_to_free_buffers because the
> >  buffer is pinned by the journal code (it's not a trylock failing).
> >  Buffer is BH_Mapped, BH_Req, BH_Uptodate (0x19) - nicely tracked by IBM.
> 
> I'll need reminding - why was the buffer unfreeable?  Because kjournald had
> a ref on it?  Whereabouts is that happening?
> 
> 

kjournald() is commiting the transaction and doing IO to buffers, since
previous DIO write kicked it back to buffered mode (due to hole
filling). 

Here is the race:


DIO Process				kjounald()
				journal_commit_transaction()
      					  ...
        			/* submited buffers for IO */
        			/* Waiting for IO to complete */
        			while (t_locked_list) {
               			  ...
              	  		  get_bh(bh);
                	          if (buffer_locked(bh)) {
                       		     spin_unlock(&journal->j_list_lock);
                        	     wait_on_buffer(bh); 
                                                              
						
invalidate_complete_page()
        ..
   ext3_releasepage()
       journal_try_to_free_buffers()
           journal_put_journal_head()
                  __journal_try_to_free_buffer()
                                 <--- freed jh

           try_to_free_buffers()
              drop_buffers()
                    if (buffer_busy(bh))
                         goto failed;
 <<--- returns EIO due to b_count