Re: [PATCH 03/13] xfs: rationalise xfs_mount_wq users

Mark Tinguely
2012-09-05T14:33:13Z

On 09/05/12 08:16, Mark Tinguely wrote:
> On 09/04/12 23:30, Dave Chinner wrote:
>> On Tue, Sep 04, 2012 at 10:48:17AM -0500, Mark Tinguely wrote:
>>> On 08/30/12 07:00, Dave Chinner wrote:
>>>> - /*
>>>> - * We shouldn't write/force the log if we are in the mount/unmount
>>>> - * process or on a read only filesystem. The workqueue still needs
>>>> to be
>>>> - * active in both cases, however, because it is used for inode reclaim
>>>> - * during these times. Use the MS_ACTIVE flag to avoid doing anything
>>>> - * during mount. Doing work during unmount is avoided by calling
>>>> - * cancel_delayed_work_sync on this work queue before tearing down
>>>> - * the ail and the log in xfs_log_unmount.
>>>> - */
>>>> - if (!(mp->m_super->s_flags& MS_ACTIVE)&&
>>>> - !(mp->m_flags& XFS_MOUNT_RDONLY)) {
>>>> + if (!(mp->m_flags& XFS_MOUNT_RDONLY)) {
>>>> /* dgc: errors ignored here */
>>>> if (mp->m_super->s_writers.frozen == SB_UNFROZEN&&
>>>> xfs_log_need_covered(mp))
>>>> @@ -408,8 +398,7 @@ xfs_sync_worker(
>>>> else
>>>> xfs_log_force(mp, 0);
>>>>
>>>> - /* start pushing all the metadata that is currently
>>>> - * dirty */
>>>> + /* start pushing all the metadata that is currently dirty */
>>>> xfs_ail_push_all(mp->m_ail);
>>>> }
>>>>
>>>
>>> It appears that the removal of the MS_ACTIVE flag is causing the
>>> "atomic_read(&bp->b_hold)> 0," ASSERT.
>>
>> I must be being slow today - I don't see why that would cause any
>> problems. The worker is not started at the end of the mount process
>> after everything is set up (i.e. just before MS_ACTIVE is removed),
>> and the worker is stopped before anything is torn down. That should
>> effectively replicate what the MS_ACTIVE flag is providing in the
>> old code.
>>
>> Can you explain in more detail what lead you to this conclusion?
>>
>> Cheers,
>>
>> Dave.
>
> You are correct, it does not make sense, but with the
> !(mp->m_super->s_flags & MS_ACTIVE)
> test removed, test 107 causes the above assert on
> different machines/architectures. Place the test in, the
> assert does not happen.
>
> I will see if I can get it to dump on the x86_32 machine.
>
> --Mark.

Make that xfstest 179. The ASSERT happens right away. I have a dump from
x86_32 machine. I will take a quick look at it.

--Mark.