On 08/10/2018 09:02 AM, Darrick J. Wong wrote:
> On Fri, Aug 10, 2018 at 08:54:00AM -0700, Dave Jiang wrote:
>>
>>
>> On 08/10/2018 08:48 AM, Darrick J. Wong wrote:
>>> On Wed, Aug 08, 2018 at 10:31:40AM -0700, Dave Jiang wrote:
>>>> This patch is the duplicate of ross's fix for ext4 for xfs.
>>>>
>>>> If the refcount of a page is lowered between the time that it is returned
>>>> by dax_busy_page() and when the refcount is again checked in
>>>> xfs_break_layouts() => ___wait_var_event(), the waiting function
>>>> xfs_wait_dax_page() will never be called. This means that
>>>> xfs_break_layouts() will still have 'retry' set to false, so we'll stop
>>>> looping and never check the refcount of other pages in this inode.
>>>>
>>>> Instead, always continue looping as long as dax_layout_busy_page() gives us
>>>> a page which it found with an elevated refcount.
>>>>
>>>> Signed-off-by: Dave Jiang <dave.jiang@xxxxxxxxx>
>>>> Reviewed-by: Jan Kara <jack@xxxxxxx>
>>>> ---
>>>>
>>>> Sorry resend, forgot to add Jan's reviewed-by.
>>>>
>>>> v2:
>>>> - Rename parameter from did_unlock to retry (Jan)
>>>>
>>>> fs/xfs/xfs_file.c | 9 ++++-----
>>>> 1 file changed, 4 insertions(+), 5 deletions(-)
>>>>
>>>> diff --git a/fs/xfs/xfs_file.c b/fs/xfs/xfs_file.c
>>>> index a3e7767a5715..cd6f0d8c4922 100644
>>>> --- a/fs/xfs/xfs_file.c
>>>> +++ b/fs/xfs/xfs_file.c
>>>> @@ -721,12 +721,10 @@ xfs_file_write_iter(
>>>>
>>>> static void
>>>> xfs_wait_dax_page(
>>>> - struct inode *inode,
>>>> - bool *did_unlock)
>>>> + struct inode *inode)
>>>> {
>>>> struct xfs_inode *ip = XFS_I(inode);
>>>>
>>>> - *did_unlock = true;
>>>> xfs_iunlock(ip, XFS_MMAPLOCK_EXCL);
>>>> schedule();
>>>> xfs_ilock(ip, XFS_MMAPLOCK_EXCL);
>>>> @@ -736,7 +734,7 @@ static int
>>>> xfs_break_dax_layouts(
>>>> struct inode *inode,
>>>> uint iolock,
>>>> - bool *did_unlock)
>>>> + bool *retry)
>>>
>>> Uhhh, this hunk doesn't apply. xfs_break_dax_layouts doesn't have an
>>> iolock parameter anymore; was this not generated off of xfs for-next?
>>
>> Sorry. It was generated against 4.18-rc8. I'll respin patch against xfs
>> for-next.
>
> I think it's just a matter of taking the old patch and changing
> "did_unlock" to "retry", right? If so, I'll just change that and be
> done with this one. :)
For the conflict part yes. Thanks! :)
>
> --D
>
>>>
>>> --D
>>>
>>>> {
>>>> struct page *page;
>>>>
>>>> @@ -746,9 +744,10 @@ xfs_break_dax_layouts(
>>>> if (!page)
>>>> return 0;
>>>>
>>>> + *retry = true;
>>>> return ___wait_var_event(&page->_refcount,
>>>> atomic_read(&page->_refcount) == 1, TASK_INTERRUPTIBLE,
>>>> - 0, 0, xfs_wait_dax_page(inode, did_unlock));
>>>> + 0, 0, xfs_wait_dax_page(inode));
>>>> }
>>>>
>>>> int
>>>>
>>>> --
>>>> To unsubscribe from this list: send the line "unsubscribe linux-xfs" in
>>>> the body of a message to majordomo@xxxxxxxxxxxxxxx
>>>> More majordomo info at http://vger.kernel.org/majordomo-info.html
