* your patch "mm: Remove false WARN_ON from pagecache_isize_extended()"
@ 2014-11-03 16:41 Jan Beulich
2014-11-03 22:18 ` Dave Chinner
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2014-11-03 16:41 UTC (permalink / raw)
To: Jan Kara; +Cc: david, xfs, linux-kernel
Jan,
having run into that warning too, I looked into it a little, and now
having found that patch am pretty uncertain: Both truncate_setsize()
and pagecache_isize_extended() document that they want to be
called with i_mutex held, so removing the WARN_ON() alone seems
either incomplete or wrong. What I found to work without violating
this documented requirement is the patch below.
Jan
--- a/fs/xfs/xfs_file.c
+++ b/fs/xfs/xfs_file.c
@@ -797,7 +797,7 @@ xfs_file_fallocate(
FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE))
return -EOPNOTSUPP;
- xfs_ilock(ip, XFS_IOLOCK_EXCL);
+ xfs_rw_ilock(ip, XFS_IOLOCK_EXCL);
if (mode & FALLOC_FL_PUNCH_HOLE) {
error = xfs_free_file_space(ip, offset, len);
if (error)
@@ -877,7 +877,7 @@ xfs_file_fallocate(
}
out_unlock:
- xfs_iunlock(ip, XFS_IOLOCK_EXCL);
+ xfs_rw_iunlock(ip, XFS_IOLOCK_EXCL);
return error;
}
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: your patch "mm: Remove false WARN_ON from pagecache_isize_extended()"
2014-11-03 16:41 your patch "mm: Remove false WARN_ON from pagecache_isize_extended()" Jan Beulich
@ 2014-11-03 22:18 ` Dave Chinner
2014-11-04 7:40 ` Jan Beulich
0 siblings, 1 reply; 4+ messages in thread
From: Dave Chinner @ 2014-11-03 22:18 UTC (permalink / raw)
To: Jan Beulich; +Cc: Jan Kara, xfs, linux-kernel
On Mon, Nov 03, 2014 at 04:41:13PM +0000, Jan Beulich wrote:
> Jan,
>
> having run into that warning too, I looked into it a little, and now
> having found that patch am pretty uncertain: Both truncate_setsize()
> and pagecache_isize_extended() document that they want to be
> called with i_mutex held, so removing the WARN_ON() alone seems
> either incomplete or wrong. What I found to work without violating
> this documented requirement is the patch below.
Or, just perhaps, the comments are wrong....
Some filesystems have stronger, more robust internal serialisation
than the VFS provides with i_mutex....
> --- a/fs/xfs/xfs_file.c
> +++ b/fs/xfs/xfs_file.c
> @@ -797,7 +797,7 @@ xfs_file_fallocate(
> FALLOC_FL_COLLAPSE_RANGE | FALLOC_FL_ZERO_RANGE))
> return -EOPNOTSUPP;
>
> - xfs_ilock(ip, XFS_IOLOCK_EXCL);
> + xfs_rw_ilock(ip, XFS_IOLOCK_EXCL);
> if (mode & FALLOC_FL_PUNCH_HOLE) {
> error = xfs_free_file_space(ip, offset, len);
> if (error)
The i_mutex is completely redundant here. Not to mention there are
multiple callers of these xfs_*_file_space() functions used to
implement fallocate(), and we're not about to change the locking
model for them....
Cheers,
Dave.
--
Dave Chinner
david@fromorbit.com
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: your patch "mm: Remove false WARN_ON from pagecache_isize_extended()"
2014-11-03 22:18 ` Dave Chinner
@ 2014-11-04 7:40 ` Jan Beulich
2014-11-04 9:08 ` Jan Kara
0 siblings, 1 reply; 4+ messages in thread
From: Jan Beulich @ 2014-11-04 7:40 UTC (permalink / raw)
To: Dave Chinner; +Cc: xfs, Jan Kara, linux-kernel
>>> On 03.11.14 at 23:18, <david@fromorbit.com> wrote:
> On Mon, Nov 03, 2014 at 04:41:13PM +0000, Jan Beulich wrote:
>> having run into that warning too, I looked into it a little, and now
>> having found that patch am pretty uncertain: Both truncate_setsize()
>> and pagecache_isize_extended() document that they want to be
>> called with i_mutex held, so removing the WARN_ON() alone seems
>> either incomplete or wrong. What I found to work without violating
>> this documented requirement is the patch below.
>
> Or, just perhaps, the comments are wrong....
Right - that's what I was suggesting with the option of the patch
being incomplete (rather than just removing the WARN_ON() it
should also remove the respective comments then).
Jan
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: your patch "mm: Remove false WARN_ON from pagecache_isize_extended()"
2014-11-04 7:40 ` Jan Beulich
@ 2014-11-04 9:08 ` Jan Kara
0 siblings, 0 replies; 4+ messages in thread
From: Jan Kara @ 2014-11-04 9:08 UTC (permalink / raw)
To: Jan Beulich; +Cc: Dave Chinner, xfs, Jan Kara, linux-kernel
On Tue 04-11-14 07:40:20, Jan Beulich wrote:
> >>> On 03.11.14 at 23:18, <david@fromorbit.com> wrote:
> > On Mon, Nov 03, 2014 at 04:41:13PM +0000, Jan Beulich wrote:
> >> having run into that warning too, I looked into it a little, and now
> >> having found that patch am pretty uncertain: Both truncate_setsize()
> >> and pagecache_isize_extended() document that they want to be
> >> called with i_mutex held, so removing the WARN_ON() alone seems
> >> either incomplete or wrong. What I found to work without violating
> >> this documented requirement is the patch below.
> >
> > Or, just perhaps, the comments are wrong....
>
> Right - that's what I was suggesting with the option of the patch
> being incomplete (rather than just removing the WARN_ON() it
> should also remove the respective comments then).
Yeah, I'll update the comments to mention there are filesystem which use
serialization different from i_mutex. Thanks for catching this.
Honza
--
Jan Kara <jack@suse.cz>
SUSE Labs, CR
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2014-11-04 9:08 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-11-03 16:41 your patch "mm: Remove false WARN_ON from pagecache_isize_extended()" Jan Beulich
2014-11-03 22:18 ` Dave Chinner
2014-11-04 7:40 ` Jan Beulich
2014-11-04 9:08 ` Jan Kara
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).