Marcelo Tosatti wrote:
>
> Hi,
>
> I was looking at the pagebuf code while I noted that
> set_buffer_dirty_uptodate (pagebuf/page_buf_io.c:1188) may sleep waiting
> for bdflush to clean some dirty buffers because it calls balance_dirty().
>
> set_buffer_dirty_update is called by functions which are part of the XFS's
> writepage codepath, which basically starts at linvfs_write_full_page
> (xfs/linux/xfs_iops.c).
>
> The issue is that linvfs_write_full_page() is taking a vnode lock before
> going through this codepath, meaning that it can sleep waiting for bdflush
> while holding this lock which seems to be unecessary.
>
> If this "vnode lock" suffers from contention because processes sleep while
> holding it, we can fix it by calling balance_dirty() _after_ we drop the
> vnode lock.
>
> In practice, this vnode lock is a being used concurrently by a lot of
> users?
[ just getting back to mail after a vacation ]
Indeed this is a problem; the patch I've been sending out
pushes the balance_dirty call after all xfs locks have been
dropped. If you haven't seen the patch, I can send it again.
Even with that patch, we have a different issue. All write paths
take the xfs iolock (think of it as a inode lock) before checking
the pagecache for a given page. If the page is not in the pagecache
then it needs to be allocated - see __pagebuf_do_delwri() calls
to grab_cache_page(). This allocation can sleep for bdflush().
However, bdflush can call prune_dcache(), which inturn can wait
for this inode's iolock. I don't have a fix for this problem.
In general, it appears that any filesystem (FS) code that allocates
memory needs to be very careful, since it can wait for bdflush;
the latter, in turn, can do FS related activity that needs FS locks.
Any suggestions?
ananth.
--------------------------------------------------------------------------
Rajagopal Ananthanarayanan ("ananth")
Member Technical Staff, SGI.
--------------------------------------------------------------------------
|