On Thu, Jan 11, 2018 at 2:53 AM, Chengguang Xu <cgxu519@xxxxxxxxxx> wrote:
>>
[...]
>>>>> + mutex_lock(&upper_sb->s_sync_lock);
>>>>
>>>> So it makes some sense to use upper_sb sync lock
>>>> to synchronize with callers of sync(2)/syncfs(2) on the upper fs,
>>>> but that could result syncfs() on overlay to take much longer if
>>>> there is a syncfs() on upper fs in progress.
>>>
>>> I have two proposals for this.
>>>
>>> A.
>>> After changing wait list to global, I think waiting without
>>> upper_sb->s_sync_lock is still safe, so we can avoid coupling
>>> with syncfs of upper filesystem and I prefer this.
>>>
>>> B.
>>> If you are worried about solution A, then we can introduce a mutex_lock
>>> inside overlayfs for serializing concurrent sync waiting operations.
>>>
>>
>> No need to introduce a new mutex you should just use sb->s_sync_lock
>> and BTW, option A is probably safe w.r.t races, because according to
>> comment above wait_sb_inodes(), s_sync_lock is meant to improve
>> performance, not to avoid races.
>
> I don’t think s_sync_lock here is only for performance, option A is safe because
> we do not manipulate sb->s_inodes_wb list, but wait_sb_inodes() will do.
>
See commit e97fedb9ef98 ("sync: serialise per-superblock sync operations")
s_sync_lock was added to solve a performance issue.
> In conclusion, we can take s_sync_lock of overlayfs instead to decouple with syncfs of
> upper filesystem and at the same time avoiding race condition for syncfs of overlayfs.
>
Yes.
Thanks,
Amir.
--
To unsubscribe from this list: send the line "unsubscribe linux-unionfs" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
