aio_fsync

aio_fsync

[Matthew Wilcox]

i'm updating the Documentation/filesystem/Locking document.
i notice that sys_fsync() takes the i_sem before calling ->fsync, but
io_submit_one() does not take i_sem before calling ->aio_fsync.

is this intentional?  do we not need to protect against concurrent
writers as sys_fsync does?

-- 
"It's not Hollywood.  War is real, war is primarily not about defeat or
victory, it is about death.  I've seen thousands and thousands of dead bodies.
Do you think I want to have an academic debate on this subject?" -- Robert Fisk
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

Re: aio_fsync

[Andrew Morton]

Matthew Wilcox <willy@debian.org> wrote:
>
> i'm updating the Documentation/filesystem/Locking document.
> i notice that sys_fsync() takes the i_sem before calling ->fsync, but
> io_submit_one() does not take i_sem before calling ->aio_fsync.
> 
> is this intentional?  do we not need to protect against concurrent
> writers as sys_fsync does?

I don't think anything implements aio_fsync().

But yes, the main reason for taking i_sem there is to prevent new writes
from coming in and hopelessly livelocking the fsync.

Unless the fsync() is implemented via a radix-tree walk.  That is not
livelockable: just do a sync_page_range() across the pages from 0 to
i_size.  This can potentially blow some amounts of CPU if there are a lot
of pages in cache but only a handful are dirty.

fsync() is lame.  If we go ahead and implement this I would suggest that we
allow userspace to pass in a start/length which represents a subsection of
the file.  That's easy to do and is a ton more useful.  And all the needed
bits and pieces already exist in the O_SYNC handling in -mm: it's just a
matter of wiring it up.


--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

Re: aio_fsync

[Suparna Bhattacharya]

On Sat, Jul 26, 2003 at 04:25:46PM -0700, Andrew Morton wrote:
> Matthew Wilcox <willy@debian.org> wrote:
> >
> > i'm updating the Documentation/filesystem/Locking document.
> > i notice that sys_fsync() takes the i_sem before calling ->fsync, but
> > io_submit_one() does not take i_sem before calling ->aio_fsync.
> > 
> > is this intentional?  do we not need to protect against concurrent
> > writers as sys_fsync does?
> 
> I don't think anything implements aio_fsync().
> 
> But yes, the main reason for taking i_sem there is to prevent new writes
> from coming in and hopelessly livelocking the fsync.
> 
> Unless the fsync() is implemented via a radix-tree walk.  That is not
> livelockable: just do a sync_page_range() across the pages from 0 to
> i_size.  This can potentially blow some amounts of CPU if there are a lot
> of pages in cache but only a handful are dirty.
> 
> fsync() is lame.  If we go ahead and implement this I would suggest that we
> allow userspace to pass in a start/length which represents a subsection of
> the file.  That's easy to do and is a ton more useful.  And all the needed
> bits and pieces already exist in the O_SYNC handling in -mm: it's just a
> matter of wiring it up.

Yes, I think I'll go ahead and do this.
Perhaps add an initial pass that goes through the mapping's dirty pages
and narrows the range to cover only dirty pages. That way, we might
be able to reduce some CPU cycles in the situation you mention above.

If we are unlucky enough to have a dirty page at the beginning
of the file and another at the end of the file, as well as loads of 
non-dirty pages in the page-cache then we just live with it taking
a little longer.  Hopefully things will usually not be that bad.

Regards
Suparna

-- 
Suparna Bhattacharya (suparna@in.ibm.com)
Linux Technology Center
IBM Software Labs, India

--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/
Don't email: <a href=mailto:"aart@kvack.org">aart@kvack.org</a>

aio_fsync

[Matthew Wilcox]

Ben, do you have patches in hand to convert users to aio_fsync?
Currently we have no users in-tree (grepping 2.5.46 for aio_fsync finds
some hpux syscalls, two places you would call it if it were set and the
prototype for it).  It's not covered in Documentation/filesystems/Locking,
nor vfs.txt.

The main problem with converting seems to be nfsd.  kiocb has a struct
file *, but nfsd doesn't have one in nfsd_sync_dir().  We could change the
signature of aio_fsync() to take an inode pointer too (nobody actually
uses the dentry).  But that doesn't seem worth doing.

Alternatives?  Add a struct inode to kiocb?  Bleugh.  Make nfsd use
struct file in various places?  Bleugh.

Should we just remove the aio_fsync pointer from fops until this problem
is solved?  Right now it's just taking up space.

-- 
Revolutions do not require corporate support.
--
To unsubscribe, send a message with 'unsubscribe linux-aio' in
the body to majordomo@kvack.org.  For more info on Linux AIO,
see: http://www.kvack.org/aio/