lkml.org 
[lkml]   [2004]   [Feb]   [1]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    Date
    From
    SubjectRe: 2.6.2-rc2 nfsd+xfs spins in i_size_read()
    Miquel van Smoorenburg wrote:
    > On Sat, 31 Jan 2004 02:38:51, Andrew Morton wrote:
    >
    >>Miquel van Smoorenburg <miquels@cistron.nl> wrote:
    >>
    >>>But the XFS problem appears to be vn_revalidate which calls i_size_write()
    >>>without holding i_sem:
    >>
    >>There's your bug.
    >
    >
    > Okay. It seems that XFS uses its own locking with xfs_ilock() etc,
    > so I am not sure if this should be fixed by using down(&inode->i_sem)
    > or by using xfs_ilock().
    >
    > Perhaps xfs_ilock() should also get the inode->i_sem semaphore
    > in the XFS_ILOCK_EXCL case ?
    >
    > Also, there are one or two more places that call i_size_write()
    > that should be looked at I guess.
    >

    Actually it gets more messy than this. i_size_write is called by
    different spots by xfs without the i_sem held. The call in
    generic_file_aio_write_nolock() is made without the lock for O_DIRECT
    writes, XFS allows multiple parallel writes for O_DIRECT
    writes.

    The vn_revalidate call is called out of linvfs_setattr,
    which is called with the i_sem held, it is also potentially called out
    of linvfs_getattr, although since the i_size is always maintained
    as it is changed, this call should not actually be updating the size.
    Possibly changing the code in vn_revalidate to do this:

    if (i_size_read(inode) < va.va_size))
    i_size_write(inode, va.va_size);

    Would be a good starting point, I suspect those calls from the nfs
    revalidate call are not really going to change the inode size. My
    guess is this will make your problem go away.

    Probably some larger code restructure is needed so that revalidate
    knows if the i_sem is held or not at this point.

    The O_DIRECT write case is the hard one. In XFS's internal view of
    the world, the inode size is maintained via the XFS_ILOCK, but we
    only hold that across metadata manipulation within the fs code,
    not across I/O such as a call to generic_file_aio_write_nolock.
    Right now the only way I see of dealing with that is to make
    writes which we know will extent the file hold the i_sem for
    the duration in the O_DIRECT case.

    Steve
    -
    To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
    the body of a message to majordomo@vger.kernel.org
    More majordomo info at http://vger.kernel.org/majordomo-info.html
    Please read the FAQ at http://www.tux.org/lkml/

    \
     
     \ /
      Last update: 2005-03-22 14:00    [W:0.026 / U:2.024 seconds]
    ©2003-2016 Jasper Spaans. hosted at Digital OceanAdvertise on this site